[CodeBlackwell]

Fixes #585.

Problem

qualified_name is the graph's stable identity key, built as file::name or file::parent.name. It is not unique per symbol: same-named symbols in one file (methods of different object literals, repeated generated helpers) produce the same key. upsert_node inserts with ON CONFLICT(qualified_name) DO UPDATE, so colliding rows collapse last writer wins and earlier definitions are silently dropped. A build then reports more nodes than the database stores, and caller/callee analysis on those names points at a single merged node.

Changes

Commit 1 — disambiguate colliding qualified_names. Deduplicate per file in store_file_nodes_edges and store_file_batch, which already hold the whole file's node list. The first occurrence keeps its bare key so existing edges still resolve to it; later collisions are suffixed with :L<line_start>, which is always unique because two definitions cannot share a start line. Unique nodes keep byte identical keys, so embeddings and incremental updates are unaffected.

Commit 2 — report disambiguated names. Adds GraphStore.find_disambiguated_nodes(), a disambiguated_nodes field in the full_build and incremental_update results, a log line, and a one line CLI summary after build/update. Counts are not rewritten: now that collisions are kept rather than dropped, the reported node total and the stored COUNT(*) already agree.

Verification

# before: build claims 4 nodes, status reports 2
# after:
$ code-review-graph build
Full build: 1 files, 4 nodes, 3 edges (postprocess=full)
Disambiguated 2 duplicate symbol name(s): .../db.ts::getById:L2, .../db.ts::getById:L3
$ code-review-graph status
Nodes: 4

New tests/test_collisions.py covers: both same-named methods persist, three module scope duplicates persist, an edge to the bare key still resolves to the first definition, the disambiguation report, and that unique names keep bare keys. Full suite green (1389 passed), ruff and mypy clean.

Caveat

A collided node's identity shifts if its own line_start moves. This affects only ambiguous duplicates, never the unique majority. Open to a different marker convention if you prefer (see #585).

🤖 Generated with Claude Code

[CodeBlackwell]

Quick status update: this fix is still current and independent of #614 (the CSS/SCSS feature PR) — the two don't overlap, so this one stays open on its own merits. The branch sits directly on today's main tip and GitHub reports it mergeable with no conflicts. Happy to adjust anything if you'd like changes.

[tirth8205]

Thanks—this prevents colliding rows from being lost, but ambiguous calls are still attached to the first definition. That can produce a confident but incorrect edge. The :L<line> identity also changes when lines move.

Please define a stable identity or migration/rebuild strategy, then make calls resolve to the correct duplicate when evidence exists—or remain explicitly ambiguous when it does not. Please add end-to-end tests for callers of each overload/duplicate and an incremental edit that shifts line numbers.

[tirth8205]

Thanks for the contribution. I rechecked exact head 99b0a41 against current main and am closing this PR.

Adding :L<line> suffixes preserves colliding rows but not graph correctness. Parser-produced edges still target the bare qualified name, so every ambiguous call resolves to the first definition and later duplicates cannot receive callers; one submitted test explicitly asserts that behavior. Harmless line shifts also change identity, and direct upsert_node() calls retain the original collapsing behavior unless callers opt into the override. There is no end-to-end coverage for correct or explicitly ambiguous caller resolution, or for identity migration after an incremental line shift. GitHub reports the branch non-mergeable, and both workflows are action_required with zero jobs.

Issue #585 remains open for a stable parser/resolver-level identity design.