Skip to content

fix(miner): wire policy_verdict_cache into the per-repo purge/status/migrate store lists (#6987)#7055

Closed
jeffrey701 wants to merge 1 commit into
JSONbored:mainfrom
jeffrey701:fix-policy-verdict-cache-stores-6987
Closed

fix(miner): wire policy_verdict_cache into the per-repo purge/status/migrate store lists (#6987)#7055
jeffrey701 wants to merge 1 commit into
JSONbored:mainfrom
jeffrey701:fix-policy-verdict-cache-stores-6987

Conversation

@jeffrey701

Copy link
Copy Markdown
Contributor

Summary

policy_verdict_cache keys its table on repo_scope TEXT PRIMARY KEY — a genuine per-repo column, structurally identical to the six repo-scoped stores store-maintenance.js already documents as purgeable — yet it was never wired into any of the three "known local stores" lists it qualifies for. Its per-repo rows could therefore persist indefinitely: uncleaned by purge, unmonitored by doctor, and unmigrated by migrate.

  • Added POLICY_VERDICT_CACHE_PURGE_SPEC = { table: "policy_verdict_cache", repoColumn: "repo_scope" } to store-maintenance.js (same shape as the six existing specs) and a purgeByRepo method to the store (mirroring the sibling repo-scoped stores).
  • Wired it into purge-cli.js's REAL_PURGE_TARGETS, status.js's storeIntegrityChecks, and migrate-cli.js's STORES.
  • policy-doc-cache.js (keyed by url, not a repo column) is intentionally left out, exactly as before and as the issue scopes it.

Why

This closes a data-hygiene gap: per-repo policy verdicts had no purge/health/migration coverage despite fitting the exact repoColumn pattern the other six stores use.

Validation

  • npm run typecheck clean.
  • The existing purge/status/migrate suites now include policy-verdict-cache and pass — this is the regression the issue asks for: the dry-run and real purge tests seed the store and assert it's counted/purged by repo scope (wouldPurge: 1 / purged: 1), the doctor store-integrity sweep asserts store-integrity:policy-verdict-cache, and migrate's store list asserts it — so all three code paths are verified to reach the newly-wired store. The only unchanged-and-still-failing local test is a pre-existing Windows path-separator case in miner-status unrelated to this change. 100% coverage on the new executable code (purgeByRepo).

Closes #6987

@jeffrey701
jeffrey701 requested a review from JSONbored as a code owner July 17, 2026 20:04
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.27%. Comparing base (c8d28b5) to head (daf44ee).
⚠️ Report is 22 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (c8d28b5) and HEAD (daf44ee). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (c8d28b5) HEAD (daf44ee)
shard-1 1 0
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7055      +/-   ##
==========================================
- Coverage   93.74%   87.27%   -6.47%     
==========================================
  Files         692      609      -83     
  Lines       68706    48144   -20562     
  Branches    18760    15133    -3627     
==========================================
- Hits        64409    42020   -22389     
- Misses       3302     4634    +1332     
- Partials      995     1490     +495     
Flag Coverage Δ
rees ?
shard-1 ?
shard-2 36.86% <50.00%> (+<0.01%) ⬆️
shard-3 33.15% <50.00%> (-0.12%) ⬇️
shard-4 34.39% <50.00%> (+0.49%) ⬆️
shard-5 31.68% <50.00%> (-0.56%) ⬇️
shard-6 46.13% <100.00%> (+0.37%) ⬆️

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

Files with missing lines Coverage Δ
packages/loopover-miner/lib/migrate-cli.js 100.00% <ø> (ø)
...ackages/loopover-miner/lib/policy-verdict-cache.js 100.00% <100.00%> (ø)
packages/loopover-miner/lib/purge-cli.js 100.00% <ø> (ø)
packages/loopover-miner/lib/status.js 94.53% <ø> (ø)
packages/loopover-miner/lib/store-maintenance.js 100.00% <100.00%> (ø)

... and 195 files with indirect coverage changes

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

loopover-orb Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-17 20:14:34 UTC

8 files · 1 AI reviewer · no blockers · CI failing · blocked

🛑 Suggested Action - Fix Blockers

Review summary
This wires policy_verdict_cache into the three per-repo store lists (purge, doctor's status-integrity sweep, migrate) exactly mirroring the six existing repo-scoped stores, plus a new purgeByRepo method reusing the shared purgeStoreByRepo/countStoreByRepo helpers. The spec correctly uses repo_scope as the repoColumn matching the actual PRIMARY KEY column, and the accompanying tests seed real rows and assert they're counted/purged/checked through all three code paths (dry-run wouldPurge:1, real purged:1, store-integrity name, migrate store list), so this is a straightforward, well-targeted fix rather than a can't-occur patch. It closes #6987 with narrow, mechanical, low-risk changes.

Nits — 2 non-blocking
  • store-maintenance.js:27's comment says 'six existing specs' referenced in policy-verdict-cache.js:5's header comment block still describes the six sibling stores without updating that older doc comment to mention the seventh (purely a doc nit, not user-facing).
  • Consider double-checking that policy-verdict-cache.js's normalizeRepoScope trimming behavior matches the other repoColumn stores' repo_full_name normalization so a purge/count by repo_scope doesn't silently miss untrimmed rows written before this PR.

CI checks failing

  • validate
  • validate-tests (1)

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 #6987
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
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: 145 registered-repo PR(s), 70 merged, 32 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jeffrey701; Gittensor profile; 145 PR(s), 32 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff adds POLICY_VERDICT_CACHE_PURGE_SPEC to store-maintenance.js, wires it into purge-cli.js's REAL_PURGE_TARGETS, status.js's storeIntegrityChecks, and migrate-cli.js's STORES exactly as requested, and leaves policy-doc-cache.js untouched.

Review context
  • Author: jeffrey701
  • 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: 145 PR(s), 32 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • Then work through the remaining 1 step 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 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-tests (1))). 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 17, 2026
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.

policy_verdict_cache missing from purge/status/migrate 'known local stores' lists

1 participant