diff --git a/application/tests/harvester_test/diff_pipeline_test.py b/application/tests/harvester_test/diff_pipeline_test.py index 07160f170..e5420a4ed 100644 --- a/application/tests/harvester_test/diff_pipeline_test.py +++ b/application/tests/harvester_test/diff_pipeline_test.py @@ -1,8 +1,8 @@ from datetime import UTC, datetime +import os import subprocess import time import unittest -import os from application.utils.harvester.diff_normalizer import DiffNormalizer from application.utils.harvester.diff_parser import DiffParser @@ -19,7 +19,6 @@ class DiffPipelineBenchmark(unittest.TestCase): """ def test_pipeline_benchmark(self): - if os.getenv("OPENCRE_RUN_NETWORK_TESTS") != "1": self.skipTest("Network benchmark disabled") @@ -29,9 +28,7 @@ def test_pipeline_benchmark(self): "master", ) client.sync() - head_commit = client.get_current_commit_sha() - previous_commit = subprocess.run( [ "git", @@ -51,23 +48,18 @@ def test_pipeline_benchmark(self): 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 index 502ac2d39..b587bbe33 100644 --- a/application/tests/harvester_test/diff_retriever_test.py +++ b/application/tests/harvester_test/diff_retriever_test.py @@ -16,6 +16,9 @@ def test_get_diff(self, mock_run): MagicMock(stdout="def456\n"), MagicMock(stdout=b"diff --git a/README.md b/README.md\n"), ] + mock_run.return_value = MagicMock( + stdout="diff --git a/README.md b/README.md\n", + ) client = MagicMock() client.get_local_path.return_value = "/tmp/repo" @@ -94,6 +97,35 @@ def test_large_diff_raises(self, mock_run): with self.assertRaises(ValueError): retriever.get_diff("a", "b") + mock_run.assert_called_once_with( + [ + "git", + "-C", + "/tmp/repo", + "diff", + "abc123", + "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), + ) + + 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/document_builder_test.py b/application/tests/harvester_test/document_builder_test.py new file mode 100644 index 000000000..ba2313c08 --- /dev/null +++ b/application/tests/harvester_test/document_builder_test.py @@ -0,0 +1,65 @@ +import unittest +from datetime import datetime + +from application.utils.harvester.document_builder import ( + DocumentBuilder, +) +from application.utils.harvester.models import ( + DiffBlock, +) + + +class DocumentBuilderTests(unittest.TestCase): + def test_build_document(self): + block = DiffBlock( + file_path="README.md", + repository="OWASP/ASVS", + commit_sha="abc123", + committed_at=datetime.now(), + added_lines=["Hello"], + ) + + document = DocumentBuilder().build( + block, + "# Title\n\nHello", + pipeline_run_id="20260714T120000Z", + ) + + self.assertEqual( + document.schema_version, + "0.2.0", + ) + + self.assertEqual( + document.artifact_id, + "art:OWASP/ASVS:README.md", + ) + + self.assertEqual( + document.pipeline_run_id, + "20260714T120000Z", + ) + + self.assertEqual( + document.text, + "# Title\n\nHello", + ) + + self.assertEqual( + document.source.repository, + "OWASP/ASVS", + ) + + self.assertEqual( + document.locator.path, + "README.md", + ) + + self.assertEqual( + len(document.heading_structure), + 1, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/document_validator_test.py b/application/tests/harvester_test/document_validator_test.py new file mode 100644 index 000000000..c445d7245 --- /dev/null +++ b/application/tests/harvester_test/document_validator_test.py @@ -0,0 +1,85 @@ +import unittest +from datetime import datetime + +from application.utils.harvester.document_validator import ( + DocumentValidator, +) +from application.utils.harvester.models import ( + Document, + HeadingNode, + Locator, + SourceInfo, +) + + +def make_document() -> Document: + return Document( + schema_version="0.2.0", + artifact_id="art:OWASP/ASVS:README.md", + pipeline_run_id="20260714T120000Z", + text="# Title", + source=SourceInfo( + type="github", + repository="OWASP/ASVS", + commit_sha="abc123", + committed_at=datetime.now(), + ), + locator=Locator( + kind="repo_path", + id="README.md", + path="README.md", + ), + heading_structure=[ + HeadingNode( + level=1, + text="Title", + start_line=1, + end_line=1, + ) + ], + span=None, + ) + + +class DocumentValidatorTests(unittest.TestCase): + def test_valid_document(self): + validator = DocumentValidator() + + self.assertTrue(validator.validate(make_document())) + + def test_missing_artifact_id(self): + validator = DocumentValidator() + + document = make_document() + document.artifact_id = "" + + self.assertFalse(validator.validate(document)) + + def test_missing_text(self): + validator = DocumentValidator() + + document = make_document() + document.text = "" + + self.assertFalse(validator.validate(document)) + + def test_invalid_source_type(self): + validator = DocumentValidator() + + document = make_document() + document.source.type = "gitlab" + + self.assertFalse(validator.validate(document)) + + def test_non_markdown_document_is_valid(self): + validator = DocumentValidator() + + document = make_document() + document.heading_structure = [] + document.text = '{"hello": "world"}' + + self.assertTrue(validator.validate(document)) + + +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 8bbff6ab8..d012594c9 100644 --- a/application/tests/harvester_test/git_repository_client_test.py +++ b/application/tests/harvester_test/git_repository_client_test.py @@ -4,6 +4,7 @@ import tempfile from pathlib import Path +from unittest.mock import MagicMock from application.utils.harvester.git_repository_client import ( GitRepositoryClient, ) @@ -153,6 +154,33 @@ def test_clone_runs_git_command(self, mock_run): mock_run.assert_called() + @patch("application.utils.harvester.git_repository_client.subprocess.run") + def test_get_file_at_commit(self, mock_run): + + mock_run.return_value = MagicMock(stdout="# Hello\nWorld\n") + + client = GitRepositoryClient("OWASP", "ASVS", "master") + + client.get_local_path = MagicMock(return_value="/tmp/repo") + + content = client.get_file_at_commit("abc123", "README.md") + + self.assertEqual(content, "# Hello\nWorld\n") + + mock_run.assert_called_once_with( + [ + "git", + "-C", + "/tmp/repo", + "show", + "abc123:README.md", + ], + capture_output=True, + text=True, + check=True, + timeout=30, + ) + if __name__ == "__main__": unittest.main() diff --git a/application/tests/harvester_test/heading_extractor_test.py b/application/tests/harvester_test/heading_extractor_test.py new file mode 100644 index 000000000..e1411b9cd --- /dev/null +++ b/application/tests/harvester_test/heading_extractor_test.py @@ -0,0 +1,107 @@ +import unittest + +from application.utils.harvester.heading_extractor import ( + HeadingExtractor, +) + + +class HeadingExtractorTests(unittest.TestCase): + def test_single_heading(self): + text = """ +# Title + +Hello + +World +""" + + headings = HeadingExtractor().extract(text) + + self.assertEqual(len(headings), 1) + + self.assertEqual(headings[0].text, "Title") + self.assertEqual(headings[0].level, 1) + self.assertEqual(headings[0].start_line, 2) + + def test_nested_headings(self): + text = """ +# Root + +## Child One + +content + +## Child Two + +more + +# Second Root +""" + + headings = HeadingExtractor().extract(text) + + self.assertEqual(len(headings), 4) + + self.assertEqual(headings[0].text, "Root") + self.assertEqual(headings[1].text, "Child One") + self.assertEqual(headings[2].text, "Child Two") + self.assertEqual(headings[3].text, "Second Root") + + def test_heading_ranges(self): + text = """ +# Root + +text + +## Child + +child + +# Next +""" + + headings = HeadingExtractor().extract(text) + self.assertEqual(headings[0].end_line, 9) + self.assertEqual(headings[1].end_line, 9) + self.assertEqual(headings[2].end_line, 10) + + def test_ignore_non_headings(self): + text = """ +Hello + +###Heading + +####NoSpace + +## Valid Heading +""" + + headings = HeadingExtractor().extract(text) + self.assertEqual(len(headings), 1) + self.assertEqual(headings[0].text, "Valid Heading") + + def test_heading_stops_at_same_level(self): + text = """ +# Root + +## A + +### X + +## B + + content + """ + + headings = HeadingExtractor().extract(text) + + self.assertEqual(headings[1].text, "A") + self.assertEqual(headings[2].text, "X") + self.assertEqual(headings[3].text, "B") + + self.assertEqual(headings[1].end_line, 7) + self.assertEqual(headings[2].end_line, 7) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/utils/harvester/__init__.py b/application/utils/harvester/__init__.py index 9961aae16..af2d96ded 100644 --- a/application/utils/harvester/__init__.py +++ b/application/utils/harvester/__init__.py @@ -26,16 +26,28 @@ FilteringBenchmarkResult, ) +from .heading_extractor import ( + HeadingExtractor, + HeadingNode, +) + +from .document_builder import DocumentBuilder +from .document_validator import DocumentValidator + __all__ = [ "build_repository_cache_path", "ChunkingConfig", "ConfigLoaderError", "DiffRetriever", + "DocumentBuilder", + "DocumentValidator", "GitRepositoryClient", "FileFilter", "FilteringMetricsCollector", "FilteringBenchmark", "FilteringBenchmarkResult", + "HeadingExtractor", + "HeadingNode", "PathRules", "PollingConfig", "RepositoryClient", diff --git a/application/utils/harvester/artifact_id.py b/application/utils/harvester/artifact_id.py new file mode 100644 index 000000000..fa3ed46f1 --- /dev/null +++ b/application/utils/harvester/artifact_id.py @@ -0,0 +1,11 @@ +def generate_artifact_id(repository: str, file_path: str) -> str: + """ + Generate a stable artifact identifier for a repository file. + + Example: + repository = "OWASP/ASVS" + file_path = "5.0/en/0x01-Frontispiece.md" + + -> art:OWASP/ASVS:5.0/en/0x01-Frontispiece.md + """ + return f"art:{repository}:{file_path}" diff --git a/application/utils/harvester/diff_normalizer.py b/application/utils/harvester/diff_normalizer.py index 756d89147..d3e923a01 100644 --- a/application/utils/harvester/diff_normalizer.py +++ b/application/utils/harvester/diff_normalizer.py @@ -1,6 +1,7 @@ import re import unicodedata +from application.utils.harvester import repository_client from .models import DiffBlock diff --git a/application/utils/harvester/diff_retriever.py b/application/utils/harvester/diff_retriever.py index 7efd45560..665d10d19 100644 --- a/application/utils/harvester/diff_retriever.py +++ b/application/utils/harvester/diff_retriever.py @@ -8,13 +8,10 @@ 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 @@ -35,7 +32,6 @@ def get_diff(self, base_commit: str, target_commit: str = "HEAD") -> str: Raises: subprocess.CalledProcessError: If git diff fails. - ValueError: If the diff exceeds the configured size limit. """ @@ -44,7 +40,6 @@ def get_diff(self, base_commit: str, target_commit: str = "HEAD") -> str: base_commit, target_commit, ) - base_commit = self._resolve_commit(base_commit) target_commit = self._resolve_commit(target_commit) @@ -70,9 +65,7 @@ def get_diff(self, base_commit: str, target_commit: str = "HEAD") -> str: 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 " diff --git a/application/utils/harvester/document_builder.py b/application/utils/harvester/document_builder.py new file mode 100644 index 000000000..9988c9f13 --- /dev/null +++ b/application/utils/harvester/document_builder.py @@ -0,0 +1,46 @@ +from .artifact_id import generate_artifact_id +from .heading_extractor import HeadingExtractor +from .models import ( + DiffBlock, + Document, + Locator, + SourceInfo, +) + + +class DocumentBuilder: + """ + Builds structured Document objects from parsed diffs. + + This bridges raw git diff ingestion and downstream + semantic chunking. + """ + + SCHEMA_VERSION = "0.2.0" + + def build(self, block: DiffBlock, full_text: str, pipeline_run_id: str) -> Document: + artifact_id = generate_artifact_id( + block.repository, + block.file_path, + ) + + headings = HeadingExtractor().extract(full_text) + + return Document( + schema_version=self.SCHEMA_VERSION, + artifact_id=artifact_id, + pipeline_run_id=pipeline_run_id, + text=full_text, + heading_structure=headings, + source=SourceInfo( + type="github", + repository=block.repository, + commit_sha=block.commit_sha, + committed_at=block.committed_at, + ), + locator=Locator( + kind="repo_path", + id=block.file_path, + path=block.file_path, + ), + ) diff --git a/application/utils/harvester/document_validator.py b/application/utils/harvester/document_validator.py new file mode 100644 index 000000000..9e099a4aa --- /dev/null +++ b/application/utils/harvester/document_validator.py @@ -0,0 +1,42 @@ +from .models import Document + + +class DocumentValidator: + """ + Validates structured Document objects before indexing. + + Ensures every required metadata field has been populated. + """ + + def validate(self, document: Document) -> bool: + if not document.schema_version: + return False + + if not document.artifact_id.startswith("art:"): + return False + + if not document.pipeline_run_id: + return False + + if not document.text: + return False + + if document.source.type != "github": + return False + + if not document.source.repository: + return False + + if not document.source.commit_sha: + return False + + if document.source.committed_at is None: + return False + + if document.locator.kind != "repo_path": + return False + + if not document.locator.path: + return False + + return True diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py index 468be2665..b650b74f7 100644 --- a/application/utils/harvester/git_repository_client.py +++ b/application/utils/harvester/git_repository_client.py @@ -284,3 +284,34 @@ def is_valid_repository(self, repository_path: Path) -> bool: def verify_repository_integrity(self) -> bool: return self.is_valid_repository(self.local_path) + + def get_file_at_commit(self, commit_sha: str, file_path: str) -> str: + """ + Retrieve the contents of a file at a specific commit. + + Args: + commit_sha: + Commit to read from. + + file_path: + Repository-relative file path. + + Returns: + File contents as a string. + """ + + result = subprocess.run( + [ + "git", + "-C", + str(self.get_local_path()), + "show", + f"{commit_sha}:{file_path}", + ], + capture_output=True, + text=True, + check=True, + timeout=30, + ) + + return result.stdout diff --git a/application/utils/harvester/heading_extractor.py b/application/utils/harvester/heading_extractor.py new file mode 100644 index 000000000..7941a805b --- /dev/null +++ b/application/utils/harvester/heading_extractor.py @@ -0,0 +1,47 @@ +from dataclasses import dataclass +from .models import HeadingNode + + +class HeadingExtractor: + """ + Extracts Markdown headings and their line ranges. + + Heading ranges extend until the next heading of the same + or higher level, or the end of the document. + """ + + def extract(self, text: str) -> list[HeadingNode]: + lines = text.splitlines() + + headings: list[HeadingNode] = [] + + for line_number, line in enumerate(lines, start=1): + stripped = line.lstrip() + + if not stripped.startswith("#"): + continue + + hashes = len(stripped) - len(stripped.lstrip("#")) + + if hashes == 0: + continue + + if len(stripped) > hashes and stripped[hashes] != " ": + continue + + headings.append( + HeadingNode( + level=hashes, + text=stripped[hashes:].strip(), + start_line=line_number, + end_line=len(lines), + ) + ) + + for index, heading in enumerate(headings): + for next_heading in headings[index + 1 :]: + if next_heading.level <= heading.level: + heading.end_line = next_heading.start_line - 1 + break + + return headings diff --git a/application/utils/harvester/models.py b/application/utils/harvester/models.py index 0eca718c9..b30985a0d 100644 --- a/application/utils/harvester/models.py +++ b/application/utils/harvester/models.py @@ -39,3 +39,49 @@ class DiffBlock: repository: str commit_sha: str committed_at: datetime | None = None + + +@dataclass(slots=True) +class SourceInfo: + type: str + repository: str + commit_sha: str + committed_at: datetime | None + + +@dataclass(slots=True) +class Locator: + kind: str + id: str + path: str + + +@dataclass(slots=True) +class SpanInfo: + heading_path: list[str] + start_line: int + end_line: int + index: int | None = None + total: int | None = None + start_char_idx: int | None = None + end_char_idx: int | None = None + + +@dataclass(slots=True) +class HeadingNode: + level: int + text: str + start_line: int + end_line: int + + +@dataclass(slots=True) +class Document: + schema_version: str + artifact_id: str + pipeline_run_id: str + text: str + source: SourceInfo + locator: Locator + heading_structure: list[HeadingNode] + span: SpanInfo | None = None