fix(harvester): address review feedback

Author: ParthAggarwal16

application/tests/harvester_test/diff_pipeline_test.py

@@ -1,4 +1,5 @@
 from datetime import UTC, datetime
+import subprocess
 import time
 import unittest
 
@@ -22,6 +23,23 @@ def test_pipeline_benchmark(self):
             "ASVS",
             "master",
         )
+        client.sync()
+
+        head_commit = client.get_current_commit_sha()
+
+        previous_commit = subprocess.run(
+            [
+                "git",
+                "-C",
+                str(client.get_local_path()),
+                "rev-parse",
+                "HEAD~1",
+            ],
+            check=True,
+            capture_output=True,
+            text=True,
+            timeout=300,
+        ).stdout.strip()
 
         retriever = DiffRetriever(client)
         parser = DiffParser()
@@ -30,14 +48,14 @@ def test_pipeline_benchmark(self):
         start = time.perf_counter()
 
         diff = retriever.get_diff(
-            "a79c0184",
-            "122d9e0969465a6041e16c806a0464b35deea444",
+            previous_commit,
+            head_commit,
         )
 
         blocks = parser.parse(
             diff,
             repository="OWASP/ASVS",
-            commit_sha="122d9e0969465a6041e16c806a0464b35deea444",
+            commit_sha=head_commit,
             committed_at=datetime.now(UTC),
         )
 

application/tests/harvester_test/diff_retriever_test.py

@@ -11,7 +11,7 @@ class DiffRetrieverTests(unittest.TestCase):
     @patch("application.utils.harvester.diff_retriever.subprocess.run")
     def test_get_diff(self, mock_run):
         mock_run.return_value = MagicMock(
-            stdout="diff --git a/README.md b/README.md\n",
+            stdout=b"diff --git a/README.md b/README.md\n",
         )
 
         client = MagicMock()
@@ -39,15 +39,14 @@ def test_get_diff(self, mock_run):
                 "def456",
             ],
             capture_output=True,
-            text=True,
             check=True,
             timeout=300,
         )
 
     @patch("application.utils.harvester.diff_retriever.subprocess.run")
     def test_large_diff_raises(self, mock_run):
         mock_run.return_value = MagicMock(
-            stdout="A" * (51 * 1024 * 1024),
+            stdout=b"A" * (51 * 1024 * 1024),
         )
 
         client = MagicMock()

application/tests/harvester_test/git_repository_client_test.py

@@ -89,21 +89,6 @@ def test_sync_fetches_when_repository_exists(self, mock_run):
 
         mock_fetch.assert_called_once()
 
-        mock_run.assert_called_once_with(
-            [
-                "git",
-                "-C",
-                str(client.get_local_path()),
-                "reset",
-                "--hard",
-                "origin/main",
-            ],
-            check=True,
-            capture_output=True,
-            text=True,
-            timeout=300,
-        )
-
     @patch("application.utils.harvester.git_repository_client.subprocess.run")
     def test_fetch_runs_git_command(self, mock_run):
         client = GitRepositoryClient(
@@ -130,6 +115,7 @@ def test_checkout_runs_git_command(self, mock_run):
                 "-C",
                 str(client.get_local_path()),
                 "checkout",
+                "--",
                 "main",
             ],
             check=True,

application/utils/harvester/checkpoint_store.py

@@ -22,8 +22,10 @@ def load(self, repository_id: str) -> RepositoryCheckpoint | None:
             .filter_by(repository_id=repository_id)
             .first()
         )
+
         if record is None:
             return None
+
         return RepositoryCheckpoint(
             repository_id=record.repository_id,
             last_processed_commit=record.last_processed_commit,
@@ -53,6 +55,7 @@ def save(self, checkpoint: RepositoryCheckpoint) -> None:
                 )
                 .first()
             )
+
             if canonical_conflict is not None:
                 session.rollback()
                 raise ValueError("duplicate canonical source identity")

application/utils/harvester/diff_parser.py

@@ -44,16 +44,16 @@ def parse(
 
                 continue
 
-            if line.startswith("+++"):
+            if line.startswith("+++ b/") or line.startswith("++/dev/null"):
                 continue
 
-            if line.startswith("---"):
+            if line.startswith("--- a/") or line.startswith("--- /dev/null"):
                 continue
 
             if line.startswith("@@"):
                 continue
 
-            if line.startswith("+") and not line.startswith("+++"):
+            if line.startswith("+"):
                 added_lines.append(line[1:])
 
         if current_file is not None:

application/utils/harvester/diff_retriever.py

@@ -56,21 +56,23 @@ def get_diff(self, base_commit: str, target_commit: str = "HEAD") -> str:
                 ],
                 check=True,
                 capture_output=True,
-                text=True,
                 timeout=300,
             )
         except subprocess.CalledProcessError as exc:
-            logger.error("Failed to retrieve diff: %s", exc.stderr)
+            logger.error(
+                "Failed to retrieve diff: %s",
+                exc.stderr.decode("utf-8", errors="replace"),
+            )
             raise
 
-        diff = result.stdout
+        diff_bytes = result.stdout
 
-        diff_size = len(diff.encode("utf-8"))
+        diff_size = len(diff_bytes)
 
         if diff_size > self.MAX_DIFF_SIZE_BYTES:
             raise ValueError(
                 f"Diff size ({diff_size} bytes) exceeds "
                 f"maximum supported size ({self.MAX_DIFF_SIZE_BYTES} bytes)."
             )
 
-        return diff
+        return diff_bytes.decode("utf-8", errors="replace")