Skip to content

fix(resolver): apply confidence filter to static receiver fallback#1427

Merged
carlos-alm merged 5 commits into
mainfrom
fix/static-receiver-confidence-1398
Jun 10, 2026
Merged

fix(resolver): apply confidence filter to static receiver fallback#1427
carlos-alm merged 5 commits into
mainfrom
fix/static-receiver-confidence-1398

Conversation

@carlos-alm

@carlos-alm carlos-alm commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The typed-method lookup in resolveByMethodOrGlobal (if (typeName) branch, call-resolver.ts) was the only byName call in the function that did not apply computeConfidence(relPath, n.file, null) >= 0.5, leaving it open to false-positive edges from distant files in polyglot projects where two unrelated classes share a method name
  • Added the same computeConfidence(relPath, n.file, null) >= 0.5 guard that every other byName call in the function already applies; all resolution paths in resolveByMethodOrGlobal are now consistently guarded
  • Added unit tests covering the two confidence cases: same-directory target (0.7, passes) and distant target (0.3, filtered)

Closes #1398

Test plan

  • npx vitest run tests/unit/call-resolver.test.ts — 12 tests pass
  • npx vitest run tests/integration/issue-1372-csharp-static-receiver.test.ts — 2 tests pass (same-dir cross-file static calls unaffected, confidence 0.7)
  • npx vitest run tests/benchmarks/resolution/resolution-benchmark.test.ts — benchmark suite passes (C# benchmark all 3 static edges preserved)

The direct qualified-name fallback in resolveByMethodOrGlobal was the only
resolution path that did not apply computeConfidence >= 0.5, risking false
edges from distant files in polyglot projects with name collisions.

Closes #1398
@claude

claude Bot commented Jun 9, 2026

Copy link
Copy Markdown

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


PR Review: fix(resolver): apply confidence filter to static receiver fallback

  • Gather context and read changed files
  • Analyze root cause and approach
  • Check configuration/test integrity
  • Assess code quality
  • Backlog compliance assessment
  • Final recommendation

Root Cause Analysis

Problem Being Solved: The !typeName branch of resolveByMethodOrGlobal (call-resolver.ts:132–142) — the "direct qualified method lookup" fallback for static receiver calls like Validators.IsValidEmail() — was the only byName call in the function that did not apply computeConfidence(relPath, n.file, null) >= 0.5. In a polyglot codebase with multiple unrelated classes sharing a method name, this path could produce false-positive call edges to semantically unrelated definitions in distant files (e.g., a Validators.IsValidEmail in a Go service crate resolving against one in a C# frontend module).

Why This Approach: The fix adds the same confidence guard that already exists at every other call site in the same function (lines 123, 155, 188, 196, 215). This is the correct repair — it restores internal consistency rather than introducing new heuristics. The threshold >= 0.5 is the established policy: same dir → 0.7 (passes), sibling-dir/grandparent → 0.5 (passes), distant subtrees → 0.3 (filtered). Valid same-project static calls are preserved; distant false-positive edges are dropped.

Risk Assessment: Low. The change is additive (adds a filter, never changes resolution order) and is guarded only in the !typeName branch (i.e., when there is no type-map entry for the receiver). The integration test for issue #1372 (fixture with Validators.cs and Program.cs in the same directory) correctly validates that legitimate adjacent-file static calls survive — confidence 0.7.


Phase 2: Configuration Change Detection

No linting rules relaxed, no timeouts raised, no coverage thresholds changed, no TypeScript flags disabled. Clean.


Phase 3: Test Integrity

Tests are strictly additive (30 new lines, 0 deletions from existing test logic). The two new test cases in tests/unit/call-resolver.test.ts:96–119:

  • Same-dir positive case (app/Program.csapp/Validators.cs): dirname equality → confidence 0.7 → passes ✓
  • Distant negative case (app/main/Program.cslib/util/Validators.cs): different subtrees → confidence 0.3 → filtered ✓

Both cases are mathematically grounded against computeConfidenceJS (resolve.ts:464–479). The test mock (makeLookup) is minimal and correct — no over-specification.


Phase 4: Code Quality

The code change is exactly one concept applied consistently:

-        .filter((n) => n.kind === 'method' || n.kind === 'function');
+        .filter(
+          (n) =>
+            (n.kind === 'method' || n.kind === 'function') &&
+            computeConfidence(relPath, n.file, null) >= 0.5,
+        );

No concerns. The guard is correctly placed inside the if (!typeName) block, does not affect the typeName-based resolution paths above it, and passes null for importedFrom (correct since there's no import statement backing this fallback).

One minor observation: computeConfidence dispatches to the native addon when available (resolve.ts:520–536). In the unit tests, makeLookup uses file paths like 'app/Validators.cs' (relative strings). The JS fallback (computeConfidenceJS) uses path.dirname on these relative strings, which works correctly on Linux/macOS. This is fine since all existing unit tests in this file use the same pattern.


Backlog Compliance

Axis Result
Zero-dep ✓ — no new dependencies entries
Foundation-aligned ✓ — pure local graph logic, no cloud, no agent behavior
Problem-fit 5 — directly prevents hallucinated dependency edges, which is the core problem the tool solves
Breaking No — this filter can only remove previously incorrect (false-positive) edges; valid edges at confidence ≥ 0.5 are preserved
Tier 1 — zero-dep and foundation-aligned

Final Recommendation

## Root Cause Analysis
Problem Being Solved: False-positive call edges in polyglot codebases where the
  !typeName static-receiver fallback had no proximity filter, unlike every other
  byName resolution path in the same function.
Why This Approach: Applied the same computeConfidence >= 0.5 guard already present
  at all five other byName call sites — restores internal consistency, no new policy.
Risk Assessment: Low. Same-project static calls (confidence ≥ 0.5) are unaffected;
  only cross-subsystem false edges (confidence 0.3) are dropped.

## Backlog Compliance
- Zero-dep: ✓
- Foundation-aligned: ✓
- Problem-fit: 5 — prevents hallucinated dependency edges
- Breaking: No
- Tier: 1

## Final Recommendation
- Rating: ⭐⭐⭐⭐ (4/5)
- Action: APPROVE
- Reasoning: Minimal, correct, well-tested fix that closes a genuine consistency
  gap. The implementation is the obvious right move (one filter predicate, no new
  policy). Tests validate both the positive and negative confidence cases with
  realistic file-path scenarios. Integration regression test (#1372) confirms
  legitimate adjacent-file static calls still resolve. No concerns.

@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR closes a gap in resolveByMethodOrGlobal where the typeName branch (lookup.byName(\${typeName}.${call.name}`)) was the only byNamecall in the function that did not apply thecomputeConfidence >= 0.5` guard, leaving it open to false-positive call edges in polyglot projects where two unrelated classes share a method name.

  • call-resolver.ts: Adds computeConfidence(relPath, n.file, null) >= 0.5 to the typed filter inside the if (typeName) block, making all resolution paths in resolveByMethodOrGlobal consistently guarded.
  • call-resolver.test.ts: Adds two new unit tests for the typeName branch — one confirming a same-directory target (confidence 0.7) is preserved, and one confirming a distant target (confidence 0.3) is filtered out.

Confidence Score: 5/5

Safe to merge — the change is a one-line guard addition that brings the typeName typed-method path in line with every other resolution path in the function, validated by two targeted unit tests.

The fix is minimal and surgical: a single filter predicate added to the one byName call that lacked it. All other resolution paths in resolveByMethodOrGlobal already applied the same guard, so the change is consistent with established patterns. The new tests confirm both the pass (same-directory) and filter (distant-file) cases, and the existing test suite exercises the surrounding logic. No regressions are expected.

No files require special attention.

Important Files Changed

Filename Overview
src/domain/graph/builder/call-resolver.ts Adds the missing computeConfidence >= 0.5 guard to the typeName typed-method lookup; all resolution paths now apply the filter consistently.
tests/unit/call-resolver.test.ts Two new unit tests cover the typeName confidence filter: same-directory pass (0.7 >= 0.5) and distant-file filter (0.3 < 0.5). Tests integrate correctly with existing makeLookup helper and typeMap string entries.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[resolveByMethodOrGlobal] --> B{call.receiver?}
    B -- yes --> C[effectiveReceiver + typeMap lookup]
    C --> D{typeName resolved?}
    D -- yes --> E["byName - typeName.method\nfilter: kind=method AND confidence >= 0.5\nNEW in this PR"]
    E --> F{typed found?}
    F -- yes --> G[return typed]
    F -- no --> H["protoEntry alias\nfilter: confidence >= 0.5"]
    H --> I{typeName set?}
    I -- no --> J["direct qualified lookup\nfilter: kind=method/fn AND confidence >= 0.5"]
    J --> K{direct found?}
    K -- yes --> L[return direct]
    K -- no --> M["compositeEntry pts\nfilter: confidence >= 0.5"]
    I -- yes --> M
    D -- no --> I
    B -- "no / this / self / super" --> N["exact byName\nfilter: confidence >= 0.5"]
    N --> O{exact found?}
    O -- yes --> P[return exact]
    O -- no --> Q["same-class fallback\nfilter: kind=method AND confidence >= 0.5"]
    Q --> R[return result or empty]
    style E fill:#90EE90,stroke:#2d6a2d
Loading

Reviews (8): Last reviewed commit: "Merge branch 'main' into fix/static-rece..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Codegraph Impact Analysis

1 functions changed15 callers affected across 5 files

  • resolveByMethodOrGlobal in src/domain/graph/builder/call-resolver.ts:84 (15 transitive callers)

…ethodOrGlobal (#1427)

The typeName lookup (typeMap-resolved receiver) was the only remaining
byName call in resolveByMethodOrGlobal without a computeConfidence >= 0.5
guard. Apply the same filter to close the gap and add two unit tests
covering the same-dir (pass) and distant (filtered) cases.
@carlos-alm

Copy link
Copy Markdown
Contributor Author

Addressed Greptile's feedback: added computeConfidence(relPath, n.file, null) >= 0.5 to the typeName branch lookup at line 109 as well. The !typeName description in the PR summary was inaccurate — both the typeName and !typeName branches now have the confidence guard, matching every other byName call in the function. Two new unit tests cover the same-directory (0.7 → passes) and distant (0.3 → filtered) cases for the typeMap-resolved path.

@carlos-alm

Copy link
Copy Markdown
Contributor Author

@greptileai

@carlos-alm

Copy link
Copy Markdown
Contributor Author

Resolved merge conflict with main: tests/unit/call-resolver.test.ts had a conflict between this PR's typeName-branch confidence filter tests (#1398) and the bare-call JS/TS module-scope guard tests (#1407) added on main. Both test suites are preserved in the resolution. All 12 unit tests pass locally.

@carlos-alm

Copy link
Copy Markdown
Contributor Author

@greptileai

@carlos-alm

Copy link
Copy Markdown
Contributor Author

Fixed the PR description: corrected the branch name from !typeName to if (typeName) — the actual code change applies the confidence guard to the typed-method lookup (if (typeName) block, line 136), and the !typeName direct-qualified lookup already carried this guard before this PR. Also updated the test count from 6 to 12 to reflect the current state after merging with main.

@carlos-alm

Copy link
Copy Markdown
Contributor Author

@greptileai

@carlos-alm carlos-alm merged commit 326fb90 into main Jun 10, 2026
22 checks passed
@carlos-alm carlos-alm deleted the fix/static-receiver-confidence-1398 branch June 10, 2026 04:48
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 10, 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.

fix(resolver): static receiver fallback in resolveByMethodOrGlobal should apply confidence scoring

1 participant