Skip to content

feat(calibration): deterministic seeded held-out/visible split of the backtest corpus - #8097

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
kai392:feat/critical-issue-backtest-split
Jul 22, 2026
Merged

feat(calibration): deterministic seeded held-out/visible split of the backtest corpus#8097
JSONbored merged 1 commit into
JSONbored:mainfrom
kai392:feat/critical-issue-backtest-split

Conversation

@kai392

@kai392 kai392 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add packages/loopover-engine/src/calibration/backtest-split.ts with splitBacktestCorpus(cases, heldOutFraction, seed): a deterministic, content-hashed partition of a BacktestCase[] corpus (calibration: pure BacktestCase corpus builder from RuleFiredEvent/HumanOverrideEvent pairs #8083) into visible and held-out slices, so a candidate rule fix can be iterated against the visible slice but scored against both.
  • Assignment hashes sha256(${seed}:${ruleId}:${targetKey}) (first 8 hex chars over 0xffffffff, held out when strictly below the fraction) — the same createHash approach as stableProposalId in packages/loopover-engine/src/miner/deny-hook-synthesis.ts, so a case's bucket depends only on (seed, ruleId, targetKey), never on its array position or corpus size: a corpus that grows over time never reshuffles previously held-out cases.
  • Original input order is preserved within each bucket (no sorting/shuffling); an out-of-range heldOutFraction throws a plain Error naming the invalid value (the negated compound guard also fails closed on NaN).
  • Pure — no IO, no Math.random(), no wall-clock reads. Deliberately no barrel export (per the issue's deliverables, which specify only the new file + test).

Closes #8087

Test plan

  • packages/loopover-engine/test/backtest-split.test.ts (node:test, per the issue's deliverable): fraction 0 → all visible; fraction 1 → all held out; byte-identical determinism on repeat calls; per-bucket input-order preservation with both buckets populated (12-case fixture); a different seed produces a different split; both out-of-range directions throw naming the value
  • test/unit/backtest-split-engine.test.ts (vitest, importing the engine source): same cases — this is what exercises the .ts for codecov/patch, since @loopover/engine imports resolve to dist (the trap that has already failed prior PRs in this issue family)
  • Local coverage on the new file: 10/10 lines, 6/6 branches (both guard sides + both bucket sides)
  • npm run test --workspace @loopover/engine green; npm run typecheck green
  • CI validate

@kai392
kai392 requested a review from JSONbored as a code owner July 22, 2026 22:23
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.00%. Comparing base (423a3d1) to head (af8fdc8).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8097      +/-   ##
==========================================
- Coverage   92.01%   92.00%   -0.01%     
==========================================
  Files         755      756       +1     
  Lines       77201    77211      +10     
  Branches    23337    23338       +1     
==========================================
+ Hits        71034    71040       +6     
  Misses       5061     5061              
- Partials     1106     1110       +4     
Flag Coverage Δ
shard-1 53.64% <100.00%> (-5.94%) ⬇️
shard-2 57.07% <0.00%> (+5.97%) ⬆️
shard-3 51.55% <0.00%> (+1.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../loopover-engine/src/calibration/backtest-split.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 22, 2026
@loopover-orb

loopover-orb Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-22 22:36:48 UTC

3 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
Adds `splitBacktestCorpus`, a pure content-hashed deterministic partition of a `BacktestCase[]` into visible/held-out slices keyed on `(seed, ruleId, targetKey)` rather than array position, matching the same sha256-composite-key pattern already used by `stableProposalId`. The logic is correct: the negated compound guard `!(f >= 0 && f <= 1)` fails closed on NaN, input order is preserved per-bucket via simple push in iteration order, and the boundary semantics (held out when strictly `< heldOutFraction`) are consistent with the tested 0/1 edge cases. Test coverage is thorough and exercises both the dist-consuming and source-importing paths to satisfy coverage tooling, and the PR closes its linked issue (#8087) with no scope creep beyond the stated deliverable.

Nits — 4 non-blocking
  • The magic numbers `8`, `16`, and `0xffffffff` in backtest-split.ts:15/35 (hex-slice length, hex radix, and max uint32) could be named constants for clarity, though the surrounding comments already explain the intent.
  • No barrel/index export is added, which is called out as deliberate in the description, but confirm downstream calibration code that will consume this (if any exists yet) knows to import from the direct path.
  • Consider extracting the hash-to-fraction computation (`parseInt(digest.slice(0,8),16)/0xffffffff`) into a small named helper (e.g. `hashToUnitInterval`) shared with `stableProposalId` if that logic is ever duplicated further.
  • If a `heldOutFraction` of exactly the boundary value (e.g. 0.5 with a hash landing exactly on 0.5) matters for reproducibility docs, consider noting the strict `<` tie-break in the JSDoc as done, which is already present and sufficient.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #8087
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 93 registered-repo PR(s), 49 merged, 2 issue(s).
Contributor context ✅ Confirmed Gittensor contributor kai392; Gittensor profile; 93 PR(s), 2 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR adds backtest-split.ts implementing splitBacktestCorpus exactly per spec (same sha256/first-8-hex/0xffffffff hashing pattern as deny-hook-synthesis.ts, range validation with value in the error message, order-preserving bucket assignment based only on seed/ruleId/targetKey) and includes tests covering fraction 0/1, determinism, order preservation, differing seeds, and both throw directions.

Review context
  • Author: kai392
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, Kotlin, Perl, Python, TypeScript, Vue
  • Official Gittensor activity: 93 PR(s), 2 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

calibration: deterministic seeded held-out/visible split of the backtest corpus

3 participants