Skip to content

Commit 67ac8dc

Browse files
committed
feat(review): detect successor-superseded reversals for the one-shot close culture
The reopen-shaped reversal signal is structurally near-impossible under this gate's one-shot design (verified: zero reversal events in the full production ledger), so bot-was-wrong evidence never reaches the calibration corpus's positive class. Detect the culture's actual reversal shape instead: a merged PR that shares a linked issue with a recently bot-closed PR, or the same author reworking a majority of its files, records reversal_superseded plus the per-rule reversed overrides the corpus consumes. Conservative matcher (borderline records nothing), idempotent per closed target, fail-safe on every write, and counted in the public reversal stats. Closes #8166
1 parent d89698d commit 67ac8dc

5 files changed

Lines changed: 358 additions & 3 deletions

File tree

src/review/outcomes-wire.ts

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
// applyAutoTune engages nothing → isHoldOnly is false → the merge path is unchanged. The breaker only engages
2424
// once a repo's merge precision actually drops below the floor over a real sample.
2525

26-
import { recordAuditEvent } from "../db/repositories";
26+
import { getPullRequest, listPullRequestFiles, recordAuditEvent } from "../db/repositories";
27+
import { evaluateSuccessorMatch, REVERSAL_SUPERSEDED_EVENT_TYPE, SUPERSEDED_LOOKBACK_MS } from "./reversal-superseded";
2728
import { createSignalStore } from "./signal-tracking-wire";
2829
import { AI_JUDGMENT_BLOCKER_CODES } from "../rules/advisory";
2930
import { tryEnqueueDecisionPackRebuild } from "../services/decision-pack";
@@ -675,6 +676,10 @@ export async function recordReversalSignals(
675676
await recordConfiguredGateBlockerOverrides(env, targetId).catch(() => undefined); // #8104
676677
await recordLinkedIssueScopeMismatchOverride(env, targetId).catch(() => undefined); // #8101
677678
}
679+
// #8166: the one-shot culture's reversal shape — this merge may supersede a bot-CLOSED sibling PR
680+
// (same linked issue, or same author reworking the same files). Best-effort, like every signal here.
681+
await recordSupersededReversals(env, repoFullName, pr.number, payload.pull_request?.user?.login ?? null).catch(() => undefined);
682+
678683
const reverted = parseRevertedPrNumber(pr.body);
679684
if (!reverted) return;
680685
const revertedTargetKey = reviewAuditTargetId(repoFullName, reverted);
@@ -926,3 +931,77 @@ export async function runSelfTuneBreaker(env: Env): Promise<void> {
926931
);
927932
}
928933
}
934+
935+
/**
936+
* #8166: scan the window for bot-CLOSED PRs this merge supersedes, and record the culture-correct reversal
937+
* signal for each match: a `reversal_superseded` row in BOTH stores (like its reopen/revert siblings, with
938+
* the matched heuristics in the audit metadata so borderline calls stay reviewable), plus the SAME per-rule
939+
* "the firing was wrong" overrides the reopen path records (#8101/#8104) — which is what finally feeds the
940+
* calibration corpus its positive class. Conservative + idempotent: evaluateSuccessorMatch's own bar
941+
* decides, a target with an existing superseded row is never re-recorded, and every step fails safe.
942+
*/
943+
export async function recordSupersededReversals(
944+
env: Env,
945+
repoFullName: string,
946+
mergedPrNumber: number,
947+
mergedAuthorLogin: string | null,
948+
): Promise<void> {
949+
try {
950+
const project = repoFullName.slice(0, 200);
951+
const mergedRecord = await getPullRequest(env, repoFullName, mergedPrNumber);
952+
if (!mergedRecord) return;
953+
const mergedFiles = (await listPullRequestFiles(env, repoFullName, mergedPrNumber)).map((file) => file.path);
954+
const merged = {
955+
authorLogin: mergedAuthorLogin ?? mergedRecord.authorLogin,
956+
linkedIssues: mergedRecord.linkedIssues,
957+
files: mergedFiles,
958+
};
959+
960+
const sinceIso = new Date(Date.now() - SUPERSEDED_LOOKBACK_MS).toISOString();
961+
const candidates = await env.DB.prepare(
962+
// Same bot-close definition as lastBotActionWasClose: real (non-dry-run) executed closes only.
963+
`SELECT DISTINCT target_key FROM audit_events
964+
WHERE event_type = 'agent.action.close' AND outcome IN ('success', 'completed')
965+
AND COALESCE(json_extract(metadata_json, '$.mode'), 'live') <> 'dry_run'
966+
AND target_key LIKE ? AND created_at >= ?`,
967+
)
968+
.bind(`${project}#%`, sinceIso)
969+
.all<{ target_key: string }>();
970+
971+
for (const row of candidates.results ?? []) {
972+
const targetKey = row.target_key;
973+
const closedNumber = Number(targetKey.slice(targetKey.lastIndexOf("#") + 1));
974+
if (!Number.isFinite(closedNumber) || closedNumber === mergedPrNumber) continue;
975+
// Idempotent per closed target: one superseded record ever, however many successors merge later.
976+
const already = await env.DB.prepare("SELECT 1 AS x FROM audit_events WHERE event_type = ? AND target_key = ? LIMIT 1")
977+
.bind(REVERSAL_SUPERSEDED_EVENT_TYPE, targetKey)
978+
.first<{ x: number }>();
979+
if (already) continue;
980+
981+
const closedRecord = await getPullRequest(env, repoFullName, closedNumber);
982+
if (!closedRecord) continue;
983+
const closedFiles = (await listPullRequestFiles(env, repoFullName, closedNumber)).map((file) => file.path);
984+
const heuristics = evaluateSuccessorMatch(merged, {
985+
authorLogin: closedRecord.authorLogin,
986+
linkedIssues: closedRecord.linkedIssues,
987+
files: closedFiles,
988+
});
989+
if (!heuristics) continue;
990+
991+
const summary = `Bot-closed PR #${closedNumber} superseded by merged PR #${mergedPrNumber}.`;
992+
await appendReviewAudit(env, { project, targetId: targetKey, eventType: REVERSAL_SUPERSEDED_EVENT_TYPE, summary });
993+
await recordAuditEvent(env, {
994+
eventType: REVERSAL_SUPERSEDED_EVENT_TYPE,
995+
actor: mergedAuthorLogin,
996+
targetKey,
997+
outcome: "completed",
998+
detail: summary,
999+
metadata: { repoFullName, pullNumber: closedNumber, supersededBy: mergedPrNumber, heuristics },
1000+
}).catch(() => undefined);
1001+
await recordConfiguredGateBlockerOverrides(env, targetKey).catch(() => undefined); // #8104
1002+
await recordLinkedIssueScopeMismatchOverride(env, targetKey).catch(() => undefined); // #8101
1003+
}
1004+
} catch (error) {
1005+
console.warn(JSON.stringify({ event: "reversal_superseded_error", repo: repoFullName, message: errorMessage(error).slice(0, 200) }));
1006+
}
1007+
}

src/review/public-stats.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ export async function getPublicStats(
310310
SELECT substr(target_key, 1, instr(target_key, '#') - 1) AS project,
311311
CAST(substr(target_key, instr(target_key, '#') + 1) AS INTEGER) AS pr_number
312312
FROM audit_events
313-
WHERE event_type IN ('reversal_reopened', 'reversal_reverted')
313+
WHERE event_type IN ('reversal_reopened', 'reversal_reverted', 'reversal_superseded')
314314
AND outcome = 'completed' AND instr(target_key, '#') > 0
315315
) ev
316316
WHERE LOWER(ev.project) IN (${inList})

src/review/reversal-superseded.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// Successor-based reversal heuristics (#8166, feeds epic #8082's positive class). This gate's own one-shot
2+
// design tells a wronged contributor "recovery = open a fresh PR", so the reopen-shaped reversal signal
3+
// (`reversal_reopened`) is structurally near-impossible here — verified in production: zero reversal events
4+
// ever, zero bot-closed PRs later merged. The culture's ACTUAL "the bot was wrong" shape is: bot CLOSES
5+
// PR #N, and a SUCCESSOR PR — same linked issue, or same author reworking the same files — later MERGES.
6+
//
7+
// PURE MODULE: the match decision only. Conservative by design (the issue's own bar): a false "the bot was
8+
// wrong" poisons calibration worse than a miss, so a match requires either a shared linked issue (the
9+
// strongest intent signal this repo has — the same set-intersection `duplicate_pr_risk` trusts) or the same
10+
// author reworking a majority of the closed PR's files. Borderline records NOTHING. The wire
11+
// (outcomes-wire.ts's recordSupersededReversals) supplies the data and writes the events.
12+
13+
export const REVERSAL_SUPERSEDED_EVENT_TYPE = "reversal_superseded";
14+
15+
/** A successor must re-touch at least this fraction of the CLOSED PR's files for the same-author path. */
16+
export const SUPERSEDED_FILE_OVERLAP_MIN = 0.5;
17+
18+
/** How far back a merge scans for bot-closed PRs it might supersede. Mirrors the calibration lookbacks'
19+
* order of magnitude — a months-later rework is a new effort, not a supersession signal. */
20+
export const SUPERSEDED_LOOKBACK_MS = 30 * 24 * 60 * 60 * 1000;
21+
22+
export type SupersededSide = {
23+
authorLogin: string | null | undefined;
24+
linkedIssues: readonly number[];
25+
files: readonly string[];
26+
};
27+
28+
export type SupersededHeuristics = {
29+
sameLinkedIssue: boolean;
30+
sameAuthorFileOverlap: boolean;
31+
/** |shared files| / |closed PR's files|; null when the closed PR has no recorded files. */
32+
fileOverlapRatio: number | null;
33+
};
34+
35+
/**
36+
* Decide whether `merged` supersedes the bot-closed `closed` PR. Returns the matched heuristics (for the
37+
* audit trail — every recorded event carries WHY it matched) or null when neither conservative path holds:
38+
* • sameLinkedIssue — both sides link at least one common issue number;
39+
* • sameAuthorFileOverlap — same author (case-insensitive; unknown authors never match) AND the merged PR
40+
* re-touches ≥ {@link SUPERSEDED_FILE_OVERLAP_MIN} of the closed PR's recorded files (a closed PR with
41+
* no recorded files can never match this path — fail-open to a miss, never a guess).
42+
* PURE and deterministic.
43+
*/
44+
export function evaluateSuccessorMatch(merged: SupersededSide, closed: SupersededSide): SupersededHeuristics | null {
45+
const sameLinkedIssue = closed.linkedIssues.length > 0 && closed.linkedIssues.some((issue) => merged.linkedIssues.includes(issue));
46+
47+
const mergedAuthor = merged.authorLogin?.trim().toLowerCase() ?? "";
48+
const closedAuthor = closed.authorLogin?.trim().toLowerCase() ?? "";
49+
const sameAuthor = mergedAuthor !== "" && mergedAuthor === closedAuthor;
50+
51+
let fileOverlapRatio: number | null = null;
52+
if (closed.files.length > 0) {
53+
const mergedFiles = new Set(merged.files);
54+
const shared = closed.files.filter((file) => mergedFiles.has(file)).length;
55+
fileOverlapRatio = shared / closed.files.length;
56+
}
57+
const sameAuthorFileOverlap = sameAuthor && fileOverlapRatio !== null && fileOverlapRatio >= SUPERSEDED_FILE_OVERLAP_MIN;
58+
59+
if (!sameLinkedIssue && !sameAuthorFileOverlap) return null;
60+
return { sameLinkedIssue, sameAuthorFileOverlap, fileOverlapRatio };
61+
}

src/services/public-accuracy-trend.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ async function loadReversalDayRows(env: Env, projects: string[], sinceIso: strin
126126
) orig
127127
JOIN (
128128
SELECT DISTINCT target_key FROM audit_events
129-
WHERE event_type IN ('reversal_reopened', 'reversal_reverted') AND outcome = 'completed'
129+
WHERE event_type IN ('reversal_reopened', 'reversal_reverted', 'reversal_superseded') AND outcome = 'completed'
130130
) rev ON rev.target_key = orig.target_key
131131
WHERE LOWER(orig.project) IN (${inList})
132132
GROUP BY day`,

0 commit comments

Comments
 (0)