From e0573f367a609908345e6dbd413bac5218b7c4b6 Mon Sep 17 00:00:00 2001 From: Janardhan Vignarajan Date: Fri, 1 May 2026 09:24:42 +0800 Subject: [PATCH 1/2] Fix stale async expression cache results (#1611) - Extend isExpressionCached and cacheTerminologyResult with an optional focusNode parameter that is serialised into the cache key - Pass qrItem as focusNode in evaluateLinkIdVariables and resource in evaluateQuestionnaireLevelVariables so that async terminology results (e.g. memberOf()) are not reused when the evaluated input changes - Add unit tests covering the compound cache key behaviour and the stale result regression scenario --- .../src/test/fhirpath.test.ts | 109 +++++++++++++++++- .../src/utils/fhirpath.ts | 35 +++--- 2 files changed, 130 insertions(+), 14 deletions(-) diff --git a/packages/smart-forms-renderer/src/test/fhirpath.test.ts b/packages/smart-forms-renderer/src/test/fhirpath.test.ts index d72066999..4f8f34c2a 100644 --- a/packages/smart-forms-renderer/src/test/fhirpath.test.ts +++ b/packages/smart-forms-renderer/src/test/fhirpath.test.ts @@ -20,10 +20,12 @@ import { addEmptyXFhirQueryVariablesToFhirPathContext, + cacheTerminologyResult, createFhirPathContext, evaluateLinkIdVariables, evaluateQuestionnaireLevelVariables, - handleFhirPathResult + handleFhirPathResult, + isExpressionCached } from '../utils/fhirpath'; import { BMICalculationExistingFhirPathContext, @@ -247,3 +249,108 @@ describe('handleFhirPathResult', () => { expect(result).toEqual(['valueA', 'valueB']); }); }); + +describe('isExpressionCached', () => { + it('returns false for expression containing %', () => { + expect(isExpressionCached('%resource.memberOf(url)', { '%resource.memberOf(url)': [true] })).toBe(false); + }); + + it('returns true when expression-only key is in cache', () => { + expect(isExpressionCached('memberOf(url)', { 'memberOf(url)': [true] })).toBe(true); + }); + + it('returns false when expression is not in cache', () => { + expect(isExpressionCached('memberOf(url)', {})).toBe(false); + }); + + it('returns true when compound key (expression + focusNode) is in cache', () => { + const focusNode = { answer: [{ valueString: 'A' }] }; + const key = `memberOf(url)|${JSON.stringify(focusNode)}`; + expect(isExpressionCached('memberOf(url)', { [key]: [true] }, focusNode)).toBe(true); + }); + + it('returns false when focusNode differs from the cached one', () => { + const cachedFocus = { answer: [{ valueString: 'A' }] }; + const newFocus = { answer: [{ valueString: 'B' }] }; + const key = `memberOf(url)|${JSON.stringify(cachedFocus)}`; + expect(isExpressionCached('memberOf(url)', { [key]: [true] }, newFocus)).toBe(false); + }); + + it('misses cache when focusNode is provided but only the expression-only key exists', () => { + const focusNode = { answer: [{ valueString: 'A' }] }; + expect(isExpressionCached('memberOf(url)', { 'memberOf(url)': [true] }, focusNode)).toBe(false); + }); +}); + +describe('cacheTerminologyResult', () => { + it('skips caching for expressions with %', () => { + const cache: Record = {}; + cacheTerminologyResult('%resource.memberOf(url)', [true], cache); + expect(cache).toEqual({}); + }); + + it('stores result under expression key when no focusNode', () => { + const cache: Record = {}; + cacheTerminologyResult('memberOf(url)', [true], cache); + expect(cache).toEqual({ 'memberOf(url)': [true] }); + }); + + it('stores result under compound key when focusNode is provided', () => { + const cache: Record = {}; + const focusNode = { answer: [{ valueString: 'A' }] }; + cacheTerminologyResult('memberOf(url)', [true], cache, focusNode); + const expectedKey = `memberOf(url)|${JSON.stringify(focusNode)}`; + expect(cache).toEqual({ [expectedKey]: [true] }); + }); +}); + +describe('evaluateLinkIdVariables - stale cache prevention', () => { + const terminologyServerUrl = 'https://example.com/terminology/fhir'; + const expression = 'answer.value'; + const variablesFhirPath: Record = { + 'item-link': [{ name: 'myVar', language: 'text/fhirpath', expression }] + }; + + it('re-evaluates when qrItem changes, preventing stale async cache results', async () => { + const qrItemA = { linkId: 'item-link', answer: [{ valueString: 'A' }] }; + const qrItemB = { linkId: 'item-link', answer: [{ valueString: 'B' }] }; + + // Simulate a previously cached async terminology result for qrItemA + const cacheKeyA = `${expression}|${JSON.stringify(qrItemA)}`; + const terminologyCache: Record = { [cacheKeyA]: ['A'] }; + + // Evaluate with qrItemB - compound key is different, so cache should miss + const result = await evaluateLinkIdVariables( + 'item-link', + variablesFhirPath, + { myVar: ['A'] }, // stale context from previous eval with qrItemA + terminologyCache, + terminologyServerUrl, + qrItemB + ); + + // Should re-evaluate with qrItemB's answer, not return stale 'A' + expect(result.fhirPathContext.myVar).toEqual(['B']); + }); + + it('skips re-evaluation when qrItem is unchanged (valid cache hit)', async () => { + const qrItem = { linkId: 'item-link', answer: [{ valueString: 'A' }] }; + + // Simulate a previously cached async terminology result for this qrItem + const cacheKey = `${expression}|${JSON.stringify(qrItem)}`; + const terminologyCache: Record = { [cacheKey]: ['A'] }; + + // Evaluate with the same qrItem - compound key matches, should hit cache + const result = await evaluateLinkIdVariables( + 'item-link', + variablesFhirPath, + { myVar: ['A'] }, // context already has correct value + terminologyCache, + terminologyServerUrl, + qrItem + ); + + // Cache hit - fhirPathContext keeps the existing value + expect(result.fhirPathContext.myVar).toEqual(['A']); + }); +}); diff --git a/packages/smart-forms-renderer/src/utils/fhirpath.ts b/packages/smart-forms-renderer/src/utils/fhirpath.ts index e2ff17cb5..46bce091d 100644 --- a/packages/smart-forms-renderer/src/utils/fhirpath.ts +++ b/packages/smart-forms-renderer/src/utils/fhirpath.ts @@ -267,13 +267,14 @@ export async function evaluateLinkIdVariables( for (const variable of linkIdVariables) { if (variable.expression && variable.name) { - if (isExpressionCached(variable.expression, fhirPathTerminologyCache)) { + const focusNode = qrItem ?? {}; + if (isExpressionCached(variable.expression, fhirPathTerminologyCache, focusNode)) { continue; } try { const fhirPathResult = fhirpath.evaluate( - qrItem ?? {}, + focusNode, { base: 'QuestionnaireResponse.item', expression: variable.expression @@ -288,9 +289,10 @@ export async function evaluateLinkIdVariables( const result = await handleFhirPathResult(fhirPathResult); fhirPathContext[`${variable.name}`] = result; - // If fhirPathResult is an async terminology call, cache the result + // If fhirPathResult is an async terminology call, cache the result keyed by both + // expression and focus node so that a changed answer invalidates the cached result if (fhirPathResult instanceof Promise) { - cacheTerminologyResult(variable.expression, result, fhirPathTerminologyCache); + cacheTerminologyResult(variable.expression, result, fhirPathTerminologyCache, focusNode); } } catch (e) { console.warn(e.message, `LinkId: ${linkId}\nExpression: ${variable.expression}`); @@ -318,7 +320,7 @@ export async function evaluateQuestionnaireLevelVariables( for (const variable of questionnaireLevelVariables) { if (variable.expression) { - if (isExpressionCached(variable.expression, fhirPathTerminologyCache)) { + if (isExpressionCached(variable.expression, fhirPathTerminologyCache, resource)) { continue; } @@ -340,9 +342,10 @@ export async function evaluateQuestionnaireLevelVariables( const result = await handleFhirPathResult(fhirPathResult); fhirPathContext[`${variable.name}`] = result; - // If fhirPathResult is an async terminology call, cache the result + // If fhirPathResult is an async terminology call, cache the result keyed by both + // expression and focus node so that a changed response invalidates the cached result if (fhirPathResult instanceof Promise) { - cacheTerminologyResult(variable.expression, result, fhirPathTerminologyCache); + cacheTerminologyResult(variable.expression, result, fhirPathTerminologyCache, resource); } } catch (e) { console.warn(e.message, `Questionnaire-level\nExpression: ${variable.expression}`); @@ -368,40 +371,46 @@ export async function handleFhirPathResult(result: any[] | Promise) { * Determines whether a FHIRPath expression result is cached. * * Expressions containing variables (e.g. `%something`) are never considered cached, because we can never know what a variable resolves to at runtime. + * When a `focusNode` is provided, it is included in the cache key so that results are not reused across different inputs (e.g. different `qrItem` values for `memberOf()` calls). * * @param {string} expression - The FHIRPath expression to check. * @param {Record} fhirPathTerminologyCache - Object storing cached expression results. + * @param {unknown} [focusNode] - Optional focus node passed to fhirpath.evaluate(). When provided it is serialised as part of the cache key. * @returns {boolean} `true` if the expression is cached and contains no variables; otherwise `false`. */ export function isExpressionCached( expression: string, - fhirPathTerminologyCache: Record + fhirPathTerminologyCache: Record, + focusNode?: unknown ): boolean { // Expressions with variables are never cached if (expression.includes('%')) { return false; } - + const key = focusNode !== undefined ? `${expression}|${JSON.stringify(focusNode)}` : expression; // Check if expression exists in cache - return Object.prototype.hasOwnProperty.call(fhirPathTerminologyCache, expression); + return Object.prototype.hasOwnProperty.call(fhirPathTerminologyCache, key); } /** * Caches the result of a FHIRPath evaluation if it was an asynchronous terminology call and the expression contains no variables (e.g. `%something`). + * When a `focusNode` is provided, it is included in the cache key to differentiate results for different inputs. * * @param {string} expression - The FHIRPath expression that was evaluated. * @param {any} result - The evaluated result of the expression with async resolved. * @param {Record} fhirPathTerminologyCache - The cache object to store results. + * @param {unknown} [focusNode] - Optional focus node passed to fhirpath.evaluate(). When provided it is serialised as part of the cache key. */ export function cacheTerminologyResult( expression: string, result: any, - fhirPathTerminologyCache: Record + fhirPathTerminologyCache: Record, + focusNode?: unknown ) { // Skip caching for expressions with variables if (expression.includes('%')) { return; } - - fhirPathTerminologyCache[expression] = result; + const key = focusNode !== undefined ? `${expression}|${JSON.stringify(focusNode)}` : expression; + fhirPathTerminologyCache[key] = result; } From eaf8d9b2b9b0c86a86ea44936fad5bd43c030426 Mon Sep 17 00:00:00 2001 From: Janardhan Vignarajan Date: Fri, 1 May 2026 09:52:11 +0800 Subject: [PATCH 2/2] -- Lint fix --- packages/smart-forms-renderer/src/test/fhirpath.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/smart-forms-renderer/src/test/fhirpath.test.ts b/packages/smart-forms-renderer/src/test/fhirpath.test.ts index 4f8f34c2a..14cf26388 100644 --- a/packages/smart-forms-renderer/src/test/fhirpath.test.ts +++ b/packages/smart-forms-renderer/src/test/fhirpath.test.ts @@ -252,7 +252,9 @@ describe('handleFhirPathResult', () => { describe('isExpressionCached', () => { it('returns false for expression containing %', () => { - expect(isExpressionCached('%resource.memberOf(url)', { '%resource.memberOf(url)': [true] })).toBe(false); + expect( + isExpressionCached('%resource.memberOf(url)', { '%resource.memberOf(url)': [true] }) + ).toBe(false); }); it('returns true when expression-only key is in cache', () => {