Skip to content

Ensure stratifier validation mandates "string" population basis correctly#1001

Merged
JPercival merged 4 commits into
mainfrom
ld-20260421-string-basis-stratifier-bug
Apr 22, 2026
Merged

Ensure stratifier validation mandates "string" population basis correctly#1001
JPercival merged 4 commits into
mainfrom
ld-20260421-string-basis-stratifier-bug

Conversation

@lukedegruchy
Copy link
Copy Markdown
Contributor

@lukedegruchy lukedegruchy commented Apr 21, 2026

Summary

This MR fixes DQM-692, where $evaluate-measure failed for Measures using a lowercase "string" population basis with criteria-based stratifiers. The root cause was a case-sensitive comparison between CQL result type names ("String") and FHIR population basis codes ("string"). The fix also refactors the population basis validation architecture to be FHIR-version-agnostic and improves error messages for invalid population basis codes.

  1. Bug fix: Added explicit handling for "string" (and "boolean") primitive FHIR type codes in doesBasisMatchResource, ensuring lowercase FHIR codes match their uppercase Java class counterparts while rejecting invalid uppercase variants like "String".
  2. Validation improvement: Replaced the assert-only population basis code validation in R4MeasureDefBuilder with proper InvalidRequestException messages that include the measure URL, suggest the correct code for casing errors, and link to the FHIR spec for unrecognized codes.
  3. Architecture refactor: Moved all version-agnostic validation logic from R4PopulationBasisValidator into PopulationBasisValidator as default interface methods. R4 now only supplies type mappings; DSTU3 remains a documented no-op since DSTU3 has no populationBasis extension.
  4. Improved value stratifier error messages with stratifier-type-specific wording (DQM-691) — "Value" stratifiers (boolean basis) get a message that omits the irrelevant population basis and lists allowed categorical types. "Non Subject Value" stratifiers (resource basis) include the population basis for context. Both messages suggest using a CRITERIA-based stratifier as an alternative.

Code Review Suggestions

  • Verify that the PopulationBasisValidator default methods (especially resolveResourceType) being default rather than private is acceptable from an API surface perspective — they are callable by any implementor, not just internally.
  • The doesBasisMatchResource logic now gates Boolean.class and String.class through explicit constant checks and rejects them from the generic getSimpleName() path. Confirm there are no other CQL/FHIR primitive types (e.g. Integer, Decimal) that could hit the same case-sensitivity issue with their FHIR codes.
  • The ListResource special case that previously existed in extractResourceType was removed during the refactor. If any existing Measures use "ListResource" as a population basis code, this would be a behavioral change. Verify whether this code path was ever exercised in production.
  • The findCaseInsensitiveMatch in R4MeasureDefBuilder iterates all FHIRAllTypes enum values. Confirm this is acceptable for the error path (it only runs on invalid input) and that no enum values have null toCode() results that could cause issues despite the null check.
  • The value stratifier error message includes List.copyOf(allowedStratifierValueTypes()) — since this comes from a HashSet, the ordering in the error message is non-deterministic. Verify this is acceptable for log-based alerting or automated error parsing downstream.

QA Test Suggestions

Setup

Load a FHIR R4 server with:

  • An Organization resource
  • 2+ Patient resources (with different genders, e.g. male and female)
  • Encounter resources for each patient
  • A CQL Library that includes a string-returning stratifier expression (e.g. if Patient.gender = 'male' then 'male' else 'female')

Test Cases

  • Lowercase "string" basis with criteria stratifier: Create a Ratio Measure with populationBasis: "string" and a stratifier.criteria expression returning String values. Run $evaluate-measure with multiple patients. Expect: 200 OK with a COMPLETE MeasureReport containing stratified results.
  • Uppercase "String" basis (invalid): Create the same Measure but with populationBasis: "String". Run $evaluate-measure. Expect: an error response containing "has an invalid population basis of 'String'. Did you mean to enter 'string' instead?".
  • Nonsensical basis code: Create a Measure with populationBasis: "Foo". Run $evaluate-measure. Expect: an error response containing "has an invalid population basis of 'Foo'" and a reference to the FHIR spec URL.
  • Existing Encounter basis measures: Run $evaluate-measure on an existing Encounter-basis Measure with criteria and value stratifiers. Expect: no regression, same results as before.
  • Boolean basis measures: Run $evaluate-measure on an existing boolean-basis Measure with stratifiers. Expect: no regression, same results as before.
  • Group-level population basis: Create a Measure with populationBasis set at the group level (on Measure.group rather than Measure) using an invalid code. Expect: the same improved error message including the measure URL.

…basis (DQM-692)

The criteria-based stratifier validator compared CQL result type names
against population basis codes using case-sensitive equals. This caused
$evaluate-measure to fail when a Measure used the valid FHIR type code
"string" (lowercase) with a criteria stratifier returning String values,
because Java's String.class.getSimpleName() returns "String" (uppercase).

Changes:
- Add explicit handling for "string" basis in doesBasisMatchResource,
  mirroring the existing "boolean" pattern, and reject invalid uppercase
  "String"/"Boolean" variants that aren't valid FHIRAllTypes codes
- Replace the assert-only validation in R4MeasureDefBuilder with proper
  InvalidRequestException messages that include the measure URL and
  suggest the correct code for casing errors
- Refactor PopulationBasisValidator from a minimal interface into a
  default-method interface containing all version-agnostic validation
  logic, eliminating the need for an abstract base class
- R4PopulationBasisValidator now only supplies FHIR-version-specific
  type mappings; Dstu3PopulationBasisValidator remains a no-op since
  DSTU3 has no populationBasis extension

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Formatting check succeeded!

@lukedegruchy lukedegruchy marked this pull request as ready for review April 21, 2026 20:13
@lukedegruchy lukedegruchy changed the title Fix criteria stratifier validation for lowercase "string" population … Ensure stratifier validation mandates "string" population basis correctly Apr 21, 2026
lukedegruchy and others added 2 commits April 22, 2026 10:15
…es (DQM-691)

Replace the misleading "population-basis mismatch" error for value stratifiers
with clear, actionable messages that differ by stratifier type:

- Value stratifiers (boolean basis): omit the irrelevant population basis and
  list the allowed categorical types
- Non Subject Value stratifiers (resource basis): include the population basis
  for context and list allowed categorical types

Add Encounter-basis unit tests for NON_SUBJECT_VALUE stratifier validation and
an integration test for ratio/Encounter-basis value stratifier errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@JPercival JPercival enabled auto-merge (squash) April 22, 2026 17:03
@sonarqubecloud
Copy link
Copy Markdown

@JPercival JPercival merged commit 9f21411 into main Apr 22, 2026
9 checks passed
@JPercival JPercival deleted the ld-20260421-string-basis-stratifier-bug branch April 22, 2026 17:16
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.

3 participants