PR2b: read-side flip + ensureLoaded + UI config + flag ON (#2337)#18
Merged
Conversation
…+ UI config + flag ON
Flips reads from `allCandidates` to `rowMatchState + conceptCache` via
structured tuples, lands $resolveReference-backed $lookup as `ensureLoaded`,
adds canonical_url / namespace / bridge_repos[] to the project config UI,
debounces the rerank trigger, and turns UNIFIED_MODEL_ENABLED to true as
the last step.
Stage-by-stage:
- A: fix latent PR1 keying bug in mergeIntoRowMatchState (def.url -> def.key,
cr.concept_url -> cr.concept_key); extract normalizeLegacyAllCandidates
into normalizers.js and call it from fetchAndSetProject so reloaded v1
projects backfill rowMatchState under the flag.
- B: ensureLoaded(conceptKeys) over batched POST /$resolveReference/
with in-flight Promise dedup; replaces lookupCandidates/lookupCode.
- C: MultiAlgoSelector adds canonical_url for custom algos and bridge
canonical_url, drops lookup_required; ConfigurationForm surfaces
target_repo canonical (with Auto-derived badge), bridge_repos[], and
namespace under an Advanced disclosure; namespace persisted on save/load.
- D: extract pure builders to viewBuilders.js (buildAlgorithmRowViews,
buildQualityRowViews, candidateToRowView, sortRowViews, conceptForMapping,
getScoreDetails, resolveAICandidateID); refactor Candidates/Concept/Score
to consume {candidate, conceptDefinition, conceptRow, bridgeConceptDefinition?}
tuples directly; rewrite setAutoMatched + setStateViews to read from
rowMatchState + conceptCache via pickTopRowView; bulk processBatch path
now merges into rowMatchState before setStateViews consumes it.
- E: rerank trigger is debounce + in-flight check (scheduleRerank from
mergeIntoRowMatchState); rerank writes rerank_score directly onto
ConceptRow; normalizer lifts search_normalized_score onto ConceptRow.rerank_score
for the single-algo reranker:true path (no separate $rerank round-trip).
- F: resolveAICandidateID resolves primary_candidate.concept_key via
conceptCache (preferred) with PR2a's canonical_reference.code + legacy
concept_id/id fallbacks.
- G: UNIFIED_MODEL_ENABLED = true.
Tests: 27 -> 79. New suites in __tests__/views.test.js,
__tests__/normalizeLegacyAllCandidates.test.js, __tests__/viewHelpers.test.js
cover view-layer joins (incl. bridge fan-out + multi-algo + multi-bridge
convergence), legacy backfill round-trip, score-bucketing, the legacy-shape
projection, and AI ID resolution. Bridge fan-out at the view layer is now
unit-tested even though the live bridge algo only attaches in production
builds.
Browser-verified against prod API from local dev: save/load round-trip is
byte-identical on rerank_scores; no $match re-fire for previously-matched
rows after reload.
Not in this PR (PR3 work):
- Drop legacy allCandidates state, schema-v2 save format, normalizeLegacy.js
for v1 backward compat
- Drop legacy `candidates` field + payload_version + concept_id/id shims
from AI Assistant payload
- Pagination append path under unified model
Bridge code path untested locally (BridgeMatchStub.canBridge() false in OSS
builds); staging verification is the gate before merging to main.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…UX polish
Addresses review issues found before staging promotion. Most are wiring
gaps exposed once the flag was flipped ON and the read path started
actually consuming rowMatchState + conceptCache.
Correctness:
- pickTopRowView reads target canonical from buildProjectContext() (the
same source the normalizer used to stamp ConceptDefinition.reference.url),
not from the caller's _repo arg. Eliminates the derived-vs-explicit
canonical mismatch that silently emptied auto-match.
- Centralize concept_identity injection in ensureConceptIdentity() inside
algorithms.jsx. Handles built-ins, API-loaded bridge/scispacy, and
custom algos via user-entered canonical_url. Replaces three inline
reconstructions in MapProject.jsx (getAlgoDef, bulk processBatch path,
lookupCandidates) and the legacy load path. Also fixes a latent bug
where custom-algo canonical_url was never wired into a concept_identity
block so the field had no runtime effect.
- matchRerankResultToKey throws on canonical-identity miss; the rerank
loop collects errors, logs them, and surfaces a single summary alert.
Dropped the fuzzy ocl_url / id+source fallbacks — the unified-model
spec relies on canonical identity and silent fallback was masking
config gaps.
- mergeIntoRowMatchState auto-triggers ensureLoaded for newly-arrived
ConceptDefinitions with lookup_status !== 'full' (bridge cascade
targets, sparse algorithm results). Plan called for state-driven
$lookup; PR2b had the helper but no automatic trigger. Forward-ref
pattern (ensureLoadedRef) matches existing scheduleRerankRef.
- fetchAndSetProject: when a saved project has match data but no target
canonical can be resolved, surface an error alert and force-open the
configuration drawer (setConfigure(true)) instead of silently rendering
an empty candidate list under the flag-ON read path.
Click-path wiring (rowView <-> legacy concept shape at the seam):
- Candidates.jsx decorates rowViews with top-level id/url/version_url so
SearchResults.handleRowClick (which looks up rows by those fields) can
resolve clicks back to the rowView. Without this, the candidate row
click never fired onShowItemSelect.
- Concept.jsx passes the conceptForMapping projection (not the raw tuple)
to setShowHighlights so SearchHighlightsDialog finds
search_meta.search_highlight (Matched Attributes).
- viewBuilders.getScoreDetails accepts either the {candidate, conceptRow}
tuple or the legacy {search_meta} shape, so the dialog's score chip
works regardless of which form the caller sends.
Save-time validation:
- Custom algos require a valid canonical_url (URL-pattern check). Save
button disabled and a structured Alert lists offending algos. The
inline TextField helperText also distinguishes missing vs malformed.
Exported getProjectConfigErrors() + isLikelyCanonicalUrl() from
algorithms.jsx as the single source of truth.
CSV export:
- __map_repo_url__ and __map_repo_id__ are now gated on `concept` (the
mapSelected entry) so they're blank on unmapped rows. Matches the rest
of the __map_* namespace (concept_id/name/url were already empty on
unmapped rows) — drops the asymmetric project-repo fallback that
predated PR2b.
UI polish + i18n:
- Compact canonical-URL caption under target repo (info icon + monospace
URL with ellipsis truncation + tooltip + muted 'derived' suffix).
Bridge repos use the same line per bridge. Replaces the orange chip
that read like an error.
- All PR2b-introduced t() calls switched from `t(key) || 'default'`
(which never falls through because i18next returns the truthy key
string) to `t(key, 'default')` / `t(key, {defaultValue, ...interp})`.
- Added the PR2b string set to en/translations.json (target_canonical_url,
canonical_auto_derived_short, bridge_canonical_short, advanced_settings,
resolution_namespace + description with {{owner}} interpolation, the
algo_canonical_url_* family, config_errors_title, target_repo_required_
on_load, etc.). resolution_namespace_description now spells out the
default ('When blank, defaults to {{owner}}'). Other locales inherit
via fallbackLng='en'.
No new tests required — covered by existing 79/79 suite. Bridge /
scispacy / AI Assistant paths still need staging verification (per the
PR description).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
Review-feedback follow-up commit (d7180d7)Addresses correctness gaps + UX polish found while testing the read-flip locally against a pre-PR2b legacy project. No new tests required — covered by the existing 79/79 suite. Bridge / scispacy / AI Assistant staging verification still gates merge per the original PR description. Correctness
Click-path wiring (rowView ↔ legacy concept shape at the seam)
CSV export
UI polish + i18n
Verified
🤖 Generated with Claude Code |
Contributor
|
Tried against 2 rows with CIEL bridge algo for ICD-11-WHO
2. mapping to bridge target (ICD) is not getting logged
3. Candidates table view not complete
4. Target code on left is always empty
5. Fetch More in Candidates is not doing anything (no calls in network, infinite loading candidates...)
|
Contributor
|
Another thing: |
Contributor
|
Trying 8 rows with only Semantic Algo, target CIEL -- Automatch worked here.
2. Fetch More button fires the call but doesnt update the candidates list
|
…back Addresses 6 of 10 issues from snyaggarwal's PR review (bridge / multi-algo flows in a build with PRIVATE_PACKAGES_GIT enabled). #6 deferred pending diagnostics; #1/#3 expected to resolve transitively. #9 — Every candidate duplicated. mergeIntoRowMatchState now drops existing candidates whose algorithm_id matches the incoming invocation before merging the new set (mirrors the legacy onResponse `reject(...)` on allCandidates). Concept_rows whose concept_key is no longer referenced by any surviving candidate are pruned. Without this, every re-fetch (legacy load + auto-match, or repeated $match calls) stacked fresh candidate UUIDs with identical concept_keys, surfacing as duplicates in algorithm view. #5 / #10 — Fetch More: re-fires + doesn't update. Pagination append branch in onResponse now feeds the appended page into the unified state via mergeIntoRowMatchState(..., {append: true}). The new option short-circuits the same-algo drop in #9 so earlier pages stay put while the new page stacks on top. Without this, Fetch More fired the request but the unified read path never saw the new results. #4 — Target Code column always empty (and likely #3 — Candidates table view not complete). Concept.jsx grew a legacyToRowView() wrapper at the top of the component. When `concept` is a legacy concept-shape object (id, display_name, url, search_meta) instead of a unified-model tuple, the wrapper synthesizes a minimal rowView so the rest of the render path works unchanged. Covers Target Code column, Search results, decision tables, anywhere Concept is invoked with a legacy projection (mapSelected, searchedConcepts). #7 — Mapped CIEL bridge concept indicator missing in algorithm view. Concept.jsx bridge branch now passes the real isSelectedForMap function to the bridge intermediary's algoScoreFirst row instead of hard-coding `false` and `placeholderMap`. The intermediary IS mappable per spec (it gets its own ConceptRow + bucket); when the user maps it from Unified view, algorithm view now shows the Mapped indicator. #8 — Rerank sent rows with empty display_name (-100000 sentinel score). buildRerankRowsForRow filters out ConceptRows whose ConceptDefinition has no usable display_name (typically bridge cascade targets still 'pending' before ensureLoaded fills them). scheduleRerank stays re-eligible (any ConceptRow with rerank_score===undefined keeps the row scheduled), so once ensureLoaded completes the rerank refires. #2 — Bridge target mapping not logged. _onMap previously gated the log call on `concept?.url`. Bridge cascade targets may arrive without an ocl_url until $resolveReference resolves them, so the action silently dropped from project history. Log now fires when EITHER url or id is present, with object_id surfaced as a fallback identifier. Not addressed in this commit: - #1 — Score not on top (bridge case): hypothesis is this resolves transitively once #8 lands (bridge targets get rerank scores after ensureLoaded completes instead of being stuck at undefined). - #3 — Candidates table view incomplete: hypothesis is this is the same root cause as #4 (Concept bails on legacy shape). Fixed by the legacyToRowView wrapper. - #6 — Auto Match doesn't fire calls with bridge-only algo: code review doesn't reveal a smoking gun. Needs Sunny's console / network log, or a diagnostic-logging follow-up. Two-algo (bridge + ES) works in the same env which suggests state / guard issue specific to the bridge-only path. Verified: 79/79 tests pass, eslint clean, NODE_ENV=production npm run build green. Bridge / scispacy / AI Assistant staging exercise still gates merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
Review-feedback follow-up commit (2d96c09) — @snyaggarwalThanks for the detailed report. Addressed 6 of 10 in this commit; 2 are expected to resolve transitively; 1 needs your diagnostics.
Notes on the duplicate fix (#9)The merge now does replace-by-algorithm, not stack. If you had any reliance on the old behavior (e.g. multiple algorithms returning the same concept building up cumulative candidates), that still works — candidates from different algorithms are unaffected; only same-(rowIndex, algorithm) re-runs replace. Multi-algo convergence in Quality view continues to dedup by concept_key. Notes on the pagination fix (#10)
🤖 Generated with Claude Code |
Contributor
|
Addresses 6 issues from @snyaggarwal's latest PR comments — including two regressions I introduced in 2d96c09 (#11, #14) — plus four target-repo / table-view / bridge-config fixes from @paynejd's local testing. #14 — Score shows NaN%. viewBuilders.getScoreDetails now returns rerankScore='' and algoScore='' when no numeric value is present, instead of parseFloat(null).toFixed(2) yielding the literal string 'NaN%'. The mapped CIEL concept in the Target Code panel shipped a visible NaN% chip because legacyToRowView (introduced 2d96c09) passes search_meta values that may be undefined when the mapping projection lost them. #11 — Rerank fires on every row visit. scheduleRerank now gates on display_name presence: a concept_row is rerank-eligible only when its ConceptDefinition has a usable name. The 2d96c09 fix that filtered name-less rows out of buildRerankRowsForRow correctly stopped the -100000 reranker sentinel, but its corollary — those rows stay rerank_score=undefined forever — caused the scheduler to re-fire on every row open. writeConceptCachePatch now chains into scheduleRerank for every row referencing a concept_key whose patch transitions display_name from absent to present (e.g. ensureLoaded completing on a bridge cascade target). #13 — AutoMatch doesn't auto-select even when top candidate qualifies. Resolves transitively with #11: bridge cascade targets that previously stayed rerank_score=undefined now receive a score after ensureLoaded completes + the chained scheduleRerank fires. pickTopRowView's rerank_score ?? -1 fallback no longer keeps them stuck at the bottom. Target-repo filtering (Quality view + Map button). Quality (score-grouped) view now ONLY shows target-repo concepts. Bridge intermediaries (CIEL when target is ICD/LOINC) are reference metadata about the cascade, not mappable themselves; surfacing them in Quality view double-counted and confused the bucketing. Algorithm view keeps the full mix so users can browse by-algorithm. Implementation: Candidates accepts a `targetCanonical` prop (sourced from buildProjectContext().target_repo.canonical_url, the same canonical the normalizer used to stamp ConceptDefinition.reference.url, so the comparison is exact) and filters qualityRowViews against it. Same canonical gates the Map button everywhere. The action column in both table and card views now hides MapButton for non-target-repo concepts. Concept.jsx Item renders a placeholder spacer for the bridge intermediary's algoScoreFirst row (reverting the 2d96c09 fix that had shown a Map indicator there) — bridge intermediaries can't be mapped, and Unified view no longer surfaces them anyway after this commit. Table view broken (display='table' in Candidates). TableResults reads ALL_COLUMNS['concepts'] paths that expect the legacy concept shape (id, url, names, descriptions, source, search_meta, ...) at top level. The rowsForTable decoration in 2d96c09 only added id/url for click lookup, leaving the table cells blank. Now spreads the conceptForMapping projection first, then the rowView fields (no collision between the two — different keys), then sets id/url/version_url explicitly at the end for handleRowClick. #12 — First-time row click with bridge: API returned candidates, UI shows none. Auto-Match works in the same setup. buildProjectContext's bridge_repo was only constructed when bridgeAlgo.target_repo_url was truthy. When a user adds the bridge algo and accepts the placeholder default '/orgs/CIEL/sources/CIEL/' without editing the field, sel.target_repo_url stays undefined — so on the next fetch, projectContext.bridge_repo is absent, the normalizer's resolveReference returns null for every bridge primary, normalizeAlgoResult returns the empty triple, and rowState ends up with zero candidates. Auto-Match exhibits the same code path but apparently works in Sunny's env (likely his algo state has target_repo_url populated by some other write path; the manual-click flow is the one that surfaces it). Fix: buildProjectContext falls back to a per-type default (BRIDGE_DEFAULT_RELATIVE_URL['ocl-ciel-bridge'] = '/orgs/CIEL/sources/CIEL/') matching the MultiAlgoSelector placeholder. Generic 'ocl-bridge' algos without target_repo_url still fail (no canonical default makes sense), but that's expected per the spec — the user must set it. Verified: 79/79 tests pass, eslint clean. Local in-browser: legacy project loads, candidates render, Quality view shows only target-repo concepts, table view shows full columns, Auto-Match selects correct rows, no rerank re-fire loop on row visits, no NaN% chips. Bridge / scispacy / AI Assistant exercise still gates merge per the original PR description; the remaining open items are S6 (Auto Match no calls with bridge-only algo — pending Sunny's diagnostics) and verifying the above fixes against his environment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
Round-2 review fixes (104a7c9) — @snyaggarwalAddresses the 4 issues you reported on the previous commit plus 4 from local testing. Net result: Quality view shows only target-repo concepts (bridge intermediaries appear only in Algorithm view, per spec), no more NaN% chips, no rerank re-fire loop, table view renders.
Things still pending diagnostics
Re-test requestWhen you have a minute, would you re-run your previous flows and confirm:
🤖 Generated with Claude Code |
…canonical fetch
Two small fixes from local testing on the Configure Mapping Project panel.
i18next no longer HTML-escapes interpolated values.
The Resolution Namespace helper text rendered "/users/jon/" as
"/users/jon/" because i18next's default
interpolation.escapeValue=true HTML-encoded the {{owner}} substitution
before React got it. React already escapes JSX output, so the i18next
escape was both redundant and visible. Set escapeValue:false globally
per react-i18next's standard recommendation.
Bridge Canonical URL is now fetched from the source repo's metadata.
MultiAlgoSelector previously only populated bridge_repo.canonical_url
when the user typed one in by hand. With no manual entry, the downstream
buildProjectContext fell through to https://ns.openconceptlab.org{relurl}
and the Bridge: caption in ConfigurationForm displayed the derived URL
even for repos with an explicit canonical — CIEL shipped as
https://ns.openconceptlab.org/orgs/CIEL/sources/CIEL/ instead of its
real canonical https://CIELterminology.org.
Fix: when a bridge algo's target_repo_url is set, fetch the repo via
APIService and patch bridge_repo.canonical_url with response.data.
canonical_url when present. Cached at module scope (canonical URLs are
stable, per the unified-model spec). Per-algo sync runs once per
(key, url) — typing in the canonical field afterwards is sticky until
the user changes the Bridge Source URL. ConfigurationForm's
deriveCanonicalUrl fallback unchanged; it now only fires for repos that
genuinely lack a canonical.
Verified: 79/79 tests pass, eslint clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…fied-score chip Interim semantic-search candidates were rendering with a 100% unified score until the debounced $rerank/ pass landed. Cause: viewBuilders .getScoreDetails fell back to candidate.score * 100 when conceptRow.rerank_score was undefined. Semantic search returns top candidates with raw score ≈ 1.0 (cosine similarity placeholder), so the scaling produced 100% for every result during the interim. After rerank completed, the chip updated to the real value — wrong AND misleading during the window. Removed the fallback. percentile stays undefined when no rerank score exists; rerankScore becomes ''. Score.jsx renders a muted em-dash in the unified slot when rerankScore is empty so the chip stays present but neutral. Quality bucket coloring also goes neutral until rerank lands. Single-algo native paths (ocl-search with reranker:true) already get search_normalized_score from the $match response, so rerank_score is populated on first arrival — those paths are unaffected. Updated the existing viewHelpers test that asserted the old fallback behavior; it now documents that an interim candidate with no rerank score yields hasPercentile=false, no qualityBucket, and a raw algoScore chip but no unified chip. Verified: 79/79 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…b visits with zero matches
fetchScispacyCandidates gated its short-circuit on existingCandidates
.length > 0. A successful scispacy invocation that returned zero
matches writes {row, results: []} into allCandidates — the array exists
but is empty. The .length > 0 check failed for those rows, so every
time the user switched away from the Candidates tab and back, the
algo re-ran. Rows where scispacy found at least one match correctly
short-circuited; the bug was specific to the zero-result case.
Gate on entry presence instead. find(...) returning a defined entry
means the invocation completed and was persisted; that's the signal
we want to skip on. Empty-results rows now skip on revisit; failures
still retry because the catch block markAlgo(-2)s without writing an
entry, so find returns undefined and the path falls through to a
fresh attempt.
Verified: 79/79 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…bridge_child rows in algo view
Bridge_child rows (cascade targets nested under their bridge intermediary)
in the Group by Algo view were rendered with noScore={algoScoreFirst}.
algoScoreFirst is always true in algo view, so the Score component was
suppressed entirely — no raw chip (correct, bridge response doesn't
score targets) AND no unified chip (wrong, the row's rerank_score is
meaningful per-(row, concept)).
Dropping the prop lets Score render. The unified chip surfaces from
conceptRow.rerank_score; the raw chip stays absent because
candidate.score is undefined for bridge_child (normalizers.js line 256)
and Score's hasRawScore guard skips the parenthetical algoScore slot.
Verified: 79/79 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… for scispacy candidates Scispacy unified-score chips were showing the raw composite_score multiplied by 100, masquerading as a rerank score. Cause: fromScispacyResultsToConcepts pre-populated search_meta.search_normalized_score = composite_score * 100. The normalizer reads search_normalized_score directly into ConceptRow.rerank_score (normalizers.js:178), so the synthesized value became the row's "rerank score" before the debounced $rerank/ pipeline got a chance to fill it in — and once set, the rerank-loop overwrite path generally didn't replace it. The user-visible disparity (0.64 raw, 64.29% unified) was the same number to different precision: composite_score ≈ 0.6429 displayed as "0.64" via toFixed(2) on one chip, "64.29%" via composite_score * 100 on the other. Not actually a rerank score. Drop the normalized_score synthesis. Scispacy ConceptRows now enter rowState with rerank_score: undefined; scheduleRerank already covers the multi-algo and ocl-scispacy paths (MapProject.jsx:2582) and the matchRerankResultToKey path resolves scispacy's canonical_url=loinc.org identity correctly. With the score*100 fallback removed in 9fcdbb2, the chip shows a muted em-dash during the interim and switches to the real rerank value when $rerank/ completes. Verified: 79/79 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Children Two bugs in the Group by Algo view sort. onSort silently flipped same-option clicks to Unified. The old conditional `if(option === sortBy || option === 'rerank_score')` treated re-clicking the currently-selected sort option as a switch to rerank_score. In algo view sorted by Raw, clicking "Raw" again silently moved the user to Unified — the toolbar selection looked correct one moment and not the next. Rewritten as a plain assignment: setSortBy gets whatever the user clicked. The implicit group-switch when picking 'algo_score' stays (raw scores aren't comparable across algos in the Quality grouping, so Raw really does imply algo view). Bridge cascade targets weren't sorted within their parent bridge. buildAlgorithmRowViews emits bridgeChildren in normalization order with no internal sort. For SAME-AS bridges with one target this is invisible, but bridges with multiple cascade targets rendered them in arbitrary order. getCandidates now applies sortRowViews to each bridge's children with the same sortBy/order as the top-level. Children don't have their own raw score; sorting by algo_score leaves them in insertion order via sortRowViews' -1 sentinel — acceptable, and Unified sort lands them correctly by their per-row rerank_score. Verified: 79/79 tests pass. Pre-existing lint errors in Candidates.jsx (unused candidateToRowView import line 52, unused arr destructure line 402, both introduced in 41057bf) are unchanged by this commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In Group by Match Quality, bridge cascade targets rendered only the
target concept's source:id+name preceded by the map_type chip. The
bridge intermediary (e.g. CIEL when target is LOINC) was dropped from
the row entirely — the user had no inline signal of *which* bridge
concept reached the target, even though the data was already on
bridgeConceptDefinition.
Two sub-cases, two behaviors:
(a) Bridge-only — the target was reached ONLY via the bridge. Concept.jsx
Item's bridgeChild branch now renders the full cascade inline:
CIEL:1234 ciel-name [SAME-AS] LOINC:52767-1 loinc-name
(b) Convergence — the target was reached by BOTH a standard algo and one
or more bridges. The standard candidate is preferred as the primary (as
before), so the row renders as a normal LOINC entry. To preserve the
bridge story without cluttering the primary line, the new
bridgeContributors array on the rowView feeds an [i] indicator next to
the algorithm chip in the secondary row. Hover yields a tooltip:
"via CIEL:1234 ciel-name — SAME-AS"
Multiple bridges on the same target stack as separate tooltip lines.
Implementation:
- viewBuilders.buildQualityRowViews emits bridgeContributors: an array of
{bridgeConceptDefinition, map_type, algorithm_id} for every
contributing bridge_child candidate *other than* the primary. Bridge-
only rows (primary IS bridge_child) have empty bridgeContributors —
the inline framing already shows the bridge, no need for a duplicate
indicator.
- Concept.jsx Item splits the primary-line render into bridgeChild
(inline framing) and non-bridgeChild branches. The algo-chip secondary
line picks up the InfoOutlined icon when convergenceTooltip is
non-empty.
Tests: two new cases in views.test.js — convergence yields a non-empty
bridgeContributors entry with the right def/map_type/algorithm_id;
bridge-only yields an empty array.
Verified: 81/81 tests pass, eslint clean, production webpack build
green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ty on algo responses Half of every input row's candidates rendered without their LOINC summary chips (COMPONENT/PROPERTY/TIME_ASPCT/etc.) in the card view. Diagnostic logging in Concept.jsx (added then removed in this branch) confirmed all affected candidates had: - algorithm_id: 'ocl-semantic' - lookup_source_type: 'algorithm' (never upgraded by ensureLoaded) - empty `concept.property` and empty `concept.properties` Root cause was client-side, not server-side. OCL $match with verbose=true already returns ConceptDetailSerializer output, which includes `property = JSONField(source='properties')` — the schema dict UI reads at ConceptSummaryProperties.jsx:22. oclmap was already passing verbose=true on every $match call. Two bugs in normalizers.js dropped the data on ingest: 1. toConceptDefinition's explicit field-copy list (lines 105-127) didn't include `property` or `extras`. Even when the verbose response had them, the normalizer threw them away before they reached conceptCache. Added both fields to the copy list with a comment pointing at the API surface they correspond to. 2. inferLookupStatus required BOTH `names` AND `descriptions` arrays for 'full' status. Many LOINC concepts have no separate `descriptions` (the LONG_COMMON_NAME is in `names`, schema fields are in `property`/`extras`) — they stayed stuck at 'partial' even when verbose-loaded, which triggered spurious ensureLoaded retries and produced misleading log signal. Reworked the heuristic: 'full' now means the response carries a verbose-payload marker (`property` field present — ConceptDetailSerializer always emits it, even as []), OR has populated `names` (the list- serializer signal). Explicitly DOESN'T check `extras` because scispacy synthesizes that field with internal metadata (LOINC_NUM, composite_score) and would false-positive into 'full'. Tests added in normalizers.test.js: - verbose response with populated `property` → full + property captured - verbose response with `property: []` → still full (marker, not length) - brief response without `property` or `names` → stays partial No server-side change needed. The "smart enough to detect partial vs full and do the lookup" behavior that was already in place now works because the lookup_status field accurately reflects payload completeness. Verified: 84/84 tests pass, eslint clean, production build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…zed_score in multi-algo mode Interim semantic candidates still rendered "100.00%" unified-score chips until $rerank/ completed — the score*100 fallback drop from 9fcdbb2 didn't fix this case because the score wasn't coming from the fallback. It was coming from search_meta.search_normalized_score, which the OCL $match endpoint emits UNCONDITIONALLY (line 878 of oclapi2 views.py sets it to FAISS normalized_score × 100). For top semantic hits the value rounds to 100. The normalizer at newConceptRow read that straight into ConceptRow.rerank_score even in multi-algo mode where it's just a per-algo native score, not a unified rerank. Threaded trustServerRerank through the normalizer context: normalizeAlgorithmInvocation(payload, { trustServerRerank }) → normalizeAlgoResult(result, { trustServerRerank }) → newConceptRow(key, result, trustServerRerank) newConceptRow now propagates search_normalized_score to rerank_score ONLY when trustServerRerank is true. Call-site values: - Bulk auto-match processBatch (line 1496): !isMultiAlgo. The $match request at line 1433 sends reranker=!isMultiAlgo, so the response's normalized score is a real rerank only when single-algo. - Per-row $match (lines 2528, 2551): !isMultiAlgo && provider==='ocl'. Matches the line 2452 request flag and the line 2579 "rerank done" condition. - Bulk bridge (line 1687) and bulk scispacy (line 1731): omitted → defaults to false. Bridge's normalized score doesn't speak the project's query semantics; scispacy never had a real rerank. - normalizeLegacyAllCandidates: true. Saved-project rerank scores were persisted from a prior session's $rerank/ output, so they ARE the canonical rerank. UX consequence with the 9fcdbb2 fix in place: during the gap between multi-algo $match returning and $rerank/ completing, the unified chip renders a muted em-dash instead of a misleading 100.00%. When rerank lands, real scores appear. When a slow algo (scispacy) returns later, its rows briefly show "—" again until the next rerank pass — that's expected behavior given the staggered timing; the double-rerank itself is a known cost (Bug 9 follow-up: debounce more aggressively or wait for all-algos-complete). Tests updated: - normalizers.test.js: added a multi-algo case asserting search_ normalized_score is ignored when the caller doesn't opt in; updated the bridge ConceptRows test to expect undefined rerank_score (bridge path doesn't opt in); updated the single-algo carry-over test to pass trustServerRerank: true. - normalizeLegacyAllCandidates.test.js: unchanged — the legacy loader passes trustServerRerank: true, so its tests still see baked-in rerank scores. Verified: 85/85 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ack a score scheduleRerank's debounce window (~200ms) is far shorter than the gap between fast algos (semantic, bridge — seconds) and slow ones (scispacy — minutes). When scispacy lands late, a second $rerank/ fires for the same row — but the implementation re-reranks EVERY ConceptRow, not just the new ones. For a row with 50 candidates from semantic + 20 from scispacy, the second pass redoes the 50 already-scored rows for no reason. buildRerankRowsForRow now skips ConceptRows whose rerank_score is already a number. The cross-encoder reranker is per-(query, candidate) so scores from successive partial batches stay on the same scale; the filter is safe. Net effect: - Rerank #1 (after semantic+bridge): scores all available ConceptRows. - Scispacy arrives 2 minutes later: its new ConceptRows have rerank_score=undefined. - Rerank #2: payload includes ONLY the new scispacy rows. The old semantic+bridge rows are skipped — their scores persist. If a previously-scored row's score is somehow cleared (e.g. cache reset), it becomes eligible again on the next scheduleRerank fire. Refresh path still works: mergeIntoRowMatchState's drop-and-replace from 2d96c09 prunes ConceptRows whose candidates were removed, so refresh creates fresh unscored rows and rerank picks them up. Verified: 85/85 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…'s reuse check with the inner fetchers Companion to b0e8d52 (scispacy fix). That commit only patched fetchScispacyCandidates's gate; the outer fetchAllCandidatesForRow still gated on results.length > 0. Sequence when scispacy returned zero matches on a prior fetch and the user clicks the row again: fetchAllCandidatesForRow(scispacy) sees results.length === 0 → canReuseExistingCandidates = false → markAlgo(scispacy, 0) // "Running: ocl-scispacy-loinc" indicator on → dispatches to fetchScispacyCandidates fetchScispacyCandidates sees existingEntry !== undefined (correct) → returns { skipped: true }, no callback fired onResponse never invoked → no algo_finished log → markAlgo stays at 0 → indicator pinned forever; in worst case scheduleRerank also stuck waiting for the marker that never lands Same fix: gate reuse on entry presence, not result count. Empty-result rows now correctly short-circuit through the reuse branch — markAlgo(1), recurse to next algo, schedule rerank — exactly like a non-empty-result reuse would. forceReload still bypasses (user-driven refresh). offset > 0 still bypasses (Fetch More pagination). _retired flip still bypasses (user toggled retired-concept inclusion). Verified: 85/85 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…flight guard
The Decision panel's tab handler was re-firing the full algo chain on
every switch back to the Candidates tab. Two compounding bugs:
const firstAlgo = getFirstAlgoDef()?.id // already a string
if(... && !find(allCandidatesRef.current[firstAlgo?.id], ...)?.results?.length) {
// ^^^ string?.id is undefined
fetchAllCandidatesForRow(firstAlgo.id)
// ^^^^^^^^^^^ also undefined → falls through
// to 'use first algo if no algoId'
}
The cache-presence test always missed (allCandidatesRef.current[undefined]
returns undefined), so the refetch always fired even when candidates were
already loaded. fetchAllCandidatesForRow(undefined) → uses the first algo
anyway, so the user-visible effect was the chain re-running.
Additionally: no in-flight guard. If the user clicked a row, switched to
Discuss before the first algo finished, then switched back to Candidates,
the chain refired CONCURRENTLY with the still-running first chain.
Result: duplicate algo_finished log entries (ocl-semantic logged at
2:01:18 and 2:01:19 for the same row) and double rerank invocations.
Fix:
- Compute firstAlgoId once and use it directly (no .id chain).
- Skip the refetch when any selected algo for this row is currently in
flight (rowStageRef.current[rowIndex][id] === 0). The status-0 marker
is set at fetchAllCandidatesForRow line 2499 the moment a dispatch
begins, so this catches the race window cleanly.
- Use Boolean(find(...)) instead of .results.length to test presence —
same fix class as 6697c52 and b0e8d52 (an algo that returned zero
matches is still "loaded").
Verified: 85/85 tests pass, production build green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r Bug 9 rerank filter Bug 9 (0ae57dd) made buildRerankRowsForRow skip ConceptRows that already have a rerank_score, so the second rerank wouldn't redundantly re-score the first batch. That filter applied to ALL rerank invocations, including the explicit bulk-auto-match call. Race that caused the regression: 1. Bulk auto-match runs every selected algo (Promise.all on algoPromises). 2. Each algo's onResponse calls scheduleRerank(__row.__index) — debounced, isBulk=false. 3. The debounced rerank fires and scores every ConceptRow for the row. 4. processRerankWithConcurrency loops over rows and calls rerank(idx, true) — isBulk=true. 5. buildRerankRowsForRow finds zero unscored rows (the debounced pass scored them all). rerank() short-circuits at the empty-rerankRows guard and returns null. 6. setAutoMatched is wired to the isBulk=true success path (line ~2829 setTimeout setAutoMatched), so the short-circuit drops the side effect. User clicks Auto-Match, sees algos finish, sees rerank finish — and no row with a clearly-recommended top candidate gets proposed. Fix: when rerank() short-circuits with isBulk=true (no work needed, the debounced pass already did it), still schedule setAutoMatched. The rows ARE scored; the bulk caller just needs to be told to propose mappings. Verified: 85/85 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…iew bridge polish Three small Concept.jsx fixes uncovered in browser testing. map-type chip styling. The MUI Chip with size='small' was still tall enough to interrupt the surrounding text's line-height — visible as a vertical bump in cascade rows like "CIEL:1234 ... [SAME-AS] LOINC:52767-1 ...". Pulled out a MAP_TYPE_CHIP_SX constant: 18px height, 11px label, tight 0/6px label padding, baseline vertical alignment, 4px radius. Sits inline with the text now. Algo-view bridge children no longer repeat the bridge prefix. In Group by Algorithm, the parent bridge intermediary ConceptItem is already rendered above its nested cascade targets. The bridgeChild Item branch (added in 571a12b for Quality view) was rendering the prefix on every nested line too, so the same "CIEL:166016 Brain natriuretic..." appeared three times in a row. Gate the prefix render on !algoScoreFirst — Quality view still gets the full cascade inline, Algo view gets the cleaner "[BROADER-THAN] LOINC:42637-9 ..." per child. First bridge-child row in algo-view was missing its top divider. ConceptItem suppresses borderTop when firstChild={true}, and bridge children inherited that flag through baseProps when the bridge group itself was the first ConceptItem in the algorithm bucket. Explicitly pass firstChild={false} on every bridge-child render — there's always a parent intermediary above them, so the divider belongs. Verified: 85/85 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Quality view Candidates arriving before $rerank/ completes were being shunted into Low Ranked Candidates. The bucketing logic at getCandidates used `score = view.conceptRow?.rerank_score || 0`, which coerced undefined to 0 — every unscored row failed the recommendedScore / availableScore thresholds and landed in low_ranked. Visually that read as "these are poor matches" when the truth is "we don't know yet, rerank is pending". Bucketing now keys on score presence first: if typeof score !== 'number' → pendingRerank else recommended / available / low_ranked Rendered as a fourth section at the bottom of Quality view with a neutral gray PENDING_RERANK_COLOR indicator. Header reads "Unranked Candidates" (translation key already existed at en.json:514 unused). Section only renders when pendingRerank.length > 0; once rerank lands on every ConceptRow it disappears and rows have already redistributed into their proper score buckets. Translation key was already in en/translations.json; no other locales touched (fallbackLng='en' covers them). Verified: 85/85 tests pass, production build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… when repo canonical resolves
Saved projects reloaded with non-scispacy candidates missing from every
row. Quality view rendered empty or scispacy-only; the data was loaded
correctly into allCandidates and project history was intact, but
Candidates.jsx filters Quality-view rows on
view.conceptDefinition.reference.url === targetCanonical and the
references didn't match.
Root cause is an async ordering race in fetchAndSetProject:
1. Project GET returns. response.data has target_repo_url but the save
format never persisted target_repo.canonical_url.
2. setProjectFromData fires fetchRepo(repoURL, _repo) — promise NOT
awaited.
3. Same tick: loadProjectContext is built with the FALLBACK derived
canonical (https://ns.openconceptlab.org{relurl}). normalizeLegacy-
AllCandidates stamps every ConceptDefinition.reference.url with it.
4. Later: fetchRepo resolves, repo state lands with the REAL canonical
(e.g. http://loinc.org for LOINC). buildProjectContext computes
targetCanonical from this real value, passed as a prop to Candidates.
5. Quality view filters by reference.url === targetCanonical:
'https://ns.openconceptlab.org/orgs/Regenstrief/sources/LOINC/' vs
'http://loinc.org' → no match → empty list.
6. Scispacy uses reference_source: 'fixed' with a hardcoded
canonical_url, so its references are stamped 'http://loinc.org'
regardless of projectContext — and survive the filter. Hence the
"only scispacy shows" symptom.
Two-part fix:
(a) On save, persist target_repo.canonical_url (plus owner/source/version
metadata) alongside target_repo_url. Future loads pick up the canonical
on first read; no fetchRepo wait.
(b) For older saves without target_repo.canonical_url (and for any
post-load repo change), a useEffect on buildProjectContext re-runs
normalizeLegacyAllCandidates whenever the live target canonical differs
from the one used for the last normalize. lastNormalizedCanonicalRef
guards against re-running on identical canonicals — idempotent.
normalizeLegacyAllCandidates already takes a projectContext arg, so the
fix is plumbing only — no normalizer changes.
Verified: 85/85 tests pass, production build green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
Round-3 commits pushed (fdc60b8…45416ea) — @snyaggarwal16 targeted fixes from a full exercise of the read-flip path. Tests now 85/85, eslint clean, production webpack build green. Working tree clean. Display / scoring correctness
Algorithm coordination / cache correctness
Bridge framing / UX
Load-path correctness
Architectural takeaways (preserve these contracts going forward)
New tests (85 total now)
Re-test requestWhen you have a minute, would you re-run against this rebased branch? Particularly:
🤖 Generated with Claude Code |
…p review Three of the four "significant but cheap to land" items from the gap audit. The fourth (scispacy lookup_status='partial') is code-path resolved already — mergeIntoRowMatchState auto-fires ensureLoaded for non-'full' definitions and ensureLoaded handles missing ocl_url via $resolveReference — pending only E2E verification. #6 — Cleared namespace not sent on save. formData.append('namespace', namespace || '') unconditionally. The previous `if(namespace)` guard meant that clearing the field in the Advanced settings drawer silently dropped the change; server kept the stale value forever. Empty string is the server-side signal to use the project owner default, so always sending it is the correct behavior. #10 — buildQualityRowViews non-deterministic primary candidate. Multi-algo convergence (e.g. ocl-search + ocl-semantic both returning the same LOINC concept) picked the primary via `find()` first-hit on Object.values(rowState.candidates) iteration order, which flipped between renders. The UI's "primary algorithm" chip blinked between 'ocl-search' and 'ocl-semantic'. Now sorts by candidate.score desc within each type group (standard first, then bridge_child), with -Infinity fallback for unscored candidates (notably bridge_child). Added a test that locks in the score-based selection regardless of insertion order. #12 — ensureLoaded honored session token instead of lookupConfig.token. The legacy Search-tab and decision-view fetches passed `lookupConfig?.token` to getLookupService.get(), but the new unified ensureLoaded path silently used currentUserToken() for both the $resolveReference call and the per-concept fetch — leaving the LookupConfig widget half-decorative. Now reads `lookupConfig?.token || currentUserToken()` once and threads `authToken` through both fetches. Added lookupConfig?.token to the useCallback deps so token changes from the settings drawer immediately rebind. (Note: ensureLoaded still uses the response's ocl_url for the per- concept fetch URL; lookupConfig.url support would require redirecting those URLs to a custom server, which is a bigger architectural change and out of scope here — track separately if users ask.) Tests: 86/86 (was 85; +1 new convergence-primary test). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four fixes from @paynejd's local prod build exercise. Bridge children sort + table-view properties are the cosmetic items from Test B; AI Assistant POST + auto-fetch of target_repo canonical are the load- bearing functional fixes. Test B indent/wrap deferred (CSS-only, needs DOM probing). #F — AI Assistant POST never fires (GET works). fetchRecommendation's candidate-count gate sourced from legacy allCandidates only. Any flow that ends with populated rowMatchState but empty allCandidates (saved-project load races, future paths that skip the parallel legacy write) bailed at `_candidates.length > 0` before the POST. Now falls back to projecting unified state via conceptForMapping when legacy is empty. buildV2RecommendationPayload gets the same treatment: prefers rowMatchState + conceptCache directly (the authoritative source under UNIFIED_MODEL_ENABLED), with the legacy normalize-over-allCandidates path as fallback. Test B — Bridge children non-deterministic sort. buildAlgorithmRowViews preserved Object.values(rowState.candidates) insertion order for nested bridge children. Same project rendered differently between refreshes. New sortBridgeChildren helper orders by rerank_score desc (when present), then concept code/id ascending. Stable + meaningful — high-scoring cascade targets surface first. Test B — Table view rendered without LOINC schema properties. conceptForMapping projected `properties` (plural) but ConceptSummaryProperties reads `property` (singular, the schema- specific dict OCL returns for LOINC: COMPONENT/PROPERTY/TIME_ASPCT/...). Card view worked because it gets the ConceptDefinition directly; table view's rowsForTable spread only had what conceptForMapping returned. Adding `property` and `extras` to the projection puts the schema chips back in the table. Test E — Scispacy candidates returned by API but not shown in UI. Root cause: Quality view filter compares `ConceptDefinition.reference.url === targetCanonical`. Scispacy uses a FIXED canonical ('http://loinc.org', per CONCEPT_IDENTITY_BY_TYPE). When the project's target_repo lacks canonical_url in its loaded metadata (RepoSearchAutocomplete returns brief data), the live canonical falls back to `https://ns.openconceptlab.org{relurl}` — mismatch, filter excludes every scispacy candidate. Fix: auto-fetch the target repo's canonical_url on selection (mirrors MultiAlgoSelector's bridge_repo canonical fetch in fdc60b8). The re-normalize useEffect from 45416ea then re-stamps any already- normalized ConceptDefinitions to the live canonical. fetchedRepoCanonicalUrlRef guards against re-fetches when the user toggles between repos. For projects targeting a real LOINC repo (canonical_url=http://loinc.org in OCL metadata), this restores parity: scispacy results pass the Quality view filter. Projects targeting a custom LOINC subset (no canonical_url, falls back to derived) still need a follow-up — the fundamental issue is that scispacy's "I always return LOINC codes" shouldn't be filtered out just because the target repo happens to be a LOINC variant — track separately if reported. Tests: 86/86 pass, eslint clean, production webpack build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four fixes from @paynejd's prod-build exercise. Two functional (AI POST 404, scispacy 503 cache-poisoning), two UI (refresh button visibility, bridge_child secondary indent + word-break). #F — AI POST 404: prompts/<key>/<VERSION>/invoke/. fetchRecommendation pinned the resolved prompt template's version into the invoke URL for every call. Bulk auto-match runs need that pin so a mid-run template publish can't shift behavior. Single-row invocations (AI Assistant button on one row) have no such consistency requirement and the user never picks a specific version — pinning made the URL hit a path the server may not host, producing a 404. Fix: when no caller- supplied resolvedPromptTemplate is provided (the single-row path), strip version + uri/url/prompt_template_uri from the resolved template so getResolvedPromptTemplateURI falls through to '/prompts/<key>/' and the invoke URL becomes '/prompts/<key>/invoke/'. Bulk path (caller passes its pre-resolved template) is unchanged. Scispacy 503 retry + loading-state race. Two bugs in fetchScispacyCandidates: 1. The .then was detached and the try/finally wrapper ran synchronously while the POST was still pending. setIsLoadingInDecisionView(false) fired immediately, the Candidates panel rendered "no candidates" before the server responded. 2. A 503 from the scispacy service (returned during its 2-5 min wake- from-sleep) flowed into the callback as if it were a successful empty response. fromScispacyResultsToConcepts produced [], the reuse-check entry was written with results:[], and the next row visit skipped the retry (Bug 2's existingEntry !== undefined gate). The user was stuck with permanently-empty scispacy candidates until they hard-refreshed. Fix: clear isLoading inside the response handler (success OR error branch). Detect error responses (response?.detail, status >= 400, missing data) and mark the algo as failed without writing the cached empty entry. Wire a .catch for hard network errors. Subsequent visits re-fetch. User sees a warning alert with retry guidance. Refresh button visible on no-candidates state. The toolbar (refresh + Group + Sort) was hidden whenever noCandidatesFound was true — leaving error states with no recovery path. Now refresh shows always; Group/Sort still hide when there are no candidates to organize. Click counts as a row-level retry which re-fires any failed algorithm. Bridge_child secondary text indent + word-break (Test B cosmetic). Bridge_child rows render with a leading [SAME-AS] chip on the primary line. ListItemText's secondary defaults to flush-left, so the secondary content (LOINC schema chips: COMPONENT/PROPERTY/...) rendered visually LEFT of the chip — the artifact in @paynejd's screenshot. Add 6px paddingLeft when bridgeChild=true so the secondary aligns with the chip's left edge. Also set overflowWrap: break-word + wordBreak: normal on the secondary container so LOINC's '^' and '/' separators (and stretches like "peptide.B") don't trigger mid-word breaks when there's whitespace available. Not addressed in this commit (need diagnostics): - Bridge as FIRST algo doesn't fire fetch (works when second). Cannot reproduce from code review. Could be bridge module init timing (bridgeRef.current undefined during first dispatch) or a state-order issue. Need a console+network log from a fresh "bridge as first algo" click — ideally with `console.log(bridgeRef.current?.canBridge())` fired at the moment of the click. - Table view sparse for bridge intermediaries / bridge_children. Need a screenshot pinpointing which columns are empty + which view mode (Algorithm/Quality grouping). My hypothesis is that the rows shown in table view are bridge primaries (CIEL) which lack LOINC schema property dict; bridge_children with full LOINC data should populate when they're the iterated row. Tests: 86/86 pass, eslint clean, production webpack build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three fixes from @paynejd's round-5 verification: AI payload metadata trim, scispacy alert copy update, and bridge_child indent (finally — the previous CSS-padding shot did not match what the user wanted). AI Assistant — strip heavy metadata from legacy candidates payload. The "max_tokens 64000 / can only afford 62098" error is a server-side output-budget / credits limit (the prompt template's max_tokens param in ocl-ai-assistant), not an input-size issue per se. Verified on the client side anyway: the legacy `candidates[]` field only stripped `_source`, leaving `extras` (LOINC source-specific data, huge), `names` (multiple locales), `descriptions` (multiple, often long) in every candidate sent to the LLM. The v2 `recommendable_concepts` field already omits these. Now `stripHeavyFields` drops `_source`, `extras`, `names`, `descriptions` from each legacy candidate. Keeps the fields the legacy prompt template needs (id, display_name, source, search_meta, url, concept_class, datatype, retired, mappings, property). Reduces tokens regardless of whether it solves the specific credit error. Scispacy alert copy. Replaced the previous "Scispacy service unavailable (HTTP 503)..." message with the user's preferred phrasing: "OCL's scispacy matching service is starting up. This may take a couple minutes. You can safely leave this row and come back. Click Refresh if results aren't here in a couple of minutes." Applied to both the response-handler error branch and the .catch network-error branch. Bridge_child indent — proper flex layout (not CSS padding). The previous attempt (paddingLeft: 6px on the secondary div) didn't work because the BROADER-THAN chip is ~100px wide, the SAME-AS chip ~70px; a fixed 6px padding can't compensate. Restructured Item's return so when useFlexLayout is true (bridge_child with map_type), the chip becomes a SIBLING flex column to ListItemText. Both the primary text AND the secondary text inside ListItemText then align to the chip's right edge, regardless of chip label width: ┌──────────────┬────────────────────────────────────┐ │ [BROADER-THAN]│ LOINC:14635-7 25-hydroxyvitamin D3 │ │ │ COMPONENT: Calcidiol PROPERTY: ... │ └──────────────┴────────────────────────────────────┘ The chip is dropped from the primary span when useFlexLayout is on (the bridgeChild path checks `!useFlexLayout && candidate?.map_type` before rendering the inline chip). Non-bridge rows are unchanged — ListItemText renders directly without the flex wrapper. The overflowWrap/wordBreak rules from the prior commit remain. Tests: 86/86 pass, eslint clean, production webpack build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…empts Per @paynejd: "revert the bridge-child indent -- this is not what i asked for and does not work. keep the other two." Reverts Concept.jsx to its state at 22eaa6a (pre-attempt). Drops both attempts at fixing the bridge_child secondary alignment: - 48dc6b5 (round 5): paddingLeft: 6px hack on the secondary div. Wrong magnitude — the chip is wider than 6px and varies by label. - 0e5805e (round 6): flex layout extracting the chip into a sibling column. Tested green but the user reports it does not match their intent. The other two round-6 fixes — MapProject.jsx AI payload trim and the scispacy alert copy — are unchanged. Status: the bridge_child secondary alignment remains an open visual issue. Need clarification on the desired layout (where should the COMPONENT/PROPERTY/etc. line start relative to the [SAME-AS] chip and the LOINC code text?) before another attempt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
Rounds 4–7 summary + open items for review — @snyaggarwalWrap-up across the last four commits since the round-3 push. 86/86 tests, eslint clean, production webpack build green. What landed since round 3
🔴 Open items needing Sunny's eyes
🔴 Open item NOT in this repo
Status snapshot
🤖 Generated with Claude Code |
…ttings overlay | extracted advanced settings component
snyaggarwal
approved these changes
May 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR2b of the unified candidate/concept data model refactor for OpenConceptLab/ocl_issues#2337. Flips reads from the legacy
allCandidatesshape to the newrowMatchState + conceptCachemodel via structured tuples, lands$resolveReference-backed$lookupasensureLoaded, surfaces canonical-URL / namespace / bridge_repos[] in the project config UI, debounces the rerank trigger, and turnsUNIFIED_MODEL_ENABLEDon as the last step.Builds on #16 (PR1) and #17 (PR2a). Spec:
plans/unified-mapper-model.md.mainUNTIL STAGING VERIFICATION COMPLETESThis PR turns
UNIFIED_MODEL_ENABLEDtotrue. Bridge, scispacy, and AI Assistant code paths only attach at production build time viaPRIVATE_PACKAGES_GITand could not be exercised in local OSS dev (BridgeMatchStub.canBridge() === false). They must be verified on the staging deployment before this PR is merged tomain.Required staging checks (details in the test plan below):
$resolveReferenceflowIf any of these fail on staging, hold the merge and triage before promotion to prod.
Suggested workflow: merge this branch to a staging branch (or however the staging build is triggered), exercise the unchecked items below on the staging URL, and only then promote to
main.What's in this PR
Stage A — write-side foundations
mergeIntoRowMatchState:def.url→def.key,cr.concept_url→cr.concept_key. Was silent because the flag was OFF.normalizeLegacyAllCandidatesintonormalizers.jsand call it fromfetchAndSetProjectso reloaded v1 projects backfillrowMatchStateunder the flag. Necessary precondition for the flag flip; PR3's schema-v2 load subsumes this.Stage B —
ensureLoadedover$resolveReferenceensureLoaded(conceptKeys): batchedPOST /$resolveReference/?namespace=...followed by per-resolved concept fetch, with in-flightPromisededup keyed byconcept_key. ReplaceslookupCandidates/lookupCode. Directocl_urlfetch when available, else canonical resolution path.Stage C — UI config
MultiAlgoSelector: new requiredcanonical_urlfield for custom algos with URL validation; bridge canonical_url surface;lookup_requiredcheckbox removed (no longer a property of the algorithm).ConfigurationForm: target_repo canonical URL displayed with "Auto-derived" badge whencanonical_url_source === 'derived'; bridge_repos[] summary with same badge convention;Resolution Namespaceunder an Advanced disclosure (defaulted to project owner).namespacestate plumbed throughMapProject; persisted on save/load.Stage D — read flip to structured tuples
viewBuilders.js(pure JS, node:test loadable):buildAlgorithmRowViews,buildQualityRowViews,candidateToRowView,sortRowViews,conceptForMapping,getScoreDetails,resolveAICandidateID.Candidates.jsx:getCandidatesrewritten to iterateRowState.candidates(algo view) /RowState.concept_rows(quality view); passes{candidate, conceptDefinition, conceptRow, bridgeConceptDefinition?, bridgeChildren?, contributingCandidates?}tuples downward.Concept.jsx: receives tuples directly; ad-hocmapping.cascade_target_*extraction removed; bridge children nested under their parent bridge in algorithm view, surface as their target concept in quality view.Score.jsx: readsconceptRow.rerank_score+candidate.score; thin wrapper over the puregetScoreDetailsadds thebucketColorUI affordance.setAutoMatchedandsetStateViewsrewritten to read fromrowMatchState + conceptCachevia apickTopRowViewhelper that filters bytarget_repo.canonical_urlso bridge intermediaries are excluded from auto-mapping.processBatch.then) now normalizes and merges intorowMatchStatebeforesetStateViewsconsumes it.Stage E — rerank debounce + in-flight check
scheduleRerank(rowIndex): debounce timer per row + in-flightSet+ rerun-needed flag for the case where newConceptRows arrive while an earlier rerank is still flying. Triggered frommergeIntoRowMatchStatewhenever anyConceptRowhasrerank_score === undefined. Replaces the "wait for every algo" implicit batching.rerank_scoredirectly ontoConceptRow; legacyallCandidateswrite preserved for save format compatibility (until PR3).newConceptRownow extractsresult.search_meta.search_normalized_scoreontoConceptRow.rerank_scoreso the single-algoreranker:true$matchpath doesn't need a separate$rerankround-trip.Stage F — AI Assistant response resolution
resolveAICandidateID: preferprimary_candidate.concept_keyresolved viaconceptCache[concept_key].reference.code; falls back to PR2a'scanonical_reference.codeshim, then legacyconcept_id/id. Used in both Candidates.jsx's UI highlight match and MapProject's CSV export.Stage G — flag flip
UNIFIED_MODEL_ENABLED = trueat MapProject.jsx.Test plan
✅ Local verification (done)
npm test— 79/79 (was 27, +52 new tests)npm run eslint— cleanNODE_ENV=production npm run build— greendocker compose build— green (production target)rerank_scorevalues byte-identical (e.g.81.87800645828247→81.87800645828247)allCandidatesenvelopes restored with fullresultsarraysrowMatchStaterehydrated with same candidate/concept_row counts$matchre-fire on row open for previously-matched rowsocl-search: candidates render, scores show, mapping persists🟡 Staging verification — REQUIRED BEFORE MERGE TO
mainThese code paths only attach at production build time. Each must pass on staging before this PR is promoted to prod.
rowMatchStatepopulates with bridge intermediary + cascade targets at correct canonicals (CIEL:https://CIELterminology.org, LOINC:http://loinc.org)bridge_contextpopulates withtarget_concept_keyspointing at the LOINC entries inrecommendable_conceptsbridge_childcandidate maps to the LOINC target (not the CIEL intermediary)pickTopRowViewfilter bytarget_repo.canonical_urlexcludes them)$resolveReference/?namespace=...fires when scispacy returns LOINC codes; per-concept GET requests follow the resolved repo URL; concept cards fill in with names/descriptions (not just bare IDs)recommendable_concepts+bridge_context+target_repo) alongside legacycandidates;payload_version: 'v2'present;resolveAICandidateIDhighlights the recommended candidate in the list with the sparkle iconRegression check (also on staging)
New tests (52)
__tests__/views.test.js(21) — algorithm grouping, quality grouping with bridge fan-out, multi-algo convergence, multi-bridge namespaces, sort orderings, missing-cache-entry handling.__tests__/normalizeLegacyAllCandidates.test.js(11) — load-path backfill including bridge cascade fan-out, multi-algo dedup, identity injection fallback, dedup across rows.__tests__/viewHelpers.test.js(20) —getScoreDetailsbucketing for recommended/available/low_ranked,conceptForMappinglegacy projection incl. bridge_child,resolveAICandidateIDresolution chain.Not in this PR (PR3 scope)
allCandidatesstatenormalizeLegacy.jsfor v1 backward-compatible loadcandidatesfield +payload_version+concept_id/idshims from AI Assistant payloadCoordination
ocl-ai-assistantprompt template revision (separate session) reads the v2 payload fields and structurally excludes bridges from the recommendation pool. PR2b can ship without it — legacycandidatesis still in the payload (PR2a Option A)._to_essentialallow-list addition inocl-ai-assistantforrecommendable_concepts+bridge_contextships alongside the prompt template revision.🤖 Generated with Claude Code