OpenConceptLab/ocl_issues#2840 | Mapper caches lookup 404s and drops bridge targets missing from the target version

src/components/map-projects/MapProject.jsx

@@ -118,6 +118,7 @@ import { formatRowNumbers, rowNumberOf, runMatchBatch, requestSingleMatch, runWi
 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 { createLookupSession, LOOKUP_NOT_FOUND } from './lookupSession'
 import { parseConceptKey } from './conceptKey'
 import { normalizeLookupConfig } from './lookupConfigFields'
 import { getDeclaredLocales, getDefaultTargetRepoVersion, getProjectTargetRepoVersion, getSavedTargetRepoVersion, getTargetRepoVersionFromUrl, getTargetRepoVersionId } from './projectTargetRepo'
@@ -537,7 +538,10 @@ const MapProject = () => {
         : cr
     })
 
-    rowMatchStateRef.current = { ...prevAll, [rowIndex]: nextRow }
+    // 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)
 
     // 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' && !lookupSessionRef.current.isMissing(d.key))
         .map(d => d.key)
       if(keysToLoad.length) {
         ensureLoadedRef.current(keysToLoad, rowIndex, isRunTraffic).then(() => {
@@ -595,15 +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<data|null>: 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 entries persist
-  // for the session.
-  const urlFetchCacheRef = React.useRef(new Map())
+  // 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
@@ -5017,6 +5018,24 @@ const MapProject = () => {
     writeConceptCachePatch(key, { ...existing, lookup_status: 'failed' })
   }, [writeConceptCachePatch])
 
+  // 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] = session.dropMissing(idx, rowState)
+      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()
@@ -5030,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 = []
@@ -5040,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) {
@@ -5067,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://')) {
@@ -5101,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,
@@ -5118,15 +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(() => {
-          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, {
@@ -5139,9 +5156,12 @@ const MapProject = () => {
           })
         } else {
           writeLookupFailure(key)
+          if(data === LOOKUP_NOT_FOUND)
+            dropMissingConcept(key, generation)
         }
       } catch (_) {
-        writeLookupFailure(key)
+        if(isCurrent())
+          writeLookupFailure(key)
       } finally {
         settle(key)
       }
@@ -5179,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
             }
@@ -5190,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, 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
@@ -5207,6 +5229,28 @@ const MapProject = () => {
     ensureLoadedRef.current = ensureLoaded
   }, [ensureLoaded])
 
+  // On a target version change, redo interrupted lookups and bring back the
+  // bridge candidates dropped under the old version.
+  React.useEffect(() => {
+    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
   // v2 candidates payload intentionally carries sparse ConceptDefinitions
   // ('partial'/'pending' — no descriptions, and for migrated projects no

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)
+})

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
+// ---------------------------------------------------------------------------
+
+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)
+})