From 307552cde13d1dd2a1bc111d4a7e33cbf26584e3 Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Fri, 4 Sep 2026 12:04:31 +0200 Subject: [PATCH] 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) --- .../services/dialogs/node-settings-dialog.ts | 5 + .../node-settings-dialog.html | 62 ++++++++---- .../node-settings-dialog.spec.ts | 94 +++++++++++++++++++ .../node-settings-dialog.ts | 55 ++++++++++- .../task-execution-viewer.ts | 19 +++- 5 files changed, 213 insertions(+), 22 deletions(-) create mode 100644 src/app/shared/node-settings-dialog/node-settings-dialog.spec.ts diff --git a/src/app/services/dialogs/node-settings-dialog.ts b/src/app/services/dialogs/node-settings-dialog.ts index d607e1c..6751820 100644 --- a/src/app/services/dialogs/node-settings-dialog.ts +++ b/src/app/services/dialogs/node-settings-dialog.ts @@ -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[]; }; diff --git a/src/app/shared/node-settings-dialog/node-settings-dialog.html b/src/app/shared/node-settings-dialog/node-settings-dialog.html index 7bad0b6..b17157c 100644 --- a/src/app/shared/node-settings-dialog/node-settings-dialog.html +++ b/src/app/shared/node-settings-dialog/node-settings-dialog.html @@ -9,8 +9,51 @@
- @for (field of fields; track field.key) { -
+ @for (field of ungroupedFields(); track field.key) { + + } + + @for (group of fieldGroups(); track group.name) { +
+ + @if (isGroupOpen(group.name)) { +
+ @for (field of group.fields; track field.key) { + + } +
+ } +
+ } +
+ +
+ @if (isPreviewOnly()) { + + } @else { + + + } +
+
+ +} + + + +
@switch (field.type) { @case ('display') {
@@ -118,18 +161,5 @@ @if (field.tip) { {{ field.tip }} } -
- } - - -
- @if (isPreviewOnly()) { - - } @else { - - - } -
- -} + diff --git a/src/app/shared/node-settings-dialog/node-settings-dialog.spec.ts b/src/app/shared/node-settings-dialog/node-settings-dialog.spec.ts new file mode 100644 index 0000000..cef7dbe --- /dev/null +++ b/src/app/shared/node-settings-dialog/node-settings-dialog.spec.ts @@ -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; + let component: NodeSettingsDialogHostComponent; + let dialog: NodeSettingsDialogService; + + async function open(initial: Record = {}) { + 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); + }); +}); diff --git a/src/app/shared/node-settings-dialog/node-settings-dialog.ts b/src/app/shared/node-settings-dialog/node-settings-dialog.ts index 734ce50..995317f 100644 --- a/src/app/shared/node-settings-dialog/node-settings-dialog.ts +++ b/src/app/shared/node-settings-dialog/node-settings-dialog.ts @@ -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 = {}; + /** + * 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>(new Set()); 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()); 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(); + 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) { diff --git a/src/app/shared/task-execution-viewer/task-execution-viewer.ts b/src/app/shared/task-execution-viewer/task-execution-viewer.ts index 386d4ac..5ae73f1 100644 --- a/src/app/shared/task-execution-viewer/task-execution-viewer.ts +++ b/src/app/shared/task-execution-viewer/task-execution-viewer.ts @@ -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' } ];