Add cached tokens tracking to LLM usage reporting - #1680
Merged
Merged
Conversation
Track cached_tokens (from prompt_tokens_details.cached_tokens) through the full pipeline: LLM response extraction → LLMCosts accumulation → pytest user_properties → GitHub report. The new "Cached" column appears next to the existing "Tokens" column. https://claude.ai/code/session_01AcrrH8vM9rV5cA2yV1RmUj Signed-off-by: Claude <noreply@anthropic.com>
Contributor
📂 Previous Runs📜 Run @ bd76554 (#22758158754)✅ Results of HolmesGPT evalsAutomatically triggered by commit bd76554 on branch Results of HolmesGPT evals
📜 Run @ 5d34d44 (#22757733306)✅ Results of HolmesGPT evalsAutomatically triggered by commit 5d34d44 on branch Results of HolmesGPT evals
📜 Run @ a4aa6df (#22757233529)✅ Results of HolmesGPT evalsAutomatically triggered by commit a4aa6df on branch Results of HolmesGPT evals
📜 Run @ 8f0f0b0 (#22756357620)✅ Results of HolmesGPT evalsAutomatically triggered by commit 8f0f0b0 on branch Results of HolmesGPT evals
📜 Run @ b68f1c6 (#22755754413)✅ Results of HolmesGPT evalsAutomatically triggered by commit b68f1c6 on branch Results of HolmesGPT evals
✅ Results of HolmesGPT evalsAutomatically triggered by commit f911932 on branch Results of HolmesGPT evals
📖 Legend
🔄 Re-run evals manually
Option 1: Comment on this PR with Or with more options (one per line): Run evals on a different branch (e.g., master) for comparison:
Quick re-run: Use Option 2: Trigger via GitHub Actions UI → "Run workflow" Option 3: Add PR labels to include extra evals in automatic regression runs:
Examples: 🏷️ Valid tags
Commands: CLI: |
Contributor
|
✅ Docker images ready for
Use these tags to pull the images for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:80c0eb61
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:80c0eb61 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:80c0eb61
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:80c0eb61
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:80c0eb61
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:80c0eb61 me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:80c0eb61
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:80c0eb61Patch Helm values in one line (choose the chart you use): HolmesGPT chart: helm upgrade --install holmesgpt ./helm/holmes \
--set registry=me-west1-docker.pkg.dev/robusta-development/development \
--set image=holmes-dev:80c0eb61 \
--set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set operator.image=holmes-operator-dev:80c0eb61Robusta wrapper chart: helm upgrade --install robusta robusta/robusta \
--reuse-values \
--set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.image=holmes-dev:80c0eb61 \
--set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.operator.image=holmes-operator-dev:80c0eb61 |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds cached_tokens and reasoning_tokens to LLM usage and cost models, extracts them from LLM responses, accumulates max per-call completion tokens, and surfaces these fields through test result collection and GitHub/terminal reporting. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Test Runner
participant LLM as LLM Service
participant Extractor as Usage Extractor
participant Cost as Cost Accumulator
participant Collector as Test Result Collector
participant Reporter as Reporter
Runner->>LLM: invoke model
LLM-->>Runner: response (usage, details)
Runner->>Extractor: extract_usage_from_response(response)
Extractor->>Extractor: parse prompt/completion/cached/reasoning
Extractor-->>Cost: LLMResponseUsage
Cost->>Cost: accumulate cached/reasoning tokens, update max_completion_tokens_per_call
Cost-->>Collector: attach usage/cost metadata to result
Collector-->>Reporter: emit rows with input/output/cached/non-cached/reasoning/max
Reporter->>Reporter: compute aggregates and render report
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
🔬 CLI Performance Benchmark🟡 Startup Time (no LLM)Measures
🟡 Full CLI with LLMMeasures
PR: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@holmes/core/llm_usage.py`:
- Line 16: The code currently coerces missing
prompt_tokens_details.cached_tokens into 0, losing the distinction between
"unavailable" and a real zero; update holmes/core/llm_usage.py to preserve
"unavailable" by making cached_tokens Optional[int] (allow None) and stop
defaulting to 0 when reading prompt_tokens_details.cached_tokens (leave as None
if absent). Find the cached_tokens annotation and any parsing/assignment that
reads prompt_tokens_details.cached_tokens (including the similar logic around
lines 35–58) and change those assignments to set None when the provider did not
supply the metric; also update any downstream checks to explicitly test for None
vs 0 where needed.
In `@holmes/core/tool_calling_llm.py`:
- Line 159: The compaction cached token counts are not being added into the
overall LLMCosts totals, so update the compaction accumulation in
ToolCallingLLM.call and ToolCallingLLM.call_stream to include
compaction.cached_tokens into the running LLMCosts (same pattern used for
prompt/completion/total tokens); specifically, when accumulating compaction
results into the costs variable (referencing compaction and costs/LLMCosts), add
costs.cached_tokens += compaction.cached_tokens or 0 so that _process_cost_info
and the final markdown/report reflect cached tokens consumed during compaction.
In `@tests/llm/utils/reporting/github_reporter.py`:
- Around line 301-307: The current logic treats falsy cached_tokens the same as
missing data; change the condition to distinguish None from 0 by checking if
result.get("cached_tokens") is None (i.e., if cached_tokens is None then set
cached_tokens_str = "—"), otherwise format the integer (even if 0) into
cached_tokens_str = f"{cached_tokens:,}" and add to total_cached_tokens_sum;
update the same pattern used at the other occurrence (the block around
cached_tokens handling at lines ~325-327) to use the None check rather than a
truthy check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2c97c778-a0e2-4c4d-be04-c5af7a8a8633
📒 Files selected for processing (5)
holmes/core/llm_usage.pyholmes/core/tool_calling_llm.pytests/llm/conftest.pytests/llm/utils/property_manager.pytests/llm/utils/reporting/github_reporter.py
Shows prompt_tokens - cached_tokens as "Non-cached" column, giving visibility into actual billable input tokens per test. https://claude.ai/code/session_01AcrrH8vM9rV5cA2yV1RmUj Signed-off-by: Claude <noreply@anthropic.com>
…0.1:65431/git/HolmesGPT/holmesgpt into claude/add-cached-tokens-column-bdEhY
Replace the single "Tokens" column with detailed breakdown: - Input: prompt_tokens summed across all LLM calls - Output: completion_tokens summed across all LLM calls - Cached / Non-cached: cached vs fresh input tokens - Reasoning: reasoning_tokens from completion_tokens_details - Max output: largest single-call completion_tokens (useful for sizing output token reservations) Also adds cached_tokens and reasoning_tokens to SSE token_count events via get_llm_usage(), and tracks max_completion_tokens_per_call through the full pipeline (LLMCosts -> property_manager -> conftest -> github_reporter). https://claude.ai/code/session_01AcrrH8vM9rV5cA2yV1RmUj Signed-off-by: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
holmes/core/llm.py (1)
686-696: Consider consolidating withextract_usage_from_responseto avoid duplication.Both
get_llm_usage()andextract_usage_from_response()inllm_usage.pyextract the same fields from the same response structure using similar logic. This duplication increases maintenance burden and risk of inconsistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@holmes/core/llm.py` around lines 686 - 696, get_llm_usage duplicates logic already implemented in extract_usage_from_response; remove the duplicated field-extraction in get_llm_usage (functions: get_llm_usage) and delegate to or call the shared helper extract_usage_from_response in llm_usage.py to populate prompt_tokens_details.cached_tokens and completion_tokens_details.reasoning_tokens (symbols: prompt_tokens_details, cached_tokens, completion_tokens_details, reasoning_tokens) so there is a single source of truth for usage extraction; if extract_usage_from_response is not accessible, move the shared extraction logic into a small private helper (e.g., _extract_usage_fields) and have both get_llm_usage and extract_usage_from_response call it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/llm/utils/reporting/github_reporter.py`:
- Around line 321-329: The current logic sets non_cached_tokens_str to "—"
whenever non_cached_tokens <= 0, which hides the valid case where prompt_tokens
> 0 and cached_tokens == prompt_tokens (a 0 non-cached token result); update the
condition in the block around prompt_tokens, cached_tokens, non_cached_tokens,
non_cached_tokens_str and total_non_cached_tokens_sum so that "—" is only used
when prompt_tokens is falsy/None (missing data), but when prompt_tokens is
present and non_cached_tokens == 0 set non_cached_tokens_str to "0" (formatted
with commas) and still add 0 to total_non_cached_tokens_sum as appropriate.
---
Nitpick comments:
In `@holmes/core/llm.py`:
- Around line 686-696: get_llm_usage duplicates logic already implemented in
extract_usage_from_response; remove the duplicated field-extraction in
get_llm_usage (functions: get_llm_usage) and delegate to or call the shared
helper extract_usage_from_response in llm_usage.py to populate
prompt_tokens_details.cached_tokens and
completion_tokens_details.reasoning_tokens (symbols: prompt_tokens_details,
cached_tokens, completion_tokens_details, reasoning_tokens) so there is a single
source of truth for usage extraction; if extract_usage_from_response is not
accessible, move the shared extraction logic into a small private helper (e.g.,
_extract_usage_fields) and have both get_llm_usage and
extract_usage_from_response call it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 50b22bae-f474-4c59-a300-4c4f6954cc71
📒 Files selected for processing (6)
holmes/core/llm.pyholmes/core/llm_usage.pyholmes/core/tool_calling_llm.pytests/llm/conftest.pytests/llm/utils/property_manager.pytests/llm/utils/reporting/github_reporter.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/llm/conftest.py
Adds a "Tokens" column (with comma-formatted count) after the Cost column in the terminal reporter summary table. The data was already being tracked in test properties and Braintrust metadata but wasn't visible in the terminal output. https://claude.ai/code/session_01AcrrH8vM9rV5cA2yV1RmUj Signed-off-by: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/llm/utils/reporting/terminal_reporter.py`:
- Around line 266-271: When formatting total tokens in the terminal report, if
result.get("total_tokens") is missing or zero but prompt_tokens or
completion_tokens exist, compute tokens_str by summing
result.get("prompt_tokens", 0) + result.get("completion_tokens", 0) and format
that sum with thousands separators; otherwise fall back to "—". Update the logic
around the total_tokens/prompt_tokens/completion_tokens handling (referencing
result, total_tokens, prompt_tokens, completion_tokens) so the report shows the
component sum when total_tokens is not provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d870f7cd-1499-4444-8b16-a14aacba20d9
📒 Files selected for processing (1)
tests/llm/utils/reporting/terminal_reporter.py
Adds a "Total tokens" column after Cost in the GitHub markdown reporter and a "Tokens" column in the terminal Rich table. This shows the total_tokens value (prompt + completion) that was already tracked but not displayed in either reporter. https://claude.ai/code/session_01AcrrH8vM9rV5cA2yV1RmUj Signed-off-by: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/llm/utils/reporting/github_reporter.py (1)
322-338:⚠️ Potential issue | 🟡 MinorRender zero cache values as
0, not—.
cached_tokens == 0andnon_cached_tokens == 0are valid outcomes here, but the truthy checks collapse them into “unavailable”. Becauseholmes/core/llm_usage.py:32-77andtests/llm/conftest.py:870-882normalize these fields to0, the new columns will otherwise hide cache misses, 100% cache hits, and all-zero totals.💡 Suggested fix
- cached_tokens = result.get("cached_tokens", 0) - if cached_tokens and cached_tokens > 0: + cached_tokens = result.get("cached_tokens") + if cached_tokens is None: + cached_tokens_str = "—" + else: cached_tokens_str = f"{cached_tokens:,}" total_cached_tokens_sum += cached_tokens - else: - cached_tokens_str = "—" ... - if non_cached_tokens > 0: + if prompt_for_calc > 0: non_cached_tokens_str = f"{non_cached_tokens:,}" total_non_cached_tokens_sum += non_cached_tokens else: non_cached_tokens_str = "—" ... - total_cached_tokens_str = f"{total_cached_tokens_sum:,}" if total_cached_tokens_sum > 0 else "—" - total_non_cached_tokens_str = f"{total_non_cached_tokens_sum:,}" if total_non_cached_tokens_sum > 0 else "—" + total_cached_tokens_str = f"{total_cached_tokens_sum:,}" + total_non_cached_tokens_str = f"{total_non_cached_tokens_sum:,}"Also applies to: 373-374
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/llm/utils/reporting/github_reporter.py` around lines 322 - 338, The code currently treats zero as "unavailable" by using truthy checks; change those checks to explicit None checks so 0 is rendered as "0". Specifically, for cached_tokens (from result.get(...)) replace the if cached_tokens and cached_tokens > 0 condition with if cached_tokens is not None to set cached_tokens_str = f"{cached_tokens:,}" and add cached_tokens to total_cached_tokens_sum; similarly, compute non_cached_tokens as currently done and replace if non_cached_tokens > 0 with if non_cached_tokens is not None to set non_cached_tokens_str = f"{non_cached_tokens:,}" and add to total_non_cached_tokens_sum. Reference symbols: cached_tokens, cached_tokens_str, total_cached_tokens_sum, non_cached_tokens, non_cached_tokens_str, total_non_cached_tokens_sum, and result.get.
🧹 Nitpick comments (1)
tests/llm/utils/reporting/github_reporter.py (1)
230-231: Use a per-call label for the max completion column.
Max outputis ambiguous next toOutput; it reads like another aggregate instead of a peak-per-call metric. A label likeMax output/callorMax completion/callwould make the column self-explanatory.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/llm/utils/reporting/github_reporter.py` around lines 230 - 231, The table header string built into the markdown variable uses an ambiguous column label "Max output"; update that header to a per-call label like "Max output/call" or "Max completion/call" in the two places where the header rows are concatenated (the lines that append to markdown: the first header row containing column names and the second separator row), so the table column is clearly described as a peak-per-call metric instead of an aggregate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/llm/utils/reporting/github_reporter.py`:
- Around line 322-338: The code currently treats zero as "unavailable" by using
truthy checks; change those checks to explicit None checks so 0 is rendered as
"0". Specifically, for cached_tokens (from result.get(...)) replace the if
cached_tokens and cached_tokens > 0 condition with if cached_tokens is not None
to set cached_tokens_str = f"{cached_tokens:,}" and add cached_tokens to
total_cached_tokens_sum; similarly, compute non_cached_tokens as currently done
and replace if non_cached_tokens > 0 with if non_cached_tokens is not None to
set non_cached_tokens_str = f"{non_cached_tokens:,}" and add to
total_non_cached_tokens_sum. Reference symbols: cached_tokens,
cached_tokens_str, total_cached_tokens_sum, non_cached_tokens,
non_cached_tokens_str, total_non_cached_tokens_sum, and result.get.
---
Nitpick comments:
In `@tests/llm/utils/reporting/github_reporter.py`:
- Around line 230-231: The table header string built into the markdown variable
uses an ambiguous column label "Max output"; update that header to a per-call
label like "Max output/call" or "Max completion/call" in the two places where
the header rows are concatenated (the lines that append to markdown: the first
header row containing column names and the second separator row), so the table
column is clearly described as a peak-per-call metric instead of an aggregate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 636d9a31-f6f8-4c11-86e4-35e8be1e9f20
📒 Files selected for processing (1)
tests/llm/utils/reporting/github_reporter.py
- cached_tokens is now Optional[int] (None = provider didn't report it, 0 = provider reported zero cached). This preserves the distinction between "unavailable" and "real zero" throughout the pipeline. - Deduplicate: get_llm_usage() in llm.py now delegates to extract_usage_from_response() instead of reimplementing the same prompt_tokens_details/completion_tokens_details parsing. - GitHub reporter: non_cached_tokens shows "0" when prompt_tokens > 0 but everything was cached, instead of hiding it as "—". - Terminal reporter: total_tokens falls back to prompt + completion sum when total_tokens is not provided by the API. https://claude.ai/code/session_01AcrrH8vM9rV5cA2yV1RmUj Signed-off-by: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
holmes/core/tool_calling_llm.py (1)
197-217:⚠️ Potential issue | 🟠 MajorDon't gate usage accumulation on
total_tokensalone.Providers can omit
usage.total_tokenswhile still returningprompt_tokens,completion_tokens,cached_tokens, orreasoning_tokens. The current guard at line 197 (if raw.total_tokens > 0) skips the entire accumulation block whentotal_tokensis 0 or missing, causing valid token metrics to be dropped from cost tracking and reporting even when individual token counts are available.The fix should check whether any token metric is present before accumulating:
💡 Suggested fix
- if raw.total_tokens > 0: + has_token_usage = ( + raw.total_tokens > 0 + or raw.prompt_tokens > 0 + or raw.completion_tokens > 0 + or raw.cached_tokens is not None + or raw.reasoning_tokens > 0 + ) + if has_token_usage: cost_logger.debug( f"{log_prefix} cost: ${raw.cost:.6f} | Tokens: {raw.prompt_tokens} prompt + {raw.completion_tokens} completion = {raw.total_tokens} total" ) if costs: costs.total_cost += raw.cost costs.prompt_tokens += raw.prompt_tokens costs.completion_tokens += raw.completion_tokens - costs.total_tokens += raw.total_tokens + costs.total_tokens += raw.total_tokens or ( + raw.prompt_tokens + raw.completion_tokens + ) if raw.cached_tokens is not None: costs.cached_tokens = (costs.cached_tokens or 0) + raw.cached_tokens costs.reasoning_tokens += raw.reasoning_tokens costs.max_completion_tokens_per_call = max( costs.max_completion_tokens_per_call, raw.completion_tokens🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@holmes/core/tool_calling_llm.py` around lines 197 - 217, The accumulation block currently gated by "if raw.total_tokens > 0" should instead run whenever any token metric is present; update the condition around the accumulation that references raw and costs (the block using cost_logger, costs.total_cost, costs.prompt_tokens, costs.completion_tokens, costs.total_tokens, costs.cached_tokens, costs.reasoning_tokens, costs.max_completion_tokens_per_call) to check for presence of any of raw.prompt_tokens, raw.completion_tokens, raw.total_tokens, raw.cached_tokens, or raw.reasoning_tokens (e.g. any is not None or >0) before aggregating; keep the separate elif raw.cost > 0 branch for when no token metrics exist, and ensure cached_tokens handling still guards against None when adding to costs.cached_tokens.
♻️ Duplicate comments (2)
holmes/core/tool_calling_llm.py (1)
206-211:⚠️ Potential issue | 🟠 MajorCompaction runs still bypass the new token fields.
The normal response path now accumulates cached/reasoning/max-completion metrics, but the separate compaction accumulation in
ToolCallingLLM.call()andToolCallingLLM.call_stream()still copies only cost/prompt/completion/total. Any compaction cache hits or reasoning tokens will be missing from final totals.💡 Suggested fix
# In both compaction accumulation blocks if compaction.total_tokens > 0: costs.num_compactions += 1 costs.total_tokens += compaction.total_tokens costs.prompt_tokens += compaction.prompt_tokens costs.completion_tokens += compaction.completion_tokens costs.total_cost += compaction.cost + if compaction.cached_tokens is not None: + costs.cached_tokens = (costs.cached_tokens or 0) + compaction.cached_tokens + costs.reasoning_tokens += compaction.reasoning_tokens + costs.max_completion_tokens_per_call = max( + costs.max_completion_tokens_per_call, + compaction.completion_tokens, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@holmes/core/tool_calling_llm.py` around lines 206 - 211, The compaction path in ToolCallingLLM.call and ToolCallingLLM.call_stream currently only accumulates cost/prompt/completion/total and omits the new token fields; update the compaction accumulation logic to also add raw.cached_tokens into costs.cached_tokens (safely handling None as in the normal path), increment costs.reasoning_tokens by raw.reasoning_tokens, and update costs.max_completion_tokens_per_call to max(current, raw.completion_tokens) so the final totals include cached, reasoning, and max-completion tokens (use the same null/coalesce behavior used in the non-compaction accumulation).tests/llm/utils/reporting/github_reporter.py (1)
239-240:⚠️ Potential issue | 🟡 MinorKeep explicit zero cache totals visible in the summary row.
Per-row formatting now distinguishes
Nonefrom0, but the summary row reverts to—unless the aggregate is positive. If every run is a cache miss (cached_tokens == 0) or fully cached (non_cached_tokens == 0), the total currently hides a valid zero.💡 Suggested fix
total_cached_tokens_sum = 0 total_non_cached_tokens_sum = 0 + saw_cached_tokens = False + saw_non_cached_tokens = False total_reasoning_tokens_sum = 0 max_completion_per_call_max = 0 @@ if cached_tokens is not None: cached_tokens_str = f"{cached_tokens:,}" total_cached_tokens_sum += cached_tokens + saw_cached_tokens = True else: cached_tokens_str = "—" @@ if prompt_for_calc > 0: non_cached_tokens_str = f"{non_cached_tokens:,}" total_non_cached_tokens_sum += non_cached_tokens + saw_non_cached_tokens = True else: non_cached_tokens_str = "—" @@ - total_cached_tokens_str = f"{total_cached_tokens_sum:,}" if total_cached_tokens_sum > 0 else "—" - total_non_cached_tokens_str = f"{total_non_cached_tokens_sum:,}" if total_non_cached_tokens_sum > 0 else "—" + total_cached_tokens_str = ( + f"{total_cached_tokens_sum:,}" if saw_cached_tokens else "—" + ) + total_non_cached_tokens_str = ( + f"{total_non_cached_tokens_sum:,}" if saw_non_cached_tokens else "—" + )Also applies to: 322-339, 372-375
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/llm/utils/reporting/github_reporter.py` around lines 239 - 240, The summary row hides valid zero totals because the code uses truthiness to decide between displaying a numeric total and an em-dash; change those checks to explicitly test for None so zero (0) is rendered as "0". Update the logic where total_cached_tokens_sum and total_non_cached_tokens_sum are formatted for the summary (and the similar blocks around the other occurrences) to use "if total_cached_tokens_sum is None" / "if total_non_cached_tokens_sum is None" rather than "if not total_cached_tokens_sum" before outputting the value so explicit zeros remain visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/llm/utils/reporting/github_reporter.py`:
- Around line 298-304: The code sets total_tokens_str to "—" when result lacks
total_tokens, which undercounts rows; instead, if result.get("total_tokens") is
falsy but result contains prompt_tokens and/or completion_tokens, compute
total_tokens = (result.get("prompt_tokens", 0) + result.get("completion_tokens",
0)), set total_tokens_str = f"{total_tokens:,}" and add that value into
total_tokens_sum; update the logic around the variables total_tokens,
prompt_tokens, completion_tokens, total_tokens_str, and total_tokens_sum to use
this computed fallback.
---
Outside diff comments:
In `@holmes/core/tool_calling_llm.py`:
- Around line 197-217: The accumulation block currently gated by "if
raw.total_tokens > 0" should instead run whenever any token metric is present;
update the condition around the accumulation that references raw and costs (the
block using cost_logger, costs.total_cost, costs.prompt_tokens,
costs.completion_tokens, costs.total_tokens, costs.cached_tokens,
costs.reasoning_tokens, costs.max_completion_tokens_per_call) to check for
presence of any of raw.prompt_tokens, raw.completion_tokens, raw.total_tokens,
raw.cached_tokens, or raw.reasoning_tokens (e.g. any is not None or >0) before
aggregating; keep the separate elif raw.cost > 0 branch for when no token
metrics exist, and ensure cached_tokens handling still guards against None when
adding to costs.cached_tokens.
---
Duplicate comments:
In `@holmes/core/tool_calling_llm.py`:
- Around line 206-211: The compaction path in ToolCallingLLM.call and
ToolCallingLLM.call_stream currently only accumulates
cost/prompt/completion/total and omits the new token fields; update the
compaction accumulation logic to also add raw.cached_tokens into
costs.cached_tokens (safely handling None as in the normal path), increment
costs.reasoning_tokens by raw.reasoning_tokens, and update
costs.max_completion_tokens_per_call to max(current, raw.completion_tokens) so
the final totals include cached, reasoning, and max-completion tokens (use the
same null/coalesce behavior used in the non-compaction accumulation).
In `@tests/llm/utils/reporting/github_reporter.py`:
- Around line 239-240: The summary row hides valid zero totals because the code
uses truthiness to decide between displaying a numeric total and an em-dash;
change those checks to explicitly test for None so zero (0) is rendered as "0".
Update the logic where total_cached_tokens_sum and total_non_cached_tokens_sum
are formatted for the summary (and the similar blocks around the other
occurrences) to use "if total_cached_tokens_sum is None" / "if
total_non_cached_tokens_sum is None" rather than "if not
total_cached_tokens_sum" before outputting the value so explicit zeros remain
visible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1a444e4-685e-4bfc-9917-f7a1c516c89e
📒 Files selected for processing (6)
holmes/core/llm.pyholmes/core/llm_usage.pyholmes/core/tool_calling_llm.pytests/llm/conftest.pytests/llm/utils/reporting/github_reporter.pytests/llm/utils/reporting/terminal_reporter.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/llm/conftest.py
- Remove unused _extract_cost_from_response function - Simplify test_cache.py to reuse extract_usage_from_response - Extract _fmt_tokens helper to reduce formatting repetition in github_reporter - Fix non_cached_tokens to show "—" when cached is unknown (not misleading number) - Add total_tokens fallback in github_reporter for consistency with terminal_reporter https://claude.ai/code/session_01AcrrH8vM9rV5cA2yV1RmUj Signed-off-by: Claude <noreply@anthropic.com>
aantn
enabled auto-merge (squash)
March 6, 2026 09:50
arikalon1
approved these changes
Mar 7, 2026
This was referenced Mar 7, 2026
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
3 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
This PR adds support for tracking and reporting cached tokens from LLM API responses. Cached tokens represent prompt tokens that were retrieved from the model's cache, reducing both latency and cost. The feature is now integrated throughout the LLM usage tracking pipeline and included in markdown reports.
Key Changes
cached_tokensfield to the NamedTuple to capture cached token counts from API responsesprompt_tokens_detailsfield in LLM responses, supporting both dict and object attribute access patternscached_tokensfield and accumulation logic in_process_cost_info()to aggregate cached tokens across multiple LLM callsproperty_manager.pyto capture and store cached tokens in test node propertiesconftest.pyto include cached tokens when collecting test results from pytest statsgenerate_markdown_report()to display cached tokens as a new column, with totals aggregation in the summary rowImplementation Details
prompt_tokens_details.cached_tokensfield when available in the LLM responsehttps://claude.ai/code/session_01AcrrH8vM9rV5cA2yV1RmUj
Summary by CodeRabbit
New Features
Reporting
Tests