Skip to content

refactor(flow): let each step kind own how it reads - #744

Draft
latekvo wants to merge 1 commit into
feat/concurrent-flow-recordingsfrom
refactor/flow-step-summary-definitions
Draft

refactor(flow): let each step kind own how it reads#744
latekvo wants to merge 1 commit into
feat/concurrent-flow-recordingsfrom
refactor/flow-step-summary-definitions

Conversation

@latekvo

@latekvo latekvo commented Aug 6, 2026

Copy link
Copy Markdown
Member

Stacked on #574.

Summary

summarizeSteps (flow-finish-recording.ts) was a 50-line switch over FlowStep["kind"]. stepTarget (flow-run.ts) was a second one, over the same union, with its own selector and condition spellings. Neither could be found from the other, and summarizeSteps' case "tool": default: arm meant a step kind added without wording rendered as a tool: step instead of failing.

Both surfaces now read from one definition per kind — the same shape as a tool owning its own log wording in ToolDefinition.interaction (#582):

const FLOW_STEP_DEFINITIONS: {
  [K in FlowStep["kind"]]: FlowStepDefinition<Extract<FlowStep, { kind: K }>>;
} = {
  "scroll-to": {
    summary: (step) => `${yamlSelectorLabel(step.target)} (${step.direction})`,
    target: (step) => { ... },
  },
  ...
};

Keying the record by FlowStep["kind"] is what replaces the default arm: a new kind is a compile error until it says how it reads on both surfaces. That also retires the renderToolArgs interpolation guard, which existed only because the default arm could hand it a step with no args field.

flow-run.ts sheds 98 lines and six imports; flow-finish-recording.ts sheds 118 and is now just the tool. The two shared label templates (conditionLabel parameterized by selector spelling, and a new whenLabel for the platform-vs-UI guard arm) mean the recorder's when: tail and the runner's no longer duplicate the platform arm.

Test

Rendering is unchanged — every summary line and report target is byte-for-byte what the switches produced.

  • Proved with a throwaway equivalence test that inlined both pre-refactor switches verbatim and compared them against the new definitions over 1264 generated steps: all 15 kinds, 10 selector shapes (loose, regex, scoped, quote-bearing), every wait condition × text-match mode × expectation, every scroll direction, and the cyclic-args path. Identical on all of them. Mutating two renderings (equals comparator, default scroll direction) failed it, so it discriminates.
  • Replaced by packages/tool-server/test/flows/flow-step-definitions.test.ts, kept: 47 assertions pinning the exact summary line and report target of every kind, in a table keyed by FlowStep["kind"] so a new kind fails typecheck:tests here too (verified by deleting the wait entry: TS1360). Deleting a kind from the definitions record fails the build the same way (TS2741).
  • npm run build, eslint --max-warnings 0, prettier --check ., npm run typecheck:tests --workspaces: clean.
  • packages/tool-server: 311 files / 3596 tests passed. packages/argent-cli: 24 files / 461 tests passed.

Note this PR targets feat/concurrent-flow-recordings, and the Unit Tests workflow only runs on PRs into main — the runs above are the signal until #574 lands.

The recorder's summary and the runner's report target were two switches over
FlowStep["kind"], in two files, each with its own selector and condition
spellings — a new step kind could get wording on one surface and silently fall
through the other's default arm.

Both now come from one definition per kind, keyed by FlowStep["kind"] so a new
kind is a compile error until it says how it reads. Same shape as a tool owning
its own log wording in ToolDefinition.interaction.

Rendering is unchanged: every summary line and report target is byte-for-byte
what the switches produced.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant