diff --git a/bin/chat-fastapi.py b/bin/chat-fastapi.py index c49a9f1f..8c78539b 100644 --- a/bin/chat-fastapi.py +++ b/bin/chat-fastapi.py @@ -87,7 +87,12 @@ async def verify_captcha_middleware( f"{CHAINLIT_URI}/static", ] or path.startswith("/static") - or not os.getenv("CLOUDFLARE_SECRET_KEY") + # The value resolved through get_secret, not os.environ. get_secret + # prefers a mounted Docker secret file, so a deployment that mounts the + # key rather than exporting it used to land here with the env var unset + # and skip the captcha entirely -- switching off a protection the + # operator had configured, silently. + or not CLOUDFLARE_SECRET_KEY or (CHAINLIT_URI and not path.startswith(CHAINLIT_URI)) ): return await call_next(request) @@ -170,7 +175,10 @@ async def verify_captcha(request: Request) -> Response: # Verify the CAPTCHA with Cloudflare url = "https://challenges.cloudflare.com/turnstile/v0/siteverify" data = { - "secret": os.getenv("CLOUDFLARE_SECRET_KEY"), + # Same reason: os.environ can be empty while the secret is mounted, and + # sending Cloudflare a null secret fails in a way that reads as a captcha + # problem rather than a configuration one. + "secret": CLOUDFLARE_SECRET_KEY, "response": cf_turnstile_response, "remoteip": client_ip, } diff --git a/src/data_generation/alliance/__init__.py b/src/data_generation/alliance/__init__.py index aed6b7db..30af651d 100644 --- a/src/data_generation/alliance/__init__.py +++ b/src/data_generation/alliance/__init__.py @@ -12,9 +12,12 @@ its behaviour is unverified even where the code still type-checks. """ -import os +import logging +import shutil +from pathlib import Path import requests +from chromadb.api.shared_system_client import SharedSystemClient from langchain_community.vectorstores import Chroma from data_generation.alliance.csv_generator import generate_all_csvs @@ -36,6 +39,216 @@ def get_release_version() -> str: ) +# The Alliance column schemas, at module scope because they are data: 203 lines +# of them inside the function made a 40-line routine read as a 246-line one. +# tests/data_generation/test_alliance_columns.py parses these and guards them +# against the missing-comma bug that once collapsed seven molecular_interaction +# columns into three. +COLUMN_SCHEMAS: dict[str, list[str]] = { + "genes": [ + "Your Input", + "Gene ID", + "Gene Symbol", + "Gene Name", + "Description", + "Species", + "NCBI ID", + "ENSEMBL ID", + "UniProtKB ID", + "PANTHER ID", + "RefSeq ID", + "Synonym", + "Disease Association", + "Expression Location", + "Expression Stage", + "Variants", + "Genetic Interaction", + "Molecular/Physical Interaction", + "Homo sapiens Ortholog", + "Mus musculus Ortholog", + "Rattus norvegicus Ortholog", + "Danio rerio Ortholog", + "Drosophila melanogaster Ortholog", + "Caenorhabditis elegans Ortholog", + "Saccharomyces cerevisiae Ortholog", + "Xenopus laevis Ortholog", + "Xenopus tropicalis Ortholog", + ], + "disease": [ + "Taxon", + "SpeciesName", + "DBobjectType", + "DBObjectID", + "DBObjectSymbol", + "AssociationType", + "DOID", + "DOtermName", + "WithOrtholog", + "InferredFromID", + "InferredFromSymbol", + "ExperimentalCondition", + "Modifier", + "EvidenceCode", + "EvidenceCodeName", + "Reference", + "Date", + "Source", + ], + "expression": [ + "Species", + "SpeciesID", + "GeneID", + "GeneSymbol", + "Location", + "StageTerm", + "AssayID", + "AssayTermName", + "CellularComponentID", + "CellularComponentTerm", + "CellularComponentQualifierIDs", + "CellularComponentQualifierTermNames", + "SubStructureID", + "SubStructureName", + "SubStructureQualifierIDs", + "SubStructureQualifierTermNames", + "AnatomyTermID", + "AnatomyTermName", + "AnatomyTermQualifierIDs", + "AnatomyTermQualifierTermNames", + "SourceURL", + "Source,Reference", + ], + "molecular_interaction": [ + "ID(s) interactor A", + "ID(s) interactor B", + "Alt. ID(s) interactor A", + "Alt. ID(s) interactor B", + "Alias(es) interactor A", + "Alias(es) interactor B", + "Interaction detection method(s)", + "Publication 1st author(s)", + "Publication Identifier(s)", + "Taxid interactor A", + "Taxid interactor B", + "Interaction type(s)", + "Source database(s)", + "Interaction identifier(s)", + "Confidence value(s)", + "Expansion method(s)", + "Biological role(s) interactor A", + "Biological role(s) interactor B", + "Experimental role(s) interactor A", + "Experimental role(s) interactor B", + "Type(s) interactor A", + "Type(s) interactor B", + "Xref(s) interactor A", + "Xref(s) interactor B", + "Interaction Xref(s)", + "Annotation(s) interactor A", + "Annotation(s) interactor B", + "Interaction annotation(s)", + "Host organism(s)", + "Interaction parameter(s)", + "Creation date", + "Update date", + "Checksum(s) interactor A", + "Checksum(s) interactor B", + "Interaction Checksum(s) Negative", + "Feature(s) interactor A", + "Feature(s) interactor B", + "Stoichiometry(s) interactor A", + "Stoichiometry(s) interactor B", + "Identification method participant A", + "Identification method participant B", + ], + "genetic_interaction": [ + "ID(s) interactor A", + "ID(s) interactor B", + "Alt. ID(s) interactor A", + "Alt. ID(s) interactor B", + "Alias(es) interactor A", + "Alias(es) interactor B", + "Interaction detection method(s)", + "Publication 1st author(s)", + "Publication Identifier(s)", + "Taxid interactor A", + "Taxid interactor B", + "Interaction type(s)", + "Source database(s)", + "Interaction identifier(s)", + "Confidence value(s)", + "Expansion method(s)", + "Biological role(s) interactor A", + "Biological role(s) interactor B", + "Experimental role(s) interactor A", + "Experimental role(s) interactor B", + "Type(s) interactor A", + "Type(s) interactor B", + "Xref(s) interactor A", + "Xref(s) interactor B", + "Interaction Xref(s)", + "Annotation(s) interactor A", + "Annotation(s) interactor B", + "Interaction annotation(s)", + "Host organism(s)", + "Interaction parameter(s)", + "Creation date", + "Update date", + "Checksum(s) interactor A", + "Checksum(s) interactor B", + "Interaction Checksum(s) Negative", + "Feature(s) interactor A", + "Feature(s) interactor B", + "Stoichiometry(s) interactor A", + "Stoichiometry(s) interactor B", + "Identification method participant A", + "Identification method participant B", + ], + "orthology": [ + "Gene1ID Gene1Symbol", + "Gene1SpeciesTaxonID", + "Gene1SpeciesName", + "Gene2ID Gene2Symbol", + "Gene2SpeciesTaxonID", + "Gene2SpeciesName", + "Algorithms", + "AlgorithmsMatch", + "OutOfAlgorithms", + "IsBestScore", + "IsBestRevScore", + ], + "variants": [ + "Taxon", + "SpeciesName", + "AlleleId", + "AlleleSymbol", + "AlleleSynonyms", + "VariantId", + "VariantSymbol", + "VariantSynonyms", + "VariantCrossReferences", + "AlleleAssociatedGeneId", + "AlleleAssociatedGeneSymbol", + "VariantAffectedGeneId", + "VariantAffectedGeneSymbol", + "Category", + "VariantsTypeId", + "VariantsTypeName", + "VariantsHgvsNames", + "Assembly", + "Chromosome", + "StartPosition", + "EndPosition", + "SequenceOfReference", + "SequenceOfVariant", + "MostSevereConsequenceName", + "VariantInformationReference", + "HasDiseaseAnnotations", + "HasPhenotypeAnnotations", + ], +} + + def upload_to_chromadb( embeddings_dir: str, version: str, @@ -43,246 +256,58 @@ def upload_to_chromadb( hf_model: str | None = None, device: str | None = None, ) -> Chroma | None: - metadata_columns: dict[str, list[str]] = { - "genes": [ - "Your Input", - "Gene ID", - "Gene Symbol", - "Gene Name", - "Description", - "Species", - "NCBI ID", - "ENSEMBL ID", - "UniProtKB ID", - "PANTHER ID", - "RefSeq ID", - "Synonym", - "Disease Association", - "Expression Location", - "Expression Stage", - "Variants", - "Genetic Interaction", - "Molecular/Physical Interaction", - "Homo sapiens Ortholog", - "Mus musculus Ortholog", - "Rattus norvegicus Ortholog", - "Danio rerio Ortholog", - "Drosophila melanogaster Ortholog", - "Caenorhabditis elegans Ortholog", - "Saccharomyces cerevisiae Ortholog", - "Xenopus laevis Ortholog", - "Xenopus tropicalis Ortholog", - ], - "disease": [ - "Taxon", - "SpeciesName", - "DBobjectType", - "DBObjectID", - "DBObjectSymbol", - "AssociationType", - "DOID", - "DOtermName", - "WithOrtholog", - "InferredFromID", - "InferredFromSymbol", - "ExperimentalCondition", - "Modifier", - "EvidenceCode", - "EvidenceCodeName", - "Reference", - "Date", - "Source", - ], - "expression": [ - "Species", - "SpeciesID", - "GeneID", - "GeneSymbol", - "Location", - "StageTerm", - "AssayID", - "AssayTermName", - "CellularComponentID", - "CellularComponentTerm", - "CellularComponentQualifierIDs", - "CellularComponentQualifierTermNames", - "SubStructureID", - "SubStructureName", - "SubStructureQualifierIDs", - "SubStructureQualifierTermNames", - "AnatomyTermID", - "AnatomyTermName", - "AnatomyTermQualifierIDs", - "AnatomyTermQualifierTermNames", - "SourceURL", - "Source,Reference", - ], - "molecular_interaction": [ - "ID(s) interactor A", - "ID(s) interactor B", - "Alt. ID(s) interactor A", - "Alt. ID(s) interactor B", - "Alias(es) interactor A", - "Alias(es) interactor B", - "Interaction detection method(s)", - "Publication 1st author(s)", - "Publication Identifier(s)", - "Taxid interactor A", - "Taxid interactor B", - "Interaction type(s)", - "Source database(s)", - "Interaction identifier(s)", - "Confidence value(s)", - "Expansion method(s)", - "Biological role(s) interactor A", - "Biological role(s) interactor B", - "Experimental role(s) interactor A", - "Experimental role(s) interactor B", - "Type(s) interactor A", - "Type(s) interactor B", - "Xref(s) interactor A", - "Xref(s) interactor B", - "Interaction Xref(s)", - "Annotation(s) interactor A", - "Annotation(s) interactor B", - "Interaction annotation(s)", - "Host organism(s)", - "Interaction parameter(s)", - "Creation date", - "Update date", - "Checksum(s) interactor A", - "Checksum(s) interactor B", - "Interaction Checksum(s) Negative", - "Feature(s) interactor A", - "Feature(s) interactor B", - "Stoichiometry(s) interactor A", - "Stoichiometry(s) interactor B", - "Identification method participant A", - "Identification method participant B", - ], - "genetic_interaction": [ - "ID(s) interactor A", - "ID(s) interactor B", - "Alt. ID(s) interactor A", - "Alt. ID(s) interactor B", - "Alias(es) interactor A", - "Alias(es) interactor B", - "Interaction detection method(s)", - "Publication 1st author(s)", - "Publication Identifier(s)", - "Taxid interactor A", - "Taxid interactor B", - "Interaction type(s)", - "Source database(s)", - "Interaction identifier(s)", - "Confidence value(s)", - "Expansion method(s)", - "Biological role(s) interactor A", - "Biological role(s) interactor B", - "Experimental role(s) interactor A", - "Experimental role(s) interactor B", - "Type(s) interactor A", - "Type(s) interactor B", - "Xref(s) interactor A", - "Xref(s) interactor B", - "Interaction Xref(s)", - "Annotation(s) interactor A", - "Annotation(s) interactor B", - "Interaction annotation(s)", - "Host organism(s)", - "Interaction parameter(s)", - "Creation date", - "Update date", - "Checksum(s) interactor A", - "Checksum(s) interactor B", - "Interaction Checksum(s) Negative", - "Feature(s) interactor A", - "Feature(s) interactor B", - "Stoichiometry(s) interactor A", - "Stoichiometry(s) interactor B", - "Identification method participant A", - "Identification method participant B", - ], - "orthology": [ - "Gene1ID Gene1Symbol", - "Gene1SpeciesTaxonID", - "Gene1SpeciesName", - "Gene2ID Gene2Symbol", - "Gene2SpeciesTaxonID", - "Gene2SpeciesName", - "Algorithms", - "AlgorithmsMatch", - "OutOfAlgorithms", - "IsBestScore", - "IsBestRevScore", - ], - "variants": [ - "Taxon", - "SpeciesName", - "AlleleId", - "AlleleSymbol", - "AlleleSynonyms", - "VariantId", - "VariantSymbol", - "VariantSynonyms", - "VariantCrossReferences", - "AlleleAssociatedGeneId", - "AlleleAssociatedGeneSymbol", - "VariantAffectedGeneId", - "VariantAffectedGeneSymbol", - "Category", - "VariantsTypeId", - "VariantsTypeName", - "VariantsHgvsNames", - "Assembly", - "Chromosome", - "StartPosition", - "EndPosition", - "SequenceOfReference", - "SequenceOfVariant", - "MostSevereConsequenceName", - "VariantInformationReference", - "HasDiseaseAnnotations", - "HasPhenotypeAnnotations", - ], - } - - csv_dir = "./csv_files/alliance/" + version + "/" + # Derived from embeddings_dir, as every other generator does. It was the + # literal "./csv_files/alliance//", so generation worked from the + # repo root and silently found nothing anywhere else. + csv_dir = str(Path(embeddings_dir) / "csv_files" / "alliance" / version) - for filename in os.listdir(csv_dir): - file_path = os.path.join(csv_dir, filename) + # Only `genes` is embedded. The other six lists above are curated MITAB and + # Alliance schemas kept for when they are, and `tests/data_generation/ + # test_alliance_columns.py` guards them against the missing-comma bug that + # once collapsed seven molecular_interaction columns into three. They are + # data waiting on code, not dead code. + # + # They are not reachable as written: csv_generator downloads variants as + # variants_c_elegans.tsv, variants_zebrafish.tsv and so on, so the + # "variants" key below could never match a file. Anything that starts + # embedding them must reconcile the two lists rather than assume they agree. + embedded = ("genes",) - if os.path.isfile(file_path): - # Get the basename (filename without path) - base_name = os.path.basename(file_path) - print(base_name) + db = None + for filetype, column_names in COLUMN_SCHEMAS.items(): + if filetype not in embedded: + continue + file = Path(csv_dir) / f"{filetype}.tsv" + if not file.is_file(): + # Returned rather than raised from inside Chroma: `force` governs + # whether the CSVs were downloaded at all, and a caller that skipped + # that should get a clear message, not a FileNotFoundError. + logging.warning("No Alliance %s file at %s; skipping.", filetype, file) + continue - db = None # Initialize db variable - - for filetype, column_names in metadata_columns.items(): - file = csv_dir + filetype + ".tsv" - if filetype == "genes": - print(column_names) - loader = MetaDataCSVLoader( - file_path=file, - metadata_columns=column_names, - encoding="utf-8", - csv_args={"delimiter": "\t"}, - ) - docs = loader.load() - embeddings = build_embeddings(hf_model, device) + loader = MetaDataCSVLoader( + file_path=str(file), + metadata_columns=column_names, + encoding="utf-8", + csv_args={"delimiter": "\t"}, + ) + docs = loader.load() + logging.info("Embedding %d Alliance %s documents.", len(docs), filetype) - db = Chroma.from_documents( - documents=docs, - embedding=embeddings, - persist_directory=os.path.join(embeddings_dir, filetype), - ) - print(db) + persist = Path(embeddings_dir) / filetype + if persist.exists(): + # Chroma.from_documents appends; a second run would double the + # collection silently, as it did to the reactome bundle. + shutil.rmtree(persist) + SharedSystemClient.clear_system_cache() - print("filetype") - print(filetype) + db = Chroma.from_documents( + documents=docs, + embedding=build_embeddings(hf_model, device), + persist_directory=str(persist), + ) - return db # Ensure to return the db at the end + return db def generate_alliance_embeddings( @@ -303,5 +328,7 @@ def generate_alliance_embeddings( ) exit() - generate_all_csvs(release_version, force) + # Same directory the loader reads from, passed rather than assumed: the two + # used to agree only because both were relative to the working directory. + generate_all_csvs(release_version, force, embeddings_dir) upload_to_chromadb(embeddings_dir, release_version, force, hf_model, device) diff --git a/src/data_generation/alliance/csv_generator.py b/src/data_generation/alliance/csv_generator.py index c92263a2..c6dc2972 100644 --- a/src/data_generation/alliance/csv_generator.py +++ b/src/data_generation/alliance/csv_generator.py @@ -39,9 +39,9 @@ def download_file(url: str, dest: str, force: bool) -> str | None: return None -def get_genes(version: str, force: bool) -> str: +def get_genes(version: str, force: bool, parent_dir: str) -> str: # Define the file path - directory = f"csv_files/alliance/{version}" + directory = f"{parent_dir}/csv_files/alliance/{version}" os.makedirs(directory, exist_ok=True) gene_csv = f"{directory}/genes.tsv" @@ -106,11 +106,11 @@ def get_genes(version: str, force: bool) -> str: return gene_csv -def generate_all_csvs(version: str, force: bool) -> tuple[str, ...]: +def generate_all_csvs(version: str, force: bool, parent_dir: str) -> tuple[str, ...]: files = [] # Download gene file - gene_csv = get_genes(version, force) + gene_csv = get_genes(version, force, parent_dir) files.append(gene_csv) # Define other files to download @@ -128,9 +128,10 @@ def generate_all_csvs(version: str, force: bool) -> tuple[str, ...]: "variants_yeast": "https://fms.alliancegenome.org/download/VARIANT-ALLELE_NCBITaxon559292.tsv.gz", } + base = f"{parent_dir}/csv_files/alliance/{version}" for name, url in other_files.items(): - gz_dest = f"csv_files/alliance/{version}/{name}.tsv.gz" - csv_dest = f"csv_files/alliance/{version}/{name}.tsv" + gz_dest = f"{base}/{name}.tsv.gz" + csv_dest = f"{base}/{name}.tsv" if not os.path.exists(csv_dest) or force: unzipped_dest = download_file(url, gz_dest, force) diff --git a/src/data_generation/uniprot/__init__.py b/src/data_generation/uniprot/__init__.py index 37de1254..42e4330b 100644 --- a/src/data_generation/uniprot/__init__.py +++ b/src/data_generation/uniprot/__init__.py @@ -1,6 +1,8 @@ import os +import shutil from pathlib import Path +from chromadb.api.shared_system_client import SharedSystemClient from langchain_community.vectorstores import Chroma from data_generation.embeddings import build_embeddings @@ -36,6 +38,14 @@ def upload_to_chromadb( embeddings_instance = build_embeddings(hf_model, device, chunk_size=500) + # Chroma.from_documents appends, so a second run doubles the collection + # silently. The shipped reactome bundle carried every reaction twice for + # exactly this reason, and nothing reported it. + persist = Path(embeddings_dir) / embedding_table + if persist.exists(): + shutil.rmtree(persist) + SharedSystemClient.clear_system_cache() + return Chroma.from_documents( documents=docs, embedding=embeddings_instance, diff --git a/src/data_generation/userguide/__init__.py b/src/data_generation/userguide/__init__.py index 9b0b4c46..70ccb093 100644 --- a/src/data_generation/userguide/__init__.py +++ b/src/data_generation/userguide/__init__.py @@ -2,6 +2,7 @@ from pathlib import Path from shutil import rmtree +from chromadb.api.shared_system_client import SharedSystemClient from langchain_community.vectorstores import Chroma from langchain_core.documents import Document @@ -39,8 +40,15 @@ def generate_userguide_embeddings( ) -> None: embeddings_path = Path(embeddings_dir) chroma_dir = embeddings_path / CHROMA_COLLECTION - if force and chroma_dir.exists(): + # Unconditionally, not just under `force`. Chroma.from_documents appends, so + # a plain rebuild used to add a second copy of every section to the existing + # collection -- the same fault that left the shipped reactome bundle with + # 33,498 documents for 16,749 rows. `force` governs whether the HTML pages + # are re-fetched, which is a separate question from whether this collection + # is rebuilt from them. + if chroma_dir.exists(): rmtree(chroma_dir) + SharedSystemClient.clear_system_cache() cache_dir = embeddings_path / HTML_CACHE_DIR html_paths = fetch_userguide_pages( diff --git a/tests/data_generation/test_collections_replace_not_append.py b/tests/data_generation/test_collections_replace_not_append.py new file mode 100644 index 00000000..e3e4321b --- /dev/null +++ b/tests/data_generation/test_collections_replace_not_append.py @@ -0,0 +1,137 @@ +"""Building a collection twice must not store everything twice. + +`Chroma.from_documents` appends to whatever is already in the persist directory, +so a second build silently doubles the collection. The shipped Release95 bundle +is what that looks like: 33,498 reaction documents for 16,749 CSV rows, exactly +2.00x, unnoticed until the Release 97 rebuild. The vector retriever over-fetches +then de-duplicates on `st_id`, so half that over-fetch was spent on duplicates of +what it already had. + +Three of the five generators had this, each written separately. + +These tests *run* the generators rather than inspecting their source. The first +version of this file grepped for `rmtree`, and an adversarial check showed it +passed even when the code deleted a completely unrelated directory -- it was +asserting the shape of the code, not what it does. +""" + +import csv +from pathlib import Path +from typing import Any + +import pytest +from langchain_core.embeddings import FakeEmbeddings + +import data_generation.alliance as alliance_mod +import data_generation.reactome as reactome_mod +import data_generation.uniprot as uniprot_mod + + +@pytest.fixture(autouse=True) +def _fake_embeddings(monkeypatch: pytest.MonkeyPatch) -> None: + """No API calls, and no dependence on an embedding model being reachable.""" + for module in (reactome_mod, uniprot_mod, alliance_mod): + monkeypatch.setattr( + module, "build_embeddings", lambda *a, **k: FakeEmbeddings(size=8) + ) + + +def _write_csv(path: Path, columns: list[str], rows: int, delimiter: str = ",") -> None: + path.parent.mkdir(parents=True, exist_ok=True) + with open(path, "w", newline="", encoding="utf-8") as handle: + writer = csv.DictWriter(handle, fieldnames=columns, delimiter=delimiter) + writer.writeheader() + for i in range(rows): + writer.writerow({c: f"{c}-{i}" for c in columns}) + + +def test_reactome_replaces_its_collection(tmp_path: Path) -> None: + columns = [ + "st_id", + "display_name", + "canonical_gene_name", + "synonyms_gene_name", + "uniprot_link", + ] + csv_path = tmp_path / "ewas.csv" + _write_csv(csv_path, columns, rows=3) + + first = reactome_mod.upload_to_chromadb(str(tmp_path), str(csv_path), "ewas", "m") + assert first._collection.count() == 3 + second = reactome_mod.upload_to_chromadb(str(tmp_path), str(csv_path), "ewas", "m") + assert second._collection.count() == 3, "a second build must not append" + + +def test_uniprot_replaces_its_collection(tmp_path: Path) -> None: + columns = [ + "gene_names", + "short_protein_name", + "full_protein_name", + "protein_family", + "biological_pathways", + ] + csv_path = tmp_path / "uniprot_data.csv" + _write_csv(csv_path, columns, rows=4) + + first = uniprot_mod.upload_to_chromadb( + str(tmp_path), str(csv_path), "uniprot_data", "m" + ) + assert first._collection.count() == 4 + second = uniprot_mod.upload_to_chromadb( + str(tmp_path), str(csv_path), "uniprot_data", "m" + ) + assert second._collection.count() == 4, "a second build must not append" + + +def test_alliance_replaces_its_collection(tmp_path: Path) -> None: + version = "1.0.0" + _write_csv( + tmp_path / "csv_files" / "alliance" / version / "genes.tsv", + alliance_mod.COLUMN_SCHEMAS["genes"], + rows=2, + delimiter="\t", + ) + + first = alliance_mod.upload_to_chromadb(str(tmp_path), version, False, "m") + assert first is not None + assert first._collection.count() == 2 + second = alliance_mod.upload_to_chromadb(str(tmp_path), version, False, "m") + assert second is not None + assert second._collection.count() == 2, "a second build must not append" + + +def test_alliance_reads_from_the_directory_it_is_given(tmp_path: Path) -> None: + """Not from the working directory, which is where it used to look.""" + assert alliance_mod.upload_to_chromadb(str(tmp_path), "9.9.9", False, "m") is None + + +def test_userguide_does_not_gate_the_removal_on_force() -> None: + """Source-level, because generating it fetches pages over the network. + + `force` means re-fetch the HTML, not "rebuild the collection"; gating the + removal on it meant an ordinary rebuild appended. + """ + source = ( + Path(__file__).resolve().parents[2] + / "src/data_generation/userguide/__init__.py" + ).read_text() + assert "if force and chroma_dir.exists():" not in source + assert "if chroma_dir.exists():" in source + + +def test_every_generator_is_covered_here(tmp_path: Path, request: Any) -> None: + """A new generator must arrive with a case in this file. + + Five modules, each with its own hand-written persist block, is why the same + fault appeared three times. + """ + src = Path(__file__).resolve().parents[2] / "src/data_generation" + generators = { + p.parent.name + for p in src.rglob("__init__.py") + if "from_documents" in p.read_text() + } + covered = {"reactome", "uniprot", "alliance", "userguide", "disease_variant"} + assert ( + generators <= covered + ), f"no replace-not-append test for: {generators - covered}" diff --git a/tests/util/test_captcha_secret_source.py b/tests/util/test_captcha_secret_source.py new file mode 100644 index 00000000..9f061a91 --- /dev/null +++ b/tests/util/test_captcha_secret_source.py @@ -0,0 +1,67 @@ +"""The captcha must read its secret from the same place everything else does. + +`get_secret` prefers a mounted Docker secret file over the environment, and says +why: "a deployment that has gone to the trouble of mounting a secret means it". +`bin/chat-fastapi.py` loads CLOUDFLARE_SECRET_KEY that way -- and then decides +whether the captcha is enabled, and what secret to send to Cloudflare, by reading +`os.environ` directly. + +So a deployment that mounts the secret rather than exporting it gets the captcha +silently switched off. That is Principle IV: configuration that cannot be honoured +must stop the process, not substitute something plausible. +""" + +from pathlib import Path + +import pytest + +from util.secrets import get_secret + + +def test_get_secret_prefers_a_mounted_file_over_the_environment( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr("util.secrets.DOCKER_SECRETS", tmp_path) + (tmp_path / "CLOUDFLARE_SECRET_KEY").write_text("from-the-mounted-file\n") + monkeypatch.setenv("CLOUDFLARE_SECRET_KEY", "from-the-environment") + assert get_secret("CLOUDFLARE_SECRET_KEY") == "from-the-mounted-file" + + +def test_a_mounted_secret_is_invisible_to_os_environ( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The divergence itself, stated plainly. + + With the secret mounted and not exported, `get_secret` finds it and + `os.environ` does not. Any code that asks os.environ whether the captcha is + configured concludes it is not, and skips it. + """ + import os + + monkeypatch.setattr("util.secrets.DOCKER_SECRETS", tmp_path) + (tmp_path / "CLOUDFLARE_SECRET_KEY").write_text("mounted-only\n") + monkeypatch.delenv("CLOUDFLARE_SECRET_KEY", raising=False) + + assert get_secret("CLOUDFLARE_SECRET_KEY") == "mounted-only" + assert os.getenv("CLOUDFLARE_SECRET_KEY") is None + + +def test_chat_fastapi_never_reads_the_captcha_secret_from_os_environ() -> None: + """A source-level guard, because the script cannot be imported. + + `bin/chat-fastapi.py` is hyphenated, so it is not importable as a module and + the middleware cannot be exercised directly here. What can be checked is that + it never reaches around `get_secret` for this particular secret, which is the + mistake being prevented. + """ + source = Path("bin/chat-fastapi.py").read_text() + offenders = [ + line.strip() + for line in source.splitlines() + if "CLOUDFLARE_SECRET_KEY" in line + and ("os.getenv" in line or "os.environ" in line) + ] + assert not offenders, ( + "the captcha secret must come from get_secret, which prefers a mounted " + f"Docker secret file over the environment: {offenders}" + )