From 931b169ddc97e582bf4053e52f1ef0ec54e9a537 Mon Sep 17 00:00:00 2001 From: Don Kackman Date: Sun, 13 Sep 2026 17:15:09 -0500 Subject: [PATCH 1/3] feat(ui): for_each entries drawn inset in the flow view A list-driven step showed as one box on the job page's flow view while its members ran one by one in the Progress list. The group box now grows and renders one inset chip per entry, beneath the header the ordinary box's three lines occupy: chips named as the workflow wrote them (an entry's name, else its index - _entry_keys' rule) with the engine's group@entry name on hover, a chip greening as its step_end arrives and the running one pulsing amber. The group box keeps its existing all-members-done rule, and the arrows and fan-in labels still attach to it as a whole; column layout is height-aware so a tall box no longer overlaps the node beneath it. The member list is read from the definition - a literal for_each list, or a variable: reference resolved against variables, which the realized workflow carries with the run's actual list folded in (dw/realize.py) - so historical jobs show their members too, with no server change. A list that cannot be read statically shows a muted for_each stand-in. Between one entry finishing and the next starting, activeMember now reports nothing: the run is on neither, and a chip still amber there would lie about progress. Co-Authored-By: Claude --- ui/src/lib/editor/FlowView.svelte | 143 ++++++++++++++++++++++++++--- ui/src/lib/editor/FlowView.test.ts | 95 ++++++++++++++++++- ui/src/lib/flow.test.ts | 62 +++++++++++++ ui/src/lib/flow.ts | 41 +++++++++ ui/src/lib/pages/JobPage.svelte | 15 ++- ui/src/lib/pages/JobPage.test.ts | 41 +++++++++ ui/src/lib/runstate.test.ts | 62 ++++++++++++- ui/src/lib/runstate.ts | 30 ++++++ 8 files changed, 474 insertions(+), 15 deletions(-) diff --git a/ui/src/lib/editor/FlowView.svelte b/ui/src/lib/editor/FlowView.svelte index 0c7d18cb..2a87288c 100644 --- a/ui/src/lib/editor/FlowView.svelte +++ b/ui/src/lib/editor/FlowView.svelte @@ -6,6 +6,8 @@ onselect = undefined, activeStep = undefined, doneSteps = [], + activeMember = undefined, + doneMembers = [], }: { workflow: Record onselect?: (stepName: string) => void @@ -14,12 +16,24 @@ activeStep?: string /** Steps that run has already finished. */ doneSteps?: string[] + /** The for_each member (`group@entry`) the run is on right now - a + * finer grain than activeStep, which names the group. */ + activeMember?: string + /** The for_each members that run has already finished, engine names. */ + doneMembers?: string[] } = $props() const showsRun = $derived(activeStep !== undefined || doneSteps.length > 0) const stateOf = $derived((name: string) => name === activeStep ? 'active' : doneSteps.includes(name) ? 'done' : '', ) + const memberStateOf = $derived((full: string) => + full === activeMember + ? 'active' + : doneMembers.includes(full) + ? 'done' + : '', + ) const graph = $derived(dataFlowGraph(workflow)) @@ -66,6 +80,24 @@ const COL_W = 232 const ROW_H = 92 const PAD = 28 + // Member chips: a list-driven step's box grows to hold one inset chip + // per entry, beneath the header the ordinary box's three lines occupy + const CHIP_H = 15 + const CHIP_STEP = 19 + const CHIP_CHARS = 26 + const CHIP_TOP = BOX_H - 2 + // The gap ROW_H left between fixed-height boxes, kept for the + // height-aware stacking below + const ROW_GAP = ROW_H - BOX_H + + /** The box's height: the standard header, plus a chip row per entry for + * a list-driven step - or one empty slot when the step carries + * for_each but its list cannot be read from the definition. */ + function heightOf(node: FlowNode): number { + const count = node.members?.length ?? 0 + if (count) return CHIP_TOP + (count - 1) * CHIP_STEP + CHIP_H + 6 + return node.forEach ? BOX_H + 26 : BOX_H + } const layout = $derived.by(() => { const { nodes, edges } = graph @@ -97,25 +129,37 @@ columns[l] = [...(columns[l] ?? []), n] } + // Boxes in a column stack by their own height now - a for_each box + // holding many chips must not overlap the node beneath it const positions: Record = {} + const heightOfName: Record = {} + const bottoms: number[] = [] columns.forEach((col, c) => { - col.forEach((n, r) => { - positions[n.name] = { x: PAD + c * COL_W, y: PAD + r * ROW_H } + let y = PAD + col.forEach((n) => { + positions[n.name] = { x: PAD + c * COL_W, y } + heightOfName[n.name] = heightOf(n) + y += heightOf(n) + ROW_GAP }) + bottoms.push(y - ROW_GAP) }) - const maxRows = Math.max(1, ...columns.map((c) => c.length)) const width = PAD * 2 + BOX_W + Math.max(0, columns.length - 1) * COL_W - const height = PAD * 2 + BOX_H + (maxRows - 1) * ROW_H + const height = Math.max(...bottoms, PAD * 2 + BOX_H) + // One clip box per distinct member-box height; the standard box keeps + // the shared clipPath below + const tallHeights = [ + ...new Set(nodes.map((n) => heightOf(n)).filter((h) => h > BOX_H)), + ] const edgeLines = edges.map((e) => { const from = positions[e.from] const to = positions[e.to] if (!from || !to) return null const x1 = from.x + BOX_W - const y1 = from.y + BOX_H / 2 + const y1 = from.y + (heightOfName[e.from] ?? BOX_H) / 2 const x2 = to.x - const y2 = to.y + BOX_H / 2 + const y2 = to.y + (heightOfName[e.to] ?? BOX_H) / 2 // A gentle horizontal-first curve keeps lines readable when an // edge skips columns or two edges share a target row. const dx = Math.max(40, (x2 - x1) / 2) @@ -123,7 +167,7 @@ return { ...e, path, labelX: (x1 + x2) / 2, labelY: (y1 + y2) / 2 } }) - return { positions, width, height, edgeLines } + return { positions, width, height, tallHeights, edgeLines } }) function kindLabel(kind: string): string { @@ -166,7 +210,8 @@ previous_result references labeled with the argument they feed. A step with more than one incoming arrow multiplies its inputs together (CLAUDE.md's cartesian-product gotcha) - its border is - highlighted and the multiplier is noted.{#if showsRun} + highlighted and the multiplier is noted. A step carrying + for_each shows the entries it runs inset.{#if showsRun} A finished step is outlined in green, the one running now in amber colour.{/if}{#if onselect} Click a step to jump to it in the form view.{/if} @@ -194,6 +239,11 @@ + {#each layout.tallHeights as h (h)} + + + + {/each} {#each layout.edgeLines ?? [] as edge, i (i)} @@ -209,6 +259,7 @@ {@const pos = layout.positions[node.name]} {#if pos} {@const fanIn = graph.fanIn.get(node.name)} + {@const boxH = heightOf(node)} BOX_H + ? `url(#flow-nodebox-${boxH})` + : 'url(#flow-nodebox)'} + aria-label={`step ${node.name}, ${kindLabel(node.kind)}${stateOf(node.name) ? ', ' + stateOf(node.name) : ''}${node.isEntryPoint ? ', entry point' : ''}${fanIn ? ', fan-in: ' + fanIn.label : ''}${node.forEach ? (node.members?.length ? `, for_each with ${node.members.length} entries` : ', for_each') : ''}`} {...nodeAttributes(node.name)} > {#if overflowTitle(node)} {overflowTitle(node)} {/if} - + {fit( node.name, @@ -243,11 +296,45 @@ >entry {/if} + {#if node.members?.length} + + {#each node.members as key, i (i)} + {@const full = `${node.name}@${key}`} + {@const state = memberStateOf(full)} + + {full} + + {fit(key, CHIP_CHARS, 'head')} + + {/each} + {:else if node.forEach} + for_each + {/if} {#if fanIn} × {fanIn.label} @@ -354,4 +441,36 @@ fill: var(--warn); font-weight: 600; } + /* The entries a for_each step runs, inset beneath its header. Machine + state on their edges, as on the nodes: done green, running the + safelight amber. */ + .member .chip { + fill: var(--panel-2); + stroke: var(--line); + stroke-width: 1; + } + .member.done .chip { + stroke: var(--good); + stroke-width: 1.5; + } + .member.active .chip { + stroke: var(--live); + stroke-width: 1.5; + animation: dw-pulse 1.6s ease-in-out infinite; + } + @media (prefers-reduced-motion: reduce) { + .member.active .chip { + animation: none; + } + } + .chiplabel { + font-size: 9px; + fill: var(--ink); + font-family: var(--font-mono); + } + .membersunknown { + font-size: 9px; + fill: var(--muted); + font-family: var(--font-mono); + } diff --git a/ui/src/lib/editor/FlowView.test.ts b/ui/src/lib/editor/FlowView.test.ts index 7a5dc445..6ad0efad 100644 --- a/ui/src/lib/editor/FlowView.test.ts +++ b/ui/src/lib/editor/FlowView.test.ts @@ -1,5 +1,5 @@ import { render } from '@testing-library/svelte' -import { expect, it } from 'vitest' +import { describe, expect, it } from 'vitest' import FlowView from './FlowView.svelte' const workflow = { @@ -102,3 +102,96 @@ it('shortens a long step name from the end, leaving room for the entry tag', () 'reference_to_video_audio_with_lipsync', ) }) + +describe('for_each members', () => { + const listWorkflow = { + variables: { shots: [{ name: 'open' }, { name: 'reveal' }] }, + steps: [ + { + name: 'shot', + for_each: 'variable:shots', + pipeline: { configuration: { component_type: 'Fake' } }, + }, + { name: 'episode', task: { command: 'mux' } }, + ], + } + + function memberFor(container: HTMLElement, full: string) { + return [...container.querySelectorAll('g.member')].find((m) => + m.getAttribute('aria-label')?.startsWith(`member ${full},`), + )! + } + + it("renders a list step's entries as inset chips in run order, and plain steps without any", () => { + const { container } = render(FlowView, { workflow: listWorkflow }) + const shot = nodeFor(container, 'shot') + expect(shot.getAttribute('aria-label')).toContain('for_each with 2 entries') + const chips = [...shot.querySelectorAll('g.member')] + expect(chips.map((c) => c.querySelector('text')?.textContent)).toEqual([ + 'open', + 'reveal', + ]) + // The whole engine name is on the chip, as hover text + expect( + chips[0].querySelector('title')?.textContent, + ).toBe('shot@open') + expect(nodeFor(container, 'episode').querySelectorAll('g.member')).toHaveLength(0) + expect( + nodeFor(container, 'episode').getAttribute('aria-label'), + ).not.toContain('for_each') + }) + + it('colors the chips a run reports done and running', () => { + const { container } = render(FlowView, { + workflow: listWorkflow, + doneMembers: ['shot@open'], + activeMember: 'shot@reveal', + }) + expect(memberFor(container, 'shot@open').classList.contains('done')).toBe( + true, + ) + expect( + memberFor(container, 'shot@reveal').classList.contains('active'), + ).toBe(true) + // and announces the state, as the group node does + expect(memberFor(container, 'shot@reveal').getAttribute('aria-label')).toBe( + 'member shot@reveal, active', + ) + }) + + it('leaves chips unstyled when no run state is supplied', () => { + const { container } = render(FlowView, { workflow: listWorkflow }) + for (const chip of container.querySelectorAll('g.member')) { + expect(chip.classList.contains('done')).toBe(false) + expect(chip.classList.contains('active')).toBe(false) + } + }) + + it('grows the box to hold its chips, and keeps a plain box for the rest', () => { + const { container } = render(FlowView, { workflow: listWorkflow }) + const height = (name: string) => + Number( + nodeFor(container, name) + .querySelector('rect.box')! + .getAttribute('height'), + ) + expect(height('shot')).toBeGreaterThan(height('episode')) + }) + + it('says for_each rather than nothing when the list cannot be read', () => { + const unknown = { + steps: [ + { + name: 'shot', + for_each: 'variable:missing', + task: { command: 'render' }, + }, + ], + } + const { container } = render(FlowView, { workflow: unknown }) + const shot = nodeFor(container, 'shot') + expect(shot.getAttribute('aria-label')).toContain('for_each') + expect(shot.querySelectorAll('g.member')).toHaveLength(0) + expect(shot.querySelector('.membersunknown')?.textContent).toBe('for_each') + }) +}) diff --git a/ui/src/lib/flow.test.ts b/ui/src/lib/flow.test.ts index 83fed24e..46868f61 100644 --- a/ui/src/lib/flow.test.ts +++ b/ui/src/lib/flow.test.ts @@ -279,3 +279,65 @@ describe('for_each references', () => { ]) }) }) + +describe('for_each members', () => { + it("lists a declared variable list's entry names as the step's members", () => { + // What a realized workflow carries: for_each still names the variable, + // and the run's actual list sits in variables (dw/realize.py) + const wf = { + variables: { shots: [{ name: 'open' }, { name: 'reveal' }] }, + steps: [ + { + name: 'shot', + for_each: 'variable:shots', + task: { command: 'render' }, + }, + ], + } + const node = dataFlowGraph(wf).nodes[0] + expect(node.forEach).toBe(true) + expect(node.members).toEqual(['open', 'reveal']) + }) + + it('keys unnamed entries by index, the way the engine names members', () => { + const wf = { + variables: { items: ['a', 'b'] }, + steps: [ + { name: 'run', for_each: 'variable:items', task: { command: 'x' } }, + ], + } + expect(dataFlowGraph(wf).nodes[0].members).toEqual(['0', '1']) + }) + + it('reads a literal for_each list written on the step itself', () => { + const wf = { + steps: [ + { + name: 'run', + for_each: [{ name: 'a' }, { name: 'b' }], + task: { command: 'x' }, + }, + ], + } + expect(dataFlowGraph(wf).nodes[0].members).toEqual(['a', 'b']) + }) + + it('marks a list-driven step whose list cannot be read, and leaves plain steps alone', () => { + const wf = { + variables: { other: 3 }, + steps: [ + { + name: 'run', + for_each: 'variable:missing', + task: { command: 'x' }, + }, + step('plain', {}), + ], + } + const graph = dataFlowGraph(wf) + expect(graph.nodes[0].forEach).toBe(true) + expect(graph.nodes[0].members).toBeNull() + expect(graph.nodes[1].forEach).toBe(false) + expect(graph.nodes[1].members).toBeNull() + }) +}) diff --git a/ui/src/lib/flow.ts b/ui/src/lib/flow.ts index c4709771..32120d24 100644 --- a/ui/src/lib/flow.ts +++ b/ui/src/lib/flow.ts @@ -109,6 +109,14 @@ export interface FlowNode { * JSON says so directly (currently just a literal * `num_images_per_prompt`). Null means "unknown, assume 1". */ producedCount: number | null + /** True when the step carries a `for_each` - list-driven, so a run + * expands it into one step per entry. */ + forEach: boolean + /** The step's entry keys when its `for_each` list is readable from the + * definition (a literal, or a declared variable holding one) - the + * names the run's members carry after the `@`. Null when there is no + * for_each or the list cannot be read statically. */ + members: string[] | null } export interface FlowEdge { @@ -149,6 +157,37 @@ function producedCount(step: Record): number | null { return typeof n === 'number' ? n : null } +const FOR_EACH_KEY = 'for_each' +const VARIABLE_PREFIX = 'variable:' + +/** A for_each step's entry keys, when the list can be read statically: a + * literal list written on the step, or a `variable:` naming a declared + * variable that holds one - which a realized workflow always does, since + * the run's actual list is folded into `variables` (dw/realize.py). The + * keys are what the engine appends to the step name: an entry's `name`, + * else its index (`_entry_keys`, dw/for_each.py). */ +function forEachMembers( + workflow: Record, + step: Record, +): string[] | null { + const value = step[FOR_EACH_KEY] + let entries: unknown[] | null = null + if (Array.isArray(value)) { + entries = value + } else if (typeof value === 'string' && value.startsWith(VARIABLE_PREFIX)) { + const declared = workflow.variables?.[value.slice(VARIABLE_PREFIX.length)] + if (Array.isArray(declared)) entries = declared + } + if (!entries?.length) return null + return entries.map((entry, index) => + entry !== null && + typeof entry === 'object' && + typeof (entry as Record).name === 'string' + ? (entry as Record).name + : String(index), + ) +} + /** The read-only data-flow view's graph: one node per step, one edge per * `previous_result:` reference (labeled with the attribute that * carries it), entry points flagged, and fan-in points - steps combining @@ -166,6 +205,8 @@ export function dataFlowGraph(workflow: Record): DataFlowGraph { detail, isEntryPoint: true, producedCount: producedCount(step), + forEach: FOR_EACH_KEY in step, + members: forEachMembers(workflow, step), } }) const edges: FlowEdge[] = [] diff --git a/ui/src/lib/pages/JobPage.svelte b/ui/src/lib/pages/JobPage.svelte index 3544746d..68e9e588 100644 --- a/ui/src/lib/pages/JobPage.svelte +++ b/ui/src/lib/pages/JobPage.svelte @@ -15,7 +15,12 @@ sectionBySubfolder, unsavedSteps, } from '../results' - import { finishedNodes, flowNodeName } from '../runstate' + import { + activeMember, + finishedMembers, + finishedNodes, + flowNodeName, + } from '../runstate' import { stepProgress } from '../progress' import FlowView from '../editor/FlowView.svelte' import CopyButton from '../CopyButton.svelte' @@ -271,6 +276,12 @@ unsavedSteps(job?.manifest, events as JobEvent[], definition), ) const running = $derived(job !== null && !TERMINAL.includes(job.status)) + // One grain finer than the group: which entries of a for_each step have + // finished and which is running, in the engine's own `group@entry` names + const finishedMemberSteps = $derived(finishedMembers(events as JobEvent[])) + const activeMemberStep = $derived( + running ? activeMember(events as JobEvent[]) : undefined, + ) // A cancel requested while loading a model or running a task step has no // checkpoint to catch it until that phase finishes - without this the UI // goes silent for however long that takes, and looks hung rather than @@ -434,6 +445,8 @@ workflow={definition} activeStep={running ? activeNode : undefined} doneSteps={finishedSteps} + activeMember={activeMemberStep} + doneMembers={finishedMemberSteps} /> {/if} diff --git a/ui/src/lib/pages/JobPage.test.ts b/ui/src/lib/pages/JobPage.test.ts index 78ebf910..810d64a9 100644 --- a/ui/src/lib/pages/JobPage.test.ts +++ b/ui/src/lib/pages/JobPage.test.ts @@ -210,6 +210,47 @@ it('lights the for_each step in the flow chart while one of its members runs', a expect(nodeFor(container, 'episode').classList.contains('active')).toBe(true) }) +it('marks the entries a for_each step runs as chips inside its box', async () => { + // The realized workflow keeps for_each and holds the run's actual list + // in its variables, so the flow view can name the members + ran.definition = { + variables: { shots: [{ name: 'open' }, { name: 'reveal' }] }, + steps: [ + { + name: 'shot', + for_each: 'variable:shots', + pipeline: { configuration: { component_type: 'Fake' } }, + }, + ], + } + detail.job = { ...job([]), status: 'running', finished_at: null } + const { container } = render(JobPage, { jobId: 'j1' }) + await waitFor(() => expect(stream.onEvent).not.toBeNull()) + stream.onEvent!({ + seq: 1, + event: 'workflow_start', + steps: ['shot@open', 'shot@reveal'], + }) + stream.onEvent!({ seq: 2, event: 'step_start', step: 'shot@open' }) + const chip = (key: string) => + [...nodeFor(container, 'shot').querySelectorAll('g.member')].find( + // the label is "member shot@open" plus ", done"/", active" when styled + (m) => m.getAttribute('aria-label')?.split(',')[0] === `member shot@${key}`, + )! + await waitFor(() => + expect(chip('open').classList.contains('active')).toBe(true), + ) + + // One entry down, one to go: the finished chip greens while the group + // box itself stays amber + stream.onEvent!({ seq: 3, event: 'step_end', step: 'shot@open', files: [] }) + await waitFor(() => + expect(chip('open').classList.contains('done')).toBe(true), + ) + expect(chip('reveal').classList.contains('done')).toBe(false) + expect(nodeFor(container, 'shot').classList.contains('active')).toBe(true) +}) + it('keeps the Progress list on the composed step while its child runs', async () => { ran.definition = { steps: [ diff --git a/ui/src/lib/runstate.test.ts b/ui/src/lib/runstate.test.ts index 43990901..9d7d61c6 100644 --- a/ui/src/lib/runstate.test.ts +++ b/ui/src/lib/runstate.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from 'vitest' -import { finishedNodes, flowNodeName } from './runstate' +import { + activeMember, + finishedMembers, + finishedNodes, + flowNodeName, +} from './runstate' import type { JobEvent } from './types' const ended = (step: string, parent_step?: string): JobEvent => @@ -52,3 +57,58 @@ describe('finishedNodes', () => { expect(finishedNodes([], members)).toEqual([]) }) }) + +describe('finishedMembers', () => { + it('lists the for_each members that have ended, engine names and all', () => { + const events = [ended('base@open'), ended('film'), ended('base@reveal')] + expect(finishedMembers(events)).toEqual(['base@open', 'base@reveal']) + }) + + it("leaves out a sub-workflow's inner members - their chips are not this graph's", () => { + expect(finishedMembers([ended('shot@reveal', 'cut')])).toEqual([]) + }) + + it('has nothing for a run that has not ended anything', () => { + expect(finishedMembers([])).toEqual([]) + }) +}) + +describe('activeMember', () => { + it('is the member the run is on, engine name and all', () => { + const events: JobEvent[] = [ + { seq: 0, event: 'step_start', step: 'base@open' }, + { seq: 1, event: 'step_end', step: 'base@open' }, + { seq: 2, event: 'step_start', step: 'base@reveal' }, + ] + expect(activeMember(events)).toBe('base@reveal') + }) + + it('is nothing in the gap after a member ended, before the next one starts', () => { + const events: JobEvent[] = [ + { seq: 0, event: 'step_start', step: 'base@open' }, + { seq: 1, event: 'step_end', step: 'base@open' }, + ] + expect(activeMember(events)).toBeUndefined() + }) + + it('is nothing once the run has moved on to a plain step', () => { + const events: JobEvent[] = [ + { seq: 0, event: 'step_start', step: 'base@open' }, + { seq: 1, event: 'step_end', step: 'base@open' }, + { seq: 2, event: 'step_start', step: 'episode' }, + ] + expect(activeMember(events)).toBeUndefined() + }) + + it("ignores a sub-workflow's inner steps, member-named or not", () => { + const events: JobEvent[] = [ + { seq: 0, event: 'step_start', step: 'base@open', parent_step: 'cut' }, + { seq: 1, event: 'step_start', step: 'inner', parent_step: 'shot1' }, + ] + expect(activeMember(events)).toBeUndefined() + }) + + it('has nothing before the run has started anything', () => { + expect(activeMember([])).toBeUndefined() + }) +}) diff --git a/ui/src/lib/runstate.ts b/ui/src/lib/runstate.ts index 8bd0fc9c..aeaada41 100644 --- a/ui/src/lib/runstate.ts +++ b/ui/src/lib/runstate.ts @@ -59,3 +59,33 @@ export function finishedNodes( .filter(([, names]) => names.every((name) => ended.has(name))) .map(([node]) => node) } + +/** The for_each members a run has finished, in the engine's own + * `group@entry` spelling - the names the flow view's member chips carry. + * Only top-level ends count: a sub-workflow's inner members, whose + * `step_end` carries the composed step as `parent_step`, belong to no + * chip this graph draws. */ +export function finishedMembers(events: JobEvent[]): string[] { + return events + .filter((event) => event.event === 'step_end' && !event.parent_step) + .map((event) => event.step as string) + .filter((step) => step.includes(MEMBER_SEPARATOR)) +} + +/** The for_each member the run is on right now, or undefined when the + * last step it started is not a member - or is a member that has already + * ended, since between one entry finishing and the next starting the run + * is on neither, and a chip still amber there would lie about progress. A + * sub-workflow's inner steps do not count, member-named or not, for the + * same reason as above. */ +export function activeMember(events: JobEvent[]): string | undefined { + const ended = new Set(finishedMembers(events)) + for (let i = events.length - 1; i >= 0; i--) { + const event = events[i] + if (event.event !== 'step_start' || event.parent_step) continue + const step = event.step as string + if (!step.includes(MEMBER_SEPARATOR) || ended.has(step)) return undefined + return step + } + return undefined +} From 5dbbd7fdc66b1c0e61343d1f9a82c320c065bc95 Mon Sep 17 00:00:00 2001 From: Don Kackman Date: Sun, 13 Sep 2026 17:21:00 -0500 Subject: [PATCH 2/3] format --- ui/src/lib/editor/FlowView.svelte | 6 +----- ui/src/lib/editor/FlowView.test.ts | 6 +++--- ui/src/lib/pages/JobPage.test.ts | 3 ++- ui/src/lib/runstate.test.ts | 2 +- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/ui/src/lib/editor/FlowView.svelte b/ui/src/lib/editor/FlowView.svelte index 2a87288c..6f789d73 100644 --- a/ui/src/lib/editor/FlowView.svelte +++ b/ui/src/lib/editor/FlowView.svelte @@ -28,11 +28,7 @@ name === activeStep ? 'active' : doneSteps.includes(name) ? 'done' : '', ) const memberStateOf = $derived((full: string) => - full === activeMember - ? 'active' - : doneMembers.includes(full) - ? 'done' - : '', + full === activeMember ? 'active' : doneMembers.includes(full) ? 'done' : '', ) const graph = $derived(dataFlowGraph(workflow)) diff --git a/ui/src/lib/editor/FlowView.test.ts b/ui/src/lib/editor/FlowView.test.ts index 6ad0efad..7376c9ea 100644 --- a/ui/src/lib/editor/FlowView.test.ts +++ b/ui/src/lib/editor/FlowView.test.ts @@ -132,10 +132,10 @@ describe('for_each members', () => { 'reveal', ]) // The whole engine name is on the chip, as hover text + expect(chips[0].querySelector('title')?.textContent).toBe('shot@open') expect( - chips[0].querySelector('title')?.textContent, - ).toBe('shot@open') - expect(nodeFor(container, 'episode').querySelectorAll('g.member')).toHaveLength(0) + nodeFor(container, 'episode').querySelectorAll('g.member'), + ).toHaveLength(0) expect( nodeFor(container, 'episode').getAttribute('aria-label'), ).not.toContain('for_each') diff --git a/ui/src/lib/pages/JobPage.test.ts b/ui/src/lib/pages/JobPage.test.ts index 810d64a9..eb0d6545 100644 --- a/ui/src/lib/pages/JobPage.test.ts +++ b/ui/src/lib/pages/JobPage.test.ts @@ -235,7 +235,8 @@ it('marks the entries a for_each step runs as chips inside its box', async () => const chip = (key: string) => [...nodeFor(container, 'shot').querySelectorAll('g.member')].find( // the label is "member shot@open" plus ", done"/", active" when styled - (m) => m.getAttribute('aria-label')?.split(',')[0] === `member shot@${key}`, + (m) => + m.getAttribute('aria-label')?.split(',')[0] === `member shot@${key}`, )! await waitFor(() => expect(chip('open').classList.contains('active')).toBe(true), diff --git a/ui/src/lib/runstate.test.ts b/ui/src/lib/runstate.test.ts index 9d7d61c6..3b60d0de 100644 --- a/ui/src/lib/runstate.test.ts +++ b/ui/src/lib/runstate.test.ts @@ -103,7 +103,7 @@ describe('activeMember', () => { it("ignores a sub-workflow's inner steps, member-named or not", () => { const events: JobEvent[] = [ { seq: 0, event: 'step_start', step: 'base@open', parent_step: 'cut' }, - { seq: 1, event: 'step_start', step: 'inner', parent_step: 'shot1' }, + { seq: 1, event: 'step_start', step: 'inner', parent_step: 'shot1' }, ] expect(activeMember(events)).toBeUndefined() }) From e1e2cd9b090f0b9dfbd64f10bcab3a83b2518c27 Mon Sep 17 00:00:00 2001 From: Don Kackman Date: Sun, 13 Sep 2026 17:34:35 -0500 Subject: [PATCH 3/3] feat(api): update workflow handling to separate definition from transport metadata --- ui/src/lib/api.test.ts | 23 +++++++++++++++++++++++ ui/src/lib/api.ts | 7 +++++-- ui/src/lib/editor/StepEditor.svelte | 14 +++++++------- ui/src/lib/pages/EditorPage.svelte | 6 +++--- ui/src/lib/pages/EditorPage.test.ts | 25 +++++++++++++++++++++++++ ui/src/lib/pages/WorkflowPage.svelte | 8 ++++---- ui/src/lib/types.ts | 9 +++++++-- 7 files changed, 74 insertions(+), 18 deletions(-) diff --git a/ui/src/lib/api.test.ts b/ui/src/lib/api.test.ts index 0b815120..e712e07d 100644 --- a/ui/src/lib/api.test.ts +++ b/ui/src/lib/api.test.ts @@ -118,6 +118,29 @@ describe('name encoding', () => { }) }) +describe('workflow definition fetch', () => { + it('keeps the definition exactly as served, with origin and writable beside it', async () => { + const body = { id: 'z-image', steps: [] } + stubFetch({ + ok: true, + body, + headers: { + 'X-Workflow-Origin': 'workspace', + 'X-Workflow-Writable': 'true', + }, + }) + const result = await api.getWorkflow('models/z-image') + expect(result.definition).toEqual(body) + // The transport metadata rides beside the definition, not inside it - + // a workflow opened by name must validate and save as the file it + // came from, and the schema refuses unknown root keys + expect(result.definition).not.toHaveProperty('origin') + expect(result.definition).not.toHaveProperty('writable') + expect(result.origin).toBe('workspace') + expect(result.writable).toBe(true) + }) +}) + describe('gallery listing and thumbnails', () => { it('fetches the whole listing in one request', async () => { const calls = stubFetch({ ok: true, body: {} }) diff --git a/ui/src/lib/api.ts b/ui/src/lib/api.ts index 34c7bf15..f4543feb 100644 --- a/ui/src/lib/api.ts +++ b/ui/src/lib/api.ts @@ -273,11 +273,14 @@ export const api = { > }>('/api/workflows'), /** The workflow plus where it came from, read off the response headers - * rather than a separate `listWorkflows` lookup. */ + * rather than a separate `listWorkflows` lookup. Beside the definition, + * the way `getPrompt` keeps a prompt's: the object the caller holds is + * what validate, save and run send back, and the schema refuses unknown + * root keys, so the transport metadata must not ride inside it. */ getWorkflow: (name: string) => fetchJson(`/api/workflows/${encodePath(name)}`).then( ({ body, response }): WorkflowWithOrigin => ({ - ...body, + definition: body, origin: response.headers.get('X-Workflow-Origin') ?? '', writable: response.headers.get('X-Workflow-Writable') !== 'false', }), diff --git a/ui/src/lib/editor/StepEditor.svelte b/ui/src/lib/editor/StepEditor.svelte index 1d3ebf2c..07affa5d 100644 --- a/ui/src/lib/editor/StepEditor.svelte +++ b/ui/src/lib/editor/StepEditor.svelte @@ -138,13 +138,13 @@ const timer = setTimeout(() => { api .getWorkflow(resolved.slice(0, -'.json'.length)) - .then((definition) => { - workflowVariables = Object.entries(definition.variables ?? {}).map( - ([name, value]) => ({ - name, - hint: typeof value === 'string' ? value : JSON.stringify(value), - }), - ) + .then((fetched) => { + workflowVariables = Object.entries( + fetched.definition.variables ?? {}, + ).map(([name, value]) => ({ + name, + hint: typeof value === 'string' ? value : JSON.stringify(value), + })) }) .catch(() => {}) }, 300) diff --git a/ui/src/lib/pages/EditorPage.svelte b/ui/src/lib/pages/EditorPage.svelte index a2cdc596..77feeeaf 100644 --- a/ui/src/lib/pages/EditorPage.svelte +++ b/ui/src/lib/pages/EditorPage.svelte @@ -211,9 +211,9 @@ fileOpen = false api .getWorkflow(name) - .then((definition) => { - workflow = definition as WorkflowDefinition - baseline = JSON.stringify(definition) + .then((fetched) => { + workflow = fetched.definition as WorkflowDefinition + baseline = JSON.stringify(fetched.definition) stepModes = storageGet(modesKey, {}) }) .catch((e) => notify.error(e.message)) diff --git a/ui/src/lib/pages/EditorPage.test.ts b/ui/src/lib/pages/EditorPage.test.ts index 1962a8b7..7b689ce2 100644 --- a/ui/src/lib/pages/EditorPage.test.ts +++ b/ui/src/lib/pages/EditorPage.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { cleanup, render, screen, waitFor } from '@testing-library/svelte' import EditorPage from './EditorPage.svelte' +import { api } from '../api' // EditorPage talks to the server on mount (pipeline/class/task catalogs, // the workflow listing, the prompt library) purely to feed forms the flow @@ -19,6 +20,11 @@ vi.mock('../api', () => ({ listWorkflows: vi .fn() .mockResolvedValue({ workflows: [], workflow_dir: 'workflows' }), + getWorkflow: vi.fn().mockResolvedValue({ + definition: { id: 'ZImage', steps: [{ name: 'generate', pipeline: {} }] }, + origin: 'workspace', + writable: true, + }), listPrompts: vi.fn().mockResolvedValue({ prompts: [], details: {} }), validate: vi.fn().mockResolvedValue({ valid: true, @@ -78,6 +84,25 @@ describe('EditorPage view switch', () => { }) describe('EditorPage validation plan', () => { + it('validates the definition it opened, without the transport metadata', async () => { + render(EditorPage, { name: 'models/z-image' }) + await waitFor(() => + expect(screen.getByLabelText('workflow id')).toBeTruthy(), + ) + await screen.getByRole('button', { name: /validate/i }).click() + await waitFor(() => + expect(vi.mocked(api.validate).mock.calls).toHaveLength(1), + ) + const payload = vi.mocked(api.validate).mock.calls[0][0] as Record< + string, + unknown + > + // origin and writable are how the fetch says where a file came from - + // they are not the file's, and the schema refuses unknown root keys + expect(payload).not.toHaveProperty('origin') + expect(payload).not.toHaveProperty('writable') + }) + it('shows what a run will do under a valid verdict', async () => { render(EditorPage, { name: '' }) await waitFor(() => diff --git a/ui/src/lib/pages/WorkflowPage.svelte b/ui/src/lib/pages/WorkflowPage.svelte index cd6f92e8..3e70b6f0 100644 --- a/ui/src/lib/pages/WorkflowPage.svelte +++ b/ui/src/lib/pages/WorkflowPage.svelte @@ -41,10 +41,10 @@ loadPromptLibrary() api .getWorkflow(name) - .then((definition) => { - workflow = definition - origin = definition.origin - writable = definition.writable + .then((fetched) => { + workflow = fetched.definition + origin = fetched.origin + writable = fetched.writable }) .catch((e) => (error = e.message)) }) diff --git a/ui/src/lib/types.ts b/ui/src/lib/types.ts index a11be174..f962141e 100644 --- a/ui/src/lib/types.ts +++ b/ui/src/lib/types.ts @@ -152,8 +152,13 @@ export interface WorkflowDefinition { } /** A workflow plus where it came from - `getWorkflow` reads these off the - * `X-Workflow-Origin` / `X-Workflow-Writable` response headers. */ -export interface WorkflowWithOrigin extends WorkflowDefinition { + * `X-Workflow-Origin` / `X-Workflow-Writable` response headers. Beside the + * definition rather than spread into it, as for a prompt: a workflow is + * validated and saved back exactly as it was read, and a stray root field + * fails the schema - the engine refuses unknown root keys rather than + * ignoring them. */ +export interface WorkflowWithOrigin { + definition: WorkflowDefinition /** 'workspace' | 'examples' | 'builtin'. */ origin: string writable: boolean