From d5b6325aa6d608067d5f516aedfe856b468a9af8 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 22 Jul 2026 21:50:34 -0700 Subject: [PATCH 1/4] feat(review): capture the model's raw response in linked_issue_scope_mismatch's fired-event metadata (#8139) The existing #8129 raw-context capture (issueText/prTitle/prBody/diff) rebuilds the prompt but never stored what the model actually returned, so a future logic backtest couldn't replay parseLinkedIssueSatisfactionOpinion/ buildLinkedIssueSatisfactionResult against real history -- only the prompt inputs, not the output they need to re-parse. Threads the raw text through runWorkersSatisfactionOpinion and the BYOK path, bounded at the capture site in processors.ts the same way the other raw-context fields are. Part of #8139 (logic/regex backtest CI check); the CI job itself is not yet implemented. --- src/queue/processors.ts | 6 ++- src/services/linked-issue-satisfaction-run.ts | 15 ++++--- src/services/linked-issue-satisfaction.ts | 5 +++ .../linked-issue-satisfaction-run.test.ts | 43 +++++++++++++++++-- 4 files changed, 59 insertions(+), 10 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 3db47fc2aa..16f280e029 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -492,7 +492,7 @@ import { buildSlopAssessment, type SlopBand } from "../signals/slop"; import { copycatWouldActOnPersistedScore } from "../signals/copycat"; import { buildStructuralImprovementAssessment } from "../signals/improvement"; import { runLoopOverLinkedIssueSatisfaction } from "../services/linked-issue-satisfaction-run"; -import { MAX_BODY_CHARS, MAX_DIFF_CHARS, MAX_ISSUE_TEXT_CHARS } from "../services/linked-issue-satisfaction"; +import { MAX_BODY_CHARS, MAX_DIFF_CHARS, MAX_ISSUE_TEXT_CHARS, MAX_MODEL_RESPONSE_CHARS } from "../services/linked-issue-satisfaction"; import { persistThresholdBacktestRuns, runThresholdBacktestAdvisory } from "../services/threshold-backtest-run"; import { thresholdBacktestBlock } from "../services/threshold-backtest"; import { decidePublicSurface } from "../signals/settings-preview"; @@ -7602,6 +7602,10 @@ export async function runLinkedIssueSatisfactionForAdvisory( prTitle: args.pr.title, prBody: (args.pr.body ?? "").trim().slice(0, MAX_BODY_CHARS), diff: diff.slice(0, MAX_DIFF_CHARS), + // #8139: the model's own raw response, so a future logic backtest can replay the deterministic + // parse/floor/sanitize step against the SAME text the original assessment actually saw -- the + // prompt-input fields above rebuild the prompt, but only this replays what came back from it. + ...(result.rawModelText ? { modelResponseText: result.rawModelText.trim().slice(0, MAX_MODEL_RESPONSE_CHARS) } : {}), }, }) .catch(() => undefined); diff --git a/src/services/linked-issue-satisfaction-run.ts b/src/services/linked-issue-satisfaction-run.ts index 6251db5f19..375d82c58d 100644 --- a/src/services/linked-issue-satisfaction-run.ts +++ b/src/services/linked-issue-satisfaction-run.ts @@ -49,7 +49,7 @@ export type LinkedIssueSatisfactionRunResult = | { status: "disabled"; reason: string } | { status: "unavailable"; reason: string } | { status: "quota_exceeded"; estimatedNeurons: number; remainingBudget: number } - | { status: "ok"; result: LinkedIssueSatisfactionResult | null; estimatedNeurons: number }; + | { status: "ok"; result: LinkedIssueSatisfactionResult | null; estimatedNeurons: number; rawModelText?: string }; const LINKED_ISSUE_SATISFACTION_MODELS = [BEST_REVIEW_MODELS[0], RELIABLE_FALLBACK_MODELS[0]] as const; const LINKED_ISSUE_SATISFACTION_ATTEMPTS_PER_MODEL = 3; @@ -58,7 +58,7 @@ const LINKED_ISSUE_SATISFACTION_MAX_CALLS = LINKED_ISSUE_SATISFACTION_MODELS.len type AiGatewayOptions = { gateway?: { id: string } }; type AiRunner = { run?: (model: string, options: Record, extra?: AiGatewayOptions) => Promise }; -type WorkersSatisfactionOpinionResult = { result: LinkedIssueSatisfactionResult | null; usage?: AiReviewActualUsage | undefined }; +type WorkersSatisfactionOpinionResult = { result: LinkedIssueSatisfactionResult | null; usage?: AiReviewActualUsage | undefined; rawText?: string | undefined }; /** One free/default-reviewer satisfaction opinion (whichever provider `env.AI` resolves to) with bounded * retry/fallback attempts, all pre-budgeted. Each attempt's raw text is validated via the pure module's own @@ -85,8 +85,9 @@ async function runWorkersSatisfactionOpinion( { max_tokens: maxTokens, temperature: 0, messages: [{ role: "system", content: system }, { role: "user", content: user }] }, extra, ); - const result = buildLinkedIssueSatisfactionResult(issueText, coerceAiText(raw)); - if (result) return { result, usage: coerceAiUsage(raw) }; + const text = coerceAiText(raw); + const result = buildLinkedIssueSatisfactionResult(issueText, text); + if (result) return { result, usage: coerceAiUsage(raw), rawText: text }; } catch (error) { if (isRateLimitError(error)) break; /* retry / fall through to fallback */ @@ -142,15 +143,17 @@ export async function runLoopOverLinkedIssueSatisfaction(env: Env, input: Linked // to null via buildLinkedIssueSatisfactionResult. let result: LinkedIssueSatisfactionResult | null; let usage: AiReviewActualUsage | undefined; + let rawModelText: string | undefined; if (input.providerKey) { const { text, usage: byokUsage } = await callAiProvider(input.providerKey, SATISFACTION_SYSTEM_PROMPT, user, maxTokens); result = text ? buildLinkedIssueSatisfactionResult(input.issueText, text) : null; usage = byokUsage; + rawModelText = text || undefined; } else { - ({ result, usage } = await runWorkersSatisfactionOpinion(env, input.issueText, SATISFACTION_SYSTEM_PROMPT, user, maxTokens)); + ({ result, usage, rawText: rawModelText } = await runWorkersSatisfactionOpinion(env, input.issueText, SATISFACTION_SYSTEM_PROMPT, user, maxTokens)); } await record(env, input, "ok", estimatedNeurons, result ? `advisory finding (${result.status})` : "no usable output", { status: result?.status ?? null, surfaced: Boolean(result), byok: Boolean(input.providerKey) }, usage); - return { status: "ok", result, estimatedNeurons }; + return { status: "ok", result, estimatedNeurons, ...(rawModelText ? { rawModelText } : {}) }; } async function record( diff --git a/src/services/linked-issue-satisfaction.ts b/src/services/linked-issue-satisfaction.ts index 5288cab0c7..83c5e6adca 100644 --- a/src/services/linked-issue-satisfaction.ts +++ b/src/services/linked-issue-satisfaction.ts @@ -34,6 +34,11 @@ const MAX_RATIONALE_LENGTH = 400; export const MAX_ISSUE_TEXT_CHARS = 6000; export const MAX_DIFF_CHARS = 60000; export const MAX_BODY_CHARS = 2000; +// #8139: bound for the model's own raw response text, captured alongside the other raw-context fields so a +// future logic backtest can replay parseLinkedIssueSatisfactionOpinion/buildLinkedIssueSatisfactionResult +// against the SAME text the original assessment actually parsed -- the prompt inputs alone (issueText/ +// prTitle/prBody/diff) are not enough to backtest the parse/floor/sanitize step, only to rebuild the prompt. +export const MAX_MODEL_RESPONSE_CHARS = 4000; export type LinkedIssueSatisfactionInput = { /** The already-fetched linked-issue title + body text (grounding already resolved this — see diff --git a/test/unit/linked-issue-satisfaction-run.test.ts b/test/unit/linked-issue-satisfaction-run.test.ts index 171dd92485..f5f2fd75bd 100644 --- a/test/unit/linked-issue-satisfaction-run.test.ts +++ b/test/unit/linked-issue-satisfaction-run.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { runLoopOverLinkedIssueSatisfaction, type LinkedIssueSatisfactionRunInput } from "../../src/services/linked-issue-satisfaction-run"; -import { MAX_BODY_CHARS, MAX_DIFF_CHARS } from "../../src/services/linked-issue-satisfaction"; +import { MAX_BODY_CHARS, MAX_DIFF_CHARS, MAX_MODEL_RESPONSE_CHARS } from "../../src/services/linked-issue-satisfaction"; import { BEST_REVIEW_MODELS, RELIABLE_FALLBACK_MODELS } from "../../src/services/ai-review"; import { buildAiReviewDiff, processJob, runLinkedIssueSatisfactionForAdvisory } from "../../src/queue/processors"; import { evaluateGateCheck } from "../../src/rules/advisory"; @@ -140,6 +140,24 @@ describe("runLoopOverLinkedIssueSatisfaction gating + fail-safe", () => { expect(result.estimatedNeurons).toBeGreaterThan(0); }); + it("also returns the model's own raw response text alongside the parsed result (#8139)", async () => { + const modelJson = satisfactionJson({ status: "addressed" }); + const run = vi.fn(async () => ({ response: modelJson })); + const result = await runLoopOverLinkedIssueSatisfaction(enabledEnv(run), baseInput); + expect(result.status).toBe("ok"); + if (result.status !== "ok") throw new Error("unreachable"); + expect(result.rawModelText).toBe(modelJson); + }); + + it("omits rawModelText (never a stray empty string) when no attempt produces a usable result (#8139)", async () => { + const run = vi.fn(async () => ({ response: "not json" })); + const result = await runLoopOverLinkedIssueSatisfaction(enabledEnv(run), baseInput); + expect(result.status).toBe("ok"); + if (result.status !== "ok") throw new Error("unreachable"); + expect(result.result).toBeNull(); + expect(result.rawModelText).toBeUndefined(); + }); + it("records the pre-budgeted retry/fallback estimate under the linked_issue_satisfaction feature", async () => { const run = vi.fn(async () => ({ response: "not json" })); const env = enabledEnv(run); @@ -482,9 +500,10 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" expect(history.overrides).toEqual([]); // firing alone is never an override }); - it("captures the bounded raw context (issueText/prTitle/prBody/diff) on the fired signal, truncating an over-limit body (#8129)", async () => { + it("captures the bounded raw context (issueText/prTitle/prBody/diff/modelResponseText) on the fired signal, truncating an over-limit body (#8129)", async () => { stubIssueFetch(); - const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) })); + const modelJson = satisfactionJson({ status: "unaddressed", confidence: 0.9 }); + const run = vi.fn(async () => ({ response: modelJson })); const env = enabledEnv(run); const longBody = "x".repeat(MAX_BODY_CHARS + 500); const longBodyPr = { ...pr, body: longBody }; @@ -501,6 +520,24 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" expect(metadata.prBody).toBe(longBody.slice(0, MAX_BODY_CHARS)); expect(metadata.prBody).toHaveLength(MAX_BODY_CHARS); expect(metadata.diff).toBe(buildAiReviewDiff(files).slice(0, MAX_DIFF_CHARS)); + // #8139: the model's own raw response is captured too, byte-identical to what was actually parsed — + // a future logic backtest replays parseLinkedIssueSatisfactionOpinion against exactly this text. + expect(metadata.modelResponseText).toBe(modelJson); + }); + + it("truncates an over-limit model response to MAX_MODEL_RESPONSE_CHARS on the fired signal (#8139)", async () => { + stubIssueFetch(); + // Padding lives inside the rationale string (still valid JSON + still parses to "unaddressed" above the + // floor) so the raw response text itself grows past the bound without changing the parsed verdict. + const modelJson = satisfactionJson({ status: "unaddressed", confidence: 0.9, rationale: "x".repeat(MAX_MODEL_RESPONSE_CHARS + 500) }); + const run = vi.fn(async () => ({ response: modelJson })); + 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); + const metadata = history.fired[0]?.metadata as Record; + expect(metadata.modelResponseText).toBe(modelJson.slice(0, MAX_MODEL_RESPONSE_CHARS)); + expect(metadata.modelResponseText).toHaveLength(MAX_MODEL_RESPONSE_CHARS); }); it("stores an empty prBody (not the string 'null'/'undefined') for a body-less PR (#8129)", async () => { From 992c32e52a492eb411dd56f2eaf2b6176c405d9c Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 22 Jul 2026 22:16:10 -0700 Subject: [PATCH 2/4] feat(ci): backtest logic/regex detection changes against recorded history in a dedicated CI job (#8139) A PR that rewrites detection logic (not just a threshold, #8138/#8142) now gets the same honest before/after backtest: a new path-filtered workflow checks out the PR's head AND base, replays linked_issue_scope_mismatch's captured raw context (#8129/#8130 + the model-response capture) through both versions of buildLinkedIssueSatisfactionResult, scores them with @loopover/engine's scoreBacktest/compareBacktestScores, posts its own advisory PR comment, and persists a calibration.logic_backtest_run audit event for #8140's track record. backtest-track-record.ts now aggregates both sibling event types, so #8105's Phase-2 decision reads threshold AND logic runs, not a partial record. Runs in CI, not ORB's Worker, because verifying a logic rewrite requires executing the PR's own code -- the same trust boundary validate-code already uses, and one the credential-holding Worker must never cross. Pure core (registry scoped to linked_issue_scope_mismatch, secret_leak permanently excluded per #8130, classify construction, comment/SQL rendering) is fully unit-tested; the CLI is thin IO glue per the export-d1-data.ts precedent. Advisory only -- never a required check, never blocks merge (#8105). --- .github/workflows/backtest-logic-check.yml | 130 +++++++++ scripts/backtest-logic-check-core.ts | 199 ++++++++++++++ scripts/backtest-logic-check.ts | 155 +++++++++++ scripts/backtest-track-record.ts | 17 +- test/unit/backtest-logic-check-core.test.ts | 278 ++++++++++++++++++++ 5 files changed, 772 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/backtest-logic-check.yml create mode 100644 scripts/backtest-logic-check-core.ts create mode 100644 scripts/backtest-logic-check.ts create mode 100644 test/unit/backtest-logic-check-core.test.ts diff --git a/.github/workflows/backtest-logic-check.yml b/.github/workflows/backtest-logic-check.yml new file mode 100644 index 0000000000..0268560559 --- /dev/null +++ b/.github/workflows/backtest-logic-check.yml @@ -0,0 +1,130 @@ +# Logic/regex-change backtest (#8139, epic #8082). When a PR touches the watched detection-logic paths, +# this job replays linked_issue_scope_mismatch's recorded raw-context history (#8129/#8130 + the #8139 +# model-response capture) through BOTH the PR's own head checkout and its base checkout — actually executing +# the two versions of the detection code, which is why this lives in CI and not in ORB's live Worker (the +# Worker holds credentials and must never execute PR-supplied logic; a CI checkout running a PR's own code +# is the exact trust boundary validate-code/validate-tests already use for every PR). Deliberately a +# separate workflow, not a ci.yml job: PRs that don't touch these paths pay nothing, and ones that do aren't +# slowed — this runs fully parallel to (and finishes long before) the ~11-minute test shards. +# Advisory only: never a required check, never blocks merge (#8105). It posts its OWN clearly-labeled PR +# comment, separate from ORB's unified review comment — see #8139's Boundaries for why. +name: backtest-logic + +on: + pull_request: + # Explicit list because the default (opened/synchronize/reopened) omits ready_for_review -- once the + # draft guard below skips draft PRs, marking a PR ready must itself trigger a real run (#6670). + # Mirrors selfhost.yml's pull_request.types comment/list exactly. + types: [opened, synchronize, reopened, ready_for_review] + # Exactly the paths whose changes can alter linked_issue_scope_mismatch-adjacent detection logic — + # see #8139's Design section; keep this list in sync with the issue's own spec. + paths: + - "src/rules/**" + - "src/review/content-lane/**" + - "src/settings/agent-actions.ts" + - "src/services/ai-review.ts" + - "src/services/linked-issue-satisfaction.ts" + +# Least privilege: the backtest only reads the repo; pull-requests: write is for its own advisory comment. +permissions: + contents: read + pull-requests: write + +concurrency: + # pull_request-only workflow, so one ref-scoped group suffices (no push/github.sha split like ci.yml + # needs): a newer push cancels the superseded run — its comment would be overwritten anyway. + group: backtest-logic-${{ github.ref }} + cancel-in-progress: true + +jobs: + backtest: + name: logic backtest (advisory) + # Skip draft PRs (#6670, anti-abuse — mirrors selfhost.yml's guard). Fork PRs are excluded at the job + # level rather than per-step: GitHub withholds repo secrets from fork-originated pull_request runs, so + # the D1 corpus read below is impossible there and the whole job (npm ci included) would be waste — + # the fork-notice job below is this workflow's half of ci.yml's paired fork==true/!=true convention. + if: ${{ github.event.pull_request.draft != true && github.event.pull_request.head.repo.fork != true }} + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + with: + persist-credentials: false + + # The PR's base commit, checked out INSIDE the head workspace: the dynamically imported base modules + # resolve bare npm specifiers by walking up from their own directory into the head checkout's + # node_modules, so one `npm ci` serves both sides of the comparison. + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + with: + ref: ${{ github.event.pull_request.base.sha }} + path: .backtest-base + persist-credentials: false + + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + with: + node-version-file: .nvmrc + cache: "npm" + + - name: Install deps + run: npm ci --ignore-scripts + + # The scripts import @loopover/engine, which resolves to its dist/ build output. + - name: Build engine package + run: npx turbo run build --filter=@loopover/engine + + # Read-only corpus export (#8084's CLI, reused as-is — no new D1 read code). The wrangler secrets are + # available here because the fork guard above already excluded fork-originated runs. + - name: Export corpus from D1 + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} + CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + run: npx tsx scripts/backtest-corpus-export.ts --rule-id linked_issue_scope_mismatch --output backtest-corpus.json --remote + + - name: Run logic backtest + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} + CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + npx tsx scripts/backtest-logic-check.ts \ + --rule-id linked_issue_scope_mismatch \ + --corpus backtest-corpus.json \ + --head-root . \ + --base-root .backtest-base \ + --output backtest-comment.md \ + --head-sha "$HEAD_SHA" \ + --base-sha "$BASE_SHA" \ + --persist --remote --db loopover \ + --repo "$GITHUB_REPOSITORY" \ + --pr "$PR_NUMBER" + + # Update-in-place keyed on the comment marker so a re-run edits the existing comment instead of + # stacking a new one per push. + - name: Post or update the PR comment + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + marker="" + # --paginate runs the --jq filter once per page, so pin to the first emitted id. + comment_id=$(gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" --paginate \ + --jq "[.[] | select(.body | contains(\"${marker}\")) | .id] | first // empty" | head -n 1) + if [ -n "$comment_id" ]; then + gh api "repos/${GITHUB_REPOSITORY}/issues/comments/${comment_id}" -X PATCH -F body=@backtest-comment.md + else + gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" -F body=@backtest-comment.md + fi + + # The fork half of ci.yml's paired fork==true/!=true convention: fork-originated pull_request runs get no + # repo secrets, so the D1-backed backtest cannot run — say so visibly instead of failing or going silent. + # Advisory only either way; a skipped backtest never blocks anything (#8105). + fork-notice: + name: logic backtest (skipped for fork PRs) + if: ${{ github.event.pull_request.draft != true && github.event.pull_request.head.repo.fork == true }} + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Explain the skip + run: echo "::notice::Fork PR — repo secrets are withheld, so the D1-backed logic backtest is skipped. Advisory only; nothing blocks." diff --git a/scripts/backtest-logic-check-core.ts b/scripts/backtest-logic-check-core.ts new file mode 100644 index 0000000000..1fad4a9e9b --- /dev/null +++ b/scripts/backtest-logic-check-core.ts @@ -0,0 +1,199 @@ +// Pure core for the logic/regex-change backtest CI check (#8139, epic #8082). A PR that rewrites a rule's +// detection logic (not just a threshold, #8138) gets replayed against the rule's real recorded history: the +// classify function feeds each BacktestCase's captured raw context (#8129/#8130, plus #8139's captured model +// response) through a caller-supplied detection function — dynamically imported by the CLI from the PR's own +// head/base checkouts — and the two resulting scores are compared with @loopover/engine's Pareto-floor +// discipline. No IO here — the CLI (backtest-logic-check.ts) does the dynamic imports, corpus read, D1 +// persist, and file writes — mirrors scripts/backtest-corpus-export-core.ts's identical pure-core / thin-IO +// split. +import { + compareBacktestScores, + renderBacktestComparison, + scoreBacktest, + type BacktestCase, + type BacktestComparison, +} from "@loopover/engine"; + +// Sibling of THRESHOLD_BACKTEST_EVENT_TYPE in src/services/threshold-backtest-run.ts (#8138's writer) — a +// distinct type so #8140's track-record reads can tell threshold runs and logic runs apart. Deliberately not +// imported from src/ (Worker-bound import graph); same hand-mirrored posture as backtest-track-record.ts. +export const LOGIC_BACKTEST_EVENT_TYPE = "calibration.logic_backtest_run"; + +// Hand-mirrors RAW_CONTEXT_EXCLUDED_CODES in src/rules/advisory.ts (Worker-bound; not imported — same +// posture as the event-type constant above). `secret_leak` is PERMANENTLY excluded (#8130): no raw context +// is ever captured for it by design, so there is nothing to honestly replay — and a registry entry for it +// would invite storing the very content #8130 exists to keep out of the audit trail. +export const LOGIC_BACKTEST_EXCLUDED_RULE_IDS = new Set(["secret_leak"]); + +/** One backtestable detection function: where it lives in a checkout and what export to load. The CLI + * imports `exportName` from `/` for BOTH the PR's head and base checkouts. */ +export type KnownLogicRule = { + filePath: string; + exportName: string; +}; + +// Mirrors #8138's KNOWN_THRESHOLDS registry shape (src/services/threshold-backtest.ts), keyed by ruleId. +// Scoped to exactly `linked_issue_scope_mismatch` (#8139): its corpus carries the richest raw context +// (issueText/prTitle/prBody/diff via #8129, modelResponseText via #8139), and only the DETERMINISTIC +// post-model step — buildLinkedIssueSatisfactionResult's parse/floor/sanitize — is honestly replayable +// (the prompt build and the model call itself are not reproducible from history). Generalizing to the +// other isConfiguredGateBlocker codes #8130 wires is explicit future scope, not this registry's job yet. +export const KNOWN_LOGIC_RULES: Record = { + linked_issue_scope_mismatch: { + filePath: "src/services/linked-issue-satisfaction.ts", + exportName: "buildLinkedIssueSatisfactionResult", + }, +}; + +/** Resolve a ruleId to its registry entry, failing loud on the permanently excluded `secret_leak` (with the + * #8130 rationale, so the error itself explains the boundary) and on any unregistered ruleId. */ +export function resolveKnownLogicRule(ruleId: string): KnownLogicRule { + if (LOGIC_BACKTEST_EXCLUDED_RULE_IDS.has(ruleId)) { + throw new Error(`rule ${ruleId} is permanently excluded from logic backtesting (#8130): no raw context is ever captured for it`); + } + const entry = KNOWN_LOGIC_RULES[ruleId]; + if (!entry) { + throw new Error(`unknown logic-backtest rule ${ruleId} (known: ${Object.keys(KNOWN_LOGIC_RULES).join(", ")})`); + } + return entry; +} + +/** The dynamically imported detection function's shape — buildLinkedIssueSatisfactionResult's own signature + * (issue text + raw model response in, `{ status }` or null out). A future registry entry must match this + * same shape or grow a per-entry adapter — the classify builder below assumes it. */ +export type LogicDetectionFn = (issueText: string | null | undefined, modelResponseText: string) => { status: string } | null; + +/** Keep only the cases a detection function can honestly be replayed against: both the captured issue text + * (#8129) and the captured model response (#8139) must be present and non-empty. Older corpus rows predate + * the model-response capture; replaying those would parse empty text and systematically predict "reversed" + * for baseline AND candidate — noise, not signal — so they are skipped and reported, never scored. */ +export function filterReplayableCases(cases: readonly BacktestCase[]): BacktestCase[] { + return cases.filter((backtestCase) => { + const metadata = backtestCase.metadata ?? {}; + return ( + typeof metadata.issueText === "string" && + metadata.issueText.trim() !== "" && + typeof metadata.modelResponseText === "string" && + metadata.modelResponseText.trim() !== "" + ); + }); +} + +/** + * Build a classify function that replays `detect` against a case's captured raw context. The corpus only + * contains actual firings (an "unaddressed" verdict that carried gate authority — see processors.ts's + * recordRuleFired site), so a candidate that reproduces the firing (`status === "unaddressed"`) predicts + * `"confirmed"` (the firing stands); any other outcome — no finding (null), "addressed"/"partial", or a + * thrown error — means the candidate would NOT have fired, predicting `"reversed"`. Same prediction + * semantics as buildConfidenceThresholdClassifier (#8138): candidate-would-not-fire ⇒ "reversed". The + * try/catch mirrors buildLinkedIssueSatisfactionResult's own never-throws fail-safe: a crashing candidate + * never fires, it does not abort the whole backtest. + */ +export function buildLogicClassifier(detect: LogicDetectionFn): (backtestCase: BacktestCase) => "reversed" | "confirmed" { + return (backtestCase) => { + const metadata = backtestCase.metadata ?? {}; + const issueText = typeof metadata.issueText === "string" ? metadata.issueText : ""; + const modelResponseText = typeof metadata.modelResponseText === "string" ? metadata.modelResponseText : ""; + let result: { status: string } | null; + try { + result = detect(issueText, modelResponseText); + } catch { + result = null; + } + return result?.status === "unaddressed" ? "confirmed" : "reversed"; + }; +} + +/** + * Backtest a logic/regex change to a single rule's detection function: score the base checkout's version and + * the head checkout's version as two classifiers over the same replayable corpus (`scoreBacktest`, #8085), + * then compare them with the Pareto-floor discipline (`compareBacktestScores`, #8086). Mirrors + * runThresholdBacktest's shape (packages/loopover-engine/src/calibration/backtest-threshold.ts) with detection + * functions in place of threshold numbers. The excluded-rule guard here is deliberate defense in depth on top + * of {@link resolveKnownLogicRule} — the scoring path itself refuses `secret_leak`, not just the registry. + */ +export function runLogicBacktest( + ruleId: string, + cases: readonly BacktestCase[], + baselineDetect: LogicDetectionFn, + candidateDetect: LogicDetectionFn, +): BacktestComparison { + if (LOGIC_BACKTEST_EXCLUDED_RULE_IDS.has(ruleId)) { + throw new Error(`rule ${ruleId} is permanently excluded from logic backtesting (#8130)`); + } + const baseline = scoreBacktest(ruleId, cases, buildLogicClassifier(baselineDetect)); + const candidate = scoreBacktest(ruleId, cases, buildLogicClassifier(candidateDetect)); + return compareBacktestScores(baseline, candidate); +} + +/** First line of the posted comment — the workflow's update step finds an existing comment by this marker so + * a re-run edits in place instead of stacking a new comment per push. */ +export const LOGIC_BACKTEST_COMMENT_MARKER = ""; + +/** + * Render the standalone advisory PR comment: marker, what was replayed against what, the engine's own + * comparison Markdown (#8088), and the never-blocks-merge note. Deliberately its OWN comment, not a section + * of ORB's unified review comment — see #8139's Boundaries (this CI job runs outside the Worker's review + * flow, and joining that comment would need new Worker↔CI coupling). + */ +export function renderLogicBacktestComment( + comparison: BacktestComparison, + info: { replayableCount: number; skippedCount: number; headSha: string; baseSha: string }, +): string { + const skippedNote = info.skippedCount > 0 ? ` ${info.skippedCount} historical case(s) lacked captured raw context and were skipped.` : ""; + return [ + LOGIC_BACKTEST_COMMENT_MARKER, + "## Logic backtest", + "", + `Replayed ${info.replayableCount} historical case(s) for \`${comparison.ruleId}\` through the base` + + ` (\`${info.baseSha.slice(0, 7)}\`) and head (\`${info.headSha.slice(0, 7)}\`) versions of its detection logic.${skippedNote}`, + "", + renderBacktestComparison(comparison), + "_Advisory only — this check never blocks merge (#8105)._", + "", + ].join("\n"); +} + +/** Single-quoted SQL string literal — mirrors backtest-corpus-export.ts's sqlStringLiteral exactly. */ +export function sqlStringLiteral(value: string): string { + return `'${value.replace(/'/g, "''")}'`; +} + +/** + * Build the INSERT persisting one run for #8140's track-record reads — a sibling row to #8138's + * THRESHOLD_BACKTEST_EVENT_TYPE events, same audit_events columns recordAuditEvent writes (the CLI runs + * outside the Worker, so it goes through `wrangler d1 execute` instead of the repositories module; the + * caller supplies id/createdAt so this stays clock-free like the rest of this file). `metadata.comparison` + * is the field backtest-track-record.ts's reader already looks for. + */ +export function buildLogicBacktestAuditInsertSql(input: { + id: string; + targetKey: string; + comparison: BacktestComparison; + headSha: string; + baseSha: string; + replayableCount: number; + skippedCount: number; + createdAt: string; +}): string { + const metadataJson = JSON.stringify({ + comparison: input.comparison, + headSha: input.headSha, + baseSha: input.baseSha, + replayableCount: input.replayableCount, + skippedCount: input.skippedCount, + }); + const values = [ + sqlStringLiteral(input.id), + sqlStringLiteral(LOGIC_BACKTEST_EVENT_TYPE), + "'loopover'", + sqlStringLiteral(input.targetKey), + // AuditEventRecord.outcome is a fixed enum — "completed" means "this run recorded successfully"; the + // real verdict lives in detail + metadata.comparison.verdict, mirroring persistThresholdBacktestRuns. + "'completed'", + sqlStringLiteral(`logic backtest for ${input.comparison.ruleId}: ${input.comparison.verdict}`), + sqlStringLiteral(metadataJson), + sqlStringLiteral(input.createdAt), + ].join(", "); + return `INSERT INTO audit_events (id, event_type, actor, target_key, outcome, detail, metadata_json, created_at) VALUES (${values})`; +} diff --git a/scripts/backtest-logic-check.ts b/scripts/backtest-logic-check.ts new file mode 100644 index 0000000000..7328744c04 --- /dev/null +++ b/scripts/backtest-logic-check.ts @@ -0,0 +1,155 @@ +#!/usr/bin/env node +// Logic/regex-change backtest CLI (#8139, epic #8082) — the CI-side runner .github/workflows/ +// backtest-logic-check.yml invokes. Loads a corpus manifest (produced by backtest-corpus-export.ts), +// dynamically imports the registered detection function from TWO checkout roots (the PR's head and its base +// — the dual checkout is what makes an honest before/after comparison possible), runs the pure core's +// scoring/comparison, writes the PR-comment Markdown, and optionally persists the run to D1 via +// `wrangler d1 execute` (the one write this epic's CI side performs — a sibling event to #8138's +// THRESHOLD_BACKTEST_EVENT_TYPE rows). All logic lives in backtest-logic-check-core.ts (unit-tested); this +// file is the thin IO wrapper — mirrors backtest-corpus-export.ts's identical split. +// +// tsx scripts/backtest-logic-check.ts --rule-id --corpus \ +// --head-root --base-root --output \ +// [--head-sha ] [--base-sha ] \ +// [--persist --repo --pr [--db loopover] [--remote]] +import { readFileSync, writeFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { pathToFileURL } from "node:url"; +import { spawnSync } from "node:child_process"; +import type { BacktestCase } from "@loopover/engine"; +import { checksumCases } from "./backtest-corpus-export-core.js"; +import { + buildLogicBacktestAuditInsertSql, + filterReplayableCases, + renderLogicBacktestComment, + resolveKnownLogicRule, + runLogicBacktest, + type LogicDetectionFn, +} from "./backtest-logic-check-core.js"; + +type Args = { + ruleId: string | undefined; + corpus: string | undefined; + headRoot: string | undefined; + baseRoot: string | undefined; + output: string | undefined; + headSha: string; + baseSha: string; + persist: boolean; + repo: string | undefined; + pr: string | undefined; + db: string; + remote: boolean; +}; + +function parseArgs(argv: string[]): Args { + const args: Args = { + ruleId: undefined, + corpus: undefined, + headRoot: undefined, + baseRoot: undefined, + output: undefined, + headSha: "", + baseSha: "", + persist: false, + repo: undefined, + pr: undefined, + db: "loopover", + remote: false, + }; + for (let i = 0; i < argv.length; i += 1) { + const flag = argv[i]; + if (flag === "--persist") args.persist = true; + else if (flag === "--remote") args.remote = true; + else if (flag === "--rule-id") args.ruleId = argv[++i]; + else if (flag === "--corpus") args.corpus = argv[++i]; + else if (flag === "--head-root") args.headRoot = argv[++i]; + else if (flag === "--base-root") args.baseRoot = argv[++i]; + else if (flag === "--output") args.output = argv[++i]; + else if (flag === "--head-sha") args.headSha = argv[++i] ?? ""; + else if (flag === "--base-sha") args.baseSha = argv[++i] ?? ""; + else if (flag === "--repo") args.repo = argv[++i]; + else if (flag === "--pr") args.pr = argv[++i]; + else if (flag === "--db") args.db = argv[++i]!; + } + return args; +} + +// Import the registered detection function from one checkout root. tsx compiles the checkout's own TS on +// the fly; bare npm specifiers inside it resolve by walking up from the file's directory, so a base checkout +// nested INSIDE the head workspace shares the head's node_modules (see the workflow's checkout layout). +async function importDetectionFn(checkoutRoot: string, filePath: string, exportName: string): Promise { + const moduleUrl = pathToFileURL(resolve(checkoutRoot, filePath)).href; + const imported = (await import(moduleUrl)) as Record; + const fn = imported[exportName]; + if (typeof fn !== "function") { + throw new Error(`${filePath} in ${checkoutRoot} has no function export named ${exportName}`); + } + return fn as LogicDetectionFn; +} + +async function main() { + const args = parseArgs(process.argv.slice(2)); + if (!args.ruleId || !args.corpus || !args.headRoot || !args.baseRoot || !args.output || (args.persist && (!args.repo || !args.pr))) { + console.error( + "Usage: tsx scripts/backtest-logic-check.ts --rule-id --corpus --head-root --base-root " + + " --output [--head-sha ] [--base-sha ] [--persist --repo --pr [--db loopover] [--remote]]", + ); + process.exit(2); + } + + const entry = resolveKnownLogicRule(args.ruleId); + + const manifest = JSON.parse(readFileSync(args.corpus, "utf8")) as { ruleId: string; checksum: string; cases: BacktestCase[] }; + if (manifest.ruleId !== args.ruleId) { + throw new Error(`corpus manifest is for rule ${manifest.ruleId}, not ${args.ruleId}`); + } + if (checksumCases(manifest.cases) !== manifest.checksum) { + throw new Error(`corpus manifest checksum mismatch — re-export with backtest-corpus-export.ts`); + } + + const baselineDetect = await importDetectionFn(args.baseRoot, entry.filePath, entry.exportName); + const candidateDetect = await importDetectionFn(args.headRoot, entry.filePath, entry.exportName); + + const replayable = filterReplayableCases(manifest.cases); + const skippedCount = manifest.cases.length - replayable.length; + const comparison = runLogicBacktest(args.ruleId, replayable, baselineDetect, candidateDetect); + + writeFileSync( + args.output, + renderLogicBacktestComment(comparison, { + replayableCount: replayable.length, + skippedCount, + headSha: args.headSha, + baseSha: args.baseSha, + }), + ); + + if (args.persist) { + const sql = buildLogicBacktestAuditInsertSql({ + id: crypto.randomUUID(), + targetKey: `${args.repo}#${args.pr}`, + comparison, + headSha: args.headSha, + baseSha: args.baseSha, + replayableCount: replayable.length, + skippedCount, + createdAt: new Date().toISOString(), + }); + const result = spawnSync("npx", ["wrangler", "d1", "execute", args.db, args.remote ? "--remote" : "--local", "--json", "--command", sql], { + encoding: "utf8", + maxBuffer: 256 * 1024 * 1024, + }); + if (result.status !== 0) { + // Best-effort, mirroring persistThresholdBacktestRuns's .catch(() => undefined): a persistence failure + // must never fail the advisory check that produced the comparison — the comment still posts. + console.error(`warning: audit-event persist failed (${result.status}): ${(result.stderr || result.stdout || "").slice(0, 500)}`); + } + } + + console.error( + `logic backtest for ${args.ruleId}: ${comparison.verdict} (${replayable.length} replayed, ${skippedCount} skipped) → ${args.output}`, + ); +} + +await main(); diff --git a/scripts/backtest-track-record.ts b/scripts/backtest-track-record.ts index 742812ce0e..a4dbf4bc69 100644 --- a/scripts/backtest-track-record.ts +++ b/scripts/backtest-track-record.ts @@ -1,20 +1,23 @@ #!/usr/bin/env node // Read-only D1 → REGRESSED-verdict track-record summary (#8140, epic #8082). Reads the BacktestComparison -// results the advisory backtest CI check persists (#8138) out of audit_events via `wrangler d1 execute -// --json`, aggregates them with the pure computeRegressedVerdictTrackRecord (@loopover/engine), and prints -// the summary #8105's Phase-2 merge-gating decision needs. The aggregation lives in the engine (pure, -// unit-tested); this file is the thin IO wrapper — mirrors backtest-corpus-export.ts's identical split. +// results the advisory backtests persist — #8138's ORB-native threshold runs AND #8139's CI-side logic +// runs, sibling event types with the same metadata.comparison shape — out of audit_events via `wrangler d1 +// execute --json`, aggregates them with the pure computeRegressedVerdictTrackRecord (@loopover/engine), and +// prints the summary #8105's Phase-2 merge-gating decision needs. The aggregation lives in the engine +// (pure, unit-tested); this file is the thin IO wrapper — mirrors backtest-corpus-export.ts's identical split. // // tsx scripts/backtest-track-record.ts --db loopover [--remote] // // --remote reads the deployed D1 (default is the local miniflare DB). NEVER pass a write command. import { spawnSync } from "node:child_process"; import { computeRegressedVerdictTrackRecord, type BacktestComparison } from "@loopover/engine"; +import { LOGIC_BACKTEST_EVENT_TYPE } from "./backtest-logic-check-core.js"; // Mirrors THRESHOLD_BACKTEST_EVENT_TYPE in src/services/threshold-backtest-run.ts (#8138's writer) and must // be kept in sync with it by hand — that module is Worker-bound (D1 repositories import graph) and // deliberately not imported into this standalone script, the same posture backtest-corpus-export.ts takes -// toward signal-tracking-wire's private helpers. +// toward signal-tracking-wire's private helpers. The logic-run sibling (#8139) lives in a scripts-local +// module with no Worker-bound imports, so THAT one is imported for real rather than hand-mirrored. const THRESHOLD_BACKTEST_EVENT_TYPE = "calibration.threshold_backtest_run"; type Args = { db: string | undefined; remote: boolean }; @@ -52,7 +55,7 @@ function main() { const rows = d1Query( args.db, args.remote, - `SELECT metadata_json FROM audit_events WHERE event_type = '${THRESHOLD_BACKTEST_EVENT_TYPE}' ORDER BY created_at ASC`, + `SELECT metadata_json FROM audit_events WHERE event_type IN ('${THRESHOLD_BACKTEST_EVENT_TYPE}', '${LOGIC_BACKTEST_EVENT_TYPE}') ORDER BY created_at ASC`, ); const comparisons: BacktestComparison[] = []; for (const row of rows) { @@ -65,7 +68,7 @@ function main() { } } const record = computeRegressedVerdictTrackRecord(comparisons); - console.log(`Backtest CI track record: ${record.totalRuns} run(s), ${record.regressedRuns} REGRESSED`); + console.log(`Backtest track record (threshold + logic runs): ${record.totalRuns} run(s), ${record.regressedRuns} REGRESSED`); console.log(`REGRESSED rate: ${record.regressedRate === null ? "N/A (no runs yet)" : record.regressedRate.toFixed(3)}`); for (const [ruleId, bucket] of record.perRule) { console.log(` ${ruleId}: total=${bucket.total} regressed=${bucket.regressed} improved=${bucket.improved} unchanged=${bucket.unchanged}`); diff --git a/test/unit/backtest-logic-check-core.test.ts b/test/unit/backtest-logic-check-core.test.ts new file mode 100644 index 0000000000..c938381bc7 --- /dev/null +++ b/test/unit/backtest-logic-check-core.test.ts @@ -0,0 +1,278 @@ +import { describe, expect, it } from "vitest"; +import type { BacktestCase } from "@loopover/engine"; +import { + buildLogicBacktestAuditInsertSql, + buildLogicClassifier, + filterReplayableCases, + KNOWN_LOGIC_RULES, + LOGIC_BACKTEST_COMMENT_MARKER, + LOGIC_BACKTEST_EVENT_TYPE, + LOGIC_BACKTEST_EXCLUDED_RULE_IDS, + renderLogicBacktestComment, + resolveKnownLogicRule, + runLogicBacktest, + sqlStringLiteral, + type LogicDetectionFn, +} from "../../scripts/backtest-logic-check-core.js"; + +const RULE_ID = "linked_issue_scope_mismatch"; + +function sampleCase(overrides: Partial = {}): BacktestCase { + return { + ruleId: RULE_ID, + targetKey: "owner/repo#1", + outcome: "unaddressed", + label: "confirmed", + firedAt: "2026-07-22T00:00:00.000Z", + decidedAt: "2026-07-22T01:00:00.000Z", + metadata: { + issueText: "fix the flaky retry in the queue consumer", + modelResponseText: '{"status":"unaddressed","rationale":"the diff never touches the queue consumer","confidence":0.9}', + }, + ...overrides, + }; +} + +/** A detection double that fires (status "unaddressed") exactly when the model response contains `needle`. */ +function detectWhenResponseContains(needle: string): LogicDetectionFn { + return (_issueText, modelResponseText) => + modelResponseText.includes(needle) ? { status: "unaddressed" } : null; +} + +describe("backtest-logic-check-core registry (#8139)", () => { + it("scopes KNOWN_LOGIC_RULES to exactly linked_issue_scope_mismatch's deterministic post-model step", () => { + expect(Object.keys(KNOWN_LOGIC_RULES)).toEqual([RULE_ID]); + expect(KNOWN_LOGIC_RULES[RULE_ID]).toEqual({ + filePath: "src/services/linked-issue-satisfaction.ts", + exportName: "buildLinkedIssueSatisfactionResult", + }); + }); + + it("resolveKnownLogicRule returns the registry entry for a known rule", () => { + expect(resolveKnownLogicRule(RULE_ID)).toBe(KNOWN_LOGIC_RULES[RULE_ID]); + }); + + it("resolveKnownLogicRule rejects secret_leak permanently, with the #8130 rationale", () => { + expect(LOGIC_BACKTEST_EXCLUDED_RULE_IDS.has("secret_leak")).toBe(true); + expect(() => resolveKnownLogicRule("secret_leak")).toThrow(/permanently excluded.*#8130/); + }); + + it("resolveKnownLogicRule rejects an unregistered rule and names the known ones", () => { + expect(() => resolveKnownLogicRule("missing_linked_issue")).toThrow(/unknown logic-backtest rule missing_linked_issue.*linked_issue_scope_mismatch/); + }); + + it("uses a distinct sibling event type to #8138's threshold runs", () => { + expect(LOGIC_BACKTEST_EVENT_TYPE).toBe("calibration.logic_backtest_run"); + expect(LOGIC_BACKTEST_EVENT_TYPE).not.toBe("calibration.threshold_backtest_run"); + }); +}); + +describe("backtest-logic-check-core filterReplayableCases (#8139)", () => { + it("keeps a case carrying both non-empty issueText and non-empty modelResponseText", () => { + const replayable = sampleCase(); + expect(filterReplayableCases([replayable])).toEqual([replayable]); + }); + + it("drops cases with no metadata at all", () => { + const bare: BacktestCase = { ruleId: RULE_ID, targetKey: "owner/repo#2", outcome: "unaddressed", label: "reversed", firedAt: "2026-07-22T00:00:00.000Z", decidedAt: "2026-07-22T01:00:00.000Z" }; + expect(filterReplayableCases([bare])).toEqual([]); + }); + + it("drops cases whose issueText is missing, empty/whitespace, or not a string", () => { + expect(filterReplayableCases([sampleCase({ metadata: { modelResponseText: "{}" } })])).toEqual([]); + expect(filterReplayableCases([sampleCase({ metadata: { issueText: " ", modelResponseText: "{}" } })])).toEqual([]); + expect(filterReplayableCases([sampleCase({ metadata: { issueText: 42, modelResponseText: "{}" } })])).toEqual([]); + }); + + it("drops pre-#8139 cases whose modelResponseText was never captured, empty, or not a string", () => { + expect(filterReplayableCases([sampleCase({ metadata: { issueText: "fix the bug" } })])).toEqual([]); + expect(filterReplayableCases([sampleCase({ metadata: { issueText: "fix the bug", modelResponseText: " " } })])).toEqual([]); + expect(filterReplayableCases([sampleCase({ metadata: { issueText: "fix the bug", modelResponseText: 7 } })])).toEqual([]); + }); +}); + +describe("backtest-logic-check-core buildLogicClassifier (#8139)", () => { + it("predicts confirmed when the detection reproduces the firing (status unaddressed)", () => { + const classify = buildLogicClassifier(() => ({ status: "unaddressed" })); + expect(classify(sampleCase())).toBe("confirmed"); + }); + + it("predicts reversed when the detection yields no finding (null)", () => { + const classify = buildLogicClassifier(() => null); + expect(classify(sampleCase())).toBe("reversed"); + }); + + it("predicts reversed for a non-firing verdict (addressed/partial)", () => { + expect(buildLogicClassifier(() => ({ status: "addressed" }))(sampleCase())).toBe("reversed"); + expect(buildLogicClassifier(() => ({ status: "partial" }))(sampleCase())).toBe("reversed"); + }); + + it("predicts reversed when the candidate detection throws — a crashing candidate never fires", () => { + const classify = buildLogicClassifier(() => { + throw new Error("candidate bug"); + }); + expect(classify(sampleCase())).toBe("reversed"); + }); + + it("feeds the case's captured issueText and modelResponseText to the detection function", () => { + const seen: Array<[string | null | undefined, string]> = []; + const classify = buildLogicClassifier((issueText, modelResponseText) => { + seen.push([issueText, modelResponseText]); + return null; + }); + const backtestCase = sampleCase(); + classify(backtestCase); + expect(seen).toEqual([[backtestCase.metadata!.issueText, backtestCase.metadata!.modelResponseText]]); + }); + + it("degrades absent metadata and non-string context fields to empty strings instead of crashing", () => { + const seen: Array<[string | null | undefined, string]> = []; + const classify = buildLogicClassifier((issueText, modelResponseText) => { + seen.push([issueText, modelResponseText]); + return null; + }); + const { metadata: _dropped, ...noMetadata } = sampleCase(); + classify(noMetadata); + classify(sampleCase({ metadata: { issueText: 42, modelResponseText: 7 } })); + expect(seen).toEqual([ + ["", ""], + ["", ""], + ]); + }); +}); + +describe("backtest-logic-check-core runLogicBacktest (#8139)", () => { + const cases: BacktestCase[] = [ + // A confirmed firing whose stored model response fires either version — a shared true negative... for + // the "reversed"-positive convention this is a case both classifiers should predict "confirmed" on. + sampleCase({ targetKey: "owner/repo#1", label: "confirmed" }), + // A reversed firing (the rule was wrong) whose response only trips the OLD logic: the new logic + // correctly declines to fire, converting a baseline miss into a candidate true positive. + sampleCase({ + targetKey: "owner/repo#2", + label: "reversed", + metadata: { issueText: "tighten the docs wording", modelResponseText: '{"status":"unaddressed","rationale":"OLD_ONLY match","confidence":0.4}' }, + }), + ]; + const oldDetect = detectWhenResponseContains('"status":"unaddressed"'); + const newDetect: LogicDetectionFn = (issueText, modelResponseText) => + modelResponseText.includes("OLD_ONLY") ? null : oldDetect(issueText, modelResponseText); + + it("scores base vs head over the same corpus and reports an improvement when the new logic fixes a reversed case", () => { + const comparison = runLogicBacktest(RULE_ID, cases, oldDetect, newDetect); + expect(comparison.ruleId).toBe(RULE_ID); + // Baseline fires on both cases: the reversed one is a false negative (predicted confirmed, was reversed). + expect(comparison.baseline.recall).toBe(0); + // Candidate declines the OLD_ONLY case: true positive, recall 1, precision 1 — improved, nothing regressed. + expect(comparison.candidate.recall).toBe(1); + expect(comparison.verdict).toBe("improved"); + expect(comparison.regressedAxes).toEqual([]); + }); + + it("reports regressed when the new logic loses a case the old logic got right", () => { + const comparison = runLogicBacktest(RULE_ID, cases, newDetect, oldDetect); + expect(comparison.verdict).toBe("regressed"); + expect(comparison.regressedAxes).toContain("recall"); + }); + + it("reports unchanged when both versions classify identically", () => { + const comparison = runLogicBacktest(RULE_ID, cases, oldDetect, oldDetect); + expect(comparison.verdict).toBe("unchanged"); + }); + + it("refuses secret_leak even when called directly, bypassing the registry", () => { + expect(() => runLogicBacktest("secret_leak", [], oldDetect, newDetect)).toThrow(/permanently excluded.*#8130/); + }); +}); + +describe("backtest-logic-check-core renderLogicBacktestComment (#8139)", () => { + const comparison = runLogicBacktest(RULE_ID, [sampleCase()], detectWhenResponseContains('"status":"unaddressed"'), detectWhenResponseContains('"status":"unaddressed"')); + + it("leads with the update-in-place marker and labels itself, the shas, and the advisory guarantee", () => { + const comment = renderLogicBacktestComment(comparison, { + replayableCount: 12, + skippedCount: 0, + headSha: "abcdef1234567890", + baseSha: "1234567890abcdef", + }); + expect(comment.startsWith(`${LOGIC_BACKTEST_COMMENT_MARKER}\n## Logic backtest`)).toBe(true); + expect(comment).toContain("Replayed 12 historical case(s) for `linked_issue_scope_mismatch`"); + expect(comment).toContain("base (`1234567`) and head (`abcdef1`)"); + expect(comment).toContain("### Backtest comparison: `linked_issue_scope_mismatch`"); + expect(comment).toContain("never blocks merge (#8105)"); + expect(comment).not.toContain("lacked captured raw context"); + }); + + it("reports skipped non-replayable cases only when there are any", () => { + const comment = renderLogicBacktestComment(comparison, { + replayableCount: 3, + skippedCount: 5, + headSha: "abcdef1234567890", + baseSha: "1234567890abcdef", + }); + expect(comment).toContain("5 historical case(s) lacked captured raw context and were skipped."); + }); +}); + +describe("backtest-logic-check-core buildLogicBacktestAuditInsertSql (#8139)", () => { + const comparison = runLogicBacktest(RULE_ID, [sampleCase()], detectWhenResponseContains('"status":"unaddressed"'), detectWhenResponseContains('"status":"unaddressed"')); + + it("escapes embedded single quotes SQL-style", () => { + expect(sqlStringLiteral("it's")).toBe("'it''s'"); + expect(sqlStringLiteral("plain")).toBe("'plain'"); + }); + + it("builds a full audit_events INSERT carrying the comparison under metadata.comparison", () => { + const sql = buildLogicBacktestAuditInsertSql({ + id: "run-id-1", + targetKey: "JSONbored/loopover#8139", + comparison, + headSha: "abcdef1234567890", + baseSha: "1234567890abcdef", + replayableCount: 1, + skippedCount: 2, + createdAt: "2026-07-22T12:00:00.000Z", + }); + expect(sql).toMatch(/^INSERT INTO audit_events \(id, event_type, actor, target_key, outcome, detail, metadata_json, created_at\) VALUES \(/); + expect(sql).toContain(`'${LOGIC_BACKTEST_EVENT_TYPE}'`); + expect(sql).toContain("'loopover'"); + expect(sql).toContain("'JSONbored/loopover#8139'"); + expect(sql).toContain("'completed'"); + expect(sql).toContain("'logic backtest for linked_issue_scope_mismatch: unchanged'"); + expect(sql).toContain("'2026-07-22T12:00:00.000Z'"); + // The metadata JSON survives the SQL round-trip: undo the '' escaping and parse it back out. + const literal = /VALUES \(.*'completed', '[^']*', '(.*)', '2026-07-22T12:00:00\.000Z'\)$/.exec(sql)![1]!; + const metadata = JSON.parse(literal.replace(/''/g, "'")) as Record; + expect(metadata.comparison).toEqual(comparison); + expect(metadata.headSha).toBe("abcdef1234567890"); + expect(metadata.baseSha).toBe("1234567890abcdef"); + expect(metadata.replayableCount).toBe(1); + expect(metadata.skippedCount).toBe(2); + }); +}); + +describe("backtest-logic-check-core end-to-end against the real detection module (#8139)", () => { + it("replays the real buildLinkedIssueSatisfactionResult as both sides and stays unchanged", async () => { + // The same dynamic-import shape the CLI performs against a checkout root — here the repo itself. + const imported = (await import("../../src/services/linked-issue-satisfaction")) as Record; + const detect = imported[KNOWN_LOGIC_RULES[RULE_ID]!.exportName] as LogicDetectionFn; + expect(typeof detect).toBe("function"); + const cases = [ + sampleCase({ label: "confirmed" }), + sampleCase({ + targetKey: "owner/repo#3", + label: "reversed", + // Below the 0.5 confidence floor: the real parse/floor step drops this "unaddressed" call, so the + // replay predicts "reversed" — a true positive under the corpus's own label. + metadata: { issueText: "fix the bug", modelResponseText: '{"status":"unaddressed","rationale":"weak guess","confidence":0.2}' }, + }), + ]; + const comparison = runLogicBacktest(RULE_ID, filterReplayableCases(cases), detect, detect); + expect(comparison.verdict).toBe("unchanged"); + expect(comparison.baseline.caseCount).toBe(2); + expect(comparison.baseline.truePositive).toBe(1); + expect(comparison.baseline.trueNegative).toBe(1); + expect(comparison.baseline.precision).toBe(1); + expect(comparison.baseline.recall).toBe(1); + }); +}); From 0d20158d04820e53f1cc0fdc4282529f294a6e1c Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 22 Jul 2026 22:36:19 -0700 Subject: [PATCH 3/4] =?UTF-8?q?fix(ci):=20fail=20the=20logic=20backtest=20?= =?UTF-8?q?open=20=E2=80=94=20an=20advisory=20job=20must=20never=20red-X?= =?UTF-8?q?=20a=20PR=20on=20infra=20errors=20(#8139)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First live run surfaced two problems at once: the repo's CLOUDFLARE_API_TOKEN is scoped Workers Scripts:Edit only (no D1 access), so the corpus export died with Cloudflare 7403 -- and that red check is not harmless, because the review engine auto-closes contributor PRs on ANY failed check, required or not. An advisory job that can go red on an under-scoped token, a D1 outage, or a comment-post hiccup would let our own plumbing close innocent contributor PRs touching the watched paths. Every D1/comment step now catches its own failure, emits a ::notice explaining exactly what was skipped and why, and keeps the job green -- 'never blocks merge' (#8105) now holds against the job's own failures, not just its verdicts. Enabling the backtest for real needs a D1-scoped token in the CLOUDFLARE_API_TOKEN secret; until then the job is a visible no-op. --- .github/workflows/backtest-logic-check.yml | 48 +++++++++++++++++----- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/.github/workflows/backtest-logic-check.yml b/.github/workflows/backtest-logic-check.yml index 0268560559..ad3d3a7047 100644 --- a/.github/workflows/backtest-logic-check.yml +++ b/.github/workflows/backtest-logic-check.yml @@ -72,15 +72,30 @@ jobs: - name: Build engine package run: npx turbo run build --filter=@loopover/engine + # Every step from here down FAILS OPEN (notice + green, never a red check): the review engine + # auto-closes a contributor PR on ANY failed check, required or not — so an advisory job that can go + # red on an infra problem (an under-scoped CLOUDFLARE_API_TOKEN, a D1 outage, a comment-post hiccup) + # would let OUR plumbing close an innocent contributor's PR. "Never blocks merge" (#8105) has to hold + # against this job's own failures, not just its verdicts. + # # Read-only corpus export (#8084's CLI, reused as-is — no new D1 read code). The wrangler secrets are # available here because the fork guard above already excluded fork-originated runs. - name: Export corpus from D1 + id: corpus env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} - run: npx tsx scripts/backtest-corpus-export.ts --rule-id linked_issue_scope_mismatch --output backtest-corpus.json --remote + run: | + if npx tsx scripts/backtest-corpus-export.ts --rule-id linked_issue_scope_mismatch --output backtest-corpus.json --remote; then + echo "available=true" >> "$GITHUB_OUTPUT" + else + echo "available=false" >> "$GITHUB_OUTPUT" + echo "::notice::Corpus export from D1 failed (missing or under-scoped CLOUDFLARE_API_TOKEN — it needs D1 read/write on the loopover database). Logic backtest skipped; advisory only, never fails the PR." + fi - name: Run logic backtest + id: backtest + if: ${{ steps.corpus.outputs.available == 'true' }} env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} @@ -88,7 +103,7 @@ jobs: BASE_SHA: ${{ github.event.pull_request.base.sha }} PR_NUMBER: ${{ github.event.pull_request.number }} run: | - npx tsx scripts/backtest-logic-check.ts \ + if npx tsx scripts/backtest-logic-check.ts \ --rule-id linked_issue_scope_mismatch \ --corpus backtest-corpus.json \ --head-root . \ @@ -98,23 +113,34 @@ jobs: --base-sha "$BASE_SHA" \ --persist --remote --db loopover \ --repo "$GITHUB_REPOSITORY" \ - --pr "$PR_NUMBER" + --pr "$PR_NUMBER"; then + echo "ready=true" >> "$GITHUB_OUTPUT" + else + echo "ready=false" >> "$GITHUB_OUTPUT" + echo "::notice::Logic backtest run failed — no comparison produced. Advisory only, never fails the PR." + fi # Update-in-place keyed on the comment marker so a re-run edits the existing comment instead of # stacking a new one per push. - name: Post or update the PR comment + if: ${{ steps.backtest.outputs.ready == 'true' }} env: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number }} run: | - marker="" - # --paginate runs the --jq filter once per page, so pin to the first emitted id. - comment_id=$(gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" --paginate \ - --jq "[.[] | select(.body | contains(\"${marker}\")) | .id] | first // empty" | head -n 1) - if [ -n "$comment_id" ]; then - gh api "repos/${GITHUB_REPOSITORY}/issues/comments/${comment_id}" -X PATCH -F body=@backtest-comment.md - else - gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" -F body=@backtest-comment.md + post_comment() { + marker="" + # --paginate runs the --jq filter once per page, so pin to the first emitted id. + comment_id=$(gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" --paginate \ + --jq "[.[] | select(.body | contains(\"${marker}\")) | .id] | first // empty" | head -n 1) + if [ -n "$comment_id" ]; then + gh api "repos/${GITHUB_REPOSITORY}/issues/comments/${comment_id}" -X PATCH -F body=@backtest-comment.md + else + gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" -F body=@backtest-comment.md + fi + } + if ! post_comment; then + echo "::notice::PR comment post failed — backtest result computed and persisted but not posted. Advisory only, never fails the PR." fi # The fork half of ci.yml's paired fork==true/!=true convention: fork-originated pull_request runs get no From 7ff11c6f5bfe0b1d29d2844c9236a967ab50902e Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 22 Jul 2026 22:43:22 -0700 Subject: [PATCH 4/4] feat(ci): persist the corpus checksum with each logic-backtest run as its reproducibility freeze point (#8139) The persisted run recorded the comparison and both shas but not WHICH corpus snapshot it scored -- without that, a skeptical re-run can't prove it replayed the same history. The manifest's own checksum (#8084) is already in hand at run time; thread it into the persisted metadata and the PR comment, so checksum + head/base shas + the public scoring code make every run independently re-runnable end to end (the reproducible-backtest posture #8136 is evaluating). --- scripts/backtest-logic-check-core.ts | 20 +++++++++++++------- scripts/backtest-logic-check.ts | 2 ++ test/unit/backtest-logic-check-core.test.ts | 5 +++++ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/scripts/backtest-logic-check-core.ts b/scripts/backtest-logic-check-core.ts index 1fad4a9e9b..f5fa7b5f8d 100644 --- a/scripts/backtest-logic-check-core.ts +++ b/scripts/backtest-logic-check-core.ts @@ -131,14 +131,15 @@ export function runLogicBacktest( export const LOGIC_BACKTEST_COMMENT_MARKER = ""; /** - * Render the standalone advisory PR comment: marker, what was replayed against what, the engine's own - * comparison Markdown (#8088), and the never-blocks-merge note. Deliberately its OWN comment, not a section - * of ORB's unified review comment — see #8139's Boundaries (this CI job runs outside the Worker's review - * flow, and joining that comment would need new Worker↔CI coupling). + * Render the standalone advisory PR comment: marker, what was replayed against what (including the corpus + * checksum — the freeze point that makes the run independently re-runnable, see #8084's manifest), the + * engine's own comparison Markdown (#8088), and the never-blocks-merge note. Deliberately its OWN comment, + * not a section of ORB's unified review comment — see #8139's Boundaries (this CI job runs outside the + * Worker's review flow, and joining that comment would need new Worker↔CI coupling). */ export function renderLogicBacktestComment( comparison: BacktestComparison, - info: { replayableCount: number; skippedCount: number; headSha: string; baseSha: string }, + info: { replayableCount: number; skippedCount: number; headSha: string; baseSha: string; corpusChecksum: string }, ): string { const skippedNote = info.skippedCount > 0 ? ` ${info.skippedCount} historical case(s) lacked captured raw context and were skipped.` : ""; return [ @@ -146,7 +147,8 @@ export function renderLogicBacktestComment( "## Logic backtest", "", `Replayed ${info.replayableCount} historical case(s) for \`${comparison.ruleId}\` through the base` + - ` (\`${info.baseSha.slice(0, 7)}\`) and head (\`${info.headSha.slice(0, 7)}\`) versions of its detection logic.${skippedNote}`, + ` (\`${info.baseSha.slice(0, 7)}\`) and head (\`${info.headSha.slice(0, 7)}\`) versions of its detection logic` + + ` (corpus checksum \`${info.corpusChecksum.slice(0, 12)}\`).${skippedNote}`, "", renderBacktestComparison(comparison), "_Advisory only — this check never blocks merge (#8105)._", @@ -164,7 +166,9 @@ export function sqlStringLiteral(value: string): string { * THRESHOLD_BACKTEST_EVENT_TYPE events, same audit_events columns recordAuditEvent writes (the CLI runs * outside the Worker, so it goes through `wrangler d1 execute` instead of the repositories module; the * caller supplies id/createdAt so this stays clock-free like the rest of this file). `metadata.comparison` - * is the field backtest-track-record.ts's reader already looks for. + * is the field backtest-track-record.ts's reader already looks for; `corpusChecksum` + the two shas are + * the freeze point (#8136's reproducibility posture) — enough for a skeptic to re-export the corpus, + * verify the checksum, and re-run both sides of this exact comparison independently. */ export function buildLogicBacktestAuditInsertSql(input: { id: string; @@ -172,6 +176,7 @@ export function buildLogicBacktestAuditInsertSql(input: { comparison: BacktestComparison; headSha: string; baseSha: string; + corpusChecksum: string; replayableCount: number; skippedCount: number; createdAt: string; @@ -180,6 +185,7 @@ export function buildLogicBacktestAuditInsertSql(input: { comparison: input.comparison, headSha: input.headSha, baseSha: input.baseSha, + corpusChecksum: input.corpusChecksum, replayableCount: input.replayableCount, skippedCount: input.skippedCount, }); diff --git a/scripts/backtest-logic-check.ts b/scripts/backtest-logic-check.ts index 7328744c04..cf76d79261 100644 --- a/scripts/backtest-logic-check.ts +++ b/scripts/backtest-logic-check.ts @@ -122,6 +122,7 @@ async function main() { skippedCount, headSha: args.headSha, baseSha: args.baseSha, + corpusChecksum: manifest.checksum, }), ); @@ -132,6 +133,7 @@ async function main() { comparison, headSha: args.headSha, baseSha: args.baseSha, + corpusChecksum: manifest.checksum, replayableCount: replayable.length, skippedCount, createdAt: new Date().toISOString(), diff --git a/test/unit/backtest-logic-check-core.test.ts b/test/unit/backtest-logic-check-core.test.ts index c938381bc7..517227fea2 100644 --- a/test/unit/backtest-logic-check-core.test.ts +++ b/test/unit/backtest-logic-check-core.test.ts @@ -194,10 +194,12 @@ describe("backtest-logic-check-core renderLogicBacktestComment (#8139)", () => { skippedCount: 0, headSha: "abcdef1234567890", baseSha: "1234567890abcdef", + corpusChecksum: "feedfacecafe0123456789", }); expect(comment.startsWith(`${LOGIC_BACKTEST_COMMENT_MARKER}\n## Logic backtest`)).toBe(true); expect(comment).toContain("Replayed 12 historical case(s) for `linked_issue_scope_mismatch`"); expect(comment).toContain("base (`1234567`) and head (`abcdef1`)"); + expect(comment).toContain("corpus checksum `feedfacecafe`"); expect(comment).toContain("### Backtest comparison: `linked_issue_scope_mismatch`"); expect(comment).toContain("never blocks merge (#8105)"); expect(comment).not.toContain("lacked captured raw context"); @@ -209,6 +211,7 @@ describe("backtest-logic-check-core renderLogicBacktestComment (#8139)", () => { skippedCount: 5, headSha: "abcdef1234567890", baseSha: "1234567890abcdef", + corpusChecksum: "feedfacecafe0123456789", }); expect(comment).toContain("5 historical case(s) lacked captured raw context and were skipped."); }); @@ -229,6 +232,7 @@ describe("backtest-logic-check-core buildLogicBacktestAuditInsertSql (#8139)", ( comparison, headSha: "abcdef1234567890", baseSha: "1234567890abcdef", + corpusChecksum: "feedfacecafe0123456789", replayableCount: 1, skippedCount: 2, createdAt: "2026-07-22T12:00:00.000Z", @@ -246,6 +250,7 @@ describe("backtest-logic-check-core buildLogicBacktestAuditInsertSql (#8139)", ( expect(metadata.comparison).toEqual(comparison); expect(metadata.headSha).toBe("abcdef1234567890"); expect(metadata.baseSha).toBe("1234567890abcdef"); + expect(metadata.corpusChecksum).toBe("feedfacecafe0123456789"); expect(metadata.replayableCount).toBe(1); expect(metadata.skippedCount).toBe(2); });