fix(watch): align incremental call resolver with full build

[carlos-alm]

Summary

The watcher's rebuildFile reverse-dependency cascade had its own call
resolver (resolveCallTargets/buildCallEdges in incremental.ts) that had
drifted from the authoritative full-build resolver in
stages/build-edges.ts. On the codegraph repo, a comment-only watch
rebuild of a widely-imported file (src/shared/kinds.ts) inflated calls
edges by ~700 — the graph grew from 10178 distinct call edges to 10780
distinct + 97 duplicate rows, and build --no-incremental reset it back to
10178. This is the underlying cause of the net-edge-delta noise behind
#1220/#1245 and the regression-guard exemptions in #1248/#1255.

Three divergences are fixed so the cascade matches a clean full build exactly:

1. Gated global fallback — the unconditional findNodeByName(call.name)
   fan-out (every same-named symbol in the graph) is replaced by the
   full-build semantics: type-aware lookup, then a global match gated to
   receiver-less / this/self/super calls and filtered to
   computeConfidence >= 0.5.
2. Dedup — buildCallEdges now dedups within a file rebuild via a
   seenCallEdges set (insertEdge is a plain INSERT with no OR IGNORE).
3. Barrel resolution — the import-scoped branch now follows barrel
   re-exports (isBarrelFile/resolveBarrelTarget) so calls to symbols
   imported through an index/barrel file resolve to the defining file instead
   of being dropped.

After the fix, a comment-only watch rebuild leaves calls at exact parity
(10178 total / 10178 distinct, zero duplicates).

Found during

Dogfooding v3.11.1 — see #1259.

Verification

• New test tests/integration/issue-1259-watch-call-resolution.test.ts
  drives rebuildFile directly (the watch path) on a fixture that exercises
  all three failure modes. It fails on the pre-fix code (extra
  commonName fan-out edge + duplicate leafVal edge) and passes after.
  The existing parity tests use buildGraph's native-orchestrator incremental
  path and never exercised the buggy JS cascade — which is why they were green.
• Full suite: 2788 passed, 11 skipped, 0 failed (160 files, regression
  guard included).
• End-to-end on the codegraph repo: watch + comment-only edit of
  src/shared/kinds.ts now reports net-zero calls change; DB distinct/total
  calls stay at 10178.

Follow-up

The cascade still under-rebuilds receiver/extends/dynamic-imports edges
(and slightly over-counts imports) vs the full build — filed separately as
#1260. This PR is scoped to the calls-edge inflation (#1259).

Test plan

• npx vitest run tests/integration/issue-1259-watch-call-resolution.test.ts
• npx vitest run tests/integration/watcher-rebuild.test.ts tests/integration/incremental-edge-parity.test.ts
• Manual: build a repo, watch, append a comment to a high-fan-in file, confirm net-zero calls delta

[claude]

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

---

PR Review: fix(watch): align incremental call resolver with full build

• Gather context and read changed files
• Phase 1: Root cause analysis
• Phase 2: Configuration change detection
• Phase 3: Test integrity
• Phase 4: Code quality review
• Phase 5: Backlog compliance
• Final assessment

---

Phase 1: Root Cause Analysis ✅

The root cause is correctly identified and the fix is surgical: incremental.ts maintained a divergent call resolver that had three concrete bugs versus the authoritative path in stages/build-edges.ts:

1. Ungated global fallback (resolveByMethodOrGlobal) — pre-fix code fanned out to every same-named symbol globally with no receiver guard and no confidence threshold. The full-build path gates this at confidence >= 0.5 and only applies it for receiver-less / this/self/super calls. The fix replicates these gates exactly (lines 499–509).
2. Missing dedup — insertEdge is a plain INSERT (no OR IGNORE), so duplicate calls from the same file rebuilt multiple times accrued duplicate rows. The fix adds a seenCallEdges Set, mirroring the full-build's identical pattern (build-edges.ts:497).
3. Barrel imports not followed — the import-scoped lookup looked up call.name in importedFrom (the barrel file itself), which has no definitions, so the lookup returned empty and the edge was dropped. The fix adds a resolveBarrelTarget fallback (lines 452–459).

The approach is correct: fix the divergence at the source rather than papering over symptoms.

---

Phase 2: Configuration Change Detection ✅

No linting rules relaxed, no timeouts increased, no coverage thresholds lowered, no build or TypeScript config touched. Clean pass.

---

Phase 3: Test Integrity ✅

The new test file (tests/integration/issue-1259-watch-call-resolution.test.ts) is a genuine regression test that:

• Drives rebuildFile directly — the exact code path that was buggy
• Constructs a fixture that exercises all three divergence modes independently
• Uses a full-build oracle (same fixture, clean buildGraph) as the ground truth
• All assertions are strict (toBe, toEqual, toContainEqual, toEqual([]))
• The test is documented to fail on the pre-fix code, which is the correct framing

No assertions were weakened. No existing tests were modified.

---

Phase 4: Code Quality

Positives:

• The lazy-cached prepared statement pattern (_barrelDb !== db guard) is consistent with the existing pattern in getRevDepStmts.
• resolveBarrelTarget guards against cycles via a visited Set.
• The two-pass cascade design (pass 1: direct edges including reexports; pass 2: barrel import edges) correctly handles the sequencing dependency where barrel resolution needs reexports edges to exist first.
• Comment quality is high — explains why each divergence existed, not just what changed.

Issues found:

Issue 1 — Minor divergence: buildImportedNamesMap doesn't handle dynamic imports (incremental.ts:385–404)

The full-build buildImportedNamesMap (build-edges.ts:514–541) processes dynamic imports first, then static imports, so that static bindings win when both contribute the same name. The incremental version iterates symbols.imports once with no priority split:

// incremental.ts (current)
for (const imp of symbols.imports) {
  const resolvedPath = resolveImportPath(...)
  for (const name of imp.names) {
    importedNames.set(name.replace(...), resolvedPath);
  }
}

vs. full-build:

// Two passes: dynamic first (lower priority), static second (wins)
for (const imp of symbols.imports) { if (!imp.dynamicImport) continue; ... }
for (const imp of symbols.imports) { if (imp.dynamicImport) continue; ... }

This is a pre-existing gap (not introduced by this PR), but this PR is explicitly aligning incremental with full-build and the PR description mentions "slightly over-counts imports" — dynamic import priority inversion could contribute to that. The PR notes this is filed as #1260, so OK to defer.

Issue 2 — resolveByMethodOrGlobal doesn't filter to kind === 'method' for typed receiver lookups (incremental.ts:494–497)

The full-build version at build-edges.ts:624:

const typed = (ctx.nodesByName.get(qualifiedName) || []).filter((n) => n.kind === 'method');

The incremental version at incremental.ts:494–497:

const typed = stmts.findNodeByName.all(qualified) as Array<{ id: number; file: string }>;

The SQL in findNodeByName (tests/integration/issue-1259-watch-call-resolution.test.ts:97–99) already restricts to kind IN ('function', 'method', ...), so method is included, but non-method kinds like function can also match a ClassName.methodName lookup where only method should match. This is a minor accuracy gap but could produce spurious calls edges for type-qualified names.

Issue 3 — Global-fallback targets are not sorted by confidence (incremental.ts:504–508)

The full-build path sorts targets by confidence descending when targets.length > 1 (build-edges.ts:597–603). The incremental path skips this sort. For calls edges the sort doesn't change which edges are inserted (all targets above 0.5 get edges), only their order in allEdgeRows. Since confidence is stored per-edge and not ordering-dependent, this has no correctness impact. Low severity.

Issue 4 — Module-level mutable statement caches (_barrelDb, _revDepDb) are shared across DB connections (incremental.ts:109–133, 213–246)

Lazily-cached prepared statements are keyed on db !== _prevDb. If two rebuildFile invocations run on different DB connections in the same process (e.g., test isolation), the cache correctly resets because the object reference changes. However, if the same DB object is reused across test cases (as in the test fixture), statements prepared against an already-closed DB would throw. The test does db.close() then checks results on a new connection (new Database(dbPath, { readonly: true })), so this doesn't bite the tests. But the module-level cache is a latent issue in long-running watch processes where DB reconnects happen. This pattern pre-existed this PR and is not introduced by it.

---

Phase 5: Backlog Compliance

• Zero-dep: ✓ — No new dependencies entries. All new code uses existing SQLite, tree-sitter, and resolve utilities.
• Foundation-aligned: ✓ — Purely local graph correction; no cloud, no codegen, no external toolchain. Fixes incorrect incremental state = improves "always-current" accuracy (P1).
• Problem-fit: 5 — Directly prevents hallucinated or inflated call edges from corrupting the context given to AI agents. A watch rebuild inflating calls by ~700 edges would cause agents to see false blast-radius for any touched symbol. This is exactly the "hallucinate dependencies" problem stated in the core mission.
• Breaking: No — purely internal correctness fix, no API/CLI/schema changes.
• Tier: 1

---

Final Assessment

Root Cause Analysis

Problem Being Solved: The watcher's incremental call resolver had three concrete divergences from the full-build resolver, causing edge inflation (~700 extra calls edges) on any reverse-dep cascade rebuild.

Why This Approach: The correct fix is semantic alignment at the source — replicate the same gates (receiver guard, confidence threshold, barrel follow-through, dedup set) rather than adding compensating workarounds. The PR does exactly this.

Risk Assessment: Low. The fix only changes what edges are inserted during rebuildFile cascades. The three changes are all restricting false-positive edges, not adding new ones. The parity test directly validates the fix against a full-build oracle. No schema, API, or CLI changes.

Backlog Compliance

• Zero-dep: ✓
• Foundation-aligned: ✓
• Problem-fit: 5 — directly prevents corrupted call-edge context given to AI agents
• Breaking: No
• Tier: 1

Minor Concerns (non-blocking)

1. buildImportedNamesMap dynamic-import priority (incremental.ts:385–404): pre-existing gap acknowledged in PR as bug: watch-mode cascade under-rebuilds receiver/extends/dynamic-import edges vs full build #1260. Acceptable to defer.
2. resolveByMethodOrGlobal typed-receiver lookup doesn't filter kind === 'method' (incremental.ts:494–497): SQL findNodeByName includes function kind matches for TypeName.methodName lookups where only method should match. Minor accuracy gap, no correctness regression vs. pre-fix.
3. Missing target-sort by confidence (incremental.ts:504–508): no correctness impact for edge insertion, only cosmetic ordering difference.

Final Recommendation

• Rating: ⭐⭐⭐⭐ (4/5)
• Action: APPROVE
• Reasoning: The fix is correct, well-scoped, and properly tested with a parity oracle. All three divergences are fixed with faithful semantics. The minor gaps (dynamic import priority, method-kind filter in typed-receiver path) are pre-existing or out-of-scope for this PR's stated goals, and bug: watch-mode cascade under-rebuilds receiver/extends/dynamic-import edges vs full build #1260 tracks the broader remaining work. The test will catch regressions on the exact failure modes that were fixed.

[greptile-apps]

Greptile Summary

This PR fixes a divergence between the watcher's incremental call-resolver (incremental.ts) and the authoritative full-build resolver (stages/build-edges.ts) that caused calls edge inflation (~+700 edges) on every reverse-dependency cascade rebuild. Three concrete fixes are applied: a gated global-fallback (receiver/confidence filter in resolveByMethodOrGlobal), per-file deduplication via seenCallEdges, and barrel re-export following in both buildImportedNamesMap and resolveCallTargets.

• incremental.ts: Replaces the unconstrained findNodeByName fan-out with resolveByMethodOrGlobal (mirrors full-build semantics), adds seenCallEdges dedup to buildCallEdges, and threads db through buildImportedNamesMap and buildCallEdges to enable barrel resolution.
• watcher.ts: Adds kind to the findNodeByName SELECT so resolveByMethodOrGlobal can filter to kind === 'method' when doing type-aware lookup.
• New integration test (issue-1259-watch-call-resolution.test.ts): Drives rebuildFile directly (the watch path) on a crafted fixture that exercises all three failure modes — duplicate call sites, barrel re-exports, and low-confidence global fan-out — and asserts exact parity with a clean full build.

Confidence Score: 5/5

Safe to merge. The changes are tightly scoped to the watch-mode call resolver, the fix is well-verified by a new integration test that directly exercises the previously-untested JS cascade, and end-to-end parity on the codegraph repo is confirmed.

All three root causes of the edge inflation are addressed with matching logic from the authoritative full-build path. The remaining divergences (dynamic-import priority ordering in buildImportedNamesMap, receiver/extends/dynamic-import edge under-rebuild) are pre-existing, acknowledged, and tracked separately as #1260 — none are introduced or worsened by this PR. The new test is specifically designed to catch regressions on the watch path that the existing parity tests missed.

No files require special attention.

Important Files Changed

Filename	Overview
src/domain/graph/builder/incremental.ts	Core fix: adds barrel-aware buildImportedNamesMap, resolveByMethodOrGlobal, and seenCallEdges dedup to align the incremental call-resolver with the full-build. One minor divergence remains: unlike the full-build native path, dynamic imports are not processed in a separate lower-priority first pass before static imports overwrite them — acknowledged as tracked in #1260 and out of scope for this PR.
src/domain/graph/watcher.ts	One-line fix: adds kind column to the findNodeByName prepared statement so the new resolveByMethodOrGlobal can filter typed lookups to kind === method.
tests/integration/issue-1259-watch-call-resolution.test.ts	New integration test that drives rebuildFile directly (bypassing the native orchestrator path that was masking the bug) and asserts exact edge parity with a fresh full build across all three failure modes. Fixture layout is deliberate — extra/deep/dup.js is placed to produce confidence 0.3 (different grandparent dir), reliably testing the filter threshold.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[resolveCallTargets] --> B{importedFrom in importedNames?}
    B -- yes --> C[findNodeInFile call.name, importedFrom]
    C --> D{targets empty AND importedFrom is barrel?}
    D -- yes --> E[resolveBarrelTarget → actualSource]
    E --> F[findNodeInFile call.name, actualSource]
    D -- no --> G[targets resolved]
    F --> G
    B -- no --> H[findNodeInFile call.name, relPath]
    H --> I{targets empty?}
    I -- yes --> J[resolveByMethodOrGlobal]
    J --> K{call.receiver set?}
    K -- yes, non-this/super --> L[typeMap lookup → qualified name search, filter kind==method]
    K -- no or this/self/super --> M[findNodeByName call.name, filter confidence >= 0.5]
    L --> N{typed results found?}
    N -- yes --> O[return typed]
    N -- no --> P[return empty - no global fan-out for unknown receivers]
    M --> O
    I -- no --> G
    G --> Q[sort by confidence desc]
    Q --> R[buildCallEdges: dedup via seenCallEdges, insertEdge]

Loading

_{Reviews (9): Last reviewed commit: "Merge branch 'main' into fix/dogfood-inc..." | Re-trigger Greptile}

[greptile-apps]

The && typeMap guard is always truthy here: typeMap is typed Map<string, unknown> and is always initialised to at least new Map() in buildCallEdges (the only call site). The dead condition also appears in the full-build's resolveByMethodOrGlobal, so this is a copy-paste carry-over, but removing it makes the guard clearer.

Suggested change

	if (call.receiver && typeMap) {
	if (call.receiver) {

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[carlos-alm]

Fixed — removed the dead && typeMap guard. typeMap is always initialized to at least new Map() in buildCallEdges, so the condition was always truthy. The check is now just if (call.receiver) as suggested.

[github-actions]

Codegraph Impact Analysis

7 functions changed → 11 callers affected across 3 files

• rebuildReverseDepEdges in src/domain/graph/builder/incremental.ts:165 (3 transitive callers)
• buildImportedNamesMap in src/domain/graph/builder/incremental.ts:385 (5 transitive callers)
• resolveCallTargets in src/domain/graph/builder/incremental.ts:445 (5 transitive callers)
• resolveByMethodOrGlobal in src/domain/graph/builder/incremental.ts:499 (4 transitive callers)
• buildCallEdges in src/domain/graph/builder/incremental.ts:533 (5 transitive callers)
• rebuildEdgesForTargetFile in src/domain/graph/builder/incremental.ts:611 (3 transitive callers)
• prepareWatcherStatements in src/domain/graph/watcher.ts:18 (3 transitive callers)

[carlos-alm]

Addressed Greptile's review feedback:

1. Dead && typeMap guard (inline comment at line 486): Removed the always-truthy && typeMap condition in resolveByMethodOrGlobal. Since typeMap is always initialized to new Map() in buildCallEdges, the guard was dead code. Now reads if (call.receiver).

2. Missing confidence sort (body issue 1): Added multi-target confidence sort in resolveCallTargets to match the full-build path in stages/build-edges.ts — when targets.length > 1, targets are now sorted by computeConfidence descending before returning. This eliminates the remaining semantic difference noted by Greptile.

3. Test assertion structure (body issue 2): Noted as fine as-is — no change needed. The two-step structure (early expect.fail with human-readable diff when lengths differ, then toEqual for set-equality) is intentional.

[carlos-alm]

@greptileai

[carlos-alm]

Addressed Greptile's body review issues:

1. resolveByMethodOrGlobal typed-receiver missing kind === 'method' filter (body issue 1, incremental.ts:503-504): Added kind to the findNodeByName SQL SELECT in watcher.ts and applied .filter((n) => n.kind === 'method') in JS, matching the full-build's build-edges.ts:624 exactly. Same change applied to the test fixture's makeStmts.

2. Parity check omits confidence and dynamic columns (body issue 2, test.ts:60-73): Added e.confidence, e.dynamic to the readEdges query. This immediately exposed a real parity gap — barrel-resolved calls had confidence 0.3 in watch vs 1.0 in full-build. Root cause: buildImportedNamesMap stored the barrel file as importedFrom instead of the defining file, so computeConfidence's importedFrom === targetFile shortcut never triggered. Fixed by mirroring the full-build's barrel follow-through in buildImportedNamesMap. All 4 parity tests (including the new stricter toEqual with confidence) pass.

[carlos-alm]

@greptileai

[carlos-alm]

@claude

[claude]

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

---

PR Review: fix(watch): align incremental call resolver with full build (follow-up)

• Gather context
• Read changed files
• Review latest changes
• Post findings

---

Review of latest commits (addressing Greptile's feedback)

Three changes landed since the last review:

1. kind === 'method' filter added to resolveByMethodOrGlobal (incremental.ts:514–516)
2. confidence and dynamic columns added to parity check in readEdges query (test.ts:63)
3. Barrel follow-through added to buildImportedNamesMap (incremental.ts:407–410)

---

Issue 1: kind === 'method' filter ✅

incremental.ts:514–517 now exactly matches build-edges.ts:624:

// incremental.ts (after fix)
const typed = (
  stmts.findNodeByName.all(qualified) as Array<{ id: number; file: string; kind?: string }>
).filter((n) => n.kind === 'method');

// build-edges.ts (reference)
const typed = (ctx.nodesByName.get(qualifiedName) || []).filter((n) => n.kind === 'method');

The kind column is now selected in watcher.ts:41 ("SELECT id, file, kind FROM nodes WHERE name = ..."), and the same SQL is in makeStmts in the test. The filter is semantically correct and faithfully mirrors the full-build path. ✅

---

Issue 2: confidence + dynamic in parity check ✅

readEdges now selects e.confidence, e.dynamic and the toEqual(fullEdges) check will catch per-edge attribute drift. This immediately exposed (and forced a fix for) the barrel confidence mismatch (0.3 vs 1.0) — the right design: a stricter assertion revealing a real bug, not a weaker assertion hiding it. ✅

---

Issue 3: Barrel follow-through in buildImportedNamesMap ✅

incremental.ts:407–410:

let targetFile = resolvedPath;
if (isBarrelFile(db, resolvedPath)) {
  const actual = resolveBarrelTarget(db, resolvedPath, cleanName);
  if (actual) targetFile = actual;
}
importedNames.set(cleanName, targetFile);

This correctly mirrors the confidence fix: when importedFrom in computeConfidence is the defining file rather than the barrel intermediary, the importedFrom === targetFile shortcut fires and returns 1.0 instead of the 0.3 cross-directory fallback. Faithful to the fix intent. ✅

One minor observation: The full-build's buildImportedNamesMap (build-edges.ts:514–541) does not follow barrels — it stores the raw resolved path including barrels. Barrel resolution happens instead in resolveCallTargets at build-edges.ts:582–587. The incremental version now does barrel follow-through in buildImportedNamesMap itself (not just in resolveCallTargets). This achieves the correct confidence, but via a slightly different path: the full-build stores importedFrom = barrel.js and resolveCallTargets then resolves through the barrel, so computeConfidence(relPath, t.file, barrel.js) gets a confidence based on the barrel being imported. The incremental version now stores importedFrom = impl.js directly.

Both paths produce confidence = 1.0 for the direct-import case, so there's no correctness regression. However this is a subtle semantic difference: if a symbol is imported via a barrel and the full-build computes confidence based on barrel proximity, the incremental version now uses direct-file proximity. In practice for the fixture's lib/impl.js imported via lib/barrel.js they're the same directory, so the result is identical. The existing parity test with confidence included now validates this. This is low risk.

---

Final Assessment

All three issues raised by the previous reviews are correctly addressed. The kind === 'method' filter achieves semantic parity with the full-build receiver-resolution path. The stricter parity test caught and forced a fix for a real confidence gap. The buildImportedNamesMap barrel follow-through ensures confidence scores match the full-build oracle.

Updated rating: ⭐⭐⭐⭐½ (4.5/5)
Action: APPROVE

The remaining gap (dynamic-import priority ordering in buildImportedNamesMap) is pre-existing and tracked as #1260. All three divergences from the original bug report are now fixed and verified by a parity test that includes confidence and dynamic.

---

View job