Conversation
… and loop-backs The dashboard rendered workflows like sdd/plan-v3 as visually disconnected columns with long diagonal edges. Two independent issues were responsible: 1. Engine: workflow_started omitted human_gate option routes The 'routes' field of the workflow_started event was built only from agent.routes, parallel.routes, and for_each.routes. Edges declared via human_gate options[].route were never emitted, so any subgraph reachable only through a gate appeared genuinely disconnected in the dashboard. Fixed by adding a list comprehension that emits one route per human_gate option. 2. Frontend: Dagre arbitrarily reversed edges when breaking cycles When workflows have multiple loop-back routes (e.g. revision loops), Dagre's greedy acyclicer reversed arbitrary edges to break cycles, scrambling the rank assignment so downstream nodes ended up above their predecessors. Added findBackEdges() that DFS's from $start to identify loop-back edges, then passes them to Dagre in reversed direction so it ranks the underlying DAG correctly. The visible edges keep their original direction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- workflow.py: drop 'if o.route' filter; GateOption.route is required str - graph-layout.ts: pass '$start' directly (findBackEdges handles missing) - graph-layout.ts: drop redundant pre-filter on remainingSources - graph-layout.ts: tighten docstring on traversal order Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4f60d5d to
42c8967
Compare
PolyphonyRequiem
left a comment
There was a problem hiding this comment.
Good catch on the dual root cause — separating the engine emission gap (gate routes literally absent from workflow_started) from Dagre's greedy-acyclicer mishandling multiple loop-backs is the right framing, and both fixes are minimal.
I had the thought to add frontend tests for findBackEdges but wanted to consult on the approach first — the frontend has no test runner configured today (no Vitest/Jest in devDependencies, no test script, no existing test files), so doing it here means standing up the harness as well. Happy to defer to a follow-up; just flagging since the function has enough branching (DFS stack tracking, cycle vs cross-edge, unreachable subgraph fallback) that it'd benefit from coverage once a runner exists.
| { | ||
| "from": a.name, | ||
| "to": o.route, | ||
| "when": f"selection == '{o.value}'", |
There was a problem hiding this comment.
Suggestion: add a regression test alongside test_workflow_started_includes_routes (tests/test_engine/test_event_emission.py:163) that asserts a human_gate's option routes appear in event.data["routes"] with when == "selection == '<value>'". Cheap insurance that the silent regression motivating this PR can't sneak back in.
| * target-ID order, and unreachable subgraphs are entered in sorted source-ID | ||
| * order, so layout is stable across renders. | ||
| */ | ||
| function findBackEdges( |
There was a problem hiding this comment.
Question: findBackEdges uses recursive DFS via a closure — fine for normal graphs, but does it hold up for workflows with sub-workflows / type: workflow agents? My read is that each sub-workflow renders its own isolated graph in the dashboard so recursion depth is bounded by the longest path within a single workflow (not by MAX_SUBWORKFLOW_DEPTH nesting), but wanted to confirm you've thought about it — happy to be wrong.
* chore: release 0.1.12 Bumps version to 0.1.12 and updates CHANGELOG with the four PRs merged since v0.1.11: - #149: Windows install diagnostics - #151: Tag-based registry versioning with # ref syntax - #152: Unified reasoning.effort configuration - #153: Dashboard layout fix for human_gate options + loop-backs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: add #155 (Windows update reliability) to 0.1.12 changelog Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Workflows with
human_gateoptions and/or multiple loop-back routes (e.g.workflows/sdd/plan-v3.yaml) rendered as visually disconnected columns in the dashboard, with long diagonal edges and downstream nodes placed above their predecessors. This PR addresses both root causes.Fixes
1. Engine: emit
human_gateoption routes inworkflow_startedworkflow_startedonly included routes fromagent.routes,parallel.routes, andfor_each.routes. Edges declared viahuman_gateoptions[].routewere never emitted, so any subgraph reachable only through a gate appeared genuinely disconnected in the dashboard. This was the root cause of the visible disconnect —design_approval_gatehad no outgoing edges at all.Added a list comprehension that emits one route per
human_gateoption, withwhenset toselection == '<value>'to mirror what the engine emits for execution-plan steps elsewhere in the file.2. Frontend: pre-classify back-edges before passing to Dagre
When a workflow has multiple loop-backs (e.g. revision loops), Dagre's greedy acyclicer reverses arbitrary edges to break cycles, which scrambles rank assignment. Downstream nodes end up above their predecessors and the layout looks like two disconnected columns even when all edges are present.
Added
findBackEdges()ingraph-layout.tsthat runs a DFS from\$startto identify loop-back edges (edges whose target is an ancestor of the source in the DFS tree). Those edges are passed to Dagre in reversed direction so it ranks the underlying DAG correctly. The visible edges in React Flow keep their original direction.This is more accurate than Dagre's greedy heuristic because it knows about the entry point. Traversal order is stable (sorted target IDs) so layout is deterministic across renders.
Files changed
Verification
Out of scope