fix(fairness): commit only to a corpus a reader can actually download - #9966
Merged
Conversation
The published `corpusChecksum` pointed at bytes nobody could obtain, for two independent reasons that happened to land on different deployments. The verifier could not reach the corpus at all. `loopover-verify` fetched `/v1/public/eval-corpus?rule_id=...` while the route has only ever read `?ruleId=`, so every corpus fetch 400'd and the tool reported "no corresponding corpus is downloadable" against a deployment serving a complete, correct, byte-matching corpus. Both sides were individually tested and individually correct: the route's tests used the route's spelling, and the CLI's tests stubbed fetch with a fixture map keyed on pathname, which discarded the query string. The one thing that was broken was the one thing neither suite looked at. The commitment itself was unverifiable wherever reviews actually execute. `precision.latestBacktestRun.corpusChecksum` took precedence over the per-rule downloadable-corpus checksum, and it is wrong three ways: it hashes the RAW corpus (targetKey, full metadata, full-precision timestamps) while readers can only download the redacted one, so it could never match; it is selected with `ORDER BY created_at DESC LIMIT 1` and no rule filter, so one rule's freeze point was stamped onto every record; and that query has no window bound, so the run could predate the window the record declares. This module already flagged the cross-stamping as latent "only because a single rule clears the publication floor" — on a self-host ledger where seven do, it stopped being latent. Changes: - The route accepts `rule_id` as an alias for `ruleId`. Fixing only the client would leave every already-published verifier reporting that same false negative against production forever, so the server meets them; the client now sends the canonical spelling, which works against every deployment. - The downloadable-corpus checksum is the sole commitment source. `latestBacktestRun` remains published in its own right as the rule-precision block's freeze point, it is just no longer misused as a public commitment. - `loadPublicEvalCorpus` reports `readFailed`, so a corpus that is empty because a read failed is distinguishable from one whose rule is genuinely quiet. Fail-safe must not also mean reporting a deployment's failure as a fact about the rule. - The omission rule moves into `isCommittableCorpus`, one pure predicate both paths ask. As an inline condition the read-failure arm was unreachable (a failed read is also an empty one) and so untestable; as a predicate over a plain value it can be driven with a degraded corpus that still carries cases. The new suite owns no fixtures: it points `fetch` at the real Hono app over a real migrated D1 and lets the real `runVerify` drive it, so the only oracle is whether the shipped tool verifies the shipped API. All four guards were mutation-tested — reverting the client spelling, dropping the route alias, ignoring `readFailed`, and restoring the run precedence each fail. Closes #9962
Contributor
|
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 |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 227d880 | Commit Preview URL Branch Preview URL |
Jul 30 2026, 10:24 PM |
Bundle ReportChanges will decrease total bundle size by 5.37kB (-0.07%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
This was referenced Jul 30, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 31, 2026
…ires (#9974) main has been red since #9966 (40133d7): every commit after it fails test/integration/public-eval-scores-route.test.ts with "expected [] to have a length of 1". Bisected -- the commit before it passes 9/9. #9966 made the DOWNLOADABLE corpus the only commitment source, deliberately dropping the persisted-backtest-run fallback: a commitment must name bytes a reader can fetch and re-hash. Correct change, but this suite's fixture recorded only the human OVERRIDES, and the corpus is built from rule-FIRED events joined to them -- so it produced a 0-case corpus, no commitment, and no record. Confirmed by probing loadPublicEvalCorpus directly: caseCount 0 with overrides alone, 20 once the fired events are seeded too. Seeding both halves restores what the fixture always meant to represent: 20 decided cases a reader can download. Changing the expectation to `records: []` would have gone green while silently dropping the recordDigest-recomputability check this suite exists for. The commitment assertion moves off the literal "freeze-point-checksum" (the old contract) and onto what /v1/public/eval-corpus actually serves, so the two surfaces cannot drift apart while both still look correct in isolation.
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
api.loopover.aipublishes an eval-score record whosecommitments.corpusChecksumthe public verifier cannot rehash, and the Orb publishes seven of them. #9962 guessed the commitment path and the corpus route disagreed about the same rule. They do not —buildPublicCorpusCommitmentsis correct, and onapi.loopover.aithe published checksum matches the served corpus exactly. There were two other defects, which happened to land on different deployments.1. The verifier could not reach the corpus at all
loopover-verifyfetches/v1/public/eval-corpus?rule_id=…. The route has only ever read?ruleId=:Every corpus fetch 400'd, nothing was ever rehashed, and the tool reported "no corresponding corpus is downloadable" against a deployment that was serving a complete, correct, byte-matching corpus. The
rule_id_requirederror string is what made the snake_case spelling look right.This survived because both sides were individually tested and individually correct. The route's tests called it with the spelling the route wanted; the CLI's tests stubbed
fetchwith a fixture map keyed on pathname, throwing the query string away. The one thing that was broken was the one thing neither suite looked at.2. The commitment was unverifiable wherever reviews actually execute
precision.latestBacktestRun.corpusChecksumtook precedence over the per-rule downloadable-corpus checksum. It is wrong three ways at once:checksumCasesover the rawBacktestCase[]—targetKey, the full metadata bag, full-precision timestamps — while the corpus a reader can fetch is the redacted one. Different inputs, so it could never match, no matter how healthy the deployment. Paired withtrust.tier: "reproducible"it claims a reproducibility the artifact cannot support.ORDER BY created_at DESC LIMIT 1and no rule filter, so one rule's freeze point was stamped onto every record.eval-score-records.tsalready warned this would have "every record but one committing to a different rule's cases", calling it latent "only because a single rule clears the publication floor". On a self-host ledger where seven clear it, it stopped being latent — which is exactly the seven-record symptom on the Orb.created_at >= ?bound, so the run can predate thewindowStart/windowEndthe record goes on to declare.The hosted Worker has review execution retired and never persists a run, so it never hit this path — which is why the surface looked fine there and broken on the Orb.
The fix
rule_idas an alias forruleId. Fixing only the client would leave every already-installed verifier reporting that same false negative against production forever, so the server meets them. The client now sends the canonical spelling, which works against every deployment old and new; the alias is for copies already in the wild, not for us.latestBacktestRunstays published in its own right as the rule-precision block's freeze point (and the fairness page still renders it) — it is only no longer misused as a public, re-derivable commitment. A rule with no downloadable corpus is now omitted rather than rescued into the surface with a commitment no reader can check.loadPublicEvalCorpusreportsreadFailed. Fail-safe must keep meaning "never 500 an unauthenticated route", but it must stop also meaning "report the deployment's failure as a fact about the rule". A blip and a genuinely quiet rule were byte-identical before this.isCommittableCorpus, one pure predicate both paths ask — the "ONE computation" the issue asks for. As an inline condition the read-failure arm was unreachable (a failed read is also an empty one) and therefore untestable; as a predicate over a plain value it can be driven with a degraded corpus that still carries cases, which is what proves the arm is load-bearing.Tests
test/unit/loopover-verify-against-real-routes.test.tsdeliberately owns no fixtures. It pointsfetchat the real Hono app over a real migrated D1 and lets the realrunVerifydrive it — its own URL construction, its own parameters, its own recomputation. The only oracle is "did the shipped tool verify the shipped API", so any future disagreement about a parameter name, a response field or a status code fails here.All four guards were mutation-tested; each mutation fails, and the restored baseline is green:
rule_idrule_idaliasisCommittableCorpusignoresreadFailedlatestBacktestRunprecedence restoredThe first mutation is worth noting: the end-to-end claim still passes, because the route alias covers it. That is the alias working as intended, and it is why there is a separate explicit test that our client sends the canonical spelling — so nothing drifts onto a deprecated alias and quietly makes it load-bearing.
Acceptance criteria
corpusChecksumis published only when the corpus route can serve the bytes it commits to, for the same rule and window.isCommittableCorpusoverloadPublicEvalCorpus).readFailed).loopover-verifyreports the corpus claim as PASS — pinned by the real-routes suite. Against production it will pass on the next deploy; it already rehashes correctly today when asked with?ruleId=, as shown above.Verification
npx tsc --noEmitclean; full static checker sweep (44 checkers) greennpx vitest run test/unit— 1297 files, 25299 tests passedui:typecheck,ui:test,ui:openapi:checkgreen;openapi.jsonregenerated and committedCloses #9962