Skip to content

fix(claudecode): include cache tokens and per-model window in [ctx: ~N%]#765

Closed
AaronZ345 wants to merge 1 commit intochenhg5:mainfrom
AaronZ345:fix/claudecode-ctx-indicator
Closed

fix(claudecode): include cache tokens and per-model window in [ctx: ~N%]#765
AaronZ345 wants to merge 1 commit intochenhg5:mainfrom
AaronZ345:fix/claudecode-ctx-indicator

Conversation

@AaronZ345
Copy link
Copy Markdown
Contributor

Two bugs broke the [ctx: ~N%] indicator in the claudecode path:

  1. Cache tokens missinghandleResult read only usage.input_tokens (per-turn delta). In long sessions almost all input is cache_read_input_tokens, so the delta stays <100, sdkPlausible := event.InputTokens >= 100 is false, and the suffix never appears.

  2. Hardcoded 200k windowcontextIndicator divided by const modelContextWindow = 200_000. Users on claude-opus-4-7[1m] saw 5x inflated percentages.

agent/codex already does the equivalent (reads total_token_usage + model_context_window from rollout events). This PR brings claudecode to parity.

Changes

  • core/message.go: add Event.ContextWindow int (additive).
  • agent/claudecode/session.go: new modelContextWindow(model) maps [1m]/-1m → 1M, default 200k. handleResult sums input_tokens + cache_read_input_tokens + cache_creation_input_tokens and stamps the window on the event.
  • core/engine.go: remove the constant, rename to defaultContextWindow (fallback only). contextIndicator(inputTokens, window int) takes window as a parameter.

Tests

  • TestHandleResultAggregatesCacheTokensAnd1MWindow — verifies three-field sum + 1M window for [1m] model.
  • TestModelContextWindow — table test across Opus/Sonnet/Haiku variants, empty string, uppercase.
  • Existing TestHandleResultParsesUsage / TestHandleResultNoUsage unchanged and green.
  • go build ./... clean.

Related: #547

Copy link
Copy Markdown
Owner

@chenhg5 chenhg5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Fixes two real bugs in the [ctx: ~N%] indicator for Claude Code sessions. Well-tested and consistent with the existing codex implementation.


Good:

  • Correctly aggregates input_tokens + cache_read_input_tokens + cache_creation_input_tokens — matches how Anthropic bills context
  • modelContextWindow() cleanly detects [1m]/-1m suffix (case-insensitive)
  • ContextWindow field on Event is a clean extension — no breaking changes
  • contextIndicator() gracefully falls back to defaultContextWindow when window <= 0
  • Two focused test additions: cache aggregation test and 7-case model window table test
  • Consistent with agent/codex which already does the equivalent

LGTM. Both bugs verified, tests comprehensive, CI green.

Two bugs broke the context indicator in the claudecode path:

1. handleResult read only usage.input_tokens — the per-turn delta.
   In long sessions almost all input is cache_read, so the delta stays
   <100, sdkPlausible is false, and the [ctx: ~N%] suffix never appears.

2. contextIndicator divided by a hardcoded modelContextWindow = 200_000.
   Users on claude-opus-4-7[1m] saw 5x inflated percentages (e.g. 10%
   actual usage rendered as 50%).

Fix:
- handleResult now sums input_tokens + cache_read_input_tokens +
  cache_creation_input_tokens and stamps ContextWindow on the event
  via modelContextWindow(cs.model): "[1m]" / "-1m" suffix → 1_000_000,
  everything else → 200_000.
- Event gains a ContextWindow int field (additive, no break).
  contextIndicator takes window as an argument; the old constant becomes
  defaultContextWindow, used only as a fallback.

agent/codex already does the equivalent (reads total_token_usage and
model_context_window from rollout events). This brings the claudecode
path to parity.
@AaronZ345
Copy link
Copy Markdown
Contributor Author

Closing — #774 subsumes both fixes from this PR:

Thanks @Cigarrr for the catch.

@AaronZ345 AaronZ345 closed this Apr 26, 2026
@AaronZ345 AaronZ345 deleted the fix/claudecode-ctx-indicator branch April 26, 2026 06:31
@AaronZ345
Copy link
Copy Markdown
Contributor Author

Closing — superseded by #774. The per-model context window fix ([1m] → 1M) is preserved in #774's claudeContextWindow(), and the cache-token summing into ctx % was the wrong fix (see #774 review thread for the result.usage aggregation issue and the codex LastTokenUsage precedent). Thanks @Cigarrr for the careful catch.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants