Propose absence semantics: one diagnosis, three different remedies (#294, #304, #307) - #309
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a docs-only proposal that unifies issues #294, #304, and #307 under the theme of “absence semantics” (empty slots overloading multiple meanings), while arguing the remedies should be independent (vocabulary vs slot vs auditor logic).
Changes:
- Introduces a written proposal diagnosing “empty slot overload” and quantifying its impact across the KB.
- Proposes three independent remedies: a GTDB grounding-status enum (#294), an auditor logic fix for
DISCONNECTED(#304), and a schema block for counter-selection (#307).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| would be wrong here, because the three instances differ in kind: | ||
|
|
||
| - **#294** is a *missing vocabulary* problem. The information mostly exists — | ||
| `gtdb_ground.py` already computes three of the states — it simply is not persisted. |
|
|
||
| ### 3.2 #304 — DISCONNECTED fires on the wrong criterion | ||
|
|
||
| Two rules interact in `network/auditor.py`: |
ReviewProposal-only — one file under I spot-checked the two load-bearing factual claims rather than taking the diagnosis on trust, since a proposal is only as good as the problem statement. Claim: Claim: the audit rewards an unsourced if taxon_data.get("abundance_level") or taxon_data.get("functional_role"):
continueAny truthy value suppresses the The judgement I'd most want to keep is §2 — refusing to unify the three into one "absence annotation" mechanism. The temptation is obvious and the reasoning against it is correct: #294 is a missing vocabulary (the information exists, unpersisted), #307 is a missing slot (the information exists in the source with nowhere to go). Those need different remedies, and a single general mechanism would have obscured that. Resisting a tidy abstraction that does not fit is worth more here than the specific remedies. No findings. Nothing to file. One note on process rather than content: this asks for three independent approve/reject decisions, which makes it a document that stops being useful once decided. Worth recording the outcome in the file itself when you decide — otherwise the next reader cannot tell a live proposal from a settled one, which is how #209 ended up being followed after it had already been reversed. |
, #304, #307) The three issues share a diagnosis — the schema can say "has value X" or "is empty", so empty is overloaded across at least four meanings: not done, impossible, undecidable from the source, and deliberately excluded. They do NOT share a remedy, which is the proposal's main structural claim. #294 is missing vocabulary, #307 is a missing slot, and #304 is not a schema problem at all — it is auditor logic reading a fine schema wrongly. Bundling them would drag a code fix into a data migration for no benefit, so each is proposed separately and they can be taken in any order. Measured rather than asserted, on 307 records / 1007 taxa: - #294: of 372 ungrounded taxa, 285 have no GTDB equivalent and 85 are ambiguous. Checking the remaining 5 is what makes the case — every one is an entry deliberately WITHHELD under #292, so the honest tally is 370 permanent, 2 withheld, and ZERO pending work. GTDB grounding is complete to its ceiling and the schema cannot say so. That is precisely why #276 read as ~40% outstanding when the ceiling is ~63%. - #304: 107 of 302 records with interactions (35%) are entirely COMMUNITY_LEVEL, which the auditor credits with no connections; and 931 of 1007 taxa (92%) are exempt from DISCONNECTED via abundance_level/functional_role. The exemption, not the connectivity check, is doing the work — so the rule does not mean what its name says, and it rewards unsourced metadata. - #307: 193 records carry engineering_design but only 40 carry free-text notes, so most have no place at all for a load-bearing negative result. The withheld finding changed the proposal while writing it: WITHHELD was added as a fifth enum value, and the "quick win" of grounding 5 stragglers was removed, because they are blocked on #292 rather than pending. The existing pin in tests/test_gtdb_withheld_groundings.py is a test compensating for vocabulary the schema lacks, which is itself an argument for the enum. Recommended order: the #304 COMMUNITY_LEVEL credit first (smallest, no migration, unblocks #273, removes the perverse incentive), then #294, then #307. §6 records what is deliberately NOT proposed, including a general absence framework — three instances is not enough evidence to design one. Decision requested per remedy; they are independent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review of this proposal found §4.2 asserting rather than measuring. It claimed "doing (1) without (2) is safe and strictly an improvement". Simulating all four configurations over the current KB shows that is wrong: today — no credit, exemption kept 32 drop the exemption only 390 credit COMMUNITY_LEVEL only 1 credit COMMUNITY_LEVEL + drop the exemption 19 Crediting COMMUNITY_LEVEL alone is indeed safe, but it drops reporting to ONE finding across 307 records. The rule becomes vacuous, and a gate that never fires is not an improvement over one that fires for the wrong reason. Dropping the exemption alone reports 390 — a 12x increase that buries the signal. Only together do they land somewhere defensible: 19, fewer than today's 32 while measuring what the name promises, and a real work-list rather than an artefact. §4.2 now carries the table and says the halves must ship together; §5's sequence is updated from "change (1) first" to "both changes together, do not split". This is the second time in this proposal that checking a number changed the recommendation — the first was discovering the 5 apparently-groundable taxa were deliberately withheld. Both were claims that looked safe enough to assert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…found a gap Review of this PR re-derived its own figures against current main rather than trusting them, which changed three things. **§4.2 is no longer a proposal.** #304 shipped as PR #311 while this sat open — both halves together, landing at exactly the predicted 19 findings, network issues 49 -> 23. It is marked IMPLEMENTED and kept as the worked example: it is the only one of the three where the simulation can be checked against the outcome. Two things it surfaced are cross-referenced (#312 all-or-nothing credit, #313 dangling-edge detection living in an orphaned script). **§3.1's numbers moved, and re-checking them found a real gap.** Refreshed to 311 records / 1024 taxa: 288 no-equivalent, 87 ambiguous, 6 groundable. Five of the six are the withheld entries from #292 as before — but the sixth is a genuine gap (#314): a taxon whose term.id was corrected after grounding ran, leaving the derived block absent. That incident is the proposal's own thesis in miniature, so it is written up as such. One blank column now encodes four distinct situations — impossible, undecidable, deliberately withheld, accidentally stale — and telling them apart required running the grounding tool over the whole KB and cross-referencing the withheld list. With the proposed enum it would be a one-line query. **Stale figures corrected**: engineering_design 193 -> 197 records, the 92% exemption stated in past tense now that it no longer exists, and the decision line narrowed from three remedies to the two still open. #314 also notes a fix worth doing regardless of what is decided here: a gtdb_classification records ncbi_source_id, so a mismatch against term.id is detectable today with a cheap test and no schema change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
e079afc to
52abc8b
Compare
…ilent holes **The docstring's story was wrong, and it mattered.** I wrote that grounding had run against NCBITaxon:1126 and been left stale by the id edit. `git log --follow` shows otherwise: the record's first commit already reads NCBITaxon:1125 with no grounding at all. The 1126 -> 1125 edit happened pre-commit, so `main` never held a stale block, only an absent one. That matters because both general assertions inspect only taxa that *have* a block — so **neither would have caught #314**. Only test 3, the instance pin, covers it. The general pair guards the sibling failure mode, where a block survives an id edit and quietly describes the previous organism. The docstring now says exactly that instead of claiming otherwise. Two silent holes, both found by mutation and both now failing loudly: - a block whose taxon has **no `term.id`** was skipped by an `if term_id and ...` guard, so deleting the id passed every test; - an **internally incoherent** block passed too — `gtdb_taxon: Nostoc` against `gtdb_id: GTDB:g__Microcystis`, or `majority_fraction: 7.5`, which `linkml-validate` also accepts. A fourth test now cross-checks `gtdb_id`, `gtdb_taxon` and the lineage tail against each other, and bounds the fraction to the (0.5, 1] the majority rule implies. Scope widened to `data/isolates/` as well. It carries no grounding today, but it uses the same `taxonomy[].taxon_term` shape, and that directory has been outside a gate's scope once already (#310). Numbers corrected: 292 taxa have no GTDB equivalent, not 288 — the figure was inherited from #314's body, which inherited it from #309, and it did not close against the total. 292 + 87 ambiguous + 2 withheld = 381, which is the ungrounded count after this PR. NEXT_TASKS_LOOP's prose said 63 references carry both a .md and a .txt where its own table says 62. Filed rather than fixed: #365, where two records carry a GTDB block on NCBITaxon:169215 — the *plant* genus Bosea — because the tool's higher-rank path matches on label, not id, and every gate passes; and #366, where gtdb_ground.py is batch-sensitive, so the remediation advice in this test's own failure message is not reproducible. Both are classified in NEXT_TASKS_LOOP, along with #363. 989 passed, 9 skipped.
…ource id (#314) (#364) * Ground the taxon left stale by an id edit, and pin grounding to its source id (#314) `Mesorhizobium_Synechococcus_B12_Synthetic_Consortium` carried one ungrounded taxon of four. Grounding had run against `NCBITaxon:1126` (*Microcystis aeruginosa*); review then established the paper names only a genus and changed the id to `NCBITaxon:1125`, and grounding was never re-run. The entry read as ungrounded when it was simply out of date. The issue's premise needed checking rather than taking: `gtdb_ground.py --ncbi-id NCBITaxon:1125` reports *no GTDB mapping*, which reads as ungroundable. It is groundable — the rank-aware aggregation only runs in `--community` mode, where the label supplies the rank — and it lands on `GTDB:g__Microcystis` at 0.991 majority over 2 GTDB taxa. Applied: 8 lines added, nothing else in the record touched. 646 grounded taxa become 647. Genus rank is the point, so the test pins it. An earlier draft had grounded this to *M. aeruginosa* on the grounds that it dominates Lake Taihu blooms; the source never names a species, and that inference was removed once already. **The durable half is the guard.** A `gtdb_classification` is derived from `term.id` and records its input in `ncbi_source_id`, but nothing tied the two together — edit the id and the block silently describes a different organism. `tests/test_gtdb_withheld_groundings.py` pins *deliberate* absence for the #292 entries; accidental staleness had nothing. Measured before writing it: 0 of 647 grounded taxa currently mismatch. So this guards the next id edit rather than reporting on this one — which is the honest description, and why the third test pins the specific instance separately. Canaried all three: a stale `ncbi_source_id` fails 2, removing the provenance field fails 3, removing the whole block fails 1. Note this does not close the ambiguity #294 exists for — an accidental blank still looks like a deliberate one. It closes the one case where the block's own provenance field can prove the answer today, with no schema change. 988 passed, 9 skipped. * Address the review of #364: correct the causal story, and close two silent holes **The docstring's story was wrong, and it mattered.** I wrote that grounding had run against NCBITaxon:1126 and been left stale by the id edit. `git log --follow` shows otherwise: the record's first commit already reads NCBITaxon:1125 with no grounding at all. The 1126 -> 1125 edit happened pre-commit, so `main` never held a stale block, only an absent one. That matters because both general assertions inspect only taxa that *have* a block — so **neither would have caught #314**. Only test 3, the instance pin, covers it. The general pair guards the sibling failure mode, where a block survives an id edit and quietly describes the previous organism. The docstring now says exactly that instead of claiming otherwise. Two silent holes, both found by mutation and both now failing loudly: - a block whose taxon has **no `term.id`** was skipped by an `if term_id and ...` guard, so deleting the id passed every test; - an **internally incoherent** block passed too — `gtdb_taxon: Nostoc` against `gtdb_id: GTDB:g__Microcystis`, or `majority_fraction: 7.5`, which `linkml-validate` also accepts. A fourth test now cross-checks `gtdb_id`, `gtdb_taxon` and the lineage tail against each other, and bounds the fraction to the (0.5, 1] the majority rule implies. Scope widened to `data/isolates/` as well. It carries no grounding today, but it uses the same `taxonomy[].taxon_term` shape, and that directory has been outside a gate's scope once already (#310). Numbers corrected: 292 taxa have no GTDB equivalent, not 288 — the figure was inherited from #314's body, which inherited it from #309, and it did not close against the total. 292 + 87 ambiguous + 2 withheld = 381, which is the ungrounded count after this PR. NEXT_TASKS_LOOP's prose said 63 references carry both a .md and a .txt where its own table says 62. Filed rather than fixed: #365, where two records carry a GTDB block on NCBITaxon:169215 — the *plant* genus Bosea — because the tool's higher-rank path matches on label, not id, and every gate passes; and #366, where gtdb_ground.py is batch-sensitive, so the remediation advice in this test's own failure message is not reproducible. Both are classified in NEXT_TASKS_LOOP, along with #363. 989 passed, 9 skipped.
Docs only. A written proposal covering #294, #304 and #307 as one theme, ending in three decisions to make.
The theme is real; the remedy is not shared
All three come from the same gap: the schema can say "has value X" or "is empty", and nothing else, so empty is overloaded across at least four meanings — not done, impossible, undecidable from the source, and deliberately excluded.
But the proposal's main structural claim is that they should not be fixed together:
gtdb_ground.pycomputes it and throws it away.Bundling them would drag a code fix into a data migration for no benefit.
Measured, not asserted (307 records / 1007 taxa)
COMMUNITY_LEVEL, which the auditor credits with no connections; 931 of 1007 taxa (92%) are exempt fromDISCONNECTEDviaabundance_level/functional_role.engineering_design, only 40 carry free-textnotes— most have nowhere at all for a load-bearing negative result.Two of these are load-bearing for the argument. GTDB grounding is complete to its ceiling and the schema cannot say so — which is precisely why #276 read as ~40% outstanding work when the achievable ceiling is ~63%. And for #304, the exemption rather than the connectivity check is doing essentially all the work, so
DISCONNECTEDdoes not mean what its name says and actively rewards unsourced metadata.Verification changed the proposal while writing it
The first draft claimed 5 taxa were "immediately actionable regardless of what is decided here". Checking what they actually were showed every one is a withheld entry blocked on #292. That correction did two things: it added
WITHHELDas a fifth enum value, and it removed the quick win.It also produced the sharpest argument for the enum. The pin in
tests/test_gtdb_withheld_groundings.pyexists to stop a tool re-run reinstating those groundings — that is a test compensating for vocabulary the schema lacks.What is proposed
gtdb_grounding_statusenum onTaxonDescriptor(GROUNDED | NO_GTDB_EQUIVALENT | AMBIGUOUS | WITHHELD | NOT_ATTEMPTED), three of which the tool already computes.COMMUNITY_LEVELinteractions as connecting members, then drop the metadata exemption. Change (1) alone is safe and strictly an improvement; (2) alone is not.counter_selectionblock onCommunityEngineeringDesign, withexcluded_taxonoptional, because ARC is exactly the case where excluded strains cannot be distinguished from retained ones.Recommended order: #304's first change, then #294, then #307 — smallest first, and it unblocks #273, which cannot decide whether to restore the network gate while
DISCONNECTEDmeans something other than its name.§6 records what is deliberately not proposed, including a general absence-annotation framework: three instances is not enough evidence to design one.
Decision requested per remedy — they are independent. No code or data changes here; 899 tests pass unaffected.
🤖 Generated with Claude Code
Review round: simulating the #304 change reversed the recommendation
§4.2 originally asserted that "doing (1) without (2) is safe and strictly an improvement". That was reasoning, not measuring. Simulating all four configurations over the current KB:
DISCONNECTEDreportedCOMMUNITY_LEVELonlyCOMMUNITY_LEVEL+ drop the exemptionCrediting
COMMUNITY_LEVELalone is safe — and drops reporting to one finding across 307 records. The rule becomes vacuous, and a gate that never fires is not an improvement over one that fires for the wrong reason. Dropping the exemption alone reports 390, a 12× increase that buries the signal.Only together do they land somewhere defensible: 19 — fewer than today's 32, while measuring what the name promises, and a genuine work-list rather than an artefact.
§4.2 now carries this table and states the halves must ship together; §5's recommended sequence changes from "change (1) first" to "both changes together, do not split".
This is the second time writing this proposal that checking a number changed the recommendation. The first was discovering the five apparently-groundable taxa were deliberately withheld, which added
WITHHELDto the proposed enum and removed a claimed quick win. Both were claims that felt safe enough to assert — which is the argument for simulating a rule change before proposing it, since the proposal is the artefact someone will act on.899 tests pass; docs-only, no code or data touched.
Review round: re-derived the proposal's own figures
Rebased onto
mainand re-measured rather than trusting the numbers. Three things changed.§4.2 is no longer a proposal — it shipped. #304 landed as PR #311 while this sat open: both halves together, exactly the predicted 19 findings, network issues 49 → 23. It is marked IMPLEMENTED and kept as the worked example, since it is the only one of the three where the simulation can be checked against the outcome. Two things it surfaced are cross-referenced — #312 (the credit is all-or-nothing, which overlaps §4.3's design question) and #313 (dangling-edge detection lives in an orphaned script nothing runs).
Re-checking §3.1 found a real gap. Refreshed to 311 records / 1024 taxa: 288 no-equivalent, 87 ambiguous, 6 groundable. Five are the withheld entries from #292 as before — but the sixth is a genuine gap, filed as #314: a taxon in 000315 whose
term.idI corrected after grounding ran (following Edison's advice to reground Microcystis to genus), leaving the derived block absent with nothing to notice.That incident is the proposal's own thesis in miniature, so it is written up as such. One blank column now encodes four distinct situations — impossible, undecidable, deliberately withheld, accidentally stale — and telling them apart required running the grounding tool across the whole KB and cross-referencing the withheld list. With the proposed enum it is a one-line query.
Stale figures corrected:
engineering_design193 → 197 records; the 92% exemption restated in past tense now that it no longer exists; and the decision line narrowed from three remedies to the two still open.#314 also notes a fix worth doing regardless of what is decided here: a
gtdb_classificationrecordsncbi_source_id, so a mismatch againstterm.idis detectable today with a cheap test and no schema change.908 tests pass; docs-only.