Uh oh!
There was an error while loading. Please reload this page.
feat(analytics): #121 data layer — bucket param, typed client, hooks, raw admin page - #478
Conversation
…#121 backend) - Optional ?bucket=day on /usage/summary, /llm/cost, /errors adds a sparse UTC-day series (days with no rows omitted; client zero-fills from range). The errors series runs its own capped scan and surfaces truncation. - Range now serializes as {"from", "to"} — the query param and the wire key agree before any TS client freezes on the accidental "from_". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
#121) - Typed wrappers (adminUsageSummary/ByUser/LlmCost/Errors) over the #120 API, omitting unset params so backend defaults stay server-owned; response types in types.ts named clear of the legacy AnalyticsOverview family. - Data hooks (useAdminAnalytics.ts) on the house useCallback+useEffect pattern; presetRange routes through lib/testMode's clock. - /admin/analytics renders live data as raw tables: usage summary, top users, LLM cost with group-by toggle, error feed. One range drives every panel; per-panel error && !data banners with retry; truncation badges. The admin gate sits OUTSIDE the hook-owning body so a non-admin visit fires zero 403ing requests. - admin-analytics testid surface registered in BOTH docs/frontend-testids.md and the eslint no-restricted-syntax files array. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reached
Next review available in:44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughChangesAdmin analytics now includes backend day-bucketed series and truncation reporting, typed frontend API access and hooks, and a new admin-gated dashboard with date controls, four data panels, retries, and truncation indicators. Admin analytics backend
Frontend analytics data flow
Admin dashboard UI
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 85b916e | Commit Preview URL Branch Preview URL | Jul 30 2026, 06:04 PM |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@backend/tests/test_admin_analytics_routes.py`:
- Around line 270-339: Move the mocked Supabase setup currently owned by the
local seeded fixture into the shared seeded fixture in tests/conftest.py,
including the analytics.table patch and common seed data. Remove the duplicate
local setup from these analytics route tests so all cases use the shared mocked
Supabase and Gemini fixtures consistently.
In `@frontend/src/lib/useAdminAnalytics.ts`:
- Around line 58-74: Update the load callback to assign each invocation a
monotonically increasing request id, clear existing data when starting a new
request, and guard setData, setError, and setLoading updates so only the latest
request can change state. Ensure an older completion cannot overwrite the
selected range and that a failed latest request does not retain stale data.
🪄 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: f3306530-f696-4d48-9873-496cc4e0ee36
📒 Files selected for processing (12)
backend/routes/admin_analytics.pybackend/tests/test_admin_analytics_routes.pydocs/frontend-testids.mdfrontend/eslint.config.mjsfrontend/src/app/(shell)/admin/analytics/page.tsxfrontend/src/components/screens/AdminAnalytics.test.tsxfrontend/src/components/screens/AdminAnalytics.tsxfrontend/src/lib/adminAnalyticsApi.test.tsfrontend/src/lib/api.tsfrontend/src/lib/types.tsfrontend/src/lib/useAdminAnalytics.test.tsfrontend/src/lib/useAdminAnalytics.ts
| def test_range_serializes_as_from(seeded): | ||
| # The wire key must be "from" (matching the query param), not the Python | ||
| # field name "from_" — the TS client types freeze on this shape. | ||
| r = client.get(f"{BASE}/usage/summary", params=RANGE) | ||
| assert r.json()["range"] == {"from": RANGE["from"], "to": RANGE["to"]} | ||
| def test_usage_summary_bucket_day_series(seeded): | ||
| r = client.get(f"{BASE}/usage/summary", params={**RANGE, "bucket": "day"}) | ||
| assert r.status_code == 200 | ||
| body = r.json() | ||
| assert [p["date"] for p in body["series"]] == ["2026-07-10", "2026-07-12", "2026-07-15"] | ||
| assert [p["count"] for p in body["series"]] == [1, 1, 2] | ||
| # Bucketing adds the series; it must not change the aggregate fields. | ||
| assert body["total_events"] == 4 | ||
| assert body["distinct_active_users"] == 2 | ||
| def test_usage_summary_without_bucket_has_no_series(seeded): | ||
| r = client.get(f"{BASE}/usage/summary", params=RANGE) | ||
| assert r.json()["series"] is None | ||
| def test_llm_cost_bucket_day_series(seeded): | ||
| r = client.get(f"{BASE}/llm/cost", params={**RANGE, "bucket": "day"}) | ||
| assert r.status_code == 200 | ||
| series = r.json()["series"] | ||
| assert [p["date"] for p in series] == ["2026-07-10", "2026-07-12", "2026-07-15"] | ||
| assert [p["calls"] for p in series] == [1, 1, 1] | ||
| assert [p["total_tokens"] for p in series] == [150, 300, 120] | ||
| assert series[0]["cost_usd"] == pytest.approx(0.01) | ||
| assert series[1]["cost_usd"] == pytest.approx(0.05) | ||
| assert series[2]["cost_usd"] == pytest.approx(0.02) | ||
| def test_errors_bucket_day_series(seeded): | ||
| r = client.get(f"{BASE}/errors", params={**RANGE, "bucket": "day"}) | ||
| assert r.status_code == 200 | ||
| body = r.json() | ||
| assert body["series"] == [{"date": "2026-07-15", "count": 1}] | ||
| assert body["total"] == 1 | ||
| assert body["truncated"] is False | ||
| def test_bucket_rejects_invalid_value(seeded): | ||
| r = client.get(f"{BASE}/usage/summary", params={**RANGE, "bucket": "hour"}) | ||
| assert r.status_code == 422 | ||
| def test_errors_bucket_series_truncation_is_surfaced(monkeypatch): | ||
| # The errors series needs its own range scan (the feed itself is paginated | ||
| # server-side); that scan can hit the cap and must say so, never silently. | ||
| store = { | ||
| "events": [ | ||
| {"event_type": "error.5xx", "category": "error", "user_id": "u1", "request_id": f"r{i}", | ||
| "payload": {"path": "/api/x", "method": "GET", "status_code": 500, "duration_ms": 1.0}, | ||
| "created_at": ts} | ||
| for i, ts in enumerate([IN1, IN2, IN3]) | ||
| ], | ||
| "llm_usage": [], | ||
| } | ||
| monkeypatch.setattr(analytics, "table", lambda name: _FakeTable(store.get(name, []))) | ||
| monkeypatch.setattr(analytics, "_PAGE", 1) | ||
| monkeypatch.setattr(analytics, "_SCAN_CAP", 2) | ||
| r = client.get(f"{BASE}/errors", params={**RANGE, "bucket": "day"}) | ||
| assert r.status_code == 200 | ||
| body = r.json() | ||
| assert body["truncated"] is True | ||
| assert [p["count"] for p in body["series"]] == [1, 1] # capped at 2 scanned rows | ||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the shared mocked-Supabase fixture.
These new cases depend on the local seeded fixture, which directly monkeypatches analytics.table (Lines 102-104). Move the setup to the shared tests/conftest.py fixture so route mocks remain consistent across backend tests.
As per coding guidelines, backend tests must use the shared fixtures in tests/conftest.py for mocked Supabase and Gemini services.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/tests/test_admin_analytics_routes.py` around lines 270 - 339, Move
the mocked Supabase setup currently owned by the local seeded fixture into the
shared seeded fixture in tests/conftest.py, including the analytics.table patch
and common seed data. Remove the duplicate local setup from these analytics
route tests so all cases use the shared mocked Supabase and Gemini fixtures
consistently.
Source: Coding guidelines
| const load = React.useCallback(async () => { | ||
| setLoading(true); | ||
| try { | ||
| setData(await fetcher()); | ||
| setError(null); | ||
| } catch (err) { | ||
| setError(humanizeError(err, "Couldn't load analytics.")); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }, [fetcher]); | ||
| React.useEffect(() => { | ||
| load(); | ||
| }, [load]); | ||
| return { data, loading, error, reload: load }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent stale requests from overwriting the selected range.
Line 61 accepts every completion. After a range or group change, an older request can resolve last and replace the new result; if the newer request fails, retained old data also hides the retry banner. Clear stale data and guard every state update with a monotonically increasing request id.
Suggested fix
function useAnalyticsQuery<T>(fetcher: () => Promise<T>): AnalyticsQuery<T> {
const [data, setData] = React.useState<T | null>(null);
const [loading, setLoading] = React.useState(true);
const [error, setError] = React.useState<string | null>(null);
+ const requestId = React.useRef(0);
const load = React.useCallback(async () => {
+ const id = ++requestId.current;
setLoading(true);
+ setData(null);+ setError(null);
try {
- setData(await fetcher());- setError(null);+ const result = await fetcher();+ if (id === requestId.current) setData(result);
} catch (err) {
- setError(humanizeError(err, "Couldn't load analytics."));+ if (id === requestId.current) {+ setError(humanizeError(err, "Couldn't load analytics."));+ }
} finally {
- setLoading(false);+ if (id === requestId.current) setLoading(false);
}
}, [fetcher]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| constload=React.useCallback(async()=>{ | |
| setLoading(true); | |
| try{ | |
| setData(awaitfetcher()); | |
| setError(null); | |
| }catch(err){ | |
| setError(humanizeError(err,"Couldn't load analytics.")); | |
| }finally{ | |
| setLoading(false); | |
| } | |
| },[fetcher]); | |
| React.useEffect(()=>{ | |
| load(); | |
| },[load]); | |
| return{ data, loading, error,reload: load}; | |
| const[data,setData]=React.useState<T|null>(null); | |
| const[loading,setLoading]=React.useState(true); | |
| const[error,setError]=React.useState<string|null>(null); | |
| constrequestId=React.useRef(0); | |
| constload=React.useCallback(async()=>{ | |
| constid=++requestId.current; | |
| setLoading(true); | |
| setData(null); | |
| setError(null); | |
| try{ | |
| constresult=awaitfetcher(); | |
| if(id===requestId.current)setData(result); | |
| }catch(err){ | |
| if(id===requestId.current){ | |
| setError(humanizeError(err,"Couldn't load analytics.")); | |
| } | |
| }finally{ | |
| if(id===requestId.current)setLoading(false); | |
| } | |
| },[fetcher]); | |
| React.useEffect(()=>{ | |
| load(); | |
| },[load]); | |
| return{ data, loading, error,reload: load}; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/lib/useAdminAnalytics.ts` around lines 58 - 74, Update the load
callback to assign each invocation a monotonically increasing request id, clear
existing data when starting a new request, and guard setData, setError, and
setLoading updates so only the latest request can change state. Ensure an older
completion cannot overwrite the selected range and that a failed latest request
does not retain stale data.
AndresL230
commented
Jul 30, 2026
Code reviewFound 1 issue:
Sapling/frontend/src/lib/useAdminAnalytics.ts Lines 52 to 76 in 010164b Sapling/frontend/src/components/screens/AdminAnalytics.tsx Lines 66 to 80 in 010164b Also fixing below the posting bar (scored <80): a stale-response race in the same hook (no out-of-order guard on rapid range/group-by changes), missing client-side 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
… guard, range clamp, testid rename - useAnalyticsQuery: monotonic request sequence so an out-of-order response can't overwrite newer data; background-reload failures keep the loaded view and toast (#463 Calendar convention) instead of vanishing. - Custom range inputs clamp the other edge (backend 422s from > to) and carry min/max bounds. - admin-analytics-costgroup-* -> admin-analytics-cost-group-* per the testids kebab convention (code + doc inventory). - TruncatedBadge copy made meaning-neutral for the errors panel's series-only truncation (#122 note). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Part of #121 (left open per closure policy — Andres verifies the UI surface and closes).
Backend (small, approved addition to the #120 API)
?bucket=dayon/usage/summary,/llm/cost,/errorsadds a sparse UTC-dayseries(client zero-fills fromrange). The errors series runs its own capped scan and surfacestruncated— never silent partial data.Rangenow serializes{"from", "to"}(was the accidental Python field namefrom_) — fixed before any TS client froze on it.Frontend
AnalyticsOverviewfamily) + thin data hooks on the houseuseCallback+useEffectpattern;presetRangeuseslib/testMode's clock./admin/analyticsrendering live data as raw tables (per [P3] Observability: frontend admin analytics data layer (client + hooks + route) #121 — charts are [P3] Observability: admin analytics dashboard UI (charts/visualizations) #122): usage summary, top users, LLM cost with group-by toggle, errors. Shared range selector (7d/30d/90d presets + custom dates) drives every panel; per-panelerror && !databanner + retry; truncation badges.auth.permission_deniedaudit event).docs/frontend-testids.mdand the eslintno-restricted-syntaxfiles array.Tests
The promoted UI journey lands with #122 (same surface, final UI) — API-level e2e coverage already exists in
events.spec.ts. Full local e2e cycle runs before merge regardless.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
fromandtoparameters.Documentation
Tests