Skip to content

feat(engine): pure BacktestCase corpus builder from fired/override event pairs#8095

Closed
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:fix/critical-issue-backtest-corpus-8083
Closed

feat(engine): pure BacktestCase corpus builder from fired/override event pairs#8095
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:fix/critical-issue-backtest-corpus-8083

Conversation

@RealDiligent

Copy link
Copy Markdown
Contributor

Summary

  • Closes calibration: pure BacktestCase corpus builder from RuleFiredEvent/HumanOverrideEvent pairs #8083
  • computeRulePrecision (Extract a shared, deployment-agnostic calibration/signal-tracking module for ORB + AMS #7982) collapses a rule's fired/override history into one aggregate precision number; a backtest instead needs each PAIRED case kept as an individual labeled record. Adds packages/loopover-engine/src/calibration/backtest-corpus.ts: the BacktestCase type (exact spec shape) and pure buildBacktestCorpus(ruleId, fired, overrides), joining a RuleFiredEvent to its judging HumanOverrideEvent by (ruleId, targetKey).
  • Pairing mirrors computeRulePrecision's "only the decided ones count" discipline: an unjudged firing is excluded, never emitted unlabeled. A re-fired, re-judged target pairs each firing with the override closest in time strictly after that specific firing, falling back to the most recent override when none strictly follows — one case per fired event, never duplicates, documented in the function's doc comment in computeRulePrecision's own style. The ruleId filter mirrors signal-tracking.ts's private overrideMatchesRule (deliberately NOT exported from there — this change is additive-only in a new file, per the issue's boundary). metadata is omitted entirely when the fired event carries none, matching RuleFiredEvent's own optional-property discipline. Pure TypeScript: no IO, no DB, no imports from src/, packages/loopover-miner, or any host adapter.
  • Barrel export added on its own line immediately after the existing signal-tracking.js line in packages/loopover-engine/src/index.ts, exactly where the issue requires.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥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:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • Ran everything this change can affect: the engine deliverable suite via npm run build && npm run test --workspace @loopover/engine (614 tests green, including the new backtest-corpus.test.ts mirroring signal-tracking.test.ts's node:test style against dist/), the root test/unit/backtest-corpus.test.ts (8 tests green), and the full root npm run typecheck. Verified per-diff-line coverage via lcov: every changed line and branch in backtest-corpus.ts is covered by the root suite (has-override vs no-override; single vs multiple overrides including the no-strictly-following fallback; matching vs non-matching ruleId; present vs absent metadata), and simulated the scoped-CI shard condition with the exact CI invocation (--changed=origin/main --coverage.all=false): the lcov is non-empty and contains the changed instrumented files. actionlint/workers/mcp/ui checks are untouched surfaces; CI runs them all.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section 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.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

UI Evidence

Not applicable — engine-only pure-function addition (no UI, docs, or extension surface touched).

Notes

  • Tests intentionally land in BOTH suites: packages/loopover-engine/test/backtest-corpus.test.ts is the issue's deliverable (node:test against dist/, the signal-tracking.test.ts precedent), and test/unit/backtest-corpus.test.ts imports the source directly so the repo's Codecov patch gate measures every changed line and branch — the engine workspace's own node --test run does not feed Codecov.

…ent pairs (JSONbored#8083)

computeRulePrecision (JSONbored#7982) collapses a rule's fired/override history into one
aggregate precision number; a backtest instead needs each PAIRED case kept as an
individual labeled record. Add calibration/backtest-corpus.ts: the BacktestCase
type and pure buildBacktestCorpus(ruleId, fired, overrides), which joins a
RuleFiredEvent to its judging HumanOverrideEvent by (ruleId, targetKey).

Pairing mirrors computeRulePrecision's "only the decided ones count" discipline:
an unjudged firing is excluded, never emitted unlabeled. A re-fired, re-judged
target pairs each firing with the override closest in time strictly after that
specific firing, falling back to the most recent override when none strictly
follows -- one case per fired event, never duplicates. The ruleId filter mirrors
signal-tracking.ts's private overrideMatchesRule (deliberately not exported from
there; this change is additive-only in a new file). metadata is omitted entirely
when the fired event carries none, matching RuleFiredEvent's own
optional-property discipline. Barrel export added directly after the
signal-tracking line in index.ts, per the issue's placement requirement.

Tests land in both suites: packages/loopover-engine/test/backtest-corpus.test.ts
(the deliverable, mirroring signal-tracking.test.ts's node:test style against
dist/) and test/unit/backtest-corpus.test.ts (root vitest, importing the source
directly so the repo's coverage gate measures every line and branch: exclusion,
single-pair labeling, nearest-following and most-recent fallback pairing,
cross-rule/cross-target filtering, and present/absent metadata).
@RealDiligent
RealDiligent requested a review from JSONbored as a code owner July 22, 2026 21:54
@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.01%. Comparing base (f442184) to head (66a6720).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8095      +/-   ##
==========================================
- Coverage   92.01%   92.01%   -0.01%     
==========================================
  Files         753      754       +1     
  Lines       77157    77182      +25     
  Branches    23330    23336       +6     
==========================================
+ Hits        70998    71019      +21     
  Misses       5053     5053              
- Partials     1106     1110       +4     
Flag Coverage Δ
shard-1 57.16% <0.00%> (+2.23%) ⬆️
shard-2 51.35% <0.00%> (-2.78%) ⬇️
shard-3 52.81% <100.00%> (-1.20%) ⬇️

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-corpus.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

Caution

🛑 LoopOver review result - reject/close recommended

Review updated: 2026-07-22 22:09:22 UTC

4 files · 1 AI reviewer · 1 blocker · CI green · clean

🛑 Suggested Action - Reject/Close

Review summary
This adds a pure, well-scoped `buildBacktestCorpus` function that joins `RuleFiredEvent`s to their judging `HumanOverrideEvent`s by `(ruleId, targetKey)`, picking the nearest strictly-after override with a most-recent fallback, and wires it into the barrel export directly under `signal-tracking.js` as required by the issue. The pairing logic in `pairedOverrideFor` (backtest-corpus.ts:44-60) is correct: ties keep first-encountered candidate deterministically, and both the nearest-after and most-recent-fallback paths are exercised by tests. The metadata-omission discipline and dual test suites (vitest unit test against source, plus a `packages/loopover-engine/test` suite against `dist/index.js` mirroring existing package convention) give solid coverage of both branches.

Nits — 4 non-blocking
  • backtest-corpus.ts:44 `pairedOverrideFor` does an O(n) scan per fired event, and the caller does `overrides.filter(...)` per fired event too, so this is O(fired × overrides) — fine at realistic per-rule volumes but worth a comment if this is ever called with a large backtest corpus.
  • The two near-duplicate test files (`test/unit/backtest-corpus.test.ts` and `packages/loopover-engine/test/backtest-corpus.test.ts`) repeat almost identical scenarios; consider whether both are required by convention or if one could just assert the barrel re-export and defer full-coverage testing to the other.
  • Confirm the `packages/loopover-engine/test/backtest-corpus.test.ts` suite is wired into CI to run against a freshly-built `dist/`, since it imports from `../dist/index.js` rather than source.
  • Pull request duplicates other open work — Check for an existing pull request or issue covering this change and coordinate or consolidate before continuing.

Why this is blocked

📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. Linked issue overlaps another open PR: Other open pull requests reference the same linked issue set: #8093, #8094. — Review the related PRs before spending reviewer time on duplicate work.

Decision drivers

  • ❌ Code review — 1 blocker (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #8083
Related work ⚠️ Same linked issue: #8093, #8094 Another open PR references the same linked issue.
Change scope ❌ 8/20 High 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: 361 registered-repo PR(s), 144 merged, 38 issue(s).
Contributor context ✅ Confirmed Gittensor contributor RealDiligent; Gittensor profile; 361 PR(s), 38 issue(s).
Improvement ✅ Minor risk: low · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR adds packages/loopover-engine/src/calibration/backtest-corpus.ts with the exact BacktestCase shape and buildBacktestCorpus function implementing the nearest-following/most-recent-fallback pairing, excludes unjudged firings, mirrors overrideMatchesRule without modifying signal-tracking.ts, and adds the barrel export in the specified location; tests cover no-override exclusion, single pairing

Review context
Contributor next steps
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 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (Linked issue overlaps another open PR; duplicate of open PR #8093). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 22, 2026
JSONbored added a commit that referenced this pull request Jul 25, 2026
…8605)

Live diagnosis on the self-hosted ORB box: metagraphed's secret_leak gate
false-positive rate climbed from 38% to 56% over 6 hours, holding
mergeable PRs. Root cause wasn't a pattern-match false positive -- it was
the fail-closed "content exceeded the scan cap" block on regenerated
OpenAPI/JSON-schema artifacts. metagraphed's openapi.json is ~1.9MB and
api-components.schema.json ~514KB, both well past the old 512,000-char
cap; three recent PRs (#8106, #8095, #8005) were all held for this reason
and all merged anyway once manually verified clean.

The underlying fetcher (grounding-wire.ts's makeGithubFileFetcher)
already requests the raw+json media type specifically to bypass GitHub's
Contents API ~1MB base64-JSON envelope ceiling, so the real limit was
always this local constant, not GitHub's. Raising it only expands scan
coverage -- more content becomes fetchable-and-scannable instead of
being marked incomplete-and-blocked -- so this can't weaken detection
on anything the old cap already caught.

4MB gives headroom above the largest observed real case (loopover's own
openapi.json is already at 525KB, past the old cap too). Fetch
count/concurrency stay capped separately, unaffected.
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

calibration: pure BacktestCase corpus builder from RuleFiredEvent/HumanOverrideEvent pairs

1 participant