chore(workflow): audit and polish step-graph and stepper family (CL-8484) - #46
TheGreatAxios wants to merge 2 commits into
Conversation
…484) Add missing story coverage found in the workflow-family audit: a branching (fan-out/fan-in) StepGraph example, and first stories for WorkflowStatusBadge and the step-primitives compound API.
ReviewVerdict: approve. Stories/audit only. Body documents branching layout and status-vocab gaps as not fixed — honest. No CI. No prior reviews. |
Review (Draper + Emil)Verdict: approve. Stories only. Copy is on-voice. Nit: story titled Branching still lays nodes in one line. Rename to what it shows, or don’t call it Branching until layout forks. Not blocking. |
TheGreatAxios
left a comment
There was a problem hiding this comment.
Stories-only chore, fine to merge — but the Branching story documents a real limitation, not just a demo: edges changes which connectors draw but not where nodes sit, so the graph can render a DAG's edges yet can't lay out a DAG. Worth filing as an issue rather than letting a story comment be the only record of the gap.
The graph still lays nodes in one line; edges only change connectors.
Re-reviewStory export Branching → LinearDagEdges. SHA 30d359e. bun test 336 pass. |
|
Closing — this PR's entire surface is gone. It only edited The useful observation in this PR — that |
Audit: workflow family vs AI Elements Task/Stepper (CL-8484)
Scope:
src/ui/step-graph.tsx,step-graph-node.tsx,horizontal-stepper.tsx,step-list.tsx,step-primitives.tsx,workflow-status-badge.tsx,src/lib/chart-geometry.ts,src/lib/step-graph-layout.ts. Compared againstVercel AI Elements' Task/Stepper (ai-sdk.dev/elements) and
agent-elements.21st.dev, used as inspiration only — nothing here is a port.
Branching
StepGraphaccepts an arbitraryedgesprop (anyfrom/topair, not justa sequential chain), but
computeStepGraphLayoutinstep-graph-layout.tspositions nodes purely by array index along one axis. Two steps that could
run in parallel (a fan-out) are still stacked one after another in a straight
line — the connectors can point anywhere, but the geometry can't express two
nodes sharing a "rank." This is strictly less than agent-elements.21st.dev's
workflow view, which lays out branches as parallel columns.
Not fixed here — a rank-based layout is layout-engine work, not polish.
Added a
Branchingstory tostep-graph.stories.tsxto make the currentbehavior (edges draw, layout doesn't fork) visible and testable.
Status vocabulary
Interchange's canonical run vocabulary, as the Workbench consumer uses it
(
workflow-run.ts'sRunStatus, mirrored inapps/web/src/routines-api.tsand
pages/routines-page.tsx's consumption of it):The workflow family currently has four different vocabularies for the
same underlying states, none of which match
RunStatusdirectly:StepGraphNodeStatus(step-graph-node.tsx)pending | running | completed | failedWorkflowStepStatus(workflow-run-progress.ts, used by horizontal-stepper)completed | current | pending | failedStepDisplayStatus(workflow-registry.ts, used by step-list.tsx)pending | active | done | failedStepPhase(workflow-run-progress.ts)pending | in-flight | awaiting-signal | awaiting-timer | completed | failedWorkflowStatusTone(running \| awaiting \| done \| paused \| fail) is adeliberate coarser layer per its own doc comment, which is fine. But none of
the four step-level vocabularies above model
stopped(a cancelled run),and each spells "in progress" and "done" differently for no functional
reason (
running/current/active/in-flight,completed/done). Aconsumer wiring a real run through more than one of these components has to
hand-roll a different mapping from
RunStatusfor each one, with no sharedhelper.
Not fixed here — reconciling four public prop types is a breaking,
cross-cutting change.
Proposed follow-ups (I will file):
RunStatus →mapping helpers (e.g.runStatusToStepPhase,runStatusToWorkflowStepStatus) next toworkflow-run.tsso a Workbenchscreen doesn't reinvent the mapping per component.
glyph, or is intentionally folded into a step's last-known phase.
Density
StepGraphhascompact/expanded;HorizontalStepperhas one densitywith a hard 5-step threshold that switches to number-only pills. The naming
doesn't line up (no
densityprop on the stepper) but the underlyingbehavior is reasonable for each component's use case. Low-value to unify;
not fixed.
Reduced motion
Verified, not a gap:
theme.css's@media (prefers-reduced-motion: reduce)block (theme.css:562) collapses every animation/transition duration to
0.01msglobally, soStatusDot's live pulse andHorizontalStepper'srail-fill transition already still correctly for a reduced-motion viewer.
HorizontalStepperadditionally guards its pulse withmotion-safe:, whichis redundant given the global rule but harmless. No fix needed.
Story coverage
Fixed in this PR:
step-graph.stories.tsx: addedBranching, a fan-out/fan-in edge setover the existing linear step list (see Branching section above for what
it does and doesn't demonstrate).
workflow-status-badge.stories.tsx: new — this component had zerostories. Renders all five
WorkflowStatusTonevalues.step-primitives.stories.tsx: new — this component had zero stories.Renders the compound API's horizontal and vertical orientations.
Checks
bun run typecheck,bun run lint,bun test(336 pass),bun run build(SWC + tsc + Tailwind + contrast gate) all green.
Not merging — for review.