Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,11 @@ declare global {
* unreachable when off). Even when ON, retrieval is INERT until the self-host vector index is populated for
* the repo (a cold/missing index degrades to no context). */
GITTENSORY_REVIEW_RAG?: string;
/** Deterministic impact map (#2184, part of #1971): operator-level kill-switch, ANDed with the per-repo
* `.gittensory.yml review.impact_map` opt-in (see review/impact-map-wire's isImpactMapEnabled /
* shouldComputeImpactMap). Default OFF — unset/false performs NO symbol extraction, NO RAG query, and adds
* NO comment/prompt section, byte-identical to today. */
/** Deterministic impact map (#2184, part of #1971): operator-level master kill-switch; the per-repo
* `.gittensory.yml review.impact_map` opt-in fully decides once this is on (#4102 precedence — see
* review/impact-map-wire's isImpactMapEnabled / shouldComputeImpactMap). Default OFF — unset/false
* performs NO symbol extraction, NO RAG query, and adds NO comment/prompt section, byte-identical to
* today. */
GITTENSORY_REVIEW_IMPACT_MAP?: string;
/** Repo quality-culture profile (#2995): when truthy, the AI reviewer prompt gains an ADDITIVE "REPO
* QUALITY-CULTURE PROFILE" reference block — typical merged-PR size + common accepted labels, derived
Expand Down
24 changes: 13 additions & 11 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6934,10 +6934,11 @@ export async function runAiReviewForAdvisory(
// manifest. Self-host only — overrides that repo's claude-code/codex model+effort, taking priority over the
// operator's global env vars. Absent/all-null ⇒ byte-identical (global env var, then provider default).
reviewSelfHostAiModel?: SelfHostAiModelConfig | undefined;
// `.gittensory.yml` review.impact_map (#2184/#2186), resolved by the caller from the cached manifest. ANDed
// here with the operator's GITTENSORY_REVIEW_IMPACT_MAP flag (shouldComputeImpactMap) to decide whether to
// compute the deterministic impact map and splice it into the reviewer prompt as additive reference
// context. Absent/false ⇒ byte-identical reviewer prompt (no impact-map computation, no RAG query for it).
// `.gittensory.yml` review.impact_map (#2184/#2186), resolved by the caller from the cached manifest.
// Precedence (#4102): the operator's GITTENSORY_REVIEW_IMPACT_MAP flag is a master kill-switch, never
// bypassable by config (shouldComputeImpactMap); an explicit true/false here then fully controls whether to
// compute the deterministic impact map and splice it into the reviewer prompt. Absent ⇒ byte-identical
// reviewer prompt (no impact-map computation, no RAG query for it).
reviewImpactMap?: boolean | undefined;
// `.gittensory.yml` review.culture_profile (#2995), resolved by the caller from the cached manifest. ANDed
// here with the GITTENSORY_REVIEW_CULTURE_PROFILE global flag to decide whether to append the repo's
Expand Down Expand Up @@ -7136,16 +7137,17 @@ export async function runAiReviewForAdvisory(
: undefined;
const ragTelemetry =
ragContextResult?.telemetry ?? emptyReviewRagTelemetry(false);
// Deterministic impact map (#2184/#2186), ANDed operator env flag + per-repo review.impact_map opt-in
// (shouldComputeImpactMap). Reuses the SAME changed files this pass already resolved — no extra fetch.
// Flag-OFF (default) → NO new branch: no symbol extraction, no RAG query, and `impactMapContext` is left
// undefined so the prompt is byte-identical to today. Fully fail-safe (computeImpactMap never throws; a
// missing/cold RAG index degrades to an empty impact map, which formats to "" and appends nothing).
// Deterministic impact map (#2184/#2186): operator env kill-switch + per-repo review.impact_map opt-in
// (shouldComputeImpactMap, #4102 precedence). Reuses the SAME changed files this pass already resolved — no
// extra fetch. Flag-OFF (default) → NO new branch: no symbol extraction, no RAG query, and
// `impactMapContext` is left undefined so the prompt is byte-identical to today. Fully fail-safe
// (computeImpactMap never throws; a missing/cold RAG index degrades to an empty impact map, which formats
// to "" and appends nothing).
let impactMapContext: string | undefined;
// The computed entries are ALSO threaded out of this function (#1971) so the publish site can render the
// "Impact map" collapsible from the exact same array — no second RAG query. Empty when the feature is off.
let impactMapEntries: ImpactMapEntry[] = [];
if (shouldComputeImpactMap(env, args.reviewImpactMap === true)) {
if (shouldComputeImpactMap(env, args.reviewImpactMap)) {
const [impactMapProject, impactMapRepo] = splitRepoForRag(args.repoFullName);
const changedSymbols = extractChangedSymbols(
files.map((file) => ({
Expand Down Expand Up @@ -9299,7 +9301,7 @@ async function maybePublishPrPublicSurface(
// Impact map (#2182-#2186): queries the SAME live vector index RAG does (computeImpactMap issues
// its own retrieveContextWithMetrics calls), so it can go stale for the SAME head SHA exactly like
// RAG — a repo with it active also bypasses the AI-review result cache.
impactMap: shouldComputeImpactMap(env, reviewImpactMap === true),
impactMap: shouldComputeImpactMap(env, reviewImpactMap),
};
const dynamicReviewContextActive =
dynamicReviewFeatures.grounding ||
Expand Down
33 changes: 22 additions & 11 deletions src/review/impact-map-wire.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// Impact-map activation wiring (#2184, config slice of #1971). Mirrors rag-wire.ts's isRagEnabled: a single
// GLOBAL env kill-switch the self-host operator controls, ANDed with the per-repo `.gittensory.yml
// review.impact_map` manifest toggle (resolved via `resolveReviewPromptOverrides`'s `impactMap` field) — so a
// repo can only ever NARROW what the operator has already turned on, never widen it. Both OFF by default:
// with the env flag unset, impact-map computation is never invoked from the review path at all (the caller
// guards on this flag before doing any RAG query or rendering), so the review stays byte-identical to today.
// Impact-map activation wiring (#2184, config slice of #1971; activation precedence fixed for #4102). Default
// OFF: the operator flag GITTENSORY_REVIEW_IMPACT_MAP is a master kill-switch, and the per-repo `.gittensory.yml
// review.impactMap` toggle (resolved via `resolveReviewPromptOverrides`'s `impactMap` field) fully controls
// activation by itself when explicitly set — impact-map has never had a `GITTENSORY_REVIEW_REPOS` cutover
// allowlist fallback (unlike rag/reputation/grounding/safety/unifiedComment), so there is nothing for a repo
// toggle to bypass. With the env flag unset, impact-map computation is never invoked from the review path at
// all (the caller guards on this flag before doing any RAG query or rendering), so the review stays
// byte-identical to today.
//
// Also hosts the AI-review grounding formatter (#2186): `formatImpactMapPromptSection` turns
// `computeImpactMap`'s output into the bounded "IMPACT MAP" block spliced into the reviewer's user prompt via
Expand All @@ -20,14 +22,23 @@ export function isImpactMapEnabled(env: { GITTENSORY_REVIEW_IMPACT_MAP?: string
return /^(1|true|yes|on)$/i.test(env.GITTENSORY_REVIEW_IMPACT_MAP ?? "");
}

/** Resolve whether impact-map computation should run for THIS repo/PR: the operator's global env kill-switch
* AND the per-repo manifest opt-in. Neither alone is sufficient — mirrors every other converged-feature gate
* in this codebase (env kill-switch first, then the manifest narrows it further). */
/** PURE (#4102): should impact-map computation run for THIS repo/PR? (1) The operator's
* GITTENSORY_REVIEW_IMPACT_MAP flag is an absolute MASTER KILL-SWITCH — off ⇒ always false, regardless of the
* manifest, and no per-repo config can bypass it (consistent with every other converged feature — see
* `resolveConvergedFeature` in `feature-activation.ts`). (2) An explicit per-repo `.gittensory.yml`
* `review.impactMap` override (`true`/`false`) FULLY controls the feature by itself once the kill-switch is
* on — impact-map has never had a `GITTENSORY_REVIEW_REPOS` cutover-allowlist fallback, so there is no
* allowlist for a repo toggle to bypass. (3) `manifestToggle` unset (`undefined`) preserves this feature's
* ORIGINAL always-off-unless-both-set default exactly: with no allowlist to fall back to, an unset manifest
* toggle stays `false`, byte-identical to every repo's behavior before this change. Exactly mirrors
* `shouldRequestInlineFindings`/`shouldEmitFixHandoff`'s shape and precedence (src/review/inline-comments.ts,
* src/review/fix-handoff.ts). */
export function shouldComputeImpactMap(
env: { GITTENSORY_REVIEW_IMPACT_MAP?: string | undefined },
manifestImpactMapEnabled: boolean,
manifestToggle: boolean | undefined,
): boolean {
return isImpactMapEnabled(env) && manifestImpactMapEnabled;
if (!isImpactMapEnabled(env)) return false;
return manifestToggle === true;
}

/** Hard cap on entries actually formatted into the AI-review prompt section — bounds prompt-token cost
Expand Down
20 changes: 16 additions & 4 deletions test/unit/impact-map-wire.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,34 @@ describe("isImpactMapEnabled", () => {
});
});

describe("shouldComputeImpactMap", () => {
it("requires BOTH the operator env flag AND the per-repo manifest opt-in", () => {
describe("shouldComputeImpactMap (#4102 precedence: env kill-switch, then manifestToggle === true fully decides)", () => {
it("is ON when the operator flag is on and the manifest explicitly opted in", () => {
expect(shouldComputeImpactMap({ GITTENSORY_REVIEW_IMPACT_MAP: "true" }, true)).toBe(true);
});

it("is OFF when the operator flag is on but the manifest didn't opt in", () => {
it("is OFF when the operator flag is on but the manifest explicitly opted out", () => {
expect(shouldComputeImpactMap({ GITTENSORY_REVIEW_IMPACT_MAP: "true" }, false)).toBe(false);
});

it("is OFF when the manifest opted in but the operator flag is off (repo cannot self-enable)", () => {
it("is OFF when the operator flag is on but the manifest toggle is unset (regression: preserves the ORIGINAL always-off default — there is no allowlist fallback for impactMap)", () => {
expect(shouldComputeImpactMap({ GITTENSORY_REVIEW_IMPACT_MAP: "true" }, undefined)).toBe(false);
});

it("is OFF when the manifest opted in but the operator flag is off — the env flag is an absolute master kill-switch no per-repo config can bypass", () => {
expect(shouldComputeImpactMap({ GITTENSORY_REVIEW_IMPACT_MAP: "false" }, true)).toBe(false);
});

it("is OFF when the operator flag is unset and the manifest opted in — the kill-switch still wins", () => {
expect(shouldComputeImpactMap({}, true)).toBe(false);
});

it("is OFF when both are off", () => {
expect(shouldComputeImpactMap({}, false)).toBe(false);
});

it("is OFF when both are unset", () => {
expect(shouldComputeImpactMap({}, undefined)).toBe(false);
});
});

describe("formatImpactMapPromptSection (#2186)", () => {
Expand Down
4 changes: 2 additions & 2 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6247,10 +6247,10 @@ describe("queue processors", () => {
AI_SUMMARIES_ENABLED: "true",
AI_PUBLIC_COMMENTS_ENABLED: "true",
AI_DAILY_NEURON_BUDGET: "100000",
// Both gates on: the global capability switch, and (like culture-profile above, unlike
// Both gates on: the global kill-switch, and (like culture-profile above, unlike
// grounding/enrichment/RAG/reputation which are env-only) the per-repo `.gittensory.yml` opt-in mocked
// below, so `dynamicReviewFeatures.impactMap` (src/queue/processors.ts) actually evaluates
// shouldComputeImpactMap's `&&` right-hand side true, not just short-circuits.
// shouldComputeImpactMap's `manifestToggle === true` arm true, not just the kill-switch short-circuit.
GITTENSORY_REVIEW_IMPACT_MAP: "true",
});
await persistRegistrySnapshot(
Expand Down