Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions src/review/eval-score-records.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,17 @@ async function finalizeRecord(input: EvalScoreRecordDigestInput): Promise<EvalSc
return { ...input, recordDigest };
}

/** #9215's coverage definition -- `decided / (decided + abstained)` -- as the single implementation every
* work-unit kind derives from, so a record's published `coverage` and a validator's re-derivation of it can
* never disagree. `null` only when nothing was seen at all (`0/0` is undefined, not zero): that is the same
* guard-the-denominator-else-null shape {@link PublicRulePrecisionRow.precision} uses below its sample floor
* (`public-rule-precision.ts`), never a masked `0`. Exported so a future emitter (#9265's `benchmark_run`)
* states coverage by calling this rather than restating the formula. */
export function evalScoreCoverage(decided: number, abstained: number): number | null {
const total = decided + abstained;
return total > 0 ? decided / total : null;
}

/**
* 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,
Expand Down Expand Up @@ -99,8 +110,13 @@ async function finalizeRecord(input: EvalScoreRecordDigestInput): Promise<EvalSc
* `recall` and `abstained` do not apply to this work-unit kind: ORB's gate rules fire deterministically (no
* agent choosing to abstain) and this data measures precision, not a false-negative rate. `recall` is
* `null` (genuinely inapplicable, never a misleading `0`); `abstained` is `0` (there is no abstention
* concept here, so `0` is the correct value, not a masked null). PURE -- no IO, no clock (the caller
* supplies `issuedAt`).
* concept here, so `0` is the correct value, not a masked null). `coverage` follows from that `abstained`:
* with no abstention concept the denominator is just `decided`, so every rule that decided anything covered
* all of it and states `1` -- computed via {@link evalScoreCoverage}, never hardcoded, so it stays correct
* for a kind that does abstain. It was previously `null` by symmetry with `recall`, which published "coverage
* unknown" for a quantity the record's own `decided`/`abstained` pin down exactly, so a validator re-deriving
* per #9215 computed `1` and disagreed with the field (#9643). Only `decided === 0` is genuinely undefined
* (`0/0`) and stays `null`. PURE -- no IO, no clock (the caller supplies `issuedAt`).
*/
export async function buildEvalScoreRecordsFromRulePrecision(
precision: PublicRulePrecision,
Expand Down Expand Up @@ -136,7 +152,9 @@ export async function buildEvalScoreRecordsFromRulePrecision(
confirmed: row.confirmed,
precision: row.precision,
recall: null,
coverage: null,
// Same literal `0` the sibling `abstained` publishes, passed rather than re-stated, so the two can
// never drift into a record whose coverage contradicts its own abstention count.
coverage: evalScoreCoverage(row.decided, 0),
abstained: 0,
},
commitments: {
Expand Down
4 changes: 3 additions & 1 deletion test/integration/public-eval-scores-route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ describe("GET /v1/public/eval-scores (#9266, epic #8534, spec #9215)", () => {
expect(body.records).toHaveLength(1);
const [record] = body.records;
expect(record?.workUnit).toEqual({ kind: "outcome_confirmed_precision", ruleId: "ai_consensus_defect" });
expect(record?.score).toEqual({ decided: 20, confirmed: 16, precision: 0.8, recall: null, coverage: null, abstained: 0 });
// coverage is 1, not null: abstained is structurally 0 for this work-unit kind, so the record's own
// decided/(decided+abstained) is fully determined and the published field states it (#9643).
expect(record?.score).toEqual({ decided: 20, confirmed: 16, precision: 0.8, recall: null, coverage: 1, abstained: 0 });
expect(record?.commitments.corpusChecksum).toBe("freeze-point-checksum");
expect(record?.subject).toEqual({ kind: "agent", id: ORB_GATE_SUBJECT_ID });

Expand Down
43 changes: 41 additions & 2 deletions test/unit/eval-score-records.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
import {
buildEvalScoreRecordsFromRulePrecision,
EVAL_SCORE_RECORD_SCHEMA_VERSION,
evalScoreCoverage,
filterEvalScoreRecords,
ORB_GATE_SUBJECT_ID,
OUTCOME_CONFIRMED_PRECISION_SCORING_RULE_VERSION,
Expand All @@ -27,6 +28,18 @@ const PRECISION_WITH_FREEZE_POINT: PublicRulePrecision = {
latestBacktestRun: { corpusChecksum: "abc123def456", at: "2026-07-27T10:00:00.000Z" },
};

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
expect(evalScoreCoverage(3, 1)).toBe(0.75); // a kind that DOES abstain needs no second implementation
expect(evalScoreCoverage(0, 4)).toBe(0); // decided nothing of what it saw -- a real 0, not undefined
});

it("returns null only when nothing was seen at all (0/0 is undefined, never a masked 0)", () => {
expect(evalScoreCoverage(0, 0)).toBeNull();
});
});

describe("buildEvalScoreRecordsFromRulePrecision (#9266)", () => {
it("returns an empty array when there is no persisted backtest run to commit to", async () => {
const records = await buildEvalScoreRecordsFromRulePrecision({ ...PRECISION_WITH_FREEZE_POINT, latestBacktestRun: null }, ISSUED_AT);
Expand Down Expand Up @@ -72,11 +85,37 @@ 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 withPrecision = records.find((r) => r.workUnit.kind === "outcome_confirmed_precision" && r.workUnit.ruleId === "ai_consensus_defect");
expect(withPrecision?.score).toEqual({ decided: 25, confirmed: 20, precision: 0.8, recall: null, coverage: null, abstained: 0 });
// 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 });

const sparse = records.find((r) => r.workUnit.kind === "outcome_confirmed_precision" && r.workUnit.ruleId === "sparse_rule");
// Below the public sample floor: precision is null (never a misleading 0), decided/confirmed still real counts.
expect(sparse?.score).toEqual({ decided: 9, confirmed: 9, precision: null, recall: null, coverage: null, abstained: 0 });
// coverage is still 1 -- the sample floor gates precision only, and 9 decided of 9 seen is full coverage.
expect(sparse?.score).toEqual({ decided: 9, confirmed: 9, precision: null, recall: null, coverage: 1, abstained: 0 });
});

it("REGRESSION (#9643): a rule that decided nothing keeps coverage null -- 0/0 is undefined, not zero", async () => {
// The one case where null is the honest answer, and the arm that makes `coverage: 1` a computation rather
// than a hardcoded constant. Previously EVERY record published null, including the 25-decided one above,
// so a validator re-deriving decided/(decided+abstained) per #9215 computed 1 and disagreed with the field.
const records = await buildEvalScoreRecordsFromRulePrecision(
{ ...PRECISION_WITH_FREEZE_POINT, rules: [{ ruleId: "never_fired", decided: 0, confirmed: 0, precision: null }] },
ISSUED_AT,
);
expect(records).toHaveLength(1);
expect(records[0]?.score).toEqual({ decided: 0, confirmed: 0, precision: null, recall: null, coverage: null, abstained: 0 });
});

it("REGRESSION (#9643): records built after the coverage change still verify against their own digest", async () => {
// 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);
expect(records).toHaveLength(2);
for (const record of records) {
expect(record.score.coverage).toBe(1);
await expect(verifyEvalScoreRecordDigest(record)).resolves.toBe(true);
}
});

it("each record's recordDigest is the sha256 of its own canonical content (independently recomputable)", async () => {
Expand Down