diff --git a/NEXT_TASKS.md b/NEXT_TASKS.md index 63b6cfaa9..f8c75f477 100644 --- a/NEXT_TASKS.md +++ b/NEXT_TASKS.md @@ -5,7 +5,7 @@ update this file as work is started/finished — move done items out, add new deferrals here. Keep the cross-Mech items in sync with the sibling repos' `NEXT_TASKS.md` (CultureMech / MIM / TraitMech). -Last reconciled: 2026-07-18. +Last reconciled: 2026-07-19. ## 0. Element enum CHEBI groundings are wrong + ungated (found 2026-07-18) @@ -51,31 +51,36 @@ validate-all + 197 tests green. any id↔label gate — `validate-products` only checks record-level `term.{id,label}` pairs, so these drifted wrong undetected while the gated record pairs stayed correct (spot-checked: `Ion_Adsorption_REE_Indigenous_Community.yaml` -uses the *correct* CHEBI:33377/CHEBI:49962). Added -`tests/test_element_enum_groundings.py` — runs in the `validate-strict` pytest -step (already a blocking gate) with no network: (a) freezes the verified `meaning:` -ids for `MetalElementEnum` + `RareEarthElementEnum` so a bad hand-edit fails, (b) -asserts no two element PVs share a CHEBI id (catches the swap/dup pattern -directly), and (c) resolves each id against its canonical ChEBI label — element -name must appear — but only when the ChEBI sqlite is already cached locally, so it -never forces a multi-GB download in CI. Verified the label check would have -flagged all three historical bugs (PALLADIUM→promethium, INDIUM→aluminium -trifluoride, YTTRIUM→zinc dichloride). - -**Still deferred — generalise beyond the element enums.** The new test is scoped -to the two element enums (kept clean and exception-free on purpose). A KB-wide -guard over *every* enum `meaning:` (via `just validate-schema-terms` / -`linkml-term-validator validate-schema`) is still blocked by the same thing that -defers `validate-terms-all`: linkml-term-validator has no exceptions mechanism, so -it fails on obsolete/unminted meanings elsewhere in the schema. Enable it once -those residuals are minted/cleaned or the LinkML tool grows a waiver. See -[[ontology-term-cleanup]] / [[chebi-mislabels-backlog]]. +uses the *correct* CHEBI:33377/CHEBI:49962). Added the guard test (PR #208, then +generalised in PR #209) — runs in the `validate-strict` pytest step (already a +blocking gate) with no network. + +**Item 2b — generalise the guard — DONE (2026-07-19, PR #209).** A survey found +only **3 enums carry `meaning:` groundings** at all — `MetalElementEnum` (17 +CHEBI), `RareEarthElementEnum` (16 CHEBI), and `CultivationSystemEnum` (1 OBI: +BIOREACTOR_UNSPECIFIED → OBI:0001046 "bioreactor"). (The `validate-terms-all` +blocker is about *data-level* `term.id` bindings across community files — a +different surface — so it does **not** block an enum-meaning guard.) Renamed the +test to `tests/test_enum_groundings.py` and made it **auto-discover** every +grounded enum from the schema, so a newly grounded enum/value is covered +automatically (or fails until registered in `EXPECTED`): (a) full discovered +`{enum: {value: meaning}}` must equal the frozen `EXPECTED`; (b) no two values +inside one enum share an id; (c) each id resolves to a non-obsolete term whose +canonical label fits (element name must appear for the element enums), per prefix, +skipped when that ontology's sqlite isn't cached locally. Covers CHEBI + OBI; +verified the label check flags all three historical bugs. + +**Note (not the same task):** a full LinkML-native schema gate over term.id +*data* bindings (`just validate-terms-all` / `linkml-term-validator`) is still +deferred — that tool has no exceptions mechanism and fails on the 34 +curator-accepted residuals. Unblock by minting/cleaning them or teaching the gate +a shared waiver (see §1). See [[ontology-term-cleanup]] / [[chebi-mislabels-backlog]]. **Impact:** shipped community records mostly ground REEs via their own (correct) `term.{id,label}` pairs, so the KGX export from those is largely fine; the wrong groundings live in the schema enum + `metal_extraction.py` map (any enum-driven export/analysis inherits them). Low blast radius today, but a latent correctness -bug and a clear gate gap — now guarded for the element enums. +bug and a clear gate gap — now guarded for every grounded enum (CHEBI + OBI). ## 1. Phase-2 id↔label enforcement rollout (report-only → blocking) diff --git a/tests/test_element_enum_groundings.py b/tests/test_element_enum_groundings.py deleted file mode 100644 index 340abb94d..000000000 --- a/tests/test_element_enum_groundings.py +++ /dev/null @@ -1,126 +0,0 @@ -"""Guard against wrong CHEBI groundings in the element enums. - -`MetalElementEnum` and `RareEarthElementEnum` each map an element name to a -CHEBI id via the permissible value's ``meaning:``. Those ids are **not** covered -by ``validate-products`` (the id↔label gate only checks record-level -``term.{id,label}`` pairs), so they drifted silently: PALLADIUM was grounded to -CHEBI:33373 *promethium* (PR #206) and 13 rare-earth/INDIUM ids were off-by-one -within the CHEBI:333xx lanthanide block or digit-transposed — e.g. INDIUM → -*aluminium trifluoride*, YTTRIUM → *zinc dichloride* (PR #207). - -This module is that missing gate. It runs in the ``validate-strict`` pytest step -(which already blocks merges) and needs no network: - -* ``test_meanings_match_expected`` freezes the verified-correct mapping so any - future hand-edit that changes an id must consciously update ``EXPECTED``. -* ``test_no_shared_meaning`` enforces that no two element PVs share a CHEBI id — - an ontology-free invariant that directly catches the swap/duplication pattern - (THULIUM and DYSPROSIUM both = CHEBI:33377, etc.). -* ``test_meanings_resolve_to_element_label`` additionally checks each id against - its **canonical ChEBI label** — but only when the ChEBI sqlite is already - present locally, so it never forces a multi-GB download in CI. - -The ``EXPECTED`` ids were verified on 2026-07-18 against the local ChEBI build: -rare earths are grounded to the ``(3+)`` cation to match each enum -``description:`` "X(3+) cation"; Dy/Er/Tm have no ``(3+)`` term in ChEBI and are -grounded to the atom (CHEBI:33377 / :33379 / :33380). -""" - -from pathlib import Path - -import pytest -import yaml - -SCHEMA = Path(__file__).parent.parent / "src" / "communitymech" / "schema" / "communitymech.yaml" - -# Verified-correct element -> CHEBI id (2026-07-18, against the local ChEBI build). -EXPECTED = { - "MetalElementEnum": { - "COPPER": "CHEBI:29036", - "IRON": "CHEBI:29033", - "ZINC": "CHEBI:27363", - "NICKEL": "CHEBI:49786", - "COBALT": "CHEBI:48828", - "VANADIUM": "CHEBI:27698", - "URANIUM": "CHEBI:27214", - "CHROMIUM": "CHEBI:28073", - "LEAD": "CHEBI:25016", - "LITHIUM": "CHEBI:49713", - "GOLD": "CHEBI:29287", - "SILVER": "CHEBI:30512", - "PALLADIUM": "CHEBI:33363", - "GALLIUM": "CHEBI:49631", - "INDIUM": "CHEBI:49664", - "TITANIUM": "CHEBI:33341", - "MERCURY": "CHEBI:16793", - }, - "RareEarthElementEnum": { - "LANTHANUM": "CHEBI:49701", - "CERIUM": "CHEBI:48782", - "PRASEODYMIUM": "CHEBI:229784", - "NEODYMIUM": "CHEBI:229785", - "SAMARIUM": "CHEBI:49890", - "EUROPIUM": "CHEBI:49591", - "GADOLINIUM": "CHEBI:49618", - "TERBIUM": "CHEBI:49902", - "DYSPROSIUM": "CHEBI:33377", # atom; ChEBI has no dysprosium(3+) cation - "HOLMIUM": "CHEBI:49650", - "ERBIUM": "CHEBI:33379", # atom; ChEBI has no erbium(3+) cation - "THULIUM": "CHEBI:33380", # atom; ChEBI has no thulium(3+) cation - "YTTERBIUM": "CHEBI:49980", - "LUTETIUM": "CHEBI:49746", - "YTTRIUM": "CHEBI:49962", - "SCANDIUM": "CHEBI:231857", - }, -} - - -def _actual_meanings(): - """{enum_name: {permissible_value: meaning_curie}} for the two element enums.""" - enums = yaml.safe_load(SCHEMA.read_text())["enums"] - out = {} - for name in EXPECTED: - pvs = enums[name]["permissible_values"] - out[name] = {pv: body.get("meaning") for pv, body in pvs.items()} - return out - - -@pytest.mark.parametrize("enum_name", list(EXPECTED)) -def test_meanings_match_expected(enum_name): - """Every element PV is grounded to its verified-correct CHEBI id.""" - assert _actual_meanings()[enum_name] == EXPECTED[enum_name] - - -def test_no_shared_meaning(): - """No two element permissible values share a CHEBI id (catches swaps/dupes).""" - seen = {} - for enum_name, pvs in _actual_meanings().items(): - for pv, meaning in pvs.items(): - if meaning in seen: - pytest.fail(f"CHEBI id {meaning} reused by {seen[meaning]} and {enum_name}.{pv}") - seen[meaning] = f"{enum_name}.{pv}" - - -def test_meanings_resolve_to_element_label(): - """Each meaning's canonical ChEBI label names its element. - - Skipped unless the ChEBI sqlite is already cached locally, so it never - triggers a multi-GB download in CI. When present, this is the check that - would have caught PALLADIUM→promethium and the rare-earth swaps at source. - """ - chebi_db = Path.home() / ".data" / "oaklib" / "chebi.db" - if not chebi_db.exists(): - pytest.skip("ChEBI sqlite not cached locally; skipping canonical-label check") - - from oaklib import get_adapter - - adapter = get_adapter(f"sqlite:{chebi_db}") - problems = [] - for enum_name, pvs in _actual_meanings().items(): - for pv, meaning in pvs.items(): - label = adapter.label(meaning) - if label is None: - problems.append(f"{enum_name}.{pv} {meaning}: id absent from ChEBI") - elif pv.lower() not in label.lower(): - problems.append(f"{enum_name}.{pv} {meaning}: label '{label}' does not name '{pv}'") - assert not problems, "Element enum groundings disagree with ChEBI:\n" + "\n".join(problems) diff --git a/tests/test_enum_groundings.py b/tests/test_enum_groundings.py new file mode 100644 index 000000000..0710d2aeb --- /dev/null +++ b/tests/test_enum_groundings.py @@ -0,0 +1,168 @@ +"""Guard every schema enum ``meaning:`` grounding against its ontology. + +Enum ``meaning:`` ids are **not** covered by ``validate-products`` (the id↔label +gate only checks record-level ``term.{id,label}`` pairs), so they drifted +silently: PALLADIUM was grounded to CHEBI:33373 *promethium* (PR #206) and 13 +rare-earth/INDIUM ids were off-by-one within the CHEBI:333xx block or +digit-transposed — e.g. INDIUM → *aluminium trifluoride*, YTTRIUM → *zinc +dichloride* (PR #207). + +This module is that missing gate, generalised from the two element enums to +**every** grounded enum in the schema. It **auto-discovers** each permissible +value that carries a ``meaning:`` and validates it, so a newly grounded enum (or a +newly grounded value) is covered automatically — or fails until registered in +``EXPECTED``. It runs in the ``validate-strict`` pytest step (already a blocking +gate) and needs no network: + +* ``test_all_meanings_match_expected`` compares the full discovered + ``{enum: {value: meaning}}`` against the frozen ``EXPECTED`` map. A changed id, + a new/removed grounded value, or a whole new grounded enum all fail here, + forcing a conscious update + re-verification. +* ``test_no_shared_meaning_within_enum`` enforces that no two values inside one + enum share an ontology id — the invariant that directly catches the + swap/duplication pattern (THULIUM and DYSPROSIUM both = CHEBI:33377, etc.). +* ``test_meanings_resolve_canonically`` additionally checks each id against its + **canonical ontology label** — for element enums the element name must appear — + but only for prefixes whose OAK sqlite is already cached locally, so it never + forces a multi-GB download in CI. + +The ``EXPECTED`` ids were verified against the local ontology builds (element +enums 2026-07-18, all enums re-confirmed 2026-07-19). Rare earths are grounded to +the ``(3+)`` cation to match each enum ``description:`` "X(3+) cation"; Dy/Er/Tm +have no ``(3+)`` term in ChEBI and are grounded to the atom (CHEBI:33377 / :33379 +/ :33380). +""" + +from pathlib import Path + +import pytest +import yaml + +SCHEMA = Path(__file__).parent.parent / "src" / "communitymech" / "schema" / "communitymech.yaml" + +# Frozen, verified element -> ontology id for every grounded enum in the schema. +# Auto-discovery (below) fails if the schema grows a grounded value not listed +# here, so this map must stay complete. +EXPECTED = { + "MetalElementEnum": { + "COPPER": "CHEBI:29036", + "IRON": "CHEBI:29033", + "ZINC": "CHEBI:27363", + "NICKEL": "CHEBI:49786", + "COBALT": "CHEBI:48828", + "VANADIUM": "CHEBI:27698", + "URANIUM": "CHEBI:27214", + "CHROMIUM": "CHEBI:28073", + "LEAD": "CHEBI:25016", + "LITHIUM": "CHEBI:49713", + "GOLD": "CHEBI:29287", + "SILVER": "CHEBI:30512", + "PALLADIUM": "CHEBI:33363", + "GALLIUM": "CHEBI:49631", + "INDIUM": "CHEBI:49664", + "TITANIUM": "CHEBI:33341", + "MERCURY": "CHEBI:16793", + }, + "RareEarthElementEnum": { + "LANTHANUM": "CHEBI:49701", + "CERIUM": "CHEBI:48782", + "PRASEODYMIUM": "CHEBI:229784", + "NEODYMIUM": "CHEBI:229785", + "SAMARIUM": "CHEBI:49890", + "EUROPIUM": "CHEBI:49591", + "GADOLINIUM": "CHEBI:49618", + "TERBIUM": "CHEBI:49902", + "DYSPROSIUM": "CHEBI:33377", # atom; ChEBI has no dysprosium(3+) cation + "HOLMIUM": "CHEBI:49650", + "ERBIUM": "CHEBI:33379", # atom; ChEBI has no erbium(3+) cation + "THULIUM": "CHEBI:33380", # atom; ChEBI has no thulium(3+) cation + "YTTERBIUM": "CHEBI:49980", + "LUTETIUM": "CHEBI:49746", + "YTTRIUM": "CHEBI:49962", + "SCANDIUM": "CHEBI:231857", + }, + "CultivationSystemEnum": { + "BIOREACTOR_UNSPECIFIED": "OBI:0001046", # "bioreactor" + }, +} + +# Enums whose permissible-value names are element names, so the canonical label +# must literally contain the value name (a stronger check than mere resolution). +ELEMENT_ENUMS = frozenset({"MetalElementEnum", "RareEarthElementEnum"}) + +# ontology prefix -> local OAK sqlite filename under ~/.data/oaklib/ +_OAK_DB = {"CHEBI": "chebi.db", "OBI": "obi.db"} + + +def _discover_meanings(): + """{enum_name: {permissible_value: meaning_curie}} for EVERY grounded enum.""" + enums = yaml.safe_load(SCHEMA.read_text())["enums"] + out = {} + for name, body in enums.items(): + grounded = { + pv: (b or {}).get("meaning") + for pv, b in ((body or {}).get("permissible_values") or {}).items() + if (b or {}).get("meaning") + } + if grounded: + out[name] = grounded + return out + + +def test_all_meanings_match_expected(): + """Every grounded enum value maps to its verified-correct ontology id. + + Auto-discovers grounded enums from the schema, so a new/changed/removed + grounding fails here until EXPECTED is updated and re-verified. + """ + assert _discover_meanings() == EXPECTED + + +def test_no_shared_meaning_within_enum(): + """No two values inside one enum share an ontology id (catches swaps/dupes).""" + problems = [] + for enum_name, pvs in _discover_meanings().items(): + seen = {} + for pv, meaning in pvs.items(): + if meaning in seen: + problems.append(f"{enum_name}: {meaning} reused by {seen[meaning]} and {pv}") + seen[meaning] = pv + assert not problems, "Duplicate enum groundings:\n" + "\n".join(problems) + + +def test_meanings_resolve_canonically(): + """Each meaning resolves to a non-obsolete term whose label fits the value. + + Runs per ontology prefix only when that prefix's OAK sqlite is already cached + locally, so it never triggers a multi-GB download in CI. For element enums the + element name must appear in the label — the check that catches a wrong id at + source (PALLADIUM→promethium, INDIUM→aluminium trifluoride, ...). + """ + from oaklib import get_adapter + + adapters = {} + checked_prefixes = set() + problems = [] + for enum_name, pvs in _discover_meanings().items(): + for pv, meaning in pvs.items(): + prefix = meaning.split(":")[0] + db = _OAK_DB.get(prefix) + if db is None: + problems.append(f"{enum_name}.{pv}: no OAK db registered for prefix {prefix}") + continue + db_path = Path.home() / ".data" / "oaklib" / db + if not db_path.exists(): + continue # ontology not cached locally; skip this prefix's checks + checked_prefixes.add(prefix) + adapter = adapters.setdefault(prefix, get_adapter(f"sqlite:{db_path}")) + label = adapter.label(meaning) + if label is None: + problems.append(f"{enum_name}.{pv} {meaning}: id absent from {prefix}") + elif label.lower().startswith("obsolete"): + problems.append(f"{enum_name}.{pv} {meaning}: obsolete term '{label}'") + elif enum_name in ELEMENT_ENUMS and pv.lower() not in label.lower(): + problems.append(f"{enum_name}.{pv} {meaning}: label '{label}' does not name '{pv}'") + + if not checked_prefixes: + pytest.skip("no ontology sqlite cached locally; skipping canonical-label check") + assert not problems, "Enum groundings disagree with the ontology:\n" + "\n".join(problems)