fix(engine): guard benchmark ground-truth against invalid window and duplicate roster

packages/loopover-engine/src/calibration/benchmark-ground-truth.ts

@@ -26,6 +26,21 @@
 //   later event — including a later reversal — cannot retroactively change a settled label. That is what
 //   makes a leaderboard reproducible instead of drifting under its own scores.
 //
+// Three inputs this module REJECTS rather than mislabels, each a caller/snapshot-builder bug that must fail
+// loudly instead of scoring corrupted numbers — the same throw-for-caller-bug discipline `splitBacktestCorpus`
+// uses for an out-of-range fraction:
+//
+//   INVALID `frozenAt`. An unparseable `frozenAt` throws `invalid_frozen_at` — checked in `benchmarkHorizonEnd`
+//   before any label is derived, so a bad window fails closed instead of producing an `Invalid Date` horizon
+//   whose every subsequent comparison silently drops the event.
+//
+//   INVALID `horizonDays`. A non-finite `horizonDays` throws `invalid_horizon_days`, in the same guard, for the
+//   same reason — a `NaN` window is a caller bug, not an empty benchmark.
+//
+//   DUPLICATE ROSTER ENTRY. A `workUnitIds` roster containing the same id twice throws
+//   `invalid_duplicate_work_unit_id` — silently de-duplicating would hide a corrupted snapshot (#9259) while
+//   double-counting the unit in `workUnits`, in `unresolved`, and in every rate divided by them.
+//
 // Pure core (this module); the IO wrapper that reads events from D1 lives engine-side of the harness, the
 // same split every sibling calibration module uses.
 
@@ -99,7 +114,16 @@ const MS_PER_DAY = 24 * 60 * 60 * 1000;
 /** The inclusive horizon end for a snapshot. Exported so the harness, the scorer, and any third-party
  *  verifier compute the SAME instant rather than three slightly different ones. */
 export function benchmarkHorizonEnd(frozenAt: string, horizonDays: number): string {
-  return new Date(Date.parse(frozenAt) + horizonDays * MS_PER_DAY).toISOString();
+  const frozenAtMs = Date.parse(frozenAt);
+  // Negated-compound guards so a NaN also fails closed, rather than flowing into `new Date(NaN).toISOString()`
+  // (a bare `RangeError`) — a named reason names the offending field the way the sibling validators do.
+  if (!Number.isFinite(frozenAtMs)) {
+    throw new Error(`invalid_frozen_at: ${frozenAt}`);
+  }
+  if (!Number.isFinite(horizonDays)) {
+    throw new Error(`invalid_horizon_days: ${horizonDays}`);
+  }
+  return new Date(frozenAtMs + horizonDays * MS_PER_DAY).toISOString();
 }
 
 function withinHorizon(occurredAt: string, frozenAtMs: number, horizonEndMs: number): boolean {
@@ -125,10 +149,22 @@ export function deriveBenchmarkGroundTruth(input: {
   events: readonly RealizedMaintainerEvent[];
   reversals?: readonly RealizedReversalEvent[] | undefined;
 }): BenchmarkGroundTruthSet {
-  const frozenAtMs = Date.parse(input.frozenAt);
+  // Reject an invalid window BEFORE any other work, so no partial result is ever computed for a bad horizon.
   const horizonEnd = benchmarkHorizonEnd(input.frozenAt, input.horizonDays);
+  const frozenAtMs = Date.parse(input.frozenAt);
   const horizonEndMs = Date.parse(horizonEnd);
 
+  // A duplicated roster entry is a snapshot-builder bug (#9259): emitting two truths for one id would
+  // double-count it in `workUnits`, in `unresolved`, and in `unresolvedRate`'s denominator. Reject it loudly
+  // rather than de-duplicating, which would let the corrupted snapshot score as if it were clean.
+  const seenWorkUnitIds = new Set<string>();
+  for (const workUnitId of input.workUnitIds) {
+    if (seenWorkUnitIds.has(workUnitId)) {
+      throw new Error(`invalid_duplicate_work_unit_id: ${workUnitId}`);
+    }
+    seenWorkUnitIds.add(workUnitId);
+  }
+
   // The SETTLED state is the LAST qualifying action in the window, not the first: a maintainer who labels,
   // then requests changes, then merges has settled on the merge. Ties on identical timestamps keep the
   // earlier-listed event, so a caller's stable input order yields a stable label.

packages/loopover-engine/test/benchmark-ground-truth.test.ts

@@ -193,6 +193,57 @@ test("an empty work-unit roster reports a NULL unresolved rate, never 0 (which w
   assert.deepEqual(scoreableGroundTruths(set), []);
 });
 
+test("REGRESSION: benchmarkHorizonEnd throws a NAMED error for an unparseable frozenAt or non-finite horizonDays", () => {
+  // Fails closed as `invalid_frozen_at` rather than the bare RangeError `new Date(NaN).toISOString()` throws.
+  assert.throws(() => benchmarkHorizonEnd("not a date", 14), /invalid_frozen_at: not a date/);
+  assert.throws(() => benchmarkHorizonEnd("", 14), /invalid_frozen_at/);
+  // A non-finite horizonDays is a caller bug, not an empty benchmark — NaN and Infinity both fail closed.
+  assert.throws(() => benchmarkHorizonEnd(FROZEN, Number.NaN), /invalid_horizon_days: NaN/);
+  assert.throws(() => benchmarkHorizonEnd(FROZEN, Number.POSITIVE_INFINITY), /invalid_horizon_days/);
+});
+
+test("REGRESSION: the window guard fires from deriveBenchmarkGroundTruth before any partial result is computed", () => {
+  assert.throws(
+    () => deriveBenchmarkGroundTruth({ ...BASE, frozenAt: "not a date", workUnitIds: ["o/r#1"], events: [] }),
+    /invalid_frozen_at: not a date/,
+  );
+  assert.throws(
+    () => deriveBenchmarkGroundTruth({ ...BASE, horizonDays: Number.NaN, workUnitIds: ["o/r#1"], events: [] }),
+    /invalid_horizon_days: NaN/,
+  );
+});
+
+test("REGRESSION: a duplicated workUnitId is rejected, never silently de-duplicated into a corrupt denominator", () => {
+  assert.throws(
+    () => deriveBenchmarkGroundTruth({ ...BASE, workUnitIds: ["o/r#1", "o/r#2", "o/r#1"], events: [] }),
+    /invalid_duplicate_work_unit_id: o\/r#1/,
+  );
+});
+
+test("the guards are ADDITIVE: a valid input still yields the exact same ground-truth set", () => {
+  const set = deriveBenchmarkGroundTruth({
+    ...BASE,
+    workUnitIds: ["o/r#1", "o/r#2"],
+    events: [
+      { workUnitId: "o/r#1", action: "merge", occurredAt: at(3) },
+      { workUnitId: "o/r#2", action: "close", occurredAt: at(6), reasonClass: "spam" },
+    ],
+    reversals: [{ workUnitId: "o/r#1", kind: "reversal_reverted", occurredAt: at(8) }],
+  });
+  assert.deepEqual(set, {
+    schemaVersion: 1,
+    snapshotRef: BASE.snapshotRef,
+    horizonDays: 14,
+    frozenAt: FROZEN,
+    horizonEnd: "2026-07-15T00:00:00.000Z",
+    truths: [
+      { workUnitId: "o/r#1", outcome: "settled", action: "merge", settledAt: at(3), reversal: { kind: "reversal_reverted", occurredAt: at(8) } },
+      { workUnitId: "o/r#2", outcome: "settled", action: "close", reasonClass: "spam", settledAt: at(6), reversal: null },
+    ],
+    coverage: { workUnits: 2, scoreable: 2, unresolved: 0, unresolvedRate: 0 },
+  });
+});
+
 test("close/label parameters are dropped when the settled action is not the one they belong to", () => {
   // A malformed upstream event carrying a reasonClass on a merge must not leak it into the label.
   const set = deriveBenchmarkGroundTruth({