feat(ci): backtest logic/regex detection changes against recorded history in a dedicated CI job#8147
Conversation
…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.
…tory 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).
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…d-X a PR on infra errors (#8139) 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.
|
Heads-up from the first live run of the new workflow (it path-matched this PR itself): the corpus export failed with Cloudflare 7403 — the repo's Pushed a follow-up commit making every D1/comment step fail open (notice + green): the review engine auto-closes contributor PRs on any failed check, so an advisory job must never go red on infra problems. The job is now a visible no-op until the token is fixed. Action item (needs your Cloudflare dashboard access): add D1 read/write scope for the |
… 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).
|
Re-ran the workflow after the token update (attempt 2, 05:49Z) — still Cloudflare 7403 on the corpus export. Local ground truth narrows it down:
Once either secret is corrected I (or any push) can re-run and it should go end-to-end. Two harmless observations from the same check: the remote database is still cloud-named |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (87.50%) is below the target coverage (99.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #8147 +/- ##
==========================================
- Coverage 92.03% 92.03% -0.01%
==========================================
Files 764 764
Lines 77540 77544 +4
Branches 23436 23439 +3
==========================================
+ Hits 71366 71369 +3
Misses 5062 5062
- Partials 1112 1113 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…#8114) (#8153) #8138 (threshold backtest, shipped ORB-native in #8142) and #8139 (logic backtest CI check, shipped in #8147) are exactly the #3047 failure shape -- new CLI flags, a new CI check with its own trigger/comment behavior, a gate-adjacent pending policy decision (#8105) -- with no docs. Adds content/docs/backtest-calibration.mdx (corpus model, the two backtest mechanisms and why they run where they run, how to read the Pareto-floor comparison, secret_leak's permanent exclusion, the track-record/corpus-export CLIs, and the advisory-only guarantee with #8105 noted as pending) plus the thin route, nav/command-palette entries, and the regenerated route tree. The two pinned docs-route-count tests move 48 -> 49.
Logic backtestReplayed 0 historical case(s) for Backtest comparison:
|
Summary
.github/workflows/backtest-logic-check.yml) checks out the PR's head and base, dynamically imports both versions of the registered detection function, replayslinked_issue_scope_mismatch's recorded raw-context history through each, scores them with@loopover/engine'sscoreBacktest/compareBacktestScores, posts its own clearly-labeled advisory PR comment (update-in-place via a marker), and persists acalibration.logic_backtest_runaudit event.modelResponseText, bounded byMAX_MODEL_RESPONSE_CHARS) in the fired-event metadata — the missing piece that makes the deterministic parse/floor/sanitize step (buildLinkedIssueSatisfactionResult) honestly replayable against real history.scripts/backtest-logic-check-core.ts:KNOWN_LOGIC_RULESregistry scoped to exactlylinked_issue_scope_mismatch, permanentsecret_leakexclusion mirroring calibration: capture bounded raw context for the remaining isConfiguredGateBlocker codes, excluding secret_leak #8130, classify construction with the same candidate-would-not-fire ⇒ "reversed" semantics as Scaffold the advisory backtest CI workflow + threshold-only comparison, and persist each run's result #8138's threshold classifier, comment + audit-INSERT rendering), the thin-IO CLI (scripts/backtest-logic-check.ts: checksum-verified corpus manifest load, dual dynamic import, best-effort D1 persist), the workflow, and 24 unit tests.scripts/backtest-track-record.ts(calibration: compute REGRESSED-verdict track-record statistics from ORB's persisted backtest results #8140) now aggregates both sibling event types (calibration.threshold_backtest_run+calibration.logic_backtest_run), so Decide + implement whether a REGRESSED backtest verdict should ever gate a merge (Phase 2) #8105's Phase-2 decision reads the complete record.Runs in CI, not ORB's Worker: verifying a logic rewrite requires executing the PR's own code — the same
pull_requesttrust boundaryvalidate-code/validate-testsalready rely on (nopull_request_targetanywhere; fork runs get no secrets and are skipped with a visible notice job). The base checkout nests inside the head workspace so onenpm ciserves both sides. Advisory only: not a required check, never blocks merge (#8105); posts its own comment rather than joining ORB's unified review comment (deliberate — see the issue's Boundaries).Closes #8139
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
npm run test:ciran green end-to-end (includes every box above). The newscripts/**files are outsidecoverage.include(established scripts exemption), but the pure core carries 24 dedicated unit tests anyway, including an end-to-end replay through the realbuildLinkedIssueSatisfactionResultand an explicitsecret_leak-exclusion test at both the registry and scoring layers. The thin-IO CLI is glue only (arg parsing, file IO, dynamic imports, one wrangler call) per theexport-d1-data.tsprecedent.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.Trust-boundary notes: the workflow uses plain
pull_request(neverpull_request_target),permissions: contents: read+pull-requests: writeonly,persist-credentials: falseon both checkouts, and gates every secret-bearing step behind the job-level fork exclusion (mirroring ci.yml's paired fork convention — forks get a visible skip-notice job instead).secret_leakremains permanently excluded from raw-context replay at both the registry and the scoring guard, each with its own test. The D1 persist is best-effort and can never fail the advisory run; the posted comment contains only backtest metrics, shas, and case counts.UI Evidence
Not applicable — no UI change.
Notes
backtest-corpus-export.tsCLI as-is (read-only). Older corpus rows withoutmodelResponseText(pre-Part-A) are skipped and reported in the comment, never scored — replaying them would bias both sides toward "reversed".KNOWN_LOGIC_RULESbeyondlinked_issue_scope_mismatchis explicit future scope per the issue's Boundaries.