Skip to content

Commit a2d2517

Browse files
authored
feat(review): wire linked_issue_scope_mismatch into the shared signal-tracking module (#8101) (#8109)
The AI-confidence-driven linked_issue_scope_mismatch finding carries real gate authority (block mode + unaddressed hard-blocks a PR) yet was invisible to the shared calibration module (#7982): createSignalStore's ORB adapter existed with zero live callers, so neither the self-correction pipeline (#7983/#7984) nor the backtest primitives (#8083-#8086) could see this judgment's history. Wire exactly one finding code and one override direction, per the issue scope: - processors.ts: inside the existing block-mode+unaddressed finding-push block (and ONLY there -- advisory mode never pushes, so it never records either), record a RuleFiredEvent {ruleId, targetKey repo#pr, outcome, occurredAt, metadata.confidence} via createSignalStore(env).recordRuleFired, best-effort with .catch(() => undefined) matching the adjacent cache-write discipline and SignalStore's own never-fail-the-review contract. - outcomes-wire.ts: when recordReversalSignals records a reversal (both the contributor-reopen path and #7985's owner reopen-then-merge path), check a fixed 30-day queryRuleHistory lookback for a linked_issue_scope_mismatch firing against the same target and, if present, record a "reversed" HumanOverrideEvent -- the human undoing of the bot action IS the judgment on that finding. Same .catch(() => undefined) discipline: a SignalStore failure (including the deliberately-propagating queryRuleHistory read error) never affects whether the underlying reversal records. No "confirmed" path, per the issue's Boundaries. Tests extend both existing suites: fired recorded exactly at block+unaddressed (with confidence metadata) and NOT for advisory mode or addressed/partial; "reversed" recorded on both reversal paths only when a prior firing exists for that target; and both write paths degrade silently -- normal return values and the reversal itself unaffected -- when the SignalStore call rejects.
1 parent cf5cd19 commit a2d2517

4 files changed

Lines changed: 188 additions & 0 deletions

File tree

src/queue/processors.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ import {
265265
} from "../selfhost/queue-common";
266266
import { aiReviewCacheInputFingerprint } from "../review/ai-review-cache-input";
267267
import { linkedIssueSatisfactionCacheInputFingerprint } from "../review/linked-issue-satisfaction-cache-input";
268+
import { createSignalStore } from "../review/signal-tracking-wire";
268269
import {
269270
AGENT_LABEL_NEEDS_REVIEW,
270271
downgradeCloseToHold,
@@ -7531,6 +7532,20 @@ export async function runLinkedIssueSatisfactionForAdvisory(
75317532
action: "Confirm this PR actually addresses the linked issue's scope, or link the correct issue.",
75327533
publicText: `AI assessment: this PR does not appear to satisfy its linked issue's scope. ${result.result.rationale}`,
75337534
});
7535+
// #8101: this AI judgment carries gate authority in block mode, so record the firing in the shared
7536+
// calibration module (#7982) — the fired/override history is what the self-correction pipeline
7537+
// (#7983/#7984) and the backtest primitives (#8083-#8086) consume. Recorded ONLY here: advisory mode
7538+
// never pushes the finding, so it never records either. Best-effort like the cache-write handling
7539+
// above and SignalStore's own contract — a recording failure must never fail the review pass.
7540+
await createSignalStore(env)
7541+
.recordRuleFired({
7542+
ruleId: "linked_issue_scope_mismatch",
7543+
targetKey: `${args.repoFullName}#${args.pr.number}`,
7544+
outcome: result.result.status,
7545+
occurredAt: nowIso(),
7546+
metadata: { confidence: result.result.confidence },
7547+
})
7548+
.catch(() => undefined);
75347549
}
75357550
return { status: result.result.status, rationale: result.result.rationale };
75367551
} catch (error) {

src/review/outcomes-wire.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
// once a repo's merge precision actually drops below the floor over a real sample.
2525

2626
import { recordAuditEvent } from "../db/repositories";
27+
import { createSignalStore } from "./signal-tracking-wire";
2728
import { tryEnqueueDecisionPackRebuild } from "../services/decision-pack";
2829
import { incr } from "../selfhost/metrics";
2930
import { loadRepoFocusManifest } from "../signals/focus-manifest-loader";
@@ -447,6 +448,32 @@ async function hasRecentOwnerReopenPendingReversal(env: Env, targetKey: string,
447448
}
448449
}
449450

451+
// #8101: when a reversal is recorded for a target that a `linked_issue_scope_mismatch` finding fired
452+
// against (fixed 30-day lookback), the human undoing of the bot action IS the human judgment on that
453+
// finding — record a "reversed" HumanOverrideEvent in the shared calibration module (#7982) so the
454+
// self-correction pipeline and the backtest primitives see it. Only this one rule and only the reversal
455+
// direction are wired (no "confirmed" signal exists anywhere in this codebase to mirror — see the issue's
456+
// Boundaries). Callers attach `.catch(() => undefined)`: like every write in this file, a SignalStore
457+
// failure (including a queryRuleHistory read error, which deliberately propagates) must never affect
458+
// whether the underlying reversal itself is recorded.
459+
const LINKED_ISSUE_SCOPE_MISMATCH_RULE_ID = "linked_issue_scope_mismatch";
460+
const LINKED_ISSUE_SCOPE_MISMATCH_LOOKBACK_MS = 30 * 24 * 60 * 60 * 1000;
461+
462+
async function recordLinkedIssueScopeMismatchOverride(env: Env, targetId: string): Promise<void> {
463+
const store = createSignalStore(env);
464+
const history = await store.queryRuleHistory(
465+
LINKED_ISSUE_SCOPE_MISMATCH_RULE_ID,
466+
Date.now() - LINKED_ISSUE_SCOPE_MISMATCH_LOOKBACK_MS,
467+
);
468+
if (!history.fired.some((event) => event.targetKey === targetId)) return;
469+
await store.recordHumanOverride({
470+
ruleId: LINKED_ISSUE_SCOPE_MISMATCH_RULE_ID,
471+
targetKey: targetId,
472+
verdict: "reversed",
473+
occurredAt: nowIso(),
474+
});
475+
}
476+
450477
/**
451478
* Record a REVERSAL — a human overriding a loopover auto-action — into the eval/audit stores (the
452479
* ground-truth accuracy signal). Mirrors reviewbot recordReversalSignals (runtime.ts ~157/274):
@@ -510,6 +537,7 @@ export async function recordReversalSignals(
510537
detail: `Bot-closed PR #${pr.number} reopened by a contributor.`,
511538
metadata: { repoFullName, pullNumber: pr.number },
512539
}).catch(() => undefined);
540+
await recordLinkedIssueScopeMismatchOverride(env, targetId).catch(() => undefined); // #8101
513541
return;
514542
}
515543

@@ -533,6 +561,7 @@ export async function recordReversalSignals(
533561
detail: `Bot-closed PR #${pr.number} reopened and merged by the repo owner.`,
534562
metadata: { repoFullName, pullNumber: pr.number },
535563
}).catch(() => undefined);
564+
await recordLinkedIssueScopeMismatchOverride(env, targetId).catch(() => undefined); // #8101
536565
}
537566
const reverted = parseRevertedPrNumber(pr.body);
538567
if (!reverted) return;

test/unit/linked-issue-satisfaction-run.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import {
1212
upsertRepositorySettings,
1313
} from "../../src/db/repositories";
1414
import { linkedIssueSatisfactionCacheInputFingerprint } from "../../src/review/linked-issue-satisfaction-cache-input";
15+
import * as signalTrackingWire from "../../src/review/signal-tracking-wire";
16+
import { createSignalStore } from "../../src/review/signal-tracking-wire";
1517
import { clearInstallationTokenCacheForTest } from "../../src/github/app";
1618
import { normalizeRegistryPayload } from "../../src/registry/normalize";
1719
import { persistRegistrySnapshot } from "../../src/registry/sync";
@@ -462,6 +464,57 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)"
462464
expect(gate.blockers).toHaveLength(0);
463465
});
464466

467+
it("BLOCK mode + 'unaddressed' records a linked_issue_scope_mismatch fired signal in the shared calibration store (#8101)", async () => {
468+
stubIssueFetch();
469+
const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) }));
470+
const env = enabledEnv(run);
471+
await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 });
472+
473+
const history = await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0);
474+
expect(history.fired).toHaveLength(1);
475+
expect(history.fired[0]).toMatchObject({
476+
ruleId: "linked_issue_scope_mismatch",
477+
targetKey: "acme/widgets#7",
478+
outcome: "unaddressed",
479+
metadata: { confidence: 0.9 },
480+
});
481+
expect(history.overrides).toEqual([]); // firing alone is never an override
482+
});
483+
484+
it("ADVISORY mode records NO fired signal for the same 'unaddressed' verdict (#8101 — no finding, no signal)", async () => {
485+
stubIssueFetch();
486+
const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) }));
487+
const env = enabledEnv(run);
488+
await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 });
489+
expect((await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0)).fired).toEqual([]);
490+
});
491+
492+
it("BLOCK mode records NO fired signal for 'addressed' or 'partial' verdicts (#8101)", async () => {
493+
for (const status of ["addressed", "partial"] as const) {
494+
stubIssueFetch();
495+
const run = vi.fn(async () => ({ response: satisfactionJson({ status }) }));
496+
const env = enabledEnv(run);
497+
await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 });
498+
expect((await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0)).fired).toEqual([]);
499+
}
500+
});
501+
502+
it("degrades silently when the SignalStore write rejects: the finding still pushes and nothing throws (#8101)", async () => {
503+
stubIssueFetch();
504+
vi.spyOn(signalTrackingWire, "createSignalStore").mockReturnValue({
505+
recordRuleFired: async () => {
506+
throw new Error("signal store down");
507+
},
508+
recordHumanOverride: async () => undefined,
509+
queryRuleHistory: async () => ({ fired: [], overrides: [] }),
510+
});
511+
const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) }));
512+
const adv = advisory();
513+
const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 });
514+
expect(result).toMatchObject({ status: "unaddressed" }); // normal return value unaffected
515+
expect(adv.findings).toHaveLength(1); // the blocker still lands
516+
});
517+
465518
it("BLOCK mode: an 'addressed'/'partial' verdict never pushes a finding (nothing to block)", async () => {
466519
stubIssueFetch();
467520
const run = vi.fn(async () => ({ response: satisfactionJson({ status: "partial" }) }));

test/unit/outcomes-wire.test.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { describe, expect, it, vi } from "vitest";
2+
import * as signalTrackingWire from "../../src/review/signal-tracking-wire";
3+
import { createSignalStore } from "../../src/review/signal-tracking-wire";
24
import { processJob } from "../../src/queue/processors";
35
import {
46
createFlagStore,
@@ -1318,3 +1320,92 @@ describe("resolveDispositionReason (enriched Discord reason)", () => {
13181320
).toBe("fallback");
13191321
});
13201322
});
1323+
1324+
// ── #8101: linked_issue_scope_mismatch reversal-override wiring ─────────────────────────────────────────────
1325+
1326+
describe("recordReversalSignals — linked_issue_scope_mismatch override (#8101)", () => {
1327+
const RULE = "linked_issue_scope_mismatch";
1328+
1329+
async function seedFiredSignal(env: Env, targetKey: string): Promise<void> {
1330+
await createSignalStore(env).recordRuleFired({
1331+
ruleId: RULE,
1332+
targetKey,
1333+
outcome: "unaddressed",
1334+
occurredAt: new Date().toISOString(),
1335+
metadata: { confidence: 0.9 },
1336+
});
1337+
}
1338+
1339+
function contributorReopen(number = 7) {
1340+
return {
1341+
action: "reopened",
1342+
repository: { name: "repo", full_name: "owner/repo", owner: { login: "owner" } },
1343+
pull_request: pullRequestPayload({ number, state: "open" }),
1344+
sender: { login: "contributor", type: "User" },
1345+
};
1346+
}
1347+
1348+
it("records a 'reversed' override when a contributor reopens a bot-closed PR that the rule fired against", async () => {
1349+
const env = createTestEnv();
1350+
await seedBotAction(env, "owner/repo#7", "close");
1351+
await seedFiredSignal(env, "owner/repo#7");
1352+
1353+
await recordReversalSignals(env, "pull_request", contributorReopen());
1354+
1355+
const history = await createSignalStore(env).queryRuleHistory(RULE, 0);
1356+
expect(history.overrides).toHaveLength(1);
1357+
expect(history.overrides[0]).toMatchObject({ ruleId: RULE, targetKey: "owner/repo#7", verdict: "reversed" });
1358+
});
1359+
1360+
it("records a 'reversed' override on the owner reopen-then-merge path (#7985) when the rule fired against the target", async () => {
1361+
const env = createTestEnv();
1362+
await seedBotAction(env, "owner/repo#7", "close");
1363+
await seedFiredSignal(env, "owner/repo#7");
1364+
// Owner reopens (writes the pending marker)...
1365+
await recordReversalSignals(env, "pull_request", {
1366+
action: "reopened",
1367+
repository: { name: "repo", full_name: "owner/repo", owner: { login: "owner" } },
1368+
pull_request: pullRequestPayload({ number: 7, state: "open" }),
1369+
sender: { login: "owner", type: "User" },
1370+
});
1371+
expect((await createSignalStore(env).queryRuleHistory(RULE, 0)).overrides).toEqual([]); // marker alone is not a reversal
1372+
// ...then merges within the window, promoting the marker to a real reversal.
1373+
await recordReversalSignals(env, "pull_request", {
1374+
action: "closed",
1375+
repository: { name: "repo", full_name: "owner/repo", owner: { login: "owner" } },
1376+
pull_request: pullRequestPayload({ number: 7, state: "closed", merged_at: new Date().toISOString() }),
1377+
sender: { login: "owner", type: "User" },
1378+
});
1379+
1380+
const history = await createSignalStore(env).queryRuleHistory(RULE, 0);
1381+
expect(history.overrides).toHaveLength(1);
1382+
expect(history.overrides[0]).toMatchObject({ ruleId: RULE, targetKey: "owner/repo#7", verdict: "reversed" });
1383+
});
1384+
1385+
it("records NO override when the reversal target has no prior fired event for this rule", async () => {
1386+
const env = createTestEnv();
1387+
await seedBotAction(env, "owner/repo#7", "close");
1388+
await seedFiredSignal(env, "owner/repo#99"); // fired against a DIFFERENT target only
1389+
1390+
await recordReversalSignals(env, "pull_request", contributorReopen());
1391+
1392+
expect((await createSignalStore(env).queryRuleHistory(RULE, 0)).overrides).toEqual([]);
1393+
expect(await reviewAuditRows(env, "reversal_reopened")).toHaveLength(1); // the reversal itself still records
1394+
});
1395+
1396+
it("degrades silently when the SignalStore read rejects: the reversal itself still records and nothing throws", async () => {
1397+
const env = createTestEnv();
1398+
await seedBotAction(env, "owner/repo#7", "close");
1399+
vi.spyOn(signalTrackingWire, "createSignalStore").mockReturnValue({
1400+
recordRuleFired: async () => undefined,
1401+
recordHumanOverride: async () => undefined,
1402+
queryRuleHistory: async () => {
1403+
throw new Error("signal store down");
1404+
},
1405+
});
1406+
1407+
await expect(recordReversalSignals(env, "pull_request", contributorReopen())).resolves.toBeUndefined();
1408+
expect(await reviewAuditRows(env, "reversal_reopened")).toHaveLength(1);
1409+
vi.restoreAllMocks();
1410+
});
1411+
});

0 commit comments

Comments
 (0)