Skip to content

feat(dashboard): show gateway and provider cache on Usage page#336

Merged
SantiagoDePolonia merged 4 commits into
mainfrom
feat/cache-usage-visible
May 17, 2026
Merged

feat(dashboard): show gateway and provider cache on Usage page#336
SantiagoDePolonia merged 4 commits into
mainfrom
feat/cache-usage-visible

Conversation

@SantiagoDePolonia
Copy link
Copy Markdown
Contributor

@SantiagoDePolonia SantiagoDePolonia commented May 17, 2026

Summary

  • Gateway response cache: cached usage entries are now visible by default on the Usage page, with an opt-in "Hide cached requests" checkbox that filters both the paginated log and the live SSE stream (broker now broadcasts cached events so the client can decide). Cached rows render italic and dimmed, with a Cache column (Exact/Semantic) and a database-zap icon on the cost cell whose ancestor tooltip reads "Saved by cache — not charged".
  • Provider prompt cache: new Provider Cache column shows the percentage of input tokens served from the upstream provider's prompt cache (OpenAI prefix cache, Anthropic ephemeral cache, DeepSeek context cache, Gemini cache). Column order swapped to Provider | Model. Cost cells use white-space: nowrap so trailing icons stay inline.
  • Refactor: extracted usage.EntryInputSegments so SummarizeRequestUsage, the admin usage log handler, and the live SSE preview share one provider-cache segmenter — handles Anthropic's split accounting where input_tokens is the uncached portion and cache_read/creation_input_tokens are reported alongside.
  • Layout: widened .content from 1200px to 1400px.

Derived fields (uncached_input_tokens, cached_input_tokens, cache_write_input_tokens, cached_input_ratio) are JSON-only on UsageLogEntry, populated by EnrichUsageLogEntry at the admin/SSE boundary so storage layers stay untouched.

Test plan

  • go test ./... (full Go suite)
  • node --test internal/admin/dashboard/static/js/modules/*.test.cjs (292 tests)
  • Live verification: paired large-prompt requests to gpt-4o-mini, claude-sonnet-4-5, deepseek-v4-flash → Provider Cache column rendered 96.2%, 99.5%, and 99.2% with correct tooltips on the dashboard.
  • Flip "Hide cached requests" — paginated log and live SSE updates respect the toggle in both directions.
  • Verify Cache column shows "Exact"/"Semantic" in bold for cached rows; cached cost cell shows database-zap icon and the ancestor tooltip "Saved by cache — not charged".
  • Resize browser past 1400px to confirm content width is comfortable and tables breathe; verify mobile (<768px) still goes 100% width.

Note: the native passthrough route /p/{provider}/... does not record usage entries today, so passthrough traffic won't appear in the Provider Cache column. Worth filing as a separate task if we want passthrough usage tracking.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • "Hide cached requests" toggle in the Usage Log filter
    • Usage log shows cache and provider-cache indicators and icons
  • Improvements

    • Wider dashboard content area for improved visibility
    • Price cells no longer wrap; usage rows show cached vs uncached token and cost details with updated tooltips
    • Live usage previews include cached entries when applicable
  • Tests

    • Expanded unit tests covering cache handling and usage summaries

Review Change Stack

SantiagoDePolonia and others added 2 commits May 17, 2026 12:06
Show cached usage entries by default on the Usage page with an opt-in
"Hide cached requests" checkbox that filters both the paginated log and
the live SSE stream. The broker now broadcasts cached usage events so
the dashboard can choose whether to surface them. Cached rows are
visually distinguished (italic, dimmed via opacity), include a Cache
column ("Exact"/"Semantic" in bold), and a database-zap icon next to
the cost with a "Saved by cache — not charged" tooltip on the ancestor
cell.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a Provider Cache column to the Usage page request log showing the
share of input tokens served from the upstream provider's prompt cache
(OpenAI prefix cache, Anthropic ephemeral cache, DeepSeek context cache,
Gemini cache). Reorder columns to Provider | Model. Keep cached-row
formatting (italic + nowrap for cost cells so the cache icon stays
inline with the price), and widen the dashboard content area to 1400px.

Refactor: extract usage.EntryInputSegments so SummarizeRequestUsage,
the admin usage log handler, and the live SSE preview share one
provider-cache segmenter (handles Anthropic's split accounting where
input_tokens is the uncached portion and cache_read/creation are
reported alongside).

The derived fields (uncached_input_tokens, cached_input_tokens,
cache_write_input_tokens, cached_input_ratio) are JSON-only on
UsageLogEntry, populated by EnrichUsageLogEntry at the admin/SSE
boundary so storage layers stay untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 17, 2026

📝 Walkthrough

Walkthrough

Adds server-side cache-derived fields and EnrichUsageLogEntry, broadcasts enriched usage events (including cached), frontend toggle usageLogHideCached with cache_mode fetch param, live-log filtering of cached previews, UI/CSS changes, and tests.

Changes

Cache-aware usage log filtering and enrichment

Layer / File(s) Summary
Cache enrichment data model and helpers
internal/usage/reader.go, internal/usage/request_summary.go, internal/usage/enrich_test.go
UsageLogEntry gains derived fields for uncached/cached/cache-write input tokens and cached input ratio. EnrichUsageLogEntry() populates these fields from provider-specific raw data. EntryInputSegments() and predicate helper added/renamed for provider normalization; tests cover multiple providers and nil-safety.
Event publishing and response enrichment
internal/live/broker.go, internal/live/broker_test.go, internal/admin/handler_usage.go
Broker publishes all usage events and enriches preview payloads via EnrichUsageLogEntry(). Handler's UsageLog endpoint enriches each response entry. Tests updated to assert cached events are broadcast.
Frontend state and live-log filtering
internal/admin/dashboard/static/js/dashboard.js, internal/admin/dashboard/static/js/modules/live-logs.js, internal/admin/dashboard/static/js/modules/live-logs.test.cjs
Adds usageLogHideCached toggle state. Live-logs module skips inserting cached usage events when toggle is enabled. Tests verify cached events are not added and non-cached events are inserted.
Usage API integration and cache-display helpers
internal/admin/dashboard/static/js/modules/usage.js, internal/admin/dashboard/static/js/modules/usage.test.cjs
fetchUsageLog() appends cache_mode=uncached|all based on toggle state. New helpers detect normalized cache type, classify cached entries, produce cache/provider-cache labels, generate cached-cost titles, and compute provider-cache ratios/percent and multiline titles. Tests cover helpers and fetch parameter behavior with mocked fetch.
UI styling and template rendering
internal/admin/dashboard/static/css/dashboard.css, internal/admin/dashboard/templates/page-usage.html, internal/admin/dashboard/static/js/modules/dashboard-layout.test.cjs
CSS extends container width to 1400px, adds td.col-price nowrap, cache-row/checkbox/icon styling. Template adds hide-cached checkbox, new cache/provider-cache columns, per-row cached classes, cache indicators, and cached-cost tooltips. Layout test updated for new width.

Sequence Diagram

sequenceDiagram
  participant DashboardClient
  participant DashboardJS
  participant HandlerUsage
  participant EnrichUsageLogEntry
  DashboardClient->>DashboardJS: fetchUsageLog(cache_mode)
  DashboardJS->>HandlerUsage: GET /admin/usage/log?cache_mode=...
  HandlerUsage->>EnrichUsageLogEntry: EnrichUsageLogEntry(&entry) for each entry
  EnrichUsageLogEntry->>HandlerUsage: return enriched entry
  HandlerUsage->>DashboardJS: return enriched JSON entries
Loading
sequenceDiagram
  participant Broker
  participant EnrichUsageLogEntry
  participant Publisher
  participant DashboardLiveLogs
  Broker->>EnrichUsageLogEntry: usagePreviewFromEntry(entry)
  EnrichUsageLogEntry->>Broker: enriched preview
  Broker->>Publisher: publish enriched EventUsageCompleted
  Publisher->>DashboardLiveLogs: live event received
  DashboardLiveLogs->>DashboardLiveLogs: skip insert if usageLogHideCached && entry.cached
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ENTERPILOT/GoModel#196: Related cache analytics/enrichment work that introduced EnrichUsageLogEntry and cache-mode support used by this PR.

Suggested labels

release:internal

Poem

🐰 I hopped through logs and counted tokens,

Hidden cached bits beneath the oaks,
A toggle blinked — the view grew neat,
Enriched entries marched in cleanly, sweet,
The dashboard hums; the rabbit's heartbeat.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: adding visibility of gateway and provider cache metrics to the Usage dashboard page, which is the primary focus across CSS styling, UI components, and backend enrichment logic.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/cache-usage-visible

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 17, 2026

Greptile Summary

This PR expands the Usage page to show gateway and provider cache details. It changes:

  • Cached usage entries are included by default with a Hide cached requests toggle.
  • Live usage events now broadcast cached entries so the client can filter them.
  • Usage rows show Cache and Provider Cache columns with cache-aware cost display.
  • Provider-cache token segmentation is shared across summaries, REST usage logs, and SSE previews.
  • Dashboard content width increases for wider usage tables.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • The already-covered Bedrock cache-write issue remains the only confirmed actionable bug from this review.

Important Files Changed

Filename Overview
internal/admin/dashboard/static/js/modules/usage.js Adds cache-mode query handling and provider-cache display helpers.
internal/live/broker.go Broadcasts cached usage events and enriches live usage previews.
internal/usage/request_summary.go Centralizes provider-cache input token segmentation for downstream consumers.

Reviews (2): Last reviewed commit: "test(usage): pin max-coalescing of provi..." | Re-trigger Greptile

Comment thread internal/usage/request_summary.go
EntryInputSegments only read cache_creation_input_tokens (Anthropic's
field name) for cache writes; Bedrock's Converse extras emit
cache_write_input_tokens (matching AWS's field name). Cache writes from
Bedrock were silently zero on the Usage page, the live SSE preview, and
in request summaries, skewing the provider-cache ratio against the
wrong total. Coalesce both keys via max — consistent with how multiple
cached-read field names are merged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/admin/dashboard/static/js/modules/live-logs.js`:
- Around line 290-292: The guard using this.usageLogHideCached and
this.liveUsageEntryCached(incoming) only returns for new items (index < 0) but
doesn't remove or stop updates to existing cached rows; update the logic in the
handler that processes incoming and index so that when this.usageLogHideCached
is true and this.liveUsageEntryCached(incoming) is true you also evict any
existing row (when index >= 0) instead of leaving it visible—e.g., locate the
insert/update branch that references index and incoming and either remove the
existing DOM row or skip updating it when the item is cached and hide-cached is
enabled, ensuring the same check (this.usageLogHideCached &&
this.liveUsageEntryCached(incoming)) applies to both insert and update paths.

In `@internal/admin/dashboard/static/js/modules/usage.js`:
- Around line 367-370: The cached-entry detection in usageEntryCached currently
returns true only for 'exact' and 'semantic' but must also treat 'cache_hit' as
cached to match live-log semantics; update the usageEntryCached function to
include 'cache_hit' (or normalize 'cache_hit' to the same branch as 'semantic')
when calling usageEntryCacheType so fetched rows get the same cached
styling/tooltip as live rows.

In `@internal/admin/dashboard/templates/page-usage.html`:
- Around line 157-159: Add an explicit id/for association for the "Hide cached
requests" checkbox: give the input element a stable id (e.g.
id="usage-log-hide-cached") and change the label to target that id via
for="usage-log-hide-cached" while keeping the existing class
"usage-log-checkbox", x-model="usageLogHideCached", and
`@change`="fetchUsageLog(true)"; ensure the input is not nested inside the label
so the label/for pair satisfies HTMLHint's input-requires-label rule.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 94cff39a-bb2a-433f-baae-8317eb5f8f0f

📥 Commits

Reviewing files that changed from the base of the PR and between 80cf7af and 48d91d7.

📒 Files selected for processing (14)
  • internal/admin/dashboard/static/css/dashboard.css
  • internal/admin/dashboard/static/js/dashboard.js
  • internal/admin/dashboard/static/js/modules/dashboard-layout.test.cjs
  • internal/admin/dashboard/static/js/modules/live-logs.js
  • internal/admin/dashboard/static/js/modules/live-logs.test.cjs
  • internal/admin/dashboard/static/js/modules/usage.js
  • internal/admin/dashboard/static/js/modules/usage.test.cjs
  • internal/admin/dashboard/templates/page-usage.html
  • internal/admin/handler_usage.go
  • internal/live/broker.go
  • internal/live/broker_test.go
  • internal/usage/enrich_test.go
  • internal/usage/reader.go
  • internal/usage/request_summary.go

Comment on lines +290 to +292
if (this.usageLogHideCached && this.liveUsageEntryCached(incoming) && index < 0) {
return;
}
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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Hide-cached filtering should also evict existing cached rows.

On Line 290, the guard only skips inserts (index < 0). If a cached row already exists, it can remain visible/updated even while hide-cached is enabled.

Suggested fix
-                if (this.usageLogHideCached && this.liveUsageEntryCached(incoming) && index < 0) {
-                    return;
-                }
+                if (this.usageLogHideCached && this.liveUsageEntryCached(incoming)) {
+                    if (index >= 0) {
+                        currentEntries.splice(index, 1);
+                        this.usageLog.entries = [...currentEntries];
+                    }
+                    return;
+                }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/admin/dashboard/static/js/modules/live-logs.js` around lines 290 -
292, The guard using this.usageLogHideCached and
this.liveUsageEntryCached(incoming) only returns for new items (index < 0) but
doesn't remove or stop updates to existing cached rows; update the logic in the
handler that processes incoming and index so that when this.usageLogHideCached
is true and this.liveUsageEntryCached(incoming) is true you also evict any
existing row (when index >= 0) instead of leaving it visible—e.g., locate the
insert/update branch that references index and incoming and either remove the
existing DOM row or skip updating it when the item is cached and hide-cached is
enabled, ensuring the same check (this.usageLogHideCached &&
this.liveUsageEntryCached(incoming)) applies to both insert and update paths.

Comment on lines +367 to +370
usageEntryCached(entry) {
const type = this.usageEntryCacheType(entry);
return type === 'exact' || type === 'semantic';
},
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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Unify cached-entry detection with live-log semantics.

Line 367 currently treats only exact|semantic as cached, while live-log handling also treats cache_hit as cached. This can cause inconsistent cache styling/tooltip behavior between fetched rows and live rows.

Suggested fix
             usageEntryCached(entry) {
                 const type = this.usageEntryCacheType(entry);
-                return type === 'exact' || type === 'semantic';
+                return type === 'exact' || type === 'semantic' || !!(entry && entry.cache_hit);
             },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
usageEntryCached(entry) {
const type = this.usageEntryCacheType(entry);
return type === 'exact' || type === 'semantic';
},
usageEntryCached(entry) {
const type = this.usageEntryCacheType(entry);
return type === 'exact' || type === 'semantic' || !!(entry && entry.cache_hit);
},
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/admin/dashboard/static/js/modules/usage.js` around lines 367 - 370,
The cached-entry detection in usageEntryCached currently returns true only for
'exact' and 'semantic' but must also treat 'cache_hit' as cached to match
live-log semantics; update the usageEntryCached function to include 'cache_hit'
(or normalize 'cache_hit' to the same branch as 'semantic') when calling
usageEntryCacheType so fetched rows get the same cached styling/tooltip as live
rows.

Comment on lines +157 to +159
<label class="usage-log-checkbox">
<input type="checkbox" x-model="usageLogHideCached" @change="fetchUsageLog(true)">
<span>Hide cached requests</span>
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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add explicit for/id association for the hide-cached checkbox.

Line 158 is currently flagged by HTMLHint (input-requires-label). An explicit for/id pair will satisfy lint and keep accessibility intent unambiguous.

Suggested fix
-                <label class="usage-log-checkbox">
-                    <input type="checkbox" x-model="usageLogHideCached" `@change`="fetchUsageLog(true)">
+                <label class="usage-log-checkbox" for="usage-hide-cached">
+                    <input id="usage-hide-cached" type="checkbox" x-model="usageLogHideCached" `@change`="fetchUsageLog(true)">
                     <span>Hide cached requests</span>
                 </label>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<label class="usage-log-checkbox">
<input type="checkbox" x-model="usageLogHideCached" @change="fetchUsageLog(true)">
<span>Hide cached requests</span>
<label class="usage-log-checkbox" for="usage-hide-cached">
<input id="usage-hide-cached" type="checkbox" x-model="usageLogHideCached" `@change`="fetchUsageLog(true)">
<span>Hide cached requests</span>
</label>
🧰 Tools
🪛 HTMLHint (1.9.2)

[warning] 158-158: No matching [ label ] tag found.

(input-requires-label)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/admin/dashboard/templates/page-usage.html` around lines 157 - 159,
Add an explicit id/for association for the "Hide cached requests" checkbox: give
the input element a stable id (e.g. id="usage-log-hide-cached") and change the
label to target that id via for="usage-log-hide-cached" while keeping the
existing class "usage-log-checkbox", x-model="usageLogHideCached", and
`@change`="fetchUsageLog(true)"; ensure the input is not nested inside the label
so the label/for pair satisfies HTMLHint's input-requires-label rule.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/usage/enrich_test.go`:
- Around line 74-118: Add a test that verifies when both bedrock
cache_creation_input_tokens and cache_write_input_tokens are present the code
picks the larger value; create or extend a test (e.g.,
TestEnrichUsageLogEntry_BedrockCacheWriteMax) that constructs a UsageLogEntry
with Provider "bedrock", InputTokens set, and RawData containing both
"cache_creation_input_tokens" and "cache_write_input_tokens" with different
values, call EnrichUsageLogEntry(&entry), and assert entry.CacheWriteInputTokens
equals the max of the two values (and keep existing assertions for
UncachedInputTokens/CachedInputTokens as appropriate) to lock in max-coalescing
behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4b3ec8ac-7732-442a-9614-1ddb5ed789af

📥 Commits

Reviewing files that changed from the base of the PR and between 48d91d7 and 8ec427b.

📒 Files selected for processing (2)
  • internal/usage/enrich_test.go
  • internal/usage/request_summary.go

Comment thread internal/usage/enrich_test.go
@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/live/broker.go 75.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Add a regression test for the case where raw_data carries both
cache_creation_input_tokens and cache_write_input_tokens; the segmenter
must pick the larger value so provider-cache write totals aren't
undercounted on the dashboard.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/usage/enrich_test.go`:
- Around line 57-72: Add a new unit test alongside
TestEnrichUsageLogEntry_NoCacheData that constructs a UsageLogEntry with
Provider "openai" and InputTokens set to 0, calls EnrichUsageLogEntry(&entry),
and asserts that UncachedInputTokens and CachedInputTokens are zero and that
CachedInputRatio is exactly 0; reference the existing test name
TestEnrichUsageLogEntry_NoCacheData and the EnrichUsageLogEntry function and the
UsageLogEntry fields InputTokens and CachedInputRatio to locate where to add the
case.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9ef84968-e134-4be7-b93a-5735865e376f

📥 Commits

Reviewing files that changed from the base of the PR and between 8ec427b and f6a8002.

📒 Files selected for processing (1)
  • internal/usage/enrich_test.go

Comment on lines +57 to +72
func TestEnrichUsageLogEntry_NoCacheData(t *testing.T) {
entry := UsageLogEntry{
Provider: "openai",
InputTokens: 100,
}
EnrichUsageLogEntry(&entry)
if entry.UncachedInputTokens != 100 {
t.Fatalf("UncachedInputTokens = %d, want 100", entry.UncachedInputTokens)
}
if entry.CachedInputTokens != 0 {
t.Fatalf("CachedInputTokens = %d, want 0", entry.CachedInputTokens)
}
if entry.CachedInputRatio != 0 {
t.Fatalf("CachedInputRatio = %f, want 0", entry.CachedInputRatio)
}
}
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.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check EnrichUsageLogEntry and EntryInputSegments for division-by-zero handling

# Search for ratio calculation in the implementation
rg -n -A5 -B5 'CachedInputRatio.*=' internal/usage/

Repository: ENTERPILOT/GoModel

Length of output: 4632


🏁 Script executed:

# Find the EnrichUsageLogEntry function definition
rg -n 'func EnrichUsageLogEntry' internal/usage/

# Also check the full enrich.go file to understand the implementation
wc -l internal/usage/enrich.go

Repository: ENTERPILOT/GoModel

Length of output: 214


🏁 Script executed:

cat internal/usage/enrich_test.go

Repository: ENTERPILOT/GoModel

Length of output: 4308


Add test for InputTokens=0 edge case to ensure explicit coverage.

The test suite lacks coverage for the case where InputTokens is zero. Although the implementation already safely handles this scenario with a guard condition, adding an explicit test locks in the expected behavior. Include a test that verifies CachedInputRatio is correctly set to 0 when InputTokens is 0.

Proposed test addition
 func TestEnrichUsageLogEntry_NoCacheData(t *testing.T) {
 	entry := UsageLogEntry{
 		Provider:    "openai",
 		InputTokens: 100,
 	}
 	EnrichUsageLogEntry(&entry)
 	if entry.UncachedInputTokens != 100 {
 		t.Fatalf("UncachedInputTokens = %d, want 100", entry.UncachedInputTokens)
 	}
 	if entry.CachedInputTokens != 0 {
 		t.Fatalf("CachedInputTokens = %d, want 0", entry.CachedInputTokens)
 	}
 	if entry.CachedInputRatio != 0 {
 		t.Fatalf("CachedInputRatio = %f, want 0", entry.CachedInputRatio)
 	}
 }
 
+func TestEnrichUsageLogEntry_ZeroInputTokens(t *testing.T) {
+	entry := UsageLogEntry{
+		Provider:    "openai",
+		InputTokens: 0,
+	}
+	EnrichUsageLogEntry(&entry)
+	if entry.CachedInputRatio != 0 {
+		t.Fatalf("CachedInputRatio = %f, want 0 when InputTokens is 0", entry.CachedInputRatio)
+	}
+}
+
 func TestEnrichUsageLogEntry_BedrockCacheWriteField(t *testing.T) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func TestEnrichUsageLogEntry_NoCacheData(t *testing.T) {
entry := UsageLogEntry{
Provider: "openai",
InputTokens: 100,
}
EnrichUsageLogEntry(&entry)
if entry.UncachedInputTokens != 100 {
t.Fatalf("UncachedInputTokens = %d, want 100", entry.UncachedInputTokens)
}
if entry.CachedInputTokens != 0 {
t.Fatalf("CachedInputTokens = %d, want 0", entry.CachedInputTokens)
}
if entry.CachedInputRatio != 0 {
t.Fatalf("CachedInputRatio = %f, want 0", entry.CachedInputRatio)
}
}
func TestEnrichUsageLogEntry_NoCacheData(t *testing.T) {
entry := UsageLogEntry{
Provider: "openai",
InputTokens: 100,
}
EnrichUsageLogEntry(&entry)
if entry.UncachedInputTokens != 100 {
t.Fatalf("UncachedInputTokens = %d, want 100", entry.UncachedInputTokens)
}
if entry.CachedInputTokens != 0 {
t.Fatalf("CachedInputTokens = %d, want 0", entry.CachedInputTokens)
}
if entry.CachedInputRatio != 0 {
t.Fatalf("CachedInputRatio = %f, want 0", entry.CachedInputRatio)
}
}
func TestEnrichUsageLogEntry_ZeroInputTokens(t *testing.T) {
entry := UsageLogEntry{
Provider: "openai",
InputTokens: 0,
}
EnrichUsageLogEntry(&entry)
if entry.CachedInputRatio != 0 {
t.Fatalf("CachedInputRatio = %f, want 0 when InputTokens is 0", entry.CachedInputRatio)
}
}
func TestEnrichUsageLogEntry_BedrockCacheWriteField(t *testing.T) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/usage/enrich_test.go` around lines 57 - 72, Add a new unit test
alongside TestEnrichUsageLogEntry_NoCacheData that constructs a UsageLogEntry
with Provider "openai" and InputTokens set to 0, calls
EnrichUsageLogEntry(&entry), and asserts that UncachedInputTokens and
CachedInputTokens are zero and that CachedInputRatio is exactly 0; reference the
existing test name TestEnrichUsageLogEntry_NoCacheData and the
EnrichUsageLogEntry function and the UsageLogEntry fields InputTokens and
CachedInputRatio to locate where to add the case.

@SantiagoDePolonia SantiagoDePolonia merged commit 3a4cfe8 into main May 17, 2026
19 checks passed
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