diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index a4decf6b09..97d3793c07 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -27587,14 +27587,24 @@ "required": true, "name": "ruleId", "in": "query" + }, + { + "schema": { + "type": "string", + "description": "Deprecated alias for `ruleId`, accepted so verifiers published before #9962 (which asked for this spelling and got a 400) work unchanged. `ruleId` wins if both are given." + }, + "required": false, + "description": "Deprecated alias for `ruleId`, accepted so verifiers published before #9962 (which asked for this spelling and got a 400) work unchanged. `ruleId` wins if both are given.", + "name": "rule_id", + "in": "query" } ], "responses": { "200": { - "description": "{ ruleId, windowDays, caseCount, truncated, checksum, cases: [{ ruleId, outcome, label, firedAt, decidedAt, metadata?: { confidence } }] } — `targetKey` and every other metadata key are DROPPED, not hashed, so no repo or PR identity is published; `metadata.confidence` stays nested where buildConfidenceThresholdClassifier reads it. Timestamps are truncated to the UTC day. `checksum` is sha256 over canonicalJson(cases), i.e. it commits to the artifact you can actually download" + "description": "{ ruleId, windowDays, caseCount, truncated, readFailed, checksum, cases: [{ ruleId, outcome, label, firedAt, decidedAt, metadata?: { confidence } }] } — `targetKey` and every other metadata key are DROPPED, not hashed, so no repo or PR identity is published; `metadata.confidence` stays nested where buildConfidenceThresholdClassifier reads it. Timestamps are truncated to the UTC day. `checksum` is sha256 over canonicalJson(cases), i.e. it commits to the artifact you can actually download. `readFailed` is true when the corpus is empty because the history read failed rather than because the rule has no cases — no commitment is published for such a corpus" }, "400": { - "description": "`ruleId` is required — a corpus is only meaningful for one rule" + "description": "`ruleId` (or its `rule_id` alias) is required — a corpus is only meaningful for one rule" }, "404": { "description": "Public stats are disabled for this deployment" diff --git a/packages/loopover-mcp/bin/loopover-verify.ts b/packages/loopover-mcp/bin/loopover-verify.ts index b2549eb243..7641047155 100644 --- a/packages/loopover-mcp/bin/loopover-verify.ts +++ b/packages/loopover-mcp/bin/loopover-verify.ts @@ -123,7 +123,13 @@ export async function runVerify(args: readonly string[], baseUrlOverride?: strin const ruleIds = [...new Set(records.map((record) => record.workUnit?.ruleId).filter((ruleId): ruleId is string => typeof ruleId === "string"))]; const corpusEntries = await Promise.all( ruleIds.map(async (ruleId) => { - const outcome = await apiGet<{ cases?: unknown; checksum?: unknown }>(baseUrl, `/v1/public/eval-corpus?rule_id=${encodeURIComponent(ruleId)}`); + // `ruleId`, not `rule_id` (#9962). The route has only ever read the camelCase spelling -- the snake_case + // one 400s -- so every corpus fetch this verifier made came back empty and the corpus claim degraded to + // "nothing could be rehashed" against a deployment that was publishing a perfectly good corpus. The + // camelCase spelling is what the OpenAPI spec and the docs have always documented, so it works against + // every deployment, old and new; the alias the route now also accepts is there for verifiers already + // installed in the wild, not for this one. + const outcome = await apiGet<{ cases?: unknown; checksum?: unknown }>(baseUrl, `/v1/public/eval-corpus?ruleId=${encodeURIComponent(ruleId)}`); return [ruleId, outcome.ok ? outcome.value : undefined] as const; }), ); diff --git a/packages/loopover-mcp/lib/verify-public-claims.ts b/packages/loopover-mcp/lib/verify-public-claims.ts index 1e12a7d73c..0cbbab0511 100644 --- a/packages/loopover-mcp/lib/verify-public-claims.ts +++ b/packages/loopover-mcp/lib/verify-public-claims.ts @@ -100,7 +100,7 @@ export async function checkRecordDigests(records: readonly VerifiableEvalRecord[ /** * CLAIM 2: each record's `corpusChecksum` is the checksum of a corpus that can actually be downloaded. * - * `corpusByRuleId` holds what `/v1/public/eval-corpus?rule_id=…` returned for each rule -- absent when that + * `corpusByRuleId` holds what `/v1/public/eval-corpus?ruleId=…` returned for each rule -- absent when that * fetch found nothing. The checksum is recomputed from the CASES as served, which is the whole point: a * commitment is only worth anything if it covers bytes the reader can obtain, so this deliberately does not * trust the corpus payload's own `checksum` field and recomputes from `cases` instead. diff --git a/src/api/routes.ts b/src/api/routes.ts index 91950ffe05..d46a7deb00 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -903,7 +903,14 @@ export function createApp() { app.get("/v1/public/eval-corpus", async (c) => { const publicStatsManifestOverride = await resolvePublicStatsManifestOverride(c.env); if (!isPublicStatsEnabled(c.env, publicStatsManifestOverride)) return c.json({ error: "not_found" }, 404); - const ruleId = c.req.query("ruleId"); + // `ruleId` is canonical (it is what the OpenAPI spec, the verifiability walkthrough and every other query + // parameter on this API use). `rule_id` is accepted as an ALIAS because #9962: every published + // `@loopover/mcp` verifier up to and including 3.x asks for `?rule_id=`, got a 400 back, and reported + // "no corresponding corpus is downloadable" -- a commitment that looked broken while the bytes were sitting + // one spelling away. Fixing only the client would leave every already-installed copy reporting that same + // false negative against production forever, so the SERVER meets them. The alias is read second, so a + // caller passing both gets the canonical spelling rather than a coin flip. + const ruleId = c.req.query("ruleId") ?? c.req.query("rule_id"); // Required, not defaulted: a corpus is only meaningful for one rule, and silently picking one would // publish a checksum for a rule the caller never asked about. if (!ruleId) return c.json({ error: "rule_id_required" }, 400); diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index dc9a99b4cc..1be794cc7e 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -1976,13 +1976,21 @@ export function buildOpenApiSpec() { operationId: "getPublicEvalCorpus", tags: ["Public"], summary: "The redacted, checksummed corpus behind one rule's published precision — downloadable without credentials", - request: { query: z.object({ ruleId: z.string() }) }, + request: { + query: z.object({ + ruleId: z.string(), + rule_id: z + .string() + .optional() + .describe("Deprecated alias for `ruleId`, accepted so verifiers published before #9962 (which asked for this spelling and got a 400) work unchanged. `ruleId` wins if both are given."), + }), + }, responses: { 200: { description: - "{ ruleId, windowDays, caseCount, truncated, checksum, cases: [{ ruleId, outcome, label, firedAt, decidedAt, metadata?: { confidence } }] } — `targetKey` and every other metadata key are DROPPED, not hashed, so no repo or PR identity is published; `metadata.confidence` stays nested where buildConfidenceThresholdClassifier reads it. Timestamps are truncated to the UTC day. `checksum` is sha256 over canonicalJson(cases), i.e. it commits to the artifact you can actually download", + "{ ruleId, windowDays, caseCount, truncated, readFailed, checksum, cases: [{ ruleId, outcome, label, firedAt, decidedAt, metadata?: { confidence } }] } — `targetKey` and every other metadata key are DROPPED, not hashed, so no repo or PR identity is published; `metadata.confidence` stays nested where buildConfidenceThresholdClassifier reads it. Timestamps are truncated to the UTC day. `checksum` is sha256 over canonicalJson(cases), i.e. it commits to the artifact you can actually download. `readFailed` is true when the corpus is empty because the history read failed rather than because the rule has no cases — no commitment is published for such a corpus", }, - 400: { description: "`ruleId` is required — a corpus is only meaningful for one rule" }, + 400: { description: "`ruleId` (or its `rule_id` alias) is required — a corpus is only meaningful for one rule" }, 404: { description: "Public stats are disabled for this deployment" }, }, }); diff --git a/src/review/eval-score-records.ts b/src/review/eval-score-records.ts index f7d2ea5429..fbe124a1af 100644 --- a/src/review/eval-score-records.ts +++ b/src/review/eval-score-records.ts @@ -82,24 +82,41 @@ export function evalScoreCoverage(decided: number, abstained: number): number | } /** - * Build `EvalScoreRecord`s from the already-computed public rule-precision block. Returns an empty array - * when there is no persisted backtest run yet (`latestBacktestRun === null`) -- per #9215's own requirement, - * a record whose commitments cannot be independently re-derived (no corpus checksum to point at) is not - * publishable, so this deliberately emits nothing rather than a record with a placeholder commitment. + * Build `EvalScoreRecord`s from the already-computed public rule-precision block. A rule is published only + * when `corpusChecksumByRuleId` carries a usable commitment for it -- per #9215's own requirement, a record + * whose commitments cannot be independently re-derived is not publishable, so this deliberately emits nothing + * rather than a record with a placeholder commitment. * - * #9805: when no backtest run is persisted, the commitment falls back to `corpusChecksumByRuleId` -- the - * checksum of the corpus `/v1/public/eval-corpus` publishes for that same rule, over the same window. That is - * not a placeholder standing in for a real commitment: it is a hash over an artifact the reader can download - * and re-hash themselves, which is exactly what the `reproducible` trust tier asserts. It exists because a - * deployment with review execution retired (the hosted Worker: see src/index.ts) never persists a backtest - * run at all, so the entire surface was empty while a complete, downloadable corpus sat behind the next - * endpoint over. + * #9805: that commitment is the checksum of the corpus `/v1/public/eval-corpus` publishes for that same rule, + * over the same window. It is not a placeholder standing in for a real commitment: it is a hash over an + * artifact the reader can download and re-hash themselves, which is exactly what the `reproducible` trust tier + * asserts. + * + * #9962: it is also the ONLY commitment source. `precision.latestBacktestRun.corpusChecksum` used to take + * precedence wherever a run was persisted, which was wrong three ways at once and wrong precisely on the + * deployments that execute reviews (the hosted Worker has execution retired, so it never hit this path and the + * defect stayed invisible there): + * + * 1. It is NOT DOWNLOADABLE. That checksum is `checksumCases` over the raw internal `BacktestCase[]` -- + * `targetKey`, the full metadata bag, full-precision timestamps -- whereas the corpus a reader can fetch + * is the REDACTED one. The two hash different bytes by construction, so the published commitment could + * never match the published corpus, no matter how healthy the deployment. Pairing that with + * `trust.tier: "reproducible"` asserts a reproducibility the artifact cannot support. + * 2. It is GLOBAL, not per rule. `loadPublicRulePrecision` selects it with `ORDER BY created_at DESC LIMIT 1` + * and no rule filter, so one rule's freeze point got stamped onto every record. This module already + * warned that stamping one checksum across every record would have every record but one committing to a + * different rule's cases, and called it latent "only because a single rule clears the publication floor"; + * on a self-host ledger where seven rules clear it, it stopped being latent. + * 3. It is UNWINDOWED. That same query has no `created_at >= ?` bound, so the run could predate the + * `windowStart`/`windowEnd` the record goes on to declare. + * + * `latestBacktestRun` remains published in its own right as the rule-precision block's freeze point (see + * public-rule-precision.ts); it is only no longer misused as a public, re-derivable corpus commitment. * * The commitment is resolved PER RULE, not once for the whole batch. Each rule's score is computed over its - * own corpus, so stamping one checksum across every record would have every record but one committing to a - * different rule's cases -- latent today only because a single rule clears the publication floor. + * own corpus, so one checksum across every record would be a different rule's cases on all but one of them. * - * Also returns an empty array when the run's checksum is {@link EMPTY_CORPUS_CHECKSUM}. A hash over zero + * Also omits a rule whose checksum is {@link EMPTY_CORPUS_CHECKSUM}. A hash over zero * cases is the same 32 bytes for every rule, every window, and every deployment, so it points at nothing a * consumer could re-derive the scores from -- it is a placeholder commitment wearing a real hash's clothes, * and pairing it with a `reproducible` trust tier claims a reproducibility the artifact cannot support. The @@ -121,23 +138,19 @@ export function evalScoreCoverage(decided: number, abstained: number): number | export async function buildEvalScoreRecordsFromRulePrecision( precision: PublicRulePrecision, issuedAt: string, - // #9805: per-rule fallback commitments, supplied by the caller so this module stays PURE. Only rules whose - // published corpus is a usable commitment belong in here -- the route drops empty and truncated ones before - // building it, because a truncated corpus's checksum covers a subset of the cases the score covers. + // The per-rule commitments, supplied by the caller so this module stays PURE. Only rules whose published + // corpus is a usable commitment belong in here -- buildPublicCorpusCommitments drops degraded reads, empty + // corpora and truncated ones before building it, because a truncated corpus's checksum covers a subset of + // the cases the score covers. Defaulted to an empty map so a caller with nothing to commit publishes nothing + // (#9962: this is now the sole source, so the default means "no records", never "fall back to the run"). corpusChecksumByRuleId: ReadonlyMap = new Map(), ): Promise { - // A persisted backtest run still wins where one exists, so a deployment that executes reviews keeps exactly - // today's behaviour and this change cannot silently move a self-host commitment. - const runChecksum = - precision.latestBacktestRun && precision.latestBacktestRun.corpusChecksum !== EMPTY_CORPUS_CHECKSUM - ? precision.latestBacktestRun.corpusChecksum - : null; const windowStart = new Date(Date.parse(issuedAt) - precision.windowDays * 24 * 60 * 60 * 1000).toISOString(); // A rule with no usable commitment is OMITTED rather than published with a placeholder -- the #9215 // requirement this module has always enforced, now applied per rule instead of to the whole batch. const publishable = precision.rules.flatMap((row) => { - const corpusChecksum = runChecksum ?? corpusChecksumByRuleId.get(row.ruleId) ?? null; + const corpusChecksum = corpusChecksumByRuleId.get(row.ruleId) ?? null; return corpusChecksum === null || corpusChecksum === EMPTY_CORPUS_CHECKSUM ? [] : [{ row, corpusChecksum }]; }); diff --git a/src/review/public-eval-corpus.ts b/src/review/public-eval-corpus.ts index cc268c4562..96451a2438 100644 --- a/src/review/public-eval-corpus.ts +++ b/src/review/public-eval-corpus.ts @@ -59,6 +59,13 @@ export type PublicEvalCorpus = { windowDays: number; caseCount: number; truncated: boolean; + /** #9962: TRUE when the history read threw and this corpus is empty for that reason rather than because the + * rule genuinely has no cases in the window. The two are otherwise indistinguishable -- same `caseCount: 0`, + * same {@link checksumPublicEvalCorpus}`([])` -- so a transient D1 blip published "this rule has decided + * nothing", which is a false statement about the rule rather than an honest one about the deployment. It is + * a published field, not just an internal one: a reader who downloads a corpus is entitled to know they are + * looking at a degraded read, and {@link buildPublicCorpusCommitments} refuses to commit to one. */ + readFailed: boolean; checksum: string; cases: PublicEvalCorpusCase[]; }; @@ -134,10 +141,14 @@ export async function loadPublicEvalCorpus(env: Env, ruleId: string, nowMs: numb // them must say so -- committing a published score to a prefix, while claiming completeness, is exactly the // unverifiable-artifact problem this endpoint exists to solve. let saturated = false; + // #9962: recorded, not swallowed. Fail-safe still means "never 500 an unauthenticated route", but it must not + // also mean "report the failure as a fact about the rule" -- see PublicEvalCorpus.readFailed. + let readFailed = false; try { ({ fired, overrides, saturated } = await createSignalStore(env).queryRuleHistory(ruleId, sinceMs, MAX_RULE_HISTORY_LIMIT)); } catch { // Fall through to an empty corpus rather than 500ing an unauthenticated route. + readFailed = true; } // Same exclusion the published per-rule precision applies: an override whose verdict was a human @@ -157,20 +168,45 @@ export async function loadPublicEvalCorpus(env: Env, ruleId: string, nowMs: numb // Either bound truncates: the cap on the built cases, or the read that fed it. Reporting only the former // is what made this field always-false. truncated: truncated || saturated, + readFailed, checksum: await checksumPublicEvalCorpus(cases), cases, }; } +/** + * PURE. Is this corpus something a published record may commit to? The ONE place that question is answered, + * so the commitment path and anything else that needs the same judgement cannot drift apart (#9962). + * + * Split out from {@link buildPublicCorpusCommitments} for the same reason {@link applyPublicEvalCorpusCap} is + * split out of the loader: every arm is then exercised directly. In particular `readFailed` and `caseCount` + * always move together through the real store -- a failed read is why a corpus is empty -- so as an inline + * condition the read-failure arm would be unreachable and untested, indistinguishable from a redundant one. As + * a predicate over a plain value it can be driven with a degraded corpus that still carries cases, which is + * what proves the arm is load-bearing rather than decorative. + */ +export function isCommittableCorpus(corpus: PublicEvalCorpus): boolean { + // Ordered most-specific first: a degraded read is a statement about the DEPLOYMENT, and must not be + // reported (or silently absorbed) as the empty-corpus case, which is a statement about the RULE. + if (corpus.readFailed) return false; + if (corpus.caseCount === 0) return false; + return !corpus.truncated; +} + /** * #9805: the publishable commitment for each of `ruleIds` -- the checksum of the corpus this deployment * serves at `/v1/public/eval-corpus?ruleId=...`, for rules whose corpus can actually back a claim. * * A rule is OMITTED (rather than mapped to a checksum a reader would be misled by) when: * + * • the READ FAILED (#9962) -- checked FIRST and on its own, ahead of the empty-corpus arm it used to hide + * behind. Both arms omit, so the observable behaviour is the same today; what changes is that the reason is + * now known at the point the decision is made instead of being inferred from a `caseCount` that a blip and + * a genuinely quiet rule produce identically. That distinction is load-bearing the moment anything wants to + * tell "we decline to commit because this rule has no cases" apart from "we decline to commit because we + * could not read", and collapsing the two is how a degraded read gets published as a fact about the rule; * • the corpus is empty -- `checksumPublicEvalCorpus([])` is the same 32 bytes for every rule, every - * window and every deployment, so it commits to nothing re-derivable. This is also where a failed read - * lands, since loadPublicEvalCorpus degrades to an empty corpus rather than throwing a public route; + * window and every deployment, so it commits to nothing re-derivable; * • the corpus is TRUNCATED at PUBLIC_EVAL_CORPUS_MAX_CASES -- the checksum would then cover a prefix of * the window while the record's `decided`/`confirmed` cover all of it. A reader who re-derived scores * from the published cases would get different numbers and reasonably conclude the published ones were @@ -188,7 +224,7 @@ export async function buildPublicCorpusCommitments( const commitments = new Map(); for (const ruleId of ruleIds) { const corpus = await loadPublicEvalCorpus(env, ruleId, nowMs); - if (corpus.caseCount === 0 || corpus.truncated) continue; + if (!isCommittableCorpus(corpus)) continue; commitments.set(ruleId, corpus.checksum); } return commitments; diff --git a/test/unit/eval-score-records.test.ts b/test/unit/eval-score-records.test.ts index 1430bdaa10..4b80ffa9af 100644 --- a/test/unit/eval-score-records.test.ts +++ b/test/unit/eval-score-records.test.ts @@ -11,6 +11,7 @@ import { type EvalScoreRecord, } from "../../src/review/eval-score-records"; import { contentDigest, sha256Hex } from "../../src/review/decision-record"; +import { buildPublicCorpusCommitments, loadPublicEvalCorpus } from "../../src/review/public-eval-corpus"; import { loadPublicRulePrecision, PUBLIC_PRECISION_MIN_DECIDED, type PublicRulePrecision } from "../../src/review/public-rule-precision"; import { createSignalStore } from "../../src/review/signal-tracking-wire"; import { persistThresholdBacktestRuns, runThresholdBacktestAdvisory } from "../../src/services/threshold-backtest-run"; @@ -28,6 +29,16 @@ const PRECISION_WITH_FREEZE_POINT: PublicRulePrecision = { latestBacktestRun: { corpusChecksum: "abc123def456", at: "2026-07-27T10:00:00.000Z" }, }; +// #9962: the per-rule DOWNLOADABLE-corpus commitments the route supplies. Since the persisted run's checksum +// is no longer a publishable commitment (it hashes the raw corpus, which no reader can fetch), these are what +// a record commits to -- so every builder call below has to pass them to publish anything at all. The +// PRECISION_WITH_FREEZE_POINT fixture keeps its run so these tests still cover the case where one exists. +const CORPUS_COMMITMENTS: ReadonlyMap = new Map([ + ["ai_consensus_defect", "a".repeat(64)], + ["sparse_rule", "b".repeat(64)], + ["never_fired", "c".repeat(64)], +]); + describe("evalScoreCoverage (#9643)", () => { it("is #9215's decided/(decided+abstained), so a validator re-deriving it agrees with the published field", () => { expect(evalScoreCoverage(25, 0)).toBe(1); // no abstention concept: everything seen was decided @@ -41,17 +52,23 @@ describe("evalScoreCoverage (#9643)", () => { }); describe("buildEvalScoreRecordsFromRulePrecision (#9266)", () => { - it("returns an empty array when there is no persisted backtest run to commit to", async () => { + it("returns an empty array when no rule has a downloadable corpus to commit to", async () => { + // #9962: a persisted backtest run no longer rescues these records into the surface with a commitment no + // reader could check, so "nothing to commit to" now means exactly "no per-rule corpus commitment". const records = await buildEvalScoreRecordsFromRulePrecision({ ...PRECISION_WITH_FREEZE_POINT, latestBacktestRun: null }, ISSUED_AT); expect(records).toEqual([]); }); - it("refuses to publish records whose freeze point commits to an empty corpus", async () => { + it("refuses to publish a record that would commit to an empty corpus", async () => { // Regression: production published decided=460/confirmed=287 alongside sha256("[]") -- a hash that is // byte-identical for every rule and every window, so it committed to nothing a consumer could re-derive. const records = await buildEvalScoreRecordsFromRulePrecision( - { ...PRECISION_WITH_FREEZE_POINT, latestBacktestRun: { corpusChecksum: EMPTY_CORPUS_CHECKSUM, at: "2026-07-27T10:00:00.000Z" } }, + PRECISION_WITH_FREEZE_POINT, ISSUED_AT, + new Map([ + ["ai_consensus_defect", EMPTY_CORPUS_CHECKSUM], + ["sparse_rule", EMPTY_CORPUS_CHECKSUM], + ]), ); expect(records).toEqual([]); }); @@ -65,11 +82,14 @@ describe("buildEvalScoreRecordsFromRulePrecision (#9266)", () => { expect(EMPTY_CORPUS_CHECKSUM).toBe(await sha256Hex("[]")); }); - it("builds one record per rule, committed to the freeze point's corpus checksum", async () => { - const records = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT); + it("builds one record per rule, committed to that rule's own downloadable-corpus checksum", async () => { + const records = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT, CORPUS_COMMITMENTS); expect(records).toHaveLength(2); for (const record of records) { - expect(record.commitments.corpusChecksum).toBe("abc123def456"); + const ruleId = record.workUnit.kind === "outcome_confirmed_precision" ? record.workUnit.ruleId : ""; + expect(record.commitments.corpusChecksum).toBe(CORPUS_COMMITMENTS.get(ruleId)); + // #9962: never the persisted run's checksum, which this fixture still carries. + expect(record.commitments.corpusChecksum).not.toBe(PRECISION_WITH_FREEZE_POINT.latestBacktestRun?.corpusChecksum); expect(record.commitments.scoringRuleVersion).toBe(OUTCOME_CONFIRMED_PRECISION_SCORING_RULE_VERSION); expect(record.commitments.windowEnd).toBe(ISSUED_AT); expect(record.commitments.windowStart).toBe(new Date(Date.parse(ISSUED_AT) - 90 * 24 * 60 * 60 * 1000).toISOString()); @@ -83,7 +103,7 @@ describe("buildEvalScoreRecordsFromRulePrecision (#9266)", () => { }); it("carries decided/confirmed/precision through verbatim, with recall null and abstained 0 (not applicable to this work-unit kind)", async () => { - const records = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT); + const records = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT, CORPUS_COMMITMENTS); const withPrecision = records.find((r) => r.workUnit.kind === "outcome_confirmed_precision" && r.workUnit.ruleId === "ai_consensus_defect"); // coverage is 1, not null: abstained is structurally 0 here, so 25/(25+0) is fully determined (#9643). expect(withPrecision?.score).toEqual({ decided: 25, confirmed: 20, precision: 0.8, recall: null, coverage: 1, abstained: 0 }); @@ -101,6 +121,7 @@ describe("buildEvalScoreRecordsFromRulePrecision (#9266)", () => { const records = await buildEvalScoreRecordsFromRulePrecision( { ...PRECISION_WITH_FREEZE_POINT, rules: [{ ruleId: "never_fired", decided: 0, confirmed: 0, precision: null, unrecognized: 0 }] }, ISSUED_AT, + CORPUS_COMMITMENTS, ); expect(records).toHaveLength(1); expect(records[0]?.score).toEqual({ decided: 0, confirmed: 0, precision: null, recall: null, coverage: null, abstained: 0 }); @@ -110,7 +131,7 @@ describe("buildEvalScoreRecordsFromRulePrecision (#9266)", () => { // recordDigest is computed over the whole record, so changing coverage changes the digest. Records are // built on read (routes.ts), never persisted, so nothing needs migrating -- but the round-trip a consumer // runs to verify a fetched record must still hold. - const records = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT); + const records = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT, CORPUS_COMMITMENTS); expect(records).toHaveLength(2); for (const record of records) { expect(record.score.coverage).toBe(1); @@ -119,7 +140,7 @@ describe("buildEvalScoreRecordsFromRulePrecision (#9266)", () => { }); it("each record's recordDigest is the sha256 of its own canonical content (independently recomputable)", async () => { - const records = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT); + const records = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT, CORPUS_COMMITMENTS); for (const record of records) { const { recordDigest, ...rest } = record; expect(await contentDigest(rest)).toBe(recordDigest); @@ -127,7 +148,7 @@ describe("buildEvalScoreRecordsFromRulePrecision (#9266)", () => { }); it("emits records sorted the same way the input rules array is ordered (no re-sort, no reordering surprise)", async () => { - const records = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT); + const records = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT, CORPUS_COMMITMENTS); expect(records.map((r) => (r.workUnit.kind === "outcome_confirmed_precision" ? r.workUnit.ruleId : ""))).toEqual([ "ai_consensus_defect", "sparse_rule", @@ -186,12 +207,12 @@ describe("filterEvalScoreRecords", () => { describe("verifyEvalScoreRecordDigest", () => { it("returns true for a record whose digest matches its own content", async () => { - const [record] = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT); + const [record] = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT, CORPUS_COMMITMENTS); expect(await verifyEvalScoreRecordDigest(record as EvalScoreRecord)).toBe(true); }); it("returns false for a record whose content was tampered with after the digest was computed", async () => { - const [record] = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT); + const [record] = await buildEvalScoreRecordsFromRulePrecision(PRECISION_WITH_FREEZE_POINT, ISSUED_AT, CORPUS_COMMITMENTS); const tampered: EvalScoreRecord = { ...(record as EvalScoreRecord), issuedAt: "2099-01-01T00:00:00.000Z" }; expect(await verifyEvalScoreRecordDigest(tampered)).toBe(false); }); @@ -254,24 +275,45 @@ describe("per-rule corpus commitments when no backtest run is persisted (#9805)" expect(records).toEqual([]); }); - it("INVARIANT: a persisted backtest run still WINS, so self-host behaviour is unchanged", async () => { + // #9962: the persisted run used to WIN here, which is what made every self-host record commit to bytes no + // reader could obtain. Its checksum is `checksumCases` over the RAW corpus (targetKey, full metadata bag, + // full-precision timestamps); the downloadable corpus is the redacted one, so the two hash different inputs + // by construction and the published commitment could never match the published corpus. + it("REGRESSION: commits to the DOWNLOADABLE corpus, not the persisted run's non-downloadable checksum", async () => { const records = await buildEvalScoreRecordsFromRulePrecision( precisionOf([rule("ai_consensus_defect")], { corpusChecksum: "d".repeat(64), at: ISSUED_AT }), ISSUED_AT, new Map([["ai_consensus_defect", "e".repeat(64)]]), ); - expect(records[0]!.commitments.corpusChecksum).toBe("d".repeat(64)); + expect(records[0]!.commitments.corpusChecksum).toBe("e".repeat(64)); + expect(records[0]!.commitments.corpusChecksum).not.toBe("d".repeat(64)); + }); + + it("REGRESSION: a persisted run does not stamp ONE checksum across every rule's record", async () => { + // The shape the Orb was actually publishing: several rules clear the floor, `latestBacktestRun` is a + // single global row (`ORDER BY created_at DESC LIMIT 1`, no rule filter), so every record carried the same + // commitment and all but one of them pointed at a different rule's cases. + const records = await buildEvalScoreRecordsFromRulePrecision( + precisionOf([rule("rule_a"), rule("rule_b")], { corpusChecksum: "d".repeat(64), at: ISSUED_AT }), + ISSUED_AT, + new Map([ + ["rule_a", "a".repeat(64)], + ["rule_b", "b".repeat(64)], + ]), + ); + const checksums = records.map((r) => r.commitments.corpusChecksum); + expect(new Set(checksums).size).toBe(2); + expect(checksums).not.toContain("d".repeat(64)); }); - it("falls back when the persisted run's checksum is the empty-corpus one, rather than publishing nothing", async () => { - // The run exists but commits to nothing; the rule's real corpus does. Preferring the run here would keep - // the surface empty for no benefit. + it("publishes nothing for a rule with no downloadable corpus, even when a run is persisted", async () => { + // Previously the run's checksum rescued this rule into the surface with an unverifiable commitment. The + // honest outcome is an omitted record, not a record a reader cannot check. const records = await buildEvalScoreRecordsFromRulePrecision( - precisionOf([rule("ai_consensus_defect")], { corpusChecksum: EMPTY_CORPUS_CHECKSUM, at: ISSUED_AT }), + precisionOf([rule("ai_consensus_defect")], { corpusChecksum: "d".repeat(64), at: ISSUED_AT }), ISSUED_AT, - new Map([["ai_consensus_defect", "f".repeat(64)]]), ); - expect(records[0]!.commitments.corpusChecksum).toBe("f".repeat(64)); + expect(records).toEqual([]); }); it("still returns [] with neither a run nor any commitment -- the #9215 rule is unchanged", async () => { @@ -319,12 +361,20 @@ describe("in-Worker backtest -> /v1/public/eval-scores records (#9639)", () => { const now = Date.now(); const { run, precision } = await seedRunAndLoad(env, now); expect(precision.rules.length).toBeGreaterThan(0); + // The run is what #9639 was about, so it still has to be persisted and readable for this to be the same + // regression -- but #9962 moved WHICH checksum gets published, so the commitments now come from the + // downloadable corpus and the run's checksum is asserted against below rather than for. + expect(precision.latestBacktestRun).not.toBeNull(); + const commitments = await buildPublicCorpusCommitments(env, precision.rules.map((r) => r.ruleId), now); - const records = await buildEvalScoreRecordsFromRulePrecision(precision, ISSUED_AT); + const records = await buildEvalScoreRecordsFromRulePrecision(precision, ISSUED_AT, commitments); expect(records).toHaveLength(precision.rules.length); for (const record of records) { - expect(record.commitments.corpusChecksum).toBe(run.corpusChecksumByRuleId.get("linked_issue_scope_mismatch")); + const ruleId = (record.workUnit as { ruleId: string }).ruleId; + expect(record.commitments.corpusChecksum).toBe((await loadPublicEvalCorpus(env, ruleId, now)).checksum); + // #9962: NOT the run's raw-corpus checksum -- that one hashes bytes no reader can download. + expect(record.commitments.corpusChecksum).not.toBe(run.corpusChecksumByRuleId.get(ruleId)); expect(record.commitments.corpusChecksum).not.toBe(EMPTY_CORPUS_CHECKSUM); expect(record.trust.tier).toBe("reproducible"); // Each record still commits to its own content, so the freeze point cannot be swapped undetected. @@ -341,6 +391,7 @@ describe("in-Worker backtest -> /v1/public/eval-scores records (#9639)", () => { await persistThresholdBacktestRuns(env, "acme/widgets", 7, run.changed, run.comparisons, run.corpusChecksumByRuleId); const precision = await loadPublicRulePrecision(env, now); - expect(await buildEvalScoreRecordsFromRulePrecision(precision, ISSUED_AT)).toEqual([]); + const commitments = await buildPublicCorpusCommitments(env, precision.rules.map((r) => r.ruleId), now); + expect(await buildEvalScoreRecordsFromRulePrecision(precision, ISSUED_AT, commitments)).toEqual([]); }); }); diff --git a/test/unit/loopover-verify-against-real-routes.test.ts b/test/unit/loopover-verify-against-real-routes.test.ts new file mode 100644 index 0000000000..831b66f0ae --- /dev/null +++ b/test/unit/loopover-verify-against-real-routes.test.ts @@ -0,0 +1,207 @@ +import { describe, expect, it, vi, afterEach } from "vitest"; + +import { createApp } from "../../src/api/routes"; +import { createSignalStore } from "../../src/review/signal-tracking-wire"; +import { PUBLIC_PRECISION_MIN_DECIDED } from "../../src/review/public-rule-precision"; +import { createTestEnv } from "../helpers/d1"; + +// #9962: the verifier and the API, wired to each other, with NOTHING in between. +// +// The defect this file exists to make impossible: `loopover-verify` asked for +// `/v1/public/eval-corpus?rule_id=…` while the route has only ever read `?ruleId=`. Every corpus fetch 400'd, +// every commitment went unrehashed, and the tool reported "1 commitment(s) published, but no corresponding +// corpus is downloadable" against a deployment that was serving a complete, correct, matching corpus the whole +// time. A published verifier that cannot verify a healthy deployment is worse than no verifier: it manufactures +// evidence against us, and it did so for as long as it did because BOTH SIDES WERE INDIVIDUALLY TESTED AND +// INDIVIDUALLY CORRECT. The route's tests called the route with the spelling the route wanted; the CLI's tests +// stubbed `fetch` with a fixture map keyed on pathname, which threw the query string away -- so the one thing +// that was broken was the one thing neither suite looked at. +// +// So this suite deliberately owns no fixtures. `fetch` is pointed straight at the real Hono app over a real +// migrated D1, and the REAL `runVerify` drives it: its own URL construction, its own query parameters, its own +// recomputation of every commitment. Any future disagreement about a parameter name, a response field, or a +// status code fails here, because the only oracle is "did the shipped tool actually verify the shipped API". +const RULE_ID = "ai_consensus_defect"; +const BASE_URL = "http://routes.test"; + +async function loadCli() { + return import("../../packages/loopover-mcp/bin/loopover-verify") as Promise<{ + runVerify: (args: readonly string[], baseUrlOverride?: string) => Promise; + }>; +} + +/** Point global `fetch` at the real app. The URL is decomposed exactly the way an HTTP server would see it -- + * path AND query preserved -- because the query string is precisely what the old fixture stub discarded. */ +function routeFetchTo(env: Env): void { + const app = createApp(); + vi.stubGlobal("fetch", async (input: string | URL) => { + const url = new URL(String(input)); + return app.request(`${url.pathname}${url.search}`, {}, env); + }); +} + +/** Enough decided cases for the rule to clear PUBLIC_PRECISION_MIN_DECIDED, so it reaches the precision block + * and therefore gets a published record with a commitment to check. */ +async function seedDecidedRule(env: Env, ruleId: string): Promise { + const store = createSignalStore(env); + const now = Date.now(); + for (let i = 0; i < PUBLIC_PRECISION_MIN_DECIDED + 2; i += 1) { + await store.recordRuleFired({ + ruleId, + targetKey: `acme/widgets#${i + 1}`, + outcome: "close", + occurredAt: new Date(now - (i + 2) * 1000).toISOString(), + metadata: { confidence: 0.3 + (i % 4) * 0.15 }, + }); + await store.recordHumanOverride({ + ruleId, + targetKey: `acme/widgets#${i + 1}`, + verdict: i % 3 === 0 ? "reversed" : "confirmed", + occurredAt: new Date(now - (i + 1) * 1000).toISOString(), + }); + } +} + +/** Run the real CLI against the real app and return its parsed `--json` report. */ +async function verifyAgainst(env: Env): Promise<{ code: number; results: { id: string; status: string; detail: string }[] }> { + routeFetchTo(env); + const { runVerify } = await loadCli(); + const chunks: string[] = []; + const spy = vi.spyOn(process.stdout, "write").mockImplementation((chunk: unknown) => { + chunks.push(String(chunk)); + return true; + }); + try { + const code = await runVerify(["--json", "--base-url", BASE_URL]); + return { code, results: JSON.parse(chunks.join("")).results }; + } finally { + spy.mockRestore(); + } +} + +const claim = (results: { id: string; status: string; detail: string }[], id: string) => results.find((result) => result.id === id)!; + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe("loopover-verify against the real public routes (#9962)", () => { + it("REGRESSION: rehashes the published corpus and PASSES the commitment claim", async () => { + const env = createTestEnv({ LOOPOVER_PUBLIC_STATS: "true" }); + await seedDecidedRule(env, RULE_ID); + + const { results } = await verifyAgainst(env); + const corpus = claim(results, "corpus-commitments"); + + // Before the fix this was `skip` with "no corresponding corpus is downloadable" -- the corpus fetch 400'd + // on the parameter name, so nothing was ever rehashed. A skip is the failure mode here, not a pass. + expect(corpus.status).toBe("pass"); + expect(corpus.detail).toMatch(/case\(s\) rehashed exactly/); + expect(corpus.detail).not.toMatch(/unverified/); + }); + + it("INVARIANT: the record digests claim passes over the same live surface", async () => { + // Guards the seeding above rather than the fix: if no record were published at all, the corpus claim would + // skip for a completely different reason and the assertion above would be vacuous. + const env = createTestEnv({ LOOPOVER_PUBLIC_STATS: "true" }); + await seedDecidedRule(env, RULE_ID); + + const { results } = await verifyAgainst(env); + expect(claim(results, "record-digests").status).toBe("pass"); + }); + + it("MUTATION GUARD: the corpus claim degrades to a skip when the corpus genuinely cannot be fetched", async () => { + // Proves the pass above is not vacuous -- that the claim really is driven by a successful corpus fetch and + // would notice its absence. The query is stripped entirely rather than misspelled, so this stays honest + // regardless of which spellings the route accepts. + const env = createTestEnv({ LOOPOVER_PUBLIC_STATS: "true" }); + await seedDecidedRule(env, RULE_ID); + const app = createApp(); + vi.stubGlobal("fetch", async (input: string | URL) => { + const url = new URL(String(input)); + const search = url.pathname === "/v1/public/eval-corpus" ? "" : url.search; + return app.request(`${url.pathname}${search}`, {}, env); + }); + const { runVerify } = await loadCli(); + const chunks: string[] = []; + const spy = vi.spyOn(process.stdout, "write").mockImplementation((chunk: unknown) => { + chunks.push(String(chunk)); + return true; + }); + await runVerify(["--json", "--base-url", BASE_URL]); + spy.mockRestore(); + + const corpus = claim(JSON.parse(chunks.join("")).results, "corpus-commitments"); + expect(corpus.status).toBe("skip"); + expect(corpus.detail).toMatch(/no corresponding corpus is downloadable/); + }); + + it("asks for the CANONICAL ruleId spelling, not the compatibility alias", async () => { + // The route accepts `rule_id` too, so the end-to-end claim above would pass either way -- which is the + // point of the alias, but it also means nothing else pins what OUR client sends. This does. The alias + // exists for verifiers already installed in the wild; new code must not drift onto a deprecated spelling + // and quietly make the alias load-bearing. + const env = createTestEnv({ LOOPOVER_PUBLIC_STATS: "true" }); + await seedDecidedRule(env, RULE_ID); + const app = createApp(); + const corpusQueries: URLSearchParams[] = []; + vi.stubGlobal("fetch", async (input: string | URL) => { + const url = new URL(String(input)); + if (url.pathname === "/v1/public/eval-corpus") corpusQueries.push(url.searchParams); + return app.request(`${url.pathname}${url.search}`, {}, env); + }); + const { runVerify } = await loadCli(); + const spy = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + await runVerify(["--json", "--base-url", BASE_URL]); + spy.mockRestore(); + + expect(corpusQueries.length).toBeGreaterThan(0); + for (const query of corpusQueries) { + expect(query.get("ruleId")).toBe(RULE_ID); + expect(query.has("rule_id")).toBe(false); + } + }); +}); + +describe("/v1/public/eval-corpus parameter spellings (#9962)", () => { + const get = (env: Env, query: string) => createApp().request(`/v1/public/eval-corpus${query}`, {}, env); + + it("serves the same corpus for the canonical ruleId and the rule_id alias", async () => { + const env = createTestEnv({ LOOPOVER_PUBLIC_STATS: "true" }); + await seedDecidedRule(env, RULE_ID); + + const canonical = await get(env, `?ruleId=${RULE_ID}`); + const alias = await get(env, `?rule_id=${RULE_ID}`); + + expect(canonical.status).toBe(200); + // The alias exists so that verifiers ALREADY PUBLISHED -- which ask for this spelling and got a 400 -- + // start working against production without their users upgrading anything. + expect(alias.status).toBe(200); + expect(await alias.json()).toEqual(await canonical.json()); + }); + + it("prefers the canonical spelling when a caller passes both", async () => { + const env = createTestEnv({ LOOPOVER_PUBLIC_STATS: "true" }); + await seedDecidedRule(env, RULE_ID); + + const response = await get(env, `?ruleId=${RULE_ID}&rule_id=some_other_rule`); + expect(response.status).toBe(200); + expect(((await response.json()) as { ruleId: string }).ruleId).toBe(RULE_ID); + }); + + it("still requires one of them", async () => { + const env = createTestEnv({ LOOPOVER_PUBLIC_STATS: "true" }); + const response = await get(env, ""); + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ error: "rule_id_required" }); + }); + + it("publishes readFailed so a reader can tell a degraded read from a quiet rule", async () => { + const env = createTestEnv({ LOOPOVER_PUBLIC_STATS: "true" }); + await seedDecidedRule(env, RULE_ID); + const healthy = (await (await get(env, `?ruleId=${RULE_ID}`)).json()) as { readFailed: boolean; caseCount: number }; + expect(healthy).toMatchObject({ readFailed: false }); + expect(healthy.caseCount).toBeGreaterThan(0); + }); +}); diff --git a/test/unit/loopover-verify-cli.test.ts b/test/unit/loopover-verify-cli.test.ts index 26dcf5218d..f777fc1147 100644 --- a/test/unit/loopover-verify-cli.test.ts +++ b/test/unit/loopover-verify-cli.test.ts @@ -161,7 +161,10 @@ describe("runVerify", () => { const requested: string[] = []; vi.stubGlobal("fetch", async (input: string | URL) => { const url = new URL(String(input)); - if (url.pathname === "/v1/public/eval-corpus") requested.push(url.searchParams.get("rule_id") ?? ""); + // #9962: `ruleId`, the spelling the route actually reads. This assertion previously read `rule_id` -- + // the same wrong spelling the CLI was sending -- so the two agreed with each other and disagreed with + // production, and the test passed while every real corpus fetch 400'd. + if (url.pathname === "/v1/public/eval-corpus") requested.push(url.searchParams.get("ruleId") ?? ""); if (url.pathname === "/v1/public/eval-scores") { return new Response(JSON.stringify({ records: [await record("rule_a"), await record("rule_a"), await record("rule_b")] }), { status: 200 }); } diff --git a/test/unit/public-eval-corpus.test.ts b/test/unit/public-eval-corpus.test.ts index 50b64014db..94f48c2f57 100644 --- a/test/unit/public-eval-corpus.test.ts +++ b/test/unit/public-eval-corpus.test.ts @@ -3,6 +3,7 @@ import { applyPublicEvalCorpusCap, buildPublicCorpusCommitments, checksumPublicEvalCorpus, + isCommittableCorpus, loadPublicEvalCorpus, PUBLIC_EVAL_CORPUS_MAX_CASES, redactBacktestCase, @@ -192,7 +193,7 @@ describe("loadPublicEvalCorpus — end to end over the real signal tables", () = it("returns an empty, still-checksummed corpus for a rule with no history", async () => { const corpus = await loadPublicEvalCorpus(createTestEnv(), "never_fired", NOW); - expect(corpus).toMatchObject({ caseCount: 0, truncated: false, cases: [] }); + expect(corpus).toMatchObject({ caseCount: 0, truncated: false, readFailed: false, cases: [] }); expect(corpus.checksum).toBe(await sha256Hex("[]")); }); @@ -204,6 +205,30 @@ describe("loadPublicEvalCorpus — end to end over the real signal tables", () = expect(corpus.checksum).toBe(await sha256Hex("[]")); }); + // #9962: fail-safe must not also mean "indistinguishable from a fact about the rule". Both corpora below are + // byte-identical apart from this one flag, which is exactly why the flag has to exist -- without it a + // transient D1 blip publishes "this rule has decided nothing" and a reader cannot tell. + it("REGRESSION: says READ FAILED when it degraded, and does not when the rule is genuinely quiet", async () => { + const broken = createTestEnv(); + broken.DB = { prepare: () => { throw new Error("boom"); } } as never; + const degraded = await loadPublicEvalCorpus(broken, "ai_consensus_defect", NOW); + const quiet = await loadPublicEvalCorpus(createTestEnv(), "ai_consensus_defect", NOW); + + expect(degraded.readFailed).toBe(true); + expect(quiet.readFailed).toBe(false); + // Everything a reader could otherwise go on is identical between the two, so `readFailed` is the ONLY + // thing carrying the distinction -- pin that, or the flag could be quietly derived from caseCount later. + expect(degraded.caseCount).toBe(quiet.caseCount); + expect(degraded.checksum).toBe(quiet.checksum); + expect(degraded.cases).toEqual(quiet.cases); + }); + + it("does not claim a read failure on a healthy read that returned cases", async () => { + const env = createTestEnv(); + await seedPair(env, { ruleId: "ai_consensus_defect", targetKey: "acme/widgets#1", verdict: "reversed", confidence: 0.8 }); + expect((await loadPublicEvalCorpus(env, "ai_consensus_defect", NOW)).readFailed).toBe(false); + }); + it("reports truncation honestly rather than silently trimming", async () => { expect((await loadPublicEvalCorpus(createTestEnv(), "r", NOW)).truncated).toBe(false); }); @@ -272,8 +297,52 @@ describe("buildPublicCorpusCommitments (#9805)", () => { expect((await buildPublicCorpusCommitments(broken, ["rule_a"], NOW)).size).toBe(0); }); + it("asks isCommittableCorpus rather than restating the rule, so the two cannot drift", async () => { + const env = createTestEnv(); + await seedPair(env, { ruleId: "rule_a", targetKey: "acme/widgets#1", verdict: "reversed", confidence: 0.8 }); + const committable = await loadPublicEvalCorpus(env, "rule_a", NOW); + const notCommittable = await loadPublicEvalCorpus(env, "never_fired", NOW); + + expect(isCommittableCorpus(committable)).toBe(true); + expect(isCommittableCorpus(notCommittable)).toBe(false); + const commitments = await buildPublicCorpusCommitments(env, ["rule_a", "never_fired"], NOW); + expect(commitments.has("rule_a")).toBe(isCommittableCorpus(committable)); + expect(commitments.has("never_fired")).toBe(isCommittableCorpus(notCommittable)); + }); + it("returns an empty map for no rules, without touching the database", async () => { const env = createTestEnv(); expect((await buildPublicCorpusCommitments(env, [], NOW)).size).toBe(0); }); }); + +// #9962: the predicate the commitment path delegates to. Driven with plain values rather than through the +// store precisely so each arm is reachable on its own -- the read-failure arm in particular cannot be reached +// via the store (a failed read is also an empty one), and an arm that only ever fires alongside another arm is +// an arm no test can prove is doing anything. +describe("isCommittableCorpus (#9962)", () => { + const base = { + ruleId: "rule_a", + windowDays: PUBLIC_PRECISION_WINDOW_DAYS, + caseCount: 3, + truncated: false, + readFailed: false, + checksum: "a".repeat(64), + cases: [] as PublicEvalCorpusCase[], + }; + + it("commits to a healthy, non-empty, complete corpus", () => { + expect(isCommittableCorpus(base)).toBe(true); + }); + + it("REGRESSION: refuses a DEGRADED corpus even when it carries cases -- the arm the store cannot reach", () => { + // The whole point of the flag. With `readFailed` absorbed into the empty check, this corpus would be + // committed to: it has cases, it is not truncated, and nothing else says the read went wrong. + expect(isCommittableCorpus({ ...base, readFailed: true })).toBe(false); + }); + + it("refuses an empty corpus (its checksum is the same 32 bytes everywhere) and a truncated one", () => { + expect(isCommittableCorpus({ ...base, caseCount: 0 })).toBe(false); + expect(isCommittableCorpus({ ...base, truncated: true })).toBe(false); + }); +});