sdc-populate: a variable whose dependency is evaluated later is left unset, including one that reads a variable of an ancestor item #2167

Summary

In @aehrc/sdc-populate 4.7.1, population evaluates FHIRPath variables once each, in a fixed order: root variables in array order, then item variables item by item, then every itemPopulationContext. A variable that references a name not bound yet fails, the failure is logged as a warning, and the variable is left unset. Population still reports success.

The order is not the one SDC recommends. Item variables are collected children first (post-order) into a plain object keyed by linkId, so an item's variables are evaluated before its ancestors' variables. A variable on a question that reads a variable declared on its parent or grandparent group is the placement SDC asks authors to use, and it fails. Whether it fails depends on the linkIds: JavaScript puts integer-like keys first, so 1 / 1.1 / 1.1.1 works and g / h / a does not.

SDC also says that tools cannot rely on that order anyway (Expression Extensions, same text in the current build):

those extensions SHOULD be ordered such that the extension that defines an environment variable always appears on a higher node, or earlier on the current node, than an extension with an expression that uses that variable. However, tools cannot rely on this ordering occurring. [...] systems can only assume that the extension is declared on an ancestor node or on the same node, not necessarily prior to the location where it's used. If a reference is not resolved once all in-scope extensions have been processed, that SHOULD be treated as an error.

and (Dependencies between expressions):

in some cases, updates to later elements may trigger an update to earlier elements, which then cascade through again. Systems ought to allow for this and allow for iterative updating of expressions until values reach a stable state - or until an unstable looping condition is detected.

Steps to reproduce

1. A Questionnaire with a root patient launchContext and this item tree:
   • group g with a variable y (text/fhirpath): 'group'
     • group h
       • string item a with a variable x (text/fhirpath): %y, and initialExpression: %x
2. Call populateQuestionnaire for any patient.

Expected: a is prefilled with group; populatedContext.x is ["group"].
Actual: no answer. populatedContext.y is ["group"], but populatedContext has no x. populateSuccess is true, with two issues:

• Error: Attempting to access an undefined environment variable: y (from x; y is declared, just not evaluated yet)
• Error: Attempting to access an undefined environment variable: x (from the initialExpression)

The same pattern in other shapes, each with initialExpression: %x on the item that declares x, or on a top-level item when x is a root variable:

In the last case, an initialExpression %ctx on the same child is prefilled with context: only variables cannot read an itemPopulationContext.

Related effects of the same code:

• When a later item with a duplicate linkId has an extension property, even extension: [], its variables replace the earlier item's at the earlier item's position, so the earlier item's variables are never evaluated.
• An item with linkId __proto__ sets the prototype of the object instead of adding a key, so its variables are never evaluated.
• Some forward references do not fail, because fhirpath.js does not evaluate them: iif(true, 'a', %y), coalesce('a', %y) and %resource.item.where(linkId = %y) (the response is empty during population). The same reference fails in an iif criterion, an and operand or a combine argument.

Reproduced with @aehrc/sdc-populate@4.7.1, fhirpath 4.10.1 and Node 22, calling populateQuestionnaire directly with a fetchResourceCallback that resolves to the resource itself. The root launchContext makes population run and makes it evaluate item variables at all (see #2162).

Cause

All in packages/sdc-populate/src/SDCPopulateQuestionnaireOperation/utils/, at f846e2d8 (tag sdc-populate@4.7.1); every file cited is unchanged on main at 9ec3e213 (4.7.2).

extractAndEvaluateFhirPathVariables in createFhirPathContext.ts (:119-154) evaluates the root variables in array order (:130-138), then the item variables in Object.entries order of a record keyed by linkId (:140-153). This is the function that also skips item variables when the root has no extensions (#2162).

extractItemLevelFhirPathVariablesRecursive fills that record after recursing into the children (:199-214):

if (items && items.length > 0) {
  for (const childItem of items) {
    extractItemLevelFhirPathVariablesRecursive({ ...params, item: childItem, ... });
  }
}

if (item.extension) {
  variables[item.linkId] = getFhirPathVariables(item.extension);
}

So the keys are inserted children first. Object.entries then returns integer-like keys (1, 2, 10) first in ascending order, and the others in insertion order. An assignment to an existing key keeps the first position, which explains the duplicate linkId case, and variables['__proto__'] = … sets the prototype instead of adding a key.

evaluateFhirPathVariables (:241-266) evaluates each variable once. When fhirpath.js throws on an unbound name, the catch logs it, pushes a warning issue and leaves the name unset. Nothing evaluates it again.

populate (populate.ts:80-101) builds the context, variables included, and only then evaluates the itemPopulationContexts (evaluateExpressions.ts:116-157), so no variable can read one.

History: the item-variable collection was added in 66169ce (December 2024, sdc-populate 3.0.0). The existing tests (createFhirPathContext.test.ts:185-243, :331-356) have no variable that reads another.

The renderer appears to use the same order: createFhirPathContext in packages/smart-forms-renderer/src/utils/fhirpath.ts evaluates item variables in the key order of the response item map (:216-237), whose keys are also inserted children first (updatableResponseItems.ts:81-97). But each pass starts from the previous pass's context (fhirpath.ts:192-197), so a variable that fails on one pass probably gets a value on the next one, read from its dependency's value of the previous pass. I have read this in the source only and have not reproduced it in the renderer. If it holds, the renderer mostly hides the problem, with a value that can be one pass behind.

Suggested fix

Two steps. The first is small and fixes the headline case.

1. Collect item variables parent first, in a list. Collect them in pre-order (an item's variables before its children's), into an array (or a Map), and evaluate in that order:

function collectItemVariables(item: QuestionnaireItem, out: Expression[][]) {
  if (item.extension) {
    out.push(getFhirPathVariables(item.extension));
  }
  for (const child of item.item ?? []) {
    collectItemVariables(child, out);
  }
}

This makes an ancestor's variables available to its descendants for any linkIds, and removes the integer-key, duplicate-linkId and __proto__ effects. It matches SDC's "typical order of calculation" (root first, then items depth-first in document order). It does not fix a forward reference on the same node or a reference to an itemPopulationContext.

2. Evaluate in dependency order. Evaluate root variables, item variables and itemPopulationContexts in an order that follows their % references, as SDC asks. Either sort them topologically: fhirpath.parse(expression) returns ExternalConstant nodes for %name, so the references can be read from the AST. Or keep the current order and re-evaluate the ones that failed until a pass binds nothing new. Report a reference that is still unresolved at the end, or a cycle, as an issue that names the variable that could not be evaluated.

Step 2 depends on how names are resolved. Population currently keeps all variables in one flat namespace; #2166 covers that. A dependency order is only well defined once %y resolves to the y in scope (on the same item or an ancestor), not to whichever y was evaluated last. Step 1 does not depend on scoping.

Tests to add

In createFhirPathContext.test.ts, for extractAndEvaluateFhirPathVariables:

• group g with variable y = 'group', child group h, grandchild a with variable x = %y: expect x to be ["group"] and no issues (step 1).
• the same for a child a of g, and for linkIds 2 / 1 and 10 / 2 (step 1).
• an item linkId __proto__ with variable x = 'proto': expect x to be ["proto"] (step 1).
• root variables x = %y then y = 'y', and the same on one item: expect x to be ["y"] (step 2).

In populate.test.ts: a group with an itemPopulationContext ctx and a child variable x = %ctx, expecting the child's initialExpression %x to prefill (step 2).