make the git diff step more robust

Author: tarekziade

reviewbot/clone_cache.py

@@ -398,21 +398,43 @@ def apply_patch(self, checkout: Checkout, diff_text: str) -> None:
         ``subprocess.CalledProcessError`` if the patch does not apply
         cleanly. ``--index`` keeps the index in sync so newly added files
         show up in :meth:`collect_changes`. The patch is written to a temp
-        file (not passed on the command line) so its size is unbounded."""
+        file (not passed on the command line) so its size is unbounded.
+
+        Strict ``git apply`` requires the hunk's context *and* its ``@@``
+        line numbers to match exactly. LLM-authored diffs routinely have
+        slightly-off line numbers or whitespace, so we retry with
+        ``--recount`` (recompute line numbers from the hunk body) and
+        ``--whitespace=fix``. The retry still requires the context lines to
+        match, so it tolerates bad geometry without applying to the wrong
+        place — ``git apply --index`` is all-or-nothing, so a failed attempt
+        leaves the tree untouched for the next one."""
         with tempfile.NamedTemporaryFile(
             "w", suffix=".patch", delete=False, encoding="utf-8"
         ) as fh:
             patch_path = fh.name
             # git apply requires the diff to end with a newline.
             fh.write(diff_text if diff_text.endswith("\n") else diff_text + "\n")
+        attempts = (
+            ("strict", ["apply", "--index", "--whitespace=nowarn", patch_path]),
+            (
+                "recount",
+                ["apply", "--index", "--whitespace=fix", "--recount", patch_path],
+            ),
+        )
         try:
-            self._git(
-                checkout.path,
-                "apply",
-                "--index",
-                "--whitespace=nowarn",
-                patch_path,
-                timeout=120,
+            last_stderr = ""
+            for label, args in attempts:
+                proc = self._git(checkout.path, *args, timeout=120, check=False)
+                if proc.returncode == 0:
+                    if label != "strict":
+                        log.info(
+                            "patch applied via '%s' fallback (strict git apply failed)",
+                            label,
+                        )
+                    return
+                last_stderr = proc.stderr.decode("utf-8", errors="replace")
+            raise subprocess.CalledProcessError(
+                1, ["git", "apply"], stderr=last_stderr.encode()
             )
         finally:
             os.unlink(patch_path)

tests/test_clone_cache_tasks.py

@@ -116,6 +116,42 @@ def test_apply_patch_failure_raises(self):
         with self.assertRaises(subprocess.CalledProcessError):
             self.cache.apply_patch(co, bad)
 
+    def test_apply_patch_recount_fallback_fixes_bad_line_numbers(self):
+        # The @@ header line counts are wrong (claims 4 lines); strict
+        # `git apply` rejects this as corrupt, mirroring the off-by-some
+        # line numbers in LLM-authored diffs. The --recount fallback
+        # recomputes them from the hunk body, so the edit still applies.
+        co = self._acquire()
+        bad_geometry = (
+            "diff --git a/hello.txt b/hello.txt\n"
+            "--- a/hello.txt\n"
+            "+++ b/hello.txt\n"
+            "@@ -1,4 +1,4 @@\n"
+            "-hi from main\n"
+            "+hi patched\n"
+        )
+        # Strict apply alone fails on the bad geometry…
+        strict = self.cache._git(
+            co.path,
+            "apply",
+            "--index",
+            "--whitespace=nowarn",
+            self._patch_file(bad_geometry),
+            check=False,
+        )
+        self.assertNotEqual(strict.returncode, 0)
+        # …but apply_patch's recount fallback lands it.
+        self.cache.apply_patch(co, bad_geometry)
+        changes = {c.path: c for c in self.cache.collect_changes(co)}
+        self.assertEqual(changes["hello.txt"].content, b"hi patched\n")
+
+    def _patch_file(self, text):
+        fh = tempfile.NamedTemporaryFile("w", suffix=".patch", delete=False)
+        self.addCleanup(os.unlink, fh.name)
+        fh.write(text if text.endswith("\n") else text + "\n")
+        fh.close()
+        return fh.name
+
 
 if __name__ == "__main__":
     unittest.main()