Skip to content

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

Draft
kevinschaper wants to merge 12 commits into
feat/flexible-association-section-matchingfrom
feat/medic-ctd-drug-indications
Draft

feat(associations): MEDIC + CTD drug-indications section + details modal (#1348)#1357
kevinschaper wants to merge 12 commits into
feat/flexible-association-section-matchingfrom
feat/medic-ctd-drug-indications

Conversation

@kevinschaper

Copy link
Copy Markdown
Member

Draft — stacked on #1355 (flexible section-matching enabler); base is that branch so the diff is just the MEDIC/CTD work. Retarget to main once #1355 merges. Supersedes #1352 (the earlier single-category approach).

What

One "Drug Indications / Treatments" section merging two sources that carry different association categories, via the enabler's multivalued category criterion — no ingest change needed:

  • MEDIC — biolink:treats (FDA-approved, text-mined) — biolink:ChemicalOrDrugOrTreatmentToDiseaseOrPhenotypicFeatureAssociation
  • CTD — biolink:treats_or_applied_or_studied_to_treat (curated) — biolink:ChemicalEntityToDiseaseOrPhenotypicFeatureAssociation

Mapping key: drug_indications, category: [medic, ctd], subject_category: [ChemicalEntity], object_category: [Disease, PhenotypicFeature]. Shown as "Indications" on a drug/chemical page and "Treatments" on a disease/phenotype page.

Also includes the association-details modal enrichment: knowledge level, the nested retrieval-source provenance chain (MEDIC's infores:medic ← infores:dailymed/FDA), aggregator source, original predicate, qualifiers, frequency data, FDA adverse-event level, negated (parseRetrievalSources util + test).

Verification

  • Merged filter verified vs api-beta: medic 6,619 + ctd 19,139 = 25,758 (exact).
  • Mapping loads with the right key/labels/fragment; get_mapping on the bare MEDIC category still resolves to drug_indications so the table endpoint works either way; counts fixtures regenerated; backend suite green (134). parseRetrievalSources unit-tested; frontend typecheck/lint clean.

https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL

…1348)

Enrich the association details modal (SectionAssociationDetails.vue) to surface
the fields it previously hid: knowledge level, the nested retrieval-source
provenance chain (e.g. MEDIC's infores:medic <- infores:dailymed/FDA),
aggregator knowledge source, original predicate, qualifiers, frequency data,
FDA adverse event level, and negated. Extract the sources JSON parsing into a
tested util (parseRetrievalSources).

Claude-Session: https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL
Replace the hidden single-category "Medical Action" mapping with a merged
"drug_indications" section built on the flexible enabler: it combines MEDIC
(biolink:treats) and CTD (biolink:treats_or_applied_or_studied_to_treat), which
carry different association categories, via the multivalued category criterion
(no ingest change needed). Shown as "Indications" on a drug/chemical page and
"Treatments" on a disease/phenotype page.

Merged filter verified vs api-beta (medic 6619 + ctd 19139 = 25758). Un-hid the
section (cleared the now-stale HIDDEN_CATEGORIES entry, since sections key on
`key` not category). Fixtures regenerated. The modal field enrichment rides
along (cherry-picked).

Claude-Session: https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL
@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown

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.

Register monarch-initiative/medic-ingest so `make fetch-docs` generates the
/Sources/medic/ page, fixing the association "Provided By" link for MEDIC edges
(.../Sources/medic/), which 404s without a manifest entry.

Claude-Session: https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL
This PR clears HIDDEN_CATEGORIES to un-hide the drug-indications section, so the
"filters out hidden categories" test (which asserted that category is hidden)
no longer holds. Update it to assert the category now renders, keyed on `key`.

Claude-Session: https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL
@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

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 (maxomedicmmrrc), 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

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

The merged drug-indications section mixes biolink:treats (MEDIC/FDA) and
biolink:treats_or_applied_or_studied_to_treat (CTD). Add a per-section
"Filter by relationship" dropdown that scopes the table by predicate, mirroring
the existing taxon filter: the table facets on predicate, emits the options, and
selected predicates become filter_queries. Gated by predicateFilterConfig
(currently just drug_indications) and shown only when >1 predicate is present.

Verified: /entity/MONDO:0005148/drug_indications = 97, filtered to treats = 46,
to treats_or_applied_or_studied_to_treat = 51.

Claude-Session: https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL
@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

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.

Add a watcher on predicateFilters (mirroring the taxonFilters watcher) so
selecting a predicate in the drug-indications "Filter by relationship" dropdown
re-runs the association query.

Claude-Session: https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL
@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

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.vuegetAssociations(..., category.id, ...)GET /entity/{id}/{category}_association_tablesolr_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.

…tional toggle

The merged drug-indications section defaults to the strong biolink:treats
(labeled "Indication") predicate and offers a single "Include investigational
(studied/applied) treatments" checkbox to expand to CTD's weaker
biolink:treats_or_applied_or_studied_to_treat (labeled "Investigational").

Replaces the predicate multi-select. The first, unfiltered fetch reveals whether
the weaker predicate is present; the toggle then appears only when both are, and
the table scopes to indications until the user opts in. Gracefully handles nodes
with only one of the two predicates (no toggle, no empty table).

Claude-Session: https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL
@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

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.

- Extract the two-phase toggle/scoping logic into a pure, unit-tested
  predicateFilterState() helper (empty-before-facet, only-default, only-weaker,
  both checked/unchecked, non-filterable) instead of inline logic in the section.
- Drop the dead PREDICATE_LABELS/formatPredicate ("Indication"/"Investigational"
  never reached the table rows, which render the raw predicate badge); emit the
  raw predicate as the facet option label.
- parseRetrievalSources now falls back to a bare resource_id for plain-CURIE
  source entries instead of silently dropping non-JSON provenance.

Claude-Session: https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL
@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

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.

…tions table

On a disease page the drug-indications/treatments table (Direct tab) fell into
the generic branch that drops subject + predicate, leaving only the object —
which for this section is the disease itself, so every row just restated the
page node. Treat drug_indications like the other disease-is-object sections:
drop the object (disease) and keep the chemical/treatment (subject) + predicate.

Claude-Session: https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL
…nferred sets

The filter watchers refetched only the active dataset, so the inferred/all
dataset kept its mount-time unfiltered result. Switching to the inferred
treatments tab then showed all predicates rather than matching the indications
checkbox (and likewise for the taxon filter). Refetch both datasets on filter
change so either tab stays consistent with the current filter.

Claude-Session: https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL
…d inferred sets

The predicate options that gate the "include investigational" checkbox were
emitted from only the active dataset's facet. On fresh load the default tab is
Direct, whose facet has only biolink:treats — the weaker CTD predicate appears
only in the inferred/all set — so the toggle stayed hidden until the user
visited the inferred tab. Emit predicate options from the union of both
datasets' predicate facets so the toggle reflects the whole section regardless
of the active tab or load order.

Claude-Session: https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL
…ilter

Including investigational (CTD) edges changes the direct count, which flipped
the default direct/inferred tab and jerked the user's view over to Direct. Pin
the currently-active tab into selectedTabs before applying the filter change
(only when the user hasn't already chosen a tab), so toggling the checkbox
filters the rows in place without restructuring which tab is active.

Claude-Session: https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant