feat(analytics): add route reliability and latency analytics - #1005
Conversation
|
Warning Review limit reached
Next review available in: 8 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: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis change adds read-only routing analytics from the request-history SQLite index. It computes filtered metrics and breakdowns, exposes them through ChangesRouting analytics
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ManagementAPI
participant RoutingAnalyticsRoute
participant RequestHistorySQLite
Client->>ManagementAPI: GET /api/routing-analytics
ManagementAPI->>RoutingAnalyticsRoute: dispatch request
RoutingAnalyticsRoute->>RoutingAnalyticsRoute: validate filters and timestamps
RoutingAnalyticsRoute->>RequestHistorySQLite: compute filtered analytics
RequestHistorySQLite-->>RoutingAnalyticsRoute: analytics result
RoutingAnalyticsRoute-->>Client: HTTP 200 JSON response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
2069e72 to
a91550c
Compare
💡 Codex Reviewopencodex/src/routing/history/indexer.ts Lines 131 to 132 in 2069e72 When opencodex/src/routing/analytics.ts Line 231 in 2069e72 For successful combo routes, the parent usage row is persisted as provider opencodex/src/routing/analytics.ts Line 283 in 2069e72 When a routing profile is revised but keeps the same opencodex/src/routing/analytics.ts Line 250 in 2069e72 For Codex pool/OAuth/provider-account routing, opencodex/src/routing/history/indexer.ts Line 86 in 2069e72 This only records opencodex/src/routing/history/indexer.ts Lines 203 to 207 in 2069e72 A parseable but hand-edited line that has only opencodex/src/routing/analytics.ts Line 225 in 2069e72 This treats opencodex/src/routing/analytics.ts Line 179 in 2069e72 The persisted Codex surface is opencodex/src/routing/history/indexer.ts Line 184 in 2069e72 On a first build, manual rebuild, or any large unindexed tail, ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…d query validation (RI-03)
[GD] Verdict: approve-commentTLDR
Full reviewSee commit e732d02 for test fix and simplify. |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@devlog/_plan/260804_router_intelligence/001_pr_stack_status.md`:
- Around line 43-44: Update the programme status statement near the document’s
introductory summary to remove or clearly label the stale “nothing merged”
claim, reflecting that RI-01 and RI-02 are merged. Preserve the existing
historical context if needed, and keep the RI status table and PR `#1004` entry
unchanged.
- Around line 129-135: Update the “Reviewed commit” and “Final commit” entries
in the stack status plan so they remain consistent: before the final push, mark
both as pending; after pushing, record the exact reviewed and final commit SHAs.
Associate the listed verification results with the corresponding SHA instead of
claiming the reviewed commit already matches the final commit.
- Around line 127-128: Correct the lineage labels in the Base SHA entry:
identify 2a72aa4a9 as the merged RI-02 commit and canonical RI-03 base,
7efb6e842 as the pre-merge RI-02 head, and 2069e724e as an RI-03 commit rather
than an RI-02 head.
In `@src/routing/analytics.ts`:
- Around line 238-239: Update the analytics aggregation around `entry`,
`cooldownFailures`, and the result object to parse every row needed for attempt
inspection, including failed and cancelled rows, while separating
recovered-success counts from `cooldownTriggeringFailures`. Increment
`cooldownFailures` only for non-success requests, track successful requests with
cooldown-triggering attempts in a new `cooldownRecoveredRequests` counter, and
expose that counter through `RoutingAnalyticsResult` and the returned result.
In `@src/routing/history/indexer.ts`:
- Around line 570-579: Change requestHistoryDb() to return a narrow read-only
query interface exposing only the query operation needed by
computeRoutingAnalytics, rather than the live Database with run, exec, and
schema mutation methods. Update the doc comment to state that callers must use
the handle synchronously and not retain it across await or beyond
closeRequestHistoryIndex(), which invalidates the underlying database.
In `@src/server/management/routing-analytics-routes.ts`:
- Around line 38-45: Update the routing analytics endpoint’s
computeRoutingAnalytics call to read an optional maxRows query parameter, parse
and validate it consistently with the existing analytics options, and forward it
so callers can request a smaller bounded window instead of always using the
default cap. Keep the existing provider, model, profileId, surface, from, and to
handling unchanged; do not add memoization unless already supported by the
surrounding implementation.
In `@tests/routing-analytics.test.ts`:
- Around line 56-72: Add a regression test alongside the existing routing
analytics tests that appends a non-4xx incomplete stream with attempts
containing a cooldown recovery kind such as "rate-limit-429", followed by a
successful attempt. Assert computeRoutingAnalytics reports failureRate as 1 and
cooldownTriggeringFailures as 1, ensuring cooldown detection uses attempt
recoveryKinds rather than only the stream status.
- Around line 191-200: Add a focused test beside “API endpoint returns the
analytics payload” covering all routing-analytics validation branches: assert
400 responses and error codes invalid_from for malformed and empty from values,
invalid_to for a non-integer to value, and invalid_range when from exceeds to.
Reuse a local request helper to exercise handleManagementAPI consistently.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: 883ffcb5-b9af-4376-9352-dbf62426398e
📒 Files selected for processing (6)
devlog/_plan/260804_router_intelligence/001_pr_stack_status.mdsrc/routing/analytics.tssrc/routing/history/indexer.tssrc/server/management-api.tssrc/server/management/routing-analytics-routes.tstests/routing-analytics.test.ts
|
Merged — RI-03 (routing analytics) is on dev. |
Summary
RI-03 of the Router Intelligence / Routing Control Plane programme. Adds
source-backed routing analytics computed from the request-history index
(RI-02), never from repeated full JSONL scans.
Read-only: this PR cannot change routing behavior (ADR-10 - no automatic
self-tuning).
Metrics
GET /api/routing-analytics(filters:provider,model,profileId,surface,from,to) returns:provider failures)
percentiles, with TTFT sample count + coverage)
terminalStatus: "incomplete")rate-limit-429/key-429/oauth-401/anthropic-oauth-429recovery kinds)
(reuses
usage/cost.tspricing; unknown prices stay unknown, never zero)high>= 100,medium>= 20, elselow)historyTruncatedflag when the analysis cap (50,000 rows) is hitScope
src/routing/analytics.ts-computeRoutingAnalytics()over the SQLiteindex (bounded, deterministic, single SQL pass + JS aggregation).
src/server/management/routing-analytics-routes.ts- the endpoint,registered in
management-api.ts.tests/routing-analytics.test.ts- 8 tests.Privacy / security
privacy-bounded columns the index already stores.
the process.
bun run privacy:scanpasses.Compatibility
/api/logs,/api/request-history, andusage.jsonlcontracts unchanged.
Dependency
feat/ri-02-request-history-indexhead7efb6e842. PR target isdev;the diff includes earlier PR commits until they merge.
Non-goals
profileIdfilter/breakdown onlyreads RI-01 traces.
Local verification (exact)
bun x tsc --noEmit-> PASSED (0 errors)bun run test tests/routing-analytics.test.ts-> 8/8 passtrace, request-log, usage-log, combos) -> 144/144 pass
bun run privacy:scan-> passedNotes for reviewers
classifyRow(): status 499 /client_cancel-> cancelled;terminalStatus: "incomplete"-> failure;status >= 400 -> failure; otherwise success.
nullestimates andpriceCoverage: 0- never afake zero cost.
Summary by CodeRabbit
New Features
Bug Fixes
Tests