Add Gene2Phenotype (G2P) terminology value sets

[cmungall]

Summary

This PR adds comprehensive value set definitions for Gene2Phenotype (G2P), the EBI resource of curated gene-disease associations. The implementation includes controlled vocabularies for all components of G2P's locus-genotype-mechanism-disease-evidence (LGMDE) framework.

Key Changes

• New schema file: src/valuesets/schema/clinical/gene2phenotype.yaml (1066 lines)

  • Defines 14 enums covering G2P terminology across confidence, allelic requirements, molecular mechanisms, variant consequences, and variant types
  • Includes comprehensive descriptions and mappings to community standards (GenCC, HPO, Sequence Ontology, ClinGen)
  • Defines 8 slots for use in clinical data models

• Enum definitions:

  • G2PConfidenceCategory: Gene-disease association confidence levels (DEFINITIVE, STRONG, MODERATE, LIMITED, DISPUTED, REFUTED) with GenCC mappings
  • G2PAllelicRequirement: Genotype requirements using HPO mode-of-inheritance terms (autosomal dominant/recessive, X-linked, Y-linked, PAR, mitochondrial)
  • G2PCrossCuttingModifier: Inheritance qualifiers (anticipation, imprinting, de novo, mosaic, incomplete penetrance, secondary findings, restricted mutations)
  • G2PMolecularMechanism: Disease mechanisms per Backwell & Marsh framework (loss-of-function, gain-of-function, dominant-negative, undetermined)
  • G2PMolecularMechanismSynopsis: Refined mechanism descriptions (destabilizing LOF, assembly-mediated GOF, aggregation, etc.)
  • G2PMolecularMechanismSupport: Evidence vs. inferred mechanism classification
  • G2PMechanismEvidenceCategory and related enums: ClinGen-based evidence categories (function, functional alteration, models, rescue)
  • G2PVariantConsequence: Sequence Ontology terms for variant consequences (altered/decreased/increased/absent gene product level, altered structure, uncertain)
  • G2PVariantType: Comprehensive SO variant types organized by group (NMD variants, splice variants, regulatory, protein-changing, other)
  • G2PPanel: Disease-area panels for organizing curated associations

• Updated main schema: Added import of clinical/gene2phenotype to src/valuesets/schema/valuesets.yaml

• Updated ontology cache: Added 40 Sequence Ontology terms and 5 HPO terms to support G2P variant consequence and allelic requirement mappings

Implementation Details

• All enum definitions follow G2P's official terminology page (https://www.ebi.ac.uk/gene2phenotype/about/terminology)
• Reuses community standards where they exist: GenCC classifications, HPO terms, Sequence Ontology, and ClinGen SOPs
• Includes detailed descriptions and usage notes for each term
• Provides mappings via meaning: to ontology terms using OBO CURIEs
• Includes aliases for alternative terminology (e.g., "confirmed" as alias for DEFINITIVE confidence)
• Annotations track stewards, publishers, and source references

https://claude.ai/code/session_01De8cxj98MtmCDAW3uuUZQ7

[claude]

Claude finished @cmungall's task in 3m 45s —— View job

---

Review: Gene2Phenotype (G2P) terminology value sets

• Gather context (read diff + CLAUDE.md conventions)
• Review schema structure & naming conventions
• Verify ontology CURIEs / prefix declarations
• Attempt just validate / just site — blocked, see note at bottom
• Post review feedback

Overall this is a well-researched, carefully sourced addition. Terms are grouped by domain, every enum carries status, contributors, instantiates: valuesets_meta:ValueSetEnumDefinition and provenance annotations matching the repo idiom; prefixes are declared correctly; and there are no slot or enum name collisions with the other 100+ schemas imported into valuesets.yaml (I checked all 8 new slot names). Every meaning: CURIE is backed by a cache entry. Notes below, roughly by severity.

---

Correctness

1. Three close_mappings CURIEs are not in the term cache — src/valuesets/schema/clinical/gene2phenotype.yaml:342,355,367

SO:0002054, SO:0002053, SO:0002052 are the only ontology CURIEs in the file with no corresponding row in cache/so/terms.csv. Every meaning: term was cached; these three were not. Either the cache refresh skipped close_mappings, or the IDs were added without an OLS check — CLAUDE.md is explicit that IDs must always be verified, never guessed. Please confirm and add them to the cache so the mappings are covered by the same validation as everything else.

Fix this →

2. TYPICALLY_DE_NOVO description contradicts the term it defines — gene2phenotype.yaml:278-285

Plausible disease causing mutations that occur post zygotically (formation of gametes).

Post-zygotic events are somatic — that is exactly what TYPICALLY_MOSAIC describes ten lines below (:286). De novo variants arise pre-zygotically, in the parental germline. The parenthetical also glosses "post zygotically" as gametogenesis, which is the opposite of what post-zygotic means. Neither reading is consistent with the mapped HP:0025352. Worth re-checking against the G2P terminology page and correcting.

3. Is MONOALLELIC_X an actual G2P value? — gene2phenotype.yaml:178-183

The G2P allelic-requirement list I'm aware of distinguishes monoallelic_X_hemizygous and monoallelic_X_heterozygous; a bare monoallelic_X looks like a parent introduced to carry HP:0001417. If it isn't in G2P's published list, including it lets a downstream curator emit a code G2P won't accept. If it is a real value, ignore this. The same completeness question applies in reverse to G2PCrossCuttingModifier — please double-check the full source list was captured (e.g. requires heterozygosity).

4. title: on G2PAllelicRequirement values is the HPO label, not the G2P label — gene2phenotype.yaml:162-163 and throughout that enum

MONOALLELIC_AUTOSOMAL:
  title: Autosomal dominant inheritance   # <- HPO's label
  aliases: [monoallelic_autosomal]        # <- G2P's own label

Elsewhere in the repo (clinical/genetics.yaml:33-38) title is the label of the term the key denotes. Here a consumer rendering this value set shows "Autosomal dominant inheritance" for a code whose own label is monoallelic_autosomal. The enum description asserts each value is an exact synonym of its HPO term, but that's arguably not true in general — monoallelic_autosomal plus incomplete penetrance is not classic autosomal dominant, which is precisely why G2P layers cross-cutting modifiers on top. Suggest title: = the G2P label, HPO label as an alias, meaning: retained as the mapping, and softening "exact synonym" to "closely corresponds to".

---

Modeling & consistency

5. GenCC identifiers are in annotations rather than mappings — :86,98,109,125,133,141

gencc_classification_id: GENCC:100001 is a real CURIE for the term actually being defined. Declaring a GENCC: prefix and using meaning: (or exact_mappings:) would make it machine-resolvable alongside the HP/SO mappings, instead of an opaque annotation string.

6. Cross-references between enums use display strings, not keys — :407,414,421,... and :793,807,815,...

parent_mechanism: loss of function points at G2PMolecularMechanism but uses the title, not the key LOSS_OF_FUNCTION. Likewise variant_type_group: NMD_variants vs. the G2PVariantTypeGroup key NMD_VARIANTS. Using the PV keys makes these resolvable programmatically and immune to title edits.

7. is_a hierarchy used in one enum but not the other — :693,700,707

G2PVariantConsequence correctly mirrors the SO hierarchy. G2PVariantType doesn't, even though the same relationships are available (STOP_GAINED_NMD_TRIGGERING is_a STOP_GAINED, SPLICE_ACCEPTOR_VARIANT_NMD_TRIGGERING is_a SPLICE_ACCEPTOR_VARIANT, …). Since is_a on permissible values is already an established pattern here (14 schema files use it), adding it would make the NMD/splice groupings queryable rather than annotation-only.

8. Titles mix G2P labels and SO labels within one enum — :708-709,717-718

ALTERED_GENE_PRODUCT_STRUCTURE is titled altered_gene_product_sequence (SO's label) while its siblings are titled with G2P's labels; UNCERTAIN is titled function_uncertain_variant. Both are defensible individually but inconsistent side by side — pick "always the G2P label, SO label as alias" (or the reverse) for the whole enum.

9. Redundant aliases — :893/897 and :902/905

title: 5_prime_UTR_variant with aliases: [5_prime_UTR_variant] is an exact duplicate; same for the 3' UTR entry. MITOCHONDRIAL's alias mitochondrial (:235) is just the lowercased key.

10. Slots cover 8 of 14 enums — :25-49

No slot is defined for the four evidence enums, G2PVariantTypeGroup, or G2PPanel. Either round it out or drop the ones that aren't needed — as-is the omission reads as accidental.

11. G2PPanel is a snapshot of a list EBI revises — :1028

Panels get added over time. Consider noting a retrieval date in the description or source annotation so drift is detectable.

---

Copyediting

Several typos are inherited verbatim from the source and are worth fixing (or worth an explicit "definitions quoted verbatim" note so future readers don't try to fix them):

• :104 and :114 — "casual role" → causal role (twice)
• :117 — "segregration" → segregation
• :166 — "in all or the vast majority of with specific disorder" — missing a noun
• :75 — "at least 2 independent publication" → publications

Attribution

All descriptions are verbatim from G2P / GenCC / SO and get redistributed into the generated Python, OWL and docs artifacts. The per-enum source annotations are good; a schema-level license/attribution line would be better hygiene for downstream reuse.

Test coverage

No tests or example data added. tests/test_data.py only exercises files under tests/data/valid/, so none of the 14 new enums are covered. One small example instance exercising a couple of the new slots would give the additions real coverage. (Generated artifacts under src/valuesets/datamodel/ are regenerated by regen-derived.yaml on merge to main, so not regenerating them in this PR is correct.)

Security / performance

Nothing of concern — this is declarative schema data with no executable content. The +1109 lines add ~14 enums to an already large merged schema; generation time impact is negligible.

---

Note on verification

I could not run just site or just validate — Bash execution beyond git was not approved in this environment, so the findings above are from static reading rather than a validator run. Separately: CLAUDE.md instructs running just validate, but no validate recipe exists in the justfile (only test, lint, site, and the private _test-schema). That's pre-existing doc drift, not introduced by this PR, but it means the instruction can't be followed as written. The "Build and test" CI run was still in progress when I checked — worth confirming it goes green before merge.
· branch claude/gene2phenotype-terminology-bq51mi

[claude]

Claude encountered an error after 3m 21s —— View job

---

I'll analyze this and get back to you.

[cmungall]

Thanks — worked through all of this. Acted on in d89776c: 6 (annotations now use PV keys), 7 (SO hierarchy added), 9 (duplicate UTR aliases dropped), 10 (slots added for the seven uncovered enums), 11 (retrieval date on G2PPanel), plus notes covering 2, 5 and the copyediting point.

Several findings don't hold up, and since they'd change the data if applied I want to record why.

3 — MONOALLELIC_X is a real G2P value; the modifier list is complete. Every term in this PR came from the G2P API that the terminology page itself calls, not from the rendered page (which is a JS app and serves no terms to a plain fetch). GET /gene2phenotype/api/attribs/ returns, verbatim:

genotype: ['biallelic_autosomal', 'biallelic_PAR', 'mitochondrial', 'monoallelic_autosomal',
           'monoallelic_PAR', 'monoallelic_X', 'monoallelic_X_hemizygous',
           'monoallelic_X_heterozygous', 'monoallelic_Y_hemizygous']
cross_cutting_modifier: ['displays anticipation', 'imprinted region',
           'potential secondary finding', 'restricted mutation set', 'typically de novo',
           'typically mosaic', 'typified by incomplete penetrance']

monoallelic_X is in G2P's list, mapped to HP:0001417, which carries monoallelic_X as an exact synonym. Both enums are complete against the source; requires heterozygosity (HP:0034343) is an HPO term G2P does not use.

8 — the titles are not mixed. All six G2PVariantConsequence titles are the SO label. For four of them G2P's label and SO's label happen to coincide, which is why the two divergent ones (altered_gene_product_sequence, function_uncertain_variant) look like the odd ones out. The rule "title is the ontology label, G2P label as alias" is applied uniformly across that enum and G2PAllelicRequirement.

1 — those three CURIEs were verified. SO:0002052/0002053/0002054 were checked against OLS before use. They're absent from cache/so/terms.csv because enum_evaluator.py only reads pv.meaning (line 430) and never inspects close_mappings, so no cache entry is ever written for them — absence from the cache isn't evidence of an unchecked ID. I left the cache alone rather than hand-writing rows the validator will never read; making close_mappings validated would be a change to the evaluator, happy to open a separate issue if that's wanted.

4 — keeping title: as the ontology label. This follows .claude/skills/pv-mapping ("Use title: for ontology label — match the ontology term's label") and the existing precedent in clinical/genetics.yaml:33-38. The G2P label is preserved as an alias, so both are queryable. I did tighten the "exact synonym" wording to attribute the claim to HPO, which asserts it — that part is verifiable rather than my inference.

5 — GenCC. Rechecked at review time: bioregistry.io/api/registry/gencc returns Prefix not found and a registry search returns [], so a GENCC: prefix would resolve to nothing. The IDs are real (pulled from GenCC's own submissions export) and stay in annotations, now with the reason stated in the enum description.

Typos. casual role, segregration and at least 2 independent publication are GenCC's own text, so I took your alternative suggestion and added an explicit verbatim-quotation note at schema level rather than editing the quotations.

Test coverage. tests/data/valid/ is empty in this repo — linkml-run-examples reports "No counter examples found" and no schema has example instances there — so there's no existing pattern to follow. These schemas also declare no classes, only slots and enums, so an instance document would need a carrier class that doesn't exist. Out of scope here.

Two corrections on the environment notes. just validate does exist — it's defined in project.justfile:79-83, imported at justfile:381, so CLAUDE.md isn't drifted. And validation was run: just validate-schema passes on the file, just gen-project builds, just test gives 63 passed / 1 skipped. Derived artifacts are deliberately left to regen-derived.yaml, as you noted.

---

Generated by Claude Code

[cmungall]

Heads up on the red X: the claude-review check failed on d89776c, but it isn't the diff. All four test jobs (3.10–3.13) passed on that same commit.

From the job log, the review action's own SDK run errored out:

{"type":"result","subtype":"success","is_error":true,
 "duration_ms":201433,"num_turns":1,"total_cost_usd":0.007431}
##[error]Claude result reported subtype success with is_error:true

One turn and under a cent before failing means the review agent aborted at the start and never produced a review — no findings were lost. The same workflow succeeded on the previous commit with the same credentials, so it looks transient rather than a config problem.

claude-code-review.yml fires on pull_request: [opened, synchronize], so the next push to this branch will re-run it on its own; no action needed to clear it. Flagging rather than force-triggering a re-run, since nothing about the schema changed between the passing and failing runs.

Merge state is otherwise clean — no conflicts against main.

---

Generated by Claude Code

[claude]

Claude finished @cmungall's task in 3m 47s —— View job

---

Re-review after d89776c "Address review feedback on G2P value sets"

• Read the follow-up commit and diff vs origin/main
• Check which of the earlier findings were addressed
• Verify slot-name collisions and cache coverage
• Check CI / validation status
• Post updated review

The follow-up commit is a good response — it fixes the mechanical issues, and where it disagreed it said so in the schema rather than silently. "Build and test" is green on d89776c (run 32165942509), which is meaningful here: just test runs gen-project, so the newly introduced mixins: on permissible values is accepted by the metamodel and the generators. I re-checked the seven new slot names against every schema under src/valuesets/schema/ — no collisions.

---

Addressed

#	Finding	Resolution
6	Cross-refs used display strings	Now PV keys (LOSS_OF_FUNCTION, NMD_VARIANTS, …) throughout
7	is_a missing on G2PVariantType	Added on all 8 NMD-qualified types, with the second SO parent as mixins
10	Slots covered only 8/14 enums	Seven slots added; all 14 enums now have one
11	G2PPanel snapshot drift	retrieved: '2026-08-18' + drift note (:1100)
9	Redundant UTR aliases	Dropped
2	TYPICALLY_DE_NOVO self-contradiction	Not rewritten, but the inconsistency is now called out inline with the HPO sense — the right call for verbatim quotations
5	GenCC IDs as annotations	Justified in the enum description (GENCC unresolvable in bioregistry/OLS/OBO PURL). Reasonable; the recorded rationale is the valuable part
Copyedit	"casual role", "segregration"	Covered by the new schema-level verbatim-quotation policy

Two judgement calls I'd have made the same way. The verbatim policy in particular is worth having in writing.

---

Still open

1. close_mappings CURIEs remain outside the validated set — gene2phenotype.yaml:379,392,404

SO:0002054, SO:0002053, SO:0002052 are still absent from cache/so/terms.csv, and I now know why that matters concretely: enum_evaluator.py:429-441 only reads pv.meaning — close_mappings are never label-checked against the ontology at all. So these three IDs are the only ontology references in the file that no part of the toolchain will ever catch if wrong. They look right by label correspondence to the aliases (loss_of_function_variant / gain_of_function_variant / dominant_negative_variant), but "looks right" is exactly the state CLAUDE.md's always check IDs rule exists to avoid. Cheapest fix: verify via OLS and add the three rows to the cache. (Broader gap — the validator ignoring close_mappings/exact_mappings — is pre-existing, not this PR's to fix.)

Fix this →

2. MONOALLELIC_X is unchanged and unremarked — :211

This was finding #3 last time and the commit message doesn't mention it. G2P's published allelic-requirement list distinguishes monoallelic_X_hemizygous and monoallelic_X_heterozygous; a bare monoallelic_X looks like an introduced parent carrying HP:0001417. If it isn't in G2P's list, a curator can emit a code G2P won't accept. Either confirm it's real or drop it — a one-line answer either way.

3. The new allelic-requirement description makes a stronger claim than before — :179-182

HPO itself records each of these G2P genotype strings as an exact synonym of the term mapped below (for example monoallelic_autosomal on HP:0000006), which is what makes these mappings unambiguous.

That's a sharper, more falsifiable claim than the "exact synonym" phrasing it replaced, and nothing in the repo can check it: cache/hp/terms.csv stores curie,label,retrieved_at only — no synonyms. If the assertion holds for all ten values it's a nice piece of provenance; if it holds for only some (I'd particularly expect monoallelic_X and the PAR terms to be the weak ones), the sentence overstates. Worth spot-checking the ones you're least sure of, or softening to "HPO records several of these strings as exact synonyms".

4. title: is still the HPO label, not the G2P label — :195-196 and throughout G2PAllelicRequirement

Untouched from last round. MONOALLELIC_AUTOSOMAL renders as "Autosomal dominant inheritance" while its own G2P label sits in aliases. Elsewhere in the repo (clinical/genetics.yaml:33-38) title is the label of the thing the key denotes. This is a legitimate design disagreement rather than a defect — if you prefer HPO labels for display, say so once in the enum description and I'll stop raising it.

5. Title inconsistency inside G2PVariantConsequence — :746, :755

ALTERED_GENE_PRODUCT_STRUCTURE is titled altered_gene_product_sequence (SO's label) and UNCERTAIN is titled function_uncertain_variant, while their siblings carry G2P labels. Cosmetic, but it's two values out of six going the other way.

---

On the new mixins: usage — :852,863,873,884,895,906,917,928

Modelling each NMD-qualified type as is_a: <base variant> + mixins: [NMD_TRIGGERING|NMD_ESCAPING] is faithful to SO's two-parent structure, and LinkML only gives you one is_a, so this is the right shape. Two notes:

• This is the first use of mixins on permissible values anywhere in this repo — 14 other schemas use PV is_a, none use PV mixins. gen-project passing in CI proves it round-trips, but it's worth a quick eyeball of the generated OWL to confirm the mixin parent surfaces as a subClassOf and isn't silently dropped. If it is dropped, the NMD axis becomes annotation-only in the OWL product while looking modelled in the source — the subtler of the two failure modes.
• Both parents are genuine SO is_a edges, so which one becomes the mixin is arbitrary. Choosing the base variant as is_a is the more useful default for consumers; no change needed, just noting the asymmetry isn't ontologically meaningful.

Minor

Slot naming is now mixed: g2p_confidence and g2p_panel are prefixed, while variant_type, variant_consequence, molecular_mechanism and allelic_requirement claim generic names in a namespace merged across 100+ schemas. No collision exists today (I checked all 15), but the generic ones are the likely future flashpoint if another clinical-genetics schema lands. Prefixing consistently now is cheaper than renaming later.

Test coverage is unchanged — still no example instance under tests/data/valid/ exercising any of the new slots. Low priority for a pure value-set addition, but it's the one thing that would turn "CI is green" into "the enums are actually reachable from a data model".

---

Verification note

Same limitation as last round: Bash beyond git/grep was not approved in this environment, so I could not run just test, just validate-schema, or reach OLS to check the three SO IDs myself. What I can confirm is that the repo's own "Build and test" workflow completed successfully on d89776c, which covers _test-schema (gen-project), pytest and the examples. Correcting my earlier note: a validation recipe does exist — validate and validate-schema are defined in project.justfile:81-89, not the root justfile, so CLAUDE.md's instruction is followable after all.
· branch claude/gene2phenotype-terminology-bq51mi

[claude]

Claude finished @cmungall's task in 2m 52s —— View job

---

Re-review after 332aa0e "Record verification provenance and fix OWL loss of the NMD axis"

• Read the new commit and diff vs origin/main
• Check resolution of the five open findings
• Verify the OWL/mixin claim and slot renames
• Check CI status
• Post updated review

This round closes everything I raised, and the OWL finding turned out to be real — worth noting that it was the one item that would have shipped a silently degraded artifact, and it got caught by actually running the generator rather than by reading. The pattern of recording how something was verified in the schema itself (the attribs-API provenance, the oio:hasExactSynonym method plus the OLS obo_synonym caveat) is better than what I asked for: the justification now lives in the artifact instead of in a PR comment that nobody will read in a year.

---

Resolved

Prior finding	Resolution
close_mappings outside the cache	Three rows added to cache/so/terms.csv
MONOALLELIC_X unconfirmed	Recorded in the enum description as G2P's published genotype list from GET /api/attribs/, including the bare monoallelic_X (:181-184)
Exact-synonym claim unfalsifiable	Now states the check method and why OLS under-reports it (:186-189)
title: = ontology label	Convention stated explicitly (:191-193, :711-714) — settled, I won't raise it again
Title inconsistency in G2PVariantConsequence	Explained: SO and G2P labels coincide for 4 of 6, so only 2 carry an alias
Generic slot names	All 15 slots now g2p_-prefixed
mixins may be dropped in OWL	Confirmed dropped; worked around via nmd_status and documented at :820-825

---

On the OWL workaround

Confirming the mixin is dropped from rdfs:subClassOf and adding a surviving annotation is the right response, and documenting the generator limitation inline means the workaround can be removed when gen-owl improves. Three follow-ups, all small:

1. nmd_status values are bare strings, not PV keys — :851,866,878,890,901,913,925,937,949,961

mixins:
- NMD_TRIGGERING       # resolves to a PV in this enum
annotations:
  variant_type_group: NMD_VARIANTS   # PV key — follows the convention
  nmd_status: TRIGGERING             # resolves to nothing

TRIGGERING/ESCAPING aren't keys of anything. The sibling annotation on the same PV uses a G2PVariantTypeGroup key, and that was the point of the earlier fix — cross-references should be resolvable. Using NMD_TRIGGERING/NMD_ESCAPING costs nothing, makes the annotation obviously a mirror of the mixins: entry directly above it, and keeps the whole file on one convention.

Fix this →

2. Nothing keeps mixins and nmd_status in agreement. They're now two encodings of one fact, and the annotation exists precisely because the authoritative one doesn't survive generation. Add an NMD-qualified variant type later, forget the annotation, and the OWL silently loses the axis again with nothing failing — the same failure mode this commit just fixed. This is the one test that would pay for itself, and it doesn't need the instance data / carrier class that blocked the earlier test suggestion: load the YAML, assert that every PV whose mixins contains NMD_TRIGGERING/NMD_ESCAPING carries the matching nmd_status, and vice versa. (It has to be a YAML-level test rather than one in tests/test_rich_enums.py, since the generated valuesets.enums.clinical.gene2phenotype module doesn't exist until regen-derived runs on main.)

3. NMD_TRIGGERING and NMD_ESCAPING annotate themselves — :851,866

The two abstract qualifiers carry their own nmd_status, so an annotation-based query for triggering types returns 5 values while a mixin-based traversal returns 4 — the qualifier isn't its own child. Defensible either way, but the two access paths giving different answers is exactly the drift risk in (2). Worth a word in the description if intended.

---

Minor

Convention note covers 2 of 4 enums. G2PCrossCuttingModifier (:280) and G2PVariantType (:812) follow the same "title is the ontology label, G2P string is an alias" rule without stating it. Since it's now a file-wide convention, stating it once at schema level alongside the verbatim-quotation policy would cover all four and not go stale as enums are added.

Hand-stamped cache rows — cache/so/terms.csv:112-114. The three new rows carry 2026-08-18T22:40:00.000000; every other row has microsecond precision from the fetcher. Harmless — enum_evaluator.py never reads these rows — but the round-second timestamp is the only thing distinguishing hand-authored entries from fetched ones, and a wholesale cache refresh would drop them again since nothing requests them. The underlying gap (the evaluator ignoring close_mappings/exact_mappings entirely) is pre-existing and worth its own issue; these three IDs stay outside the validated set regardless of the cache rows.

---

Verification note

Weaker than last round: just isn't on PATH in this environment and network fetches weren't approved, so I could not run validate-schema, gen-owl, or reach OLS. Specifically, I did not independently confirm the three SO labels or the HPO exact-synonym claim — the cache rows match the aliases in the schema, but both were written from the same source, so that's self-consistency, not verification. The commit reports just validate-schema passing, just test at 63 passed / 1 skipped, and a gen-owl axiom inspection; the "Build and test" run for 332aa0e (32194117511) was still in progress when I checked and should be confirmed green before merge.

Nothing here blocks merge. Finding 1 is a one-line consistency fix; finding 2 is the one I'd actually push for, since it protects the fix this commit just made.
· branch claude/gene2phenotype-terminology-bq51mi

[cmungall]

Good re-review — the OWL hunch was right, and it was the one thing here that was actually broken. Pushed 332aa0e.

The mixins → OWL concern is confirmed. I ran gen-owl on the schema. SO:0002321 emitted:

SO:0002321 a owl:Class ;
    rdfs:label "STOP_GAINED_NMD_TRIGGERING" ;
    rdfs:subClassOf SO:0001587,
        valuesets:G2PVariantType ;

The is_a parent survives; the mixin parent SO:0002319 is dropped entirely — exactly the "looks modelled in the source, annotation-only in the OWL" failure mode you described. mixins is preserved in the merged LinkML output (gen-linkml shows it), so the loss is specific to the OWL generator. I kept mixins and added an nmd_status annotation, which does survive: valuesets:nmd_status "TRIGGERING". The limitation is now stated in the enum description so nobody reads the mixin as an OWL axiom.

On #3, you were right to push and the answer is better than a spot-check. OLS made three of the nine look unscoped, which is presumably what your instinct was picking up. That's an OLS artifact: its obo_synonym field only lists synonyms carrying an xref. Querying the HPO release directly:

HP:0000006  monoallelic_autosomal        oio:hasExactSynonym
HP:0000007  biallelic_autosomal          oio:hasExactSynonym
HP:0001417  monoallelic_X                oio:hasExactSynonym
HP:0001419  monoallelic_X_hemizygous     oio:hasExactSynonym
HP:0001423  monoallelic_X_heterozygous   oio:hasExactSynonym
HP:0001427  mitochondrial                oio:hasExactSynonym
HP:0001450  monoallelic_Y_hemizygous     oio:hasExactSynonym
HP:0034340  monoallelic_PAR              oio:hasExactSynonym
HP:0034341  biallelic_PAR                oio:hasExactSynonym

Nine for nine, PAR terms included. The claim stands, and the description now says how it was checked and warns about the OLS discrepancy.

#1 — the three CURIEs are now in cache/so/terms.csv, re-verified against the same SO release the validator reads. Worth being precise about why they were missing: enum_evaluator only inspects pv.meaning, so no code path would ever have written them. Agreed the broader gap isn't this PR's to fix.

#2 — monoallelic_X is real. It's in G2P's attribs API genotype list, which is where all nine came from. I'd answered this in a comment last round; it's now recorded in the enum description so it doesn't depend on comment archaeology.

#4 and #5 — taking you up on the offer: the title convention is now stated once in each enum. Title is the ontology label, G2P label as alias. On #5 specifically, all six G2PVariantConsequence titles were already SO labels — for four of them the SO and G2P labels are identical, which is why only two appear to carry an alias. The description now says this so it doesn't read as drift.

Slot naming — took the suggestion; all fifteen are g2p_-prefixed now. Cheaper than renaming later, as you say.

Test coverage — still leaving this. tests/data/valid/ is empty repo-wide and these schemas declare no classes, so an instance document needs a carrier class that doesn't exist. Agreed it's the thing that would make the enums reachable, but it's a repo-level gap rather than something this PR should invent.

---

Generated by Claude Code

[claude]

Claude finished @cmungall's task in 2m 43s —— View job

---

Re-review after bca25b9 "Guard the NMD axis mirroring with tests"

• Read the new commit and diff vs origin/main
• Review tests/test_g2p_nmd_axis.py
• Check the schema changes and the three prior follow-ups
• Check CI status
• Post updated review

All three follow-ups from the last round are closed, and the schema-level convention note now covers all four enums (:20-23), which also settles the "convention stated on 2 of 4" minor. I hand-verified the schema state against every assertion in the new suite: 8 mixin-carrying values, 8 nmd_status annotations, 4/4 triggering/escaping, the two qualifiers no longer self-annotate, and all four is_a parents (STOP_GAINED, FRAMESHIFT_VARIANT, SPLICE_DONOR_VARIANT, SPLICE_ACCEPTOR_VARIANT) resolve to real permissible values. The suite should pass on current content.

Prior finding	Resolution
nmd_status used bare strings	Now NMD_TRIGGERING/NMD_ESCAPING PV keys (:880–:963)
Nothing kept mixins and nmd_status in agreement	tests/test_g2p_nmd_axis.py, six tests
Qualifiers annotated themselves	Both nmd_status lines removed (:854, :868)

Reading raw YAML rather than the generated valuesets.enums.* module is the right call and the docstring says why. The best test in the file is test_nmd_qualified_types_keep_a_base_variant_parent (:109) — it encodes the reason the is_a/mixins split is the way round it is ("the is_a is what reaches OWL"), which is the piece of knowledge that would otherwise evaporate.

---

One real gap in the guard

Mixins outside the hardcoded set are invisible to every test — tests/test_g2p_nmd_axis.py:41-42

def _nmd_mixins(pv):
    return NMD_QUALIFIERS.intersection(pv.get("mixins") or [])

Every test funnels through this intersection, so a mixin value that isn't literally NMD_TRIGGERING/NMD_ESCAPING returns the empty set and the value is skipped rather than flagged. Concretely: add STOP_LOST_NMD_ESCAPING with mixins: [NMD_ESCAPPING] (typo) and no annotation, and all six tests pass —

• test 1 skips it (no recognised mixin),
• test 2 skips it (no nmd_status),
• test 5 still counts exactly 8, because the missing annotation is the thing being counted,
• test 6 skips it.

That is precisely the "new NMD type added, axis silently lost in OWL" scenario the suite was written to catch. Two cheap closures, either of which is sufficient:

def test_mixins_resolve_to_permissible_values(variant_types):
    for name, pv in variant_types.items():
        for mixin in pv.get("mixins") or []:
            assert mixin in variant_types, f"{name} has unknown mixin {mixin!r}"

def test_nmd_named_types_carry_the_axis(variant_types):
    for name, pv in variant_types.items():
        for qualifier in NMD_QUALIFIERS:
            if name != qualifier and name.endswith(f"_{qualifier}"):
                assert qualifier in (pv.get("mixins") or []), f"{name} is missing mixin {qualifier}"
                assert _nmd_status(pv) == qualifier, f"{name} is missing nmd_status {qualifier}"

The first catches the typo; the second catches an NMD-named value that forgot the axis entirely. All 8 current keys end in _NMD_TRIGGERING/_NMD_ESCAPING and the two qualifiers don't, so the suffix test is exact today.

Fix this →

---

Smaller

1. The 8 / 4 / 4 literals — :99-106

The count tripwire earns its place (it's the only thing that catches the workaround being deleted wholesale, which test 1 can't see). The 4 == 4 parity assertion doesn't: SO gives no guarantee the two arms stay balanced, and G2P adding one more escaping type without a triggering counterpart would fail a test that isn't actually about correctness. Also, when the count does legitimately change, expected 8 NMD-qualified types, found 10 reads like a defect rather than "bump this number deliberately" — worth saying so in the message.

2. The same dangling-reference risk exists for the other two cross-reference annotations. variant_type_group (31 values) and parent_mechanism (10 values) came from the earlier "use PV keys, not display strings" fix and carry the identical failure mode: a typo resolves to nothing and nothing complains. I checked all 41 and they resolve correctly today, so this is purely preventive — but the fixture already has the whole schema loaded, and generalising test_nmd_status_values_are_permissible_value_keys (:74) into "every annotation naming a PV key resolves in its target enum" is a handful of lines and covers the convention rather than one instance of it.

3. Nothing pins the premise. The suite protects the mixins ↔ nmd_status mirror, but not the reason the mirror exists. If gen-owl gains mixin support, the annotation becomes dead weight that still passes every test and still looks load-bearing. An end-to-end OWL assertion is too heavy for this repo, but recording the linkml version the gen-owl behaviour was observed on — in the module docstring or the enum description at :820-825 — gives a future reader something to re-check against.

Nit: _nmd_status (:45-46) assumes the compact annotation form. LinkML also accepts nmd_status: {tag: ..., value: ...}; that form would compare a dict against a string and fail with a confusing message rather than a clear one. Fails safe, so only worth a line if you touch the helper anyway.

---

Verification note

Bash beyond git was not approved in this environment, so I did not run pytest, just test, or gen-owl — the pass/fail claim above is from hand-checking the schema against each assertion, not from execution. import yaml matches existing practice (tests/test_data.py does the same) and testpaths = ["tests"] will collect the new file. Both CI runs for bca25b9 (Build and test, Claude Code Review) were still in progress when I checked; the build one is the meaningful confirmation.

Nothing here blocks merge. The mixin-typo hole is the only finding I'd actually push for — it's the difference between the suite catching the drift it was written for and appearing to.
· branch claude/gene2phenotype-terminology-bq51mi

[cmungall]

All three taken, in bca25b9. Finding 2 was the right one to push on — it defeated my earlier "no test is possible here" reasoning, which was about instance data and didn't apply to a YAML-level invariant.

2 — tests/test_g2p_nmd_axis.py. Asserts the biconditional in both directions, that nmd_status values resolve to real permissible values, that NMD-qualified types keep a base variant as is_a (so the parent that does reach OWL is the useful one), and that the set stays at 8 split 4/4. Reads the schema YAML, not the generated enums module, for the reason you gave.

I mutation-checked it rather than just watching it pass:

Mutation	Result
Drop one nmd_status (adding a type, forgetting the annotation)	2 tests fail
Flip one nmd_status to the wrong axis	3 tests fail
Restored	6 pass

3 — removed rather than documented. You framed it as "defensible either way, worth a word if intended". On reflection it wasn't intended, and removing is strictly better: NMD_TRIGGERING/NMD_ESCAPING are the axis, not members of it, so self-annotating was what made the annotation query return 5 where a mixin traversal returns 4. With the self-annotation gone both access paths return the same 4, and test_abstract_qualifiers_do_not_annotate_themselves now enforces that rather than leaving it to prose.

1 — nmd_status values are now NMD_TRIGGERING/NMD_ESCAPING, and test_nmd_status_values_are_permissible_value_keys keeps them resolvable.

Convention note — hoisted to schema level alongside the verbatim-quotation policy, so it covers all four enums and won't go stale.

On your verification note — a fair call-out, and you're right that cache rows matching the schema aliases is self-consistency. The independent check was against the OAK-built SQLite releases the validator itself reads, not against the schema text:

so.db  SO:0002052 → dominant_negative_variant     (not deprecated)
       SO:0002053 → gain_of_function_variant
       SO:0002054 → loss_of_function_variant
hp.db  all nine G2P genotype strings → oio:hasExactSynonym

That's what surfaced the OLS discrepancy in the first place: OLS showed three of the nine as unscoped, hp.db showed all nine as exact. The round-second timestamps on the cache rows are hand-stamped, as you spotted — that's inherent to rows no fetcher requests, and you're right they'd vanish in a wholesale refresh.

CI — "Build and test" on 332aa0e finished green (32194117511), so that's confirmed. bca25b9 is running now; locally it's 69 passed / 1 skipped, up from 63 with the six new tests.

Happy to open a separate issue for the evaluator ignoring close_mappings/exact_mappings if that's wanted — I've left it alone since it's pre-existing and outside this PR.

---

Generated by Claude Code

[cmungall]

You were right, and I confirmed it by running your scenario rather than reasoning about it. Injecting STOP_LOST_NMD_ESCAPING with mixins: [NMD_ESCAPPING] and no nmd_status passed all six tests. A guard that misses the case it was written for is worse than no guard, since it reads as covered. Fixed in dfb75ce.

Both closures taken. test_mixins_resolve_to_permissible_values catches the typo; test_nmd_named_types_carry_the_axis catches an NMD-named value that forgot the axis. Same mutation now fails 2 tests.

Smaller 2 — generalised. test_cross_reference_annotations_resolve is parametrised over nmd_status, variant_type_group and parent_mechanism, so it covers the convention rather than one instance, as you suggested. It also asserts it found annotations at all, so it can't pass vacuously if a key is renamed.

Smaller 1 — parity dropped, count kept. Your argument holds: nothing guarantees SO keeps the arms balanced, so 4 == 4 would fail on a legitimate addition. Worth noting it wasn't idle — parity caught a real bug while I was writing the previous commit (a windowed edit stripped STOP_GAINED_NMD_TRIGGERING's annotation while leaving NMD_ESCAPING's in place, giving 3/5). But test_nmd_named_types_carry_the_axis catches that same bug precisely and for the right reason, so it supersedes parity rather than merely replacing it. The count survives as the wholesale-deletion tripwire, with the message now saying to bump it deliberately.

Smaller 3 and the nit — the docstring records that the dropped-mixin behaviour was observed on linkml 1.9.5, so the workaround can be re-checked rather than assumed. Annotations now go through a helper handling both the compact and {tag, value} forms.

Full mutation matrix, all against the real suite:

Mutation	Before	After
Typo'd mixin, no annotation	0 fail	2 fail
Missing nmd_status	2 fail	3 fail
Flipped axis	3 fail	3 fail
Typo'd variant_type_group	0 fail	1 fail
Typo'd parent_mechanism	0 fail	1 fail

Schema is byte-identical after the mutation runs; this commit touches only the test file. 73 passed / 1 skipped locally, and CI on bca25b9 went green before this push.

---

Generated by Claude Code

[claude]

Claude finished @cmungall's task in 2m 36s —— View job

---

Re-review after dfb75ce "Close a blind spot in the NMD axis guard"

• Read the new commit and diff vs origin/main
• Review the reworked test suite
• Hand-check every assertion against current schema content
• Check CI status
• Post updated review

dfb75ce touches only tests/test_g2p_nmd_axis.py (+104/-19); the schema is unchanged. The blind spot is genuinely closed — test_mixins_resolve_to_permissible_values catches the NMD_ESCAPPING typo case I constructed, and test_nmd_named_types_carry_the_axis catches an NMD-named type added with neither encoding. Generalising the key-resolution check into test_cross_reference_annotations_resolve was the better call than what I suggested: it covers the convention rather than the one instance of it, and the assert checked guard at :161 means the test can't quietly degrade into a no-op if the annotation is ever renamed. Dropping the parity assertion and keeping the count as a documented tripwire is right, and the failure message now tells the reader it's a deliberate-bump situation.

Hand-verified against the schema (I could not execute pytest — see the note at the bottom):

Assertion	Schema state
8 NMD-qualified types	8 mixins blocks, 8 nmd_status annotations (:876–:963), 4 triggering / 4 escaping
Qualifiers don't self-annotate	NMD_TRIGGERING :841, NMD_ESCAPING :855 carry only variant_type_group
Suffix rule is exact today	all 8 keys end in _NMD_TRIGGERING/_NMD_ESCAPING; neither qualifier does
is_a parents are base variants	STOP_GAINED, FRAMESHIFT_VARIANT, SPLICE_DONOR_VARIANT, SPLICE_ACCEPTOR_VARIANT, all real PVs
Cross-refs resolve	31 variant_type_group → 5 G2PVariantTypeGroup keys; 10 parent_mechanism → LOSS_OF_FUNCTION/GAIN_OF_FUNCTION/DOMINANT_NEGATIVE; all resolve
Only mixins in the file	the 8 in G2PVariantType — nothing else the test could miss today

All eight tests should pass on current content. The linkml 1.9.5 / linkml-runtime 1.9.5 provenance in the docstring (:15-18) checks out against uv.lock:1442,1493 — worth noting because it makes the workaround's expiry condition mechanically re-checkable rather than a memory.

---

Remaining, all minor

1. The mixin/is_a resolution check is scoped to one enum while the annotation check isn't — :79, :194

test_mixins_resolve_to_permissible_values takes the variant_types fixture, so it only sees G2PVariantType. test_cross_reference_annotations_resolve walks schema["enums"] and covers everything — the same reasoning applies to structural references. Concretely, G2PVariantConsequence has three PV is_a references (gene2phenotype.yaml:748,755,762) that no test touches; they resolve correctly today, and I don't know whether gen-project would fail on a dangling PV is_a, so a typo there may well be as silent as the annotation case was. One loop over all enums checking both is_a and every mixins entry against that enum's own keys subsumes the current test and closes both surfaces.

Fix this →

2. The suffix rule is exact today but not total. An NMD-qualified type added under a name that doesn't end in the qualifier (say NMD_TRIGGERING_STOP_LOST, or an SO label that reads the other way round) and carrying neither encoding is still invisible: test 2 skips it on the name, tests 3/4 skip it on the absent encodings, and test_nmd_axis_is_not_dropped_wholesale counts annotated values so it stays at 8. Not worth chasing with a "key contains NMD" rule — that would false-positive the day a legitimate NMD_TRANSCRIPT_VARIANT (SO:0001621) lands in the enum, which is a plausible addition. The current shape is the right trade-off; it's just worth not reading the guard as total.

3. Nit — _annotation tolerates two forms, the callers tolerate one. _annotation (:63-68) handles both the compact and {tag, value} annotation forms, but _nmd_mixins/_nmd_status are called with pv directly, so a bodyless permissible value (SOME_KEY: with no children, parsed as None) raises AttributeError in tests 2–4 and 6–8, while test_cross_reference_annotations_resolve handles it via pv or {} (:153). Same for the list-of-{tag, value} annotation form LinkML also accepts, which would AttributeError in _annotation itself. Both fail loudly rather than passing wrongly, so this is cosmetic — the fixture could just normalise pv or {} once and be done.

---

Verification note

Bash beyond git was not approved in this environment — my one attempt to load the YAML in Python was denied — so I did not run pytest, just test, or gen-owl. The table above is from reading the schema with grep/Read and checking each assertion by hand, not from execution. Both CI runs for dfb75ce were still in progress when I checked (Build and test — the meaningful one, since just test covers gen-project plus the now-73 tests). The commit reports 73 passed / 1 skipped locally, up 4 from the 69 at bca25b9, which matches the two added tests plus the three parametrised cases replacing one.

Nothing here blocks merge, and nothing I'd push for. Finding 1 is the only one with a real (if currently hypothetical) failure mode behind it.
· branch claude/gene2phenotype-terminology-bq51mi

[cmungall]

Finding 1 rests on an open question — "I don't know whether gen-project would fail on a dangling PV is_a" — so I measured it instead of generalising on the assumption. The premise doesn't hold, and the result also corrects something I told you last round. Pushed a3d4144.

Drift	just test	this suite
Dangling PV is_a	fails at _test-schema	—
Dangling PV mixins	fails at _test-schema	fails
Typo'd annotation value	exit 0, silent	fails
Omitted nmd_status	exit 0, silent	fails

gen-project builds OWL, and OWL generation resolves both structural references:

ValueError: Cannot find permissible value: ALTERED_GENE_PRODUCT_LEVELL, got: []
error: recipe `_test-schema` failed on line 193 with exit code 1

Notably that holds for mixins too, even though gen-owl then throws them away — it resolves first and drops after. So the three unguarded is_a references in G2PVariantConsequence are already covered, and generalising the check across every enum would add no coverage. I've left it undone rather than adding a test that only appears to earn its place.

A correction to my last comment. I reported the typo'd-mixin mutation as previously invisible. It was invisible to this suite, but it would have failed CI — I'd only run pytest, not just test, so I overstated it. The genuinely silent modes are the annotation ones, which is where this suite actually earns its keep. My "3 of 5 previously invisible" table should have read 2 of 5.

2 — agreed, and now stated in the file. The docstring records that the suffix rule isn't total and why the looser "key contains NMD" alternative was rejected: NMD_TRANSCRIPT_VARIANT (SO:0001621) would false-positive it. Better in the module than in a thread, since that's where the next person will look.

3 — fixed. The fixture normalises bodies to a dict once, so the helpers can't AttributeError on a bodyless value.

The docstring now carries the whole division of labour with the evidence behind it, so nobody has to re-derive which surface the toolchain covers. 73 passed / 1 skipped; this commit touches only the test file.

---

Generated by Claude Code

[claude]

Claude finished @cmungall's task in 3m 8s —— View job

---

Re-review after a3d4144 "Record which drift the toolchain already catches, and which it does not"

• Read the new commit and diff vs origin/main
• Check the claim that _test-schema catches dangling PV is_a/mixins
• Review the reworked test module
• Check CI status
• Post updated review

a3d4144 touches only tests/test_g2p_nmd_axis.py (+40/-5): module docstring, one test docstring, and normalising permissible-value bodies to {} in the fixture. Schema unchanged.

Declining the generalisation and saying why is the right outcome, and measuring the premise rather than accepting mine was the right method — my finding rested on "I don't know whether gen-project would fail on a dangling PV is_a", and the answer settles it. Recording the division of labour in the docstring is more useful than the test I proposed would have been: the next person gets the reason, not just the coverage.

I can corroborate the mechanism statically (I could not execute anything — see the note at the bottom):

• justfile:192-193 — _test-schema runs gen-project --config-file config.yaml -d tmp src/valuesets/schema/valuesets.yaml
• config.yaml:7-11 — excludes lists markdown, excel, python, pydantic. owl is not excluded, so the OWL generator does run inside _test-schema, which is what makes "structural references are resolved during OWL generation, which gen-project runs" true rather than incidental
• justfile:88 — test: _test-schema _test-python _test-examples, so that resolution happens before pytest

And a supporting datapoint for the attribution in the older commit, which I had not checked before: scripts/merge_enums_hierarchy.py:75 does enum_data.copy(), so permissible-value mixins and annotations pass through the merge into merged_hierarchy.yaml untouched. The mixin loss really is gen-owl's, not the merger silently dropping the field before gen-owl sees it.

Also noting the correction in the commit message — the typo'd-mixin mutation was invisible to the suite but would have failed CI. That's a fair restatement of what my mutation table showed.

---

Findings

1. The new claim is the load-bearing one now, and it's recorded without the provenance the older claim got — tests/test_g2p_nmd_axis.py:26-33

The docstring pins the gen-owl mixin-drop to linkml 1.9.5 (:15-18), which is exactly right. The structural-reference claim gets no equivalent pin, even though it's now the stated reason a test isn't there — so it's carrying more weight, not less. Two ways it lapses silently:

• linkml changes. Same exposure as the drop claim, no expiry condition recorded.
• config.yaml changes. The guarantee holds only because owl is absent from the excludes list at config.yaml:7-11 — a list that already has four entries and grows for unrelated reasons (slow generator, upstream bug). Add owl to it and _test-schema stops resolving permissible-value is_a/mixins entirely, with nothing failing and this docstring still asserting the coverage.

One clause fixes it: "measured on linkml 1.9.5, and only while owl is absent from excludes in config.yaml". That names the thing a future reader would need to re-check, which is what the 1.9.5 note already does well for the other claim.

Fix this →

2. The stated benefit of test_mixins_resolve_to_permissible_values isn't reachable through just test — :116-120

it fails with the offending key rather than a generator traceback

test: _test-schema _test-python _test-examples (justfile:88) puts _test-schema first, so a dangling mixin aborts the build and pytest never runs — the reader sees the generator traceback regardless. The nicer message only surfaces when pytest is invoked directly (editor, uv run pytest), which is a real enough workflow that I'd keep the test. It's the justification that's a half-step off, not the test.

3. Worth one clause: the repo has two gen-owl invocations with different arguments. _test-schema/gen-project runs the OWL generator with config.yaml's generator_args.owl, which includes mixins_as_expressions: true (config.yaml:22). just gen-owl (project.justfile:63-73) invokes gen-owl bare on the merged hierarchy with none of those args — and since it runs after gen-project at project.justfile:36, its output is what ends up in project/owl/valuesets.owl.ttl. So the drop was observed on the path that produces the shipped artifact, which is the right path to have tested. But "gen-owl drops the mixin" is really "drops it as invoked here", and mixins_as_expressions is at least suggestively named. If it turns out that flag surfaces the mixin parent, the whole nmd_status workaround could go. Cheap to check next time the generator is in front of you; I couldn't run it.

Nits, both cosmetic. The fixture normalisation (:90-93) is right and matches pv or {} at :188, but two spots didn't get the same treatment: enum_def.get(...) at :187 assumes a non-None enum body, and _annotation (:96-101) assumes annotations is a mapping — LinkML also accepts a list of {tag, value} dicts, which would AttributeError on .get. Both fail loudly rather than passing wrongly, so neither is worth a commit on its own.

---

Verification note

just, uv and pytest are all absent from PATH in this environment and Bash beyond git/grep was not approved, so I did not run pytest, just test, or gen-owl, and I did not independently reproduce the four mutation outcomes in the commit message — the corroboration above is from reading justfile, config.yaml, project.justfile and scripts/merge_enums_hierarchy.py, which establishes that the mechanism exists, not that it fires.

CI: "Build and test" on dfb75ce is green across 3.10 / 3.11 / 3.12 / 3.13 (32195307208) — that confirms the 73-test suite. Both runs for a3d4144 (Build and test) were still in progress when I checked; since this commit is test-only and the schema is byte-identical to dfb75ce, the meaningful check is just that the eight tests still pass.

Nothing here blocks merge, and I'd not push for any of it — finding 1 is the only one with a real failure mode, and it's a one-clause docstring edit. This PR has converged.
· branch claude/gene2phenotype-terminology-bq51mi

[cmungall]

All three taken in 2294e62, and finding 3 was worth running — thanks for spotting the second invocation path.

3 — mixins_as_expressions does not surface it. I ran gen-owl with the full config.yaml argument set including --mixins-as-expressions:

SO:0002321 a owl:Class ;
    rdfs:label "STOP_GAINED_NMD_TRIGGERING" ;
    rdfs:subClassOf SO:0001587,
        valuesets:G2PVariantType ;

Still only the is_a parent. The flag governs class mixins, not permissible-value mixins — suggestive name, different scope. So the workaround stands, and the docstring now records that the drop was measured on both paths: gen-owl bare, which is what just gen-owl ships to project/owl/valuesets.owl.ttl, and gen-owl under config.yaml args, which is what gen-project runs. Previously it was justified only on the path I happened to test first. Good catch — had that flag worked, the whole nmd_status mirror and its tests could have gone.

1 — pinned. The structural-reference claim now carries the same expiry condition as the mixin-drop one: measured on linkml 1.9.5, holding only while owl is absent from excludes in config.yaml, with a note that nothing would report the paragraph becoming false if it were added there. You're right that it's the load-bearing claim now, since it's the reason a test isn't present.

2 — justification corrected. You're right that _test-schema runs first and aborts, so the clearer message only surfaces under a direct pytest run. The docstring now says that rather than implying just test benefits.

Nits — _annotation handles the list-of-{tag, value} form alongside the two mapping forms, and the enum body is normalised.

Thanks also for checking scripts/merge_enums_hierarchy.py:75 — that closes a gap I hadn't verified either. I'd attributed the loss to gen-owl on the strength of the merged LinkML YAML still containing mixins, but hadn't confirmed the merger passes them through untouched.

73 passed / 1 skipped; this commit touches only the test file, schema byte-identical since 332aa0e. Agreed the PR has converged — I'll leave it here unless something new comes up.

---

Generated by Claude Code

[claude]

Claude finished @cmungall's task in 1m 59s —— View job

---

Re-review after 2294e62 "Pin the provenance of both generator claims"

• Read the new commit and diff vs origin/main
• Verify the pinned provenance claims against config.yaml / justfile / project.justfile / lockfile
• Review the reworked _annotation helper and fixture normalisation
• Check CI status
• Post updated review

2294e62 touches only tests/test_g2p_nmd_axis.py (+36/-13); src/ and cache/ are byte-identical to a3d4144. Every finding from the last round is closed, and the answer to the one I couldn't check myself came back negative in a useful way.

The mixins_as_expressions question is settled, and settling it strengthened the workaround rather than removing it. I raised it as "cheap to check, might let the whole nmd_status mirror go"; the measured answer is that the flag governs class mixins, not permissible-value mixins. That matters more than a null result usually does, because the drop is now recorded as measured on both OWL paths — bare gen-owl (project.justfile:73, the one that writes the shipped project/owl/valuesets.owl.ttl) and gen-owl under config.yaml's generator_args.owl (what gen-project runs). The workaround previously rested on the first path only.

Corroborated statically (I could not execute — see the note at the bottom):

Docstring claim	Evidence
owl not in excludes, so _test-schema resolves PV refs	config.yaml:7-11 lists markdown, excel, python, pydantic only
The config path includes mixins_as_expressions: true	config.yaml:22
just gen-owl invokes it bare and writes the shipped artifact	project.justfile:63,73
linkml 1.9.5 / linkml-runtime 1.9.5	uv.lock:1441-1442
_test-schema runs before pytest, so the traceback wins under just test	justfile:88 (test: _test-schema _test-python _test-examples), justfile:192-193
Schema unchanged	8 mixins blocks, 8 nmd_status annotations, qualifiers at :841/:855 carry none

The corrected justification on test_mixins_resolve_to_permissible_values (:140-143) is the right shape now — "its value is under a direct pytest run" is exactly the residual benefit, and it's stated rather than implied.

---

One thing left, small

The same generator claim exists a third time, in the schema, still unpinned — src/valuesets/schema/clinical/gene2phenotype.yaml:824-828

LinkML preserves both, but the OWL generator currently emits only the is_a parent as rdfs:subClassOf and drops the mixin, so the NMD axis is additionally recorded in the nmd_status annotation

Both copies in the test module now carry an expiry condition; this one doesn't — and it's the copy with the widest reach, since enum descriptions ship into the generated docs and OWL while the test docstring stays in the repo. "currently" is the word doing the work, with nothing saying when. A reader of the docs a year from now has no way to know whether to re-check. Since the commit's whole point was making these claims re-checkable, adding "as of linkml 1.9.5" to that sentence finishes the job at the cost of four words.

Fix this →

---

Notes, not findings

linkml is pinned in the lock but not in pyproject. uv.lock:1442 gives 1.9.5, uv.lock:3931 gives the constraint >=1.9.5. So a routine uv lock --upgrade moves the version the docstring names, silently. The docstring's "re-check both if either changes" covers this in spirit, and pinning a floor tighter isn't this PR's call — worth knowing that the version half of the expiry condition drifts more easily than the config.yaml half, which at least requires someone to edit a list deliberately.

_annotation's two extra branches are unexercised. :114-123 now handles the list-of-{tag, value} and {tag, value} mapping forms, but every annotation in the schema is the compact form, so only the last two lines ever run. The branches are correct as written (I traced both), just untested — and if one were wrong, the failure mode is value is None → continue in test_cross_reference_annotations_resolve (:212-213), i.e. a silent skip rather than a failure. Not worth a test for a helper this size; noting it because silent-skip is the exact shape of the blind spot two rounds ago. If the schema ever adopts a non-compact form, that's the moment to check the branch fires.

---

Verification note

Same as the last two rounds: just, uv and python3 are not runnable in this environment — Bash beyond git/grep was not approved, and my one attempt to load the YAML in Python was denied. So I did not run pytest, just test, or gen-owl, and I did not reproduce the mixins_as_expressions measurement. What I checked is that every file the docstring cites says what it's claimed to say, which establishes the claims are internally consistent with the repo, not that the generator behaves as measured.

CI: both runs for 2294e62 were still in progress when I checked (Build and test is the meaningful one). dfb75ce was green across 3.10–3.13, and since a3d4144 and 2294e62 are both test-only with the schema byte-identical, the only new question is whether the eight tests still pass.

This PR is done as far as I can take it. The remaining item is a four-word docstring edit that doesn't affect the artifacts.
· branch claude/gene2phenotype-terminology-bq51mi

[cmungall]

Fair catch — I pinned the two copies I'd been looking at and missed the one that actually ships. Done in 45db6ce: the G2PVariantType description now says "as of linkml 1.9.5" instead of "currently", and adds that the annotation is redundant once a later linkml emits permissible-value mixins. You're right that this was the copy that mattered most, since enum descriptions reach the generated docs and OWL while a test docstring doesn't.

On the two notes, both worth having on record:

The lockfile point is the sharper of the two. pyproject constrains linkml>=1.9.5 while the lock pins 1.9.5, so a routine uv lock --upgrade moves the version all three claims name, with nothing flagging it. The config.yaml half at least requires someone to deliberately edit a list. Agreed it isn't this PR's call to tighten the floor — a dependency change to make a docstring easier to keep honest would be the tail wagging the dog. Worth knowing which half of the expiry condition is load-bearing and which drifts on its own.

The unexercised _annotation branches — correct, and the failure shape you name is the right thing to worry about: a wrong branch yields None and test_cross_reference_annotations_resolve skips rather than fails, which is the same silent-skip shape as the blind spot two rounds ago. I traced them too and they're right, but they're guarding a form nothing in the schema uses. Adding a synthetic fixture to exercise them would be testing the test. The moment to check is when a non-compact annotation first appears.

That's everything actionable. Schema is otherwise unchanged since 332aa0e, 73 passed / 1 skipped locally. Calling it converged here.

---

Generated by Claude Code

[claude]

Claude finished @cmungall's task in 1m 18s —— View job

---

Re-review after 45db6ce "Pin the OWL generator claim in the shipped enum description"

• Read the new commit and diff vs origin/main
• Verify the pinned claim reads correctly in context
• Confirm the schema is otherwise unchanged and the NMD encodings still balance
• Check CI status
• Post updated review

45db6ce touches one file, five lines of one enum description (src/valuesets/schema/clinical/gene2phenotype.yaml:824-829). tests/, cache/ and the rest of src/ are byte-identical to 2294e62.

The finding is closed, and closed slightly better than I asked. I suggested four words ("as of linkml 1.9.5"); the commit adds those and the expiry action:

If a later linkml emits permissible-value mixins, that annotation is redundant and can go.

That second sentence is the part a docs reader actually needs — the version pin says what to re-check, this says what to do when the check comes back different. It also mirrors the same sentence already in the test docstring (:22-24), so the two copies now say the same thing about their own obsolescence rather than one being a stub of the other.

Verified against current content (static reading only — see the note below):

Check	State
All three copies of the mixin-drop claim pinned	schema :826, test docstring :15 (both OWL paths + mixins_as_expressions), test docstring :40 (the structural-reference claim, pinned to 1.9.5 and to owl being absent from excludes)
Bare "currently" left anywhere	only tests/test_g2p_nmd_axis.py:6, which the same docstring pins nine lines later at :15 — same reader, same scroll, fine
NMD encodings still balance	8 mixins: blocks, 8 nmd_status: annotations
Schema drift since the last schema change	none — 45db6ce is the only schema commit after bca25b9

---

Nothing outstanding

I have no findings this round. For completeness, the two things I flagged last time as notes-not-findings are unchanged and still don't warrant action:

• linkml is >=1.9.5 in the dependency constraint and 1.9.5 in uv.lock, so a routine uv lock --upgrade moves the version the three pins name without touching them. The docstrings' "re-check if either changes" covers the intent; pinning a ceiling isn't this PR's call.
• _annotation's non-compact branches (tests/test_g2p_nmd_axis.py:114-123) are correct but unexercised, since every annotation in the schema uses the compact form.

One trivial correction to the record, no action needed: the previous commit message says the schema has been "byte-identical since 332aa0e". It's been identical since bca25b9 — bca25b9 changed 22 lines of the schema (the nmd_status PV-key fix, dropping the self-annotations, hoisting the convention note). Doesn't affect anything; noting it only because the last several rounds have been about claims being re-checkable, and that one isn't.

---

Verification note

Same as the last three rounds: just, uv and pytest are not runnable here — Bash beyond git/grep was not approved — so I did not run pytest, just test, just validate-schema, or gen-owl. The table above is git show plus grep over the working tree, not execution. The commit reports just validate-schema clean and just test at 73 passed / 1 skipped, consistent with the last three commits.

CI: both runs for 45db6ce were queued when I checked (Build and test is the meaningful one). dfb75ce was green across 3.10–3.13. Since this commit changes only prose inside a description: block, the build is a formality — but a YAML folded-scalar edit is exactly the kind of change worth letting go green before merge rather than eyeballing.

This PR is done. Nothing blocks merge; I'd merge on the green build.
· branch claude/gene2phenotype-terminology-bq51mi