fix(eval): stamp the in-Worker threshold backtest with its corpus freeze point - #9804
Conversation
…eze point loadPublicRulePrecision selects the latest backtest run with `json_extract(metadata_json, '$.corpusChecksum') IS NOT NULL` across two event types, but only one writer ever satisfied it. The CI-side logic-backtest CLI writes corpusChecksum; persistThresholdBacktestRuns -- which runs in-Worker on every review pass touching a KNOWN_THRESHOLDS constant -- wrote only `comparison` and `constantName`. So a deployment whose backtest history is entirely in-Worker had latestBacktestRun null forever, and buildEvalScoreRecordsFromRulePrecision early-returns [] on that: the whole /v1/public/eval-scores EvalScoreRecord feed served zero records with no diagnostic, no matter how many backtests had run. checksumCases moves from scripts/backtest-corpus-export-core.ts into packages/loopover-engine/src/calibration/ so the Worker can reach it, rather than being copied -- two implementations of a freeze point is worse than none, because a manifest frozen by one and validated by the other disagrees for no visible reason. backtest-corpus-export-core re-exports it so its existing importers are unaffected, and backtest-checksum.test.ts pins the digest of a fixed fixture against the pre-move implementation: manifests already on disk carry pre-move checksums, and two CLIs re-validate against this function before trusting a corpus. The checksum is taken over the corpus map that was actually scored, not re-read at persist time. A freeze point that does not match the cases behind the verdict invites a reader to re-derive different numbers and conclude the published ones are wrong. comparison/constantName are untouched -- rule-calibration-trend.ts reads $.comparison.verdict off these rows -- and corpusChecksum is omitted rather than nulled when absent, so the reader's IS NOT NULL filter stays the one place that decides whether a run is a usable commitment. Closes #9639
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | f5114b4 | Commit Preview URL Branch Preview URL |
Jul 29 2026, 12:44 PM |
|
Important 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏳ LoopOver is waiting…LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9804 +/- ##
==========================================
- Coverage 91.59% 91.59% -0.01%
==========================================
Files 915 915
Lines 112700 112644 -56
Branches 27077 27080 +3
==========================================
- Hits 103229 103173 -56
Misses 8144 8144
Partials 1327 1327
Flags with carried forward coverage won't be shown. Click here to find out more.
|
codecov/patch flagged backtest-checksum.ts at 83.78% under the `engine` flag while `backend` reported 100%: the file lives in packages/loopover-engine/src, which the engine's own node --test suite measures, but the only test for it was in the root vitest suite. #9639 required the moved function to be covered in its new home. This is that, and it earns its place beyond the flag: the engine suite runs against dist/, so it is the only check of the freeze point through the exact artifact published consumers import rather than through the source. Both tests now share one fixture and one pinned digest, and the fixture is a real BacktestCase (the engine suite typechecks its tests, which caught that the root one had been getting by on a cast). The digest is recomputed from the pre-move implementation for the new fixture, so it still pins byte-stability against what exported manifests carry.
…st-corpus-checksum
…st-corpus-checksum # Conflicts: # test/unit/eval-score-records.test.ts
Review catch: importing checksumCases from the "@loopover/engine" barrel reversed the rationale documented a few lines above that import. The barrel's re-export graph pulls in web-tree-sitter and claude-agent-sdk, and scripts/replay-runner/Dockerfile deliberately constructs @loopover/engine from three named dist files rather than npm-installing it -- so the barrel import would have failed to resolve in the container at runtime, and even if resolved, would have widened a trusted computing base that is measured inside a TEE. Uses the subpath export this change already added. dist/calibration/ backtest-checksum.js is added to the Dockerfile's COPY list; it compiles to a single node:crypto import (its only other import is type-only and erased), so it widens the image's file list without widening its dependency footprint at all. replay-runner-manifest:check caught the drift, as its own Dockerfile comment says it will; the regenerated manifest is committed alongside.
|
Both correct — fixed in 84b6dd9. The subpath import. You're right that this reversed the file's own documented rationale, and the consequence is worse than style: The Dockerfile.
Also merged current main in and resolved a conflict with #9811 in Engine suite: 889 pass. Affected host suites: 58 pass. |
…n change (#9829) main's CI has been red since #9813 (gate.ignoredCheckRuns) merged -- the last four ci.yml runs on main all failed, and every branch cut from or merged with main inherits it. #9804 went red on these tests with no changes of its own anywhere near them. #9813's implementation is correct; only the expectations were left behind: * fetchLiveCiAggregate / fetchLiveCiAggregatePreferGraphQl gained an eighth parameter (ignoredCheckRuns) while four toHaveBeenCalledWith assertions still passed seven; * the durable cache key gained an `|ign:` segment, while queue.test.ts's durable-cache regression still expected `|adv:` alone; * the whole ignoredCheckRuns block (19 lines) was added to .loopover.yml.example but not to config/examples/loopover.full.yml, which config-templates.test.ts requires byte-identical from `# WHERE IT LIVES` onward. No production behaviour changes. This matters beyond a red build: the gate auto-CLOSES a contributor PR on red CI, so a contributor who opened a PR after #9813 landed would be closed for a defect that is not theirs and that they cannot fix -- the same harm #9798 and #9801 describe, except this one does not self-correct. Closes #9828
…Case codecov/patch held at 97.29% on this file under the `engine` flag: one line, the sort comparator's equality arm. It is genuinely unreachable -- Object.keys yields each key exactly once, so a comparator never sees two equal keys -- and the `v8 ignore` pragma covering it was flag-dependent: vitest's v8 provider honoured it, the engine package's own coverage run did not. Rather than chase pragmas across two coverage tools, the branch is gone. Sorting the keys with the default comparator gives the identical total order for strings, so the freeze point does not move -- which is not an assertion, it is what the pinned pre-move digest in both test suites verifies. An unreachable branch can only sit uncovered or carry an ignore comment; not writing one beats both.
Closes #9639
The seam
loadPublicRulePrecisionselects the latest backtest run across two event types, filtered oncorpusChecksum IS NOT NULL. Only one of the two writers ever satisfied that filter:corpusChecksum?buildLogicBacktestAuditInsertSql(scripts/backtest-logic-check-core.ts)--persistpersistThresholdBacktestRuns(src/services/threshold-backtest-run.ts)KNOWN_THRESHOLDSconstantA deployment whose backtest history is entirely in-Worker therefore had
latestBacktestRun: nullforever — andbuildEvalScoreRecordsFromRulePrecisionearly-returns[]on that, so the entire/v1/public/eval-scoresfeed served zero records with no diagnostic. Both halves were individually correct; the defect lived only in the seam, which is why the end-to-end test in Deliverable 4 matters more than the unit ones.The move, not a copy
checksumCasesmoves intopackages/loopover-engine/src/calibration/so the Worker can reach it. Copying it intosrc/would have been the worse failure: a manifest frozen by one implementation and validated by the other disagrees for no visible reason, and two CLIs (backtest-logic-check.ts,attested-backtest-run.ts) re-validate a manifest against this function before trusting its cases — so a drifted copy doesn't fail loudly, it silently rejects every corpus ever exported.scripts/backtest-corpus-export-core.tsre-exports it, so its existing importers and its own test are unaffected.backtest-checksum.test.tspins the digest of a fixed fixture —d980dbee…, computed fromorigin/main's pre-move implementation, not from the new one. If that constant ever has to be edited to make the test pass, the freeze point has drifted and every manifest on disk is invalidated. The fixture's keys are deliberately unsorted so the canonicalization is exercised too, and separate cases assert case-order and value changes do move the digest.node:cryptorather than Web Crypto becausechecksumCasesmust stay synchronous (it is called from pure, non-async manifest builders); the Worker runs withnodejs_compat, andattester.ts,attestation-envelope.ts,backtest-split.tsandcounterfactual-fixtures.tsin that same directory already importcreateHashon exactly that basis.Details worth reviewing
EMPTY_CORPUS_CHECKSUM— that stays the single gate.corpusChecksumis omitted, never nulled, when a direct caller supplies no map, so the reader'sIS NOT NULLfilter remains the one place that decides usability.comparison/constantNameuntouched —rule-calibration-trend.tsreads$.comparison.verdictoff these rows. Asserted.Deliverables
checksumCasesin the engine, exported from the barrel + a subpath export, all threescripts/importers rewired, byte-stability test on a fixed fixturepersistThresholdBacktestRunswritesmetadata.corpusChecksum, asserted inthreshold-backtest-run.test.tsloadPublicRulePrecisionreturns non-nulllatestBacktestRunwhen the only persisted run is athreshold_backtest_run— fails against the old writer (verified by reverting the one line)buildEvalScoreRecordsFromRulePrecisionemits one record per rule for that same fixture — also fails against the old writercomparison/constantNamestill present and unchangedEach of the three regression tests was re-run with the writer line removed; all three fail, so none of them passes vacuously.
Scope note
This fixes self-host and any deployment that executes reviews. It does not by itself make
api.loopover.ai/v1/public/eval-scoresnon-empty: hosted review execution is retired (src/index.tsacks-and-drops review-execution jobs), sopersistThresholdBacktestRunsnever runs there. The hosted read-path gap is a separate defect — the public corpus forai_consensus_defectcurrently publishes 460 real cases with a re-derivable checksum while the records array stays empty — and gets its own issue and PR.Validation
tsc --noEmit·engine-parity:drift-check·dead-source-files:check·import-specifiers:check·validate:no-hand-written-js·manifest:drift-check— clean. 85 tests pass across the affected suites.Coverage on every changed measured file —
backtest-checksum.ts,threshold-backtest-run.ts,public-rule-precision.ts,eval-score-records.ts— 100% lines, 100% branches. The onev8 ignoreis the sort comparator's0arm, unreachable becauseObject.entriesyields each key once; the reachable arms are covered by the property-order test.