fix(wasm): align sort_targets_by_confidence with native engine

[carlos-alm]

The native engine sorts call targets by confidence before emitting edges; the WASM/TS engine did not. For multi-target calls with duplicate (source_id, target_id) pairs, the stored confidence could differ between engines depending on insertion order.

Adds equivalent descending-confidence sort to buildFileCallEdges in build-edges.ts, matching the native sort order and eliminating the latent parity gap.

Closes #1476

[claude]

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

---

PR Review — fix(wasm): align sort_targets_by_confidence with native engine

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

---

Root Cause Analysis

Problem Being Solved: The WASM/TS engine's buildFileCallEdges function in build-edges.ts was not sorting call targets by confidence before iterating and emitting edges. The native Rust engine does sort (via sort_targets_by_confidence in build_edges.rs:929-937). For multi-target calls where the same (source_id, target_id) pair appears more than once — due to e.g. different resolution paths producing the same node ID — the batchInsertEdges upsert semantics mean the last inserted row wins. Without sorting, the stored confidence is nondeterministic with respect to the order targets arrive from resolveCallTargets. The fix makes it deterministic and matching the native engine: highest-confidence target wins.

Why This Approach: Mirrors the native engine exactly — both use computeConfidence(relPath, t.file, importedFrom) as the sort key, both sort descending before iterating targets. This is the correct fix at the right level (source, not workaround).

Risk Assessment: Low. The sort is guarded by targets.length > 1 (no-op for the common single-target case), uses a pure function (computeConfidence) already called for each target in the loop body, and creates a shallow copy ([...targets]) to avoid mutating the original. The only observable behavior change is which confidence value is stored when duplicate pairs would be inserted — previously nondeterministic (insertion-order-dependent), now deterministic (highest confidence wins).

---

Detailed Code Analysis

Core fix — build-edges.ts:1115-1121

if (targets.length > 1) {
  targets = [...targets].sort(
    (a, b) =>
      computeConfidence(relPath, b.file, importedFrom ?? null) -
      computeConfidence(relPath, a.file, importedFrom ?? null),
  );
}

This correctly mirrors the native implementation at build_edges.rs:929-937:

fn sort_targets_by_confidence(targets: &mut Vec<&NodeInfo>, rel_path: &str, imported_from: Option<&str>) {
    if targets.len() > 1 {
        targets.sort_by(|a, b| {
            let conf_a = resolve::compute_confidence(rel_path, &a.file, imported_from);
            let conf_b = resolve::compute_confidence(rel_path, &b.file, imported_from);
            conf_b.partial_cmp(&conf_a).unwrap_or(std::cmp::Ordering::Equal)
        });
    }
}

✅ Correct sort direction (descending — b - a in JS, conf_b.partial_cmp(&conf_a) in Rust)
✅ Correct sort key (same computeConfidence arguments in both engines)
✅ Correct placement (before the emit loop, matching native placement at build_edges.rs:556)

One nuance worth noting: the JS sort recomputes computeConfidence once per comparison (O(n log n) calls), and the loop body at build-edges.ts:1126 recomputes it again for each target. This is the same pattern already present throughout the file — computeConfidence is a cheap, pure function, so no concern here. Just consistent with the existing style.

Placement concern — sort is outside the pts fallback path

The sort correctly fires before the direct-call emit loop (line 1123). However, the pts fallback path (lines 1175–1253) and the receiver-pts path (lines 1220–1253) each iterate their own aliasTargets without sorting. The native engine also calls sort_targets_by_confidence at build_edges.rs:398 inside the pts fallback (the emit_pts_alias_edges path). This PR addresses the main path only.

This is a latent parity gap — not introduced by this PR — but worth noting for future work.

Other commits in the PR

The PR bundles six additional commits that are unrelated to the sort fix:

Commit	Scope	Assessment
85a26df — remove dead code (Rust: cfg, complexity, dataflow, incremental, clojure)	chore(native)	Clean clippy/compiler hygiene. The clojure.rs #[allow(unused_assignments)] annotation is the right tool (lifetime anchor pattern). All changes are non-behavioral. ✅
184d221 — extract PtsAliasCtx struct in build_edges.rs	refactor(native)	Improves readability of emit_pts_alias_edges. No behavioral change. ✅
9a52c7c — snapshot-pre-bash.sh + track-bash-writes.sh hooks	fix(hooks)	Fills a real gap (Closes #1457): sed -i and other shell file writes were invisible to guard-git.sh. The two hooks correctly use git status --porcelain before/after with per-worktree temp files. ✅
.claude/settings.json — wire the two new hooks	Companion to above	Correct hook wiring. ✅
ef8ea4f — remove unused biome suppression in visitor.test.ts	chore(tests)	Removes a // biome-ignore lint comment, not a test assertion. ✅
a372b82 — titan-run SKILL.md: add close and grind to --start-from enum	fix(titan-run)	Corrects a documentation gap — close and grind are valid values but were missing from the argument hint and the phase list. ✅
3892e7d — gitignore napi-generated artifacts	chore	Adds index.js, index.d.ts, *.node to .gitignore under crates/codegraph-core/. Correct. ✅

---

Phase 2: Configuration Change Detection

No linting rules relaxed. No test timeouts increased. No TypeScript strict mode flags changed. The settings.json change only adds two new hook entries — it does not remove or relax any existing hook. No concerns.

Phase 3: Test Integrity

tests/unit/visitor.test.ts removes one line — a // biome-ignore lint/... suppression comment over an await import(...) at the module top level. The assertion count is unchanged. No test weakening.

No test was added to cover the sort-order fix specifically. This is a minor gap — a test asserting that duplicate (source_id, target_id) pairs resolve to the highest-confidence confidence value would prevent regression. However, given the fix is a direct port of existing native behavior and the sort is trivially correct, the missing test is acceptable, not blocking.

Phase 4: Code Quality

No issues found. The implementation is idiomatic, clean, and mirrors the native exactly.

Phase 5: Backlog Compliance

• Zero-dep: ✓ — no new runtime dependencies
• Foundation-aligned: ✓ — fixes a local graph-build correctness issue; no cloud calls, no schema changes, no MCP contract changes
• Problem-fit: 4 — correct edges stored in the DB directly improves the accuracy of any downstream query (impact, blast-radius, call-chains) that an AI agent uses to orient itself. A wrong confidence value can cause the wrong edge to win the upsert, silently producing a lower-fidelity graph.
• Breaking: No — stored confidence values may change for the small subset of multi-target calls where duplicate pairs were previously inserted in non-descending order. This is a correctness improvement, not a contract break.
• Tier: 1

Critical Concerns

Minor — scope bundling: This PR packages 6 commits across 4 unrelated concerns (sort fix, dead-code cleanup, bash-write hooks, titan-run docs) into a single PR. Per the project's "One PR = one concern" rule in CLAUDE.md, these should be separate PRs. However, all changes are independently correct and low-risk, so this is a process observation, not a blocker.

Minor — pts fallback path not sorted: The pts alias loop (build-edges.ts:1175-1253) iterates aliasTargets without sorting by confidence before emit. The native engine does sort in emit_pts_alias_edges (build_edges.rs:398). This pre-existing gap is not introduced by this PR and is not blocking.

Final Recommendation

• Rating: ⭐⭐⭐⭐ (4/5)
• Action: APPROVE
• Reasoning: The core fix is correct, minimal, and exactly mirrors the native engine. All supporting changes in the other commits are clean and non-behavioral. The only deductions are the scope-bundling (process) and the missing regression test — neither is blocking. The sort-fix itself closes a real parity gap that could produce different query results depending on which engine built the graph.

---

[greptile-apps]

Greptile Summary

Aligns the WASM/TS edge builder with the native Rust engine by sorting call targets by confidence descending before the dedup loop, so the highest-confidence entry wins when multiple targets share the same (source_id, target_id) key.

• Direct targets array (main call path, line 1115) is now sorted before the seenCallEdges dedup loop, mirroring sort_targets_by_confidence in build_edges.rs.
• Both points-to alias paths (Phase 8.3, line 1204; Phase 8.3f, line 1248) also receive the equivalent sort, closing the parity gap flagged in the prior review.

Confidence Score: 5/5

Targeted, additive sort; does not mutate shared data structures and only affects output when a call has multiple targets resolving to the same edge key.

The change is narrowly scoped — a spread-copy followed by a comparison sort — applied consistently across all three target-resolution paths. The sort comparator is correct (b−a for descending), the original arrays are never mutated, and the dedup logic already relied on first-insertion winning, so inserting a higher-confidence entry first produces the intended result. The previously flagged alias paths are also fixed here, leaving no known parity gaps.

No files require special attention.

Important Files Changed

Filename	Overview
src/domain/graph/builder/stages/build-edges.ts	Adds descending-confidence sort before edge dedup for all three target-resolution paths (direct targets, Phase 8.3 alias, Phase 8.3f receiver alias), matching native engine behaviour

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Resolve call targets] --> B{targets.length > 1?}
    B -- Yes --> C["Sort targets DESC by computeConfidence\n(NEW — mirrors native sort_targets_by_confidence)"]
    B -- No --> D[Use targets as-is]
    C --> E[Dedup loop over sorted targets\nFirst occurrence wins]
    D --> E

    E --> F{targets.length == 0 AND\nptsMap available?}
    F -- Yes --> G[Phase 8.3: resolveViaPointsTo loop]
    G --> H["Per alias: resolveCallTargets → aliasTargets"]
    H --> I{aliasTargets.length > 1?}
    I -- Yes --> J["Sort aliasTargets DESC by computeConfidence\n(NEW)"]
    I -- No --> K[Use aliasTargets as-is]
    J --> L[Dedup loop over sortedAliasTargets]
    K --> L

    F -- Yes/receiver --> M[Phase 8.3f: receiver pts loop]
    M --> N["Per alias: resolveCallTargets → aliasTargets"]
    N --> O{aliasTargets.length > 1?}
    O -- Yes --> P["Sort aliasTargets DESC by computeConfidence\n(NEW)"]
    O -- No --> Q[Use aliasTargets as-is]
    P --> R[Dedup loop over sortedAliasTargets]
    Q --> R

Loading

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

[github-actions]

Codegraph Impact Analysis

1 functions changed → 3 callers affected across 2 files

• buildFileCallEdges in src/domain/graph/builder/stages/build-edges.ts:979 (3 transitive callers)

[carlos-alm]

Fixed — added descending-confidence sort to aliasTargets in both the Phase 8.3 and Phase 8.3f pts fallback loops in build-edges.ts, matching the sort_targets_by_confidence call in the native emit_pts_alias_edges (build_edges.rs:398). Both reviewers (Claude and Greptile) identified this residual gap and it is now closed in commit a0892c9.

[carlos-alm]

@greptileai

[carlos-alm]

Resolved merge conflict with main — the branch's copies of .claude/hooks/snapshot-pre-bash.sh, .claude/hooks/track-bash-writes.sh, and .claude/skills/titan-run/SKILL.md were add/add conflicts against the finalized versions already merged to main via PRs #1482 and #1483. Accepted main's versions (the corrected per-command-hash snapshot keying and the complete --start-from enum). Core fix in build-edges.ts and build_edges.rs is unchanged.

[carlos-alm]

@greptileai