$validate-code: a code not in an enumerated include is looked up once, not once per listed code

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

tests/tx/validate-enumerated-miss.test.js

@@ -0,0 +1,135 @@
+/**
+ * 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);
+  checker.worker = { deadCheck: () => {} };
+
+  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();
+  });
+});

tx/workers/validate.js

@@ -1618,6 +1618,30 @@ 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) {
+      this.worker.deadCheck('checkConceptSet#3');
+      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 +1750,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;