From 102d4da7520fc0ad6f3ce456af6e390b14fddb6c Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Tue, 3 Nov 2026 11:08:21 -0600 Subject: [PATCH 1/2] validate: a code not in an enumerated include is located once, not once per listed code When the code is not in an include's concept list, checkConceptSet tries each listed concept as the same concept written in another form (ICD-11's short code and entity id), calling sameConceptDifferentForm for each. That function first locates the code and asks whether it has a classification code at all; only if so does it locate the listed concept and compare. The first part depends only on the code, so it was repeated, with the same answer, once per listed concept: a locate per listed code on every miss. otherFormMatch asks that part once, and compares the listed concepts only when the code has another form. For every code and list it picks the same listed concept (or none) as the old loop; the new test checks that over 200 lists on a two-spelling code system. An empty list still locates nothing. On tx-dev.fhir.org a miss against a 1,185-code RxNorm list took 99 s, and one against a 33,341-code list got no answer in 600 s. A miss against a 300-code list located the code 301 times; it now locates it once or twice. Co-Authored-By: Claude Opus 5.5 --- tests/tx/validate-enumerated-miss.test.js | 134 ++++++++++++++++++++++ tx/workers/validate.js | 33 ++++-- 2 files changed, 158 insertions(+), 9 deletions(-) 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..1a5b582f --- /dev/null +++ b/tests/tx/validate-enumerated-miss.test.js @@ -0,0 +1,134 @@ +/** + * 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 { ValueSetChecker } = require('../../tx/workers/validate'); + +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); + }); +}); + +/** + * otherFormMatch against the loop it replaces, on a code system with two spellings per concept + * (as ICD-11 has): an entity id, and for most concepts a short code. The old loop called + * sameConceptDifferentForm for each listed concept in turn and took the first that said yes. + */ +describe('otherFormMatch gives what the old loop gave', () => { + // canonical code -> short code (null: no short code, so only one spelling) + const concepts = { 100: '1A00', 200: '1B00', 300: null, 400: '1D00' }; + const byForm = new Map(); + for (const [id, short] of Object.entries(concepts)) { + byForm.set(id, { id, short }); + if (short) byForm.set(short, { id, short }); + } + const fakeCs = () => ({ + locate: jest.fn(async (code) => ({ context: byForm.get(code) || null })), + classificationCode: async (ctxt) => ctxt.short, + code: async (ctxt) => ctxt.id + }); + const checker = Object.create(ValueSetChecker.prototype); + + async function oldLoop(cs, code, list) { + if (list && !list.some(cc => cc.code === code)) { + for (const cc of list) { + if (await checker.sameConceptDifferentForm(cs, code, cc.code)) { + return cc; + } + } + } + return null; + } + + // every code as asked (each spelling, and unknown ones) against many lists of listed codes + const asked = [...byForm.keys(), 'nope', '9Z99']; + let seed = 7; + const rnd = (n) => { seed = (seed * 1103515245 + 12345) % 2147483648; return seed % n; }; + const lists = [[]]; + for (let i = 0; i < 200; i++) { + lists.push(Array.from({ length: 1 + rnd(6) }, () => ({ code: asked[rnd(asked.length)] }))); + } + + test('the same listed concept (or none) for every code and list', async () => { + let matches = 0; + for (const code of asked) { + for (const list of lists) { + const expected = await oldLoop(fakeCs(), code, list); + const actual = await checker.otherFormMatch(fakeCs(), code, list); + expect(actual).toBe(expected); + if (actual) matches++; + } + } + expect(matches).toBeGreaterThan(50); // the comparison exercised real matches, not just misses + }); + + test('a code with one spelling is located once, however long the list', async () => { + const list = Array.from({ length: 1000 }, (_, i) => ({ code: 'x' + i })); + for (const code of ['300', 'nope']) { + const cs = fakeCs(); + expect(await checker.otherFormMatch(cs, code, list)).toBeNull(); + expect(cs.locate).toHaveBeenCalledTimes(1); + } + }); + + test('an empty list locates nothing, as before', async () => { + const cs = fakeCs(); + expect(await checker.otherFormMatch(cs, '100', [])).toBeNull(); + expect(cs.locate).not.toHaveBeenCalled(); + }); +}); diff --git a/tx/workers/validate.js b/tx/workers/validate.js index d4b5a8d6..44b24e75 100644 --- a/tx/workers/validate.js +++ b/tx/workers/validate.js @@ -1618,6 +1618,29 @@ class ValueSetChecker { return !!b.context && (await cs.code(a.context)) === (await cs.code(b.context)); } + /** + * The listed concept that is the code written in its other form (ICD-11's short code and + * entity id), or null. sameConceptDifferentForm asks two things of each listed concept: does + * the code have another form at all (locate the code, ask for its classification code), and + * is this listed concept that form. The first depends only on the code, so it is asked once + * here, and the listed concepts are compared only when the answer is yes. + */ + async otherFormMatch(cs, code, concepts) { + if (!concepts || concepts.length === 0 || concepts.some(cc => cc.code === code)) { + return null; + } + const a = await cs.locate(code); + if (!a.context || !(await cs.classificationCode(a.context))) { + return null; + } + for (const cc of concepts) { + if (await this.sameConceptDifferentForm(cs, code, cc.code)) { + return cc; + } + } + return null; + } + 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 @@ -1726,15 +1749,7 @@ 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)) { - for (const cc of cset.concept) { - if (await this.sameConceptDifferentForm(cs, code, cc.code)) { - altMatch = cc; - break; - } - } - } + const altMatch = await this.otherFormMatch(cs, code, cset.concept); for (let cc of cset.concept || []) { this.worker.deadCheck('checkConceptSet#1'); let c = cc.code; From cf52d51f98aa0a818aa719316192b43f41871126 Mon Sep 17 00:00:00 2001 From: Josh Mandel Date: Tue, 3 Nov 2026 16:29:34 -0600 Subject: [PATCH 2/2] checkConceptSet: check the time limit while looking for another form of the code When the code is not one of an include's listed codes, checkConceptSet asks the code system, for each listed code, whether it is the same concept written another way (sameConceptDifferentForm). That is a locate() per listed code - 33,341 for VSAC Medication Brand Name - and the loop had no deadline check, so it ran to the end however long that took. It now calls deadCheck like the loop after it. Co-Authored-By: Claude Opus 5.5 --- tests/tx/validate-deadline.test.js | 80 +++++++++++++++++++++++ tests/tx/validate-enumerated-miss.test.js | 1 + tx/workers/validate.js | 1 + 3 files changed, 82 insertions(+) create mode 100644 tests/tx/validate-deadline.test.js diff --git a/tests/tx/validate-deadline.test.js b/tests/tx/validate-deadline.test.js new file mode 100644 index 00000000..0f73c4b7 --- /dev/null +++ b/tests/tx/validate-deadline.test.js @@ -0,0 +1,80 @@ +/** + * checkConceptSet honours the operation's time limit while it looks for a listed code + * written in another form. + * + * When the code being validated is not one of an include's listed codes and the code system + * says the code has another form (ICD-11 short code vs uri), checkConceptSet asks, for every + * listed code, whether the two are the same concept (sameConceptDifferentForm): locate() calls + * per listed code. The loop had no deadline check, so with a slow code system it ran to the + * end, however long that took, before the next loop noticed the time limit had passed. + */ + +const { Languages } = require('../../library/languages'); +const { OperationContext } = require('../../tx/operation-context'); +const { Renderer } = require('../../tx/library/renderer'); +const { ValueSetChecker } = require('../../tx/workers/validate'); + +/** Burn CPU synchronously for ms milliseconds (the deadline is charged in compute time). */ +function spin(ms) { + const end = performance.now() + ms; + while (performance.now() < end) { /* burn */ } +} + +/** A code system whose codes have another form (as ICD-11's do), where every lookup costs 1 ms of compute. */ +function slowCodeSystem() { + const cs = { + locates: 0, + async locate(code) { + cs.locates++; + spin(1); + return { context: { code }, message: undefined }; + }, + async classificationCode() { return 'other-form'; }, + async code(context) { return context.code; }, + system: () => 'http://example.org/slow', + version: () => null + }; + return cs; +} + +function checkerFor(opContext) { + // only the parts of a ValueSetChecker that checkConceptSet uses on this path + const checker = Object.create(ValueSetChecker.prototype); + checker.worker = { + opContext, + renderer: new Renderer(null, null, null), + deadCheck: (place) => opContext.deadCheck(place) + }; + checker.valueSet = { vurl: 'http://example.org/vs' }; + checker.params = {}; + checker.indentCount = 0; + return checker; +} + +describe('checkConceptSet deadline', () => { + test('stops looking for another form of the code once the time limit has passed', async () => { + const LISTED = 3000; // ~3 s of lookups at 1 ms each + const opContext = new OperationContext(Languages.fromAcceptLanguage('en'), null, null, 0.2); // 200 ms + opContext.debugging = false; // a debugger attached to jest would otherwise disable the check + const cs = slowCodeSystem(); + const cset = { system: 'http://example.org/slow', concept: Array.from({ length: LISTED }, (_, i) => ({ code: 'c' + i })) }; + + const start = performance.now(); + let error = null; + try { + await checkerFor(opContext).checkConceptSet('Coding', 'in', cs, cset, 'not-listed', null, + { vurl: 'http://example.org/vs' }, null, {}, {}, {}, null, null, []); + } catch (e) { + error = e; + } + const took = performance.now() - start; + + expect(error).not.toBeNull(); + expect(error.message).toContain('Operation exceeded time limit'); + expect(error.message).toContain('checkConceptSet#3'); + // the check is sampled (every CHECK_FREQUENCY calls), so a few hundred lookups past + // the limit, not all 3,000 of them + expect(cs.locates).toBeLessThan(LISTED / 2); + expect(took).toBeLessThan(1500); + }); +}); diff --git a/tests/tx/validate-enumerated-miss.test.js b/tests/tx/validate-enumerated-miss.test.js index 1a5b582f..1f49e44a 100644 --- a/tests/tx/validate-enumerated-miss.test.js +++ b/tests/tx/validate-enumerated-miss.test.js @@ -83,6 +83,7 @@ describe('otherFormMatch gives what the old loop gave', () => { code: async (ctxt) => ctxt.id }); const checker = Object.create(ValueSetChecker.prototype); + checker.worker = { deadCheck: () => {} }; async function oldLoop(cs, code, list) { if (list && !list.some(cc => cc.code === code)) { diff --git a/tx/workers/validate.js b/tx/workers/validate.js index 44b24e75..e637afbc 100644 --- a/tx/workers/validate.js +++ b/tx/workers/validate.js @@ -1634,6 +1634,7 @@ class ValueSetChecker { return null; } for (const cc of concepts) { + this.worker.deadCheck('checkConceptSet#3'); if (await this.sameConceptDifferentForm(cs, code, cc.code)) { return cc; }