fix(incremental): purge old paths on rename — parse git diff --name-status

[Qalipso]

Fixes #684.

Problem

git diff --name-only reports a rename a.py → b.py as only the new path, so incremental_update() never sees a.py. The purge loop only removes paths present in the changed/dependent set, so the old path's File/Function/Class nodes and every edge keyed to it survive — an incremental update permanently diverges from a full rebuild (stale nodes, dangling importer edges, inflated counts).

Fix

• Change discovery now uses git diff --name-status -z (both in the primary call and the --cached fallback).
• New _decode_name_status_paths() parses the NUL-delimited records and emits both sides of R*/C* entries (order-preserving, deduped). The old path then flows through the existing purge loop's is_file() check and is removed — no changes needed to the purge logic itself.
• _decode_nul_paths had no remaining callers and is removed.

Tests

• Unit: record parser — M/A/D, R100 (two paths), C75, dedupe, empty input.
• get_changed_files with a mocked rename returns both paths.
• End-to-end repro from the issue: real git repo, git mv a.py b.py + commit, get_changed_files(base="HEAD~1") → {a.py, b.py}, then incremental_update — asserts the old path is fully purged and the new one is present (full-rebuild parity).
• Updated the three existing get_changed_files mocks to the --name-status output format.

Full suite: 1966 passed, 5 skipped. ruff check clean.

AI-assisted; every change reviewed and understood.

[jondkinney]

I reviewed this alongside #686, which fixes the same issue with a very similar approach — noting it here so the maintainer can compare them easily, since the two would conflict if both landed.

My read is that each PR has one thing the other would benefit from:

This PR's parser implementation looks the more robust of the two. Using a slice (fields[i + 1 : i + 3]) means a truncated final record clamps harmlessly instead of relying on an index guard, and the seen set dedupes paths. #686 uses index arithmetic with an i + 1 < n check, where a malformed trailing R100 carrying only one path falls through and is treated as an ordinary single-path entry. Git shouldn't emit that, so it's a degenerate case rather than a real bug — but the slicing approach here sidesteps it by construction.

#686's tests are the stronger of the two. It adds test_get_changed_files_reports_both_rename_paths, which asserts the exact expected output for a rename record (["old/a.py", "new/b.py", "keep.py"]), plus an assertion that --name-status is actually in the git argv. This PR updates the existing fixtures to the new format and imports _decode_name_status_paths, but doesn't add a rename-specific case pinning both paths.

Combining the two — this parser with #686's tests — would probably give the best result, if that's helpful.

I checked the assumption both PRs depend on and it holds: incremental.py:1053 does if not abs_path.is_file(): store.remove_file_data(...), so surfacing the old path is sufficient for it to be purged. Both correctly leave D records alone, since --name-only already surfaced deletions.

One scope note that applies equally to both: this fixes the git-diff path, while start_watch_thread()'s GraphUpdateHandler has no on_moved handler, so --watch users would still accumulate stale nodes after a rename. I opened #742 for that separately — not a gap in this PR, just related.

Thanks for working on this.

[github-actions]

code-review-graph review

Overall risk: 0.40 (MEDIUM) — 14 changed function(s)/class(es), 0 affected flow(s), 2 test gap(s)

Risk-scored changes

Risk	Level	Symbol	Location	Tested
0.40	medium	code_review_graph/incremental.py::_decode_name_status_paths	code_review_graph/incremental.py:546	yes
0.35	low	tests/test_incremental.py::TestRenamePurgeParity._git	tests/test_incremental.py:1054	yes
0.30	low	code_review_graph/incremental.py::get_changed_files	code_review_graph/incremental.py:588	yes
0.15	low	tests/test_incremental.py::TestRenamePurgeParity.test_decode_name_status_emits_both_rename_paths	tests/test_incremental.py:1060	(test)
0.15	low	tests/test_incremental.py::TestRenamePurgeParity.test_decode_name_status_copy_records_and_dedupe	tests/test_incremental.py:1064	(test)
0.15	low	tests/test_incremental.py::TestRenamePurgeParity.test_decode_name_status_empty	tests/test_incremental.py:1068	(test)
0.05	low	tests/test_incremental.py::TestGitOperations	tests/test_incremental.py:526	no
0.05	low	tests/test_incremental.py::TestGitOperations.test_get_changed_files	tests/test_incremental.py:528	(test)
0.05	low	tests/test_incremental.py::TestGitOperations.test_get_changed_files_fallback	tests/test_incremental.py:543	(test)
0.05	low	tests/test_incremental.py::TestGitOperations.test_get_changed_files_rejects_failed_fallback	tests/test_incremental.py:555	(test)

Test gaps

• tests/test_incremental.py::TestGitOperations (tests/test_incremental.py:526)
• tests/test_incremental.py::TestRenamePurgeParity (tests/test_incremental.py:1050)

Token savings: this graph-backed report used ~24,581 fewer tokens (~90%) than reading every changed file in full (estimated, chars/4 approximation).

---

Powered by code-review-graph — local-first analysis; no code leaves the CI runner.