diff --git a/.gitignore b/.gitignore index 26b6ceb61..bc53fd244 100644 --- a/.gitignore +++ b/.gitignore @@ -85,3 +85,5 @@ tmp/ cres/* ### Local project management tooling project management scripts/ + +.harvester_cache/ diff --git a/application/tests/harvester_test/diff_normalizer_test.py b/application/tests/harvester_test/diff_normalizer_test.py new file mode 100644 index 000000000..04eb1ce3f --- /dev/null +++ b/application/tests/harvester_test/diff_normalizer_test.py @@ -0,0 +1,117 @@ +import unittest +from datetime import datetime + +from application.utils.harvester.diff_normalizer import ( + DiffNormalizer, +) + +from application.utils.harvester.models import ( + DiffBlock, +) + + +DIFF_METADATA = { + "repository": "OWASP/ASVS", + "commit_sha": "abc123", + "committed_at": datetime(2026, 1, 1), +} + + +class DiffNormalizerTests(unittest.TestCase): + def test_whitespace_normalization(self): + normalizer = DiffNormalizer() + + blocks = [ + DiffBlock( + file_path="README.md", + added_lines=[ + " Hello World ", + "\t\tTabs\t\tEverywhere\t", + "", + " ", + "Unicode\u00a0Space", + "Mix\t of\t tabs and spaces", + " Multiple words together ", + "\u00a0\u00a0Leading unicode spaces\u00a0", + " ## Authentication ", + " - Use MFA ", + " `inline code` ", + " **Important** ", + ], + **DIFF_METADATA, + ) + ] + + result = normalizer.normalize(blocks) + + self.assertEqual( + result[0].added_lines, + [ + "Hello World", + "Tabs Everywhere", + "Unicode Space", + "Mix of tabs and spaces", + "Multiple words together", + "Leading unicode spaces", + "## Authentication", + "- Use MFA", + "`inline code`", + "**Important**", + ], + ) + + def test_remove_empty_lines(self): + normalizer = DiffNormalizer() + + blocks = [ + DiffBlock( + file_path="README.md", + added_lines=[ + "", + " ", + "Hello", + ], + **DIFF_METADATA, + ) + ] + + result = normalizer.normalize(blocks) + + self.assertEqual( + result[0].added_lines, + [ + "Hello", + ], + ) + + def test_multiple_blocks(self): + normalizer = DiffNormalizer() + + blocks = [ + DiffBlock( + file_path="a.md", + added_lines=[" One "], + **DIFF_METADATA, + ), + DiffBlock( + file_path="b.md", + added_lines=[" Two "], + **DIFF_METADATA, + ), + ] + + result = normalizer.normalize(blocks) + + self.assertEqual( + result[0].added_lines, + ["One"], + ) + + self.assertEqual( + result[1].added_lines, + ["Two"], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/diff_parser_test.py b/application/tests/harvester_test/diff_parser_test.py new file mode 100644 index 000000000..a4444e8d9 --- /dev/null +++ b/application/tests/harvester_test/diff_parser_test.py @@ -0,0 +1,103 @@ +from datetime import UTC, datetime +import unittest + +from application.utils.harvester.diff_parser import ( + DiffParser, +) + +TEST_REPOSITORY = "OWASP/ASVS" +TEST_COMMIT_SHA = "abc123" +TEST_COMMITTED_AT = datetime.now(UTC) + + +class DiffParserTests(unittest.TestCase): + def test_single_file_diff(self): + parser = DiffParser() + + diff = """diff --git a/test.md b/test.md +--- a/test.md ++++ b/test.md +@@ +-old ++new ++another +""" + + blocks = parser.parse( + diff, + repository=TEST_REPOSITORY, + commit_sha=TEST_COMMIT_SHA, + committed_at=TEST_COMMITTED_AT, + ) + + self.assertEqual(len(blocks), 1) + + self.assertEqual( + blocks[0].file_path, + "test.md", + ) + + self.assertEqual( + blocks[0].added_lines, + [ + "new", + "another", + ], + ) + + self.assertEqual(blocks[0].repository, TEST_REPOSITORY) + self.assertEqual(blocks[0].commit_sha, TEST_COMMIT_SHA) + self.assertEqual(blocks[0].committed_at, TEST_COMMITTED_AT) + + def test_multiple_files(self): + parser = DiffParser() + + diff = """diff --git a/a.md b/a.md +@@ ++one +diff --git a/b.md b/b.md +@@ ++two +""" + + blocks = parser.parse( + diff, + repository=TEST_REPOSITORY, + commit_sha=TEST_COMMIT_SHA, + committed_at=TEST_COMMITTED_AT, + ) + + self.assertEqual(len(blocks), 2) + + self.assertEqual(blocks[0].file_path, "a.md") + self.assertEqual(blocks[1].file_path, "b.md") + + self.assertEqual(blocks[0].repository, TEST_REPOSITORY) + self.assertEqual(blocks[1].repository, TEST_REPOSITORY) + + def test_deleted_lines_are_ignored(self): + parser = DiffParser() + + diff = """diff --git a/test.md b/test.md +@@ +-old ++new +""" + + blocks = parser.parse( + diff, + repository=TEST_REPOSITORY, + commit_sha=TEST_COMMIT_SHA, + committed_at=TEST_COMMITTED_AT, + ) + + self.assertEqual( + blocks[0].added_lines, + [ + "new", + ], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/diff_pipeline_test.py b/application/tests/harvester_test/diff_pipeline_test.py new file mode 100644 index 000000000..07160f170 --- /dev/null +++ b/application/tests/harvester_test/diff_pipeline_test.py @@ -0,0 +1,73 @@ +from datetime import UTC, datetime +import subprocess +import time +import unittest +import os + +from application.utils.harvester.diff_normalizer import DiffNormalizer +from application.utils.harvester.diff_parser import DiffParser +from application.utils.harvester.diff_retriever import DiffRetriever +from application.utils.harvester.git_repository_client import GitRepositoryClient + + +class DiffPipelineBenchmark(unittest.TestCase): + """ + Simple benchmark to ensure the complete diff pipeline remains fast. + + This is not intended as a strict performance benchmark, only as a + regression guard against accidental slowdowns. + """ + + def test_pipeline_benchmark(self): + + if os.getenv("OPENCRE_RUN_NETWORK_TESTS") != "1": + self.skipTest("Network benchmark disabled") + + client = GitRepositoryClient( + "OWASP", + "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() + normalizer = DiffNormalizer() + + start = time.perf_counter() + + diff = retriever.get_diff( + previous_commit, + head_commit, + ) + + blocks = parser.parse( + diff, + repository="OWASP/ASVS", + commit_sha=head_commit, + committed_at=datetime.now(UTC), + ) + + normalizer.normalize(blocks) + + elapsed = time.perf_counter() - start + + print(f"\nPipeline took {elapsed:.3f}s") + + self.assertLess(elapsed, 5) diff --git a/application/tests/harvester_test/diff_retriever_test.py b/application/tests/harvester_test/diff_retriever_test.py new file mode 100644 index 000000000..502ac2d39 --- /dev/null +++ b/application/tests/harvester_test/diff_retriever_test.py @@ -0,0 +1,99 @@ +import unittest +from unittest.mock import MagicMock +from unittest.mock import patch +from unittest.mock import call + +from application.utils.harvester.diff_retriever import ( + DiffRetriever, +) + + +class DiffRetrieverTests(unittest.TestCase): + @patch("application.utils.harvester.diff_retriever.subprocess.run") + def test_get_diff(self, mock_run): + mock_run.side_effect = [ + MagicMock(stdout="abc123\n"), + MagicMock(stdout="def456\n"), + MagicMock(stdout=b"diff --git a/README.md b/README.md\n"), + ] + + client = MagicMock() + client.get_local_path.return_value = "/tmp/repo" + + retriever = DiffRetriever(client) + + diff = retriever.get_diff( + "abc123", + "def456", + ) + + self.assertEqual( + diff, + "diff --git a/README.md b/README.md\n", + ) + + mock_run.assert_has_calls( + [ + call( + [ + "git", + "-C", + "/tmp/repo", + "rev-parse", + "--verify", + "--end-of-options", + "abc123^{commit}", + ], + check=True, + capture_output=True, + text=True, + timeout=60, + ), + call( + [ + "git", + "-C", + "/tmp/repo", + "rev-parse", + "--verify", + "--end-of-options", + "def456^{commit}", + ], + check=True, + capture_output=True, + text=True, + timeout=60, + ), + call( + [ + "git", + "-C", + "/tmp/repo", + "diff", + "abc123", + "def456", + ], + check=True, + capture_output=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=b"A" * (51 * 1024 * 1024), + ) + + client = MagicMock() + client.get_local_path.return_value = "/tmp/repo" + + retriever = DiffRetriever(client) + + with self.assertRaises(ValueError): + retriever.get_diff("a", "b") + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/git_repository_client_test.py b/application/tests/harvester_test/git_repository_client_test.py index 941afd67e..8bbff6ab8 100644 --- a/application/tests/harvester_test/git_repository_client_test.py +++ b/application/tests/harvester_test/git_repository_client_test.py @@ -115,6 +115,7 @@ def test_checkout_runs_git_command(self, mock_run): "-C", str(client.get_local_path()), "checkout", + "--", "main", ], check=True, diff --git a/application/utils/harvester/__init__.py b/application/utils/harvester/__init__.py index eda28b629..9961aae16 100644 --- a/application/utils/harvester/__init__.py +++ b/application/utils/harvester/__init__.py @@ -19,6 +19,8 @@ from .repository_cache import build_repository_cache_path from .file_filter import FileFilter from .filtering_metrics import FilteringMetricsCollector +from .diff_retriever import DiffRetriever + from .filtering_benchmark import ( FilteringBenchmark, FilteringBenchmarkResult, @@ -28,6 +30,7 @@ "build_repository_cache_path", "ChunkingConfig", "ConfigLoaderError", + "DiffRetriever", "GitRepositoryClient", "FileFilter", "FilteringMetricsCollector", diff --git a/application/utils/harvester/checkpoint_store.py b/application/utils/harvester/checkpoint_store.py index e9e3b62f4..ed757bb56 100644 --- a/application/utils/harvester/checkpoint_store.py +++ b/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") diff --git a/application/utils/harvester/diff_normalizer.py b/application/utils/harvester/diff_normalizer.py new file mode 100644 index 000000000..756d89147 --- /dev/null +++ b/application/utils/harvester/diff_normalizer.py @@ -0,0 +1,47 @@ +import re +import unicodedata + +from .models import DiffBlock + + +class DiffNormalizer: + """ + Normalizes extracted diff content. + + Whitespace is collapsed, Unicode normalized, + and empty lines removed. + """ + + def normalize_line(self, line: str) -> str: + line = unicodedata.normalize("NFKC", line) + line = re.sub(r"\s+", " ", line) + return line.strip() + + def normalize(self, blocks: list[DiffBlock]) -> list[DiffBlock]: + """ + Normalize every added line in each DiffBlock. + """ + normalized: list[DiffBlock] = [] + + for block in blocks: + cleaned_lines: list[str] = [] + + for line in block.added_lines: + line = self.normalize_line(line) + + if not line: + continue + + cleaned_lines.append(line) + + normalized.append( + DiffBlock( + file_path=block.file_path, + added_lines=cleaned_lines, + repository=block.repository, + commit_sha=block.commit_sha, + committed_at=block.committed_at, + ) + ) + + return normalized diff --git a/application/utils/harvester/diff_parser.py b/application/utils/harvester/diff_parser.py new file mode 100644 index 000000000..12b14784c --- /dev/null +++ b/application/utils/harvester/diff_parser.py @@ -0,0 +1,69 @@ +from datetime import datetime +import re + +from .models import DiffBlock + + +class DiffParser: + """ + Parses unified git diffs into DiffBlock objects. + + Only added lines are extracted. + Deleted lines and diff metadata are ignored. + """ + + def parse( + self, diff: str, repository: str, commit_sha: str, committed_at: datetime + ) -> list[DiffBlock]: + """ + Convert a unified git diff into DiffBlock objects. + """ + blocks: list[DiffBlock] = [] + + current_file: str | None = None + added_lines: list[str] = [] + + for line in diff.splitlines(): + if line.startswith("diff --git"): + if current_file is not None: + blocks.append( + DiffBlock( + file_path=current_file, + added_lines=added_lines, + repository=repository, + commit_sha=commit_sha, + committed_at=committed_at, + ) + ) + + match = re.match(r"diff --git a/(.+?) b/", line) + + current_file = match.group(1) if match else None + added_lines = [] + + continue + + if line.startswith("+++ b/") or line.startswith("+++ /dev/null"): + continue + + if line.startswith("--- a/") or line.startswith("--- /dev/null"): + continue + + if line.startswith("@@"): + continue + + if line.startswith("+"): + added_lines.append(line[1:]) + + if current_file is not None: + blocks.append( + DiffBlock( + file_path=current_file, + added_lines=added_lines, + repository=repository, + commit_sha=commit_sha, + committed_at=committed_at, + ) + ) + + return blocks diff --git a/application/utils/harvester/diff_retriever.py b/application/utils/harvester/diff_retriever.py new file mode 100644 index 000000000..7efd45560 --- /dev/null +++ b/application/utils/harvester/diff_retriever.py @@ -0,0 +1,100 @@ +import logging +import subprocess + +from .git_repository_client import GitRepositoryClient + +logger = logging.getLogger(__name__) + + +class DiffRetriever: + """ + + Retrieves unified git diffs between two commits. + + This class is responsible only for retrieving raw diff text. + + Parsing and normalization are handled by downstream components. + + """ + + MAX_DIFF_SIZE_BYTES = 50 * 1024 * 1024 + + def __init__(self, repository_client: GitRepositoryClient) -> None: + self.repository_client = repository_client + + def get_diff(self, base_commit: str, target_commit: str = "HEAD") -> str: + """ + Return the unified git diff between two commits. + + Args: + base_commit: + Base commit SHA. + target_commit: + Target commit SHA or branch. + + Raises: + subprocess.CalledProcessError: + If git diff fails. + + ValueError: + If the diff exceeds the configured size limit. + """ + logger.info( + "Retrieving diff between %s and %s", + base_commit, + target_commit, + ) + + base_commit = self._resolve_commit(base_commit) + target_commit = self._resolve_commit(target_commit) + + try: + result = subprocess.run( + [ + "git", + "-C", + str(self.repository_client.get_local_path()), + "diff", + base_commit, + target_commit, + ], + check=True, + capture_output=True, + timeout=300, + ) + except subprocess.CalledProcessError as exc: + logger.error( + "Failed to retrieve diff: %s", + exc.stderr.decode("utf-8", errors="replace"), + ) + raise + + diff_bytes = result.stdout + + 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_bytes.decode("utf-8", errors="replace") + + def _resolve_commit(self, commit: str) -> str: + result = subprocess.run( + [ + "git", + "-C", + str(self.repository_client.get_local_path()), + "rev-parse", + "--verify", + "--end-of-options", + f"{commit}^{{commit}}", + ], + check=True, + capture_output=True, + text=True, + timeout=60, + ) + return result.stdout.strip() diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py index bed925d85..468be2665 100644 --- a/application/utils/harvester/git_repository_client.py +++ b/application/utils/harvester/git_repository_client.py @@ -162,6 +162,7 @@ def checkout(self, reference: str) -> None: "-C", str(self.local_path), "checkout", + "--", reference, ], check=True, diff --git a/application/utils/harvester/models.py b/application/utils/harvester/models.py index 227c0d64e..0eca718c9 100644 --- a/application/utils/harvester/models.py +++ b/application/utils/harvester/models.py @@ -25,3 +25,17 @@ class FilteringMetrics(BaseModel): total_files: int retained_files: int filtered_files: int + + +@dataclass(slots=True) +class DiffBlock: + """ + Intermediate representation of normalized additions + extracted from a repository diff. + """ + + file_path: str + added_lines: list[str] + repository: str + commit_sha: str + committed_at: datetime | None = None