Part of #4496. P4 — medium severity, high confidence.
Context
computeImpactMap (src/review/impact-map.ts:64-99) calls retrieveContextWithMetrics once per changed-symbol file, capped at MAX_IMPACT_MAP_INPUT_FILES = 20 (impact-map.ts:38, 72-84). retrieveContextWithMetrics (src/review/rag.ts:460-534) performs a REAL embedding-model inference call (embedTexts, rag.ts:470-472) plus a live vector-index query per invocation — the ONLY memoization anywhere in the file is chunkCountCache (rag.ts:439, 60-second TTL), which caches solely the boolean "does this project/repo have any chunks at all" existence check, not query results, and only caches the positive case ("only cache the positive (cold→hot is one-way)", rag.ts:443).
So an unchanged PR re-embeds and re-queries the vector index from scratch — up to 20 times per pass — on every scheduled sweep tick past the 30-minute ai_review cooldown. Impact-map is folded into dynamicReviewFeatures for the same reason as grounding/RAG (processors.ts:9739-9742: "it can go stale for the SAME head SHA exactly like RAG"), so it bypasses the durable ai_review result cache and relies solely on the 30-minute AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS cooldown as its only throttle.
Notably, the sibling rag grounding feature (buildReviewRagContextWithMetrics, src/review/rag-wire.ts:180-194) issues retrieveContextWithMetrics exactly ONCE per pass for the whole PR — impact-map amplifies the identical per-fire cost by up to 20x for the same class of recurring, cooldown-gated re-embed.
Impact-map is double-gated off by default (global GITTENSORY_REVIEW_IMPACT_MAP env flag AND per-repo review.impact_map manifest opt-in, src/review/impact-map-wire.ts:19-31), so blast radius is limited to repos that have explicitly opted in — unlike the grounding bug (#4499), which affects the self-host DEFAULT.
Requirements
- Cache
retrieveContextWithMetrics results (or at minimum the computed embedding vector) keyed by a hash of (project, repo, queryText), with a short TTL — repeated impact-map computations for the same changed symbols on an unchanged head should not re-embed and re-query the vector index every cooldown cycle.
- Ensure the cache correctly invalidates when the underlying vector index itself changes (a new commit gets embedded/indexed) — this is about avoiding REDUNDANT identical queries, not serving stale results after the index legitimately changed.
- Invariant + regression tests (non-negotiable): an invariant test asserting a second
computeImpactMap call with the identical changed-symbol set makes zero additional embedTexts/vector-query calls; a regression test reproducing the exact bug (repeated sweep-tick-driven computeImpactMap calls on an unchanged head past the cooldown, asserting the up-to-20x embed+query loop only fires once); a test confirming a genuinely different query text (a real code change) DOES trigger a fresh embed+query (the negative case).
Deliverables
Expected outcome
Impact-map's per-cooldown-cycle cost drops from up to 20 real embedding+vector-query calls to zero on an unchanged head, for the (currently opt-in, but growing) set of repos with the feature enabled.
References
src/review/impact-map.ts:64-99 (the up-to-20x loop)src/review/rag.ts:439-474, 460-534 (chunkCountCache, retrieveContextWithMetrics)src/queue/processors.ts:9739-9742 (dynamic-feature cache-bypass rationale, impact-map's own comment)src/review/rag-wire.ts:180-194 (the sibling single-call-per-pass comparison)src/review/impact-map-wire.ts:19-31 (the double opt-in gate)
Effort
M
Part of #4496. P4 — medium severity, high confidence.
Context
computeImpactMap(src/review/impact-map.ts:64-99) callsretrieveContextWithMetricsonce per changed-symbol file, capped atMAX_IMPACT_MAP_INPUT_FILES = 20(impact-map.ts:38, 72-84).retrieveContextWithMetrics(src/review/rag.ts:460-534) performs a REAL embedding-model inference call (embedTexts,rag.ts:470-472) plus a live vector-index query per invocation — the ONLY memoization anywhere in the file ischunkCountCache(rag.ts:439, 60-second TTL), which caches solely the boolean "does this project/repo have any chunks at all" existence check, not query results, and only caches the positive case ("only cache the positive (cold→hot is one-way)",rag.ts:443).So an unchanged PR re-embeds and re-queries the vector index from scratch — up to 20 times per pass — on every scheduled sweep tick past the 30-minute
ai_reviewcooldown. Impact-map is folded intodynamicReviewFeaturesfor the same reason as grounding/RAG (processors.ts:9739-9742: "it can go stale for the SAME head SHA exactly like RAG"), so it bypasses the durableai_reviewresult cache and relies solely on the 30-minuteAI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MScooldown as its only throttle.Notably, the sibling
raggrounding feature (buildReviewRagContextWithMetrics,src/review/rag-wire.ts:180-194) issuesretrieveContextWithMetricsexactly ONCE per pass for the whole PR — impact-map amplifies the identical per-fire cost by up to 20x for the same class of recurring, cooldown-gated re-embed.Impact-map is double-gated off by default (global
GITTENSORY_REVIEW_IMPACT_MAPenv flag AND per-reporeview.impact_mapmanifest opt-in,src/review/impact-map-wire.ts:19-31), so blast radius is limited to repos that have explicitly opted in — unlike the grounding bug (#4499), which affects the self-host DEFAULT.Requirements
retrieveContextWithMetricsresults (or at minimum the computed embedding vector) keyed by a hash of(project, repo, queryText), with a short TTL — repeated impact-map computations for the same changed symbols on an unchanged head should not re-embed and re-query the vector index every cooldown cycle.computeImpactMapcall with the identical changed-symbol set makes zero additionalembedTexts/vector-query calls; a regression test reproducing the exact bug (repeated sweep-tick-drivencomputeImpactMapcalls on an unchanged head past the cooldown, asserting the up-to-20x embed+query loop only fires once); a test confirming a genuinely different query text (a real code change) DOES trigger a fresh embed+query (the negative case).Deliverables
hash(project, repo, queryText)with an appropriate short TTLExpected outcome
Impact-map's per-cooldown-cycle cost drops from up to 20 real embedding+vector-query calls to zero on an unchanged head, for the (currently opt-in, but growing) set of repos with the feature enabled.
References
src/review/impact-map.ts:64-99(the up-to-20x loop)src/review/rag.ts:439-474, 460-534(chunkCountCache,retrieveContextWithMetrics)src/queue/processors.ts:9739-9742(dynamic-feature cache-bypass rationale, impact-map's own comment)src/review/rag-wire.ts:180-194(the sibling single-call-per-pass comparison)src/review/impact-map-wire.ts:19-31(the double opt-in gate)Effort
M