diff --git a/src/communitymech/datamodel/communitymech.py b/src/communitymech/datamodel/communitymech.py index b3d41f49..216d26ef 100644 --- a/src/communitymech/datamodel/communitymech.py +++ b/src/communitymech/datamodel/communitymech.py @@ -1,5 +1,5 @@ # Auto generated from communitymech.yaml by pythongen.py version: 0.0.1 -# Generation date: 2026-08-08T01:28:55 +# Generation date: 2026-08-10T18:01:58 # Schema: communitymech # # id: https://w3id.org/communitymech @@ -693,6 +693,9 @@ class EcologicalInteraction(YAMLRoot): scope: Optional[Union[str, "InteractionScopeEnum"]] = "PAIRWISE" source_taxon: Optional[Union[dict, TaxonDescriptor]] = None target_taxon: Optional[Union[dict, TaxonDescriptor]] = None + participating_taxa: Optional[ + Union[Union[dict, TaxonDescriptor], list[Union[dict, TaxonDescriptor]]] + ] = empty_list() metabolites: Optional[ Union[Union[dict, MetaboliteDescriptor], list[Union[dict, MetaboliteDescriptor]]] ] = empty_list() @@ -731,6 +734,13 @@ def __post_init__(self, *_: str, **kwargs: Any): if self.target_taxon is not None and not isinstance(self.target_taxon, TaxonDescriptor): self.target_taxon = TaxonDescriptor(**as_dict(self.target_taxon)) + self._normalize_inlined_as_dict( + slot_name="participating_taxa", + slot_type=TaxonDescriptor, + key_name="preferred_term", + keyed=False, + ) + self._normalize_inlined_as_dict( slot_name="metabolites", slot_type=MetaboliteDescriptor, @@ -3799,6 +3809,15 @@ class slots: range=Optional[Union[dict, TaxonDescriptor]], ) +slots.ecologicalInteraction__participating_taxa = Slot( + uri=COMMUNITYMECH.participating_taxa, + name="ecologicalInteraction__participating_taxa", + curie=COMMUNITYMECH.curie("participating_taxa"), + model_uri=COMMUNITYMECH.ecologicalInteraction__participating_taxa, + domain=None, + range=Optional[Union[Union[dict, TaxonDescriptor], list[Union[dict, TaxonDescriptor]]]], +) + slots.ecologicalInteraction__metabolites = Slot( uri=COMMUNITYMECH.metabolites, name="ecologicalInteraction__metabolites", diff --git a/src/communitymech/network/auditor.py b/src/communitymech/network/auditor.py index afdf8b9c..728cfbec 100644 --- a/src/communitymech/network/auditor.py +++ b/src/communitymech/network/auditor.py @@ -112,6 +112,30 @@ def issue_severity(issue: dict) -> str: return issue.get("severity") or severity_of(issue["type"]) +def _participant_terms(participant: object) -> set[str]: + """Every string a `participating_taxa` entry could use to name a member. + + Matches how the auditor already resolves `source_taxon`/`target_taxon` — + by `preferred_term`, by `term.label` and by `term.id` — because a + participant named only by CURIE is still a participant, and an id-only + match is the case an independent name-only matcher got wrong on 8 entries + when #319 was measured. + """ + if isinstance(participant, str): + return {participant} + if not isinstance(participant, dict): + return set() + found = set() + if participant.get("preferred_term"): + found.add(participant["preferred_term"]) + term = participant.get("term") or {} + if isinstance(term, dict): + for key in ("label", "id"): + if term.get(key): + found.add(term[key]) + return found + + class NetworkIntegrityAuditor: """Audit community YAML files for network data integrity issues.""" @@ -328,7 +352,31 @@ def resolve_member(name: str | None, taxon_id: str | None) -> tuple[str | None, # connections at all and every taxon in the 107 records that use them # exclusively counted as disconnected by construction (#304). if scope == "COMMUNITY_LEVEL": - connected_taxa.update(taxonomy_by_term) + # `participating_taxa` narrows the credit to the members the + # statement is actually about (#312). Absent or empty means + # "every member", which is what every record says today — so + # this changes no finding until a curator names participants. + named = interaction.get("participating_taxa") or [] + if named: + for participant in named: + # Name first, id only if no name matched. Curators copy + # the whole `taxon_term` block, which carries both — and + # resolving the id as well as the name credited every + # member sharing that id, so an entry naming one strain + # of 28 credited all 28 and the narrowing did nothing. + # Precedence, not union: the more specific reference wins. + by_name = { + token + for token in _participant_terms(participant) + if token in taxonomy_by_term + } + if by_name: + connected_taxa.update(by_name) + continue + for token in _participant_terms(participant): + connected_taxa.update(taxonomy_keys_by_id.get(token, [])) + else: + connected_taxa.update(taxonomy_by_term) # Check source_taxon (only required for PAIRWISE interactions) source = interaction.get("source_taxon") diff --git a/src/communitymech/schema/communitymech.yaml b/src/communitymech/schema/communitymech.yaml index 6524ea73..a918488c 100644 --- a/src/communitymech/schema/communitymech.yaml +++ b/src/communitymech/schema/communitymech.yaml @@ -1202,6 +1202,33 @@ classes: target_taxon: description: Target organism (if applicable) range: TaxonDescriptor + participating_taxa: + description: >- + Which members a COMMUNITY_LEVEL interaction is actually about, when it is + about some of them rather than all. Optional and usually absent: a statement + that genuinely holds across the whole community should leave it empty, and + an empty list means "every member", not "none". + + Exists because the connectivity audit credits a COMMUNITY_LEVEL interaction + as connecting every member of the record, which is unavoidably coarse - in a + record carrying both kinds of edge, a taxon in no pairwise edge is credited + by an unrelated community-level one. 407 of 522 taxa were credited solely + that way. Naming participants narrows the credit to them (#312). + + Not a second way to write a pairwise edge. If an interaction is between two + named organisms it wants `source_taxon`/`target_taxon` and PAIRWISE scope; + this is for a statement over a named subset, such as three of eight members + cross-feeding while the rest do not. + + Name participants by `preferred_term`, not by CURIE alone. Entries resolve + by preferred_term, term.label or term.id, and in strain-level records the + id does not identify a member: GLBRC_Populus_Variovorax_SynCom28 has 28 taxa + under one NCBITaxon id, so naming two of them by CURIE credits all 28 and the + slot appears to do nothing. That is the sound reading of an ambiguous + reference, and it is also exactly the record shape where narrowing matters + most (#524). + range: TaxonDescriptor + multivalued: true metabolites: description: Metabolites involved in the interaction range: MetaboliteDescriptor diff --git a/tests/test_community_level_connectivity_credit.py b/tests/test_community_level_connectivity_credit.py index bb6080bc..f5ee70c2 100644 --- a/tests/test_community_level_connectivity_credit.py +++ b/tests/test_community_level_connectivity_credit.py @@ -153,20 +153,25 @@ def test_the_worked_example_still_shows_the_limit(): assert any(i.get("scope") == "COMMUNITY_LEVEL" for i in interactions) -def test_the_schema_still_cannot_name_participants(): - """The root cause, and the thing whose arrival should retire this file. - - `EcologicalInteraction` has `source_taxon`/`target_taxon` and nothing else, - which is why the credit can only be all-or-nothing. When - `participating_taxa` (or whatever #307 settles on) lands, this fails — and - the credit rule should become precise in the same change. +def test_the_schema_can_now_name_participants(): + """#312's refinement landed; this file records what it did not change. + + `participating_taxa` is on `EcologicalInteraction` and the auditor narrows + the community-level credit to the members it names. The numbers above are + unaffected because no record uses it yet — absent or empty still means + "every member", which is what makes the slot safe to land ahead of curation. + + When records do start naming participants, `credited_solely_by_the_rule` + should fall and the bound in + `test_the_share_credited_solely_by_the_rule_has_not_stepped_up` will need + re-measuring downward rather than widening. See tests/test_participating_taxa.py. """ schema = yaml.safe_load( (REPO / "src/communitymech/schema/communitymech.yaml").read_text(encoding="utf-8") ) attributes = schema["classes"]["EcologicalInteraction"]["attributes"] - assert "participating_taxa" not in attributes, ( - "EcologicalInteraction can now name its participants, so the " - "all-or-nothing credit in audit_community should be narrowed to them " - "and this file retired (#312, #307)." + assert "participating_taxa" in attributes, ( + "the slot #312 asked for is gone again; the all-or-nothing credit in " + "audit_community has nothing to narrow against" ) + assert attributes["participating_taxa"].get("multivalued") is True diff --git a/tests/test_participating_taxa.py b/tests/test_participating_taxa.py new file mode 100644 index 00000000..71ecb84c --- /dev/null +++ b/tests/test_participating_taxa.py @@ -0,0 +1,193 @@ +"""`participating_taxa` narrows the community-level connectivity credit (#312). + +A `COMMUNITY_LEVEL` interaction was credited as connecting **every** member of +the record. Defensible — such an interaction asserts something holding across +the community rather than between a named pair — and unavoidably coarse, because +`EcologicalInteraction` had only `source_taxon` and `target_taxon` and no way to +say *which* members participate. In a record carrying both kinds of edge, a +taxon in no pairwise edge was credited by an unrelated community-level one: +**407 of 522 taxa** were credited solely that way. + +`participating_taxa` is the refinement #312 proposed. Optional, and absent or +empty means "every member" — so the corpus behaves identically today (measured: +55 findings before, 55 after, same breakdown). Nothing changes until a curator +names participants, which is the property that makes this safe to land ahead of +any curation. + +Two things here were found by running the code rather than reading it, and both +are pinned below: + +* an entry naming a member by CURIE resolved to the id string, which is not a + key of `taxonomy_by_term` — so naming participants by id credited *nobody* + and disconnected the whole record. The name path returned the right count + throughout, so a test of the happy path alone would have shipped it. +* once that was fixed, naming by CURIE in the worked example credits **all 28** + members, because those 28 taxa share one NCBITaxon id. That is correct, and it + is the trap in #524: the slot silently does nothing in exactly the strain-level + records whose over-broad credit motivated it. +""" + +from __future__ import annotations + +import pathlib +import tempfile + +import pytest +import yaml + +from communitymech.network.auditor import IssueType, NetworkIntegrityAuditor + +REPO = pathlib.Path(__file__).parent.parent +COMMUNITIES = REPO / "kb/communities" +# #312's illustration: 28 taxa, every interaction COMMUNITY_LEVEL. +EXAMPLE = COMMUNITIES / "GLBRC_Populus_Variovorax_SynCom28.yaml" + + +def _disconnected(document: dict) -> int: + """DISCONNECTED findings for one record, audited in isolation.""" + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "record.yaml" + path.write_text( + yaml.safe_dump(document, sort_keys=False, allow_unicode=True, width=4096), + encoding="utf-8", + ) + issues = NetworkIntegrityAuditor(pathlib.Path(directory)).audit_community(path) or [] + return sum(1 for issue in issues if issue["type"] == IssueType.DISCONNECTED) + + +@pytest.fixture +def example() -> dict: + return yaml.safe_load(EXAMPLE.read_text(encoding="utf-8")) + + +def _members(document: dict) -> list[dict]: + return [(entry.get("taxon_term") or {}) for entry in document.get("taxonomy") or []] + + +def test_the_example_still_has_the_shape_the_test_needs(example): + """Guard: if the record changes, the numbers below stop meaning anything.""" + assert len(_members(example)) == 28 + interactions = example["ecological_interactions"] + assert len(interactions) == 3 + assert all(i.get("scope") == "COMMUNITY_LEVEL" for i in interactions) + + +def test_absent_participating_taxa_still_credits_everyone(example): + """The default, and why landing this changes no finding today.""" + assert _disconnected(example) == 0 + + +def test_an_empty_list_means_every_member_not_none(example): + """`[]` is "unspecified", not "nobody". + + The opposite reading would turn an interaction that omits the slot by + accident into 28 spurious DISCONNECTED findings. + """ + for interaction in example["ecological_interactions"]: + interaction["participating_taxa"] = [] + assert _disconnected(example) == 0 + + +def test_naming_participants_narrows_the_credit(example): + """The point of #312: 2 named of 28 leaves 26 uncredited.""" + members = _members(example) + named = [ + {"preferred_term": m.get("preferred_term"), "term": m.get("term")} for m in members[:2] + ] + for interaction in example["ecological_interactions"]: + interaction["participating_taxa"] = named + assert _disconnected(example) == 26 + + +def test_narrowing_one_of_three_interactions_is_not_enough(example): + """Credit is a union across interactions, so the others still cover everyone. + + Worth pinning because it is how the first run of this check fooled me: I + narrowed only the first interaction, saw 0, and briefly took the feature + for broken rather than the probe. + """ + members = _members(example) + example["ecological_interactions"][0]["participating_taxa"] = [ + {"preferred_term": members[0].get("preferred_term"), "term": members[0].get("term")} + ] + assert _disconnected(example) == 0 + + +def test_a_curie_only_entry_resolves_rather_than_crediting_nobody(example): + """The bug the canary caught: ids are not keys of `taxonomy_by_term`. + + Before the fix this returned 28 — every member disconnected — because the + id string matched no member key and the interaction credited nothing. + """ + members = _members(example) + by_id = [{"term": {"id": m["term"]["id"]}} for m in members[:2] if m.get("term", {}).get("id")] + assert by_id, "the example lost its term ids" + for interaction in example["ecological_interactions"]: + interaction["participating_taxa"] = by_id + assert _disconnected(example) != 28, ( + "a CURIE-named participant credited nobody, which disconnects the whole " + "record — ids resolve through `taxonomy_keys_by_id`, not `taxonomy_by_term`" + ) + + +def test_a_curie_is_ambiguous_where_members_share_an_id(example): + """#524, pinned as a property rather than left as a surprise. + + All 28 taxa carry `NCBITaxon:34072`. Naming two by CURIE credits all 28, + which is the only sound reading of an ambiguous reference — and means the + slot appears to do nothing in exactly the strain-level records that + motivated it. The guidance ("name by preferred_term") is on the slot. + """ + members = _members(example) + ids = {m.get("term", {}).get("id") for m in members} + assert len(ids) == 1, "the example no longer shares one id across its members" + + by_id = [{"term": {"id": next(iter(ids))}}] + for interaction in example["ecological_interactions"]: + interaction["participating_taxa"] = by_id + assert _disconnected(example) == 0 + + +def test_the_corpus_is_unchanged_by_this_feature(): + """Nothing uses the slot yet, so no finding may move. + + Asserted on the corpus rather than trusted from the schema: an + `ifabsent` or a default that quietly populated the slot would change 312 + records' connectivity without anyone editing a record. + """ + users = [ + path.name + for path in sorted(COMMUNITIES.glob("*.yaml")) + for interaction in (yaml.safe_load(path.read_text()) or {}).get("ecological_interactions") + or [] + if isinstance(interaction, dict) and interaction.get("participating_taxa") + ] + assert users == [], ( + f"{len(users)} records now use participating_taxa. That is fine and " + f"expected eventually — but the connectivity numbers in " + f"tests/test_community_level_connectivity_credit.py were measured " + f"without it, so check them: {sorted(set(users))[:5]}" + ) + + +def test_a_name_beats_an_id_on_the_same_entry(example): + """Precedence, not union — and the flaw that nearly shipped this useless. + + Curators copy the whole `taxon_term` block into a participant, so an entry + normally carries `preferred_term` *and* `term.id`. Resolving both and + unioning them meant the id credited all 28 members sharing it, so an entry + naming one strain credited every strain and the narrowing did nothing. + + The name path alone gave the right answer at every step, and the id path + alone gave the right answer; only an entry carrying both was wrong. That is + why this case is pinned separately from the two above. + """ + members = _members(example) + both = [{"preferred_term": m.get("preferred_term"), "term": m.get("term")} for m in members[:2]] + assert all(entry["term"].get("id") for entry in both), "the fixture lost its ids" + for interaction in example["ecological_interactions"]: + interaction["participating_taxa"] = both + assert _disconnected(example) == 26, ( + "an entry carrying both a preferred_term and a shared term.id credited " + "every member sharing that id, defeating the narrowing (#312/#524)" + )