Fix four slots whose documented meaning the corpus never honoured (#748, #749, #751, #752) - #755
Open
realmarcin wants to merge 1 commit into
Open
Fix four slots whose documented meaning the corpus never honoured (#748, #749, #751, #752)#755realmarcin wants to merge 1 commit into
realmarcin wants to merge 1 commit into
Conversation
Resolves #748 and #751; adds the METPO proposal #749 and #752 need, and the in-repo half of #752. **#748 — abundance_level is ordinal, not a percentage.** The values named bands (DOMINANT >1%, ABUNDANT 0.1-1%, COMMON 0.01-0.1%, RARE <0.01%) that the corpus never meant: in SYNTHETIC records of at most four members, where every member necessarily exceeds 1%, 76 of 91 assignments claimed the member was below it. The decisive fact is that the numeric slots already exist and are empty — `relative_abundance` and `absolute_abundance` landed in #161 and are used by 0 of 1068 taxa. So the enum now defines an ordinal rank within a record and points at `relative_abundance` for a measured share, which makes every existing assignment correct instead of requiring a 392-taxon sweep. Two records from #747 were re-ranked, having been curated against the old threshold. **#751 — biological_processes now holds biological processes.** 17 entries were swapped to genuine process terms, each verified by ancestor against GO:0008150 rather than by label; 12 uses of `GO:0009055` electron transfer activity were removed rather than substituted, because OLS returns nothing for interspecies or extracellular electron transfer and every GO electron-transport-chain term means an intracellular series of complexes. Substituting one would repeat the error. 13 duplicates created or exposed by the swap were removed. **#752 — SYNTROPHY no longer asserts obligacy.** It read "Obligate metabolic cooperation", which its own worked case disproves: PMID:40298437 grows both members of CommunityMech:000336 as monocultures in the same experiment. **#749/#752 — eight METPO classes proposed** in `proposals/metpo_communitymech_interaction_semantics_v1/`: obligate and facultative syntrophy, and a `material transfer interaction` branch — cell fusion, cytoplasmic exchange, HGT, interspecies and direct interspecies electron transfer — deliberately a *sibling* of the sign-based types rather than a subtype, since an interaction can be both a fusion and a mutualism. METPO, TraitMech and CellStructureMech were all checked first; none has these concepts, and METPO already models obligate/facultative this way for oxygen preference. Both new gate tests were mutation-tested with a control arm. An earlier attempt at the #751 sweep cleared emptied keys with a regex and silently merged non-empty lists into the preceding one across 204 records. It was caught by reading the diff, reverted before any validation ran, and redone with a local structural check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Crb17nwErCwBJrkSCp37Ua
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 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
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.
Four review issues from #747, resolved together because they share one cause:
a slot whose documented meaning drifted from its actual use, with no gate able
to see the difference.
abundance_levelbiological_processesInteractionTypeEnum.SYNTROPHYInteractionTypeEnumgaps#748 —
abundance_levelis a rank, not a percentageThe values named bands: DOMINANT >1%, ABUNDANT 0.1-1%, COMMON 0.01-0.1%, RARE
<0.01%. In
community_origin: SYNTHETICrecords of at most four members — whereevery member necessarily exceeds 1% — 76 of 91 assignments claimed the member
was below it.
The fact that decided the fix: the numeric slots already exist and are empty.
relative_abundanceandabsolute_abundancelanded in #161 and are used by0 of 1068 taxa. The enum was carrying a job it was never checked on while the
slot built for that job sat unused.
So
AbundanceEnumis now an ordinal rank within a record, pointing atrelative_abundancefor a measured share. Every existing assignment becomescorrect under that reading — both members of a coculture marked ABUNDANT means
"co-equal partners", which is a useful thing to record and evidently what the
words were being used for. No 392-taxon sweep.
Two records from #747 were re-ranked, since they had been curated against the
threshold: C. ljungdahlii DOMINANT→ABUNDANT (the 25% minority partner) and the
Methanoregula endosymbiont DOMINANT→COMMON (2%, smallest of three bins).
tests/test_abundance_level_is_ordinal.pypins it with four checks, including aconsistency check between the ordinal and numeric slots that is vacuous today and
says so — a companion test fails the moment anyone populates a number, so the
vacuity cannot go unnoticed.
#751 —
biological_processesnow holds biological processes17 entries swapped to genuine process terms, each verified by ancestry
against
GO:0008150rather than by label — "electron transfer activity" readslike a process to anyone not looking it up:
GO:0008810cellulase activityGO:0030245cellulose catabolic processGO:0097599xylanase activityGO:0045493xylan catabolic processGO:0004568chitinase activityGO:0006032chitin catabolic processGO:0009039urease activityGO:0043419urea catabolic processGO:0018597ammonia monooxygenase activityGO:0019329ammonia oxidationGO:0016661oxidoreductase, nitrogenous donorsGO:0019332aerobic respiration, using nitrite as electron donorGO:0008509anion transporter activityGO:0015703chromate transportGO:0140487metal ion sequestering activityGO:0061687detoxification of inorganic compoundGO:0046872metal ion bindingGO:0061687detoxification of inorganic compoundGO:0005576extracellular regionGO:0009306protein secretion12 uses of
GO:0009055were removed, not substituted. EBI OLS returnsnothing for extracellular electron transport or interspecies electron
transfer, and every GO electron-transport-chain term denotes an intracellular
series of complexes — wrong for DIET through conductive pili. Substituting one
would repeat exactly the error being fixed.
GO:0070812organohalide respirationis untouched: it resolves to nothing in the pinned build and is already an
exception in
conf/id_label_targets.yaml, so it is a resolution failure, not anaspect error.
Regrounding two activities onto one process term collided in 11 interactions;
2 more duplicates predated the sweep. All 13 removed.
#752 — SYNTROPHY stops asserting obligacy
It read "Obligate metabolic cooperation", which its own worked case
disproves: PMID:40298437 grows both members of CommunityMech:000336 as
monocultures in the same experiment. Obligacy is rarely establishable from one
record, and demanding it in the type pushes curators to assert it silently.
The value now describes the exchange and says obligacy is not implied.
#749 + #752 — eight METPO classes proposed
proposals/metpo_communitymech_interaction_semantics_v1/.Checked before proposing anything, per the "METPO-maximizing, not
METPO-forcing" rule: METPO's 255 terms have
SyntrophyandElectron transferbut no cell fusion, HGT or conjugation; TraitMech and CellStructureMech have
none of the six concepts; GO has no inter-organism relation for any of them.
obligate syntrophy interaction/facultative syntrophy interaction—mirroring METPO's own
obligately anaerobic/facultatively anaerobicunderoxygen preference. This requires amendingMETPO:1007127's definition todrop "obligate", or a facultative child contradicts its parent; that amendment
is part of the proposal, not a side effect.
material transfer interactionbranch — cell fusion, cytoplasmic materialexchange, HGT, interspecies and direct interspecies electron transfer —
deliberately a sibling of the sign-based types, not a subtype. An
interaction can be both a fusion and a mutualism; those answer different
questions, and forcing one slot to carry both is what ran the enum out of room.
A mistake worth recording
My first pass at the #751 sweep cleared emptied
biological_processes:keys witha regex over the whole file. It silently merged non-empty lists into the
preceding one across 204 records. I caught it by reading the diff before
running any validator, reverted, and redid it with a local structural check that
only drops a key when the very next line is not one of its items. The rerun
touches 17 files.
Validation
validate-strict332 files / 0 ERROR ·validate-terms-all332 passed ·validate-gtdb-all0 incoherent ·lint(ruff + mypy) clean ·check-docs-currentmatches ·pytest2963 passed.Both new gate tests were mutation-tested with a control arm: reintroducing a
percentage band and planting a molecular function each turned the right test red
by name, the restores took, and unmutated copies through the identical harness
were green.
The one local failure,
test_no_snippet_stops_mid_word, is the pre-existing onecaused by an untracked
references_cache/PMID_38150661.mdin the workingtree. Not in this diff; CI cannot see it.
Separately filed while investigating: #754, on committed reference-cache text
and its licence.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Crb17nwErCwBJrkSCp37Ua