Skip to content

Fix/expression recursion hang guard - #69

Open
rukshn wants to merge 4 commits into
developfrom
fix/expression-recursion-hang-guard
Open

rukshn wants to merge 4 commits into
developfrom
fix/expression-recursion-hang-guard

Conversation

@rukshn

@rukshn rukshn commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Adding a guard to catch a looping and printing the trace where the loop occurred

rukshn and others added 4 commits September 2, 2026 15:30
Bundles three things so there is one buildable state for regenerating
cohort_fup (previously all of this was uncommitted working tree):

- Empty-safe single-value FHIRPath functions, ported verbatim from
  40190cc on fix/CQL-to-fhirpath-not-working-for-select (that branch does
  not contain the feature/segment tip, so the fix was unreachable from
  here). A bare X.round() raises in HAPI when the focus is empty, which
  is every calculatedExpression at render time - one of them makes the
  whole Questionnaire fail to render. Verified against HAPI
  org.hl7.fhir.r4 6.0.22: cohort_fup registration raised
  "focus for 0 can only have one value, but has 0 values" on bmi.
  See fix/20260831-fhirpath-empty-safe-math.md.

- Choice/yes-no aware equality (fix/20260902-select-operand-coded-equality.md):
  a yes/no code literal compared with a native boolean item becomes a
  boolean literal, and an operand that only forwards a select's value
  (GetRepeatedValue, parenthesis) gets coded membership. An operand
  already reduced to a code (COALESCE union) keeps a plain '='.

- The in-progress dynamic display text work (${REF} -> cqf-expression on
  item._text) that shares fhir_form.py, so this commit is self-contained.

409 tests pass.
`Length(a & b)` was emitted as `a & b.select($this.length())`. FHIRPath
navigation binds tighter than `&`, so the call applied to `b` alone and the
expression evaluated `string & integer`; HAPI raises "right operand to & has
the wrong type integer", the exception leaves
initializeCalculatedExpressions, and the whole Questionnaire fails to render.

That is what crashed cohort_fup on the device (GlitchTip issue 97, "Failed to
render questionnaire 50fb25b1-…"): `symptom_l` builds a comma-joined symptom
label list guarded by `LENGTH(CONCATENATE(...)) > 0`, 18 times. Nothing raises
while the form is empty, which is why it looked like "crashes when I select
symptoms".

Verified against HAPI org.hl7.fhir.r4 6.0.22 (the version OpenSRP 2.2.2-qa
ships): the pushed registration Questionnaire raises with any answer present,
and evaluates with 0 raises at 0/1/2/3 answers per repeating choice once the 18
sites are corrected.

The mis-binding predates fix/20260831-fhirpath-empty-safe-math.md - a bare
`.length()` mis-bound identically - so this is the same missing-parentheses
bug, not a regression from it.

See fix/20260902-fhirpath-suffix-precedence.md. 419 tests pass.
Large projects whose entry page chains many rhombus-guarded goto steps hung
forever: expression generation re-expands each predecessor from scratch and
expands every step's pnav_rel_* display bridge twice, so the chain costs ~2^n
expansions of ever bigger expression trees. Python's recursion limit is never
reached, so the run just never finished, with no log line naming a node.

Add tricc_oo/visitors/loop_guard.py: LoopGuard (iteration budget for a while
loop), RecursionGuard (depth + per-top-level-call budget, used as a context
manager) and trip(), which logs the reason, the expansion path, the nodes
revisited on it and a Python stack trace, then raises TriccLoopError. Guard
targets are only turned into labels when a guard trips, so guarding a hot call
stays cheap.

Wire the guards into the two loops that could run away: get_node_expression is
now a guarded wrapper around the unchanged body (_get_node_expression), and
stashed_node_func caps its while loop -- check_stashed_loop only detects a
frozen stash list, so an oscillating one could still spin forever.

Budgets are tunable through TRICC_MAX_EXPRESSION_CALLS / _DEPTH,
TRICC_MAX_LOOP_ITERATIONS and TRICC_LOOP_GUARD_PDB; the defaults leave three
orders of magnitude over the reference projects' healthy peaks.

Analysis in fix/20260902-expression-recursion-hang-guard.md, reading the
failure in docs/troubleshooting.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A guard trip listed the whole expansion path and a revisit tally, which still
left the reader to work out where the recursion actually turned back. Report it
directly, narrowest first: the node the recursion looped on with the repeating
segment around it, then the path from the nearest branching node down to that
node -- the stretch of flowchart to open in draw.io.

A repetition spanning other nodes (A -> B -> A) is a graph loop and wins over an
immediate repeat (A -> A), which is instead reported as a re-entry that doubles
the work per level -- what actually happens on a chain of rhombus-guarded goto
steps. When no node repeats, the report says the recursion fans out and points
at the deepest node.

Branching means the graph forks at a node: more than one predecessor (the
direction expansion walks) or more than one next node. The search starts
strictly above the loop node so the segment always has a start; a loop node
that branches itself is called out separately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

1 participant