From 227d880b6601058e399204f51bcf46ba212f5a3f Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 30 Jul 2026 15:21:24 -0700 Subject: [PATCH] fix(fairness): commit only to a corpus a reader can actually download MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The published `corpusChecksum` pointed at bytes nobody could obtain, for two independent reasons that happened to land on different deployments. The verifier could not reach the corpus at all. `loopover-verify` fetched `/v1/public/eval-corpus?rule_id=...` while the route has only ever read `?ruleId=`, so every corpus fetch 400'd and the tool reported "no corresponding corpus is downloadable" against a deployment serving a complete, correct, byte-matching corpus. Both sides were individually tested and individually correct: the route's tests used the route's spelling, and the CLI's tests stubbed fetch with a fixture map keyed on pathname, which discarded the query string. The one thing that was broken was the one thing neither suite looked at. The commitment itself was unverifiable wherever reviews actually execute. `precision.latestBacktestRun.corpusChecksum` took precedence over the per-rule downloadable-corpus checksum, and it is wrong three ways: it hashes the RAW corpus (targetKey, full metadata, full-precision timestamps) while readers can only download the redacted one, so it could never match; it is selected with `ORDER BY created_at DESC LIMIT 1` and no rule filter, so one rule's freeze point was stamped onto every record; and that query has no window bound, so the run could predate the window the record declares. This module already flagged the cross-stamping as latent "only because a single rule clears the publication floor" — on a self-host ledger where seven do, it stopped being latent. Changes: - The route accepts `rule_id` as an alias for `ruleId`. Fixing only the client would leave every already-published verifier reporting that same false negative against production forever, so the server meets them; the client now sends the canonical spelling, which works against every deployment. - The downloadable-corpus checksum is the sole commitment source. `latestBacktestRun` remains published in its own right as the rule-precision block's freeze point, it is just no longer misused as a public commitment. - `loadPublicEvalCorpus` reports `readFailed`, so a corpus that is empty because a read failed is distinguishable from one whose rule is genuinely quiet. Fail-safe must not also mean reporting a deployment's failure as a fact about the rule. - The omission rule moves into `isCommittableCorpus`, one pure predicate both paths ask. As an inline condition the read-failure arm was unreachable (a failed read is also an empty one) and so untestable; as a predicate over a plain value it can be driven with a degraded corpus that still carries cases. The new suite owns no fixtures: it points `fetch` at the real Hono app over a real migrated D1 and lets the real `runVerify` drive it, so the only oracle is whether the shipped tool verifies the shipped API. All four guards were mutation-tested — reverting the client spelling, dropping the route alias, ignoring `readFailed`, and restoring the run precedence each fail. Closes #9962 --- apps/loopover-ui/public/openapi.json | 14 +- packages/loopover-mcp/bin/loopover-verify.ts | 8 +- .../loopover-mcp/lib/verify-public-claims.ts | 2 +- src/api/routes.ts | 9 +- src/openapi/spec.ts | 14 +- src/review/eval-score-records.ts | 61 ++++-- src/review/public-eval-corpus.ts | 42 +++- test/unit/eval-score-records.test.ts | 97 ++++++-- ...oopover-verify-against-real-routes.test.ts | 207 ++++++++++++++++++ test/unit/loopover-verify-cli.test.ts | 5 +- test/unit/public-eval-corpus.test.ts | 71 +++++- 11 files changed, 470 insertions(+), 60 deletions(-) create mode 100644 test/unit/loopover-verify-against-real-routes.test.ts 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); + }); +});