From 0369dc175312cc6aadbbf312a78365720b595cd2 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Fri, 23 Oct 2026 13:48:07 +0530 Subject: [PATCH 1/2] OpenConceptLab/ocl_issues#2840 | Mapper caches lookup 404s for the session and drops bridge targets missing from the target version --- src/components/map-projects/MapProject.jsx | 52 ++++++++++++++++--- .../__tests__/normalizers.test.js | 40 ++++++++++++++ src/components/map-projects/normalizers.js | 27 ++++++++++ 3 files changed, 112 insertions(+), 7 deletions(-) diff --git a/src/components/map-projects/MapProject.jsx b/src/components/map-projects/MapProject.jsx index a7761f9f..1006200e 100644 --- a/src/components/map-projects/MapProject.jsx +++ b/src/components/map-projects/MapProject.jsx @@ -117,7 +117,7 @@ import { getPreviewLimitError, isAIPreviewLimitError } from './previewLimits' import { formatRowNumbers, rowNumberOf, runMatchBatch, requestSingleMatch, runWithConcurrency, INTERACTIVE_WAIT_CAP_MS } from './matchBatch' import { getAIAssistantChoices, getModelUsed, getPromptTemplateKey, createLatestRequestGate } from './aiVisibility' import { DEFAULT_ENCODER_MODEL } from './rerankerModels' -import { normalizeAlgorithmInvocation, hasSuccessfulAlgorithmResponse, getAlgorithmStagesFromResponses, lookupStatusRank, buildRecommendableConceptEntry, stripConstantClassAndDatatype, buildLookupConceptUrl, serializeCandidates, toStoredRowMatchState } from './normalizers' +import { normalizeAlgorithmInvocation, hasSuccessfulAlgorithmResponse, getAlgorithmStagesFromResponses, lookupStatusRank, buildRecommendableConceptEntry, stripConstantClassAndDatatype, buildLookupConceptUrl, serializeCandidates, toStoredRowMatchState, dropMissingBridgeTargets } from './normalizers' import { parseConceptKey } from './conceptKey' import { normalizeLookupConfig } from './lookupConfigFields' import { getDeclaredLocales, getDefaultTargetRepoVersion, getProjectTargetRepoVersion, getSavedTargetRepoVersion, getTargetRepoVersionFromUrl, getTargetRepoVersionId } from './projectTargetRepo' @@ -537,7 +537,11 @@ const MapProject = () => { : cr }) - rowMatchStateRef.current = { ...prevAll, [rowIndex]: nextRow } + // A bridge can propose codes the target version lacks; once a lookup has + // found one missing, later rows don't propose it again (ocl_issues#2840). + const keptRow = dropMissingBridgeTargets(nextRow, missingConceptKeysRef.current) + + rowMatchStateRef.current = { ...prevAll, [rowIndex]: keptRow } setRowMatchState(rowMatchStateRef.current) // Merge ConceptDefinitions into the project-wide conceptCache, keyed by @@ -567,7 +571,7 @@ const MapProject = () => { // free. Forward-ref pattern because ensureLoaded is declared later. if(ensureLoadedRef.current) { const keysToLoad = concept_definitions - .filter(d => d && d.lookup_status !== 'full') + .filter(d => d && d.lookup_status !== 'full' && !missingConceptKeysRef.current.has(d.key)) .map(d => d.key) if(keysToLoad.length) { ensureLoadedRef.current(keysToLoad, rowIndex, isRunTraffic).then(() => { @@ -601,9 +605,13 @@ const MapProject = () => { // ocl-scispacy uses a hardcoded canonical 'http://loinc.org' while // ocl-bridge cascade targets use target_repo's canonical — they produce // different concept_keys but resolve to the same OCL URL). Failed/404 - // entries are deleted so they remain retryable; successful entries persist - // for the session. + // entries are deleted so they remain retryable; successful and 404 entries + // persist for the session, so a missing code is fetched once + // (ocl_issues#2840). const urlFetchCacheRef = React.useRef(new Map()) + // concept_keys whose lookup came back 404: bridge_child candidates for them + // are dropped (ocl_issues#2840). + const missingConceptKeysRef = React.useRef(new Set()) // Rerank scheduling (plans/unified-mapper-model.md "Rerank — debounce + // in-flight check"). Replaces the legacy "wait for every algo to @@ -5017,6 +5025,23 @@ const MapProject = () => { writeConceptCachePatch(key, { ...existing, lookup_status: 'failed' }) }, [writeConceptCachePatch]) + // A bridge_child whose concept isn't in the target version isn't a + // proposal: drop it from every row (ocl_issues#2840). + const dropMissingConcept = React.useCallback((key) => { + missingConceptKeysRef.current.add(key) + const prevAll = rowMatchStateRef.current + let changed = false + const nextAll = {} + Object.entries(prevAll).forEach(([idx, rowState]) => { + nextAll[idx] = dropMissingBridgeTargets(rowState, missingConceptKeysRef.current) + if(nextAll[idx] !== rowState) changed = true + }) + if(changed) { + rowMatchStateRef.current = nextAll + setRowMatchState(nextAll) + } + }, []) + const ensureLoaded = React.useCallback(async (conceptKeys, logRowIndex, isRunTraffic = false) => { if(!Array.isArray(conceptKeys) || conceptKeys.length === 0) return const ctx = buildProjectContext() @@ -5118,7 +5143,11 @@ const MapProject = () => { if(err?.response?.status === 404) log({action: 'lookup', description: t('map_project.lookup_concept_404', {code, attempt: attempt + 1}), extras: {url: oclUrl, error: t('map_project.lookup_concept_404', {code, attempt: attempt + 1})}}, logRowIndex) }, - }).catch(() => { + }).catch(err => { + // Still 404 after the retries: the code isn't there. Keep that for + // the session so other rows don't fetch it again (ocl_issues#2840). + if(err?.response?.status === 404) + return {notFound: true} urlFetchCacheRef.current.delete(oclUrl) return null }) @@ -5139,6 +5168,8 @@ const MapProject = () => { }) } else { writeLookupFailure(key) + if(data?.notFound) + dropMissingConcept(key) } } catch (_) { writeLookupFailure(key) @@ -5197,7 +5228,7 @@ const MapProject = () => { } await Promise.all([directPromise, resolvePromise, ...pending]) - }, [buildProjectContext, writeConceptCachePatch, writeLookupFailure, lookupConfig?.token]) + }, [buildProjectContext, writeConceptCachePatch, writeLookupFailure, dropMissingConcept, lookupConfig?.token]) // Wire the forward-ref consumed by mergeIntoRowMatchState. useEffect // so the ref always points at the latest closure (ensureLoaded captures @@ -5207,6 +5238,13 @@ const MapProject = () => { ensureLoadedRef.current = ensureLoaded }, [ensureLoaded]) + // A code missing from one target version may be in another, so the + // session's 404s only hold for the version they were fetched against. + React.useEffect(() => { + missingConceptKeysRef.current = new Set() + urlFetchCacheRef.current = new Map() + }, [selectedTargetRepoVersion, lookupConfig?.url]) + // Lazily enrich a row's saved candidates when it is opened. The persisted // v2 candidates payload intentionally carries sparse ConceptDefinitions // ('partial'/'pending' — no descriptions, and for migrated projects no diff --git a/src/components/map-projects/__tests__/normalizers.test.js b/src/components/map-projects/__tests__/normalizers.test.js index 2329b94b..e40f29ab 100644 --- a/src/components/map-projects/__tests__/normalizers.test.js +++ b/src/components/map-projects/__tests__/normalizers.test.js @@ -28,6 +28,7 @@ import { compactProperty, stripConstantClassAndDatatype, filterAndTrimNames, + dropMissingBridgeTargets, filterAndTrimDescriptions, primarySubtag, uniqByPrimarySubtag, @@ -1784,3 +1785,42 @@ test('getAlgorithmStagesFromResponses: done (1) when any response succeeded, fai )), { 'ocl-semantic': 1, 'ocl-search': -2, 'custom-1': 1 }) assert.deepEqual(getAlgorithmStagesFromResponses({}), {}) }) + +// --------------------------------------------------------------------------- +// dropMissingBridgeTargets (ocl_issues#2840) +// --------------------------------------------------------------------------- + +test('dropMissingBridgeTargets drops bridge_child candidates for missing codes and their concept_rows', () => { + const rowState = { + algorithm_responses: {r1: {id: 'r1'}}, + candidates: { + p: {id: 'p', type: 'bridge', concept_key: 'ciel|1'}, + a: {id: 'a', type: 'bridge_child', concept_key: 'loinc|missing'}, + b: {id: 'b', type: 'bridge_child', concept_key: 'loinc|present'} + }, + concept_rows: {'ciel|1': {}, 'loinc|missing': {}, 'loinc|present': {}} + } + const result = dropMissingBridgeTargets(rowState, new Set(['loinc|missing'])) + assert.deepEqual(Object.keys(result.candidates), ['p', 'b']) + assert.deepEqual(Object.keys(result.concept_rows), ['ciel|1', 'loinc|present']) + assert.equal(result.algorithm_responses, rowState.algorithm_responses) +}) + +test('dropMissingBridgeTargets keeps a missing key another algorithm also proposed', () => { + const rowState = { + candidates: { + a: {id: 'a', type: 'bridge_child', concept_key: 'loinc|x'}, + s: {id: 's', type: 'standard', concept_key: 'loinc|x'} + }, + concept_rows: {'loinc|x': {}} + } + const result = dropMissingBridgeTargets(rowState, new Set(['loinc|x'])) + assert.deepEqual(Object.keys(result.candidates), ['s']) + assert.deepEqual(Object.keys(result.concept_rows), ['loinc|x']) +}) + +test('dropMissingBridgeTargets returns the same row when nothing is missing', () => { + const rowState = {candidates: {a: {id: 'a', type: 'bridge_child', concept_key: 'loinc|y'}}, concept_rows: {'loinc|y': {}}} + assert.equal(dropMissingBridgeTargets(rowState, new Set(['loinc|z'])), rowState) + assert.equal(dropMissingBridgeTargets(rowState, new Set()), rowState) +}) diff --git a/src/components/map-projects/normalizers.js b/src/components/map-projects/normalizers.js index 1073e51a..50d174f6 100644 --- a/src/components/map-projects/normalizers.js +++ b/src/components/map-projects/normalizers.js @@ -454,6 +454,33 @@ export const normalizeAlgorithmInvocation = (rawPayload, ctx = {}) => { } } +/** + * Drop the bridge_child candidates whose concept the target version doesn't + * have (its lookup came back 404), and the concept_rows nothing references + * after that. The bridge filters cascade targets by the target repo, not its + * pinned version, so it can propose codes the version lacks (ocl_issues#2840). + * Returns the same rowState when nothing is dropped. + * + * @param {Object} rowState {algorithm_responses, candidates, concept_rows} + * @param {Set} missingKeys concept_keys known to be missing. + */ +export const dropMissingBridgeTargets = (rowState, missingKeys) => { + if (!rowState?.candidates || !missingKeys?.size) return rowState + const candidates = {} + let dropped = false + Object.entries(rowState.candidates).forEach(([id, c]) => { + if (c?.type === 'bridge_child' && missingKeys.has(c.concept_key)) dropped = true + else candidates[id] = c + }) + if (!dropped) return rowState + const referencedKeys = new Set(Object.values(candidates).map(c => c?.concept_key)) + const conceptRows = {} + Object.entries(rowState.concept_rows || {}).forEach(([key, cr]) => { + if (referencedKeys.has(key)) conceptRows[key] = cr + }) + return { ...rowState, candidates, concept_rows: conceptRows } +} + const LOOKUP_RANK = { pending: 0, failed: 0, partial: 1, full: 2 } export const lookupStatusRank = (status) => LOOKUP_RANK[status] ?? 0 From 4d9411d9f4cc8e5c7e0bc322978b83ab69bc5a34 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Mon, 26 Oct 2026 08:17:17 +0530 Subject: [PATCH 2/2] OpenConceptLab/ocl_issues#2840 | review feedbacks --- src/components/map-projects/MapProject.jsx | 118 +++++++++--------- .../__tests__/lookupSession.test.js | 95 ++++++++++++++ .../__tests__/normalizers.test.js | 2 +- src/components/map-projects/lookupSession.js | 87 +++++++++++++ src/components/map-projects/normalizers.js | 31 +++-- 5 files changed, 268 insertions(+), 65 deletions(-) create mode 100644 src/components/map-projects/__tests__/lookupSession.test.js create mode 100644 src/components/map-projects/lookupSession.js diff --git a/src/components/map-projects/MapProject.jsx b/src/components/map-projects/MapProject.jsx index 1006200e..d68cd7f7 100644 --- a/src/components/map-projects/MapProject.jsx +++ b/src/components/map-projects/MapProject.jsx @@ -117,7 +117,8 @@ import { getPreviewLimitError, isAIPreviewLimitError } from './previewLimits' import { formatRowNumbers, rowNumberOf, runMatchBatch, requestSingleMatch, runWithConcurrency, INTERACTIVE_WAIT_CAP_MS } from './matchBatch' import { getAIAssistantChoices, getModelUsed, getPromptTemplateKey, createLatestRequestGate } from './aiVisibility' import { DEFAULT_ENCODER_MODEL } from './rerankerModels' -import { normalizeAlgorithmInvocation, hasSuccessfulAlgorithmResponse, getAlgorithmStagesFromResponses, lookupStatusRank, buildRecommendableConceptEntry, stripConstantClassAndDatatype, buildLookupConceptUrl, serializeCandidates, toStoredRowMatchState, dropMissingBridgeTargets } from './normalizers' +import { normalizeAlgorithmInvocation, hasSuccessfulAlgorithmResponse, getAlgorithmStagesFromResponses, lookupStatusRank, buildRecommendableConceptEntry, stripConstantClassAndDatatype, buildLookupConceptUrl, serializeCandidates, toStoredRowMatchState } from './normalizers' +import { createLookupSession, LOOKUP_NOT_FOUND } from './lookupSession' import { parseConceptKey } from './conceptKey' import { normalizeLookupConfig } from './lookupConfigFields' import { getDeclaredLocales, getDefaultTargetRepoVersion, getProjectTargetRepoVersion, getSavedTargetRepoVersion, getTargetRepoVersionFromUrl, getTargetRepoVersionId } from './projectTargetRepo' @@ -537,9 +538,8 @@ const MapProject = () => { : cr }) - // A bridge can propose codes the target version lacks; once a lookup has - // found one missing, later rows don't propose it again (ocl_issues#2840). - const keptRow = dropMissingBridgeTargets(nextRow, missingConceptKeysRef.current) + // Drop bridge targets already found missing from the target version. + const keptRow = lookupSessionRef.current.dropMissing(rowIndex, nextRow) rowMatchStateRef.current = { ...prevAll, [rowIndex]: keptRow } setRowMatchState(rowMatchStateRef.current) @@ -571,7 +571,7 @@ const MapProject = () => { // free. Forward-ref pattern because ensureLoaded is declared later. if(ensureLoadedRef.current) { const keysToLoad = concept_definitions - .filter(d => d && d.lookup_status !== 'full' && !missingConceptKeysRef.current.has(d.key)) + .filter(d => d && d.lookup_status !== 'full' && !lookupSessionRef.current.isMissing(d.key)) .map(d => d.key) if(keysToLoad.length) { ensureLoadedRef.current(keysToLoad, rowIndex, isRunTraffic).then(() => { @@ -599,19 +599,12 @@ const MapProject = () => { // later rows don't wait on them again (ocl_online#258). const stuckLookupsRef = React.useRef(new WeakSet()) - // URL-level fetch dedup for ensureLoaded. Keyed by ocl_url. Stores - // Promise: in-flight entries deduplicate concurrent requests for - // the same URL even when they arrive under different concept_keys (e.g. - // ocl-scispacy uses a hardcoded canonical 'http://loinc.org' while - // ocl-bridge cascade targets use target_repo's canonical — they produce - // different concept_keys but resolve to the same OCL URL). Failed/404 - // entries are deleted so they remain retryable; successful and 404 entries - // persist for the session, so a missing code is fetched once - // (ocl_issues#2840). - const urlFetchCacheRef = React.useRef(new Map()) - // concept_keys whose lookup came back 404: bridge_child candidates for them - // are dropped (ocl_issues#2840). - const missingConceptKeysRef = React.useRef(new Set()) + // Per target version: URL-level fetch dedup for ensureLoaded (different + // concept_keys can resolve to the same OCL URL), 404s included, and the + // concept_keys found missing. + const lookupSessionRef = React.useRef(null) + if(!lookupSessionRef.current) + lookupSessionRef.current = createLookupSession() // Rerank scheduling (plans/unified-mapper-model.md "Rerank — debounce + // in-flight check"). Replaces the legacy "wait for every algo to @@ -5025,15 +5018,16 @@ const MapProject = () => { writeConceptCachePatch(key, { ...existing, lookup_status: 'failed' }) }, [writeConceptCachePatch]) - // A bridge_child whose concept isn't in the target version isn't a - // proposal: drop it from every row (ocl_issues#2840). - const dropMissingConcept = React.useCallback((key) => { - missingConceptKeysRef.current.add(key) + // Drop a missing concept's bridge_child candidates from every row, unless + // the lookup was for an earlier target version. + const dropMissingConcept = React.useCallback((key, generation) => { + const session = lookupSessionRef.current + if(!session.markMissing(generation, key)) return const prevAll = rowMatchStateRef.current let changed = false const nextAll = {} Object.entries(prevAll).forEach(([idx, rowState]) => { - nextAll[idx] = dropMissingBridgeTargets(rowState, missingConceptKeysRef.current) + nextAll[idx] = session.dropMissing(idx, rowState) if(nextAll[idx] !== rowState) changed = true }) if(changed) { @@ -5055,6 +5049,11 @@ const MapProject = () => { // the LookupConfig widget half-decorative. Without one, the session's user // token goes only to the OCL API (getLookupToken), never to another host. const currentToken = currentUserToken() + // Results from before a target version change are ignored. + const session = lookupSessionRef.current + const generation = session.generation + const inFlight = inFlightLookupsRef.current + const isCurrent = () => session.isCurrent(generation) const directFetches = [] const toResolve = [] @@ -5065,12 +5064,12 @@ const MapProject = () => { if(!key) return const def = cache[key] if(def?.lookup_status === 'full') return - if(inFlightLookupsRef.current.has(key)) { - pending.push(inFlightLookupsRef.current.get(key)) + if(inFlight.has(key)) { + pending.push(inFlight.get(key)) return } const promise = new Promise(resolve => settlers.set(key, resolve)) - inFlightLookupsRef.current.set(key, promise) + inFlight.set(key, promise) pending.push(promise) if(def?.ocl_url) { @@ -5092,24 +5091,21 @@ const MapProject = () => { const reference = parseConceptKey(key) toResolve.push({key, reference}) } catch (_) { - inFlightLookupsRef.current.delete(key) + inFlight.delete(key) settlers.get(key)() } } }) const settle = (key) => { - inFlightLookupsRef.current.delete(key) + inFlight.delete(key) const fn = settlers.get(key) if(fn) fn() } const fetchConceptByOclUrl = async (key, oclUrl, source, logRowIndex) => { - // URL-level dedup: if a different concept_key is already fetching this - // same OCL URL (or fetched it successfully), reuse that promise instead - // of issuing a duplicate network request. - let dataPromise = urlFetchCacheRef.current.get(oclUrl) - if(!dataPromise) { + // URL-level dedup: reuse a fetch of the same OCL URL by another concept_key. + const dataPromise = session.fetchOnce(oclUrl, () => { const attemptFetch = async () => { let service = APIService.new() if(oclUrl.startsWith('http://') || oclUrl.startsWith('https://')) { @@ -5126,14 +5122,12 @@ const MapProject = () => { if(!result.ok) throw result.error || new Error(result.reason) const data = result.response?.data - if(data?.id) return data - urlFetchCacheRef.current.delete(oclUrl) - return null + return data?.id ? data : null } // Retry on 404: the lookup repo may still be lazily building the concept. // Attempts happen at t=0s, t=3s, t=7s. const code = parseConceptKey(key).code - dataPromise = retryWithBackoff(attemptFetch, { + return retryWithBackoff(attemptFetch, { maxRetries: 2, baseDelayMs: 3000, backoffFactor: 4/3, @@ -5143,19 +5137,13 @@ const MapProject = () => { if(err?.response?.status === 404) log({action: 'lookup', description: t('map_project.lookup_concept_404', {code, attempt: attempt + 1}), extras: {url: oclUrl, error: t('map_project.lookup_concept_404', {code, attempt: attempt + 1})}}, logRowIndex) }, - }).catch(err => { - // Still 404 after the retries: the code isn't there. Keep that for - // the session so other rows don't fetch it again (ocl_issues#2840). - if(err?.response?.status === 404) - return {notFound: true} - urlFetchCacheRef.current.delete(oclUrl) - return null }) - urlFetchCacheRef.current.set(oclUrl, dataPromise) - } + }) try { const data = await dataPromise + if(!isCurrent()) + return if(data?.id) { const existing = conceptCacheRef.current[key] || {} writeConceptCachePatch(key, { @@ -5168,11 +5156,12 @@ const MapProject = () => { }) } else { writeLookupFailure(key) - if(data?.notFound) - dropMissingConcept(key) + if(data === LOOKUP_NOT_FOUND) + dropMissingConcept(key, generation) } } catch (_) { - writeLookupFailure(key) + if(isCurrent()) + writeLookupFailure(key) } finally { settle(key) } @@ -5210,7 +5199,8 @@ const MapProject = () => { const item = items[i] const repoUrl = lookupConfig?.url || (item?.resolved === true ? item?.result?.url : null) if(!repoUrl) { - writeLookupFailure(key) + if(isCurrent()) + writeLookupFailure(key) settle(key) return } @@ -5221,14 +5211,15 @@ const MapProject = () => { }) .catch(() => { toResolve.forEach(({key}) => { - writeLookupFailure(key) + if(isCurrent()) + writeLookupFailure(key) settle(key) }) }) } await Promise.all([directPromise, resolvePromise, ...pending]) - }, [buildProjectContext, writeConceptCachePatch, writeLookupFailure, dropMissingConcept, lookupConfig?.token]) + }, [buildProjectContext, writeConceptCachePatch, writeLookupFailure, dropMissingConcept, lookupConfig?.url, lookupConfig?.token]) // Wire the forward-ref consumed by mergeIntoRowMatchState. useEffect // so the ref always points at the latest closure (ensureLoaded captures @@ -5238,11 +5229,26 @@ const MapProject = () => { ensureLoadedRef.current = ensureLoaded }, [ensureLoaded]) - // A code missing from one target version may be in another, so the - // session's 404s only hold for the version they were fetched against. + // On a target version change, redo interrupted lookups and bring back the + // bridge candidates dropped under the old version. React.useEffect(() => { - missingConceptKeysRef.current = new Set() - urlFetchCacheRef.current = new Map() + const interrupted = new Set(inFlightLookupsRef.current.keys()) + inFlightLookupsRef.current = new Map() + const { rowMatchState: nextState, restored } = lookupSessionRef.current.reset(rowMatchStateRef.current) + if(nextState !== rowMatchStateRef.current) { + rowMatchStateRef.current = nextState + setRowMatchState(nextState) + } + Object.entries(nextState || {}).forEach(([idx, rowState]) => { + const keysToLoad = uniq([ + ...(restored[idx] || []), + ...Object.keys(rowState?.concept_rows || {}).filter(key => interrupted.has(key)) + ]) + if(keysToLoad.length && ensureLoadedRef.current) + ensureLoadedRef.current(keysToLoad, Number(idx)).then(() => { + if(scheduleRerankRef.current) scheduleRerankRef.current(Number(idx)) + }) + }) }, [selectedTargetRepoVersion, lookupConfig?.url]) // Lazily enrich a row's saved candidates when it is opened. The persisted diff --git a/src/components/map-projects/__tests__/lookupSession.test.js b/src/components/map-projects/__tests__/lookupSession.test.js new file mode 100644 index 00000000..b527629a --- /dev/null +++ b/src/components/map-projects/__tests__/lookupSession.test.js @@ -0,0 +1,95 @@ +import test from 'node:test' +import assert from 'node:assert/strict' + +import { createLookupSession, LOOKUP_NOT_FOUND } from '../lookupSession.js' +import { restoreBridgeTargets } from '../normalizers.js' + +const notFound = () => Object.assign(new Error('Not Found'), {response: {status: 404}}) + +const deferred = () => { + let resolve, reject + const promise = new Promise((res, rej) => { resolve = res; reject = rej }) + return {promise, resolve, reject} +} + +const bridgeRow = () => ({ + algorithm_responses: {}, + candidates: { + p: {id: 'p', type: 'bridge', concept_key: 'ciel|1'}, + a: {id: 'a', type: 'bridge_child', concept_key: 'loinc|missing', parent_candidate_id: 'p'}, + b: {id: 'b', type: 'bridge_child', concept_key: 'loinc|present', parent_candidate_id: 'p'} + }, + concept_rows: {'ciel|1': {}, 'loinc|missing': {concept_key: 'loinc|missing'}, 'loinc|present': {}} +}) + +test('fetchOnce fetches a URL once and keeps a final 404', async () => { + const session = createLookupSession() + let calls = 0 + const fetcher = () => { calls += 1; return Promise.reject(notFound()) } + assert.equal(await session.fetchOnce('/c/1/', fetcher), LOOKUP_NOT_FOUND) + assert.equal(await session.fetchOnce('/c/1/', fetcher), LOOKUP_NOT_FOUND) + assert.equal(calls, 1) +}) + +test('fetchOnce keeps other failures retryable', async () => { + const session = createLookupSession() + let calls = 0 + const fetcher = () => { calls += 1; return Promise.reject(new Error('boom')) } + assert.equal(await session.fetchOnce('/c/1/', fetcher), null) + assert.equal(await session.fetchOnce('/c/1/', fetcher), null) + assert.equal(calls, 2) +}) + +test('reset fetches a URL found missing for the previous version again', async () => { + const session = createLookupSession() + let calls = 0 + const fetcher = () => { calls += 1; return calls === 1 ? Promise.reject(notFound()) : Promise.resolve({id: '1'}) } + assert.equal(await session.fetchOnce('/c/1/', fetcher), LOOKUP_NOT_FOUND) + session.reset({}) + assert.deepEqual(await session.fetchOnce('/c/1/', fetcher), {id: '1'}) +}) + +test('a 404 still retrying when the target version changes marks nothing missing in the new one', async () => { + const session = createLookupSession() + const lookup = deferred() + const generation = session.generation + const pending = session.fetchOnce('/c/missing/', () => lookup.promise) + + session.reset({}) + lookup.reject(notFound()) + + assert.equal(await pending, LOOKUP_NOT_FOUND) + assert.equal(session.isCurrent(generation), false) + assert.equal(session.markMissing(generation, 'loinc|missing'), false) + assert.equal(session.isMissing('loinc|missing'), false) + const row = bridgeRow() + assert.equal(session.dropMissing(0, row), row) +}) + +test('reset restores the bridge candidates dropped under the previous version', () => { + const session = createLookupSession() + assert.equal(session.markMissing(session.generation, 'loinc|missing'), true) + const dropped = session.dropMissing(0, bridgeRow()) + assert.deepEqual(Object.keys(dropped.candidates), ['p', 'b']) + + const {rowMatchState, restored} = session.reset({0: dropped}) + assert.deepEqual(Object.keys(rowMatchState[0].candidates).sort(), ['a', 'b', 'p']) + assert.deepEqual(rowMatchState[0].concept_rows['loinc|missing'], {concept_key: 'loinc|missing'}) + assert.deepEqual(restored, {0: ['loinc|missing']}) + assert.equal(session.isMissing('loinc|missing'), false) + assert.deepEqual(session.reset(rowMatchState).restored, {}) +}) + +test('reset leaves rows unchanged when nothing was dropped', () => { + const session = createLookupSession() + const state = {0: bridgeRow()} + const result = session.reset(state) + assert.equal(result.rowMatchState, state) + assert.deepEqual(result.restored, {}) +}) + +test('restoreBridgeTargets skips candidates whose bridge parent is gone', () => { + const row = {candidates: {q: {id: 'q', type: 'bridge', concept_key: 'ciel|2'}}, concept_rows: {'ciel|2': {}}} + const dropped = {candidates: {a: {id: 'a', type: 'bridge_child', concept_key: 'loinc|x', parent_candidate_id: 'p'}}, concept_rows: {}} + assert.equal(restoreBridgeTargets(row, dropped), row) +}) diff --git a/src/components/map-projects/__tests__/normalizers.test.js b/src/components/map-projects/__tests__/normalizers.test.js index e40f29ab..898f5c04 100644 --- a/src/components/map-projects/__tests__/normalizers.test.js +++ b/src/components/map-projects/__tests__/normalizers.test.js @@ -1787,7 +1787,7 @@ test('getAlgorithmStagesFromResponses: done (1) when any response succeeded, fai }) // --------------------------------------------------------------------------- -// dropMissingBridgeTargets (ocl_issues#2840) +// dropMissingBridgeTargets // --------------------------------------------------------------------------- test('dropMissingBridgeTargets drops bridge_child candidates for missing codes and their concept_rows', () => { diff --git a/src/components/map-projects/lookupSession.js b/src/components/map-projects/lookupSession.js new file mode 100644 index 00000000..664e79f6 --- /dev/null +++ b/src/components/map-projects/lookupSession.js @@ -0,0 +1,87 @@ +/** + * Lookup state for one target version: fetched concept URLs (404s included), + * the concept_keys found missing, and the bridge_child candidates dropped for + * them. reset() starts a new generation and restores those candidates. + */ +import { dropMissingBridgeTargets, restoreBridgeTargets } from './normalizers.js' + +export const LOOKUP_NOT_FOUND = Object.freeze({notFound: true}) + +const isNotFound = err => err?.response?.status === 404 + +export const createLookupSession = () => { + let generation = 0 + let missingKeys = new Set() + let urlFetches = new Map() + let dropped = {} + + const recordDropped = (rowIndex, before, after) => { + const prev = dropped[rowIndex] || {candidates: {}, concept_rows: {}} + const candidates = {...prev.candidates} + const conceptRows = {...prev.concept_rows} + Object.entries(before.candidates || {}).forEach(([id, c]) => { + if(!after.candidates[id]) candidates[id] = c + }) + Object.entries(before.concept_rows || {}).forEach(([key, cr]) => { + if(!after.concept_rows[key]) conceptRows[key] = cr + }) + dropped[rowIndex] = {candidates, concept_rows: conceptRows} + } + + return { + get generation() { return generation }, + isCurrent: gen => gen === generation, + isMissing: key => missingKeys.has(key), + + // Keeps a found concept or a final 404; anything else stays retryable. + fetchOnce(url, fetcher) { + const cache = urlFetches + if(cache.has(url)) return cache.get(url) + const promise = Promise.resolve() + .then(fetcher) + .then(data => { + if(data?.id) return data + cache.delete(url) + return null + }, err => { + if(isNotFound(err)) return LOOKUP_NOT_FOUND + cache.delete(url) + return null + }) + cache.set(url, promise) + return promise + }, + + markMissing(gen, key) { + if(gen !== generation) return false + missingKeys.add(key) + return true + }, + + dropMissing(rowIndex, rowState) { + const kept = dropMissingBridgeTargets(rowState, missingKeys) + if(kept !== rowState) recordDropped(rowIndex, rowState, kept) + return kept + }, + + reset(rowMatchState) { + generation += 1 + missingKeys = new Set() + urlFetches = new Map() + const toRestore = dropped + dropped = {} + let rows = rowMatchState + const restored = {} + Object.entries(toRestore).forEach(([rowIndex, removed]) => { + const rowState = rows?.[rowIndex] + const next = restoreBridgeTargets(rowState, removed) + if(next === rowState) return + rows = {...rows, [rowIndex]: next} + restored[rowIndex] = Object.keys(next.candidates) + .filter(id => !rowState.candidates[id]) + .map(id => next.candidates[id].concept_key) + }) + return {rowMatchState: rows, restored} + }, + } +} diff --git a/src/components/map-projects/normalizers.js b/src/components/map-projects/normalizers.js index 50d174f6..024b388d 100644 --- a/src/components/map-projects/normalizers.js +++ b/src/components/map-projects/normalizers.js @@ -455,14 +455,9 @@ export const normalizeAlgorithmInvocation = (rawPayload, ctx = {}) => { } /** - * Drop the bridge_child candidates whose concept the target version doesn't - * have (its lookup came back 404), and the concept_rows nothing references - * after that. The bridge filters cascade targets by the target repo, not its - * pinned version, so it can propose codes the version lacks (ocl_issues#2840). - * Returns the same rowState when nothing is dropped. - * - * @param {Object} rowState {algorithm_responses, candidates, concept_rows} - * @param {Set} missingKeys concept_keys known to be missing. + * Drop bridge_child candidates for concept_keys missing from the target + * version, and the concept_rows left unreferenced. Returns the same rowState + * when nothing is dropped. */ export const dropMissingBridgeTargets = (rowState, missingKeys) => { if (!rowState?.candidates || !missingKeys?.size) return rowState @@ -481,6 +476,26 @@ export const dropMissingBridgeTargets = (rowState, missingKeys) => { return { ...rowState, candidates, concept_rows: conceptRows } } +/** + * Put back candidates dropMissingBridgeTargets removed, unless their bridge + * parent is gone. Returns the same rowState when nothing is restored. + */ +export const restoreBridgeTargets = (rowState, dropped) => { + if (!rowState?.candidates || !dropped?.candidates) return rowState + const candidates = { ...rowState.candidates } + const conceptRows = { ...(rowState.concept_rows || {}) } + let restored = false + Object.entries(dropped.candidates).forEach(([id, c]) => { + if (candidates[id] || !candidates[c?.parent_candidate_id]) return + candidates[id] = c + if (!conceptRows[c.concept_key]) + conceptRows[c.concept_key] = dropped.concept_rows?.[c.concept_key] || { concept_key: c.concept_key, rerank_score: undefined } + restored = true + }) + if (!restored) return rowState + return { ...rowState, candidates, concept_rows: conceptRows } +} + const LOOKUP_RANK = { pending: 0, failed: 0, partial: 1, full: 2 } export const lookupStatusRank = (status) => LOOKUP_RANK[status] ?? 0