fix: scope-aware caller selection for nested functions

[carlos-alm]

Summary

• Fixes false-positive [dead] classification for nested/closure functions (e.g. nodeId inside exportMermaid)
• Root cause: caller selection loop picked the last definition where line <= call.line, creating self-call edges that got filtered out → fan_in = 0 → [dead]
• Fix: uses endLine to find the innermost enclosing scope, so calls correctly attribute the outer function as caller
• All 3 affected nodeId instances in branch-compare.js, export.js, and queries.js now correctly show as [core]

Test plan

• New integration test verifies outer→inner edge (not inner→inner self-call)
• All 636 tests pass
• Lint clean
• node src/cli.js where nodeId -T shows [core] for all 3 instances
• node src/cli.js fn-impact nodeId -T shows correct caller chains

[claude]

Claude finished @carlos-alm's task —— View job

---

Reviewing this PR for scope-aware caller selection fix

Review Todo List

• Phase 1: Root Cause Analysis - Understand the problem and approach
• Phase 2: Configuration Change Detection - Check for suspicious config changes
• Phase 3: Test Integrity - Verify test changes are proper
• Phase 4: Code Quality Review - Examine the fix implementation
• Provide final rating and recommendation

Phase 1: Root Cause Analysis ✅

Problem Being Solved: The PR fixes false-positive [dead] classification for nested/closure functions. Previously, a function like nodeId inside exportMermaid would be incorrectly marked as [dead] because the caller selection logic created self-call edges that got filtered out, leading to fan_in = 0.

Root Cause Identified: The caller selection loop used line <= call.line to pick the last definition that starts before the call. For nested functions, this would pick the inner function itself as the caller of its own calls, creating inner→inner self-call edges that get filtered out.

Fix Approach: The solution uses endLine to find the innermost enclosing scope. It now checks if the call is within the definition's range (def.line <= call.line <= def.endLine) and picks the narrowest enclosing scope, ensuring calls correctly attribute to the outer function as the caller.

This is a legitimate bug fix addressing a real structural analysis issue, not hiding symptoms.

Phase 2: Configuration Change Detection ✅

No High-Risk Changes Detected:

• No changes to biome/linting rules
• No test timeouts or coverage threshold modifications
• No build configuration changes
• One minor docs change: removal of "viral" from a LinkedIn reference (neutral cleanup)

The change is focused entirely on the core parsing logic with proper test coverage.

Phase 3: Test Integrity ✅

Excellent Test Coverage: The new integration test at lines 361-404 specifically validates the fix:

test('enclosing function is the caller of a nested function, not a self-call', () => {
  // Creates outer() { function inner() { return 42; } return inner(); }
  // Verifies outer->inner edge exists (correct)
  // Verifies inner->inner edge does NOT exist (the bug this fixes)
});

Test Quality:

• ✅ Creates isolated test fixture with nested function scenario
• ✅ Tests positive case: outer->inner edge should exist
• ✅ Tests negative case: inner->inner self-call should NOT exist
• ✅ Uses proper test isolation with beforeAll/afterAll cleanup
• ✅ No weakened assertions or test relaxation

Phase 4: Code Quality Review ✅

Implementation Quality (src/builder.js:788-811):

let caller = null;
let callerSpan = Infinity;  // NEW: Track narrowest enclosing scope
for (const def of symbols.definitions) {
  if (def.line <= call.line) {
    const end = def.endLine || Infinity;
    if (call.line <= end) {  // NEW: Call is within definition range
      const span = end - def.line;
      if (span < callerSpan) {  // NEW: Pick narrowest scope
        // ... assign caller
        callerSpan = span;
      }
    } else if (!caller) {  // NEW: Fallback only if no enclosing scope found
      // ... fallback logic
    }
  }
}

Strengths:

• ✅ Precise scope detection: Uses endLine to determine if call is truly within a function's scope
• ✅ Innermost scope selection: callerSpan ensures nested functions don't get overshadowed by outer ones
• ✅ Robust fallback: Maintains existing behavior for edge cases (definitions without endLine)
• ✅ No breaking changes: Preserves existing logic paths, just makes them more precise
• ✅ Clear logic flow: Code is readable and comments explain the intent

Architecture: The fix targets the exact root cause (caller selection) without side effects on other parsing logic.

Edge Cases Handled:

• Functions without endLine (graceful degradation with || Infinity)
• Multiple nested levels (innermost wins via callerSpan comparison)
• Calls outside any function scope (fallback to file node)

Final Assessment

## Root Cause Analysis
**Problem Being Solved**: False-positive [dead] classification for nested functions due to incorrect self-call edge creation
**Why This Approach**: Uses endLine for precise scope detection instead of simple line ordering - directly targets the root cause
**Risk Assessment**: Low risk - focused fix with excellent test coverage and no configuration changes

## Critical Concerns
None identified. This is a well-implemented bug fix.

## Final Recommendation
- Rating: ⭐⭐⭐⭐☆ (4/5)
- Action: APPROVE
- Reasoning: Excellent root cause analysis, precise implementation, comprehensive test coverage. The fix directly addresses the structural analysis bug without side effects. Only minor deduction for not including specific examples of the 3 affected nodeId instances mentioned in the PR description being verified in tests.

[greptile-apps]

Greptile Summary

This PR fixes a bug where nested/closure functions were incorrectly classified as [dead] due to false self-call edges. The root cause was that the caller selection algorithm picked the last definition where line <= call.line, which for nested functions meant the inner function itself. The fix uses endLine to identify the innermost enclosing scope by selecting the definition with the smallest span that contains the call line.

• Enhanced caller selection in builder.js to check if call is within definition's range (call.line <= endLine) and pick the narrowest enclosing scope
• Added fallback logic for definitions where call is outside the range but no enclosing scope found yet
• New integration test verifies outer→inner edge exists and inner→inner self-call edge does not exist
• Minor doc update removes "viral" from LinkedIn post reference

Confidence Score: 5/5

• This PR is safe to merge - fixes a specific bug with targeted logic change and good test coverage
• The fix is well-designed and addresses the root cause of the bug by using scope span to identify the correct caller. The algorithm correctly handles nested scopes by picking the narrowest enclosing definition. Test coverage directly validates the bug fix (outer→inner edge exists, inner→inner does not). The fallback logic for definitions without endLine or calls outside all scopes is reasonable. No breaking changes or risky refactoring.
• No files require special attention

Important Files Changed

Filename	Overview
src/builder.js	Enhanced caller selection to use endLine for scope-aware attribution, fixing false-positive [dead] classification for nested functions
tests/integration/build.test.js	Added test case verifying nested function caller attribution (outer→inner, not inner→inner)

_{Last reviewed commit: 0cb6324}

[greptile-apps]

_{3 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}