feat(dashboard): add token cache meter and overview refinements#428
Conversation
- Aggregate provider prompt-cache split (uncached/cached/cache-write) into UsageSummary via a shared Go fold reusing EntryInputSegments across readers. - Add overview cache meter: 100%-stacked input-token bar (regular/locally cached/prompt cached) with empty state and on-bar percentages. - Total Requests now includes local cache hits (with breakdown tooltip). - Provider cards link to per-provider docs via the standard help icon. - Move the daily/weekly/monthly/yearly switch onto the usage chart header. - Regenerate API reference for the new UsageSummary fields. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Place the Cache Hits summary card directly after Total Requests, ahead of Estimated Cost, on the overview. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUsage summaries now include cache-split input token totals through readers, API responses, and generated schemas. The dashboard adds cache-aware totals, a cache meter, and provider documentation links. ChangesCache-aware usage summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Confidence Score: 3/5The dashboard can display stale cache data after a failed usage refresh.
internal/admin/dashboard/static/js/modules/usage.js
 T-Rex LogsWhat T-Rex did
Reviews (2): Last reviewed commit: "test(usage): cover foldInputSegments; a1..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return nil, fmt.Errorf("failed to query usage summary: %w", err) | ||
| } | ||
|
|
||
| if err := r.accumulateInputSegments(ctx, where, args, summary); err != nil { |
There was a problem hiding this comment.
This second pass reads the matching rows after the aggregate query has already returned. If usage rows are inserted or removed between the two queries, TotalRequests and TotalInput come from one snapshot while the new cache-split fields come from another, so the dashboard meter can show a split that does not match the reported totals.
Context Used: CLAUDE.md (source)
| for cursor.Next(ctx) { | ||
| var row struct { | ||
| InputTokens int `bson:"input_tokens"` | ||
| Provider string `bson:"provider"` | ||
| RawData map[string]any `bson:"raw_data"` | ||
| } |
There was a problem hiding this comment.
The MongoDB split pass now decodes raw_data directly into map[string]any. If an existing document has raw_data stored as a string, array, or other non-object value, the decode error aborts /admin/usage/summary even though the aggregate totals could still be returned; the SQL path degrades to a warning for malformed raw JSON.
Context Used: CLAUDE.md (source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/templates/page-overview.html`:
- Around line 101-107: The interval toggle buttons in the chart header only
expose selection via styling, so assistive tech can’t tell which option is
active. Update the button group in the chart header template to add a
descriptive label on the wrapper and bind aria-pressed on each of the interval
buttons in the interval picker, using the existing interval state and
setInterval behavior to reflect the current selection.
In `@internal/admin/handler_test.go`:
- Around line 273-286: The nil-reader test in
TestUsageSummary_NilReaderZeroesCacheSplit only checks zero values after
unmarshalling into usage.UsageSummary, so it can miss missing JSON fields.
Update the test to assert the raw response body from UsageSummary contains the
keys uncached_input_tokens, cached_input_tokens, and cache_write_input_tokens,
while still verifying they deserialize to zero. Use the existing handler setup
and response recorder to validate both shape and normalization.
In `@internal/usage/reader_mongodb.go`:
- Around line 154-193: The two-pass read in MongoDBReader from the summary query
and accumulateInputSegments is not snapshot-consistent, so the grouped totals
and split totals can diverge under concurrent writes. Fix this by making both
Aggregate and the follow-up Find run in the same read session/transaction
snapshot, or by moving the raw_data split accumulation into the aggregation
pipeline so a single read produces both results. Use the MongoDBReader methods
that perform these reads as the place to apply the change.
In `@internal/usage/reader_postgresql.go`:
- Around line 55-74: The summary query in PostgreSQLReader currently runs the
aggregate and accumulateInputSegments as separate reads, so they can see
different committed rows; update the usage summary path to use a single read
snapshot/transaction for both passes, or combine them into one statement. Make
the fix in the summary-building flow and the accumulateInputSegments helper so
the totals remain consistent with TotalRequests and TotalInput.
In `@internal/usage/reader_sqlite_cache_split_test.go`:
- Around line 18-128: The SQLite-only coverage in
TestSQLiteReaderSummary_AggregatesProviderCacheSplit does not exercise the new
second-pass logic in reader_postgresql.go and reader_mongodb.go. Reuse the same
fixture and expected totals to add parity assertions for NewPostgreSQLReader and
NewMongoDBReader, so the same summary checks run against those reader
implementations as well.
In `@internal/usage/reader_sqlite.go`:
- Around line 50-69: The summary logic in GetSummary/accumulateInputSegments
currently uses two separate SQLite reads, which can see different data if writes
happen between them. Update SQLiteReader so both the totals pass and the
input-segment pass run against the same snapshot, either by wrapping the
existing query path and accumulateInputSegments in a single read transaction or
by collapsing the aggregation into one query flow. Keep the fix localized around
GetSummary, accumulateInputSegments, and foldInputSegments so the row set used
for TotalRequests/TotalInput matches the cache-split fields.
In `@internal/usage/reader.go`:
- Around line 70-92: Add direct unit tests for foldInputSegments to cover its
shared row-processing behavior: create a small fake inputSegmentRows and verify
the happy path plus malformed raw_data JSON, Scan failures from rows.Scan, and
terminal rows.Err handling. Use the foldInputSegments helper and the
inputSegmentRows interface to locate the logic, and assert the summary updates
and wrapped error behavior without relying on a real database.
🪄 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: 57f163ee-9b45-4799-a3bf-35026a372ed4
📒 Files selected for processing (14)
cmd/gomodel/docs/docs.godocs/openapi.jsoninternal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/static/js/modules/providers.jsinternal/admin/dashboard/static/js/modules/providers.test.cjsinternal/admin/dashboard/static/js/modules/usage.jsinternal/admin/dashboard/static/js/modules/usage.test.cjsinternal/admin/dashboard/templates/page-overview.htmlinternal/admin/handler_test.gointernal/usage/reader.gointernal/usage/reader_mongodb.gointernal/usage/reader_postgresql.gointernal/usage/reader_sqlite.gointernal/usage/reader_sqlite_cache_split_test.go
| if err := r.accumulateInputSegments(ctx, matchFilters, summary); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return summary, nil | ||
| } | ||
|
|
||
| // accumulateInputSegments streams the matched documents and folds each row's | ||
| // provider prompt-cache split into the summary, reusing EntryInputSegments. The | ||
| // $group aggregate above cannot return per-document raw_data, so this is a | ||
| // second pass over the same filter projecting only the needed fields. This is | ||
| // the dashboard summary path, not a request hot path. | ||
| func (r *MongoDBReader) accumulateInputSegments(ctx context.Context, matchFilters bson.D, summary *UsageSummary) error { | ||
| projection := bson.D{ | ||
| {Key: "input_tokens", Value: 1}, | ||
| {Key: "provider", Value: 1}, | ||
| {Key: "raw_data", Value: 1}, | ||
| } | ||
| cursor, err := r.collection.Find(ctx, matchFilters, options.Find().SetProjection(projection)) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to query usage input segments: %w", err) | ||
| } | ||
| defer cursor.Close(ctx) | ||
|
|
||
| for cursor.Next(ctx) { | ||
| var row struct { | ||
| InputTokens int `bson:"input_tokens"` | ||
| Provider string `bson:"provider"` | ||
| RawData map[string]any `bson:"raw_data"` | ||
| } | ||
| if err := cursor.Decode(&row); err != nil { | ||
| return fmt.Errorf("failed to decode usage input segment row: %w", err) | ||
| } | ||
| summary.addInputSegments(row.InputTokens, row.Provider, row.RawData) | ||
| } | ||
| if err := cursor.Err(); err != nil { | ||
| return fmt.Errorf("error iterating usage input segment cursor: %w", err) | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
MongoDB needs a consistent snapshot across both passes.
Aggregate and the follow-up Find are independent reads, so concurrent inserts/updates can make the cache-split totals disagree with the grouped totals in the same response. Please use one consistent read snapshot/session for both operations, or fold the split accumulation into the aggregation pipeline.
🤖 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/reader_mongodb.go` around lines 154 - 193, The two-pass read
in MongoDBReader from the summary query and accumulateInputSegments is not
snapshot-consistent, so the grouped totals and split totals can diverge under
concurrent writes. Fix this by making both Aggregate and the follow-up Find run
in the same read session/transaction snapshot, or by moving the raw_data split
accumulation into the aggregation pipeline so a single read produces both
results. Use the MongoDBReader methods that perform these reads as the place to
apply the change.
| if err := r.accumulateInputSegments(ctx, where, args, summary); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return summary, nil | ||
| } | ||
|
|
||
| // accumulateInputSegments streams the matched rows and folds each row's | ||
| // provider prompt-cache split into the summary. It runs a second pass (the | ||
| // aggregate above cannot also return per-row raw_data) over the same filter, | ||
| // selecting only the columns EntryInputSegments needs. This is the dashboard | ||
| // summary path, not a request hot path. | ||
| func (r *PostgreSQLReader) accumulateInputSegments(ctx context.Context, where string, args []any, summary *UsageSummary) error { | ||
| rows, err := r.pool.Query(ctx, `SELECT input_tokens, provider, raw_data FROM "usage"`+where, args...) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to query usage input segments: %w", err) | ||
| } | ||
| defer rows.Close() | ||
| return foldInputSegments(rows, summary) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Use one PostgreSQL snapshot for both summary reads.
These two statements can observe different committed rows, so the cache-split totals may no longer reconcile with TotalRequests and TotalInput. Please execute both passes inside the same read snapshot/transaction, or rewrite this as one statement.
🤖 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/reader_postgresql.go` around lines 55 - 74, The summary query
in PostgreSQLReader currently runs the aggregate and accumulateInputSegments as
separate reads, so they can see different committed rows; update the usage
summary path to use a single read snapshot/transaction for both passes, or
combine them into one statement. Make the fix in the summary-building flow and
the accumulateInputSegments helper so the totals remain consistent with
TotalRequests and TotalInput.
| func TestSQLiteReaderSummary_AggregatesProviderCacheSplit(t *testing.T) { | ||
| db, err := sql.Open("sqlite", ":memory:") | ||
| if err != nil { | ||
| t.Fatalf("failed to open sqlite database: %v", err) | ||
| } | ||
| defer db.Close() | ||
|
|
||
| store, err := NewSQLiteStore(db, 0) | ||
| if err != nil { | ||
| t.Fatalf("failed to create sqlite store: %v", err) | ||
| } | ||
|
|
||
| ts := time.Date(2026, 6, 16, 12, 0, 0, 0, time.UTC) | ||
|
|
||
| // Provider (uncached-mode) rows — these feed both GetSummary and the oracle. | ||
| providerEntries := []*UsageEntry{ | ||
| { | ||
| ID: "openai-subset", RequestID: "r1", ProviderID: "p1", Timestamp: ts, | ||
| Model: "gpt-5", Provider: "openai", Endpoint: "/v1/chat/completions", | ||
| InputTokens: 120, OutputTokens: 30, TotalTokens: 150, | ||
| RawData: map[string]any{"prompt_cached_tokens": 80}, | ||
| }, | ||
| { | ||
| ID: "anthropic-split", RequestID: "r2", ProviderID: "p2", Timestamp: ts, | ||
| Model: "claude-sonnet-4-6", Provider: "anthropic", Endpoint: "/v1/messages", | ||
| InputTokens: 50, OutputTokens: 20, TotalTokens: 70, | ||
| RawData: map[string]any{"cache_read_input_tokens": 90, "cache_creation_input_tokens": 30}, | ||
| }, | ||
| { | ||
| ID: "anthropic-nofields", RequestID: "r3", ProviderID: "p3", Timestamp: ts, | ||
| Model: "claude-sonnet-4-6", Provider: "anthropic", Endpoint: "/v1/messages", | ||
| InputTokens: 50, OutputTokens: 20, TotalTokens: 70, | ||
| }, | ||
| { | ||
| ID: "gemini-subset", RequestID: "r4", ProviderID: "p4", Timestamp: ts, | ||
| Model: "gemini-2.5-pro", Provider: "gemini", Endpoint: "/v1/chat/completions", | ||
| InputTokens: 200, OutputTokens: 40, TotalTokens: 240, | ||
| RawData: map[string]any{"cached_tokens": 120}, | ||
| }, | ||
| { | ||
| ID: "groq-generic", RequestID: "r5", ProviderID: "p5", Timestamp: ts, | ||
| Model: "llama-3.3", Provider: "groq", Endpoint: "/v1/chat/completions", | ||
| InputTokens: 100, OutputTokens: 10, TotalTokens: 110, | ||
| RawData: map[string]any{"cached_tokens": 10}, | ||
| }, | ||
| } | ||
|
|
||
| // Local-cache hit — seeded but must be excluded from the uncached-mode summary. | ||
| localHit := &UsageEntry{ | ||
| ID: "local-hit", RequestID: "r6", ProviderID: "p6", Timestamp: ts, | ||
| Model: "gpt-5", Provider: "openai", Endpoint: "/v1/chat/completions", | ||
| CacheType: CacheTypeExact, InputTokens: 999, OutputTokens: 999, TotalTokens: 1998, | ||
| RawData: map[string]any{"prompt_cached_tokens": 500}, | ||
| } | ||
|
|
||
| ctx := context.Background() | ||
| if err := store.WriteBatch(ctx, append(append([]*UsageEntry{}, providerEntries...), localHit)); err != nil { | ||
| t.Fatalf("failed to seed usage entries: %v", err) | ||
| } | ||
|
|
||
| reader, err := NewSQLiteReader(db) | ||
| if err != nil { | ||
| t.Fatalf("failed to create sqlite reader: %v", err) | ||
| } | ||
|
|
||
| summary, err := reader.GetSummary(ctx, UsageQueryParams{ | ||
| StartDate: time.Date(2026, 6, 16, 0, 0, 0, 0, time.UTC), | ||
| EndDate: time.Date(2026, 6, 16, 0, 0, 0, 0, time.UTC), | ||
| TimeZone: "UTC", | ||
| }) | ||
| if err != nil { | ||
| t.Fatalf("GetSummary returned error: %v", err) | ||
| } | ||
|
|
||
| // Local-cache row excluded by the default uncached mode. | ||
| if summary.TotalRequests != len(providerEntries) { | ||
| t.Fatalf("TotalRequests = %d, want %d (local-cache row must be excluded)", summary.TotalRequests, len(providerEntries)) | ||
| } | ||
|
|
||
| // Oracle: sum EntryInputSegments over the same provider rows independently. | ||
| var wantUncached, wantCached, wantWrite int64 | ||
| for _, e := range providerEntries { | ||
| u, c, w := EntryInputSegments(UsageLogEntry{InputTokens: e.InputTokens, Provider: e.Provider, RawData: e.RawData}) | ||
| wantUncached += u | ||
| wantCached += c | ||
| wantWrite += w | ||
| } | ||
|
|
||
| if summary.UncachedInputTokens != wantUncached { | ||
| t.Fatalf("UncachedInputTokens = %d, want %d", summary.UncachedInputTokens, wantUncached) | ||
| } | ||
| if summary.CachedInputTokens != wantCached { | ||
| t.Fatalf("CachedInputTokens = %d, want %d", summary.CachedInputTokens, wantCached) | ||
| } | ||
| if summary.CacheWriteInputTokens != wantWrite { | ||
| t.Fatalf("CacheWriteInputTokens = %d, want %d", summary.CacheWriteInputTokens, wantWrite) | ||
| } | ||
|
|
||
| // Explicit magic numbers guard against a regression that still happens to be | ||
| // self-consistent with a broken oracle. cached = 80+90+0+120+10 = 300 can only | ||
| // be reached by per-row max-coalescing, not max(sum-per-field). | ||
| if summary.CachedInputTokens != 300 { | ||
| t.Fatalf("CachedInputTokens = %d, want 300", summary.CachedInputTokens) | ||
| } | ||
| if summary.CacheWriteInputTokens != 30 { | ||
| t.Fatalf("CacheWriteInputTokens = %d, want 30", summary.CacheWriteInputTokens) | ||
| } | ||
| if summary.UncachedInputTokens != 310 { | ||
| t.Fatalf("UncachedInputTokens = %d, want 310", summary.UncachedInputTokens) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Backfill parity tests for PostgreSQL and MongoDB.
This fixture is solid for SQLite, but the new second-pass logic in internal/usage/reader_postgresql.go and internal/usage/reader_mongodb.go still has no equivalent coverage. Add the same expected totals against those readers too, otherwise driver-specific scan/decode regressions can slip through unnoticed.
🤖 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/reader_sqlite_cache_split_test.go` around lines 18 - 128, The
SQLite-only coverage in TestSQLiteReaderSummary_AggregatesProviderCacheSplit
does not exercise the new second-pass logic in reader_postgresql.go and
reader_mongodb.go. Reuse the same fixture and expected totals to add parity
assertions for NewPostgreSQLReader and NewMongoDBReader, so the same summary
checks run against those reader implementations as well.
| if err := r.accumulateInputSegments(ctx, where, args, summary); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return summary, nil | ||
| } | ||
|
|
||
| // accumulateInputSegments streams the matched rows and folds each row's | ||
| // provider prompt-cache split into the summary. It runs a second pass (the | ||
| // aggregate above cannot also return per-row raw_data) over the same filter, | ||
| // selecting only the columns EntryInputSegments needs. This is the dashboard | ||
| // summary path, not a request hot path. | ||
| func (r *SQLiteReader) accumulateInputSegments(ctx context.Context, where string, args []any, summary *UsageSummary) error { | ||
| rows, err := r.db.QueryContext(ctx, `SELECT input_tokens, provider, raw_data FROM usage`+where, args...) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to query usage input segments: %w", err) | ||
| } | ||
| defer rows.Close() | ||
| return foldInputSegments(rows, summary) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Keep both SQLite summary passes on the same snapshot.
GetSummary now reads totals and cache-split rows in two separate statements. A write between those calls can make TotalRequests/TotalInput come from one row set while the cache-split fields come from another. Please run both passes inside one read transaction or collapse them into a single query path.
🤖 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/reader_sqlite.go` around lines 50 - 69, The summary logic in
GetSummary/accumulateInputSegments currently uses two separate SQLite reads,
which can see different data if writes happen between them. Update SQLiteReader
so both the totals pass and the input-segment pass run against the same
snapshot, either by wrapping the existing query path and accumulateInputSegments
in a single read transaction or by collapsing the aggregation into one query
flow. Keep the fix localized around GetSummary, accumulateInputSegments, and
foldInputSegments so the row set used for TotalRequests/TotalInput matches the
cache-split fields.
| // foldInputSegments scans (input_tokens, provider, raw_data) rows and folds each | ||
| // into the summary via addInputSegments, so the SQLite and PostgreSQL readers | ||
| // share one row-handling path (only the query/driver differs between them). | ||
| func foldInputSegments(rows inputSegmentRows, summary *UsageSummary) error { | ||
| for rows.Next() { | ||
| var inputTokens int | ||
| var provider string | ||
| var rawDataJSON *string | ||
| if err := rows.Scan(&inputTokens, &provider, &rawDataJSON); err != nil { | ||
| return fmt.Errorf("failed to scan usage input segment row: %w", err) | ||
| } | ||
| var rawData map[string]any | ||
| if rawDataJSON != nil && *rawDataJSON != "" { | ||
| if err := json.Unmarshal([]byte(*rawDataJSON), &rawData); err != nil { | ||
| slog.Warn("failed to unmarshal raw_data JSON", "error", err) | ||
| } | ||
| } | ||
| summary.addInputSegments(inputTokens, provider, rawData) | ||
| } | ||
| if err := rows.Err(); err != nil { | ||
| return fmt.Errorf("error iterating usage input segment rows: %w", err) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add direct tests for foldInputSegments.
This helper now carries the shared scan/unmarshal/error-wrapping behavior for the SQL readers, but the current coverage only exercises the SQLite happy path. A tiny fake inputSegmentRows test would lock down malformed raw_data, Scan failures, and rows.Err() handling without needing a real database.
🤖 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/reader.go` around lines 70 - 92, Add direct unit tests for
foldInputSegments to cover its shared row-processing behavior: create a small
fake inputSegmentRows and verify the happy path plus malformed raw_data JSON,
Scan failures from rows.Scan, and terminal rows.Err handling. Use the
foldInputSegments helper and the inputSegmentRows interface to locate the logic,
and assert the summary updates and wrapped error behavior without relying on a
real database.
Address review feedback on PR #428: - Add aria-pressed + a group label to the chart interval picker so assistive tech can tell which interval is active (not just the .active class). - Assert the nil-reader usage summary response includes the cache-split keys, so a dropped field can't masquerade as a zero after unmarshalling. - Add direct unit tests for the shared foldInputSegments helper: happy path, malformed raw_data (graceful degradation), scan error, and iteration error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @coderabbitai and @greptile-apps. Dispositions: Fixed (5ce08f8):
Won't fix (intentional):
|
|
Thanks for the detailed follow-up. All dispositions are reasonable:
LGTM. |
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
| const requests = Number((this.summary && this.summary.total_requests) || 0); | ||
| return (Number.isFinite(requests) ? requests : 0) + this.summaryCacheHits(); | ||
| }, |
There was a problem hiding this comment.
When a later usage summary or daily request fails, fetchUsage() resets summary to zeros but leaves the previous cacheOverview in place. This new total then adds old cache hits to the empty summary, so the Total Requests card and cache meter can show data from the previous period after a failed refresh.
When the usage summary/daily refresh fails, fetchUsage reset summary to zeros but left the previous period's cacheOverview in place. Because Total Requests and the cache meter now derive from cacheOverview, a failed refresh showed the old cache hits next to an empty summary. Reset cacheOverview alongside summary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Good catch @greptile-apps — valid P1. Fixed in ab86ba2: |
|
Fix looks correct — |
There was a problem hiding this comment.
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/admin/dashboard/static/js/modules/usage.js`:
- Around line 297-302: The success-path range switch in usage.js still renders
`renderChart()` with a stale `cacheOverview`, so the cache meter and request
breakdown can briefly show the previous period. Update the success flow in the
usage state reset/render path to clear `cacheOverview` before the first render
when cache analytics are enabled, or defer the overview-related render until
after `fetchCacheOverview()` completes; use the existing `summary`, `daily`,
`cacheOverview`, `renderChart()`, and `fetchCacheOverview()` symbols to place
the fix.
🪄 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: fd223882-9767-4f0c-ad32-e0e6598c68f5
📒 Files selected for processing (2)
internal/admin/dashboard/static/js/modules/usage.jsinternal/admin/dashboard/static/js/modules/usage.test.cjs
On a range switch, summary updated immediately while cacheOverview was only replaced when the async fetchCacheOverview resolved, so the cache meter and Total Requests briefly mixed the new summary with the previous period's cache data. Clear cacheOverview before the first render when the overview is visible. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Valid, @coderabbitai — fixed in dc0bf5a. The success path now clears |
|
Tip For best results, initiate chat on the files or code changes.
🐇✨ |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/admin/dashboard/static/js/modules/usage.js (2)
297-303: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClear the dashboard state in the exception path too.
This reset only covers the
handleFetchResponse()failure branch. If eitherfetch()rejects orsummaryRes.json()/dailyRes.json()throws, control jumps to Lines 328-333 and the previoussummary,daily, andcacheOverviewstay rendered. That still shows stale-period data after a failed refresh.Suggested fix
} catch (e) { if (typeof this._isAbortError === 'function' && this._isAbortError(e)) { return; } console.error('Failed to fetch usage:', e); + this.summary = this.emptyUsageSummary(); + this.daily = []; + this.cacheOverview = this.emptyCacheOverview(); + this.renderChart(); } finally {🤖 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 297 - 303, The exception path in the usage dashboard refresh still leaves stale data rendered because only the `handleFetchResponse()` failure branch clears state. Update the `refreshUsage()` flow in `usage.js` so that any rejection from `fetch()` or `summaryRes.json()` / `dailyRes.json()` also resets `summary`, `daily`, and `cacheOverview` via `emptyUsageSummary()` and `emptyCacheOverview()`, then re-renders through `renderChart()` before exiting the error path.
61-65: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign the new cache-hit math with the existing cache-visibility contract.
summaryCacheHits()and the local meter slice both return zero unlesscacheAnalyticsEnabled()is true, butinternal/admin/dashboard/static/js/modules/workflows.js:89-116already treats Redis/semantic cache config as cache-enabled even whenCACHE_ENABLEDis unset. In that configuration, the new Total Requests breakdown and cache meter silently fall back to provider-only numbers. Reuse the same visibility predicate here (or delegate toworkflowCacheVisible()) so the overview follows the runtime contract.Also applies to: 111-111
🤖 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 61 - 65, The cache-hit summary logic is still gated only by cacheAnalyticsEnabled(), so it ignores the broader cache-visibility contract already used by workflows.js. Update summaryCacheHits() and the local meter slice in usage.js to use the same visibility predicate as workflowCacheVisible() (or delegate to it) so Redis/semantic cache setups without CACHE_ENABLED still show cache data consistently.
🤖 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.
Outside diff comments:
In `@internal/admin/dashboard/static/js/modules/usage.js`:
- Around line 297-303: The exception path in the usage dashboard refresh still
leaves stale data rendered because only the `handleFetchResponse()` failure
branch clears state. Update the `refreshUsage()` flow in `usage.js` so that any
rejection from `fetch()` or `summaryRes.json()` / `dailyRes.json()` also resets
`summary`, `daily`, and `cacheOverview` via `emptyUsageSummary()` and
`emptyCacheOverview()`, then re-renders through `renderChart()` before exiting
the error path.
- Around line 61-65: The cache-hit summary logic is still gated only by
cacheAnalyticsEnabled(), so it ignores the broader cache-visibility contract
already used by workflows.js. Update summaryCacheHits() and the local meter
slice in usage.js to use the same visibility predicate as workflowCacheVisible()
(or delegate to it) so Redis/semantic cache setups without CACHE_ENABLED still
show cache data consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ff2dff4f-f325-4a35-a110-5a84386e2ac3
📒 Files selected for processing (2)
internal/admin/dashboard/static/js/modules/usage.jsinternal/admin/dashboard/static/js/modules/usage.test.cjs
The fetchUsage catch block only logged, so a fetch() rejection or json() parse error left the previous period's summary/daily/cacheOverview rendered — unlike the handled HTTP-failure branch which clears them. Reset and re-render on the exception path too, so all failure modes behave consistently. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @coderabbitai:
|
|
Tip For best results, initiate chat on the files or code changes.
And your explanation on 🐇👍 ✏️ Learnings added
|
) * fix(virtualmodels): don't gate IaC startup on catalog availability Declarative virtual models (VIRTUAL_MODELS / config.yaml) were validated at startup against the model catalog, which loads asynchronously and is empty on a cold cache. ValidateManagedConfig required every managed redirect target to be catalog-supported, so a brand-new deployment with valid IaC virtual models and no warm cache aborted startup with "target model not found". Separate the two concerns the check conflated: - Structural invariants (valid selector, no self-/cross-redirect target) are pure properties of the declaration, so they still fail startup loudly (validateRedirectStructure). - Catalog availability is runtime state, already handled by skipping unavailable targets at resolve time, so it is no longer a startup gate. The admin write path keeps it (firstUnsupportedTarget) since it runs against a warm catalog and an unknown target there is a caller mistake. A managed redirect with an unknown target now boots and is simply unavailable until the catalog provides it, consistent with the resolve-time skip and the background refresh that already tolerates a transient catalog gap. Tests: cold-catalog startup no longer aborts and resolves once warm; admin upsert still rejects an unsupported target; structural invalids still abort. Adds the standalone tests/e2e/test-iac-virtualmodels.sh IaC harness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(e2e): add release scenarios for load balancing, throughput, and cache analytics Cover the features shipped since v0.1.44 that the matrix had no behavioral coverage of (only the single-target alias path via S13/S24/S31): - S118-S125 load-balanced virtual models (#433): round-robin, weighted, cost, rename via old_source, plus negatives (unknown strategy, unknown target, rename of a non-existent source). - S126-S128 token throughput (#434): window shape across granularities, negative granularity handling, and live-traffic reflection. - S129-S132 cache analytics (#428): cache_mode on the usage summary, cache overview availability gating, and locally-cached token accounting from an exact-cache hit. Each scenario is self-contained ($QA_SUFFIX-scoped, cleans up after itself) and validated through run-release-e2e.sh. IaC virtual-model behavior needs gateways booted with custom config, so it lives in the standalone tests/e2e/test-iac-virtualmodels.sh harness instead of this running-stack matrix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
Summary
Adds a cache meter to the dashboard overview plus several overview refinements. The meter answers "what share of what we send is actually cached?" — something the existing absolute Cache Hits / Local Cache cards didn't show.
What's included
--success/ lime--prompt-cache-color) and theme-correct.UsageSummary— new additive fieldsuncached_input_tokens/cached_input_tokens/cache_write_input_tokens, aggregated per row via a shared Go fold that reusesEntryInputSegmentsacross the SQLite/PostgreSQL/MongoDB readers (the provider quirks — field-name coalescing, Anthropic additive-vs-subset accounting — stay in one place rather than being re-encoded in SQL).provider + cachebreakdown tooltip), computed on the frontend so the uncached-mode summary the meter relies on stays clean.?help icon linking to…/docs/providers/<slug>, but only for provider types that have a docs page (allowlist derived fromdocs/providers/*.mdx; handlesopencode_go→opencode-go).UsageSummaryfields.Provider-specific behavior
The prompt-cache split is normalized across providers via the existing
EntryInputSegments: Anthropic reports cache reads/writes as additive toinput_tokens, while OpenAI/Gemini/etc. report them as a subset — the aggregate reconciles both so the meter is accurate per provider.Testing
go build ./...,go vet, gofmt clean; all Go tests pass — including a SQLite cross-check that the aggregated split equalsΣ EntryInputSegmentsover a mixed-provider fixture (and catches the invalid sum-per-field shortcut), plus handler tests for the new JSON fields.🤖 Generated with Claude Code
Summary by CodeRabbit