Repository navigation
🤖 fix: send each native tool_reference once per request - #5935
Conversation
A tool_catalog_search result referenced every matched deferred tool even when an earlier result in the same request already loaded it, and Anthropic expands every tool_reference into the full definition again (#5413). dedupeNativeToolReferences is a pure, idempotent, request-only projection over the final per-request messages (history pipeline + per-step transforms + replay mirror): repeats are dropped in transcript order, an all-repeat result becomes a deterministic text result, and persisted results keep their raw matches so a compacted prefix that dropped the first reference re-references later.
A continuous-compaction prefix swap replaces the prefix the dedupe projection is decided against, and the SDK accumulates later step inputs from prepareStep's returned messages, so deduping inside transformStepMessages (pre-swap) permanently lost a retained tail's only surviving reference. Move the per-step dedupe after the swap block (and apply it to the rebuilt first step and the settled-step budget floor), mirror it last in the replay builder, and guard the ordering with a live-swap red-green test.
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: 1e7f03c181
ℹ️ 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".
The reference dedupe is a request-only projection, but the AI SDK accumulates the next step's input from the returned (projected) messages. A later prefix swap that dropped a reference's first occurrence could not fall back to the retained duplicate because the carried copy was already projected to text, unloading the tool for the rest of the turn. Keep the raw tool-search results seen by this run's steps in a per-run map (first sighting is raw: results enter the step messages when their tool executes and are only projected afterwards) and feed the swap from the restored transcript; the existing post-swap dedupe re-projects whatever is still a repeat. Codex P1 on #5935.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a3204c92e
ℹ️ 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".
Round 2 Codex findings on the dedupe placement: - Pipeline: dedupe ran before transformModelMessages, whose orphan strip (self-healing for interrupted/corrupted history) can remove a result whose reference consumed a tool's dedupe slot, unloading the tool from the final request. Dedupe now runs after the transforms, before cache control. - Swap restore: the per-run raw-result store was keyed by toolCallId, which providers can reuse across turns and steps, so restoration could replace an unrelated retained result (the fixture's own bash result exhibits the collision). Dedupe now reports each projection it makes and the swap path restores by matching the serialized projected part, chain-compressed to the fully raw form. Codex P2 x2 round 2 on #5935.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55e524e941
ℹ️ 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".
Round 3 Codex findings showed the per-run projection store leaks edge cases by construction: byte-identical projections under a reused toolCallId collide in the map, and fallback/retry streams allocate a fresh closure that cannot restore projections carried from before the restart. Remove the store entirely. The dedupe now replaces each duplicate tool_reference with a deterministic text marker naming the tool, and the swap path inverts markers back to raw references with a pure function of the messages (unprojectNativeToolReferences) before re-projecting against the swapped transcript. Restoration is per occurrence and stateless, so ID collisions and stream restarts cannot lose it. Codex P2 x2 round 3 on #5935.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa97dc3872
ℹ️ 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".
Anthropic rejects a tool result mixing text with tool_reference blocks, so the invertible markers may only appear in fully repeated results (text-only output). A mixed result drops its repeats instead; the raw form stays recoverable from persisted history on the next turn. Also move the marker unprojection into swapPrefix itself so every swap site inverts identically - the consumed-swap fallback rebuild called swapPrefix directly and would have left a retained marker without its reference. Codex P1+P2 round 4 on #5935.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c96d3da34
ℹ️ 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".
… byte-exactly The marker scheme could not invert a mixed result: the dropped repeat left no trace beside the retained references (text may not mix with them), so a prefix swap that dropped the repeat's anchor lost the reference. Stash the raw output on the projected part under providerOptions.mux instead; markers are gone, a fully repeated result returns to the deterministic already-loaded text, and unprojectNativeToolReferences restores the stash byte-exactly for both full and mixed projections.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3847486d7a
ℹ️ 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".
Review-loop checkpoint — pausing for human directionPer the repo's early-stop rule (loop not converging), I'm pausing instead of pushing a round-7 fix. Findings per round: r1: 1, r2: 2, r3: 2, r4: 2, r5: 2, r6: 4 — all six rounds in the same dedupe/inversion area, across three successive inversion mechanisms (per-run restore map → text markers → raw-output stash). Each redesign fixed the prior class and spawned new subsystem interactions. Round 6 findings, verified by execution:
Options:
State: UAT PASS @1e7f03c181, r1–r5 threads all resolved with fixes, r6 threads open, CI red on head per finding 1. Goal paused; say the word and I resume with (a), (b), or (c). |
…rites - measureBudgetContent skips providerOptions.mux.rawToolSearchOutput on tool-result parts: the stash never reaches the wire, so it must not inflate assembled-request estimates toward the hard ceiling. - addCacheControlToLastContentPart merges cache options into existing providerOptions instead of replacing them, so the restoration stash survives cache application on fallback paths. - requestShaping mixed-result expectation now includes the stash. - Projection tests assert structure (text output, dropped names, stash bytes, idempotence) instead of the exact generated sentence.
Summary
In Anthropic native tool-search mode, every
tool_catalog_searchresult emitstool_referenceblocks for all matched deferred tools, and Anthropic expands every reference into the full definition, repeats included (#5413). A later search re-matching an already-loaded tool re-paid its whole schema on every subsequent request (observed: ~2K tokens per repeat; +87 to +521 tokens per repeated tool in the #5413 grid). #5416 fixed the budget accounting only.This adds
dedupeNativeToolReferences, a pure, idempotent, request-only projection over the final per-request messages: repeats are dropped in transcript order. A fully repeated result becomes a deterministic "All matched tools are already loaded: …" text result; a mixed result drops only its repeats, since Anthropic rejects a tool result mixing text with references. Every projected part stashes its raw output underproviderOptions.mux(adapters only serialize their own namespace, so it never reaches the wire), which makes the projection byte-exactly invertible for both shapes. Untouched messages keep their identity so exact-append budget anchors and the cached prefix stay intact. Persisted results keep their raw matches, so when compaction drops the first referencing result, a later result re-references the tool by construction. Scoped (activeTools) mode is unchanged;countToolReferencesstays as the budget backstop.Implementation
Applied wherever final per-request messages are produced, so live and replayed requests are byte-identical by construction:
messagePipeline(after the orphan-strip transforms), StreamManager'sprepareStep, and the replay request builder's per-step mirror. InprepareStepthe dedupe runs after the continuous-compaction prefix swap, andswapPrefixitself restores stashed raw outputs in the retained tail, so every swap site (per-step and the consumed-swap fallback rebuild) re-decides references against the swapped transcript with no per-run restoration state.Note: existing conversations pay a one-time cache-prefix miss after deploy (repeat references disappear from replayed history).
Validation
tsgo --noEmit, eslint, prettier clean; toolCatalog, streamManager (requestShaping, continuousCompaction, errorRecovery, contextBudget), aiService, messagePipeline, replayVerify/fixture/cacheAudit suites green. No live count_tokens harness exists in-repo; token effect not measured against the provider.Delivery record
Generated with
xum• Model:anthropic:claude-fable-5• Thinking:high• Cost:$121.28