Repository navigation
refactor(coderd/x/chatd/chatstate): share one outstanding tool-call scan - #30883
Open
ibetitsmike wants to merge 3 commits into
Open
ibetitsmike wants to merge 3 commits into
ibetitsmike wants to merge 3 commits into
Conversation
Cancellation synthesis and the pending-call checks each repeated the last-assistant lookup, tool-result tracking, and eligibility rules. outstandingToolCalls now owns that scan; its callers only build cancellation messages or the pending-call map.
…rom call policy lastAssistantTurn loads the last assistant message and the tool calls already answered; outstandingToolCalls keeps the eligibility rules. Test fixtures share one chatMessage builder, and the waiting-state cancellation test now covers provider-executed, answered, and unparsable cases and part order.
CancelRequiresAction from A1 now runs on a seed with one dynamic and one non-dynamic outstanding call, so dropping the dynamic filter fails the transition matrix. Also tighten the outstandingToolCalls doc and build the unparsable test row directly.
Collaborator
Author
|
@codex review |
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. |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
ibetitsmike
marked this pull request as ready for review
October 16, 2026 14:08
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cancellation synthesis and the pending-call checks in
chatstateeach repeated the same scan of the last assistant message. They looked up the message, parsed it, tracked answered calls, and applied the same exclusion rules. This PR keeps one copy of that scan.lastAssistantTurnloads the last assistant message's parts and the set of tool calls already answered, either inside that message (provider-executed results) or by a tool message after it.outstandingToolCallsowns eligibility: tool calls only, no provider-executed calls, no answered calls, and optionally only the chat's dynamic tools. It keeps the no-read fast path when a dynamic-only scan finds no dynamic tools.synthesizePendingToolCancellationsandpendingDynamicToolCallIDsnow only build cancellation messages or the pending-call map.pendingAllToolCallIDsandoutstandingToolCallIDsare deleted;FinishInterruptioncallsoutstandingToolCallsdirectly.Behavior is unchanged except for error text on the pending-call paths, which now uses the cancellation path's wording:
get last assistant:becomesget last assistant message:,parse assistant:becomesparse assistant message:, and the dynamic-tools parse error gains aparse dynamic tool names:prefix. Nothing in the repo matches on these strings.synthesizePendingToolCancellations, cc 23, 62% covered)outstandingToolCalls, cc 11, 89.5% covered)The diff is +139/-227 across production and tests. Test fixtures now share one
chatMessagebuilder.Testing
CancelRequiresActionmatrix case now seeds one dynamic and one non-dynamic call. Breaking any of those behaviors or the dynamic filter fails a test, and the new assertions also pass on the old code.go test ./coderd/x/chatd/...,go test -race ./coderd/x/chatd/chatstate,golangci-lintonchatstate(0 issues), andmake pre-commitpass.Review record