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..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,17 +264,51 @@ 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[] = []; 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(); @@ -297,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, }; } @@ -391,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, @@ -400,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/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..d5913b6fa 100644 --- a/test/unit/orb-analytics.test.ts +++ b/test/unit/orb-analytics.test.ts @@ -609,3 +609,134 @@ 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(); + }); +}); + +// #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(); + }); +}); 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 + }); +});