feat(routing): add evidence-based route health scoring - #1013
Conversation
|
Warning Review limit reached
Next review available in: 27 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 (11)
📝 WalkthroughWalkthroughThe router now evaluates candidate health and capability evidence during policy routing. Request history supports synchronous incremental refresh. Codex account state contributes cooldown evidence. Traces and dry runs expose candidate evidence, with expanded routing and fallback tests. ChangesHealth-aware routing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RouteRequest
participant Router
participant HealthEvidence
participant RequestHistoryIndex
participant CandidateEvaluator
RouteRequest->>Router: resolve policy and candidates
Router->>RequestHistoryIndex: read recent request history
Router->>HealthEvidence: combine history with Codex account state
HealthEvidence->>CandidateEvaluator: provide health evidence
CandidateEvaluator->>Router: return eligible, scored candidates
Router->>RouteRequest: select route or fallback
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 909ce21d4f
ℹ️ 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".
[GD] Verdict: changes-requestedTLDR
Full verdictSemantic propagation
UsefulnessFixes a real gap: policy routing previously scored candidates by declared priority only. This PR adds deterministic, source-backed health (success rate, consecutive failures, incomplete streams, latency, recency decay) plus authoritative live cooldown/soft-avoid for Codex pool targets, with documented constants and unknown-safe defaults. Useful. Bugs / correctness
Security
Spec / standards
Reviews
Base / CI
Simplification
Gatenone (not draft/WIP/do-not-merge); ship-gate blocked only on Bottom lineThe review loop is closed: every confirmed finding and bot thread is fixed with evidence and regression coverage, and local gates are green. The single remaining item is the base sync after #1012 merges (deliberately deferred per your instruction) — once synced and CI is green on the mergeable head, this PR is ready for merge. |
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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 46-48: The RI-06 entry in the PR stack status table has a stale
head SHA; replace “pending (post-sync head)” with
f9d441446d520d1fdb7b3a49588bcb9e313a2bc9 while preserving PR `#1013`’s existing
“in progress” status.
In `@src/router.ts`:
- Around line 502-519: Thread one routing-decision timestamp through the
candidate health and evaluation flow. In the router path building candidate
evidence, read Date.now() once and pass it to both healthEvidenceForCandidate
and codexPoolHealthEvidence; add an optional now parameter to
evaluatePolicyProfile and forward the same value to its cooldown comparison and
healthScore call, preserving existing defaults for callers that omit it.
In `@src/routing/capability.ts`:
- Around line 99-108: Add "kiro" and "mimo-free" to TOOL_CAPABLE_ADAPTERS, and
introduce a shared literal adapter-ID union reused by OcxProviderConfig,
ProviderRegistryEntry, and the capability set so invalid IDs are rejected at
compile time. Preserve the existing adapter IDs, and explicitly support the
accepted "azure" resolver alias when resolving adapter capabilities.
- Around line 157-165: Update the tools capability calculation around
capabilities, catalogRow, and isNative so an existing catalog row without tools
preserves unknown evidence rather than being converted to true or omitted;
return an explicit false only when the catalog represents a negative capability.
Replace the redundant isNative ternary with isNative, revise the comment so
catalog evidence is not described as authoritative, and add coverage for
ordinary, native, and parallel-enabled providers.
In `@src/routing/evaluator.ts`:
- Around line 303-311: The health optimization path in the route scoring logic
leaves unknown health unblended, allowing an unknown candidate to outrank a
measured one. Update the scoring flow around configuredPriorityScore and the
health blend to use a named HEALTH_UNKNOWN_NEUTRAL_SCORE value of 0.5 for
allow-policy candidates when healthValue is null, while preserving the weighted
blend and evidence behavior. Add a regression test covering the stated
optimize.health = 0.8 ranking scenario.
In `@src/routing/health.ts`:
- Around line 131-143: Update the aggregate availability logic around
getCodexAccountCooldownUntil and getCodexAccountSoftAvoidUntil so the soft-avoid
branch treats each live account as unavailable when it is either hard-cooled or
soft-avoided. Preserve the existing priority: return cooldownUntilMs only when
every live account is hard-cooled; otherwise return softAvoidUntilMs when every
account is covered by either state, using the latest relevant expiry.
- Around line 204-220: Update the attempt-row query in the health calculation to
filter row_json for the requested provider and model before applying LIMIT,
using the serialized provider/model LIKE prefilter described in the review. Keep
attemptSamplesFor as the exact match check before adding samples, preserving the
existing timestamp, attempt_count, exclusion, ordering, and limit behavior.
- Around line 186-211: Batch historical health reads across the routing decision
instead of having healthEvidenceForCandidate perform separate synchronous SQLite
queries and JSON parses per candidate. Open the history index once, scan and
parse shared request and attempt rows once, then derive candidate-specific
evidence from the parsed entries; keep live cooldown and soft-avoid state
uncached. Update the routing flow around healthEvidenceForCandidate and the
history-reading logic in health.ts, and if introducing a TTL cache, cache only
historical evidence with an explicitly defined staleness window.
In `@src/routing/profile.ts`:
- Around line 156-164: Remove the duplicate provider-namespace collision check
and its issue push from alias validation, keeping the existing check that uses
the “provider routing namespace” message. Move the explanatory comment to that
surviving block if needed, and add a focused alias-validation regression test
asserting the exact single issue array for an alias such as “openai/fast” with
an “openai” provider.
In `@src/routing/trace.ts`:
- Around line 225-228: Update buildCandidate to normalize capability, quota, and
cost evidence before adding it to the dry-run response: whitelist supported
fields, cap string and array values to the trace limits, and mark
truncated.strings whenever capping occurs. Do not copy caller-supplied unknown
nested fields verbatim, ensuring the candidate respects MAX_TRACE_STRING and
MAX_TRACE_BYTES before persistence normalization.
In `@src/server/management/routing-profile-routes.ts`:
- Around line 106-114: The dry-run candidate evidence must mirror real routing
for OpenAI Codex candidates, including getEffectiveActiveCodexAccountId(config)
and codexPoolHealthEvidence(config), so cooldown and soft-avoid exclusions are
reported. In the route handler, resolve the profile once into resolvedProfile
and reuse it; extract shared evidence assembly into an exported
policyCandidateHealthEvidence(config, candidate) helper in health.ts if
appropriate, then use it in both call sites. Add a regression test covering a
cooling active Codex account and asserting a cooldown exclusion.
🪄 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: 637f48c0-40d1-43f0-809d-9f869e3e966c
📒 Files selected for processing (13)
devlog/_plan/260804_router_intelligence/001_pr_stack_status.mdsrc/router.tssrc/routing/capability.tssrc/routing/evaluator.tssrc/routing/health.tssrc/routing/history/indexer.tssrc/routing/profile.tssrc/routing/trace.tssrc/server/management/routing-profile-routes.tstests/health-scoring.test.tstests/policy-execution.test.tstests/request-history-index.test.tstests/routing-profile.test.ts
- thread one clock read through policy health evaluation (router + evaluator) - blend unknown health under 'allow' at a neutral 0.5 instead of outranking measured health - mixed pool cooldown/soft-avoid states degrade to soft-avoid - TTL-cache historical health evidence; live cooldown stays uncached - LIKE-prefilter attempt-row scan so LIMIT is not starved by non-matching rows - add kiro/mimo-free/azure to tool-capable adapters - remove duplicate provider-namespace alias check (dev already had it) - whitelist + bound candidate evidence in trace builder - dry-run mirrors the router's health assembly via shared helper - regression tests for all of the above
[GD] MergeMerging this — why it helps: RI-06 adds evidence-based route health scoring to policy routing with deterministic, documented-constant formulas (success rate, consecutive failures, incomplete streams, latency, recency decay) plus authoritative live Codex pool cooldown/soft-avoid, unknown-safe profile handling, and bounded privacy-safe decision traces. The review round closed 8 bot threads and 11 CodeRabbit threads; |
Summary
RI-06 of the Router Intelligence / Routing Control Plane programme. Adds
evidence-based route health scoring to policy routing: candidates are
scored on live cooldown/circuit state plus source-backed history (success
rate, consecutive failures, incomplete-stream rate, recent latency, sample
count, recency-decayed weights).
Transparent deterministic formulas with documented constants - no ML.
Scope
src/routing/health.ts:healthEvidenceForCandidate()- live Codex account cooldown/soft-avoid(authoritative) + historical evidence from the request-history index
(per provider/model/accountRef, 14-day window, 100-sample cap,
exponential recency decay, half-life 7 days).
healthScore()- deterministic composite with documented constants(
SUCCESS_WEIGHT 0.50,INCOMPLETE_WEIGHT 0.15,LATENCY_WEIGHT 0.20,RECOVERY_WEIGHT 0.15,LATENCY_TARGET_MS 60000,MIN_CONFIDENCE_SAMPLES 20,SOFT_AVOID_MULTIPLIER 0.5). Unknownevidence returns null (never zero); low samples scale confidence down;
live hard cooldown scores 0.
src/routing/history/indexer.ts- synchronous refresh(
openRequestHistoryIndexSync) so routing time can read health evidencewithout async plumbing; the async single-flight wrapper delegates to it.
src/routing/evaluator.ts- health scoring folded into the candidatescore:
total = priority*(1-healthWeight) + healthValue*healthWeightwhere
healthWeight = profile.optimize.health. Hard cooldown excludes thecandidate (
cooldownexclusion). Unknown health follows the profileunknownEvidence.healthpolicy:exclude(default excludes),penalize(deterministic 0.3 floor),
allow(priority-only score). Historical healthnever overrides explicit ineligibility.
src/routing/trace.ts- trace candidates now carry capability/health/quota/cost evidence, so the durable trace records the health evidence that
shaped the decision.
src/router.ts- policy execution assembles health evidence per candidate.tests/health-scoring.test.ts- 9 tests.Failure classification
client_cancel) are neutral - never healthdamage.
routing layer already records (host/account split per [Bug]: DNS and network reachability failures incorrectly rotate Codex pool accounts #914 work) - this PR
consumes that boundary rather than re-classifying.
Privacy / security
no prompts, credentials, or raw bodies.
bun run privacy:scanpasses.Compatibility
optimize.health: 0or supplies no health evidence with
unknownEvidence.health: allow.Dependency
feat/ri-05-capability-aware-routinghead56f17f45c.Non-goals
([Bug]: DNS and network reachability failures incorrectly rotate Codex pool accounts #914) - those stay owned by their existing units; we only read their
resulting state.
Local verification (exact)
bun x tsc --noEmit-> PASSED (0 errors)bun run test tests/health-scoring.test.ts-> 9/9 passbun run privacy:scan-> passedNotes for reviewers
tests/policy-execution.test.tsandtests/routing-profile.test.tsreflect the intentional new healthcomponent (unknown health + default
penalize-> 0.3 floor).available window; recorded in the stack ledger.
Summary by CodeRabbit
New Features
Bug Fixes