Skip to content

Fix incremental update leaving stale nodes after a file rename#686

Open
thejesh23 wants to merge 1 commit into
tirth8205:mainfrom
thejesh23:fix/crg-001
Open

Fix incremental update leaving stale nodes after a file rename#686
thejesh23 wants to merge 1 commit into
tirth8205:mainfrom
thejesh23:fix/crg-001

Conversation

@thejesh23

Copy link
Copy Markdown

Linked issue

Closes #684

What & why

get_changed_files() discovered changes with git diff --name-only, which reports a rename a.py → b.py as only the new path b.py. incremental_update() therefore never saw a.py, and its purge loop only removes paths that are in the changed/dependent set and missing on disk — the old path is in neither. Unlike full_build() (which purges existing_files - current_abs), the incremental path had no "purge files no longer on disk" step, so the renamed-away file's File/Function/Class nodes and every edge keyed to it survived. The incremental graph diverged from a full rebuild.

This switches change discovery to git diff --name-status -z and adds _parse_name_status_z(), which emits both the old and new path for rename/copy (R/C) records. The old path then flows through the existing purge loop and is removed, so an incremental update again equals a full rebuild. The --cached fallback branch gets the same treatment. The now-unused _decode_nul_paths() helper is removed.

How it was tested

uv run pytest tests/test_incremental.py tests/test_integration_git.py -q   # 85 passed
uv run pytest tests/ --tb=short -q   # 1962 passed (1 pre-existing Windows/py3.14 failure, unrelated)
uv run ruff check code_review_graph/
uv run mypy code_review_graph/incremental.py --ignore-missing-imports --no-strict-optional   # Success

New tests (both fail on main, pass here):

  • test_incremental.py::TestGitOperations::test_get_changed_files_reports_both_rename_paths — unit test of the name-status parser.
  • test_integration_git.py::test_incremental_update_purges_renamed_source — real-git test asserting the renamed-away source is purged and node/edge/file counts match a fresh full_build.

Checklist

  • Tests added for new functionality
  • All tests pass: uv run pytest tests/ --tb=short -q (one pre-existing, unrelated Windows/py3.14 failure)
  • Linting passes: uv run ruff check code_review_graph/
  • Type checking passes: uv run mypy code_review_graph/incremental.py --ignore-missing-imports --no-strict-optional
  • Lines are at most 100 characters
  • Docs updated where behavior changed (docstrings on the new/changed functions)

get_changed_files() used 'git diff --name-only', which reports a rename
a.py -> b.py as only the new path b.py. incremental_update() therefore
never saw a.py, and its purge loop (which only removes paths present in
the changed/dependent set and missing on disk) left a.py's File/Function/
Class nodes and all edges keyed to it in the graph. The result diverged
from a full rebuild, which drops the old path.

Switch to 'git diff --name-status -z' and parse it so renames/copies
emit BOTH the old and new path. The old path is then seen by the existing
purge loop and removed, making an incremental update equal a full rebuild.

Adds a unit test for the name-status parser and a real-git integration
test asserting the renamed-away source is purged and node/edge/file counts
match a fresh full_build.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: File rename leaves stale nodes/edges — incremental update diverges from full rebuild

1 participant