feat(lab): CL-06 routing compatibility policy - #1394
Conversation
Extend Routing Profiles with optional Compatibility Lab evidence gates, bounded read-only verdict lookup, evaluator penalties, trace explainability, management API parity, and Models editor controls without routing-path probes.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughRouting profiles now support Compatibility Lab suite requirements. The server derives deterministic route subjects, reads bounded verdict snapshots, evaluates compatibility during routing, records bounded traces, exposes management API data, and provides localized GUI controls. ChangesRouting compatibility
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
UI screenshot waived by the Hygiene✅ Deterministic PR hygiene checks passed. |
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 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/260807_compatibility_lab/001_pr_stack_status.md`:
- Line 28: Update the CL-06 entry in the PR stack status table to replace
“PENDING CI” with the verified current PR-quality-gates state “READY,”
preserving the Draft status and existing commit and PR references.
In `@gui/src/i18n/en.ts`:
- Around line 84-85: Update the routing.compatibility.loadingCatalog and
routing.compatibility.catalogUnavailable translations to use the American
spelling “catalog” instead of “catalogue,” preserving the existing messages and
punctuation.
In `@gui/src/pages/RoutingProfiles.tsx`:
- Around line 74-77: Replace the duplicate LabCatalogScenario type with an alias
to the imported CompatibilitySuiteDraft type. Update references to use that
shared type while preserving the existing suiteId and evidenceLayer behavior.
- Around line 85-99: Update uniqueCatalogSuites to validate each scenario’s
suiteId and evidenceLayer before calling catalogSuiteKey or adding it to the
result; skip malformed scenarios so sorting never invokes localeCompare on
undefined. Preserve valid scenarios and existing deduplication and ordering
behavior, allowing the page load to continue when catalogJson.scenarios contains
partial entries.
In `@src/routing/compatibility/assemble.ts`:
- Around line 47-54: The assembled verdicts in the suites mapping should not
hardcode fresh: true. Remove fresh from ObservedSuiteVerdict and omit it from
the assembler output, keeping freshness derived from asOf and the policy’s
effective max age.
- Around line 71-90: Gate the candidate subject-resolution loop around
routedProviderConfig and resolvePolicyRouteSubject on profile.compatibility,
matching the existing compatibility snapshot condition. Skip populating
subjectIds and resolvedByCandidate when the profile has no compatibility policy,
while preserving the current resolution behavior for compatible profiles.
- Around line 100-106: Update attachCompatibilityEvidence and its call site in
assemblePolicyCandidateEvidence to reuse the already resolved subject from
resolvedByCandidate instead of calling resolvePolicyRouteSubject a second time.
Pass that subject into the compatibility evidence flow, remove the redundant
resolution and subjectId mutation, and preserve the unknown-evidence fallback
when the initial subject construction fails.
In `@src/routing/compatibility/policy.ts`:
- Around line 20-37: The effective max-age lookup must be bounded and
non-repeated: in src/routing/compatibility/policy.ts:20-37, hoist or memoize
queryLabCatalog results by evidenceLayer and suiteId so effectiveMaxAgeMs does
not query the catalog for every candidate. In src/routing/profile.ts:353-411,
update routingProfileIssues to reject compatibility.requiredSuites when it
exceeds the defined maximum length, preventing unbounded evaluation cost.
- Around line 72-75: Update the requirement loop around
MAX_TRACE_COMPATIBILITY_SUITES so it evaluates every policy.requiredSuites entry
without breaking when the trace cap is reached. Preserve the cap by routing each
trace append through the existing pushTrace helper, replacing direct
outcome.suiteTraces.push calls while continuing to accumulate exclusions and
penalties for all requirements.
In `@src/routing/compatibility/subject.ts`:
- Around line 53-61: Update buildRouteSubjectV1 to create one shared
effective-route helper that applies resolveWireProtocolOverride to determine the
effective adapter. Reuse that adapter for effectiveAdapter, upstreamProtocol,
resolveProductionBehaviorValues, and providerInstanceKey so all route identity
fields remain consistent and continue using the shared routing/config layers.
In `@src/routing/compatibility/version.ts`:
- Around line 51-57: Replace the runtime-only fallback assigned to cachedVersion
with the embedded/generated compatibility version from
generated/compatibility-version.json, or invalidate the compatibility subject
when that artifact is unavailable. Ensure the fallback does not hash fixed
schema fields, an empty file list, and Bun.version or create a subject that
resolvePolicyRouteSubject can reuse.
In `@src/routing/evaluator.ts`:
- Around line 403-406: Update evaluateCompatibilityForCandidate so compatibility
always receives a score: use a satisfied score of 1 for verified evidence and
the existing penalty score for unknown, stale, or degraded evidence under
penalize mode. Define and reuse COMPATIBILITY_WEIGHT and
COMPATIBILITY_SATISFIED_SCORE alongside COMPATIBILITY_UNKNOWN_PENALTY_SCORE in
the compatibility types, and apply the weight through the same normalized
accounting as other dimensions so unused weight returns to priority.
In `@src/routing/profile.ts`:
- Line 517: Update normalizeRoutingProfile to call normalizedCompatibility(raw)
once, store its result in a local variable, and reuse that variable for both the
conditional check and compatibility property value. Preserve the current
omission of compatibility when the result is absent.
In `@src/routing/trace.ts`:
- Around line 569-599: Update normalizeRouteDecisionTrace() to propagate
compatibility truncation into the persisted truncated marker: set
truncated.compatibility when either caps.compatibility or incoming.compatibility
indicates truncation. Preserve the existing truncation handling for all other
fields.
- Line 114: Update the evaluator’s priorityWeight calculation to subtract 0.05
whenever compatibilityValue is present, before adding the existing 0.05 *
compatibilityValue score. Keep the current health, quota, and cost deductions
unchanged and ensure candidates without compatibilityValue retain their existing
weighting.
In `@src/server/management/routing-profile-routes.ts`:
- Around line 102-104: Update the dry-run handler to capture one timestamp and
pass it to both assemblePolicyCandidateEvidence and evaluatePolicyProfile, using
the evaluator’s now parameter rather than its Date.now default. Preserve the
existing candidate, exclusion, health, compatibility, and quota evaluation flow
while ensuring both calls use the same instant.
In `@tests/routing-compatibility.test.ts`:
- Around line 98-105: Update the test “exact route identity changes when adapter
changes” to keep provider ID, model ID, and endpoint identical for both resolved
routes, changing only the adapter configuration before comparing subjectId
values. Reuse the same provider/model/base URL inputs while varying the adapter
passed to routed, preserving the existing definedness and inequality assertions.
- Around line 224-236: The parity test currently invokes evaluatePolicyProfile
identically twice, so it does not exercise separate dry-run and production
boundaries. Replace the duplicate calls in the “dry-run and production evaluator
parity with same evidence” test with the management dry-run path and production
routing path, supplying fixed evidence, then assert matching selected candidate,
exclusions, score penalty, and compatibility trace; keep the regression test
near the existing routing tests.
- Around line 270-276: Update the test “compatibility profile revision changes
only when compatibility changes” to clone the existing compat profile rather
than normalize the unrelated legacy profile. Modify exactly one compatibility
field, such as minStatus, in the clone, normalize it with the compat ID, and
assert its revision differs from the unchanged compat revision while the
unchanged profile still matches the base revision.
In `@tests/routing-profile-management-editor.test.ts`:
- Around line 157-182: Strengthen the test around the existing PUT and GET
requests by adding mutation/projection counters to deps(), recording their
values after the PUT, and asserting the GET /api/lab/catalog leaves them
unchanged. Also snapshot the relevant configuration after the PUT and verify it
remains unchanged after the GET, while preserving the existing successful
response and scenarios-array assertions.
🪄 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: 1e3eb837-fe11-4533-8b70-9aecc0f90702
📒 Files selected for processing (28)
devlog/_plan/260807_compatibility_lab/001_pr_stack_status.mddevlog/_plan/260807_compatibility_lab/006_cl06_routing_compatibility.mdgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/tr.tsgui/src/i18n/zh.tsgui/src/pages/RoutingProfiles.tsxgui/src/routing-profile-editor-data.tssrc/router.tssrc/routing/compatibility/assemble.tssrc/routing/compatibility/behavior.tssrc/routing/compatibility/endpoint.tssrc/routing/compatibility/policy.tssrc/routing/compatibility/reader.tssrc/routing/compatibility/subject.tssrc/routing/compatibility/types.tssrc/routing/compatibility/version.tssrc/routing/evaluator.tssrc/routing/profile.tssrc/routing/trace.tssrc/server/management/routing-profile-routes.tssrc/types.tstests/routing-compatibility.test.tstests/routing-profile-editor-data.test.tstests/routing-profile-management-editor.test.ts
|
CL-06 review remediation is now on head I independently validated the original CodeRabbit findings against the CL-00/CL-06 contracts rather than applying them mechanically. Fixed:
Two original comments are intentionally not changed:
Scoring note: I fixed the inversion without adopting the suggested universal satisfied score of Additional independent fixes beyond the Rabbit list: exact ProtocolSubjectV1 vs RouteSubjectV1 handling, exact suite-version/digest selection, behavior-fingerprint completeness, and the routing-time installation-salt mutation were all corrected in this pass. Validation on this head: React Doctor is green; Cross-platform CI core gates, packaging on all three OSes, keyring jobs, storage policy, and shards 1/4 through 3/4 are green. macOS main and shard 4/4 were still running at the time of this comment; I will not call the full matrix green until the workflow itself completes successfully. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gui/src/pages/RoutingProfiles.tsx (1)
868-915: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLocalize the Compatibility field labels.
Lines 869, 882, and 900 render
maxEvidenceAgeMs,unknownEvidence, anddegradedEvidenceas hardcoded user-visible text. Add locale keys and render them witht(...). Update every supported locale file with the new keys.Proposed fix
- <code>maxEvidenceAgeMs</code> + {t("routing.compatibility.maxEvidenceAgeMs")} ... - <code>unknownEvidence</code> + {t("routing.compatibility.unknownEvidence")} ... - <code>degradedEvidence</code> + {t("routing.compatibility.degradedEvidence")}🤖 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 `@gui/src/pages/RoutingProfiles.tsx` around lines 868 - 915, Localize the user-visible labels in the compatibility fields rendered by the RoutingProfiles component: replace the hardcoded maxEvidenceAgeMs, unknownEvidence, and degradedEvidence text with t(...) lookups. Add matching translation keys and values to every supported locale file, preserving the existing routing translation namespace and label semantics.Source: Path instructions
🤖 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 `@scripts/generate-compatibility-version.ts`:
- Around line 85-97: Add src/generated/compatibility-version.json to the package
preparation flow by invoking generateCompatibilityVersionManifest through the
package.json prepare:package script, and add the same generated path to
.gitignore. Keep the existing generator behavior and packaging configuration
unchanged.
In `@src/routing/compatibility/behavior.ts`:
- Around line 59-66: Update authTransportFor to accept the already-resolved auth
mode as an argument, and pass the mode resolved with effective.authMode,
registryEntry?.authKind, and the "key" fallback from the caller. Remove the
helper’s independent effective.authMode resolution so auth.mode and
auth.transport always use the same registry-backed value.
In `@tests/routing-compatibility.test.ts`:
- Around line 362-364: Update the VERIFIED evidence construction in the
satisfied evaluatePolicyProfile case to pass timestamp 1000 to evidence instead
of relying on its default current timestamp. Keep the evaluator’s now value and
the UNKNOWN case unchanged so the verified evidence is fresh and remains
higher-scoring than the penalized candidate.
---
Outside diff comments:
In `@gui/src/pages/RoutingProfiles.tsx`:
- Around line 868-915: Localize the user-visible labels in the compatibility
fields rendered by the RoutingProfiles component: replace the hardcoded
maxEvidenceAgeMs, unknownEvidence, and degradedEvidence text with t(...)
lookups. Add matching translation keys and values to every supported locale
file, preserving the existing routing translation namespace and label semantics.
🪄 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: 4800e38a-85a2-4dd9-b435-412d972b2d42
📒 Files selected for processing (23)
gui/src/pages/RoutingProfiles.tsxscripts/generate-compatibility-version.tsscripts/prepare-package.tssrc/lab/observe/from-conformance.tssrc/lab/subject/installation-salt.tssrc/lab/subject/protocol-subject.tssrc/lab/subject/route-subject.tssrc/routing/compatibility/assemble.tssrc/routing/compatibility/behavior.tssrc/routing/compatibility/catalog.tssrc/routing/compatibility/endpoint.tssrc/routing/compatibility/policy.tssrc/routing/compatibility/reader.tssrc/routing/compatibility/subject.tssrc/routing/compatibility/types.tssrc/routing/compatibility/version.tssrc/routing/evaluator.tssrc/routing/profile.tssrc/routing/trace.tssrc/server/management/routing-profile-routes.tstests/compatibility-version.test.tstests/routing-compatibility-boundaries.test.tstests/routing-compatibility.test.ts
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
|
Summary
compatibilityblock toOcxRoutingProfileConfig(required suites, min statusPROBED/VERIFIED, max evidence age, unknown/degraded policies) with backward-compatible revision hashing when omitted.RouteSubjectV1via shared CL-03 subject/fingerprint helpers; consume existing Lab projection through one bounded SQLite snapshot per evaluation.RouteDecisionTraceV1with bounded suite-level compatibility detail.Frozen compatibility schema
Exact-route evidence matching
Production routing derives
RouteSubjectV1from provider identity, routed provider config, adapter/protocol/surface, DNS-free endpoint fingerprint, behavior fingerprint, provider-instance fingerprint, and OpenCodex compatibility version — reusing CL-03buildRouteSubjectV1/subjectIdForSubject.Routing-path side-effect prohibition
Policy evaluation and dry-run only read existing projection verdicts. No probes, protocol conformance runs, live-route tests, upstream compatibility calls, DNS resolution, Agent Fabric work, projection rebuilds, or ledger replay.
Verdict / freshness semantics
PROBED,VERIFIEDonly (VERIFIEDsatisfiesPROBED).UNSUPPORTEDexcludes;DEGRADEDfollowsdegradedEvidence;UNKNOWN/CLAIMED/BLOCKEDfollowunknownEvidence.maxEvidenceAgeMs/ catalogue freshness ceilings.Backward compatibility
Profiles without
compatibilityretain pre-CL-06 validation, eligibility, scoring, routing, and revision behavior.Validation performed
bun x tsc --noEmitbun test tests/routing-compatibility.test.ts(27)bun test tests/routing-profile*.test.ts tests/route-decision-trace.test.ts(64)bun test tests/lab-read-surfaces.test.tsbun test tests/routing-policy-surface-parity.test.tsbun run privacy:scanExplicit non-goals
CL-07 (Agent Fabric / task effectiveness) is not started in this PR.
Test plan
Summary by CodeRabbit