You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Once #8129/#8130 land (merged), BacktestCase.metadata carries the raw diff/issue content each rule evaluated (except secret_leak, permanently excluded — see #8130). #8138 shipped ORB-native threshold-only backtesting (#8142): it lives entirely inside ORB's live Worker request handling (src/services/threshold-backtest.ts + src/services/threshold-backtest-run.ts, called from resolveThresholdBacktestAdvisory in src/queue/processors.ts) because it never needs to execute anything — parsing a changed numeric constant out of a diff and comparing it against a fixed, already-deployed classifier function requires no untrusted code execution.
Logic/regex changes are different in kind, not just degree: verifying that a rewritten detection function is actually better requires running the PR's own new code against historical cases, and doing that honestly also requires running the old code as a baseline for comparison. ORB's Worker is a persistent production service holding GitHub App credentials and D1 access, processing webhooks for potentially many repos — there's no safe way to execute PR-supplied logic there. A dedicated CI job doesn't have this problem: checking out a PR's branch and running scripts against it is the exact same trust boundary this repo's validate-code/validate-tests jobs already rely on for every single contributor PR today. This isn't a new risk being introduced — it's the one piece of this epic that actually needs the mechanism CI already provides, unlike #8138 which deliberately avoided CI because it didn't need it.
Design
Dedicated new workflow (.github/workflows/backtest-logic-check.yml), not a job folded into the existing CI workflow — keeps it fully decoupled and parallel, so PRs that don't touch the watched paths pay nothing, and PRs that do aren't slowed down (the existing validate-tests shards already take ~11 minutes each; this job is not on that critical path).
Mirrors selfhost.yml's trigger shape: pull_request (types: [opened, synchronize, reopened, ready_for_review]), paths: filtered to exactly: src/rules/**, src/review/content-lane/**, src/settings/agent-actions.ts, src/services/ai-review.ts, src/services/linked-issue-satisfaction.ts. The same draft-skip guard and a concurrency group keyed on github.ref (cancel-in-progress: true) so superseded pushes cancel in-progress runs.
Post its own clearly-labeled PR comment (e.g. "Logic backtest") via the GitHub API/CLI, separate from ORB's own unified review comment — see Boundaries for why.
Tests for the pure-core logic: classify-function construction against fixture raw-context cases, the secret_leak exclusion, the comparison/render call.
Test Coverage Requirements
99%+ patch coverage (branch-counted) on the pure-core script logic. The thin IO wrapper (checkout-root file imports, D1 read/write, gh pr comment) is exempt under this repo's established pure-core/thin-IO precedent, provided it contains no logic beyond argument handling, the calls, and IO.
Expected Outcome
A PR that rewrites detection logic (not just a threshold) gets the same honest, objective backtest comparison a threshold-only change already gets from #8138 — computed by actually running both the old and new versions of the logic against real recorded history, not just eyeballing the diff.
src/services/threshold-backtest.ts / src/services/threshold-backtest-run.ts (the ORB-native precedent — NOT what this issue extends architecturally, since this needs code execution and that deliberately doesn't)
.github/workflows/selfhost.yml (the CI trigger/concurrency/draft-skip pattern to mirror)
scripts/export-d1-core.ts/scripts/export-d1-data.ts (the pure-core/thin-IO pattern, and the existing D1-read mechanism to reuse)
packages/loopover-engine/src/calibration/ (scoreBacktest, compareBacktestScores, renderBacktestComparison — imported directly, not reimplemented)
Deliberately does not feed into ORB's own unified PR comment. ORB's unified comment is built inside the Worker's own webhook-driven review flow; this CI job runs independently of that flow and has no way to join the same comment without new Worker↔CI coupling (a new authenticated internal API endpoint, new attack surface) that isn't justified for this piece. It posts its own separate, clearly-labeled comment instead — an accepted, deliberate tradeoff, not an oversight.
Do not attempt to make ORB's live Worker execute PR-supplied logic directly, in this issue or later — that's the risk this CI-based design exists specifically to avoid. If a future, genuinely sandboxed execution primitive becomes available (unrelated to and independent of this issue), swapping the CI-based execution step for it is a contained follow-up, since the scoring/compare/render/persist logic here is already the same shared, reusable @loopover/engine code either way.
maintainer-only — CI wiring for a real trust/safety-relevant execution boundary, not a build task for contributors.
Context
Once #8129/#8130 land (merged),
BacktestCase.metadatacarries the raw diff/issue content each rule evaluated (exceptsecret_leak, permanently excluded — see #8130). #8138 shipped ORB-native threshold-only backtesting (#8142): it lives entirely inside ORB's live Worker request handling (src/services/threshold-backtest.ts+src/services/threshold-backtest-run.ts, called fromresolveThresholdBacktestAdvisoryinsrc/queue/processors.ts) because it never needs to execute anything — parsing a changed numeric constant out of a diff and comparing it against a fixed, already-deployed classifier function requires no untrusted code execution.Logic/regex changes are different in kind, not just degree: verifying that a rewritten detection function is actually better requires running the PR's own new code against historical cases, and doing that honestly also requires running the old code as a baseline for comparison. ORB's Worker is a persistent production service holding GitHub App credentials and D1 access, processing webhooks for potentially many repos — there's no safe way to execute PR-supplied logic there. A dedicated CI job doesn't have this problem: checking out a PR's branch and running scripts against it is the exact same trust boundary this repo's
validate-code/validate-testsjobs already rely on for every single contributor PR today. This isn't a new risk being introduced — it's the one piece of this epic that actually needs the mechanism CI already provides, unlike #8138 which deliberately avoided CI because it didn't need it.Design
.github/workflows/backtest-logic-check.yml), not a job folded into the existingCIworkflow — keeps it fully decoupled and parallel, so PRs that don't touch the watched paths pay nothing, and PRs that do aren't slowed down (the existingvalidate-testsshards already take ~11 minutes each; this job is not on that critical path).selfhost.yml's trigger shape:pull_request(types: [opened, synchronize, reopened, ready_for_review]),paths:filtered to exactly:src/rules/**,src/review/content-lane/**,src/settings/agent-actions.ts,src/services/ai-review.ts,src/services/linked-issue-satisfaction.ts. The same draft-skip guard and aconcurrencygroup keyed ongithub.ref(cancel-in-progress: true) so superseded pushes cancel in-progress runs.KNOWN_LOGIC_RULESregistry (mirrors Scaffold the advisory backtest CI workflow + threshold-only comparison, and persist each run's result #8138'sKNOWN_THRESHOLDS) mappingruleId→{ filePath, exportName }for the specific detection function to backtest. Scoped to exactlylinked_issue_scope_mismatchin this issue (richest raw context, via calibration: capture bounded raw context (issueText/prTitle/prBody/diff) in linked_issue_scope_mismatch's fired-event metadata #8129) — generalizing the registry to the otherisConfiguredGateBlockercodes calibration: capture bounded raw context for the remaining isConfiguredGateBlocker codes, excluding secret_leak #8130 wires is explicit future scope, not this issue.scripts/export-d1-core.ts/scripts/export-d1-data.tsare, that given a checkout root + ruleId:classifyfunction that feeds each case's captured raw context (metadata.issueText/prTitle/prBody/diff, per calibration: capture bounded raw context (issueText/prTitle/prBody/diff) in linked_issue_scope_mismatch's fired-event metadata #8129/calibration: capture bounded raw context for the remaining isConfiguredGateBlocker codes, excluding secret_leak #8130) through the imported function and compares its verdict to the case's recordedlabel.scoreBacktest(calibration: pure confusion-matrix scorer for a candidate rule classifier against the backtest corpus #8085) — imported directly from@loopover/engine, not reimplemented — to produce aBacktestScoreReport.compareBacktestScores/renderBacktestComparison(calibration: Pareto-floor comparator between two BacktestScoreReports #8086/calibration: render a backtest score/comparison report as Markdown #8088, same direct engine import) to build the comparison.wrangler d1 executeCLI pattern), using a new sibling audit-event type alongside Scaffold the advisory backtest CI workflow + threshold-only comparison, and persist each run's result #8138'sTHRESHOLD_BACKTEST_EVENT_TYPE.secret_leak— mirrors calibration: capture bounded raw context for the remaining isConfiguredGateBlocker codes, excluding secret_leak #8130's own permanent exclusion. No raw context exists for it to backtest against, by design.Deliverables
KNOWN_LOGIC_RULESregistry, scoped tolinked_issue_scope_mismatch.secret_leakexclusion, the comparison/render call.Test Coverage Requirements
99%+ patch coverage (branch-counted) on the pure-core script logic. The thin IO wrapper (checkout-root file imports, D1 read/write,
gh pr comment) is exempt under this repo's established pure-core/thin-IO precedent, provided it contains no logic beyond argument handling, the calls, and IO.Expected Outcome
A PR that rewrites detection logic (not just a threshold) gets the same honest, objective backtest comparison a threshold-only change already gets from #8138 — computed by actually running both the old and new versions of the logic against real recorded history, not just eyeballing the diff.
Links & Resources
src/services/threshold-backtest.ts/src/services/threshold-backtest-run.ts(the ORB-native precedent — NOT what this issue extends architecturally, since this needs code execution and that deliberately doesn't).github/workflows/selfhost.yml(the CI trigger/concurrency/draft-skip pattern to mirror)scripts/export-d1-core.ts/scripts/export-d1-data.ts(the pure-core/thin-IO pattern, and the existing D1-read mechanism to reuse)packages/loopover-engine/src/calibration/(scoreBacktest,compareBacktestScores,renderBacktestComparison— imported directly, not reimplemented)Boundaries
secret_leaknever gets logic-change backtesting, permanently (see calibration: capture bounded raw context for the remaining isConfiguredGateBlocker codes, excluding secret_leak #8130's own Boundaries).linked_issue_scope_mismatchonly in this issue; generalizingKNOWN_LOGIC_RULESto the other codes calibration: capture bounded raw context for the remaining isConfiguredGateBlocker codes, excluding secret_leak #8130 wires is separate future work.@loopover/enginecode either way.maintainer-only — CI wiring for a real trust/safety-relevant execution boundary, not a build task for contributors.