From 9d384ea216984c81814129092b323bd37d2e7116 Mon Sep 17 00:00:00 2001 From: "marcin p. joachimiak" <4625870+realmarcin@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:17:02 -0700 Subject: [PATCH] Let a community-level interaction name its participants (#312) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A COMMUNITY_LEVEL interaction was credited as connecting every member of the record, because EcologicalInteraction had no way to say which members it was about. In a record carrying both kinds of edge that credits a taxon in no pairwise edge via an unrelated community-level one: 407 of 522 taxa were credited solely that way. `participating_taxa` narrows it. Optional, and absent or empty means "every member" — so the corpus is bit-identical today: 55 findings before, 55 after, same breakdown across all four types. Nothing moves until a curator names participants, which is what makes it safe to land ahead of any curation. Three things came out of running it rather than reading it. Ids are not keys of taxonomy_by_term, so a participant named by CURIE credited *nobody* and disconnected the whole record. The name path returned the right count throughout, so a happy-path test would have shipped it. Fixing that by resolving names and ids together was worse, and nearly made the slot useless: curators copy the whole taxon_term block, so an entry normally carries both, and the id credited every member sharing it. In the worked example that is all 28 — one strain named, all 28 credited, no narrowing. It is precedence, not union: a name that matches wins, and the id is a fallback. Even then a CURIE is ambiguous where members share an id, which is exactly the strain-level SynCom shape the coarse credit hurts most. That is sound — an ambiguous reference cannot mean fewer members — but it means the slot silently does nothing there, so the guidance is on the slot and the behaviour is pinned rather than left to be rediscovered (#524). Co-Authored-By: Claude Opus 5 --- src/communitymech/datamodel/communitymech.py | 21 +- src/communitymech/network/auditor.py | 50 ++++- src/communitymech/schema/communitymech.yaml | 27 +++ ...est_community_level_connectivity_credit.py | 27 ++- tests/test_participating_taxa.py | 193 ++++++++++++++++++ 5 files changed, 305 insertions(+), 13 deletions(-) create mode 100644 tests/test_participating_taxa.py 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)" + )