Skip to content

feat(miner): persist resolved policy verdicts across discover runs - #5516

Merged
JSONbored merged 2 commits into
mainfrom
feat/policy-verdict-cache-4843
Jul 13, 2026
Merged

feat(miner): persist resolved policy verdicts across discover runs#5516
JSONbored merged 2 commits into
mainfrom
feat/policy-verdict-cache-4843

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Adds policy-verdict-cache.js, a local cache of resolved AiPolicyVerdict objects keyed by repo + the ETag of whichever doc (AI-USAGE.md or CONTRIBUTING.md) decided the verdict, following the same local-store.js/schema-version.js conventions as every other store in this package.
  • fetchRepoDoc (opportunity-fanout.js) now returns {content, etag} instead of bare content, so resolveRepoAiPolicy can see the ETag of whichever doc it fetched.
  • resolveRepoAiPolicy checks the verdict cache before calling resolveAiPolicyVerdict: a hit requires BOTH the decisive doc name and its ETag to match the cached entry (guards against a repo whose decisive doc changes between runs, e.g. AI-USAGE.md going from absent to present); a miss always resolves fresh and re-caches.
  • Same fail-open discipline as Add conditional-GET caching for policy-doc fetches #4842's policy-doc cache throughout: a cache read/write failure degrades to "resolve fresh," never affects discovery, and discover-cli.js opens the store in its own try/catch so a corrupt/unwritable cache DB can't abort a run.

Test plan

  • npx vitest run test/unit test/contract — 780 files / 15197 tests passing, 0 regressions (one pre-existing unrelated flake in github-graphql-cache.test.ts reproduced only under full-suite parallel load, confirmed passing in isolation and untouched by this diff)
  • npm run build --workspace @jsonbored/gittensory-miner (node --check over every lib file, including the new one)
  • npx tsc --noEmit clean
  • npm run docs:drift-check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run miner:env-reference:check
  • npm audit --audit-level=moderate — 0 vulnerabilities
  • Dedicated tests for: cold-cache resolve+cache, cache-hit reuse (proven via a deliberately wrong planted verdict), recompute on ETag change, recompute on decisive-doc mismatch even with a colliding ETag, CONTRIBUTING.md-decisive path, cache read/write failure fail-open, no-etag-no-cache, inert-without-a-cache, real on-disk persistence, default-store open/close, and the corrupt-cache-degrades-not-fails regression

Closes #4843

@superagent-security

Copy link
Copy Markdown
Contributor

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

@JSONbored JSONbored self-assigned this Jul 13, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 13, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 7ab8e0a Commit Preview URL

Branch Preview URL
Jul 13 2026, 12:48 AM

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.78%. Comparing base (d25c60d) to head (7ab8e0a).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5516   +/-   ##
=======================================
  Coverage   94.77%   94.78%           
=======================================
  Files         565      566    +1     
  Lines       44964    45023   +59     
  Branches    14675    14675           
=======================================
+ Hits        42614    42673   +59     
  Misses       1616     1616           
  Partials      734      734           
Flag Coverage Δ
shard-1 43.81% <0.00%> (-0.46%) ⬇️
shard-2 35.35% <84.93%> (-0.01%) ⬇️
shard-3 31.69% <36.98%> (-0.27%) ⬇️
shard-4 31.62% <28.76%> (-0.36%) ⬇️
shard-5 33.09% <38.35%> (+0.18%) ⬆️
shard-6 44.12% <20.54%> (+0.54%) ⬆️

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

Files with missing lines Coverage Δ
packages/gittensory-miner/lib/discover-cli.js 100.00% <100.00%> (ø)
...ackages/gittensory-miner/lib/opportunity-fanout.js 98.23% <100.00%> (+0.15%) ⬆️
...kages/gittensory-miner/lib/policy-verdict-cache.js 100.00% <100.00%> (ø)
🚀 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 added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 13, 2026
@loopover-orb

loopover-orb Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Tip

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

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-13 00:35:49 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
Adds a persisted local SQLite cache of resolved AiPolicyVerdict objects keyed by repo + the ETag of the decisive doc, mirroring #4842's policy-doc cache exactly (own try/catch open in discover-cli.js, fail-open on read/write errors, no-cache when etag is absent). fetchRepoDoc now returns {content, etag} so resolveRepoAiPolicy can match a cache hit against both the decisive doc name and its ETag, correctly forcing a recompute when the decisive doc changes (e.g. AI-USAGE.md going from absent to present) even if the ETag string happens to collide. Tests are substantive rather than fabricated — they plant a deliberately wrong verdict under a matching cache key to prove the cache path (not a fresh resolve) produced the result, and one test round-trips through a real on-disk SQLite file across process-like reopens.

Blockers

  • packages/gittensory-miner/lib/policy-verdict-cache.js:54 and packages/gittensory-miner/lib/opportunity-fanout.js:240 key cached verdicts only by `repoFullName`, so two different `apiBaseUrl`/forge hosts with the same `owner/repo`, decisive doc, and ETag string will share a verdict even when their policy document contents differ; include the forge/API host in the cache key, for example `repo_scope TEXT NOT NULL` or `api_base_url TEXT NOT NULL` plus `repo_full_name`, and pass `options.apiBaseUrl` from `resolveOrCacheVerdict`.
Nits — 6 non-blocking
  • opportunity-fanout.js is now ~506 lines, past the repo's own 400-line guideline; the new readCachedPolicyVerdict/writeCachedPolicyVerdict/resolveOrCacheVerdict helpers could move into policy-verdict-cache.js or a small wiring module.
  • The PR references issue Add a persisted cross-invocation cache of resolved policy verdicts #4843 but the external history check reports only partial coverage of that issue's stated scope — worth confirming the issue is fully closed by this diff before merge.
  • policy-verdict-cache.js's returned store object (get/put/close) reads as deep nesting per static analysis, but it's just an object literal with method bodies — no actual control-flow complexity to simplify.
  • Consider naming the 304/404 status checks in opportunity-fanout.js:214-215 as constants (e.g. HTTP_NOT_MODIFIED, HTTP_NOT_FOUND) for readability, matching the doc-cache convention above it.
  • If opportunity-fanout.js keeps growing with each new cache (Add conditional-GET caching for policy-doc fetches #4842, Add a persisted cross-invocation cache of resolved policy verdicts #4843), consider extracting a generic 'readCache/writeCache with fail-open' helper shared by both doc and verdict caches to avoid near-duplicate try/catch scaffolding.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4843
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: 44 registered-repo PR(s), 36 merged, 424 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 44 PR(s), 424 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 44 PR(s), 424 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
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.
[BETA] Chat with Gittensory

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

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

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 13, 2026
Add a local cache (policy-verdict-cache.js) keyed by repo + the ETag of
whichever doc (AI-USAGE.md or CONTRIBUTING.md) decided the verdict.
fetchRepoDoc now surfaces the ETag it used alongside doc content, so
resolveRepoAiPolicy can skip resolveAiPolicyVerdict outright once a
same-run conditional-GET (#4842) confirms the deciding doc's ETag
hasn't moved -- reusing the prior verdict is exactly as correct as
recomputing it, since both only run against the identical doc text.

A cache miss (cold cache, changed ETag, or a different doc becoming
decisive) always falls through to a fresh resolution and re-caches the
result. Same fail-open discipline as the doc cache: any cache read or
write failure degrades to "resolve fresh" rather than affecting
discovery, and discover-cli.js opens the store in its own try/catch so
a corrupt/unwritable cache DB can never abort a run.

Closes #4843
…t repo

A bare owner/repo is not a safe cache key across tenants: two different
forge hosts (#4784's per-tenant apiBaseUrl) can each have their own
unrelated repo of the same name, and if their policy docs happened to
produce the same ETag string, a verdict resolved against one host's
docs could be incorrectly served for the other's. Key the cache by
`${apiBaseUrl}::${repoFullName}` instead, mirroring policy-doc-cache.js's
own precedent of keying on the full request URL rather than a bare path.

Renames the store's column/field from repo_full_name to repo_scope to
reflect that the value is now a caller-owned composite key, not a
plain repo identifier -- the store itself stays opaque to what the key
represents, same as before.
@JSONbored
JSONbored force-pushed the feat/policy-verdict-cache-4843 branch from 6423305 to 7ab8e0a Compare July 13, 2026 00:46
@JSONbored

Copy link
Copy Markdown
Owner Author

Fixed the cross-host cache-key issue: the policy-verdict cache is now keyed by `${apiBaseUrl}::${repoFullName}` instead of a bare `owner/repo`, matching `policy-doc-cache.js`'s own precedent of keying on the full request URL. Renamed the store's column/field from `repo_full_name` to `repo_scope` to reflect that it's now a caller-owned composite key. Added a dedicated regression test proving two different tenant forge hosts with the same `owner/repo` (and even a colliding ETag string) get independent cache entries. Full local gate re-run clean; force-pushed the rebased branch.

@JSONbored
JSONbored merged commit 0d5c340 into main Jul 13, 2026
14 of 15 checks passed
@JSONbored
JSONbored deleted the feat/policy-verdict-cache-4843 branch July 13, 2026 00:47
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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a persisted cross-invocation cache of resolved policy verdicts

1 participant