Skip to content

fix: gate token-based compaction on latest prompt size, not event sum#474

Open
phillip-hicks-siro wants to merge 1 commit into
google:mainfrom
Siro-ai:fix/token-trigger-latest-prompt-main
Open

fix: gate token-based compaction on latest prompt size, not event sum#474
phillip-hicks-siro wants to merge 1 commit into
google:mainfrom
Siro-ai:fix/token-trigger-latest-prompt-main

Conversation

@phillip-hicks-siro

Copy link
Copy Markdown

Fixes #473.

TokenBasedContextCompactor.shouldCompact summed usageMetadata.promptTokenCount across active events. Each of those values is the full prompt size of the call that produced it (system instruction + tools + entire history), so the sum re-counts the same history once per event and grows with call count rather than context size — and after a compaction, retained tail events still carry pre-compaction counts, leaving the sum permanently above any threshold (compact-every-call thrashing). Details and production impact in #473.

This ports the Python ADK's semantics (apps/compaction.py::_latest_prompt_token_count):

  • use the most recently observed promptTokenCount — the measured size of the last real request
  • fall back to a chars/4 estimate over the effective request contents (getContents, mirroring the content request processor) when no usage metadata is available

compact() and retention behavior are unchanged.

Testing: rewrote the threshold unit test against the new semantics and added: a latest-not-sum regression case, newest-metadata-wins, estimate fallback (over and under threshold), and the no-signal case. Full unit:core suite passes (954 tests).

@google-cla

google-cla Bot commented Jul 6, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@phillip-hicks-siro

Copy link
Copy Markdown
Author

@googlebot I signed it!

@phillip-hicks-siro phillip-hicks-siro force-pushed the fix/token-trigger-latest-prompt-main branch from d21d8f4 to cfa5513 Compare July 6, 2026 21:13
@Varun-S10 Varun-S10 self-assigned this Jul 7, 2026
@Varun-S10

Copy link
Copy Markdown
Contributor

Hi @kalenkevich, I have checked the issue and validated the proposed changes. I was able to reproduce the bug. Could you please review this PR?

@Varun-S10 Varun-S10 added the needs review [Status] The PR/issue is awaiting review from the maintainer label Jul 7, 2026
@Varun-S10

Copy link
Copy Markdown
Contributor

Hi @phillip-hicks-siro, We noticed that a few of the pre-checks are failing on this PR. Could you please resolve them?

@Varun-S10 Varun-S10 added request clarification [Status] The maintainer need clarification or more information from the author and removed needs review [Status] The PR/issue is awaiting review from the maintainer labels Jul 8, 2026
shouldCompact previously summed usageMetadata.promptTokenCount across
all active events. Each of those values is the full prompt size of the
call that produced it, so the sum re-counts the same history once per
event and grows with call count rather than context size: sessions
cross any threshold after a handful of calls regardless of actual
context pressure, and after a compaction the retained events still
carry pre-compaction counts, leaving the sum permanently above the
threshold (compact-every-call thrashing).

Port the Python ADK semantics (apps/compaction.py): use the most
recently observed promptTokenCount - the measured size of the last
request, which already includes system instruction, tools, and full
history - and fall back to a chars/4 estimate over the effective
request contents (getContents) when no usage metadata is available.

Fixes google#473
@phillip-hicks-siro phillip-hicks-siro force-pushed the fix/token-trigger-latest-prompt-main branch from cfa5513 to ad6f98f Compare July 8, 2026 13:33
@phillip-hicks-siro

Copy link
Copy Markdown
Author

@Varun-S10 Pre-check error resolved!

@phillip-hicks-siro

Copy link
Copy Markdown
Author

The windows-latest failure is unrelated to this change: tests/integration/a2a/basic/a2a_agent_test.ts failed at suite setup ("CLI exited prematurely with code 1" spawning the A2A server child process, test_case_utils.ts:332) before any test ran. The same suite passed on ubuntu and macos in this run, all 2307 individual tests passed on windows, and this PR touches only the token-based compactor and its tests. Could a maintainer re-run the windows job? Happy to rebase instead if preferred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

request clarification [Status] The maintainer need clarification or more information from the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TokenBasedContextCompactor sums promptTokenCount across events, triggering compaction on call count instead of context size

2 participants