diff: preserve left/right anchor paths

[fallintoplace]

Summary

• Track both pre-image and post-image file paths while parsing unified diff hunks.
• Use old-side path for LEFT anchoring and post-image path for RIGHT/context.
• Add regression tests for rename hunks and deleted-file hunks.

[arighi]

I don't really understand this patch, can you show an example of what is improving/fixing?

Moreover, a couple of issues found by codex:

• High — LEFT anchors for renames and deletions are removed before this code can use them. src/api.rs:4151 now looks up LEFT findings using
  old_file, but the earlier DiffIndex still indexes both sides under the +++ path (src/diff_index.rs:59, src/diff_index.rs:98). Deleted
  files are not indexed at all. Consequently, drop_unanchored_locations() removes old-path rename/deletion locations (src/main.rs:1041)
  before LKML attachments are generated. The commit’s primary feature therefore does not work end-to-end. DiffIndex also needs separate old/
  new paths, with LEFT entries indexed under the old path.

• Medium — deleted source text can be mistaken for an old-file header. src/api.rs:4067 treats every --- line as a file header. Inside a
  valid hunk, deleting a line beginning with -- produces exactly that prefix. The hunk collector already stops before such a line at src/
  api.rs:4095; the outer loop now consumes it as a header and overwrites current_old_file. Subsequent hunks in the same file then cannot
  match LEFT findings. Header parsing should be constrained to file-header state, or hunk bodies should be consumed according to their
  declared ranges.

Thanks.

[fallintoplace]

The patch is trying to fix a pretty specific anchoring problem with renamed or deleted files.

The simple example is a rename:

diff --git a/app/main.rs b/app/main_renamed.rs
rename from app/main.rs
rename to app/main_renamed.rs
--- a/app/main.rs
+++ b/app/main_renamed.rs
@@ -10,4 +10,4 @@ main flow
     trace!("before");
-    println!("old behavior");
+    println!("new behavior");
     trace!("after");

If a finding points at the removed line, that finding is on the old side of the diff. So the correct anchor is something like:

file = app/main.rs, side = LEFT

But the hunk also has a new path:

app/main_renamed.rs

Before this patch, the hunk lookup mostly used the new/right-side path, so a valid LEFT-side finding using the old filename could fail to match the hunk. In practice that means we can lose the exact diff context for the finding, especially when generating the LKML/report payload.

The same thing happens for deleted files:

diff --git a/app/removed.rs b/app/removed.rs
deleted file mode 100644
--- a/app/removed.rs
+++ /dev/null
@@ -5,2 +0,0 @@
-    console!("remove me");
-    cleanup();

There is no real new/right-side file here, because the file was deleted. But a finding on the deleted line should still be able to anchor to:

file = app/removed.rs, side = LEFT

So the goal of the patch is to preserve both paths from the diff:

• use the --- path for LEFT-side anchors
• use the +++ path for RIGHT-side anchors

That said, I agree with the issue Codex pointed out.

[arighi]

Thanks for clarifying it. There are some other issues found by codex, let me know what you think.

1. High — LEFT anchors are still removed before the new code can use them.

   The PR teaches collect_diff_hunks() about old/new paths, but leaves DiffIndex unchanged. It still:

   • Ignores the --- path.
   • Indexes removed and LEFT context lines under the +++ path.
   • Drops deleted files entirely when +++ /dev/null is encountered.

   See src/diff_index.rs:43, especially lines 59–68 and 89–107. The existing deleted-file test even asserts that LEFT anchors are
   unavailable at src/diff_index.rs:284.

   drop_unanchored_locations() runs before LKML hunk attachment generation, so a rename finding using the old path—or any deleted-file LEFT
   finding—loses its location first. The new tests at src/api.rs:5860 bypass that sanitizer and therefore do not demonstrate the feature
   working end-to-end.

2. Medium/High — the header-like payload problem is fixed only in the second parser.

   Commit eaff5d8 makes collect_diff_hunks() retain body lines such as --- looks like a header. However, DiffIndex::from_unified_diff()
   still interprets every --- or +++ line as a file header before considering whether it is inside a hunk.

   Consequently, the new HEADER_LIKE_BODY_PATCH scenario would still corrupt DiffIndex: --- looks... terminates indexing, +++ still body...
   becomes a bogus path, and the sanitizer can remove subsequent locations. Again, the new regression test exercises only
   collect_finding_hunks(), not the full validation pipeline.

3. Medium — the new hunk-body parser regresses traditional multi-file unified diffs.

   At src/api.rs:4322, hunk parsing now consumes every line beginning with space, +, -, or \ until a differently prefixed line appears. It
   does not stop after the old/new counts declared in the @@ header.

   For a valid unified diff without diff --git separators, the next file’s --- old/path and +++ new/path headers will be swallowed as body
   lines of the preceding hunk. The previous implementation explicitly stopped at those headers.

   The robust solution is to consume exactly the declared old/new ranges. That disambiguates real body lines from file headers without
   depending on diff --git.

4. Low — unrelated security-warning changes should be split out.

   The README and runtime subprocess-backend warnings at src/main.rs:2460 are unrelated to diff anchoring. They alter stderr for every
   subprocess-backed review and make this PR harder to evaluate and revert. They should be a separate PR unless there is missing context
   connecting them.

[arighi]

Looks good now. Thanks for all the iterations and improvements on this PR!