Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ import {
} from "../selfhost/queue-common";
import { aiReviewCacheInputFingerprint } from "../review/ai-review-cache-input";
import { linkedIssueSatisfactionCacheInputFingerprint } from "../review/linked-issue-satisfaction-cache-input";
import { createSignalStore } from "../review/signal-tracking-wire";
import {
AGENT_LABEL_NEEDS_REVIEW,
downgradeCloseToHold,
Expand Down Expand Up @@ -7531,6 +7532,20 @@ export async function runLinkedIssueSatisfactionForAdvisory(
action: "Confirm this PR actually addresses the linked issue's scope, or link the correct issue.",
publicText: `AI assessment: this PR does not appear to satisfy its linked issue's scope. ${result.result.rationale}`,
});
// #8101: this AI judgment carries gate authority in block mode, so record the firing in the shared
// calibration module (#7982) — the fired/override history is what the self-correction pipeline
// (#7983/#7984) and the backtest primitives (#8083-#8086) consume. Recorded ONLY here: advisory mode
// never pushes the finding, so it never records either. Best-effort like the cache-write handling
// above and SignalStore's own contract — a recording failure must never fail the review pass.
await createSignalStore(env)
.recordRuleFired({
ruleId: "linked_issue_scope_mismatch",
targetKey: `${args.repoFullName}#${args.pr.number}`,
outcome: result.result.status,
occurredAt: nowIso(),
metadata: { confidence: result.result.confidence },
})
.catch(() => undefined);
}
return { status: result.result.status, rationale: result.result.rationale };
} catch (error) {
Expand Down
29 changes: 29 additions & 0 deletions src/review/outcomes-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
// once a repo's merge precision actually drops below the floor over a real sample.

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

// #8101: when a reversal is recorded for a target that a `linked_issue_scope_mismatch` finding fired
// against (fixed 30-day lookback), the human undoing of the bot action IS the human judgment on that
// finding — record a "reversed" HumanOverrideEvent in the shared calibration module (#7982) so the
// self-correction pipeline and the backtest primitives see it. Only this one rule and only the reversal
// direction are wired (no "confirmed" signal exists anywhere in this codebase to mirror — see the issue's
// Boundaries). Callers attach `.catch(() => undefined)`: like every write in this file, a SignalStore
// failure (including a queryRuleHistory read error, which deliberately propagates) must never affect
// whether the underlying reversal itself is recorded.
const LINKED_ISSUE_SCOPE_MISMATCH_RULE_ID = "linked_issue_scope_mismatch";
const LINKED_ISSUE_SCOPE_MISMATCH_LOOKBACK_MS = 30 * 24 * 60 * 60 * 1000;

async function recordLinkedIssueScopeMismatchOverride(env: Env, targetId: string): Promise<void> {
const store = createSignalStore(env);
const history = await store.queryRuleHistory(
LINKED_ISSUE_SCOPE_MISMATCH_RULE_ID,
Date.now() - LINKED_ISSUE_SCOPE_MISMATCH_LOOKBACK_MS,
);
if (!history.fired.some((event) => event.targetKey === targetId)) return;
await store.recordHumanOverride({
ruleId: LINKED_ISSUE_SCOPE_MISMATCH_RULE_ID,
targetKey: targetId,
verdict: "reversed",
occurredAt: nowIso(),
});
}

/**
* Record a REVERSAL — a human overriding a loopover auto-action — into the eval/audit stores (the
* ground-truth accuracy signal). Mirrors reviewbot recordReversalSignals (runtime.ts ~157/274):
Expand Down Expand Up @@ -510,6 +537,7 @@ export async function recordReversalSignals(
detail: `Bot-closed PR #${pr.number} reopened by a contributor.`,
metadata: { repoFullName, pullNumber: pr.number },
}).catch(() => undefined);
await recordLinkedIssueScopeMismatchOverride(env, targetId).catch(() => undefined); // #8101
return;
}

Expand All @@ -533,6 +561,7 @@ export async function recordReversalSignals(
detail: `Bot-closed PR #${pr.number} reopened and merged by the repo owner.`,
metadata: { repoFullName, pullNumber: pr.number },
}).catch(() => undefined);
await recordLinkedIssueScopeMismatchOverride(env, targetId).catch(() => undefined); // #8101
}
const reverted = parseRevertedPrNumber(pr.body);
if (!reverted) return;
Expand Down
53 changes: 53 additions & 0 deletions test/unit/linked-issue-satisfaction-run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
upsertRepositorySettings,
} from "../../src/db/repositories";
import { linkedIssueSatisfactionCacheInputFingerprint } from "../../src/review/linked-issue-satisfaction-cache-input";
import * as signalTrackingWire from "../../src/review/signal-tracking-wire";
import { createSignalStore } from "../../src/review/signal-tracking-wire";
import { clearInstallationTokenCacheForTest } from "../../src/github/app";
import { normalizeRegistryPayload } from "../../src/registry/normalize";
import { persistRegistrySnapshot } from "../../src/registry/sync";
Expand Down Expand Up @@ -462,6 +464,57 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)"
expect(gate.blockers).toHaveLength(0);
});

it("BLOCK mode + 'unaddressed' records a linked_issue_scope_mismatch fired signal in the shared calibration store (#8101)", async () => {
stubIssueFetch();
const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) }));
const env = enabledEnv(run);
await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 });

const history = await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0);
expect(history.fired).toHaveLength(1);
expect(history.fired[0]).toMatchObject({
ruleId: "linked_issue_scope_mismatch",
targetKey: "acme/widgets#7",
outcome: "unaddressed",
metadata: { confidence: 0.9 },
});
expect(history.overrides).toEqual([]); // firing alone is never an override
});

it("ADVISORY mode records NO fired signal for the same 'unaddressed' verdict (#8101 — no finding, no signal)", async () => {
stubIssueFetch();
const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) }));
const env = enabledEnv(run);
await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 });
expect((await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0)).fired).toEqual([]);
});

it("BLOCK mode records NO fired signal for 'addressed' or 'partial' verdicts (#8101)", async () => {
for (const status of ["addressed", "partial"] as const) {
stubIssueFetch();
const run = vi.fn(async () => ({ response: satisfactionJson({ status }) }));
const env = enabledEnv(run);
await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 });
expect((await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0)).fired).toEqual([]);
}
});

it("degrades silently when the SignalStore write rejects: the finding still pushes and nothing throws (#8101)", async () => {
stubIssueFetch();
vi.spyOn(signalTrackingWire, "createSignalStore").mockReturnValue({
recordRuleFired: async () => {
throw new Error("signal store down");
},
recordHumanOverride: async () => undefined,
queryRuleHistory: async () => ({ fired: [], overrides: [] }),
});
const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) }));
const adv = advisory();
const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 });
expect(result).toMatchObject({ status: "unaddressed" }); // normal return value unaffected
expect(adv.findings).toHaveLength(1); // the blocker still lands
});

it("BLOCK mode: an 'addressed'/'partial' verdict never pushes a finding (nothing to block)", async () => {
stubIssueFetch();
const run = vi.fn(async () => ({ response: satisfactionJson({ status: "partial" }) }));
Expand Down
91 changes: 91 additions & 0 deletions test/unit/outcomes-wire.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { describe, expect, it, vi } from "vitest";
import * as signalTrackingWire from "../../src/review/signal-tracking-wire";
import { createSignalStore } from "../../src/review/signal-tracking-wire";
import { processJob } from "../../src/queue/processors";
import {
createFlagStore,
Expand Down Expand Up @@ -1318,3 +1320,92 @@ describe("resolveDispositionReason (enriched Discord reason)", () => {
).toBe("fallback");
});
});

// ── #8101: linked_issue_scope_mismatch reversal-override wiring ─────────────────────────────────────────────

describe("recordReversalSignals — linked_issue_scope_mismatch override (#8101)", () => {
const RULE = "linked_issue_scope_mismatch";

async function seedFiredSignal(env: Env, targetKey: string): Promise<void> {
await createSignalStore(env).recordRuleFired({
ruleId: RULE,
targetKey,
outcome: "unaddressed",
occurredAt: new Date().toISOString(),
metadata: { confidence: 0.9 },
});
}

function contributorReopen(number = 7) {
return {
action: "reopened",
repository: { name: "repo", full_name: "owner/repo", owner: { login: "owner" } },
pull_request: pullRequestPayload({ number, state: "open" }),
sender: { login: "contributor", type: "User" },
};
}

it("records a 'reversed' override when a contributor reopens a bot-closed PR that the rule fired against", async () => {
const env = createTestEnv();
await seedBotAction(env, "owner/repo#7", "close");
await seedFiredSignal(env, "owner/repo#7");

await recordReversalSignals(env, "pull_request", contributorReopen());

const history = await createSignalStore(env).queryRuleHistory(RULE, 0);
expect(history.overrides).toHaveLength(1);
expect(history.overrides[0]).toMatchObject({ ruleId: RULE, targetKey: "owner/repo#7", verdict: "reversed" });
});

it("records a 'reversed' override on the owner reopen-then-merge path (#7985) when the rule fired against the target", async () => {
const env = createTestEnv();
await seedBotAction(env, "owner/repo#7", "close");
await seedFiredSignal(env, "owner/repo#7");
// Owner reopens (writes the pending marker)...
await recordReversalSignals(env, "pull_request", {
action: "reopened",
repository: { name: "repo", full_name: "owner/repo", owner: { login: "owner" } },
pull_request: pullRequestPayload({ number: 7, state: "open" }),
sender: { login: "owner", type: "User" },
});
expect((await createSignalStore(env).queryRuleHistory(RULE, 0)).overrides).toEqual([]); // marker alone is not a reversal
// ...then merges within the window, promoting the marker to a real reversal.
await recordReversalSignals(env, "pull_request", {
action: "closed",
repository: { name: "repo", full_name: "owner/repo", owner: { login: "owner" } },
pull_request: pullRequestPayload({ number: 7, state: "closed", merged_at: new Date().toISOString() }),
sender: { login: "owner", type: "User" },
});

const history = await createSignalStore(env).queryRuleHistory(RULE, 0);
expect(history.overrides).toHaveLength(1);
expect(history.overrides[0]).toMatchObject({ ruleId: RULE, targetKey: "owner/repo#7", verdict: "reversed" });
});

it("records NO override when the reversal target has no prior fired event for this rule", async () => {
const env = createTestEnv();
await seedBotAction(env, "owner/repo#7", "close");
await seedFiredSignal(env, "owner/repo#99"); // fired against a DIFFERENT target only

await recordReversalSignals(env, "pull_request", contributorReopen());

expect((await createSignalStore(env).queryRuleHistory(RULE, 0)).overrides).toEqual([]);
expect(await reviewAuditRows(env, "reversal_reopened")).toHaveLength(1); // the reversal itself still records
});

it("degrades silently when the SignalStore read rejects: the reversal itself still records and nothing throws", async () => {
const env = createTestEnv();
await seedBotAction(env, "owner/repo#7", "close");
vi.spyOn(signalTrackingWire, "createSignalStore").mockReturnValue({
recordRuleFired: async () => undefined,
recordHumanOverride: async () => undefined,
queryRuleHistory: async () => {
throw new Error("signal store down");
},
});

await expect(recordReversalSignals(env, "pull_request", contributorReopen())).resolves.toBeUndefined();
expect(await reviewAuditRows(env, "reversal_reopened")).toHaveLength(1);
vi.restoreAllMocks();
});
});