fix(agent): stop double-counting api_content in the token estimator - #75892
Merged
kshitijk4poor merged 2 commits intoAug 1, 2026
Merged
Conversation
`api_content` is a SUBSTITUTE for `content`, not an addition to it. `turn_context.substitute_api_content()` pops the sidecar and overwrites `content` at every API-bound message-build site (the `api_messages` build in `conversation_loop`, the max-iterations summary in `chat_completion_helpers`, the chat-completions transport), so exactly one of the two is ever sent to the provider. The preflight estimator counted both, because both `_estimate_message_chars` and `_estimate_message_tokens_without_images` walked every key of the persisted dict with a single-entry denylist (`_anthropic_content_blocks`). Any message whose sidecar differs from its clean stored content was counted twice — exactly 2.00x on a 40KB sidecar. The sidecar exists to keep the provider prompt-cache prefix byte-stable, so it is written on precisely the long, cache-pinned messages where the doubling hurts most. Because `estimate_messages_tokens_rough()` also feeds the compaction threshold via `context_compressor` and `conversation_loop`, the inflated estimate makes compression fire on phantom bytes. Fix: substitute rather than sum, mirroring the wire. The two estimator helpers had drifted into near-identical copies of the same shadow-building loop, so this factors the shared logic into `_wire_message_shadow()` and fixes the class once instead of patching one site and leaving the other. Image accounting is unchanged: base64 payloads are still replaced with a placeholder and charged at the flat `_count_image_tokens` rate, and the `_multimodal` text_summary path is preserved. Tests: three cases in `TestEstimateMessagesTokensRough` — sidecar equal to content is counted once, a sidecar that DIFFERS is still counted (a lower bound, so it fails if the field were dropped rather than substituted, which would undercount the real request), and a sidecar cannot smuggle raw base64 past the flat image rate. Verified on Linux (Python 3.11): 53 passed in tests/agent/test_model_metadata.py, 57 passed with tests/agent/test_context_breakdown.py, 656 passed / 3 skipped across the compression/context/token/estimate/prune surface of tests/agent. Mutation-tested: reverting the substitution fails the new equality test. `scripts/check-windows-footguns.py` is not applicable — no file I/O, process management, terminal handling, subprocesses, or signals.
…adow Review follow-up on NousResearch#75102. The shadow substituted the sidecar whenever the ``api_content`` key was merely PRESENT, but the wire only substitutes a non-empty string sidecar on a user/assistant row (see ``turn_context.substitute_api_content``). For any other shape the sidecar is popped and discarded while the clean ``content`` is sent -- so the shadow dropped real content from the estimate and UNDERcounted, the dangerous direction: compaction fires too late and the turn dies on a hard context-length error instead of merely compressing early. Gate the substitution on the same predicate, and cover the divergent shapes (None, empty string, int, list, non-user/assistant role) with a test that fails against the unconditional version. Also rename the image test: it never carried a sidecar, so it was not testing what its name claimed. It is a non-regression pin on the flat per-image accounting that moved into ``_wire_message_shadow()``, and is now named for that.
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
2 participants
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.
Summary
Stops the preflight token estimator from double-counting
api_content— the sidecar is a substitute forcontent, not an addition, so counting both inflated estimates 2.00x on cache-pinned messages and made compaction fire on bytes never sent to the provider.Changes
agent/model_metadata.py: factors shared shadow-building logic from_estimate_message_charsand_estimate_message_tokens_without_imagesinto_wire_message_shadow(), which substitutesapi_contentforcontentmirroringturn_context.substitute_api_content()'s guard exactly (non-empty string sidecar on user/assistant row only)tests/agent/test_model_metadata.py: 4 new tests covering substitution, lower-bound on sidecar counting, non-string/role guard, and image stripping survivalValidation
Salvage of #75102 by @israellot. Cherry-picked both commits with authorship preserved.
Closes #75102