feat(routing): record durable route decision traces - #1003
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds a versioned route-decision trace contract, classifies routing outcomes, captures combo evidence, propagates traces through request and usage logs, normalizes persisted data, and adds coverage for routing, privacy, truncation, persistence, and determinism. ChangesRouter Intelligence trace capture
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RequestHandler
participant routeModel
participant RequestLog
participant UsageLog
Client->>RequestHandler: submit model request
RequestHandler->>routeModel: resolve model route
routeModel-->>RequestHandler: route and RouteDecisionTraceV1
RequestHandler->>RequestLog: store routeDecision in log context
RequestLog->>UsageLog: serialize normalized routeDecision
UsageLog-->>RequestLog: hydrate valid trace or omit invalid trace
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 |
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 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/000_master_plan.md`:
- Around line 163-183: Align the RouteDecisionTraceV1.requestedModel schema
documentation with the enforced 128-character MAX_TRACE_STRING limit. Update the
requestedModel comment and related contract references near the trace string
limits so values up to 128 characters are the documented maximum; keep the
existing normalizer behavior unchanged.
- Line 60: Make the locale acceptance requirement consistent across the master
plan: update the references around the six listed locale files and the
acceptance criteria at lines 107-108 and 393-398 to require en, de, ja, ko, ru,
and zh, or explicitly document the intentional exclusion of zh. Ensure
documentation and GUI acceptance checks use the same locale scope.
In `@devlog/_plan/260804_router_intelligence/001_pr_stack_status.md`:
- Line 42: Update the RI-01 status ledger to record the exact SHA of the commit
used for the passing verification results instead of leaving the head or final
commit as pending; keep the reviewed commit distinct from the eventual final
commit, and replace the final SHA placeholder only after that commit is created.
Apply the same correction to the related entries in the RI-01 status section.
In `@src/router.ts`:
- Around line 538-556: Update routeModel to resolve route.combo’s configuration
once, omit tieBreak when that lookup is undefined, and derive it only from the
resolved strategy. Pass the resolved combo into comboRouteCandidates so
candidate generation reuses it instead of calling getCombo again.
In `@src/routing/trace.ts`:
- Around line 322-338: Update enforceByteBudget to measure serialized JSON in
UTF-8 bytes rather than UTF-16 code units: compute each JSON.stringify result
once and compare its encoded byte length against MAX_TRACE_BYTES. Apply this to
both the initial trace and slimmed trace checks while preserving the existing
truncation behavior and candidate slicing.
- Around line 474-478: Update the evidence-block construction around
parseCapability, parseHealth, parseQuota, parseCost, and parseScore to compute
each parser result once per candidate, store or bind the result locally, and
conditionally include that single value. Preserve the current behavior of
omitting falsy parsed values.
- Around line 276-279: Update the trace type and normalization flow so
requirement truncation is represented explicitly: add an optional requirements
flag to the truncated shape, change the truncation branch after slicing
requirements to set truncated.requirements instead of truncated.candidates, and
whitelist requirements in normalizeRouteDecisionTrace so the flag survives
hydration.
In `@src/server/request-log.ts`:
- Line 349: The usage row now persists routeDecision for every entry, increasing
snapshot row size and reducing the dashboard window under
OcxConfig.managementUsageMaxReadBytes. Update the request-log serialization
around entry.routeDecision to persist traces only for combo and fallback routes,
while preserving diagnostics for those routes and omitting low-value
single-candidate traces; verify representative row sizes and the resulting
snapshot row count.
- Around line 255-267: Update normalizeRouteDecisionTraceForLog and its
routeDecision caller to omit the field when normalizeRouteDecisionTrace returns
null instead of falling back to the unvalidated entry. Store the normalized
result in a local const, spread routeDecision only when it is valid, and
preserve the existing optional-field behavior for invalid persisted traces.
In `@tests/route-decision-trace.test.ts`:
- Around line 220-237: Add a regression test for
requestLogEntryFromPersistedUsage using a persisted entry with an invalid
routeDecision, such as an empty candidates array, and assert
hydrated.routeDecision is undefined. Then update
normalizeRouteDecisionTraceForLog so failed validation does not fall back to the
unvalidated entry; return the normalized valid trace or omit the trace instead.
- Around line 125-141: Add a new test case that directly exercises the
preserved-selection truncation branch at trace.ts lines 258-259 by calling
buildRouteDecisionTrace with a candidates array where the selected candidate has
a candidateIndex positioned beyond the MAX_TRACE_CANDIDATES cap. Set up the test
so only the candidate at the high index (e.g., index 11) is eligible while
others are excluded, then verify that the selected candidate survives truncation
and that selected.candidateIndex is correctly adjusted to point to its final
position in the truncated candidates array, confirming the off-by-one guard is
working as documented in the trace.ts comment at lines 254-255.
- Around line 175-182: Update the credential-trace test around the “trace never
contains credentials or prompt content” case to construct the secret sentinel at
runtime from non-token-shaped pieces, reuse that single value for both provider
configuration and the serialized-trace assertion, and replace the broad “prompt”
assertion with checks against the actual routing inputs used by this test. Run
the privacy scan afterward and adjust the sentinel construction if it still
matches the scanner.
🪄 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: c1f3b4ce-7ee8-4211-a1ad-12fd0baa1c9a
📒 Files selected for processing (12)
devlog/_plan/260804_router_intelligence/000_master_plan.mddevlog/_plan/260804_router_intelligence/001_pr_stack_status.mdsrc/router.tssrc/routing/trace.tssrc/server/chat-completions.tssrc/server/claude-messages.tssrc/server/request-log.tssrc/server/responses/compact.tssrc/server/responses/core.tssrc/server/search.tssrc/usage/log.tstests/route-decision-trace.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e0522b22b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
- byte-accurate trace budget (UTF-8, not code units) - explicit truncated.requirements flag (type + normalizer whitelist) - parse evidence blocks once per candidate (dead code removed) - combo resolved once; tieBreak only when the combo lookup succeeds - hydration drops invalid traces instead of forwarding them unvalidated - regression tests: preserved-selection truncation, corrupt hydration, credential assertions against real routing inputs - plan docs: requestedModel bound 128, locale scope made explicit - ledger: record reviewed commit SHAs
[GD] Full review — feat(routing): record durable route decision traces (#1003)Reviewed heads: VerdictApprove for merge pending the maintainer's final call. No Critical or High CodeRabbit findings (12) — triageResolved (10):
Kept with rationale (2):
Additional finding from the author review pass (not in CodeRabbit)
Simplify candidates applied (behavior-preserving)
Verification (exact)
Stack noteThis is the stack bottom (#1004..1018 build on it). The fixes here are |
…I-01) - combo requests persist one immutable combo trace; child adoption can no longer overwrite it with a concrete child route trace (P1) - attach the initial route trace immediately after routeModel so pre-dispatch rejections still record the decision - normalizer now marks every cap it applies (candidates/exclusions/ requirements/strings) and unions it with incoming flags - startup hydration keeps expanding the read window to the file start and budgets for trace-sized rows - byte-budget fallback re-measures after shrinking and strips exclusions deterministically as a last resort - tests: combo server-path trace, normalizer flags, trace-sized hydration
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@src/routing/trace.ts`:
- Around line 325-361: Extend enforceByteBudget after the exclusion reduction so
oversized traces continue deterministic reduction of requirements and optional
evidence, while preserving the selected candidate and its index. Repeatedly
reduce or remove these fields as needed, and perform a final
serializedByteLength(result) <= MAX_TRACE_BYTES check before returning to
guarantee the hard byte limit.
- Around line 426-431: Apply collection bounds to persisted arrays before any
validation, parsing, or traversal in the trace normalizer, including
reasoningEfforts and the candidates, exclusions, and requirements paths around
the referenced sections. Perform caps checks and mappings only on the bounded
inputs so corrupt rows cannot trigger unbounded CPU or memory use. When
truncating candidates, preserve the selected candidate and remap
selected.candidateIndex to its new position.
- Around line 548-553: Update the decisionId validation in the trace normalizer
to accept only the documented wire format: exactly 12 lowercase hexadecimal
characters. Replace the current non-empty-string check while preserving the
existing null return for invalid values and length-cap handling for valid
identifiers.
In `@tests/server-combo-failover-e2e.test.ts`:
- Around line 427-453: The existing combo test covers only successful child
adoption; add a focused exhausted-combo test near it that configures every
target to return a retryable failure and verifies both attempt receipts retain
the parent combo routeDecision (routeKind "combo", requestedModel "combo/free",
and both candidates) while recording all physical attempts. Exercise the
terminal failure path through adoptFailedChildLog without changing production
behavior.
🪄 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: 6ae08174-dae9-4315-942d-1a7e9b04540b
📒 Files selected for processing (9)
devlog/_plan/260804_router_intelligence/000_master_plan.mddevlog/_plan/260804_router_intelligence/001_pr_stack_status.mdsrc/router.tssrc/routing/trace.tssrc/server/request-log.tssrc/server/responses/core.tssrc/usage/log.tstests/route-decision-trace.test.tstests/server-combo-failover-e2e.test.ts
…mits (RI-01) - byte budget: deterministic shrink loop (exclusions -> drop exclusions -> halve candidates, selected preserved) until the 16 KiB bound holds - normalizer: slice candidates/exclusions/requirements before parsing so a corrupt oversized row cannot force unbounded parse work - decisionId: enforce the documented 12-hex format on hydration - tests: terminal combo failure keeps the combo trace through child adoption
|
Merging this foundation PR: every request now records a bounded, privacy-safe route-decision trace through the canonical |
Summary
RI-01 of the Router Intelligence / Routing Control Plane programme
(
devlog/_plan/260804_router_intelligence/000_master_plan.md). Adds aversioned, bounded, privacy-safe route decision trace for every existing
deterministic routing path, persisted through the canonical
usage.jsonlledger and hydrated into request-log DTOs.
The trace answers: which route kind resolved the request, which candidates
were considered, why each was excluded or skipped, and what was selected -
without changing any routing behavior.
Scope
src/routing/trace.ts:RouteDecisionTraceV1types, bounded builder,defensive normalizer, explicit truncation metadata.
src/router.ts:RouteResultnow carriesrouteKind/routeReason/routeDecision.routeModel()records a trace for all five existing routekinds:
explicit-account,explicit-provider,native,combo,default-provider. Combo routes trace every configured target witheligibility and exclusion reasons (
unconfigured,disabled,cooldown,already-attempted,not-selected).PersistedUsageEntry.routeDecision(additive field through theexisting whitelist normalizer in
src/usage/log.ts).RequestLogContext/RequestLogEntrycarry the trace;addFinalRequestLog,addRequestLog(->usage.jsonl), andrequestLogEntryFromPersistedUsage(/api/logs) round-trip it./v1/responses(core.ts, incl. subagent fallbackre-routes),
/v1/responses/compact,/v1/chat/completions,/v1/messagesrouted path, and the Codex-account-qualified web-search path.Selection vs execution
The trace records the selection decision before dispatch. Fallback
execution attempts remain the existing
attempts[]array on the usageentry; the two are never merged.
Bounds (deterministic)
truncated.{candidates,exclusions,strings}Privacy / security
tokens, raw account emails, raw quota responses, authorization headers,
hidden reasoning, raw upstream bodies.
references; account-namespace routes record the user-chosen namespace handle,
never the underlying account id or credential.
bun run privacy:scanpasses.Compatibility
usage.jsonlrows (norouteDecision) parse unchanged./api/logs,/api/usage, per-key rollups).routeModel()provider/model resolution isbyte-identical; the trace is a pure addition.
Dependency
devate44d234f08e03dd4dbf0c4aa13af43046d86b0a6(upstream head, 2026-08-04). This PR is the first in a 10-PR stacked
programme; later PRs build on it. Nothing is merged by this programme.
Non-goals
Local verification (exact)
bun x tsc --noEmit-> PASSED (0 errors)bun run test tests/route-decision-trace.test.ts-> 14/14 pass(75 assertions)
combo-management-api, codex-account-namespaces) -> 253/253 pass
bun run test tests/server-combo-failover-e2e.test.ts-> 44/44 passbun run privacy:scan-> passedrecorded in the stack ledger
(
devlog/_plan/260804_router_intelligence/001_pr_stack_status.md).Tests added
tests/route-decision-trace.test.tscovers: all five route kinds,combo-candidate eligibility/exclusions, candidate-count truncation with the
selected candidate preserved, exclusion/string caps, credential absence,
JSONL round-trip + hydration, legacy rows, corrupt rows, hand-edited
oversized rows, and determinism.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation