From 64e658bb33ea85ea2272a23b0b33cd8c819cbe56 Mon Sep 17 00:00:00 2001 From: "marcin p. joachimiak" <4625870+realmarcin@users.noreply.github.com> Date: Sun, 2 Aug 2026 00:16:40 -0700 Subject: [PATCH 1/2] Gate snippets truncated at a genus abbreviation; fix the four found (#299) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **The obvious fix would have been wrong.** #299 proposed a minimum-length rule for evidence snippets. Measuring first killed that idea: of 5106 snippets, 627 are under 40 characters and the shortest ones are correct — `pH 6.8`, `150 g/L`, `glucose 10 g/L`, `Firmicutes`. Those are exactly the right evidence for a pH, a concentration or a taxon, so a length floor would have buried a real signal under hundreds of false positives. The signature that does work is specific and self-verifying: the snippet ends in a single capital letter and a period, *and the cached reference continues with a lowercase word*. The second half is what gives it precision — it separates a truncated genus from a sentence that legitimately ends in an abbreviation or a unit. Across the whole KB it flags 4 snippets, all genuine, with no false positives; `27 °C.` and `1.0 ppm F.` are correctly ignored because nothing lowercase follows them. All four are fixed. Each had stopped at precisely the word it existed to identify: The main products from C. -> + ljungdahlii fermentation at a pH of 6.0… …by P. aeruginosa and G. -> + sulfurreducens respectively. (x2) benzoate-fermenting bacterium S. -> + gentianae is approximately -45 kJ… The second signature from #299 — stopping immediately before a non-ASCII character in the cache, the U+2010 case — is deliberately *not* gated. It fires once repo-wide, on a snippet that ends a complete clause before an em-dash, so it is currently all false positive; the reasoning is recorded in the test's docstring rather than lost. Snippets absent from their cache (291) are skipped rather than reported: that is a stale-or-mis-rendered cache, which the snippet audit already buckets, and conflating it with truncation would make this gate untrustworthy. Mutation-checked: re-truncating one snippet fails exactly its file's case. 899 tests pass; lint clean. Co-Authored-By: Claude Fable 5 --- ...lii_Kluyveri_Syngas_Alcohol_Coculture.yaml | 2 +- ...mate_Fumarate_Electroactive_Coculture.yaml | 4 +- ...pirillum_Gentianae_Benzoate_Coculture.yaml | 2 +- tests/test_snippet_truncation.py | 112 ++++++++++++++++++ 4 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 tests/test_snippet_truncation.py diff --git a/kb/communities/Clostridium_Ljungdahlii_Kluyveri_Syngas_Alcohol_Coculture.yaml b/kb/communities/Clostridium_Ljungdahlii_Kluyveri_Syngas_Alcohol_Coculture.yaml index dde1902ad..af5971a3c 100644 --- a/kb/communities/Clostridium_Ljungdahlii_Kluyveri_Syngas_Alcohol_Coculture.yaml +++ b/kb/communities/Clostridium_Ljungdahlii_Kluyveri_Syngas_Alcohol_Coculture.yaml @@ -423,7 +423,7 @@ related_ingredients: - reference: PMID:27877166 supports: SUPPORT evidence_source: IN_VITRO - snippet: The main products from C. + snippet: The main products from C. ljungdahlii fermentation at a pH of 6.0 were ethanol and acetate explanation: Introduces ethanol and acetate as the main C. ljungdahlii products. - reference: PMID:27877166 supports: SUPPORT diff --git a/kb/communities/Geobacter_Pseudomonas_Formate_Fumarate_Electroactive_Coculture.yaml b/kb/communities/Geobacter_Pseudomonas_Formate_Fumarate_Electroactive_Coculture.yaml index 24fccfbfb..5240cec9a 100644 --- a/kb/communities/Geobacter_Pseudomonas_Formate_Fumarate_Electroactive_Coculture.yaml +++ b/kb/communities/Geobacter_Pseudomonas_Formate_Fumarate_Electroactive_Coculture.yaml @@ -338,7 +338,7 @@ related_ingredients: - reference: PMID:28992596 supports: SUPPORT evidence_source: IN_VITRO - snippet: preferential utilization of formate and fumarate by P. aeruginosa and G. + snippet: preferential utilization of formate and fumarate by P. aeruginosa and G. sulfurreducens respectively. explanation: Snippet names formate explicitly as the substrate preferentially utilized by P. aeruginosa in the coculture medium. - preferred_term: fumarate @@ -352,7 +352,7 @@ related_ingredients: - reference: PMID:28992596 supports: SUPPORT evidence_source: IN_VITRO - snippet: preferential utilization of formate and fumarate by P. aeruginosa and G. + snippet: preferential utilization of formate and fumarate by P. aeruginosa and G. sulfurreducens respectively. explanation: Snippet names fumarate explicitly as the substrate preferentially utilized by G. sulfurreducens in the coculture medium. metals_present: [] diff --git a/kb/communities/Syntrophus_Methanospirillum_Gentianae_Benzoate_Coculture.yaml b/kb/communities/Syntrophus_Methanospirillum_Gentianae_Benzoate_Coculture.yaml index 78f7147c5..e556b96ae 100644 --- a/kb/communities/Syntrophus_Methanospirillum_Gentianae_Benzoate_Coculture.yaml +++ b/kb/communities/Syntrophus_Methanospirillum_Gentianae_Benzoate_Coculture.yaml @@ -78,7 +78,7 @@ taxonomy: - reference: PMID:33657722 supports: SUPPORT evidence_source: IN_VITRO - snippet: benzoate-fermenting bacterium S. + snippet: benzoate-fermenting bacterium S. gentianae is approximately -45 kJ (mol benzoate)-1 explanation: Supports the fermenting role of S. gentianae in the benzoate coculture. - taxon_term: preferred_term: Methanospirillum hungatei diff --git a/tests/test_snippet_truncation.py b/tests/test_snippet_truncation.py new file mode 100644 index 000000000..be0d32684 --- /dev/null +++ b/tests/test_snippet_truncation.py @@ -0,0 +1,112 @@ +"""Catch evidence snippets truncated at a genus abbreviation (#299). + +Snippet validation checks only that the text is a substring of the cached +reference. A fragment therefore validates exactly as well as a full clause, and +two mechanical truncations slip through unnoticed: + + snippet: The main products from C. # + " ljungdahlii fermentation…" + snippet: benzoate-fermenting bacterium S. # + " gentianae is approximately…" + +Both come from splitting sentences naively: an abbreviated genus reads as a full +stop, so the snippet stops at exactly the word it existed to identify. + +**Why there is no minimum-length rule here.** The obvious guard is a length +floor, and the data says it would be wrong. Of 5106 snippets, 627 are under 40 +characters and the shortest are legitimate — ``pH 6.8``, ``150 g/L``, +``glucose 10 g/L``, ``Firmicutes``. Those are precisely the right evidence for a +pH, a concentration, or a taxon, and a floor would bury a real signal under +hundreds of false positives. + +The signature used instead is specific and self-verifying: the snippet ends in a +single capital letter followed by a period, **and the cache continues with a +lowercase word**. That second half is what makes it precise — it distinguishes a +truncated genus from a sentence that legitimately ends in an abbreviation or a +unit. Measured over the whole KB it flagged 4 snippets, all genuine, and zero +false positives; ``27 °C.`` and ``1.0 ppm F.`` are correctly ignored because no +lowercase continuation follows. + +A second signature — a snippet stopping immediately before a non-ASCII character +present in the cache, which is how the U+2010 HYPHEN in isolate codes truncates +things — is **not** gated here. It fired once across the KB, on a snippet that +ends a complete clause before an em-dash, so it is currently all false positive. +It is documented in #299 in case isolate-code records make it worth revisiting. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest +import yaml + +COMMUNITIES = Path(__file__).parent.parent / "kb/communities" +CACHE = Path(__file__).parent.parent / "references_cache" + +# Snippet ends with a single capital + period, e.g. "… from C." — the shape an +# abbreviated genus leaves behind when a sentence splitter treats it as a stop. +_ABBREV_TAIL = re.compile(r"(?:^|\s)[A-Z]\.$") +# The cache continues with a lowercase word, i.e. the species epithet. +_LOWER_CONT = re.compile(r"^\s+[a-z]") + + +def _cache_text(reference: str) -> str | None: + """Return the normalised cached text for a reference, or None if uncached.""" + key = reference.replace("PMID:", "PMID_").replace("doi:", "DOI_").replace("/", "_") + for candidate in CACHE.glob(key + ".*"): + return " ".join(candidate.read_text(errors="replace").split()) + return None + + +def _evidence_items(node): + """Yield every evidence-shaped mapping anywhere in a record.""" + if isinstance(node, dict): + if "snippet" in node and "reference" in node: + yield node + for value in node.values(): + yield from _evidence_items(value) + elif isinstance(node, list): + for value in node: + yield from _evidence_items(value) + + +def _community_files() -> list[Path]: + return sorted(COMMUNITIES.glob("*.yaml")) + + +def test_there_are_community_files_to_check(): + """Guard against the glob matching nothing and the suite passing vacuously.""" + assert len(_community_files()) > 100 + + +@pytest.mark.parametrize("path", _community_files(), ids=lambda p: p.name) +def test_no_snippet_truncated_at_a_genus_abbreviation(path: Path): + """No snippet may stop at an abbreviated genus that the cache continues. + + Snippets absent from their cache are skipped: that is a different defect + (a stale or mis-rendered cache) with its own audit, and this test should not + report it as truncation. + """ + record = yaml.safe_load(path.read_text()) + offenders: list[str] = [] + + for item in _evidence_items(record): + snippet = " ".join(str(item.get("snippet") or "").split()) + cached = _cache_text(str(item.get("reference"))) + if not snippet or not cached: + continue + start = cached.find(snippet) + if start < 0: + continue + continuation = cached[start + len(snippet) :] + if _ABBREV_TAIL.search(snippet) and _LOWER_CONT.match(continuation): + offenders.append(f"{snippet[-52:]!r} + {continuation[:26]!r}") + + assert not offenders, ( + f"{path.name} has {len(offenders)} snippet(s) truncated at a genus " + f"abbreviation — the sentence splitter treated e.g. 'C.' as a full stop, " + f"so the snippet stops at the word it was meant to identify:\n " + + "\n ".join(offenders) + + "\nExtend each to the end of the clause; the continuation shown is what " + "the cached reference actually says next." + ) From 61925f8642ca26f86482d9136bb868606c96a7b1 Mon Sep 17 00:00:00 2001 From: "marcin p. joachimiak" <4625870+realmarcin@users.noreply.github.com> Date: Sun, 2 Aug 2026 19:44:22 -0700 Subject: [PATCH 2/2] Search every cache variant, not whichever one glob returned first (#306) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of this PR found the gate's own cache resolution was filesystem-order dependent. `references_cache/` holds 379 .md, 214 .txt and 78 .json files, and 63 references carry more than one — usually a .md alongside a .txt for the same PMID. Those are not interchangeable: per #265 the .md typically holds open-access full text while the .txt may be only the abstract. Taking the first glob match therefore read an arbitrary one. Measured by running the scan twice with opposite preferences, 4471 snippets are locatable via .md against 4400 via .txt — so **71 snippets were checked or silently skipped depending on directory iteration order**, which is not stable across machines. It never surfaced as a failure because the four truncations this PR fixes are flagged under either preference. It was latent: a future truncation among those 71 would be caught or missed by accident. All non-.json variants are now searched. They are deliberately not concatenated — this check inspects what *follows* the snippet, and joining two files would manufacture a continuation across the boundary. .json files are CrossRef metadata rather than prose and are excluded outright. Incidentally about twice as fast (6.7s -> 3.7s), because the cheap regex on the snippet now short-circuits before any cache file is read. Filed as #306, which also notes that evidence_snippet_audit.py and the vendored reference validator are worth checking for the same ordering dependence — if they resolve caches this way, their RENDERING/MISMATCH/NOCONTENT buckets inherit it. Mutation-checked again: re-truncating a snippet whose reference has only a .md cache still fails exactly its file's case. 899 tests pass; lint clean. Co-Authored-By: Claude Fable 5 --- tests/test_snippet_truncation.py | 41 ++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/tests/test_snippet_truncation.py b/tests/test_snippet_truncation.py index be0d32684..508fcda6b 100644 --- a/tests/test_snippet_truncation.py +++ b/tests/test_snippet_truncation.py @@ -50,12 +50,26 @@ _LOWER_CONT = re.compile(r"^\s+[a-z]") -def _cache_text(reference: str) -> str | None: - """Return the normalised cached text for a reference, or None if uncached.""" +def _cache_texts(reference: str) -> list[str]: + """Return every cached text for a reference, normalised to single spaces. + + A reference often has more than one cache file — 63 of them carry both a + `.md` and a `.txt`, and per #265 those differ in substance: the `.md` usually + holds open-access full text while the `.txt` may be only the abstract. + Taking the first glob match would make this check filesystem-order dependent; + measured, 71 snippets are findable in one variant and not the other (#306). + + All of them are searched instead. They are deliberately *not* concatenated: + this check inspects what follows the snippet, and joining two files would + manufacture a continuation across the boundary. `.json` files are CrossRef + metadata rather than prose and are excluded. + """ key = reference.replace("PMID:", "PMID_").replace("doi:", "DOI_").replace("/", "_") - for candidate in CACHE.glob(key + ".*"): - return " ".join(candidate.read_text(errors="replace").split()) - return None + return [ + " ".join(candidate.read_text(errors="replace").split()) + for candidate in sorted(CACHE.glob(key + ".*")) + if candidate.suffix != ".json" + ] def _evidence_items(node): @@ -92,15 +106,16 @@ def test_no_snippet_truncated_at_a_genus_abbreviation(path: Path): for item in _evidence_items(record): snippet = " ".join(str(item.get("snippet") or "").split()) - cached = _cache_text(str(item.get("reference"))) - if not snippet or not cached: - continue - start = cached.find(snippet) - if start < 0: + if not snippet or not _ABBREV_TAIL.search(snippet): continue - continuation = cached[start + len(snippet) :] - if _ABBREV_TAIL.search(snippet) and _LOWER_CONT.match(continuation): - offenders.append(f"{snippet[-52:]!r} + {continuation[:26]!r}") + for cached in _cache_texts(str(item.get("reference"))): + start = cached.find(snippet) + if start < 0: + continue + continuation = cached[start + len(snippet) :] + if _LOWER_CONT.match(continuation): + offenders.append(f"{snippet[-52:]!r} + {continuation[:26]!r}") + break # one report per snippet, not one per cache variant assert not offenders, ( f"{path.name} has {len(offenders)} snippet(s) truncated at a genus "