From a97992bd5e2c2824f15ed6f758c41512513fa48d Mon Sep 17 00:00:00 2001 From: Florent Tapponnier Date: Mon, 22 Jun 2026 15:09:19 +0200 Subject: [PATCH] fix(citable): quorum guard against mostly-draft aggregator cache - Aggregator throws AllBenchmarksDraftError when <50% of live specs produce a live bench (was: only when 0/N live). Prevents a mixed set like 2/26 live being cached for 60s while /api/stat returns live for the same slugs. - Bumped all-benchmarks cache key v13 to v14 to flush the current poisoned snapshot on prod. - Warm /api/citable, /api/llm-context, /api/mcp explicitly on prod deploy so the aggregator cache starts hot. - Added tests for the new quorum threshold. --- .github/workflows/prod-deploy.yml | 14 +++++++++++++ src/lib/spec.test.ts | 35 ++++++++++++++++++++++++++++++- src/lib/spec.ts | 33 +++++++++++++++++++++-------- 3 files changed, 72 insertions(+), 10 deletions(-) diff --git a/.github/workflows/prod-deploy.yml b/.github/workflows/prod-deploy.yml index 2d443b7d..44c7930d 100644 --- a/.github/workflows/prod-deploy.yml +++ b/.github/workflows/prod-deploy.yml @@ -68,6 +68,20 @@ jobs: | xargs -P 4 -I{} curl -s -o /dev/null -w "%{http_code} %{time_total}s {}\n" --max-time 120 {} \ || echo "warm-up failed (non-blocking)" + # Warm /api/citable explicitly: the aggregator cache is independent + # from per-bench caches, and the bench-page warm-up only populates + # the latter. Without this step, /api/citable's first hit after + # deploy runs 26 parallel Prom queries on a cold function instance, + # which has historically produced mostly-placeholder snapshots that + # cached for 60s and shipped status=insufficient to LLM agents. + - name: Warm citable aggregate + run: | + base="https://openchainbench.com" + for ep in /api/citable /api/llm-context /api/mcp; do + code=$(curl -s -o /dev/null -w "%{http_code}" --max-time 120 "$base$ep" || echo "000") + echo "$code $base$ep" + done + - name: Summary run: | { diff --git a/src/lib/spec.test.ts b/src/lib/spec.test.ts index de704f6b..6a79ba40 100644 --- a/src/lib/spec.test.ts +++ b/src/lib/spec.test.ts @@ -125,7 +125,10 @@ describe("aggregateBenchmarks (all-draft poisoning regression)", () => { ); }); - test("returns the list when at least one bench is live", async () => { + test("returns the list when at least one bench is live (below quorum floor)", async () => { + // Three specs only: under the QUORUM_MIN_LIVE_SPECS floor of 4, so + // the quorum guard is intentionally lenient. The original all-draft + // throw still applies above 0 live, so a single live bench passes. const specs = [fakeSpec("a"), fakeSpec("b"), fakeSpec("c")]; const loader = async (slug: string) => { if (slug === "b") return fakeLiveBench(slug); @@ -138,6 +141,36 @@ describe("aggregateBenchmarks (all-draft poisoning regression)", () => { expect(live[0].slug).toBe("b"); }); + test("throws when fewer than half of live specs produce live benches (mostly-draft poisoning)", async () => { + // Regression for /api/citable serving status=insufficient for 24/26 + // benches even though /api/stat returned live for the same slugs. + // The aggregator used to cache the mixed set for 60s and surface + // it to LLM agents and journalists. + const liveSpecs = Array.from({ length: 10 }, (_, i) => fakeSpec(`s${i}`)); + const loader = async (slug: string) => { + // 2 of 10 live, 8 rejected — mirrors the production failure mode. + if (slug === "s0" || slug === "s1") return fakeLiveBench(slug); + throw new Error("prom timeout"); + }; + await expect(aggregateBenchmarks(liveSpecs, loader)).rejects.toBeInstanceOf( + AllBenchmarksDraftError, + ); + }); + + test("passes when at least half of live specs produce live benches", async () => { + const liveSpecs = Array.from({ length: 10 }, (_, i) => fakeSpec(`s${i}`)); + const loader = async (slug: string) => { + // 5 of 10 live — meets the half threshold exactly. + if (["s0", "s1", "s2", "s3", "s4"].includes(slug)) { + return fakeLiveBench(slug); + } + throw new Error("prom timeout"); + }; + const out = await aggregateBenchmarks(liveSpecs, loader); + expect(out).toHaveLength(10); + expect(out.filter((b) => b.status === "live")).toHaveLength(5); + }); + test("never silently returns an all-draft stable list", async () => { // The bug being regressed: pre-fix, aggregateBenchmarks returned an // all-draft list when Prom was blacked out at cold start, and diff --git a/src/lib/spec.ts b/src/lib/spec.ts index dc050e28..b7e9ce97 100644 --- a/src/lib/spec.ts +++ b/src/lib/spec.ts @@ -256,15 +256,25 @@ export async function aggregateBenchmarks( } } const live = benchmarks.filter((b) => b.status === "live"); - if (benchmarks.length > 0 && live.length === 0) { - // Throw so unstable_cache keeps the previous good value during a - // Prom blackout. Previously we returned the all-draft set with a - // warning, which got cached for 60s and made /api/citable, sitemap, - // and hub pages all report every bench as draft while the per-slug - // /api/stat/ path returned live data. Build-time callers wrap - // in try/catch (see loadAllBenchmarksSafe). + const liveSpecs = specs.filter((s) => s.status === "live"); + // Quorum guard. The previous "all-draft only" threshold let through + // mixed sets where 24 of 26 specs declared live had collapsed to + // placeholder while 2 trickled in live — that mostly-bad set then + // cached for 60s and made /api/citable report status=insufficient + // for benches whose per-slug /api/stat path returned live data. When + // most editorially live specs fail to produce a live bench in the + // current run, treat the cycle as a load failure and throw, so + // unstable_cache keeps the previous good value. + // + // Floor of 4 avoids tripping the guard on tiny dev fixtures with + // a single deliberately-failing bench in the test suite. + const QUORUM_MIN_LIVE_SPECS = 4; + const quorumLost = + liveSpecs.length >= QUORUM_MIN_LIVE_SPECS && + live.length * 2 < liveSpecs.length; + if (benchmarks.length > 0 && (live.length === 0 || quorumLost)) { console.warn( - `[DRAFT-TRACE] all_draft slug_count=${benchmarks.length} throwing to preserve previous cache value`, + `[DRAFT-TRACE] aggregate_quorum_lost live=${live.length}/${liveSpecs.length} (total=${benchmarks.length}) throwing to preserve previous cache value`, ); throw new AllBenchmarksDraftError(benchmarks.length); } @@ -296,7 +306,12 @@ const loadAllBenchmarksCached = unstable_cache( // all-draft so unstable_cache no longer caches the bad set, but any // already-stored v12 snapshot in Upstash KV would still serve for up // to 60s after deploy. Bumping the key sidesteps that window. - ["all-benchmarks-v13"], + // v14: bumped to flush a mostly-draft snapshot (24/26 placeholders) + // that survived the v13 fix because the previous throw only fired + // on a literally all-draft set. The aggregateBenchmarks quorum guard + // now throws on any cycle where fewer than half of editorially live + // specs produce a live bench. + ["all-benchmarks-v14"], { revalidate: 60, tags: ["benchmarks"] }, ); export const loadAllBenchmarks = cache(loadAllBenchmarksCached);