feat(associations): flexible association-type section matching

[kevinschaper]

Draft — the enabler that lets a node-page association section be defined by more than a single biolink category. Groundwork for LOINC (#1349) and in-app MEDIC+CTD (#1348); no user-visible change on its own (fully backward-compatible).

Why

Sections are currently keyed 1:1 on the association category. That can't express sections whose edges all share one generic category (LOINC's biolink:Association) or that span several categories (MEDIC + CTD). This generalizes the matching.

What

AssociationTypeMapping becomes a set of optional list criteria — category, predicate, subject_category, object_category, primary_knowledge_source, provided_by — plus a stable key. Values within a criterion are OR'd; criteria are AND'd; an omitted criterion is unconstrained.

• Schema (model.yaml → regenerated model.py/model.ts): key + multivalued criteria on AssociationTypeMapping; key on AssociationCount.
• Counts: get_solr_query_fragment builds the composite AND-of-OR clause (single-category mappings render identically); parse_association_counts attributes each facet result by rebuilding its exact query string and accumulates on key.
• Table: get_association_table treats its arg as a section key, resolves the mapping, and applies its category list + predicate/subject/object/source filters. Unknown key ⇒ literal category filter (back-compat). /entity/{id}/{key} path param loosens from the enum to a string.
• Frontend: sections key on ac.key (falling back to category), so the section id and table URL carry the key.
• Loader normalizes scalar yaml → list and defaults key to the category, so existing mappings/behaviour are unchanged.

Backward compatibility

Every existing single-category mapping keeps key == category, single-value criteria render byte-identical, and counts are unchanged — verified by regenerating the counts fixtures from code (identical values) and the passing suite.

Tests

Backend: mapping/parser/query/implementation suites green (134); mapping tests updated to the list API incl. composite + OR-within cases. Frontend: useAssociationCategories covers key-preferred ids; typecheck/lint clean. (API-layer tests hit a pre-existing AttributeError: JSON collection error also present on main.)

Follow-ons

• LOINC clinical-measurement sections (Add clinical measurement (LOINC) support and association sections on node pages #1349): mostly config on top of this.
• In-app MEDIC+CTD merged section (Display MEDIC + CTD drug-indication ("treats") associations on node pages #1348) via category: [medic, ctd].
• biolink:ClinicalMeasurement entity-type support + hierarchy widget (separate).

https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL

[netlify]

✅ Deploy Preview for monarch-app canceled.

Name	Link
🔨 Latest commit	185ebbb
🔍 Latest deploy log	https://app.netlify.com/projects/monarch-app/deploys/6a51bd14366322000803edcf

[claude]

Review

This is solid groundwork — the composite-criteria design (list-valued category/predicate/subject_category/object_category/primary_knowledge_source/provided_by, AND-of-OR query building, key defaulting to category for backward compat) is clean, and the fixture/test updates for the query-building and counting logic are thorough and internally consistent (I traced get_solr_query_fragment through both the query-building side in build_association_counts_query and the response-parsing side in parse_association_counts — they use the identical fragment so the round-trip lookup is sound).

🐛 Bug: broken direction detection in get_generic_entity_grid

backend/src/monarch_py/implementations/solr/solr_implementation.py:1093-1099 (unchanged by this diff, but silently broken by the datamodel change):

```python
if mapping and mapping.subject_category and mapping.object_category:
# Use YAML metadata to determine direction
if context_category == mapping.subject_category:
...
elif context_category == mapping.object_category:
...
else:
# falls back to a crude "Gene" in first_col_cat string heuristic
```

AssociationTypeMapping.subject_category/object_category are now Optional[list[str]] instead of Optional[str]. context_category is still a plain string (e.g. "biolink:Disease"), so context_category == mapping.subject_category is now comparing a string to a list — this is always False, for every mapping, every time. The YAML-metadata branch can never be taken anymore; every call to get_generic_entity_grid silently falls through to the "Gene" in first_col_cat heuristic.

That heuristic actively gives the wrong answer in reproducible cases, e.g. context entity is a biolink:Disease, column_assoc_categories=["biolink:CausalGeneToDiseaseAssociation"] (subject=Gene, object=Disease): correct direction is context_field="object", but because "Gene" appears in the category name, the heuristic sets context_field="subject" — backwards. This affects the public generic entity-grid endpoint in entity_grid.py.

Fix is presumably just:
```python
if context_category in (mapping.subject_category or []):
...
elif context_category in (mapping.object_category or []):
```

Worth noting the existing tests (test_generic_grid_field_mapping, test_get_generic_entity_grid_accepts_multiple_row_categories) only assert result is not None — they don't check context_field/column_field, so this regression wouldn't be caught by CI. Might be worth adding an assertion on the resolved direction/fields for at least one non-Gene-context case.

Minor / design notes

• AssociationTypeMappings.get_mapping(category) (association_type_utils.py:27) returns the first mapping whose category list contains the given category. That's fine today (categories are still 1:1), but once composite mappings that share a category (e.g. the planned LOINC biolink:Association sections) land, this becomes ambiguous — and it's exactly what get_generic_entity_grid uses to resolve direction metadata (compounding the bug above). Might be worth a comment/guard now, or switching that caller to get_mapping_by_key once callers can supply a key, since it'll be an easy landmine to hit when Add clinical measurement (LOINC) support and association sections on node pages #1349/Display MEDIC + CTD drug-indication ("treats") associations on node pages #1348 build on this.
• Escaping inconsistency: _or_group in association_type_utils.py builds Solr clauses via raw f-string interpolation (f'{field}:"{value}"'), whereas the rest of the query-building path (SolrQuery.add_field_filter_query) runs values through an escape() helper. Today all values come from the static association_type_mappings.yaml, so it's not exploitable, but it's a divergent pattern in the same file that computes Solr fragments — worth aligning if these functions ever take less-trusted input.
• API contract change: /entity/{id}/{category}'s path param moved from the AssociationCategory enum to a bare str. That's called out in the PR description as intentional, but it does mean OpenAPI no longer documents/validates the allowed values, and a typo'd category now silently returns an empty table instead of a 422. Worth double-checking that's the desired trade-off for API consumers.

Nice touches

• The MULTIVALUED_CRITERIA scalar→list normalization in the loader keeps the existing YAML untouched — good backward-compat move.
• test_solr_or_within_criterion / test_solr_composite_mapping directly exercise the new AND-of-OR logic.
• useAssociationCategories.test.ts's new "prefers key over category" test matches the stated LOINC/MEDIC+CTD use case well.

[claude]

Review: flexible association-type section matching

Overall this is a well-scoped, backward-compatible generalization — the "OR within a criterion, AND across criteria" model is clean, the query-fragment-rebuild approach in parse_association_counts (rather than regexing the query string back apart) is a nice robustness improvement, and the new unit tests (composite mapping, OR-within-criterion, key defaulting) cover the new surface well.

Potential issues

1. get_association_table silently drops criteria filters when a matched mapping has no category (solr_implementation.py)
```python
mapping = AssociationTypeMappings.get_mapping_by_key(key)
if mapping and mapping.category:
categories = mapping.category
criteria_filters = get_solr_criteria_filters(mapping)
else:
categories = [key]
criteria_filters = []
```
The schema explicitly makes category optional/unconstrained ("an omitted criterion places no constraint on that field"), so a future mapping keyed purely on predicate/subject_category/object_category (no category) is a legitimate case per the model. But here, if mapping is found by key and simply has no category, the code falls into the else branch, discards criteria_filters entirely, and treats the raw key as a literal category filter — which is wrong. No current mapping hits this (all real mappings set category), but it contradicts the stated design and will misbehave the moment a category-less mapping is added. Worth guarding on if mapping: instead of if mapping and mapping.category:, falling back to [key] only when no mapping is found at all.

2. AssociationTypeMappings.get_mapping(category) becomes ambiguous once multiple sections share a category
This is exactly the scenario the PR enables (e.g. LOINC's generic biolink:Association), and the docstring you added even acknowledges it ("Get the first mapping that includes the given category"). get_generic_entity_grid and get_traversable_associations both resolve subject/object direction via this category-keyed lookup. Once a category maps to more than one AssociationTypeMapping (per the follow-on LOINC/MEDIC+CTD work), these call sites can silently pick the wrong mapping's subject_category/object_category, producing incorrect direction resolution. Not a bug today, but worth a # TODO or a follow-up issue so it isn't forgotten before #1349/#1348 land.

3. /entity/{id}/{key} path param validation loosened from enum to str
category: AssociationCategory = Path(...) → category: str = Path(...) in entity.py. This is called out as intentional for back-compat, but it removes the allowlist FastAPI/pydantic previously enforced before the value reaches Solr. When the key doesn't match any known mapping, get_association_table now does categories = [key] with the raw, attacker-controlled path segment, which flows unescaped into category:"<value>" style Solr filter construction (_or_group/build_association_query don't escape quotes). Previously this parameter was constrained to fixed enum members, so this wasn't reachable; now arbitrary strings are. Worth confirming the Solr client/query builder is safe against a key containing ", ), or Solr query syntax before merging — even if practically low-severity (read-only facet/filter query), it's a new user-controlled input reaching query construction that wasn't there before.

4. Query semantics quietly change for existing mappings that already had subject_category/object_category set
Before this PR, get_solr_query_fragment only ever emitted category:"X" — subject_category/object_category were descriptive metadata, never used to filter. After this PR, any existing mapping that already had these fields populated (confirmed via the regenerated fixtures — DiseaseToPhenotypicFeatureAssociation's facet query gained AND subject_category:"biolink:Disease" AND object_category:"biolink:PhenotypicFeature") now has them enforced as real Solr filters. The regenerated fixture counts matched, which is good, but that fixture is synthetic/derived from the code itself rather than a live Solr comparison, so it confirms internal consistency rather than ruling out real-world edge cases (e.g. reasoning/closure producing a subject/object category that doesn't literally match the mapping's declared one). Might be worth a quick smoke-test against the real index for the couple of existing mappings that set these fields, just to be sure counts genuinely don't move in production.

Minor / style

• _or_group (Solr) and the inline _or_group_sql closure in get_sql_query_fragment duplicate the same OR-join logic with a different separator — could be unified with a sep param, but low priority given the small size.
• No test currently exercises the "mapping matched by key but category is empty" path called out in Support basic association counts on /entity/{id} #1 — worth adding once you're comfortable with the fix, since the schema explicitly allows it.

Nits

• Good catch removing the double-blank-line noise in the fixture files as part of regeneration.
• get_mapping/get_mapping_by_key are linear scans over all mappings per call; fine at current scale, not worth optimizing.

Nice groundwork PR — the design generalizes cleanly and the backward-compat story (verified via regenerated fixtures + existing suite) is convincing for the common case. The items above are mostly forward-looking correctness/ambiguity risks that matter once LOINC/MEDIC+CTD mappings actually land, plus one API-surface hardening question (#3) worth a quick look before merge.

[claude]

Review: flexible association-type section matching

This is a well-scoped, backward-compatible generalization of AssociationTypeMapping to multivalued match criteria + a stable key. The backward-compat strategy (verified against regenerated fixtures, and the follow-up fix commits for the subject_category/object_category membership bug and the legacy-undercounting issue) is solid engineering — nice catch on the GeneToExpressionSiteAssociation (-83%) / GenotypeToGeneAssociation (-99.8%) regressions before merge.

A few things worth a look before this lands as the base for #1349/#1348:

Correctness / design risk

• uses_full_criteria's implicit toggle (backend/src/monarch_py/utils/association_type_utils.py:500-513): whether a mapping's predicate/subject_category/object_category/primary_knowledge_source/provided_by are treated as real Solr filters vs. inert direction-metadata is decided entirely by whether agm.key happens to differ from a value in agm.category. That's an implicit, string-identity-based contract with no structural enforcement in the YAML — a future author adding a key: to an existing single-category legacy section purely for URL/UI reasons (without intending to opt into extra filtering) would silently start constraining the query on subject_category/object_category, reproducing exactly the undercounting bug the last commit just fixed. An explicit boolean (e.g. match_criteria: full or similar) would make this opt-in visible in the YAML itself instead of depending on key/category divergence.

• Predicate/source-only mappings aren't fully supported: get_association_table (backend/src/monarch_py/implementations/solr/solr_implementation.py:786-792) only pulls a mapping's real criteria when mapping.category is truthy — if mapping and mapping.category: ... else: categories = [key]. If a future mapping (e.g. something keyed purely on predicate/provided_by with no category) is looked up by key, this silently falls into the "unknown key" fallback and filters on the literal key string as a category instead of using the mapping's actual criteria. Not exercised today (every current/planned mapping still has a category), but worth a comment or guard given the framework is explicitly built for criteria beyond category.

Minor / quality

• _or_group/_or_group_sql in association_type_utils.py reimplement OR-of-values Solr clause building that already exists as SolrQuery.add_field_filter_query (backend/src/monarch_py/datamodels/solr.py:102). Different quoting convention (quoted phrases vs. escape()-based), so not a drop-in swap, but two parallel implementations of the same concept in the codebase is a bit of a maintenance trap.
• parse_association_counts's new lookup dict (backend/src/monarch_py/implementations/solr/solr_parsers.py:268-279) is keyed by the literal rebuilt query string across all mappings x suffixes. If two mappings ever produce an identical get_solr_query_fragment() output (e.g. a config mistake — duplicate category with no other criteria), one will silently overwrite the other with no error. Might be worth a cheap startup/test assertion that all mapping fragments are unique.
• entity.py's /entity/{id}/{category} path param loosening from the AssociationCategory enum to str (mentioned in the PR description) means FastAPI no longer 422s on a bogus category/key at the routing layer — it'll now just resolve via the "unknown key -> literal category" fallback and return an empty table. Probably fine/intentional given keys are now free-form, just flagging the behavior change for anyone relying on the old validation.

Test coverage

Backend tests look thorough for the new list/OR/AND semantics and the legacy-vs-composite distinction (test_solr_legacy_mapping_matches_on_category_only, test_solr_or_within_criterion, etc.). One gap: no test exercises get_association_table's two branches in solr_implementation.py directly (mapping found with criteria vs. unknown-key literal fallback) — coverage there is currently only indirect via the mapping-level unit tests. Frontend coverage for the key-preferred id looks good.

Overall: solid, low-risk groundwork with good regression discipline. The uses_full_criteria heuristic is the one piece I'd reconsider making explicit before more sections start relying on it in #1349/#1348.