feat(ci): backtest logic/regex detection changes against recorded history in a dedicated CI job

.github/workflows/backtest-logic-check.yml

@@ -0,0 +1,156 @@
+# 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
+
+      # 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: |
+          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 }}
+          HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+          BASE_SHA: ${{ github.event.pull_request.base.sha }}
+          PR_NUMBER: ${{ github.event.pull_request.number }}
+        run: |
+          if 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"; 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: |
+          post_comment() {
+            marker="<!-- loopover-logic-backtest -->"
+            # --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
+  # 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."

scripts/backtest-logic-check-core.ts

@@ -0,0 +1,205 @@
+// 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<string>(["secret_leak"]);
+
+/** One backtestable detection function: where it lives in a checkout and what export to load. The CLI
+ *  imports `exportName` from `<checkoutRoot>/<filePath>` 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<string, KnownLogicRule> = {
+  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 = "<!-- loopover-logic-backtest -->";
+
+/**
+ * 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; corpusChecksum: 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` +
+      ` (corpus checksum \`${info.corpusChecksum.slice(0, 12)}\`).${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; `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;
+  targetKey: string;
+  comparison: BacktestComparison;
+  headSha: string;
+  baseSha: string;
+  corpusChecksum: string;
+  replayableCount: number;
+  skippedCount: number;
+  createdAt: string;
+}): string {
+  const metadataJson = JSON.stringify({
+    comparison: input.comparison,
+    headSha: input.headSha,
+    baseSha: input.baseSha,
+    corpusChecksum: input.corpusChecksum,
+    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})`;
+}