Skip to content

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks - #491

Merged
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps
Jul 31, 2026
Merged

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks#491
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps

Conversation

@Jose-Gael-Cruz-Lopez

@Jose-Gael-Cruz-LopezJose-Gael-Cruz-Lopez commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

PR #479 shipped the #122 dashboard, but a criterion-by-criterion audit against the issue spec found three concrete gaps. This PR closes them:

  1. "Cost & tokens over time" — tokens were never charted. The LLM cost panel now projects the CostDayPoint series into zero-filled day series and renders a second lazy DayLineChart ("Tokens per day"), with a compact-count axis formatter (formatCompactCount in daySeries.ts) so token magnitudes fit the y-gutter. No backend work — the API already returned calls/total_tokens per day.
  2. "Error-rate trend" was an error-count trend. New JSX-free errorRateSeries joins the errors/day series with the usage summary's events/day series by date (both zero-filled over the panel range): rate = errors ÷ events as a percentage; zero-event days report 0, never NaN/Infinity. The absolute errors/day chart stays; the rate chart renders alongside it (muted note if the usage series hasn't loaded).
  3. The three line charts had no table fallback — day values were reachable only by pointer hover. New DaySeriesTable mirrors the existing "View data" <details> pattern for every day-series chart (usage events/day; cost calls/tokens/cost; errors count/rate). Also wraps all minWidth tables in an overflowX: auto container so narrow viewports scroll the table, not the page.

Constraints preserved: hand-rolled SVG only (no chart lib), next/dynamic + ssr:false lazy-loading and the JSX-free daySeries.ts split untouched, role="img" + aria-labels on new charts, brand tokens, no glass/gradient-text, and every name/testid asserted by e2e/admin-analytics.spec.ts unchanged.

Verification

  • vitest on touched files: 25 tests pass — the old View data ≥ 2 smoke assert is replaced with panel-scoped assertions of specific day values (incl. a 25% rate = 1 error ÷ 4 events fixture)
  • tsc --noEmit clean; eslint exit 0 on all touched files
  • ⚠️ Local E2E lane not runnable on this machine (unprovisioned stack — details in PR perf(frontend): #111 runtime animations + lazy/next-image loading #490); e2e.yml runs the admin-analytics journey on push to main. All names/testids that journey asserts are unchanged.

Closes#122

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added accessible data tables and “View data” fallbacks for analytics charts.
    • Added daily error-rate reporting based on usage events.
    • Added daily usage event charts and expanded LLM cost reporting for tokens, calls, and cost.
    • Added compact formatting for large counts.
  • Improvements

    • Improved responsive display of analytics tables.
    • Missing daily usage and error values are now shown as zero.

…series table fallbacks
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@supabase

supabaseBot commented Jul 31, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitaiBot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 04e4f8ea-eac5-44ee-9c15-ba36a7ea6208

📥 Commits

Reviewing files that changed from the base of the PR and between f135770 and a03d410.

📒 Files selected for processing (2)
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
📝 Walkthrough

Walkthrough

The analytics dashboard adds daily error-rate calculations, compact count formatting, accessible chart data tables, responsive raw tables, and expanded usage, cost, token, call, and error visualizations with integration coverage.

Changes

Analytics dashboard

Layer / File(s)Summary
Daily series utilities
frontend/src/lib/daySeries.ts, frontend/src/components/AnalyticsCharts.tsx, frontend/src/components/AnalyticsCharts.test.tsx
Adds errorRateSeries and formatCompactCount, re-exports them, and tests date joining, zero-event handling, domain filtering, and rounding boundaries.
Chart data tables and metric panels
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Adds DaySeriesTable, responsive table wrappers, zero-filled usage data, daily cost/token/call projections, and labeled chart fallbacks. Tests cover the expanded chart set and table values.
Error-rate dashboard flow
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Calculates and displays daily error rates against usage events, including rate states, error tables, zero-filled days, and accessibility assertions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant AdminAnalytics
participant errorRateSeries
participant DaySeriesTable
AdminAnalytics->>errorRateSeries: pass zero-filled errors and usage events
errorRateSeries-->>AdminAnalytics: return daily error-rate points
AdminAnalytics->>DaySeriesTable: pass errors and rate series
DaySeriesTable-->>AdminAnalytics: render expandable daily data table
Loading

Possibly related PRs

Suggested reviewers:andresl230

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the three analytics dashboard gaps addressed by this pull request.
Description check✅ PassedThe description provides a detailed summary, related issue, implementation details, and verification results, although it uses alternate section headings.
Linked Issues check✅ PassedThe changes address issue #122 by adding token trends, error-rate trends, accessible table fallbacks, and responsive chart data tables.
Out of Scope Changes check✅ PassedThe changes are limited to analytics charts, calculations, accessibility fallbacks, responsive tables, and related tests.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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/122-analytics-gaps

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.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 31, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga03d410Commit Preview URL

Branch Preview URL
Jul 31 2026, 06:53 AM

…opy, compact-count boundary
- DaySeriesTable and the two raw-table disclosures now carry per-chart
labels ("View data: errors per day", …) so the five identical summaries
are distinguishable to screen-reader users.
- Errors panel zero-fills unconditionally: a no-error range renders real
0s and a flat line, not em-dash "unknown" cells.
- Rate-chart fallback copy branches on summary error/loaded-empty/loading
instead of always promising the series will load.
- formatCompactCount promotes 999,950+ to "1M" instead of emitting
"1000k"; boundary cases and the errorRateSeries domain-drop contract
pinned in unit tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
MemberAuthor

Automated review pass (Claude Code) on a72a5ad. The reviewer independently re-ran the touched tests (25/25), tsc, and eslint in a throwaway worktree, and probed the new helpers directly with Node. Verdict on the core: the rate join is sound on every reachable path — UTC day keys on both sides (backend _bucket_datezeroFillDays' toISOString()), NaN-proof zero-event guard, errors a strict subset of the events domain so the join can't drop an error day; lazy-loading structure and every locator e2e/admin-analytics.spec.ts asserts verified intact.

No criticals. The review's minors are now fixed in f135770:

  • Five identical "View data" disclosures → per-chart labels ("View data: errors per day", …), with the unit test asserting each.
  • Errors panel now zero-fills unconditionally, so a no-error range shows real 0s and a flat line instead of em-dash "unknown" cells.
  • The rate-chart fallback copy branches on summary error / loaded-but-empty / still-loading instead of always promising the series will load.
  • formatCompactCount(999_950) returned "1000k" → now promotes to "1M"; unit boundaries (999/1000/999,949/999,950) and the errorRateSeries domain-drop contract are pinned in tests.

Not addressed (accepted, with reasoning): rate can exceed 100% if the summary scan hits the 100k _SCAN_CAP while the errors scan doesn't — the TruncatedBadge renders in the Usage panel rather than beside the rate chart. Extreme edge; worth badge propagation only if truncation becomes common.

Outstanding process item (already flagged in the PR body): the pre-merge local E2E lane couldn't run on this machine — the admin-analytics journey should be run on a provisioned machine before merge, or watch the post-merge e2e.yml run closely. A follow-up journey step asserting the "Error rate per day" chart and one DaySeriesTable value would give the new surfaces lane coverage.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@frontend/src/components/screens/AdminAnalytics.tsx`:
- Around line 471-516: In the Errors panel render block, validate that
summary.data belongs to the current d.range before constructing eventPoints or
calling errorRateSeries; when summary.range differs, leave the denominator
unavailable so the rate shows the invalid/loading state rather than fabricated
0% values. Update the fallback message branching to distinguish this range
mismatch from summary.error and a genuinely empty events series.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f7cfa34d-f710-4538-9c93-b3ede3e984bd

📥 Commits

Reviewing files that changed from the base of the PR and between 432c207 and f135770.

📒 Files selected for processing (5)
  • frontend/src/components/AnalyticsCharts.test.tsx
  • frontend/src/components/AnalyticsCharts.tsx
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
  • frontend/src/lib/daySeries.ts

Comment threadfrontend/src/components/screens/AdminAnalytics.tsx
Jose-Gael-Cruz-Lopezand others added 2 commits July 30, 2026 23:29
…ry ranges
The summary hook keeps its last payload while a range change reloads, so
the errors panel could zero-fill an old-range series against the new
range and fabricate an all-0% rate line. The join now requires day-key
range agreement; mismatch renders the waiting state. Also: TruncatedBadge
propagates to the rate chart when the summary scan was truncated (the
denominator undercounts, so the rate can overshoot), and formatPct floors
tiny nonzero rates at "<0.1%" so they never read as "no errors".
Regression test pins the stale-range guard.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
Collaborator

Merged main in, reviewed, ran the E2E lane — all green

Merged main. This branch was based on 432c207, before #487/#488/#489/#490 landed. Clean merge, no conflicts.

Full local E2E cycle at a03d410, one flock, SAPLING_MODEL_MODE=function:

35 passed (1.3m)
0 finding(s), 1 suppressed (allowlisted)

admin-analytics.spec.ts passes against the restructured dashboard — every selector it uses (the role="img" chart names, admin-analytics-cost-group-model, admin-analytics-range-7d, the event-path table cells) still resolves, and the new "Tokens per day" / "Error rate per day" aria-labels don't substring-collide with the existing "Events per day" / "Errors per day" ones, which is the failure mode a new chart could plausibly have introduced.

Review — no findings. Two agents over correctness and conventions. Things specifically checked and cleared:

  • errorRateSeries division guard (p.value > 0 ? … : 0) — no NaN/Infinity path.
  • The "error dates outside the events domain get dropped" edge is real in the exported helper and is deliberately pinned by its own unit test — but unreachable from the screen: both series are zero-filled from the same range.from/to, and error.4xx/error.5xx are rows in the same events table, so an error day always has ≥1 event. The rangesAgree guard from 7be1223 covers the mid-flight range-change case, with a regression test.
  • formatCompactCount boundaries hand-verified at 999 / 1000 / 999,950 / 1,000,000 — the 999.95.toFixed(1)"1M" promotion behaves as its comment claims.
  • Tokens chart reads p.total_tokens, cost chart reads p.cost_usd — no label/data mismatch on any new chart (the kind of bug a test asserting only the aria-label would wave through).
  • The stale-response race from feat(analytics): #121 data layer — bucket param, typed client, hooks, raw admin page #478's review is still guarded by the seqRef monotonic sequence in useAdminAnalytics.ts; not reintroduced.
  • Testid convention, both halves: the PR adds no <button>/<input>/<textarea> (the new <details>/<summary>/<table> aren't matched by the enforcement selector), so nothing needed registering in docs/frontend-testids.md or eslint.config.mjs.
  • No charting dependency added — package.json/lockfile untouched, still hand-rolled SVG.

Gates: tsc --noEmit clean · npm run lint 0 errors · npx vitest run 58 files / 415 tests.

Merging.

@AndresL230
AndresL230 merged commit 622b932 into mainJul 31, 2026
6 checks passed
@AndresL230
AndresL230 deleted the feat/122-analytics-gaps branch August 2, 2026 18:30
Sign up for freeto 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.

[P3] Observability: admin analytics dashboard UI (charts/visualizations)

2 participants

@Jose-Gael-Cruz-Lopez@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks by Jose-Gael-Cruz-Lopez · Pull Request #491 · SaplingLearn/Sapling · GitHub
Skip to content

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks - #491

Merged
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps
Jul 31, 2026
Merged

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks#491
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps

Conversation

@Jose-Gael-Cruz-Lopez

@Jose-Gael-Cruz-LopezJose-Gael-Cruz-Lopez commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

PR #479 shipped the #122 dashboard, but a criterion-by-criterion audit against the issue spec found three concrete gaps. This PR closes them:

  1. "Cost & tokens over time" — tokens were never charted. The LLM cost panel now projects the CostDayPoint series into zero-filled day series and renders a second lazy DayLineChart ("Tokens per day"), with a compact-count axis formatter (formatCompactCount in daySeries.ts) so token magnitudes fit the y-gutter. No backend work — the API already returned calls/total_tokens per day.
  2. "Error-rate trend" was an error-count trend. New JSX-free errorRateSeries joins the errors/day series with the usage summary's events/day series by date (both zero-filled over the panel range): rate = errors ÷ events as a percentage; zero-event days report 0, never NaN/Infinity. The absolute errors/day chart stays; the rate chart renders alongside it (muted note if the usage series hasn't loaded).
  3. The three line charts had no table fallback — day values were reachable only by pointer hover. New DaySeriesTable mirrors the existing "View data" <details> pattern for every day-series chart (usage events/day; cost calls/tokens/cost; errors count/rate). Also wraps all minWidth tables in an overflowX: auto container so narrow viewports scroll the table, not the page.

Constraints preserved: hand-rolled SVG only (no chart lib), next/dynamic + ssr:false lazy-loading and the JSX-free daySeries.ts split untouched, role="img" + aria-labels on new charts, brand tokens, no glass/gradient-text, and every name/testid asserted by e2e/admin-analytics.spec.ts unchanged.

Verification

  • vitest on touched files: 25 tests pass — the old View data ≥ 2 smoke assert is replaced with panel-scoped assertions of specific day values (incl. a 25% rate = 1 error ÷ 4 events fixture)
  • tsc --noEmit clean; eslint exit 0 on all touched files
  • ⚠️ Local E2E lane not runnable on this machine (unprovisioned stack — details in PR perf(frontend): #111 runtime animations + lazy/next-image loading #490); e2e.yml runs the admin-analytics journey on push to main. All names/testids that journey asserts are unchanged.

Closes#122

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added accessible data tables and “View data” fallbacks for analytics charts.
    • Added daily error-rate reporting based on usage events.
    • Added daily usage event charts and expanded LLM cost reporting for tokens, calls, and cost.
    • Added compact formatting for large counts.
  • Improvements

    • Improved responsive display of analytics tables.
    • Missing daily usage and error values are now shown as zero.

…series table fallbacks
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@supabase

supabaseBot commented Jul 31, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitaiBot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 04e4f8ea-eac5-44ee-9c15-ba36a7ea6208

📥 Commits

Reviewing files that changed from the base of the PR and between f135770 and a03d410.

📒 Files selected for processing (2)
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
📝 Walkthrough

Walkthrough

The analytics dashboard adds daily error-rate calculations, compact count formatting, accessible chart data tables, responsive raw tables, and expanded usage, cost, token, call, and error visualizations with integration coverage.

Changes

Analytics dashboard

Layer / File(s)Summary
Daily series utilities
frontend/src/lib/daySeries.ts, frontend/src/components/AnalyticsCharts.tsx, frontend/src/components/AnalyticsCharts.test.tsx
Adds errorRateSeries and formatCompactCount, re-exports them, and tests date joining, zero-event handling, domain filtering, and rounding boundaries.
Chart data tables and metric panels
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Adds DaySeriesTable, responsive table wrappers, zero-filled usage data, daily cost/token/call projections, and labeled chart fallbacks. Tests cover the expanded chart set and table values.
Error-rate dashboard flow
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Calculates and displays daily error rates against usage events, including rate states, error tables, zero-filled days, and accessibility assertions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant AdminAnalytics
participant errorRateSeries
participant DaySeriesTable
AdminAnalytics->>errorRateSeries: pass zero-filled errors and usage events
errorRateSeries-->>AdminAnalytics: return daily error-rate points
AdminAnalytics->>DaySeriesTable: pass errors and rate series
DaySeriesTable-->>AdminAnalytics: render expandable daily data table
Loading

Possibly related PRs

Suggested reviewers:andresl230

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the three analytics dashboard gaps addressed by this pull request.
Description check✅ PassedThe description provides a detailed summary, related issue, implementation details, and verification results, although it uses alternate section headings.
Linked Issues check✅ PassedThe changes address issue #122 by adding token trends, error-rate trends, accessible table fallbacks, and responsive chart data tables.
Out of Scope Changes check✅ PassedThe changes are limited to analytics charts, calculations, accessibility fallbacks, responsive tables, and related tests.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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/122-analytics-gaps

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.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 31, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga03d410Commit Preview URL

Branch Preview URL
Jul 31 2026, 06:53 AM

…opy, compact-count boundary
- DaySeriesTable and the two raw-table disclosures now carry per-chart
labels ("View data: errors per day", …) so the five identical summaries
are distinguishable to screen-reader users.
- Errors panel zero-fills unconditionally: a no-error range renders real
0s and a flat line, not em-dash "unknown" cells.
- Rate-chart fallback copy branches on summary error/loaded-empty/loading
instead of always promising the series will load.
- formatCompactCount promotes 999,950+ to "1M" instead of emitting
"1000k"; boundary cases and the errorRateSeries domain-drop contract
pinned in unit tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
MemberAuthor

Automated review pass (Claude Code) on a72a5ad. The reviewer independently re-ran the touched tests (25/25), tsc, and eslint in a throwaway worktree, and probed the new helpers directly with Node. Verdict on the core: the rate join is sound on every reachable path — UTC day keys on both sides (backend _bucket_datezeroFillDays' toISOString()), NaN-proof zero-event guard, errors a strict subset of the events domain so the join can't drop an error day; lazy-loading structure and every locator e2e/admin-analytics.spec.ts asserts verified intact.

No criticals. The review's minors are now fixed in f135770:

  • Five identical "View data" disclosures → per-chart labels ("View data: errors per day", …), with the unit test asserting each.
  • Errors panel now zero-fills unconditionally, so a no-error range shows real 0s and a flat line instead of em-dash "unknown" cells.
  • The rate-chart fallback copy branches on summary error / loaded-but-empty / still-loading instead of always promising the series will load.
  • formatCompactCount(999_950) returned "1000k" → now promotes to "1M"; unit boundaries (999/1000/999,949/999,950) and the errorRateSeries domain-drop contract are pinned in tests.

Not addressed (accepted, with reasoning): rate can exceed 100% if the summary scan hits the 100k _SCAN_CAP while the errors scan doesn't — the TruncatedBadge renders in the Usage panel rather than beside the rate chart. Extreme edge; worth badge propagation only if truncation becomes common.

Outstanding process item (already flagged in the PR body): the pre-merge local E2E lane couldn't run on this machine — the admin-analytics journey should be run on a provisioned machine before merge, or watch the post-merge e2e.yml run closely. A follow-up journey step asserting the "Error rate per day" chart and one DaySeriesTable value would give the new surfaces lane coverage.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@frontend/src/components/screens/AdminAnalytics.tsx`:
- Around line 471-516: In the Errors panel render block, validate that
summary.data belongs to the current d.range before constructing eventPoints or
calling errorRateSeries; when summary.range differs, leave the denominator
unavailable so the rate shows the invalid/loading state rather than fabricated
0% values. Update the fallback message branching to distinguish this range
mismatch from summary.error and a genuinely empty events series.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f7cfa34d-f710-4538-9c93-b3ede3e984bd

📥 Commits

Reviewing files that changed from the base of the PR and between 432c207 and f135770.

📒 Files selected for processing (5)
  • frontend/src/components/AnalyticsCharts.test.tsx
  • frontend/src/components/AnalyticsCharts.tsx
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
  • frontend/src/lib/daySeries.ts

Comment threadfrontend/src/components/screens/AdminAnalytics.tsx
Jose-Gael-Cruz-Lopezand others added 2 commits July 30, 2026 23:29
…ry ranges
The summary hook keeps its last payload while a range change reloads, so
the errors panel could zero-fill an old-range series against the new
range and fabricate an all-0% rate line. The join now requires day-key
range agreement; mismatch renders the waiting state. Also: TruncatedBadge
propagates to the rate chart when the summary scan was truncated (the
denominator undercounts, so the rate can overshoot), and formatPct floors
tiny nonzero rates at "<0.1%" so they never read as "no errors".
Regression test pins the stale-range guard.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
Collaborator

Merged main in, reviewed, ran the E2E lane — all green

Merged main. This branch was based on 432c207, before #487/#488/#489/#490 landed. Clean merge, no conflicts.

Full local E2E cycle at a03d410, one flock, SAPLING_MODEL_MODE=function:

35 passed (1.3m)
0 finding(s), 1 suppressed (allowlisted)

admin-analytics.spec.ts passes against the restructured dashboard — every selector it uses (the role="img" chart names, admin-analytics-cost-group-model, admin-analytics-range-7d, the event-path table cells) still resolves, and the new "Tokens per day" / "Error rate per day" aria-labels don't substring-collide with the existing "Events per day" / "Errors per day" ones, which is the failure mode a new chart could plausibly have introduced.

Review — no findings. Two agents over correctness and conventions. Things specifically checked and cleared:

  • errorRateSeries division guard (p.value > 0 ? … : 0) — no NaN/Infinity path.
  • The "error dates outside the events domain get dropped" edge is real in the exported helper and is deliberately pinned by its own unit test — but unreachable from the screen: both series are zero-filled from the same range.from/to, and error.4xx/error.5xx are rows in the same events table, so an error day always has ≥1 event. The rangesAgree guard from 7be1223 covers the mid-flight range-change case, with a regression test.
  • formatCompactCount boundaries hand-verified at 999 / 1000 / 999,950 / 1,000,000 — the 999.95.toFixed(1)"1M" promotion behaves as its comment claims.
  • Tokens chart reads p.total_tokens, cost chart reads p.cost_usd — no label/data mismatch on any new chart (the kind of bug a test asserting only the aria-label would wave through).
  • The stale-response race from feat(analytics): #121 data layer — bucket param, typed client, hooks, raw admin page #478's review is still guarded by the seqRef monotonic sequence in useAdminAnalytics.ts; not reintroduced.
  • Testid convention, both halves: the PR adds no <button>/<input>/<textarea> (the new <details>/<summary>/<table> aren't matched by the enforcement selector), so nothing needed registering in docs/frontend-testids.md or eslint.config.mjs.
  • No charting dependency added — package.json/lockfile untouched, still hand-rolled SVG.

Gates: tsc --noEmit clean · npm run lint 0 errors · npx vitest run 58 files / 415 tests.

Merging.

@AndresL230
AndresL230 merged commit 622b932 into mainJul 31, 2026
6 checks passed
@AndresL230
AndresL230 deleted the feat/122-analytics-gaps branch August 2, 2026 18:30
Sign up for freeto 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.

[P3] Observability: admin analytics dashboard UI (charts/visualizations)

2 participants

@Jose-Gael-Cruz-Lopez@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks by Jose-Gael-Cruz-Lopez · Pull Request #491 · SaplingLearn/Sapling · GitHub
Skip to content

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks - #491

Merged
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps
Jul 31, 2026
Merged

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks#491
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps

Conversation

@Jose-Gael-Cruz-Lopez

@Jose-Gael-Cruz-LopezJose-Gael-Cruz-Lopez commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

PR #479 shipped the #122 dashboard, but a criterion-by-criterion audit against the issue spec found three concrete gaps. This PR closes them:

  1. "Cost & tokens over time" — tokens were never charted. The LLM cost panel now projects the CostDayPoint series into zero-filled day series and renders a second lazy DayLineChart ("Tokens per day"), with a compact-count axis formatter (formatCompactCount in daySeries.ts) so token magnitudes fit the y-gutter. No backend work — the API already returned calls/total_tokens per day.
  2. "Error-rate trend" was an error-count trend. New JSX-free errorRateSeries joins the errors/day series with the usage summary's events/day series by date (both zero-filled over the panel range): rate = errors ÷ events as a percentage; zero-event days report 0, never NaN/Infinity. The absolute errors/day chart stays; the rate chart renders alongside it (muted note if the usage series hasn't loaded).
  3. The three line charts had no table fallback — day values were reachable only by pointer hover. New DaySeriesTable mirrors the existing "View data" <details> pattern for every day-series chart (usage events/day; cost calls/tokens/cost; errors count/rate). Also wraps all minWidth tables in an overflowX: auto container so narrow viewports scroll the table, not the page.

Constraints preserved: hand-rolled SVG only (no chart lib), next/dynamic + ssr:false lazy-loading and the JSX-free daySeries.ts split untouched, role="img" + aria-labels on new charts, brand tokens, no glass/gradient-text, and every name/testid asserted by e2e/admin-analytics.spec.ts unchanged.

Verification

  • vitest on touched files: 25 tests pass — the old View data ≥ 2 smoke assert is replaced with panel-scoped assertions of specific day values (incl. a 25% rate = 1 error ÷ 4 events fixture)
  • tsc --noEmit clean; eslint exit 0 on all touched files
  • ⚠️ Local E2E lane not runnable on this machine (unprovisioned stack — details in PR perf(frontend): #111 runtime animations + lazy/next-image loading #490); e2e.yml runs the admin-analytics journey on push to main. All names/testids that journey asserts are unchanged.

Closes#122

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added accessible data tables and “View data” fallbacks for analytics charts.
    • Added daily error-rate reporting based on usage events.
    • Added daily usage event charts and expanded LLM cost reporting for tokens, calls, and cost.
    • Added compact formatting for large counts.
  • Improvements

    • Improved responsive display of analytics tables.
    • Missing daily usage and error values are now shown as zero.

…series table fallbacks
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@supabase

supabaseBot commented Jul 31, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitaiBot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 04e4f8ea-eac5-44ee-9c15-ba36a7ea6208

📥 Commits

Reviewing files that changed from the base of the PR and between f135770 and a03d410.

📒 Files selected for processing (2)
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
📝 Walkthrough

Walkthrough

The analytics dashboard adds daily error-rate calculations, compact count formatting, accessible chart data tables, responsive raw tables, and expanded usage, cost, token, call, and error visualizations with integration coverage.

Changes

Analytics dashboard

Layer / File(s)Summary
Daily series utilities
frontend/src/lib/daySeries.ts, frontend/src/components/AnalyticsCharts.tsx, frontend/src/components/AnalyticsCharts.test.tsx
Adds errorRateSeries and formatCompactCount, re-exports them, and tests date joining, zero-event handling, domain filtering, and rounding boundaries.
Chart data tables and metric panels
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Adds DaySeriesTable, responsive table wrappers, zero-filled usage data, daily cost/token/call projections, and labeled chart fallbacks. Tests cover the expanded chart set and table values.
Error-rate dashboard flow
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Calculates and displays daily error rates against usage events, including rate states, error tables, zero-filled days, and accessibility assertions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant AdminAnalytics
participant errorRateSeries
participant DaySeriesTable
AdminAnalytics->>errorRateSeries: pass zero-filled errors and usage events
errorRateSeries-->>AdminAnalytics: return daily error-rate points
AdminAnalytics->>DaySeriesTable: pass errors and rate series
DaySeriesTable-->>AdminAnalytics: render expandable daily data table
Loading

Possibly related PRs

Suggested reviewers:andresl230

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the three analytics dashboard gaps addressed by this pull request.
Description check✅ PassedThe description provides a detailed summary, related issue, implementation details, and verification results, although it uses alternate section headings.
Linked Issues check✅ PassedThe changes address issue #122 by adding token trends, error-rate trends, accessible table fallbacks, and responsive chart data tables.
Out of Scope Changes check✅ PassedThe changes are limited to analytics charts, calculations, accessibility fallbacks, responsive tables, and related tests.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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/122-analytics-gaps

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.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 31, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga03d410Commit Preview URL

Branch Preview URL
Jul 31 2026, 06:53 AM

…opy, compact-count boundary
- DaySeriesTable and the two raw-table disclosures now carry per-chart
labels ("View data: errors per day", …) so the five identical summaries
are distinguishable to screen-reader users.
- Errors panel zero-fills unconditionally: a no-error range renders real
0s and a flat line, not em-dash "unknown" cells.
- Rate-chart fallback copy branches on summary error/loaded-empty/loading
instead of always promising the series will load.
- formatCompactCount promotes 999,950+ to "1M" instead of emitting
"1000k"; boundary cases and the errorRateSeries domain-drop contract
pinned in unit tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
MemberAuthor

Automated review pass (Claude Code) on a72a5ad. The reviewer independently re-ran the touched tests (25/25), tsc, and eslint in a throwaway worktree, and probed the new helpers directly with Node. Verdict on the core: the rate join is sound on every reachable path — UTC day keys on both sides (backend _bucket_datezeroFillDays' toISOString()), NaN-proof zero-event guard, errors a strict subset of the events domain so the join can't drop an error day; lazy-loading structure and every locator e2e/admin-analytics.spec.ts asserts verified intact.

No criticals. The review's minors are now fixed in f135770:

  • Five identical "View data" disclosures → per-chart labels ("View data: errors per day", …), with the unit test asserting each.
  • Errors panel now zero-fills unconditionally, so a no-error range shows real 0s and a flat line instead of em-dash "unknown" cells.
  • The rate-chart fallback copy branches on summary error / loaded-but-empty / still-loading instead of always promising the series will load.
  • formatCompactCount(999_950) returned "1000k" → now promotes to "1M"; unit boundaries (999/1000/999,949/999,950) and the errorRateSeries domain-drop contract are pinned in tests.

Not addressed (accepted, with reasoning): rate can exceed 100% if the summary scan hits the 100k _SCAN_CAP while the errors scan doesn't — the TruncatedBadge renders in the Usage panel rather than beside the rate chart. Extreme edge; worth badge propagation only if truncation becomes common.

Outstanding process item (already flagged in the PR body): the pre-merge local E2E lane couldn't run on this machine — the admin-analytics journey should be run on a provisioned machine before merge, or watch the post-merge e2e.yml run closely. A follow-up journey step asserting the "Error rate per day" chart and one DaySeriesTable value would give the new surfaces lane coverage.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@frontend/src/components/screens/AdminAnalytics.tsx`:
- Around line 471-516: In the Errors panel render block, validate that
summary.data belongs to the current d.range before constructing eventPoints or
calling errorRateSeries; when summary.range differs, leave the denominator
unavailable so the rate shows the invalid/loading state rather than fabricated
0% values. Update the fallback message branching to distinguish this range
mismatch from summary.error and a genuinely empty events series.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f7cfa34d-f710-4538-9c93-b3ede3e984bd

📥 Commits

Reviewing files that changed from the base of the PR and between 432c207 and f135770.

📒 Files selected for processing (5)
  • frontend/src/components/AnalyticsCharts.test.tsx
  • frontend/src/components/AnalyticsCharts.tsx
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
  • frontend/src/lib/daySeries.ts

Comment threadfrontend/src/components/screens/AdminAnalytics.tsx
Jose-Gael-Cruz-Lopezand others added 2 commits July 30, 2026 23:29
…ry ranges
The summary hook keeps its last payload while a range change reloads, so
the errors panel could zero-fill an old-range series against the new
range and fabricate an all-0% rate line. The join now requires day-key
range agreement; mismatch renders the waiting state. Also: TruncatedBadge
propagates to the rate chart when the summary scan was truncated (the
denominator undercounts, so the rate can overshoot), and formatPct floors
tiny nonzero rates at "<0.1%" so they never read as "no errors".
Regression test pins the stale-range guard.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
Collaborator

Merged main in, reviewed, ran the E2E lane — all green

Merged main. This branch was based on 432c207, before #487/#488/#489/#490 landed. Clean merge, no conflicts.

Full local E2E cycle at a03d410, one flock, SAPLING_MODEL_MODE=function:

35 passed (1.3m)
0 finding(s), 1 suppressed (allowlisted)

admin-analytics.spec.ts passes against the restructured dashboard — every selector it uses (the role="img" chart names, admin-analytics-cost-group-model, admin-analytics-range-7d, the event-path table cells) still resolves, and the new "Tokens per day" / "Error rate per day" aria-labels don't substring-collide with the existing "Events per day" / "Errors per day" ones, which is the failure mode a new chart could plausibly have introduced.

Review — no findings. Two agents over correctness and conventions. Things specifically checked and cleared:

  • errorRateSeries division guard (p.value > 0 ? … : 0) — no NaN/Infinity path.
  • The "error dates outside the events domain get dropped" edge is real in the exported helper and is deliberately pinned by its own unit test — but unreachable from the screen: both series are zero-filled from the same range.from/to, and error.4xx/error.5xx are rows in the same events table, so an error day always has ≥1 event. The rangesAgree guard from 7be1223 covers the mid-flight range-change case, with a regression test.
  • formatCompactCount boundaries hand-verified at 999 / 1000 / 999,950 / 1,000,000 — the 999.95.toFixed(1)"1M" promotion behaves as its comment claims.
  • Tokens chart reads p.total_tokens, cost chart reads p.cost_usd — no label/data mismatch on any new chart (the kind of bug a test asserting only the aria-label would wave through).
  • The stale-response race from feat(analytics): #121 data layer — bucket param, typed client, hooks, raw admin page #478's review is still guarded by the seqRef monotonic sequence in useAdminAnalytics.ts; not reintroduced.
  • Testid convention, both halves: the PR adds no <button>/<input>/<textarea> (the new <details>/<summary>/<table> aren't matched by the enforcement selector), so nothing needed registering in docs/frontend-testids.md or eslint.config.mjs.
  • No charting dependency added — package.json/lockfile untouched, still hand-rolled SVG.

Gates: tsc --noEmit clean · npm run lint 0 errors · npx vitest run 58 files / 415 tests.

Merging.

@AndresL230
AndresL230 merged commit 622b932 into mainJul 31, 2026
6 checks passed
@AndresL230
AndresL230 deleted the feat/122-analytics-gaps branch August 2, 2026 18:30
Sign up for freeto 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.

[P3] Observability: admin analytics dashboard UI (charts/visualizations)

2 participants

@Jose-Gael-Cruz-Lopez@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks by Jose-Gael-Cruz-Lopez · Pull Request #491 · SaplingLearn/Sapling · GitHub
Skip to content

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks - #491

Merged
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps
Jul 31, 2026
Merged

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks#491
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps

Conversation

@Jose-Gael-Cruz-Lopez

@Jose-Gael-Cruz-LopezJose-Gael-Cruz-Lopez commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

PR #479 shipped the #122 dashboard, but a criterion-by-criterion audit against the issue spec found three concrete gaps. This PR closes them:

  1. "Cost & tokens over time" — tokens were never charted. The LLM cost panel now projects the CostDayPoint series into zero-filled day series and renders a second lazy DayLineChart ("Tokens per day"), with a compact-count axis formatter (formatCompactCount in daySeries.ts) so token magnitudes fit the y-gutter. No backend work — the API already returned calls/total_tokens per day.
  2. "Error-rate trend" was an error-count trend. New JSX-free errorRateSeries joins the errors/day series with the usage summary's events/day series by date (both zero-filled over the panel range): rate = errors ÷ events as a percentage; zero-event days report 0, never NaN/Infinity. The absolute errors/day chart stays; the rate chart renders alongside it (muted note if the usage series hasn't loaded).
  3. The three line charts had no table fallback — day values were reachable only by pointer hover. New DaySeriesTable mirrors the existing "View data" <details> pattern for every day-series chart (usage events/day; cost calls/tokens/cost; errors count/rate). Also wraps all minWidth tables in an overflowX: auto container so narrow viewports scroll the table, not the page.

Constraints preserved: hand-rolled SVG only (no chart lib), next/dynamic + ssr:false lazy-loading and the JSX-free daySeries.ts split untouched, role="img" + aria-labels on new charts, brand tokens, no glass/gradient-text, and every name/testid asserted by e2e/admin-analytics.spec.ts unchanged.

Verification

  • vitest on touched files: 25 tests pass — the old View data ≥ 2 smoke assert is replaced with panel-scoped assertions of specific day values (incl. a 25% rate = 1 error ÷ 4 events fixture)
  • tsc --noEmit clean; eslint exit 0 on all touched files
  • ⚠️ Local E2E lane not runnable on this machine (unprovisioned stack — details in PR perf(frontend): #111 runtime animations + lazy/next-image loading #490); e2e.yml runs the admin-analytics journey on push to main. All names/testids that journey asserts are unchanged.

Closes#122

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added accessible data tables and “View data” fallbacks for analytics charts.
    • Added daily error-rate reporting based on usage events.
    • Added daily usage event charts and expanded LLM cost reporting for tokens, calls, and cost.
    • Added compact formatting for large counts.
  • Improvements

    • Improved responsive display of analytics tables.
    • Missing daily usage and error values are now shown as zero.

…series table fallbacks
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@supabase

supabaseBot commented Jul 31, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitaiBot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 04e4f8ea-eac5-44ee-9c15-ba36a7ea6208

📥 Commits

Reviewing files that changed from the base of the PR and between f135770 and a03d410.

📒 Files selected for processing (2)
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
📝 Walkthrough

Walkthrough

The analytics dashboard adds daily error-rate calculations, compact count formatting, accessible chart data tables, responsive raw tables, and expanded usage, cost, token, call, and error visualizations with integration coverage.

Changes

Analytics dashboard

Layer / File(s)Summary
Daily series utilities
frontend/src/lib/daySeries.ts, frontend/src/components/AnalyticsCharts.tsx, frontend/src/components/AnalyticsCharts.test.tsx
Adds errorRateSeries and formatCompactCount, re-exports them, and tests date joining, zero-event handling, domain filtering, and rounding boundaries.
Chart data tables and metric panels
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Adds DaySeriesTable, responsive table wrappers, zero-filled usage data, daily cost/token/call projections, and labeled chart fallbacks. Tests cover the expanded chart set and table values.
Error-rate dashboard flow
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Calculates and displays daily error rates against usage events, including rate states, error tables, zero-filled days, and accessibility assertions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant AdminAnalytics
participant errorRateSeries
participant DaySeriesTable
AdminAnalytics->>errorRateSeries: pass zero-filled errors and usage events
errorRateSeries-->>AdminAnalytics: return daily error-rate points
AdminAnalytics->>DaySeriesTable: pass errors and rate series
DaySeriesTable-->>AdminAnalytics: render expandable daily data table
Loading

Possibly related PRs

Suggested reviewers:andresl230

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the three analytics dashboard gaps addressed by this pull request.
Description check✅ PassedThe description provides a detailed summary, related issue, implementation details, and verification results, although it uses alternate section headings.
Linked Issues check✅ PassedThe changes address issue #122 by adding token trends, error-rate trends, accessible table fallbacks, and responsive chart data tables.
Out of Scope Changes check✅ PassedThe changes are limited to analytics charts, calculations, accessibility fallbacks, responsive tables, and related tests.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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/122-analytics-gaps

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.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 31, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga03d410Commit Preview URL

Branch Preview URL
Jul 31 2026, 06:53 AM

…opy, compact-count boundary
- DaySeriesTable and the two raw-table disclosures now carry per-chart
labels ("View data: errors per day", …) so the five identical summaries
are distinguishable to screen-reader users.
- Errors panel zero-fills unconditionally: a no-error range renders real
0s and a flat line, not em-dash "unknown" cells.
- Rate-chart fallback copy branches on summary error/loaded-empty/loading
instead of always promising the series will load.
- formatCompactCount promotes 999,950+ to "1M" instead of emitting
"1000k"; boundary cases and the errorRateSeries domain-drop contract
pinned in unit tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
MemberAuthor

Automated review pass (Claude Code) on a72a5ad. The reviewer independently re-ran the touched tests (25/25), tsc, and eslint in a throwaway worktree, and probed the new helpers directly with Node. Verdict on the core: the rate join is sound on every reachable path — UTC day keys on both sides (backend _bucket_datezeroFillDays' toISOString()), NaN-proof zero-event guard, errors a strict subset of the events domain so the join can't drop an error day; lazy-loading structure and every locator e2e/admin-analytics.spec.ts asserts verified intact.

No criticals. The review's minors are now fixed in f135770:

  • Five identical "View data" disclosures → per-chart labels ("View data: errors per day", …), with the unit test asserting each.
  • Errors panel now zero-fills unconditionally, so a no-error range shows real 0s and a flat line instead of em-dash "unknown" cells.
  • The rate-chart fallback copy branches on summary error / loaded-but-empty / still-loading instead of always promising the series will load.
  • formatCompactCount(999_950) returned "1000k" → now promotes to "1M"; unit boundaries (999/1000/999,949/999,950) and the errorRateSeries domain-drop contract are pinned in tests.

Not addressed (accepted, with reasoning): rate can exceed 100% if the summary scan hits the 100k _SCAN_CAP while the errors scan doesn't — the TruncatedBadge renders in the Usage panel rather than beside the rate chart. Extreme edge; worth badge propagation only if truncation becomes common.

Outstanding process item (already flagged in the PR body): the pre-merge local E2E lane couldn't run on this machine — the admin-analytics journey should be run on a provisioned machine before merge, or watch the post-merge e2e.yml run closely. A follow-up journey step asserting the "Error rate per day" chart and one DaySeriesTable value would give the new surfaces lane coverage.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@frontend/src/components/screens/AdminAnalytics.tsx`:
- Around line 471-516: In the Errors panel render block, validate that
summary.data belongs to the current d.range before constructing eventPoints or
calling errorRateSeries; when summary.range differs, leave the denominator
unavailable so the rate shows the invalid/loading state rather than fabricated
0% values. Update the fallback message branching to distinguish this range
mismatch from summary.error and a genuinely empty events series.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f7cfa34d-f710-4538-9c93-b3ede3e984bd

📥 Commits

Reviewing files that changed from the base of the PR and between 432c207 and f135770.

📒 Files selected for processing (5)
  • frontend/src/components/AnalyticsCharts.test.tsx
  • frontend/src/components/AnalyticsCharts.tsx
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
  • frontend/src/lib/daySeries.ts

Comment threadfrontend/src/components/screens/AdminAnalytics.tsx
Jose-Gael-Cruz-Lopezand others added 2 commits July 30, 2026 23:29
…ry ranges
The summary hook keeps its last payload while a range change reloads, so
the errors panel could zero-fill an old-range series against the new
range and fabricate an all-0% rate line. The join now requires day-key
range agreement; mismatch renders the waiting state. Also: TruncatedBadge
propagates to the rate chart when the summary scan was truncated (the
denominator undercounts, so the rate can overshoot), and formatPct floors
tiny nonzero rates at "<0.1%" so they never read as "no errors".
Regression test pins the stale-range guard.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
Collaborator

Merged main in, reviewed, ran the E2E lane — all green

Merged main. This branch was based on 432c207, before #487/#488/#489/#490 landed. Clean merge, no conflicts.

Full local E2E cycle at a03d410, one flock, SAPLING_MODEL_MODE=function:

35 passed (1.3m)
0 finding(s), 1 suppressed (allowlisted)

admin-analytics.spec.ts passes against the restructured dashboard — every selector it uses (the role="img" chart names, admin-analytics-cost-group-model, admin-analytics-range-7d, the event-path table cells) still resolves, and the new "Tokens per day" / "Error rate per day" aria-labels don't substring-collide with the existing "Events per day" / "Errors per day" ones, which is the failure mode a new chart could plausibly have introduced.

Review — no findings. Two agents over correctness and conventions. Things specifically checked and cleared:

  • errorRateSeries division guard (p.value > 0 ? … : 0) — no NaN/Infinity path.
  • The "error dates outside the events domain get dropped" edge is real in the exported helper and is deliberately pinned by its own unit test — but unreachable from the screen: both series are zero-filled from the same range.from/to, and error.4xx/error.5xx are rows in the same events table, so an error day always has ≥1 event. The rangesAgree guard from 7be1223 covers the mid-flight range-change case, with a regression test.
  • formatCompactCount boundaries hand-verified at 999 / 1000 / 999,950 / 1,000,000 — the 999.95.toFixed(1)"1M" promotion behaves as its comment claims.
  • Tokens chart reads p.total_tokens, cost chart reads p.cost_usd — no label/data mismatch on any new chart (the kind of bug a test asserting only the aria-label would wave through).
  • The stale-response race from feat(analytics): #121 data layer — bucket param, typed client, hooks, raw admin page #478's review is still guarded by the seqRef monotonic sequence in useAdminAnalytics.ts; not reintroduced.
  • Testid convention, both halves: the PR adds no <button>/<input>/<textarea> (the new <details>/<summary>/<table> aren't matched by the enforcement selector), so nothing needed registering in docs/frontend-testids.md or eslint.config.mjs.
  • No charting dependency added — package.json/lockfile untouched, still hand-rolled SVG.

Gates: tsc --noEmit clean · npm run lint 0 errors · npx vitest run 58 files / 415 tests.

Merging.

@AndresL230
AndresL230 merged commit 622b932 into mainJul 31, 2026
6 checks passed
@AndresL230
AndresL230 deleted the feat/122-analytics-gaps branch August 2, 2026 18:30
Sign up for freeto 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.

[P3] Observability: admin analytics dashboard UI (charts/visualizations)

2 participants

@Jose-Gael-Cruz-Lopez@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks by Jose-Gael-Cruz-Lopez · Pull Request #491 · SaplingLearn/Sapling · GitHub
Skip to content

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks - #491

Merged
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps
Jul 31, 2026
Merged

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks#491
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps

Conversation

@Jose-Gael-Cruz-Lopez

@Jose-Gael-Cruz-LopezJose-Gael-Cruz-Lopez commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

PR #479 shipped the #122 dashboard, but a criterion-by-criterion audit against the issue spec found three concrete gaps. This PR closes them:

  1. "Cost & tokens over time" — tokens were never charted. The LLM cost panel now projects the CostDayPoint series into zero-filled day series and renders a second lazy DayLineChart ("Tokens per day"), with a compact-count axis formatter (formatCompactCount in daySeries.ts) so token magnitudes fit the y-gutter. No backend work — the API already returned calls/total_tokens per day.
  2. "Error-rate trend" was an error-count trend. New JSX-free errorRateSeries joins the errors/day series with the usage summary's events/day series by date (both zero-filled over the panel range): rate = errors ÷ events as a percentage; zero-event days report 0, never NaN/Infinity. The absolute errors/day chart stays; the rate chart renders alongside it (muted note if the usage series hasn't loaded).
  3. The three line charts had no table fallback — day values were reachable only by pointer hover. New DaySeriesTable mirrors the existing "View data" <details> pattern for every day-series chart (usage events/day; cost calls/tokens/cost; errors count/rate). Also wraps all minWidth tables in an overflowX: auto container so narrow viewports scroll the table, not the page.

Constraints preserved: hand-rolled SVG only (no chart lib), next/dynamic + ssr:false lazy-loading and the JSX-free daySeries.ts split untouched, role="img" + aria-labels on new charts, brand tokens, no glass/gradient-text, and every name/testid asserted by e2e/admin-analytics.spec.ts unchanged.

Verification

  • vitest on touched files: 25 tests pass — the old View data ≥ 2 smoke assert is replaced with panel-scoped assertions of specific day values (incl. a 25% rate = 1 error ÷ 4 events fixture)
  • tsc --noEmit clean; eslint exit 0 on all touched files
  • ⚠️ Local E2E lane not runnable on this machine (unprovisioned stack — details in PR perf(frontend): #111 runtime animations + lazy/next-image loading #490); e2e.yml runs the admin-analytics journey on push to main. All names/testids that journey asserts are unchanged.

Closes#122

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added accessible data tables and “View data” fallbacks for analytics charts.
    • Added daily error-rate reporting based on usage events.
    • Added daily usage event charts and expanded LLM cost reporting for tokens, calls, and cost.
    • Added compact formatting for large counts.
  • Improvements

    • Improved responsive display of analytics tables.
    • Missing daily usage and error values are now shown as zero.

…series table fallbacks
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@supabase

supabaseBot commented Jul 31, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitaiBot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 04e4f8ea-eac5-44ee-9c15-ba36a7ea6208

📥 Commits

Reviewing files that changed from the base of the PR and between f135770 and a03d410.

📒 Files selected for processing (2)
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
📝 Walkthrough

Walkthrough

The analytics dashboard adds daily error-rate calculations, compact count formatting, accessible chart data tables, responsive raw tables, and expanded usage, cost, token, call, and error visualizations with integration coverage.

Changes

Analytics dashboard

Layer / File(s)Summary
Daily series utilities
frontend/src/lib/daySeries.ts, frontend/src/components/AnalyticsCharts.tsx, frontend/src/components/AnalyticsCharts.test.tsx
Adds errorRateSeries and formatCompactCount, re-exports them, and tests date joining, zero-event handling, domain filtering, and rounding boundaries.
Chart data tables and metric panels
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Adds DaySeriesTable, responsive table wrappers, zero-filled usage data, daily cost/token/call projections, and labeled chart fallbacks. Tests cover the expanded chart set and table values.
Error-rate dashboard flow
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Calculates and displays daily error rates against usage events, including rate states, error tables, zero-filled days, and accessibility assertions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant AdminAnalytics
participant errorRateSeries
participant DaySeriesTable
AdminAnalytics->>errorRateSeries: pass zero-filled errors and usage events
errorRateSeries-->>AdminAnalytics: return daily error-rate points
AdminAnalytics->>DaySeriesTable: pass errors and rate series
DaySeriesTable-->>AdminAnalytics: render expandable daily data table
Loading

Possibly related PRs

Suggested reviewers:andresl230

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the three analytics dashboard gaps addressed by this pull request.
Description check✅ PassedThe description provides a detailed summary, related issue, implementation details, and verification results, although it uses alternate section headings.
Linked Issues check✅ PassedThe changes address issue #122 by adding token trends, error-rate trends, accessible table fallbacks, and responsive chart data tables.
Out of Scope Changes check✅ PassedThe changes are limited to analytics charts, calculations, accessibility fallbacks, responsive tables, and related tests.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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/122-analytics-gaps

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.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 31, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga03d410Commit Preview URL

Branch Preview URL
Jul 31 2026, 06:53 AM

…opy, compact-count boundary
- DaySeriesTable and the two raw-table disclosures now carry per-chart
labels ("View data: errors per day", …) so the five identical summaries
are distinguishable to screen-reader users.
- Errors panel zero-fills unconditionally: a no-error range renders real
0s and a flat line, not em-dash "unknown" cells.
- Rate-chart fallback copy branches on summary error/loaded-empty/loading
instead of always promising the series will load.
- formatCompactCount promotes 999,950+ to "1M" instead of emitting
"1000k"; boundary cases and the errorRateSeries domain-drop contract
pinned in unit tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
MemberAuthor

Automated review pass (Claude Code) on a72a5ad. The reviewer independently re-ran the touched tests (25/25), tsc, and eslint in a throwaway worktree, and probed the new helpers directly with Node. Verdict on the core: the rate join is sound on every reachable path — UTC day keys on both sides (backend _bucket_datezeroFillDays' toISOString()), NaN-proof zero-event guard, errors a strict subset of the events domain so the join can't drop an error day; lazy-loading structure and every locator e2e/admin-analytics.spec.ts asserts verified intact.

No criticals. The review's minors are now fixed in f135770:

  • Five identical "View data" disclosures → per-chart labels ("View data: errors per day", …), with the unit test asserting each.
  • Errors panel now zero-fills unconditionally, so a no-error range shows real 0s and a flat line instead of em-dash "unknown" cells.
  • The rate-chart fallback copy branches on summary error / loaded-but-empty / still-loading instead of always promising the series will load.
  • formatCompactCount(999_950) returned "1000k" → now promotes to "1M"; unit boundaries (999/1000/999,949/999,950) and the errorRateSeries domain-drop contract are pinned in tests.

Not addressed (accepted, with reasoning): rate can exceed 100% if the summary scan hits the 100k _SCAN_CAP while the errors scan doesn't — the TruncatedBadge renders in the Usage panel rather than beside the rate chart. Extreme edge; worth badge propagation only if truncation becomes common.

Outstanding process item (already flagged in the PR body): the pre-merge local E2E lane couldn't run on this machine — the admin-analytics journey should be run on a provisioned machine before merge, or watch the post-merge e2e.yml run closely. A follow-up journey step asserting the "Error rate per day" chart and one DaySeriesTable value would give the new surfaces lane coverage.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@frontend/src/components/screens/AdminAnalytics.tsx`:
- Around line 471-516: In the Errors panel render block, validate that
summary.data belongs to the current d.range before constructing eventPoints or
calling errorRateSeries; when summary.range differs, leave the denominator
unavailable so the rate shows the invalid/loading state rather than fabricated
0% values. Update the fallback message branching to distinguish this range
mismatch from summary.error and a genuinely empty events series.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f7cfa34d-f710-4538-9c93-b3ede3e984bd

📥 Commits

Reviewing files that changed from the base of the PR and between 432c207 and f135770.

📒 Files selected for processing (5)
  • frontend/src/components/AnalyticsCharts.test.tsx
  • frontend/src/components/AnalyticsCharts.tsx
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
  • frontend/src/lib/daySeries.ts

Comment threadfrontend/src/components/screens/AdminAnalytics.tsx
Jose-Gael-Cruz-Lopezand others added 2 commits July 30, 2026 23:29
…ry ranges
The summary hook keeps its last payload while a range change reloads, so
the errors panel could zero-fill an old-range series against the new
range and fabricate an all-0% rate line. The join now requires day-key
range agreement; mismatch renders the waiting state. Also: TruncatedBadge
propagates to the rate chart when the summary scan was truncated (the
denominator undercounts, so the rate can overshoot), and formatPct floors
tiny nonzero rates at "<0.1%" so they never read as "no errors".
Regression test pins the stale-range guard.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
Collaborator

Merged main in, reviewed, ran the E2E lane — all green

Merged main. This branch was based on 432c207, before #487/#488/#489/#490 landed. Clean merge, no conflicts.

Full local E2E cycle at a03d410, one flock, SAPLING_MODEL_MODE=function:

35 passed (1.3m)
0 finding(s), 1 suppressed (allowlisted)

admin-analytics.spec.ts passes against the restructured dashboard — every selector it uses (the role="img" chart names, admin-analytics-cost-group-model, admin-analytics-range-7d, the event-path table cells) still resolves, and the new "Tokens per day" / "Error rate per day" aria-labels don't substring-collide with the existing "Events per day" / "Errors per day" ones, which is the failure mode a new chart could plausibly have introduced.

Review — no findings. Two agents over correctness and conventions. Things specifically checked and cleared:

  • errorRateSeries division guard (p.value > 0 ? … : 0) — no NaN/Infinity path.
  • The "error dates outside the events domain get dropped" edge is real in the exported helper and is deliberately pinned by its own unit test — but unreachable from the screen: both series are zero-filled from the same range.from/to, and error.4xx/error.5xx are rows in the same events table, so an error day always has ≥1 event. The rangesAgree guard from 7be1223 covers the mid-flight range-change case, with a regression test.
  • formatCompactCount boundaries hand-verified at 999 / 1000 / 999,950 / 1,000,000 — the 999.95.toFixed(1)"1M" promotion behaves as its comment claims.
  • Tokens chart reads p.total_tokens, cost chart reads p.cost_usd — no label/data mismatch on any new chart (the kind of bug a test asserting only the aria-label would wave through).
  • The stale-response race from feat(analytics): #121 data layer — bucket param, typed client, hooks, raw admin page #478's review is still guarded by the seqRef monotonic sequence in useAdminAnalytics.ts; not reintroduced.
  • Testid convention, both halves: the PR adds no <button>/<input>/<textarea> (the new <details>/<summary>/<table> aren't matched by the enforcement selector), so nothing needed registering in docs/frontend-testids.md or eslint.config.mjs.
  • No charting dependency added — package.json/lockfile untouched, still hand-rolled SVG.

Gates: tsc --noEmit clean · npm run lint 0 errors · npx vitest run 58 files / 415 tests.

Merging.

@AndresL230
AndresL230 merged commit 622b932 into mainJul 31, 2026
6 checks passed
@AndresL230
AndresL230 deleted the feat/122-analytics-gaps branch August 2, 2026 18:30
Sign up for freeto 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.

[P3] Observability: admin analytics dashboard UI (charts/visualizations)

2 participants

@Jose-Gael-Cruz-Lopez@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks by Jose-Gael-Cruz-Lopez · Pull Request #491 · SaplingLearn/Sapling · GitHub
Skip to content

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks - #491

Merged
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps
Jul 31, 2026
Merged

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks#491
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps

Conversation

@Jose-Gael-Cruz-Lopez

@Jose-Gael-Cruz-LopezJose-Gael-Cruz-Lopez commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

PR #479 shipped the #122 dashboard, but a criterion-by-criterion audit against the issue spec found three concrete gaps. This PR closes them:

  1. "Cost & tokens over time" — tokens were never charted. The LLM cost panel now projects the CostDayPoint series into zero-filled day series and renders a second lazy DayLineChart ("Tokens per day"), with a compact-count axis formatter (formatCompactCount in daySeries.ts) so token magnitudes fit the y-gutter. No backend work — the API already returned calls/total_tokens per day.
  2. "Error-rate trend" was an error-count trend. New JSX-free errorRateSeries joins the errors/day series with the usage summary's events/day series by date (both zero-filled over the panel range): rate = errors ÷ events as a percentage; zero-event days report 0, never NaN/Infinity. The absolute errors/day chart stays; the rate chart renders alongside it (muted note if the usage series hasn't loaded).
  3. The three line charts had no table fallback — day values were reachable only by pointer hover. New DaySeriesTable mirrors the existing "View data" <details> pattern for every day-series chart (usage events/day; cost calls/tokens/cost; errors count/rate). Also wraps all minWidth tables in an overflowX: auto container so narrow viewports scroll the table, not the page.

Constraints preserved: hand-rolled SVG only (no chart lib), next/dynamic + ssr:false lazy-loading and the JSX-free daySeries.ts split untouched, role="img" + aria-labels on new charts, brand tokens, no glass/gradient-text, and every name/testid asserted by e2e/admin-analytics.spec.ts unchanged.

Verification

  • vitest on touched files: 25 tests pass — the old View data ≥ 2 smoke assert is replaced with panel-scoped assertions of specific day values (incl. a 25% rate = 1 error ÷ 4 events fixture)
  • tsc --noEmit clean; eslint exit 0 on all touched files
  • ⚠️ Local E2E lane not runnable on this machine (unprovisioned stack — details in PR perf(frontend): #111 runtime animations + lazy/next-image loading #490); e2e.yml runs the admin-analytics journey on push to main. All names/testids that journey asserts are unchanged.

Closes#122

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added accessible data tables and “View data” fallbacks for analytics charts.
    • Added daily error-rate reporting based on usage events.
    • Added daily usage event charts and expanded LLM cost reporting for tokens, calls, and cost.
    • Added compact formatting for large counts.
  • Improvements

    • Improved responsive display of analytics tables.
    • Missing daily usage and error values are now shown as zero.

…series table fallbacks
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@supabase

supabaseBot commented Jul 31, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitaiBot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 04e4f8ea-eac5-44ee-9c15-ba36a7ea6208

📥 Commits

Reviewing files that changed from the base of the PR and between f135770 and a03d410.

📒 Files selected for processing (2)
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
📝 Walkthrough

Walkthrough

The analytics dashboard adds daily error-rate calculations, compact count formatting, accessible chart data tables, responsive raw tables, and expanded usage, cost, token, call, and error visualizations with integration coverage.

Changes

Analytics dashboard

Layer / File(s)Summary
Daily series utilities
frontend/src/lib/daySeries.ts, frontend/src/components/AnalyticsCharts.tsx, frontend/src/components/AnalyticsCharts.test.tsx
Adds errorRateSeries and formatCompactCount, re-exports them, and tests date joining, zero-event handling, domain filtering, and rounding boundaries.
Chart data tables and metric panels
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Adds DaySeriesTable, responsive table wrappers, zero-filled usage data, daily cost/token/call projections, and labeled chart fallbacks. Tests cover the expanded chart set and table values.
Error-rate dashboard flow
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Calculates and displays daily error rates against usage events, including rate states, error tables, zero-filled days, and accessibility assertions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant AdminAnalytics
participant errorRateSeries
participant DaySeriesTable
AdminAnalytics->>errorRateSeries: pass zero-filled errors and usage events
errorRateSeries-->>AdminAnalytics: return daily error-rate points
AdminAnalytics->>DaySeriesTable: pass errors and rate series
DaySeriesTable-->>AdminAnalytics: render expandable daily data table
Loading

Possibly related PRs

Suggested reviewers:andresl230

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the three analytics dashboard gaps addressed by this pull request.
Description check✅ PassedThe description provides a detailed summary, related issue, implementation details, and verification results, although it uses alternate section headings.
Linked Issues check✅ PassedThe changes address issue #122 by adding token trends, error-rate trends, accessible table fallbacks, and responsive chart data tables.
Out of Scope Changes check✅ PassedThe changes are limited to analytics charts, calculations, accessibility fallbacks, responsive tables, and related tests.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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/122-analytics-gaps

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.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 31, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga03d410Commit Preview URL

Branch Preview URL
Jul 31 2026, 06:53 AM

…opy, compact-count boundary
- DaySeriesTable and the two raw-table disclosures now carry per-chart
labels ("View data: errors per day", …) so the five identical summaries
are distinguishable to screen-reader users.
- Errors panel zero-fills unconditionally: a no-error range renders real
0s and a flat line, not em-dash "unknown" cells.
- Rate-chart fallback copy branches on summary error/loaded-empty/loading
instead of always promising the series will load.
- formatCompactCount promotes 999,950+ to "1M" instead of emitting
"1000k"; boundary cases and the errorRateSeries domain-drop contract
pinned in unit tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
MemberAuthor

Automated review pass (Claude Code) on a72a5ad. The reviewer independently re-ran the touched tests (25/25), tsc, and eslint in a throwaway worktree, and probed the new helpers directly with Node. Verdict on the core: the rate join is sound on every reachable path — UTC day keys on both sides (backend _bucket_datezeroFillDays' toISOString()), NaN-proof zero-event guard, errors a strict subset of the events domain so the join can't drop an error day; lazy-loading structure and every locator e2e/admin-analytics.spec.ts asserts verified intact.

No criticals. The review's minors are now fixed in f135770:

  • Five identical "View data" disclosures → per-chart labels ("View data: errors per day", …), with the unit test asserting each.
  • Errors panel now zero-fills unconditionally, so a no-error range shows real 0s and a flat line instead of em-dash "unknown" cells.
  • The rate-chart fallback copy branches on summary error / loaded-but-empty / still-loading instead of always promising the series will load.
  • formatCompactCount(999_950) returned "1000k" → now promotes to "1M"; unit boundaries (999/1000/999,949/999,950) and the errorRateSeries domain-drop contract are pinned in tests.

Not addressed (accepted, with reasoning): rate can exceed 100% if the summary scan hits the 100k _SCAN_CAP while the errors scan doesn't — the TruncatedBadge renders in the Usage panel rather than beside the rate chart. Extreme edge; worth badge propagation only if truncation becomes common.

Outstanding process item (already flagged in the PR body): the pre-merge local E2E lane couldn't run on this machine — the admin-analytics journey should be run on a provisioned machine before merge, or watch the post-merge e2e.yml run closely. A follow-up journey step asserting the "Error rate per day" chart and one DaySeriesTable value would give the new surfaces lane coverage.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@frontend/src/components/screens/AdminAnalytics.tsx`:
- Around line 471-516: In the Errors panel render block, validate that
summary.data belongs to the current d.range before constructing eventPoints or
calling errorRateSeries; when summary.range differs, leave the denominator
unavailable so the rate shows the invalid/loading state rather than fabricated
0% values. Update the fallback message branching to distinguish this range
mismatch from summary.error and a genuinely empty events series.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f7cfa34d-f710-4538-9c93-b3ede3e984bd

📥 Commits

Reviewing files that changed from the base of the PR and between 432c207 and f135770.

📒 Files selected for processing (5)
  • frontend/src/components/AnalyticsCharts.test.tsx
  • frontend/src/components/AnalyticsCharts.tsx
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
  • frontend/src/lib/daySeries.ts

Comment threadfrontend/src/components/screens/AdminAnalytics.tsx
Jose-Gael-Cruz-Lopezand others added 2 commits July 30, 2026 23:29
…ry ranges
The summary hook keeps its last payload while a range change reloads, so
the errors panel could zero-fill an old-range series against the new
range and fabricate an all-0% rate line. The join now requires day-key
range agreement; mismatch renders the waiting state. Also: TruncatedBadge
propagates to the rate chart when the summary scan was truncated (the
denominator undercounts, so the rate can overshoot), and formatPct floors
tiny nonzero rates at "<0.1%" so they never read as "no errors".
Regression test pins the stale-range guard.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
Collaborator

Merged main in, reviewed, ran the E2E lane — all green

Merged main. This branch was based on 432c207, before #487/#488/#489/#490 landed. Clean merge, no conflicts.

Full local E2E cycle at a03d410, one flock, SAPLING_MODEL_MODE=function:

35 passed (1.3m)
0 finding(s), 1 suppressed (allowlisted)

admin-analytics.spec.ts passes against the restructured dashboard — every selector it uses (the role="img" chart names, admin-analytics-cost-group-model, admin-analytics-range-7d, the event-path table cells) still resolves, and the new "Tokens per day" / "Error rate per day" aria-labels don't substring-collide with the existing "Events per day" / "Errors per day" ones, which is the failure mode a new chart could plausibly have introduced.

Review — no findings. Two agents over correctness and conventions. Things specifically checked and cleared:

  • errorRateSeries division guard (p.value > 0 ? … : 0) — no NaN/Infinity path.
  • The "error dates outside the events domain get dropped" edge is real in the exported helper and is deliberately pinned by its own unit test — but unreachable from the screen: both series are zero-filled from the same range.from/to, and error.4xx/error.5xx are rows in the same events table, so an error day always has ≥1 event. The rangesAgree guard from 7be1223 covers the mid-flight range-change case, with a regression test.
  • formatCompactCount boundaries hand-verified at 999 / 1000 / 999,950 / 1,000,000 — the 999.95.toFixed(1)"1M" promotion behaves as its comment claims.
  • Tokens chart reads p.total_tokens, cost chart reads p.cost_usd — no label/data mismatch on any new chart (the kind of bug a test asserting only the aria-label would wave through).
  • The stale-response race from feat(analytics): #121 data layer — bucket param, typed client, hooks, raw admin page #478's review is still guarded by the seqRef monotonic sequence in useAdminAnalytics.ts; not reintroduced.
  • Testid convention, both halves: the PR adds no <button>/<input>/<textarea> (the new <details>/<summary>/<table> aren't matched by the enforcement selector), so nothing needed registering in docs/frontend-testids.md or eslint.config.mjs.
  • No charting dependency added — package.json/lockfile untouched, still hand-rolled SVG.

Gates: tsc --noEmit clean · npm run lint 0 errors · npx vitest run 58 files / 415 tests.

Merging.

@AndresL230
AndresL230 merged commit 622b932 into mainJul 31, 2026
6 checks passed
@AndresL230
AndresL230 deleted the feat/122-analytics-gaps branch August 2, 2026 18:30
Sign up for freeto 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.

[P3] Observability: admin analytics dashboard UI (charts/visualizations)

2 participants

@Jose-Gael-Cruz-Lopez@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks by Jose-Gael-Cruz-Lopez · Pull Request #491 · SaplingLearn/Sapling · GitHub
Skip to content

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks - #491

Merged
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps
Jul 31, 2026
Merged

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks#491
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps

Conversation

@Jose-Gael-Cruz-Lopez

@Jose-Gael-Cruz-LopezJose-Gael-Cruz-Lopez commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

PR #479 shipped the #122 dashboard, but a criterion-by-criterion audit against the issue spec found three concrete gaps. This PR closes them:

  1. "Cost & tokens over time" — tokens were never charted. The LLM cost panel now projects the CostDayPoint series into zero-filled day series and renders a second lazy DayLineChart ("Tokens per day"), with a compact-count axis formatter (formatCompactCount in daySeries.ts) so token magnitudes fit the y-gutter. No backend work — the API already returned calls/total_tokens per day.
  2. "Error-rate trend" was an error-count trend. New JSX-free errorRateSeries joins the errors/day series with the usage summary's events/day series by date (both zero-filled over the panel range): rate = errors ÷ events as a percentage; zero-event days report 0, never NaN/Infinity. The absolute errors/day chart stays; the rate chart renders alongside it (muted note if the usage series hasn't loaded).
  3. The three line charts had no table fallback — day values were reachable only by pointer hover. New DaySeriesTable mirrors the existing "View data" <details> pattern for every day-series chart (usage events/day; cost calls/tokens/cost; errors count/rate). Also wraps all minWidth tables in an overflowX: auto container so narrow viewports scroll the table, not the page.

Constraints preserved: hand-rolled SVG only (no chart lib), next/dynamic + ssr:false lazy-loading and the JSX-free daySeries.ts split untouched, role="img" + aria-labels on new charts, brand tokens, no glass/gradient-text, and every name/testid asserted by e2e/admin-analytics.spec.ts unchanged.

Verification

  • vitest on touched files: 25 tests pass — the old View data ≥ 2 smoke assert is replaced with panel-scoped assertions of specific day values (incl. a 25% rate = 1 error ÷ 4 events fixture)
  • tsc --noEmit clean; eslint exit 0 on all touched files
  • ⚠️ Local E2E lane not runnable on this machine (unprovisioned stack — details in PR perf(frontend): #111 runtime animations + lazy/next-image loading #490); e2e.yml runs the admin-analytics journey on push to main. All names/testids that journey asserts are unchanged.

Closes#122

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added accessible data tables and “View data” fallbacks for analytics charts.
    • Added daily error-rate reporting based on usage events.
    • Added daily usage event charts and expanded LLM cost reporting for tokens, calls, and cost.
    • Added compact formatting for large counts.
  • Improvements

    • Improved responsive display of analytics tables.
    • Missing daily usage and error values are now shown as zero.

…series table fallbacks
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@supabase

supabaseBot commented Jul 31, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitaiBot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 04e4f8ea-eac5-44ee-9c15-ba36a7ea6208

📥 Commits

Reviewing files that changed from the base of the PR and between f135770 and a03d410.

📒 Files selected for processing (2)
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
📝 Walkthrough

Walkthrough

The analytics dashboard adds daily error-rate calculations, compact count formatting, accessible chart data tables, responsive raw tables, and expanded usage, cost, token, call, and error visualizations with integration coverage.

Changes

Analytics dashboard

Layer / File(s)Summary
Daily series utilities
frontend/src/lib/daySeries.ts, frontend/src/components/AnalyticsCharts.tsx, frontend/src/components/AnalyticsCharts.test.tsx
Adds errorRateSeries and formatCompactCount, re-exports them, and tests date joining, zero-event handling, domain filtering, and rounding boundaries.
Chart data tables and metric panels
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Adds DaySeriesTable, responsive table wrappers, zero-filled usage data, daily cost/token/call projections, and labeled chart fallbacks. Tests cover the expanded chart set and table values.
Error-rate dashboard flow
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Calculates and displays daily error rates against usage events, including rate states, error tables, zero-filled days, and accessibility assertions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant AdminAnalytics
participant errorRateSeries
participant DaySeriesTable
AdminAnalytics->>errorRateSeries: pass zero-filled errors and usage events
errorRateSeries-->>AdminAnalytics: return daily error-rate points
AdminAnalytics->>DaySeriesTable: pass errors and rate series
DaySeriesTable-->>AdminAnalytics: render expandable daily data table
Loading

Possibly related PRs

Suggested reviewers:andresl230

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the three analytics dashboard gaps addressed by this pull request.
Description check✅ PassedThe description provides a detailed summary, related issue, implementation details, and verification results, although it uses alternate section headings.
Linked Issues check✅ PassedThe changes address issue #122 by adding token trends, error-rate trends, accessible table fallbacks, and responsive chart data tables.
Out of Scope Changes check✅ PassedThe changes are limited to analytics charts, calculations, accessibility fallbacks, responsive tables, and related tests.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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/122-analytics-gaps

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.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 31, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga03d410Commit Preview URL

Branch Preview URL
Jul 31 2026, 06:53 AM

…opy, compact-count boundary
- DaySeriesTable and the two raw-table disclosures now carry per-chart
labels ("View data: errors per day", …) so the five identical summaries
are distinguishable to screen-reader users.
- Errors panel zero-fills unconditionally: a no-error range renders real
0s and a flat line, not em-dash "unknown" cells.
- Rate-chart fallback copy branches on summary error/loaded-empty/loading
instead of always promising the series will load.
- formatCompactCount promotes 999,950+ to "1M" instead of emitting
"1000k"; boundary cases and the errorRateSeries domain-drop contract
pinned in unit tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
MemberAuthor

Automated review pass (Claude Code) on a72a5ad. The reviewer independently re-ran the touched tests (25/25), tsc, and eslint in a throwaway worktree, and probed the new helpers directly with Node. Verdict on the core: the rate join is sound on every reachable path — UTC day keys on both sides (backend _bucket_datezeroFillDays' toISOString()), NaN-proof zero-event guard, errors a strict subset of the events domain so the join can't drop an error day; lazy-loading structure and every locator e2e/admin-analytics.spec.ts asserts verified intact.

No criticals. The review's minors are now fixed in f135770:

  • Five identical "View data" disclosures → per-chart labels ("View data: errors per day", …), with the unit test asserting each.
  • Errors panel now zero-fills unconditionally, so a no-error range shows real 0s and a flat line instead of em-dash "unknown" cells.
  • The rate-chart fallback copy branches on summary error / loaded-but-empty / still-loading instead of always promising the series will load.
  • formatCompactCount(999_950) returned "1000k" → now promotes to "1M"; unit boundaries (999/1000/999,949/999,950) and the errorRateSeries domain-drop contract are pinned in tests.

Not addressed (accepted, with reasoning): rate can exceed 100% if the summary scan hits the 100k _SCAN_CAP while the errors scan doesn't — the TruncatedBadge renders in the Usage panel rather than beside the rate chart. Extreme edge; worth badge propagation only if truncation becomes common.

Outstanding process item (already flagged in the PR body): the pre-merge local E2E lane couldn't run on this machine — the admin-analytics journey should be run on a provisioned machine before merge, or watch the post-merge e2e.yml run closely. A follow-up journey step asserting the "Error rate per day" chart and one DaySeriesTable value would give the new surfaces lane coverage.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@frontend/src/components/screens/AdminAnalytics.tsx`:
- Around line 471-516: In the Errors panel render block, validate that
summary.data belongs to the current d.range before constructing eventPoints or
calling errorRateSeries; when summary.range differs, leave the denominator
unavailable so the rate shows the invalid/loading state rather than fabricated
0% values. Update the fallback message branching to distinguish this range
mismatch from summary.error and a genuinely empty events series.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f7cfa34d-f710-4538-9c93-b3ede3e984bd

📥 Commits

Reviewing files that changed from the base of the PR and between 432c207 and f135770.

📒 Files selected for processing (5)
  • frontend/src/components/AnalyticsCharts.test.tsx
  • frontend/src/components/AnalyticsCharts.tsx
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
  • frontend/src/lib/daySeries.ts

Comment threadfrontend/src/components/screens/AdminAnalytics.tsx
Jose-Gael-Cruz-Lopezand others added 2 commits July 30, 2026 23:29
…ry ranges
The summary hook keeps its last payload while a range change reloads, so
the errors panel could zero-fill an old-range series against the new
range and fabricate an all-0% rate line. The join now requires day-key
range agreement; mismatch renders the waiting state. Also: TruncatedBadge
propagates to the rate chart when the summary scan was truncated (the
denominator undercounts, so the rate can overshoot), and formatPct floors
tiny nonzero rates at "<0.1%" so they never read as "no errors".
Regression test pins the stale-range guard.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
Collaborator

Merged main in, reviewed, ran the E2E lane — all green

Merged main. This branch was based on 432c207, before #487/#488/#489/#490 landed. Clean merge, no conflicts.

Full local E2E cycle at a03d410, one flock, SAPLING_MODEL_MODE=function:

35 passed (1.3m)
0 finding(s), 1 suppressed (allowlisted)

admin-analytics.spec.ts passes against the restructured dashboard — every selector it uses (the role="img" chart names, admin-analytics-cost-group-model, admin-analytics-range-7d, the event-path table cells) still resolves, and the new "Tokens per day" / "Error rate per day" aria-labels don't substring-collide with the existing "Events per day" / "Errors per day" ones, which is the failure mode a new chart could plausibly have introduced.

Review — no findings. Two agents over correctness and conventions. Things specifically checked and cleared:

  • errorRateSeries division guard (p.value > 0 ? … : 0) — no NaN/Infinity path.
  • The "error dates outside the events domain get dropped" edge is real in the exported helper and is deliberately pinned by its own unit test — but unreachable from the screen: both series are zero-filled from the same range.from/to, and error.4xx/error.5xx are rows in the same events table, so an error day always has ≥1 event. The rangesAgree guard from 7be1223 covers the mid-flight range-change case, with a regression test.
  • formatCompactCount boundaries hand-verified at 999 / 1000 / 999,950 / 1,000,000 — the 999.95.toFixed(1)"1M" promotion behaves as its comment claims.
  • Tokens chart reads p.total_tokens, cost chart reads p.cost_usd — no label/data mismatch on any new chart (the kind of bug a test asserting only the aria-label would wave through).
  • The stale-response race from feat(analytics): #121 data layer — bucket param, typed client, hooks, raw admin page #478's review is still guarded by the seqRef monotonic sequence in useAdminAnalytics.ts; not reintroduced.
  • Testid convention, both halves: the PR adds no <button>/<input>/<textarea> (the new <details>/<summary>/<table> aren't matched by the enforcement selector), so nothing needed registering in docs/frontend-testids.md or eslint.config.mjs.
  • No charting dependency added — package.json/lockfile untouched, still hand-rolled SVG.

Gates: tsc --noEmit clean · npm run lint 0 errors · npx vitest run 58 files / 415 tests.

Merging.

@AndresL230
AndresL230 merged commit 622b932 into mainJul 31, 2026
6 checks passed
@AndresL230
AndresL230 deleted the feat/122-analytics-gaps branch August 2, 2026 18:30
Sign up for freeto 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.

[P3] Observability: admin analytics dashboard UI (charts/visualizations)

2 participants

@Jose-Gael-Cruz-Lopez@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks by Jose-Gael-Cruz-Lopez · Pull Request #491 · SaplingLearn/Sapling · GitHub
Skip to content

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks - #491

Merged
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps
Jul 31, 2026
Merged

feat(analytics): #122 gaps — tokens/day chart, error-rate trend, day-series table fallbacks#491
AndresL230 merged 4 commits into
mainfrom
feat/122-analytics-gaps

Conversation

@Jose-Gael-Cruz-Lopez

@Jose-Gael-Cruz-LopezJose-Gael-Cruz-Lopez commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

PR #479 shipped the #122 dashboard, but a criterion-by-criterion audit against the issue spec found three concrete gaps. This PR closes them:

  1. "Cost & tokens over time" — tokens were never charted. The LLM cost panel now projects the CostDayPoint series into zero-filled day series and renders a second lazy DayLineChart ("Tokens per day"), with a compact-count axis formatter (formatCompactCount in daySeries.ts) so token magnitudes fit the y-gutter. No backend work — the API already returned calls/total_tokens per day.
  2. "Error-rate trend" was an error-count trend. New JSX-free errorRateSeries joins the errors/day series with the usage summary's events/day series by date (both zero-filled over the panel range): rate = errors ÷ events as a percentage; zero-event days report 0, never NaN/Infinity. The absolute errors/day chart stays; the rate chart renders alongside it (muted note if the usage series hasn't loaded).
  3. The three line charts had no table fallback — day values were reachable only by pointer hover. New DaySeriesTable mirrors the existing "View data" <details> pattern for every day-series chart (usage events/day; cost calls/tokens/cost; errors count/rate). Also wraps all minWidth tables in an overflowX: auto container so narrow viewports scroll the table, not the page.

Constraints preserved: hand-rolled SVG only (no chart lib), next/dynamic + ssr:false lazy-loading and the JSX-free daySeries.ts split untouched, role="img" + aria-labels on new charts, brand tokens, no glass/gradient-text, and every name/testid asserted by e2e/admin-analytics.spec.ts unchanged.

Verification

  • vitest on touched files: 25 tests pass — the old View data ≥ 2 smoke assert is replaced with panel-scoped assertions of specific day values (incl. a 25% rate = 1 error ÷ 4 events fixture)
  • tsc --noEmit clean; eslint exit 0 on all touched files
  • ⚠️ Local E2E lane not runnable on this machine (unprovisioned stack — details in PR perf(frontend): #111 runtime animations + lazy/next-image loading #490); e2e.yml runs the admin-analytics journey on push to main. All names/testids that journey asserts are unchanged.

Closes#122

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added accessible data tables and “View data” fallbacks for analytics charts.
    • Added daily error-rate reporting based on usage events.
    • Added daily usage event charts and expanded LLM cost reporting for tokens, calls, and cost.
    • Added compact formatting for large counts.
  • Improvements

    • Improved responsive display of analytics tables.
    • Missing daily usage and error values are now shown as zero.

…series table fallbacks
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@supabase

supabaseBot commented Jul 31, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitaiBot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 04e4f8ea-eac5-44ee-9c15-ba36a7ea6208

📥 Commits

Reviewing files that changed from the base of the PR and between f135770 and a03d410.

📒 Files selected for processing (2)
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
📝 Walkthrough

Walkthrough

The analytics dashboard adds daily error-rate calculations, compact count formatting, accessible chart data tables, responsive raw tables, and expanded usage, cost, token, call, and error visualizations with integration coverage.

Changes

Analytics dashboard

Layer / File(s)Summary
Daily series utilities
frontend/src/lib/daySeries.ts, frontend/src/components/AnalyticsCharts.tsx, frontend/src/components/AnalyticsCharts.test.tsx
Adds errorRateSeries and formatCompactCount, re-exports them, and tests date joining, zero-event handling, domain filtering, and rounding boundaries.
Chart data tables and metric panels
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Adds DaySeriesTable, responsive table wrappers, zero-filled usage data, daily cost/token/call projections, and labeled chart fallbacks. Tests cover the expanded chart set and table values.
Error-rate dashboard flow
frontend/src/components/screens/AdminAnalytics.tsx, frontend/src/components/screens/AdminAnalytics.test.tsx
Calculates and displays daily error rates against usage events, including rate states, error tables, zero-filled days, and accessibility assertions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant AdminAnalytics
participant errorRateSeries
participant DaySeriesTable
AdminAnalytics->>errorRateSeries: pass zero-filled errors and usage events
errorRateSeries-->>AdminAnalytics: return daily error-rate points
AdminAnalytics->>DaySeriesTable: pass errors and rate series
DaySeriesTable-->>AdminAnalytics: render expandable daily data table
Loading

Possibly related PRs

Suggested reviewers:andresl230

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the three analytics dashboard gaps addressed by this pull request.
Description check✅ PassedThe description provides a detailed summary, related issue, implementation details, and verification results, although it uses alternate section headings.
Linked Issues check✅ PassedThe changes address issue #122 by adding token trends, error-rate trends, accessible table fallbacks, and responsive chart data tables.
Out of Scope Changes check✅ PassedThe changes are limited to analytics charts, calculations, accessibility fallbacks, responsive tables, and related tests.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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/122-analytics-gaps

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.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 31, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga03d410Commit Preview URL

Branch Preview URL
Jul 31 2026, 06:53 AM

…opy, compact-count boundary
- DaySeriesTable and the two raw-table disclosures now carry per-chart
labels ("View data: errors per day", …) so the five identical summaries
are distinguishable to screen-reader users.
- Errors panel zero-fills unconditionally: a no-error range renders real
0s and a flat line, not em-dash "unknown" cells.
- Rate-chart fallback copy branches on summary error/loaded-empty/loading
instead of always promising the series will load.
- formatCompactCount promotes 999,950+ to "1M" instead of emitting
"1000k"; boundary cases and the errorRateSeries domain-drop contract
pinned in unit tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
MemberAuthor

Automated review pass (Claude Code) on a72a5ad. The reviewer independently re-ran the touched tests (25/25), tsc, and eslint in a throwaway worktree, and probed the new helpers directly with Node. Verdict on the core: the rate join is sound on every reachable path — UTC day keys on both sides (backend _bucket_datezeroFillDays' toISOString()), NaN-proof zero-event guard, errors a strict subset of the events domain so the join can't drop an error day; lazy-loading structure and every locator e2e/admin-analytics.spec.ts asserts verified intact.

No criticals. The review's minors are now fixed in f135770:

  • Five identical "View data" disclosures → per-chart labels ("View data: errors per day", …), with the unit test asserting each.
  • Errors panel now zero-fills unconditionally, so a no-error range shows real 0s and a flat line instead of em-dash "unknown" cells.
  • The rate-chart fallback copy branches on summary error / loaded-but-empty / still-loading instead of always promising the series will load.
  • formatCompactCount(999_950) returned "1000k" → now promotes to "1M"; unit boundaries (999/1000/999,949/999,950) and the errorRateSeries domain-drop contract are pinned in tests.

Not addressed (accepted, with reasoning): rate can exceed 100% if the summary scan hits the 100k _SCAN_CAP while the errors scan doesn't — the TruncatedBadge renders in the Usage panel rather than beside the rate chart. Extreme edge; worth badge propagation only if truncation becomes common.

Outstanding process item (already flagged in the PR body): the pre-merge local E2E lane couldn't run on this machine — the admin-analytics journey should be run on a provisioned machine before merge, or watch the post-merge e2e.yml run closely. A follow-up journey step asserting the "Error rate per day" chart and one DaySeriesTable value would give the new surfaces lane coverage.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@frontend/src/components/screens/AdminAnalytics.tsx`:
- Around line 471-516: In the Errors panel render block, validate that
summary.data belongs to the current d.range before constructing eventPoints or
calling errorRateSeries; when summary.range differs, leave the denominator
unavailable so the rate shows the invalid/loading state rather than fabricated
0% values. Update the fallback message branching to distinguish this range
mismatch from summary.error and a genuinely empty events series.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f7cfa34d-f710-4538-9c93-b3ede3e984bd

📥 Commits

Reviewing files that changed from the base of the PR and between 432c207 and f135770.

📒 Files selected for processing (5)
  • frontend/src/components/AnalyticsCharts.test.tsx
  • frontend/src/components/AnalyticsCharts.tsx
  • frontend/src/components/screens/AdminAnalytics.test.tsx
  • frontend/src/components/screens/AdminAnalytics.tsx
  • frontend/src/lib/daySeries.ts

Comment threadfrontend/src/components/screens/AdminAnalytics.tsx
Jose-Gael-Cruz-Lopezand others added 2 commits July 30, 2026 23:29
…ry ranges
The summary hook keeps its last payload while a range change reloads, so
the errors panel could zero-fill an old-range series against the new
range and fabricate an all-0% rate line. The join now requires day-key
range agreement; mismatch renders the waiting state. Also: TruncatedBadge
propagates to the rate chart when the summary scan was truncated (the
denominator undercounts, so the rate can overshoot), and formatPct floors
tiny nonzero rates at "<0.1%" so they never read as "no errors".
Regression test pins the stale-range guard.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
Collaborator

Merged main in, reviewed, ran the E2E lane — all green

Merged main. This branch was based on 432c207, before #487/#488/#489/#490 landed. Clean merge, no conflicts.

Full local E2E cycle at a03d410, one flock, SAPLING_MODEL_MODE=function:

35 passed (1.3m)
0 finding(s), 1 suppressed (allowlisted)

admin-analytics.spec.ts passes against the restructured dashboard — every selector it uses (the role="img" chart names, admin-analytics-cost-group-model, admin-analytics-range-7d, the event-path table cells) still resolves, and the new "Tokens per day" / "Error rate per day" aria-labels don't substring-collide with the existing "Events per day" / "Errors per day" ones, which is the failure mode a new chart could plausibly have introduced.

Review — no findings. Two agents over correctness and conventions. Things specifically checked and cleared:

  • errorRateSeries division guard (p.value > 0 ? … : 0) — no NaN/Infinity path.
  • The "error dates outside the events domain get dropped" edge is real in the exported helper and is deliberately pinned by its own unit test — but unreachable from the screen: both series are zero-filled from the same range.from/to, and error.4xx/error.5xx are rows in the same events table, so an error day always has ≥1 event. The rangesAgree guard from 7be1223 covers the mid-flight range-change case, with a regression test.
  • formatCompactCount boundaries hand-verified at 999 / 1000 / 999,950 / 1,000,000 — the 999.95.toFixed(1)"1M" promotion behaves as its comment claims.
  • Tokens chart reads p.total_tokens, cost chart reads p.cost_usd — no label/data mismatch on any new chart (the kind of bug a test asserting only the aria-label would wave through).
  • The stale-response race from feat(analytics): #121 data layer — bucket param, typed client, hooks, raw admin page #478's review is still guarded by the seqRef monotonic sequence in useAdminAnalytics.ts; not reintroduced.
  • Testid convention, both halves: the PR adds no <button>/<input>/<textarea> (the new <details>/<summary>/<table> aren't matched by the enforcement selector), so nothing needed registering in docs/frontend-testids.md or eslint.config.mjs.
  • No charting dependency added — package.json/lockfile untouched, still hand-rolled SVG.

Gates: tsc --noEmit clean · npm run lint 0 errors · npx vitest run 58 files / 415 tests.

Merging.

@AndresL230
AndresL230 merged commit 622b932 into mainJul 31, 2026
6 checks passed
@AndresL230
AndresL230 deleted the feat/122-analytics-gaps branch August 2, 2026 18:30
Sign up for freeto 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.

[P3] Observability: admin analytics dashboard UI (charts/visualizations)

2 participants

@Jose-Gael-Cruz-Lopez@AndresL230