fix(diff): read the target commit's blob, not the working tree - #77
Merged
Conversation
Both git-diff paths read the old side from the object store and the new side with `open(file_path)`. `compute_diff_for_commits` promises "added_nodes: New AST nodes in to_commit", and `ast-rag update` takes --from-commit/--to-commit as required options, so any pair other than <x>..HEAD on a clean tree parses the current source and stamps it with to_commit. `update_from_git` has the same line and writes to Neo4j, so the graph ends up holding code that never existed at the commit it is labelled with. Uncommitted work already has its own path in `get_workspace_diff`. Read the new side with `_read_blob` at the target commit. Absence there, not absence on disk, now decides the deletion branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Both git-diff paths read the old side from the git object store via
_read_bloband the new side withopen(file_path)— the working tree.compute_diff_for_commitsdocuments "added_nodes: New AST nodes into_commit", and
ast-rag updatetakes--from-commitand--to-commitasrequired options. So any pair other than
<x>..HEADon a clean tree parsesthe current source and stamps it with
to_commit.update_from_githas the same line and that one writes to Neo4j — the graphends up holding code that never existed at the commit it is labelled with.
Uncommitted work already has its own path in
get_workspace_diff, so the twowere not meant to be the same thing.
The new side now comes from
_read_blobat the target commit. Absence there,rather than absence on disk, decides the deletion branch — which also fixes the
case where a file is deleted at
to_commitbut still present locally.When
to_commitis HEAD and the tree is clean the blob and the file areidentical, so the common path is unchanged.
Related Issue
None filed — found while auditing the temporal/versioning claims in #59.
Type of Change
Checklist
ast-rag evaluate --all— needs live services; no query or scoring path touchedTesting
New
tests/test_diff_reads_the_named_commit.pybuilds a throwaway git repowhere
alphasits on line 1 at A, line 2 at B, and line 7 in an uncommittededit, then asks for
A..B. No Neo4j, Qdrant or network needed.Without the fix all three fail, reporting the working tree:
That
valid_fromis the point: the node is stamped with a commit whose sourceit did not come from.
One of the three asserts the weaker property directly — editing an uncommitted
file must not change the reported diff of two fixed commits.
Suite: 257 passed / 1 skipped / 1 xfailed before, 260 / 1 / 1 after (3 new).
Note for review
update_from_gitfilters candidates throughfile_changed_since_last_index,which hashes the file on disk. That is a separate pre-existing issue —
during a historical replay it can skip files whose current contents happen to
match the cache — and is left alone here.