Skip to content

fix(insights): show cache tokens in /insights token breakdown - #18632

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/insights-token-cache-breakdown
Open

fix(insights): show cache tokens in /insights token breakdown#18632
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/insights-token-cache-breakdown

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented May 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

The /insights report's total_tokens includes cache_read_tokens + cache_write_tokens, but the (in: / out: ) breakdown only showed input and output tokens. For Anthropic users with prompt caching enabled, cache tokens can dominate the total (e.g., 5.2M total but only 2.6K input / 88K output), making the in/out numbers appear swapped or wrong.

Root Cause

In agent/insights.py, the _compute_overview() method correctly sums all four token categories into total_tokens:

total_tokens = total_input + total_output + total_cache_read + total_cache_write

But both display methods (format_terminal and format_gateway) only showed input and output:

Tokens: 5,239,805 (in: 2,642 / out: 88,463)   # 5.15M cache tokens hidden!

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • See commit messages for detailed changes

How to Test

  1. Run pytest tests/ -q — all tests should pass
  2. Verify the specific scenario described above is resolved

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture and workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A

The /insights report total_tokens includes cache_read_tokens +
cache_write_tokens, but the (in: / out: ) breakdown only showed
input and output tokens. For Anthropic users with prompt caching,
the cache tokens dominate the total, making the in/out numbers
appear wrong or swapped.

- Terminal format: add a 'Cache tokens' line when cache > 0
- Gateway format: append '/ cache: N' to the token breakdown
- Both formats hide cache when all values are 0 (no noise)
- Add 5 regression tests for cache display behavior

Closes NousResearch#18615
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels May 2, 2026
Cyrene963 pushed a commit to Cyrene963/hermes-agent that referenced this pull request May 3, 2026
Community PRs applied:
- NousResearch#18596: Enable secret redaction by default (SECURITY)
- NousResearch#18650: Sanitize malformed tool messages + auto-recover on API 400
- NousResearch#18607: Emergency compression before max_iterations exhaustion
- NousResearch#18603: Compression fallback to main model on 413 rate limit
- NousResearch#18638: Pass threshold_percent on model switch
- NousResearch#18663: Strip extra_content from tool_calls for strict APIs
- NousResearch#18618: Forward explicit_api_key to OpenRouter
- NousResearch#18632: Show cache tokens in /insights breakdown
- NousResearch#18614: Add idempotency guard for patch duplicate loops
- NousResearch#18600: Raise ValueError when HERMES_HOME unset in profile mode
- NousResearch#18616: Allow ZWJ emoji in context files
- NousResearch#18582: Reload .env on /restart
- NousResearch#18547: Stabilize system prompt prefix for KV cache reuse
- NousResearch#18692: Strip FTS5 operators from session search truncation terms

Fix: Add order_by_last_active=True to list_sessions_rich call
(pre-existing commit 142b4bf code sync)

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for identifying that the visible input/output figures do not sum to total_tokens.

Problems

  • agent/insights.py:765 reintroduces cache-token display that merged PR #11477 deliberately removed as unreliable (commit e33cb65a9). Current main preserves that policy in tests/agent/test_insights.py:465-468, and format_gateway() still omits cache at agent/insights.py:871.

Suggested changes

  • Please reconcile the cache-total proposal with #11477's reliability rationale before restoring the metric. If maintainers want a combined cache total despite that prior decision, make it an explicit presentation-policy change and update the current display-contract tests accordingly.

Automated hermes-sweeper review.

Comment thread agent/insights.py
lines.append(f" Sessions: {o['total_sessions']:<12} Messages: {o['total_messages']:,}")
lines.append(f" Tool calls: {o['total_tool_calls']:<12,} User messages: {o['user_messages']:,}")
lines.append(f" Input tokens: {o['total_input_tokens']:<12,} Output tokens: {o['total_output_tokens']:,}")
cache_total = (o.get('total_cache_read_tokens') or 0) + (o.get('total_cache_write_tokens') or 0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This restores cache-token output that current main deliberately removed in #11477 (e33cb65a9) because cache metrics were unreliable. Current tests retain the no-cache display contract; please reconcile this change with that policy before reintroducing the value.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Summary

Two PRs address the same token-breakdown mismatch: #4428 added conditional cache-token output to terminal and gateway reports, while #18632 independently restores that presentation and adds focused display-contract tests.

Related pull requests

  • #4428 [merged] duplicate — (+8/-1) — historical implementation: #4428 conditionally exposed cache reads/writes so the visible breakdown matched total_tokens; it remains relevant as the merged reference implementation, although contributor review on #18632 states that commit e33cb65 later removed this display as unreliable.
  • #18632 related — (+77/-1) — overlapping fix with tests: #18632 restores conditional cache totals in both formatters and adds five cache-display tests. The contributor review marked it keep_open but salvageability=none because the diff reverses the reliability policy established by #11477; it requires an explicit presentation-policy decision and corresponding display-contract tests before proceeding.

Duplicates

#4428 and #18632 implement essentially the same conditional cache-token presentation; #18632 additionally supplies targeted tests, while #4428 is the merged historical implementation later superseded by the removal cited in review.

Suggested consolidation

Keep #18632 open with a salvage path: retain its focused test coverage and conditional-formatting work only if the author first reconciles the proposal with #11477 and the reliability rationale behind commit e33cb65, then updates the tests to encode the resulting explicit presentation policy. Do not treat merged #4428 as the current fix: it is the duplicate historical implementation and was subsequently superseded by that removal.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup4428 ["PRs duplicating each other"]
        P4428["PR #4428 (merged)"]
        P18632["PR #18632 (open)"]
    end
    class P4428 merged
    class P18632 open
    class P18632 target
    click P4428 "https://github.com/NousResearch/hermes-agent/pull/4428"
    click P18632 "https://github.com/NousResearch/hermes-agent/pull/18632"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 7 kB of PR diffs, 3 kB of issue/PR text, <1 kB of discussion (1 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants