diff --git a/.claude/skills/titan-gate/SKILL.md b/.claude/skills/titan-gate/SKILL.md index a660bcae5..fbd495838 100644 --- a/.claude/skills/titan-gate/SKILL.md +++ b/.claude/skills/titan-gate/SKILL.md @@ -45,6 +45,8 @@ codegraph check --staged --cycles --blast-radius 30 --boundaries -T --json This checks: manifesto rules, new cycle introduction, blast radius threshold, and architecture boundary violations. Exit code 0 = pass, 1 = fail. +The `blast-radius` predicate is call-graph-shape-aware: a touched function's absolute transitive-caller count only counts toward the threshold if the diff actually changed that function's own declaration line or the set of call targets referenced in its body. A function whose body changed without touching a call expression or its own signature (e.g. an inline literal replaced by a named constant) is exempt even with a high absolute caller count — that fan-in is pre-existing risk already accepted by the codebase, not risk this diff introduced. This is the authoritative blast-radius pass/fail — see Step 8. + Also run detailed impact analysis: ```bash @@ -302,9 +304,9 @@ Advisory — prevents jumping ahead and creating conflicts. ## Step 8 — Blast radius check -From diff-impact results: -- Transitive blast radius > 30 → FAIL -- Transitive blast radius > 15 → WARN +The FAIL/PASS decision comes from Step 1's `codegraph check --staged --blast-radius 30` predicate — do not re-derive it from diff-impact's raw numbers, which are absolute transitive-caller counts and are NOT shape-aware: +- Step 1's `blast-radius` predicate failed → FAIL (a touched function's own call graph shape genuinely changed and its transitive callers exceed 30) +- Step 1's `blast-radius` predicate passed but a changed function's raw `transitiveCallers` (from diff-impact) > 15 → WARN (worth a second look even though not gating — this may include functions Step 1 exempted as pre-existing fan-in) - Historically coupled file NOT staged? → WARN ("consider also updating X") --- diff --git a/docs/examples/claude-code-skills/titan-gate/SKILL.md b/docs/examples/claude-code-skills/titan-gate/SKILL.md index a660bcae5..fbd495838 100644 --- a/docs/examples/claude-code-skills/titan-gate/SKILL.md +++ b/docs/examples/claude-code-skills/titan-gate/SKILL.md @@ -45,6 +45,8 @@ codegraph check --staged --cycles --blast-radius 30 --boundaries -T --json This checks: manifesto rules, new cycle introduction, blast radius threshold, and architecture boundary violations. Exit code 0 = pass, 1 = fail. +The `blast-radius` predicate is call-graph-shape-aware: a touched function's absolute transitive-caller count only counts toward the threshold if the diff actually changed that function's own declaration line or the set of call targets referenced in its body. A function whose body changed without touching a call expression or its own signature (e.g. an inline literal replaced by a named constant) is exempt even with a high absolute caller count — that fan-in is pre-existing risk already accepted by the codebase, not risk this diff introduced. This is the authoritative blast-radius pass/fail — see Step 8. + Also run detailed impact analysis: ```bash @@ -302,9 +304,9 @@ Advisory — prevents jumping ahead and creating conflicts. ## Step 8 — Blast radius check -From diff-impact results: -- Transitive blast radius > 30 → FAIL -- Transitive blast radius > 15 → WARN +The FAIL/PASS decision comes from Step 1's `codegraph check --staged --blast-radius 30` predicate — do not re-derive it from diff-impact's raw numbers, which are absolute transitive-caller counts and are NOT shape-aware: +- Step 1's `blast-radius` predicate failed → FAIL (a touched function's own call graph shape genuinely changed and its transitive callers exceed 30) +- Step 1's `blast-radius` predicate passed but a changed function's raw `transitiveCallers` (from diff-impact) > 15 → WARN (worth a second look even though not gating — this may include functions Step 1 exempted as pre-existing fan-in) - Historically coupled file NOT staged? → WARN ("consider also updating X") --- diff --git a/src/cli/commands/check.ts b/src/cli/commands/check.ts index a00cec970..9bac0c693 100644 --- a/src/cli/commands/check.ts +++ b/src/cli/commands/check.ts @@ -29,7 +29,10 @@ export const command: CommandDefinition = { ['--staged', 'Analyze staged changes'], ['--rules', 'Also run manifesto rules alongside diff predicates'], ['--cycles', 'Assert no dependency cycles involve changed files'], - ['--blast-radius ', 'Assert no function exceeds N transitive callers'], + [ + '--blast-radius ', + 'Assert no function whose call graph shape changed (signature or call targets) exceeds N transitive callers', + ], ['--signatures', 'Assert no exported function/method/class declaration lines were modified'], ['--boundaries', 'Assert no cross-owner boundary violations'], ['--depth ', 'Max BFS depth for blast radius (default: 3)'], diff --git a/src/features/check.ts b/src/features/check.ts index 7b959df94..12533d64c 100644 --- a/src/features/check.ts +++ b/src/features/check.ts @@ -20,6 +20,19 @@ interface ParsedDiff { changedRanges: Map; oldRanges: Map; newFiles: Set; + /** + * One entry per `changedRanges` run (same file, same start/end), carrying + * the actual added-line text plus whatever removed-line text immediately + * preceded it within the same hunk (empty for a pure insertion). Powers + * `checkMaxBlastRadius`'s call-graph-shape exemption — see issue #1740. + */ + changedEdits: Map; +} + +/** An added-line run paired with whatever it replaced, for shape comparison. */ +interface DiffTextEdit extends DiffRange { + addedText: string[]; + removedText: string[]; } const HUNK_RE = /^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/; @@ -65,10 +78,21 @@ class DiffLineTracker { private removedRunEnd: number | null = null; private addedRunStart: number | null = null; private addedRunEnd: number | null = null; + private currentRemovedText: string[] = []; + private currentAddedText: string[] = []; + /** + * Text of the most recently closed removed run, staged so the next added + * run (if any, within the same hunk) can be paired with it into one + * `DiffTextEdit`. Cleared at the start of every hunk (`startHunk`) so + * pairing never crosses a hunk boundary — an unpaired trailing deletion in + * one hunk must never be attributed to an unrelated insertion in the next. + */ + private pendingRemovedText: string[] = []; startHunk(oldStart: number, newStart: number): void { this.oldLineCursor = oldStart; this.newLineCursor = newStart; + this.pendingRemovedText = []; } /** @@ -84,11 +108,13 @@ class DiffLineTracker { file: string, oldRanges: Map, changedRanges: Map, + changedEdits: Map, ): void { if (line.startsWith('-')) { - this.flushAdded(file, changedRanges); + this.flushAdded(file, changedRanges, changedEdits); if (this.removedRunStart === null) this.removedRunStart = this.oldLineCursor; this.removedRunEnd = this.oldLineCursor; + this.currentRemovedText.push(line.slice(1)); this.oldLineCursor++; return; } @@ -96,33 +122,66 @@ class DiffLineTracker { this.flushRemoved(file, oldRanges); if (this.addedRunStart === null) this.addedRunStart = this.newLineCursor; this.addedRunEnd = this.newLineCursor; + this.currentAddedText.push(line.slice(1)); this.newLineCursor++; return; } - // A context line or a "\ No newline" marker ends both runs. + // A context line or a "\ No newline" marker ends both runs. Also clear + // any staged `pendingRemovedText`: `flushAdded` only pairs it with an + // added run that starts flushing right here, immediately adjacent (no + // intervening line) to the removal that staged it. A context line breaks + // that adjacency, so a later, unrelated added run must not be paired + // with a removal that sat on the other side of unchanged context. This + // only matters for diffs with non-zero context (`getGitDiff` always + // passes `--unified=0`, so it never fires in production) — but + // `parseDiffOutput` is a public export and must stay correct for any + // diff input. this.flushRemoved(file, oldRanges); - this.flushAdded(file, changedRanges); + this.flushAdded(file, changedRanges, changedEdits); + this.pendingRemovedText = []; if (line.startsWith(' ')) { this.oldLineCursor++; this.newLineCursor++; } } - /** Closes out the current removed-line run, if any, into `oldRanges`. */ + /** + * Closes out the current removed-line run, if any, into `oldRanges` and + * stages its text as `pendingRemovedText` for the next added run to + * (optionally) pair with. + */ flushRemoved(file: string, oldRanges: Map): void { if (this.removedRunStart !== null) { oldRanges.get(file)!.push({ start: this.removedRunStart, end: this.removedRunEnd! }); this.removedRunStart = null; this.removedRunEnd = null; + this.pendingRemovedText = this.currentRemovedText; + this.currentRemovedText = []; } } - /** Closes out the current added-line run, if any, into `changedRanges`. */ - flushAdded(file: string, changedRanges: Map): void { + /** + * Closes out the current added-line run, if any, into `changedRanges` and + * records the paired `DiffTextEdit` (added text + whatever removed text was + * staged immediately before it) into `changedEdits`. + */ + flushAdded( + file: string, + changedRanges: Map, + changedEdits: Map, + ): void { if (this.addedRunStart !== null) { - changedRanges.get(file)!.push({ start: this.addedRunStart, end: this.addedRunEnd! }); + const range = { start: this.addedRunStart, end: this.addedRunEnd! }; + changedRanges.get(file)!.push(range); + changedEdits.get(file)!.push({ + ...range, + addedText: this.currentAddedText, + removedText: this.pendingRemovedText, + }); this.addedRunStart = null; this.addedRunEnd = null; + this.currentAddedText = []; + this.pendingRemovedText = []; } } @@ -131,15 +190,17 @@ class DiffLineTracker { file: string, oldRanges: Map, changedRanges: Map, + changedEdits: Map, ): void { this.flushRemoved(file, oldRanges); - this.flushAdded(file, changedRanges); + this.flushAdded(file, changedRanges, changedEdits); } } export function parseDiffOutput(diffOutput: string): ParsedDiff { const changedRanges = new Map(); const oldRanges = new Map(); + const changedEdits = new Map(); const newFiles = new Set(); let currentFile: string | null = null; let prevIsDevNull = false; @@ -156,10 +217,11 @@ export function parseDiffOutput(diffOutput: string): ParsedDiff { } const newFile = extractNewFileName(line); if (newFile) { - if (currentFile) tracker.flush(currentFile, oldRanges, changedRanges); + if (currentFile) tracker.flush(currentFile, oldRanges, changedRanges, changedEdits); currentFile = newFile; if (!changedRanges.has(currentFile)) changedRanges.set(currentFile, []); if (!oldRanges.has(currentFile)) oldRanges.set(currentFile, []); + if (!changedEdits.has(currentFile)) changedEdits.set(currentFile, []); if (prevIsDevNull) newFiles.add(currentFile); prevIsDevNull = false; continue; @@ -172,7 +234,7 @@ export function parseDiffOutput(diffOutput: string): ParsedDiff { // clear the file context instead — the deleted file's hunk body that // follows has no corresponding DB entry to check against anyway (its // nodes are purged from the graph), so there is nothing to track here. - if (currentFile) tracker.flush(currentFile, oldRanges, changedRanges); + if (currentFile) tracker.flush(currentFile, oldRanges, changedRanges, changedEdits); currentFile = null; prevIsDevNull = false; continue; @@ -181,16 +243,16 @@ export function parseDiffOutput(diffOutput: string): ParsedDiff { const hunkMatch = line.match(HUNK_RE); if (hunkMatch) { - tracker.flush(currentFile, oldRanges, changedRanges); + tracker.flush(currentFile, oldRanges, changedRanges, changedEdits); tracker.startHunk(parseInt(hunkMatch[1]!, 10), parseInt(hunkMatch[3]!, 10)); continue; } - tracker.consume(line, currentFile, oldRanges, changedRanges); + tracker.consume(line, currentFile, oldRanges, changedRanges, changedEdits); } - if (currentFile) tracker.flush(currentFile, oldRanges, changedRanges); + if (currentFile) tracker.flush(currentFile, oldRanges, changedRanges, changedEdits); - return { changedRanges, oldRanges, newFiles }; + return { changedRanges, oldRanges, newFiles, changedEdits }; } // ─── Predicates ─────────────────────────────────────────────────────── @@ -223,6 +285,9 @@ interface BlastRadiusResult { maxFound: number; threshold: number; violations: BlastRadiusViolation[]; + /** Count of touched functions whose call graph shape didn't change — see issue #1740. */ + exemptedCount: number; + note?: string; } type DefRow = { @@ -245,15 +310,126 @@ function defEndLine(def: DefRow, nextDef: DefRow | undefined): number { return def.end_line || (nextDef ? nextDef.line - 1 : 999999); } +// ─── Call-graph shape detection (issue #1740) ────────────────────────── +// +// A function's absolute transitive-caller count is a property of the whole +// codebase's dependency structure, not of what a given diff changed inside +// that function. Gating on the raw count means any touch to a high-fan-in +// "spine" function (e.g. one reachable only through another near-universally +// called function) always fails, even fully behavior-preserving edits like +// replacing an inline literal with a named constant. The functions below +// let `checkMaxBlastRadius` tell "pre-existing fan-in" (already accepted by +// the codebase) apart from "risk introduced by this diff": only a def whose +// diff touches its own declaration line or changes the set of call targets +// referenced in its body counts its absolute caller count toward the gate. + +/** + * Matches a quoted string/template literal (double, single, or backtick + * delimited, with `\`-escaping honored). Stripped from a line before token + * extraction so that call-shaped text living inside string content (e.g. a + * log message mentioning `bar(x)`) doesn't masquerade as an actual call + * target — see `parenTokens`. + */ +const STRING_LITERAL_RE = /"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'|`(?:[^`\\]|\\.)*`/g; + +/** + * Matches an identifier immediately followed by `(`. Used only to compare + * the SET of such tokens between an edit's removed and added text — not to + * classify any single line as "a call" — which sidesteps having to tell a + * real call apart from `if (...)`/`while (...)`/a declaration's own `(...)` + * on syntax alone: if a token (call, keyword, or otherwise) appears on both + * sides, it nets out and isn't treated as a change; only a token gained or + * lost between the two sides counts. + * + * Known limitations, both deliberate, documented trade-offs for a + * mechanical, non-parsing check — see issue #1740: + * - Paren-less call syntax (e.g. Ruby's `foo x, y`, Lua's `foo "arg"`) is + * invisible to this heuristic, so a newly introduced paren-less call could + * be missed and its function wrongly exempted. + * - Only quoted string/template literals are stripped before matching, not + * comments — comment syntax varies too widely across the 34 supported + * languages to strip safely with a single regex. An `identifier(` pattern + * inside a comment can still affect the token-set comparison. + */ +const PAREN_TOKEN_RE = /([A-Za-z_$][A-Za-z0-9_$]*)\s*\(/g; + +function parenTokens(lines: string[]): Set { + const tokens = new Set(); + for (const line of lines) { + for (const m of line.replace(STRING_LITERAL_RE, '').matchAll(PAREN_TOKEN_RE)) { + tokens.add(m[1]!); + } + } + return tokens; +} + +function sameTokenSet(a: Set, b: Set): boolean { + if (a.size !== b.size) return false; + for (const t of a) if (!b.has(t)) return false; + return true; +} + +/** + * Returns the highest line number among `defId`'s own `parameter` children + * (or `defLine` if it has none — e.g. a zero-arg function, or a `class` def, + * whose parameters if any belong to its methods, not the class itself). + * Lets `callGraphShapeChanged` treat the declaration as spanning the whole + * parameter list, not just `def.line` — otherwise an edit to a parameter on + * line 2+ of a multi-line TypeScript signature is invisible to the + * declaration-line guard, and since a parameter list contains no + * `identifier(` patterns, `parenTokens` sees equal (empty) sets on both + * sides too, silently exempting a genuine signature change. See issue #1740. + */ +function signatureEndLine(db: BetterSqlite3Database, defId: number, defLine: number): number { + const row = db + .prepare( + `SELECT MAX(n.line) AS maxLine FROM nodes n + JOIN edges e ON e.source_id = n.id + WHERE e.kind = 'parameter_of' AND e.target_id = ?`, + ) + .get(defId) as { maxLine: number | null }; + return Math.max(defLine, row.maxLine ?? defLine); +} + +/** + * Returns true if the diff altered `def`'s call graph shape: an overlapping + * edit touches the declaration — anywhere from its own line through the end + * of its parameter list, per `sigEndLine` (signature/name risk — existing + * callers may need to change) — or changes the set of paren-preceded tokens + * referenced in its body (call-target risk — a callee was added, removed, or + * swapped). A range that overlaps the def but has no matching entry in + * `edits` (e.g. hand-built ranges in tests, or any future caller that only + * has range data) is conservatively treated as shape-changed — missing data + * must never silently exempt a def. + */ +function callGraphShapeChanged( + defLine: number, + sigEndLine: number, + endLine: number, + ranges: DiffRange[], + edits: DiffTextEdit[], +): boolean { + for (const range of ranges) { + if (range.start > endLine || range.end < defLine) continue; + if (range.start <= sigEndLine && range.end >= defLine) return true; + const edit = edits.find((e) => e.start === range.start && e.end === range.end); + if (!edit) return true; + if (!sameTokenSet(parenTokens(edit.removedText), parenTokens(edit.addedText))) return true; + } + return false; +} + export function checkMaxBlastRadius( db: BetterSqlite3Database, changedRanges: Map, + changedEdits: Map, threshold: number, noTests: boolean, maxDepth: number, ): BlastRadiusResult { const violations: BlastRadiusViolation[] = []; let maxFound = 0; + let exemptedCount = 0; const defsStmt = db.prepare( `SELECT * FROM nodes WHERE file = ? AND kind IN ('function', 'method', 'class') ORDER BY line`, ); @@ -261,12 +437,22 @@ export function checkMaxBlastRadius( for (const [file, ranges] of changedRanges) { if (noTests && isTestFile(file)) continue; const defs = defsStmt.all(file) as DefRow[]; + const edits = changedEdits.get(file) ?? []; for (let i = 0; i < defs.length; i++) { const def = defs[i]!; const endLine = defEndLine(def, defs[i + 1]); if (!rangesOverlap(def.line, endLine, ranges)) continue; + // The diff touched this def, but not its call graph shape — its + // absolute caller count is pre-existing risk, not risk this diff + // introduced. Skip the (potentially expensive) BFS entirely. + const sigEndLine = signatureEndLine(db, def.id, def.line); + if (!callGraphShapeChanged(def.line, sigEndLine, endLine, ranges, edits)) { + exemptedCount++; + continue; + } + const { totalDependents: totalCallers } = bfsTransitiveCallers(db, def.id, { noTests, maxDepth, @@ -285,7 +471,17 @@ export function checkMaxBlastRadius( } } - return { passed: violations.length === 0, maxFound, threshold, violations }; + const result: BlastRadiusResult = { + passed: violations.length === 0, + maxFound, + threshold, + violations, + exemptedCount, + }; + if (exemptedCount > 0) { + result.note = `${exemptedCount} touched function(s) exempted — no call graph shape change detected (pre-existing fan-in, not new risk introduced by this diff)`; + } + return result; } interface SignatureViolation { @@ -497,7 +693,14 @@ function runPredicates( if (flags.blastRadiusThreshold != null) { predicates.push({ name: 'blast-radius', - ...checkMaxBlastRadius(db, diff.changedRanges, flags.blastRadiusThreshold, noTests, maxDepth), + ...checkMaxBlastRadius( + db, + diff.changedRanges, + diff.changedEdits, + flags.blastRadiusThreshold, + noTests, + maxDepth, + ), }); } if (flags.enableSignatures) { diff --git a/tests/integration/check.test.ts b/tests/integration/check.test.ts index aa3546fa8..428074af1 100644 --- a/tests/integration/check.test.ts +++ b/tests/integration/check.test.ts @@ -69,6 +69,15 @@ beforeAll(() => { // tests/math.test.js: testAdd (line 1-5) insertNode(db, 'testAdd', 'function', 'tests/math.test.js', 1, 5); + // src/multiline.js: multiLineSig (line 20-24) — declaration spans lines + // 20-22 (opening line + two parameters, one per line), body on 23-24. + // Exercises the multi-line-signature guard (issue #1740 follow-up). + const multiLineSig = insertNode(db, 'multiLineSig', 'function', 'src/multiline.js', 20, 24); + const paramA = insertNode(db, 'a', 'parameter', 'src/multiline.js', 21); + const paramB = insertNode(db, 'b', 'parameter', 'src/multiline.js', 22); + insertEdge(db, paramA, multiLineSig, 'parameter_of'); + insertEdge(db, paramB, multiLineSig, 'parameter_of'); + // --- Call edges (for blast radius) --- // handleRequest -> add -> multiply (chain of 2) insertEdge(db, handleRequest, add, 'calls'); @@ -79,6 +88,9 @@ beforeAll(() => { insertEdge(db, parseInput, formatResult, 'calls'); // processNode -> handleRequest insertEdge(db, processNode, handleRequest, 'calls'); + // handleRequest -> multiLineSig (direct); processNode -> handleRequest + // (above) gives multiLineSig a transitive caller too. + insertEdge(db, handleRequest, multiLineSig, 'calls'); // --- Import edges (for cycles) --- // Create a file-level cycle: math.js <-> utils.js @@ -225,6 +237,100 @@ describe('parseDiffOutput', () => { expect(changedRanges.has('src/a.js')).toBe(true); expect(changedRanges.has('src/b.js')).toBe(true); }); + + // ─── changedEdits (issue #1740) ─────────────────────────────────────── + + test('changedEdits pairs a replacement run with its added/removed text', () => { + const diff = [ + '--- a/src/math.js', + '+++ b/src/math.js', + '@@ -3,1 +3,1 @@', + '- return a + b + 0;', + '+ return a + b + ZERO_OFFSET;', + ].join('\n'); + + const { changedRanges, changedEdits } = parseDiffOutput(diff); + expect(changedRanges.get('src/math.js')).toEqual([{ start: 3, end: 3 }]); + expect(changedEdits.get('src/math.js')).toEqual([ + { + start: 3, + end: 3, + addedText: [' return a + b + ZERO_OFFSET;'], + removedText: [' return a + b + 0;'], + }, + ]); + }); + + test('changedEdits records empty removedText for a pure insertion', () => { + const diff = ['--- a/src/math.js', '+++ b/src/math.js', '@@ -3,0 +4,1 @@', '+ // note'].join( + '\n', + ); + + const { changedEdits } = parseDiffOutput(diff); + expect(changedEdits.get('src/math.js')).toEqual([ + { start: 4, end: 4, addedText: [' // note'], removedText: [] }, + ]); + }); + + test('changedEdits does not pair a pure deletion with an unrelated later hunk', () => { + // First hunk is a pure deletion (no added lines) so it never becomes a + // changedEdits entry; the second hunk's pure insertion must NOT be + // paired with the first hunk's removed text (they are unrelated edits). + const diff = [ + '--- a/src/math.js', + '+++ b/src/math.js', + '@@ -3,1 +3,0 @@', + '- // stale comment', + '@@ -10,0 +9,1 @@', + '+ // fresh comment', + ].join('\n'); + + const { changedEdits } = parseDiffOutput(diff); + expect(changedEdits.get('src/math.js')).toEqual([ + { start: 9, end: 9, addedText: [' // fresh comment'], removedText: [] }, + ]); + }); + + test('changedEdits pairs multi-line replacement blocks as a single edit', () => { + const diff = [ + '--- a/src/math.js', + '+++ b/src/math.js', + '@@ -1,2 +1,2 @@', + '-line A', + '-line A2', + '+line B', + '+line B2', + ].join('\n'); + + const { changedEdits } = parseDiffOutput(diff); + expect(changedEdits.get('src/math.js')).toEqual([ + { start: 1, end: 2, addedText: ['line B', 'line B2'], removedText: ['line A', 'line A2'] }, + ]); + }); + + test('changedEdits does not pair a removal with an unrelated addition separated by a context line', () => { + // Within a single hunk (non-zero context, e.g. a plain `git diff` + // without `--unified=0`), a context line between a removal and a later, + // unrelated addition must break the pairing — otherwise the removal's + // text gets wrongly attributed as "replaced by" the addition purely + // because it was the most recently closed removed run. `getGitDiff` + // always uses `--unified=0` so this never happens in production, but + // `parseDiffOutput` is a public export and must stay correct for any + // unified diff. + const diff = [ + '--- a/src/math.js', + '+++ b/src/math.js', + '@@ -1,2 +1,2 @@', + '-old line 1', + ' unchanged context', + '+new line 3', + ].join('\n'); + + const { changedEdits } = parseDiffOutput(diff); + expect(changedEdits.get('src/math.js')).toEqual([ + { start: 2, end: 2, addedText: ['new line 3'], removedText: [] }, + ]); + }); }); // ─── checkNoNewCycles ───────────────────────────────────────────────── @@ -253,14 +359,17 @@ describe('checkMaxBlastRadius', () => { // multiply has callers: add(d1), handleRequest+formatResult(d2), processNode+parseInput(d3) = 5 // With depth 1 only, multiply has 1 caller (add), so threshold 3 passes const ranges = new Map([['src/math.js', [{ start: 7, end: 12 }]]]); - const result = checkMaxBlastRadius(db, ranges, 3, false, 1); + const result = checkMaxBlastRadius(db, ranges, new Map(), 3, false, 1); expect(result.passed).toBe(true); }); test('fails when max callers exceeds threshold', () => { // add has callers: handleRequest, formatResult at depth 1; parseInput, processNode at depth 2 + // No changedEdits data is provided (empty map) — a range with no matching + // edit entry is conservatively treated as a call-graph-shape change, so + // this predates and is unaffected by the issue #1740 exemption. const ranges = new Map([['src/math.js', [{ start: 1, end: 5 }]]]); - const result = checkMaxBlastRadius(db, ranges, 1, false, 3); + const result = checkMaxBlastRadius(db, ranges, new Map(), 1, false, 3); expect(result.passed).toBe(false); expect(result.violations.length).toBe(1); expect(result.violations[0].name).toBe('add'); @@ -270,10 +379,160 @@ describe('checkMaxBlastRadius', () => { test('respects maxDepth', () => { // With depth 1, add has 2 direct callers (handleRequest, formatResult) const ranges = new Map([['src/math.js', [{ start: 1, end: 5 }]]]); - const result = checkMaxBlastRadius(db, ranges, 10, false, 1); + const result = checkMaxBlastRadius(db, ranges, new Map(), 10, false, 1); expect(result.passed).toBe(true); expect(result.maxFound).toBeLessThanOrEqual(10); }); + + // ─── Call-graph shape exemption (issue #1740) ──────────────────────── + // + // `add` (src/math.js, lines 1-5) has several transitive callers in the + // fixture graph (handleRequest, formatResult direct; parseInput, + // processNode transitive) — a real "high fan-in spine function" shape. + // These tests drive `checkMaxBlastRadius` through `parseDiffOutput` (not + // hand-built ranges) so `changedEdits` is populated for real, proving the + // exemption logic itself rather than just the safe-fallback path above. + + test('passes for a high-fan-in function when only an internal literal changes (no call graph shape change)', () => { + // Body-only edit on line 3 (inside add's 1-5 span), declaration + // untouched, and neither side of the edit contains any call syntax. + const diff = [ + '--- a/src/math.js', + '+++ b/src/math.js', + '@@ -3,1 +3,1 @@', + '- return a + b + 0;', + '+ return a + b + ZERO_OFFSET;', + ].join('\n'); + const { changedRanges, changedEdits } = parseDiffOutput(diff); + + // Threshold 1 would normally fail on add's multiple callers (as proven + // by the "fails when max callers exceeds threshold" test above using + // the same threshold) — but this diff never changed add's call graph + // shape, so its pre-existing fan-in should not fail the gate. + const result = checkMaxBlastRadius(db, changedRanges, changedEdits, 1, false, 3); + expect(result.passed).toBe(true); + expect(result.violations).toEqual([]); + expect(result.exemptedCount).toBe(1); + expect(result.note).toMatch(/exempted/); + }); + + test('still fails for a high-fan-in function when the diff adds a new call', () => { + // Same line, but the replacement introduces a brand new call target + // (`validate`) that wasn't referenced before — a genuine call-graph + // shape change, so the pre-existing fan-in must still gate. + const diff = [ + '--- a/src/math.js', + '+++ b/src/math.js', + '@@ -3,1 +3,1 @@', + '- return a + b;', + '+ return validate(a) + b;', + ].join('\n'); + const { changedRanges, changedEdits } = parseDiffOutput(diff); + + const result = checkMaxBlastRadius(db, changedRanges, changedEdits, 1, false, 3); + expect(result.passed).toBe(false); + expect(result.violations.length).toBe(1); + expect(result.violations[0].name).toBe('add'); + expect(result.exemptedCount).toBe(0); + }); + + test('still fails for a high-fan-in function when the diff changes its own declaration line', () => { + // add's declaration is at line 1. Even though neither side references a + // new call target, touching the declaration line itself is a signature + // risk and must not be exempted. + const diff = [ + '--- a/src/math.js', + '+++ b/src/math.js', + '@@ -1,1 +1,1 @@', + '-function add(a, b) {', + '+function add(a, b, c) {', + ].join('\n'); + const { changedRanges, changedEdits } = parseDiffOutput(diff); + + const result = checkMaxBlastRadius(db, changedRanges, changedEdits, 1, false, 3); + expect(result.passed).toBe(false); + expect(result.violations.length).toBe(1); + expect(result.violations[0].name).toBe('add'); + }); + + test('still fails when a call target is swapped even though a paren token is reused', () => { + // Both sides reference identifiers followed by `(`, but the SETS differ + // (formatResult replaced by parseInput) — a real callee swap, not a + // no-op, so it must not be exempted. + const diff = [ + '--- a/src/math.js', + '+++ b/src/math.js', + '@@ -3,1 +3,1 @@', + '- return formatResult(a) + b;', + '+ return parseInput(a) + b;', + ].join('\n'); + const { changedRanges, changedEdits } = parseDiffOutput(diff); + + const result = checkMaxBlastRadius(db, changedRanges, changedEdits, 1, false, 3); + expect(result.passed).toBe(false); + expect(result.exemptedCount).toBe(0); + }); + + test('still fails for a high-fan-in function when a parameter is added on line 2+ of a multi-line signature', () => { + // multiLineSig's declaration spans lines 20 (opening) through 22 (its + // last parameter, `b`, on its own line) — only line 20 is `def.line`. + // Editing line 22 to add a parameter must still be read as a + // declaration/signature change even though it never touches line 20 + // itself, and a parameter list has no `identifier(` tokens for the + // paren-token comparison to catch either. + const diff = [ + '--- a/src/multiline.js', + '+++ b/src/multiline.js', + '@@ -22,1 +22,1 @@', + '- b,', + '+ b, c,', + ].join('\n'); + const { changedRanges, changedEdits } = parseDiffOutput(diff); + + const result = checkMaxBlastRadius(db, changedRanges, changedEdits, 1, false, 3); + expect(result.passed).toBe(false); + expect(result.violations.length).toBe(1); + expect(result.violations[0].name).toBe('multiLineSig'); + expect(result.exemptedCount).toBe(0); + }); + + test('passes for a high-fan-in function when a multi-line signature is untouched and only the body changes', () => { + // Sanity check for the fix above: an edit strictly inside the body + // (line 23, past the last parameter on line 22) with no new call target + // must still be exempted — the signature-end-line widening must not + // over-exempt the whole function span. + const diff = [ + '--- a/src/multiline.js', + '+++ b/src/multiline.js', + '@@ -23,1 +23,1 @@', + '- return a + b + 0;', + '+ return a + b + ZERO_OFFSET;', + ].join('\n'); + const { changedRanges, changedEdits } = parseDiffOutput(diff); + + const result = checkMaxBlastRadius(db, changedRanges, changedEdits, 1, false, 3); + expect(result.passed).toBe(true); + expect(result.exemptedCount).toBe(1); + }); + + test('still fails when a real call is replaced by a string literal merely mentioning the same identifier', () => { + // The removed line makes a genuine call to `bar`; the added line only + // *mentions* `bar(` inside a string literal. Without stripping string + // content first, the naive regex would read both sides as referencing + // the token "bar" and wrongly treat this as a no-op. + const diff = [ + '--- a/src/math.js', + '+++ b/src/math.js', + '@@ -3,1 +3,1 @@', + '- return bar() + b;', + "+ return 'invoke bar(x)' + b;", + ].join('\n'); + const { changedRanges, changedEdits } = parseDiffOutput(diff); + + const result = checkMaxBlastRadius(db, changedRanges, changedEdits, 1, false, 3); + expect(result.passed).toBe(false); + expect(result.exemptedCount).toBe(0); + }); }); // ─── checkNoSignatureChanges ──────────────────────────────────────────