Skip to content

Migrate the 185 mechanical microbe-domain edges (#301, part 1 of 2) - #328

Merged
realmarcin merged 2 commits into
mainfrom
feat/301-migrate-mechanical-families
Aug 6, 2026
Merged

Migrate the 185 mechanical microbe-domain edges (#301, part 1 of 2)#328
realmarcin merged 2 commits into
mainfrom
feat/301-migrate-mechanical-families

Conversation

@realmarcin

Copy link
Copy Markdown
Contributor

First of the two migrations planned in proposals/metpo_traitmech_v9 (#326).

What moved

67 METPO object properties inherit rdfs:domain METPO:1000525 (microbe) via
METPO:2000001. OWL domain is an inference rule, not a constraint, so a causal-graph
edge using one entails its subject is a microbe — and CausalNodeTypeEnum has no
organism member. This migrates the 185 edges whose target was already decided or is
upstream:

Family Edges Source
any subject → TRAIT object 62 METPO:2007700 confers v8
activity subject produces X 103 RO:0002234 has output RO, upstream
TRAIT uses carbon source 12 METPO:2007806 has carbon source v9
TRAIT uses energy source 8 METPO:2007807 has energy source v9
185

Every edge keeps its direction. This pass only relabels and re-grounds — that is what
keeps a 130-file change reviewable, and it's why the evidence-snippet baseline is
untouched (a reversal changes an edge's baseline key; nothing here does).

Family B takes an RO term rather than a new METPO one because biolink gives has output
domain: biological process or activity, which its BIOLOGICAL_PROCESS/PATHWAY/
MOLECULAR_FUNCTION subjects satisfy. Its mapping row is gated to exactly those three,
so a protein-subject edge is refused rather than silently re-acquiring the very domain
violation this removes.

The guard earned its keep

The script keys targets on (subject, object, predicate_id), not the endpoint pair.
Two corpus cases make the pair unsafe, and the count guard caught both before anything
was written:

  • ecology/biosafety_level_1.yaml has low_pathogen_hazard --produces--> bsl1_trait
    (a target) and low_pathogen_hazard --qualifies for--> bsl1_trait
    (ungrounded, not a target) on the same endpoints. Keying by pair would have
    rewritten the ungrounded edge too — a silent corruption.
  • physiology/chemoheterotrophic.yaml has both uses energy source and uses carbon source between the same two nodes, taking different targets, so one key cannot
    carry both.

Both were canaried after the fix: the ungrounded edge is untouched, and the two
same-endpoint edges get their correct distinct targets.

The proof it landed

MICROBE_DOMAIN_ON_NONORGANISM 366 → 181 — exactly the number the v9 round-trip plan
predicted, which is the point of having stated it.

What was checked

  • just validate-strict — 477 files, 0 errors.
  • just qcgreen, every ratchet at blocking: 0.
  • uv run pytest420 passed. Ruff clean.
  • Evidence-snippet baseline unchanged at 2737, 0 new — the expected signature of a
    no-reversal pass.
  • Gates verified against the data before writing them: has carbon source objects are
    12/12 CHEMICAL; has energy source is 2 CHEMICAL + 6 ENVIRONMENTAL_FACTOR, which is why
    its object_types is wider than its sibling's.
  • Widened the confers row to admit CAPACITY — one family-A edge has a CAPACITY
    subject, which the v8 gate did not list and which would otherwise have been refused.

Scope

Does not touch the 181 edges needing the other 11 v9 terms (enzyme, transport, and the
rest of produces) — that's part 2, taking the baseline 181 → 0. The
METPO:20078xx ids remain placeholders until upstream mints them.

🤖 Generated with Claude Code

67 METPO object properties inherit rdfs:domain METPO:1000525 (microbe)
via METPO:2000001. OWL domain is an inference rule, not a constraint, so
a causal-graph edge using one entails that its subject IS a microbe, and
CausalNodeTypeEnum has no organism member. This moves the 185 edges whose
target was already decided or is upstream:

  62   any subject -> TRAIT object      -> METPO:2007700 confers      (v8)
  103  activity subject `produces` X    -> RO:0002234    has output   (RO)
  12   TRAIT uses carbon source         -> METPO:2007806 has carbon source
  8    TRAIT uses energy source         -> METPO:2007807 has energy source

Every edge keeps its direction; this pass only relabels and re-grounds,
which is what keeps a 130-file change reviewable. The 181 that need the
other 11 v9 terms follow in part 2.

Family B takes RO rather than a new METPO term because biolink gives
has output domain 'biological process or activity', which the
BIOLOGICAL_PROCESS, PATHWAY and MOLECULAR_FUNCTION subjects satisfy.
Its mapping row is gated to exactly those three, so a protein-subject
edge is refused rather than silently re-acquiring the domain violation
this migration removes.

The script keys targets on (subject, object, predicate_id), NOT on the
endpoint pair. Two corpus cases make the pair unsafe and the guard caught
both before anything was written:

- ecology/biosafety_level_1.yaml has `low_pathogen_hazard --produces-->
  bsl1_trait` (a target) and `low_pathogen_hazard --qualifies for-->
  bsl1_trait` (UNGROUNDED, not a target) on the same endpoints. Keying by
  pair would have rewritten the ungrounded edge too.
- physiology/chemoheterotrophic.yaml has both `uses energy source` and
  `uses carbon source` between the same two nodes, taking DIFFERENT
  targets, so one key cannot carry both.

Also widened the confers mapping row to admit CAPACITY: one family-A edge
has a CAPACITY subject, which the v8 gate did not list.

Baseline: MICROBE_DOMAIN_ON_NONORGANISM 366 -> 181, matching the round-trip
plan in proposals/metpo_traitmech_v9 exactly. Evidence-snippet baseline
unchanged at 2737 with 0 new, as expected when no edge is reversed.
validate-strict 477 files / 0 errors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@culturebot-reviewer

culturebot-reviewer Bot commented Aug 6, 2026

Copy link
Copy Markdown

Claude finished @realmarcin's task in 6m 34s —— View job


Review

I verified the migration mechanically rather than taking the numbers on faith. The data pass is clean; one bookkeeping defect.

What I confirmed

  • The 185 removed baseline rows are exactly METPO:2000202×165 + METPO:2000006×12 + METPO:2000010×8, with zero rows added — so nothing regressed into the ratchet. conf/predicate_domain_audit_baseline.tsv is 182 lines (181 + header), matching the claimed 366 → 181.
  • Across data/traits/** the diff removes 500 lines: 370 predicate/predicate_id lines (= 185 edges × 2) and 130 file headers. Nothing else. No snippet, reference, notes, node, or description line is touched anywhere, which is the strongest possible form of the "evidence baseline unchanged" claim. 130 files, 130 MIGRATE_MICROBE_DOMAIN_EDGES events.
  • All 20 family-D edges have subject_type=TRAIT, so the TRAIT subject gate is exact and not a guess; energy-source objects are organic_molecule, reduced_chemical_substrate, and light×6, which is what makes the wider CHEMICAL|ENVIRONMENTAL_FACTOR cell correct rather than sloppy.
  • RO:0002234 is has output — correct term, correct label. METPO:2007806 / 2007807 match proposals/metpo_traitmech_v9/metpo_proposal_properties_robot.tsv on both id and label, and their 12/8 counts match the proposal's table exactly.
  • The biosafety_level_1.yaml guard case is real and I checked it: low_pathogen_hazard --qualifies for--> bsl1_trait is at line 112, same endpoints as the migrated edge, still ungrounded, untouched. Triple-keying was the right call.
  • confers is now 207 edges = 145 (v8) + 62. CAPACITY is a valid CausalNodeTypeEnum member.

🟡 The confers widening is recorded in one place and contradicted in two

mappings/predicate_grounding.tsv:102 gains CAPACITY in subject_types, but the note in that same row still reads:

subject_types lists the 9 node types actually observed across the 145 migrated edges

The cell now lists 10, and the corpus now has 207. A row that contradicts itself is worse than one that is merely out of date, because the note is the thing a future curator reads to decide whether widening it again is allowed.

The same claim is stale in the artifact that actually goes upstream — proposals/metpo_traitmech_v8/metpo_proposal_properties_robot.tsv:3:

covers 145 causal-graph edges across 114 trait records … Subject types observed: BIOLOGICAL_PROCESS 81, GENE_OR_PROTEIN 32, … STATE 1.

CAPACITY (morphology/cell_width_small.yaml#cell_width_small_mreb_setpoint, rod_diameter_control --> cell_width_small_trait) is absent from that enumeration. That enumeration is the domain evidence a METPO curator will use to decide what METPO:2007700 admits, and this PR is the change that made it wrong. The v9 proposal documents the 62-edge addition but never mentions CAPACITY, so nothing else in the repo carries it.

Fix this →


🔵 The residual report now describes a corpus state that was not written

reports/predicate_grounding_residual.tsv moves has output from 4 unmapped to 3 blocked_by_node_type. Three of those four are genuinely blocked. The fourth — data/traits/upper/biological_process.yaml:129, molecular_function --has output--> chemical_output, whose own description already cites RO:0002234 — now passes the new gate, which is why the count dropped. But no predicate_id was written for it: the diff adds exactly 370 predicate lines, all of them replacements. So the committed report says that edge is groundable while the corpus still leaves it bare. A just ground-predicates --apply closes the gap; worth doing here so the report and the data agree at the same commit.

🔵 No history/ record

history/records/ is unchanged for a 130-file data/traits/** change. Advisory per history/README.md, and the Curation history check passed — noting it, not blocking on it.


Everything else about this is right, including the parts that were easy to get wrong: the object_type == TRAIT test running before the produces test, has output gated to the three activity node types so part 2's protein subjects are refused rather than silently re-broken, and the count guard aborting on a partial match instead of writing one. CI is green across all 7 completed checks.

Verdict: request changes — for the self-contradicting confers row and the stale v8 proposal evidence. Both are small; the migration itself needs nothing.
· branch feat/301-migrate-mechanical-families

@culturebot-reviewer culturebot-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migration itself verified clean: the 185 removed baseline rows are exactly METPO:2000202x165 + METPO:2000006x12 + METPO:2000010x8 with zero rows added; the data diff touches only the 370 predicate/predicate_id lines and nothing else (no snippet, reference or node line anywhere); all 20 family-D subjects really are TRAIT; RO:0002234 and the two v9 ids/labels all check out; and the biosafety_level_1.yaml guard case is real (line 112, same endpoints, still ungrounded and untouched).

Requesting changes for one bookkeeping defect. mappings/predicate_grounding.tsv:102 gains CAPACITY in subject_types while its own note still reads 'the 9 node types actually observed across the 145 migrated edges' - the cell now lists 10 and the corpus has 207. The same claim is stale in the artifact that goes upstream, proposals/metpo_traitmech_v8/metpo_proposal_properties_robot.tsv:3, whose 'Subject types observed' enumeration omits CAPACITY. That enumeration is the domain evidence a METPO curator uses to decide what METPO:2007700 admits, and this PR is what made it wrong.

Also worth folding in (non-blocking): reports/predicate_grounding_residual.tsv drops 'has output' from 4 to 3 because data/traits/upper/biological_process.yaml:129 now passes the new gate, but no predicate_id was written for it - so the committed report and the corpus disagree at the same commit. A 'just ground-predicates --apply' closes that. And there is no history/ record for a 130-file data/traits change (advisory only).

Full detail in #328 (comment)

…328 review)

The confers mapping row gained CAPACITY in subject_types but its note
still said "9 node types across 145 migrated edges" - the cell listed 10
and the corpus had 207. A row that contradicts itself is worse than one
merely out of date, since that note is what a curator reads to decide
whether widening it again is allowed. Now says 10 types across 207 edges
and records where CAPACITY entered.

The same claim was stale in the artifact that actually goes upstream:
proposals/metpo_traitmech_v8's METPO:2007700 row carried "covers 145
causal-graph edges across 114 trait records" with a subject-type
enumeration that omitted CAPACITY. That enumeration is the domain
evidence a METPO curator uses to decide what the term admits, and this
PR is what made it wrong. Updated to 207 edges / 172 records with the
full 10-type breakdown, and it names #328 as the cause so the v8
narrative stays historically readable.

Also ran ground-predicates --apply, which the new has output row made
newly applicable to one edge: upper/biological_process.yaml's
`molecular_function --has output--> chemical_output`, whose own
description already cited RO:0002234 but which carried no predicate_id.
The committed residual report had begun describing it as groundable while
the data left it bare; report and corpus now agree at the same commit.
The two edges reported blocked_by_node_type are the gates working as
intended - an EXPERIMENTAL_FACTOR subject for has output and a
BIOLOGICAL_PROCESS object for confers are both correctly refused.

No history/records entry, per the advisory; that convention gap is
tracked in #325.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@realmarcin

Copy link
Copy Markdown
Contributor Author

Both fixed, and the 🔵 residual one was the more useful catch.

🟡 The self-contradicting confers row

Right, and the framing is right too — a row that contradicts itself is worse than one merely stale, because that note is what a curator reads to decide whether widening it again is allowed. It now reads 10 node types across 207 migrated edges, and records that CAPACITY entered via this PR.

The upstream artifact was the more important half. proposals/metpo_traitmech_v8's METPO:2007700 row said "covers 145 causal-graph edges across 114 trait records" with an enumeration omitting CAPACITY. Recomputed from the corpus: 207 edges / 172 records, subject types BIOLOGICAL_PROCESS 139, GENE_OR_PROTEIN 32, ENVIRONMENTAL_FACTOR 12, QUALITY 6, CHEMICAL 5, CELLULAR_LOCALIZATION 5, PATHWAY 4, MOLECULAR_FUNCTION 2, STATE 1, CAPACITY 1. The cell names #328 as the cause, so the v8 narrative stays historically readable rather than being retconned.

🔵 The residual report describing unwritten state — fixed

Good catch, and subtle: the new has output row made grounding newly applicable to upper/biological_process.yaml's molecular_function --has output--> chemical_output, whose own description already cited RO:0002234 but which carried no predicate_id. The committed report had started calling it groundable while the data left it bare.

Ran just ground-predicates --apply; report and corpus now agree at the same commit. The two remaining blocked_by_node_type rows are the gates working as intended — an EXPERIMENTAL_FACTOR subject for has output and a BIOLOGICAL_PROCESS object for confers are both correctly refused.

🔵 history/ record

Acknowledged, still not added — the convention gap and whether it should be enforced is tracked in #325, filed during the v8 migration for the same reason.

Re-verified

just qc green, just validate-strict 477 / 0 errors, uv run pytest 420 passed, reports and pages/ regenerated. Baseline still 181, unchanged by these fixes.

@culturebot-reviewer

culturebot-reviewer Bot commented Aug 6, 2026

Copy link
Copy Markdown

Claude finished @realmarcin's task in 4m 12s —— View job


Re-review of the fix commit (36e7720b)

Both requested changes landed, and I re-verified them against the corpus rather than the commit message.

The confers bookkeeping — fixed and correct, not just edited

  • mappings/predicate_grounding.tsv:102 now says "the 10 node types actually observed across the 207 migrated edges (145 from Migrate 164 enables→TRAIT edges onto the v8 predicates (#302, #303) #323, plus 62 more in Migrate the 185 mechanical microbe-domain edges (#301, part 1 of 2) #328, which is where CAPACITY entered)". Cell and note agree, and the note now carries the provenance of the widening, which is the part a future curator actually needs.
  • proposals/metpo_traitmech_v8/metpo_proposal_properties_robot.tsv:3 → 207 edges / 172 records. I counted independently: predicate_id: METPO:2007700 appears 207 times across 172 files under data/traits/**. Exact match.
  • The new enumeration sums to 207, and its delta against the old one is BIOLOGICAL_PROCESS +58, CHEMICAL +2, QUALITY +1, CAPACITY +1 = 62, exactly the family-A count this PR claims. The counts are internally consistent with the migration, not just plausible.
  • CAPACITY 1 is exact. Twelve trait files contain both a CAPACITY node and a confers edge; I checked the subject of every confers edge in each. Only morphology/cell_width_small.yaml's rod_diameter_control (line 58, node_type: CAPACITY) is one — every other file's confers subject is an ENVIRONMENTAL_FACTOR, BIOLOGICAL_PROCESS, or CHEMICAL node that happens to co-occur with an unrelated CAPACITY node. So the count is one, and the "1" is the right one.
  • The row keeps naming Migrate the 185 mechanical microbe-domain edges (#301, part 1 of 2) #328 as the cause rather than rewriting v8's history — the right call for an artifact that goes upstream with a narrative attached.

The residual/corpus disagreement — closed

data/traits/upper/biological_process.yaml:136 now carries predicate_id: RO:0002234 on molecular_function --has output--> chemical_output, with a GROUND_CAUSAL_PREDICATES history entry. The gate holds on its own terms: the subject node is MOLECULAR_FUNCTION (line 36), one of the three activity types has output admits. reports/predicate_grounding_residual.tsv:89 still reads has output 3 blocked_by_node_type, but upper/biological_process.yaml has dropped out of its example list — the count is now 3 because three edges are genuinely blocked, not because one was silently counted as grounded. Report and corpus agree at this commit.

No collateral damage

Outside those three files the commit touches only pages/**, and every one of those is the regenerated Corpus as of … UTC timestamp line. conf/predicate_domain_audit_baseline.tsv is untouched, so the 181 baseline stands. All 7 completed checks green.

One factual note, not a defect: the PR comment says "the two remaining blocked_by_node_type rows", but the report has three such rows (causally upstream of 13, confers 3, has output 3) — the two named are edge examples, not the whole blocked set. Nothing committed is wrong; it just understates how much the gates are refusing.

The history/ record is still absent, still advisory, and #325 is the right place for it.

Verdict: approve. Both bookkeeping defects are properly fixed, and the fix for the second one changed data, so it was worth doing here rather than deferring.


Note: Python was not available to me in this run, so the count verification above was done with git, ripgrep, and file reads rather than by re-running the repo's own audit scripts. The 207/172 figure and the CAPACITY-is-1 finding are direct corpus counts; the nine other per-type subject counts I checked by arithmetic consistency and spot-check (neutrophilic, free_living, ph_delta, cell_width_small), not by exhaustive recount.
· branch feat/301-migrate-mechanical-families

@culturebot-reviewer culturebot-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both requested changes verified against the corpus, not just the commit message.

The confers row now agrees with itself (10 types / 207 edges) and records that CAPACITY entered via #328. I counted independently: METPO:2007700 appears 207 times across 172 files under data/traits/**, an exact match for the v8 proposal's updated traits_addressed. The new subject-type enumeration sums to 207, and its delta against the old one (BIOLOGICAL_PROCESS +58, CHEMICAL +2, QUALITY +1, CAPACITY +1) is exactly the 62 family-A edges this PR migrated. CAPACITY 1 is exact: twelve files contain both a CAPACITY node and a confers edge, and only cell_width_small.yaml's rod_diameter_control is actually a confers subject.

The residual/corpus disagreement is closed - biological_process.yaml:136 now carries predicate_id RO:0002234, its subject node is MOLECULAR_FUNCTION so the new gate holds on its own terms, and upper/biological_process.yaml has dropped out of the report's has-output example list, so the 3 blocked edges are genuinely blocked rather than one being silently miscounted.

Outside those three files the commit touches only pages/**, all of it the regenerated timestamp line. The 181 baseline is untouched. All checks green.

Minor, non-blocking: the PR comment says 'the two remaining blocked_by_node_type rows' but the report has three (causally upstream of 13, confers 3, has output 3) - the two named are edge examples. Nothing committed is wrong.

@realmarcin
realmarcin merged commit 6097162 into main Aug 6, 2026
8 checks passed
@realmarcin
realmarcin deleted the feat/301-migrate-mechanical-families branch August 6, 2026 04:01
realmarcin added a commit that referenced this pull request Aug 6, 2026
The 🟡 is the same failure #328 had, and the reviewer is right that #327
set the opposite precedent. Adding the `has input` row made
upper/biological_process.yaml's `molecular_function --has input-->
chemical_input` edge LOOK mapped, so it dropped out of the residual
report, but nothing grounded it -- leaving it in neither the grounded set
nor the backlog, with the next `ground-predicates --apply` silently
touching a file this PR had not.

Grounded to RO:0002233, which its own evidence note already named
("Has input [RO:0002233]") and which its sibling has-output edge has
carried all along. Curation event appended by the tool.

Both 🔵s recorded as decisions in the curation event rather than left as
omissions, which is what the reviewer asked for:

- The node is typed BIOLOGICAL_PROCESS, not MOLECULAR_FUNCTION. The
  latter would have grounded to GO:0008776 via node_grounding.tsv, but
  this trait's own research report asks for a process label plus explicit
  reaction edges for the SLP step, and that is exactly what this is.
- The step carries has-output atp but deliberately not has-output
  acetate. The existing acetyl_phosphate is-converted-to acetate edge
  already states that half of the reaction on the chemical chain;
  asserting it again at the process level is the duplication at two
  levels of description that #303 criticised in
  dissimilatory_iron_reduction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
realmarcin added a commit that referenced this pull request Aug 6, 2026
…) (#339)

* Model the acetate kinase step instead of a chemical producing ATP (#331)

`acetyl_phosphate --produces--> atp` had a CHEMICAL subject on
METPO:2007800, whose definition is that the ACTIVITY OR STATE of the
subject brings the object into existence. A chemical has neither in that
sense. RO:0001001 (derives into) did not fit either, and #327 said why:
ATP's matter comes mostly from ADP and only the phosphoryl group
transfers, so it fails that relation's inherits-the-significant-portion-
of-the-matter test. That is why it was filed rather than swept into #327.

The resolution was already written in the edge's own description -- "The
acetate kinase step generates ATP" -- and in its evidence, which says ACK
converts acetyl phosphate to acetate yielding ATP. The process was named
but not modelled. Added it as a BIOLOGICAL_PROCESS node and split the
edge into `acetate_kinase_step has input acetyl_phosphate` (RO:0002233)
and `acetate_kinase_step has output atp` (RO:0002234), both range-correct.

Wired on BOTH sides deliberately: atp had exactly one edge, so replacing
it without an input edge would have orphaned the new node and atp
together. audit-graphs confirms no new findings.

This was the last CHEMICAL-subject `produces` edge, so METPO:2007800's
subject_types gate drops CHEMICAL and now matches its definition exactly.
Added a `has input` mapping row (RO:0002233), gated to the three activity
node types for the same reason `has output` is.

Evidence-snippet baseline moves 2737 -> 2738, and the delta is exactly
explainable: the pre-existing MISSING_SNIPPET re-keys onto the new
has-output edge, plus one genuinely new finding because the has-input
edge reuses the same evidence, which carries notes but no verbatim
snippet. A snippet cannot be invented -- the schema specifies a verbatim
quote and the source text is not to hand -- so it joins the known
backlog rather than being fabricated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Ground the has-input edge the new mapping row exposed (PR #339 review)

The 🟡 is the same failure #328 had, and the reviewer is right that #327
set the opposite precedent. Adding the `has input` row made
upper/biological_process.yaml's `molecular_function --has input-->
chemical_input` edge LOOK mapped, so it dropped out of the residual
report, but nothing grounded it -- leaving it in neither the grounded set
nor the backlog, with the next `ground-predicates --apply` silently
touching a file this PR had not.

Grounded to RO:0002233, which its own evidence note already named
("Has input [RO:0002233]") and which its sibling has-output edge has
carried all along. Curation event appended by the tool.

Both 🔵s recorded as decisions in the curation event rather than left as
omissions, which is what the reviewer asked for:

- The node is typed BIOLOGICAL_PROCESS, not MOLECULAR_FUNCTION. The
  latter would have grounded to GO:0008776 via node_grounding.tsv, but
  this trait's own research report asks for a process label plus explicit
  reaction edges for the SLP step, and that is exactly what this is.
- The step carries has-output atp but deliberately not has-output
  acetate. The existing acetyl_phosphate is-converted-to acetate edge
  already states that half of the reaction on the chemical chain;
  asserting it again at the process level is the duplication at two
  levels of description that #303 criticised in
  dissimilatory_iron_reduction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
realmarcin added a commit that referenced this pull request Aug 6, 2026
* Say what write_validated_trait actually does to formatting (#322)

The comment claimed that re-running the helper over an existing file
"produces a byte-identical diff". Measured against the corpus: it holds
for 127 of 477 files. The other 350 are reformatted, because safe_dump
re-wraps long strings at its own width and drops hand-written quoting.

The claim is not harmless. It is what a bulk-script author reads before
deciding how to rewrite N files, and believing it means burying the real
change in reflow churn across every long string in every file touched --
the difference between a reviewable migration and an unreviewable one.
Both the #323 and #328 migrations edit raw lines instead, and this issue
exists because the claim was checked rather than trusted.

Corrected to state the real behaviour with the measured number, to say
plainly that it matters for BULK use and not for a single record the
helper already owns, and to note that making the claim true would mean
normalising all 477 files once behind a round-trip test -- a large
one-time reformat, which is the part of #322 still open.

Three tests pin the two causes (re-wrapping, dropped quoting) and the
case where the round trip DOES hold, so the claim cannot quietly return.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Bind the round-trip tests to the helper's real options (PR #343 review)

The finding is exact and it is this PR's own thesis one level up. The
tests asserted on yaml.safe_dump against a re-declared DUMP_OPTS dict,
with nothing tying it to the helper. Adding width=4096 to the real
options would have stopped the re-wrapping, falsified both the comment's
stated cause and its 350/127 counts, and left every test passing --
because they were testing PyYAML's defaults, not write_validated_trait.

Fixed structurally rather than by adding another assertion: the options
are now EMIT_OPTS at module scope in write_validated.py and imported by
the tests, so there is no copy to drift. The helper composes them with
its yaml_kwargs exactly as before.

Rewritten to exercise the real thing:

- a hand-edited corpus file written through write_validated_trait to a
  tmp_path comes back with different bytes
- the helper's OWN output round-trips byte-identically, which is the
  boundary that makes it safe for one record and unsafe for a bulk pass
- the documented 127/350 split is asserted against the live corpus, so
  the numbers in the comment cannot go stale as a snapshot -- which is
  how the original claim came to mislead in the first place. It will fail
  loudly when #322's normalisation lands, which is the right moment to
  update them.

Canaried by injecting width=4096 into the REAL options: the split test
fails. The first test still passes there, because quoting differences
alone keep some files reformatted -- worth knowing that the split test is
the one carrying that guarantee.

The corpus walk is lru_cached; three tests need it and each pass parses
all 477 records (20s -> 7s).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Bind the split to the emit path, and stop the docstring over-claiming (PR #343 round 2)

The 🟡 is a fair hit: that test's docstring said "change them so the
reformatting stops and this fails", and I had ALREADY measured that it
does not -- injecting width=4096 leaves it passing, because dropped
quoting alone still reformats some files. I reported that honestly in the
PR comment and then left the docstring asserting the opposite, which is
worse than not having measured it. Corrected to say what is true: this
one demonstrates the behaviour end to end, and the corpus-split test is
what carries the guarantee.

The 🔵 was the same class as the original finding, one step smaller:
_split() rebuilt `yaml.safe_dump(doc, **EMIT_OPTS)` itself, so it bound
to the options but not to how they are COMPOSED with yaml_kwargs. A
change to the composition would have gone unnoticed. Extracted
emit_trait_yaml() as the single emit path, used by write_validated_trait
and imported by the tests, so there is nothing left to re-implement.

Canaried the composition specifically this time, not just the options
dict: injecting width into the composition expression fails the split
test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
realmarcin added a commit that referenced this pull request Aug 7, 2026
… (#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>
realmarcin added a commit that referenced this pull request Aug 7, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant