diff --git a/.changeset/flow-runs-region-step-logs.md b/.changeset/flow-runs-region-step-logs.md new file mode 100644 index 000000000..d2bdb4384 --- /dev/null +++ b/.changeset/flow-runs-region-step-logs.md @@ -0,0 +1,21 @@ +--- +"@object-ui/app-shell": minor +--- + +feat(studio): nest per-iteration / per-region step logs in the flow Runs panel (#1505) + +The run-observability `FlowRunsPanel` (Studio → flow preview → Runs) rendered a +run's step log as a flat list, so a `loop` container showed as a single step and +its body steps — one set per iteration — appeared as an undifferentiated repeat +of the same node ids, with `parallel` branches and `try`/`catch` handlers +likewise flattened. The automation engine already tags each structured-region +body step with its container (`parentNodeId`) plus an `iteration` / `regionKind` +(ADR-0031, framework #1505); the panel ignored those fields. + +`FlowRunsPanel` now reconstructs the execution tree from the flat, pre-order step +log (`buildStepTree`) and nests body steps under their container node, grouped by +a per-iteration / per-branch / handler header (`Iteration 2`, `Branch 1`, `Try`, +`Catch`). The reconstruction is robust to repeated node ids (a loop body node +runs once per iteration) and to regions nested inside regions, and degrades +safely — a body step whose container was dropped by durable-history truncation +still surfaces at the top level rather than vanishing. diff --git a/packages/app-shell/src/views/metadata-admin/previews/FlowRunsPanel.render.test.tsx b/packages/app-shell/src/views/metadata-admin/previews/FlowRunsPanel.render.test.tsx index b64fc7799..06e87ef42 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/FlowRunsPanel.render.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/FlowRunsPanel.render.test.tsx @@ -44,4 +44,37 @@ describe('FlowRunsPanel (render)', () => { // The string reason must now be in the DOM (pre-fix it was silently dropped). expect(await screen.findByText(/catch region failed/)).toBeTruthy(); }); + + // #1505: a loop's body steps used to render as a flat, indistinguishable + // repeat of the same node ids. They must now nest under per-iteration headers. + it('nests loop body steps under per-iteration headers', async () => { + const LOOP_RUN = { + id: 'run_loop_01', + status: 'completed', + startedAt: '2026-07-04T13:51:13.000Z', + durationMs: 30, + trigger: { type: 'manual' }, + steps: [ + { nodeId: 'start', nodeType: 'start', status: 'success' }, + { nodeId: 'each_order', nodeType: 'loop', status: 'success' }, + { nodeId: 'charge', nodeType: 'http', status: 'success', parentNodeId: 'each_order', iteration: 0, regionKind: 'loop-body' }, + { nodeId: 'charge', nodeType: 'http', status: 'success', parentNodeId: 'each_order', iteration: 1, regionKind: 'loop-body' }, + ], + }; + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response(JSON.stringify({ success: true, data: { runs: [LOOP_RUN] } }), { status: 200 })), + ); + + render(); + fireEvent.click(await screen.findByRole('button', { expanded: false })); + + // Each iteration gets its own header, so the two runs of the body are + // distinguishable rather than a flat repeat of `charge`. + expect(await screen.findByText('Iteration 1')).toBeTruthy(); + expect(screen.getByText('Iteration 2')).toBeTruthy(); + // The loop container renders once; its body step renders once per iteration. + expect(screen.getByText('each_order')).toBeTruthy(); + expect(screen.getAllByText('charge')).toHaveLength(2); + }); }); diff --git a/packages/app-shell/src/views/metadata-admin/previews/FlowRunsPanel.test.ts b/packages/app-shell/src/views/metadata-admin/previews/FlowRunsPanel.test.ts index 8610a45ae..a070384b6 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/FlowRunsPanel.test.ts +++ b/packages/app-shell/src/views/metadata-admin/previews/FlowRunsPanel.test.ts @@ -1,7 +1,8 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, vi, afterEach } from 'vitest'; -import { fetchFlowRuns, errorText } from './FlowRunsPanel'; +import { fetchFlowRuns, errorText, buildStepTree, regionLabel } from './FlowRunsPanel'; +import type { StepTreeNode } from './FlowRunsPanel'; const RUN = { id: 'run-1', @@ -75,3 +76,91 @@ describe('errorText', () => { expect(errorText({ code: 'E' })).toBeUndefined(); }); }); + +// ── #1505: structured-region (loop / parallel / try-catch) step grouping ── + +/** Compact a step tree to `nodeId(child,child…)` strings for legible asserts. */ +function outline(nodes: StepTreeNode[]): string[] { + return nodes.map((n) => + n.children.length ? `${n.step.nodeId}(${outline(n.children).join(',')})` : n.step.nodeId, + ); +} + +describe('buildStepTree', () => { + it('keeps a flat top-level log flat (no regions)', () => { + const tree = buildStepTree([ + { nodeId: 'start', status: 'success' }, + { nodeId: 'notify', status: 'success' }, + ]); + expect(outline(tree)).toEqual(['start', 'notify']); + }); + + it("nests a loop's body steps (across iterations) under the loop node", () => { + const tree = buildStepTree([ + { nodeId: 'start', status: 'success' }, + { nodeId: 'loop1', nodeType: 'loop', status: 'success' }, + { nodeId: 'send', status: 'success', parentNodeId: 'loop1', iteration: 0, regionKind: 'loop-body' }, + { nodeId: 'log', status: 'success', parentNodeId: 'loop1', iteration: 0, regionKind: 'loop-body' }, + { nodeId: 'send', status: 'success', parentNodeId: 'loop1', iteration: 1, regionKind: 'loop-body' }, + { nodeId: 'log', status: 'success', parentNodeId: 'loop1', iteration: 1, regionKind: 'loop-body' }, + ]); + expect(outline(tree)).toEqual(['start', 'loop1(send,log,send,log)']); + // The per-iteration index is preserved on each child (drives the header split). + expect(tree[1].children.map((c) => c.step.iteration)).toEqual([0, 0, 1, 1]); + }); + + it('nests parallel branch steps under the parallel node', () => { + const tree = buildStepTree([ + { nodeId: 'par', nodeType: 'parallel', status: 'success' }, + { nodeId: 'a', status: 'success', parentNodeId: 'par', iteration: 0, regionKind: 'parallel-branch' }, + { nodeId: 'b', status: 'success', parentNodeId: 'par', iteration: 1, regionKind: 'parallel-branch' }, + ]); + expect(outline(tree)).toEqual(['par(a,b)']); + }); + + it('nests try and catch handler steps under the try_catch node', () => { + const tree = buildStepTree([ + { nodeId: 'tc', nodeType: 'try_catch', status: 'success' }, + { nodeId: 'risky', status: 'failure', parentNodeId: 'tc', regionKind: 'try' }, + { nodeId: 'recover', status: 'success', parentNodeId: 'tc', regionKind: 'catch' }, + ]); + expect(outline(tree)).toEqual(['tc(risky,recover)']); + }); + + it('reconstructs nested regions (a loop inside a loop)', () => { + const tree = buildStepTree([ + { nodeId: 'outer', nodeType: 'loop', status: 'success' }, + { nodeId: 'inner', nodeType: 'loop', status: 'success', parentNodeId: 'outer', iteration: 0, regionKind: 'loop-body' }, + { nodeId: 'body', status: 'success', parentNodeId: 'inner', iteration: 0, regionKind: 'loop-body' }, + { nodeId: 'inner', nodeType: 'loop', status: 'success', parentNodeId: 'outer', iteration: 1, regionKind: 'loop-body' }, + { nodeId: 'body', status: 'success', parentNodeId: 'inner', iteration: 0, regionKind: 'loop-body' }, + ]); + expect(outline(tree)).toEqual(['outer(inner(body),inner(body))']); + }); + + it('surfaces an orphaned body step (truncated history) at the top level', () => { + // The loop container step was dropped (e.g. by durable-history compaction); + // its body step must still show rather than vanish. + const tree = buildStepTree([ + { nodeId: 'body', status: 'success', parentNodeId: 'gone', iteration: 3, regionKind: 'loop-body' }, + ]); + expect(outline(tree)).toEqual(['body']); + }); +}); + +describe('regionLabel', () => { + it('labels loop iterations 1-based', () => { + expect(regionLabel({ nodeId: 'x', status: 'success', regionKind: 'loop-body', iteration: 0 })).toBe('Iteration 1'); + expect(regionLabel({ nodeId: 'x', status: 'success', regionKind: 'loop-body', iteration: 4 })).toBe('Iteration 5'); + }); + it('labels parallel branches 1-based', () => { + expect(regionLabel({ nodeId: 'x', status: 'success', regionKind: 'parallel-branch', iteration: 1 })).toBe('Branch 2'); + }); + it('labels try / catch handlers', () => { + expect(regionLabel({ nodeId: 'x', status: 'success', regionKind: 'try' })).toBe('Try'); + expect(regionLabel({ nodeId: 'x', status: 'success', regionKind: 'catch' })).toBe('Catch'); + }); + it('is null for a top-level step (no region)', () => { + expect(regionLabel({ nodeId: 'x', status: 'success' })).toBeNull(); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/previews/FlowRunsPanel.tsx b/packages/app-shell/src/views/metadata-admin/previews/FlowRunsPanel.tsx index 5cc203674..969eb6042 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/FlowRunsPanel.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/FlowRunsPanel.tsx @@ -4,8 +4,12 @@ * FlowRunsPanel — run history for a flow, fetched from the automation engine * (`GET /api/v1/automation/{name}/runs`, the observability surface next to * resume/screen). Renders each run's status / start time / duration with an - * expandable per-node step log (the `ExecutionLog.steps` ADR-0019/#1479 shape), - * so authors can see where a run paused or failed without leaving the Studio. + * expandable step log (the `ExecutionLog.steps` ADR-0019/#1479 shape). Body + * steps that ran inside a structured control-flow region — a `loop` iteration, + * a `parallel` branch, or a `try`/`catch` handler — are nested under their + * container node and grouped by iteration / branch (#1505), so authors can see + * where a run paused or failed *and which iteration did it*, without leaving the + * Studio. * * Degrades like the palette fetch: offline / plugin-absent / older backend → * a quiet "history unavailable" note, never an error state that blocks the @@ -29,6 +33,23 @@ interface RunStep { status: 'success' | 'failure' | 'skipped' | string; durationMs?: number; error?: RunError; + // #1505: structured-region grouping. A step that ran inside a `loop` / + // `parallel` / `try_catch` body region is tagged by the engine with its + // immediate container so the panel can nest it, instead of showing the + // container as one opaque step. Absent on top-level (main-graph) steps. + parentNodeId?: string; + /** Zero-based loop iteration or parallel branch index of the enclosing region. */ + iteration?: number; + /** Region kind the step ran in: `loop-body` | `parallel-branch` | `try` | `catch`. */ + regionKind?: string; +} + +/** A step plus the region body steps that ran under it — the execution tree the + * panel renders. Reconstructed from the engine's flat, pre-order step log by + * {@link buildStepTree}. */ +export interface StepTreeNode { + step: RunStep; + children: StepTreeNode[]; } /** Run log entry (spec `ExecutionLogSchema`, fields we render). */ @@ -56,6 +77,89 @@ export function errorText(e: RunError | undefined | null): string | undefined { return typeof m === 'string' && m ? m : undefined; } +/** + * Reconstruct the execution tree from the engine's flat, pre-order step log + * (#1505). Each step carries its **immediate** structured-region container in + * `parentNodeId`; the container's own step always precedes its body steps in the + * array, and a whole region's steps are contiguous (the engine appends + * `NodeExecutionResult.childSteps` in one shot). A stack walk therefore rebuilds + * the nesting exactly, and it is robust to the two things that break naive + * grouping: repeated `nodeId`s (a loop body node runs once per iteration) and + * regions nested inside regions. + * + * Degrades safely: a step whose `parentNodeId` has no open ancestor — e.g. a + * container step was dropped by durable-history truncation — is surfaced at the + * top level rather than silently discarded. + */ +export function buildStepTree(steps: RunStep[]): StepTreeNode[] { + const roots: StepTreeNode[] = []; + const stack: StepTreeNode[] = []; + for (const step of steps) { + const node: StepTreeNode = { step, children: [] }; + if (step.parentNodeId == null) { + stack.length = 0; // a top-level step closes every open region + roots.push(node); + } else { + // Pop until the stack top is this step's container. + while (stack.length > 0 && stack[stack.length - 1].step.nodeId !== step.parentNodeId) { + stack.pop(); + } + if (stack.length > 0) { + stack[stack.length - 1].children.push(node); + } else { + roots.push(node); // container not found (truncated log) — don't lose the step + } + } + stack.push(node); // every step may itself contain a nested region + } + return roots; +} + +/** + * Human label for a body step's enclosing region (#1505). `loop`/`parallel` + * carry a zero-based `iteration` surfaced 1-based; `try`/`catch` carry only the + * region kind. Returns `null` for a top-level step (no region grouping). + */ +export function regionLabel(step: RunStep): string | null { + const { regionKind, iteration } = step; + if (!regionKind) return null; + switch (regionKind) { + case 'loop-body': + return iteration == null ? 'Iteration' : `Iteration ${iteration + 1}`; + case 'parallel-branch': + return iteration == null ? 'Branch' : `Branch ${iteration + 1}`; + case 'try': + return 'Try'; + case 'catch': + return 'Catch'; + default: + return iteration == null ? regionKind : `${regionKind} ${iteration + 1}`; + } +} + +/** Grouping key so consecutive body steps of the same iteration/branch/handler + * share one header. */ +function regionSignature(step: RunStep): string { + return `${step.regionKind ?? ''}#${step.iteration ?? ''}`; +} + +/** Split a container's children into consecutive runs that share a region label + * (an iteration, a branch, a try/catch handler), so each gets one header. */ +function groupChildren(children: StepTreeNode[]): { label: string | null; items: StepTreeNode[] }[] { + const groups: { label: string | null; items: StepTreeNode[] }[] = []; + let sig: string | undefined; + for (const child of children) { + const s = regionSignature(child.step); + if (groups.length === 0 || s !== sig) { + groups.push({ label: regionLabel(child.step), items: [child] }); + sig = s; + } else { + groups[groups.length - 1].items.push(child); + } + } + return groups; +} + type LoadState = 'loading' | 'ready' | 'unavailable'; /** Fetch a flow's run history. Exposed for tests. */ @@ -99,7 +203,7 @@ function fmtDuration(ms?: number): string | null { return `${Math.round(ms / 60_000)}m`; } -function StepRow({ step }: { step: RunStep }) { +function StepRow({ step, depth = 0 }: { step: RunStep; depth?: number }) { const cls = step.status === 'success' ? 'text-emerald-600 dark:text-emerald-400' @@ -108,7 +212,7 @@ function StepRow({ step }: { step: RunStep }) { : 'text-muted-foreground'; const stepErr = errorText(step.error); return ( -
  • +
  • {step.status} {step.nodeId} {step.nodeType && {step.nodeType}} @@ -124,6 +228,44 @@ function StepRow({ step }: { step: RunStep }) { ); } +/** Header for a run of body steps in one iteration / branch / try-catch handler. */ +function RegionHeader({ label, depth }: { label: string; depth: number }) { + return ( +
  • + + ↳ + + {label} +
    • + ); +} + +/** Render a step and, nested beneath it, its structured-region body steps — + * grouped by iteration / branch / handler (#1505). Recurses for nested regions. */ +function StepNode({ node, depth }: { node: StepTreeNode; depth: number }) { + const groups = node.children.length > 0 ? groupChildren(node.children) : []; + return ( + <> + + {groups.map((g, gi) => ( + + {g.label != null && } + {g.items.map((child, ci) => ( + + ))} + + ))} + + ); +} + function RunRow({ run }: { run: FlowRun }) { const [open, setOpen] = React.useState(false); const meta = statusMeta(run.status); @@ -164,9 +306,9 @@ function RunRow({ run }: { run: FlowRun }) { {steps.length === 0 ? (
    No step log recorded.
    ) : ( -