From 1888f71503f99b1b2be571c2e3939e6a03f583cd Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 25 Jul 2026 22:32:03 -0700 Subject: [PATCH] feat(calibration): record advisory-mode rule-fired signals too, tagged with the resolved gate mode (#8760) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit recordConfiguredGateBlockerSignals only recorded findings whose gate RESOLVED to "block" — but every gate defaults to advisory/off, so the majority of real traffic (advisory findings a human actually saw) recorded nothing, and every downstream backtest corpus, reliability curve, and knob-loosening decision was built from the biased block-mode-only minority. - isConfiguredGateBlocker is now a =="block" projection of a new resolveConfiguredGateMode(finding, policy) -> "block"|"advisory"|"off" — one source of truth, so recorder and gate can never drift; every mode default is byte-identical to the old switch. - The recorder captures every non-"off" finding with metadata.gateMode so consumers can segment bot-actioned evidence from shown-to-a-human evidence. The advisory_only sub-floor AI demotion resolves "advisory" (a real finding a human saw), never "off". manifest_linked_issue_required composes the stricter of its two gates. - Consumer-segmentation audit (in the recorder's doc comment): every current consumer is decided-case-driven (corpus pairing drops unpaired fires; the repeat alarm uses its own composite-ruleId namespace), so advisory fires sit inert until #8761's implicit confirmations start pairing them. --- src/queue/processors.ts | 5 +- src/rules/advisory.ts | 96 +++++++++++++------ .../configured-gate-blocker-signals.test.ts | 65 +++++++++++-- 3 files changed, 124 insertions(+), 42 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index f4623c4de7..7975b61a54 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -10481,8 +10481,9 @@ async function maybePublishPrPublicSurface( let evaluation = shouldEvaluateGate ? evaluateGateCheck(advisory, gatePolicy) : undefined; - // #8104: record RuleFiredEvent for every configured gate blocker except linked_issue_scope_mismatch - // (#8101). Same advisory+policy as evaluateGateCheck above so the filter stays in lock-step. + // #8104/#8760: record RuleFiredEvent for every configured-gate-class finding whose resolved mode isn't + // "off" (tagged metadata.gateMode), except linked_issue_scope_mismatch (#8101). Same advisory+policy as + // evaluateGateCheck above so the mode resolution stays in lock-step. if (evaluation) { // #8130: thread the SAME memoized diff the AI review consumed so ai_consensus_defect/ai_review_split // fired events capture the raw context their detection evaluated (never re-fetched). diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts index 7189af1d9d..cd6d088a06 100644 --- a/src/rules/advisory.ts +++ b/src/rules/advisory.ts @@ -1016,79 +1016,97 @@ function isEvaluationBlocker(code: string, policy: GateCheckPolicy): boolean { // resolveAiReviewLowConfidenceHold (below) and for the packages/loopover-engine gate-decision twin's own copy. export const DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE = 0.93; -function isConfiguredGateBlocker(finding: AdvisoryFinding, policy: GateCheckPolicy): boolean { +/** + * Resolve a finding's configured gate MODE (#8760): the single source of truth both the gate's own blocker + * decision ({@link isConfiguredGateBlocker} — mode === "block") and the calibration recorder + * ({@link recordConfiguredGateBlockerSignals} — mode !== "off") derive from, so the two can never drift. + * Every mode default below is byte-identical to the pre-#8760 isConfiguredGateBlocker switch; an unknown + * code resolves "off" (never blocks, never records — pure-warning codes like `pre_merge_check_failed` or + * `readiness_score_below_threshold` are deliberately not configured-gate-class findings). + */ +function resolveConfiguredGateMode(finding: AdvisoryFinding, policy: GateCheckPolicy): GateRuleMode { const code = finding.code; // Missing linked issue defaults to ADVISORY — issues aren't always available, so it only blocks when a // repo explicitly opts in with linkedIssueGateMode: "block". Duplicates still default to blocking. - if (code === "missing_linked_issue") return gateMode(policy.linkedIssueGateMode ?? "advisory") === "block"; - if (code === "duplicate_pr_risk") return gateMode(policy.duplicatePrGateMode ?? "block") === "block"; + if (code === "missing_linked_issue") return gateMode(policy.linkedIssueGateMode ?? "advisory"); + if (code === "duplicate_pr_risk") return gateMode(policy.duplicatePrGateMode ?? "block"); // A dual-model AI consensus defect blocks ONLY when the maintainer opted into aiReview: block. It is the // most conservative AI signal (two independent models) but still confirmed-contributor gated by // evaluateGateCheck, and advisory by default. // A consensus defect (both reviewers) OR a SPLIT (one reviewer flagged a blocker the other did not) both block // when aiReviewGateMode is `block`. (#ai-review-split) The close-confidence floor + disposition (#4603) decide // what happens to a SUB-floor finding: `one_shot`/`hold_for_review` (both still block here -- hold_for_review's - // difference is downstream, in resolveAiReviewLowConfidenceHold below) leave this branch's return unchanged; - // only `advisory_only` demotes a sub-floor finding to a non-blocker. + // difference is downstream, in resolveAiReviewLowConfidenceHold below) leave this branch's mode unchanged; + // only `advisory_only` demotes a sub-floor finding — to "advisory" (a real finding a human saw, worth + // corpus evidence), never to "off". if (code === "ai_consensus_defect" || code === "ai_review_split") { - if (gateMode(policy.aiReviewGateMode ?? "advisory") !== "block") return false; + const mode = gateMode(policy.aiReviewGateMode ?? "advisory"); + if (mode !== "block") return mode; if ((policy.aiReviewLowConfidenceDisposition ?? "hold_for_review") === "advisory_only") { const floor = policy.aiReviewCloseConfidence ?? DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE; const confidence = finding.confidence ?? 1; - if (confidence < floor) return false; + if (confidence < floor) return "advisory"; } - return true; + return "block"; } - if (code === REVIEW_THREAD_BLOCKER_CODE) return true; + if (code === REVIEW_THREAD_BLOCKER_CODE) return "block"; // A leaked-secret finding (`secret_leak`) ALWAYS hard-blocks: a committed credential must be removed and // rotated before merge, with no opt-in. This finding is produced ONLY by the flag-gated safety scan // (LOOPOVER_REVIEW_SAFETY); when the flag is off the finding never exists, so this branch is unreachable and the // gate verdict is byte-identical to today. - if (code === "secret_leak") return true; + if (code === "secret_leak") return "block"; // A maintainer pre-merge check (#review-pre-merge-checks) marked `enforce: true` produces this DETERMINISTIC // finding when it fails (a required title/description phrase or label is missing). It always blocks: the // per-check `enforce` flag in `.loopover.yml` IS the opt-in (mirroring secret_leak — the finding only exists // when the maintainer configured an enforced check). The advisory variant (`pre_merge_check_failed`) is a plain // warning and is never blocked here. No AI judgment is involved, so this can never cause an AI false-close. - if (code === "pre_merge_check_required") return true; + if (code === "pre_merge_check_required") return "block"; // Focus-manifest missing-tests policy (#555): blocks ONLY when the maintainer opts into manifestPolicy: // block. Path holds are intentionally separate and configured via hardGuardrailGlobs. if (code === "manifest_missing_tests") { - return gateMode(policy.manifestPolicyGateMode ?? "off") === "block"; + return gateMode(policy.manifestPolicyGateMode ?? "off"); } // Focus-manifest linked-issue policy (#555, #4618): blocks when EITHER the manifest-policy gate OR the // linked-issue gate is opted into block. resolveEffectiveSettings promotes linkedIssueGateMode to "block" // whenever the yml-only `linkedIssuePolicy: required` knob is set (mirroring the requireLinkedIssue // promotion), so this finding's own escalation must honor that gate too -- not just manifestPolicyGateMode // -- or the promotion would have no actual blocking effect and the config-surface-reduction fix would be a - // no-op. + // no-op. Mode composition: the STRICTER of the two gates wins (block > advisory > off), preserving the + // original "blocks when either blocks" semantics exactly. if (code === "manifest_linked_issue_required") { - return gateMode(policy.manifestPolicyGateMode ?? "off") === "block" || gateMode(policy.linkedIssueGateMode ?? "advisory") === "block"; + const manifest = gateMode(policy.manifestPolicyGateMode ?? "off"); + const linked = gateMode(policy.linkedIssueGateMode ?? "advisory"); + if (manifest === "block" || linked === "block") return "block"; + return manifest === "advisory" || linked === "advisory" ? "advisory" : "off"; } // Self-authored linked-issue gate: blocks only when the maintainer opts in with `block`. Defaults to // advisory — the finding surfaces in the panel without ever closing the PR unless explicitly configured. - if (code === "self_authored_linked_issue") return gateMode(policy.selfAuthoredLinkedIssueGateMode ?? "advisory") === "block"; + if (code === "self_authored_linked_issue") return gateMode(policy.selfAuthoredLinkedIssueGateMode ?? "advisory"); // Linked-issue satisfaction gate (#1961/#3906): blocks only when the maintainer opts in with `block`. The // finding itself is only ever produced when the caller already resolved `block` mode (see // runLinkedIssueSatisfactionForAdvisory), so this is a defense-in-depth mirror of that gate, not the // primary enforcement point. - if (code === "linked_issue_scope_mismatch") return gateMode(policy.linkedIssueSatisfactionGateMode ?? "advisory") === "block"; + if (code === "linked_issue_scope_mismatch") return gateMode(policy.linkedIssueSatisfactionGateMode ?? "advisory"); // Content-lane linked-issue deliverable gate (#content-lane-deliverable): blocks only when the maintainer // opts in with `block`. The finding itself is only ever produced when the caller already resolved a non- // "off" mode (see runContentLaneDeliverableCheckForAdvisory), so this is a defense-in-depth mirror of that // gate, not the primary enforcement point -- mirrors linked_issue_scope_mismatch immediately above. - if (code === "content_lane_deliverable_missing") return gateMode(policy.contentLaneDeliverableGateMode ?? "off") === "block"; + if (code === "content_lane_deliverable_missing") return gateMode(policy.contentLaneDeliverableGateMode ?? "off"); // Backtest-regression gate (#8105): blocks only under an explicit opt-in, mirroring the content-lane gate // above -- the finding itself only exists in block mode (see resolveThresholdBacktestAdvisory). - if (code === "backtest_regression") return gateMode(policy.backtestRegressionGateMode ?? "advisory") === "block"; + if (code === "backtest_regression") return gateMode(policy.backtestRegressionGateMode ?? "advisory"); // Lockfile-tamper-risk gate (#2563): blocks only when the maintainer opts in with `block`. Defaults to `off` // (the finding is never even produced — see maybeAddLockfileTamperFinding's mode gate in queue/processors.ts), // so this branch only matters once a repo has explicitly turned the scan on. - if (code === "lockfile_tamper_risk") return gateMode(policy.lockfileIntegrityGateMode ?? "off") === "block"; + if (code === "lockfile_tamper_risk") return gateMode(policy.lockfileIntegrityGateMode ?? "off"); // CLA / license-compatibility gate (#2564): blocks only when the maintainer opts into claMode: block. // Defaults to off (evaluateClaCheck never even runs for an off repo, so the finding does not exist). - if (code === CLA_CONSENT_MISSING_CODE) return gateMode(policy.claGateMode ?? "off") === "block"; - return false; + if (code === CLA_CONSENT_MISSING_CODE) return gateMode(policy.claGateMode ?? "off"); + return "off"; +} + +function isConfiguredGateBlocker(finding: AdvisoryFinding, policy: GateCheckPolicy): boolean { + return resolveConfiguredGateMode(finding, policy) === "block"; } // #8130: codes whose raw evaluated content must NEVER be captured into fired-event metadata. `secret_leak` @@ -1104,11 +1122,24 @@ export const RAW_CONTEXT_EXCLUDED_CODES = new Set(["secret_leak"]); export const RAW_CONTEXT_MAX_DIFF_CHARS = 120000; /** - * Record a {@link RuleFiredEvent} for every finding that `isConfiguredGateBlocker` would put into - * `configuredBlockers`, excluding `linked_issue_scope_mismatch` (#8104 / complements #8101). Call from the - * env-bearing gate path immediately after {@link evaluateGateCheck} with the SAME advisory + policy so the - * filter matches `evaluateGateCheckCore`'s own. Best-effort: a SignalStore failure never throws and never - * affects the gate verdict. + * Record a {@link RuleFiredEvent} for every configured-gate-class finding whose resolved gate mode is not + * "off" (#8760; originally block-mode-only per #8104), excluding `linked_issue_scope_mismatch` (#8101 owns + * it). Call from the env-bearing gate path immediately after {@link evaluateGateCheck} with the SAME + * advisory + policy so the mode resolution matches `evaluateGateCheckCore`'s own. Best-effort: a + * SignalStore failure never throws and never affects the gate verdict. + * + * #8760: pre-#8760 this recorded only findings that RESOLVED to "block" — but every gate defaults to + * advisory/off, so the majority of real traffic (advisory findings a human actually saw) recorded nothing + * and every downstream corpus was built from the biased block-mode-only minority. Now every non-"off" + * finding records, tagged `metadata.gateMode` ("block" | "advisory") so consumers can segment. + * Consumer-segmentation audit (per #8760's Requirements): every current consumer is DECIDED-case-driven — + * `buildBacktestCorpus` pairs fired events with override verdicts and drops unpaired fires; + * `computeRulePrecision`, the reliability curve, and the knob evaluators all run over that decided corpus; + * the rule-repeat alarm reads its own separately-namespaced composite ruleId (`:`, written by + * recordGateBlockersAndCheckRepeatAlarm) and never sees these events. Advisory-mode fires therefore sit + * inert (unpaired) until a matching override/confirmation exists — no existing metric shifts from this + * change alone; #8761's implicit confirmations are what will start pairing them, carrying their own + * `basis` tag for weighting. * * #8130: non-excluded codes also capture the raw context their detection actually evaluated, so their * corpora can backtest logic/detection changes rather than only thresholds: @@ -1131,14 +1162,17 @@ export async function recordConfiguredGateBlockerSignals( context: { aiReviewDiff?: string } = {}, ): Promise { const effective = applyMergeReadinessGate(policy); - const configuredBlockers = advisoryResult.findings.filter((finding) => isConfiguredGateBlocker(finding, effective)); + const recordable = advisoryResult.findings + .map((finding) => ({ finding, mode: resolveConfiguredGateMode(finding, effective) })) + .filter(({ finding, mode }) => mode !== "off" && finding.code !== "linked_issue_scope_mismatch"); const store = createSignalStore(env); const targetKey = `${repoFullName}#${prNumber}`; const occurredAt = nowIso(); await Promise.all( - configuredBlockers.map((finding) => { - if (finding.code === "linked_issue_scope_mismatch") return Promise.resolve(); - const metadata: Record = {}; + recordable.map(({ finding, mode }) => { + // gateMode always present (#8760): downstream consumers segment block-mode evidence (a bot action + // followed, a reversal is meaningful) from advisory-mode evidence (a human saw it, no bot action). + const metadata: Record = { gateMode: mode }; if (finding.confidence !== undefined) metadata.confidence = finding.confidence; if (!RAW_CONTEXT_EXCLUDED_CODES.has(finding.code)) { if (AI_JUDGMENT_BLOCKER_CODES.has(finding.code)) { @@ -1153,7 +1187,7 @@ export async function recordConfiguredGateBlockerSignals( targetKey, outcome: finding.severity ?? "blocker", occurredAt, - ...(Object.keys(metadata).length > 0 ? { metadata } : {}), + metadata, }) .catch(() => undefined); }), diff --git a/test/unit/configured-gate-blocker-signals.test.ts b/test/unit/configured-gate-blocker-signals.test.ts index c885609305..0174a24dd6 100644 --- a/test/unit/configured-gate-blocker-signals.test.ts +++ b/test/unit/configured-gate-blocker-signals.test.ts @@ -80,7 +80,7 @@ describe("recordConfiguredGateBlockerSignals (#8104)", () => { targetKey: "owner/repo#7", outcome: "critical", }); - expect(history.fired[0]?.metadata).toBeUndefined(); + expect(history.fired[0]?.metadata).toEqual({ gateMode: "block" }); }); it("records a fired signal for a deterministic code (secret_leak)", async () => { @@ -126,9 +126,8 @@ describe("recordConfiguredGateBlockerSignals (#8104)", () => { expect((await createSignalStore(env).queryRuleHistory("secret_leak", 0)).fired).toHaveLength(1); }); - it("records NO fired signal when isConfiguredGateBlocker returns false", async () => { + it("records an ADVISORY-mode finding too, tagged metadata.gateMode 'advisory' (#8760 — pre-#8760 this recorded nothing)", async () => { const env = createTestEnv(); - // missing_linked_issue defaults to advisory — not a configured blocker. await recordConfiguredGateBlockerSignals( env, advisory([finding({ code: "missing_linked_issue" })]), @@ -136,7 +135,55 @@ describe("recordConfiguredGateBlockerSignals (#8104)", () => { "owner/repo", 7, ); - expect((await createSignalStore(env).queryRuleHistory("missing_linked_issue", 0)).fired).toEqual([]); + const [fired] = (await createSignalStore(env).queryRuleHistory("missing_linked_issue", 0)).fired; + expect(fired).toBeDefined(); + expect((fired!.metadata as { gateMode: string }).gateMode).toBe("advisory"); + }); + + it("records NOTHING for a code whose gate resolves 'off' (#8760: off means the gate never evaluated, not silent-advisory)", async () => { + const env = createTestEnv(); + // manifest_missing_tests defaults to off; lockfile_tamper_risk defaults to off. + await recordConfiguredGateBlockerSignals( + env, + advisory([finding({ code: "manifest_missing_tests" }), finding({ code: "lockfile_tamper_risk" })]), + {}, + "owner/repo", + 7, + ); + expect((await createSignalStore(env).queryRuleHistory("manifest_missing_tests", 0)).fired).toEqual([]); + expect((await createSignalStore(env).queryRuleHistory("lockfile_tamper_risk", 0)).fired).toEqual([]); + }); + + it("tags a sub-floor AI finding demoted by advisory_only as gateMode 'advisory' — a real finding a human saw, never dropped (#8760)", async () => { + const env = createTestEnv(); + await recordConfiguredGateBlockerSignals( + env, + advisory([finding({ code: "ai_consensus_defect", confidence: 0.2 })]), + { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: "advisory_only", aiReviewCloseConfidence: 0.9 }, + "owner/repo", + 7, + ); + const [fired] = (await createSignalStore(env).queryRuleHistory("ai_consensus_defect", 0)).fired; + expect((fired!.metadata as { gateMode: string }).gateMode).toBe("advisory"); + }); + + it("composes manifest_linked_issue_required's mode as the stricter of its two gates (#8760: advisory when neither blocks but one is advisory; off when both off)", async () => { + const env = createTestEnv(); + // linkedIssueGateMode defaults to advisory -> resolved advisory (manifest off). + await recordConfiguredGateBlockerSignals(env, advisory([finding({ code: "manifest_linked_issue_required" })]), {}, "owner/repo", 7); + const [fired] = (await createSignalStore(env).queryRuleHistory("manifest_linked_issue_required", 0)).fired; + expect((fired!.metadata as { gateMode: string }).gateMode).toBe("advisory"); + + // Both gates explicitly off -> nothing records. + const envOff = createTestEnv(); + await recordConfiguredGateBlockerSignals( + envOff, + advisory([finding({ code: "manifest_linked_issue_required" })]), + { manifestPolicyGateMode: "off", linkedIssueGateMode: "off" }, + "owner/repo", + 7, + ); + expect((await createSignalStore(envOff).queryRuleHistory("manifest_linked_issue_required", 0)).fired).toEqual([]); }); it("uses outcome 'blocker' when finding.severity is missing (nullish coalescing arm)", async () => { @@ -181,7 +228,7 @@ describe("recordConfiguredGateBlockerSignals — raw context capture (#8130)", ( { aiReviewDiff: "+const key = 'AKIA-REAL-SECRET';" }, ); const [fired] = (await createSignalStore(env).queryRuleHistory("secret_leak", 0)).fired; - expect(fired!.metadata).toEqual({ confidence: 0.99 }); + expect(fired!.metadata).toEqual({ gateMode: "block", confidence: 0.99 }); expect(fired!.metadata).not.toHaveProperty("diff"); expect(fired!.metadata).not.toHaveProperty("rawSignal"); }); @@ -206,7 +253,7 @@ describe("recordConfiguredGateBlockerSignals — raw context capture (#8130)", ( const env = createTestEnv(); await recordConfiguredGateBlockerSignals(env, advisory([finding({ code: "ai_review_split", confidence: 0.9 })]), blockAi, "owner/repo", 7); const [fired] = (await createSignalStore(env).queryRuleHistory("ai_review_split", 0)).fired; - expect(fired!.metadata).toEqual({ confidence: 0.9 }); + expect(fired!.metadata).toEqual({ gateMode: "block", confidence: 0.9 }); }); it("captures a non-diff-based code's own evaluated signal (its detail) as rawSignal — the audited fallback", async () => { @@ -220,14 +267,14 @@ describe("recordConfiguredGateBlockerSignals — raw context capture (#8130)", ( { aiReviewDiff: "+irrelevant" }, ); const [fired] = (await createSignalStore(env).queryRuleHistory("missing_linked_issue", 0)).fired; - expect(fired!.metadata).toEqual({ rawSignal: "No linked issue reference found in the PR body." }); + expect(fired!.metadata).toEqual({ gateMode: "block", rawSignal: "No linked issue reference found in the PR body." }); }); - it("records no metadata at all for a non-diff code with no confidence and an empty detail", async () => { + it("records ONLY the gateMode tag for a non-diff code with no confidence and an empty detail (#8760: gateMode is always present)", async () => { const env = createTestEnv(); await recordConfiguredGateBlockerSignals(env, advisory([finding({ code: "missing_linked_issue", detail: "" })]), blockLinked, "owner/repo", 7); const [fired] = (await createSignalStore(env).queryRuleHistory("missing_linked_issue", 0)).fired; - expect(fired!.metadata).toBeUndefined(); + expect(fired!.metadata).toEqual({ gateMode: "block" }); }); it("still skips linked_issue_scope_mismatch entirely (#8101's own site records it)", async () => {