From 3bcb181cc2de83cf7c130b1eacd2c89ef0e8758d Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Thu, 17 Sep 2026 07:01:24 +0700 Subject: [PATCH] fix(quota): apply the equal-split fallback in the pool usage snapshot (#13159) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The equal-split fallback was applied when computing allocations but not when building the pool usage snapshot, so `sqliteQuotaStore.ts:201` still read `totalWeight > 0 ? alloc.weight : 0` — a zero-weight pool reported every member at 0 instead of its equal share. Probe on your head: 9/9 pass in `tests/unit/quota-pool-usage-equal-split.test.ts`, with the bug confirmed unfixed on the tip. Thanks, @datrixlab — fixing the snapshot path and not just the allocation path is the part that makes the dashboard numbers agree with the enforcement. **Batch validation** — boarded with the other 10 PRs of your batch into one worktree cut from `release/v3.8.51`; every PR verified as an ancestor of the combined HEAD before validating. - Focused tests across all 11 PRs: **104/104 pass** on the combined tree. - Gates on the combined tree: `check-changelog-integrity` PASS, `check-complexity` PASS, `check-cognitive-complexity` PASS, `typecheck:core` PASS, `check:open-sse-typecheck` PASS. - `check-file-size` is red, but reproduces with byte-identical line counts on the pure `release/v3.8.51` tip (`open-sse/handlers/imageGeneration.ts` 3304, `open-sse/services/combo/roundRobinCombo.ts` 1221, `open-sse/utils/stream.ts` 3115). Inherited base-red, nothing added by this batch — it is also why this PR's "Fast Quality Gates" check was red. --- src/lib/quota/redisQuotaStore.ts | 8 +- src/lib/quota/sqliteQuotaStore.ts | 8 +- .../unit/quota-pool-usage-equal-split.test.ts | 159 ++++++++++++++++++ 3 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 tests/unit/quota-pool-usage-equal-split.test.ts diff --git a/src/lib/quota/redisQuotaStore.ts b/src/lib/quota/redisQuotaStore.ts index 7ff3588a1e..4f0cdc409e 100644 --- a/src/lib/quota/redisQuotaStore.ts +++ b/src/lib/quota/redisQuotaStore.ts @@ -271,7 +271,13 @@ export class RedisQuotaStore implements QuotaStore { const consumed = await this.peek(alloc.apiKeyId, dim); consumedTotal += consumed; - const effectiveWeight = totalWeight > 0 ? alloc.weight : 0; + // Equal-split fallback, matching enforce.ts: when every allocation in the + // pool has weight 0, each counts as an equal share, so the snapshot + // describes the budget enforcement actually grants. The old ternary could + // not do this -- totalWeight === 0 implies every alloc.weight is 0, so both + // arms returned 0 -- and every key rendered as borrowing all it consumed. + const effectiveWeight = + totalWeight > 0 ? alloc.weight : allocations.length > 0 ? 100 / allocations.length : 0; const fairShare = (effectiveWeight / 100) * planDim.limit; const deficit = consumed - fairShare; const borrowing = consumed > fairShare; diff --git a/src/lib/quota/sqliteQuotaStore.ts b/src/lib/quota/sqliteQuotaStore.ts index 8685e68c85..0e3e6295c4 100644 --- a/src/lib/quota/sqliteQuotaStore.ts +++ b/src/lib/quota/sqliteQuotaStore.ts @@ -198,7 +198,13 @@ export class SqliteQuotaStore implements QuotaStore { const consumed = await this.peek(alloc.apiKeyId, dim); consumedTotal += consumed; - const effectiveWeight = totalWeight > 0 ? alloc.weight : 0; + // Equal-split fallback, matching enforce.ts: when every allocation in the + // pool has weight 0, each counts as an equal share, so the snapshot + // describes the budget enforcement actually grants. The old ternary could + // not do this -- totalWeight === 0 implies every alloc.weight is 0, so both + // arms returned 0 -- and every key rendered as borrowing all it consumed. + const effectiveWeight = + totalWeight > 0 ? alloc.weight : allocations.length > 0 ? 100 / allocations.length : 0; const fairShare = (effectiveWeight / 100) * planDim.limit; const deficit = consumed - fairShare; const borrowing = consumed > fairShare; diff --git a/tests/unit/quota-pool-usage-equal-split.test.ts b/tests/unit/quota-pool-usage-equal-split.test.ts new file mode 100644 index 0000000000..6276c9234c --- /dev/null +++ b/tests/unit/quota-pool-usage-equal-split.test.ts @@ -0,0 +1,159 @@ +/** + * tests/unit/quota-pool-usage-equal-split.test.ts + * + * Regression: enforce.ts applies an equal-split fallback when every allocation + * in a pool has weight 0 (see quota-equal-split.test.ts), but neither quota + * store did the same in poolUsageWithDimensions. GET /api/quota/pools/[id]/usage + * therefore reported fairShare = 0 for every key in such a pool, so each one + * rendered as borrowing its entire consumption while enforcement was happily + * granting it 100/N of the budget. Same pool, two answers. + * + * The tell was that the store's ternary could not do anything: weights are + * non-negative, so `totalWeight === 0` implies every `alloc.weight` is already + * 0, and `totalWeight > 0 ? alloc.weight : 0` returned `alloc.weight` either + * way. The branch existed exactly where the fallback belonged. + * + * This is the same divergence class as quota-pool-usage-summed-budget.test.ts, + * which corrected the accountCount side of the enforce↔usage split. + * + * Levels: + * A (structural): both stores compute effectiveWeight with the equal-split + * fallback, in the same shape enforce.ts uses, and the no-op form is gone. + * B (logic): replicate the store's per-key math to prove the fallback fixes + * fairShare and clears the bogus borrowing flag, and that pools with real + * weights are untouched. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const ROOT = join(fileURLToPath(import.meta.url), "..", "..", ".."); +const read = (rel: string) => readFileSync(join(ROOT, rel), "utf8"); + +const STORES = [ + "src/lib/quota/sqliteQuotaStore.ts", + "src/lib/quota/redisQuotaStore.ts", +] as const; + +// --------------------------------------------------------------------------- +// Level A — structural: both stores carry the enforce.ts fallback +// --------------------------------------------------------------------------- + +for (const store of STORES) { + test(`${store} applies the equal-split fallback to effectiveWeight`, () => { + const src = read(store); + assert.ok( + /100 \/ allocations\.length/.test(src), + "store must fall back to an equal 100/N share when the pool has no weights" + ); + assert.ok( + /allocations\.length > 0\s*\?\s*100 \/ allocations\.length\s*:\s*0/.test(src), + "the equal split must guard against an empty allocation list, like enforce.ts" + ); + }); + + test(`${store} no longer zeroes effectiveWeight when the pool has no weights`, () => { + const src = read(store); + assert.ok( + !/const effectiveWeight = totalWeight > 0 \? alloc\.weight : 0;/.test(src), + "the old ternary was a no-op and must not come back" + ); + }); +} + +test("enforce.ts is still the shape the stores are mirroring", () => { + const src = read("src/lib/quota/enforce.ts"); + assert.ok( + /allocCount > 0\s*\?\s*100 \/ allocCount\s*:\s*0/.test(src), + "enforce.ts must keep the equal-split fallback the stores now mirror" + ); +}); + +// --------------------------------------------------------------------------- +// Level B — logic: the fallback corrects fairShare and the borrowing flag +// +// Replicates the per-key math from poolUsageWithDimensions: +// fairShare = (effectiveWeight / 100) × planDim.limit +// deficit = consumed - fairShare +// borrowing = consumed > fairShare +// --------------------------------------------------------------------------- + +/** The store's per-key snapshot, parameterised by the effectiveWeight rule. */ +function perKey( + allocations: Array<{ weight: number }>, + index: number, + limit: number, + consumed: number, + withFallback: boolean +) { + const totalWeight = allocations.reduce((sum, a) => sum + a.weight, 0); + const alloc = allocations[index]!; + const effectiveWeight = withFallback + ? totalWeight > 0 + ? alloc.weight + : allocations.length > 0 + ? 100 / allocations.length + : 0 + : totalWeight > 0 + ? alloc.weight + : 0; + const fairShare = (effectiveWeight / 100) * limit; + return { fairShare, deficit: consumed - fairShare, borrowing: consumed > fairShare }; +} + +test("all-zero pool: the snapshot now grants the same share enforcement does", () => { + const allocations = [{ weight: 0 }, { weight: 0 }]; + const LIMIT = 1000; + const CONSUMED = 100; + + const before = perKey(allocations, 0, LIMIT, CONSUMED, false); + assert.equal(before.fairShare, 0, "sanity: the bug gave every key a zero share"); + assert.equal(before.deficit, CONSUMED, "sanity: the whole consumption read as a deficit"); + assert.equal(before.borrowing, true, "sanity: every key was flagged as borrowing"); + + const after = perKey(allocations, 0, LIMIT, CONSUMED, true); + assert.equal(after.fairShare, 500, "two unweighted keys split the limit evenly"); + assert.equal(after.deficit, -400, "a key inside its share runs a negative deficit"); + assert.equal(after.borrowing, false, "and must not be flagged as borrowing"); +}); + +test("all-zero pool: the snapshot share matches what enforce.ts computes", () => { + const allocations = [{ weight: 0 }, { weight: 0 }, { weight: 0 }, { weight: 0 }]; + const LIMIT = 1000; + + // enforce.ts: effectiveWeight = 100 / allocCount when poolTotalWeight === 0 + const enforced = (100 / allocations.length / 100) * LIMIT; + const snapshot = perKey(allocations, 2, LIMIT, 0, true).fairShare; + + assert.equal(snapshot, enforced, "usage and enforcement must agree on the fair share"); + assert.equal(snapshot, 250, "four unweighted keys get a quarter each"); +}); + +test("weighted pools are untouched by the fallback", () => { + const allocations = [{ weight: 70 }, { weight: 30 }]; + const LIMIT = 1000; + + for (const index of [0, 1]) { + const before = perKey(allocations, index, LIMIT, 0, false); + const after = perKey(allocations, index, LIMIT, 0, true); + assert.equal(after.fairShare, before.fairShare, "a weighted pool keeps its shares"); + } + assert.equal(perKey(allocations, 0, LIMIT, 0, true).fairShare, 700); + assert.equal(perKey(allocations, 1, LIMIT, 0, true).fairShare, 300); +}); + +test("a pool where one key carries all the weight still starves the rest", () => { + // totalWeight > 0, so the fallback must not engage: a 0-weight key next to a + // weighted one is a deliberate allocation, not an unconfigured pool. + const allocations = [{ weight: 100 }, { weight: 0 }]; + const LIMIT = 1000; + + assert.equal(perKey(allocations, 0, LIMIT, 0, true).fairShare, 1000); + assert.equal( + perKey(allocations, 1, LIMIT, 0, true).fairShare, + 0, + "an explicitly unweighted key in a weighted pool keeps its zero share" + ); +});