2026 06 gg bug sweep

.github/workflows/pr-pipeline.yml

@@ -49,7 +49,7 @@ jobs:
       - name: Use Node.js
         uses: actions/setup-node@v4
         with:
-          node-version: '20'
+          node-version: '24'
           cache: 'npm'
 
       - name: Install dependencies

tests/cs/cs-snomed-ecl.test.js

@@ -26,12 +26,101 @@ describe('ECL Validator Test Suite', () => {
   beforeAll(async () => {
     // Load test SNOMED data
     opContext = new OperationContext('en', await TestUtilities.loadTranslations());
-    const factory = new SnomedServicesFactory(opContext.i18n, join(__dirname, '../../data/snomed-testing.cache'));
+    const factory = new SnomedServicesFactory(opContext.i18n, join(__dirname, '../../tx/data/snomed-testing.cache'));
     await factory.load();
     snomedServices = factory.snomedServices;
     eclValidator = new ECLValidator(snomedServices);
   });
 
+  describe('ECL cardinality counts distinct values (issue #230 bug 3)', () => {
+    const expandCodes = (ecl) => {
+      const ctx = snomedServices.filterECL(ecl, true, opContext);
+      return new Set((ctx.descendants || []).map(i => snomedServices.concepts.getConceptId(i).toString()));
+    };
+
+    // 192008 has two raw 116676008 (Associated morphology) relationship rows but
+    // only ONE distinct value — the local analog of the issue's 903008. It must
+    // satisfy [1..1] and [1..*], and must NOT satisfy [2..*]. Counting raw rows
+    // (the bug) gave the opposite.
+    test('a single distinct morphology satisfies [1..1]/[1..*], not [2..*]', () => {
+      const one = expandCodes('* : [1..1] 116676008 = *');
+      const atLeastOne = expandCodes('* : [1..*] 116676008 = *');
+      const two = expandCodes('* : [2..*] 116676008 = *');
+      expect(one.has('192008')).toBe(true);
+      expect(atLeastOne.has('192008')).toBe(true);
+      expect(two.has('192008')).toBe(false);
+    });
+
+    test('[1..1] and [2..*] partition [1..*] (distinct-value counting is consistent)', () => {
+      const one = expandCodes('* : [1..1] 116676008 = *');
+      const two = expandCodes('* : [2..*] 116676008 = *');
+      const atLeastOne = expandCodes('* : [1..*] 116676008 = *');
+      expect(one.size + two.size).toBe(atLeastOne.size);
+      for (const c of one) {
+        expect(two.has(c)).toBe(false);
+      }
+    });
+
+    // Value-constrained form, matching the shape of the external case
+    // `< 64572001 : [1..1] 363698007 = << 10200004` whose count rose once
+    // distinct counting was applied. 192008 has Associated morphology 442021009
+    // in two raw rows but one distinct value, so it satisfies [1..1] (not [2..*])
+    // for that specific value.
+    test('value-constrained [1..1] includes a concept with one distinct value stated across rows', () => {
+      const one = expandCodes('* : [1..1] 116676008 = 442021009');
+      const atLeastOne = expandCodes('* : [1..*] 116676008 = 442021009');
+      const two = expandCodes('* : [2..*] 116676008 = 442021009');
+      expect(one.has('192008')).toBe(true);
+      expect(atLeastOne.has('192008')).toBe(true);
+      expect(two.has('192008')).toBe(false);
+    });
+  });
+
+  describe('ECL memberOf (issue #230 bugs 1 & 2)', () => {
+    const expand = (ecl) => {
+      const ctx = snomedServices.filterECL(ecl, true, opContext);
+      return ctx.descendants || [];
+    };
+    const codesOf = (ecl) => expand(ecl).map(i => snomedServices.concepts.getConceptId(i).toString());
+
+    // Bug 1: a bare ^<concept that is not a reference set> must be rejected with
+    // an error that names the concept (was: returned the fabricated 0xFFFFFFFF
+    // sentinel / the concept itself).
+    test('bare ^<non-reference-set> is rejected with a value-identifying error', () => {
+      expect(() => snomedServices.filterECL('^404684003', true, opContext))
+        .toThrow(/404684003 is not a reference set/);
+    });
+
+    // Bug 1: returns the refset's referenced-component concepts, including
+    // concepts that are themselves inactive (active refers to the membership row,
+    // not the referenced concept). 900000000000526001 (an association refset)
+    // has one member: the inactive concept 307530000.
+    test('memberOf returns referenced concepts, including inactive ones', () => {
+      const codes = codesOf('^900000000000526001');
+      expect(codes).toContain('307530000');
+      const idx = expand('^900000000000526001').find(
+        i => snomedServices.concepts.getConceptId(i).toString() === '307530000');
+      expect(snomedServices.isActive(idx)).toBe(false); // inactive concept, still returned
+    });
+
+    // Bug 1: never fabricate the 0xFFFFFFFF (4294967295) sentinel, and never
+    // return the operand concept itself.
+    test('memberOf never emits the sentinel or the operand itself', () => {
+      const codes = codesOf('^900000000000526001');
+      expect(codes).not.toContain('4294967295');
+      expect(codes).not.toContain('900000000000526001');
+    });
+
+    // Bug 2: a wrapped/computed operand must be evaluated, not rejected as
+    // "non-concept-reference refset is not yet supported".
+    test('wrapped memberOf operands are evaluated, not rejected as unsupported', () => {
+      expect(() => snomedServices.filterECL('^(<<192008)', true, opContext)).not.toThrow();
+      expect(() => snomedServices.filterECL('^(404684003)', true, opContext)).not.toThrow();
+      // computed operand is lenient: non-refset concepts in the set are skipped
+      expect(Array.isArray(codesOf('^(404684003)'))).toBe(true);
+    });
+  });
+
   describe('ECL Lexer Tests', () => {
 
     test('should tokenize simple concept reference', () => {

tests/cs/cs-snomed.test.js

@@ -30,13 +30,19 @@ const {Designations} = require("../../tx/library/designations");
 const {TestUtilities} = require("../test-utilities");
 const folders = require('../../library/folder-setup');
 
-// Shared cache file paths and utilities
+// Shared cache file paths and utilities.
+// testCachePath is the scratch location the import test (below) writes/deletes.
+// committedCachePath is the checked-in copy under tx/data (which IS in git),
+// used by the consumer tests when no fresh import has been produced this run.
 const testCachePath = folders.ensureFilePath('snomed-testing.cache');
+const committedCachePath = path.resolve(__dirname, '../../tx/data/snomed-testing.cache');
 const fallbackCachePath = folders.ensureFilePath('sct_intl_20250201.cache');
 
 function findAvailableCacheFile() {
   if (fs.existsSync(testCachePath)) {
     return testCachePath;
+  } else if (fs.existsSync(committedCachePath)) {
+    return committedCachePath;
   } else if (fs.existsSync(fallbackCachePath)) {
     return fallbackCachePath;
   }

tests/library/canonical-resource.test.js

@@ -0,0 +1,57 @@
+const { CanonicalResource } = require('../../tx/library/canonical-resource');
+
+/**
+ * isMoreRecent() is documented to return a boolean. The 'alpha' and (especially)
+ * the 'default' branch compare versions with localeCompare(), which yields
+ * -1/0/1 — and -1 is truthy. The default branch previously returned that raw
+ * number, so it both returned a non-boolean and reported "more recent" exactly
+ * when the version actually sorted EARLIER. These tests lock in the boolean
+ * contract and correct ordering for both branches.
+ */
+
+function cr(version, algorithm) {
+  const jsonObj = { resourceType: 'CodeSystem', url: 'http://example.org/cs', version };
+  if (algorithm !== undefined) {
+    jsonObj.versionAlgorithmString = algorithm;
+  }
+  return new CanonicalResource(jsonObj);
+}
+
+describe('CanonicalResource.isMoreRecent — boolean contract', () => {
+  describe('alpha algorithm', () => {
+    test('returns a strict boolean', () => {
+      expect(typeof cr('b', 'alpha').isMoreRecent(cr('a', 'alpha'))).toBe('boolean');
+      expect(typeof cr('a', 'alpha').isMoreRecent(cr('b', 'alpha'))).toBe('boolean');
+    });
+
+    test('true when this version sorts after other', () => {
+      expect(cr('b', 'alpha').isMoreRecent(cr('a', 'alpha'))).toBe(true);
+    });
+
+    test('false when this version sorts before other', () => {
+      expect(cr('a', 'alpha').isMoreRecent(cr('b', 'alpha'))).toBe(false);
+    });
+  });
+
+  describe('default (unrecognised) algorithm', () => {
+    test('returns a strict boolean even for the fallback branch', () => {
+      expect(typeof cr('b', 'totally-unknown').isMoreRecent(cr('a', 'totally-unknown'))).toBe('boolean');
+      expect(typeof cr('a', 'totally-unknown').isMoreRecent(cr('b', 'totally-unknown'))).toBe('boolean');
+    });
+
+    test('true when this version sorts after other', () => {
+      expect(cr('b', 'totally-unknown').isMoreRecent(cr('a', 'totally-unknown'))).toBe(true);
+    });
+
+    // The original bug: localeCompare('a','b') === -1 (truthy) was returned as-is,
+    // so this earlier-sorting version was wrongly reported as more recent.
+    test('false (not truthy -1) when this version sorts before other', () => {
+      const result = cr('a', 'totally-unknown').isMoreRecent(cr('b', 'totally-unknown'));
+      expect(result).toBe(false);
+    });
+  });
+
+  test('returns false when versions are equal', () => {
+    expect(cr('a', 'totally-unknown').isMoreRecent(cr('a', 'totally-unknown'))).toBe(false);
+  });
+});

tests/library/codesystem.test.js

@@ -539,8 +539,9 @@ describe('CodeSystem', () => {
       expect(Array.isArray(parsed.identifier)).toBe(true);
       expect(parsed.identifier.length).toBe(2);
 
-      // R5-only filter operators should be removed
-      expect(parsed.filter[0].operator).not.toContain('generalizes');
+      // 'generalizes' is a valid R4 operator (filter-operator value set, 4.0.1)
+      // and must be retained on R5 -> R4 conversion
+      expect(parsed.filter[0].operator).toContain('generalizes');
       expect(parsed.filter[0].operator).toContain('=');
       expect(parsed.filter[0].operator).toContain('is-a');
     });
@@ -654,7 +655,7 @@ describe('CodeSystem', () => {
         filter: [
           {
             "code": "concept",
-            "operator": ["generalizes"], // Only R5 operator
+            "operator": ["child-of"], // Only an R5-only operator (stripped for R4)
             "value": "A string value"
           }
         ]
@@ -922,7 +923,8 @@ describe('CodeSystem', () => {
 
       expect(xmlOutput).toContain('<CodeSystem xmlns="http://hl7.org/fhir">');
       expect(xmlOutput).not.toContain('versionAlgorithmString');
-      expect(xmlOutput).not.toContain('<operator value="generalizes"/>');
+      // 'generalizes' is valid in R4 and must be retained
+      expect(xmlOutput).toContain('<operator value="generalizes"/>');
       expect(xmlOutput).toContain('<operator value="="/>');
       expect(xmlOutput).toContain('<operator value="is-a"/>');
 

tests/library/ucum.test.js

@@ -10,13 +10,12 @@ const { XMLParser } = require('fast-xml-parser');
 
 
 const {
-  UcumException, Decimal, Pair, Registry, UcumVersionDetails
+  UcumException, Decimal, Pair, Registry
 } = require('../../tx/library/ucum-types.js');
 const { UcumService} = require('../../tx/library/ucum-service');
 
 describe('UCUM Library Tests', () => {
   let ucumService;
-  const jestConsole = console;
 
   beforeEach(() => {
     global.console = require('console');
@@ -44,6 +43,37 @@ describe('UCUM Library Tests', () => {
     });
   });
 
+  describe('Issue 252: special-unit handlers register under their real code (not undefined)', () => {
+    // CelsiusHandler/FahrenheitHandler override getCode() but never set a `.code`
+    // field; register() must key on getCode() so they don't both land under
+    // `undefined` (with Fahrenheit overwriting Celsius).
+    let registry;
+    beforeAll(() => { registry = new Registry(); });
+
+    test('Cel is registered under "Cel"', () => {
+      expect(registry.exists('Cel')).toBe(true);
+      expect(registry.get('Cel').getCode()).toBe('Cel');
+    });
+
+    test('[degF] is registered under "[degF]"', () => {
+      expect(registry.exists('[degF]')).toBe(true);
+      expect(registry.get('[degF]').getCode()).toBe('[degF]');
+    });
+
+    test('Celsius and Fahrenheit are distinct (no overwrite)', () => {
+      expect(registry.get('Cel')).not.toBe(registry.get('[degF]'));
+    });
+
+    test('nothing is registered under the undefined key', () => {
+      expect(registry.exists(undefined)).toBe(false);
+    });
+
+    test('offset-free special units (HoldingHandler) still register correctly', () => {
+      expect(registry.exists('[pH]')).toBe(true);
+      expect(registry.get('[pH]').getCode()).toBe('[pH]');
+    });
+  });
+
   describe('Decimal Class Tests', () => {
     describe('Integer Conversion', () => {
       test('should convert decimals to integers correctly', () => {
@@ -290,7 +320,7 @@ describe('UCUM Library Tests', () => {
         // Test a smaller range for performance
         for (let i = 90.5; i < 91; i += 0.01) {
           const decimal = new Decimal(i.toString());
-          const expected = i * 2.2046226218487758072297380134503;
+          const expected = i / 0.45359237; // pounds per kg = 1 / 0.45359237 (exact lb definition)
           const actual = ucumService.convert(decimal, 'kg', '[lb_av]');
           const actualFloat = parseFloat(actual.asDecimal());
 
@@ -562,12 +592,6 @@ if (functionalTestCases.length > 0) {
 
     // Helper functions
 
-    function getTestCasesByType(type) {
-      return functionalTestCases
-        .filter(tc => tc.type === type)
-        .map(tc => [tc.id, tc]);
-    }
-
     function runValidationCase(testCase) {
       const { id, unit, valid, reason } = testCase.attributes;
       const expectedValid = valid === 'true';
@@ -792,14 +816,6 @@ function parseXmlTestCases(xmlContent) {
     }
   }
 
-  // Log summary of loaded tests
-  const testCounts = testCases.reduce((counts, tc) => {
-    counts[tc.type] = (counts[tc.type] || 0) + 1;
-    return counts;
-  }, {});
-
-  // console.log('Loaded test cases by type:', testCounts);
-
   return testCases;
 }
 

tests/tx/cs-provider-list.test.js

@@ -0,0 +1,43 @@
+const { ListCodeSystemProvider } = require('../../tx/cs/cs-provider-list');
+
+/**
+ * ListCodeSystemProvider.codeSystems is an ARRAY (despite the field's
+ * "Map<String, CodeSystem>" doc comment). Loaders must append with .push().
+ * library.js#loadUrl previously called .set() on it (copied from loadNpm but
+ * rewritten Map-style), which threw because arrays have no .set — so any
+ * `url:`/`url/cs:` package source failed to load. These tests lock the contract.
+ */
+
+describe('ListCodeSystemProvider.codeSystems contract', () => {
+  test('is an array, initially empty', () => {
+    const cp = new ListCodeSystemProvider();
+    expect(Array.isArray(cp.codeSystems)).toBe(true);
+    expect(cp.codeSystems).toHaveLength(0);
+  });
+
+  test('has no .set method (loaders must use .push)', () => {
+    const cp = new ListCodeSystemProvider();
+    expect(cp.codeSystems.set).toBeUndefined();
+    expect(typeof cp.codeSystems.push).toBe('function');
+  });
+
+  test('pushed code systems are returned by listCodeSystems', async () => {
+    const cp = new ListCodeSystemProvider();
+    cp.codeSystems.push({ url: 'http://a', vurl: 'http://a|1', id: 'a' });
+    cp.codeSystems.push({ url: 'http://b', vurl: 'http://b|1', id: 'b' });
+    const list = await cp.listCodeSystems('R5', null);
+    expect(list).toHaveLength(2);
+    expect(list.map(c => c.url)).toEqual(['http://a', 'http://b']);
+  });
+
+  test('assignIds iterates the array and assigns unique ids', () => {
+    const cp = new ListCodeSystemProvider();
+    cp.codeSystems.push({ url: 'http://a' });            // no id
+    cp.codeSystems.push({ url: 'http://b', id: 'a' });   // collides after first gets id
+    const ids = new Set();
+    cp.assignIds(ids);
+    const assigned = cp.codeSystems.map(c => c.id);
+    expect(new Set(assigned).size).toBe(2);              // all unique
+    expect(ids.size).toBe(2);
+  });
+});

tests/tx/designations.test.js

@@ -355,6 +355,13 @@ describe('Designations', () => {
       expect(passes).toBe(true);
     });
 
+    test('empty filter passes() matches every value (was broken by this.null typo)', () => {
+      const emptyFilter = new SearchFilterText('');
+      expect(emptyFilter.passes('anything at all')).toBe(true);
+      expect(emptyFilter.passes('')).toBe(true);
+      expect(emptyFilter.passes('Blood pressure measurement', true)).toEqual({ passes: true, rating: 0 });
+    });
+
     test('should return rating for matches', () => {
       const result = searchFilter.passes('Blood pressure measurement', true);
       expect(result.passes).toBe(true);