Check the middle of gtdb_lineage, not just its head and tail (#454) - #455
Conversation
The freshness checks compare gtdb_id, gtdb_taxon and the lineage's tail; the prokaryote-only gate (#365) reads its head. A segment corrupted in between passed every gate and the whole suite: d__Bacteria;p__Bacteroidota;c__Chlorobiia d__Archaea;p__Nonsense;c__Chlorobiia <- indistinguishable That was tolerable while gtdb_ground.py wrote every lineage from the crosswalk. #450 made seven of them hand-written curator pins, and the review that caught it could only do so by asking the crosswalk - which CI has no checkout of, so a crosswalk-based check would skip exactly where it is needed. Both new checks are corpus-internal and need no mapping. Ranks must carry a known prefix and get finer left to right, which is per-record and runs inside validate-strict as gtdb_lineage_malformed. And because GTDB is a hierarchy, a taxon must sit under exactly one parent path across every record naming it - that one needs the whole corpus at once, so it runs in `just validate-gtdb-all` and in the test suite rather than per file. The corpus check is a weaker claim than "this lineage matches GTDB" and a much cheaper one. It cannot catch a corruption that is internally consistent, but it catches the case that matters: a hand-edited pin drifting from the 720 blocks the tool wrote. Measured: 727 blocks, 740 distinct taxa, zero conflicts, zero malformed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The shape check accepted a lineage that skips a rank; the corpus check keys a taxon by the literal path above it, so the same lineage would place its tail under a different path than the full chain does - and be reported as a hierarchy conflict naming the record that is correct. Two halves of one module disagreeing about what a valid lineage is, and the failure lands on the innocent file. Requiring contiguity, and a start at d__, turns that into a precise per-record error against the record that actually has the problem. No crosswalk lineage skips a rank, so this constrains hand-written pins rather than tool output - which is the case #454 exists for. Walk interaction participants too. source_taxon/target_taxon share taxon_term's range, so a block is schema-valid there, and the sibling #365 gate already walks them; none exist today, so missing them would have been silent. The KB test counted taxonomy entries, not blocks - 1032 rather than 727 - so if `_blocks` ever stopped matching, both checks would return [] for every record and the test would stay green on an empty corpus. It now counts what the checks actually walk. Also: my insertion split the #365 comment from the loop it documents, so it read as documentation for the new check and claimed something false of it; the docstring said the shape check compares against gtdb_id, which it never reads; and the CLI and justfile described only the evidence-count half of a script that can now exit 1 for a lineage conflict, including that a single-file run cannot find a cross-record one. The review also cleared the risk I was most unsure of: across all 92,711 crosswalk rows there is no GTDB segment with more than one parent path, and though 1,237 bare names are reused across ranks (UBA1381 is an order, a family and a genus), keying on the rank-prefixed segment keeps them distinct. The corpus check has no latent false positive from name reuse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… promised The CLI docstring said passing one file checks its shape. It did not - the script imported only check_corpus - so `just validate-gtdb-all`, the recipe the docstring points at, still had the failure mode round 1 was meant to remove: a rank-skipping lineage surfacing only as a hierarchy conflict that names the record which is correct. The shape check now runs there too, and prints first, so a curator sees the malformed record before the conflict it causes. Verified: a skip-rank file alone exits 1, a clean file exits 0. Also fixed the #365 comment, which I orphaned a second time - merged into the new paragraph one call site down, so it documented the wrong loop and claimed something false of it. Each comment now sits with the loop it describes, and the #454 one names the contiguity and d__-first rules that are its substance. Round 2 confirmed the riskiest part of round 1: making the shape rule stricter rejects nothing real. All 727 KB lineages pass, and replicating the tool's own lineage builder over all 92,711 crosswalk rows at every truncation level gives 47,996 distinct lineages, none of which skips a rank or fails to start at d__. The tool cannot write data its own validator rejects. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review complete — two rounds, merge verdictRound 1The two halves of the module disagreed about what a valid lineage is. The shape check accepted a rank-skipping lineage; the corpus check keys a taxon by the literal path above it, so that same lineage put its tail under a different path than the full chain — and got reported as a hierarchy conflict naming the record that was correct. Requiring contiguity and a Also fixed: the KB test counted taxonomy entries (1032) rather than blocks (727), so if Round 1 also cleared the risk I was least sure of: across all 92,711 crosswalk rows there is no GTDB segment with more than one parent path, and although 1,237 bare names are reused across ranks ( Round 2The CLI docstring promised the shape check ran there; it didn't — only Round 2 verified the riskiest change — making the rule stricter could have rejected legitimate data. It doesn't: all 727 KB lineages pass, and replicating the tool's own lineage builder over all 92,711 crosswalk rows at every truncation level yields 47,996 distinct lineages, 0 rejected. The tool cannot write data its own validator rejects. State727 blocks · 740 distinct taxa · 0 conflicts, 0 malformed · |
1 participant
Closes #454.
The gap
gtdb_lineageis a denormalised path, and the existing checks only look at its ends — freshness comparesgtdb_id,gtdb_taxonand the tail; the prokaryote-only gate (#365) reads the head. A segment corrupted in between passed every gate and the entire test suite:That was tolerable while
gtdb_ground.pywrote every lineage from the crosswalk. #450 made seven of them hand-written curator pins, which is when it stopped being theoretical — and the review that found it could only do so by querying the crosswalk, which CI has no checkout of. A crosswalk-based check would skip precisely where it is needed.Two checks, both offline
Shape, per record — every segment carries a known rank prefix, and ranks get finer left to right. Runs inside
validate-strictasgtdb_lineage_malformed. Catchesd__Bacteria;c__Chlorobiia;p__Bacteroidota, a missing prefix, an unknown prefix, a repeated rank.Hierarchy, across records — GTDB is a strict hierarchy, so a taxon has exactly one parent. Reading all 727 blocks as (taxon, path-above-it) pairs, a segment appearing under two different paths is a contradiction between two records, whichever is wrong. This needs the whole corpus at once, so it runs in
just validate-gtdb-alland in the test suite rather than per file.Neither needs the mapping, which is the point: they run wherever CI runs.
What it does and does not catch
The corpus check is a weaker claim than "this lineage matches GTDB" and a far cheaper one. It cannot catch a corruption that is internally consistent — a taxon named in one record only, misspelled throughout. It does catch the case that motivated it: a hand-edited pin drifting from the 720 blocks the tool wrote. The docstring says so plainly rather than implying more.
Measured
727 blocks · 740 distinct GTDB taxa · 0 conflicts, 0 malformed.
14 new tests, including the exact corruption that prompted this, the malformed-input guards (
gtdb_lineagehas norangein the schema, so a YAML list is schema-valid), and a KB sweep. Verified end to end:validate_strict.pyon a record with reversed ranks exits 1 with agtdb_lineage_malformedrow.just qcgreen.🤖 Generated with Claude Code