Skip to content

feat(review): bypass the AI-review cache when impact-map is active - #3878

Merged
JSONbored merged 1 commit into
mainfrom
feat/impact-map-cache-bypass
Jul 7, 2026
Merged

feat(review): bypass the AI-review cache when impact-map is active#3878
JSONbored merged 1 commit into
mainfrom
feat/impact-map-cache-bypass

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • computeImpactMap (src/review/impact-map.ts) issues its own retrieveContextWithMetrics RAG queries against the live vector index, exactly like RAG's own retrieval -- so it can go stale for an unchanged head SHA without any of dynamicReviewFeatures' existing booleans (grounding/rag/enrichment/reputation/cultureProfile) flipping. Adds impactMap to dynamicReviewFeatures and ORs it into dynamicReviewContextActive (src/queue/processors.ts), following the exact pattern the culture-profile addition (feat(review): infer a cached per-repo quality-culture profile as AI-review grounding context #2995) used.
  • Adds impactMap to AiReviewCacheInput's features shape (src/review/ai-review-cache-input.ts) and bumps AI_REVIEW_CACHE_INPUT_VERSION v2->v3, so no prior cached fingerprint is silently reused under the new payload shape.
  • Adds a webhook-level regression test (test/unit/queue.test.ts, mirroring the existing culture-profile/grounding cache-bypass tests) asserting that with GITTENSORY_REVIEW_IMPACT_MAP on and the manifest opting in, a cached AI review is marked non-durable (cacheable=0) even when grounding/rag/enrichment/reputation are all off.

This exact one-line addition was implemented once already inside the PR that landed the deterministic impact-map foundation (#2182-#2186, merged as #3796), then reverted there before merge because writing a safe webhook-level test for it was disproportionate to that PR's scope. This PR is that follow-up, done properly with its own focused test.

No linked issue: #2182-#2186 are already closed (shipped in #3796); this is a small, self-contained gap-fill on top of already-merged work, not new user-facing scope.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • 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.
  • This is small enough that the summary explains why an issue is not needed (see Summary).

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally -- 100% branch coverage on every changed line/branch in this diff (verified via targeted lcov branch inspection against the exact diff ranges).
  • 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.

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. (N/A -- no auth/session/CORS surface touched.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A -- no API/MCP surface change.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A -- no UI change.)
  • Visible UI changes include a UI Evidence section. (N/A -- backend-only change, no UI.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

impact-map computation queries the same live vector index RAG does
(computeImpactMap issues its own retrieveContextWithMetrics calls), so it
can go stale for the same head SHA without dynamicReviewFeatures' existing
booleans flipping, exactly like grounding/rag/enrichment/reputation/
cultureProfile. Adds impactMap to dynamicReviewFeatures and
dynamicReviewContextActive, and to AiReviewCacheInput's features shape
(bumping the cache-input version so no prior fingerprint is silently
reused under the new payload shape).

This was implemented once already in the PR that landed the deterministic
impact-map foundation, then reverted there because a safe webhook-level
test was disproportionate to that PR's scope. Adds that test here.
@superagent-security

Copy link
Copy Markdown
Contributor

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

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

loopover-orb Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-07 00:37:46 UTC

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

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This PR wires a fifth `impactMap` boolean into the existing `dynamicReviewFeatures` cache-bypass mechanism, following the exact structural pattern the cultureProfile addition (#2995) used: add the field to `AiReviewCacheInput.features`, include it in the fingerprint payload, OR it into `dynamicReviewContextActive`, and bump `AI_REVIEW_CACHE_INPUT_VERSION` v2→v3 so no stale fingerprint is silently reused. The rationale (computeImpactMap issues its own live RAG queries against the vector index, so it can go stale for an unchanged head SHA just like grounding/rag) is sound and mirrors an already-accepted pattern. The new webhook-level test at test/unit/queue.test.ts asserts `cacheable=0` when impact-map is on and everything else is off, which is the correct regression coverage for this exact change, and all existing `features: {...}` object literals across the test file were mechanically updated to include `impactMap: false` so prior tests keep compiling/passing.

Nits — 5 non-blocking

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 51 registered-repo PR(s), 43 merged, 352 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 352 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
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: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 51 PR(s), 352 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.54%. Comparing base (daef526) to head (26573e5).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3878   +/-   ##
=======================================
  Coverage   93.54%   93.54%           
=======================================
  Files         338      338           
  Lines       33335    33335           
  Branches    12192    12192           
=======================================
  Hits        31182    31182           
  Misses       1528     1528           
  Partials      625      625           
Files with missing lines Coverage Δ
src/queue/processors.ts 94.52% <ø> (ø)
src/review/ai-review-cache-input.ts 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 manual-review Gittensor contributor context label Jul 7, 2026
@JSONbored
JSONbored merged commit 56421ce into main Jul 7, 2026
10 checks passed
@JSONbored
JSONbored deleted the feat/impact-map-cache-bypass branch July 7, 2026 00:41
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.

1 participant