Skip to content

feat(review): render the deterministic impact map in the unified review comment (#1971) - #4056

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
real-venus:feat/gittensory-impact-map-render
Jul 7, 2026
Merged

feat(review): render the deterministic impact map in the unified review comment (#1971)#4056
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
real-venus:feat/gittensory-impact-map-render

Conversation

@real-venus

Copy link
Copy Markdown
Contributor

Summary

Renders the deterministic impact map in the unified review comment. The impact-map feature (#2182-#2186) already computes a RAG-derived "which other modules plausibly need a second look" map and splices it into the AI-review prompt, and buildImpactMapCollapsible (src/review/unified-comment-bridge.ts) already knows how to render it — but no production caller ever passed the entries to buildUnifiedCommentBody, so the map was invisible to the human reading the PR comment; only the model saw it.

This threads the entries this pass already computed out of runAiReviewForAdvisory (right next to inlineFindings) and renders them as the "Impact map" collapsible at the publish site — reusing the same array, so there is no second RAG query.

Closes #1971

Design

  • runAiReviewForAdvisory: the block-scoped impactMap computed for the prompt is hoisted to a function-scoped impactMapEntries and added to the review result (a new optional impactMap?: ImpactMapEntry[] field, mirroring inlineFindings).
  • Publish site (maybePublishPrPublicSurface): passes impactMap: aiReview?.impactMap ?? [] to buildUnifiedCommentBody, right after findingCategories. ImpactMapEntry IS the bridge's ImpactMapSummaryInput shape (documented as such), so it's a zero-transform pass-through.
  • Byte-identical when off / empty / cached. Entries exist only on a fresh review when shouldComputeImpactMap ANDs the operator GITTENSORY_REVIEW_IMPACT_MAP flag with the per-repo review.impact_map manifest opt-in. A cache hit / frozen reuse / skipped review carries none (undefined ⇒ []), and buildImpactMapCollapsible returns null for an empty list — so no section renders and the comment is unchanged. No new flag, no config surface, no migration.

Tests

  • test/unit/impact-map-processor-wiring.test.ts: extended the flag-on case to assert the review result now carries the threaded impactMap entries (correct changedModule + affectedModules), so the publish site has real data to render.
  • The render itself (buildUnifiedCommentBody → "Impact map" collapsible) is already covered by test/unit/impact-map-collapsible.test.ts (section present with entries, absent/empty without); the new publish-site pass-through's branches (present vs cache-hit/skipped) are exercised by the existing full-review + cache-hit queue tests.

Validation

  • git diff --check — clean.
  • npm run typecheck — clean.
  • npm run test:coverage (full, unsharded) — no logic failures; codecov/patch = 100% of the changed src/** lines and branches. Only the known Windows spawn claude/codex/docker ENOENT env suites fail locally (green on Linux CI).

If any required check was skipped, explain why:

  • test:workers / build:mcp / ui:* / actionlint / npm audit were not run locally — this is an additive backend review-rendering change touching no worker entrypoint, MCP package, UI, workflow, or dependency; CI covers them.

Safety

  • No secrets/wallet/hotkey/trust-score/reward terms exposed — impact-map paths come from the repo's OWN RAG index (never user input) and the renderer already escapes each path cell defensively.
  • Public GitHub text stays sanitized; the section is a single deterministic table, default-OFF.
  • No auth/cookie/CORS/GitHub App/session change.
  • No API/OpenAPI/MCP surface change.
  • No UI change.
  • No docs/changelog change needed (GITTENSORY_REVIEW_IMPACT_MAP + review.impact_map are already documented).

UI Evidence

N/A — backend review-comment rendering; no dashboard/frontend/extension surface.

Notes

  • Deterministic, no AI: the rendered map is computeImpactMap's RAG-retrieval output, labelled "from the codebase index, not an AI guess" by the existing renderer. No extra env.AI.run call — the entries are the ones the prompt already used.

…ew comment (JSONbored#1971)

The impact-map feature (JSONbored#2182-JSONbored#2186) computes a deterministic RAG-derived
impact map and splices it into the AI-review prompt, and buildImpactMapCollapsible
(unified-comment-bridge) already knows how to render it — but no production caller
ever passed the entries to buildUnifiedCommentBody, so the map was invisible to the
maintainer/contributor reading the PR comment.

Thread the entries this pass already computed out of runAiReviewForAdvisory (next
to inlineFindings) and render them as the "Impact map" collapsible at the publish
site — reusing the SAME array, no second RAG query. Present only on a fresh review
(a cache hit / frozen reuse carries none); empty or feature-off ⇒
buildImpactMapCollapsible returns null ⇒ the comment is byte-identical.

Closes JSONbored#1971
@real-venus
real-venus requested a review from JSONbored as a code owner July 7, 2026 17:53
@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:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 7, 2026
@loopover-orb

loopover-orb Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Tip

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

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-07 18:04:12 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
Threads the impact-map entries already computed in runAiReviewForAdvisory (for the AI prompt) out through the review result and into maybePublishPrPublicSurface, reusing the exact same array so there's no second RAG query. The wiring mirrors the established inlineFindings/findingCategories pattern closely: impactMapEntries is always initialized to [] and only populated when shouldComputeImpactMap gates it on, a cached/frozen review lacks the field entirely so aiReview?.impactMap ?? [] correctly degrades to empty, and buildImpactMapCollapsible (unchanged) already returns null for an empty list — so off/cached/empty stays byte-identical as claimed. The change is narrow, closes the linked issue #1971, needs no schema or config changes, and CI is green.

Nits — 5 non-blocking
  • The new tests only prove the negative case (frozen reuse ⇒ no 'Impact map' section) and that runAiReviewForAdvisory's result carries impactMap entries in isolation — there's no end-to-end test asserting a FRESH review with the flag on and non-empty entries actually renders the 'Impact map' collapsible in the published comment body, which is the core behavior this PR claims to add at src/queue/processors.ts's `impactMap: aiReview?.impactMap ?? []` call site.
  • The flagged 'generic_secret_assignment' at test/unit/queue.test.ts:18286 looks like a fake test env value (GITHUB_APP_PRIVATE_KEY/AI budget fixtures used throughout this test file), but worth a quick confirm it's not an actual credential before merging.
  • src/queue/processors.ts is already ~10k lines; not something this diff should fix, but each new optional field threaded through these giant inline result-object type literals (inlineFindings, impactMap, metadata, cacheable...) makes them harder to scan — consider whether these result shapes are due for extraction into a named type at some point.
  • Add one integration test alongside the existing frozen-reuse test in test/unit/queue.test.ts that turns the impact-map flag+manifest opt-in ON with a fresh (non-cached) review and asserts the published comment body actually contains the 'Impact map' collapsible text, closing the positive-path coverage gap.
  • Consider a brief comment or test noting whether a fresh review's cache-write path persists the impactMap field at all (i.e., confirm it's intentionally excluded from the cached review object, not just accidentally dropped) since that's what makes the frozen-reuse-has-none behavior correct.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #1971
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: 116 registered-repo PR(s), 69 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor real-venus; Gittensor profile; 116 PR(s), 0 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: real-venus
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript, JavaScript, MDX, Rust, Svelte, Swift
  • Official Gittensor activity: 116 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 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.59%. Comparing base (66a70b8) to head (48b6f88).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4056   +/-   ##
=======================================
  Coverage   93.59%   93.59%           
=======================================
  Files         379      379           
  Lines       35582    35584    +2     
  Branches    13049    13050    +1     
=======================================
+ Hits        33302    33304    +2     
  Misses       1618     1618           
  Partials      662      662           
Files with missing lines Coverage Δ
src/queue/processors.ts 94.95% <100.00%> (+<0.01%) ⬆️
🚀 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 0d1af9a into JSONbored:main Jul 7, 2026
8 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. labels Jul 7, 2026
@real-venus
real-venus deleted the feat/gittensory-impact-map-render branch July 7, 2026 18:05
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.

Deterministic architecture / impact map

1 participant