Sharpen the one genuine GTDB demotion, and record why no rule can find them (#445) - #450
Merged
Conversation
…d them (#445) #445 asked whether the tool should detect a demotion - an NCBI clade GTDB keeps but places a rank lower - and prefer the exact equivalent. Measured over all 159 higher-rank groundings, it should not. Six groundings look sharpenable, in the sense that every row behind the winning taxon agrees at a finer rank. Only one is a demotion: * Gemmatimonadota (x2) and Thermotogota keep their own names in GTDB (is_reclassified false). The NCBI taxon *is* the GTDB phylum, so sharpening to a class would assert what the data does not say. A rule keyed on agreement alone mis-sharpens all three. * Rhodospirillales -> o__RF32 looks weak at 7 of 328 rows but is a 0.704 majority by genomes, which is the denominator the tool actually uses. My first two measurements of this whole question were wrong for related reasons - one keyed on the genome-level id column instead of the name columns the tool uses for higher ranks, the other used 1-based header positions as 0-based indices. * Ca. Methanophagales -> o__Alkanophagales is a real reclassification, but no GTDB taxon bears the NCBI clade's name, so there is nothing to sharpen to. What marks a demotion is that a finer rank still carries the clade's name, and testing that mechanically is where it fails: the shared stem of Parvarchaeota/Parvarchaeales is 10 characters, of Dormiibacterota/Dormibacteria 5 (NCBI doubles the i), of Methanophagales/Methanospirareceae 7. No threshold separates the two real cases from the false one, so this stays a curator's call. Sharpen Ca. Parvarchaeota from p__Nanoarchaeota to o__Parvarchaeales. Every one of the named-species rows behind it is Parvarchaeales, so the sharper term carries the same 20/20 confidence, while the phylum also absorbs NCBI Ca. Woesearchaeota, Nanobdellota and Ca. Iainarchaeota - saying nothing that distinguishes this entry, in a record whose Micrarchaeota entry is a near neighbour. Order rather than family: c__Nanoarchaeia does not bear the name and f__Parvarchaeaceae is narrower than the NCBI phylum. Pin the two sharpenings and the one deliberate non-sharpening, so a tool re-run cannot quietly undo a decision that no rule can re-derive. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Network integrity findingsWarnings only — a member with no interaction yet, or a participant matched by ontology id rather than by name, or one on a community-level interaction that resolves to no member. Reported, but does not fail the build. The full report is attached to the workflow run as an artifact. |
This was referenced Aug 6, 2026
Closed
…rs were wrong Both reviewers independently refuted this PR's central claim. Parvarchaeota is not the only genuine demotion - it is one of three (#451): * Chlorobiota -> p__Bacteroidota, unanimous at c__Chlorobiia (24 named rows, 132 genomes) * Ignavibacteriota -> p__Bacteroidota, unanimous at c__Ignavibacteria (3 rows, 7 genomes) Both are textbook GTDB demotions of an NCBI phylum into a class, both pass this PR's own screen, and both incur the harm worse than the case #444 fixed: GTDB:p__Bacteroidota is the most-shared higher-rank term in the KB, on seven groundings, so three distinct phylum concepts were collapsing into a term that distinguished none of them. Sharpened and pinned. The measurement was also wrong under every definition (#452). "Six sharpenable" is 8 on raw rows and 20 under the tool's own default, which is the policy every stored block was built under - and the prose enumerated seven while saying six. Redone on the default: of 20, fifteen keep their NCBI name in GTDB and must not be sharpened, and of the five reclassified, two have nothing to sharpen to (f__CAG-239 is a placeholder, f__Methanospirareceae does not bear the name). The naive rule would mis-sharpen fifteen, not three, which strengthens the conclusion while invalidating every number it rested on. The stem-length argument now has five data points and still holds: demotions sit at 5, 8, 10 and 14 characters and the non-demotion at 7, so no threshold separates them. Rewrite the lineage test, which compared gtdb_id against gtdb_lineage - both curator-written in the same block, so a wrong sharpening with a matching hand-written lineage passed. It now goes back to the crosswalk and asserts every named-species row carries the chosen term at its own rank. Also: correct the Parvarchaeota note, which gave a false reason for order over family (the two are coextensive here) and undercounted the absorbed phyla as three; stop attributing the pin's protection to --apply, which only touches ungrounded taxa; and replace SKILL.md's stale "two blocks carry it" with the eight that do, plus a pointer to grep since nothing enforces the list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…could not fail Round 2 found the screen itself was blind, not merely under-run. It demanded strict string unanimity at the finer rank, so any clade GTDB had split for monophyly was invisible - which is how Nitrososphaerota was missed. Its rows read Nitrososphaeria 57 and Nitrososphaeria_A 8, the same clade split for monophyly, so re-sweeping with the suffix set aside finds it. Sharpened from p__Thermoproteota to c__Nitrososphaeria, and the stored fraction now counts only the unsuffixed name: 552 of 631 genomes where the phylum vote claimed 631 of 631. Less confident and more informative. The crosswalk test added in round 1 could not fail on the regression it existed to catch. "Every named row carries X at rank Y" is true of every *ancestor* too, so reverting the pins to p__Bacteroidota and p__Nanoarchaeota still passed. What separates a demotion from its own ancestor is the name, so a second test now asserts the chosen term bears the clade name and the vote's term does not - verified to fail when a pin is reverted. The crosswalk test is kept and made suffix-aware, because it catches a different error: an unrelated clade. Numbers corrected again: 21 sharpenable under the tool default rather than 20, five demotions rather than three, Ignavibacteriota's stem is 13 not 14, and Ca. Eiseniibacteriota belongs in the nothing-to-sharpen-to bullet, missing from a table that claimed to enumerate the reclassified cases. Also: p__Bacteroidota carries five other groundings, not seven - the count included the two this PR removes - and it is not the most-shared term either way, so the superlative is gone. Chlorobiota's class-over-order reason was unsupported, the two being coextensive here. SKILL.md named Bacteroides ovatus, which carries no pin; the ninth block is a second Allobosea. Canaried: all four pins are skipped by --apply, and the only record that moves is Rifle_Aquifer, on its pre-existing NOT_ATTEMPTED Geobacter entry that main drifts on identically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Round 3 falsified the completeness claim for the third time, with the largest demotion in the KB. Betaproteobacteria (NCBITaxon:28216) grounds to c__Gammaproteobacteria in two records; GTDB demoted it to the order Burkholderiales, which holds 41903 of the 41937 named-species genomes - more than all previously pinned cases combined. Both records are now pinned. What matters is *why* it was missed twice. It defeats both screens by construction, not by accident: it is not unanimous (0.999, with 31 genomes in Enterobacterales and 3 in Pseudomonadales), and GTDB renamed while demoting, so Betaproteobacteria and Burkholderiales share one letter and the name test scores it 1 against a floor of 5. So the docstring no longer claims to enumerate demotions. Three rounds produced three misses from three different structural blind spots - suffix splits, sub-unanimity, rename-while-demoting - and the honest conclusion is that a screen is not what makes one findable. The module records the calls a curator made and the evidence for each, so a tool re-run cannot undo them silently. The crosswalk test demanded that every named-species row carry the chosen term, which is the same unanimity assumption round 3 refuted, and Betaproteobacteria fails it. It now verifies the block's own stored counts against the crosswalk - support_genomes, total_genomes and majority_fraction - which is both weaker about unanimity and stronger about everything else. The name test skips the two renamed-while-demoted pins explicitly rather than dropping its threshold to admit them, which would admit anything. Two notes were wrong. The Nitrososphaeria one had GTDB's suffix convention backwards: _A marks lineages that are *not* monophyletic with the type, and those eight rows are Caldarchaeales thermophiles, not Thaumarchaeota - which is why the block counts only the unsuffixed name. It also cited Crenarchaeota and Korarchaeota, neither of which appears as an NCBI phylum in the crosswalk. The Ignavibacteria one called the order narrower than the class when the two are coextensive under the rows the block counts, contradicting the Chlorobiia note about the identical fact pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… it ship The two Betaproteobacteria blocks claimed "4 GTDB taxa under the NCBI taxon" beside a curation_note naming three orders. Three is right - Burkholderiales, Enterobacterales, Pseudomonadales - and no denominator yields four. The reason it shipped is the interesting part: nothing tested mapping_source at all. Every other field on a pin was checked against the crosswalk, and this one was prose. The numeric test now also asserts the rank token and the alternative count, and was verified to fail when the wrong string is put back. Also stop quoting counts the docstring cannot keep right. Three rounds running, a headline number in this file has been wrong - six, then twenty, then twenty-one - while the argument it supports has never depended on the exact figure. The LCP table now says plainly that it lists the cases that came up rather than claiming to enumerate, and the non-reclassified group is described rather than counted. Round 4 also re-derived the whole question independently: re-running the tool's vote for every higher-rank grounding and diffing against what is stored, exactly seven differ, and all seven are curated. No eighth demotion is hiding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Round 5 returned a merge verdict with four non-blocking nits; three are fixed here and the fourth is filed as #454. The provenance assertion added in round 4 used a substring match, so a block reading "11 GTDB taxa" satisfied an expected count of 1. It is a word-boundary match now. The docstring used `->` for two different things - a taxon's stored grounding and the finer term a sharpening would move it to - which made it read as though Rhodospirillales and Ca. Eiseniibacteriota were grounded at the placeholder terms rather than at o__RF32 and p__Eisenbacteria. Both are now named explicitly. The Parvarchaeales note argued that the phylum term failed to separate this entry from its Micrarchaeota neighbour. GTDB puts Micrarchaeota in its own phylum, so p__Nanoarchaeota would in fact have separated them; the note's load-bearing point - six NCBI phyla collapsing into that one term - stands on its own and now stands alone. Filed #454 for the one real gap round 5 found: nothing validates the middle segments of gtdb_lineage, which matters more now that seven of them are hand-written pins rather than tool output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
Review complete — five roundsThe inner loop ran the full five rounds. Rounds 1–3 each falsified this PR's central claim; rounds 4 and 5 verified the result and returned a merge verdict. Issues filed: #451, #452, #453, #454. The claim was wrong three times, for three different reasonsThe PR opened by asserting
So the module no longer claims to enumerate demotions. Three rounds, three misses, three distinct structural blind spots is the finding; a screen is not what makes one findable. What ships is the record of the calls a curator made and the evidence for each. Tests: three that could not fail
The suite now verifies each pin's stored numbers — CountsA headline number in the docstring was wrong in three consecutive rounds (six → 20 → 21, actual 23). The argument never depended on the figure, so the precise counts are gone rather than corrected again. Round 5 verdict
Merging. |
realmarcin
added a commit
that referenced
this pull request
Aug 6, 2026
…455) * Check the middle of gtdb_lineage, not just its head and tail (#454) 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> * Address the #455 review: make the two halves of the check agree 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> * Address the #455 review, round 2: wire the shape check where the docs 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> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #445.
The question, and the answer
#445 asked whether
gtdb_ground.pyshould detect a demotion — an NCBI clade GTDB keeps but places a rank lower — and prefer the exact equivalent, rather than leaving it to a curator to notice.Measured over all 159 higher-rank groundings: it should not. Six look sharpenable (every row behind the winning taxon agrees at a finer rank), but only one is a demotion:
Gemmatimonadota×2,Thermotogotais_reclassified: false) — the NCBI taxon is the GTDB phylum. A rule keyed on agreement alone mis-sharpens all three.Rhodospirillales→o__RF32Ca. Methanophagales→o__AlkanophagalesCa. Parvarchaeota→p__NanoarchaeotaWhat marks a demotion is that a finer rank still carries the clade's name — and testing that mechanically is exactly where it breaks. Shared stems:
Parvarchaeota/Parvarchaeales= 10 chars,Dormiibacterota/Dormibacteria= 5 (NCBI doubles thei),Methanophagales/Methanospirareceae= 7. No threshold separates the two real cases from the false one. So it stays a curator's call, and the durable artifact is the recorded decision, not a heuristic.The fix
Candidatus Parvarchaeotasharpened fromp__Nanoarchaeotatoo__Parvarchaeales. Every one of the named-species rows behind the grounding is Parvarchaeales, so the sharper term carries the same 20/20 confidence — sharpening costs nothing here. The phylum term was not wrong, only uninformative:p__Nanoarchaeotaalso absorbs NCBI Ca. Woesearchaeota, Nanobdellota and Ca. Iainarchaeota, so it said nothing distinguishing this entry, in a record whoseMicrarchaeota (ARMAN-1/2)entry is a near neighbour.Order rather than family:
c__Nanoarchaeiadoes not bear the clade name, andf__Parvarchaeaceaeis narrower than the NCBI phylum —o__Parvarchaealesis the most senior GTDB term that still means Parvarchaeota.Tests
Pin both sharpenings (Parvarchaeales, and #444's Dormibacteria) and the deliberate non-sharpening (Alkanophagales), so a tool re-run cannot quietly undo a decision no rule can re-derive. A fourth test checks that a sharpened term is genuinely inside the taxon it replaced — a sharpening moves down the lineage, never sideways.
Method note
Three measurements were needed to get this right. The first keyed on the genome-level taxid column instead of the name columns the tool uses for higher ranks (yielding meaningless "1 row" agreement); the second read the 1-based header positions as 0-based indices (so "domain" was really phylum). Both are recorded in the test docstring, because the wrong answers were plausible-looking.
Canaried:
--applylogsskipping curated NCBITaxon:1462422, applies 0 blocks, leaves the file byte-identical.just qcgreen.🤖 Generated with Claude Code