feat(associations): MEDIC + CTD drug-indications section + details modal (#1348) - #1357

Review

Reviewed the diff for this PR (stacked on #1355). Overall this is a clean, well-scoped change — the new drug_indications mapping is well-commented, the association-details modal enrichment is neatly organized, and the new parseRetrievalSources util has solid unit test coverage (happy path, empty/missing, malformed entries).

Backend (association_type_mappings.yaml, association_type_utils.py)

• The merged MEDIC + CTD mapping (key: drug_indications, multivalued category/subject_category/object_category) looks correct against the enabler's get_mapping/get_solr_query_fragment logic in the base branch — no other mapping entry references either biolink:ChemicalOrDrugOrTreatmentToDiseaseOrPhenotypicFeatureAssociation or biolink:ChemicalEntityToDiseaseOrPhenotypicFeatureAssociation, so get_mapping(category)'s "first match" semantics can't collide here.
• One non-blocking observation (inherited from the stacked base branch, not introduced here): AssociationTypeMappings.get_traversable_associations picks _first(mapping.category) as the canonical category for a mapping. For this two-category mapping, that means any 1-hop traversal link built from the CTD side of drug_indications would carry the MEDIC category string. Worth double-checking if/when the entity-grid traversal feature is turned on for this section, since this mapping is the first real multi-category consumer of that path.
• Fixture regeneration (association_counts*.py) is mechanical and consistent with the new merged facet query — verified against the described counts (6,619 + 19,139 = 25,758).

Frontend

• use-association-categories.ts: un-hiding HIDDEN_CATEGORIES (now empty) makes sense now that the mapping fully defines subject_category/object_category, removing the need for the old "not yet implemented" 1-hop caveat. Minor nit: new Set<string>([]) could just be new Set<string>().
• retrievalSources.ts: nicely defensive — parses each sources entry independently and skips malformed ones without throwing. The double-encoded shape (a string[] where each entry is itself a JSON-encoded array of retrieval-source records) is unusual, but the code also handles a flatter shape (else if (parsed) out.push(parsed)), so it's reasonably forward-compatible if the serialization shape changes upstream.
• SectionAssociationDetails.vue: all the new fields referenced (knowledge_level, sources, aggregator_knowledge_source, original_predicate, qualifiers, has_count/has_total/has_percentage/has_quotient, FDA_adverse_event_level, negated, *_qualifier_label) exist on DirectionalAssociation/ExpandedAssociation/Association, so no type mismatches. Each new block is properly gated with v-if.
• qualifierRows labels every entry from the generic qualifiers array as "Qualifier" — loses the distinction between different qualifier types if more than one is present. Minor UX nit, not a bug.

Test coverage

• parseRetrievalSources is well covered by unit tests.
• The new Vue computed properties (formatRole, qualifierRows, hasFrequencyData) and the new template blocks in SectionAssociationDetails.vue don't have dedicated component tests — low risk given how simple the logic is, but worth noting since the PR description only mentions the backend suite, the retrieval-sources unit test, and frontend typecheck/lint.

Security / Performance

• No concerns. JSON.parse is wrapped in try/catch per-entry, Vue's template interpolation auto-escapes rendered values (no v-html used), and the added computed properties are cheap.

Nice work overall — the mapping design (multivalued category criterion) and the provenance-chain parsing are both solid, pragmatic solutions to a genuinely awkward data-merging problem.

Review: MEDIC + CTD drug-indications section + details modal

Reviewed against gh pr diff 1357 (diff scoped to the stack on top of #1355). Overall this is a clean, well-tested change — the mapping merge is correctly scoped and the new UI additions are properly guarded and type-safe.

Summary

• Merges MEDIC (biolink:ChemicalOrDrugOrTreatmentToDiseaseOrPhenotypicFeatureAssociation) and CTD (biolink:ChemicalEntityToDiseaseOrPhenotypicFeatureAssociation) into one drug_indications section via the new multivalued category criterion, replacing the old disabled "Medical Action" placeholder entry.
• Un-hides the drug-indications category in useAssociationCategories (the HIDDEN_CATEGORIES set is now empty but kept as an extension point).
• Adds a parseRetrievalSources util + several new AppDetail blocks to SectionAssociationDetails.vue (knowledge level, nested provenance chain, aggregator source, original predicate, qualifiers, frequency data, FDA adverse-event level, negated).
• Regenerates backend fixtures and adds a medic-ingest entry to docs/sources-manifest.yaml.

Correctness

• No double-counting risk: MEDIC and CTD categories are mutually exclusive strings, and get_solr_query_fragment ORs them into a single Solr query per mapping, so a given association is only counted once even though two categories feed the same section. No other mapping entry shares either category.
• AssociationTypeMappings.get_mapping() returns the first mapping containing a given category — since drug_indications is the only entry referencing either category, resolving on the bare MEDIC category is unambiguous.
• The new key: drug_indications is unique across the mapping file (only explicit key currently defined), so no collision with get_mapping_by_key.
• All new template fields (knowledge_level, aggregator_knowledge_source, original_predicate, FDA_adverse_event_level, *_qualifier_label, negated) exist on the Association/DirectionalAssociation TS types, and the icons used (scale-balanced, sitemap, database, code, clipboard-list, equals, circle-exclamation, circle-xmark) are already registered in global/icons.ts, so nothing here should break at runtime.
• docs/sources-manifest.yaml entry is alphabetically placed correctly (maxo → medic → mmrrc), consistent with the existing ctd-ingest entry already present.

Code quality / style

• parseRetrievalSources is small, defensive (skips unparseable entries, handles both string and pre-parsed input despite the stricter TS type), and has good unit-test coverage for the happy path, empty/missing input, and malformed entries.
• One minor gap: the branch else if (parsed) out.push(parsed) (single non-array object case) isn't exercised by retrievalSources.test.ts — worth a quick test case if that shape is expected from the API, otherwise consider dropping the branch if MEDIC/CTD always serialize as arrays.
• Minor CSS nit: .retrieval-source .role and .retrieval-source .upstream both just set color: $gray; — could be combined into a single selector, purely cosmetic.
• The YAML comment above the new mapping entry is a nice touch for explaining why two categories are merged into one section.

Test coverage

• useAssociationCategories.test.ts update clearly documents the behavior change (drug-indications no longer hidden, now keyed on key) and the existing suite (special ordering, key-vs-category id precedence) still passes with the new fixture shape.
• Backend fixtures (association_counts*.py) were regenerated rather than hand-edited, which is the right way to keep large fixture literals in sync with real query output.
• No new backend unit test explicitly asserts get_mapping() resolving the bare MEDIC category to drug_indications (the PR description mentions this was manually verified). Since this is a key claim for backward compatibility of the table endpoint, a small regression test in test_association_type_mapping.py would be a cheap, valuable addition as a permanent guardrail against a future mapping-file edit accidentally breaking it.

Security / performance

• No security concerns — this is read-only display of already-fetched association metadata, and Vue's default text interpolation escapes it.
• Performance is fine: parseRetrievalSources runs per-association inside a computed(), and the payloads involved are small.

Nits

• This is a draft PR stacked on feat(associations): flexible association-type section matching #1355, so this review is against the diff as it currently stands on this branch; context like association_type_utils.py / get_mapping comes from the base branch and wasn't touched here — referenced only for verification.

Nice work — this looks ready modulo the small optional test-coverage suggestion above, once #1355 lands and this is retargeted to main.

Review

Reviewed the diff against feat/flexible-association-section-matching (this PR's actual base). Overall this is a clean, well-scoped addition that follows existing conventions closely (the predicate filter is a near-exact mirror of the existing taxon filter plumbing in SectionAssociations.vue/AssociationsTable.vue, which makes it easy to follow and low-risk).

Correctness

• association_type_mappings.yaml: the merged drug_indications entry (multivalued category/subject_category/object_category) is correctly picked up by get_mapping_by_key in get_association_table (solr_implementation.py:795), and get_solr_criteria_filters correctly turns the extra criteria into AND'd/OR'd Solr clauses. Verified the frontend always sends ac.key || ac.category (use-association-categories.ts:19), so the table endpoint reliably receives "drug_indications" rather than a bare category — good.
• predicateFilterConfig.ts keys off the section key ("drug_indications"), which matches what category.id resolves to for this merged section — confirmed via the fixture ({'key': 'drug_indications', ...}) and the composable. Consistent and correct.
• predicate as a bare Solr facet/filter field is legitimate — it's already used the same way in solr_query_utils.py (build_association_query/build_association_table_query default sort), so AssociationsTable.vue's new facetFields/filterQueries additions aren't introducing an unindexed field.
• One very minor nit on the PR description rather than the code: it credits get_mapping() for the "bare MEDIC category still resolves to drug_indications" behavior, but the table endpoint actually never calls get_mapping() — it calls get_mapping_by_key(), which falls back to a literal single-category filter (solr_implementation.py:800) when the key doesn't match. The net effect described (bare category still returns results) is correct, just via the fallback branch rather than get_mapping(). Worth a description tweak for future readers, not a code change.
• SectionAssociationDetails.vue's new fields (knowledge_level, sources, original_predicate, aggregator_knowledge_source, qualifiers, has_count/has_total/has_percentage/has_quotient, FDA_adverse_event_level, negated) all exist on the generated Association/DirectionalAssociation model, and all new icon names (scale-balanced, sitemap, database, code, clipboard-list, equals, circle-exclamation, circle-xmark) are already registered in frontend/src/global/icons.ts, so nothing here will silently fail to render.
• parseRetrievalSources correctly handles the "array of JSON-encoded strings" shape from the API and skips malformed entries without throwing — nice defensive parsing for external/generated data.

Test coverage

• Good unit coverage for the two new pure utils (retrievalSources.test.ts, predicateFilterConfig.test.ts) and the updated useAssociationCategories test.
• There's no component-level test exercising the new predicate-filter wiring in AssociationsTable.vue/SectionAssociations.vue or the new provenance/qualifiers rendering in SectionAssociationDetails.vue, but that matches the existing convention in this codebase (the taxon filter it mirrors isn't component-tested either), so it's not a regression in test rigor — just flagging as a possible follow-up if you want to raise the bar here.

Minor/style

• The Set<string>([]) for HIDDEN_CATEGORIES in use-association-categories.ts now has nothing in it after this change — if nothing is expected to land there soon, consider just removing the hidden-category filter step entirely (with a comment noting where to re-add it) rather than keeping an always-empty set around. Not blocking, just a small simplification opportunity.
• formatRole/formatPredicate fallback logic (default "Source", de-prefix + startCase) reads well and degrades gracefully for unknown/future predicates and roles.

Security / performance

• No security concerns — this is read-only display/filtering of already-authorized association data, and the only "parsing untrusted data" path (JSON.parse in parseRetrievalSources) is wrapped in try/catch.
• Performance is fine: facet requests are incremental (only added when the section is predicate-filterable), and the JSON.stringify dedup check in emitPredicateOptions/emitTaxonOptions is bounded by a small facet-value list per section, not the full association set.

Nice work — this is a solid, low-risk stacked PR that reuses established patterns well.

Review

Nice PR — the "combine two categories under one section key" approach is clean, and the details-modal enrichment (provenance chain, knowledge level, qualifiers, frequency data) is a solid addition. I verified the diff (gh pr diff 1357) against the current checkout. A few notes below; nothing blocking.

Correctness

• get_mapping_by_key resolution confirmed correct. I traced the call path: AssociationsTable.vue → getAssociations(..., category.id, ...) → GET /entity/{id}/{category} → _association_table → solr_implementation.get_association_table, which calls AssociationTypeMappings.get_mapping_by_key(key) first (matches on mapping.key), falling back to treating the value as a literal category only when no key matches. So passing category.id = "drug_indications" from the frontend resolves correctly to the multivalued mapping — the PR description's note that "get_mapping on the bare MEDIC category still resolves to drug_indications" is actually a moot backup path; the primary lookup is already key-based. Good.
• association_counts bucketing (parse_association_counts) also keys off mapping.key, so MEDIC + CTD counts aggregate correctly under one drug_indications bucket — consistent with your verified 6,619 + 19,139 = 25,758 total.
• The new predicate-filter plumbing in AssociationsTable.vue/SectionAssociations.vue closely mirrors the existing taxon-filter pattern (same "all selected ⇒ no filter" semantics, same "freeze options while a filter is active" guard). Good consistency — this made it easy to verify there's no behavioral asymmetry between the two filters.
• frontend/src/util/typeConfig.ts#fieldFor and sectionTitles.ts both have safe fallbacks for the new non-biolink:-prefixed "drug_indications" key, so nothing breaks there. One minor, pre-existing wrinkle worth being aware of (not introduced by this PR): fieldFor falls back to "subject" for unmapped ids, which drives AssociationsTable.vue's inferredSubclassLabel/isSubclassRow logic. Since drug_indications is genuinely bidirectional (chemical=subject on drug pages, disease/phenotype=object on disease pages), the "all" tab's inferred-subclass tooltip may be subject-biased when viewed from a disease/phenotype page. Cosmetic (tooltip text only), likely low priority, but worth a follow-up if it's noticeable in practice.

Code quality / style

• retrievalSources.ts and predicateFilterConfig.ts are small, well-documented, single-purpose utils consistent with the codebase's existing util/ conventions (e.g. taxonFilterConfig.ts).
• parseRetrievalSources correctly guards against malformed JSON per-entry (try/catch, skip) rather than failing the whole parse — good defensive parsing for external/API-shaped data.
• The YAML comment on the new drug_indications mapping entry is clear about why two categories are merged, which will help future maintainers.

Test coverage

• Good unit coverage for the new pure-function utils (retrievalSources.test.ts, predicateFilterConfig.test.ts), including the "malformed entry doesn't throw" and "no filterable sections other than drug_indications" edge cases.
• The useAssociationCategories.test.ts update correctly reflects the behavior change (drug-indications no longer hidden, now keyed on key).
• No new coverage for the modal-enrichment template logic itself (qualifierRows, hasFrequencyData, formatRole) — these are simple computed properties so risk is low, but a couple of component-level assertions (e.g. "qualifiers render in order," "frequency block hidden when all four fields are null") would cheaply lock in the modal's behavior if this file grows in scope later. Not blocking given the rest of the codebase's testing depth for similar detail-view components.

Performance / security

• No concerns. Filter queries are still parameterized through Solr fq clauses using the existing quoting pattern (field:"value"), consistent with how taxon filters already build fq; no new injection surface introduced (values come from facet-derived option lists, not raw user text).
• Facet computation (facetFields) now conditionally includes "predicate" only for the one filterable section, so this doesn't add facet overhead to other sections.

Minor nit

• HIDDEN_CATEGORIES in use-association-categories.ts is now an empty Set<string>([]) — since it's currently unused, consider either removing the plumbing entirely or leaving a short note that it's intentionally kept as an extension point for future hidden sections (right now a reader might wonder if this is dead code).

Overall: solid, well-tested, and the multivalued-category enabler is used correctly end-to-end. Once #1355 merges and this is retargeted to main, looks good to me.

Review

Reviewed the diff (base is feat/flexible-association-section-matching per the PR description, so this is scoped to the MEDIC/CTD work + details-modal enrichment). Overall this is a clean, well-tested change — nice use of small, dedicated, unit-tested util/ modules (predicateFilterConfig.ts, retrievalSources.ts) instead of burying logic in the SFCs.

What it does

• Merges MEDIC (biolink:treats) and CTD (biolink:treats_or_applied_or_studied_to_treat) into a single drug_indications association section via the new multivalued category/subject_category/object_category mapping criteria.
• Adds a predicate filter/toggle so the section defaults to the strong "Indications" predicate and lets users opt into the weaker "investigational" CTD relationship.
• Enriches the association-details modal with knowledge level, nested retrieval-source provenance, aggregator source, original predicate, qualifiers, frequency data, FDA adverse-event level, and negation.

Code quality

• predicateFilterConfig.ts and retrievalSources.ts are small, well-documented, and independently testable — good separation from the Vue components, matching the existing pattern used for taxonFilterConfig.ts.
• parseRetrievalSources defensively handles both JSON-encoded string entries and already-parsed objects, and swallows malformed entries without throwing — solid.
• The mapping YAML comment explaining why MEDIC + CTD are merged (different categories, multivalued criterion) is genuinely useful context that isn't obvious from the code.
• Removing the old "Medical Action" 1-hop-qualifier special case in favor of a direct subject_category/object_category match looks like a real fix, not just a rename — worth a sentence in the PR description confirming the old special-casing is fully superseded (verification section mentions counts match but not this specific behavior).

Potential issues

• Double round-trip on first load for drug_indications: selectedPredicateIds() in SectionAssociations.vue intentionally returns [] until predicate facet options arrive, then recomputes and (if the default predicate is present alongside the weaker one) triggers a second, filtered fetch via the predicateFilters watcher in AssociationsTable.vue. That's a deliberate "learn facets, then filter" pattern mirroring the taxon-filter flow, but it does mean every drug/disease page pays for two fetches of this section instead of one. Worth confirming this is an acceptable tradeoff (it likely is, given facet responses are cheap, but flagging since it's not mentioned in the PR description).
• Solr query string interpolation: predicateFilters.map((p) => \predicate:"${p}"`)inAssociationsTable.vuebuilds thefq` clause via raw interpolation, same as the pre-existing taxon-filter code. Values currently only come from controlled facet labels (predicate IRIs), so risk is low, but it's the same pattern that would need escaping if a filter value ever originated from free-text/user input.
• Duplicated "default + other present" logic: showInvestigationalToggle and selectedPredicateIds in SectionAssociations.vue independently re-derive "is the default predicate present AND is another predicate present" — a small shared helper (e.g. hasBothPredicates(categoryId)) would remove the duplication and reduce the chance the two drift out of sync later.
• Generic "Qualifier" label reused per entry: in SectionAssociationDetails.vue, qualifierRows pushes a row labeled literally "Qualifier" for each item in a.qualifiers, so an association with multiple raw qualifiers renders several rows all labeled "Qualifier" with no way to tell them apart. Minor UX nit.

Test coverage

• Good coverage for the new pure-logic utils (predicateFilterConfig.test.ts, retrievalSources.test.ts) and the updated useAssociationCategories.test.ts.
• Gap: the new computeds added directly to SectionAssociationDetails.vue (qualifierRows, hasFrequencyData, formatRole) have no tests, unlike the sibling logic that was extracted into util/. Given the project's pattern of extracting testable logic out of SFCs, consider moving at least qualifierRows/hasFrequencyData into a util for consistency and coverage.
• Gap: no test exercises the two-phase predicate-filter integration between SectionAssociations.vue and AssociationsTable.vue (selectedPredicateIds, showInvestigationalToggle) — this is the most stateful new logic in the PR and currently relies only on the isolated predicateFilterConfig.ts unit tests plus manual verification.

Backend

• The YAML mapping change and fixture regeneration look correct and internally consistent (confirmed the facet query in the regenerated fixtures correctly OR's both categories and both object categories together, AND'd with the shared subject category).
• Nice touch adding medic-ingest to docs/sources-manifest.yaml for documentation coverage.

Nothing blocking — the flagged items are mostly polish/consistency suggestions. Since this is a draft stacked on #1355, worth a final pass once retargeted to main to make sure the diff is still exactly what's described.

Review

Reviewed the diff (this branch only — the multivalued category/subject_category/object_category and key matching machinery it builds on comes from #1355, already verified in association_type_utils.py). Overall this is clean, well-scoped, and the new pure-function utilities (predicateFilterConfig.ts, retrievalSources.ts) are genuinely well tested — better unit coverage than the pre-existing taxonFilterConfig.ts this mirrors, which has none.

Correctness

• association_type_utils.py::get_mapping matches by category in mapping.category, so the bare MEDIC category correctly resolves to the new drug_indications mapping — confirms the PR description's claim.
• use-association-categories.ts now keys sections on ac.key || ac.category, and AssociationsTable.vue/SectionAssociations.vue consistently thread category.id through as that key (predicate filterability check, facet fields, query calls) — no stale references to the old bare-category keying found anywhere in frontend/src or backend/src.
• predicateFilterState's "unfiltered first fetch, then scope to default predicate once both are known to exist" flow is subtle but correct: emitPredicateOptions only recomputes options while predicateFilters is empty, which avoids an oscillation where applying the filter changes the facet counts, which would otherwise re-toggle showToggle. Nice touch, and the accompanying test file exercises the key state transitions (both present, only default, only weaker, no options yet).
• parseRetrievalSources degrades gracefully (bad JSON → treated as bare CURIE, empty string / null → dropped), matches the fixture-based tests, and the tests cover the actual MEDIC nested-chain shape.

Minor observations (non-blocking)

1. SectionAssociationDetails.vue qualifier rows (qualifierRows computed): every entry from a.qualifiers (the generic array) is labeled just "Qualifier", so if an association has more than one, the details modal shows several identical "Qualifier:" labels with no way to tell them apart. Probably fine for now since most associations will have 0–1, but worth a follow-up if MEDIC/CTD data turns out to carry multiple.
2. Toggle/facet state persists across node navigation (includeInvestigationalByCategory, predicateOptionsByCategory in SectionAssociations.vue): since rows are keyed by category.id (e.g. "drug_indications") rather than by node, navigating from one node's page to another that also has a drug_indications section could reuse stale toggle/option state instead of resetting. This mirrors the pre-existing taxonOptionsByCategory/selectedTaxonsByCategory pattern exactly, so it's not a new issue introduced by this PR, just calling it out in case it's worth fixing for both at once.
3. retrievalSources.ts toRecord: an object with no recognizable fields (e.g. {}) currently produces a RetrievalSource with everything undefined, and the template would render an empty "Source:" row (formatRole falls back to "Source" when resource_role is falsy). Unlikely given real API payloads, but a quick resource_id presence check in toRecord would make the "never silently drop real provenance" comment's intent airtight without any behavior change for the actual data.

Test coverage

Good — new pure logic (predicateFilterConfig, retrievalSources) is unit tested with meaningful edge cases, and the useAssociationCategories test was updated to reflect the new key-based behavior rather than just widened. No component-level test for the AssociationsTable/SectionAssociations wiring itself, but that matches existing convention in this codebase (no tests for the analogous taxon-filter wiring either).

Security / performance

Nothing of concern — this is UI display logic and a Solr query mapping change; no new user input reaches string interpolation into a query in an unsafe way beyond the existing patterns (values are still quoted the same way _or_group already did for other multivalued criteria).

Nice work, especially the provenance-chain parsing and the thoughtful predicate-toggle UX to avoid an empty table for nodes that only carry the weaker CTD predicate.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.53%. Comparing base (3ce8dc4) to head (f0ee7ae).

@@           Coverage Diff           @@
##             main    #1357   +/-   ##
=======================================
  Coverage   83.53%   83.53%           
=======================================
  Files         126      126           
  Lines        6822     6822           
=======================================
  Hits         5699     5699           
  Misses       1123     1123           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

✅ Deploy Preview for monarch-app ready!

Name	Link
🔨 Latest commit	f0ee7ae
🔍 Latest deploy log	https://app.netlify.com/projects/monarch-app/deploys/6aa0889358dea20008e0f571
😎 Deploy Preview	https://deploy-preview-1357--monarch-app.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.