Skip to content

refactor(combo): decompose combo.ts and enforce the live latency budget - #53

Merged
LMPrado-DZ23 merged 6 commits into
release/v3.8.55from
refactor/combo-decomposition-latency-budget
Sep 20, 2026
Merged

LMPrado-DZ23 merged 6 commits into
release/v3.8.55from
refactor/combo-decomposition-latency-budget

Conversation

@LMPrado-DZ23

@LMPrado-DZ23 LMPrado-DZ23 commented Sep 19, 2026 •

Copy link
Copy Markdown
Owner

combo.ts line count

lines (check-file-size count)
before 4080
after 3779

The frozen entry in config/quality/file-size-baseline.json is lowered 4080 → 3779. It only goes down.

What each new module owns

Module Owns
open-sse/services/combo/autoCandidates.ts Building the AutoProviderCandidate rows the auto strategy scores: the connection pool per provider, the per-connection/per-fingerprint target expansion, the blended price per 1M tokens, the latency profile (24h p95 → live metric → per-model bootstrap, plus jitter, error rate and speed telemetry) and the quota picture (reset-window affinity, remaining quota, the opt-in hard cutoff, the soft status penalty). combo.ts re-exports buildAutoCandidates, so every importer resolves as before.
open-sse/services/combo/latencyBudget.ts Applying the request's maxLatencyMs to live routing: which candidates are routable and which failover targets survive. With no budget both functions are the identity.

exceedsLatencyBudget() was added to open-sse/services/routing/attemptPolicy.ts — the module that already owned the budget machinery — so the decision's latency_over_budget reason and the live filter cannot drift apart.

Equivalence: provider selection is unchanged for requests with no latency budget

Baseline modules extracted read-only with git show origin/release/v3.8.55:… and wired to each other; both halves run against the same shared scoring modules (scoring.ts, engine.ts, taskFitness.ts, decisionStore.ts), the same seeded case generator, a seeded Math.random, a frozen clock, and each in its own process so rotator/self-healing state starts identical.

== PIPELINE GATE base vs head          (real resolveAutoStrategyOrder, 700 seeded cases)
selection equivalence: 700/700 cases identical
Math.random() calls: base=425 head=425 (equal)
per-case Math.random() call counts: all equal

== DECISION-LAYER GATE base vs head    (selectAutoProviderWithDecision + failover ordering)
selection equivalence: 700/700 cases identical
Math.random() calls: base=824 head=824 (equal)
per-case Math.random() call counts: all equal

Sample coverage: 618 selections, 80 strict-budget refusals, 77 rotations, 33 exploration picks, 342 explicit-router picks, 58 early responses, 5155 candidate rows compared field by field (score, eligibility, exclusion reasons, quota, circuit, estimates, factor breakdown). Both halves are self-reproducible (base vs base and head vs head are also 700/700).

The candidate-builder split has its own equivalence run against the pre-split version: 120/120 seeded target-list cases produce byte-identical candidate rows.

== PREVIEW PURITY
previews run: 598
Math.random() calls inside previewRoutingDecision: 0
repeated previews returning the same decision: 299/299
live selections identical with vs without previews interleaved: yes (299 live selections)

Every harness installs a fetch/http trap after all open-sse/ imports (so proxyFetch.ts's import-time globalThis.fetch replacement cannot bypass it), asserts it is the live fetch, and throws on any call. All runs report outbound network attempts: 0. The harnesses exercise selection only — they call resolveAutoStrategyOrder / selectAutoProviderWithDecision / buildAutoCandidates and never reach dispatch; candidates are injected through the buildAutoCandidates dependency.

What live traffic now enforces

  • A request that sends X-OmniRoute-Latency-Budget: <ms> gets a RoutingBudget.maxLatencyMs for that request only. No header, no budget — there is no stored combo-level latency cap.
  • A candidate whose estimatedLatencyMs (its p95 estimate) exceeds the budget is excluded from selection and dropped from the failover chain, and is reported on the recorded decision with latency_over_budget. Unlike the cost cap, over-budget targets are never merely moved last, so failover cannot walk back over the budget.
  • It binds every router strategy (rules and the explicit ones), because the caller asked for it on this request.
  • If nothing fits, the request is refused with a 503 naming the budget instead of being served over it.
  • Checked per attempt against the candidate's estimate — there is still no cumulative spend check and no elapsed-time check across attempts. docs/routing/ROUTING_CONTRACT.md says exactly this.

Gates

  • new-code complexity (the mode CI runs): node scripts/check/check-complexity-ratchets.mjs --base-ref <merge-base> → complexityNewCode=-3 (10 violations in touched files vs 13 on the base), cognitiveComplexityNewCode=-1 (5 vs 6). autoCandidates.ts itself reports 0 violations of complexity, sonarjs/cognitive-complexity and max-lines-per-function. The first push did move the complexity rather than reduce it; the second commit splits the builder into one function per signal, which is what turns the per-file line green.
  • four typechecks with tsc directly (open-sse/tsconfig.json, tsconfig.typecheck-core.json, tsconfig.typecheck-api.json, tsconfig.typecheck-dashboard.json) — 0 errors each
  • eslint --max-warnings=0 --suppressions-location config/quality/eslint-suppressions.json --pass-on-unpruned-suppressions --no-warn-ignored on every changed file — exit 0
  • check-file-size: combo.ts at its new lower number; the only violation is the known pre-existing src/lib/db/core.ts: 1770 > 1745
  • mutation-coverage drift (findCoverageDrift called directly): {} over 5080 unit tests, including the new one
  • focused routing / auto-combo / combo suites: 189/189 pass. The broad sweep's remaining failures are equally red on release/v3.8.55 (the models-catalog-combo-metadata timeouts swing 5→12 failures run to run on the base tree too, g13-combo-chatcore-golden fails identically on base, and tests/unit/autoCombo/provider-family-combos.test.ts fails 3/3 on base). The repeated SqliteError: UNIQUE constraint failed: call_logs.id in the shard log is pre-existing noise from concurrent call-log writes in combo dispatch tests — nothing in this PR touches call-log persistence, and it is logged, not asserted.

prettier --check still flags open-sse/services/combo.ts — it is not prettier-clean on release/v3.8.55 either (the diegosouzapw#3470 cooldown-retry work deliberately kept ~860 lines at their original indentation), so it is left alone; every other changed file passes.

The diegosouzapw#12600 guard is now behavioural

tests/unit/reset-aware-request-scope-12600.test.ts used to read combo.ts as text and assert getQuotaFetchScope( appeared in it — a proxy that broke the moment the builder moved. It now drives buildAutoCandidates with a spy quota fetcher and caching off: two model families on one Antigravity account produce two quota fetches, two models of one family produce one. The companion "one definition, used by the reset-aware fetcher" assertion stays a source check, because no observable behaviour distinguishes one definition from a duplicate.

🤖 Generated with Claude Code

…o.ts

buildAutoCandidates() and its private helpers (bootstrap p95 table,
history-sample threshold, output-token ratio, context affinity) move
verbatim to open-sse/services/combo/autoCandidates.ts, which owns building
the AutoProviderCandidate rows the scoring engine reads: pricing, latency
profile, quota/cutoff state, circuit and session signals. combo.ts keeps
re-exporting buildAutoCandidates, so every importer resolves as before.

Pure move. The only edits inside the moved code are the dynamic
import("../../../src/lib/usageDb") path (one directory deeper) and prettier
wrapping one call over three lines. combo.ts drops the 21 imports only the
builder used.

combo.ts: 4080 -> 3779 lines (check-file-size count); its frozen
file-size entry is lowered to 3779.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: a053fea2-f37c-4a13-9131-756498830b0f


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

zodyprado-web and others added 3 commits September 19, 2026 21:29
…gnal

The first extraction moved buildAutoCandidates out of combo.ts unchanged,
which relocated its complexity instead of reducing it: the new file scored
3 cyclomatic and 1 cognitive violation where it had none, and Fast Quality
Gates rejected it.

buildAutoCandidates is now a sequencer over named helpers, each owning one
input the scoring engine reads: loadHistoricalLatencyStats,
loadProviderConnections, expandCandidateTargets, resolveCandidateCost,
resolveLatencyProfile (resolveP95LatencyMs + resolveErrorRate) and
resolveQuotaSignals (statusSignals + the quota fetch). The file now reports
zero complexity, cognitive-complexity and max-lines-per-function violations.

Behaviour is unchanged and checked: 120/120 seeded target-list cases build
byte-identical candidate rows against the pre-split version extracted with
`git show`, with zero outbound network attempts under a fetch trap. The
connection-pool lookup keeps its original provider key (no alias fallback),
which the helper's comment now records.

tests/unit/reset-aware-request-scope-12600.test.ts scanned combo.ts for the
shared getQuotaFetchScope() call; it now scans combo/autoCandidates.ts,
where the builder lives.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
RoutingBudget.maxLatencyMs and the latency_over_budget exclusion reason
existed in the contract, but only route previews applied them: live traffic
enforced the cost half only. A request can now send
`X-OmniRoute-Latency-Budget: <ms>` and the live auto combo honours it.

- exceedsLatencyBudget() (routing/attemptPolicy.ts) is the single definition
  of the test, shared by the recorded decision's exclusion reasons and the
  live filter, so what a decision says was excluded is what selection and
  failover refused to use. No budget, or an unknown estimate, never excludes.
- combo/latencyBudget.ts narrows the routable candidates and drops
  over-budget targets from the failover chain. Unlike the cost cap it never
  merely reorders: falling back must not silently break the budget.
- The budget travels on the decision context, so an excluded candidate is
  reported with `latency_over_budget` instead of looking merely unselected.
- It binds every router strategy, not only `rules`, because the caller asked
  for it on this request. If nothing fits, the request gets a 503 naming the
  budget rather than an over-budget answer.
- Opt-in and per request: with no header, nothing changes. Proven over a
  700-case seeded sample with a frozen clock — 700/700 decisions, candidate
  exclusions and failover orders identical to release/v3.8.55, with equal
  Math.random() call counts overall and per case, and zero outbound network
  attempts under a fetch trap.

docs/routing/ROUTING_CONTRACT.md now states what live traffic enforces: the
latency budget is per attempt against each candidate's p95 estimate, there is
still no cumulative spend or elapsed-time check, and previews are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
zodyprado-web and others added 2 commits September 19, 2026 23:29
…ource text

The diegosouzapw#12600 guard read combo.ts as text and asserted `getQuotaFetchScope(`
appeared in it. That was always a proxy for "the builder scopes the quota
fetch per model family", and it broke the moment the builder moved — a regex
over a file path cannot survive a refactor.

It now drives buildAutoCandidates with a spy quota fetcher and caching off:
two model families on one Antigravity account produce two quota fetches, two
models of one family produce one. That is the behaviour diegosouzapw#12600 shipped (a
Claude-empty account must not hide its Gemini window), and it holds wherever
the builder lives next.

The companion assertion that quotaStrategies.ts uses the shared helper and
does not redefine it stays as a source check: there is no observable
behaviour that distinguishes one definition from a duplicate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants