From 76067e1930dec43c1e162f931e7e4dd8af08bc3f Mon Sep 17 00:00:00 2001 From: Manish Kumar Date: Fri, 25 Sep 2026 01:37:41 -0500 Subject: [PATCH] test: raise coverage gate to 97% and cover remaining branches - Measure subprocess coverage (patch = subprocess) so the real-FAISS derive tests count toward index/derive.py - Add tests for lifecycle legacy/validation failures, trusted ingestion title/type/skip/failure paths and build_manifest, ollama_embed on_attempt reporting, and derive's defensive failure paths - Raise fail_under from 95 to 97 (now 99.87%) Co-Authored-By: Claude Opus 5.5 --- .coveragerc | 3 +- tests/test_derive_public_index.py | 62 +++++++++++++++++ tests/test_index_lifecycle.py | 86 ++++++++++++++++++++++++ tests/test_rag_engine.py | 25 +++++++ tests/test_trusted_ingestion.py | 107 ++++++++++++++++++++++++++++++ 5 files changed, 282 insertions(+), 1 deletion(-) diff --git a/.coveragerc b/.coveragerc index b857b2b..4b8da6a 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1,5 +1,6 @@ [run] source = . +patch = subprocess omit = obsolete/* scripts/* @@ -7,7 +8,7 @@ omit = */site-packages/* [report] -fail_under = 95 +fail_under = 97 show_missing = true exclude_lines = pragma: no cover diff --git a/tests/test_derive_public_index.py b/tests/test_derive_public_index.py index c936d15..a4c7e5e 100644 --- a/tests/test_derive_public_index.py +++ b/tests/test_derive_public_index.py @@ -34,6 +34,25 @@ def meta(i, v): "embedding_model": "nomic-embed-text", "embedding_dimension": 768, "metadata_schema_version": "v1"}}) if mode == "already_public": m = read_manifest(src); m["visibility_policy"] = "PUBLIC_ONLY"; write_manifest(src, m) + import os, index.derive as derive + if mode == "exists": os.makedirs(work + "/out/srcbuild-public") + if mode == "stale_partial": + os.makedirs(work + "/out/.srcbuild-public.partial"); open(work + "/out/.srcbuild-public.partial/junk", "w").close() + class Store(VectorStore): + def load(self, path): + if mode == "source_load_fails": return False + ok = super().load(path) + if mode == "unknown_after_validation" and path == src: self.metadata[1]["visibility"] = "SECRET" + if mode == "metadata_differs" and path.endswith(".partial"): self.metadata[0]["text"] = "tampered" + return ok + derive.VectorStore = Store + real_validate = derive.validate_index_directory + def validate(d, **kw): + if mode == "unknown_after_validation": return {{"ok": True}} + if mode == "derived_invalid" and kw.get("require_public_only"): return {{"ok": False, "reason": "forced"}} + return real_validate(d, **kw) + derive.validate_index_directory = validate + if mode == "vectors_differ": derive.np = type("np", (), {{**vars(np), "array_equal": staticmethod(lambda a, b: False)}}) out = {{}} try: rep = derive_public_only(src, work + "/out") @@ -114,3 +133,46 @@ def test_a_public_only_manifest_over_a_source_containing_internal_chunks_is_refu # Not derivable, and not servable: the mixed source fails validation before anything is copied. out = _run(tmp_path, "unknown_visibility") assert not out["ok"] + + +def test_refuses_to_overwrite_an_existing_derived_candidate(tmp_path): + out = _run(tmp_path, "exists") + assert not out["ok"] and "already exists" in out["error"] + + +def test_a_stale_partial_directory_from_a_crashed_run_is_replaced(tmp_path): + out = _run(tmp_path, "stale_partial") + assert out["ok"] and out["validation_ok"], out + assert not (tmp_path / "out" / ".srcbuild-public.partial").exists() + + +def test_refuses_when_the_source_vector_store_does_not_load(tmp_path): + out = _run(tmp_path, "source_load_fails") + assert not out["ok"] and "did not load" in out["error"] + + +def test_unknown_visibility_is_refused_even_if_validation_missed_it(tmp_path): + out = _run(tmp_path, "unknown_after_validation") + assert not out["ok"] and "missing/unknown visibility" in out["error"] and "[1]" in out["error"] + + +def _no_derived_output(tmp_path): + return not (tmp_path / "out" / "srcbuild-public").exists() and not (tmp_path / "out" / ".srcbuild-public.partial").exists() + + +def test_a_derived_artifact_that_fails_validation_is_discarded(tmp_path): + out = _run(tmp_path, "derived_invalid") + assert not out["ok"] and "failed validation: forced" in out["error"] + assert _no_derived_output(tmp_path) + + +def test_derived_vectors_that_are_not_bit_identical_are_discarded(tmp_path): + out = _run(tmp_path, "vectors_differ") + assert not out["ok"] and "not bit-identical" in out["error"] + assert _no_derived_output(tmp_path) + + +def test_derived_metadata_that_differs_from_the_source_is_discarded(tmp_path): + out = _run(tmp_path, "metadata_differs") + assert not out["ok"] and "metadata is not identical" in out["error"] + assert _no_derived_output(tmp_path) diff --git a/tests/test_index_lifecycle.py b/tests/test_index_lifecycle.py index ad83b83..302ad7f 100644 --- a/tests/test_index_lifecycle.py +++ b/tests/test_index_lifecycle.py @@ -313,3 +313,89 @@ def test_manifest_write_is_atomic_file_replace(tmp_path): write_manifest(tmp_path / "d", {"a": 1}) assert not (tmp_path / "d" / "manifest.json.tmp").exists() assert json.loads((tmp_path / "d" / "manifest.json").read_text()) == {"a": 1} + + +def _mock_store(cls, *, loads=True, ntotal=1, metadata=None, dim=768): + cls.return_value.load.return_value = loads + cls.return_value.index.ntotal = ntotal + cls.return_value.metadata = _valid_metadata() if metadata is None else metadata + cls.return_value.dim = dim + + +def _legacy_dir(path): + path.mkdir(parents=True) + (path / "index.faiss").write_bytes(b"index") + (path / "metadata.pkl").write_bytes(b"metadata") + return path + + +def test_legacy_validation_rejects_missing_artifacts(tmp_path): + directory = tmp_path / "legacy" + directory.mkdir() + (directory / "index.faiss").write_bytes(b"index") + result = validate_index_directory(directory, allow_legacy=True) + assert not result["ok"] and "legacy index missing artifacts" in result["reason"] + + +def test_legacy_validation_rejects_a_store_that_does_not_load(tmp_path): + directory = _legacy_dir(tmp_path / "legacy") + with patch("index.lifecycle.VectorStore") as cls: + _mock_store(cls, loads=False) + result = validate_index_directory(directory, allow_legacy=True) + assert not result["ok"] and result["reason"] == "legacy vector store did not load" + + +def test_legacy_validation_rejects_an_inconsistent_store(tmp_path): + directory = _legacy_dir(tmp_path / "legacy") + with patch("index.lifecycle.VectorStore") as cls: + _mock_store(cls, ntotal=3) + result = validate_index_directory(directory, allow_legacy=True) + assert not result["ok"] and "legacy index inconsistent" in result["reason"] + + +def test_legacy_validation_accepts_a_consistent_store(tmp_path): + directory = _legacy_dir(tmp_path / "legacy") + with patch("index.lifecycle.VectorStore") as cls: + _mock_store(cls) + result = validate_index_directory(directory, allow_legacy=True) + assert result["ok"] and result["legacy"] is True and result["metadata_count"] == 1 + + +def test_validate_index_directory_rejects_missing_artifacts(tmp_path): + directory = tmp_path / "candidate" + directory.mkdir() + (directory / "index.faiss").write_bytes(b"index") + result = validate_index_directory(directory) + assert not result["ok"] and "missing artifacts" in result["reason"] + assert "metadata.pkl" in result["reason"] and "manifest.json" in result["reason"] + + +def test_validate_index_directory_rejects_a_store_that_does_not_load(tmp_path): + directory = _index_dir(tmp_path / "candidate") + with patch("index.lifecycle.VectorStore") as cls: + _mock_store(cls, loads=False) + result = validate_index_directory(directory) + assert not result["ok"] and result["reason"] == "vector store did not load" + + +def test_validate_index_directory_rejects_wrong_dimension(tmp_path): + directory = _index_dir(tmp_path / "candidate") + with patch("index.lifecycle.VectorStore") as cls: + _mock_store(cls, dim=384) + result = validate_index_directory(directory) + assert not result["ok"] and result["reason"] == "dimension mismatch: 384" + + +def test_validate_index_directory_rejects_absolute_path_only_sources(tmp_path): + directory = _index_dir(tmp_path / "candidate") + with patch("index.lifecycle.VectorStore") as cls: + _mock_store(cls, metadata=[{**_valid_metadata()[0], "source": "/home/user/repo/README.md"}]) + result = validate_index_directory(directory) + assert not result["ok"] and "absolute-path-only source identities" in result["reason"] + + +def test_candidate_dir_is_build_id_under_staging_root(tmp_path): + from index.lifecycle import candidate_dir + + assert candidate_dir(tmp_path, "b7") == tmp_path / "b7" + assert candidate_dir(str(tmp_path), "b7") == tmp_path / "b7" diff --git a/tests/test_rag_engine.py b/tests/test_rag_engine.py index 024b34a..e635e2e 100644 --- a/tests/test_rag_engine.py +++ b/tests/test_rag_engine.py @@ -50,6 +50,31 @@ def test_ollama_embed_dim_mismatch(mock_post): with pytest.raises(ValueError, match="Embedding dim mismatch"): ollama_embed("test text") +@patch("rag.engine.time.sleep") +@patch("rag.engine.requests.post") +def test_ollama_embed_reports_each_attempt_to_on_attempt(mock_post, mock_sleep): + """Call on_attempt after every attempt, failed or successful, without changing the retry behavior.""" + mock_response = MagicMock() + mock_response.json.return_value = {"embedding": [0.1] * 768} + mock_post.side_effect = [requests.exceptions.ConnectionError("down"), mock_response] + attempts = [] + + vec = ollama_embed("test text", on_attempt=lambda n, ok: attempts.append((n, ok))) + assert vec.shape == (768,) + assert attempts == [(1, False), (2, True)] + mock_sleep.assert_called_once_with(2) + +@patch("rag.engine.time.sleep") +@patch("rag.engine.requests.post") +def test_ollama_embed_reports_every_failed_attempt_before_raising(mock_post, mock_sleep): + """Report all three failed attempts to on_attempt, then re-raise the last error.""" + mock_post.side_effect = requests.exceptions.ConnectionError("down") + attempts = [] + + with pytest.raises(requests.exceptions.ConnectionError): + ollama_embed("test text", on_attempt=lambda n, ok: attempts.append((n, ok))) + assert attempts == [(1, False), (2, False), (3, False)] + # ========================================================= # UNIT TESTS FOR ollama_generate # ========================================================= diff --git a/tests/test_trusted_ingestion.py b/tests/test_trusted_ingestion.py index 6879bf5..ae58bbf 100644 --- a/tests/test_trusted_ingestion.py +++ b/tests/test_trusted_ingestion.py @@ -164,3 +164,110 @@ def test_chunk_metadata_carries_bundle_for_api_scope_filter(tmp_path): assert by_path["README.md"]["bundle"] is None chunk = chunks_for_document(by_path["atacseq/README.md"], "b1", "nomic-embed-text")[0] assert chunk["bundle"] == "atacseq" + + +@pytest.mark.parametrize("text,expected", [ + ("intro\n# Title Here\nbody", "Title Here"), + ("# \nbody", "fallback"), + ("no heading at all", "fallback"), +]) +def test_title_from_markdown_uses_first_h1_or_fallback(text, expected): + from ingestion.trusted import title_from_markdown + + assert title_from_markdown(text, "fallback") == expected + + +@pytest.mark.parametrize("repo,path,expected", [ + ("omnibioai-tes", "docs/README.md", "README"), + ("omnibioai-docs", "site/docs/page.md", "PUBLICATION_PAGE"), + ("omnibioai-tes", "docs/security/model.md", "SECURITY_DOCUMENTATION"), + ("omnibioai-tes", "docs/testing.md", "TEST_SOURCE"), + ("omnibioai-tes", "docs/guide.md", "DOCUMENTATION"), +]) +def test_document_type_for_classifies_by_name_repo_and_path(repo, path, expected): + from ingestion.trusted import document_type_for + + assert document_type_for(repo, path) == expected + + +def test_source_policy_skips_denylisted_dirs_and_skip_segment_files(tmp_path): + policy = SourcePolicy() + assert not policy.should_walk_dir(tmp_path, tmp_path, "node_modules") + assert policy.should_walk_dir(tmp_path, tmp_path, "docs") + assert not policy.should_walk_dir(tmp_path / "archive", tmp_path, "docs") + assert not policy.should_select_file("omnibioai-docs", "archive/notes.md") + assert not policy.should_select_file("omnibioai-tes", "docs/guide.txt") + + +def test_files_under_a_skip_segment_directory_are_not_discovered(tmp_path): + repo = tmp_path / "omnibioai-tes" + (repo / "docs").mkdir(parents=True) + (repo / "archive").mkdir() + (repo / "docs/guide.md").write_text("# Guide\n\nDocs") + (repo / "archive/README.md").write_text("# Old\n\nStale") + + found, _ = discover_documents(str(tmp_path), SourcePolicy(repository_names=["omnibioai-tes"])) + + assert [d["relative_path"] for d in found] == ["docs/guide.md"] + + +def test_unreadable_empty_and_missing_sources_are_reported(tmp_path, monkeypatch): + from pathlib import Path + + repo = tmp_path / "omnibioai-tes" + (repo / "docs").mkdir(parents=True) + (repo / "docs/empty.md").write_text(" \n") + (repo / "docs/broken.md").write_text("# Broken") + real_read_text = Path.read_text + + def read_text(self, *args, **kwargs): + if self.name == "broken.md": + raise OSError("disk error") + return real_read_text(self, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", read_text) + found, stats = discover_documents(str(tmp_path), SourcePolicy(repository_names=["omnibioai-tes", "omnibioai-missing"])) + + assert found == [] + assert stats["failures"] == [{"repo": "omnibioai-tes", "path": "docs/broken.md", "reason": "disk error"}] + assert stats["skipped_documents"] == [{"repo": "omnibioai-tes", "path": "docs/empty.md", "reason": "empty"}] + assert stats["missing_repositories"] == ["omnibioai-missing"] + + +def test_invalid_resolved_visibility_fails_closed_to_review_required(tmp_path, monkeypatch): + from ingestion.trusted import VisibilityResolver + + repo = tmp_path / "omnibioai-tes" + repo.mkdir() + (repo / "README.md").write_text("# TES\n\nReadme") + monkeypatch.setattr(VisibilityResolver, "resolve", lambda self, repo_name, rel_path: ("SECRET", "bogus")) + + found, stats = discover_documents(str(tmp_path), SourcePolicy(repository_names=["omnibioai-tes"])) + assert found == [] and stats["skipped_documents"][0]["reason"] == "review-required" + + found, _ = discover_documents(str(tmp_path), SourcePolicy(repository_names=["omnibioai-tes"], include_review_required=True)) + assert found[0]["visibility"] == "REVIEW_REQUIRED" + assert found[0]["visibility_source"] == "invalid-visibility-fail-closed" + + +def test_build_manifest_counts_visibility_documents_and_revisions(): + from ingestion.trusted import build_manifest + + metadata = [ + {"repo": "a", "document_id": "d1", "source_revision": "r1", "visibility": "PUBLIC"}, + {"repo": "a", "document_id": "d1", "source_revision": "r1", "visibility": "PUBLIC"}, + {"repo": "b", "document_id": "d2", "source_revision": "r2", "visibility": "INTERNAL"}, + {"repo": "b", "document_id": "d3", "source_revision": "r2"}, + ] + stats = {"configured_repositories": ["a", "b", "c"], "repositories_discovered": ["a", "b"], + "missing_repositories": ["c"], "skipped_documents": [{"path": "x"}], "failures": []} + + m = build_manifest("b1", metadata, stats, embedding_provider="ollama", embedding_model="nomic-embed-text", + embedding_identity="ollama:nomic-embed-text", embedding_dimension=768) + + assert m["build_id"] == "b1" and m["vector_backend"] == "FAISS IndexFlatIP" + assert m["visibility_counts"] == {"INTERNAL": 1, "PUBLIC": 2, "REVIEW_REQUIRED": 1} + assert m["parsed"] == 3 and m["chunked"] == 4 and m["selected_document_count"] == 3 + assert m["repositories"] == ["a", "b"] and m["repository_revisions"] == {"a": "r1", "b": "r2"} + assert m["missing_repositories"] == ["c"] and m["discovered"] == ["a", "b"] + assert m["build_status"] == "CLEAN" and m["promotion_status"] == "CANDIDATE"