Repository navigation
refactor(coderd/x/chatd/chatsanitize): drop Anthropic guard escalation passes - #30970
ibetitsmike wants to merge 3 commits into
Conversation
|
@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. Keep it up! 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". |
|
@codex review
|
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
ApplyAnthropicProviderToolGuardnow validates the result of its targeted repair once, then returns it or fails closed. This removes 163 production lines (2 added): the escalation that ran after a failed repair (an all-messages repair, then two strip passes) and its helpersisSafeAnthropicProviderToolPrompt,providerExecutedToolMessageIndexes,stripAnthropicProviderToolHistoryFromMessagesandaddAnthropicProviderToolSanitizationStats, plus their internal tests.git grepfinds no other callers.Why it is safe
The escalation ran but could never repair a prompt.
SanitizeAnthropicProviderToolHistoryloops until nothing is left to remove outside the last assistant message when that message carries signed or redacted reasoning, so after the targeted repair any violation left is in that message. Every later pass skips that message, so the guard failed anyway.chatsanitize,chatloopand chatd Anthropic tests pass in. Outputs and errors matched in 1,000,000 of 1,000,000. 200,020 of 1,000,000 prompts reached the escalation; 0 came out repaired.invalid pair in latest signed assistant fails closedpins the failure path (nil prompt,ErrAnthropicProviderToolPromptUnsafe, log fields). It passes before and after; no other test covered that path.drops provider result without output texttakes over the one deleted assertion no other test made: it kills the mutant that keeps an empty-output provider result as empty text. The log detail cap test stays. All 12 mutants of the code the deleted tests touched fail a remaining test.Review focus
Failures log less: the fallback and strip warnings are gone, and the error log drops
immutable_latest_signed_assistant(always true), no longer says "nuclear strip", and itsmessage_indexvalues point into the prompt the guard validated.Validation
go test ./coderd/x/chatd/chatsanitize/... ./coderd/x/chatd/chatloop/... -count=1,go test ./coderd/x/chatd/ -run 'Anthropic|Guard|Sanitiz' -count=1 -p 4 -parallel 4,go vet,golangci-lintonchatsanitize: pass.Stack
This stack moves the Anthropic guard unit test into
chatsanitize, then deletes the guard escalation that can never repair a prompt.chatsanitize