Skip to content

Small fixes: 422s instead of 500s, a request that never got an answer, LOINC data, and two robustness fixes - #337

Open
jmandel wants to merge 16 commits into
HealthIntersections:mainfrom
jmandel:fix/small-fixes
Open

jmandel wants to merge 16 commits into
HealthIntersections:mainfrom
jmandel:fix/small-fixes

Conversation

@jmandel

@jmandel jmandel commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Twelve independent fixes, one commit each, each with its own test. Any subset can be taken, and each commit can be reverted alone. They were found while load-testing FHIRsmith with real IG Publisher traffic and comparing its answers with tx.fhir.org's. The last one adds a test and no code.

Each section gives the request that shows the problem (a curl against tx-dev.fhir.org where it reproduces there), the code path on main, the change, and the test. Every test was run against main without its fix, and fails there.

Three other changes from #332 have their own PRs: the event-loop yield changes (#338), the supplement index (#340) and locating a code once per include (#339). The edition-only SNOMED CT pin change from #332 is withdrawn.

One open question for you, in section 6 (K3): $validate-code against a value set with concept in <unknown concept> still answers 400 "filter not understood", as on main. Should it answer result false instead?

  1. $expand from a code system with no content answers 422, not 500
  2. A modifier extension the server doesn't know is refused with 422, not 500
  3. The too-costly message names a value set with no url by its id, or as '??', never 'undefined'
  4. LOINC: class type 3 is stored as 'Claims attachment', so look it up as that
  5. A supplement's designations match a text filter only when they have every word
  6. SNOMED CT: a filter on a concept the edition doesn't have selects nothing, rather than failing
  7. CodeSystem/$validate-code answers an Issue raised before the inner try, instead of never answering
  8. LOINC importer: an externally defined answer list adds no blank Answer code
  9. Keep idle connections open longer than nginx does
  10. VSAC: clamp a refresh interval longer than a timer can wait
  11. Send the JSON string already built, instead of serialising the response twice
  12. Country codes: test that a regex filter with alternation matches whole codes

1. $expand from a code system with no content answers 422, not 500

Register row K2.

What was wrong

A value set that lists codes from a code system whose content is not-present can't be expanded, since there is nothing to check the codes against. Refusing is right, but the refusal had no status, so the client got 500, as if the server had failed. A code system the server doesn't have at all already answers 422.

curl -s -X POST 'https://tx-dev.fhir.org/r4/ValueSet/$expand' \
  -H 'Content-Type: application/fhir+json' -H 'Accept: application/fhir+json' \
  -d '{"resourceType":"Parameters","parameter":[{"name":"valueSet","resource":{"resourceType":"ValueSet","status":"active","compose":{"include":[{"system":"http://terminology.hl7.org/CodeSystem/time-period-ranges","concept":[{"code":"P10Y--P15Y"}]}]}}}]}'

Before (tx-dev, 3 Nov): 500 business-rule "The code system definition for http://terminology.hl7.org/CodeSystem/time-period-ranges has no content, so this expansion cannot be performed". After: 422, same message.

Code path

tx/workers/expand.js, ValueSetExpander.checkSource (line 667). Three Issues are thrown there with no status (lines 698, 701, 703): a code system with no content, one whose content is not-present, and a supplement named as the system.

Change

All three carry 422. The messages are unchanged.

Test

tests/tx/expand.test.js, "A code system with nothing to expand from": an include of color-rgb (not-present) and one of example-supplement each give 422 with their message. The test fails on main (500) and passes with the change.

2. A modifier extension the server doesn't know is refused with 422, not 500

Register row K10.

What was wrong

Extensions.checkNoModifiers refuses an element that carries a modifier extension the server doesn't know. Refusing is right, since the extension changes what the element means. But the Issue had no status, so the client got 500, as if the server had failed. A real case: VSAC's value set 2.16.840.1.113883.11.20.9.46 (Ability) has one (vsacOpModifier) on an exclude filter.

curl -s -X POST 'https://tx-dev.fhir.org/r4/ValueSet/$expand' \
  -H 'Content-Type: application/fhir+json' -H 'Accept: application/fhir+json' \
  -d '{"resourceType":"Parameters","parameter":[{"name":"valueSet","resource":{"resourceType":"ValueSet","url":"http://example.org/fhir/ValueSet/unknown-filter-modifier","status":"active","compose":{"include":[{"system":"http://snomed.info/sct","filter":[{"modifierExtension":[{"url":"http://example.org/fhir/StructureDefinition/unknown-op-modifier","valueString":"childOf"}],"property":"concept","op":"is-a","value":"442499005"}]}]}}},{"name":"count","valueInteger":10}]}'

Before (tx-dev, 3 Nov): 500 business-rule "Cannot process resource http://example.org/fhir/ValueSet/unknown-filter-modifier at "filter" due to the presence of the modifier extension http://example.org/fhir/StructureDefinition/unknown-op-modifier". After: 422, same message.

Code path

tx/library/extensions.js, Extensions.checkNoModifiers (line 32). It throws at lines 47 and 49 (one or several extensions) with no status.

Change

Both Issues carry 422.

Test

tests/tx/expand.test.js, "a filter with an unknown modifier extension: 422": a filter with an unknown modifier extension gives 422 business-rule, and the message names the extension. The test fails on main (500) and passes with the change.

3. The too-costly message names a value set with no url by its id, or as '??', never 'undefined'

Register row K11.

What was wrong

A value set given in the request needn't have a url. When it is too costly to expand, the message named it 'undefined' or '??', depending on where the limit was hit.

curl -s -X POST 'https://tx-dev.fhir.org/r4/ValueSet/$expand' \
  -H 'Content-Type: application/fhir+json' -H 'Accept: application/fhir+json' \
  -d '{"resourceType":"Parameters","parameter":[{"name":"valueSet","resource":{"resourceType":"ValueSet","status":"active","compose":{"include":[{"system":"urn:iso:std:iso:3166:-2"}]}}}]}'

Before (tx-dev, 3 Nov): 422 "The value set 'undefined' expansion has too many codes to produce (>3000)". After: "The value set '??' expansion …". A value set with an id but no url is named by its id.

Code path

tx/workers/expand.js, five places raise VALUESET_TOO_COSTLY:

  • includeCode (line 355) and excludeCode (539), which use srcURL or '??';
  • checkSource (735), includeCodes (849) and expandSeen (1650), which use source.vurl or vsSrc.vurl. vurl is undefined when there is no url, and includeCodes is the path this request takes.

Change

A small helper, ValueSetExpander.valueSetName(url), gives the url, else the value set's id, else '??'. All five places use it. Plainer words, such as "the value set in the request", would read better, but tx-ecosystem's snomed-expand-inactive2 expects the exact text '??', so that test would have to change first.

Test

tests/tx/expand.test.js, "The too-costly message": a value set with no url, one with only an id, and one with a url, each with and without activeOnly, expanded with limit 2. The message names them '??', by the id, and by the url. The test fails on main and passes with the change.

4. LOINC: class type 3 is stored as 'Claims attachment', so look it up as that

Register row L6.

What was wrong

$lookup of 1,161 LOINC codes (class type 3) always answered 500. Among them are 18719-5, 19005-8, 18834-2, 18725-2, 26436-6, 18783-1, 18782-3 and 18726-0, all seen in IG Publisher traffic.

curl -s -H 'Accept: application/fhir+json' 'https://tx-dev.fhir.org/r4/CodeSystem/$lookup?system=http://loinc.org&code=18719-5'

Before (tx-dev, 3 Nov): 500 "Code system provider http://loinc.org produced an invalid $lookup response at parameter.property.value: valueString must be a string, not a undefined". After (local server, Loinc_2.83b.db): 200, with CLASSTYPE = 3, "Claims attachment". The same for the other seven. An expansion with the filter CLASSTYPE = 3 went from 0 codes to 1,161.

Code path

The importer stores class type 3 as "Claims attachment", singular (tx/importers/import-loinc.module.js, descClassType, line 1673). The provider's map classTypes (tx/cs/cs-loinc.js:56) knew only "Claims attachments". So for those codes, classTypes[row.Value] (line 543) was undefined, and the $lookup response check rejected the property. The CLASSTYPE = 3 filter (line 1018) searched for the plural, and found nothing.

Change

The map uses the importer's spelling, and still accepts the plural.

Test

tests/cs/cs-loinc.test.js, "CLASSTYPE 3 (Claims attachment)": on a test database with such a code, $lookup reports the property, and the CLASSTYPE = 3 filter finds the code. The test fails on main and passes with the change.

5. A supplement's designations match a text filter only when they have every word

Register row X7.

What was wrong

#335 adds the codes whose supplement designations match a text filter to the code system's own matches. It tests each designation with SearchFilterText.passes, which is true when any one word of the filter matches. LOINC's and SNOMED CT's own text search needs every word.

The Uzbek LOINC supplement is loaded on the server, and its language is en. So a two-word English LOINC filter now lists codes whose supplement text has either word:

curl -s -H 'Accept: application/fhir+json' 'https://tx-dev.fhir.org/r4/ValueSet/$expand?url=http://loinc.org/vs&filter=serum%20glucose&count=1000'

Before (tx-dev, 3 Nov): a full page of 1,000 codes, only 47 of them with "glucose" in the display (such as "8(9)-Cholestenol [Mass/volume] in Serum or Plasma"). Before #335, no codes. After (local server): 38 codes. 34 have "glucose" in the display. The other 4 have a word that starts with it ("Glucosamine", "glucosidase"), which is how words match in text filters.

Code path

tx/cs/cs-api.js, CodeSystemProvider._supplementSearchCodes (line 351) calls filter.passes(...) on each supplement display and designation. passes is in tx/library/designations.js, SearchFilterText.passes (line 49).

Change

A new SearchFilterText.passesAll matches each word as passes does (the word's stem starts with the filter word's stem), but needs all of them. _supplementSearchCodes uses it.

Whether a supplement nobody asked for should take part in the search at all is a separate question (register X5), and is unchanged here.

Test

  • tests/tx/designations.test.js: passesAll needs every word, passes only one.
  • tests/cs/cs-loinc.test.js: with a supplement loaded, "serum glucose" matches a designation only when it has both words.

Both fail on main and pass with the change.

6. SNOMED CT: a filter on a concept the edition doesn't have selects nothing, rather than failing

Register row K3.

What was wrong

These concept filters threw "The SNOMED CT Concept … is not known" on a concept the edition doesn't have, and $expand answered 500:

  • =, is-a, descendent-of, child-of, generalizes and descendent-leaf;
  • in, for any unknown id in its list.

This 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 have. tx.fhir.org gave the same 500 during a US Core IG build on 30 Oct. The FHIR CodeSystem and LOINC providers answer an unknown code in such a filter with an empty set.

curl -s -X POST 'https://tx-dev.fhir.org/r4/ValueSet/$expand' \
  -H 'Content-Type: application/fhir+json' -H 'Accept: application/fhir+json' \
  -d '{"resourceType":"Parameters","parameter":[{"name":"valueSet","resource":{"resourceType":"ValueSet","status":"active","compose":{"include":[{"system":"http://snomed.info/sct","filter":[{"property":"concept","op":"in","value":"41000147108"}]}]}}}]}'

Before (tx-dev, 3 Nov): 500 exception "The SNOMED CT Concept 41000147108 is not known". After (local server): 200, an empty expansion.

The same happens with is-a 584021000052104 and system-version=http://snomed.info/sct|http://snomed.info/sct/731000124108 (US Edition). After the change, $expand gives 200 with no codes, and $validate-code of 22298006 against that value set gives result false instead of 500.

Code path

tx/cs/cs-snomed.js:

  • SnomedProvider.filter (line 3550) hands each concept op to a SnomedServices helper, and each helper throws on an unknown concept (lines 825–901);
  • SnomedServices.filterIn (line 931) throws on an unknown id (line 949).

Change

  • In SnomedProvider.filter, a concept the edition doesn't have selects no concepts, or every concept for is-not-a.
  • filterIn skips unknown ids.

ECL is unchanged: an unknown concept there is still INVALID_ECL.

Open point. $validate-code against a value set with concept in <unknown> still answers 400 "filter … not understood", as main does. This is because SnomedProvider.doesFilter (line 3362) still says no for an unknown concept in an in list. It could answer result false instead, as is-a now does. Which one do you want?

Test

tests/cs/cs-snomed.test.js, "a concept the edition does not have selects no concepts, rather than failing":

  • each of =, is-a, descendent-of, child-of, generalizes, descendent-leaf and in on 41000147108 selects nothing, and 16763008 is not in it;
  • in 41000147108,16763008 gives 16763008;
  • is-not-a gives every concept.

The test fails on main and passes with the change.

7. CodeSystem/$validate-code answers an Issue raised before the inner try, instead of never answering

Register row G1 (its CodeSystem/$validate-code case).

What was wrong

CodeSystem/$validate-code with an unknown cache-id never answers. The client waits until its own timeout, and the connection is held meanwhile. ValueSet/$validate-code and the other operations answer 404 cache-id-unknown.

curl --max-time 20 -s -w '%{http_code}\n' -H 'Accept: application/fhir+json' -H 'X-Cache-Id: no-such-cache-xyz' \
  'https://tx-dev.fhir.org/r4/CodeSystem/$validate-code?url=http://loinc.org&code=2345-7'

Before (tx-dev, 3 Nov): no response in 20 s. After: 404, an OperationOutcome with cache-id-unknown.

Code path

tx/workers/validate.js:

  • ValidateWorker.handleCodeSystemInner (line 2108) calls setupAdditionalResources and TxParameters.readParams before its try (lines 2113–2116). setupAdditionalResources (tx/workers/worker.js:746) throws a 404 cache-id-unknown Issue, or a 400 cache-id-duplicate Issue. readParams throws a 400 Issue for exclude-system (tx/params.js:472).
  • Inside the try, the inner catch turns every Issue not marked handleAsOO into a result (handlePrepareError), and rethrows the rest.
  • So the only Issues that reach the outer catch in handleCodeSystem (line 2080) unmarked are those thrown before the inner try. There, an unmarked Issue fell into an empty branch, commented "this is actually handled in the inner method" (line 2099). No response was sent.

Change

The outer catch answers every Issue that reaches it with its OperationOutcome and status, as the ValueSet handler does. This only affects requests that got no response at all on main. Issues raised inside the validation are still turned into a result by the inner method, as before.

The same change also answers CodeSystem/$validate-code?…&exclude-system=… (400, "not supported"), which likewise got no response on main.

Test

tests/tx/cache-id.test.js: CodeSystem/$validate-code with an unknown cache-id answers 404 with the cache-id-unknown coding. On main the test times out; with the change it passes.

8. LOINC importer: an externally defined answer list adds no blank Answer code

Register row L7.

What was wrong

AnswerList.csv has one row with no AnswerStringId for each externally defined answer list (ExtDefinedYN = Y: ICD-10, RxNorm and so on; 65 lists in LOINC 2.83). The importer turned each such row into an answer with the code ''. All 65 lists share that one blank Answer, with an Answer/AnswerList link to each. So Loinc_2.83b.db has 257,267 codes, one more than LOINC 2.83 has.

curl -s -H 'Accept: application/fhir+json' 'https://tx-dev.fhir.org/r4/ValueSet/$expand?url=http://loinc.org/vs&count=0'

Before (tx-dev, 3 Nov): total 257,267. The extra code is '', type Answer, with no display. After, with a database built by the fixed importer: 257,266.

Code path

tx/importers/import-loinc.module.js, processListItem (line 1103) reads answerCode from the row (line 1120), and adds it as a code even when it is empty.

Change

A row with no AnswerStringId adds the list, and nothing else.

This changes the database, not the server, so nothing changes until the LOINC database is rebuilt. Then tx-ecosystem's loinc-expand-all-limited and loinc-expand-all-limited-total, which expect 257,267 today, need to expect 257,266.

Test

tests/tx/loinc-import-answer-lists.test.js imports a small AnswerList.csv:

  • an externally defined list (no AnswerStringId) becomes a code, with no blank answer;
  • a list with answers still adds each answer once, linked both ways.

The first fails on main and passes with the change.

9. Keep idle connections open longer than nginx does

Register row F4.

What was wrong

Node closes an idle keep-alive connection after 5 s, but nginx keeps idle upstream connections for 60 s. When nginx reuses a connection just as Node closes it, the request is reset. nginx does not retry a POST, so the client gets a 502. In our load tests behind nginx, up to 2% of requests failed this way at some load levels, mostly just after the event loop had stalled.

There's no single request that shows it: it is a race between nginx reusing a connection and Node closing it. In a test, a connection left idle for 6 s and then reused fails on main, and works with this change.

Code path

server.js:839: app.listen(PORT, …) keeps Node's defaults (keepAliveTimeout 5 s, headersTimeout 60 s).

Change

A small helper, library/keep-alive.js, sets keepAliveTimeout to 65 s, longer than nginx's 60 s. It sets headersTimeout to 66 s, since Node needs it longer than keepAliveTimeout, or it applies it to the idle connection first. server.js calls the helper on the server app.listen returns. The values are fixed; a config setting is easy to add if a deployment needs another value.

Test

tests/library/keep-alive.test.js:

  • the two timeouts are set, and headersTimeout is the longer;
  • a server with them keeps a connection that has been idle for 6 s, so a second request on the same socket succeeds.

Without the helper the test fails.

10. VSAC: clamp a refresh interval longer than a timer can wait

Register row F5.

What was wrong

Node runs a timer whose delay is above 2^31−1 ms (about 596 hours) after 1 ms instead. So a VSAC refreshIntervalHours above 596 makes the refresh run back to back. Setting 100000, meaning "effectively never", gave thousands of unhandled rejections within seconds.

This is configuration, so there's no request to show it. Set refreshIntervalHours: 100000 in the VSAC library entry and start the server.

Code path

tx/vs/vs-vsac.js, VSACValueSetProvider._startRefreshTimer (line 113) passes refreshIntervalHours * 3,600,000 straight to setInterval.

Change

_refreshIntervalMs() clamps the interval to the longest delay a timer accepts, and logs a warning. A value that is not a positive number falls back to the 24-hour default, also with a warning.

Test

tests/vs/vs-vsac-refresh-interval.test.js:

  • an ordinary interval (24 hours, half an hour) is used as given;
  • 100,000 hours is clamped to 2^31−1 ms;
  • -1, 'abc', NaN and Infinity fall back to 24 hours;
  • setInterval is called with the clamped delay.

The tests fail on main and pass with the change.

11. Send the JSON string already built, instead of serialising the response twice

Register row P5.

What was wrong

The res.json wrapper in tx.js stringifies the resource to check it and log its size, then calls Express's own res.json(data), which stringifies it again. That is about 3 ms of extra CPU per MB of response, and large expansions are several MB.

This costs CPU on every JSON response, but changes no response, so there's no curl that shows it.

Code path

tx/tx.js, TXModule.setupEndpoint (line 278), the res.json wrapper (lines 424–482). In the JSON branch, JSON.stringify(data) is at line 464, then originalJson(data) at line 468.

Change

Send the string already built: res.send(jsonStr). The bytes and headers are the same. Content-Type is set just above, so res.send keeps it and adds charset=utf-8 as res.json does. The app sets no json spaces, json replacer or json escape, so Express's res.json would build the same string.

The XML-fallback branch (line 461) still calls originalJson; it is rare.

Test

tests/tx/serialise-once.test.js: a $lookup answers 200 with application/fhir+json; charset=utf-8 and a compact JSON body, and Express's res.json is not called. On main it is called once, so the test fails; with the change it passes.

12. Country codes: test that a regex filter with alternation matches whole codes

Register row M1 (the fix itself is in #335).

What was wrong

Nothing, now. Before #335, the FHIR CodeSystem and country providers anchored a regex filter as '^' + value + '$'. So US|CA became ^US|CA$, which means "starts with US, or ends with CA", and matched USA, LCA and TCA. #335 anchors every regex filter as ^(?:pattern)$, and tests alternation for the FHIR CodeSystem provider only.

Code path

tx/cs/cs-country.js, CountryCodeServices.filter (line 203). The regex is built at line 215, already anchored as a whole.

Change

None to the code. This adds one test for the country provider.

Test

tests/cs/cs-country.test.js, "a pattern with alternation matches whole codes only": US|CA gives exactly CA and US. With the pre-#335 anchoring put back, it fails; on main it passes.

It is folded in here because it is test-only: it guards a fix that is already in, for one more provider. Drop the commit if you'd rather not have it.

Results

  • Each fix's own test fails on main without the fix, and passes with it.
  • The touched suites, run in band on this branch: 681 passed, 1 failed. The failure is the LOINC import test in tests/cs/cs-loinc.test.js running past its 120 s timeout; it does the same on main on this machine.
  • HL7 tx-ecosystem test cases (validator 7.0.3), with this branch merged with the other three from Small fixes: requests no longer wait behind batches, one hung reply, regex alternation, and a few speed-ups #332's split: 5,048 of 5,061 pass. The 13 failures are the same 13 main 9059fb9 fails: cs-batch-validate, -bad and -codings (R4 and R5, and R5 cached), and translate-reverse-r5+ and -a (R5, plain and cached).

Added since opening

$batch-validate-code against a large enumerated value set runs out of memory (register row K13)

What was wrong. A 1,000-entry $batch-validate-code of RxNorm codes against VSAC Medication Brand Name (2.16.840.1.113883.3.88.12.80.16, 33,341 codes) ran out of heap on main after 33 s, with no answer.

Code path.

  • ValueSetChecker in tx/workers/validate.js adds two notes per entry with renderer.displayValueSetInclude, which lists every code. One is "Prepare include", the other "check code". Each note is a 267 KB string, kept in both logEntries and the TimeTracker.
  • In tx/workers/batch-validate.js, processBatch gives every entry a copy of one OperationContext. They share one _openProviders list, so every entry's provider stays reachable until the batch ends, and through it the entry's context and notes, plus one open RxNorm connection per entry.
  • With a full GC every 0.5 s, the heap still in use grew about 3.2 MB per entry: +959 MB over 300 entries.

Change. Either part alone keeps the heap flat, and both are kept.

  • displayValueSetInclude writes the first 20 codes, then "+N more (M codes)". Includes of 20 codes or fewer, and filters, read as before. These notes only feed the diagnostics output.
  • OperationContext.copy({ownProviders: true}) gives a copy its own provider list. processBatch uses it for each entry and closes that entry's providers in a finally once the entry is answered. Plain copy() is unchanged.

Numbers (local, same data):

  • The 1,000-entry batch returns 200 in 18.6 s, with all 1,000 answers true and the heap flat.
  • On a 300-entry batch, every entry main answered within its time budget is byte-identical to the branch's answer.
  • On that batch, main's response was 31 MB against 336 KB on the branch. That is because 61 of main's entries carried too-costly errors, each with a 503 KB diagnostics string.

Test. tests/tx/batch-validate-memory.test.js (7 tests, with a fake validate worker); 4 of them fail on main.

An unknown RxNorm code scans the whole archive table (register row R6)

What was wrong. Looking up a code RxNorm doesn't have takes about 43 ms. locate finds nothing in rxnconso and falls back to RXNATOMARCHIVE (tx/cs/cs-rxnorm.js), which has no index on the columns it filters on, so each miss reads all ~372k rows (SCAN).

Change. An index on RXNATOMARCHIVE(RXCUI, SAB):

  • When it's added: in the importer, and also when the database is opened, the same way the provider already adds five late indexes at load. So deployed databases get it without a re-import. It takes 0.2 s once and adds 8 MB to a 2.39 GB file.
  • Read-only databases: load() now logs a warning and carries on. On main, a read-only database missing any of these indexes failed to load.

Numbers (on a copy of rxnorm_03022026.db):

  • an unknown-code lookup: 42.6 ms → 0.10 ms (SEARCH USING INDEX);
  • CodeSystem/$validate-code with an unknown code: 89–103 ms → 3–12 ms.

Test. tests/cs/cs-rxnorm-archive-index.test.js checks that the importer makes the index, that load adds it, and that a read-only database still loads. It uses a small built database, since the main RxNorm suite is skipped offline, and all 3 tests fail on main.

A related change, a time-limit check inside the loop that looks for another form of a code, is in #339, since that PR rewrites the same loop.

Every $expand with a cache-id re-serialises and re-hashes the whole cache (register row P6)

What was wrong. We replayed IG Publisher traffic at 16 concurrent builds, with the main thread saturated. ExpansionCache.computeKey was on the stack in 24% of main's busy CPU profile. Of that, 16% was spent serialising (the map callback that calls JSON.stringify) and about 7% in sha256.

Code path.

  • TerminologyWorker.setupAdditionalResources (tx/workers/worker.js) sets additionalResources to resourceCache.get(cacheId), which is everything the client has put in its $cache-control cache.
  • ValueSetExpander (tx/workers/expand.js) calls computeKey(valueSet, params, additionalResources) on every $expand.
  • computeKey did sha256(JSON.stringify(r.jsonObj)).substring(0, 16) for each of those resources, every time.

Change.

  • CanonicalResource, the base of CodeSystem, ValueSet and ConceptMap, now has expansionKeyHash(). It computes the same 16-character hash on first use, and keeps it in a non-enumerable field tied to the jsonObj it hashed. A replacement resource, or a replaced jsonObj, gets hashed again.
  • setExpansionKeyHash(hash) lets a store that already knows the hash record it.
  • computeKey uses the remembered hash. Raw JSON is hashed exactly as before. The sort, join and final sha256 are unchanged, so keys are byte-identical and expansions already cached stay valid.
  • The hash is computed on first use, not when a resource enters the cache. This does the same work once per cached resource, and nothing for resources that never reach an $expand.

What made it safe, and one fix it needed. Nothing may change a cached resource's JSON while it is in use. The r4→r5 conversion happens in the constructors, and expand and validate work on copies. Filter analysis is remembered on filter elements under non-enumerable symbols, which JSON never sees. One thing did write: $compare's tidyIncludes merged, removed and sorted the compose includes of the value sets it compared, in place. Those were cache-id resources and also the server's own value sets. It now works on a structuredClone of each compose.

Test. tests/tx/expansion-key-hash.test.js, 8 tests:

  • computeKey against a copy of the previous implementation, for a mix of wrappers, raw JSON and parameters;
  • one JSON.stringify and one createHash per resource across 100 keys;
  • a replacement resource gets a new hash and a new key;
  • the pre-fill hook;
  • the field never serialises;
  • $expand, $validate-code, $lookup and $compare through a sealed cache-id, with the cached resources' JSON frozen. Before the $compare fix this test fails with a 500, "Cannot assign to read only property".

Numbers. Same replay: one server, 16 concurrent builds, private data copies, 8 GB cap.

main branch before this this change
req/s (steady phase) 219.7 226.9 248.6 (256.0 in the profiled run)
main-thread CPU per request 4.13 ms 3.98 ms 3.60 ms
time for the whole replay 132 s 129 s 101 s
computeKey share of main's CPU profile 24.0% 0.56%

The main thread stays about 89% busy in all three, so the saving shows up as throughput. Answers: the 11,810 calls of the C=4 warm-up pass are identical with and without the change, comparing bodies with ports normalised. At C=16, the 3–4 $batch-validate-code bodies that differ also differ between two runs of main.

The abort check reads RSS from /proc on every deadCheck (register row: none, found while profiling)

What was wrong. memoryGuard.overAbortThreshold, which every deadCheck calls (about 11 times a request), read a fresh process.memoryUsage.rss() each time. Each read costs about 7 µs, because libuv reads /proc/self/stat. Once P6 was fixed, that was 4.2% of the main thread at saturation.

Change. The abort check reuses an RSS sample up to ABORT_SAMPLE_MS (10 ms) old. The request gate's 250 ms sampling, the re-measure after a forced GC and the thresholds are unchanged.

Test. tests/library/memory-guard.test.js has 3 new tests: 100 checks inside 10 ms make one RSS read; a rise is seen as soon as the sample is more than 10 ms old; the gate keeps its own interval. The existing memory-guard tests pass.

Numbers (same replay, on top of P6):

  • overAbortThreshold share of the main thread: 4.2% → 0.36%;
  • 248.6 → 275.5 req/s, 3.60 → 3.17 ms of main-thread CPU per request (264.7 req/s in the profiled run);
  • answers identical.

🤖 Generated with Claude Code

jmandel and others added 12 commits November 3, 2026 11:36
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 <noreply@anthropic.com>
… 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 <noreply@anthropic.com>
… '??', 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 <noreply@anthropic.com>
…s 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 <noreply@anthropic.com>
…very word

HealthIntersections#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 HealthIntersections#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 <noreply@anthropic.com>
…hing, 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 <noreply@anthropic.com>
…fore 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 <noreply@anthropic.com>
…r 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…ponse 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 <noreply@anthropic.com>
…e codes

HealthIntersections#335 anchors every regex filter as ^(?:pattern)$, and tests alternation for the FHIR CodeSystem
provider. This adds the same check for the country provider: US|CA matches US and CA, and not
USA, LCA or TCA.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jmandel and others added 2 commits November 3, 2026 17:06
…e large includes in log notes

A 1,000-entry ValueSet/$batch-validate-code of RxNorm codes against VSAC
Medication Brand Name (33,341 codes) ran out of heap. Two things combined:

- Each entry's log notes ("Prepare include", "check code in") wrote out
  every listed code: two ~267 KB strings per entry, kept in the context's
  logEntries and its TimeTracker steps.
- Each entry's context shared the batch's provider list, so every entry's
  providers (each with its own sqlite connection and a reference to the
  entry's context) stayed reachable until the whole batch ended.

displayValueSetInclude now writes the first 20 codes and a count. Batch
entries get their own provider list (OperationContext.copy({ownProviders}))
which is closed as soon as the entry is answered, including on error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ans the archive

RxNormServices.locate() looks a code RXNCONSO doesn't have up in
RXNATOMARCHIVE, by RXCUI and SAB. The table had no index on RXCUI, so
every unknown code scanned all ~372k rows: 43 ms per miss, against
0.1 ms with the index.

The importer now makes the index, and RXNORM_LOAD_INDEXES adds it to a
database imported before it did, the way the other late indexes are
added (a one-off 0.2 s on the current RxNorm database). A database that
can't be written is now used as it is, with a warning, instead of
failing to load: the load-time indexes only make lookups faster.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jmandel and others added 2 commits November 3, 2026 18:55
ExpansionCache.computeKey puts a content hash of every additional resource
into the expansion cache key. With a cache-id those resources are the
client's whole $cache-control cache, so every $expand serialised and
sha256-hashed everything the IG had uploaded. Under replayed IG Publisher
traffic at C=16 that was about a quarter of the main thread.

CodeSystem, ValueSet and ConceptMap (CanonicalResource) now work the hash
out on first use and remember it, in a non-enumerable field tied to the
jsonObj it was computed from (a replaced jsonObj, or a replacement
resource, is hashed again). setExpansionKeyHash lets a store that already
knows the hash record it. The hash is exactly the one computeKey made, and
the key is built the same way, so keys are byte-identical and expansions
already cached stay valid.

Remembering the hash needs a cached resource's json not to change once it
is in use. One thing did change it: $compare sorted, merged and removed the
compose includes of the value sets it compared, in place - cache-id
resources and the server's own. It now works on copies.

Tests: computeKey against the previous implementation for a mix of
resources and parameters; one serialisation and hash per resource across
many keys; a new hash for a replacement resource; and expand, validate-code,
lookup and compare through a sealed cache-id with the cached json frozen.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
overAbortThreshold, which every deadCheck calls, read a fresh
process.memoryUsage.rss() each time: about 11 reads a request, each a
read of /proc/self/stat by libuv, about 3% of the main thread at
saturation. It now reuses a sample up to ABORT_SAMPLE_MS (10 ms) old. The
request gate still samples every 250 ms, a GC while refusing still
re-measures at once, and the thresholds are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@grahamegrieve

Copy link
Copy Markdown
Contributor

Thanks. A few notes:

  • fix up readme #5: agreed, every word. For the record, a global lang pack should take part in text
    search, so X5 doesn't need changing. The uz supplement's language en is right: its
    displays are LOINC's English names and its designations are tagged uz. The English
    copies plus any-word matching are what produced the noise.
  • qa changes #6: I've asked the other server maintainers what an unknown concept in a filter should
    give, so please hold fe0abaa until that's settled. It can come out of this PR without
    affecting the rest.
  • setting up packages #12: fine as a test-only commit.
  • The last commit (aae4f8d, reuse the RSS sample for 10 ms) duplicates 4243a51 from SNOMED CT: optional memory-mapped cache files, chosen per edition in config.json #330,
    which is now merged, and it's the only conflict with main. Please drop it.
  • In future, smaller PRs please - one fix or a few related ones each. Twelve-plus independent
    fixes in one PR are hard to review and merge piecemeal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants