🧮 refactor: Pure Step Reducer Behind useStepHandler - #16289
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5fb4eae0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
a5fb4ea to
389e58d
Compare
Move the content-part primitives (updateContent, content index and edit prefix resolution, step metadata, reasoning label reset) out of useStepHandler into hooks/SSE/steps, and add pure reducers for message, reasoning, agent update and summary events. The hook keeps its refs, message lookup, buffering and cache writes and calls the reducers; its public signature is unchanged. Tool call events follow in the next change.
The inline handler read the phase through optional chaining; keep that in applyMessageDelta so a bare MESSAGE_CREATION step still streams.
389e58d to
9d792ab
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d792ab1e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 0, | ||
| ).message; | ||
|
|
||
| const part = result.content?.[0] as Record<string, unknown>; |
There was a problem hiding this comment.
Replace the untyped content-part assertion
Avoid casting this value to Record<string, unknown>; the repository's type-safety guidance explicitly rejects this pattern because it hides the actual content-part contract. In this test, the broad assertion allows checks for misspelled or removed reasoning fields to keep compiling, so narrow the existing TMessageContentParts union by type or use the corresponding reasoning-part type instead.
AGENTS.md reference: AGENTS.md:L1-L1
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f79a930. The label-reset test now asserts the whole reset part with toStrictEqual({ type: think, think, reasoning_label_step_id }), so there is no cast and any leftover label key, even one set to undefined, fails the test.
Summary
The run-step state machine lives inline in
useStepHandler, a 1,600-line hook whose transitions can only be tested by rendering it with Recoil, Jotai and a message cache. This lifts the content-part transitions into pure functions underclient/src/hooks/SSE/steps/, the second step toward an extractable chat package. Stacked on #16288.steps/content.tsholds the primitives the hook used internally (updateContent, content-index and edit-prefix resolution, step metadata, the reasoning label reset).steps/text.tsadds reducers for message deltas, reasoning deltas, agent updates, and summary open, delta and completion. Each takes the current response message and one event and returns the next message, with no React, Recoil, Jotai or React Query imports.useStepHandlerkeeps its refs, message lookup, delta buffering and cache writes, and calls the reducers; its signature is unchanged. Tool-call events follow in the next PR in this stack.The position invariant is kept: a part's index in
message.contentis its step index plus the retained edit prefix, and no reducer splices. One deliberate difference:updateContentnow copies a part when stamping agent/group metadata instead of mutating the previous message's part object in place.Type of change
Testing
Tested environments/configuration:
lcdev servers on the top of this stack (the tool-call PR, which contains this change), agents endpoint via Anthropicclaude-sonnet-4-6with Run Code enabled (the OpenAI key in my environment returns 401).think, text, tool_call x3, think, tool_call, think, text) with unique tool call ids, and nothing duplicated.Automated tests:
client/src/hooks/SSE/__tests__/steps.spec.ts: recorded sequences per event family, out-of-order steps, redelivered chunks, the edit-prefix fold, failed summaries keeping their slot, and explicit position-invariant assertions.cd client && npx jest hooks/SSE: 10 suites, 372 tests pass;useStepHandler.spec.tsis unchanged.cd client && npm run typecheck: passes.npm run static-checks -- --against origin/canary: passes.Screenshots / recordings
No user-facing change.
Risk / compatibility
Behavior-preserving refactor of the streaming hot path. The diff is 1,599 lines, most of it moved code plus 447 lines of tests.
Checklist