Skip to content

fix(miner): stamp prediction-ledger schema version and add tenant_id column#6702

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
e11734937-beep:fix/prediction-ledger-schema-tenant-6596
Jul 16, 2026
Merged

fix(miner): stamp prediction-ledger schema version and add tenant_id column#6702
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
e11734937-beep:fix/prediction-ledger-schema-tenant-6596

Conversation

@e11734937-beep

Copy link
Copy Markdown
Contributor

Summary

packages/loopover-miner/lib/prediction-ledger.js was the one canonical local store that never called applySchemaMigrations (#4832): it carried no PRAGMA user_version stamp, and its predictions table lacked the additive, nullable tenant_id TEXT column its four sibling ledgers already received (#4939). migrate-cli.js's doc claim that "every store already applies its own pending migrations as a side effect of being opened" was therefore false for this one file.

Change

  • initPredictionLedger now calls applySchemaMigrations(db, [addTenantIdColumn]) immediately after the bootstrap schema, matching the call-site position in event-ledger.js/run-state.js.
  • addTenantIdColumn adds tenant_id TEXT guarded by a PRAGMA table_info(predictions) column-presence check, idempotent against an already-migrated file -- mirroring event-ledger.js's and run-state.js's own addTenantIdColumn exactly.
  • No consumer reads or writes tenant_id: appendPrediction still inserts without it and rowToEntry still ignores it, so every existing and new row has tenant_id = NULL and self-host behavior is byte-identical (Add tenant-scoping columns to all local ledger schemas #4939's stated contract for the other four stores).

A fresh store now stamps user_version = 2 (baseline 1 + the one migration); a pre-existing file upgrades in place with every row preserved.

Tests (test/unit/miner-prediction-ledger.test.ts)

  • A freshly-opened store reports readSchemaVersion 2 and has the tenant_id column.
  • A crafted pre-migration on-disk file (bare v1 schema, user_version 0, one existing row) upgrades in place: the column is added, the old row survives, and tenant_id reads back null for both the old and a newly-appended row.
  • Re-running the migration against a file that already has the column (version reset below target) is a no-op -- exercising the column-present skip branch (no duplicate-column ALTER, no throw).

Reproduce-first verified: with only the tests applied, all three fail (user_version 0, not 2); with the fix, they pass.

Validation

  • npm run typecheck -- 0 errors.
  • npx vitest run test/unit/miner-prediction-ledger.test.ts -- 11/11 pass.
  • Related suites green: miner-migrate-cli, miner-status, miner-schema-version, miner-store-maintenance -- 80/80.
  • npm run build:miner -- passes (node --check).
  • Coverage: 100% line + branch on the changed lines (both arms of the column-presence guard covered), clearing the codecov/patch gate.
  • git diff --check clean; rebased on latest main.

Closes #6596

…column

prediction-ledger.js was the one canonical local store that never called applySchemaMigrations, so it carried no PRAGMA user_version stamp and lacked the additive nullable tenant_id column its four sibling ledgers already have (JSONbored#4939). Add an addTenantIdColumn migration (guarded by a PRAGMA table_info column-presence check, idempotent like its siblings) and run it via applySchemaMigrations right after the bootstrap schema, matching event-ledger.js/run-state.js. A fresh store now stamps user_version 2 and an existing file upgrades in place; tenant_id is NULL for every row and no consumer reads or writes it, so self-host behavior is byte-identical.

Closes JSONbored#6596
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.65%. Comparing base (58ab17e) to head (bee409c).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6702   +/-   ##
=======================================
  Coverage   93.65%   93.65%           
=======================================
  Files         682      682           
  Lines       68112    68117    +5     
  Branches    18708    18708           
=======================================
+ Hits        63788    63793    +5     
  Misses       3347     3347           
  Partials      977      977           
Flag Coverage Δ
shard-1 43.90% <100.00%> (+0.01%) ⬆️
shard-2 36.96% <0.00%> (-0.01%) ⬇️
shard-3 32.30% <100.00%> (-0.16%) ⬇️
shard-4 34.59% <100.00%> (+0.36%) ⬆️
shard-5 31.64% <100.00%> (+0.25%) ⬆️
shard-6 45.76% <100.00%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
packages/loopover-miner/lib/prediction-ledger.js 90.00% <100.00%> (+0.60%) ⬆️

... and 1 file with indirect coverage changes

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 16, 2026
@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-16 19:59:50 UTC

2 files · 1 AI reviewer · no blockers · readiness 77/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR closes the tenant_id/schema-version parity gap for prediction-ledger.js by mirroring the exact addTenantIdColumn pattern already used in event-ledger.js, guarding the column addition behind a PRAGMA table_info check, and wiring applySchemaMigrations at the same call-site position as its siblings. The tests craft a genuine pre-migration on-disk file (bare v1 schema, user_version 0, one row) and verify in-place upgrade, fresh-store stamping, and idempotent re-run — these are real, non-fabricated scenarios since the crafted seed file matches the table shape the code wrote before this migration existed. The change is narrow, additive, and consumer code (appendPrediction/rowToEntry) is correctly left untouched so tenant_id stays NULL and behavior is byte-identical, exactly as claimed.

Nits — 2 non-blocking
  • The migration comment in prediction-ledger.js:132-136 is copy-pasted near-verbatim from event-ledger.js's addTenantIdColumn comment — consider trimming the duplicated rationale now that it lives in two files.
  • Consider extracting the identical addTenantIdColumn PRAGMA-guard pattern (now duplicated across event-ledger.js, run-state.js, and prediction-ledger.js) into a shared helper in schema-version.js if a fourth sibling ever needs it.

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 #6596
Related work ⚠️ 2 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
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: 71 registered-repo PR(s), 43 merged, 2 issue(s).
Contributor context ✅ Confirmed Gittensor contributor e11734937-beep; Gittensor profile; 71 PR(s), 2 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff adds the applySchemaMigrations call right after the CREATE TABLE statement, defines addTenantIdColumn with the required PRAGMA table_info guard mirroring sibling stores, and leaves appendPrediction/rowToEntry untouched so tenant_id stays NULL, matching all stated requirements.

Review context
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 3 steps in the Signals table above.
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 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 46b1aca into JSONbored:main Jul 16, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

prediction-ledger.js is missing both the schema-version convention and the tenant_id column every sibling store has

1 participant