From ad1033716254632bdd125726700683251cd01c4f Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 26 Jul 2026 03:19:06 -0700 Subject: [PATCH] fix(orb): score enforcement closes as their own class, not quality mispredictions A policy close (contributor cap, blacklist, copycat, review-nag, screenshot-table, linked-issue hard rule) carries no gate blocker, so blockerClass was 'none' and the recorded reason fell back to the gate's own conclusion -- writing 'success' on a PR the bot deliberately closed. Calibration then scored every one as a quality misprediction ('said merge, ended closed') when the gate never made a quality claim at all. On the live fleet all 210 rows in that class carried the bare summary 'success', indistinguishable from a real verdict. Closes #8825. An enforcement close is a deliberate decision, not an error, and can be neither confirmed nor disconfirmed as a quality prediction -- so it is excluded from precision/accuracy scoring entirely rather than counted on either side, and reported separately as policyActions so the volume stays visible. - processors: name the closeKind in the recorded reason (policy_close:) instead of falling back to the conclusion - orb-collector: bucket that prefix as policy_action, checked before the substring rules so linked-issue-hard-rule doesn't flatten into issue_policy - analytics: carry the bucket through the confusion matrix; exclude policy_action cells from decisionAccuracy/precision while still counting them in decided and reversalRate Rows exported before the bucket existed carry null and keep scoring as ordinary quality verdicts, so historical data is unchanged. --- src/orb/analytics.ts | 23 +++++++++++++++--- src/queue/processors.ts | 17 +++++++++++++- src/selfhost/orb-collector.ts | 5 ++++ test/unit/orb-analytics.test.ts | 30 ++++++++++++++++++++---- test/unit/selfhost-orb-collector.test.ts | 5 ++++ 5 files changed, 72 insertions(+), 8 deletions(-) diff --git a/src/orb/analytics.ts b/src/orb/analytics.ts index 7acfb90f1a..cb5568e9d8 100644 --- a/src/orb/analytics.ts +++ b/src/orb/analytics.ts @@ -41,6 +41,10 @@ export interface Cell { verdict: string | null; outcome: string; reversal_flag: string; + /** #8825: `policy_action` marks a deliberate enforcement close (contributor cap, blacklist, copycat, + * review-nag, screenshot-table, linked-issue hard rule) rather than a claim about code quality. Null on + * rows exported before the bucket existed — treated as a normal quality verdict, matching prior behavior. */ + gate_reasoncode_bucket?: string | null; n: number; } @@ -68,6 +72,10 @@ export interface InstanceMetrics { * Measured on the live fleet the two differ by ~6 points (93.6% vs 99.6%) — the gap is real errors, not * rounding. null when the instance made no merge/close verdicts at all (holds only). */ decisionAccuracy: number | null; + /** #8825: enforcement closes excluded from the precision/accuracy scoring above (contributor cap, blacklist, + * copycat, review-nag, screenshot-table, linked-issue hard rule). Reported so the volume of policy actions + * stays visible instead of vanishing from every metric. */ + policyActions: number; } /** #2350: one self-hosted instance whose combined volume/precision/reversal-rate pattern looks like it is @@ -133,10 +141,18 @@ export function percentile(sorted: number[], p: number): number | null { export function foldInstance(instanceId: string, cells: Cell[]): InstanceMetrics { let wouldMerge = 0, mergeConfirmed = 0, mergeFalse = 0; let wouldClose = 0, closeConfirmed = 0, closeFalse = 0; - let reversals = 0, decided = 0; + let reversals = 0, decided = 0, policyActions = 0; for (const c of cells) { decided += c.n; if (c.reversal_flag !== "none") reversals += c.n; + // #8825: a deliberate enforcement close is not a quality prediction and can be neither confirmed nor + // disconfirmed as one, so it is excluded from the precision/accuracy scoring entirely — counted on its own + // (policyActions) rather than silently inflating either side. It still contributes to `decided` and + // reversalRate, which measure activity and human overrides rather than gate correctness. + if (c.gate_reasoncode_bucket === "policy_action") { + policyActions += c.n; + continue; + } if (c.verdict === "merge") { wouldMerge += c.n; if (c.outcome === "merged" && c.reversal_flag !== "reverted") mergeConfirmed += c.n; @@ -157,6 +173,7 @@ export function foldInstance(instanceId: string, cells: Cell[]): InstanceMetrics fnRate: wouldClose > 0 ? closeFalse / wouldClose : null, reversalRate: reversals / decided, // decided ≥ 1 (the instance has at least one cell) decisionAccuracy: verdicts > 0 ? (mergeConfirmed + closeConfirmed) / verdicts : null, + policyActions, }; } @@ -173,9 +190,9 @@ export async function computeFleetAnalytics(env: Env, opts: { windowDays?: numbe try { const matrix = await env.DB .prepare( - `SELECT instance_id, gate_verdict AS verdict, outcome, reversal_flag, COUNT(*) AS n + `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`, + GROUP BY instance_id, gate_verdict, outcome, reversal_flag, gate_reasoncode_bucket`, ) .bind(cutoff) .all(); diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 455b5e5aa4..2b5be672a6 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -3275,13 +3275,28 @@ async function runAgentMaintenancePlanAndExecute( // downstream autonomous disposition (merge/close/hold), not only the gate check conclusion. In particular, a // failing gate can still become a concrete auto-close after CI/conflict/duplicate/linked-issue planning; recording // only the gate's hold-shaped conclusion would blind the close-precision breaker to those live closes. + // #8825: a POLICY close (contributor cap, blacklist, copycat, review-nag, screenshot-table, linked-issue + // hard rule) carries no gate blocker, so blockerClass is "none" and the reason used to fall back to the + // gate's own conclusion -- recording "success" on a PR the bot deliberately closed. Every one of those rows + // then scored as a quality misprediction ("said merge, ended closed") when the gate never made a quality + // claim at all; measured on the live fleet, all 210 rows in that class carried the bare summary "success". + // Naming the closeKind makes the policy action distinguishable from a quality verdict at scoring time. + const policyCloseKind = + disposition.actionClass === "close" + ? breakerOnPlan.find((planned) => planned.actionClass === "close" && planned.closeKind !== undefined)?.closeKind + : undefined; await recordNativeGateDecision(env, { project: repoFullName, pullNumber: pr.number, headSha: pr.headSha, conclusion: gate.conclusion, action: disposition.actionClass, - reasonCode: disposition.blockerClass === "none" ? gate.conclusion : disposition.blockerClass, + reasonCode: + disposition.blockerClass !== "none" + ? disposition.blockerClass + : policyCloseKind !== undefined + ? `policy_close:${policyCloseKind}` + : gate.conclusion, // #2352: this row is the ACTUAL autonomous disposition that the precision breaker evaluates, so preserve // the same miner-authored scope as the gate-check audit row below. Omitting it defaults to non-miner and can // erase a prior miner-authored prediction for the same head. diff --git a/src/selfhost/orb-collector.ts b/src/selfhost/orb-collector.ts index a3bd194282..b178d5cbd1 100644 --- a/src/selfhost/orb-collector.ts +++ b/src/selfhost/orb-collector.ts @@ -138,6 +138,11 @@ export async function getOrCreateAnonSecret(db: D1Database): Promise { export function bucketReasonCode(summary: string | null | undefined): string { if (!summary) return "none"; const s = summary.toLowerCase(); + // #8825: a POLICY close (contributor cap, blacklist, copycat, review-nag, screenshot-table, linked-issue + // hard rule) is a deliberate enforcement action, NOT a claim about code quality — scoring it as a quality + // prediction distorts precision in both directions. Checked FIRST so the prefix wins over the substring + // rules below (e.g. `policy_close:linked-issue-hard-rule` must not bucket as plain issue_policy). + if (s.startsWith("policy_close:")) return "policy_action"; if (s.includes("linked_issue") || s.includes("linked issue")) return "issue_policy"; if (s.includes("duplicate")) return "duplicate_risk"; if (s.includes("slop")) return "slop_advisory"; diff --git a/test/unit/orb-analytics.test.ts b/test/unit/orb-analytics.test.ts index 560a3d58a3..e434bd9a0e 100644 --- a/test/unit/orb-analytics.test.ts +++ b/test/unit/orb-analytics.test.ts @@ -8,15 +8,15 @@ async function signals( env: Env, instance: string, n: number, - o: { verdict?: string | null; outcome?: string; reversal?: string; ms?: number | null } = {}, + o: { verdict?: string | null; outcome?: string; reversal?: string; ms?: number | null; bucket?: string | null } = {}, ): Promise { for (let i = 0; i < n; i++) { await env.DB .prepare( - `INSERT INTO orb_signals (instance_id, repo_hash, pr_hash, gate_verdict, outcome, reversal_flag, time_to_close_ms) - VALUES (?, ?, ?, ?, ?, ?, ?)`, + `INSERT INTO orb_signals (instance_id, repo_hash, pr_hash, gate_verdict, outcome, reversal_flag, time_to_close_ms, gate_reasoncode_bucket) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, ) - .bind(instance, `repo${seq}`, `pr${seq++}`, o.verdict ?? "merge", o.outcome ?? "merged", o.reversal ?? "none", o.ms ?? null) + .bind(instance, `repo${seq}`, `pr${seq++}`, o.verdict ?? "merge", o.outcome ?? "merged", o.reversal ?? "none", o.ms ?? null, o.bucket ?? null) .run(); } } @@ -103,6 +103,28 @@ describe("computeFleetAnalytics()", () => { expect(1 - inst.reversalRate).toBe(1); }); + it("#8825: a POLICY close is excluded from accuracy scoring — it is enforcement, not a quality prediction", async () => { + const env = createTestEnv(); + await signals(env, "i", 8, { verdict: "close", outcome: "closed" }); // real quality closes, all confirmed + await signals(env, "i", 2, { verdict: "close", outcome: "merged" }); // real quality misses + // Enforcement closes: the gate made no quality claim, so they must not count either way. + await signals(env, "i", 40, { verdict: "close", outcome: "closed", bucket: "policy_action" }); + const inst = (await computeFleetAnalytics(env)).instances[0]!; + expect(inst.decisionAccuracy).toBeCloseTo(8 / 10); // NOT 48/50 — the 40 enforcement closes are excluded + expect(inst.closePrecision).toBeCloseTo(8 / 10); + expect(inst.policyActions).toBe(40); + expect(inst.decided).toBe(50); // still counted as activity + }); + + it("#8825: rows exported BEFORE the bucket existed (null) still score as ordinary quality verdicts", async () => { + const env = createTestEnv(); + await signals(env, "i", 4, { verdict: "close", outcome: "closed", bucket: null }); + await signals(env, "i", 1, { verdict: "close", outcome: "merged", bucket: "other" }); + const inst = (await computeFleetAnalytics(env)).instances[0]!; + expect(inst.decisionAccuracy).toBeCloseTo(4 / 5); + expect(inst.policyActions).toBe(0); + }); + it("decisionAccuracy is null for a holds-only instance (no decision to score) and drives the fleet median", async () => { const env = createTestEnv(); await signals(env, "holds-only", 6, { verdict: "hold", outcome: "merged" }); diff --git a/test/unit/selfhost-orb-collector.test.ts b/test/unit/selfhost-orb-collector.test.ts index a9da84bbb3..13745187bc 100644 --- a/test/unit/selfhost-orb-collector.test.ts +++ b/test/unit/selfhost-orb-collector.test.ts @@ -59,6 +59,11 @@ describe("bucketReasonCode()", () => { expect(bucketReasonCode("self_authored_with_maintainer_cut")).toBe("author_policy"); expect(bucketReasonCode("ci_state failing")).toBe("ci_readiness"); expect(bucketReasonCode("something_unmapped")).toBe("other"); + // #8825: an enforcement close buckets as policy_action, and the prefix wins over the substring rules + // below it -- `policy_close:linked-issue-hard-rule` must NOT flatten into plain issue_policy. + expect(bucketReasonCode("policy_close:contributor_cap")).toBe("policy_action"); + expect(bucketReasonCode("policy_close:linked-issue-hard-rule")).toBe("policy_action"); + expect(bucketReasonCode("policy_close:blacklist")).toBe("policy_action"); }); });