Re-ground 9 GENE_OR_PROTEIN-object enables edges by idiom (#334) - #341
Conversation
biolink declares enables range 'biological process or activity', which only BIOLOGICAL_PROCESS, PATHWAY and MOLECULAR_FUNCTION satisfy, so the 33 edges pointing elsewhere each entail a false type. #334 asks for group-by-idiom rather than one sweep, because the 33 mix several distinct claims; this takes the GENE_OR_PROTEIN-object group's unambiguous half. Reading all 13 protein-object edges showed roughly seven idioms, not one, so only those with an exact and range-safe upstream term are migrated: encodes (biolink:encodes) 1 a gene cluster encodes its product part of (biolink:part_of) 3 a subunit is part of its complex contributes to (RO:0002326) 5 an energy source or acquired repertoire All three declare no rdfs:domain or rdfs:range -- checked, not assumed -- so none can reintroduce the class being removed. All three already had mapping rows, so nothing new was minted. Four protein-object edges are deliberately NOT migrated, because each needs a modelling decision rather than a relabel: molecular_oxygen -> cytochrome bd, cytoplasmic_na -> Mrp complex substrate availability; the honest form is probably the complex taking the chemical as has-input, which reverses the edge flagellar_motor_torque -> flagellar_motor the subject is a property of the object; the edge is confused Hfq -> RpoS "enables sRNA-dependent translation of RpoS" -- the object should be that translation process, which is not currently a node ENABLES_RANGE_VIOLATION: 33 -> 24. The remaining 24 stay baselined. Worth recording: this change touched 8 trait files and rewrote 8 pages. Before #304 landed earlier today it would have rewritten 477. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Claude finished @realmarcin's task in 4m 29s —— View job ReviewThe per-idiom split is the right call, the baseline/report/page regeneration is consistent (33 → 24 rows, verified in 🟡
This is pre-existing — 8 other trait files already use it — so it is not a defect this PR introduced. But the PR asserts "All three declare no 🟡 The snippet on the edge is "MreB filaments interact with Rod complexes". The target node in the same file is described as "Cell-wall elongation machinery associated with MreB". The edge description still reads "MreB organizes rod-shape elongation machinery." Three statements of association, and the predicate now asserts parthood. Whether MreB is a constituent of the Rod complex or a filament that guides it is genuinely unsettled, and the corpus is split on it — 🟡 The IMF edges are the same idiom the PR defers, migrated instead of deferred. Biolink defines That is the same energy-availability-powering-a-machine shape the PR sets aside for 🔵 Descriptions were left phrased for Curation events are on all 8 records and ordered correctly. No record was added under I could not run Requesting changes on the three 🟡 items. |
There was a problem hiding this comment.
Three 🟡 items — see the detailed review comment.
-
biolink:encodes(methanogenesis.yaml:165) is not a slot in the pinned biolink-model 4.4.0;reports/biolink_coverage.tsv:28already records it as unmatched. The PR's "checked, not assumed" claim is vacuous for a term absent from the model. Pre-existing, but the mapping-row note asserting an exact biolink match should be corrected or the edge re-grounded to RO:0002205. -
mreB part of rod_complex(rod_shaped.yaml:108, bacillus_shaped.yaml:106) contradicts its own snippet ("MreB filaments interact with Rod complexes"), its own edge description ("organizes"), and the target node's description ("associated with MreB").interacts withmatches the quoted evidence and needs no modelling decision. -
The ion-motive-force edges (motile.yaml:148, motility.yaml:121, flagellated.yaml:142, gliding.yaml:107) are the same energy-availability idiom the PR deliberately defers for molecular_oxygen and cytoplasmic_na. Biolink's
contributes tois about contributing to the occurrence or generation of the other entity; IMF powers the motor's rotation, it does not help generate the motor. These pass the audit becausecontributes todeclares no range, not because the claim got truer.
The per-idiom approach is right and the baseline/report/page regeneration is consistent (33 → 24 verified). The FliH-FliI-FliJ part-of and the virulence-gene-acquisition contributes-to edges are both good.
All three findings were right, and the first exposed a flaw in how I
verified the others.
`biolink:encodes` is NOT a slot in the pinned biolink 4.4.0. My check was
`slots.get(name) or {}` and then read domain/range off the result, so an
ABSENT term returned None for both and I recorded it as "declares no
domain or range -- checked, not assumed". A term absent from the model
trivially declares nothing; the check could not distinguish absent from
unconstrained. The repo's own reports/biolink_coverage.tsv already flags
this row as the only applied biolink: CURIE with no backing slot.
The grounding is pre-existing (9 files) and is kept, because a gene
CLUSTER to protein COMPLEX edge does not fit `has gene product`'s
`range: gene product mixin` -- repointing would trade a missing term for
a wrong range. What is fixed is the mapping row's note, which claimed an
exact biolink label match that biolink_coverage.tsv contradicts. Filed
as #342.
mreB part-of rod_complex over-claimed. The edge's own snippet says MreB
filaments INTERACT WITH Rod complexes and the target node is described as
machinery associated with MreB; the corpus is split too, cell_shape.yaml
typing rod_complex a PATHWAY with MreB inside it. That is precisely the
kind of modelling decision this work defers, so both edges move to
interacts with (biolink:interacts_with, already mapped, range-safe),
which says what the quoted sentence says.
The four motive-force edges are reverted to enables and re-deferred.
biolink defines contributes to as contributing to the occurrence or
GENERATION of the object; a motive force does not generate the motor, it
powers rotation -- as their descriptions say. Migrating them while
deferring molecular_oxygen and cytoplasmic_na was inconsistent, and they
passed the audit only because contributes to declares no range, which is
the failure mode the audit exists to catch.
Also realigned the two mreB descriptions, which still read "organizes"
from the enables phrasing, and added correction events to all six records
rather than editing the originals -- a correction is its own event.
ENABLES_RANGE_VIOLATION: 33 -> 28, not the 24 first claimed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
All three were right, and the first exposed a flaw in how I verified the other two. 🟡
|
|
Claude finished @realmarcin's task in 3m 28s —— View job Re-reviewAll three 🟡 items are fixed, and I re-ran the domain/range check properly this time rather than trusting either of our earlier claims:
One item left. 🟡 The retracted verification is still asserted in the record it was wrong about. The correction events went to the six records whose edges changed.
That sentence is now contradicted inside this same PR by 🔵 On Keeping I still can't execute Python here, so --- · |
There was a problem hiding this comment.
All three earlier items are fixed and verified: mreB moves to interacts with with realigned descriptions, the four motive-force edges are reverted to enables and re-baselined, and the biolink:encodes mapping note now states the coinage plainly. Re-ran the domain/range check properly — part of (4806) and contributes to (3853) declare none, interacts with (2965) is named thing/named thing, encodes is absent. Baseline and report are byte-identical at 28 ENABLES_RANGE_VIOLATION rows, down from 33; pages are in sync.
One 🟡 left: methanogenesis.yaml carries the biolink:encodes edge but got no correction event, so its curation history still asserts 'All three declare no rdfs:domain or rdfs:range' — contradicted by predicate_grounding.tsv:28 in this same PR. human_pathogen.yaml has the same now-partly-stale enumeration. The six records whose edges changed all got correction events; the record the finding was actually about did not. See the detailed comment.
The correction events went to the six records whose EDGES changed, but not to methanogenesis.yaml -- which carries the biolink:encodes edge, the one the vacuous domain/range check actually covered. Its history still asserted "All three declare no rdfs:domain or rdfs:range", contradicted inside this same PR by predicate_grounding.tsv calling that term a local coinage. Applying "a correction is its own event" to every record except the one where the finding landed was the wrong place to stop. Added correction events to methanogenesis.yaml, stating that the justification was vacuous because the term is absent from the pinned biolink model, that the grounding is retained because a gene CLUSTER to protein COMPLEX edge does not fit `has gene product`'s range, and that the edge is still an improvement on enables whose declared range it genuinely violated. human_pathogen.yaml carries the same three-idiom enumeration and was partly stale for a second reason -- the "energy source ... the machine it powers" half was retracted when the motive-force edges were reverted. Its own surviving edge is unaffected, and the event says so. The 🔵 about `source=biolink` / `skos:exactMatch` still being machine- readable is real and deliberately not fixed here: introducing a `local` value to that column's vocabulary (METPO/biolink/RO/RDFS) affects every consumer of it. Folded into #342 with the reviewer's other suggestion, sweeping biolink_coverage.tsv for any further unbacked CURIE. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Right, and the place I stopped was the wrong one. 🟡 The record the finding actually landed inI applied "a correction is its own event" to the six records whose edges changed, and skipped Correction event added there, recording that the justification was vacuous because the term is absent from the pinned model, that the grounding is retained because a gene cluster → protein complex edge doesn't fit
🔵
|
|
Claude finished @realmarcin's task in 2m 13s —— View job Re-review (round 3) — approved
The last 🟡 is fixed. Re-checked the rest rather than trusting the earlier rounds:
🔵 Residual, not worth another round: all 8 records carry the same boilerplate three-idiom enumeration, and only these two now carry the encodes retraction — the other six correct only their own edges. The substantive correction is recorded where the edge lives, plus in Scoping Approved. |
There was a problem hiding this comment.
The last 🟡 is fixed. methanogenesis.yaml:295 — the record carrying the biolink:encodes edge, the one the vacuous check actually covered — now has a correction event stating the justification was vacuous because the term is absent from the pinned model, why the grounding is retained anyway (has gene product's range: gene product mixin would not accept a protein complex), and that it is tracked in #342. human_pathogen.yaml:299 got one covering both stale halves of its enumeration and noting its own surviving edge is unaffected. Originals untouched; both events append at 09:30:00Z, after the 08:00 and 09:00 events, so ordering holds on all 8 records.
Re-checked the rest rather than trusting the earlier rounds: baseline and report are byte-identical at 28 ENABLES_RANGE_VIOLATION rows; the five migrations are exactly 5× enables/RO:0002327 out and encodes ×1, part of ×1, interacts with ×2, contributes to ×1 in; zero snippet lines changed anywhere in the diff; the two regenerated pages carry the new events and their own per-record stamps moved while the six untouched trait pages did not. CI is green on qc, validate-strict, pr-sanity, curation-history and label-correspondence.
🔵 Residual, not worth another round: all 8 records carry the same boilerplate three-idiom enumeration, and only these two now carry the encodes retraction — the other six correct only their own edges. The substantive correction is recorded where the edge lives, plus in predicate_grounding.tsv and #342, so spraying six more identical events would add noise rather than accuracy.
Scoping source=biolink/skos:exactMatch to #342 is right — a local value in that column's vocabulary affects every consumer and is not this PR's call. Approving.
… (#344) * Normalise the corpus so the round-trip claim becomes enforceable (#322) write_validated_trait's comment claimed a byte-identical round trip. #343 corrected the claim to match reality -- it held for 127 of 477 records. This makes the claim TRUE instead, which is the half #322 left open. 350 records are rewritten through the helper's own emission path. No data changes; only formatting. safe_dump re-wraps long strings at its own width and drops hand-written quoting, so the diff is entirely those two things. Verified three independent ways rather than trusted: - The normaliser is TWO-PASS (#324's lesson): it computes and semantically checks every file before writing any, and aborts on the first mismatch rather than leaving the corpus half-normalised. 0 problems across 477. - Every rewritten file was re-parsed and compared to its pre-write document; a formatting pass that changed data would have failed there. - Independently, every file's parsed document was compared against `git show HEAD:` afterwards. 477 compared, 0 semantic differences. The strongest evidence is what did NOT change: pages/, reports/ and conf/ are byte-identical, because every generator and audit parses the YAML. Identical data in, identical artifacts out. The tests flip from documenting the gap to enforcing its absence. What asserted a 127/350 split now asserts that NO record fails to round trip, and the end-to-end test asserts equality rather than difference. Canaried by re-quoting a single scalar by hand: the suite fails. The payoff is that bulk scripts can use this helper again. #323, #328 and #341 all hand-rolled raw-line editors specifically to avoid the reflow churn, and that workaround is no longer necessary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Re-trigger CI The initial push produced no workflow runs at all despite the diff matching several paths: filters (data/traits/**, src/traitmech/**.py, tests/**.py). Close/reopen did not trigger them either. Empty commit to fire a synchronize event. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Re-trigger CI after the Actions outage This PR was opened at 19:24Z on 2026-08-06, during a critical GitHub Actions incident (stspg.io/rcz3fcm83sff) in which webhook triggers were throttled to ~15% and many push and pull_request events never created workflow runs. It received none, so it has sat CLEAN-looking and entirely unverified -- which is the state #346's detector now reports on every merge to main, and it flagged this PR within a minute of landing. The incident is resolved. Empty commit to fire a synchronize event now that they are being delivered again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Pin the corpus total, and fix prose the flip contradicted (PR #344 review) Both 🟡 are fair and the first is a regression I introduced when flipping the test. _split() skips anything that will not parse. The old assertion was `(len(same), len(changed)) == (127, 350)`, which pinned the total at 477, so a skipped record failed it. `assert not changed` does not -- a record edited into invalid YAML would drop silently out of the guard and the test would pass. Now asserts len(same) == len(TRAITS) as well. Canaried by appending invalid YAML to a record: it fails, and names the count. The second is the stale-prose-beside-updated-code mistake I have now made several times in this session. The module docstring still opened with the claim being "false for most of the corpus" three lines above saying it is now true, and test_the_helpers_own_output_round_trips still described the helper as "unsafe for a bulk rewrite" -- the exact opposite of what this PR establishes and of write_validated.py's own comment. Rewritten: the docstring is now past-tense about the pre-#322 state, and that test is described as what it actually asserts, the emitter's idempotence, which is a different property from the corpus being in that form. The 🔵 about a `str` representer emitting block scalars for the ~50 curation-history prose blocks is a good idea and is filed as #347 rather than taken here: it changes the emitted format, so it would mean re-normalising all 477 records inside a PR already under review, and the round-trip test would then enforce whichever form is chosen -- a one-way decision worth making deliberately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…iew) The sharpest point is one I should have caught: "audit-graphs improved 1535 -> 1521" was ANCHOR ACCOUNTING, not connectivity. pressure_optimum's 5-node island and ploidy's 4-node island stopped being reported UNREACHABLE_FROM_TRAIT without a single edge changing -- they just acquired an ungrounded anchor inside them. FRAGMENTED_GRAPH still says components=2 for both, and is still 218 overall. The islands are intact. Making a metric fall without improving the graph is exactly what I should be catching rather than causing, and the PR body said the opposite. Two of the retyped nodes duplicated traits that already exist and were left ungrounded, which the playbook requires of every TRAIT row (462 of 482 TRAIT nodes carry one, so ungrounded is the exception here): - hhp_tolerance IS piezotolerant, already traitmech:000003 -> grounded to it - growth_external_ph_5_5_9 is the same concept as its own record's ph_delta_high_trait -> grounded to METPO:1000478, which makes the duplication explicit rather than leaving two unrelated-looking anchors The playbook still told curators this case was unsettled. It now gives the rule the retyping followed -- a node described as a capacity to, an ability to, or a tolerance of is a disposition, so retype and use confers, and ground it -- plus the separate row for genuine states, with the occurrence-or-generation test that distinguishes them from #341's motive-force edges. Four edge descriptions still said "enables" under the new predicate; reworded. Filed #352 for the 🔵: salt tolerance, low-pH tolerance, oxygen tolerance and growth at 4C are the same concept still typed CAPACITY, surviving only because their in-edges did not happen to violate the range -- an unrelated fact. That wants the typing rule as its trigger, not a range-violation sweep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…351) * Retype six mis-typed dispositions, and route six real states (#334) The recommendation going in was to check node TYPES before touching predicates, because that has been the actual defect three times running (#328's assay_measurement, #331's unmodelled acetate kinase step, #330's negated node name). It holds again: half of the STATE/CAPACITY objects here are not states at all. Their own descriptions give them away -- "Capacity of an organism to grow and survive under high hydrostatic pressure", "Ability to grow when external phosphate is unavailable", "Detoxification-mediated tolerance of arsenic oxyanions". Those describe what an organism CAN do, which is what a TRAIT is. Five nodes retyped (one carries two in-edges), six edges then take METPO:2007700 confers unchanged, because with the object correctly a TRAIT the existing relation already fits. The other six objects ARE genuine states -- a proton gradient, a community composition, an internal microoxic environment -- so retyping them would be wrong. They take RO:0002326 (contributes to), which fits because the subject genuinely contributes to the OCCURRENCE OR GENERATION of the object. That is biolink's actual wording, and it is the same test the motive-force edges FAILED in #341, where the subject powers a machine it does not generate. Getting the same relation right here and wrong there is the distinction, not an inconsistency. ENABLES_RANGE_VIOLATION: 28 -> 16. Retyping to TRAIT adds reachability anchors, so audit-graphs improved: 1535 -> 1521 findings, none new. Also widened the confers gate for ORGANELLE, which appeared once buoyancy became a TRAIT (gas_vesicle confers buoyancy) -- listed rather than left open so each new subject kind stays an explicit decision. The #322 round-trip guard caught the hand-built curation events again; 11 records re-emitted through the helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Ground the new TRAIT anchors, and correct my audit claim (PR #351 review) The sharpest point is one I should have caught: "audit-graphs improved 1535 -> 1521" was ANCHOR ACCOUNTING, not connectivity. pressure_optimum's 5-node island and ploidy's 4-node island stopped being reported UNREACHABLE_FROM_TRAIT without a single edge changing -- they just acquired an ungrounded anchor inside them. FRAGMENTED_GRAPH still says components=2 for both, and is still 218 overall. The islands are intact. Making a metric fall without improving the graph is exactly what I should be catching rather than causing, and the PR body said the opposite. Two of the retyped nodes duplicated traits that already exist and were left ungrounded, which the playbook requires of every TRAIT row (462 of 482 TRAIT nodes carry one, so ungrounded is the exception here): - hhp_tolerance IS piezotolerant, already traitmech:000003 -> grounded to it - growth_external_ph_5_5_9 is the same concept as its own record's ph_delta_high_trait -> grounded to METPO:1000478, which makes the duplication explicit rather than leaving two unrelated-looking anchors The playbook still told curators this case was unsettled. It now gives the rule the retyping followed -- a node described as a capacity to, an ability to, or a tolerance of is a disposition, so retype and use confers, and ground it -- plus the separate row for genuine states, with the occurrence-or-generation test that distinguishes them from #341's motive-force edges. Four edge descriptions still said "enables" under the new predicate; reworded. Filed #352 for the 🔵: salt tolerance, low-pH tolerance, oxygen tolerance and growth at 4C are the same concept still typed CAPACITY, surviving only because their in-edges did not happen to violate the range -- an unrelated fact. That wants the typing rule as its trigger, not a range-violation sweep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Repair a description my own reword mangled (PR #351 round 2) The reword script matched on a 60-character prefix and substituted into it, which turned "proper polar growth and normal branch spacing" into "proper powth and normal branch spacing" in branched_shaped.yaml. Repaired, and swept every edge this PR touched for the same signature -- that is the only one. A regex that edits prose it does not parse was the wrong tool for four descriptions; doing them by hand would have been faster and safer. CURATION_PLAYBOOK.md still said "33 pre-existing edges" for ENABLES_RANGE_VIOLATION. It is 16 after this PR -- and the page is where a curator learns whether their edge shape is allowed, so a stale count there misstates how settled the area is. Third stale-prose spot in this PR, which is the same pattern #350 ended on. Added GROUND_CAUSAL_NODES events for the two groundings, per the 🔵. They were a real change to the records, not just metadata: an ungrounded TRAIT node still counts as a reachability anchor, which is how the audit number moved without the graph changing. Folded the other 🔵 into #352: nothing detects two nodes sharing a grounding within one graph, which is the machine-readable signature of exactly the duplication that issue is about, and would have found the ph_delta_high case without anyone reading descriptions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
biolink declares
enablesrange 'biological process or activity' — onlyBIOLOGICAL_PROCESS,PATHWAYandMOLECULAR_FUNCTIONsatisfy it, so the 33 edgespointing elsewhere each entail a false type.
#334 asks for group-by-idiom rather than one sweep, because the 33 mix distinct claims.
This takes the
GENE_OR_PROTEIN-object group's unambiguous half.What reading them showed
The 13 protein-object edges are roughly seven idioms, not one. Only those with an exact
and range-safe upstream term are migrated:
encodes(biolink:encodes)part of(biolink:part_of)contributes to(RO:0002326)All three declare no
rdfs:domainorrdfs:range— checked, not assumed — so none canreintroduce the class being removed. All three already had mapping rows, so nothing new
was minted.
Four deliberately left
Each needs a modelling decision, not a relabel:
molecular_oxygen → cytochrome bdandcytoplasmic_na → Mrp complex— substrateavailability; the honest form is probably the complex taking the chemical as
has input,which reverses the edge.
flagellar_motor_torque → flagellar_motor— the subject is a property of the object;the edge is confused.
Hfq → RpoS— "enables sRNA-dependent translation of RpoS": the object should be thattranslation process, which isn't currently a node.
Result
ENABLES_RANGE_VIOLATION: 33 → 24. The remaining 24 stay baselined under #334.A nice confirmation
This change touched 8 trait files and rewrote 8 pages. Before #304 landed earlier today
it would have rewritten 477 — the reviewability fix paying off on the very next data PR.
What was checked
just qcgreen ·just validate-strict477 / 0 errors · 434 tests pass ·audit-graphsunchanged at 1535 · snippet baseline steady at 2738 · curation events on all8 records with histories verified in order.
Partial burn-down of #334 (stays open for the remaining 24).
🤖 Generated with Claude Code