From 0be52083be76376d131af9ef2a5e1991bab43261 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 03:39:28 -0700 Subject: [PATCH] fix(eval): commit each published score to the corpus a reader can download Verified live on api.loopover.ai: /v1/public/stats publishes ai_consensus_defect at decided 460 / confirmed 287, /v1/public/eval-corpus serves 460 real cases whose checksum re-derives from the downloaded bytes, and /v1/public/eval-scores serves `{"records":[]}`. The commitment came only from a persisted calibration.*_backtest_run audit event. #9639 fixes that writer, but it runs inside a review pass, and hosted review execution is retired (src/index.ts acks-and-drops review-execution jobs off the queue), so on loopover.ai the event never exists. The walkthrough tells an anonymous reader to fetch .records and re-derive recordDigest; they got an empty array, explained by a doc line attributing it to an empty corpus that demonstrably is not empty. Each record now falls back to its OWN rule's published corpus checksum -- the exact bytes /v1/public/eval-corpus serves, over the same window. That is not the placeholder commitment #9215 forbids: it is a hash over an artifact the reader downloads and re-hashes, which is what the `reproducible` tier asserts. A persisted run still wins where one exists, so self-host is unchanged. Resolving per rule also fixes a latent bug: one run's checksum was stamped onto every record, so with more than one published rule every record but one would have committed to a different rule's cases. TRUNCATION HAD TO BE MADE DETECTABLE FIRST. PUBLIC_EVAL_CORPUS_MAX_CASES was 5_000 while the corpus is built from a rule-history read that listAuditEventsByType hard-clamps to 2_000 -- and queryRuleHistory passed no limit at all, so it took the default of 500. The cap could never bind, and `truncated` was structurally always false, while /v1/public/stats counts `decided` with an unbounded SQL COUNT(*). The two surfaces agreed only while the window stayed under 500 cases; at 460 they were 40 from silently diverging, with the corpus serving a prefix and reporting completeness. queryRuleHistory now takes an explicit bound and reports `saturated`, the corpus ORs that into `truncated`, and a truncated corpus is never published as a commitment -- its checksum would cover a prefix of the window the score covers. Closes #9805 --- .../content/docs/verify-this-review.mdx | 27 ++++-- .../src/calibration/signal-tracking.ts | 16 +++- .../lib/signal-tracking-store.ts | 9 +- src/api/routes.ts | 9 +- src/review/eval-score-records.ts | 39 +++++++-- src/review/public-eval-corpus.ts | 52 +++++++++++- src/review/signal-tracking-wire.ts | 28 ++++++- .../public-eval-scores-route.test.ts | 70 ++++++++++++++++ .../configured-gate-blocker-signals.test.ts | 4 +- test/unit/eval-score-records.test.ts | 83 +++++++++++++++++++ .../linked-issue-satisfaction-run.test.ts | 2 +- test/unit/miner-attempt-cli.test.ts | 2 +- ...-discover-cli-eligibility-metadata.test.ts | 4 +- test/unit/miner-discover-cli.test.ts | 4 +- test/unit/public-eval-corpus.test.ts | 72 +++++++++++++++- test/unit/signal-tracking-wire.test.ts | 61 +++++++++++++- 16 files changed, 451 insertions(+), 31 deletions(-) diff --git a/apps/loopover-ui/content/docs/verify-this-review.mdx b/apps/loopover-ui/content/docs/verify-this-review.mdx index 0bd3d14799..333bb6ce02 100644 --- a/apps/loopover-ui/content/docs/verify-this-review.mdx +++ b/apps/loopover-ui/content/docs/verify-this-review.mdx @@ -138,11 +138,28 @@ transport — recompute `recordDigest` over the record's own remaining fields an curl -s "https://api.loopover.ai/v1/public/eval-scores" | jq '.records' ``` -That array is empty whenever the latest backtest run's corpus is empty. A checksum over zero cases -is byte-identical for every rule and every window, so it commits to nothing a reader could re-derive -the scores from — publishing a record against it would assert a reproducibility that does not exist. -An empty `records` array therefore means *the numbers are not currently committed to a corpus*, never -that the numbers are zero. +Each record's `commitments.corpusChecksum` is the checksum of the corpus you downloaded in step 2 — +the same rule, the same window, the same bytes. So you can close the loop yourself: hash the corpus, +read the record, compare. + +```bash +curl -s "https://api.loopover.ai/v1/public/eval-corpus?ruleId=ai_consensus_defect" | jq -r '.checksum' +curl -s "https://api.loopover.ai/v1/public/eval-scores" \ + | jq -r '.records[] | select(.workUnit.ruleId == "ai_consensus_defect") | .commitments.corpusChecksum' +``` + +A rule is **omitted** from `records` — rather than published with a commitment you could not check — +in exactly three cases: + +- **its corpus is empty.** A checksum over zero cases is byte-identical for every rule, every window + and every deployment, so it commits to nothing you could re-derive the scores from; +- **its corpus is truncated** (`truncated: true` in step 2). The checksum would then cover a prefix of + the window while the record's `decided`/`confirmed` cover all of it, so re-deriving from the + published cases would give you different numbers than the ones published; +- **the deployment has neither a persisted backtest run nor a usable corpus** for that rule. + +An empty `records` array therefore means *these numbers are not currently committed to a corpus* — +never that the numbers are zero. The aggregated run history is readable directly too, again against the deployment's own database (operator / self-host): diff --git a/packages/loopover-engine/src/calibration/signal-tracking.ts b/packages/loopover-engine/src/calibration/signal-tracking.ts index bcaf69dcc0..e0b6c7770e 100644 --- a/packages/loopover-engine/src/calibration/signal-tracking.ts +++ b/packages/loopover-engine/src/calibration/signal-tracking.ts @@ -49,7 +49,21 @@ export interface SignalStore { recordHumanOverride(event: HumanOverrideEvent): Promise; /** Every fired + override event for `ruleId` at or after `sinceMs` (epoch millis), oldest first. A host MAY * scope this further (e.g. to one repo) internally; the interface itself is unscoped beyond `ruleId`. */ - queryRuleHistory(ruleId: string, sinceMs: number): Promise<{ fired: RuleFiredEvent[]; overrides: HumanOverrideEvent[] }>; + /** + * Read one rule's history since `sinceMs`. + * + * `limit` bounds EACH of the two reads (#9805). It is part of the interface rather than an implementation + * detail because a caller that publishes the result has to know whether it saw the whole window -- + * /v1/public/eval-corpus reported `truncated: false` over a read it could not have completed, precisely + * because the bound was invisible from here. + * + * `saturated` is true when either read came back AT its bound, i.e. rows were almost certainly left behind. + */ + queryRuleHistory( + ruleId: string, + sinceMs: number, + limit?: number, + ): Promise<{ fired: RuleFiredEvent[]; overrides: HumanOverrideEvent[]; saturated: boolean }>; } /** Per-rule confusion-style report over a window: how many times it fired, how many of those got an explicit diff --git a/packages/loopover-miner/lib/signal-tracking-store.ts b/packages/loopover-miner/lib/signal-tracking-store.ts index 6deb90190a..e216bc7625 100644 --- a/packages/loopover-miner/lib/signal-tracking-store.ts +++ b/packages/loopover-miner/lib/signal-tracking-store.ts @@ -100,7 +100,12 @@ export function createSignalTrackingStore(eventLedger: SignalTrackingLedger): Si payload: toHumanOverridePayload(event), }); }, - async queryRuleHistory(ruleId: string, sinceMs: number): Promise<{ fired: RuleFiredEvent[]; overrides: HumanOverrideEvent[] }> { + // #9805: the interface carries a `limit`/`saturated` pair so a publishing caller can tell a complete read + // from a truncated one. This store reads the WHOLE local event ledger in memory -- there is no bound to + // hit and nothing is ever left behind -- so `limit` is accepted for interface conformance and ignored, + // and `saturated` is unconditionally false. That is a true statement here, not a stub: reporting `true` + // would claim missing rows that do not exist. + async queryRuleHistory(ruleId: string, sinceMs: number): Promise<{ fired: RuleFiredEvent[]; overrides: HumanOverrideEvent[]; saturated: boolean }> { const sinceIso = new Date(sinceMs).toISOString(); const fired: RuleFiredEvent[] = []; const overrides: HumanOverrideEvent[] = []; @@ -127,7 +132,7 @@ export function createSignalTrackingStore(eventLedger: SignalTrackingLedger): Si }); } } - return { fired, overrides }; + return { fired, overrides, saturated: false }; }, }; } diff --git a/src/api/routes.ts b/src/api/routes.ts index b23cecbac9..d1b9869e43 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -304,6 +304,7 @@ import { isFairnessAnalyticsEnabled, resolveFairnessAnalyticsManifestOverride } import { isRagEnabled } from "../review/rag-wire"; import { loadDecisionLedgerTip, loadPublicDecisionRecord, loadPublicLedgerRow, verifyDecisionLedger } from "../review/decision-record"; import { buildEvalScoreRecordsFromRulePrecision, filterEvalScoreRecords } from "../review/eval-score-records"; +import { buildPublicCorpusCommitments } from "../review/public-eval-corpus"; import { anchorSigningInput, buildLedgerAnchorPayload, currentAnchorKey, parseAnchorPublicKeys, publicAnchorStatus, signLedgerAnchorPayload } from "../review/ledger-anchor"; import { resolveProofPage } from "../review/proof-summary"; import { renderProofBadgeSvg } from "./proof-badge"; @@ -901,7 +902,13 @@ export function createApp() { // IO-touching source (the benchmark_run records from #9265) is exactly where real error handling belongs, // added when that source actually exists, not as untestable defensive code here. const precision = await loadPublicRulePrecision(c.env); - const records = await buildEvalScoreRecordsFromRulePrecision(precision, new Date().toISOString()); + // #9805: the per-rule fallback commitment, when no backtest run is persisted. Loaded HERE rather than + // inside the record builder so that module stays pure -- and loaded through the same + // loadPublicEvalCorpus the /v1/public/eval-corpus route serves, so the checksum a record commits to is by + // construction the one a reader re-derives from the bytes they downloaded, not a parallel computation + // that could drift from it. + const corpusChecksumByRuleId = await buildPublicCorpusCommitments(c.env, precision.rules.map((rule) => rule.ruleId)); + const records = await buildEvalScoreRecordsFromRulePrecision(precision, new Date().toISOString(), corpusChecksumByRuleId); const filtered = filterEvalScoreRecords(records, { subject: c.req.query("subject"), since: c.req.query("since"), diff --git a/src/review/eval-score-records.ts b/src/review/eval-score-records.ts index c1b6520764..9796d24224 100644 --- a/src/review/eval-score-records.ts +++ b/src/review/eval-score-records.ts @@ -76,6 +76,18 @@ async function finalizeRecord(input: EvalScoreRecordDigestInput): Promise { - if (!precision.latestBacktestRun) return []; - const { corpusChecksum } = precision.latestBacktestRun; - if (corpusChecksum === EMPTY_CORPUS_CHECKSUM) return []; +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. + 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; + return corpusChecksum === null || corpusChecksum === EMPTY_CORPUS_CHECKSUM ? [] : [{ row, corpusChecksum }]; + }); + const records = await Promise.all( - precision.rules.map((row) => + publishable.map(({ row, corpusChecksum }) => finalizeRecord({ schemaVersion: EVAL_SCORE_RECORD_SCHEMA_VERSION, subject: { kind: "agent", id: ORB_GATE_SUBJECT_ID }, diff --git a/src/review/public-eval-corpus.ts b/src/review/public-eval-corpus.ts index 9681e14945..cc268c4562 100644 --- a/src/review/public-eval-corpus.ts +++ b/src/review/public-eval-corpus.ts @@ -29,12 +29,18 @@ import { buildBacktestCorpus } from "@loopover/engine/calibration/backtest-corpus"; import { canonicalJson, sha256Hex } from "./decision-record"; import { NON_ATTRIBUTABLE_OVERRIDE_PROVENANCES, PUBLIC_PRECISION_WINDOW_DAYS } from "./public-rule-precision"; -import { createSignalStore } from "./signal-tracking-wire"; +import { createSignalStore, MAX_RULE_HISTORY_LIMIT } from "./signal-tracking-wire"; /** Hard cap on published cases. The window is already bounded, but an unbounded array on an * unauthenticated route is a footgun the moment a rule gets noisy; a truncated corpus is reported * honestly via `truncated` rather than silently trimmed. */ -export const PUBLIC_EVAL_CORPUS_MAX_CASES = 5_000; +// #9805: was 5_000, which could never bind. The corpus is built from a rule-history read that +// listAuditEventsByType hard-clamps to MAX_RULE_HISTORY_LIMIT rows, so a cap above that ceiling is +// unreachable and `truncated` was structurally always false -- while /v1/public/stats counts `decided` with +// an UNBOUNDED SQL COUNT(*). The two surfaces therefore agreed only while the window stayed under the read +// bound, and would have silently diverged after that: a complete-looking corpus serving a prefix of the +// cases the published precision was computed over. Pinned to the real ceiling so the cap and the read agree. +export const PUBLIC_EVAL_CORPUS_MAX_CASES = MAX_RULE_HISTORY_LIMIT; /** One published case: a {@link BacktestCase} minus `targetKey`, with `metadata` narrowed to the single * key the shipped classifier reads. `metadata` is omitted entirely (never `undefined`) when the firing @@ -124,8 +130,12 @@ export async function loadPublicEvalCorpus(env: Env, ruleId: string, nowMs: numb const sinceMs = nowMs - PUBLIC_PRECISION_WINDOW_DAYS * 24 * 60 * 60 * 1000; let fired: Awaited["queryRuleHistory"]>>["fired"] = []; let overrides: Awaited["queryRuleHistory"]>>["overrides"] = []; + // A read that came back at its bound almost certainly left rows behind, and a corpus that silently omits + // 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; try { - ({ fired, overrides } = await createSignalStore(env).queryRuleHistory(ruleId, sinceMs)); + ({ 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. } @@ -144,8 +154,42 @@ export async function loadPublicEvalCorpus(env: Env, ruleId: string, nowMs: numb ruleId, windowDays: PUBLIC_PRECISION_WINDOW_DAYS, caseCount: cases.length, - truncated, + // 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, checksum: await checksumPublicEvalCorpus(cases), cases, }; } + +/** + * #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 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; + * • 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 + * wrong. Omitting the record says "not committed"; publishing it would say something false. + * + * Sequential rather than Promise.all: each call is its own D1 read over a 90-day window, and the rule list + * is the handful that clear the publication floor -- fanning them out buys nothing and makes the read + * burst on an unauthenticated route. + */ +export async function buildPublicCorpusCommitments( + env: Env, + ruleIds: readonly string[], + nowMs: number = Date.now(), +): Promise> { + const commitments = new Map(); + for (const ruleId of ruleIds) { + const corpus = await loadPublicEvalCorpus(env, ruleId, nowMs); + if (corpus.caseCount === 0 || corpus.truncated) continue; + commitments.set(ruleId, corpus.checksum); + } + return commitments; +} diff --git a/src/review/signal-tracking-wire.ts b/src/review/signal-tracking-wire.ts index 60d27c10d7..8295295e93 100644 --- a/src/review/signal-tracking-wire.ts +++ b/src/review/signal-tracking-wire.ts @@ -69,6 +69,15 @@ function toHumanOverrideEvent(ruleId: string, row: { targetKey: string | null; m * are NOT fail-open the same way: a read error propagates, since a caller computing a precision report needs * to know its input is incomplete rather than silently scoring against a partial (possibly empty) history. */ +/** listAuditEventsByType's own default, restated so callers that do not pass a limit keep today's behaviour + * explicitly rather than by inheritance (#9805). */ +export const DEFAULT_RULE_HISTORY_LIMIT = 500; + +/** The most rows one queryRuleHistory read can return: listAuditEventsByType hard-clamps its limit to this, + * so asking for more silently yields this many. Anything built on top of a rule-history read is bounded by + * it, and a cap declared ABOVE it can never be the thing that actually truncates. */ +export const MAX_RULE_HISTORY_LIMIT = 2_000; + export function createSignalStore(env: Env): SignalStore { return { async recordRuleFired(event: RuleFiredEvent): Promise { @@ -93,15 +102,28 @@ export function createSignalStore(env: Env): SignalStore { createdAt: event.occurredAt || nowIso(), }).catch(() => undefined); }, - async queryRuleHistory(ruleId: string, sinceMs: number): Promise<{ fired: RuleFiredEvent[]; overrides: HumanOverrideEvent[] }> { + // #9805: `limit` is explicit rather than left to listAuditEventsByType's default of 500. The published + // corpus needs to know whether it saw the WHOLE window, and a caller that cannot choose the bound cannot + // tell a complete read from a truncated one. Defaulted so every existing caller is byte-identical. + // + // `saturated` is the honest signal: the row count came back exactly at the bound, so there are almost + // certainly more rows the caller did not see. It is deliberately not "did we hit MAX_CASES" -- the read + // bound is what actually limits the corpus, and conflating the two is how /v1/public/eval-corpus came to + // report `truncated: false` over a read it could not have completed. + async queryRuleHistory( + ruleId: string, + sinceMs: number, + limit: number = DEFAULT_RULE_HISTORY_LIMIT, + ): Promise<{ fired: RuleFiredEvent[]; overrides: HumanOverrideEvent[]; saturated: boolean }> { const sinceIso = new Date(sinceMs).toISOString(); const [firedRows, overrideRows] = await Promise.all([ - listAuditEventsByType(env, ruleFiredEventType(ruleId), sinceIso), - listAuditEventsByType(env, humanOverrideEventType(ruleId), sinceIso), + listAuditEventsByType(env, ruleFiredEventType(ruleId), sinceIso, limit), + listAuditEventsByType(env, humanOverrideEventType(ruleId), sinceIso, limit), ]); return { fired: firedRows.map((row) => toRuleFiredEvent(ruleId, row)), overrides: overrideRows.map((row) => toHumanOverrideEvent(ruleId, row)), + saturated: firedRows.length >= limit || overrideRows.length >= limit, }; }, }; diff --git a/test/integration/public-eval-scores-route.test.ts b/test/integration/public-eval-scores-route.test.ts index adbcccfae5..e879da7841 100644 --- a/test/integration/public-eval-scores-route.test.ts +++ b/test/integration/public-eval-scores-route.test.ts @@ -100,3 +100,73 @@ describe("GET /v1/public/eval-scores (#9266, epic #8534, spec #9215)", () => { expect(await res.json()).toEqual({ records: [] }); }); }); + +// #9805 Deliverable 5: the reader's ACTUAL workflow, end to end over both public routes. Everything else in +// this file feeds the builder a seeded backtest-run row; this one has no persisted run at all -- the hosted +// topology, where review execution is retired -- and checks that what /v1/public/eval-scores commits to is +// byte-identical to what /v1/public/eval-corpus serves. +describe("a stranger can tie a record to the corpus they downloaded (#9805)", () => { + beforeEach(() => { + clearPublicStatsManifestOverrideCacheForTest(); + }); + + async function seedDecidedCorpus(env: Env, ruleId: string, count: number): Promise { + const store = createSignalStore(env); + for (let i = 0; i < count; i += 1) { + await store.recordRuleFired({ + ruleId, + targetKey: `acme/widgets#${i + 1}`, + outcome: "close", + occurredAt: new Date(NOW - 5000 - i).toISOString(), + metadata: { confidence: 0.4 + (i % 5) * 0.1 }, + }); + await store.recordHumanOverride({ + ruleId, + targetKey: `acme/widgets#${i + 1}`, + verdict: i % 4 === 0 ? "reversed" : "confirmed", + occurredAt: new Date(NOW - 1000 - i).toISOString(), + }); + } + } + + it("REGRESSION: publishes records with NO persisted backtest run, each committing to the corpus the other route serves", async () => { + const env = createTestEnv(); + env.LOOPOVER_PUBLIC_STATS = "true"; + await seedDecidedCorpus(env, "ai_consensus_defect", 20); + + const app = createApp(); + const scores = await (await app.request("/v1/public/eval-scores", {}, env)).json<{ records: EvalScoreRecord[] }>(); + expect(scores.records).toHaveLength(1); + + // Exactly what the walkthrough tells a reader to do: download the corpus, hash it, compare. + const corpus = await (await app.request("/v1/public/eval-corpus?ruleId=ai_consensus_defect", {}, env)).json<{ checksum: string; caseCount: number; truncated: boolean }>(); + expect(corpus.caseCount).toBe(20); + expect(corpus.truncated).toBe(false); + expect(scores.records[0]!.commitments.corpusChecksum).toBe(corpus.checksum); + + // And the record still commits to its own content, so the freeze point cannot be swapped undetected. + const { recordDigest, ...rest } = scores.records[0]!; + expect(await contentDigest(rest)).toBe(recordDigest); + expect(scores.records[0]!.subject.id).toBe(ORB_GATE_SUBJECT_ID); + }); + + it("publishes nothing for a rule with no corpus, rather than a record a reader could not check", async () => { + const env = createTestEnv(); + env.LOOPOVER_PUBLIC_STATS = "true"; + // Overrides only: `decided` counts them via SQL, so the rule reaches rulePrecision -- but with no firings + // there are no labeled cases, so there is no corpus to commit to. This is the exact shape that would + // otherwise publish a record against the rule-independent empty-corpus digest. + const store = createSignalStore(env); + for (let i = 0; i < 20; i += 1) { + await store.recordHumanOverride({ + ruleId: "ai_consensus_defect", + targetKey: `acme/widgets#${i + 1}`, + verdict: "confirmed", + occurredAt: new Date(NOW - 1000 - i).toISOString(), + }); + } + + const res = await createApp().request("/v1/public/eval-scores", {}, env); + expect(await res.json<{ records: EvalScoreRecord[] }>()).toEqual({ records: [] }); + }); +}); diff --git a/test/unit/configured-gate-blocker-signals.test.ts b/test/unit/configured-gate-blocker-signals.test.ts index 0174a24dd6..b8f6a5899c 100644 --- a/test/unit/configured-gate-blocker-signals.test.ts +++ b/test/unit/configured-gate-blocker-signals.test.ts @@ -200,7 +200,7 @@ describe("recordConfiguredGateBlockerSignals (#8104)", () => { throw new Error("signal store down"); }, recordHumanOverride: async () => undefined, - queryRuleHistory: async () => ({ fired: [], overrides: [] }), + queryRuleHistory: async () => ({ fired: [], overrides: [], saturated: false }), }); await expect( recordConfiguredGateBlockerSignals( @@ -355,7 +355,7 @@ describe("recordGateScoreSignals (#8223)", () => { throw new Error("signal store down"); }, recordHumanOverride: async () => undefined, - queryRuleHistory: async () => ({ fired: [], overrides: [] }), + queryRuleHistory: async () => ({ fired: [], overrides: [], saturated: false }), }); await expect( recordGateScoreSignals(createTestEnv(), { slopGateMode: "block", slopRisk: 72, qualityGateMode: "advisory", readinessScore: 40, qualityGateMinScore: 70 }, "owner/repo", 7), diff --git a/test/unit/eval-score-records.test.ts b/test/unit/eval-score-records.test.ts index 910e58ca3f..ff47c9875b 100644 --- a/test/unit/eval-score-records.test.ts +++ b/test/unit/eval-score-records.test.ts @@ -154,3 +154,86 @@ describe("verifyEvalScoreRecordDigest", () => { expect(await verifyEvalScoreRecordDigest(tampered)).toBe(false); }); }); + +// #9805: on a deployment with review execution retired (the hosted Worker), no backtest run is ever +// persisted, so this surface served [] while /v1/public/eval-corpus published a complete, downloadable +// corpus for the same rule over the same window. The fallback commits each record to THAT corpus. +describe("per-rule corpus commitments when no backtest run is persisted (#9805)", () => { + const precisionOf = (rules: PublicRulePrecision["rules"], latestBacktestRun: PublicRulePrecision["latestBacktestRun"] = null): PublicRulePrecision => ({ + windowDays: 90, + rules, + reversals: { reopened: 0, reverted: 0, superseded: 0 }, + latestBacktestRun, + }); + const rule = (ruleId: string): PublicRulePrecision["rules"][number] => + ({ ruleId, decided: 40, confirmed: 25, precision: 0.625 }) as PublicRulePrecision["rules"][number]; + + it("REGRESSION: publishes a record per rule instead of [], committing to that rule's published corpus", async () => { + const records = await buildEvalScoreRecordsFromRulePrecision( + precisionOf([rule("ai_consensus_defect")]), + ISSUED_AT, + new Map([["ai_consensus_defect", "a".repeat(64)]]), + ); + expect(records).toHaveLength(1); + expect(records[0]!.commitments.corpusChecksum).toBe("a".repeat(64)); + expect(records[0]!.trust.tier).toBe("reproducible"); + await expect(verifyEvalScoreRecordDigest(records[0]!)).resolves.toBe(true); + }); + + it("REGRESSION: two rules get their OWN checksums -- one run's checksum stamped across every record was the latent bug", async () => { + const records = await buildEvalScoreRecordsFromRulePrecision( + precisionOf([rule("rule_a"), rule("rule_b")]), + ISSUED_AT, + new Map([ + ["rule_a", "a".repeat(64)], + ["rule_b", "b".repeat(64)], + ]), + ); + const byRule = new Map(records.map((r) => [(r.workUnit as { ruleId: string }).ruleId, r.commitments.corpusChecksum])); + expect(byRule.get("rule_a")).toBe("a".repeat(64)); + expect(byRule.get("rule_b")).toBe("b".repeat(64)); + expect(byRule.get("rule_a")).not.toBe(byRule.get("rule_b")); + }); + + it("omits only the rule with no commitment, still publishing the others", async () => { + const records = await buildEvalScoreRecordsFromRulePrecision( + precisionOf([rule("has_corpus"), rule("no_corpus")]), + ISSUED_AT, + new Map([["has_corpus", "c".repeat(64)]]), + ); + expect(records.map((r) => (r.workUnit as { ruleId: string }).ruleId)).toEqual(["has_corpus"]); + }); + + it("refuses the empty-corpus checksum even when supplied as a fallback", async () => { + const records = await buildEvalScoreRecordsFromRulePrecision( + precisionOf([rule("ai_consensus_defect")]), + ISSUED_AT, + new Map([["ai_consensus_defect", EMPTY_CORPUS_CHECKSUM]]), + ); + expect(records).toEqual([]); + }); + + it("INVARIANT: a persisted backtest run still WINS, so self-host behaviour is unchanged", 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)); + }); + + 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. + const records = await buildEvalScoreRecordsFromRulePrecision( + precisionOf([rule("ai_consensus_defect")], { corpusChecksum: EMPTY_CORPUS_CHECKSUM, at: ISSUED_AT }), + ISSUED_AT, + new Map([["ai_consensus_defect", "f".repeat(64)]]), + ); + expect(records[0]!.commitments.corpusChecksum).toBe("f".repeat(64)); + }); + + it("still returns [] with neither a run nor any commitment -- the #9215 rule is unchanged", async () => { + expect(await buildEvalScoreRecordsFromRulePrecision(precisionOf([rule("ai_consensus_defect")]), ISSUED_AT)).toEqual([]); + }); +}); diff --git a/test/unit/linked-issue-satisfaction-run.test.ts b/test/unit/linked-issue-satisfaction-run.test.ts index fd38e2e9de..5e7da9b65d 100644 --- a/test/unit/linked-issue-satisfaction-run.test.ts +++ b/test/unit/linked-issue-satisfaction-run.test.ts @@ -612,7 +612,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" throw new Error("signal store down"); }, recordHumanOverride: async () => undefined, - queryRuleHistory: async () => ({ fired: [], overrides: [] }), + queryRuleHistory: async () => ({ fired: [], overrides: [], saturated: false }), }); const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) })); const adv = advisory(); diff --git a/test/unit/miner-attempt-cli.test.ts b/test/unit/miner-attempt-cli.test.ts index 56ca1989c5..923d9d2224 100644 --- a/test/unit/miner-attempt-cli.test.ts +++ b/test/unit/miner-attempt-cli.test.ts @@ -1333,7 +1333,7 @@ describe("runAttempt (#5132)", () => { const store = { recordRuleFired: overrides.recordRuleFired ?? (async (event: RuleFiredEvent) => void fired.push(event)), recordHumanOverride: async () => undefined, - queryRuleHistory: async () => ({ fired: [], overrides: [] }), + queryRuleHistory: async () => ({ fired: [], overrides: [], saturated: false }), } as unknown as SignalStore; return { store, fired }; } diff --git a/test/unit/miner-discover-cli-eligibility-metadata.test.ts b/test/unit/miner-discover-cli-eligibility-metadata.test.ts index c414a7b3da..e7b51457ef 100644 --- a/test/unit/miner-discover-cli-eligibility-metadata.test.ts +++ b/test/unit/miner-discover-cli-eligibility-metadata.test.ts @@ -254,7 +254,7 @@ describe("recordEligibilityExclusionSignals via runDiscover (#8544)", () => { }); }), recordHumanOverride: vi.fn(async () => undefined), - queryRuleHistory: vi.fn(async () => ({ fired: [], overrides: [] })), + queryRuleHistory: vi.fn(async () => ({ fired: [], overrides: [], saturated: false })), }, }; } @@ -351,7 +351,7 @@ describe("recordEligibilityExclusionSignals via runDiscover (#8544)", () => { initSignalTrackingStore: () => ({ recordRuleFired, recordHumanOverride: vi.fn(async () => undefined), - queryRuleHistory: vi.fn(async () => ({ fired: [], overrides: [] })), + queryRuleHistory: vi.fn(async () => ({ fired: [], overrides: [], saturated: false })), }), }); expect(exitCode).toBe(0); diff --git a/test/unit/miner-discover-cli.test.ts b/test/unit/miner-discover-cli.test.ts index 734b9c8544..28c1dd2f79 100644 --- a/test/unit/miner-discover-cli.test.ts +++ b/test/unit/miner-discover-cli.test.ts @@ -1652,7 +1652,7 @@ describe("runDiscover onResult hook (#6522)", () => { fired.push({ ruleId: event.ruleId, targetKey: event.targetKey, outcome: event.outcome }); }), recordHumanOverride: vi.fn(async () => undefined), - queryRuleHistory: vi.fn(async () => ({ fired: [], overrides: [] })), + queryRuleHistory: vi.fn(async () => ({ fired: [], overrides: [], saturated: false })), }, }; } @@ -1733,7 +1733,7 @@ describe("runDiscover onResult hook (#6522)", () => { initSignalTrackingStore: () => ({ recordRuleFired, recordHumanOverride: vi.fn(async () => undefined), - queryRuleHistory: vi.fn(async () => ({ fired: [], overrides: [] })), + queryRuleHistory: vi.fn(async () => ({ fired: [], overrides: [], saturated: false })), }), }); expect(exitCode).toBe(0); diff --git a/test/unit/public-eval-corpus.test.ts b/test/unit/public-eval-corpus.test.ts index 6cc223df05..50b64014db 100644 --- a/test/unit/public-eval-corpus.test.ts +++ b/test/unit/public-eval-corpus.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { applyPublicEvalCorpusCap, + buildPublicCorpusCommitments, checksumPublicEvalCorpus, loadPublicEvalCorpus, PUBLIC_EVAL_CORPUS_MAX_CASES, @@ -10,7 +11,7 @@ import { } from "../../src/review/public-eval-corpus"; import { canonicalJson, sha256Hex } from "../../src/review/decision-record"; import { PUBLIC_PRECISION_WINDOW_DAYS } from "../../src/review/public-rule-precision"; -import { createSignalStore } from "../../src/review/signal-tracking-wire"; +import { createSignalStore, MAX_RULE_HISTORY_LIMIT } from "../../src/review/signal-tracking-wire"; import { createTestEnv } from "../helpers/d1"; // #9636: the anonymously-downloadable corpus. The load-bearing properties are that NOTHING identifying @@ -207,3 +208,72 @@ describe("loadPublicEvalCorpus — end to end over the real signal tables", () = expect((await loadPublicEvalCorpus(createTestEnv(), "r", NOW)).truncated).toBe(false); }); }); + +// #9805: the commitment map behind /v1/public/eval-scores' fallback. Every entry must be a checksum a reader +// can reproduce from the corpus this same deployment serves -- so the interesting cases are the ones this +// deliberately OMITS, since a wrong entry is worse than a missing record. +describe("buildPublicCorpusCommitments (#9805)", () => { + it("maps each rule to the SAME checksum /v1/public/eval-corpus publishes for it", async () => { + const env = createTestEnv(); + await seedPair(env, { ruleId: "rule_a", targetKey: "acme/widgets#1", verdict: "reversed", confidence: 0.8 }); + await seedPair(env, { ruleId: "rule_b", targetKey: "acme/widgets#2", verdict: "confirmed", confidence: 0.4 }); + + const commitments = await buildPublicCorpusCommitments(env, ["rule_a", "rule_b"], NOW); + + // The record's commitment and the reader's download must be the same bytes, by construction. + expect(commitments.get("rule_a")).toBe((await loadPublicEvalCorpus(env, "rule_a", NOW)).checksum); + expect(commitments.get("rule_b")).toBe((await loadPublicEvalCorpus(env, "rule_b", NOW)).checksum); + }); + + it("gives two rules DIFFERENT checksums -- the per-rule bug this exists to prevent", async () => { + const env = createTestEnv(); + await seedPair(env, { ruleId: "rule_a", targetKey: "acme/widgets#1", verdict: "reversed", confidence: 0.8 }); + await seedPair(env, { ruleId: "rule_b", targetKey: "acme/widgets#2", verdict: "confirmed", confidence: 0.4 }); + + const commitments = await buildPublicCorpusCommitments(env, ["rule_a", "rule_b"], NOW); + expect(commitments.get("rule_a")).not.toBe(commitments.get("rule_b")); + }); + + it("omits a rule with an EMPTY corpus rather than mapping it to the rule-independent empty digest", async () => { + const env = createTestEnv(); + const commitments = await buildPublicCorpusCommitments(env, ["never_fired"], NOW); + expect(commitments.has("never_fired")).toBe(false); + // The value it would otherwise have carried is identical for every rule and deployment. + expect((await loadPublicEvalCorpus(env, "never_fired", NOW)).checksum).toBe(await checksumPublicEvalCorpus([])); + }); + + it("omits a TRUNCATED corpus -- its checksum covers a prefix while the score covers the whole window", async () => { + // Real saturation, not a stub: MAX_RULE_HISTORY_LIMIT firings so the read comes back AT its bound, plus + // one override so the corpus is non-empty -- otherwise the empty check would be doing the omitting and + // this would prove nothing about truncation. + const env = createTestEnv(); + const store = createSignalStore(env); + for (let i = 0; i < MAX_RULE_HISTORY_LIMIT; i += 1) { + await store.recordRuleFired({ + ruleId: "busy_rule", + targetKey: `acme/widgets#${i}`, + outcome: "close", + occurredAt: new Date(NOW - 20_000 - i).toISOString(), + metadata: { confidence: 0.5 }, + }); + } + await store.recordHumanOverride({ ruleId: "busy_rule", targetKey: "acme/widgets#0", verdict: "reversed", occurredAt: new Date(NOW - 10_000).toISOString() }); + + const corpus = await loadPublicEvalCorpus(env, "busy_rule", NOW); + expect(corpus.truncated).toBe(true); + expect(corpus.caseCount).toBeGreaterThan(0); // so the omission below is truncation, not emptiness + + expect((await buildPublicCorpusCommitments(env, ["busy_rule"], NOW)).has("busy_rule")).toBe(false); + }, 60_000); + + it("omits a rule whose corpus read FAILED, because that degrades to an empty corpus", async () => { + const broken = createTestEnv(); + broken.DB = { prepare: () => { throw new Error("boom"); } } as never; + expect((await buildPublicCorpusCommitments(broken, ["rule_a"], NOW)).size).toBe(0); + }); + + it("returns an empty map for no rules, without touching the database", async () => { + const env = createTestEnv(); + expect((await buildPublicCorpusCommitments(env, [], NOW)).size).toBe(0); + }); +}); diff --git a/test/unit/signal-tracking-wire.test.ts b/test/unit/signal-tracking-wire.test.ts index 246a74cc15..b4b97181bc 100644 --- a/test/unit/signal-tracking-wire.test.ts +++ b/test/unit/signal-tracking-wire.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { listAuditEventsByType, recordAuditEvent } from "../../src/db/repositories"; -import { createSignalStore } from "../../src/review/signal-tracking-wire"; +import { createSignalStore, DEFAULT_RULE_HISTORY_LIMIT } from "../../src/review/signal-tracking-wire"; import { createTestEnv } from "../helpers/d1"; const ONE_HOUR_MS = 60 * 60 * 1000; @@ -271,3 +271,62 @@ describe("listAuditEventsByType (#7982) — corrupt-row resilience", () => { expect(rows[0]?.metadata).toEqual({}); }); }); + +// #9805: the read bound is now explicit, and `saturated` reports whether it was hit. This matters because +// /v1/public/eval-corpus publishes a checksum over what this returns -- committing a published score to a +// silently-truncated corpus is the failure the flag exists to make impossible. +describe("queryRuleHistory read bound and saturation (#9805)", () => { + const seed = async (env: Env, ruleId: string, n: number) => { + const store = createSignalStore(env); + for (let i = 0; i < n; i += 1) { + await store.recordRuleFired({ + ruleId, + targetKey: `acme/widgets#${i}`, + outcome: "close", + occurredAt: new Date(Date.now() - 10_000 - i).toISOString(), + metadata: { confidence: 0.5 }, + }); + } + }; + + it("reports saturated=false when the read comfortably fits inside its bound", async () => { + const env = createTestEnv(); + await seed(env, "small_rule", 3); + const history = await createSignalStore(env).queryRuleHistory("small_rule", Date.now() - 86_400_000, 10); + expect(history.fired).toHaveLength(3); + expect(history.saturated).toBe(false); + }); + + it("reports saturated=true when the read comes back AT its bound, and returns only that many", async () => { + const env = createTestEnv(); + await seed(env, "big_rule", 6); + const history = await createSignalStore(env).queryRuleHistory("big_rule", Date.now() - 86_400_000, 4); + expect(history.fired).toHaveLength(4); // rows were left behind + expect(history.saturated).toBe(true); + }); + + it("saturates on the OVERRIDES side too, not just firings", async () => { + const env = createTestEnv(); + const store = createSignalStore(env); + for (let i = 0; i < 5; i += 1) { + await store.recordHumanOverride({ + ruleId: "override_heavy", + targetKey: `acme/widgets#${i}`, + verdict: "confirmed", + occurredAt: new Date(Date.now() - 10_000 - i).toISOString(), + }); + } + const history = await store.queryRuleHistory("override_heavy", Date.now() - 86_400_000, 5); + expect(history.fired).toHaveLength(0); + expect(history.saturated).toBe(true); + }); + + it("defaults to DEFAULT_RULE_HISTORY_LIMIT when no bound is given, preserving every pre-existing caller", async () => { + const env = createTestEnv(); + await seed(env, "defaulted", 2); + const history = await createSignalStore(env).queryRuleHistory("defaulted", Date.now() - 86_400_000); + expect(history.fired).toHaveLength(2); + expect(history.saturated).toBe(false); + expect(DEFAULT_RULE_HISTORY_LIMIT).toBe(500); + }); +});