From aed4fe6fe4c8beb4a54b9534fae74db2418c06a8 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Wed, 27 May 2026 20:26:17 +0530 Subject: [PATCH 01/27] feat(harvester): implement repository configuration loader --- application/tests/harvester_test/__init__.py | 3 + .../fixtures/invalid_chunk_size.yaml | 17 +++ .../fixtures/invalid_missing_id.yaml | 17 +++ .../harvester_test/fixtures/invalid_yaml.yaml | 4 + .../harvester_test/fixtures/valid_repos.yaml | 16 +++ .../harvester_test/test_config_loader.py | 45 ++++++++ application/utils/harvester/__init__.py | 22 ++++ application/utils/harvester/config_loader.py | 26 +++++ application/utils/harvester/repos.yaml | 43 ++++++++ application/utils/harvester/schemas.py | 102 ++++++++++++++++++ 10 files changed, 295 insertions(+) create mode 100644 application/tests/harvester_test/__init__.py create mode 100644 application/tests/harvester_test/fixtures/invalid_chunk_size.yaml create mode 100644 application/tests/harvester_test/fixtures/invalid_missing_id.yaml create mode 100644 application/tests/harvester_test/fixtures/invalid_yaml.yaml create mode 100644 application/tests/harvester_test/fixtures/valid_repos.yaml create mode 100644 application/tests/harvester_test/test_config_loader.py create mode 100644 application/utils/harvester/__init__.py create mode 100644 application/utils/harvester/config_loader.py create mode 100644 application/utils/harvester/repos.yaml create mode 100644 application/utils/harvester/schemas.py diff --git a/application/tests/harvester_test/__init__.py b/application/tests/harvester_test/__init__.py new file mode 100644 index 000000000..06b0f7440 --- /dev/null +++ b/application/tests/harvester_test/__init__.py @@ -0,0 +1,3 @@ +""" +tests for Module A configuration layer (empty for now) +""" diff --git a/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml b/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml new file mode 100644 index 000000000..4c81538f4 --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml @@ -0,0 +1,17 @@ +repositories: + - type: github + + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/invalid_missing_id.yaml b/application/tests/harvester_test/fixtures/invalid_missing_id.yaml new file mode 100644 index 000000000..4c81538f4 --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_missing_id.yaml @@ -0,0 +1,17 @@ +repositories: + - type: github + + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/invalid_yaml.yaml b/application/tests/harvester_test/fixtures/invalid_yaml.yaml new file mode 100644 index 000000000..3f74ab21c --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_yaml.yaml @@ -0,0 +1,4 @@ +repositories: + - repository_id: owasp-top10 + source: + type github diff --git a/application/tests/harvester_test/fixtures/valid_repos.yaml b/application/tests/harvester_test/fixtures/valid_repos.yaml new file mode 100644 index 000000000..98ddf7775 --- /dev/null +++ b/application/tests/harvester_test/fixtures/valid_repos.yaml @@ -0,0 +1,16 @@ +repositories: + - id: owasp-asvs + type: github + owner: OWASP + repo: ASVS + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/test_config_loader.py b/application/tests/harvester_test/test_config_loader.py new file mode 100644 index 000000000..0a000cf0a --- /dev/null +++ b/application/tests/harvester_test/test_config_loader.py @@ -0,0 +1,45 @@ +from pathlib import Path +import pytest +from application.utils.harvester.config_loader import ( + ConfigLoaderError, + load_repo_config, +) + +FIXTURES_DIR = Path(__file__).parent / "fixtures" + + +def test_load_valid_config(): + config_path = FIXTURES_DIR / "valid_repos.yaml" + + config = load_repo_config(config_path) + + assert len(config.repositories) == 1 + + repo = config.repositories[0] + + assert repo.id == "owasp-asvs" + assert repo.owner == "OWASP" + assert repo.repo == "ASVS" + + +def test_missing_repository_id(): + config_path = FIXTURES_DIR / "invalid_missing_id.yaml" + with pytest.raises(ConfigLoaderError): + load_repo_config(config_path) + + +def test_invalid_chunk_size(): + config_path = FIXTURES_DIR / "invalid_chunk_size.yaml" + with pytest.raises(ConfigLoaderError): + load_repo_config(config_path) + + +def test_invalid_yaml_syntax(): + config_path = FIXTURES_DIR / "invalid_yaml.yaml" + with pytest.raises(ConfigLoaderError): + load_repo_config(config_path) + + +def test_missing_config_file(): + with pytest.raises(FileNotFoundError): + load_repo_config("does_not_exist.yaml") diff --git a/application/utils/harvester/__init__.py b/application/utils/harvester/__init__.py new file mode 100644 index 000000000..928af5c96 --- /dev/null +++ b/application/utils/harvester/__init__.py @@ -0,0 +1,22 @@ +from .config_loader import ( + ConfigLoaderError, + load_repo_config, +) + +from .schemas import ( + ChunkingConfig, + PathRules, + PollingConfig, + RepositoryConfig, + ReposFile, +) + +__all__ = [ + "ChunkingConfig", + "ConfigLoaderError", + "PathRules", + "PollingConfig", + "RepositoryConfig", + "ReposFile", + "load_repo_config", +] diff --git a/application/utils/harvester/config_loader.py b/application/utils/harvester/config_loader.py new file mode 100644 index 000000000..bba76026c --- /dev/null +++ b/application/utils/harvester/config_loader.py @@ -0,0 +1,26 @@ +from pathlib import Path +import yaml +from pydantic import ValidationError +from .schemas import ReposFile + + +class ConfigLoaderError(Exception): + # when repo config loading fails + pass + + +def load_repo_config(path: str | Path) -> ReposFile: + config_path = Path(path) + + if not config_path.exists(): + raise FileNotFoundError(f"Configuration file not found: {config_path}") + try: + with config_path.open("r", encoding="utf-8") as file: + raw_config = yaml.safe_load(file) + except yaml.YAMLError as exc: + raise ConfigLoaderError(f"Invalid YAML syntax in {config_path}") from exc + + try: + return ReposFile.model_validate(raw_config) + except ValidationError as exc: + raise ConfigLoaderError(f"schema validation failed for {config_path}") from exc diff --git a/application/utils/harvester/repos.yaml b/application/utils/harvester/repos.yaml new file mode 100644 index 000000000..f39d07257 --- /dev/null +++ b/application/utils/harvester/repos.yaml @@ -0,0 +1,43 @@ +repositories: + - id: owasp-asvs + type: github + enabled: true + owner: OWASP + repo: ASVS + branch: master + paths: + include: + - "4.0/en/**/*.md" + + exclude: + - "**/archive/**" + + chunking: + strategy: markdown + max_tokens: 1200 + overlap_tokens: 100 + + polling: + mode: incremental + interval_minutes: 60 + + - id: owasp-cheatsheets + type: github + enabled: true + + owner: OWASP + repo: CheatSheetSeries + branch: master + + paths: + include: + - "cheatsheets/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1000 + overlap_tokens: 100 + + polling: + mode: incremental + interval_minutes: 120 diff --git a/application/utils/harvester/schemas.py b/application/utils/harvester/schemas.py new file mode 100644 index 000000000..d1599e737 --- /dev/null +++ b/application/utils/harvester/schemas.py @@ -0,0 +1,102 @@ +# core routes: +# PathRules +# ChunkingConfig +# ollingConfig +# RepositoryConfig +# ReposFile + +from typing import Literal +from pydantic import BaseModel, Field, ConfigDict + + +# this will control which repo paths are included and excluded during ingestions +class PathRules(BaseModel): + model_config = ConfigDict(extra="forbid") + + include: list[str] = Field( + ..., + min_length=1, + description="Glob patterns to include during ingestions", + ) + exclude: list[str] = Field( + default_factory=list, description="Glob patterns to exclude during ingestions" + ) + + +# this will define how the harvested data should be chunked before downstream +class ChunkingConfig(BaseModel): + model_config = ConfigDict(extra="forbid") + + strategy: Literal["markdown", "plaintext"] = Field( + ..., + description="Chunking startergy used for text segmentation", + ) + + max_tokens: int = Field(..., gt=0, description="max toekn size per chunk") + + overlap_tokens: int = Field( # a bit concerned about this + ge=0, # this can also be = 0 i suppose + default=100, + description="token overlap between adjacent chunks", + ) + + +# this one defines repository synchronize behaviour +class PollingConfig(BaseModel): + model_config = ConfigDict(extra="forbid") + + mode: Literal["full", "incremental"] = Field( + ..., description="repository sync mode" + ) + + interval_minutes: int = Field(..., gt=0, description="polling interval in minutes") + + +# top level repository ingestion configuration +class RepositoryConfig(BaseModel): + model_config = ConfigDict(extra="forbid") + + id: str = Field( + ..., + min_length=1, + description="unique repository identifier.", + ) + + type: Literal["github"] = Field( + ..., + description="repository source type.", + ) + enabled: bool = Field( + default=True, + description="whether ingestion is enabled for this repository.", + ) + owner: str = Field( + ..., + min_length=1, + description="repository organization.", + ) + repo: str = Field( + ..., + min_length=1, + description="repository name.", + ) + branch: str = Field( + default="main", + min_length=1, + description="Repository branch to ingest.", + ) + + paths: PathRules + chunking: ChunkingConfig + polling: PollingConfig + + +# Root configuration object loaded from repos.yaml. +class ReposFile(BaseModel): + model_config = ConfigDict(extra="forbid") + + repositories: list[RepositoryConfig] = Field( + ..., + min_length=1, + description="List of repositories configured for ingestion.", + ) From f370f730c8058deb0fea54f5cad2ad4bae2c0eee Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Fri, 29 May 2026 11:49:22 +0530 Subject: [PATCH 02/27] fix(harvester): address CodeRabbit validation feedback --- .../harvester_test/fixtures/invalid_chunk_size.yaml | 5 +++-- .../tests/harvester_test/test_config_loader.py | 2 +- application/utils/harvester/config_loader.py | 6 ++++-- application/utils/harvester/schemas.py | 11 ++--------- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml b/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml index 4c81538f4..b06fdc0ad 100644 --- a/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml +++ b/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml @@ -1,6 +1,7 @@ repositories: - - type: github + - id: owasp-asvs + type: github owner: OWASP repo: ASVS @@ -10,7 +11,7 @@ repositories: chunking: strategy: markdown - max_tokens: 1200 + max_tokens: 0 polling: mode: incremental diff --git a/application/tests/harvester_test/test_config_loader.py b/application/tests/harvester_test/test_config_loader.py index 0a000cf0a..4a02ff2d0 100644 --- a/application/tests/harvester_test/test_config_loader.py +++ b/application/tests/harvester_test/test_config_loader.py @@ -30,7 +30,7 @@ def test_missing_repository_id(): def test_invalid_chunk_size(): config_path = FIXTURES_DIR / "invalid_chunk_size.yaml" - with pytest.raises(ConfigLoaderError): + with pytest.raises(ConfigLoaderError, match="max_tokens"): load_repo_config(config_path) diff --git a/application/utils/harvester/config_loader.py b/application/utils/harvester/config_loader.py index bba76026c..77dba72bb 100644 --- a/application/utils/harvester/config_loader.py +++ b/application/utils/harvester/config_loader.py @@ -12,7 +12,7 @@ class ConfigLoaderError(Exception): def load_repo_config(path: str | Path) -> ReposFile: config_path = Path(path) - if not config_path.exists(): + if not config_path.is_file(): raise FileNotFoundError(f"Configuration file not found: {config_path}") try: with config_path.open("r", encoding="utf-8") as file: @@ -23,4 +23,6 @@ def load_repo_config(path: str | Path) -> ReposFile: try: return ReposFile.model_validate(raw_config) except ValidationError as exc: - raise ConfigLoaderError(f"schema validation failed for {config_path}") from exc + raise ConfigLoaderError( + f"schema validation failed for {config_path} : {exc}" + ) from exc diff --git a/application/utils/harvester/schemas.py b/application/utils/harvester/schemas.py index d1599e737..9a94f3285 100644 --- a/application/utils/harvester/schemas.py +++ b/application/utils/harvester/schemas.py @@ -1,10 +1,3 @@ -# core routes: -# PathRules -# ChunkingConfig -# ollingConfig -# RepositoryConfig -# ReposFile - from typing import Literal from pydantic import BaseModel, Field, ConfigDict @@ -29,10 +22,10 @@ class ChunkingConfig(BaseModel): strategy: Literal["markdown", "plaintext"] = Field( ..., - description="Chunking startergy used for text segmentation", + description="Chunking strategy used for text segmentation", ) - max_tokens: int = Field(..., gt=0, description="max toekn size per chunk") + max_tokens: int = Field(..., gt=0, description="max token size per chunk") overlap_tokens: int = Field( # a bit concerned about this ge=0, # this can also be = 0 i suppose From 9478e54856ae737e79b5eefb95991dacb2d9c9f0 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 31 May 2026 14:37:17 +0530 Subject: [PATCH 03/27] feat(harvester): add repository config loader and validation --- .../fixtures/duplicate_repo_ids.yaml | 34 +++++++++++++++++++ .../fixtures/duplicate_repositories.yaml | 34 +++++++++++++++++++ .../fixtures/empty_include_paths.yaml | 16 +++++++++ .../fixtures/invalid_polling_interval.yaml | 17 ++++++++++ .../harvester_test/test_repos_validator.py | 34 +++++++++++++++++++ .../utils/harvester/repos_validator.py | 27 +++++++++++++++ 6 files changed, 162 insertions(+) create mode 100644 application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml create mode 100644 application/tests/harvester_test/fixtures/duplicate_repositories.yaml create mode 100644 application/tests/harvester_test/fixtures/empty_include_paths.yaml create mode 100644 application/tests/harvester_test/fixtures/invalid_polling_interval.yaml create mode 100644 application/tests/harvester_test/test_repos_validator.py create mode 100644 application/utils/harvester/repos_validator.py diff --git a/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml b/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml new file mode 100644 index 000000000..b9f76c2e5 --- /dev/null +++ b/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml @@ -0,0 +1,34 @@ +repositories: + - id: asvs + type: github + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 + + - id: asvs + type: github + owner: OWASP + repo: CheatSheetSeries + + paths: + include: + - "cheatsheets/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/duplicate_repositories.yaml b/application/tests/harvester_test/fixtures/duplicate_repositories.yaml new file mode 100644 index 000000000..62f019db8 --- /dev/null +++ b/application/tests/harvester_test/fixtures/duplicate_repositories.yaml @@ -0,0 +1,34 @@ +repositories: + - id: asvs-1 + type: github + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 + + - id: asvs-2 + type: github + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/empty_include_paths.yaml b/application/tests/harvester_test/fixtures/empty_include_paths.yaml new file mode 100644 index 000000000..afe23361a --- /dev/null +++ b/application/tests/harvester_test/fixtures/empty_include_paths.yaml @@ -0,0 +1,16 @@ +repositories: + - id: asvs + type: github + owner: OWASP + repo: ASVS + + paths: + include: [] + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml b/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml new file mode 100644 index 000000000..94923046c --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml @@ -0,0 +1,17 @@ +repositories: + - id: asvs + type: github + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 0 diff --git a/application/tests/harvester_test/test_repos_validator.py b/application/tests/harvester_test/test_repos_validator.py new file mode 100644 index 000000000..35bc4b8e4 --- /dev/null +++ b/application/tests/harvester_test/test_repos_validator.py @@ -0,0 +1,34 @@ +from pathlib import Path +import pytest +from application.utils.harvester.config_loader import ( + load_repo_config, +) +from application.utils.harvester.repos_validator import ( + RepositoryValidationError, + validate_repositories, +) + +FIXTURES_DIR = Path(__file__).parent / "fixtures" + + +def test_duplicate_repository_ids(): + config_path = FIXTURES_DIR / "duplicate_repo_ids.yaml" + + config = load_repo_config(config_path) + + with pytest.raises( + RepositoryValidationError, + match="Duplicate repository id", + ): + validate_repositories(config) + + +def test_duplicate_repositories(): + config_path = FIXTURES_DIR / "duplicate_repositories.yaml" + config = load_repo_config(config_path) + + with pytest.raises( + RepositoryValidationError, + match="Duplicate repository detected", + ): + validate_repositories(config) diff --git a/application/utils/harvester/repos_validator.py b/application/utils/harvester/repos_validator.py new file mode 100644 index 000000000..60eccdf0c --- /dev/null +++ b/application/utils/harvester/repos_validator.py @@ -0,0 +1,27 @@ +# as the name suggests +from application.utils.harvester.schemas import ReposFile + + +class RepositoryValidationError(Exception): + """Raised when repository configuration fails semantic validation.""" + + +def validate_repositories(config: ReposFile) -> None: + seen_ids: set[str] = set() + seen_repositories: set[tuple[str, str]] = set() + + for repository in config.repositories: + if repository.id in seen_ids: + raise RepositoryValidationError( + f"Duplicate repository id found: {repository.id}" + ) + seen_ids.add(repository.id) + repository_key = ( + repository.owner, + repository.repo, + ) + if repository_key in seen_repositories: + raise RepositoryValidationError( + f"Duplicate repository detected: {repository.owner}/{repository.repo}" + ) + seen_repositories.add(repository_key) From 43fb6cb6e2740d67447d153c43e6cc5335bdcf32 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 31 May 2026 14:55:00 +0530 Subject: [PATCH 04/27] addressing coderabbit --- application/utils/harvester/repos_validator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/application/utils/harvester/repos_validator.py b/application/utils/harvester/repos_validator.py index 60eccdf0c..74da14df3 100644 --- a/application/utils/harvester/repos_validator.py +++ b/application/utils/harvester/repos_validator.py @@ -17,8 +17,8 @@ def validate_repositories(config: ReposFile) -> None: ) seen_ids.add(repository.id) repository_key = ( - repository.owner, - repository.repo, + repository.owner.casefold(), + repository.repo.casefold(), ) if repository_key in seen_repositories: raise RepositoryValidationError( From b9622463fef447a5c82f32b2dde0f71b125c41b9 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 31 May 2026 15:38:36 +0530 Subject: [PATCH 05/27] addressing coderabbit again --- application/utils/harvester/schemas.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/application/utils/harvester/schemas.py b/application/utils/harvester/schemas.py index 9a94f3285..1c48a0c43 100644 --- a/application/utils/harvester/schemas.py +++ b/application/utils/harvester/schemas.py @@ -1,5 +1,5 @@ from typing import Literal -from pydantic import BaseModel, Field, ConfigDict +from pydantic import BaseModel, Field, ConfigDict, model_validator # this will control which repo paths are included and excluded during ingestions @@ -33,6 +33,15 @@ class ChunkingConfig(BaseModel): description="token overlap between adjacent chunks", ) + @model_validator(mode="after") + def overlap_must_be_less_than_max(self) -> "ChunkingConfig": + if self.overlap_tokens >= self.max_tokens: + raise ValueError( + f"overlap_tokens ({self.overlap_tokens}) must be less than " + f"max_tokens ({self.max_tokens})" + ) + return self + # this one defines repository synchronize behaviour class PollingConfig(BaseModel): From b556308ef5ebf1ca055c084378a061065d615041 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 31 May 2026 17:44:34 +0530 Subject: [PATCH 06/27] Add repository validation rules and fixtures --- .../fixtures/duplicate_include_paths.yaml | 22 ++++++++++++++ .../harvester_test/fixtures/empty_owner.yaml | 21 +++++++++++++ .../fixtures/invalid_chunking_strategy.yaml | 21 +++++++++++++ .../fixtures/invalid_polling_mode.yaml | 21 +++++++++++++ .../harvester_test/test_config_loader.py | 30 +++++++++++++++++++ .../harvester_test/test_repos_validator.py | 11 +++++++ .../utils/harvester/exclude_patterns.txt | 10 +++++++ .../utils/harvester/repos_validator.py | 8 ++++- 8 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 application/tests/harvester_test/fixtures/duplicate_include_paths.yaml create mode 100644 application/tests/harvester_test/fixtures/empty_owner.yaml create mode 100644 application/tests/harvester_test/fixtures/invalid_chunking_strategy.yaml create mode 100644 application/tests/harvester_test/fixtures/invalid_polling_mode.yaml create mode 100644 application/utils/harvester/exclude_patterns.txt diff --git a/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml b/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml new file mode 100644 index 000000000..7b8cfef04 --- /dev/null +++ b/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml @@ -0,0 +1,22 @@ +repositories: + - id: duplicate-includes + type: github + enabled: true + + owner: OWASP + repo: ASVS + branch: master + + paths: + include: + - "docs/**/*.md" + - "docs/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + overlap_tokens: 100 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/empty_owner.yaml b/application/tests/harvester_test/fixtures/empty_owner.yaml new file mode 100644 index 000000000..a32fe69da --- /dev/null +++ b/application/tests/harvester_test/fixtures/empty_owner.yaml @@ -0,0 +1,21 @@ +repositories: + - id: empty-owner + type: github + enabled: true + + owner: "" + repo: ASVS + branch: master + + paths: + include: + - "docs/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + overlap_tokens: 100 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/invalid_chunking_strategy.yaml b/application/tests/harvester_test/fixtures/invalid_chunking_strategy.yaml new file mode 100644 index 000000000..c4cb10905 --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_chunking_strategy.yaml @@ -0,0 +1,21 @@ +repositories: + - id: invalid-strategy + type: github + enabled: true + + owner: OWASP + repo: ASVS + branch: master + + paths: + include: + - "docs/**/*.md" + + chunking: + strategy: invalid_strategy + max_tokens: 1200 + overlap_tokens: 100 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml b/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml new file mode 100644 index 000000000..d21b643f4 --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml @@ -0,0 +1,21 @@ +repositories: + - id: invalid-polling + type: github + enabled: true + + owner: OWASP + repo: ASVS + branch: master + + paths: + include: + - "docs/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + overlap_tokens: 100 + + polling: + mode: realtime + interval_minutes: 60 diff --git a/application/tests/harvester_test/test_config_loader.py b/application/tests/harvester_test/test_config_loader.py index 4a02ff2d0..b6ca0e51a 100644 --- a/application/tests/harvester_test/test_config_loader.py +++ b/application/tests/harvester_test/test_config_loader.py @@ -43,3 +43,33 @@ def test_invalid_yaml_syntax(): def test_missing_config_file(): with pytest.raises(FileNotFoundError): load_repo_config("does_not_exist.yaml") + + +def test_empty_owner(): + config_path = FIXTURES_DIR / "empty_owner.yaml" + + with pytest.raises( + ConfigLoaderError, + match="owner", + ): + load_repo_config(config_path) + + +def test_invalid_chunking_strategy(): + config_path = FIXTURES_DIR / "invalid_chunking_strategy.yaml" + + with pytest.raises( + ConfigLoaderError, + match="strategy", + ): + load_repo_config(config_path) + + +def test_invalid_polling_mode(): + config_path = FIXTURES_DIR / "invalid_polling_mode.yaml" + + with pytest.raises( + ConfigLoaderError, + match="mode", + ): + load_repo_config(config_path) diff --git a/application/tests/harvester_test/test_repos_validator.py b/application/tests/harvester_test/test_repos_validator.py index 35bc4b8e4..517a4bbc1 100644 --- a/application/tests/harvester_test/test_repos_validator.py +++ b/application/tests/harvester_test/test_repos_validator.py @@ -32,3 +32,14 @@ def test_duplicate_repositories(): match="Duplicate repository detected", ): validate_repositories(config) + + +def test_duplicate_include_paths(): + config_path = FIXTURES_DIR / "duplicate_include_paths.yaml" + config = load_repo_config(config_path) + + with pytest.raises( + RepositoryValidationError, + match="duplicate include paths", + ): + validate_repositories(config) diff --git a/application/utils/harvester/exclude_patterns.txt b/application/utils/harvester/exclude_patterns.txt new file mode 100644 index 000000000..cd7f642f8 --- /dev/null +++ b/application/utils/harvester/exclude_patterns.txt @@ -0,0 +1,10 @@ +**/.git/** +**/node_modules/** +**/__pycache__/** +**/*.png +**/*.jpg +**/*.jpeg +**/*.svg +**/*.gif +**/*.pdf +**/archive/** diff --git a/application/utils/harvester/repos_validator.py b/application/utils/harvester/repos_validator.py index 74da14df3..2d1f21340 100644 --- a/application/utils/harvester/repos_validator.py +++ b/application/utils/harvester/repos_validator.py @@ -22,6 +22,12 @@ def validate_repositories(config: ReposFile) -> None: ) if repository_key in seen_repositories: raise RepositoryValidationError( - f"Duplicate repository detected: {repository.owner}/{repository.repo}" + f"Duplicate repository detected: " + f"{repository.owner}/{repository.repo}" ) seen_repositories.add(repository_key) + include_patterns = set(repository.paths.include) + if len(include_patterns) != len(repository.paths.include): + raise RepositoryValidationError( + f"Repository '{repository.id}' " f"has duplicate include paths" + ) From fc4b58323b22342da169f50796389dd787542b77 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 31 May 2026 18:17:43 +0530 Subject: [PATCH 07/27] normalize repository ids during validation --- application/utils/harvester/repos_validator.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/application/utils/harvester/repos_validator.py b/application/utils/harvester/repos_validator.py index 2d1f21340..7f4858771 100644 --- a/application/utils/harvester/repos_validator.py +++ b/application/utils/harvester/repos_validator.py @@ -11,23 +11,23 @@ def validate_repositories(config: ReposFile) -> None: seen_repositories: set[tuple[str, str]] = set() for repository in config.repositories: - if repository.id in seen_ids: + repo_id_key = repository.id.casefold() + if repo_id_key in seen_ids: raise RepositoryValidationError( f"Duplicate repository id found: {repository.id}" ) - seen_ids.add(repository.id) + seen_ids.add(repo_id_key) repository_key = ( repository.owner.casefold(), repository.repo.casefold(), ) if repository_key in seen_repositories: raise RepositoryValidationError( - f"Duplicate repository detected: " - f"{repository.owner}/{repository.repo}" + f"Duplicate repository detected: {repository.owner}/{repository.repo}" ) seen_repositories.add(repository_key) include_patterns = set(repository.paths.include) if len(include_patterns) != len(repository.paths.include): raise RepositoryValidationError( - f"Repository '{repository.id}' " f"has duplicate include paths" + f"Repository '{repository.id}' has duplicate include paths" ) From 713f99d75106b1a121c79c710aebfc4c2301db74 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Tue, 2 Jun 2026 09:51:15 +0530 Subject: [PATCH 08/27] Refine harvester validation exports and YAML fixtures --- .../tests/harvester_test/fixtures/invalid_yaml.yaml | 6 +++--- application/utils/harvester/__init__.py | 7 ++++++- application/utils/harvester/repos_validator.py | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/application/tests/harvester_test/fixtures/invalid_yaml.yaml b/application/tests/harvester_test/fixtures/invalid_yaml.yaml index 3f74ab21c..c60d3e23b 100644 --- a/application/tests/harvester_test/fixtures/invalid_yaml.yaml +++ b/application/tests/harvester_test/fixtures/invalid_yaml.yaml @@ -1,4 +1,4 @@ repositories: - - repository_id: owasp-top10 - source: - type github + - id: broken + paths: + include: [unclosed diff --git a/application/utils/harvester/__init__.py b/application/utils/harvester/__init__.py index 928af5c96..0be31a8a4 100644 --- a/application/utils/harvester/__init__.py +++ b/application/utils/harvester/__init__.py @@ -2,7 +2,10 @@ ConfigLoaderError, load_repo_config, ) - +from .repos_validator import ( + RepositoryValidationError, + validate_repositories, +) from .schemas import ( ChunkingConfig, PathRules, @@ -19,4 +22,6 @@ "RepositoryConfig", "ReposFile", "load_repo_config", + "RepositoryValidationError", + "validate_repositories", ] diff --git a/application/utils/harvester/repos_validator.py b/application/utils/harvester/repos_validator.py index 7f4858771..81cdc8273 100644 --- a/application/utils/harvester/repos_validator.py +++ b/application/utils/harvester/repos_validator.py @@ -1,5 +1,5 @@ # as the name suggests -from application.utils.harvester.schemas import ReposFile +from .schemas import ReposFile class RepositoryValidationError(Exception): From c7ecbf08cff5e4c1c42049eeae7e244575b59d77 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 7 Jun 2026 12:14:07 +0530 Subject: [PATCH 09/27] removed stable dev comments --- application/utils/harvester/schemas.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/application/utils/harvester/schemas.py b/application/utils/harvester/schemas.py index 1c48a0c43..fd9d7d720 100644 --- a/application/utils/harvester/schemas.py +++ b/application/utils/harvester/schemas.py @@ -27,8 +27,8 @@ class ChunkingConfig(BaseModel): max_tokens: int = Field(..., gt=0, description="max token size per chunk") - overlap_tokens: int = Field( # a bit concerned about this - ge=0, # this can also be = 0 i suppose + overlap_tokens: int = Field( + ge=0, default=100, description="token overlap between adjacent chunks", ) From 0be6df87c7642908c76dc915c48b54c30227acff Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 7 Jun 2026 12:44:40 +0530 Subject: [PATCH 10/27] Add validator success test and clean exports --- application/tests/harvester_test/test_repos_validator.py | 8 ++++++++ application/utils/harvester/__init__.py | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/application/tests/harvester_test/test_repos_validator.py b/application/tests/harvester_test/test_repos_validator.py index 517a4bbc1..8c03a9de8 100644 --- a/application/tests/harvester_test/test_repos_validator.py +++ b/application/tests/harvester_test/test_repos_validator.py @@ -43,3 +43,11 @@ def test_duplicate_include_paths(): match="duplicate include paths", ): validate_repositories(config) + + +def test_validate_valid_repositories(): + config_path = FIXTURES_DIR / "valid_repos.yaml" + + config = load_repo_config(config_path) + + validate_repositories(config) diff --git a/application/utils/harvester/__init__.py b/application/utils/harvester/__init__.py index 0be31a8a4..9c74939c7 100644 --- a/application/utils/harvester/__init__.py +++ b/application/utils/harvester/__init__.py @@ -20,8 +20,8 @@ "PathRules", "PollingConfig", "RepositoryConfig", + "RepositoryValidationError", "ReposFile", "load_repo_config", - "RepositoryValidationError", "validate_repositories", ] From c728fed6204d49958af61f0863699684f4f05d7b Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Thu, 2 Jul 2026 22:00:20 +0530 Subject: [PATCH 11/27] test(harvester): align tests with project conventions --- .../harvester_test/config_loader_test.py | 62 +++++++++++++++ .../harvester_test/repos_validator_test.py | 58 ++++++++++++++ .../harvester_test/test_config_loader.py | 75 ------------------- .../harvester_test/test_repos_validator.py | 53 ------------- 4 files changed, 120 insertions(+), 128 deletions(-) create mode 100644 application/tests/harvester_test/config_loader_test.py create mode 100644 application/tests/harvester_test/repos_validator_test.py delete mode 100644 application/tests/harvester_test/test_config_loader.py delete mode 100644 application/tests/harvester_test/test_repos_validator.py diff --git a/application/tests/harvester_test/config_loader_test.py b/application/tests/harvester_test/config_loader_test.py new file mode 100644 index 000000000..01f22ce18 --- /dev/null +++ b/application/tests/harvester_test/config_loader_test.py @@ -0,0 +1,62 @@ +from pathlib import Path +import unittest + +from application.utils.harvester.config_loader import ( + ConfigLoaderError, + load_repo_config, +) + +FIXTURES_DIR = Path(__file__).parent / "fixtures" + + +class ConfigLoaderTests(unittest.TestCase): + def test_load_valid_config(self): + config_path = FIXTURES_DIR / "valid_repos.yaml" + + config = load_repo_config(config_path) + + self.assertEqual(len(config.repositories), 1) + + repo = config.repositories[0] + + self.assertEqual(repo.id, "owasp-asvs") + self.assertEqual(repo.owner, "OWASP") + self.assertEqual(repo.repo, "ASVS") + + def test_missing_repository_id(self): + config_path = FIXTURES_DIR / "invalid_missing_id.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + def test_invalid_chunk_size(self): + config_path = FIXTURES_DIR / "invalid_chunk_size.yaml" + + with self.assertRaisesRegex(ConfigLoaderError, "max_tokens"): + load_repo_config(config_path) + + def test_invalid_yaml_syntax(self): + config_path = FIXTURES_DIR / "invalid_yaml.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + def test_missing_config_file(self): + with self.assertRaises(FileNotFoundError): + load_repo_config("does_not_exist.yaml") + + def test_invalid_polling_interval(self): + config_path = FIXTURES_DIR / "invalid_polling_interval.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + def test_empty_include_paths(self): + config_path = FIXTURES_DIR / "empty_include_paths.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/repos_validator_test.py b/application/tests/harvester_test/repos_validator_test.py new file mode 100644 index 000000000..3e0347d41 --- /dev/null +++ b/application/tests/harvester_test/repos_validator_test.py @@ -0,0 +1,58 @@ +from pathlib import Path +import unittest + +from application.utils.harvester.config_loader import ( + load_repo_config, +) +from application.utils.harvester.repos_validator import ( + RepositoryValidationError, + validate_repositories, +) + +FIXTURES_DIR = Path(__file__).parent / "fixtures" + + +class ReposValidatorTests(unittest.TestCase): + def test_duplicate_repository_ids(self): + config_path = FIXTURES_DIR / "duplicate_repo_ids.yaml" + + config = load_repo_config(config_path) + + with self.assertRaisesRegex( + RepositoryValidationError, + "Duplicate repository id", + ): + validate_repositories(config) + + def test_duplicate_repositories(self): + config_path = FIXTURES_DIR / "duplicate_repositories.yaml" + + config = load_repo_config(config_path) + + with self.assertRaisesRegex( + RepositoryValidationError, + "Duplicate repository detected", + ): + validate_repositories(config) + + def test_duplicate_include_paths(self): + config_path = FIXTURES_DIR / "duplicate_include_paths.yaml" + + config = load_repo_config(config_path) + + with self.assertRaisesRegex( + RepositoryValidationError, + "duplicate include paths", + ): + validate_repositories(config) + + def test_validate_valid_repositories(self): + config_path = FIXTURES_DIR / "valid_repos.yaml" + + config = load_repo_config(config_path) + + validate_repositories(config) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/test_config_loader.py b/application/tests/harvester_test/test_config_loader.py deleted file mode 100644 index b6ca0e51a..000000000 --- a/application/tests/harvester_test/test_config_loader.py +++ /dev/null @@ -1,75 +0,0 @@ -from pathlib import Path -import pytest -from application.utils.harvester.config_loader import ( - ConfigLoaderError, - load_repo_config, -) - -FIXTURES_DIR = Path(__file__).parent / "fixtures" - - -def test_load_valid_config(): - config_path = FIXTURES_DIR / "valid_repos.yaml" - - config = load_repo_config(config_path) - - assert len(config.repositories) == 1 - - repo = config.repositories[0] - - assert repo.id == "owasp-asvs" - assert repo.owner == "OWASP" - assert repo.repo == "ASVS" - - -def test_missing_repository_id(): - config_path = FIXTURES_DIR / "invalid_missing_id.yaml" - with pytest.raises(ConfigLoaderError): - load_repo_config(config_path) - - -def test_invalid_chunk_size(): - config_path = FIXTURES_DIR / "invalid_chunk_size.yaml" - with pytest.raises(ConfigLoaderError, match="max_tokens"): - load_repo_config(config_path) - - -def test_invalid_yaml_syntax(): - config_path = FIXTURES_DIR / "invalid_yaml.yaml" - with pytest.raises(ConfigLoaderError): - load_repo_config(config_path) - - -def test_missing_config_file(): - with pytest.raises(FileNotFoundError): - load_repo_config("does_not_exist.yaml") - - -def test_empty_owner(): - config_path = FIXTURES_DIR / "empty_owner.yaml" - - with pytest.raises( - ConfigLoaderError, - match="owner", - ): - load_repo_config(config_path) - - -def test_invalid_chunking_strategy(): - config_path = FIXTURES_DIR / "invalid_chunking_strategy.yaml" - - with pytest.raises( - ConfigLoaderError, - match="strategy", - ): - load_repo_config(config_path) - - -def test_invalid_polling_mode(): - config_path = FIXTURES_DIR / "invalid_polling_mode.yaml" - - with pytest.raises( - ConfigLoaderError, - match="mode", - ): - load_repo_config(config_path) diff --git a/application/tests/harvester_test/test_repos_validator.py b/application/tests/harvester_test/test_repos_validator.py deleted file mode 100644 index 8c03a9de8..000000000 --- a/application/tests/harvester_test/test_repos_validator.py +++ /dev/null @@ -1,53 +0,0 @@ -from pathlib import Path -import pytest -from application.utils.harvester.config_loader import ( - load_repo_config, -) -from application.utils.harvester.repos_validator import ( - RepositoryValidationError, - validate_repositories, -) - -FIXTURES_DIR = Path(__file__).parent / "fixtures" - - -def test_duplicate_repository_ids(): - config_path = FIXTURES_DIR / "duplicate_repo_ids.yaml" - - config = load_repo_config(config_path) - - with pytest.raises( - RepositoryValidationError, - match="Duplicate repository id", - ): - validate_repositories(config) - - -def test_duplicate_repositories(): - config_path = FIXTURES_DIR / "duplicate_repositories.yaml" - config = load_repo_config(config_path) - - with pytest.raises( - RepositoryValidationError, - match="Duplicate repository detected", - ): - validate_repositories(config) - - -def test_duplicate_include_paths(): - config_path = FIXTURES_DIR / "duplicate_include_paths.yaml" - config = load_repo_config(config_path) - - with pytest.raises( - RepositoryValidationError, - match="duplicate include paths", - ): - validate_repositories(config) - - -def test_validate_valid_repositories(): - config_path = FIXTURES_DIR / "valid_repos.yaml" - - config = load_repo_config(config_path) - - validate_repositories(config) From d4e7731a0d7c8328c80bcd11a6020dcb40ff5e44 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Fri, 3 Jul 2026 09:38:16 +0530 Subject: [PATCH 12/27] chore: ignore Claude and Cursor project files --- application/utils/harvester/exclude_patterns.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/application/utils/harvester/exclude_patterns.txt b/application/utils/harvester/exclude_patterns.txt index cd7f642f8..a3f890866 100644 --- a/application/utils/harvester/exclude_patterns.txt +++ b/application/utils/harvester/exclude_patterns.txt @@ -1,6 +1,8 @@ **/.git/** **/node_modules/** **/__pycache__/** +**/.claude/** +**/.cursor/** **/*.png **/*.jpg **/*.jpeg From f11f869f716dbca9b0d5ec5fd86f2729cfc4aed4 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Fri, 3 Jul 2026 14:46:42 +0530 Subject: [PATCH 13/27] refactor(harvester): address review feedback --- .../tests/harvester_test/config_loader_test.py | 3 ++- .../fixtures/duplicate_include_paths.yaml | 2 +- .../harvester_test/fixtures/duplicate_repo_ids.yaml | 4 ++-- .../fixtures/duplicate_repositories.yaml | 4 ++-- .../harvester_test/fixtures/empty_include_paths.yaml | 2 +- .../tests/harvester_test/fixtures/empty_owner.yaml | 2 +- .../harvester_test/fixtures/invalid_chunk_size.yaml | 2 +- .../harvester_test/fixtures/invalid_missing_id.yaml | 2 +- .../fixtures/invalid_polling_interval.yaml | 2 +- .../fixtures/invalid_polling_mode.yaml | 2 +- .../tests/harvester_test/fixtures/valid_repos.yaml | 2 +- application/utils/harvester/config_loader.py | 12 ++++++++---- application/utils/harvester/exclude_patterns.txt | 8 +++++++- application/utils/harvester/schemas.py | 4 ++-- 14 files changed, 31 insertions(+), 20 deletions(-) diff --git a/application/tests/harvester_test/config_loader_test.py b/application/tests/harvester_test/config_loader_test.py index 01f22ce18..01c404f18 100644 --- a/application/tests/harvester_test/config_loader_test.py +++ b/application/tests/harvester_test/config_loader_test.py @@ -3,6 +3,7 @@ from application.utils.harvester.config_loader import ( ConfigLoaderError, + ConfigFileNotFoundError, load_repo_config, ) @@ -42,7 +43,7 @@ def test_invalid_yaml_syntax(self): load_repo_config(config_path) def test_missing_config_file(self): - with self.assertRaises(FileNotFoundError): + with self.assertRaises(ConfigFileNotFoundError): load_repo_config("does_not_exist.yaml") def test_invalid_polling_interval(self): diff --git a/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml b/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml index 7b8cfef04..5db642199 100644 --- a/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml +++ b/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml @@ -13,7 +13,7 @@ repositories: - "docs/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 overlap_tokens: 100 diff --git a/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml b/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml index b9f76c2e5..0cc4c1c1e 100644 --- a/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml +++ b/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml @@ -9,7 +9,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: @@ -26,7 +26,7 @@ repositories: - "cheatsheets/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: diff --git a/application/tests/harvester_test/fixtures/duplicate_repositories.yaml b/application/tests/harvester_test/fixtures/duplicate_repositories.yaml index 62f019db8..41614d39c 100644 --- a/application/tests/harvester_test/fixtures/duplicate_repositories.yaml +++ b/application/tests/harvester_test/fixtures/duplicate_repositories.yaml @@ -9,7 +9,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: @@ -26,7 +26,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: diff --git a/application/tests/harvester_test/fixtures/empty_include_paths.yaml b/application/tests/harvester_test/fixtures/empty_include_paths.yaml index afe23361a..abe0bab41 100644 --- a/application/tests/harvester_test/fixtures/empty_include_paths.yaml +++ b/application/tests/harvester_test/fixtures/empty_include_paths.yaml @@ -8,7 +8,7 @@ repositories: include: [] chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: diff --git a/application/tests/harvester_test/fixtures/empty_owner.yaml b/application/tests/harvester_test/fixtures/empty_owner.yaml index a32fe69da..8063a3f1b 100644 --- a/application/tests/harvester_test/fixtures/empty_owner.yaml +++ b/application/tests/harvester_test/fixtures/empty_owner.yaml @@ -12,7 +12,7 @@ repositories: - "docs/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 overlap_tokens: 100 diff --git a/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml b/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml index b06fdc0ad..79d4cc8f0 100644 --- a/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml +++ b/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml @@ -10,7 +10,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 0 polling: diff --git a/application/tests/harvester_test/fixtures/invalid_missing_id.yaml b/application/tests/harvester_test/fixtures/invalid_missing_id.yaml index 4c81538f4..102e675b9 100644 --- a/application/tests/harvester_test/fixtures/invalid_missing_id.yaml +++ b/application/tests/harvester_test/fixtures/invalid_missing_id.yaml @@ -9,7 +9,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: diff --git a/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml b/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml index 94923046c..3f34f8baa 100644 --- a/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml +++ b/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml @@ -9,7 +9,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: diff --git a/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml b/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml index d21b643f4..0aead47ad 100644 --- a/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml +++ b/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml @@ -12,7 +12,7 @@ repositories: - "docs/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 overlap_tokens: 100 diff --git a/application/tests/harvester_test/fixtures/valid_repos.yaml b/application/tests/harvester_test/fixtures/valid_repos.yaml index 98ddf7775..fe52c8430 100644 --- a/application/tests/harvester_test/fixtures/valid_repos.yaml +++ b/application/tests/harvester_test/fixtures/valid_repos.yaml @@ -8,7 +8,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: diff --git a/application/utils/harvester/config_loader.py b/application/utils/harvester/config_loader.py index 77dba72bb..db0431e11 100644 --- a/application/utils/harvester/config_loader.py +++ b/application/utils/harvester/config_loader.py @@ -5,15 +5,19 @@ class ConfigLoaderError(Exception): - # when repo config loading fails - pass + """Base class for configuration loading errors.""" + + +class ConfigFileNotFoundError(ConfigLoaderError): + """Raised when the configuration file cannot be found.""" def load_repo_config(path: str | Path) -> ReposFile: config_path = Path(path) if not config_path.is_file(): - raise FileNotFoundError(f"Configuration file not found: {config_path}") + raise ConfigFileNotFoundError(f"Configuration file not found: {config_path}") + try: with config_path.open("r", encoding="utf-8") as file: raw_config = yaml.safe_load(file) @@ -24,5 +28,5 @@ def load_repo_config(path: str | Path) -> ReposFile: return ReposFile.model_validate(raw_config) except ValidationError as exc: raise ConfigLoaderError( - f"schema validation failed for {config_path} : {exc}" + f"Schema validation failed for {config_path}: {exc}" ) from exc diff --git a/application/utils/harvester/exclude_patterns.txt b/application/utils/harvester/exclude_patterns.txt index a3f890866..499850ae8 100644 --- a/application/utils/harvester/exclude_patterns.txt +++ b/application/utils/harvester/exclude_patterns.txt @@ -1,4 +1,10 @@ -**/.git/** +# Placeholder for repository-level exclude patterns. + +# This file will be consumed by the Week 4 noise-reduction pipeline + +# to filter non-documentation files during harvesting. + +**/.git/* **/node_modules/** **/__pycache__/** **/.claude/** diff --git a/application/utils/harvester/schemas.py b/application/utils/harvester/schemas.py index fd9d7d720..c65d62b60 100644 --- a/application/utils/harvester/schemas.py +++ b/application/utils/harvester/schemas.py @@ -20,7 +20,7 @@ class PathRules(BaseModel): class ChunkingConfig(BaseModel): model_config = ConfigDict(extra="forbid") - strategy: Literal["markdown", "plaintext"] = Field( + strategy: Literal["markdown_heading", "html_readability", "fixed_size"] = Field( ..., description="Chunking strategy used for text segmentation", ) @@ -29,7 +29,7 @@ class ChunkingConfig(BaseModel): overlap_tokens: int = Field( ge=0, - default=100, + default=20, description="token overlap between adjacent chunks", ) From fbc6e20e930c083ebed4491c04027170404301f1 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Wed, 10 Jun 2026 12:23:33 +0530 Subject: [PATCH 14/27] Add repository synchronization foundation --- .../test_git_repository_client.py | 39 ++++++ .../harvester_test/test_repository_cache.py | 12 ++ .../utils/harvester/git_repository_client.py | 116 ++++++++++++++++++ .../utils/harvester/repository_cache.py | 7 ++ .../utils/harvester/repository_client.py | 24 ++++ 5 files changed, 198 insertions(+) create mode 100644 application/tests/harvester_test/test_git_repository_client.py create mode 100644 application/tests/harvester_test/test_repository_cache.py create mode 100644 application/utils/harvester/git_repository_client.py create mode 100644 application/utils/harvester/repository_cache.py create mode 100644 application/utils/harvester/repository_client.py diff --git a/application/tests/harvester_test/test_git_repository_client.py b/application/tests/harvester_test/test_git_repository_client.py new file mode 100644 index 000000000..b0aa7678d --- /dev/null +++ b/application/tests/harvester_test/test_git_repository_client.py @@ -0,0 +1,39 @@ +from application.utils.harvester.git_repository_client import ( + GitRepositoryClient, +) + + +def test_repository_url_generation(): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + assert client.repository_url == "https://github.com/OWASP/ASVS.git" + + +def test_local_repository_path(): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + assert str(client.get_local_path()) == ".harvester_cache/owasp/asvs" + + +def test_repository_exists_locally_false(): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + assert client.exists_locally() is False + + +def test_verify_repository_integrity_false(): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + assert client.verify_repository_integrity() is False diff --git a/application/tests/harvester_test/test_repository_cache.py b/application/tests/harvester_test/test_repository_cache.py new file mode 100644 index 000000000..5cf4b745f --- /dev/null +++ b/application/tests/harvester_test/test_repository_cache.py @@ -0,0 +1,12 @@ +from application.utils.harvester.repository_cache import ( + build_repository_cache_path, +) + + +def test_build_repository_cache_path(): + path = build_repository_cache_path( + "OWASP", + "ASVS", + ) + + assert str(path) == ".harvester_cache/owasp/asvs" diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py new file mode 100644 index 000000000..de97836ed --- /dev/null +++ b/application/utils/harvester/git_repository_client.py @@ -0,0 +1,116 @@ +import subprocess +from pathlib import Path + +from .repository_cache import build_repository_cache_path +from .repository_client import RepositoryClient +import logging + +logger = logging.getLogger(__name__) + + +class GitRepositoryClient(RepositoryClient): + def __init__(self, owner: str, repository: str, branch: str = "main") -> None: + self.owner = owner + self.repository = repository + self.branch = branch + + self.local_path = build_repository_cache_path( + owner, + repository, + ) + + @property + def repository_url(self) -> str: + return f"https://github.com/{self.owner}/{self.repository}.git" + + def clone(self) -> None: + logger.info( + "Cloning repository %s/%s", + self.owner, + self.repository, + ) + self.local_path.parent.mkdir(parents=True, exist_ok=True) + + subprocess.run( + [ + "git", + "clone", + "--branch", + self.branch, + self.repository_url, + str(self.local_path), + ], + check=True, + ) + + def fetch(self) -> None: + logger.info( + "Fetching repository %s/%s", + self.owner, + self.repository, + ) + + subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "fetch", + "--all", + ], + check=True, + ) + + def checkout(self, reference: str) -> None: + subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "checkout", + reference, + ], + check=True, + ) + + def get_local_path(self) -> Path: + return self.local_path + + def exists_locally(self) -> bool: + return self.local_path.exists() + + def sync(self) -> None: + logger.info( + "Synchronizing repository %s/%s", + self.owner, + self.repository, + ) + if self.exists_locally(): + self.fetch() + else: + self.clone() + + def get_current_commit_sha(self) -> str: + result = subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "rev-parse", + "HEAD", + ], + capture_output=True, + text=True, + check=True, + ) + + return result.stdout.strip() + + def verify_repository_integrity(self) -> bool: + git_directory = self.local_path / ".git" + + return ( + self.local_path.exists() + and self.local_path.is_dir() + and git_directory.exists() + ) diff --git a/application/utils/harvester/repository_cache.py b/application/utils/harvester/repository_cache.py new file mode 100644 index 000000000..f2ee3343e --- /dev/null +++ b/application/utils/harvester/repository_cache.py @@ -0,0 +1,7 @@ +from pathlib import Path + +CACHE_ROOT = Path(".harvester_cache") + + +def build_repository_cache_path(owner: str, repository: str) -> Path: + return CACHE_ROOT / owner.casefold() / repository.casefold() diff --git a/application/utils/harvester/repository_client.py b/application/utils/harvester/repository_client.py new file mode 100644 index 000000000..6e6df5439 --- /dev/null +++ b/application/utils/harvester/repository_client.py @@ -0,0 +1,24 @@ +from abc import ABC, abstractmethod +from pathlib import Path + + +class RepositoryClient(ABC): + @abstractmethod + def clone(self) -> None: + """Clone repository locally.""" + + @abstractmethod + def fetch(self) -> None: + """Fetch latest remote changes.""" + + @abstractmethod + def checkout(self, reference: str) -> None: + """Checkout repository reference.""" + + @abstractmethod + def get_local_path(self) -> Path: + """Return local repository path.""" + + @abstractmethod + def exists_locally(self) -> bool: + """Check if repository already exists locally.""" From 29fec6aa652f43540f758d7a705f4756f61c4a7d Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Wed, 10 Jun 2026 16:03:50 +0530 Subject: [PATCH 15/27] Addressing coderabbit and adding couple of extra gaurdrails --- .../test_git_repository_client.py | 81 ++++++++++++++++++- .../harvester_test/test_repository_cache.py | 18 ++++- application/utils/harvester/__init__.py | 7 ++ .../utils/harvester/git_repository_client.py | 37 ++++++++- .../utils/harvester/repository_cache.py | 11 ++- .../utils/harvester/repository_client.py | 19 ++++- 6 files changed, 162 insertions(+), 11 deletions(-) diff --git a/application/tests/harvester_test/test_git_repository_client.py b/application/tests/harvester_test/test_git_repository_client.py index b0aa7678d..1f622fb39 100644 --- a/application/tests/harvester_test/test_git_repository_client.py +++ b/application/tests/harvester_test/test_git_repository_client.py @@ -2,6 +2,8 @@ GitRepositoryClient, ) +from unittest.mock import patch + def test_repository_url_generation(): client = GitRepositoryClient( @@ -18,7 +20,7 @@ def test_local_repository_path(): repository="ASVS", ) - assert str(client.get_local_path()) == ".harvester_cache/owasp/asvs" + assert str(client.get_local_path()) == ".harvester_cache/owasp/asvs/main" def test_repository_exists_locally_false(): @@ -37,3 +39,80 @@ def test_verify_repository_integrity_false(): ) assert client.verify_repository_integrity() is False + + +def test_sync_clones_when_repository_missing(): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + with ( + patch.object( + client, + "verify_repository_integrity", + return_value=False, + ), + patch.object(client, "clone") as mock_clone, + ): + client.sync() + + mock_clone.assert_called_once() + + +def test_sync_fetches_when_repository_exists(): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + with ( + patch.object( + client, + "verify_repository_integrity", + return_value=True, + ), + patch.object(client, "fetch") as mock_fetch, + ): + client.sync() + + mock_fetch.assert_called_once() + + +@patch("application.utils.harvester.git_repository_client.subprocess.run") +def test_fetch_runs_git_command(mock_run): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + client.fetch() + + mock_run.assert_called_once() + + +@patch("application.utils.harvester.git_repository_client.subprocess.run") +def test_checkout_runs_git_command(mock_run): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + client.checkout("main") + + mock_run.assert_called_once() + + +@patch("application.utils.harvester.git_repository_client.subprocess.run") +def test_get_current_commit_sha_runs_git_command(mock_run): + mock_run.return_value.stdout = "abc123\n" + + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + sha = client.get_current_commit_sha() + + assert sha == "abc123" + mock_run.assert_called_once() diff --git a/application/tests/harvester_test/test_repository_cache.py b/application/tests/harvester_test/test_repository_cache.py index 5cf4b745f..4a33601d9 100644 --- a/application/tests/harvester_test/test_repository_cache.py +++ b/application/tests/harvester_test/test_repository_cache.py @@ -9,4 +9,20 @@ def test_build_repository_cache_path(): "ASVS", ) - assert str(path) == ".harvester_cache/owasp/asvs" + assert str(path) == ".harvester_cache/owasp/asvs/main" + + +def test_different_branches_have_different_cache_paths(): + main_path = build_repository_cache_path( + owner="OWASP", + repository="ASVS", + branch="main", + ) + + dev_path = build_repository_cache_path( + owner="OWASP", + repository="ASVS", + branch="dev", + ) + + assert main_path != dev_path diff --git a/application/utils/harvester/__init__.py b/application/utils/harvester/__init__.py index 9c74939c7..2ac608b3e 100644 --- a/application/utils/harvester/__init__.py +++ b/application/utils/harvester/__init__.py @@ -14,11 +14,18 @@ ReposFile, ) +from .git_repository_client import GitRepositoryClient +from .repository_client import RepositoryClient +from .repository_cache import build_repository_cache_path + __all__ = [ + "build_repository_cache_path", "ChunkingConfig", "ConfigLoaderError", + "GitRepositoryClient", "PathRules", "PollingConfig", + "RepositoryClient", "RepositoryConfig", "RepositoryValidationError", "ReposFile", diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py index de97836ed..9e6584efc 100644 --- a/application/utils/harvester/git_repository_client.py +++ b/application/utils/harvester/git_repository_client.py @@ -17,6 +17,7 @@ def __init__(self, owner: str, repository: str, branch: str = "main") -> None: self.local_path = build_repository_cache_path( owner, repository, + branch, ) @property @@ -24,12 +25,24 @@ def repository_url(self) -> str: return f"https://github.com/{self.owner}/{self.repository}.git" def clone(self) -> None: + if self.exists_locally(): + logger.warning( + "Repository %s/%s already exists locally", + self.owner, + self.repository, + ) + return + logger.info( "Cloning repository %s/%s", self.owner, self.repository, ) - self.local_path.parent.mkdir(parents=True, exist_ok=True) + + self.local_path.parent.mkdir( + parents=True, + exist_ok=True, + ) subprocess.run( [ @@ -41,6 +54,9 @@ def clone(self) -> None: str(self.local_path), ], check=True, + capture_output=True, + text=True, + timeout=300, ) def fetch(self) -> None: @@ -59,9 +75,19 @@ def fetch(self) -> None: "--all", ], check=True, + capture_output=True, + text=True, + timeout=300, ) def checkout(self, reference: str) -> None: + logger.info( + "Checking out %s in %s/%s", + reference, + self.owner, + self.repository, + ) + subprocess.run( [ "git", @@ -71,6 +97,9 @@ def checkout(self, reference: str) -> None: reference, ], check=True, + capture_output=True, + text=True, + timeout=300, ) def get_local_path(self) -> Path: @@ -85,7 +114,8 @@ def sync(self) -> None: self.owner, self.repository, ) - if self.exists_locally(): + + if self.verify_repository_integrity(): self.fetch() else: self.clone() @@ -99,9 +129,10 @@ def get_current_commit_sha(self) -> str: "rev-parse", "HEAD", ], + check=True, capture_output=True, text=True, - check=True, + timeout=300, ) return result.stdout.strip() diff --git a/application/utils/harvester/repository_cache.py b/application/utils/harvester/repository_cache.py index f2ee3343e..418425681 100644 --- a/application/utils/harvester/repository_cache.py +++ b/application/utils/harvester/repository_cache.py @@ -1,7 +1,12 @@ +import os from pathlib import Path -CACHE_ROOT = Path(".harvester_cache") +CACHE_ROOT = Path(os.getenv("HARVESTER_CACHE_DIR", ".harvester_cache")) -def build_repository_cache_path(owner: str, repository: str) -> Path: - return CACHE_ROOT / owner.casefold() / repository.casefold() +def build_repository_cache_path( + owner: str, + repository: str, + branch: str = "main", +) -> Path: + return CACHE_ROOT / owner.casefold() / repository.casefold() / branch.casefold() diff --git a/application/utils/harvester/repository_client.py b/application/utils/harvester/repository_client.py index 6e6df5439..22ee67700 100644 --- a/application/utils/harvester/repository_client.py +++ b/application/utils/harvester/repository_client.py @@ -9,11 +9,11 @@ def clone(self) -> None: @abstractmethod def fetch(self) -> None: - """Fetch latest remote changes.""" + """Fetch latest repository changes.""" @abstractmethod def checkout(self, reference: str) -> None: - """Checkout repository reference.""" + """Checkout a branch, tag, or commit.""" @abstractmethod def get_local_path(self) -> Path: @@ -21,4 +21,17 @@ def get_local_path(self) -> Path: @abstractmethod def exists_locally(self) -> bool: - """Check if repository already exists locally.""" + """Return whether repository exists locally.""" + + @abstractmethod + def sync(self) -> None: + """Clone if missing, otherwise fetch latest changes.""" + + @abstractmethod + def get_current_commit_sha(self) -> str: + """Return HEAD commit SHA.""" + + @abstractmethod + def verify_repository_integrity(self) -> bool: + """Verify local repository integrity.""" + From 278302315ba8bc228884e644be1c347bfe8fc702 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Wed, 10 Jun 2026 16:24:25 +0530 Subject: [PATCH 16/27] Addressing coderabbit final --- .../test_git_repository_client.py | 17 +++ .../utils/harvester/git_repository_client.py | 143 +++++++++++------- .../utils/harvester/repository_client.py | 1 - 3 files changed, 107 insertions(+), 54 deletions(-) diff --git a/application/tests/harvester_test/test_git_repository_client.py b/application/tests/harvester_test/test_git_repository_client.py index 1f622fb39..151272345 100644 --- a/application/tests/harvester_test/test_git_repository_client.py +++ b/application/tests/harvester_test/test_git_repository_client.py @@ -116,3 +116,20 @@ def test_get_current_commit_sha_runs_git_command(mock_run): assert sha == "abc123" mock_run.assert_called_once() + + +@patch("application.utils.harvester.git_repository_client.subprocess.run") +def test_clone_runs_git_command(mock_run): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + with patch.object( + client, + "verify_repository_integrity", + return_value=False, + ): + client.clone() + + mock_run.assert_called_once() diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py index 9e6584efc..793fb5535 100644 --- a/application/utils/harvester/git_repository_client.py +++ b/application/utils/harvester/git_repository_client.py @@ -25,7 +25,7 @@ def repository_url(self) -> str: return f"https://github.com/{self.owner}/{self.repository}.git" def clone(self) -> None: - if self.exists_locally(): + if self.verify_repository_integrity(): logger.warning( "Repository %s/%s already exists locally", self.owner, @@ -44,20 +44,29 @@ def clone(self) -> None: exist_ok=True, ) - subprocess.run( - [ - "git", - "clone", - "--branch", - self.branch, - self.repository_url, - str(self.local_path), - ], - check=True, - capture_output=True, - text=True, - timeout=300, - ) + try: + subprocess.run( + [ + "git", + "clone", + "--branch", + self.branch, + self.repository_url, + str(self.local_path), + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) + except subprocess.CalledProcessError as exc: + logger.error( + "Failed to clone repository %s/%s: %s", + self.owner, + self.repository, + exc.stderr, + ) + raise def fetch(self) -> None: logger.info( @@ -66,19 +75,28 @@ def fetch(self) -> None: self.repository, ) - subprocess.run( - [ - "git", - "-C", - str(self.local_path), - "fetch", - "--all", - ], - check=True, - capture_output=True, - text=True, - timeout=300, - ) + try: + subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "fetch", + "--all", + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) + except subprocess.CalledProcessError as exc: + logger.error( + "Failed to fetch repository %s/%s: %s", + self.owner, + self.repository, + exc.stderr, + ) + raise def checkout(self, reference: str) -> None: logger.info( @@ -88,19 +106,29 @@ def checkout(self, reference: str) -> None: self.repository, ) - subprocess.run( - [ - "git", - "-C", - str(self.local_path), - "checkout", + try: + subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "checkout", + reference, + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) + except subprocess.CalledProcessError as exc: + logger.error( + "Failed to checkout %s in %s/%s: %s", reference, - ], - check=True, - capture_output=True, - text=True, - timeout=300, - ) + self.owner, + self.repository, + exc.stderr, + ) + raise def get_local_path(self) -> Path: return self.local_path @@ -121,19 +149,28 @@ def sync(self) -> None: self.clone() def get_current_commit_sha(self) -> str: - result = subprocess.run( - [ - "git", - "-C", - str(self.local_path), - "rev-parse", - "HEAD", - ], - check=True, - capture_output=True, - text=True, - timeout=300, - ) + try: + result = subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "rev-parse", + "HEAD", + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) + except subprocess.CalledProcessError as exc: + logger.error( + "Failed to retrieve commit SHA for %s/%s: %s", + self.owner, + self.repository, + exc.stderr, + ) + raise return result.stdout.strip() diff --git a/application/utils/harvester/repository_client.py b/application/utils/harvester/repository_client.py index 22ee67700..c545cce42 100644 --- a/application/utils/harvester/repository_client.py +++ b/application/utils/harvester/repository_client.py @@ -34,4 +34,3 @@ def get_current_commit_sha(self) -> str: @abstractmethod def verify_repository_integrity(self) -> bool: """Verify local repository integrity.""" - From 721afa8b194ff2b4512cf02b4011887e09845d89 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Mon, 6 Jul 2026 17:17:21 +0530 Subject: [PATCH 17/27] test(harvester): align week 2 tests with project conventions --- .../git_repository_client_test.py | 138 ++++++++++++++++++ .../harvester_test/repository_cache_test.py | 37 +++++ .../test_git_repository_client.py | 135 ----------------- .../harvester_test/test_repository_cache.py | 28 ---- 4 files changed, 175 insertions(+), 163 deletions(-) create mode 100644 application/tests/harvester_test/git_repository_client_test.py create mode 100644 application/tests/harvester_test/repository_cache_test.py delete mode 100644 application/tests/harvester_test/test_git_repository_client.py delete mode 100644 application/tests/harvester_test/test_repository_cache.py diff --git a/application/tests/harvester_test/git_repository_client_test.py b/application/tests/harvester_test/git_repository_client_test.py new file mode 100644 index 000000000..4f548a8be --- /dev/null +++ b/application/tests/harvester_test/git_repository_client_test.py @@ -0,0 +1,138 @@ +import unittest +from unittest.mock import patch + +from application.utils.harvester.git_repository_client import ( + GitRepositoryClient, +) + + +class GitRepositoryClientTests(unittest.TestCase): + def test_repository_url_generation(self): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + self.assertEqual( + client.repository_url, + "https://github.com/OWASP/ASVS.git", + ) + + def test_local_repository_path(self): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + self.assertEqual( + str(client.get_local_path()), + ".harvester_cache/owasp/asvs/main", + ) + + def test_repository_exists_locally_false(self): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + self.assertFalse(client.exists_locally()) + + def test_verify_repository_integrity_false(self): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + self.assertFalse(client.verify_repository_integrity()) + + def test_sync_clones_when_repository_missing(self): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + with ( + patch.object( + client, + "verify_repository_integrity", + return_value=False, + ), + patch.object(client, "clone") as mock_clone, + ): + client.sync() + + mock_clone.assert_called_once() + + def test_sync_fetches_when_repository_exists(self): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + with ( + patch.object( + client, + "verify_repository_integrity", + return_value=True, + ), + patch.object(client, "fetch") as mock_fetch, + ): + client.sync() + + mock_fetch.assert_called_once() + + @patch("application.utils.harvester.git_repository_client.subprocess.run") + def test_fetch_runs_git_command(self, mock_run): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + client.fetch() + + mock_run.assert_called_once() + + @patch("application.utils.harvester.git_repository_client.subprocess.run") + def test_checkout_runs_git_command(self, mock_run): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + client.checkout("main") + + mock_run.assert_called_once() + + @patch("application.utils.harvester.git_repository_client.subprocess.run") + def test_get_current_commit_sha_runs_git_command(self, mock_run): + mock_run.return_value.stdout = "abc123\n" + + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + sha = client.get_current_commit_sha() + + self.assertEqual(sha, "abc123") + mock_run.assert_called_once() + + @patch("application.utils.harvester.git_repository_client.subprocess.run") + def test_clone_runs_git_command(self, mock_run): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + with patch.object( + client, + "verify_repository_integrity", + return_value=False, + ): + client.clone() + + mock_run.assert_called_once() + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/repository_cache_test.py b/application/tests/harvester_test/repository_cache_test.py new file mode 100644 index 000000000..547a4ca23 --- /dev/null +++ b/application/tests/harvester_test/repository_cache_test.py @@ -0,0 +1,37 @@ +import unittest + +from application.utils.harvester.repository_cache import ( + build_repository_cache_path, +) + + +class RepositoryCacheTests(unittest.TestCase): + def test_build_repository_cache_path(self): + path = build_repository_cache_path( + "OWASP", + "ASVS", + ) + + self.assertEqual( + str(path), + ".harvester_cache/owasp/asvs/main", + ) + + def test_different_branches_have_different_cache_paths(self): + main_path = build_repository_cache_path( + owner="OWASP", + repository="ASVS", + branch="main", + ) + + dev_path = build_repository_cache_path( + owner="OWASP", + repository="ASVS", + branch="dev", + ) + + self.assertNotEqual(main_path, dev_path) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/test_git_repository_client.py b/application/tests/harvester_test/test_git_repository_client.py deleted file mode 100644 index 151272345..000000000 --- a/application/tests/harvester_test/test_git_repository_client.py +++ /dev/null @@ -1,135 +0,0 @@ -from application.utils.harvester.git_repository_client import ( - GitRepositoryClient, -) - -from unittest.mock import patch - - -def test_repository_url_generation(): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - assert client.repository_url == "https://github.com/OWASP/ASVS.git" - - -def test_local_repository_path(): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - assert str(client.get_local_path()) == ".harvester_cache/owasp/asvs/main" - - -def test_repository_exists_locally_false(): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - assert client.exists_locally() is False - - -def test_verify_repository_integrity_false(): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - assert client.verify_repository_integrity() is False - - -def test_sync_clones_when_repository_missing(): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - with ( - patch.object( - client, - "verify_repository_integrity", - return_value=False, - ), - patch.object(client, "clone") as mock_clone, - ): - client.sync() - - mock_clone.assert_called_once() - - -def test_sync_fetches_when_repository_exists(): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - with ( - patch.object( - client, - "verify_repository_integrity", - return_value=True, - ), - patch.object(client, "fetch") as mock_fetch, - ): - client.sync() - - mock_fetch.assert_called_once() - - -@patch("application.utils.harvester.git_repository_client.subprocess.run") -def test_fetch_runs_git_command(mock_run): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - client.fetch() - - mock_run.assert_called_once() - - -@patch("application.utils.harvester.git_repository_client.subprocess.run") -def test_checkout_runs_git_command(mock_run): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - client.checkout("main") - - mock_run.assert_called_once() - - -@patch("application.utils.harvester.git_repository_client.subprocess.run") -def test_get_current_commit_sha_runs_git_command(mock_run): - mock_run.return_value.stdout = "abc123\n" - - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - sha = client.get_current_commit_sha() - - assert sha == "abc123" - mock_run.assert_called_once() - - -@patch("application.utils.harvester.git_repository_client.subprocess.run") -def test_clone_runs_git_command(mock_run): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - with patch.object( - client, - "verify_repository_integrity", - return_value=False, - ): - client.clone() - - mock_run.assert_called_once() diff --git a/application/tests/harvester_test/test_repository_cache.py b/application/tests/harvester_test/test_repository_cache.py deleted file mode 100644 index 4a33601d9..000000000 --- a/application/tests/harvester_test/test_repository_cache.py +++ /dev/null @@ -1,28 +0,0 @@ -from application.utils.harvester.repository_cache import ( - build_repository_cache_path, -) - - -def test_build_repository_cache_path(): - path = build_repository_cache_path( - "OWASP", - "ASVS", - ) - - assert str(path) == ".harvester_cache/owasp/asvs/main" - - -def test_different_branches_have_different_cache_paths(): - main_path = build_repository_cache_path( - owner="OWASP", - repository="ASVS", - branch="main", - ) - - dev_path = build_repository_cache_path( - owner="OWASP", - repository="ASVS", - branch="dev", - ) - - assert main_path != dev_path From 1738cb429141169f6605f256864ecd41386fd1c8 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sat, 18 Jul 2026 18:01:08 +0530 Subject: [PATCH 18/27] fix(harvester): address repository client review feedback --- application/tests/harvester_test/repository_cache_test.py | 5 +++-- application/utils/harvester/git_repository_client.py | 1 + application/utils/harvester/repos.yaml | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/application/tests/harvester_test/repository_cache_test.py b/application/tests/harvester_test/repository_cache_test.py index 547a4ca23..a308239f1 100644 --- a/application/tests/harvester_test/repository_cache_test.py +++ b/application/tests/harvester_test/repository_cache_test.py @@ -1,4 +1,5 @@ import unittest +from pathlib import Path from application.utils.harvester.repository_cache import ( build_repository_cache_path, @@ -13,8 +14,8 @@ def test_build_repository_cache_path(self): ) self.assertEqual( - str(path), - ".harvester_cache/owasp/asvs/main", + path, + Path(".harvester_cache/owasp/asvs/main"), ) def test_different_branches_have_different_cache_paths(self): diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py index 793fb5535..eff7c17d6 100644 --- a/application/utils/harvester/git_repository_client.py +++ b/application/utils/harvester/git_repository_client.py @@ -113,6 +113,7 @@ def checkout(self, reference: str) -> None: "-C", str(self.local_path), "checkout", + "--", reference, ], check=True, diff --git a/application/utils/harvester/repos.yaml b/application/utils/harvester/repos.yaml index f39d07257..c448e5111 100644 --- a/application/utils/harvester/repos.yaml +++ b/application/utils/harvester/repos.yaml @@ -13,7 +13,7 @@ repositories: - "**/archive/**" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 overlap_tokens: 100 @@ -34,7 +34,7 @@ repositories: - "cheatsheets/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1000 overlap_tokens: 100 From fee277f41513954cf3c6d7931bab2f43dbb3e543 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Wed, 8 Jul 2026 13:02:05 +0530 Subject: [PATCH 19/27] feat(harvester): implement incremental change detection --- .../harvester_test/change_detector_test.py | 52 +++++++++++ .../harvester_test/checkpoint_store_test.py | 92 +++++++++++++++++++ .../utils/harvester/change_detector.py | 73 +++++++++++++++ .../utils/harvester/checkpoint_store.py | 54 +++++++++++ application/utils/harvester/models.py | 16 ++++ 5 files changed, 287 insertions(+) create mode 100644 application/tests/harvester_test/change_detector_test.py create mode 100644 application/tests/harvester_test/checkpoint_store_test.py create mode 100644 application/utils/harvester/change_detector.py create mode 100644 application/utils/harvester/checkpoint_store.py create mode 100644 application/utils/harvester/models.py diff --git a/application/tests/harvester_test/change_detector_test.py b/application/tests/harvester_test/change_detector_test.py new file mode 100644 index 000000000..a74590fd8 --- /dev/null +++ b/application/tests/harvester_test/change_detector_test.py @@ -0,0 +1,52 @@ +import unittest +from unittest.mock import MagicMock +from unittest.mock import patch + +from application.utils.harvester.change_detector import ( + ChangeDetector, +) + + +class ChangeDetectorTests(unittest.TestCase): + @patch("application.utils.harvester.change_detector.subprocess.run") + def test_get_modified_files_since(self, mock_run): + mock_run.return_value = MagicMock( + stdout="a.md\nb.md\na.md\n", + ) + + client = MagicMock() + detector = ChangeDetector(client) + + files = detector.get_modified_files_since("abc123") + + self.assertEqual( + files, + [ + "a.md", + "b.md", + ], + ) + + @patch("application.utils.harvester.change_detector.subprocess.run") + def test_get_commits_since(self, mock_run): + mock_run.return_value = MagicMock( + stdout="111\n222\n333\n", + ) + + client = MagicMock() + detector = ChangeDetector(client) + + commits = detector.get_commits_since("abc123") + + self.assertEqual( + commits, + [ + "111", + "222", + "333", + ], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/checkpoint_store_test.py b/application/tests/harvester_test/checkpoint_store_test.py new file mode 100644 index 000000000..9859b1fed --- /dev/null +++ b/application/tests/harvester_test/checkpoint_store_test.py @@ -0,0 +1,92 @@ +import unittest +from datetime import datetime +from pathlib import Path + +from application.utils.harvester.checkpoint_store import ( + CheckpointStore, +) +from application.utils.harvester.models import ( + RepositoryCheckpoint, +) + + +class CheckpointStoreTests(unittest.TestCase): + def test_save_and_load_checkpoint(self): + tmp_dir = Path(self._testMethodName) + + try: + store = CheckpointStore( + tmp_dir / "checkpoints.json", + ) + + checkpoint = RepositoryCheckpoint( + repository_id="owasp-asvs", + last_processed_commit="abc123", + updated_at=datetime.now(), + ) + + store.save(checkpoint) + + loaded = store.load("owasp-asvs") + + if loaded is None: + self.fail("Checkpoint should have been loaded") + + self.assertEqual( + loaded.last_processed_commit, + "abc123", + ) + + finally: + if tmp_dir.exists(): + import shutil + + shutil.rmtree(tmp_dir) + + def test_load_missing_file(self): + tmp_dir = Path(self._testMethodName) + + try: + store = CheckpointStore( + tmp_dir / "missing.json", + ) + + self.assertIsNone( + store.load("repo"), + ) + + finally: + if tmp_dir.exists(): + import shutil + + shutil.rmtree(tmp_dir) + + def test_load_missing_repository(self): + tmp_dir = Path(self._testMethodName) + + try: + store = CheckpointStore( + tmp_dir / "checkpoint.json", + ) + + store.save( + RepositoryCheckpoint( + repository_id="repo-a", + last_processed_commit="abc123", + updated_at=datetime.now(), + ) + ) + + self.assertIsNone( + store.load("repo-b"), + ) + + finally: + if tmp_dir.exists(): + import shutil + + shutil.rmtree(tmp_dir) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/utils/harvester/change_detector.py b/application/utils/harvester/change_detector.py new file mode 100644 index 000000000..8b0153492 --- /dev/null +++ b/application/utils/harvester/change_detector.py @@ -0,0 +1,73 @@ +import logging +import subprocess + +from .git_repository_client import GitRepositoryClient + +logger = logging.getLogger(__name__) + + +class ChangeDetector: + def __init__(self, repository_client: GitRepositoryClient): + self.repository_client = repository_client + + def get_modified_files_since(self, commit_sha: str) -> list[str]: + logger.info( + "Detecting changes since commit %s", + commit_sha, + ) + + try: + result = subprocess.run( + [ + "git", + "-C", + str(self.repository_client.get_local_path()), + "diff", + "--name-only", + commit_sha, + "HEAD", + ], + capture_output=True, + text=True, + check=True, + timeout=60, + ) + except subprocess.CalledProcessError as exc: + logger.error("Git command failed: %s", exc.stderr) + raise + + files = [ + file_path for file_path in result.stdout.splitlines() if file_path.strip() + ] + + return sorted(set(files)) + + def get_commits_since(self, commit_sha: str) -> list[str]: + try: + result = subprocess.run( + [ + "git", + "-C", + str(self.repository_client.get_local_path()), + "log", + "--format=%H", + f"{commit_sha}..HEAD", + ], + capture_output=True, + text=True, + check=True, + timeout=60, + ) + except subprocess.CalledProcessError as exc: + logger.error("Git command failed: %s", exc.stderr) + raise + + commits = [sha for sha in result.stdout.splitlines() if sha.strip()] + + logger.info( + "Detected %s commits since %s", + len(commits), + commit_sha, + ) + + return commits diff --git a/application/utils/harvester/checkpoint_store.py b/application/utils/harvester/checkpoint_store.py new file mode 100644 index 000000000..542d131dd --- /dev/null +++ b/application/utils/harvester/checkpoint_store.py @@ -0,0 +1,54 @@ +import json +from datetime import datetime +from pathlib import Path + +from .models import RepositoryCheckpoint + + +class CheckpointStore: + def __init__(self, checkpoint_file: Path): + self.checkpoint_file = checkpoint_file + + def load(self, repository_id: str) -> RepositoryCheckpoint | None: + if not self.checkpoint_file.exists(): + return None + + data = json.loads( + self.checkpoint_file.read_text( + encoding="utf-8", + ) + ) + + if repository_id not in data: + return None + + checkpoint = data[repository_id] + + return RepositoryCheckpoint( + repository_id=repository_id, + last_processed_commit=checkpoint["last_processed_commit"], + updated_at=datetime.fromisoformat(checkpoint["updated_at"]), + ) + + def save(self, checkpoint: RepositoryCheckpoint) -> None: + data = {} + if self.checkpoint_file.exists(): + data = json.loads(self.checkpoint_file.read_text(encoding="utf-8")) + + data[checkpoint.repository_id] = { + "last_processed_commit": checkpoint.last_processed_commit, + "updated_at": checkpoint.updated_at.isoformat(), + } + + self.checkpoint_file.parent.mkdir( + parents=True, + exist_ok=True, + ) + + self.checkpoint_file.write_text( + json.dumps( + data, + indent=2, + ), + encoding="utf-8", + ) diff --git a/application/utils/harvester/models.py b/application/utils/harvester/models.py new file mode 100644 index 000000000..fe936535a --- /dev/null +++ b/application/utils/harvester/models.py @@ -0,0 +1,16 @@ +from dataclasses import dataclass +from datetime import datetime + + +@dataclass(slots=True) +class RepositoryCheckpoint: + repository_id: str + last_processed_commit: str | None + updated_at: datetime + + +@dataclass(slots=True) +class RepositoryChangeSet: + repository_id: str + commit_sha: str + modified_files: list[str] From ccb515a5acf6f217369f38964d68cf405670d33d Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sat, 18 Jul 2026 18:51:21 +0530 Subject: [PATCH 20/27] fix(harvester): write checkpoints atomically --- application/utils/harvester/checkpoint_store.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/application/utils/harvester/checkpoint_store.py b/application/utils/harvester/checkpoint_store.py index 542d131dd..1af13168c 100644 --- a/application/utils/harvester/checkpoint_store.py +++ b/application/utils/harvester/checkpoint_store.py @@ -1,4 +1,5 @@ import json +import os from datetime import datetime from pathlib import Path @@ -27,13 +28,19 @@ def load(self, repository_id: str) -> RepositoryCheckpoint | None: return RepositoryCheckpoint( repository_id=repository_id, last_processed_commit=checkpoint["last_processed_commit"], - updated_at=datetime.fromisoformat(checkpoint["updated_at"]), + updated_at=datetime.fromisoformat( + checkpoint["updated_at"], + ), ) def save(self, checkpoint: RepositoryCheckpoint) -> None: data = {} if self.checkpoint_file.exists(): - data = json.loads(self.checkpoint_file.read_text(encoding="utf-8")) + data = json.loads( + self.checkpoint_file.read_text( + encoding="utf-8", + ) + ) data[checkpoint.repository_id] = { "last_processed_commit": checkpoint.last_processed_commit, @@ -45,10 +52,13 @@ def save(self, checkpoint: RepositoryCheckpoint) -> None: exist_ok=True, ) - self.checkpoint_file.write_text( + temp_file = self.checkpoint_file.with_suffix(".tmp") + temp_file.write_text( json.dumps( data, indent=2, ), encoding="utf-8", ) + + os.replace(temp_file, self.checkpoint_file) From 853f34e09cae1e9d4653acff221566b4d30b6365 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Wed, 8 Jul 2026 14:10:50 +0530 Subject: [PATCH 21/27] feat(harvester): implement repository file filtering --- .../tests/harvester_test/file_filter_test.py | 62 +++++++++++++++++++ .../filtering_benchmark_test.py | 49 +++++++++++++++ .../harvester_test/filtering_metrics_test.py | 35 +++++++++++ application/utils/harvester/__init__.py | 10 +++ application/utils/harvester/file_filter.py | 54 ++++++++++++++++ .../utils/harvester/filtering_benchmark.py | 38 ++++++++++++ .../utils/harvester/filtering_metrics.py | 23 +++++++ application/utils/harvester/models.py | 7 +++ 8 files changed, 278 insertions(+) create mode 100644 application/tests/harvester_test/file_filter_test.py create mode 100644 application/tests/harvester_test/filtering_benchmark_test.py create mode 100644 application/tests/harvester_test/filtering_metrics_test.py create mode 100644 application/utils/harvester/file_filter.py create mode 100644 application/utils/harvester/filtering_benchmark.py create mode 100644 application/utils/harvester/filtering_metrics.py diff --git a/application/tests/harvester_test/file_filter_test.py b/application/tests/harvester_test/file_filter_test.py new file mode 100644 index 000000000..ed7958fa9 --- /dev/null +++ b/application/tests/harvester_test/file_filter_test.py @@ -0,0 +1,62 @@ +import unittest + +from application.utils.harvester.file_filter import ( + FileFilter, +) + + +class FileFilterTests(unittest.TestCase): + def test_extension_filtering(self): + file_filter = FileFilter() + + result = file_filter.filter_files( + [ + "README.md", + "image.png", + "script.js", + ] + ) + + self.assertEqual( + result, + ["README.md"], + ) + + def test_regex_filtering(self): + file_filter = FileFilter() + + result = file_filter.filter_files( + [ + ".github/workflows/test.yml", + "docs/setup.md", + ] + ) + + self.assertEqual( + result, + ["docs/setup.md"], + ) + + def test_combined_filtering(self): + file_filter = FileFilter() + + result = file_filter.filter_files( + [ + "README.md", + ".github/workflows/test.yml", + "node_modules/react/index.js", + "docs/setup.md", + ] + ) + + self.assertEqual( + result, + [ + "README.md", + "docs/setup.md", + ], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/filtering_benchmark_test.py b/application/tests/harvester_test/filtering_benchmark_test.py new file mode 100644 index 000000000..c34105270 --- /dev/null +++ b/application/tests/harvester_test/filtering_benchmark_test.py @@ -0,0 +1,49 @@ +import unittest + +from application.utils.harvester.file_filter import ( + FileFilter, +) +from application.utils.harvester.models import ( + FilteringMetrics, +) + + +class FilteringBenchmarkTests(unittest.TestCase): + def test_filtering_benchmark(self): + files = [ + "README.md", + ".github/workflows/ci.yml", + "docs/guide.md", + "image.png", + "notes.txt", + "package-lock.json", + ] + + file_filter = FileFilter() + + retained_files = file_filter.filter_files(files) + + metrics = FilteringMetrics( + total_files=len(files), + retained_files=len(retained_files), + filtered_files=len(files) - len(retained_files), + ) + + self.assertEqual( + metrics.total_files, + 6, + ) + + self.assertEqual( + metrics.retained_files, + 3, + ) + + self.assertEqual( + metrics.filtered_files, + 3, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/filtering_metrics_test.py b/application/tests/harvester_test/filtering_metrics_test.py new file mode 100644 index 000000000..252cb0941 --- /dev/null +++ b/application/tests/harvester_test/filtering_metrics_test.py @@ -0,0 +1,35 @@ +import unittest + +from application.utils.harvester.filtering_metrics import ( + FilteringMetricsCollector, +) + + +class FilteringMetricsCollectorTests(unittest.TestCase): + def test_filtering_metrics_collection(self): + collector = FilteringMetricsCollector() + + collector.record_retained() + collector.record_retained() + collector.record_filtered() + + metrics = collector.build() + + self.assertEqual( + metrics.total_files, + 3, + ) + + self.assertEqual( + metrics.retained_files, + 2, + ) + + self.assertEqual( + metrics.filtered_files, + 1, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/utils/harvester/__init__.py b/application/utils/harvester/__init__.py index 2ac608b3e..eda28b629 100644 --- a/application/utils/harvester/__init__.py +++ b/application/utils/harvester/__init__.py @@ -17,12 +17,22 @@ from .git_repository_client import GitRepositoryClient from .repository_client import RepositoryClient from .repository_cache import build_repository_cache_path +from .file_filter import FileFilter +from .filtering_metrics import FilteringMetricsCollector +from .filtering_benchmark import ( + FilteringBenchmark, + FilteringBenchmarkResult, +) __all__ = [ "build_repository_cache_path", "ChunkingConfig", "ConfigLoaderError", "GitRepositoryClient", + "FileFilter", + "FilteringMetricsCollector", + "FilteringBenchmark", + "FilteringBenchmarkResult", "PathRules", "PollingConfig", "RepositoryClient", diff --git a/application/utils/harvester/file_filter.py b/application/utils/harvester/file_filter.py new file mode 100644 index 000000000..706dffe2c --- /dev/null +++ b/application/utils/harvester/file_filter.py @@ -0,0 +1,54 @@ +import re + +DEFAULT_ALLOWED_EXTENSIONS = { + ".md", + ".mdx", + ".rst", + ".txt", + ".adoc", +} + +DEFAULT_EXCLUDE_PATTERNS = [ + r"^\.github/", + r"^\.git/", + r"^node_modules/", + r"^dist/", + r"^build/", + r"^coverage/", + r"^vendor/", + r".*package-lock\.json$", + r".*yarn\.lock$", + r".*pnpm-lock\.yaml$", +] + + +class FileFilter: + def __init__( + self, + exclude_patterns: list[str] | None = None, + allowed_extensions: set[str] | None = None, + ): + self.exclude_patterns = exclude_patterns or DEFAULT_EXCLUDE_PATTERNS + self.allowed_extensions = allowed_extensions or DEFAULT_ALLOWED_EXTENSIONS + + def is_excluded_by_pattern(self, file_path: str) -> bool: + return any(re.search(pattern, file_path) for pattern in self.exclude_patterns) + + def is_allowed_extension(self, file_path: str) -> bool: + return any( + file_path.endswith(extension) for extension in self.allowed_extensions + ) + + def filter_files(self, files: list[str]) -> list[str]: + filtered_files = [] + + for file_path in files: + if self.is_excluded_by_pattern(file_path): + continue + + if not self.is_allowed_extension(file_path): + continue + + filtered_files.append(file_path) + + return filtered_files diff --git a/application/utils/harvester/filtering_benchmark.py b/application/utils/harvester/filtering_benchmark.py new file mode 100644 index 000000000..2fc095e4e --- /dev/null +++ b/application/utils/harvester/filtering_benchmark.py @@ -0,0 +1,38 @@ +from dataclasses import dataclass + +from .file_filter import FileFilter +from .models import FilteringMetrics + + +@dataclass +class FilteringBenchmarkResult: + total_files: int + retained_files: int + filtered_files: int + retention_rate: float + filtering_rate: float + + +class FilteringBenchmark: + def __init__( + self, + file_filter: FileFilter, + metrics: FilteringMetrics, + ): + self.file_filter = file_filter + self.metrics = metrics + + def run(self, file_paths: list[str]) -> FilteringBenchmarkResult: + retained = self.file_filter.filter_files(file_paths) + + total = len(file_paths) + retained_count = len(retained) + filtered_count = total - retained_count + + return FilteringBenchmarkResult( + total_files=total, + retained_files=retained_count, + filtered_files=filtered_count, + retention_rate=(retained_count / total if total else 0.0), + filtering_rate=(filtered_count / total if total else 0.0), + ) diff --git a/application/utils/harvester/filtering_metrics.py b/application/utils/harvester/filtering_metrics.py new file mode 100644 index 000000000..d496c3e2a --- /dev/null +++ b/application/utils/harvester/filtering_metrics.py @@ -0,0 +1,23 @@ +from .models import FilteringMetrics + + +class FilteringMetricsCollector: + def __init__(self): + self.total_files = 0 + self.retained_files = 0 + self.filtered_files = 0 + + def record_retained(self) -> None: + self.total_files += 1 + self.retained_files += 1 + + def record_filtered(self) -> None: + self.total_files += 1 + self.filtered_files += 1 + + def build(self) -> FilteringMetrics: + return FilteringMetrics( + total_files=self.total_files, + retained_files=self.retained_files, + filtered_files=self.filtered_files, + ) diff --git a/application/utils/harvester/models.py b/application/utils/harvester/models.py index fe936535a..9a3a12b28 100644 --- a/application/utils/harvester/models.py +++ b/application/utils/harvester/models.py @@ -1,5 +1,6 @@ from dataclasses import dataclass from datetime import datetime +from pydantic import BaseModel @dataclass(slots=True) @@ -14,3 +15,9 @@ class RepositoryChangeSet: repository_id: str commit_sha: str modified_files: list[str] + + +class FilteringMetrics(BaseModel): + total_files: int + retained_files: int + filtered_files: int From 10b8f2fd822183484248fc740581593e7fb125f0 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sat, 18 Jul 2026 21:08:19 +0530 Subject: [PATCH 22/27] fix(harvester): improve filtering benchmark and sync behavior --- .../filtering_benchmark_test.py | 37 +++++-------------- .../git_repository_client_test.py | 18 ++++++++- .../utils/harvester/filtering_benchmark.py | 8 +--- .../utils/harvester/git_repository_client.py | 24 ++++++++++++ 4 files changed, 51 insertions(+), 36 deletions(-) diff --git a/application/tests/harvester_test/filtering_benchmark_test.py b/application/tests/harvester_test/filtering_benchmark_test.py index c34105270..a75c435ec 100644 --- a/application/tests/harvester_test/filtering_benchmark_test.py +++ b/application/tests/harvester_test/filtering_benchmark_test.py @@ -1,11 +1,7 @@ import unittest -from application.utils.harvester.file_filter import ( - FileFilter, -) -from application.utils.harvester.models import ( - FilteringMetrics, -) +from application.utils.harvester.file_filter import FileFilter +from application.utils.harvester.filtering_benchmark import FilteringBenchmark class FilteringBenchmarkTests(unittest.TestCase): @@ -19,30 +15,15 @@ def test_filtering_benchmark(self): "package-lock.json", ] - file_filter = FileFilter() + benchmark = FilteringBenchmark(file_filter=FileFilter()) - retained_files = file_filter.filter_files(files) + result = benchmark.run(files) - metrics = FilteringMetrics( - total_files=len(files), - retained_files=len(retained_files), - filtered_files=len(files) - len(retained_files), - ) - - self.assertEqual( - metrics.total_files, - 6, - ) - - self.assertEqual( - metrics.retained_files, - 3, - ) - - self.assertEqual( - metrics.filtered_files, - 3, - ) + self.assertEqual(result.total_files, 6) + self.assertEqual(result.retained_files, 3) + self.assertEqual(result.filtered_files, 3) + self.assertEqual(result.retention_rate, 0.5) + self.assertEqual(result.filtering_rate, 0.5) if __name__ == "__main__": diff --git a/application/tests/harvester_test/git_repository_client_test.py b/application/tests/harvester_test/git_repository_client_test.py index 4f548a8be..3075b1a6b 100644 --- a/application/tests/harvester_test/git_repository_client_test.py +++ b/application/tests/harvester_test/git_repository_client_test.py @@ -63,7 +63,8 @@ def test_sync_clones_when_repository_missing(self): mock_clone.assert_called_once() - def test_sync_fetches_when_repository_exists(self): + @patch("application.utils.harvester.git_repository_client.subprocess.run") + def test_sync_fetches_when_repository_exists(self, mock_run): client = GitRepositoryClient( owner="OWASP", repository="ASVS", @@ -81,6 +82,21 @@ def test_sync_fetches_when_repository_exists(self): 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( diff --git a/application/utils/harvester/filtering_benchmark.py b/application/utils/harvester/filtering_benchmark.py index 2fc095e4e..5de2e7b69 100644 --- a/application/utils/harvester/filtering_benchmark.py +++ b/application/utils/harvester/filtering_benchmark.py @@ -1,7 +1,6 @@ from dataclasses import dataclass from .file_filter import FileFilter -from .models import FilteringMetrics @dataclass @@ -14,13 +13,8 @@ class FilteringBenchmarkResult: class FilteringBenchmark: - def __init__( - self, - file_filter: FileFilter, - metrics: FilteringMetrics, - ): + def __init__(self, file_filter: FileFilter): self.file_filter = file_filter - self.metrics = metrics def run(self, file_paths: list[str]) -> FilteringBenchmarkResult: retained = self.file_filter.filter_files(file_paths) diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py index eff7c17d6..8ff722eb2 100644 --- a/application/utils/harvester/git_repository_client.py +++ b/application/utils/harvester/git_repository_client.py @@ -146,6 +146,30 @@ def sync(self) -> None: if self.verify_repository_integrity(): self.fetch() + + try: + subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "reset", + "--hard", + f"origin/{self.branch}", + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) + except subprocess.CalledProcessError as exc: + logger.error( + "Failed to reset repository %s/%s: %s", + self.owner, + self.repository, + exc.stderr, + ) + raise else: self.clone() From 5f8c5f38d8cb730c57efb371af8108705508bad0 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Fri, 10 Jul 2026 13:47:17 +0530 Subject: [PATCH 23/27] feat(harvester): add git diff retrieval pipeline --- .gitignore | 2 + .../harvester_test/diff_retriever_test.py | 49 +++++++++++++++++++ application/utils/harvester/__init__.py | 3 ++ application/utils/harvester/diff_retriever.py | 39 +++++++++++++++ 4 files changed, 93 insertions(+) create mode 100644 application/tests/harvester_test/diff_retriever_test.py create mode 100644 application/utils/harvester/diff_retriever.py diff --git a/.gitignore b/.gitignore index ecc6b7d04..9efee842b 100644 --- a/.gitignore +++ b/.gitignore @@ -83,3 +83,5 @@ tmp/ cres/* ### Local project management tooling project management scripts/ + +.harvester_cache/ 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..e716f2452 --- /dev/null +++ b/application/tests/harvester_test/diff_retriever_test.py @@ -0,0 +1,49 @@ +import unittest +from unittest.mock import MagicMock +from unittest.mock import patch + +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.return_value = MagicMock( + stdout="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_called_once_with( + [ + "git", + "-C", + "/tmp/repo", + "diff", + "abc123", + "def456", + ], + capture_output=True, + text=True, + check=True, + timeout=300, + ) + + +if __name__ == "__main__": + unittest.main() 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/diff_retriever.py b/application/utils/harvester/diff_retriever.py new file mode 100644 index 000000000..6067c9216 --- /dev/null +++ b/application/utils/harvester/diff_retriever.py @@ -0,0 +1,39 @@ +import logging +import subprocess + +from .git_repository_client import GitRepositoryClient + +logger = logging.getLogger(__name__) + + +class DiffRetriever: + def __init__(self, repository_client: GitRepositoryClient) -> None: + self.repository_client = repository_client + + def get_diff(self, base_commit: str, target_commit: str = "HEAD") -> str: + logger.info( + "Retrieving diff between %s and %s", + base_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, + text=True, + timeout=300, + ) + except subprocess.CalledProcessError as exc: + logger.error("Failed to retrieve diff: %s", exc.stderr) + raise + + return result.stdout From 6f6bd22fa3076025e13b22bc53976856c2e8dd06 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Fri, 10 Jul 2026 14:00:26 +0530 Subject: [PATCH 24/27] feat(harvester): parse unified git diffs --- .../tests/harvester_test/diff_parser_test.py | 89 +++++++++++++++++++ application/utils/harvester/diff_parser.py | 43 +++++++++ application/utils/harvester/models.py | 6 ++ 3 files changed, 138 insertions(+) create mode 100644 application/tests/harvester_test/diff_parser_test.py create mode 100644 application/utils/harvester/diff_parser.py 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..db001df4f --- /dev/null +++ b/application/tests/harvester_test/diff_parser_test.py @@ -0,0 +1,89 @@ +import unittest + +from application.utils.harvester.diff_parser import ( + DiffParser, +) + + +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) + + self.assertEqual( + len(blocks), + 1, + ) + + self.assertEqual( + blocks[0].file_path, + "test.md", + ) + + self.assertEqual( + blocks[0].added_lines, + [ + "new", + "another", + ], + ) + + 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) + + self.assertEqual( + len(blocks), + 2, + ) + + self.assertEqual( + blocks[0].file_path, + "a.md", + ) + + self.assertEqual( + blocks[1].file_path, + "b.md", + ) + + def test_deleted_lines_are_ignored(self): + parser = DiffParser() + + diff = """diff --git a/test.md b/test.md +@@ +-old ++new +""" + + blocks = parser.parse(diff) + + self.assertEqual( + blocks[0].added_lines, + [ + "new", + ], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/utils/harvester/diff_parser.py b/application/utils/harvester/diff_parser.py new file mode 100644 index 000000000..d9c8e9d32 --- /dev/null +++ b/application/utils/harvester/diff_parser.py @@ -0,0 +1,43 @@ +import re + +from .models import DiffBlock + + +class DiffParser: + def parse(self, diff: str) -> list[DiffBlock]: + 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) + ) + + match = re.match(r"diff --git a/(.+?) b/", line) + + if match: + current_file = match.group(1) + added_lines = [] + + continue + + if line.startswith("+++"): + continue + + if line.startswith("---"): + continue + + if line.startswith("@@"): + continue + + if line.startswith("+") and not line.startswith("+++"): + added_lines.append(line[1:]) + + if current_file is not None: + blocks.append(DiffBlock(file_path=current_file, added_lines=added_lines)) + + return blocks diff --git a/application/utils/harvester/models.py b/application/utils/harvester/models.py index 9a3a12b28..df8528317 100644 --- a/application/utils/harvester/models.py +++ b/application/utils/harvester/models.py @@ -21,3 +21,9 @@ class FilteringMetrics(BaseModel): total_files: int retained_files: int filtered_files: int + + +@dataclass(slots=True) +class DiffBlock: + file_path: str + added_lines: list[str] From d3f59178b41cc8f2d87e5a8ed08e5e725d0072f8 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Fri, 10 Jul 2026 14:45:58 +0530 Subject: [PATCH 25/27] feat(harvester): normalize extracted diff content --- .../harvester_test/diff_normalizer_test.py | 105 ++++++++++++++++++ .../utils/harvester/diff_normalizer.py | 33 ++++++ 2 files changed, 138 insertions(+) create mode 100644 application/tests/harvester_test/diff_normalizer_test.py create mode 100644 application/utils/harvester/diff_normalizer.py 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..19d3c8ec6 --- /dev/null +++ b/application/tests/harvester_test/diff_normalizer_test.py @@ -0,0 +1,105 @@ +import unittest + +from application.utils.harvester.diff_normalizer import ( + DiffNormalizer, +) + +from application.utils.harvester.models import ( + DiffBlock, +) + + +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** ", + ], + ) + ] + + 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", + ], + ) + ] + + 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 "], + ), + DiffBlock( + file_path="b.md", + added_lines=[" Two "], + ), + ] + + 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/utils/harvester/diff_normalizer.py b/application/utils/harvester/diff_normalizer.py new file mode 100644 index 000000000..babdf3508 --- /dev/null +++ b/application/utils/harvester/diff_normalizer.py @@ -0,0 +1,33 @@ +import textacy.preprocessing as prep + +from .models import DiffBlock + + +class DiffNormalizer: + def normalize_line(self, line: str) -> str: + line = prep.normalize.unicode(line) + line = prep.normalize.whitespace(line) + return line.strip() + + def normalize(self, blocks: list[DiffBlock]) -> list[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, + ) + ) + + return normalized From fde38f53ae5a2e3167f996d9a800669441a76592 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Fri, 10 Jul 2026 17:14:07 +0530 Subject: [PATCH 26/27] Enhance diff pipeline with metadata and normalization --- .../harvester_test/diff_normalizer_test.py | 12 +++++ .../tests/harvester_test/diff_parser_test.py | 52 ++++++++++++------- .../harvester_test/diff_pipeline_test.py | 50 ++++++++++++++++++ .../harvester_test/diff_retriever_test.py | 14 +++++ .../utils/harvester/diff_normalizer.py | 14 +++++ application/utils/harvester/diff_parser.py | 33 ++++++++++-- application/utils/harvester/diff_retriever.py | 39 +++++++++++++- application/utils/harvester/models.py | 8 +++ 8 files changed, 199 insertions(+), 23 deletions(-) create mode 100644 application/tests/harvester_test/diff_pipeline_test.py diff --git a/application/tests/harvester_test/diff_normalizer_test.py b/application/tests/harvester_test/diff_normalizer_test.py index 19d3c8ec6..04eb1ce3f 100644 --- a/application/tests/harvester_test/diff_normalizer_test.py +++ b/application/tests/harvester_test/diff_normalizer_test.py @@ -1,4 +1,5 @@ import unittest +from datetime import datetime from application.utils.harvester.diff_normalizer import ( DiffNormalizer, @@ -9,6 +10,13 @@ ) +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() @@ -30,6 +38,7 @@ def test_whitespace_normalization(self): " `inline code` ", " **Important** ", ], + **DIFF_METADATA, ) ] @@ -62,6 +71,7 @@ def test_remove_empty_lines(self): " ", "Hello", ], + **DIFF_METADATA, ) ] @@ -81,10 +91,12 @@ def test_multiple_blocks(self): DiffBlock( file_path="a.md", added_lines=[" One "], + **DIFF_METADATA, ), DiffBlock( file_path="b.md", added_lines=[" Two "], + **DIFF_METADATA, ), ] diff --git a/application/tests/harvester_test/diff_parser_test.py b/application/tests/harvester_test/diff_parser_test.py index db001df4f..a4444e8d9 100644 --- a/application/tests/harvester_test/diff_parser_test.py +++ b/application/tests/harvester_test/diff_parser_test.py @@ -1,9 +1,14 @@ +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): @@ -18,13 +23,15 @@ def test_single_file_diff(self): +another """ - blocks = parser.parse(diff) - - self.assertEqual( - len(blocks), - 1, + 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", @@ -38,6 +45,10 @@ def test_single_file_diff(self): ], ) + 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() @@ -49,22 +60,20 @@ def test_multiple_files(self): +two """ - blocks = parser.parse(diff) - - self.assertEqual( - len(blocks), - 2, + blocks = parser.parse( + diff, + repository=TEST_REPOSITORY, + commit_sha=TEST_COMMIT_SHA, + committed_at=TEST_COMMITTED_AT, ) - self.assertEqual( - blocks[0].file_path, - "a.md", - ) + self.assertEqual(len(blocks), 2) - self.assertEqual( - blocks[1].file_path, - "b.md", - ) + 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() @@ -75,7 +84,12 @@ def test_deleted_lines_are_ignored(self): +new """ - blocks = parser.parse(diff) + blocks = parser.parse( + diff, + repository=TEST_REPOSITORY, + commit_sha=TEST_COMMIT_SHA, + committed_at=TEST_COMMITTED_AT, + ) self.assertEqual( blocks[0].added_lines, 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..f27624170 --- /dev/null +++ b/application/tests/harvester_test/diff_pipeline_test.py @@ -0,0 +1,50 @@ +from datetime import UTC, datetime +import time +import unittest + +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): + client = GitRepositoryClient( + "OWASP", + "ASVS", + "master", + ) + + retriever = DiffRetriever(client) + parser = DiffParser() + normalizer = DiffNormalizer() + + start = time.perf_counter() + + diff = retriever.get_diff( + "a79c0184", + "122d9e0969465a6041e16c806a0464b35deea444", + ) + + blocks = parser.parse( + diff, + repository="OWASP/ASVS", + commit_sha="122d9e0969465a6041e16c806a0464b35deea444", + 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 e716f2452..0e890a07f 100644 --- a/application/tests/harvester_test/diff_retriever_test.py +++ b/application/tests/harvester_test/diff_retriever_test.py @@ -44,6 +44,20 @@ def test_get_diff(self, mock_run): 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/utils/harvester/diff_normalizer.py b/application/utils/harvester/diff_normalizer.py index babdf3508..fe8773349 100644 --- a/application/utils/harvester/diff_normalizer.py +++ b/application/utils/harvester/diff_normalizer.py @@ -1,15 +1,26 @@ import textacy.preprocessing as prep +from application.utils.harvester import repository_client 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 = prep.normalize.unicode(line) line = prep.normalize.whitespace(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: @@ -27,6 +38,9 @@ def normalize(self, blocks: list[DiffBlock]) -> list[DiffBlock]: DiffBlock( file_path=block.file_path, added_lines=cleaned_lines, + repository=block.repository, + commit_sha=block.commit_sha, + committed_at=block.committed_at, ) ) diff --git a/application/utils/harvester/diff_parser.py b/application/utils/harvester/diff_parser.py index d9c8e9d32..8bdaeb42e 100644 --- a/application/utils/harvester/diff_parser.py +++ b/application/utils/harvester/diff_parser.py @@ -1,10 +1,23 @@ +from datetime import datetime import re from .models import DiffBlock class DiffParser: - def parse(self, diff: str) -> list[DiffBlock]: + """ + 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 @@ -14,7 +27,13 @@ def parse(self, diff: str) -> list[DiffBlock]: if line.startswith("diff --git"): if current_file is not None: blocks.append( - DiffBlock(file_path=current_file, added_lines=added_lines) + 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) @@ -38,6 +57,14 @@ def parse(self, diff: str) -> list[DiffBlock]: added_lines.append(line[1:]) if current_file is not None: - blocks.append(DiffBlock(file_path=current_file, added_lines=added_lines)) + 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 index 6067c9216..78c03bb13 100644 --- a/application/utils/harvester/diff_retriever.py +++ b/application/utils/harvester/diff_retriever.py @@ -7,10 +7,37 @@ class DiffRetriever: + MAX_DIFF_SIZE_BYTES = 50 * 1024 * 1024 + """ + + 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. + + """ + 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, @@ -36,4 +63,14 @@ def get_diff(self, base_commit: str, target_commit: str = "HEAD") -> str: logger.error("Failed to retrieve diff: %s", exc.stderr) raise - return result.stdout + diff = result.stdout + + diff_size = len(diff.encode("utf-8")) + + 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 diff --git a/application/utils/harvester/models.py b/application/utils/harvester/models.py index df8528317..c6481dd36 100644 --- a/application/utils/harvester/models.py +++ b/application/utils/harvester/models.py @@ -25,5 +25,13 @@ class FilteringMetrics(BaseModel): @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 From 8dea1c74f29be37bcdc024da66b1831e1d59ab17 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sat, 18 Jul 2026 23:51:05 +0530 Subject: [PATCH 27/27] fix(harvester): address review feedback --- .../harvester_test/diff_pipeline_test.py | 24 ++++++++++++++++--- .../harvester_test/diff_retriever_test.py | 5 ++-- .../utils/harvester/checkpoint_store.py | 19 ++++++++------- application/utils/harvester/diff_parser.py | 6 ++--- application/utils/harvester/diff_retriever.py | 12 ++++++---- application/utils/harvester/file_filter.py | 16 ++++++------- 6 files changed, 51 insertions(+), 31 deletions(-) diff --git a/application/tests/harvester_test/diff_pipeline_test.py b/application/tests/harvester_test/diff_pipeline_test.py index f27624170..5ce0a6fc2 100644 --- a/application/tests/harvester_test/diff_pipeline_test.py +++ b/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), ) diff --git a/application/tests/harvester_test/diff_retriever_test.py b/application/tests/harvester_test/diff_retriever_test.py index 0e890a07f..416499e5f 100644 --- a/application/tests/harvester_test/diff_retriever_test.py +++ b/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,7 +39,6 @@ def test_get_diff(self, mock_run): "def456", ], capture_output=True, - text=True, check=True, timeout=300, ) @@ -47,7 +46,7 @@ def test_get_diff(self, mock_run): @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() diff --git a/application/utils/harvester/checkpoint_store.py b/application/utils/harvester/checkpoint_store.py index 1af13168c..5d01ae1b4 100644 --- a/application/utils/harvester/checkpoint_store.py +++ b/application/utils/harvester/checkpoint_store.py @@ -11,14 +11,14 @@ def __init__(self, checkpoint_file: Path): self.checkpoint_file = checkpoint_file def load(self, repository_id: str) -> RepositoryCheckpoint | None: - if not self.checkpoint_file.exists(): - return None - - data = json.loads( - self.checkpoint_file.read_text( - encoding="utf-8", + try: + data = json.loads( + self.checkpoint_file.read_text( + encoding="utf-8", + ) ) - ) + except FileNotFoundError: + return None if repository_id not in data: return None @@ -34,13 +34,14 @@ def load(self, repository_id: str) -> RepositoryCheckpoint | None: ) def save(self, checkpoint: RepositoryCheckpoint) -> None: - data = {} - if self.checkpoint_file.exists(): + try: data = json.loads( self.checkpoint_file.read_text( encoding="utf-8", ) ) + except FileNotFoundError: + data = {} data[checkpoint.repository_id] = { "last_processed_commit": checkpoint.last_processed_commit, diff --git a/application/utils/harvester/diff_parser.py b/application/utils/harvester/diff_parser.py index 8bdaeb42e..d0f124bf9 100644 --- a/application/utils/harvester/diff_parser.py +++ b/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: diff --git a/application/utils/harvester/diff_retriever.py b/application/utils/harvester/diff_retriever.py index 78c03bb13..fce640d52 100644 --- a/application/utils/harvester/diff_retriever.py +++ b/application/utils/harvester/diff_retriever.py @@ -56,16 +56,18 @@ 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( @@ -73,4 +75,4 @@ def get_diff(self, base_commit: str, target_commit: str = "HEAD") -> str: f"maximum supported size ({self.MAX_DIFF_SIZE_BYTES} bytes)." ) - return diff + return diff_bytes.decode("utf-8", errors="replace") diff --git a/application/utils/harvester/file_filter.py b/application/utils/harvester/file_filter.py index 706dffe2c..7df0f9604 100644 --- a/application/utils/harvester/file_filter.py +++ b/application/utils/harvester/file_filter.py @@ -11,14 +11,14 @@ DEFAULT_EXCLUDE_PATTERNS = [ r"^\.github/", r"^\.git/", - r"^node_modules/", - r"^dist/", - r"^build/", - r"^coverage/", - r"^vendor/", - r".*package-lock\.json$", - r".*yarn\.lock$", - r".*pnpm-lock\.yaml$", + r"(?:^|/)node_modules/", + r"(?:^|/)dist/", + r"(?:^|/)build/", + r"(?:^|/)coverage/", + r"(?:^|/)vendor/", + r"package-lock\.json$", + r"yarn\.lock$", + r"pnpm-lock\.yaml$", ]