From a2eb304884da5ba83c3ade1de24ac1c4da58daed Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 02:18:12 -0700 Subject: [PATCH 1/2] feat(retention): bound orb_signals by folding it into per-day rollups orb_signals had no retention rule and grew without bound -- one row per PR per self-host instance, forever. It could not simply be added to RETENTION_POLICY, because two live surfaces read history the raw rows carry: * listFleetInstances reports a LIFETIME signalCount per instance, which a plain prune would silently redefine as "signals in the retention window" with no change of label; * /v1/internal/fleet/analytics takes an arbitrary ?days=, and the public weekly fleet trend (#9676) looks back 8 weeks -- both would report windows they no longer had data for. So this is the #9474 pattern: fold the aging rows into a durable aggregate in the SAME transaction that deletes them. orb_signal_rollups keeps the whole confusion matrix per instance per day rather than a scalar count, because the fleet numbers are not a total: foldInstance derives decisionAccuracy from (verdict, outcome, reversal_flag, gate_reasoncode_bucket) cells, so a scalar rollup would preserve signalCount and destroy every accuracy figure past the window. Per-day cells preserve both -- a historical week reconstructs to the same number it showed while the raw rows existed. Reads union the two sources: computeFleetAnalytics, loadPublicFleetAccuracyTrend and listFleetInstances each add the rollups back in, with the registered-instance trust gate applied at READ time on both halves. The fold deliberately keeps unregistered instances' history, since an instance can be registered after its signals arrive and discarding it at prune time would make that later registration silently lossy. What is genuinely dropped is per-PR identity (repo_hash/pr_hash) -- per-instance HMACs that cannot be reversed or joined across instances, whose only role is the ingest dedup key, which matters while a row can still be re-exported and not after it has aged out. Closes #9783 --- migrations/0202_orb_signal_rollups.sql | 59 +++++++ scripts/check-schema-drift.ts | 4 + src/db/retention.ts | 69 ++++++++ src/orb/analytics.ts | 18 +- src/orb/fleet-admin.ts | 7 +- src/services/public-fleet-accuracy-trend.ts | 17 +- test/unit/orb-analytics.test.ts | 61 +++++++ test/unit/public-fleet-accuracy-trend.test.ts | 77 ++++++++ test/unit/retention.test.ts | 165 ++++++++++++++++++ 9 files changed, 472 insertions(+), 5 deletions(-) create mode 100644 migrations/0202_orb_signal_rollups.sql diff --git a/migrations/0202_orb_signal_rollups.sql b/migrations/0202_orb_signal_rollups.sql new file mode 100644 index 000000000..d795e81cb --- /dev/null +++ b/migrations/0202_orb_signal_rollups.sql @@ -0,0 +1,59 @@ +-- #9783: orb_signals grows without bound. Every self-host instance exports into it hourly, and it is a +-- PUBLIC data source -- computeFleetAnalytics reads it for the /fairness headline, and #9775's weekly fleet +-- trend reads it again. +-- +-- It could not simply be added to RETENTION_POLICY. Two live surfaces depend on history the raw rows carry: +-- fleet-admin.ts's listFleetInstances reports a LIFETIME signalCount per instance, and +-- /v1/internal/fleet/analytics accepts an arbitrary ?days= window. A plain prune would silently turn the +-- first into "signals in the retention window" with no change of label, and make the second report a window +-- it no longer has data for -- the same window/denominator mismatch #9676 and #9793 have been correcting. +-- +-- So this is the #9474 pattern (orb_pr_outcomes -> orb_outcome_rollups), which exists for exactly this +-- shape: fold the aging rows into a durable aggregate in the SAME transaction that deletes them, so the +-- information survives even though the per-PR rows do not. +-- +-- GRANULARITY IS DAILY, and deliberately keeps the whole confusion matrix rather than a bare count. The +-- fleet numbers are not a single total: computeFleetAnalytics folds (verdict, outcome, reversal_flag, +-- gate_reasoncode_bucket) cells through foldInstance to get decisionAccuracy, and #9775's trend buckets the +-- same cells by day. Rolling up to a scalar would preserve signalCount and destroy every accuracy figure +-- beyond the window; rolling up these cells per day preserves BOTH -- a historical week reconstructs to the +-- same numbers it would have shown while the raw rows existed. +-- +-- What is genuinely dropped is per-PR identity: repo_hash/pr_hash. Those are per-instance HMACs that cannot +-- be reversed or joined across instances, so individually they carry no long-term analytical value; their +-- only role is the (instance_id, repo_hash, pr_hash) dedup key, which matters while a row can still be +-- re-exported and not after it has aged out. +CREATE TABLE IF NOT EXISTS orb_signal_rollups ( + instance_id TEXT NOT NULL, + -- UTC day (YYYY-MM-DD) of the folded rows' decision_timestamp, falling back to received_at -- the same + -- COALESCE the weekly trend buckets on, so a folded day lands in the week it would have landed in live. + day TEXT NOT NULL, + -- NOT NULL with an empty-string sentinel, unlike orb_signals where both of these are nullable. A PRIMARY + -- KEY over nullable columns does not work here: SQLite does not enforce NOT NULL on PRIMARY KEY columns of + -- a rowid table, and every NULL compares DISTINCT, so two folds of the same day would silently insert + -- duplicate cells instead of accumulating into one. The fold COALESCEs to '' on the way in. + -- + -- '' is safe for both readers, which is why a sentinel is preferable to dropping the columns: foldInstance + -- only ever tests `verdict === "merge"` / `=== "close"` and `gate_reasoncode_bucket === "policy_action"`, + -- and '' fails those exactly as NULL did -- an unset verdict counted as a hold before and still does. + gate_verdict TEXT NOT NULL DEFAULT '', + outcome TEXT NOT NULL, + reversal_flag TEXT NOT NULL, + gate_reasoncode_bucket TEXT NOT NULL DEFAULT '', + n INTEGER NOT NULL, + updated_at TEXT NOT NULL, + -- One row per cell per instance per day. The prune folds with an upsert that ADDS to `n`, so a day folded + -- across two prune runs (rows arriving late, or a slice boundary splitting a day) accumulates rather than + -- overwriting. + PRIMARY KEY (instance_id, day, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket) +); + +-- The read path: every consumer scans a day range across all instances, then joins to orb_instances to keep +-- unregistered ones out of published numbers. +CREATE INDEX IF NOT EXISTS orb_signal_rollups_day ON orb_signal_rollups (day, instance_id); + +-- The prune scans orb_signals by received_at (both the slice-boundary SELECT and the DELETE predicate), so +-- it needs an index leading with that column or every hourly retention run degrades to a full table scan + +-- sort. 0193 and 0196 (#9472/#9473) did this for every policy table of their era, and the retention suite +-- now enforces it for all of them -- this is simply that same index for the entry added here. +CREATE INDEX IF NOT EXISTS idx_orb_signals_received_at ON orb_signals (received_at); diff --git a/scripts/check-schema-drift.ts b/scripts/check-schema-drift.ts index 9e4fcfeec..259540a7c 100644 --- a/scripts/check-schema-drift.ts +++ b/scripts/check-schema-drift.ts @@ -66,6 +66,10 @@ export const RAW_SQL_ONLY_TABLES: Set = new Set([ "orb_reuse_counters", "orb_risk_control_arms", "orb_signals", + // #9783: per-instance/per-day confusion-matrix cells folded from orb_signals (above) by the retention + // prune, and unioned back in by computeFleetAnalytics, loadPublicFleetAccuracyTrend and + // listFleetInstances -- all via env.DB.prepare, no Drizzle use. Same shape as orb_outcome_rollups. + "orb_signal_rollups", "orb_webhook_events", "override_audit", "predicted_gate_calibration_ledger", diff --git a/src/db/retention.ts b/src/db/retention.ts index ec19f6803..3d70a332e 100644 --- a/src/db/retention.ts +++ b/src/db/retention.ts @@ -22,6 +22,13 @@ export const RETENTION_POLICY: readonly RetentionRule[] = [ // needs would be gone by the time the fold ran, and the rollup would permanently over-count exactly the // PRs the live query never counted. { table: "orb_pr_outcomes", column: "occurred_at", days: 90 }, + // #9783: orb_signals had no rule at all and grew without bound, while being a PUBLIC data source + // (computeFleetAnalytics' /fairness headline, and #9775's weekly fleet trend). It could not simply be + // pruned: listFleetInstances reports a LIFETIME signalCount, and /v1/internal/fleet/analytics takes an + // arbitrary ?days=. Both are preserved by folding into orb_signal_rollups first -- see the special case in + // pruneExpiredRecords, and migrations/0202 for why the rollup keeps the whole confusion matrix per day + // rather than a scalar count. + { table: "orb_signals", column: "received_at", days: 90 }, { table: "audit_events", column: "created_at", days: 90 }, { table: "ai_usage_events", column: "created_at", days: 90 }, { table: "product_usage_events", column: "occurred_at", days: 180 }, @@ -120,6 +127,12 @@ export const RETENTION_POLICY: readonly RetentionRule[] = [ // not scan-optimal, which is acceptable for the lower row counts of the tables that fall back today. export const RETENTION_PK_COLUMN: Readonly> = { audit_events: "id", + // #9783: orb_signals has `id INTEGER PRIMARY KEY` (migrations/0060), so it maps cleanly here rather than + // joining RETENTION_COMPOSITE_PK_TABLES -- its UNIQUE (instance_id, repo_hash, pr_hash) is the ingest + // dedup key, not its primary key. The fold below uses its own bounded-slice path rather than the generic + // PK-batched delete, but the mapping still has to exist: the completeness guard requires every policy + // table to be in one list or the other, precisely so a new entry cannot ship silently unmapped. + orb_signals: "id", ai_usage_events: "id", product_usage_events: "id", github_rate_limit_observations: "id", @@ -257,6 +270,62 @@ export async function pruneExpiredRecords( // is deliberately UNBATCHED for this one table: the aging cohort is one row per fleet-wide PR terminal // per day (hundreds at most, vs the six-figure log tables the batching exists for), and a bounded delete // would reintroduce the split-commit problem for whatever the bound left behind. + // #9783: orb_signals feeds published fleet accuracy AND a lifetime per-instance count, so its rows fold + // into per-instance/per-DAY confusion-matrix cells before deletion. Same atomic fold+delete and same + // bounded inclusive-boundary slicing as orb_pr_outcomes below, for the same two reasons: a fold and + // delete that could commit separately would double- or under-count, and an exclusive boundary against a + // run of tied timestamps selects zero rows and spins forever. + // + // Unlike that one, EVERY expiring row folds -- there is no "the live query never counted these" subset + // to skip. The registered-instance filter belongs at READ time (computeFleetAnalytics and the public + // trend both join orb_instances), not here: an instance can be registered after its signals arrive, and + // discarding an unregistered instance's history at prune time would make that later registration + // silently lossy. + if (rule.table === "orb_signals") { + let deleted = 0; + for (;;) { + const boundaryRow = await env.DB.prepare( + `SELECT ${rule.column} AS boundary FROM ${rule.table} WHERE ${retentionWhere(rule)} ORDER BY ${rule.column} LIMIT 1 OFFSET ${batchSize - 1}`, + ) + .bind(cutoff) + .first<{ boundary: string }>(); + const sliceIsFinal = boundaryRow == null; + const sliceWhere = sliceIsFinal ? `${rule.column} < ?1` : `${rule.column} <= ?1`; + const sliceBound = sliceIsFinal ? cutoff : boundaryRow.boundary; + const batchResults = await env.DB.batch([ + env.DB.prepare( + // Bucketed on COALESCE(decision_timestamp, received_at) -- the same expression the weekly trend + // buckets live rows on, so a folded day lands in the week it would have landed in anyway. + // gate_verdict / gate_reasoncode_bucket COALESCE to '' because the rollup's PRIMARY KEY spans + // them and SQLite compares every NULL as distinct (migrations/0202 has the full note). + `INSERT INTO orb_signal_rollups (instance_id, day, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket, n, updated_at) + SELECT s.instance_id, + substr(COALESCE(s.decision_timestamp, s.received_at), 1, 10) AS day, + COALESCE(s.gate_verdict, '') AS gate_verdict, + s.outcome, + s.reversal_flag, + COALESCE(s.gate_reasoncode_bucket, '') AS gate_reasoncode_bucket, + COUNT(*) AS n, + ?2 AS updated_at + FROM orb_signals s + WHERE s.${sliceWhere} + GROUP BY s.instance_id, day, COALESCE(s.gate_verdict, ''), s.outcome, s.reversal_flag, COALESCE(s.gate_reasoncode_bucket, '') + ON CONFLICT(instance_id, day, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket) DO UPDATE SET + n = orb_signal_rollups.n + excluded.n, + updated_at = excluded.updated_at`, + ).bind(sliceBound, nowIso()), + env.DB.prepare(`DELETE FROM ${rule.table} WHERE ${sliceWhere}`).bind(sliceBound), + ]); + /* v8 ignore next 2 -- defensive: batch() returns one result per statement on both backends; the + * `?.`/`?? 0` arms only satisfy the driver types and degrade the COUNT, never the prune. */ + const changes = Number(batchResults[1]?.meta?.changes ?? 0); + deleted += changes; + if (sliceIsFinal || changes === 0 || deleted >= maxPerTable) break; + } + results.push({ table: rule.table, column: rule.column, cutoff, deleted }); + continue; + } + if (rule.table === "orb_pr_outcomes") { // #9474: this table feeds a CUMULATIVE public counter (getOrbGlobalStats -> the homepage "all-time" // merged/closed totals), so every row must be folded into the durable orb_outcome_rollups totals in the diff --git a/src/orb/analytics.ts b/src/orb/analytics.ts index 9e6fb7961..ec7690710 100644 --- a/src/orb/analytics.ts +++ b/src/orb/analytics.ts @@ -260,11 +260,25 @@ export async function computeFleetAnalytics(env: Env, opts: { windowDays?: numbe let cycleRows: CycleTime[] = []; let registered = new Set(); try { + // #9783: UNION the live rows with the folded rollups. orb_signals is pruned at 90 days into + // orb_signal_rollups (per instance, per day, whole confusion matrix), so reading only the raw table + // would make every window that reaches past the prune silently under-count -- which is exactly the + // failure mode that made adding retention here non-trivial in the first place. Both halves emit the same + // Cell shape, and foldInstance sums cells, so duplicate (instance, verdict, outcome, ...) tuples across + // the two sources add up correctly rather than needing a merge. + // + // Both halves take the same bound because `cutoff` above is already date-only, which is exactly what + // `day` is -- so the boundary date is included on both sides. (The public weekly trend cannot do this: + // it bounds on a full ISO instant, where a bare `day >= ?1` would drop the boundary day because a string + // prefix sorts before the string it prefixes. See public-fleet-accuracy-trend.ts.) const matrix = await env.DB .prepare( `SELECT instance_id, gate_verdict AS verdict, outcome, reversal_flag, gate_reasoncode_bucket, COUNT(*) AS n - FROM orb_signals WHERE received_at >= ? - GROUP BY instance_id, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket`, + FROM orb_signals WHERE received_at >= ?1 + GROUP BY instance_id, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket + UNION ALL + SELECT instance_id, gate_verdict AS verdict, outcome, reversal_flag, gate_reasoncode_bucket, n + FROM orb_signal_rollups WHERE day >= ?1`, ) .bind(cutoff) .all(); diff --git a/src/orb/fleet-admin.ts b/src/orb/fleet-admin.ts index f1d7e8df0..93e7aa992 100644 --- a/src/orb/fleet-admin.ts +++ b/src/orb/fleet-admin.ts @@ -20,7 +20,12 @@ export async function listFleetInstances(env: Env): Promise<{ instances: FleetIn const rows = await env.DB.prepare( `SELECT i.instance_id AS instanceId, i.registered AS registered, i.first_seen_at AS firstSeenAt, i.last_seen_at AS lastSeenAt, i.registered_at AS registeredAt, - (SELECT COUNT(*) FROM orb_signals s WHERE s.instance_id = i.instance_id) AS signalCount + -- #9783: raw rows PLUS folded rollups, so this stays a genuine LIFETIME count after orb_signals + -- starts pruning at 90 days. Reading only the raw table would silently turn it into "signals in + -- the retention window" with no change of label -- one of the two reasons a plain prune could not + -- be added to this table. + ((SELECT COUNT(*) FROM orb_signals s WHERE s.instance_id = i.instance_id) + + COALESCE((SELECT SUM(r.n) FROM orb_signal_rollups r WHERE r.instance_id = i.instance_id), 0)) AS signalCount FROM orb_instances i ORDER BY i.last_seen_at DESC`, ).all<{ instanceId: string; registered: number; firstSeenAt: string; lastSeenAt: string; registeredAt: string | null; signalCount: number }>(); return { instances: (rows.results ?? []).map((row) => ({ ...row, registered: row.registered === 1 })) }; diff --git a/src/services/public-fleet-accuracy-trend.ts b/src/services/public-fleet-accuracy-trend.ts index db9d272d8..ed05de3a4 100644 --- a/src/services/public-fleet-accuracy-trend.ts +++ b/src/services/public-fleet-accuracy-trend.ts @@ -113,12 +113,25 @@ export async function loadPublicFleetAccuracyTrend(env: Env, nowMs: number = Dat // genuine `Cell`, which is what foldInstance consumes, and summing per-instance cells within a week // yields exactly the pooled figure anyway. Narrowing the projection would mean hand-rolling a // near-Cell type and re-deriving the accounting -- the duplication foldInstance is exported to prevent. + // #9783: UNION live rows with the folded rollups -- orb_signals prunes at 90 days into + // orb_signal_rollups, and this series looks back 8 weeks, so a run shortly after a prune would otherwise + // show weeks collapsing to null as their raw rows aged out. The registered-instance join applies to both + // halves: the fold deliberately keeps unregistered instances' history (they can be registered later), so + // the trust gate has to be enforced here at read time. `SELECT substr(COALESCE(s.decision_timestamp, s.received_at), 1, 10) AS day, s.instance_id, s.gate_verdict AS verdict, s.outcome, s.reversal_flag, s.gate_reasoncode_bucket, COUNT(*) AS n FROM orb_signals s JOIN orb_instances i ON i.instance_id = s.instance_id AND i.registered = 1 - WHERE COALESCE(s.decision_timestamp, s.received_at) >= ? - GROUP BY day, s.instance_id, s.gate_verdict, s.outcome, s.reversal_flag, s.gate_reasoncode_bucket`, + WHERE COALESCE(s.decision_timestamp, s.received_at) >= ?1 + GROUP BY day, s.instance_id, s.gate_verdict, s.outcome, s.reversal_flag, s.gate_reasoncode_bucket + UNION ALL + SELECT r.day AS day, r.instance_id, + r.gate_verdict AS verdict, r.outcome, r.reversal_flag, r.gate_reasoncode_bucket, r.n + FROM orb_signal_rollups r + JOIN orb_instances i2 ON i2.instance_id = r.instance_id AND i2.registered = 1 + -- substr, not a bare >=: day is YYYY-MM-DD and ?1 is a full ISO instant, so on the boundary date the + -- shorter string sorts BEFORE the longer one it prefixes and that whole day would be dropped. + WHERE r.day >= substr(?1, 1, 10)`, sinceIso, ); // isoWeekStart is applied here rather than in SQL: SQLite's strftime('%W') is not ISO-8601 week numbering, diff --git a/test/unit/orb-analytics.test.ts b/test/unit/orb-analytics.test.ts index 17e93b2e5..c6a25ec05 100644 --- a/test/unit/orb-analytics.test.ts +++ b/test/unit/orb-analytics.test.ts @@ -609,3 +609,64 @@ describe("fleetFramingEligible (#9168)", () => { expect(a.gamingDetectionEligible).toBe(false); }); }); + +// #9783: orb_signals prunes at 90 days into orb_signal_rollups, so a window that reaches past the prune has +// to read both halves or the headline silently under-counts as history ages out. +describe("computeFleetAnalytics() over folded history (#9783)", () => { + const dayAgo = (n: number) => new Date(Date.now() - n * 86_400_000).toISOString(); + + const foldedCell = async (env: Env, day: string, over: { verdict?: string; outcome?: string; n: number }) => + env.DB + .prepare( + `INSERT INTO orb_signal_rollups (instance_id, day, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket, n, updated_at) + VALUES ('inst', ?, ?, ?, 'none', 'quality', ?, ?)`, + ) + .bind(day.slice(0, 10), over.verdict ?? "merge", over.outcome ?? "merged", over.n, dayAgo(0)) + .run(); + + it("scores a window whose raw rows have already been folded away", async () => { + const env = createTestEnv(); + await env.DB.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES ('inst', 1)").run(); + await foldedCell(env, dayAgo(20), { n: 8 }); + await foldedCell(env, dayAgo(20), { outcome: "closed", n: 2 }); + + const analytics = await computeFleetAnalytics(env, { windowDays: 30 }); + expect(analytics.fleet.decisionAccuracy).toBeCloseTo(0.8, 10); + }); + + it("sums raw and folded halves rather than reading only one", async () => { + const env = createTestEnv(); + await env.DB.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES ('inst', 1)").run(); + for (let i = 0; i < 5; i += 1) { + await env.DB + .prepare( + `INSERT INTO orb_signals (instance_id, repo_hash, pr_hash, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket, decision_timestamp, received_at) + VALUES ('inst', 'r', ?1, 'merge', 'merged', 'none', 'quality', ?2, ?2)`, + ) + .bind(`p${i}`, dayAgo(2)) + .run(); + } + await foldedCell(env, dayAgo(20), { outcome: "closed", n: 5 }); + + // 5 confirmed live + 5 disconfirmed folded. Reading either half alone would report 100% or 0%. + expect((await computeFleetAnalytics(env, { windowDays: 30 })).fleet.decisionAccuracy).toBeCloseTo(0.5, 10); + }); + + it("includes the window's boundary day, and excludes folded days outside it", async () => { + // The two halves are bounded by the same date-only `cutoff`, so the boundary day must land INSIDE the + // window on the folded side exactly as it does on the raw side -- and a day past it must still be out, + // or the rollup would quietly widen every window it is unioned into. + const env = createTestEnv(); + await env.DB.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES ('inst', 1)").run(); + await foldedCell(env, dayAgo(30), { n: 7 }); + await foldedCell(env, dayAgo(45), { n: 99 }); + expect((await computeFleetAnalytics(env, { windowDays: 30 })).fleet.pooled.mergeVerdicts).toBe(7); + }); + + it("INVARIANT: folded history from an UNREGISTERED instance still never reaches the headline", async () => { + const env = createTestEnv(); + await env.DB.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES ('inst', 0)").run(); + await foldedCell(env, dayAgo(20), { outcome: "closed", n: 40 }); + expect((await computeFleetAnalytics(env, { windowDays: 30 })).fleet.decisionAccuracy).toBeNull(); + }); +}); diff --git a/test/unit/public-fleet-accuracy-trend.test.ts b/test/unit/public-fleet-accuracy-trend.test.ts index 08cca008a..9bbfb81f1 100644 --- a/test/unit/public-fleet-accuracy-trend.test.ts +++ b/test/unit/public-fleet-accuracy-trend.test.ts @@ -124,6 +124,9 @@ describe("loadPublicFleetAccuracyTrend — end to end over orb_signals", () => { const thisWeek = `${isoWeekStart(NOW)}T09:00:00.000Z`; + const trendWeek = (trend: Awaited>, weekStart: string) => + trend.find((week) => week.weekStart === weekStart); + it("scores a registered instance's week from real rows", async () => { const env = createTestEnv(); await seed(env, { @@ -195,6 +198,80 @@ describe("loadPublicFleetAccuracyTrend — end to end over orb_signals", () => { for (const week of trend) expect(week).toMatchObject({ verdicts: null, accuracyPct: null }); }); + // #9783: orb_signals now prunes at 90 days, folding into orb_signal_rollups. This series looks back 8 + // weeks, so a week can outlive its raw rows only if the reader unions the rollup back in. + describe("reads folded history (#9783)", () => { + const foldedWeek = isoWeekStart(NOW - 3 * 7 * 86_400_000); + const foldCell = async (env: Env, over: { instance?: string; verdict?: string; outcome?: string; reversal?: string; bucket?: string; n: number }) => { + await env.DB + .prepare( + `INSERT INTO orb_signal_rollups (instance_id, day, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket, n, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, '2026-06-22T00:00:00.000Z')`, + ) + .bind(over.instance ?? "inst-a", foldedWeek, over.verdict ?? "merge", over.outcome ?? "merged", over.reversal ?? "none", over.bucket ?? "quality", over.n) + .run(); + }; + + it("reconstructs a week whose raw rows are gone, to the SAME number they would have shown", async () => { + const env = createTestEnv(); + await env.DB.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES ('inst-a', 1)").run(); + await foldCell(env, { n: 8 }); + await foldCell(env, { outcome: "closed", n: 2 }); + + const trend = await loadPublicFleetAccuracyTrend(env, NOW); + expect(trend.find((w) => w.weekStart === foldedWeek)).toEqual({ weekStart: foldedWeek, verdicts: 10, accuracyPct: 80 }); + }); + + it("sums a week split across raw rows and folded cells rather than picking one side", async () => { + // The boundary week: part of it aged out and folded, part is still live. Both halves must count. + const env = createTestEnv(); + await seed(env, { instance: "inst-a", registered: true, rows: Array.from({ length: 5 }, () => ({ verdict: "merge", outcome: "merged", decidedAt: `${foldedWeek}T09:00:00.000Z` })) }); + await foldCell(env, { outcome: "closed", n: 5 }); + + expect(trendWeek(await loadPublicFleetAccuracyTrend(env, NOW), foldedWeek)).toMatchObject({ verdicts: 10, accuracyPct: 50 }); + }); + + it("INVARIANT: an UNREGISTERED instance's folded history still never moves a published number", async () => { + // The fold deliberately keeps unregistered instances (they may be registered later), so the trust gate + // has to be re-applied at READ time -- otherwise pruning would quietly opt strangers in. + const env = createTestEnv(); + await env.DB.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES ('stranger', 0)").run(); + await foldCell(env, { instance: "stranger", outcome: "closed", n: 40 }); + for (const week of await loadPublicFleetAccuracyTrend(env, NOW)) expect(week).toMatchObject({ verdicts: null, accuracyPct: null }); + }); + + it("REGRESSION: includes the window's FIRST day, which a bare `day >= ?1` string compare drops", async () => { + // day is 'YYYY-MM-DD' and the bound is a full ISO instant. '2026-05-04' < '2026-05-04T00:00:00.000Z' + // as strings -- a prefix sorts before the string it prefixes -- so the oldest week would silently + // vanish the moment its rows were folded. + const env = createTestEnv(); + const oldestWeek = isoWeekStart(NOW - (PUBLIC_FLEET_TREND_WEEKS - 1) * 7 * 86_400_000); + await env.DB.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES ('inst-a', 1)").run(); + await env.DB + .prepare( + `INSERT INTO orb_signal_rollups (instance_id, day, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket, n, updated_at) + VALUES ('inst-a', ?, 'merge', 'merged', 'none', 'quality', 9, '2026-06-22T00:00:00.000Z')`, + ) + .bind(oldestWeek) + .run(); + expect(trendWeek(await loadPublicFleetAccuracyTrend(env, NOW), oldestWeek)).toMatchObject({ verdicts: 9 }); + }); + + it("scores folded holds and policy_action cells exactly as the live path does", async () => { + // The fold keeps these cells rather than dropping them, and '' / the real bucket value must go on + // meaning the same thing to foldInstance after the round trip. + const env = createTestEnv(); + await env.DB.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES ('inst-a', 1)").run(); + await foldCell(env, { n: 5 }); + await foldCell(env, { verdict: "hold", n: 95 }); + await foldCell(env, { verdict: "close", outcome: "closed", bucket: "policy_action", n: 50 }); + // Empty-string verdict: the fold's COALESCE sentinel for a NULL gate_verdict, which foldInstance must + // treat as a hold (neither 'merge' nor 'close') exactly as NULL was treated. + await foldCell(env, { verdict: "", n: 30 }); + expect(trendWeek(await loadPublicFleetAccuracyTrend(env, NOW), foldedWeek)).toMatchObject({ verdicts: 5, accuracyPct: 100 }); + }); + }); + it("degrades to all-null weeks (never throws) when the read fails", async () => { const broken = createTestEnv(); broken.DB = { prepare: () => { throw new Error("boom"); } } as never; diff --git a/test/unit/retention.test.ts b/test/unit/retention.test.ts index 941d6cccf..682810694 100644 --- a/test/unit/retention.test.ts +++ b/test/unit/retention.test.ts @@ -2,6 +2,8 @@ import { describe, expect, it } from "vitest"; import { createApp } from "../../src/api/routes"; import { getDb } from "../../src/db/client"; import { dedupeSignalSnapshots, pruneExpiredRecords, RETENTION_COMPOSITE_PK_TABLES, RETENTION_PK_COLUMN, RETENTION_POLICY, retentionCutoffIsoForTable } from "../../src/db/retention"; +import { computeFleetAnalytics } from "../../src/orb/analytics"; +import { listFleetInstances } from "../../src/orb/fleet-admin"; import { getOrbGlobalStats } from "../../src/orb/outcomes"; import { agentContextSnapshots, aiReviewCache, aiSlopCache, aiUsageEvents, groundingFileContentCache, linkedIssueSatisfactionCache, webhookEvents } from "../../src/db/schema"; import { processJob, runRetentionPrune } from "../../src/queue/processors"; @@ -975,3 +977,166 @@ describe("retentionCutoffIsoForTable (#9474)", () => { expect(Math.abs(Date.parse(cutoff!) - (Date.now() - 180 * 86_400_000))).toBeLessThan(1000); }); }); + +// #9783: orb_signals is a PUBLIC data source (computeFleetAnalytics' /fairness headline, #9775's weekly fleet +// trend) AND carries a lifetime per-instance count, so it could not simply be pruned. These pin the property +// that made a rule addable at all: the published numbers must be IDENTICAL either side of the prune. +describe("orb_signals fold-before-delete (#9783)", () => { + const RULE = { table: "orb_signals", column: "received_at", days: 90 } as (typeof RETENTION_POLICY)[number]; + + const seedSignals = async (env: Env) => { + await env.DB.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES ('reg', 1), ('stranger', 0)").run(); + const rows: Array<[string, string, string, string, string, string]> = [ + // instance, pr, verdict, outcome, reversal, when + ["reg", "p1", "merge", "merged", "none", daysAgo(100)], + ["reg", "p2", "merge", "merged", "none", daysAgo(100)], + ["reg", "p3", "merge", "closed", "none", daysAgo(100)], + ["reg", "p4", "close", "closed", "reopened", daysAgo(100)], + ["reg", "p5", "hold", "merged", "none", daysAgo(100)], + // an unregistered instance's history still folds -- it may be registered later + ["stranger", "p6", "merge", "merged", "none", daysAgo(100)], + // inside the window, must survive untouched + ["reg", "p7", "merge", "merged", "none", daysAgo(1)], + ]; + for (const [instance, pr, verdict, outcome, reversal, at] of rows) { + await env.DB.prepare( + `INSERT INTO orb_signals (instance_id, repo_hash, pr_hash, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket, decision_timestamp, received_at) + VALUES (?, 'r', ?, ?, ?, ?, 'quality', ?, ?)`, + ) + .bind(instance, pr, verdict, outcome, reversal, at, at) + .run(); + } + }; + + const bulkSignals = async (env: Env, rows: Array<{ pr: string; at: string; outcome?: string }>) => { + await env.DB.prepare("INSERT OR IGNORE INTO orb_instances (instance_id, registered) VALUES ('reg', 1)").run(); + const values = rows + .map((r) => `('reg', 'r', '${r.pr}', 'merge', '${r.outcome ?? "merged"}', 'none', 'quality', '${r.at}', '${r.at}')`) + .join(","); + await env.DB.prepare( + `INSERT INTO orb_signals (instance_id, repo_hash, pr_hash, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket, decision_timestamp, received_at) VALUES ${values}`, + ).run(); + }; + const rollupTotal = async (env: Env) => + (await env.DB.prepare("SELECT COALESCE(SUM(n), 0) AS n FROM orb_signal_rollups").first<{ n: number }>())?.n; + const rawCount = async (env: Env) => + (await env.DB.prepare("SELECT COUNT(*) AS n FROM orb_signals").first<{ n: number }>())?.n; + + it("REGRESSION: deletes in BOUNDED slices, not one unbatched statement -- and every row still folds exactly once", async () => { + const env = createTestEnv(); + await bulkSignals(env, Array.from({ length: 25 }, (_, i) => ({ pr: `p${i}`, at: daysAgo(200 - i) }))); + + const results = await pruneExpiredRecords(env, { nowMs: NOW, policy: [RULE], batchSize: 10 }); + + expect(results[0]?.deleted).toBe(25); // every aged row removed, across multiple bounded slices + expect(await rawCount(env)).toBe(0); + expect(await rollupTotal(env)).toBe(25); // folded exactly once each -- no slice double-counted + }); + + it("INVARIANT: a run of rows sharing ONE timestamp still makes progress -- the inclusive slice boundary cannot spin", async () => { + // An exclusive `<` boundary against a tie run selects zero rows and loops forever. All 12 rows here share + // one received_at, so the first slice's boundary IS that timestamp: inclusive is what guarantees progress. + const env = createTestEnv(); + const tied = daysAgo(150); + await bulkSignals(env, Array.from({ length: 12 }, (_, i) => ({ pr: `p${i}`, at: tied }))); + + const results = await pruneExpiredRecords(env, { nowMs: NOW, policy: [RULE], batchSize: 5 }); + + expect(results[0]?.deleted).toBe(12); + expect(await rawCount(env)).toBe(0); + expect(await rollupTotal(env)).toBe(12); // one cell of n=12, folded once + }); + + it("INVARIANT: maxPerTable caps one run, and the fleet number is exact at every intermediate point", async () => { + const env = createTestEnv(); + await bulkSignals(env, Array.from({ length: 20 }, (_, i) => ({ pr: `p${i}`, at: daysAgo(200 - i) }))); + const before = await computeFleetAnalytics(env, { windowDays: 365 }); + + const first = await pruneExpiredRecords(env, { nowMs: NOW, policy: [RULE], batchSize: 5, maxPerTable: 10 }); + expect(first[0]?.deleted).toBe(10); + expect(await rawCount(env)).toBe(10); + // Half folded, half still raw: the unioned read has to be exact HERE, not only once the cohort drains. + expect((await computeFleetAnalytics(env, { windowDays: 365 })).fleet.pooled.mergeVerdicts).toBe(20); + expect((await computeFleetAnalytics(env, { windowDays: 365 })).fleet.decisionAccuracy).toBe(before.fleet.decisionAccuracy); + + const second = await pruneExpiredRecords(env, { nowMs: NOW, policy: [RULE], batchSize: 5, maxPerTable: 10 }); + expect(second[0]?.deleted).toBe(10); + expect(await rawCount(env)).toBe(0); + expect(await rollupTotal(env)).toBe(20); + expect((await computeFleetAnalytics(env, { windowDays: 365 })).fleet.decisionAccuracy).toBe(before.fleet.decisionAccuracy); + }); + + it("INVARIANT: dryRun neither folds nor deletes", async () => { + const env = createTestEnv(); + await seedSignals(env); + await pruneExpiredRecords(env, { nowMs: NOW, policy: [RULE], dryRun: true }); + expect(await rawCount(env)).toBe(7); + expect(await rollupTotal(env)).toBe(0); + }); + + it("REGRESSION: fleet accuracy is IDENTICAL before and after the prune, and the aged raw rows are gone", async () => { + const env = createTestEnv(); + await seedSignals(env); + const before = await computeFleetAnalytics(env, { windowDays: 365 }); + // 4 scored verdicts among the aged rows (p5's hold is excluded) + 1 recent = 5; p1/p2/p7 confirmed, + // p3 merged-verdict-that-closed and p4 close-that-reopened disconfirmed. + expect(before.fleet.decisionAccuracy).toBeCloseTo(3 / 5, 10); + + const results = await pruneExpiredRecords(env, { nowMs: NOW, policy: [RULE] }); + + expect(results[0]?.deleted).toBe(6); // every aged row, including the unregistered instance's + const remaining = await env.DB.prepare("SELECT pr_hash FROM orb_signals").all<{ pr_hash: string }>(); + expect(remaining.results.map((row) => row.pr_hash)).toEqual(["p7"]); + // The whole point: the published number did not move. + const after = await computeFleetAnalytics(env, { windowDays: 365 }); + expect(after.fleet.decisionAccuracy).toBe(before.fleet.decisionAccuracy); + }); + + it("preserves the confusion matrix per day, not just a count -- a scalar rollup would destroy accuracy", async () => { + const env = createTestEnv(); + await seedSignals(env); + await pruneExpiredRecords(env, { nowMs: NOW, policy: [RULE] }); + + const cells = await env.DB + .prepare("SELECT instance_id, day, gate_verdict, outcome, reversal_flag, n FROM orb_signal_rollups ORDER BY instance_id, gate_verdict, outcome, reversal_flag") + .all<{ instance_id: string; day: string; gate_verdict: string; outcome: string; reversal_flag: string; n: number }>(); + const rows = cells.results ?? []; + // Distinct cells survive as distinct rows; only the two identical merge/merged ones coalesce. + expect(rows.filter((r) => r.instance_id === "reg")).toHaveLength(4); + expect(rows.find((r) => r.gate_verdict === "merge" && r.outcome === "merged" && r.instance_id === "reg")?.n).toBe(2); + expect(rows.find((r) => r.gate_verdict === "close")?.reversal_flag).toBe("reopened"); + expect(rows.every((r) => /^\d{4}-\d{2}-\d{2}$/.test(r.day))).toBe(true); + // The unregistered instance's history is kept, so registering it later is not silently lossy. + expect(rows.some((r) => r.instance_id === "stranger")).toBe(true); + }); + + it("keeps signalCount a genuine LIFETIME count across the prune", async () => { + const env = createTestEnv(); + await seedSignals(env); + const before = (await listFleetInstances(env)).instances.find((i) => i.instanceId === "reg")?.signalCount; + expect(before).toBe(6); + + await pruneExpiredRecords(env, { nowMs: NOW, policy: [RULE] }); + + expect((await listFleetInstances(env)).instances.find((i) => i.instanceId === "reg")?.signalCount).toBe(before); + }); + + it("accumulates rather than overwriting when the same day folds across two prune runs", async () => { + // A day can be split by a slice boundary, or land in two runs when rows arrive late. `n` must ADD. + const env = createTestEnv(); + await seedSignals(env); + await pruneExpiredRecords(env, { nowMs: NOW, policy: [RULE] }); + await env.DB.prepare( + `INSERT INTO orb_signals (instance_id, repo_hash, pr_hash, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket, decision_timestamp, received_at) + VALUES ('reg', 'r', 'late1', 'merge', 'merged', 'none', 'quality', ?1, ?1)`, + ) + .bind(daysAgo(100)) + .run(); + await pruneExpiredRecords(env, { nowMs: NOW, policy: [RULE] }); + + const cell = await env.DB + .prepare("SELECT n FROM orb_signal_rollups WHERE instance_id = 'reg' AND gate_verdict = 'merge' AND outcome = 'merged' AND reversal_flag = 'none'") + .first<{ n: number }>(); + expect(cell?.n).toBe(3); // 2 from the first run + 1 from the second, not overwritten to 1 + }); +}); From efb886ac361117d4913f3c428f9e312766b86a47 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 03:36:35 -0700 Subject: [PATCH 2/2] fix(orb): null cycle-time percentiles a folded window can no longer support Review catch on the orb_signals rollup: the confusion matrix folds into per-day cells and sums back exactly, but cycleP50Ms/cycleP95Ms are percentiles over individual time_to_close_ms values. Percentiles are not summable, so no per-day count can reconstruct them -- and orb_signal_rollups deliberately stores counts rather than a sample of durations, since an unbounded sample is the growth problem retention exists to solve. So computeFleetAnalytics' cycle query, left reading raw orb_signals, would compute percentiles from only the rows that survived the prune and report them as describing the whole requested window. windowDays is clamped to 365 while orb_signals now prunes at 90, so ?days= beyond 90 silently narrows -- the exact failure this change prevents for the matrix. The percentiles now go null past the retention horizon, with cycleTimeObservable saying why: the posture gamingDetectionEligible and fleetFramingEligible already established, where a surface labels a gap instead of being handed a number that quietly means something narrower than its window. The horizon is read from RETENTION_POLICY rather than restated, so the window this module trusts cannot drift from the one the prune enforces. The default 90-day window -- the public headline -- is unaffected: it equals the retention window, so cycle time stays fully observable there. Existing consumers (federated-bundle/federated-import) already type these as number | null. --- src/orb/analytics.ts | 41 +++++++++++++++++-- test/unit/orb-analytics.test.ts | 70 +++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 3 deletions(-) diff --git a/src/orb/analytics.ts b/src/orb/analytics.ts index ec7690710..7ec3df5a2 100644 --- a/src/orb/analytics.ts +++ b/src/orb/analytics.ts @@ -29,6 +29,11 @@ // Exported so the federated bundle export (#1970, src/orb/federated-bundle.ts) gates its own published // precision on the SAME volume bar the fleet median uses — a bundle must not advertise a precision the fleet // would refuse to count. +// #9783: the retention horizon is imported rather than restated, so the window this module trusts for cycle +// time cannot drift from the window the prune actually enforces. retention.ts imports only a JSON util, so +// this introduces no cycle. +import { retentionCutoffIsoForTable } from "../db/retention"; + export const MIN_DECIDED = 5; // an instance needs at least this many decided PRs to count toward the fleet median const OUTLIER_BAND = 0.25; // |instance precision − fleet median| beyond this flags the instance const GAMING_VOLUME_MULTIPLIER = 2; // an instance's decided count more than this many times the fleet median @@ -162,6 +167,11 @@ export interface FleetAnalytics { * public surface can label them honestly instead of letting fleet framing imply corroboration that a * single-instance sample cannot provide. */ fleetFramingEligible: boolean; + /** #9783: whether cycleP50Ms/cycleP95Ms cover the REQUESTED window. False when the window reaches past + * orb_signals' retention horizon: the folded rollups carry counts, not durations, so percentiles over the + * surviving rows would describe a shorter window than the caller asked for. The percentiles are null in + * that case rather than quietly narrowed -- this says which it is. */ + cycleTimeObservable: boolean; } function median(xs: number[]): number | null { @@ -254,7 +264,27 @@ export async function computeFleetAnalytics(env: Env, opts: { windowDays?: numbe const windowDays = Number.isFinite(opts.windowDays) && (opts.windowDays as number) > 0 ? Math.min(opts.windowDays as number, 365) : 90; // Date-only cutoff (like computeGateEval) so it compares correctly whether received_at is ISO ('…T…Z') // or SQLite's CURRENT_TIMESTAMP space format ('YYYY-MM-DD HH:MM:SS'). - const cutoff = new Date(Date.now() - windowDays * 86_400_000).toISOString().slice(0, 10); + const nowMs = Date.now(); + const cutoff = new Date(nowMs - windowDays * 86_400_000).toISOString().slice(0, 10); + + // #9783 follow-up: cycle time is the ONE metric the rollup cannot carry. The confusion matrix folds into + // per-day cells and sums back exactly, but cycleP50Ms/cycleP95Ms are percentiles over individual + // time_to_close_ms values -- percentiles are not summable, so no per-day count can reconstruct them, and + // orb_signal_rollups deliberately stores counts rather than a sample of durations (an unbounded sample is + // the growth problem retention exists to solve). + // + // So a window reaching past the retention horizon would compute percentiles over only the rows that + // survived, and report them as if they described the whole window -- silently, exactly the failure this + // change exists to prevent for the matrix. Instead the percentiles go null and this flag says why, the + // same posture gamingDetectionEligible/fleetFramingEligible take: a public surface can label the gap + // rather than be handed a number that quietly means something narrower than its window. + // + // Compared as instants, before the date-truncation above: at the default windowDays === the retention + // window the two are equal and cycle time is fully observable. (The date-only cutoff then widens the query + // by under a day at the oldest edge, where the prune may already have taken rows -- a sub-day sliver out + // of ninety, which percentiles are robust to and which predates this change.) + const retentionCutoffIso = retentionCutoffIsoForTable("orb_signals", nowMs); + const cycleTimeObservable = retentionCutoffIso === null || nowMs - windowDays * 86_400_000 >= Date.parse(retentionCutoffIso); let cells: Cell[] = []; let cycleRows: CycleTime[] = []; @@ -311,6 +341,10 @@ export async function computeFleetAnalytics(env: Env, opts: { windowDays?: numbe // claim a fleet aggregate. instanceCount is 0 here, so the public surface labels it a self-report and // publishes the nulls it already had. fleetFramingEligible: false, + // The cycle percentiles here are null because there is no data at all, not because the window outran + // retention -- so this reports the window's real observability rather than piling a second reason onto + // the same nulls. + cycleTimeObservable, }; } @@ -405,8 +439,8 @@ export async function computeFleetAnalytics(env: Env, opts: { windowDays?: numbe reversalRate: median(nums((i) => i.reversalRate)), decisionAccuracy: pooledDecisionAccuracy, decisionAccuracyMedian: median(nums((i) => i.decisionAccuracy)), - cycleP50Ms: percentile(cycle, 50), - cycleP95Ms: percentile(cycle, 95), + cycleP50Ms: cycleTimeObservable ? percentile(cycle, 50) : null, + cycleP95Ms: cycleTimeObservable ? percentile(cycle, 95) : null, pooled, }, instances, @@ -414,6 +448,7 @@ export async function computeFleetAnalytics(env: Env, opts: { windowDays?: numbe gamingPatternFlags, gamingDetectionEligible, fleetFramingEligible: eligible.length >= FLEET_FRAMING_MIN_INSTANCES, + cycleTimeObservable, }; } diff --git a/test/unit/orb-analytics.test.ts b/test/unit/orb-analytics.test.ts index c6a25ec05..d5913b6fa 100644 --- a/test/unit/orb-analytics.test.ts +++ b/test/unit/orb-analytics.test.ts @@ -670,3 +670,73 @@ describe("computeFleetAnalytics() over folded history (#9783)", () => { expect((await computeFleetAnalytics(env, { windowDays: 30 })).fleet.decisionAccuracy).toBeNull(); }); }); + +// #9783 review follow-up: the confusion matrix folds into per-day cells and sums back exactly, but cycle time +// is percentiles over individual time_to_close_ms values -- not summable, and deliberately not stored in the +// rollup. So a window reaching past the retention horizon must not quietly report percentiles computed from +// only the surviving rows. +describe("cycle-time observability across the retention horizon (#9783)", () => { + const dayAgo = (n: number) => new Date(Date.now() - n * 86_400_000).toISOString(); + + const seedCycle = async (env: Env) => { + await env.DB.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES ('inst', 1)").run(); + for (let i = 0; i < 6; i += 1) { + await env.DB + .prepare( + `INSERT INTO orb_signals (instance_id, repo_hash, pr_hash, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket, decision_timestamp, received_at, time_to_close_ms) + VALUES ('inst', 'r', ?1, 'merge', 'merged', 'none', 'quality', ?2, ?2, ?3)`, + ) + .bind(`p${i}`, dayAgo(2), (i + 1) * 1000) + .run(); + } + }; + + it("publishes percentiles for the default window, which the retention window exactly covers", async () => { + const env = createTestEnv(); + await seedCycle(env); + const analytics = await computeFleetAnalytics(env, { windowDays: 90 }); + expect(analytics.cycleTimeObservable).toBe(true); + expect(analytics.fleet.cycleP50Ms).not.toBeNull(); + expect(analytics.fleet.cycleP95Ms).not.toBeNull(); + }); + + it("REGRESSION: nulls the percentiles for a window that outruns retention, instead of silently narrowing them", async () => { + // The exact reported scenario: ?days=365 against a table pruned at 90. Reading only the survivors would + // return a real-looking p50/p95 describing 90 days while the caller asked for 365. + const env = createTestEnv(); + await seedCycle(env); + const analytics = await computeFleetAnalytics(env, { windowDays: 365 }); + expect(analytics.cycleTimeObservable).toBe(false); + expect(analytics.fleet.cycleP50Ms).toBeNull(); + expect(analytics.fleet.cycleP95Ms).toBeNull(); + }); + + it("still folds the confusion matrix over that same long window -- only cycle time is affected", async () => { + // The distinction that makes nulling cycle time acceptable: accuracy IS reconstructable from the rollup, + // so a 365-day window keeps a real decisionAccuracy while cycle time goes null. + const env = createTestEnv(); + await env.DB.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES ('inst', 1)").run(); + await env.DB + .prepare( + `INSERT INTO orb_signal_rollups (instance_id, day, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket, n, updated_at) + VALUES ('inst', ?, 'merge', 'merged', 'none', 'quality', 8, ?)`, + ) + .bind(dayAgo(200).slice(0, 10), dayAgo(0)) + .run(); + + const analytics = await computeFleetAnalytics(env, { windowDays: 365 }); + expect(analytics.fleet.decisionAccuracy).toBeCloseTo(1, 10); + expect(analytics.cycleTimeObservable).toBe(false); + expect(analytics.fleet.cycleP50Ms).toBeNull(); + }); + + it("reports observability honestly even on the no-data fallback", async () => { + const broken = createTestEnv(); + broken.DB = { prepare: () => { throw new Error("boom"); } } as never; + const analytics = await computeFleetAnalytics(broken, { windowDays: 365 }); + // Null because there is no data AND because the window outran retention -- the flag says which window + // was asked for, rather than implying the nulls are only about missing data. + expect(analytics.cycleTimeObservable).toBe(false); + expect(analytics.fleet.cycleP50Ms).toBeNull(); + }); +});