feat(predicate): predicate definition + partial hierarchy explainer (#1358)

[kevinschaper]

Draft — first cut of #1358 (predicate explainers).

What

Predicates now carry an unobtrusive info icon (AppPredicateBadge) that opens a modal explaining the relationship:

• its biolink definition,
• a partial is_a hierarchy (most-specific → general), and
• domain / range / inverse when present.

How

• Reuses the existing useBiolinkModel (loads biolink-model.yaml, cached 24h in localStorage) and its getPredicateInfo.
• Adds getPredicateAncestors(predicate, depth) that walks is_a.
• New AppPredicateInfo.vue — the icon + modal; the biolink model is loaded lazily on first open (not per badge), so there's no cost until a user asks. Degrades gracefully when a predicate isn't in the model.

Tests

getPredicateAncestors unit-tested (chain walk, depth limit, unknown predicate) by injecting a fake model via the localStorage cache. vue-tsc + eslint clean.

Open UX questions (for review)

• Placement/density: the icon currently appears on every predicate badge, incl. every association-table row. That may be too dense — options: show only on hover, only in the details modal + section headers, or a shared single modal instead of one per badge.
• Tooltip vs modal: could add a hover tooltip with just the definition (cheap) and reserve the modal for the fuller hierarchy/mappings.
• Hierarchy depth (currently 3) and whether to link each ancestor.
• Could also surface mappings (exact/narrow/broad) which getPredicateInfo already returns.

Motivating case: the drug-indications section's treats vs treats_or_applied_or_studied_to_treat, and LOINC's correlated_with vs related_to.

https://claude.ai/code/session_018c9UddtKsKtm35YNrxHWuL

[netlify]

✅ Deploy Preview for monarch-app ready!

Name	Link
🔨 Latest commit	d1fe74c
🔍 Latest deploy log	https://app.netlify.com/projects/monarch-app/deploys/6a554d1c12579b000861aa1d
😎 Deploy Preview	https://deploy-preview-1359--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.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.34%. Comparing base (f28d52c) to head (d1fe74c).
⚠️ Report is 36 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1359      +/-   ##
==========================================
+ Coverage   83.17%   83.34%   +0.16%     
==========================================
  Files         126      126              
  Lines        6617     6648      +31     
==========================================
+ Hits         5504     5541      +37     
+ Misses       1113     1107       -6     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Review: predicate definition + partial hierarchy explainer (#1358)

Nice, self-contained first cut — reuses the existing useBiolinkModel composable well and keeps the new UI lazy. A few things worth addressing before this comes out of draft.

Overview

Adds AppPredicateInfo.vue (info icon + modal showing a predicate's definition, is_a hierarchy, and domain/range/inverse), wires it into every AppPredicateBadge, and adds getPredicateAncestors() to use-biolink-model.ts to walk the is_a chain. Includes unit tests for the new ancestor-walking logic.

Correctness

• Chain display order vs. PR description: AppPredicateInfo.vue's chain computed reverses ancestors and appends the current predicate last, so the rendered list reads general → specific (top → bottom, increasing indent), with the current predicate at the bottom. The PR description says the hierarchy is "most-specific → general." Worth double-checking this is the intended UX — as written it's the opposite of what's described (though the visual general-at-top / specific-at-bottom layout with increasing indent reads fine on its own).
• { name: props.predicate } as PredicateInfo (AppPredicateInfo.vue) — the as cast is unnecessary; PredicateInfo only requires name, everything else is optional, so the object literal already satisfies the type.
• getPredicateInfo(props.predicate) is called both directly in onOpen() and again inside getPredicateAncestors(). Harmless (cheap object lookups) but slightly redundant — getPredicateAncestors could accept the already-resolved PredicateInfo to avoid the double lookup.
• Minor duplication: AppPredicateInfo.vue's local format() and AppPredicateBadge.vue's getFormattedPredicateLabel() do almost the same "strip biolink: prefix, replace _ with space" transform. Could be a shared util if it comes up again.

Performance

• useBiolinkModel() creates a fresh model/isLoading ref per call site rather than sharing state via a module-level singleton. Since AppPredicateInfo is now instantiated once per AppPredicateBadge (i.e. potentially once per row in association tables), every distinct badge's first click re-parses the full cached biolink model JSON out of localStorage independently, even though another badge on the same page already parsed it moments earlier. Given biolink-model.yaml is not small, this repeated JSON.parse per-instance could add up if a user opens several explainers on a dense page. Hoisting the loaded model to module scope (shared across all useBiolinkModel() callers) would let it be parsed once per session and reused everywhere, which fits the PR's own "no cost until asked" goal even better.
• The density question you raised in the PR description (icon on every predicate badge, including every table row) is the right thing to resolve before merging — combined with the point above, hover-only or a shared single modal would reduce both visual noise and redundant parsing.

Test coverage

• getPredicateAncestors itself is well tested (chain walk, depth limit, unknown predicate).
• There's no test for AppPredicateInfo.vue itself — e.g. the chain ordering, hasMeta computed, or the "no definition found" fallback path. Given most other components in frontend/unit have a matching ComponentName.test.ts, a small mount-based test (vue-test-utils) covering open/close and the fallback state would be a good addition, especially since the chain-ordering behavior above is subtle enough to regress silently.
• No test exercises a predicate whose is_a chain has a duplicate/self-referential entry — low priority (would require a malformed model), but since ancestors are rendered v-for keyed by item.name without dedup, a repeated name would produce a Vue duplicate-key warning. Not worth a full test, just flagging.

Security

No concerns — fetch target and YAML parsing are pre-existing, and the new template interpolates via {{ }} (auto-escaped), not v-html.

Style/conventions

Matches the codebase well: AppButton/AppModal/AppStatus/AppIcon composition, scoped SCSS, JSDoc-style comments on props and functions. circle-info and angle-right icons are already registered, AppStatus codes (loading/warning) are valid.

Overall solid groundwork — the main things I'd want resolved before merge are the density/placement question (already flagged by you) and the per-instance model-loading cost, since they're related: fewer/shared modal instances would mitigate both.

[claude]

Review: predicate definition + hierarchy explainer

Nice feature — reusing useBiolinkModel/getPredicateInfo, lazy-loading on first open, and graceful degradation when a predicate isn't in the model are all solid choices. The getPredicateAncestors unit tests cover the chain-walk/depth-limit/unknown-predicate cases well. A few things worth addressing before this comes out of draft:

Potential bug: fetch errors are indistinguishable from "not found"

AppPredicateInfo.vue and SectionAssociationDetails.vue both destructure only isLoading/loadBiolinkModel/getPredicateInfo from useBiolinkModel() and never look at error. If the fetch of biolink-model.yaml fails (network blip, CORS, W3ID outage), info.value stays null and the UI falls through to:

<AppStatus v-else code="warning">
  No definition found for this relationship in the biolink model.
</AppStatus>

— which reads the same as a predicate that's legitimately absent from the model. AppStatus already has a purpose-built code="error" variant (tooltip "See dev console for more info"), and the sibling PredicateDetailPanel.vue in this repo already branches on a biolinkError state for this exact case. Worth mirroring that pattern here so real failures are visible/debuggable rather than silently misreported.

Performance: no de-duplication across concurrent loads

useBiolinkModel() creates fresh isLoading/model/error refs on every call — there's no module-level singleton or in-flight-promise cache, only the 24h localStorage cache once a load completes. This PR adds a new call site per predicate badge (potentially one per row in a large association table) plus one in SectionAssociationDetails.vue. Before the localStorage cache is warm, if a user opens the info popover on two different badges (or an association-details page loads while a badge is also opened) in quick succession, each triggers its own independent network fetch of the same (fairly large) YAML file. Consider hoisting a shared in-flight promise so concurrent loadBiolinkModel() calls coalesce into a single request.

Duplicated formatting/URL logic

The "strip biolink: prefix, replace _ with space" logic is implemented three times nearly identically:

• AppPredicateBadge.vue's getFormattedPredicateLabel
• AppPredicateInfo.vue's format

...and the Biolink docs URL builder (https://biolink.github.io/biolink-model/${predicate.replace(/^biolink:/, "")}/) is duplicated verbatim in both AppPredicateInfo.vue and SectionAssociationDetails.vue. Since frontend/src/util/ already exists for this kind of thing, pulling both into a shared helper (or onto use-biolink-model.ts, since it's already the canonical place for predicate-name handling) would remove the duplication and the risk of the two copies drifting apart.

Minor

• In AppPredicateInfo.vue's chain computed, { name: props.predicate } as PredicateInfo — the as cast is unnecessary since PredicateInfo.name is the only required field; the object literal already satisfies the type.
• There's a subtle asymmetry baked into chain: ancestor names come back from getPredicateInfo already stripped of the biolink: prefix and space-separated (matching the YAML's own is_a key), while the current predicate's name is the raw, still-prefixed prop. It happens to work because format() normalizes both at render time, but it's a subtle enough invariant that a future refactor could break it silently — worth a comment or a lightweight component test pinning the rendered order/labels.

Test coverage

Only the pure getPredicateAncestors function is tested. There's no coverage of AppPredicateInfo.vue's template logic (hierarchy chain assembly/labels, docs URL, loading/error/not-found branches) or the new "Relationship" block in SectionAssociationDetails.vue. Given the name-formatting subtlety above, at least one shallow-mount test for AppPredicateInfo.vue would help guard against regressions.

Accessibility / density (already flagged in the PR description)

Agreed this needs to be resolved before merging: putting an AppButton (with its own tab stop and an aria-label="Explain X") on every predicate badge, including every row of large association tables, adds a lot of repetitive keyboard/screen-reader noise with no per-row disambiguation. Of the options listed in the description, "icon on hover only" or a single shared modal (driven by predicate value rather than one modal instance per badge) both seem preferable to what's here now for anything rendered in a table.

Overall this is a well-scoped first cut with a sensible lazy-loading design — the error-swallowing and duplicated-logic points are the two I'd prioritize fixing before merge; the rest are smaller nits.

[claude]

Review: Predicate definition + hierarchy explainer

Overview

Nice, well-scoped first cut. AppPredicateInfo.vue adds an info-icon + modal that lazily loads the (already-cached) biolink model and shows a predicate's definition, partial is_a hierarchy, and inverse. The use-biolink-model.ts refactor from per-instance composable state to a module-level singleton (shared model/isLoading/error + a single in-flight loadPromise) is a genuine improvement — it avoids re-fetching/re-parsing the biolink model per badge instance, and correctly de-dupes concurrent callers. getPredicateAncestors is simple and well unit-tested (chain walk, depth limit, unknown predicate).

Bugs / correctness

• Misleading empty state on real fetch failures. useBiolinkModel() exposes error, and it's already surfaced elsewhere in the codebase (PredicateDetailPanel.vue's biolinkError), but neither AppPredicateInfo.vue nor SectionAssociationDetails.vue reads it. If loadBiolinkModel() fails (network hiccup, CORS, etc.), info/predicateInfo just stay null, and the modal renders "No definition found for this relationship in the biolink model" — which reads as "this predicate isn't modeled" rather than "the fetch failed." Worth branching on error to show a distinct message (and maybe a retry affordance), especially since loadPromise is nulled out on failure specifically to allow a retry.
• Unverified docs URL. https://biolink.github.io/biolink-model/${predicate}/ is a new URL pattern (not used anywhere else in the codebase) and appears twice, hand-built by string concatenation. Worth double-checking against the real biolink-model docs site (mkdocs-generated sites often use different slug casing, e.g. related_to.md vs related-to/) before merging, so the "Biolink Model" attribution link isn't a dead link in the modal and the association-details section.

Code quality

• Duplicated URL/formatting logic. The docsUrl computed (.replace(/^biolink:/, "") + template literal) and the biolink:-stripping label formatter are each re-implemented independently in AppPredicateInfo.vue and SectionAssociationDetails.vue. Since use-biolink-model.ts already owns predicate-name normalization (getPredicateInfo's cleanName logic), consider adding a shared getPredicateDocsUrl/formatPredicateLabel helper there (or a small util) so both consumers use one implementation.
• { name: props.predicate } as PredicateInfo in AppPredicateInfo.vue's chain computed — the as cast is unneeded since name is PredicateInfo's only required field; the object literal already satisfies the type.
• predicateString in AppPredicateBadge.vue re-does the array-vs-string handling that the predicate computed already partially performs (for the highlighting case). Not wrong, but worth a one-line comment on why association.predicate itself can still be an array even after the highlighting fallback, so future readers don't "simplify" it into a bug.
• Good defensive touches: graceful degradation when a predicate isn't in the model, steps < depth guard against unbounded/cyclic is_a walks, and the CSS fix keeping the info icon glued to the wrapping predicate label (with a comment explaining the whitespace-sensitive template markup) — a bit fragile but well-documented.

Performance

• Lazy-load-on-first-open (rather than per-badge) is the right call given the biolink model is fetched/parsed once and reused — matches what the PR description promises. The shared singleton means even multiple badges on an association table now only trigger one fetch/parse for the whole session, an improvement over what a naive per-badge implementation would have done.
• One nuance worth a sanity check: PredicateDetailPanel.vue (source dashboard) already consumes useBiolinkModel(); before this PR each call site got independent local state, now all consumers share one global isLoading/error/model. That's almost certainly desirable, but it's a behavior change not mentioned in the PR description — worth confirming the dashboard panel's loading/error UI still behaves as expected when another part of the app (e.g. a predicate badge) triggers the shared load.

Security

No concerns. No new v-html usage (the existing one in AppPredicateBadge.vue predates this PR), URLs are built from a fixed domain + sanitized predicate string, and the biolink model is fetched from the existing hardcoded w3id.org source.

Test coverage

• getPredicateAncestors is nicely unit-tested (chain walk, depth cutoff, unknown predicate), and injecting the fake model via the localStorage cache is a clean way to avoid mocking fetch.
• No coverage yet for AppPredicateInfo.vue itself (loading / found / not-found / error branches) or the new "Relationship" block in SectionAssociationDetails.vue. Given there are several conditional render branches (including the error-state gap noted above), even a lightweight component test would help lock in behavior before the open UX questions are resolved.

Notes on open questions in the PR description

The placement/density and tooltip-vs-modal questions you raised seem like the right things to resolve before taking this out of draft — agree that an icon on every association-table row could get visually noisy; a hover tooltip for the definition with the modal reserved for the fuller hierarchy seems like a reasonable middle ground given the existing v-tooltip usage pattern elsewhere in AppButton.

[claude]

Review: Predicate definition + hierarchy explainer (#1358)

Nice feature — the modal design (definition + partial is_a hierarchy + inverse) directly addresses real ambiguity like treats vs treats_or_applied_or_studied_to_treat. The useBiolinkModel refactor to a module-level singleton (shared model/isLoading state + in-flight loadPromise dedup) is a solid improvement over the old per-instance state, and the lazy-on-open loading in AppPredicateInfo.vue is the right call for something rendered on every badge.

Bugs / correctness

• Stale-write race in SectionAssociationDetails.vue (frontend/src/pages/node/SectionAssociationDetails.vue:187-196): the watch(predicateValue, async (predicate) => {...}, { immediate: true }) callback awaits loadBiolinkModel() and then unconditionally assigns predicateInfo.value = getPredicateInfo(predicate). If the user switches to a different association before that await resolves (most likely on the very first load, while the full biolink-model.yaml is being fetched/parsed), a slower callback can overwrite a faster one and show the wrong relationship's definition. Guard with something like if (predicate !== predicateValue.value) return; after the await.

• Fetch errors are swallowed in AppPredicateInfo.vue: onOpen() calls loadBiolinkModel(), and error is available from the composable, but it's never destructured/used. On a network failure the modal falls through to info.value === null and shows "No definition found for this relationship in the biolink model" — which reads as "this predicate isn't documented" rather than "the fetch failed." The existing frontend/src/components/dashboard/PredicateDetailPanel.vue (same composable) does surface biolinkError distinctly from "no info" — worth mirroring that here so failures aren't mistaken for missing metadata.

• getPredicateChildren doesn't check aliases the way getPredicateInfo does (frontend/src/composables/use-biolink-model.ts:190-205): it only matches a child's is_a against the exact/space-underscore variants of the parent name, not against any of the parent's aliases. If a slot's is_a refers to a predicate via an alias rather than its canonical name, that child would silently be missed. Probably rare given the model's data, but worth matching the alias-checking behavior of getPredicateInfo for consistency.

Design / consistency questions

• Eager load vs. "no cost until asked": the PR description frames the biolink model load as lazy/on-demand (good, and true for AppPredicateInfo.vue's per-badge modal), but SectionAssociationDetails.vue calls loadBiolinkModel() immediately via the immediate: true watcher, so simply opening the association-details panel for any association triggers the full model fetch/parse (once per 24h cache window) even if the user never looks at the "Relationship" block. May be an intentional tradeoff since the definition is shown inline rather than behind a click — just flagging the inconsistency with the stated lazy-loading goal.

• External docs URL is unverified: docsFor() builds links as https://biolink.github.io/biolink-model/${name}/ in both AppPredicateInfo.vue and SectionAssociationDetails.vue. Worth confirming this matches the actual current biolink-model docs site structure/slug format before merging — a confidently-rendered wrong link is worse than no link.

• Reuse opportunity: frontend/src/components/dashboard/PredicateDetailPanel.vue already renders a biolink definition (+ domain/range/mappings) from the same useBiolinkModel composable, with its own "No description available" / "No biolink definition found" copy. This PR adds a second, independent rendering of largely the same data (definition text, docs link) for the node page. Not a blocker, but a shared formatting helper (or the same info component reused in both places) would avoid two copies of "what does an empty/missing predicate description look like" drifting apart over time.

Test coverage

• The new getPredicateAncestors/getPredicateChildren unit tests are clear and cover the useful edge cases (chain walk, depth limit, unknown predicate, leaf node).
• No test coverage for AppPredicateInfo.vue itself (modal open/lazy-load behavior, loading/error/empty states) or for the new "Relationship" block wired into SectionAssociationDetails.vue. Since this is genuinely new UI logic (not just the pure-function hierarchy walk), a couple of component-level smoke tests would help catch regressions in the loading/error/empty branches noted above.

Minor

• AppPredicateBadge.vue's CSS split (.predicate-label gets overflow-wrap: anywhere, .predicate-info-icon gets white-space: nowrap) with the explanatory comment is a nice touch for keeping the icon from orphaning onto its own line in narrow table columns.
• No security concerns — the new definition text is rendered via {{ }} interpolation (not v-html), and external links go through the existing AppLink component.

Overall a solid first cut with clean architecture; the race condition and swallowed-error cases above are what I'd actually fix before taking this out of draft — the rest are worth a look but not blocking.

[claude]

Review: predicate definition + partial hierarchy explainer

Nice feature — reusing the existing useBiolinkModel cache and lazily loading it only on first modal open keeps the cost near-zero for users who never click the icon. The refactor of the composable to module-level singleton state (shared model/isLoading refs + a shared loadPromise) is a good change independent of the feature itself: it avoids re-fetching/re-parsing the large YAML once per component instance, and de-dupes concurrent in-flight fetches.

Potential bugs

• Race condition in SectionAssociationDetails.vue's watcher (frontend/src/pages/node/SectionAssociationDetails.vue:185-194): the watch(predicateValue, async (predicate) => {...}) doesn't guard against out-of-order resolution. If a user switches between associations quickly while the biolink model is still loading (first load only, since it's now a shared singleton promise), an earlier-started lookup can resolve after a later one and overwrite predicateInfo.value with stale data for a predicate that's no longer selected. Worth capturing a request token/guard (e.g. compare against predicateValue.value before assigning) to avoid showing a mismatched definition.

• Duplicated, inconsistently-correct docs-URL construction: AppPredicateInfo.vue's docsFor() (line 134-137) replaces spaces with underscores before building the biolink docs URL (needed because is_a/mixin parent names come from the YAML as space-separated strings, e.g. "treats or applied or studied to treat"), but SectionAssociationDetails.vue's predicateDocsUrl (line 177-183) does not do this replacement. It happens to work today because association.predicate is already underscore-formatted, but the two nearly-identical URL builders can silently drift out of sync. Consider hoisting a single predicateDocsUrl(name) helper into use-biolink-model.ts and using it from both call sites.

• Stale cached state if predicate prop changes on a mounted AppPredicateInfo (AppPredicateInfo.vue:152-159): onOpen() short-circuits with if (info.value) return;, so if the same component instance were ever reused for a different predicate (e.g. via highlighting.predicate toggling in AppPredicateBadge), the modal would keep showing the previously-loaded definition/hierarchy. Low risk given current usage (predicate is effectively static per association row), but there's no watch(() => props.predicate, ...) reset, so it's a latent trap if that assumption changes.

Code quality / conventions

• Test file naming: the repo's other composable tests follow the use-x.test.ts pattern (useSourceVersions.test.ts, use-kg-data.test.ts, use-sql-query.test.ts), but this PR adds frontend/unit/biolinkModelPredicates.test.ts for use-biolink-model.ts. Renaming to use-biolink-model.test.ts would match convention and make it easier to find.
• getPredicateChildren does a full Object.entries(model.value.slots) linear scan on every modal open — fine at biolink-model's current predicate count, but if this pattern gets reused elsewhere (e.g. rendering multiple explainers open at once) it's worth being aware it's O(n) per call rather than pre-indexed.

Test coverage

• Good, focused unit tests for getPredicateAncestors/getPredicateChildren (chain walk, depth limit, unknown predicate, mixin-based children) using a fake injected model via the localStorage cache — solid approach for testing without hitting the network.
• No component-level tests yet for AppPredicateInfo.vue itself (loading state, "no definition found" state, modal open/close, the "+N more…" expansion). Given the PR is a draft, that's understandable, but worth adding before merge since these are the actual user-facing states.
• No regression test for the predicateDocsUrl inconsistency noted above.

Security

• No security concerns. The YAML is parsed via js-yaml's default load() (safe schema, not unsafeLoad), and all external links go through AppLink, which correctly opens absolute/external URLs in a new tab.

Other notes

• The PR description already flags the open UX questions (icon density on every badge, tooltip vs. modal, hierarchy depth) — agreed those are worth resolving before merging out of draft, particularly the "info icon on every association-table row" density concern, since it adds a decent amount of visual noise to already-dense tables.

Overall this is a well-structured first cut — the singleton composable refactor is a nice unrelated improvement, and the hierarchy tree UI is a thoughtful way to show partial is_a context. Main things to address before merging: the watcher race condition, de-duplicating the docs-URL builder, and adding component-level tests for AppPredicateInfo.vue.

[claude]

Review: predicate definition + partial hierarchy explainer (#1358)

Nice feature, and the lazy-load-on-first-open design plus the module-level singleton cache in use-biolink-model.ts are well thought out. A few things worth addressing before this comes out of draft.

🐛 Likely bug: highlighted predicate breaks the explainer on association tables

frontend/src/components/AppPredicateBadge.vue:43-53

const predicate = computed(
  () =>
    props?.association?.highlighting?.predicate?.[0] ??
    props.association.predicate,
);

const predicateString = computed(() => {
  const value = predicate.value;
  return (Array.isArray(value) ? value[0] : value) ?? "";
});

association.highlighting.predicate is Solr search-highlighting output (see AssociationHighlighting in api/model.ts:629-639), and it's rendered elsewhere via v-html (getFormattedPredicateLabel), which strongly implies it contains embedded markup (Solr's default <em> wrap around matched terms). That highlighted value is exactly what now gets passed into AppPredicateInfo as :predicate="predicateString".

Concretely, on AssociationsTable.vue:74 (<AppPredicateBadge :association="row" :highlight="true" /> — the exact "association-table row" case called out in the PR description), when a predicate match is highlighted:

• getPredicateInfo()/getPredicateAncestors()/getPredicateChildren() in use-biolink-model.ts will fail to match the slot (the raw string now contains <em> tags), so the modal will incorrectly show "No definition found" for a perfectly valid predicate.
• The modal heading, aria-label, and hierarchy rows use {{ formatted }} (plain text interpolation, not v-html), so if it did partially match, the literal tag text (e.g. <em>) would render as visible characters instead of being stripped/highlighted.

Suggest deriving predicateString from the plain association.predicate (not the highlighting-decorated value) specifically for the explainer, independent of what's used for the visual label.

Minor: shared composable state now has cross-component side effects

frontend/src/composables/use-biolink-model.ts:37-41

Turning model/isLoading/error into module-level singletons is the right call for avoiding refetching/reparsing, but it also means every consumer of useBiolinkModel() now shares the same loading/error flags — including the pre-existing PredicateDetailPanel.vue, which renders biolinkLoading/biolinkError directly in its template (PredicateDetailPanel.vue:14,24). If that panel and a predicate-explainer modal are ever visible/mounted at the same time, opening one will flip the other's loading/error UI too. Low risk today since they don't appear to share a page currently, but worth a comment noting the tradeoff (or scoping loading/error per-consumer while keeping the model itself shared).

Test coverage

The composable functions (getPredicateAncestors, getPredicateChildren, alias resolution) are well tested with good edge cases (cycles, depth limit, unknown predicate). Missing: any test for AppPredicateInfo.vue itself (opening the modal, lazy-load-once behavior, error/empty states) or the SectionAssociationDetails.vue integration. Given this is a draft, that may be intentional pending the open UX questions, but flagging before merge.

Style / consistency

• Good reuse of existing patterns: AppButton design="small" + @click fallthrough matches existing usage (TheBanner.vue, BaseChart.vue), AppLink/AppModal usage matches conventions elsewhere.
• The predicate-label/predicate-info-icon CSS split (keeping overflow-wrap only on the label so the icon doesn't orphan onto its own line) is a nice, well-commented touch.
• docsFor() in AppPredicateInfo.vue and the biolink docs URL construction in SectionAssociationDetails.vue duplicate the same replace(/^biolink:/, "") + slug logic — could be pulled into a shared helper (e.g. alongside format()) to avoid drift if the docs URL scheme changes.

Nit

getPredicateChildren() does a full linear scan of all slots on every modal open. Fine given it's on-demand/lazy per the PR's design goals, but if this hierarchy view expands (e.g. always-visible tooltips per the "open questions"), consider memoizing a parent→children index once after load rather than re-scanning per call.

Nothing here blocks the underlying approach — the lazy-load architecture and hierarchy-walking logic are solid. The highlighting bug is the one I'd fix before this leaves draft, since it affects the primary use case (association table rows) called out in the PR description.

[claude]

Review: predicate definition + partial hierarchy explainer (#1358)

Nice feature and the module-level singleton refactor of useBiolinkModel (shared model/isLoading/error, in-flight loadPromise dedup) is a solid improvement over the old per-instance version. A few things worth addressing before this comes out of draft.

🐛 Bug: stale predicate info when a badge instance is reused for a different predicate

AppPredicateInfo.vue's onOpen() guards the fetch with only:

async function onOpen() {
  show.value = true;
  if (info.value) return;   // <-- not keyed on props.predicate
  await loadBiolinkModel();
  info.value = getPredicateInfo(props.predicate);
  ancestors.value = getPredicateAncestors(props.predicate);
  children.value = getPredicateChildren(props.predicate);
}

info/ancestors/children are cached forever after the first open, regardless of whether props.predicate has since changed on the same component instance. Two concrete places this bites:

• SectionAssociationDetails.vue: <AppPredicateBadge :association="association" /> is not re-keyed when selectedAssociation changes (AssociationsTable.vue:193-195), so the same AppPredicateBadge/AppPredicateInfo instance persists across row selections. Select association A ("treats"), open the explainer, close it, select association B ("correlated_with"), reopen the explainer — it still shows A's definition/hierarchy.
• AssociationsTable.vue rows are rendered via AppTable.vue:72: <tr v-for="(row, rowIndex) in rows" :key="rowIndex"> — keyed by index, not row identity. Any sort/filter that changes which association lands at a given index will reuse that row's AppPredicateBadge instance, so a previously-opened explainer at that position can show cached info for a predicate that's no longer in that row.

Fix: invalidate the cache when props.predicate changes, e.g. track the predicate the cached data belongs to and reset info.value/ancestors.value/children.value (or just re-fetch) whenever it differs, rather than relying on info.value truthiness alone.

⚠️ Design goal not met: model fetch isn't actually gated on "user asks"

The PR description states the biolink model is "loaded lazily on first open (not per badge), so there's no cost until a user asks." That's true for AppPredicateInfo's own button, but SectionAssociationDetails.vue adds a second, independent load path:

watch(predicateValue, async (predicate) => {
  predicateInfo.value = null;
  if (!predicate) return;
  await loadBiolinkModel();
  predicateInfo.value = getPredicateInfo(predicate);
}, { immediate: true });

This fires as soon as an association row is selected in the table — before the user ever clicks the info icon — so selecting any row now triggers the external fetch(BIOLINK_MODEL_URL) (or at least a cache check + JSON parse) whether or not the definition box or the icon is ever opened. Worth either reusing the same on-demand pattern (e.g. only fetch when the "Evidence"/definition row becomes visible or is expanded) or acknowledging in the PR description that "on demand" now means "on row selection" rather than "on click."

Duplication

Predicate-string extraction, label formatting (replace(/^biolink:/, "").replace(/_/g, " ")), and the biolink docs URL builder are implemented independently in both AppPredicateInfo.vue (format, docsFor) and SectionAssociationDetails.vue (predicateLabel, predicateDocsUrl), plus a third variant of the array/undefined-unwrap in AppPredicateBadge.vue's predicateString. Consider hoisting formatPredicateLabel/predicateDocsUrl into use-biolink-model.ts (or a small util) so the three call sites can't drift.

Performance / density

AppPredicateBadge now always mounts an AppButton + AppModal per row (even though the modal itself is v-if-gated internally), so large association tables get one extra component instance per row just for the info affordance. You've already flagged this as an open UX question in the PR description (hover-only, section-header-only, or a single shared modal) — agree this is worth resolving before merging out of draft, especially combined with the index-keyed :key="rowIndex" reuse noted above.

Minor / nitpicks

• getPredicateAncestors's cycle guard seeds seen only with visited parents, not the starting predicate itself, so a 2-node is_a cycle (a → b → a) can surface the queried predicate as its own ancestor for one hop — the included test (terminates on an is_a cycle without looping) actually encodes this behavior rather than guarding against it. Low real-world risk since Biolink itself shouldn't have cycles, but seeding seen with the starting predicate's name up front would make the guarantee exact.
• Test file is frontend/unit/biolinkModelPredicates.test.ts; existing convention in frontend/unit/ mirrors the source filename (useSourceDashboard.test.ts, AppButton.test.ts, url.test.ts). Consider use-biolink-model.test.ts for consistency.
• Test coverage is solid for the composable's pure functions, but there's no component test for AppPredicateInfo.vue (loading/error/empty states, "+N more" expansion) or the new definition block in SectionAssociationDetails.vue. A @vue/test-utils mount test would also have caught the stale-cache issue above.

Nice touches

• shallowRef for the (large, read-only) parsed model, and the loadPromise de-dup for concurrent callers, are good calls.
• Graceful degradation (loading / error / "no definition found") states are all handled.
• CSS fix keeping the info icon glued to the wrapping predicate label (overflow-wrap split between .predicate-label and .predicate-info-icon) is a thoughtful detail for constrained table columns.

Overall solid first cut — the stale-cache bug is the one I'd treat as blocking before this leaves draft; the rest are polish.