Skip to content

refactor(miner): extract shared local SQLite store helper - #4523

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
oktofeesh1:feat/miner-calibration-status-4261
Jul 9, 2026
Merged

refactor(miner): extract shared local SQLite store helper#4523
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
oktofeesh1:feat/miner-calibration-status-4261

Conversation

@oktofeesh1

@oktofeesh1 oktofeesh1 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

(Supersedes the original content of this PR — #4261 was independently solved by merged #4504 before this PR landed, so this branch was reset to main and repointed at a fresh, unblocked gittensor:priority item: #4272.)

run-state.js, claim-ledger.js, portfolio-queue.js, and event-ledger.js each hand-duplicated the same ~15 lines of boilerplate: env-var/config-dir/XDG path resolution, mkdirSync(0o700) + chmodSync(0o600), and PRAGMA busy_timeout. This extracts that into a shared local-store.js (resolveLocalStoreDbPath / normalizeLocalStoreDbPath / openLocalStoreDb):

  • Each store keeps its own .sqlite3 file, table, and env var — this is a DRY pass only, not a merge into one database. Public APIs and on-disk file layout are unchanged.
  • As a side effect of routing through the shared open helper, run-state.js now also sets PRAGMA busy_timeout — the one inconsistency the audit found among the four stores — matching the other three's wait-don't-fail behavior under concurrent access.
  • Adds a "Local storage" README section documenting all four stores together (file, table, module, env var), and records the decision that the manage status PR-portfolio view stays a read-time join (portfolio-queue.js + event-ledger.js's manage_pr_update events) for now rather than becoming a dedicated table.

Closes #4272

Test plan

  • test/unit/miner-local-store.test.ts — path resolution, path normalization/rejection, openLocalStoreDb permissions + busy-timeout pragma (default and custom), :memory: skip path, and a regression test confirming all four stores still resolve to independent files with no accidental table merge
  • test/unit/miner-local-store-readme.test.ts — README documents all four stores + the PR-portfolio decision
  • All pre-existing tests for the four migrated stores still pass unchanged (resolveXDbPath/invalid_x_db_path behavior preserved)
  • Manually exercised claim claim, claim list, state set, state get, and ledger list against a clean GITTENSORY_MINER_CONFIG_DIR, confirmed independent 0600 files
  • npm run typecheck
  • npm run test:ci (full local gate, green)

@oktofeesh1
oktofeesh1 requested a review from JSONbored as a code owner July 9, 2026 22:27
@superagent-security

Copy link
Copy Markdown
Contributor

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

@oktofeesh1
oktofeesh1 marked this pull request as draft July 9, 2026 22:30
Extract the ~15 lines of path-resolution/permission/busy-timeout
boilerplate hand-duplicated across run-state.js, claim-ledger.js,
portfolio-queue.js, and event-ledger.js into a shared local-store.js
(resolveLocalStoreDbPath / normalizeLocalStoreDbPath / openLocalStoreDb).
Each store keeps its own file, table, and env var, and public APIs and
on-disk layout are unchanged — this is a DRY pass, not a merge.

As a side effect of routing through the shared open helper, run-state.js
now also sets PRAGMA busy_timeout (the one inconsistency among the four
stores), matching the other three's wait-don't-fail behavior under
concurrent access.

Add a "Local storage" README section documenting all four stores
together (file, table, module, env var), and record the decision that
the manage-status PR portfolio stays a read-time join for now rather
than a dedicated table.

Closes JSONbored#4272
@oktofeesh1
oktofeesh1 force-pushed the feat/miner-calibration-status-4261 branch from a081a75 to d03ea0b Compare July 9, 2026 22:55
@oktofeesh1 oktofeesh1 changed the title feat(miner-calibration): read-only Phase 7 calibration status CLI refactor(miner): extract shared local SQLite store helper Jul 9, 2026
@oktofeesh1
oktofeesh1 marked this pull request as ready for review July 9, 2026 22:56
@loopover-orb loopover-orb Bot added gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. gittensor:priority Maintainer-selected Gittensor priority — scores a 1.5x multiplier. labels Jul 9, 2026
@loopover-orb

loopover-orb Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-09 23:08:59 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
Pure DRY refactor extracting the ~15 lines of path-resolution + mkdir/chmod + busy_timeout boilerplate duplicated across run-state.js, claim-ledger.js, portfolio-queue.js, and event-ledger.js into a new local-store.js helper (resolveLocalStoreDbPath/normalizeLocalStoreDbPath/openLocalStoreDb). Behavior is preserved 1:1 for each store's env-var name, default filename, and error string, and the diff correctly traces through each call site (verified against full post-change file content) with no signature or default-value drift. The one real behavior change — run-state.js now gets `PRAGMA busy_timeout` via the shared helper, which it previously lacked — is called out explicitly in the description and is a genuine fix, not scope creep. Test coverage is solid: unit tests on the three helper functions plus a regression test confirming all four stores still resolve to independent files/tables (guards against an accidental merge), and closes the linked #4272 issue.

Nits — 4 non-blocking
  • packages/gittensory-miner/lib/plan-store.js still hand-duplicates the identical path-resolution/mkdir/chmod/busy_timeout block and wasn't migrated to local-store.js — worth a fast follow-up now that the helper exists, even though it's reasonable to keep this PR scoped to the four stores feat(miner-manage): local SQLite schema for run-state, claim ledger, and PR portfolio #4272 named.
  • packages/gittensory-miner/lib/local-store.js:45-49 — the busy-timeout default (5000) and permission bits (0o700/0o600) are now consolidated in one place, a good spot to name them as constants (e.g. `DEFAULT_BUSY_TIMEOUT_MS`) for readability, though this isn't a regression since the originals had the same inline literals.
  • The external brief's 'non-inclusive terminology' flag on `sqlite_master` in miner-local-store.test.ts is a false positive — that's SQLite's own built-in system catalog table name and isn't renameable.
  • Consider migrating plan-store.js to local-store.js in a follow-up so all five local stores share one code path (packages/gittensory-miner/lib/plan-store.js currently mirrors the pre-refactor pattern from the other four).
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4272
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: 32 registered-repo PR(s), 25 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor oktofeesh1; Gittensor profile; 32 PR(s), 0 issue(s).
Gate result ✅ Passing No configured blocker found.
Linked issue satisfaction

Addressed
The diff creates the shared `local-store.js` helper and migrates all four stores (run-state, claim-ledger, portfolio-queue, event-ledger) to use it while preserving each store's own file/table/env var, adds the requested README 'Local storage' section documenting all four stores plus the PR-portfolio read-time-join decision, and includes a regression test verifying the four stores remain independe

Review context
  • Author: oktofeesh1
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 32 PR(s), 0 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.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.04%. Comparing base (b069dde) to head (d03ea0b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4523   +/-   ##
=======================================
  Coverage   94.04%   94.04%           
=======================================
  Files         422      422           
  Lines       37574    37574           
  Branches    13724    13724           
=======================================
  Hits        35335    35335           
  Misses       1583     1583           
  Partials      656      656           
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

@loopover-orb
loopover-orb Bot merged commit 23693c3 into JSONbored:main Jul 9, 2026
10 checks passed
@loopover-orb loopover-orb Bot added gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. and removed gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. gittensor:priority Maintainer-selected Gittensor priority — scores a 1.5x multiplier. labels Jul 9, 2026
@JSONbored JSONbored added gittensor:priority Maintainer-selected Gittensor priority — scores a 1.5x multiplier. gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. and removed gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 9, 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. gittensor:priority Maintainer-selected Gittensor priority — scores a 1.5x multiplier.

Development

Successfully merging this pull request may close these issues.

feat(miner-manage): local SQLite schema for run-state, claim ledger, and PR portfolio

2 participants