feat(dashboard): live token throughput chart, prompt cache gauge, and overview refinements#434
Merged
Merged
Conversation
…ache split Add GetTokenThroughput to the usage reader (sqlite/postgres/mongodb): a fixed, trailing window of token-volume buckets — input, output, prompt-cached and locally-cached — at second/minute/hour/day granularity, exposed at GET /admin/usage/throughput for the overview live chart. Buckets are timezone-aware (offset from the dashboard's X-GoModel-Timezone header) so day buckets start at local midnight, matching the Daily chart. The provider prompt-cache split is folded in Go from raw_data via the existing EntryInputSegments, shared across all backends. Also extend GetDailyUsage with the per-period uncached/cached/cache-write split (a second streaming pass keyed by the same period) so the Daily chart can plot prompt-cached tokens. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e gauge - Live Token Throughput: a stacked bar chart (seconds/minutes/hours/days) fed by the usage SSE stream and /admin/usage/throughput, with day-boundary markers, live legend totals, and timezone-aware day buckets. The day-view hover tooltip shows the date alone (buckets start at local midnight). - Prompt Cache Rate: a compact half-circle gauge showing the share of input tokens served from the provider prompt cache over the selected period. - Daily Token Usage: add a prompt-cached series, collapse local cache to a single line, and recolour to the cost-based palette (brown = paid, blue = cached, more transparent = cheaper). - Unify chart styling (fonts, gridlines, tooltips) and align the cache meter colours and segment order with the charts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
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)
📝 WalkthroughWalkthroughAdds live token throughput charts and prompt cache rate UI to the admin dashboard, plus a new ChangesLive Token Throughput & Prompt Cache Rate
Sequence Diagram(s)sequenceDiagram
participant live_logs as live-logs.js
participant dashboardLiveTokensModule as dashboardLiveTokensModule
participant TokenThroughput as GET /admin/usage/throughput
participant Chart as Chart.js
live_logs->>dashboardLiveTokensModule: noteLiveTokenUsage("usage.flushed")
dashboardLiveTokensModule->>dashboardLiveTokensModule: debounce fetchLiveTokens()
dashboardLiveTokensModule->>TokenThroughput: fetch throughput window
TokenThroughput-->>dashboardLiveTokensModule: TokenThroughput JSON
dashboardLiveTokensModule->>Chart: renderLiveTokensChart()
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 |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: 10
🤖 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/css/dashboard.css`:
- Around line 1563-1578: The live status pulse animation in
live-dot.is-streaming should respect reduced-motion preferences. Add a
prefers-reduced-motion override near the live-dot-pulse keyframes or
.live-dot.is-streaming rule so users who disable motion get a static dot instead
of the infinite animation. Use the live-dot.is-streaming selector and
live-dot-pulse keyframes to locate the change.
In `@internal/admin/dashboard/static/js/modules/charts.js`:
- Around line 184-203: The prompt cache rate helpers in charts.js treat missing
split token fields as no data, which breaks legacy rows that only have total
input tokens. Update promptCacheRateHasData() and
promptCacheRate()/promptCacheRateText() to reuse the same fallback logic already
used elsewhere in this module: when uncached_input_tokens, cached_input_tokens,
and cache_write_input_tokens are absent, derive the denominator from
input_tokens/total_input_tokens instead of returning “—”. Keep the existing
symbols promptCacheRate, promptCacheRateHasData, and promptCacheRateText aligned
so the gauge shows 0% for legacy-only ranges.
In `@internal/admin/dashboard/static/js/modules/live-tokens.js`:
- Around line 49-50: The live token polling flow in liveTokens.js is applying
stale responses after a granularity change because the inFlight guard lets an
older request finish and then reuse the new liveTokensGranularity state. Update
the request lifecycle in the polling/update path (the inFlight handling, the
fetch trigger around the liveTokensGranularity change, and the response
application logic) so any response started under a previous granularity is
discarded or refetched before buckets/labels are rendered. Use the existing
inFlight and buckets state plus the liveTokensGranularity-dependent code paths
to ensure only the latest granularity’s payload updates the UI.
In `@internal/admin/dashboard/templates/page-overview.html`:
- Around line 48-50: The live throughput chart and the prompt-cache gauge use
canvas elements without an accessible name or text equivalent, so screen readers
cannot understand the widget. Update the chart markup in the overview template
around liveTokensChart and the prompt-cache gauge to provide an accessible
alternative, such as a descriptive label and accompanying text summary or
ARIA-described fallback content tied to the existing chart wrappers. Make sure
the accessible text reflects the current trend/data state for each widget.
In `@internal/admin/handler_test.go`:
- Around line 25-45: Add real handler coverage for the new TokenThroughput flow
in handler tests, not just the mock fields. Extend mockUsageReader and the
TokenThroughput path to capture the forwarded time alignment arguments
(time.Time and offset) so the handler can be verified, then add table-driven
tests around the handler method that exercises invalid granularity, nil-reader
empty response, reader error propagation, nil-result fallback, and correct
forwarding of the timezone/alignment values. Use the existing mockUsageReader
and TokenThroughput-related handler entry points to keep the tests focused on
behavior changes and error handling.
In `@internal/admin/handler_usage.go`:
- Around line 363-377: The dashboard timezone handling in handler_usage.go is
collapsing a full location into a single current offset via dashboardTimeZone
and now.In(location).Zone(), which breaks bucket alignment across DST
boundaries. Update the token throughput path around GetTokenThroughput so it
preserves timezone semantics for the whole trailing window instead of passing
only offset int64, and use the actual location/zone information when grouping
buckets in the usageReader implementation and any EmptyTokenThroughput fallback.
In `@internal/usage/reader_sqlite_daily_split_test.go`:
- Around line 60-89: The current test in reader.GetDailyUsage only covers the
default uncached path, so it would miss regressions where local-cache rows start
affecting daily split calculations. Extend the existing
reader_sqlite_daily_split_test to add a cached/all-mode case in UsageQueryParams
(using GetDailyUsage) that includes a local-cache row and then assert the
provider prompt-cache split remains unchanged in DailyUsage. Keep the checks
focused on the existing DailyUsage fields (InputTokens, UncachedInputTokens,
CachedInputTokens) and reuse the current by/date lookup pattern to verify both
modes.
In `@internal/usage/reader.go`:
- Around line 333-338: The throughput contract currently accepts only a fixed
UTC offset, which loses the request’s real time zone and can misalign hour/day
buckets across DST changes. Update GetTokenThroughput to take an IANA zone or
*time.Location instead of offset, and make the implementation align bucket
boundaries using that location so it stays consistent with GetDailyUsage and
other local-time grouping.
- Around line 145-180: `foldPeriodInputSegments` is folding local-cache rows
into the provider prompt-cache split because it only receives `period`,
`input_tokens`, `provider`, and `raw_data`. Update the second-pass
query/projection and the `inputSegmentRows` scan path in `reader.go` to include
`cache_type`, then have `foldPeriodInputSegments` skip any non-provider cache
rows before calling `EntryInputSegments`/`accumulatePeriodSplit`. Keep the
provider-only split logic aligned with `GetDailyUsage` so cached/all modes don’t
misclassify local-cache usage.
In `@internal/usage/throughput.go`:
- Around line 59-80: The current throughputWindow/throughputBucketStart contract
collapses timezone handling to a single UTC offset, which breaks DST-aware
bucketing across the trailing window. Update the throughput bucketing flow to
carry a *time.Location or compute the offset per bucket instead of using only
the offset for now, and adjust the callers in handler_usage so requests are
grouped by the correct local day/hour even across DST transitions. Keep the
alignment logic in throughputWindow and throughputBucketStart consistent with
the location-aware 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: 79ffd325-2bee-4a3e-8d14-dd0dc488b7ed
📒 Files selected for processing (21)
internal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/static/js/dashboard.jsinternal/admin/dashboard/static/js/modules/charts.jsinternal/admin/dashboard/static/js/modules/charts.test.cjsinternal/admin/dashboard/static/js/modules/live-logs.jsinternal/admin/dashboard/static/js/modules/live-tokens.jsinternal/admin/dashboard/static/js/modules/usage.jsinternal/admin/dashboard/static/js/modules/usage.test.cjsinternal/admin/dashboard/templates/layout.htmlinternal/admin/dashboard/templates/page-overview.htmlinternal/admin/handler_test.gointernal/admin/handler_usage.gointernal/admin/routes.gointernal/admin/routes_test.gointernal/usage/reader.gointernal/usage/reader_mongodb.gointernal/usage/reader_postgresql.gointernal/usage/reader_sqlite.gointernal/usage/reader_sqlite_daily_split_test.gointernal/usage/throughput.gointernal/usage/throughput_test.go
Comment on lines
+184
to
+203
| promptCacheRate() { | ||
| const summary = this.summary || {}; | ||
| const uncached = Math.max(0, Number(summary.uncached_input_tokens) || 0); | ||
| const cached = Math.max(0, Number(summary.cached_input_tokens) || 0); | ||
| const cacheWrite = Math.max(0, Number(summary.cache_write_input_tokens) || 0); | ||
| const denom = uncached + cached + cacheWrite; | ||
| return denom > 0 ? (cached / denom) * 100 : 0; | ||
| }, | ||
|
|
||
| promptCacheRateHasData() { | ||
| const summary = this.summary || {}; | ||
| const denom = (Number(summary.uncached_input_tokens) || 0) + | ||
| (Number(summary.cached_input_tokens) || 0) + | ||
| (Number(summary.cache_write_input_tokens) || 0); | ||
| return denom > 0; | ||
| }, | ||
|
|
||
| promptCacheRateText() { | ||
| if (!this.promptCacheRateHasData()) return '—'; | ||
| return Math.round(this.promptCacheRate()) + '%'; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fallback to total_input_tokens when split fields are absent.
Lines 193-199 treat a zero split sum as “no data”, but Lines 300-303 already acknowledge older rows can lack uncached_input_tokens / cached_input_tokens / cache_write_input_tokens and fall back to input_tokens. For a legacy-only range, the gauge text will show — even though the correct prompt-cache rate is 0%.
Suggested fix
promptCacheRate() {
const summary = this.summary || {};
const uncached = Math.max(0, Number(summary.uncached_input_tokens) || 0);
const cached = Math.max(0, Number(summary.cached_input_tokens) || 0);
const cacheWrite = Math.max(0, Number(summary.cache_write_input_tokens) || 0);
- const denom = uncached + cached + cacheWrite;
+ const split = uncached + cached + cacheWrite;
+ const denom = split > 0 ? split : Math.max(0, Number(summary.total_input_tokens) || 0);
return denom > 0 ? (cached / denom) * 100 : 0;
},
promptCacheRateHasData() {
const summary = this.summary || {};
- const denom = (Number(summary.uncached_input_tokens) || 0) +
- (Number(summary.cached_input_tokens) || 0) +
- (Number(summary.cache_write_input_tokens) || 0);
+ const split = (Number(summary.uncached_input_tokens) || 0) +
+ (Number(summary.cached_input_tokens) || 0) +
+ (Number(summary.cache_write_input_tokens) || 0);
+ const denom = split > 0 ? split : (Number(summary.total_input_tokens) || 0);
return denom > 0;
},📝 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
| promptCacheRate() { | |
| const summary = this.summary || {}; | |
| const uncached = Math.max(0, Number(summary.uncached_input_tokens) || 0); | |
| const cached = Math.max(0, Number(summary.cached_input_tokens) || 0); | |
| const cacheWrite = Math.max(0, Number(summary.cache_write_input_tokens) || 0); | |
| const denom = uncached + cached + cacheWrite; | |
| return denom > 0 ? (cached / denom) * 100 : 0; | |
| }, | |
| promptCacheRateHasData() { | |
| const summary = this.summary || {}; | |
| const denom = (Number(summary.uncached_input_tokens) || 0) + | |
| (Number(summary.cached_input_tokens) || 0) + | |
| (Number(summary.cache_write_input_tokens) || 0); | |
| return denom > 0; | |
| }, | |
| promptCacheRateText() { | |
| if (!this.promptCacheRateHasData()) return '—'; | |
| return Math.round(this.promptCacheRate()) + '%'; | |
| promptCacheRate() { | |
| const summary = this.summary || {}; | |
| const uncached = Math.max(0, Number(summary.uncached_input_tokens) || 0); | |
| const cached = Math.max(0, Number(summary.cached_input_tokens) || 0); | |
| const cacheWrite = Math.max(0, Number(summary.cache_write_input_tokens) || 0); | |
| const split = uncached + cached + cacheWrite; | |
| const denom = split > 0 ? split : Math.max(0, Number(summary.total_input_tokens) || 0); | |
| return denom > 0 ? (cached / denom) * 100 : 0; | |
| }, | |
| promptCacheRateHasData() { | |
| const summary = this.summary || {}; | |
| const split = (Number(summary.uncached_input_tokens) || 0) + | |
| (Number(summary.cached_input_tokens) || 0) + | |
| (Number(summary.cache_write_input_tokens) || 0); | |
| const denom = split > 0 ? split : (Number(summary.total_input_tokens) || 0); | |
| return denom > 0; | |
| }, | |
| promptCacheRateText() { | |
| if (!this.promptCacheRateHasData()) return '—'; | |
| return Math.round(this.promptCacheRate()) + '%'; |
🤖 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/charts.js` around lines 184 - 203,
The prompt cache rate helpers in charts.js treat missing split token fields as
no data, which breaks legacy rows that only have total input tokens. Update
promptCacheRateHasData() and promptCacheRate()/promptCacheRateText() to reuse
the same fallback logic already used elsewhere in this module: when
uncached_input_tokens, cached_input_tokens, and cache_write_input_tokens are
absent, derive the denominator from input_tokens/total_input_tokens instead of
returning “—”. Keep the existing symbols promptCacheRate,
promptCacheRateHasData, and promptCacheRateText aligned so the gauge shows 0%
for legacy-only ranges.
Comment on lines
+363
to
+377
| now := time.Now().UTC() | ||
| // Align buckets to the dashboard's timezone so day buckets start at local | ||
| // midnight (matching the Daily chart), not UTC. | ||
| _, location := dashboardTimeZone(c) | ||
| if location == nil { | ||
| location = time.UTC | ||
| } | ||
| _, offsetSeconds := now.In(location).Zone() | ||
| offset := int64(offsetSeconds) | ||
|
|
||
| if h.usageReader == nil { | ||
| return c.JSON(http.StatusOK, usage.EmptyTokenThroughput(gran, now, offset)) | ||
| } | ||
|
|
||
| result, err := h.usageReader.GetTokenThroughput(c.Request().Context(), gran, now, offset) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Don't reduce the dashboard timezone to a single current offset.
now.In(location).Zone() freezes one offset for the entire trailing window. For hour/day buckets that cross a DST transition, records near the boundary will be grouped into the wrong local bucket. Because every GetTokenThroughput implementation only receives offset int64, that loss of timezone information happens here and cannot be corrected downstream.
🤖 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/handler_usage.go` around lines 363 - 377, The dashboard
timezone handling in handler_usage.go is collapsing a full location into a
single current offset via dashboardTimeZone and now.In(location).Zone(), which
breaks bucket alignment across DST boundaries. Update the token throughput path
around GetTokenThroughput so it preserves timezone semantics for the whole
trailing window instead of passing only offset int64, and use the actual
location/zone information when grouping buckets in the usageReader
implementation and any EmptyTokenThroughput fallback.
Comment on lines
+333
to
+338
| // GetTokenThroughput returns a fixed-width window of token-volume buckets | ||
| // (input/output/prompt-cached/locally-cached) ending at end, for the | ||
| // overview live-throughput chart. The window is global (not user-path | ||
| // scoped). offset is the request timezone's offset from UTC in seconds, so | ||
| // buckets align to local boundaries (e.g. day buckets at local midnight). | ||
| GetTokenThroughput(ctx context.Context, gran ThroughputGranularity, end time.Time, offset int64) (*TokenThroughput, error) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Don't reduce throughput bucketing to a single UTC offset.
This contract drops the dashboard's actual time zone and keeps only the current offset. That breaks hour/day buckets whenever the trailing window crosses a DST transition, so GetTokenThroughput can drift away from local boundaries and disagree with GetDailyUsage, which groups with the full zone. Pass an IANA zone (or *time.Location) instead of a fixed offset.
🤖 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 333 - 338, The throughput contract
currently accepts only a fixed UTC offset, which loses the request’s real time
zone and can misalign hour/day buckets across DST changes. Update
GetTokenThroughput to take an IANA zone or *time.Location instead of offset, and
make the implementation align bucket boundaries using that location so it stays
consistent with GetDailyUsage and other local-time grouping.
Comment on lines
+59
to
+80
| // throughputWindow returns the bucket width in seconds and the inclusive first | ||
| // and exclusive upper bucket-start unix seconds for the window ending at end. | ||
| // offset is the timezone's offset from UTC (seconds east of UTC) so buckets | ||
| // align to local boundaries — notably the "day" buckets start at local midnight, | ||
| // matching the Daily Token Usage chart instead of UTC midnight. | ||
| func throughputWindow(gran ThroughputGranularity, end time.Time, offset int64) (bucketSeconds, first, upper int64) { | ||
| bucketSeconds = int64(gran.BucketSize / time.Second) | ||
| if bucketSeconds <= 0 { | ||
| bucketSeconds = 1 | ||
| } | ||
| current := ((end.Unix()+offset)/bucketSeconds)*bucketSeconds - offset | ||
| first = current - int64(gran.WindowCount-1)*bucketSeconds | ||
| upper = current + bucketSeconds | ||
| return bucketSeconds, first, upper | ||
| } | ||
|
|
||
| // throughputBucketStart aligns a unix timestamp to its local bucket start, using | ||
| // the same offset math as throughputWindow. SQLite/PostgreSQL compute this in SQL | ||
| // (so they can filter and group on an index); MongoDB and tests use this helper. | ||
| func throughputBucketStart(ts, bucketSeconds, offset int64) int64 { | ||
| return ((ts+offset)/bucketSeconds)*bucketSeconds - offset | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Don't collapse timezone alignment to a single UTC offset.
This bucketing math assumes one offset applies to the entire trailing window, but internal/admin/handler_usage.go:357-385 passes only the offset for now. In DST-observing zones, buckets on the other side of the transition shift by an hour, so requests can land in the wrong local day/hour bucket even though this path is meant to be timezone-aware. Pass a *time.Location (or otherwise compute per-bucket offsets) through this contract instead of reducing the zone to one int64.
🤖 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/throughput.go` around lines 59 - 80, The current
throughputWindow/throughputBucketStart contract collapses timezone handling to a
single UTC offset, which breaks DST-aware bucketing across the trailing window.
Update the throughput bucketing flow to carry a *time.Location or compute the
offset per bucket instead of using only the offset for now, and adjust the
callers in handler_usage so requests are grouped by the correct local day/hour
even across DST transitions. Keep the alignment logic in throughputWindow and
throughputBucketStart consistent with the location-aware behavior.
- live-tokens: discard stale throughput responses and refetch when the granularity changes mid-flight, so the chart never renders old buckets under the newly selected window's labels. - a11y: add an aria-label text equivalent for the live throughput canvas, and honor prefers-reduced-motion for the streaming status pulse. - usage: keep local-cache rows out of the daily provider prompt-cache split regardless of cache mode by passing cache_type through the second pass (sqlite/postgres/mongodb), with a regression test under all-mode. - tests: add TokenThroughput handler tests (invalid/missing granularity, nil-reader empty window, success + forwarded granularity/end/offset, forwarded timezone offset, gateway/generic error paths). Co-Authored-By: Claude Opus 4.8 (1M context) <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 (2)
internal/usage/reader_postgresql.go (1)
451-452: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftBucket in SQL instead of streaming raw usage rows.
This endpoint backs a continuously refreshing dashboard, but the query currently pulls every matching row in the full window and rebuckets in Go. On the
dayview that means rereading up to 30 days of usage on each refresh, which will not scale on larger tables. Push the bucket aggregation into PostgreSQL (or persist the prompt-cache split separately) so this path returns per-bucket totals instead of raw rows.🤖 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 451 - 452, The usage query in the PostgreSQL reader is still returning raw rows and rebucketing later in Go, which makes the dashboard refresh path scan too much data. Update the query built in the reader logic that uses bucketExpr and the "usage" table so PostgreSQL performs the per-bucket aggregation and returns bucket totals directly, rather than streaming all matching rows for post-processing.internal/usage/reader_mongodb.go (1)
780-800: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftGroup throughput buckets in MongoDB before decoding.
The Mongo path does a plain
Findover the whole window and folds every document in Go. Because the overview polls this endpoint repeatedly, thedayview becomes a recurring 30-day document scan/decode on large collections. Use an aggregation pipeline that groups by bucket and emits bucket totals instead of streaming raw rows.🤖 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 780 - 800, The MongoDB token-throughput path in the reader currently uses a plain Find and decodes every document before bucketing, which causes repeated full-window scans. Replace the Find/cursor loop in the throughput query logic with an aggregation pipeline that groups by the computed bucket and aggregates totals in MongoDB, then adapt the decode path to read bucket-level results instead of raw rows. Keep the existing bucket math aligned with throughputBucketStart and preserve the current fields used by the accumulator, such as CacheType, Provider, and token counts.
🤖 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/handler_test.go`:
- Around line 3085-3099: The TokenThroughput test only covers a fixed-offset
timezone, so it misses DST-related bucket misalignment in the throughput path.
Update the bucketing logic used by TokenThroughput to carry the full IANA
timezone (not just Zone() offset) when aligning hour/day buckets, or otherwise
make the alignment DST-aware across the window. Add a regression in the
TokenThroughput/throughput test coverage using a DST-observing zone such as
America/New_York and verify bucket boundaries stay correct across the
transition.
---
Outside diff comments:
In `@internal/usage/reader_mongodb.go`:
- Around line 780-800: The MongoDB token-throughput path in the reader currently
uses a plain Find and decodes every document before bucketing, which causes
repeated full-window scans. Replace the Find/cursor loop in the throughput query
logic with an aggregation pipeline that groups by the computed bucket and
aggregates totals in MongoDB, then adapt the decode path to read bucket-level
results instead of raw rows. Keep the existing bucket math aligned with
throughputBucketStart and preserve the current fields used by the accumulator,
such as CacheType, Provider, and token counts.
In `@internal/usage/reader_postgresql.go`:
- Around line 451-452: The usage query in the PostgreSQL reader is still
returning raw rows and rebucketing later in Go, which makes the dashboard
refresh path scan too much data. Update the query built in the reader logic that
uses bucketExpr and the "usage" table so PostgreSQL performs the per-bucket
aggregation and returns bucket totals directly, rather than streaming all
matching rows for post-processing.
🪄 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: d526387c-5603-4bcb-a372-c37bc43b9197
📒 Files selected for processing (9)
internal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/static/js/modules/live-tokens.jsinternal/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_daily_split_test.go
Comment on lines
+3085
to
+3099
| e := echo.New() | ||
| req := httptest.NewRequest(http.MethodGet, "/admin/usage/throughput?granularity=day", nil) | ||
| req.Header.Set("X-GoModel-Timezone", "Asia/Kolkata") // UTC+5:30, no DST | ||
| rec := httptest.NewRecorder() | ||
| c := e.NewContext(req, rec) | ||
|
|
||
| if err := h.TokenThroughput(c); err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
| if rec.Code != http.StatusOK { | ||
| t.Errorf("expected 200, got %d", rec.Code) | ||
| } | ||
| if reader.lastThroughputOffset != int64(5*3600+30*60) { | ||
| t.Errorf("forwarded offset = %d, want 19800 (Asia/Kolkata)", reader.lastThroughputOffset) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Fixed-offset coverage hides the DST bucket-alignment bug.
This only verifies a timezone whose UTC offset never changes. The handler currently forwards a single Zone() offset for the entire throughput window, so hour/day buckets will drift across DST changes in zones like America/New_York. Please carry the IANA timezone through bucketing (or make the alignment DST-aware) and add a regression here.
🤖 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/handler_test.go` around lines 3085 - 3099, The TokenThroughput
test only covers a fixed-offset timezone, so it misses DST-related bucket
misalignment in the throughput path. Update the bucketing logic used by
TokenThroughput to carry the full IANA timezone (not just Zone() offset) when
aligning hour/day buckets, or otherwise make the alignment DST-aware across the
window. Add a regression in the TokenThroughput/throughput test coverage using a
DST-observing zone such as America/New_York and verify bucket boundaries stay
correct across the transition.
… TODOs Capture why GetTokenThroughput streams window rows and folds the prompt-cache split in Go (EntryInputSegments is the single source of truth and the split is per-row non-linear, so it cannot be grouped in SQL), and add follow-up TODOs: TODO(perf) to persist the split as columns at write time so the aggregations can GROUP BY, and TODO(tz) to thread the full *time.Location for DST-correct hour/day buckets. Addresses reviewer scalability/timezone notes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SantiagoDePolonia
added a commit
that referenced
this pull request
Jun 29, 2026
) * 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>
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
2 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
Overview-dashboard improvements centred on token-usage visibility:
Backend
New
GET /admin/usage/throughput?granularity=second|minute|hour|dayreturning a fixed trailing window of token-volume buckets, implemented across all three stores (sqlite/postgres/mongodb). Buckets are timezone-aware (offset from theX-GoModel-Timezoneheader) so day buckets start at local midnight, matching the Daily chart. The provider prompt-cache split is folded in Go fromraw_datavia the existingEntryInputSegments, shared across backends.GET /admin/usage/dailygains the per-period uncached/cached/cache-write split so the Daily chart can plot prompt-cached tokens.Provider behaviour
The prompt-cache series reflect provider-reported prompt-cache reads (OpenAI
cached_tokens, Anthropiccache_read_input_tokens, …) — the same normalisation the cache meter already uses. GoModel's own exact-response-cache hits are shown separately as Locally Cached.Tests
make test-race,make lint, gofmt and the dashboard JS suite pass via pre-commit.Notes
🤖 Generated with Claude Code
Summary by CodeRabbit