Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Jul 11, 2025

Description

This PR fixes the token counting discrepancy where the UI was displaying inflated context token counts (e.g., ~170k instead of ~44k) because cache tokens were being included in the context window calculation.

Problem

The function was calculating as:

contextTokens = tokensIn + tokensOut + cacheWrites + cacheReads

However, cache tokens do not consume actual context window space - they are a caching optimization. Including them in the context window display was misleading users about how much of their context window was actually being used.

Solution

Modified the calculation to only include actual context tokens:

contextTokens = tokensIn + tokensOut

Changes

  • Modified src/shared/getApiMetrics.ts to exclude cache tokens from the contextTokens calculation
  • Updated all related tests in src/shared/__tests__/getApiMetrics.spec.ts to reflect the new behavior
  • All tests now pass with the corrected calculation

Testing

  • ✅ All unit tests pass
  • ✅ Linting passes
  • ✅ Type checking passes

Impact

This change only affects the UI display of context tokens. It does not impact:

  • Cost calculations (cache tokens are still tracked separately)
  • API usage metrics
  • Context management logic (sliding window, condensing)

The fix ensures users see an accurate representation of their actual context window usage.


Important

Fixes token counting in getApiMetrics.ts by excluding cache tokens for OpenAI protocol and updates related tests.

  • Behavior:
    • Fixes token counting in getApiMetrics.ts by excluding cache tokens from contextTokens for OpenAI protocol.
    • Updates getApiMetrics() to calculate contextTokens based on apiProtocol.
  • Tests:
    • Updates tests in getApiMetrics.spec.ts to reflect new contextTokens calculation logic.
  • Misc:
    • Adds apiProtocol handling in Task.ts to determine protocol based on provider.
    • Adds getApiProtocol() function in provider-settings.ts to determine API protocol.

This description was created by Ellipsis for 62c9d5d. You can customize this summary. It will automatically update as commits are pushed.

- Modified getApiMetrics.ts to calculate contextTokens as tokensIn + tokensOut only
- Updated tests to reflect the new calculation behavior
- Cache tokens (cacheWrites and cacheReads) no longer count towards context window
- Fixes inflated context token display in UI that was showing ~170k instead of ~44k
@delve-auditor
Copy link

delve-auditor bot commented Jul 11, 2025

No security or compliance issues detected. Reviewed everything up to 62c9d5d.

Security Overview
  • 🔎 Scanned files: 6 changed file(s)
Detected Code Changes
Change Type Relevant files
Enhancement ► provider-settings.ts
    Add API protocol helpers and Anthropic style provider constants
► message.ts
    Add apiProtocol field to schema
► Task.ts
    Implement API protocol handling in messages
► ExtensionMessage.ts
    Add apiProtocol type definition
Bug Fix ► getApiMetrics.ts
    Update context tokens calculation based on API protocol
► getApiMetrics.spec.ts
    Update tests for API protocol logic
Refactor ► runEvals.ts
    Simplify log directory handling
► runTask.ts
    Streamline logging configuration
Configuration changes ► docker-compose.yml
    Update ports and container configuration
► global-settings.ts
    Update announcement ID

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@daniel-lxs daniel-lxs marked this pull request as ready for review July 11, 2025 18:43
@daniel-lxs daniel-lxs requested review from cte, jr and mrubens as code owners July 11, 2025 18:43
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Jul 11, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 11, 2025
@mrubens
Copy link
Collaborator

mrubens commented Jul 11, 2025

Hmm, I'm confused - I thought that these were additive, at least for some providers. Maybe we're just handling openrouter incorrectly now since we added cached tokens to it?

- Modified claude-code.ts to yield incremental token usage after each assistant message
- This ensures the UI receives real-time token updates during streaming
- Prevents accumulation issues in Task.ts which was causing incorrect totals
- Fixes #5600 where assistant messages showed incorrect low token counts
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Jul 11, 2025
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@daniel-lxs daniel-lxs marked this pull request as draft July 11, 2025 20:21
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 11, 2025
@daniel-lxs daniel-lxs marked this pull request as ready for review July 11, 2025 20:36
@dosubot dosubot bot added the UI/UX UI/UX related or focused label Jul 11, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Jul 11, 2025
@mrubens mrubens merged commit 0ecae9d into main Jul 11, 2025
11 checks passed
@mrubens mrubens deleted the fix/exclude-cache-tokens-from-context-window branch July 11, 2025 20:54
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 11, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jul 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files. UI/UX UI/UX related or focused

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants