feat(review): deterministic impact map (#2182-#2186) - #3796
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3796 +/- ##
==========================================
+ Coverage 93.37% 93.38% +0.01%
==========================================
Files 320 323 +3
Lines 32647 32727 +80
Branches 11960 11982 +22
==========================================
+ Hits 30483 30563 +80
Misses 1530 1530
Partials 634 634
🚀 New features to boost your workflow:
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gittensory-ui | 60a35e0 | Commit Preview URL Branch Preview URL |
Jul 06 2026, 11:59 AM |
This was
linked to
issues
Jul 6, 2026
Closed
JSONbored
force-pushed
the
feat/impact-map-foundation
branch
from
July 6, 2026 11:21
41e157f to
cb913aa
Compare
![superagent-security[bot]](https://avatars.githubusercontent.com/in/3287076?s=60&v=4){kind=small}
| * INPUT file order; each entry's `affectedModules` follows RAG's own retrieval order (cosine + optional BM25 | ||
| * rerank, both already deterministic). Fail-safe: no vector/inference adapter, a cold/empty index, or any | ||
| * retrieval error yields an EMPTY impact map, never a throw. | ||
| */ |
Contributor
There was a problem hiding this comment.
Unbounded RAG queries from attacker-controlled changed-file count in computeImpactMap
Unbounded loop over changed files issues one RAG query per file without a max-files cap.
Cap the number of changed files processed (e.g. 10–20) before the loop to bound vector-query cost.
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name="src/review/impact-map.ts">
<violation number="1" location="src/review/impact-map.ts:55">
<priority>P2</priority>
<title>Unbounded RAG queries from attacker-controlled changed-file count in computeImpactMap</title>
<evidence>computeImpactMap iterates over every changed file with extracted symbols and issues a separate retrieveContextWithMetrics call per file. The symbols array length is determined by the number of changed files in the PR (attacker-controlled input), and there is no upper bound on iterations. A PR with hundreds or thousands of changed JS/TS files could exhaust vector-query quota, cause worker timeouts, or inflate inference cost.</evidence>
<recommendation>Cap the number of files processed by computeImpactMap (e.g., MAX_IMPACT_MAP_ENTRIES = 10 or 20) before the loop, matching the downstream prompt/comment caps, so the computation cost is bounded regardless of PR size.</recommendation>
</violation>
</file>
superagent-security
Bot
added
the
pr:flagged
…2184) Adds a pure changed-symbol extractor (impact-symbols.ts, #2182), a deterministic impact-map computation over the existing RAG index (impact-map.ts, #2183), and the review.impact_map config toggle (#2184) that gates both, ANDed with a new operator env kill-switch (GITTENSORY_REVIEW_IMPACT_MAP). Default OFF and byte-identical when unset; no wiring into rendering or AI grounding yet (follow-up PR). Part of #1971.
…2185) Adds buildImpactMapCollapsible (and its ImpactMapSummaryInput type) to unified-comment-bridge.ts, following the same pattern as the existing Changed-files/Finding-categories collapsibles: one row per changed module with its changed symbols and a bounded, "+N more"-capped list of plausibly-affected modules. Public-safe path escaping. Wired into buildUnifiedCommentBody as an additive, flag-gated section (omitted entirely when the impact map is absent/empty). Part of #1971.
Adds GittensoryAiReviewInput.impactMapContext (mirroring ragContext) and splices it into the reviewer's user prompt when present, plus formatImpactMapPromptSection in impact-map-wire.ts to format computeImpactMap's output into a bounded "IMPACT MAP" block. Wires the computation into runAiReviewForAdvisory (processors.ts), reusing the already-resolved changed files and gated by BOTH the operator's GITTENSORY_REVIEW_IMPACT_MAP flag and the per-repo review.impact_map manifest opt-in. Additive-only; byte-identical when the flag is off or the computed map is empty. Part of #1971. Completes the group (#2182-#2186).
…angler version The rebased branch carried a worker-configuration.d.ts generated against an older wrangler version; wrangler types --check flagged it as drifted.
Superagent (P2): computeImpactMap issued one retrieveContextWithMetrics call per changed-symbol file with no upper bound -- a PR touching hundreds of files (contributor-controlled) would issue hundreds of vector queries. Add MAX_IMPACT_MAP_INPUT_FILES (20, matching boundary-test-generation.ts's MAX_TOUCHES precedent for the same per-changed-file-loop concern) and filter+slice before the query loop, preserving deterministic input order. Also fixes config-templates.test.ts drift: the review.impact_map documentation block was only added to .gittensory.yml.example, not its config/examples/gittensory.full.yml counterpart (both copies), which the drift test requires to stay byte-identical from the canonical body marker onward.
JSONbored
force-pushed
the
feat/impact-map-foundation
branch
from
July 6, 2026 11:57
cb913aa to
da8c4c9
Compare
JSONbored
force-pushed
the
feat/impact-map-foundation
branch
from
July 6, 2026 11:57
da8c4c9 to
60a35e0
Compare
Owner
Author
|
Fixed all three real issues:
Deliberately did not wire `impact_map` into `dynamicReviewFeatures`'s AI-review-cache bypass in this PR — #3802 (repo quality-culture profile) is concurrently touching that exact same fingerprint object, and doing both at once risks the same collision this rebase just untangled. Left as its own tracked follow-up. 595 tests pass across all touched files, typecheck/actionlint/audit clean. |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
superagent-security
Bot
removed
the
pr:flagged
loopover-orb
Bot
added
the
gittensor:bug
24 tasks
This was referenced Jul 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/review/impact-symbols.ts— a pure, bounded regex extractor that pulls exported top-level symbol names (function/class/const/type/interface/enum) touched by a PR's changed-file patches. Reusesrag.ts'sRagBoundaryvocabulary. Fail-safe (never throws; unparseable/non-JS-TS input yields no symbols).src/review/impact-map.ts—computeImpactMapresolves, per changed file with at least one extracted symbol, the other repo files the existing RAG index (retrieveContextWithMetrics) surfaces as related — "files that plausibly need re-checking," not a guaranteed call graph. Bounded (MAX_AFFECTED_MODULES_PER_ENTRY = 8), deterministic ordering, fail-safe.review.impact_map(FocusManifestReviewConfig.impactMap) — default-null/off, follows the exacteffort_score/changed_files_summarywiring pattern (parse/present/round-trip/resolveReviewPromptOverrides). ANDed with a new operator kill-switchGITTENSORY_REVIEW_IMPACT_MAP(src/review/impact-map-wire.ts'sisImpactMapEnabled/shouldComputeImpactMap, mirroringisRagEnabled).buildImpactMapCollapsibleinunified-comment-bridge.ts— a compact "Impact map" collapsible (changed module → changed symbols → plausibly-affected modules, capped with "+N more"), wired intobuildUnifiedCommentBodythe same waybuildChangedFilesSummaryCollapsibleis.GittensoryAiReviewInput.impactMapContext(mirrorsragContextexactly) +formatImpactMapPromptSection(bounded "IMPACT MAP" prompt block, entry-count and char-budget capped) spliced into the reviewer's user prompt. Wired intorunAiReviewForAdvisory(processors.ts), reusing the already-resolved changed files (no extra fetch).GITTENSORY_REVIEW_IMPACT_MAPunset (the wrangler.jsonc default) orreview.impact_mapunset/false, every new code path is unreachable and the review/comment/prompt stay byte-identical to today.impactMaptoprocessors.ts'sdynamicReviewFeatures(the AI-review cache dynamic-context bypass, since impact-map queries the same live vector index RAG does and could go stale in the same way). I reverted that one-line addition because giving it proper webhook-level test coverage (via theprocessJob({ type: "agent-regate-pr" })harness) needed a fixture setup disproportionate to this PR; I filed it as a follow-up instead of leaving it untested or expanding scope further.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not touchsite/,CNAME, or**/lovable/**.Validation
git diff --checknpm run actionlint(no workflow files changed)npm run typechecknpm run test:coveragelocally (targeted to the changed files — new modules at 100% line/branch;unified-comment-bridge.ts/ai-review.ts/focus-manifest.tsdiff lines fully covered, verified againstgit diffline ranges)npm run test:workersnpm run build:mcpnpm run test:mcp-pack(unaffected — no MCP package changes)npm run ui:openapi:checknpm run ui:lint(0 errors; only pre-existing warnings on the two docs files I touched)npm run ui:typecheck(pre-existing environment gap in this sandbox:@lovable.dev/vite-tanstack-configis unresolvable even on a cleangit stash'd tree — not introduced by this PR)npm run ui:build(blocked by the same pre-existingui:typecheck-adjacent environment gap)npm audit --audit-level=moderateIf any required check was skipped, explain why:
ui:typecheck/ui:build: this sandbox is missing the private@lovable.dev/vite-tanstack-configdevDependency; reproduced on a cleanmaincheckout viagit stash, so it predates and is unrelated to this PR.ui:lint(which does run) found 0 errors on the two docs files I edited.npm run test:mcp-pack/npm run actionlint: no MCP package or workflow files touched by this diff.Also ran and green:
npm run db:migrations:check,npm run db:schema-drift:check,npm run selfhost:env-reference:check,npm run cf-typegen:check,npm run docs:drift-check(caught + fixed two real doc gaps for the newGITTENSORY_REVIEW_IMPACT_MAPflag).New tests added:
test/unit/impact-symbols.test.ts,test/unit/impact-map.test.ts,test/unit/impact-map-wire.test.ts,test/unit/impact-map-collapsible.test.ts,test/unit/impact-map-grounding.test.ts,test/unit/impact-map-processor-wiring.test.ts, plus updates totest/unit/focus-manifest.test.ts/test/unit/signals-coverage.test.tsfor the new config field.Safety
docs.tuning.tsx,docs.privacy-security.tsx,.gittensory.yml.example).UI Evidence
Not applicable — no visible UI/frontend changes in this PR (two documentation route text edits only, verified via
ui:lint).Notes
impactMapto the AI-review cache'sdynamicReviewFeaturesdynamic-context bypass inprocessors.ts, with proper webhook-level test coverage — see rationale in the Summary above.