Skip to content

fix(review): surface per-model terminal errors and readable diagnostics in review-failure telemetry #23

fix(review): surface per-model terminal errors and readable diagnostics in review-failure telemetry

fix(review): surface per-model terminal errors and readable diagnostics in review-failure telemetry #23

# 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."