🧩 refactor: Resolve Tool-Card State Once

[danny-avila]

Summary

Closes the structural issue behind three review cycles' worth of findings (AI-1810).

Each tool card derived its state several times over: the visible label from one expression, the aria-live announcement from another, the icon and shimmer from a third — and since #14906, the follow-scroll from a fourth. Nothing tied them together. They agreed only because each was written to agree, and they drifted the moment a new input arrived.

The evidence is unusually direct. 13 of the 17 Codex findings on #14873 were one derivation updated and another left behind. #14892 added more, including a duration rendering beside "failed". And in #14892's own review, the one place that did have a single derivation — label and announcement sharing an expression — produced zero findings, and its bug fix was one line instead of seven.

The change

resolveToolCallPhase is the single source of truth: one pure function returning running | completed | cancelled | failed, encoding precedence rules that were each established by a specific past finding —

• an explicit status is authoritative and never gated on output parsing;
• explicit cancellation outranks a failure-shaped result;
• under the legacy heuristic the opposite holds, because "not submitting and not finished" is also satisfied by a genuine failure;
• a closed step is never running, whatever progress says.

Those two opposite precedence rules have been collapsed into one another twice under review, so they are pinned side by side in a test.

ProgressText now takes phase instead of the error + errorSuffix pair. That pair encoded three terminal states in two booleans — error meant cancelled, a present errorSuffix meant failed — and left every consumer to reconstruct the distinction. It is exactly the shape that produced the round-1 finding on #14892.

Two deletions that fell out

Both are dead code the consolidation exposed, not behaviour removed:

• progress left ProgressText entirely. The phase already carries everything it decided. Eight call sites stop passing it.
• The useProgress mask went with it. Passing 1 in still matters (it stops the 200ms interval); masking the output no longer does, because the phase treats an explicit close as terminal outright. The "both halves are load-bearing" subtlety documented in 🧩 refactor: Extend Step Status To Remaining Cards; Separate Cancelled From Failed #14873 is now one half.

Scope

The nine cards rendering the shared ProgressText. The three with bespoke layouts — WebSearch, SubagentCall, OpenAIImageGen — still resolve their own state and are the natural follow-up; they can adopt the resolver without adopting the component, which is the point of keeping the resolver a plain function.

CodeAnalyze is the legacy assistants-endpoint card: it never receives run-step metadata, so it genuinely has two states and maps them directly rather than through the resolver, whose heuristic needs signals it does not have. Its announcement and icon now read that same value.

Behaviour

Refactor-only, with one alignment worth naming: FileAuthoringCall, BashCall and ExecuteCode's follow-scroll used progress < 1 && !cancelled, missing the !hasError term its neighbouring icon expression had. It now reads phase === 'running' like everything else. No user-visible change today (a closed failed step already has progress >= 1), but it is the divergence AI-1810 was filed about, so it is unified rather than preserved.

Testing

• client/src/utils/__tests__/toolCallPhase.spec.ts — 12 new tests over the resolver: every explicit-status branch, every legacy-heuristic branch, both precedence directions, and a test asserting the two opposite rules side by side so a future edit cannot quietly unify them.
• Existing suites are the regression net, unchanged in intent: 4891/4891 client tests pass across 408 suites, including the OpenAIImageGen and card suites that encode the cancelled/failed precedence in both directions.
• Two test doubles were updated to the new contract rather than worked around: the ProgressText mocks re-implemented the old prop logic, and FileAuthoringCall's useToolCallState mock used the old positional signature.
• npx tsc --noEmit clean over the touched tree; eslint and import-sort clean on all 16 files.

Change Type

• Refactor (no functional changes)

Checklist

• My code adheres to this project's style guidelines
• I have performed a self-review of my own code
• I have commented in any complex areas of my code
• My changes do not introduce new warnings
• Local unit tests pass with my changes

🤖 Generated with Claude Code

https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5

---

Generated by Claude Code

[danny-avila]

@codex review

Worth a reviewer's attention specifically:

• The precedence inversion is deliberate. An explicit cancelled close outranks a failure-shaped result, but under the legacy heuristic failure outranks cancellation. That looks like an inconsistency and is not — the heuristic infers a stop from "not submitting and not finished", which a genuine failure also satisfies. Collapsing the two has already been done twice under review; there is a test pinning them side by side.
• ProgressText lost its progress prop. It became dead once phase drove every branch. If you see a reason to keep it, say so — the alternative is that a future consumer passes a progress that disagrees with the phase beside it.
• The useProgress mask was removed, not overlooked. 🧩 refactor: Extend Step Status To Remaining Cards; Separate Cancelled From Failed #14873 established that both halves were load-bearing. Passing 1 in still is; masking the output is not, because the phase treats an explicit close as terminal directly. That reasoning is the thing to check hardest.
• Two test doubles were rewritten, which normally deserves suspicion in a refactor. Both re-implemented the component's old logic (progress < 1 ? … : … plus an errorSuffix string) rather than exercising it, so they had to move to the new contract.

Scope note: WebSearch, SubagentCall and OpenAIImageGen still resolve their own state. That is deliberate for this PR — they have bespoke layouts and can adopt the resolver without adopting the component — but it does mean AI-1810 is not fully closed until they follow.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 702b43901a

ℹ️ 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".

[chatgpt-codex-connector]

Avoid inferring cancellation from animated progress

For a live legacy call without runStepStatus, the final SSE update sets the reported progress to 1, but useProgress deliberately continues returning its previous value and then 0.99 for roughly 200 ms. If submission ends during that window, this condition classifies the successful call as cancelled; both ToolCall and RetrievalCall now pass that animated value here, whereas their previous cancellation predicates used initialProgress and therefore recognized the final update immediately. This can briefly render and announce “Cancelled” for a successfully completed tool call, so the heuristic needs to distinguish reported completion from display animation.

Useful? React with 👍 / 👎.

[danny-avila]

Confirmed and fixed in a74e9ad. Verified against the hook rather than taken on trust — useProgress does exactly what you describe:

} else if (initialProgress >= 1 && progress < 1) {
  setProgress(0.99);
  timeout = setTimeout(() => { setProgress(1); }, 200);
}

The input is now split: reportedProgress is what the stream said and drives the cancellation inference; displayProgress is the animated value and drives running vs completed, so the label and shimmer still follow the animation instead of snapping. Both are required, so a future caller has to decide which one they mean.

One correction to the framing, in my favour and against it. You're right that ToolCall and RetrievalCall were immune and my change exposed them — but useToolCallState already had this bug: its predicate read rawProgress, not initialProgress. So every card the hook backs (BashCall, ExecuteCode, ReadFileCall, SkillCall, FileAuthoringCall) could already flash "Cancelled" on a successful call before this PR. Consolidating is what made the divergence visible; the fix now covers all of them.

Three tests pin the window: reported-complete mid-settle is running, genuinely unfinished is still cancelled, and it settles to completed with no cancelled frame in between. 4894/4894 client tests pass.

---

Generated by Claude Code

[danny-avila]

@codex review

Round 1 addressed in a74e9ad, replied inline. Head is a74e9ad.

The finding was correct and the fix is a split input: reportedProgress (what the stream said) drives the cancellation inference, displayProgress (the animated useProgress value) drives running vs completed. Both required, so the distinction can't be skipped by a future caller.

Worth flagging because it sharpens the PR's own argument: useToolCallState already had this bug before the refactor — it inferred from rawProgress, so all five cards it backs could already flash "Cancelled" on a successful call. ToolCall and RetrievalCall were the immune ones. Consolidating is what surfaced the divergence, and the single resolver is why one fix now covers all of them instead of seven.

Also note the earlier Tests: Ubuntu (shard 1/2) red on 702b439 was infrastructure, not code: the job died in setup with HTTP 429 fetching actions/download-artifact and never ran a test. I re-ran the failed jobs; this push supersedes that run anyway.

4894/4894 client tests across 408 suites; typecheck, eslint and import-sort clean.

---

Generated by Claude Code

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: a74e9ad18b

ℹ️ 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".