Fold the simulation parameters into a section that starts closed
Provider and model are what anyone opening the simulation dialog came for. Shown flat beside them, the five optional parameters turned the common case into a seven-field form for a choice most runs do not make. NodeSettingField gains an optional group, and the dialog renders those fields in a collapsible section, closed until opened. A closed section that holds values says how many, so one that is doing something never looks like one that is not - and it counts a temperature of 0, which is a real setting rather than an empty field. The field markup moved into one ng-template used by both the plain list and the sections. It is about a hundred lines of switch; a second copy would have drifted. The open-state is a signal rather than a mutated Set. The component is OnPush, so a Set only re-rendered when the change arrived through a template event - true here by luck, and false the moment anything toggled a section from code. A test caught it. 577 frontend tests green; the collapsed-by-default assertions fail when the group is forced open. Initial bundle now 7.28 kB over budget, up from 4.26. The node editor is untouched: that one is still to be discussed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
7ac7d08c98
commit
307552cde1
|
|
@ -21,6 +21,11 @@ export type NodeSettingField = {
|
|||
/** Bounds for a `number` field, so the input refuses out-of-range values as you type. */
|
||||
min?: number;
|
||||
max?: number;
|
||||
/**
|
||||
* Puts the field in a collapsible section of this name, closed until opened. For settings that
|
||||
* are optional and rarely touched, so they stop competing with the ones you came here for.
|
||||
*/
|
||||
group?: string;
|
||||
options?: NodeSettingOption[];
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -9,8 +9,51 @@
|
|||
</div>
|
||||
|
||||
<div class="grid gap-4 overflow-auto pr-1">
|
||||
@for (field of fields; track field.key) {
|
||||
<div class="text-sm font-medium text-slate-700">
|
||||
@for (field of ungroupedFields(); track field.key) {
|
||||
<ng-container *ngTemplateOutlet="fieldControl; context: { $implicit: field }"></ng-container>
|
||||
}
|
||||
|
||||
@for (group of fieldGroups(); track group.name) {
|
||||
<section class="rounded-md border border-slate-200">
|
||||
<button
|
||||
type="button"
|
||||
class="flex w-full items-center gap-2 px-3 py-2 text-left text-xs font-semibold uppercase tracking-wide text-slate-600 hover:text-slate-900"
|
||||
[attr.aria-expanded]="isGroupOpen(group.name)"
|
||||
(click)="toggleGroup(group.name, $event)">
|
||||
<mat-icon [fontIcon]="isGroupOpen(group.name) ? 'expand_less' : 'expand_more'" class="!h-4 !w-4 !text-base !leading-4"></mat-icon>
|
||||
<span>{{ group.name }}</span>
|
||||
@if (!isGroupOpen(group.name) && groupSetCount(group.fields) > 0) {
|
||||
<span class="ml-auto rounded-full bg-indigo-50 px-2 text-[10px] font-bold text-indigo-700">
|
||||
{{ groupSetCount(group.fields) }} set
|
||||
</span>
|
||||
}
|
||||
</button>
|
||||
@if (isGroupOpen(group.name)) {
|
||||
<div class="grid gap-4 border-t border-slate-200 p-3">
|
||||
@for (field of group.fields; track field.key) {
|
||||
<ng-container *ngTemplateOutlet="fieldControl; context: { $implicit: field }"></ng-container>
|
||||
}
|
||||
</div>
|
||||
}
|
||||
</section>
|
||||
}
|
||||
</div>
|
||||
|
||||
<div class="mt-auto flex justify-end gap-2">
|
||||
@if (isPreviewOnly()) {
|
||||
<button type="button" mat-stroked-button (click)="cancel($event)">Close</button>
|
||||
} @else {
|
||||
<button type="button" mat-stroked-button (click)="cancel($event)">Cancel</button>
|
||||
<button type="button" mat-flat-button (click)="save($event)">Save</button>
|
||||
}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
}
|
||||
|
||||
<!-- One field control, so the plain list and the collapsible sections cannot drift apart. -->
|
||||
<ng-template #fieldControl let-field>
|
||||
<div class="text-sm font-medium text-slate-700">
|
||||
@switch (field.type) {
|
||||
@case ('display') {
|
||||
<fieldset class="mt-1 border border-slate-200 rounded-md bg-slate-50 p-3">
|
||||
|
|
@ -118,18 +161,5 @@
|
|||
@if (field.tip) {
|
||||
<small class="text-red-600 mt-1 block">{{ field.tip }}</small>
|
||||
}
|
||||
</div>
|
||||
}
|
||||
</div>
|
||||
|
||||
<div class="mt-auto flex justify-end gap-2">
|
||||
@if (isPreviewOnly()) {
|
||||
<button type="button" mat-stroked-button (click)="cancel($event)">Close</button>
|
||||
} @else {
|
||||
<button type="button" mat-stroked-button (click)="cancel($event)">Cancel</button>
|
||||
<button type="button" mat-flat-button (click)="save($event)">Save</button>
|
||||
}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
}
|
||||
</ng-template>
|
||||
|
|
|
|||
|
|
@ -0,0 +1,94 @@
|
|||
import { ComponentFixture, TestBed } from '@angular/core/testing';
|
||||
import { NodeSettingField, NodeSettingsDialogService } from '@services/dialogs/node-settings-dialog';
|
||||
import { NodeSettingsDialogHostComponent } from './node-settings-dialog';
|
||||
|
||||
const PARAMETER_GROUP = 'Model parameters';
|
||||
|
||||
function fields(): NodeSettingField[] {
|
||||
return [
|
||||
{ key: 'provider', label: 'Provider', type: 'select', options: [{ label: 'p', value: 'p' }], required: true },
|
||||
{ key: 'model', label: 'Model', type: 'select', options: [{ label: 'm', value: 'm' }], required: true },
|
||||
{ key: 'temperature', label: 'Temperature', type: 'number', min: 0, max: 2, group: PARAMETER_GROUP },
|
||||
{ key: 'seed', label: 'Seed', type: 'number', group: PARAMETER_GROUP }
|
||||
];
|
||||
}
|
||||
|
||||
describe('NodeSettingsDialogHostComponent collapsible groups', () => {
|
||||
let fixture: ComponentFixture<NodeSettingsDialogHostComponent>;
|
||||
let component: NodeSettingsDialogHostComponent;
|
||||
let dialog: NodeSettingsDialogService;
|
||||
|
||||
async function open(initial: Record<string, string | number | boolean> = {}) {
|
||||
dialog.open({ title: 'Simulation Settings', fields: fields(), initial });
|
||||
fixture.detectChanges();
|
||||
await fixture.whenStable();
|
||||
fixture.detectChanges();
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
await TestBed.configureTestingModule({ imports: [NodeSettingsDialogHostComponent] }).compileComponents();
|
||||
fixture = TestBed.createComponent(NodeSettingsDialogHostComponent);
|
||||
component = fixture.componentInstance;
|
||||
dialog = TestBed.inject(NodeSettingsDialogService);
|
||||
});
|
||||
|
||||
afterEach(() => TestBed.resetTestingModule());
|
||||
|
||||
it('shows the ungrouped fields and keeps the group closed', async () => {
|
||||
// Provider and model are what anyone opening this came for; the parameters are for the runs
|
||||
// where you already know you want them.
|
||||
await open();
|
||||
|
||||
expect(component.ungroupedFields().map((field) => field.key)).toEqual(['provider', 'model']);
|
||||
expect(component.fieldGroups().map((group) => group.name)).toEqual([PARAMETER_GROUP]);
|
||||
expect(component.isGroupOpen(PARAMETER_GROUP)).toBe(false);
|
||||
expect(fixture.nativeElement.textContent).toContain(PARAMETER_GROUP);
|
||||
// Closed means the controls are not rendered, not merely hidden.
|
||||
expect(fixture.nativeElement.querySelectorAll('input[type="number"]').length).toBe(0);
|
||||
});
|
||||
|
||||
it('opens and closes the group on demand', async () => {
|
||||
await open();
|
||||
|
||||
component.toggleGroup(PARAMETER_GROUP);
|
||||
fixture.detectChanges();
|
||||
expect(component.isGroupOpen(PARAMETER_GROUP)).toBe(true);
|
||||
expect(fixture.nativeElement.querySelectorAll('input[type="number"]').length).toBe(2);
|
||||
|
||||
component.toggleGroup(PARAMETER_GROUP);
|
||||
fixture.detectChanges();
|
||||
expect(component.isGroupOpen(PARAMETER_GROUP)).toBe(false);
|
||||
});
|
||||
|
||||
it('says how many are set while the group is closed', async () => {
|
||||
// A closed section that is doing something must not look like one that is not.
|
||||
await open({ temperature: 0 });
|
||||
|
||||
expect(component.groupSetCount(component.fieldGroups()[0].fields)).toBe(1);
|
||||
expect(fixture.nativeElement.textContent).toContain('1 set');
|
||||
});
|
||||
|
||||
it('counts a set value of zero, which is a real setting', async () => {
|
||||
await open({ temperature: 0, seed: 42 });
|
||||
|
||||
expect(component.groupSetCount(component.fieldGroups()[0].fields)).toBe(2);
|
||||
});
|
||||
|
||||
it('counts nothing when the group is untouched', async () => {
|
||||
await open();
|
||||
|
||||
expect(component.groupSetCount(component.fieldGroups()[0].fields)).toBe(0);
|
||||
expect(fixture.nativeElement.textContent).not.toContain('set');
|
||||
});
|
||||
|
||||
it('reopens a later dialog with the group closed again', async () => {
|
||||
await open();
|
||||
component.toggleGroup(PARAMETER_GROUP);
|
||||
expect(component.isGroupOpen(PARAMETER_GROUP)).toBe(true);
|
||||
|
||||
dialog.close(null);
|
||||
await open();
|
||||
|
||||
expect(component.isGroupOpen(PARAMETER_GROUP)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -1,4 +1,5 @@
|
|||
import { ChangeDetectionStrategy, Component, effect, ElementRef, inject } from '@angular/core';
|
||||
import { ChangeDetectionStrategy, Component, effect, ElementRef, inject, signal } from '@angular/core';
|
||||
import { CommonModule } from '@angular/common';
|
||||
import { FormsModule } from '@angular/forms';
|
||||
import { MatButtonModule } from '@angular/material/button';
|
||||
import { MatCheckboxModule } from '@angular/material/checkbox';
|
||||
|
|
@ -16,7 +17,7 @@ import {
|
|||
@Component({
|
||||
selector: 'app-node-settings-dialog-host',
|
||||
standalone: true,
|
||||
imports: [FormsModule, MatButtonModule, MatCheckboxModule, MatFormFieldModule, MatIconModule, MatInputModule, MatSelectModule, MatTooltipModule],
|
||||
imports: [CommonModule, FormsModule, MatButtonModule, MatCheckboxModule, MatFormFieldModule, MatIconModule, MatInputModule, MatSelectModule, MatTooltipModule],
|
||||
templateUrl: './node-settings-dialog.html',
|
||||
changeDetection: ChangeDetectionStrategy.OnPush
|
||||
})
|
||||
|
|
@ -29,6 +30,14 @@ export class NodeSettingsDialogHostComponent {
|
|||
draft: NodeSettingsValues = {};
|
||||
fields: NodeSettingField[] = [];
|
||||
passwordVisibility: Record<string, boolean> = {};
|
||||
/**
|
||||
* Which collapsible sections the user has opened. Every one starts closed.
|
||||
*
|
||||
* A signal, not a plain Set: this component is OnPush, so a mutated Set only re-renders when the
|
||||
* change happens to arrive through a template event. That held here by luck and would stop
|
||||
* holding the moment anything toggled a section from code.
|
||||
*/
|
||||
private readonly openGroups = signal<ReadonlySet<string>>(new Set<string>());
|
||||
|
||||
constructor() {
|
||||
effect(() => {
|
||||
|
|
@ -38,6 +47,7 @@ export class NodeSettingsDialogHostComponent {
|
|||
this.fields = state.fields;
|
||||
this.draft = this.buildDraft(state.fields, state.initial);
|
||||
this.passwordVisibility = {};
|
||||
this.openGroups.set(new Set<string>());
|
||||
queueMicrotask(() => {
|
||||
const target = this.host.nativeElement.querySelector('[data-autofocus="true"]') as HTMLElement | null;
|
||||
target?.focus();
|
||||
|
|
@ -92,6 +102,47 @@ export class NodeSettingsDialogHostComponent {
|
|||
};
|
||||
}
|
||||
|
||||
/** The fields shown directly, in their given order. */
|
||||
ungroupedFields(): NodeSettingField[] {
|
||||
return this.fields.filter((field) => !field.group);
|
||||
}
|
||||
|
||||
/** The collapsible sections, in the order their first field appears. */
|
||||
fieldGroups(): Array<{ name: string; fields: NodeSettingField[] }> {
|
||||
const groups = new Map<string, NodeSettingField[]>();
|
||||
for (const field of this.fields) {
|
||||
if (!field.group) continue;
|
||||
if (!groups.has(field.group)) groups.set(field.group, []);
|
||||
groups.get(field.group)!.push(field);
|
||||
}
|
||||
return [...groups.entries()].map(([name, fields]) => ({ name, fields }));
|
||||
}
|
||||
|
||||
isGroupOpen(name: string): boolean {
|
||||
return this.openGroups().has(name);
|
||||
}
|
||||
|
||||
toggleGroup(name: string, event?: Event) {
|
||||
event?.preventDefault();
|
||||
event?.stopPropagation();
|
||||
this.openGroups.update((current) => {
|
||||
const next = new Set(current);
|
||||
if (!next.delete(name)) next.add(name);
|
||||
return next;
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* How many fields in a closed section carry a value, so a section that is doing something never
|
||||
* looks the same as one that is not.
|
||||
*/
|
||||
groupSetCount(fields: NodeSettingField[]): number {
|
||||
return fields.filter((field) => {
|
||||
const value = this.draft[field.key];
|
||||
return value !== undefined && value !== null && value !== '';
|
||||
}).length;
|
||||
}
|
||||
|
||||
private buildDraft(fields: NodeSettingField[], initial: NodeSettingsValues): NodeSettingsValues {
|
||||
const values: NodeSettingsValues = {};
|
||||
for (const field of fields) {
|
||||
|
|
|
|||
|
|
@ -129,14 +129,25 @@ export class TaskExecutionViewerComponent implements OnDestroy {
|
|||
private route = inject(ActivatedRoute);
|
||||
private lastExecutionId: string | null = null;
|
||||
private lastExecutionStatus: string | null = null;
|
||||
/** The optional sampling knobs offered alongside provider and model. */
|
||||
private static readonly SIMULATOR_PARAMETER_GROUP = 'Model parameters';
|
||||
|
||||
/**
|
||||
* The optional sampling knobs, behind a section that starts closed. Provider and model are what
|
||||
* anyone opening this dialog came for; these are for the runs where you already know you want
|
||||
* them, and shown flat they made the common case look like a five-field form.
|
||||
*/
|
||||
private static readonly SIMULATOR_PARAMETER_FIELDS: NodeSettingField[] = [
|
||||
{ key: 'temperature', label: 'Temperature', type: 'number', min: 0, max: 2,
|
||||
group: TaskExecutionViewerComponent.SIMULATOR_PARAMETER_GROUP,
|
||||
placeholder: 'Leave empty for the default', tip: '0 makes the run as repeatable as the model allows' },
|
||||
{ key: 'topP', label: 'Top P', type: 'number', min: 0, max: 1, placeholder: 'Leave empty for the default' },
|
||||
{ key: 'topK', label: 'Top K', type: 'number', min: 1, placeholder: 'Leave empty for the default' },
|
||||
{ key: 'maxTokens', label: 'Max tokens', type: 'number', min: 1, placeholder: 'Leave empty for the default' },
|
||||
{ key: 'topP', label: 'Top P', type: 'number', min: 0, max: 1,
|
||||
group: TaskExecutionViewerComponent.SIMULATOR_PARAMETER_GROUP, placeholder: 'Leave empty for the default' },
|
||||
{ key: 'topK', label: 'Top K', type: 'number', min: 1,
|
||||
group: TaskExecutionViewerComponent.SIMULATOR_PARAMETER_GROUP, placeholder: 'Leave empty for the default' },
|
||||
{ key: 'maxTokens', label: 'Max tokens', type: 'number', min: 1,
|
||||
group: TaskExecutionViewerComponent.SIMULATOR_PARAMETER_GROUP, placeholder: 'Leave empty for the default' },
|
||||
{ key: 'seed', label: 'Seed', type: 'number',
|
||||
group: TaskExecutionViewerComponent.SIMULATOR_PARAMETER_GROUP,
|
||||
placeholder: 'Leave empty for the default', tip: 'Fixes the randomness, so two runs can be compared' }
|
||||
];
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue