From 130bcc7e88252e3e5a5fd1bbf6fa2c67d27f8950 Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Sun, 1 Nov 2026 18:25:56 -0600 Subject: [PATCH 01/18] provider: index supplements by the URL they supplement instead of scanning every CodeSystem loadSupplements() walked every loaded CodeSystem (thousands, from the npm packages) and every request resource on each code-system lookup, several times per $validate-code: 18-24% of the CPU time under IG-build traffic. The provider's CodeSystem map now keeps an index of supplements by supplemented URL, maintained on set/delete (data kept with the map, not a cache), and a worker filters the request's supplement resources once per request. Replaying IG-build traffic against one process: at 100 requests/s the process was busy 51% of the time instead of 94%, and it kept up with about 160 requests/s instead of 80. Co-Authored-By: Claude Opus 5.5 --- tests/tx/codesystem-map.test.js | 42 ++++++++++++++++++++++ tx/library.js | 4 +-- tx/provider.js | 63 +++++++++++++++++++++++++++++++-- tx/workers/worker.js | 9 ++++- 4 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 tests/tx/codesystem-map.test.js diff --git a/tests/tx/codesystem-map.test.js b/tests/tx/codesystem-map.test.js new file mode 100644 index 00000000..03d88ab3 --- /dev/null +++ b/tests/tx/codesystem-map.test.js @@ -0,0 +1,42 @@ +/** + * CodeSystemMap keeps an index of supplements by the URL they supplement, maintained on every + * set/delete, so Provider.loadSupplements() does not scan every loaded CodeSystem per lookup. + */ +const { CodeSystemMap } = require('../../tx/provider'); + +const cs = (url, version, extra = {}) => ({ url, version, vurl: url + '|' + version, jsonObj: { url, version, content: 'complete', ...extra } }); +const supp = (url, version, of) => cs(url, version, { content: 'supplement', supplements: of }); + +describe('CodeSystemMap supplement index', () => { + test('indexes supplements by the supplemented URL, without its version', () => { + const m = new CodeSystemMap(); + m.set('http://x/base', cs('http://x/base', '1')); + m.set('http://x/s1', supp('http://x/s1', '1', 'http://x/base')); + m.set('http://x/s2|2', supp('http://x/s2', '2', 'http://x/base|1')); + m.set('http://x/other', supp('http://x/other', '1', 'http://y/base')); + expect([...m.supplementsOf('http://x/base')].map((c) => c.url).sort()).toEqual(['http://x/s1', 'http://x/s2']); + expect([...m.supplementsOf('http://y/base')].map((c) => c.url)).toEqual(['http://x/other']); + expect([...m.supplementsOf('http://z')]).toEqual([]); + }); + + test('replacing or deleting an entry keeps the index right', () => { + const m = new CodeSystemMap(); + m.set('k', supp('http://x/s1', '1', 'http://x/base')); + m.set('k', supp('http://x/s1', '2', 'http://x/other')); + expect([...m.supplementsOf('http://x/base')]).toEqual([]); + expect([...m.supplementsOf('http://x/other')].map((c) => c.version)).toEqual(['2']); + m.delete('k'); + expect([...m.supplementsOf('http://x/other')]).toEqual([]); + expect(m.supplementIndex.size).toBe(0); + m.set('a', supp('http://x/s1', '1', 'http://x/base')); + m.clear(); + expect([...m.supplementsOf('http://x/base')]).toEqual([]); + }); + + test('behaves as a Map', () => { + const m = new CodeSystemMap([['a', cs('http://x/a', '1')]]); + expect(m.get('a').url).toBe('http://x/a'); + expect(m.size).toBe(1); + expect(m instanceof Map).toBe(true); + }); +}); diff --git a/tx/library.js b/tx/library.js index 1e769d40..9894b709 100644 --- a/tx/library.js +++ b/tx/library.js @@ -31,7 +31,7 @@ const {IETFLanguageCodeFactory} = require("./cs/cs-lang"); const {LanguageDefinitions} = require("../library/languages"); const {VersionUtilities} = require("../library/version-utilities"); const {ListCodeSystemProvider} = require("./cs/cs-provider-list"); -const { Provider } = require("./provider"); +const { Provider, CodeSystemMap } = require("./provider"); const {I18nSupport} = require("../library/i18nsupport"); const folders = require('../library/folder-setup'); const {VSACValueSetProvider} = require("./vs/vs-vsac"); @@ -1118,7 +1118,7 @@ class Library { const provider = new Provider(); provider.i18n = this.i18n; provider.codeSystemFactories = new Map(this.codeSystemFactories); // all of them - provider.codeSystems = new Map(); + provider.codeSystems = new CodeSystemMap(); provider.valueSetProviders = []; provider.conceptMapProviders = []; provider.path = path; diff --git a/tx/provider.js b/tx/provider.js index c551072d..2ff85805 100644 --- a/tx/provider.js +++ b/tx/provider.js @@ -23,6 +23,64 @@ const { applyIdPrefix, CORE_PREFIX } = require("./library/resource-ids"); * - a list of concept map providers that can provide concept map instances * */ +/** + * The provider's CodeSystem map, with an index of the supplements it holds by the canonical URL + * they supplement. The index is maintained on every set/delete, so loadSupplements() looks at the + * few candidates instead of scanning every loaded CodeSystem on every code-system lookup (that scan + * was about a fifth of the event loop's busy time under IG-build traffic). + */ +class CodeSystemMap extends Map { + constructor(entries) { + super(); + this.supplementIndex = new Map(); + if (entries) { + for (const [k, v] of entries) this.set(k, v); + } + } + + static supplementedUrl(cs) { + const u = cs && cs.jsonObj && cs.jsonObj.content === 'supplement' ? cs.jsonObj.supplements : null; + return u ? String(u).split('|')[0] : null; + } + + set(k, v) { + if (super.has(k)) this.#unindex(k, super.get(k)); + super.set(k, v); + const u = CodeSystemMap.supplementedUrl(v); + if (u) { + let m = this.supplementIndex.get(u); + if (!m) this.supplementIndex.set(u, m = new Map()); + m.set(k, v); + } + return this; + } + + delete(k) { + if (super.has(k)) this.#unindex(k, super.get(k)); + return super.delete(k); + } + + clear() { + this.supplementIndex.clear(); + super.clear(); + } + + #unindex(k, v) { + const u = CodeSystemMap.supplementedUrl(v); + const m = u && this.supplementIndex.get(u); + if (m) { + m.delete(k); + if (!m.size) this.supplementIndex.delete(u); + } + } + + /** the CodeSystems held here that supplement url (any version) */ + supplementsOf(url) { + const m = this.supplementIndex.get(url); + return m ? m.values() : []; + } +} + class Provider { i18n; fhirVersion; @@ -115,7 +173,8 @@ class Provider { for (let csp of this.codeSystemFactories.values()) { csp.listSupplements(supplements, url, version, statedSupplements); } - for (let cs of this.codeSystems.values()) { + const candidates = this.codeSystems instanceof CodeSystemMap ? this.codeSystems.supplementsOf(url) : this.codeSystems.values(); + for (let cs of candidates) { if (cs.isSupplementFor(url, version)) { if (cs.isGlobalSupplement() || this.isStatedSupplement(cs, statedSupplements)) { supplements.set(cs.vurl, cs); @@ -737,4 +796,4 @@ class Provider { } -module.exports = { Provider }; +module.exports = { CodeSystemMap, Provider }; diff --git a/tx/workers/worker.js b/tx/workers/worker.js index b52e9056..4a2d3403 100644 --- a/tx/workers/worker.js +++ b/tx/workers/worker.js @@ -293,7 +293,14 @@ class TerminologyWorker { return supplements; } - for (const resource of this.additionalResources) { + // only the request's supplement CodeSystems matter here; derive them once per request (again + // if the list changes) instead of walking every request resource on every lookup + if (this._supplementSource !== this.additionalResources || this._supplementSourceLength !== this.additionalResources.length) { + this._supplementResources = this.additionalResources.filter((r) => r.resourceType === 'CodeSystem' && r instanceof CodeSystem && r.jsonObj.supplements); + this._supplementSource = this.additionalResources; + this._supplementSourceLength = this.additionalResources.length; + } + for (const resource of this._supplementResources) { this.deadCheck('loadSupplements'); if (resource.resourceType === 'CodeSystem' && resource instanceof CodeSystem) { const cs = resource; From fd18740074f19ffe9836695d49d7b89e914062ee Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Sun, 1 Nov 2026 16:00:02 -0600 Subject: [PATCH 02/18] batch-validate: yield the event loop between entries Validating an entry never yields, so a 1,000-entry batch from an IG build held the event loop for about 1 s and every request arriving meanwhile waited for it. checkAndYield before each entry; it only yields after YIELD_INTERVAL ms of compute, so small batches are unaffected. Measured on main with a real 1,000-code SNOMED CT batch running and a single-code $validate-code (4-6 ms on its own) sent every 10 ms: those lookups typically waited 0.6-0.9 s (worst 1.35 s) before, about 50 ms (worst 73-238 ms) after; the batch was no slower. Co-Authored-By: Claude Opus 5.5 --- tests/tx/batch-validate-yield.test.js | 42 +++++++++++++++++++++++++++ tx/workers/batch-validate.js | 4 +++ 2 files changed, 46 insertions(+) create mode 100644 tests/tx/batch-validate-yield.test.js diff --git a/tests/tx/batch-validate-yield.test.js b/tests/tx/batch-validate-yield.test.js new file mode 100644 index 00000000..621396f5 --- /dev/null +++ b/tests/tx/batch-validate-yield.test.js @@ -0,0 +1,42 @@ +/** + * $batch-validate-code gives the event loop a turn between entries. + * + * Validating one entry never yields by itself, so a 1,000-entry batch from an IG build used to + * hold the event loop for its whole duration (~1 s on production hardware), and every other + * request arriving meanwhile waited for it (a single lookup typically waited 0.4-0.9 s). + * processBatch now calls checkAndYield before each entry; checkAndYield only actually yields + * after YIELD_INTERVAL ms of compute, so small batches are unaffected. + */ +const request = require('supertest'); +const { getTestApp, shutdownTestApp } = require('./setup'); +const { OperationContext } = require('../../tx/operation-context'); + +const CS_URL = 'http://example.org/fhir/CodeSystem/batch-yield-cs'; +const CS = { + resourceType: 'CodeSystem', url: CS_URL, version: '1.0.0', status: 'active', content: 'complete', + concept: [{ code: 'a', display: 'Alpha' }, { code: 'b', display: 'Beta' }] +}; +const entry = (code) => ({ name: 'validation', resource: { resourceType: 'Parameters', parameter: [{ name: 'code', valueCode: code }] } }); + +describe('$batch-validate-code yields between entries', () => { + let app; + beforeAll(async () => { app = await getTestApp(); }, 60000); + afterAll(async () => { await shutdownTestApp(); }); + + test('checkAndYield is called once per entry and the results are unchanged', async () => { + const spy = jest.spyOn(OperationContext.prototype, 'checkAndYield'); + try { + const codes = ['a', 'zz', 'b', 'a', 'b']; + const res = await request(app).post('/tx/r5/CodeSystem/$batch-validate-code') + .set('Content-Type', 'application/json') + .send({ resourceType: 'Parameters', parameter: [{ name: 'tx-resource', resource: CS }, { name: 'url', valueUri: CS_URL }, ...codes.map(entry)] }); + expect(res.status).toBe(200); + const calls = spy.mock.calls.filter((c) => c[0] === 'batch-validate').length; + expect(calls).toBe(codes.length); + const results = res.body.parameter.map((r) => r.resource.parameter.find((p) => p.name === 'result').valueBoolean); + expect(results).toEqual([true, false, true, true, true]); + } finally { + spy.mockRestore(); + } + }); +}); diff --git a/tx/workers/batch-validate.js b/tx/workers/batch-validate.js index e209a8a0..a12e1fd6 100644 --- a/tx/workers/batch-validate.js +++ b/tx/workers/batch-validate.js @@ -105,6 +105,10 @@ class BatchValidateWorker extends TerminologyWorker { for (const p of params.parameter) { if (p.name == 'validation') { + // Validating an entry never yields the event loop by itself, and IG builds send batches + // of up to 1,000 entries (~1 s of compute): give other requests a turn between entries + // (checkAndYield yields only after YIELD_INTERVAL ms of compute, so small batches don't). + await this.opContext.checkAndYield('batch-validate'); let op = new Parameters(); op.jsonObj.parameter = []; for (const gp of globalParams) { From 59f1abd1d8ca7e004a7b8d35798cfab165fb72c8 Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Mon, 2 Nov 2026 01:32:11 -0600 Subject: [PATCH 03/18] operation-context: yield every 10 ms of compute instead of 25 While a long operation runs, other requests wait about two slices: 22 ms typical instead of 51 (worst 42 vs 86) during a 1,000-code batch, with no measurable cost to the batch (842 vs 879 ms). Measured with a single lookup sent every 10 ms during the batch. Co-Authored-By: Claude Opus 5.5 --- tx/operation-context.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tx/operation-context.js b/tx/operation-context.js index 29837db6..2b729750 100644 --- a/tx/operation-context.js +++ b/tx/operation-context.js @@ -824,7 +824,7 @@ const CHECK_FREQUENCY = 100; // I/O. checkAndYield() awaits a real setImmediate every YIELD_INTERVAL ms of // compute, so pending I/O (new connections, /metadata, timers) runs between // slices and one heavy $expand can no longer time out every other client. -const YIELD_INTERVAL = 25; +const YIELD_INTERVAL = 10; const { setImmediate: yieldToEventLoop } = require('timers/promises'); class OperationContext { From 0be16dd5b0169f1ad159adf7e7b0be8d614211d2 Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Sun, 1 Nov 2026 16:00:02 -0600 Subject: [PATCH 04/18] tx: send the JSON string already built instead of serialising the response twice The res.json wrapper stringified the resource to log its size and then called res.json(data), which stringified it again: about 3 ms of extra work per MB of response. Send the string (same headers: Content-Type was set above). Co-Authored-By: Claude Opus 5.5 --- tx/tx.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tx/tx.js b/tx/tx.js index 09d5ec42..f5d5630f 100644 --- a/tx/tx.js +++ b/tx/tx.js @@ -465,7 +465,8 @@ class TXModule { res.setHeader('Content-Type', jsonFmt); this.checkProperJson(jsonStr); responseSize = Buffer.byteLength(jsonStr, 'utf8'); - result = originalJson(data); + // send the string built above: res.json(data) would serialise the resource again + result = res.send(jsonStr); } // Log the request with request ID From c3a95ae61aaf833a4139fd4e2f30d5f5524ad1bc Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Sun, 1 Nov 2026 16:00:02 -0600 Subject: [PATCH 05/18] validate: answer CodeSystem/$validate-code when an Issue is raised before the inner try An unknown cache-id (and any other Issue thrown by setupAdditionalResources) fell into the 'handled in the inner method' branch of handleCodeSystem, so no response was ever sent and the socket was held until the client gave up. Answer with the OperationOutcome, as the ValueSet handler does. Co-Authored-By: Claude Opus 5.5 --- tests/tx/cache-id.test.js | 11 +++++++++++ tx/workers/validate.js | 13 ++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/tx/cache-id.test.js b/tests/tx/cache-id.test.js index a7ca4f0b..d70fc130 100644 --- a/tests/tx/cache-id.test.js +++ b/tests/tx/cache-id.test.js @@ -200,6 +200,17 @@ describe('cache-id protocol (integration)', () => { expect(isUnknownCacheError(res)).toBe(true); }); + test('CodeSystem/$validate-code with an unknown cache-id is answered (it used to never respond)', async () => { + const res = await request(app).post('/tx/r5/CodeSystem/$validate-code').timeout(10000) + .set('Content-Type', 'application/json') + .send({ resourceType: 'Parameters', parameter: [ + { name: 'cache-id', valueId: 'never-issued-this-cache' }, + { name: 'coding', valueCoding: { system: 'http://hl7.org/fhir/administrative-gender', code: 'male' } } + ] }); + expect(res.status).toBe(404); + expect(isUnknownCacheError(res)).toBe(true); + }); + test('by url with no cache-id and a VS not in the library fails (not a silent pass)', async () => { const r = await post(app, VALIDATE, [ { name: 'url', valueString: colorsVS.url }, diff --git a/tx/workers/validate.js b/tx/workers/validate.js index 8fc01f0b..9d3e9b30 100644 --- a/tx/workers/validate.js +++ b/tx/workers/validate.js @@ -2048,13 +2048,12 @@ class ValidateWorker extends TerminologyWorker { this.log.error(error); debugLog(error); if (error instanceof Issue) { - if (error.isHandleAsOO()) { - let oo = new OperationOutcome(); - oo.addIssue(error); - return res.status(error.statusCode || 500).json(oo.jsonObj); - } else { - // this is actually handled in the inner method - } + // Issues raised inside the validation are turned into a result by the inner method; + // one that reaches here (e.g. cache-id-unknown from setupAdditionalResources, before + // the inner try) must still be answered, or the request never completes. + let oo = new OperationOutcome(); + oo.addIssue(error); + return res.status(error.statusCode || 500).json(oo.jsonObj); } else { return res.status(error.statusCode || 500).json(outcomeFromError(error)); } From 0a5fce3c60c0f0cbc3b94216d715ccb4a0aaceca Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Sun, 1 Nov 2026 18:14:52 -0600 Subject: [PATCH 06/18] An edition-only SNOMED CT pin resolves to the latest loaded version of the edition registerProvider overwrote the partial-version key (the edition URI without its date, or a semver major.minor) on every load, so with several versions of an edition loaded the pin resolved to whichever was listed last. It now keeps the latest version, whatever the load order. Co-Authored-By: Claude Opus 5.5 --- tests/tx/library-partial-version.test.js | 38 ++++++++++++++++++++++++ tx/library.js | 7 ++++- 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 tests/tx/library-partial-version.test.js diff --git a/tests/tx/library-partial-version.test.js b/tests/tx/library-partial-version.test.js new file mode 100644 index 00000000..ce9845ae --- /dev/null +++ b/tests/tx/library-partial-version.test.js @@ -0,0 +1,38 @@ +'use strict'; + +// A partial version (a SNOMED CT edition URI without its date, or a semver major.minor) resolves +// to the latest version loaded for it, whatever the library's load order. + +const { Library } = require('../../tx/library'); + +const SCT = 'http://snomed.info/sct'; +const US = 'http://snomed.info/sct/731000124108'; + +function factory(system, version, partial) { + return { system: () => system, version: () => version, getPartialVersion: () => partial }; +} + +function load(order) { + const lib = new Library(null, null, { info() {}, debug() {}, warn() {}, error() {} }); + order.forEach((f, i) => lib.registerProvider('test', f, i === 0)); + return lib; +} + +describe('library: partial-version keys', () => { + const older = factory(SCT, US + '/version/20240301', US); + const newer = factory(SCT, US + '/version/20250901', US); + + test.each([['newest first', [newer, older]], ['oldest first', [older, newer]]])('SNOMED edition-only pin, %s', (_, order) => { + const lib = load(order); + expect(lib.codeSystemFactories.get(SCT + '|' + US)).toBe(newer); + // exact versions are untouched + expect(lib.codeSystemFactories.get(SCT + '|' + US + '/version/20240301')).toBe(older); + expect(lib.codeSystemFactories.get(SCT + '|' + US + '/version/20250901')).toBe(newer); + }); + + test.each([['newest first', ['1.2.10', '1.2.3']], ['oldest first', ['1.2.3', '1.2.10']]])('semver major.minor, %s', (_, vs) => { + const fs = vs.map((v) => factory('http://example.org/cs', v, '1.2')); + const lib = load(fs); + expect(lib.codeSystemFactories.get('http://example.org/cs|1.2').version()).toBe('1.2.10'); + }); +}); diff --git a/tx/library.js b/tx/library.js index 9894b709..2fedf212 100644 --- a/tx/library.js +++ b/tx/library.js @@ -108,7 +108,12 @@ class Library { this.codeSystemFactories.set(factory.system()+"|"+ver, factory); const verMin = factory.getPartialVersion(); if (verMin) { - this.codeSystemFactories.set(factory.system()+"|"+verMin, factory); + // a partial version (major.minor, or a SNOMED CT edition URI without its date) names the + // latest version loaded for it, whatever the order the library lists them in + const prev = this.codeSystemFactories.get(factory.system()+"|"+verMin); + if (!prev || VersionUtilities.compareVersionsGeneral(ver, prev.version() ?? "") > 0) { + this.codeSystemFactories.set(factory.system()+"|"+verMin, factory); + } } } From 03944f226cca7239a1eb37a1f063614fbac1b4b8 Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Sun, 1 Nov 2026 21:49:26 -0600 Subject: [PATCH 07/18] FHIR CodeSystem and country providers: anchor a regex with alternation as a whole '^' + value + '$' turned 'code1|code3' into "starts with code1 or ends with code3". The pattern is now wrapped: ^(?:value)$. Co-Authored-By: Claude Opus 5.5 --- tests/cs/cs-country.test.js | 17 +++++++++++++++++ tests/cs/cs-cs.test.js | 10 +++++++++- tx/cs/cs-country.js | 2 +- tx/cs/cs-cs.js | 4 ++-- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/tests/cs/cs-country.test.js b/tests/cs/cs-country.test.js index f16b0866..36fe1a90 100644 --- a/tests/cs/cs-country.test.js +++ b/tests/cs/cs-country.test.js @@ -261,6 +261,23 @@ describe('CountryCodeServices', () => { expect(results[0].code).toBe('US'); }); + test('a pattern with alternation matches whole codes only', async () => { + // '^' + 'US|CA' + '$' would be "starts with US or ends with CA", taking in USA + const ctxt = await provider.getPrepContext(false); + await provider.filter(ctxt, true, 'code', 'regex', 'US|CA'); + const filters = await provider.executeFilters(ctxt); + const filter = filters[0]; + + const results = []; + filter.cursor = -1; + while (await provider.filterMore(ctxt, filter)) { + const concept = await provider.filterConcept(ctxt, filter); + results.push(concept.code); + } + + expect(results.sort()).toEqual(['CA', 'US']); + }); + test('should filter all 2-letter codes', async () => { const ctxt = await provider.getPrepContext(false); await provider.filter(ctxt, true, 'code', 'regex', '[A-Z]{2}'); diff --git a/tests/cs/cs-cs.test.js b/tests/cs/cs-cs.test.js index 1f4b3747..791f0d46 100644 --- a/tests/cs/cs-cs.test.js +++ b/tests/cs/cs-cs.test.js @@ -1633,10 +1633,18 @@ describe('FHIR CodeSystem Provider', () => { expect(results.findConceptByCode('code1')).toBeNull(); }); + test('regex with alternation matches whole codes only', async () => { + // '^' + 'code1|code3' + '$' would be "starts with code1 or ends with code3" + const results = await simpleProvider.filter(filterContext, true, 'code', 'regex', 'code1|code3'); + expect(results.findConceptByCode('code1')).toBeDefined(); + expect(results.findConceptByCode('code3')).toBeDefined(); + expect(results.size()).toBe(2); + }); + test('should handle invalid regex gracefully', async () => { await expect( simpleProvider.filter(filterContext, true, 'code', 'regex', '[invalid') - ).rejects.toThrow('The regex \'[invalid\' is not valid: error parsing regexp: missing closing ]: `[invalid$`'); + ).rejects.toThrow('The regex \'[invalid\' is not valid: error parsing regexp: missing closing ]: `[invalid)$`'); }); }); diff --git a/tx/cs/cs-country.js b/tx/cs/cs-country.js index 25e75cea..aea78224 100644 --- a/tx/cs/cs-country.js +++ b/tx/cs/cs-country.js @@ -212,7 +212,7 @@ class CountryCodeServices extends CodeSystemProvider { try { // Create regex with anchors to match the Pascal implementation (^value$) - const regex = regexUtilities.compile('^' + value + '$'); + const regex = regexUtilities.compile('^(?:' + value + ')$'); for (const concept of this.codes) { if (regex.test(concept.code)) { diff --git a/tx/cs/cs-cs.js b/tx/cs/cs-cs.js index 37b14bfb..4ade57ad 100644 --- a/tx/cs/cs-cs.js +++ b/tx/cs/cs-cs.js @@ -1588,7 +1588,7 @@ class FhirCodeSystemProvider extends BaseCSServices { else if (op === 'regex') { // Regular expression match try { - const regex = regexUtilities.compile('^' + value + '$'); + const regex = regexUtilities.compile('^(?:' + value + ')$'); const allCodes = this.codeSystem.getAllCodes(); for (const code of allCodes) { this.opContext.deadCheck('cs:conceptFilter:regex'); @@ -1736,7 +1736,7 @@ class FhirCodeSystemProvider extends BaseCSServices { } else if (op === 'regex') { try { - const regex = regexUtilities.compile('^' + value + '$'); + const regex = regexUtilities.compile('^(?:' + value + ')$'); return properties.some(p => regex.test(this._getPropertyValue(p))); } catch (error) { return false; From 9ab694a6956f18d90dccd68d44e14758fb7290b7 Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Mon, 2 Nov 2026 07:45:32 -0600 Subject: [PATCH 08/18] validate: a code not in an enumerated include is located once, not once per listed code Before trying each listed concept as the same concept in another form (ICD-11's short code and entity id), checkConceptSet called sameConceptDifferentForm for every listed concept, and each call located the code to ask whether it has another form at all. That answer does not depend on the listed concept, so it is now asked once per include and the loop runs only when it is yes. Same answers. A miss against a 33,000-code VSAC RxNorm list used to get no answer within 600 s (a 1,185-code list took 99 s); a 300-code list in the new test located the code 301 times, now a handful. Co-Authored-By: Claude Opus 5.5 --- tests/tx/validate-enumerated-miss.test.js | 64 +++++++++++++++++++++++ tx/workers/validate.js | 13 ++++- 2 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 tests/tx/validate-enumerated-miss.test.js diff --git a/tests/tx/validate-enumerated-miss.test.js b/tests/tx/validate-enumerated-miss.test.js new file mode 100644 index 00000000..e52c9681 --- /dev/null +++ b/tests/tx/validate-enumerated-miss.test.js @@ -0,0 +1,64 @@ +/** + * A code that is not in an enumerated include costs one lookup of the code, not one per listed code. + * + * Before checking whether a listed concept is the same concept written in another form (ICD-11's + * short code and entity id), the checker located the code once per listed concept, though the + * answer (does the code have another form at all?) does not depend on the listed concept. A miss + * against a 33,000-code VSAC list never finished. It is now asked once per include. + */ +const request = require('supertest'); +const { getTestApp, shutdownTestApp } = require('./setup'); +const { FhirCodeSystemProvider } = require('../../tx/cs/cs-cs'); + +const CS_URL = 'http://example.org/fhir/CodeSystem/enumerated-miss-cs'; +const VS_URL = 'http://example.org/fhir/ValueSet/enumerated-miss-vs'; +const N = 300; +const codes = Array.from({ length: N }, (_, i) => 'c' + i); +const CS = { + resourceType: 'CodeSystem', url: CS_URL, version: '1.0.0', status: 'active', content: 'complete', + concept: [...codes, 'unlisted'].map(code => ({ code, display: 'Display ' + code })) +}; +const VS = { + resourceType: 'ValueSet', url: VS_URL, version: '1.0.0', status: 'active', + compose: { include: [{ system: CS_URL, concept: codes.map(code => ({ code })) }] } +}; + +async function validate(app, code) { + return request(app).post('/tx/r5/ValueSet/$validate-code') + .set('Content-Type', 'application/json') + .send({ resourceType: 'Parameters', parameter: [ + { name: 'tx-resource', resource: CS }, + { name: 'valueSet', resource: VS }, + { name: 'coding', valueCoding: { system: CS_URL, code } } + ] }); +} + +const resultOf = (res) => res.body.parameter.find(p => p.name === 'result').valueBoolean; + +describe('$validate-code: a miss against an enumerated include', () => { + let app; + beforeAll(async () => { app = await getTestApp(); }, 60000); + afterAll(async () => { await shutdownTestApp(); }); + + test.each([ + ['in the code system but not listed', 'unlisted'], + ['not in the code system', 'no-such-code'] + ])('a code %s is located a handful of times, not once per listed code', async (_label, code) => { + const spy = jest.spyOn(FhirCodeSystemProvider.prototype, 'locate'); + try { + const res = await validate(app, code); + expect(res.status).toBe(200); + expect(resultOf(res)).toBe(false); + const lookupsOfCode = spy.mock.calls.filter(c => c[0] === code).length; + expect(lookupsOfCode).toBeLessThan(10); + } finally { + spy.mockRestore(); + } + }); + + test('a listed code still validates', async () => { + const res = await validate(app, 'c150'); + expect(res.status).toBe(200); + expect(resultOf(res)).toBe(true); + }); +}); diff --git a/tx/workers/validate.js b/tx/workers/validate.js index 9d3e9b30..9b72e8c8 100644 --- a/tx/workers/validate.js +++ b/tx/workers/validate.js @@ -1617,6 +1617,15 @@ class ValueSetChecker { return !!b.context && (await cs.code(a.context)) === (await cs.code(b.context)); } + /** + * True when the code has a second spelling at all (a classification code), so that an + * enumerated concept could be the same concept written differently. + */ + async hasOtherForm(cs, code) { + const a = await cs.locate(code); + return !!a.context && !!(await cs.classificationCode(a.context)); + } + async noteClassificationCode(cs, context, code, normalForm) { // the code system decides whether this spelling of the code earns a classificationCode // (ICD-11: when it was written with entity ids, in any order - not when it was written @@ -1734,7 +1743,9 @@ class ValueSetChecker { // an enumerated concept may be written in the other form (ICD-11 short code vs uri) let altMatch = null; - if (cset.concept && !cset.concept.some(cc => cc.code === code)) { + // whether the code has another form does not depend on the listed concept: ask it once + // here, not once per listed concept (a locate per listed code on every miss) + if (cset.concept && !cset.concept.some(cc => cc.code === code) && (await this.hasOtherForm(cs, code))) { for (const cc of cset.concept) { if (await this.sameConceptDifferentForm(cs, code, cc.code)) { altMatch = cc; From 528fc32471f2c7e559512ce257c6dc78f3eac3a1 Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Mon, 2 Nov 2026 07:45:32 -0600 Subject: [PATCH 09/18] VSAC: clamp a refresh interval longer than a timer can wait Node fires a timer whose delay is above 2^31-1 ms (about 596 hours) after 1 ms instead, so a refreshIntervalHours above that ran the VSAC refresh back to back (100,000 hours gave thousands of unhandled rejections within seconds). The interval is clamped to the longest a timer can wait, and a value that is not a positive number falls back to the 24 hour default, each with a warning. Co-Authored-By: Claude Opus 5.5 --- tests/vs/vs-vsac-refresh-interval.test.js | 49 +++++++++++++++++++++++ tx/vs/vs-vsac.js | 25 +++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 tests/vs/vs-vsac-refresh-interval.test.js diff --git a/tests/vs/vs-vsac-refresh-interval.test.js b/tests/vs/vs-vsac-refresh-interval.test.js new file mode 100644 index 00000000..650dbf79 --- /dev/null +++ b/tests/vs/vs-vsac-refresh-interval.test.js @@ -0,0 +1,49 @@ +// The VSAC refresh timer: Node fires a timer whose delay is above 2^31-1 ms (about 596 hours) after +// 1 ms instead, so a long refreshIntervalHours used to run the refresh back to back + +const { VSACValueSetProvider } = require('../../tx/vs/vs-vsac'); + +function provider(refreshIntervalHours) { + const p = Object.create(VSACValueSetProvider.prototype); + p.refreshIntervalHours = refreshIntervalHours; + p.refreshTimer = null; + return p; +} + +describe('VSAC refresh interval', () => { + let warn; + beforeEach(() => { + warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => warn.mockRestore()); + + test('an ordinary interval is used as given', () => { + expect(provider(24)._refreshIntervalMs()).toBe(24 * 3600 * 1000); + expect(provider(0.5)._refreshIntervalMs()).toBe(1800 * 1000); + expect(warn).not.toHaveBeenCalled(); + }); + + test('an interval longer than a timer can wait is clamped, not turned into 1 ms', () => { + expect(provider(100000)._refreshIntervalMs()).toBe(2 ** 31 - 1); + expect(warn).toHaveBeenCalledTimes(1); + }); + + test('a value that is not a positive number falls back to 24 hours', () => { + for (const h of [-1, 'abc', NaN, Infinity]) { + expect(provider(h)._refreshIntervalMs()).toBe(24 * 3600 * 1000); + } + }); + + test('the timer is started with the clamped delay', () => { + const spy = jest.spyOn(global, 'setInterval'); + const p = provider(100000); + try { + p._startRefreshTimer(); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy.mock.calls[0][1]).toBe(2 ** 31 - 1); + } finally { + clearInterval(p.refreshTimer); + spy.mockRestore(); + } + }); +}); diff --git a/tx/vs/vs-vsac.js b/tx/vs/vs-vsac.js index 337a3573..cea52c95 100644 --- a/tx/vs/vs-vsac.js +++ b/tx/vs/vs-vsac.js @@ -11,6 +11,9 @@ const { applyIdPrefix, stripIdPrefix } = require("../library/resource-ids"); // Persisted watermark for the phase-1b _lastUpdated scan. const VSAC_LAST_UPDATED_KEY = 'vsac_last_updated_date'; +// The longest delay a Node timer accepts; a longer one fires after 1 ms. +const MAX_TIMER_MS = 2 ** 31 - 1; + // Canonical URL prefix for VSAC value sets, so operators can enter a bare OID. const VSAC_VALUESET_URL_PREFIX = 'http://cts.nlm.nih.gov/fhir/ValueSet/'; @@ -115,7 +118,7 @@ class VSACValueSetProvider extends AbstractValueSetProvider { clearInterval(this.refreshTimer); } - const intervalMs = this.refreshIntervalHours * 60 * 60 * 1000; + const intervalMs = this._refreshIntervalMs(); this.refreshTimer = setInterval(async () => { try { await this.refreshValueSets(); @@ -126,6 +129,26 @@ class VSACValueSetProvider extends AbstractValueSetProvider { }, intervalMs); } + /** + * The refresh interval in ms. Node runs a timer whose delay is above 2^31-1 ms (about 596 hours) + * or not a positive number after 1 ms instead, so the refresh would run back to back: such a + * setting is clamped (or, when it isn't a positive number, replaced by the 24 hour default). + * @private + */ + _refreshIntervalMs() { + const hours = Number(this.refreshIntervalHours); + if (!Number.isFinite(hours) || hours <= 0) { + console.warn(`VSAC: refreshIntervalHours ${this.refreshIntervalHours} is not a positive number; using 24`); + return 24 * 60 * 60 * 1000; + } + const ms = hours * 60 * 60 * 1000; + if (ms > MAX_TIMER_MS) { + console.warn(`VSAC: refreshIntervalHours ${hours} is longer than a timer can wait; using ${Math.floor(MAX_TIMER_MS / 3600000)} hours`); + return MAX_TIMER_MS; + } + return ms; + } + /** * Stop the refresh timer (for cleanup) */ From 2780b8133031e3bbe41122654a8812f947ca704b Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Mon, 2 Nov 2026 07:48:34 -0600 Subject: [PATCH 10/18] server: keep idle connections open longer than nginx does Node closes an idle keep-alive connection after 5 s; nginx keeps idle upstream connections for 60 s. When nginx reuses a connection just as Node closes it, the request is reset, and nginx does not retry a POST, so the client gets a 502 (seen in load tests as bursts of resets on reused connections, up to 2% of requests at some steps). keepAliveTimeout is now 65 s, and headersTimeout 66 s, above it as Node requires. Co-Authored-By: Claude Opus 5.5 --- library/keep-alive.js | 18 ++++++++++++++ server.js | 5 +++- tests/library/keep-alive.test.js | 40 ++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 library/keep-alive.js create mode 100644 tests/library/keep-alive.test.js diff --git a/library/keep-alive.js b/library/keep-alive.js new file mode 100644 index 00000000..06eb65af --- /dev/null +++ b/library/keep-alive.js @@ -0,0 +1,18 @@ +// Keep-alive timeouts for the HTTP server behind a reverse proxy. +// +// nginx keeps idle upstream connections open for 60 s (keepalive_timeout in an upstream block), +// while Node closes an idle one after 5 s. When nginx reuses a connection at the moment Node +// closes it, the request fails with a reset, and nginx does not retry a POST: the client gets a +// 502. Node must therefore keep idle connections open longer than the proxy does, and +// headersTimeout must be longer still, or Node applies it to the idle connection first. + +const KEEP_ALIVE_TIMEOUT_MS = 65000; +const HEADERS_TIMEOUT_MS = 66000; + +function applyKeepAliveTimeouts(server) { + server.keepAliveTimeout = KEEP_ALIVE_TIMEOUT_MS; + server.headersTimeout = HEADERS_TIMEOUT_MS; + return server; +} + +module.exports = { applyKeepAliveTimeouts, KEEP_ALIVE_TIMEOUT_MS, HEADERS_TIMEOUT_MS }; diff --git a/server.js b/server.js index cc724cd9..442d7e12 100644 --- a/server.js +++ b/server.js @@ -18,6 +18,7 @@ const escape = require('escape-html'); const { resolveWithin } = require('./library/path-safety'); const { readCgroupMemoryLimit } = require('./library/cgroup-memory'); const memoryGuard = require('./library/memory-guard'); +const { applyKeepAliveTimeouts } = require('./library/keep-alive'); const { NpmAudit } = require('./library/npm-audit'); const Mailer = require('./library/mailer'); const DailyReport = require('./library/daily-report'); @@ -836,10 +837,12 @@ async function startServer() { }); // Start server - app.listen(PORT, () => { + const server = app.listen(PORT, () => { stats.markStarted(packageJson.version); serverLog.info(`=== Server running on http://localhost:${PORT} ===`); }); + // idle connections stay open longer than nginx keeps them (see library/keep-alive.js) + applyKeepAliveTimeouts(server); npmAudit.start(); dailyReport = new DailyReport(config.dailyReport, { log: Logger.getInstance().child({ module: 'daily-report' }), diff --git a/tests/library/keep-alive.test.js b/tests/library/keep-alive.test.js new file mode 100644 index 00000000..41a53f38 --- /dev/null +++ b/tests/library/keep-alive.test.js @@ -0,0 +1,40 @@ +// Idle connections must outlive the reverse proxy's (nginx: 60 s), or nginx reuses a connection +// Node is closing and a POST on it fails with a 502. Node's default is 5 s. + +const http = require('http'); +const net = require('net'); +const { applyKeepAliveTimeouts, KEEP_ALIVE_TIMEOUT_MS, HEADERS_TIMEOUT_MS } = require('../../library/keep-alive'); + +describe('keep-alive timeouts', () => { + test('idle connections stay open longer than nginx keeps them, and headersTimeout is longer still', () => { + const server = applyKeepAliveTimeouts(http.createServer()); + expect(server.keepAliveTimeout).toBe(KEEP_ALIVE_TIMEOUT_MS); + expect(server.keepAliveTimeout).toBeGreaterThan(60000); + expect(server.headersTimeout).toBe(HEADERS_TIMEOUT_MS); + expect(server.headersTimeout).toBeGreaterThan(server.keepAliveTimeout); + }); + + test('a connection idle for longer than Node\'s 5 s default can still be reused', async () => { + const server = applyKeepAliveTimeouts(http.createServer((req, res) => res.end('ok'))); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + const socket = net.connect(server.address().port, '127.0.0.1'); + let closed = false; + socket.on('close', () => { closed = true; }); + const send = () => new Promise((resolve, reject) => { + socket.once('data', d => resolve(d.toString())); + socket.once('error', reject); + socket.once('close', () => reject(new Error('connection closed'))); + socket.write('GET / HTTP/1.1\r\nHost: x\r\nConnection: keep-alive\r\n\r\n'); + }); + try { + expect(await send()).toMatch(/^HTTP\/1.1 200/); + await new Promise(resolve => setTimeout(resolve, 6000)); + expect(closed).toBe(false); + expect(await send()).toMatch(/^HTTP\/1.1 200/); + } finally { + socket.destroy(); + server.closeAllConnections(); + await new Promise(resolve => server.close(resolve)); + } + }, 15000); +}); From d368a6d154f949ac7c7b2d14e15ec9640ca1ce6b Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Mon, 2 Nov 2026 07:48:54 -0600 Subject: [PATCH 11/18] ConceptUsageTracker: yield during the startup scan The scan that runs just after startup reads every value set (about 4,000 with tx.fhir.org's sources) and held the event loop for about a second, so requests arriving then waited for all of it. It now gives the event loop a turn every 10 ms of work. Same counts. Co-Authored-By: Claude Opus 5.5 --- tests/tx/usage-tracker-yield.test.js | 37 ++++++++++++++++++++++++++++ tx/usage-tracker.js | 11 +++++++++ 2 files changed, 48 insertions(+) create mode 100644 tests/tx/usage-tracker-yield.test.js diff --git a/tests/tx/usage-tracker-yield.test.js b/tests/tx/usage-tracker-yield.test.js new file mode 100644 index 00000000..6717d83f --- /dev/null +++ b/tests/tx/usage-tracker-yield.test.js @@ -0,0 +1,37 @@ +// The ConceptUsageTracker's startup scan reads every value set, about a second of work on a full +// source list; it used to run without a break, so requests arriving just after startup waited +// for all of it. It now gives the event loop a turn every few milliseconds. + +const ConceptUsageTracker = require('../../tx/usage-tracker'); + +function busy(ms) { + const end = Date.now() + ms; + while (Date.now() < end) { /* compute */ } +} + +function libraryOf(count) { + const vs = (i) => ({ jsonObj: { compose: { include: [{ system: 'http://example.org/cs', concept: [{ code: 'c' + (i % 7) }] }] } } }); + return { + valueSetProviders: [{ + listAllValueSets: async () => Array.from({ length: count }, (_, i) => 'http://example.org/vs/' + i), + fetchValueSet: async (url) => { busy(1); return vs(Number(url.split('/').pop())); } + }] + }; +} + +describe('ConceptUsageTracker scan', () => { + test('other work runs during the scan, and the counts are unchanged', async () => { + const tracker = new ConceptUsageTracker(); + let ranDuringScan = 0; + let scanning = true; + const tick = () => { if (scanning) { ranDuringScan++; setImmediate(tick); } }; + setImmediate(tick); + const count = await tracker.scanValueSets(libraryOf(100)); + scanning = false; + expect(count).toBe(100); + expect(ranDuringScan).toBeGreaterThan(3); + const usages = tracker.usages('http://example.org/cs'); + expect(usages.get('c0').count).toBe(15); + expect(usages.get('c6').count).toBe(14); + }); +}); diff --git a/tx/usage-tracker.js b/tx/usage-tracker.js index 4fb0c7f3..39229c4e 100644 --- a/tx/usage-tracker.js +++ b/tx/usage-tracker.js @@ -1,4 +1,10 @@ +const { setImmediate: yieldToEventLoop } = require('timers/promises'); + +// The startup scan reads every value set (thousands with the npm packages, VSAC), about a second +// of work: give requests a turn every YIELD_INTERVAL_MS of it. +const YIELD_INTERVAL_MS = 10; + class ConceptUsageTracker { constructor() { @@ -7,9 +13,14 @@ class ConceptUsageTracker { async scanValueSets(library) { let c = 0; + let sliceStart = Date.now(); for (let vsp of library.valueSetProviders) { let list = await vsp.listAllValueSets(); for (let url of list) { + if (Date.now() - sliceStart >= YIELD_INTERVAL_MS) { + await yieldToEventLoop(); + sliceStart = Date.now(); + } let vs = await vsp.fetchValueSet(url); if (vs && vs.jsonObj.compose) { if (await this.scanValueSet(vs.jsonObj.compose)) { From 393ab486fbd746cb574e09fb7c8f05543bf9b88f Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Tue, 3 Nov 2026 07:07:39 -0600 Subject: [PATCH 12/18] LOINC importer: an externally defined answer list adds no blank Answer code AnswerList.csv has one row with no AnswerStringId for each externally defined answer list (ExtDefinedYN = Y; 65 in 2.83). Each such row was imported as an answer whose code is '', shared by all of them and linked to all 65 lists, so LOINC 2.83 had 257,267 codes rather than 257,266 (the extra one is code '', type Answer, no display). The list itself is still a code. Co-Authored-By: Claude Opus 5.5 --- tests/tx/loinc-import-answer-lists.test.js | 46 ++++++++++++++++++++++ tx/importers/import-loinc.module.js | 6 +++ 2 files changed, 52 insertions(+) create mode 100644 tests/tx/loinc-import-answer-lists.test.js diff --git a/tests/tx/loinc-import-answer-lists.test.js b/tests/tx/loinc-import-answer-lists.test.js new file mode 100644 index 00000000..33f974b2 --- /dev/null +++ b/tests/tx/loinc-import-answer-lists.test.js @@ -0,0 +1,46 @@ +// LOINC importer: an externally defined answer list (no AnswerStringId) adds the list, and no +// blank Answer code. AnswerList.csv in 2.83 has 65 such rows; they all became one Answer whose +// code is '', so LOINC had one code too many (257,267 rather than 257,266) +const { LoincDataMigrator } = require('../../tx/importers/import-loinc.module'); + +function migrator() { + const m = new LoincDataMigrator(); + m.codeKey = 0; + m.codes = new Map(); + m.rels = new Map([['Answer', 40], ['AnswerList', 39]]); + m.relationships = []; + m.addRelationship = (db, s, t, r) => m.relationships.push([s, t, r]); + m.addExternalMapping = () => {}; + return m; +} + +function db() { + const inserts = []; + return { inserts, run: (sql, params) => inserts.push(params) }; +} + +// AnswerListId, AnswerListName, AnswerListOID, ExtDefinedYN, ExtDefinedAnswerListCodeSystem, +// ExtDefinedAnswerListLink, AnswerStringId, LocalAnswerCode, LocalAnswerCodeSystem, SequenceNumber, DisplayText, ... +const row = (list, name, answer, display) => [list, name, '', answer ? 'N' : 'Y', '', '', answer, '', '', '1', display, '', '', '', '', '', '', '', '']; + +describe('LOINC import: answer lists', () => { + test('an externally defined list (no AnswerStringId) is a code, with no blank answer', async () => { + const m = migrator(); + const d = db(); + await m.processListItem(d, row('LL1892-0', 'ICD-9_ICD-10', '', ''), null); + await m.processListItem(d, row('LL1894-6', 'RxNorm', '', ''), 'LL1892-0'); + expect(d.inserts.map(p => p[1])).toEqual(['LL1892-0', 'LL1894-6']); + expect(m.codes.has('')).toBe(false); + expect(m.relationships).toEqual([]); + }); + + test('a list with answers still adds each answer once, linked both ways', async () => { + const m = migrator(); + const d = db(); + await m.processListItem(d, row('LL1-1', 'Yes/No', 'LA33-6', 'Yes'), null); + await m.processListItem(d, row('LL1-1', 'Yes/No', 'LA32-8', 'No'), 'LL1-1'); + await m.processListItem(d, row('LL2-2', 'Yes', 'LA33-6', 'Yes'), 'LL1-1'); + expect(d.inserts.map(p => [p[1], p[2]])).toEqual([['LL1-1', 3], ['LA33-6', 4], ['LA32-8', 4], ['LL2-2', 3]]); + expect(m.relationships).toHaveLength(6); + }); +}); diff --git a/tx/importers/import-loinc.module.js b/tx/importers/import-loinc.module.js index 68090ac9..fa5e3b18 100644 --- a/tx/importers/import-loinc.module.js +++ b/tx/importers/import-loinc.module.js @@ -1118,6 +1118,12 @@ class LoincDataMigrator { } const answerCode = removeQuotes(items[6]); + if (!answerCode) { + // an externally defined answer list (ExtDefinedYN = Y: ICD-10, RxNorm, ...) has one row with + // no AnswerStringId. The list is a code; the row names no answer, so there is nothing to add + // (it used to become a single Answer code '' shared by all 65 such lists) + return; + } let answerCodeKey; if (this.codes.has(answerCode)) { From 7470b4d4f39665c5aa3e74ada8f5c0987054803a Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Tue, 3 Nov 2026 07:57:35 -0600 Subject: [PATCH 13/18] LOINC: class type 3 is stored as 'Claims attachment', so look it up as that The importer stores CLASSTYPE 3 as 'Claims attachment' (descClassType), but the provider's classTypes map knew only 'Claims attachments'. For the 1,161 LOINC 2.83 codes of that class type (18719-5, 19005-8, ...) the CLASSTYPE property's value was undefined, so $lookup failed its response check with a 500, every time. The CLASSTYPE = 3 filter searched for the plural and found none of them. The map uses the importer's spelling, and still accepts the plural. Co-Authored-By: Claude Opus 5.5 --- tests/cs/cs-loinc.test.js | 32 ++++++++++++++++++++++++++++++++ tx/cs/cs-loinc.js | 5 ++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/cs/cs-loinc.test.js b/tests/cs/cs-loinc.test.js index 64038168..23d93de8 100644 --- a/tests/cs/cs-loinc.test.js +++ b/tests/cs/cs-loinc.test.js @@ -1097,6 +1097,38 @@ describe('LOINC Provider', () => { ]); }); + // The importer stores class type 3 as 'Claims attachment' (1,161 codes in LOINC 2.83, such + // as 18719-5). The test data has none, so one code is given it here for the test + test('CLASSTYPE 3 (Claims attachment): $lookup reports it, and the CLASSTYPE = 3 filter finds it', async () => { + const code = '104597-0'; + const rw = new sqlite3.Database(testDbPath); + const run = (sql, args = []) => new Promise((resolve, reject) => rw.run(sql, args, function (err) { err ? reject(err) : resolve(this); })); + const get = (sql, args = []) => new Promise((resolve, reject) => rw.get(sql, args, (err, row) => err ? reject(err) : resolve(row))); + const prop = await get(`SELECT Properties.PropertyKey, Properties.PropertyValueKey FROM Properties, PropertyTypes, Codes + WHERE Properties.PropertyTypeKey = PropertyTypes.PropertyTypeKey AND PropertyTypes.Description = 'CLASSTYPE' + AND Properties.CodeKey = Codes.CodeKey AND Codes.Code = ?`, [code]); + const added = await run(`INSERT INTO PropertyValues (Value) VALUES ('Claims attachment')`); + try { + await run('UPDATE Properties SET PropertyValueKey = ? WHERE PropertyKey = ?', [added.lastID, prop.PropertyKey]); + + const params = []; + await provider.extendLookup(code, ['CLASSTYPE'], params); + const classType = params.find(p => p.name === 'property' && p.part.find(x => x.name === 'code').valueCode === 'CLASSTYPE'); + expect(classType.part.find(x => x.name === 'value').valueString).toBe('3'); + expect(classType.part.find(x => x.name === 'description').valueString).toBe('Claims attachment'); + + const filterContext = await provider.getPrepContext(true); + await provider.filter(filterContext, true, 'CLASSTYPE', '=', '3'); + const filters = await provider.executeFilters(filterContext); + expect(await provider.filterSize(filterContext, filters[0])).toBe(1); + expect((await provider.filterLocate(filterContext, filters[0], code)).code).toBe(code); + } finally { + await run('UPDATE Properties SET PropertyValueKey = ? WHERE PropertyKey = ?', [prop.PropertyValueKey, prop.PropertyKey]); + await run('DELETE FROM PropertyValues WHERE PropertyValueKey = ?', [added.lastID]); + await new Promise(resolve => rw.close(resolve)); + } + }); + test('should extend lookup with properties and relationships', async () => { const testCode = expectedResults.basic.knownCodes[0]; const paramSet = []; diff --git a/tx/cs/cs-loinc.js b/tx/cs/cs-loinc.js index 33a3a664..25da1f82 100644 --- a/tx/cs/cs-loinc.js +++ b/tx/cs/cs-loinc.js @@ -53,13 +53,16 @@ function mapRelationship(equivalence, fromLoinc) { } } +// CLASSTYPE as the importer stores it (descClassType in import-loinc.module.js) and its number. +// The importer writes 'Claims attachment', singular; the plural is accepted too const classTypes = { '1': 'Laboratory class', '2': 'Clinical class', - '3': 'Claims attachments', + '3': 'Claims attachment', '4': 'Surveys', 'Laboratory class' : '1', 'Clinical class' : '2', + 'Claims attachment' : '3', 'Claims attachments' : '3', 'Surveys' : '4' }; From cfe9a3d34d8b52671ac5dec04d7e083e7b0fd228 Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Tue, 3 Nov 2026 07:57:44 -0600 Subject: [PATCH 14/18] A modifier extension the server doesn't know is refused with 422, not 500 Extensions.checkNoModifiers threw an Issue with no status, so $expand of a value set whose filter carries an unknown modifier extension (VSAC's vsacOpModifier, for one) answered 500 business-rule. Refusing is right, as the extension changes what the element means, but the problem is in the resource's content, not the server: the Issue now carries 422. Co-Authored-By: Claude Opus 5.5 --- tests/tx/expand.test.js | 35 +++++++++++++++++++++++++++++++++++ tx/library/extensions.js | 6 ++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/tests/tx/expand.test.js b/tests/tx/expand.test.js index b04ca664..54f9aff7 100644 --- a/tests/tx/expand.test.js +++ b/tests/tx/expand.test.js @@ -477,6 +477,41 @@ describe('Expand Worker', () => { }); }); + describe('Value sets that cannot be expanded', () => { + const expand = (parameter) => request(app) + .post('/tx/r5/ValueSet/$expand') + .set('Accept', 'application/json') + .set('Content-Type', 'application/json') + .send({ resourceType: 'Parameters', parameter }); + const messages = (response) => response.body.issue.map(i => (i.details && i.details.text) || i.diagnostics); + + // The extension changes what the filter means, so refusing is right; but the problem is in + // the request's content, not the server + test('a filter with an unknown modifier extension: 422', async () => { + const response = await expand([{ + name: 'valueSet', + resource: { + resourceType: 'ValueSet', + url: 'http://example.org/fhir/ValueSet/unknown-filter-modifier', + status: 'active', + compose: { + include: [{ + system: 'http://hl7.org/fhir/administrative-gender', + filter: [{ + modifierExtension: [{ url: 'http://example.org/fhir/StructureDefinition/unknown-op-modifier', valueString: 'childOf' }], + property: 'concept', op: 'is-a', value: 'male' + }] + }] + } + } + }]); + + expect(response.status).toBe(422); + expect(response.body.issue[0].code).toBe('business-rule'); + expect(messages(response)[0]).toContain('http://example.org/fhir/StructureDefinition/unknown-op-modifier'); + }); + }); + describe('Expansion caching', () => { test('should return same expansion for identical requests', async () => { // Make the same expand request twice diff --git a/tx/library/extensions.js b/tx/library/extensions.js index 2e52c9b3..43680835 100644 --- a/tx/library/extensions.js +++ b/tx/library/extensions.js @@ -36,6 +36,8 @@ const Extensions = { if (element.jsonObj) { element = element.jsonObj } + // A modifier extension this server doesn't know changes what the element means, so it + // can't be processed: a problem with the resource's content (422), not with the server if (element.modifierExtension) { let urls = new Set(); for (const extension of element.modifierExtension) { @@ -44,9 +46,9 @@ const Extensions = { const resId = resource ? resource : ""; const urlList = [...urls].join('\', \''); if (urls.size > 1) { - throw new Issue("error", "business-rule", null, null, 'Cannot process resource '+resId+' at "' + name + '" due to the presence of modifier extensions '+urlList); + throw new Issue("error", "business-rule", null, null, 'Cannot process resource '+resId+' at "' + name + '" due to the presence of modifier extensions '+urlList, null, 422); } else { - throw new Issue("error", "business-rule", null, null, 'Cannot process resource '+resId+' at "' + name + '" due to the presence of the modifier extension '+urlList); + throw new Issue("error", "business-rule", null, null, 'Cannot process resource '+resId+' at "' + name + '" due to the presence of the modifier extension '+urlList, null, 422); } } return true; From d25cd50fb06535f786b6556d53be79a6221d576d Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Tue, 3 Nov 2026 07:57:59 -0600 Subject: [PATCH 15/18] The too-costly message names a value set with no url by its id, or as '??', never 'undefined' A value set given in the request needn't have a url. The VALUESET_TOO_COSTLY message then named it 'undefined' (from vurl) or '??', depending on where the limit was hit. A small helper, valueSetName, gives the url when there is one, else the value set's id, else '??' (the text tx-ecosystem's snomed-expand-inactive2 expects), and all five places that raise the message use it. Co-Authored-By: Claude Opus 5.5 --- tests/tx/expand.test.js | 15 +++++++++++++++ tx/workers/expand.js | 28 +++++++++++++++++----------- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/tests/tx/expand.test.js b/tests/tx/expand.test.js index 54f9aff7..f931021b 100644 --- a/tests/tx/expand.test.js +++ b/tests/tx/expand.test.js @@ -510,6 +510,21 @@ describe('Expand Worker', () => { expect(response.body.issue[0].code).toBe('business-rule'); expect(messages(response)[0]).toContain('http://example.org/fhir/StructureDefinition/unknown-op-modifier'); }); + + // A value set in the request needn't have a url: the message names it by its id, or as + // '??', which is what tx-ecosystem's snomed-expand-inactive2 expects - never 'undefined' + test('too costly: a value set with no url is named by its id, or as \'??\'', async () => { + const valueSet = { resourceType: 'ValueSet', status: 'active', compose: { include: [{ system: 'http://hl7.org/fhir/administrative-gender' }] } }; + for (const [vs, name] of [[valueSet, '??'], [{ ...valueSet, id: 'genders' }, 'genders'], + [{ ...valueSet, url: 'http://example.org/fhir/ValueSet/genders' }, 'http://example.org/fhir/ValueSet/genders']]) { + for (const extra of [[], [{ name: 'activeOnly', valueBoolean: true }]]) { + const response = await expand([{ name: 'valueSet', resource: vs }, { name: 'limit', valueInteger: 2 }, ...extra]); + expect(response.status).toBe(422); + expect(response.body.issue[0].code).toBe('too-costly'); + expect(messages(response)[0]).toBe(`The value set '${name}' expansion has too many codes to produce (>2)`); + } + } + }); }); describe('Expansion caching', () => { diff --git a/tx/workers/expand.js b/tx/workers/expand.js index a9124096..f4c86bbf 100644 --- a/tx/workers/expand.js +++ b/tx/workers/expand.js @@ -221,6 +221,18 @@ class ValueSetExpander { this.csCounter = new Map(); } + /** + * The value set as a message names it: its url, or - for a value set in the request, which + * needn't have one - its id, or '??' (the text the tx-ecosystem tests expect) + */ + valueSetName(url) { + if (url) { + return url; + } + const id = this.valueSet && this.valueSet.jsonObj ? this.valueSet.jsonObj.id : null; + return id ? id : '??'; + } + async listDisplaysFromProvider(displays, cs, context) { await cs.designations(context, displays, true); displays.source = cs; @@ -349,10 +361,7 @@ class ValueSetExpander { this.noTotal(); throw new Issue('information', 'informational', null, null, null, null).setFinished(); } else { - if (!srcURL) { - srcURL = '??'; - } - throw new Issue("error", "too-costly", null, 'VALUESET_TOO_COSTLY', this.worker.i18n.translate('VALUESET_TOO_COSTLY', this.params.httpLanguages, [srcURL, '>' + this.limitCount]), null, 422).withDiagnostics(this.worker.opContext.diagnostics()); + throw new Issue("error", "too-costly", null, 'VALUESET_TOO_COSTLY', this.worker.i18n.translate('VALUESET_TOO_COSTLY', this.params.httpLanguages, [this.valueSetName(srcURL), '>' + this.limitCount]), null, 422).withDiagnostics(this.worker.opContext.diagnostics()); } } @@ -533,10 +542,7 @@ class ValueSetExpander { if (this.count > -1 && this.offset > -1 && this.count + this.offset > 0 && this.fullList.length >= this.count + this.offset) { throw new Issue('information', 'informational', null, null, null, null).setFinished(); } else { - if (!srcURL) { - srcURL = '??'; - } - throw new Issue("error", "too-costly", null, 'VALUESET_TOO_COSTLY', this.worker.i18n.translate('VALUESET_TOO_COSTLY', this.params.httpLanguages, [srcURL, '>' + this.limitCount]), null, 422).withDiagnostics(this.worker.opContext.diagnostics()); + throw new Issue("error", "too-costly", null, 'VALUESET_TOO_COSTLY', this.worker.i18n.translate('VALUESET_TOO_COSTLY', this.params.httpLanguages, [this.valueSetName(srcURL), '>' + this.limitCount]), null, 422).withDiagnostics(this.worker.opContext.diagnostics()); } } @@ -732,7 +738,7 @@ class ValueSetExpander { // Report the versioned url (vurl), matching the message the // enumeration path produces when it hits the same limit - this // guard is just that refusal moved earlier. - throw new Issue("error", "too-costly", null, 'VALUESET_TOO_COSTLY', this.worker.i18n.translate('VALUESET_TOO_COSTLY', this.params.httpLanguages, [source && source.vurl ? source.vurl : srcURL, '>' + this.limitCount]), null, 422).withDiagnostics(this.worker.opContext.diagnostics()); + throw new Issue("error", "too-costly", null, 'VALUESET_TOO_COSTLY', this.worker.i18n.translate('VALUESET_TOO_COSTLY', this.params.httpLanguages, [this.valueSetName(source && source.url ? source.vurl : srcURL), '>' + this.limitCount]), null, 422).withDiagnostics(this.worker.opContext.diagnostics()); } } } @@ -846,7 +852,7 @@ class ValueSetExpander { } else { const iter = await cs.iterator(null); if (valueSets.length === 0 && this.limitCount > 0 && (iter && iter.total > this.limitCount) && this.offset < 0) { - throw new Issue("error", "too-costly", null, 'VALUESET_TOO_COSTLY', this.worker.i18n.translate('VALUESET_TOO_COSTLY', this.params.httpLanguages, [vsSrc.vurl, '>' + this.limitCount]), null, 422).withDiagnostics(this.worker.opContext.diagnostics()); + throw new Issue("error", "too-costly", null, 'VALUESET_TOO_COSTLY', this.worker.i18n.translate('VALUESET_TOO_COSTLY', this.params.httpLanguages, [this.valueSetName(vsSrc.url ? vsSrc.vurl : null), '>' + this.limitCount]), null, 422).withDiagnostics(this.worker.opContext.diagnostics()); } let tcount = 0; @@ -1647,7 +1653,7 @@ class ValueSetExpander { if (this.offset + this.count < 0 && this.fullList.length > this.limit) { this.log.log('Operation took too long @ expand (' + this.constructor.name + ')'); - throw new Issue("error", "too-costly", null, 'VALUESET_TOO_COSTLY', this.worker.i18n.translate('VALUESET_TOO_COSTLY', this.params.httpLanguages, [source.vurl, '>' + this.limit]), null, 422).withDiagnostics(this.worker.opContext.diagnostics()); + throw new Issue("error", "too-costly", null, 'VALUESET_TOO_COSTLY', this.worker.i18n.translate('VALUESET_TOO_COSTLY', this.params.httpLanguages, [this.valueSetName(source.url ? source.vurl : null), '>' + this.limit]), null, 422).withDiagnostics(this.worker.opContext.diagnostics()); } else { let t = 0; let o = 0; From d4fa8b3ffe667f81707db612df810acc886d9d55 Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Tue, 3 Nov 2026 07:58:51 -0600 Subject: [PATCH 16/18] A supplement's designations match a text filter only when they have every word #335 adds the codes whose supplement designations match a text filter to LOINC's and SNOMED CT's own matches (CodeSystemProvider._supplementSearchCodes). It tested each designation with SearchFilterText.passes, which is true when any one word of the filter matches, while the code system's own search needs every word. So with the Uzbek LOINC supplement loaded (language en), $expand of http://loinc.org/vs with filter "serum glucose" listed codes whose supplement text had either word: of the first 1,000, only 47 had "glucose" in their display. Before #335 it listed none. SearchFilterText.passesAll matches each word as passes does (by stem), but needs all of them, and the supplement search uses it. Which supplements take part is unchanged. Co-Authored-By: Claude Opus 5.5 --- tests/cs/cs-loinc.test.js | 28 ++++++++++++++++++++++++++++ tests/tx/designations.test.js | 10 ++++++++++ tx/cs/cs-api.js | 7 ++++--- tx/library/designations.js | 27 +++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 3 deletions(-) diff --git a/tests/cs/cs-loinc.test.js b/tests/cs/cs-loinc.test.js index 23d93de8..4f774e7d 100644 --- a/tests/cs/cs-loinc.test.js +++ b/tests/cs/cs-loinc.test.js @@ -7,6 +7,7 @@ const { OperationContext } = require('../../tx/operation-context'); const {validateParameter} = require("../../library/utilities"); const {Designations, SearchFilterText} = require("../../tx/library/designations"); const {TestUtilities} = require("../test-utilities"); +const {CodeSystem} = require('../../tx/library/codesystem'); const folders = require('../../library/folder-setup'); describe('LOINC Module Import', () => { @@ -916,6 +917,33 @@ describe('LOINC Provider', () => { expect(size).toBeGreaterThanOrEqual(0); }); + // LOINC's own text search needs every word (no display in the test data has "serum + // glucose"); a supplement's designations must match the same way, not on any one word + test('a supplement\'s designations match a text filter only when they have every word', async () => { + const supplement = new CodeSystem({ + resourceType: 'CodeSystem', url: 'http://example.org/loinc-supplement', version: '1', status: 'active', + content: 'supplement', supplements: 'http://loinc.org', language: 'en', + concept: [ + { code: '101448-9', designation: [{ language: 'en', value: 'Serum antibody screen' }] }, + { code: '104597-0', display: 'Blood glucose by meter' }, + { code: '102820-8', designation: [{ language: 'en', value: 'Glucose level in serum' }] } + ] + }); + const supplemented = await factory.build(opContext, [supplement]); + try { + const filterContext = await supplemented.getPrepContext(true); + await supplemented.searchFilter(filterContext, new SearchFilterText('serum glucose'), false); + const filters = await supplemented.executeFilters(filterContext); + const codes = []; + while (await supplemented.filterMore(filterContext, filters[0])) { + codes.push(await supplemented.code(await supplemented.filterConcept(filterContext, filters[0]))); + } + expect(codes).toEqual(['102820-8']); + } finally { + supplemented.close(); + } + }); + test('lists text filter matches in description order, sorted or not', async () => { for (const sort of [false, true]) { const filterContext = await provider.getPrepContext(true); diff --git a/tests/tx/designations.test.js b/tests/tx/designations.test.js index b89ec59f..9d23dbb1 100644 --- a/tests/tx/designations.test.js +++ b/tests/tx/designations.test.js @@ -615,6 +615,16 @@ describe('Designations', () => { expect(filter.passes('cardiac rhythm')).toBe(false); }); + test('passesAll needs every word of the filter, passes only one', () => { + const filter = new SearchFilterText('serum glucose'); + + expect(filter.passes('Cholesterol in Serum')).toBe(true); + expect(filter.passesAll('Cholesterol in Serum')).toBe(false); + expect(filter.passesAll('Glucose [Mass/volume] in Serum or Plasma')).toBe(true); + expect(filter.passesAll('Glucoses, serum')).toBe(true); // stemmed, in any order + expect(new SearchFilterText('').passesAll('anything')).toBe(true); + }); + test('should calculate match scores', () => { const filter = new SearchFilterText('blood pressure'); const stems = ['blood', 'pressure', 'measurement']; diff --git a/tx/cs/cs-api.js b/tx/cs/cs-api.js index 911b1ff4..998a6e8a 100644 --- a/tx/cs/cs-api.js +++ b/tx/cs/cs-api.js @@ -342,7 +342,8 @@ class CodeSystemProvider { /** * The codes whose displays or designations in a supplement in use, in the search language, - * pass a text filter - a supplement's designations are searched like the code system's own + * pass a text filter - a supplement's designations are searched like the code system's own, + * so every word of the filter must match (as LOINC's and SNOMED CT's own text search need) * * @param {SearchFilterText} filter * @param {Language} searchLang - from searchLanguage() @@ -353,12 +354,12 @@ class CodeSystemProvider { for (const supplement of this.supplements || []) { const suppLang = supplement.jsonObj.language || this.defLang(); for (const concept of supplement.getAllConcepts()) { - if (concept.display && CodeSystemProvider.inSearchLanguage(suppLang, searchLang) && filter.passes(concept.display)) { + if (concept.display && CodeSystemProvider.inSearchLanguage(suppLang, searchLang) && filter.passesAll(concept.display)) { codes.add(concept.code); continue; } for (const d of concept.designation || []) { - if (d.value && CodeSystemProvider.inSearchLanguage(d.language, searchLang, suppLang) && filter.passes(d.value)) { + if (d.value && CodeSystemProvider.inSearchLanguage(d.language, searchLang, suppLang) && filter.passesAll(d.value)) { codes.add(concept.code); break; } diff --git a/tx/library/designations.js b/tx/library/designations.js index bbac6bbf..a5fb1c7b 100644 --- a/tx/library/designations.js +++ b/tx/library/designations.js @@ -82,6 +82,33 @@ class SearchFilterText { return returnRating ? {passes: false, rating: 0} : false; } + /** + * Whether every word of the filter is in the value, a word matching as in passes() (its + * stem starts with the filter word's stem). passes() needs only one word + */ + passesAll(value) { + validateParameter(value, 'value', String); + + if (this.isNull) { + return true; + } + + const stems = []; + let i = 0; + while (i < value.length) { + if (this._isAlphaNumeric(value[i])) { + const j = i; + while (i < value.length && this._isAlphaNumeric(value[i])) { + i++; + } + stems.push(this.stemmer.stem(value.substring(j, i).toLowerCase())); + } else { + i++; + } + } + return this.stems.every(filterStem => stems.some(stem => stem.startsWith(filterStem))); + } + /** * Check if designations pass the filter */ From c342728956ef08861f47256302c7a707e6bd8ca0 Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Tue, 3 Nov 2026 07:59:34 -0600 Subject: [PATCH 17/18] $expand from a code system with no content answers 422, not 500 A value set that lists codes from a code system whose content is not-present (THO's time-period-ranges, for one) can't be expanded: there is nothing to check the codes against. checkSource refused it with an Issue that had no status, so the client got 500 business-rule. The same went for a code system with no content element, and for a supplement named as a system. They now answer 422, as a code system the server doesn't have at all already does. Co-Authored-By: Claude Opus 5.5 --- tests/tx/expand.test.js | 13 +++++++++++++ tx/workers/expand.js | 7 ++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/tx/expand.test.js b/tests/tx/expand.test.js index f931021b..7ceae798 100644 --- a/tests/tx/expand.test.js +++ b/tests/tx/expand.test.js @@ -525,6 +525,19 @@ describe('Expand Worker', () => { } } }); + + // As for a code system the server doesn't have (422 not-found): there is nothing to expand from + test('codes from a code system with no content (not-present), or from a supplement: 422', async () => { + for (const [system, code, text] of [['http://hl7.org/fhir/color-rgb', '#FF0000', 'has no content'], + ['http://hl7.org/fhir/CodeSystem/example-supplement', 'chol', 'defines a supplement']]) { + const response = await expand([{ + name: 'valueSet', + resource: { resourceType: 'ValueSet', status: 'active', compose: { include: [{ system, concept: [{ code }] }] } } + }]); + expect(response.status).toBe(422); + expect(messages(response)[0]).toContain(text); + } + }); }); describe('Expansion caching', () => { diff --git a/tx/workers/expand.js b/tx/workers/expand.js index f4c86bbf..b994df74 100644 --- a/tx/workers/expand.js +++ b/tx/workers/expand.js @@ -700,13 +700,14 @@ class ValueSetExpander { if (vsInfo && vsInfo.isSimple) { vsInfo.handleByCS = cs.handlesSelecting(); } + // A code system with no content to expand from is refused like an unknown one: 422 if (!cs.contentMode()) { - throw new Issue('error', 'business-rule', null, null, 'The code system definition for ' + cset.system + ' has no content property, so this expansion cannot be performed', 'invalid'); + throw new Issue('error', 'business-rule', null, null, 'The code system definition for ' + cset.system + ' has no content property, so this expansion cannot be performed', 'invalid', 422); } else if (cs.contentMode() !== 'complete') { if (cs.contentMode() === 'not-present') { - throw new Issue('error', 'business-rule', null, null, 'The code system definition for ' + cset.system + ' has no content, so this expansion cannot be performed', 'invalid'); + throw new Issue('error', 'business-rule', null, null, 'The code system definition for ' + cset.system + ' has no content, so this expansion cannot be performed', 'invalid', 422); } else if (cs.contentMode() === 'supplement') { - throw new Issue('error', 'business-rule', null, null, 'The code system definition for ' + cset.system + ' defines a supplement, so this expansion cannot be performed', 'invalid'); + throw new Issue('error', 'business-rule', null, null, 'The code system definition for ' + cset.system + ' defines a supplement, so this expansion cannot be performed', 'invalid', 422); } else if (cs.contentMode() === 'fragment') { this.addParamUri(exp, 'used-fragment', cs.system()+ (cs.version() ? '|' + cs.version(): "")); Extensions.addBoolean(exp, "http://hl7.org/fhir/StructureDefinition/valueset-unclosed", true); From f6ba83d2adf33d2ccb341277cac6dc306170ff0a Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Tue, 3 Nov 2026 08:00:56 -0600 Subject: [PATCH 18/18] SNOMED CT: a filter on a concept the edition doesn't have selects nothing, rather than failing concept =, is-a, descendent-of, child-of, generalizes and descendent-leaf on a concept the edition doesn't have threw "The SNOMED CT Concept ... is not known", and $expand answered 500. That happens in real traffic: US Core's VSAC-derived value sets ask for descendants of 584021000052104, a concept from a national extension that the US Edition doesn't include. concept in failed the same way on any unknown id in its list. The provider now does what the FHIR CodeSystem, LOINC and other providers do for an unknown code: such a filter selects no concepts (is-not-a selects every concept), and concept in skips the unknown ids. ECL is unchanged: an unknown concept there is still INVALID_ECL. doesFilter is unchanged too, so $validate-code still reports concept in with an unknown id as a filter it doesn't understand. Co-Authored-By: Claude Opus 5.5 --- tests/cs/cs-snomed.test.js | 25 +++++++++++++++++++++++++ tx/cs/cs-snomed.js | 12 +++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/cs/cs-snomed.test.js b/tests/cs/cs-snomed.test.js index 45e9b11a..c2457ad7 100644 --- a/tests/cs/cs-snomed.test.js +++ b/tests/cs/cs-snomed.test.js @@ -1526,6 +1526,31 @@ describe('SNOMED CT descendent-leaf and is-not-a filters', () => { expect(isA.length + notA.length).toBe(provider.sct.concepts.count()); expect(notA.filter(c => isA.includes(c))).toEqual([]); }); + + // 41000147108 is from the Dutch edition, which the test edition doesn't include. As an + // unknown code does in other code systems, it selects nothing (is-not-a: everything) in an + // expansion. (doesFilter still says no to it, so $validate-code reports the filter as not + // understood where it asks.) + test('a concept the edition does not have selects no concepts, rather than failing', async () => { + const unknown = '41000147108'; + expect(provider.sct.concepts.findConcept(provider.sct.stringToIdOrZero(unknown)).found).toBe(false); + const iterated = async (op, value) => { + const fc = await provider.getPrepContext(true); + await provider.filter(fc, true, 'concept', op, value); + const [set] = await provider.executeFilters(fc); + const out = []; + while (await provider.filterMore(fc, set)) { + out.push(await provider.code(await provider.filterConcept(fc, set))); + } + return out; + }; + for (const op of ['=', 'is-a', 'descendent-of', 'child-of', 'generalizes', 'descendent-leaf', 'in']) { + expect(await iterated(op, unknown)).toEqual([]); + expect(await located(op, unknown, '16763008')).toBe(false); + } + expect(await iterated('in', unknown + ',16763008')).toEqual(['16763008']); + expect((await iterated('is-not-a', unknown)).length).toBe(provider.sct.concepts.count()); + }); }); // display regex (S6): a concept matches when it has an active description (FSN or synonym, diff --git a/tx/cs/cs-snomed.js b/tx/cs/cs-snomed.js index de742e15..d3a78bdb 100644 --- a/tx/cs/cs-snomed.js +++ b/tx/cs/cs-snomed.js @@ -946,7 +946,7 @@ class SnomedServices { const conceptResult = this.concepts.findConcept(id); if (!conceptResult.found) { - throw new Error(`The SNOMED CT Concept ${id} is not known`); + continue; // a concept the edition doesn't have adds nothing } const refSetIndex = this.getConceptRefSet(conceptResult.index, false); @@ -3555,6 +3555,16 @@ class SnomedProvider extends BaseCSServices { if (id === 0n && op !== 'in') { throw new Error(`Invalid concept ID: ${value}`); } + // A concept the edition doesn't have (one from another edition, say) selects no + // concepts - every concept, for is-not-a - as an unknown code does in other code systems + if (op !== 'in' && !this.sct.concepts.findConcept(id).found) { + const none = new SnomedFilterContext(); + if (op === 'is-not-a') { + none.descendants = this.sct._eclEnumerateAllConcepts(this.opContext); + } + filterContext.filters.push(none); + return null; + } switch (op) { case '=': {