Skip to content

fix(native): remove spurious findCaller fallback that dropped 68 call edges#637

Merged
carlos-alm merged 3 commits intomainfrom
fix/issue-613-native-call-edges
Mar 26, 2026
Merged

fix(native): remove spurious findCaller fallback that dropped 68 call edges#637
carlos-alm merged 3 commits intomainfrom
fix/issue-613-native-call-edges

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

  • Removes a buggy else if branch in Rust build_call_edges (crates/codegraph-core/src/edge_builder.rs) that assigned calls to the nearest preceding definition even when the call was outside that definition's span
  • This caused ~68 call edges to be silently dropped: caller resolved to same node as target → self-call filter removed the edge → edge lost
  • The JS findCaller (src/domain/graph/builder/stages/build-edges.ts:345) never had this fallback — it correctly falls back to the file node when no definition contains the call

Root cause

// BUG: assigns caller to nearest def BEFORE the call line,
// even when call is outside that def's body
} else if def.line <= call.line && caller_id == file_node_id {
    caller_id = def.node_id;  // wrong — should stay as file_node_id
}

Example: parseJS defined at L18-22, called at L25 (outside body). Rust incorrectly set caller=parseJS, target=parseJS → self-call → edge dropped. JS correctly set caller=file_node → file → parseJS edge preserved.

Test plan

  • All 560 integration tests pass
  • Rebuild codegraph with native engine, verify call edge count matches WASM (~4118 vs prior ~4050)
  • Verify no regression in caller attribution for test files

Closes #613

…buted 68 call edges

The Rust `build_call_edges` had an `else if` branch that assigned calls to
the nearest preceding definition even when the call was outside that
definition's span. This caused ~68 call edges to be silently dropped
(caller == target → self-call filter) instead of correctly falling back to
the file node as caller. The JS `findCaller` never had this branch.

Closes #613
@claude
Copy link
Copy Markdown

claude bot commented Mar 26, 2026

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


PR Review Complete

Reviewing PR #637: fix(native): remove spurious findCaller fallback that dropped 68 call edges

Todo List:

  • Phase 1: Root cause analysis - understand the problem being solved
  • Phase 2: Configuration change detection - check for risky changes
  • Phase 3: Test integrity - verify tests aren't being weakened
  • Phase 4: Code quality review - examine the actual fix
  • Phase 5: Backlog compliance assessment - rate against BACKLOG.md criteria
  • Final Assessment: Provide comprehensive review and recommendation

Root Cause Analysis

Problem Being Solved: The Rust native engine had a spurious else if fallback in findCaller logic (crates/codegraph-core/src/edge_builder.rs:185-189) that incorrectly assigned call sites to the nearest preceding function definition, even when the call occurred outside that function's span. This caused ~68 call edges to be silently dropped because the caller and target resolved to the same node (self-call), which then got filtered out by the deduplication logic.

Why This Approach: The fix removes the buggy fallback entirely, aligning the Rust implementation with the JavaScript version in build-edges.ts:345-370. The JS implementation correctly only assigns calls to definitions when the call is within the definition's span (def.line <= call.line <= end), otherwise falling back to the file node. This is the correct behavior.

Risk Assessment: LOW RISK. This is a precise surgical fix that removes incorrect logic rather than adding complexity. The behavior change is to make native engine match WASM engine, improving consistency.

Configuration Change Detection

No risky configuration changes - This is purely a bug fix in Rust code logic.

Test Integrity

No test weakening - All 560 integration tests reportedly pass. No test expectations were lowered or assertions removed.

Code Quality Review

Examined the fix (crates/codegraph-core/src/edge_builder.rs:185-189):

// REMOVED (was buggy):
} else if def.line <= call.line && caller_id == file_node_id {
    if let Some(id) = def.node_id {
        caller_id = id;  // WRONG: assigns to nearest def even if call is outside span
    }
}

Analysis:

  • ✅ The logic was indeed incorrect - it only checked def.line <= call.line without verifying call.line <= def.end_line
  • ✅ This caused calls outside function definitions (e.g., at module scope) to be incorrectly attributed to the last function
  • ✅ Example from PR description: parseJS defined at L18-22, called at L25. Rust incorrectly set caller=parseJS, target=parseJS → self-call → edge dropped
  • ✅ The fix makes native behavior match WASM behavior exactly
  • ✅ Simple removal with no replacement needed - the correct fallback (file node) was already in place

Backlog Compliance

  • Zero-dep: ✅ - No new dependencies added
  • Foundation-aligned: ✅ - Improves accuracy of core parsing pipeline, supports P1 (always-current)
  • Problem-fit: 5/5 - Directly addresses core problem of AI agents getting incorrect dependency graphs, preventing hallucinated dependencies
  • Breaking: No - This is a bug fix that corrects behavior, not a breaking change
  • Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Critical Concerns

None identified. This is a textbook example of a good bug fix:

Final Assessment

## Root Cause Analysis
**Problem Being Solved**: Native Rust engine had spurious findCaller fallback that incorrectly attributed calls to nearest preceding function even when call was outside function body, causing ~68 self-call edges to be filtered out
**Why This Approach**: Surgical removal of buggy logic to align native with WASM behavior - the correct fallback (file node) was already present
**Risk Assessment**: LOW - precise fix removes incorrect logic, improves native/WASM consistency

## Backlog Compliance  
- Zero-dep: ✓ — no new dependencies
- Foundation-aligned: ✓ — improves core parsing accuracy
- Problem-fit: 5 — directly prevents AI agents from getting wrong dependency graphs
- Breaking: No — bug fix, not behavioral change
- Tier: 1

## Critical Concerns
None. This is an exemplary bug fix: small, focused, removes incorrect logic, aligns dual-engine behavior.

## Final Recommendation
- Rating: ⭐⭐⭐⭐⭐ (5/5)
- Action: **APPROVE** 
- Reasoning: Perfect bug fix - addresses documented native/WASM parity issue, removes spurious logic causing edge loss, aligns with JS implementation, all tests pass. This directly improves the accuracy of dependency graphs that AI agents rely on.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 26, 2026

Greptile Summary

This PR removes a 4-line else if branch in the Rust build_call_edges function (crates/codegraph-core/src/edge_builder.rs) that incorrectly assigned call sites to the nearest preceding function definition even when the call was outside that definition's body span. The root cause was that caller_id was set to a preceding def's node ID instead of remaining as file_node_id, causing the caller and target to resolve to the same node and subsequently be filtered out by the self-call dedup check — silently dropping ~68 call edges from the native engine's output.

Key points:

  • The fix is minimal and correct: removing the bad branch restores parity with the JS findCaller logic, which has always fallen back to the file node for out-of-span calls
  • The narrowest-enclosing-span logic (the if branch above) is unaffected — calls genuinely inside a definition still resolve correctly
  • 560 integration tests pass; two manual verification steps in the test plan (native edge count ~4118, no caller-attribution regression) are not yet checked off but represent post-merge validation rather than blockers

Confidence Score: 5/5

Safe to merge — minimal targeted removal of a confirmed buggy branch with no side effects on surrounding logic.

The change is a 4-line deletion of a well-understood bug. The narrowest-enclosing-span logic above is untouched, the fix matches the established JS counterpart behavior, and all 560 integration tests pass. No new logic is introduced, so the regression surface is essentially zero.

No files require special attention.

Important Files Changed

Filename Overview
crates/codegraph-core/src/edge_builder.rs Removes 4-line buggy else if branch that incorrectly assigned caller to nearest preceding definition when the call site was outside that definition's body span, causing ~68 edges to be silently dropped by the self-call dedup filter.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[For each call site] --> B{def.line ≤ call.line\nAND call.line ≤ def.end_line?}
    B -->|Yes — call is inside def span| C{span < caller_span?}
    C -->|Yes| D[caller_id = def.node_id\ncaller_span = span]
    C -->|No| E[Keep current caller_id]
    B -->|No — call is outside def span| F_OLD["❌ REMOVED else-if:\nif def.line ≤ call.line\nAND caller_id == file_node_id"]
    F_OLD -->|Matched| G_OLD["❌ caller_id = def.node_id\n(WRONG: call is outside body!)"]
    G_OLD --> H_OLD["caller == target → self-call\nfilter drops the edge 💥"]
    B -->|No — call is outside def span| F_NEW["✅ AFTER FIX:\nno else-if branch"]
    F_NEW --> I_NEW["caller_id stays = file_node_id\n(correct)"]
    I_NEW --> J_NEW["file → target edge emitted ✅"]

    style F_OLD fill:#ffcccc,stroke:#cc0000
    style G_OLD fill:#ffcccc,stroke:#cc0000
    style H_OLD fill:#ffcccc,stroke:#cc0000
    style F_NEW fill:#ccffcc,stroke:#006600
    style I_NEW fill:#ccffcc,stroke:#006600
    style J_NEW fill:#ccffcc,stroke:#006600
Loading

Reviews (2): Last reviewed commit: "Merge branch 'main' into fix/issue-613-n..." | Re-trigger Greptile

@carlos-alm
Copy link
Copy Markdown
Contributor Author

@greptileai

@carlos-alm carlos-alm merged commit 6724c46 into main Mar 26, 2026
18 checks passed
@carlos-alm carlos-alm deleted the fix/issue-613-native-call-edges branch March 26, 2026 22:55
@github-actions github-actions bot locked and limited conversation to collaborators Mar 26, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(native): native engine under-extracts ~68 call edges vs WASM

1 participant