fix(combo): stop quota-weighted routing onto out-of-credit connections - #13006
Conversation
77688e9 to
f6d58f8
Compare
f6d58f8 to
40bfcd8
Compare
|
Rebased onto #13221 ( Conflict was only The 402 fixture tests now insert live connection rows. Invented pin UUIDs no longer reach the weighted draw after the eligibility filter. HEAD: |
Pins and allowlists only select among currently active, non-banned rows for the target provider. A 30-second strategy-level connection cache could keep a just-disabled account in the pool; reuse the existing invalidating DB cache instead. When a quota-capable provider has no eligible rows, do not fall back to an unscoped provider target. Antigravity automatic exhaustion now requires a reported remaining of zero. The old 99% usage cutoff treated a positive remainder below 1% as empty, which contradicted quota-weighted reserve-pool scoring. Explicit operator cutoffs stay independent. Signed-off-by: Minxi Hou <houminxi@gmail.com>
The empty-pool skip already dropped a quota-capable target with no eligible rows. Add the allowlist-only-ineligible case, and drop the stale resetAwareConnectionCache mention from the module comment. Signed-off-by: Minxi Hou <houminxi@gmail.com>
A combo using quota-weighted routing kept dispatching to a connection that had already answered 402 for a depleted balance. Two things let that happen. Upstream refusing a request for credits never reached the quota cache from the combo attempt path, so the connection kept whatever remaining percentage its last snapshot held and the next weighted draw could pick it straight again. A 402 is the most authoritative reading of an account's balance we ever get, so record it the same way a 429 is recorded. Credits come back, so the connection stays active and a later refresh or window reset clears the mark. Snapshot age was also unbounded: an observation from five hours ago carried the same weight as one from five minutes ago, which is how a snapshot claiming 1% remaining outlived the balance it described. Age now decides pool placement only. A stale snapshot means the headroom is unknown, not gone, so those connections fall back to the secondary pool and still receive traffic when nothing fresher has room. Signed-off-by: Minxi Hou <houminxi@gmail.com>
Signed-off-by: Minxi Hou <houminxi@gmail.com>
…store Invented pin UUIDs no longer reach the weighted draw after eligibility filters missing rows. The 402 and staleness cases now insert live connections so they still cover snapshot accounting. Signed-off-by: Minxi Hou <houminxi@gmail.com>
40bfcd8 to
00e07ff
Compare
The 402 snapshot tests cover quotaStrategies. Register them in tap.testFiles so mutant kills count on this branch. Signed-off-by: Minxi Hou <houminxi@gmail.com>
|
Pushed Remaining Docs / Fast Quality / ESLint / unit-shard red matches current |
49b6c3e
into
diegosouzapw:release/v3.8.51
diegosouzapw#13006) Both gaps are real and they compound: `executeTargetAttempt` already classified the 402 through `isQuotaExhaustionResponse` and then dropped it, so the only writer into the quota cache was the 429 path in `chat.ts`. A snapshot reading `remaining=1%, is_exhausted=0` five hours stale is then exactly what quota-weighted routing will keep picking. --- Validated in one consolidated worktree cut from `release/v3.8.51`, boarded with the other 19 PRs of this batch. Two in-batch conflicts, both additive and resolved by keeping each side: the `ENVIRONMENT.md` table (diegosouzapw#13035 + diegosouzapw#13011) and the `chatHelpers.ts` import block (diegosouzapw#12975 on the tip + diegosouzapw#13017). - `typecheck:core` clean; `check:dashboard-typecheck` OK (206 pre-existing, within baseline); `check:changelog-integrity` OK; `check:docs-counts` migrations ✓ - complexity 2816 / baseline 3218 and cognitive-complexity 1271 / baseline 1437 — both under baseline - 531 of 532 focused assertions green across the batch's 46 test files - `check-file-size` rebaselined for the batch's real growth (annotation `_rebaseline_2026_09_11_mergebatch_v3851_houminxi`, landed on diegosouzapw#13038), attributed per PR The single red is **not this batch**: `tests/unit/combo/quota-weighted-strategy.test.ts` → "A/B isolation: 7 hard-empty + 2 at 0.5% + 1 at 40%, floor=1" asserts an order between two connections of identical weight and flakes on the pure tip too — 2 failures in 4 runs at `origin/release/v3.8.51` with nothing from this batch applied.⚠️ base-red inherited: diegosouzapw#12732 — `Docs Gates`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` reproduce on the pure tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and `open-sse/utils/stream.ts` at 3115 > frozen 3098, untouched here). Thanks @HouMinXi — the live evidence on these (X500 logs, `storage.sqlite` state, real `/v1/models` probes, the 36-minute outage write-up) is what let a 20-PR batch be reviewed as a unit.
diegosouzapw#13006) Both gaps are real and they compound: `executeTargetAttempt` already classified the 402 through `isQuotaExhaustionResponse` and then dropped it, so the only writer into the quota cache was the 429 path in `chat.ts`. A snapshot reading `remaining=1%, is_exhausted=0` five hours stale is then exactly what quota-weighted routing will keep picking. --- Validated in one consolidated worktree cut from `release/v3.8.51`, boarded with the other 19 PRs of this batch. Two in-batch conflicts, both additive and resolved by keeping each side: the `ENVIRONMENT.md` table (diegosouzapw#13035 + diegosouzapw#13011) and the `chatHelpers.ts` import block (diegosouzapw#12975 on the tip + diegosouzapw#13017). - `typecheck:core` clean; `check:dashboard-typecheck` OK (206 pre-existing, within baseline); `check:changelog-integrity` OK; `check:docs-counts` migrations ✓ - complexity 2816 / baseline 3218 and cognitive-complexity 1271 / baseline 1437 — both under baseline - 531 of 532 focused assertions green across the batch's 46 test files - `check-file-size` rebaselined for the batch's real growth (annotation `_rebaseline_2026_09_11_mergebatch_v3851_houminxi`, landed on diegosouzapw#13038), attributed per PR The single red is **not this batch**: `tests/unit/combo/quota-weighted-strategy.test.ts` → "A/B isolation: 7 hard-empty + 2 at 0.5% + 1 at 40%, floor=1" asserts an order between two connections of identical weight and flakes on the pure tip too — 2 failures in 4 runs at `origin/release/v3.8.51` with nothing from this batch applied.⚠️ base-red inherited: diegosouzapw#12732 — `Docs Gates`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` reproduce on the pure tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and `open-sse/utils/stream.ts` at 3115 > frozen 3098, untouched here). Thanks @HouMinXi — the live evidence on these (X500 logs, `storage.sqlite` state, real `/v1/models` probes, the 36-minute outage write-up) is what let a 20-PR batch be reviewed as a unit.
Problem
A combo on
quota-weightedkept dispatching to a connection that had alreadyanswered
402for a depleted balance.Observed on a live deployment: the connection returned
while its stored quota snapshot still read
remaining=1%,is_exhausted=0,last refreshed five hours earlier. The router had no reason not to pick it
again, so it did.
Two separate gaps let that happen.
A 402 never reached the quota cache from the combo path.
executeTargetAttemptalready classified the response throughisQuotaExhaustionResponseand then dropped the result on the floor. The onlywriter into the cache was
markAccountExhaustedFrom429, reached fromchat.tson a 429 and nowhere else. So the connection kept whatever remainingpercentage its last snapshot happened to hold.
Snapshot age was unbounded. An observation from five hours ago carried the
same weight as one from five minutes ago. That is how a snapshot claiming 1%
outlived the balance it described.
Change
markAccountExhaustedFromCreditsrecords a 402 the same way a 429 isrecorded, called from the combo attempt path where the classification already
existed. The connection stays active - credits come back, and a later refresh
or window reset clears the mark through the paths that already clear a 429.
orderTargetsByQuotaWeightednow consults the live snapshot when it builds thepools. A 402-marked connection scores zero. A snapshot older than
QUOTA_WEIGHTED_MAX_SNAPSHOT_AGE_MS(10 minutes, twice the active refreshcadence so one missed tick is absorbed) drops from the primary pool to the
secondary one.
Staleness means unknown, not empty. A stale connection is still routed to when
nothing fresher has room, and an all-stale set still returns a full ordering
rather than nothing. The soft floor is untouched: default 1%, and
0 < remaining <= floorstill lands in the secondary pool.Nothing is ever deactivated or deleted.
isQuotaExhaustedForRequestisdeliberately left alone as an eviction test - with
DEFAULT_QUOTA_THRESHOLD_PERCENTat 99 it would empty the secondary pool.Tests
tests/unit/combo/quota-weighted-stale-402.test.ts- 11 tests.tests/unit/combo/**- 253 passing.typecheck:coreclean.Each guard was verified by injection: deleting the 402 call site, inverting it
to 429, neutering
markAccountExhaustedFromCredits, dropping themarked === 0override, removing the staleness filter from the primary pool,and relaxing the age comparison from
>to>=each turn a specific test red.The first attempt at the 402 test called the helper directly and stayed green
with the call site deleted, which is worth noting: the tests that cover it now
drive a real 402 through
executeTargetAttempt.