From f8a045cd92fbbcbf60f34c37a4fc5eb6607296ed Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Thu, 30 Apr 2026 16:37:05 -0400 Subject: [PATCH] Surface empty stratifiers as contained OperationOutcomes instead of failing the Measure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #1019. Previously, R4MeasureDefBuilder.getStratifierType threw InvalidMeasureDefinitionException when a stratifier had no criteria.expression and no components, aborting the entire Measure evaluation. Now the rest of the Measure evaluates and the offending stratifiers are reported as contained OperationOutcomes on the MeasureReport. Build-time changes (R4MeasureDefBuilder): - Detect empty stratifiers in buildStratifierDef and emit a placeholder StratifierDef with a null type and no components. The placeholder is a no-op for the evaluator (empty components list) and the report builder (no strata produced), and keeps the 1:1 ordering with Measure.group.stratifier that R4MeasureReportBuilder.buildGroup asserts on. - Collect a non-fatal error per offending stratifier in a buildErrors list on the builder, then push them onto MeasureDef.errors after the MeasureDef is constructed so the existing OperationOutcome pipeline picks them up. - Drop the now-unused InvalidMeasureDefinitionException class. Report-builder change (R4MeasureReportBuilderContext): - addContained deduplicates by fhirType()/idPart, and freshly-built OperationOutcomes all collapsed to "OperationOutcome/", so only the first error ever made it into the report. addOperationOutcomes now assigns each OperationOutcome a distinct id (operation-outcome-N) so every MeasureDef error surfaces as its own contained resource. Test changes: - The regression test now reuses LibrarySimple.cql/LibrarySimple.json (no enhancements needed — all referenced defines already exist) and the standalone EmptyStratifier.cql and EmptyStratifier Library JSON are removed. - MeasureStratifierTest.emptyStratifier asserts a contained OperationOutcome for both stratifier-1 and stratifier-2, that the group still has both stratifier slots, and that Initial Population evaluates over the expected 10 patients. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../InvalidMeasureDefinitionException.java | 12 --- .../cr/measure/r4/R4MeasureDefBuilder.java | 40 ++++++--- .../r4/R4MeasureReportBuilderContext.java | 9 +- .../cr/measure/r4/MeasureStratifierTest.java | 30 ++++--- .../input/cql/EmptyStratifier.cql | 41 --------- .../resources/library/EmptyStratifier.json | 26 ------ .../resources/measure/EmptyStratifier.json | 87 ++++--------------- 7 files changed, 70 insertions(+), 175 deletions(-) delete mode 100644 cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/InvalidMeasureDefinitionException.java delete mode 100644 cqf-fhir-cr/src/test/resources/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest/input/cql/EmptyStratifier.cql delete mode 100644 cqf-fhir-cr/src/test/resources/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest/input/resources/library/EmptyStratifier.json diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/InvalidMeasureDefinitionException.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/InvalidMeasureDefinitionException.java deleted file mode 100644 index a87c7e5714..0000000000 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/InvalidMeasureDefinitionException.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.opencds.cqf.fhir.cr.measure.common; - -/** - * Thrown when a Measure resource fails structural validation at MeasureDef build time — e.g. a - * stratifier with no {@code criteria.expression} and no components, or other shape errors that make - * the Measure un-evaluable. - */ -public class InvalidMeasureDefinitionException extends RuntimeException { - public InvalidMeasureDefinitionException(String message) { - super(message); - } -} diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureDefBuilder.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureDefBuilder.java index 4e2c12dbe4..1e5c138131 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureDefBuilder.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureDefBuilder.java @@ -38,7 +38,6 @@ import org.opencds.cqf.fhir.cr.measure.common.ConceptDef; import org.opencds.cqf.fhir.cr.measure.common.ContinuousVariableObservationAggregateMethod; import org.opencds.cqf.fhir.cr.measure.common.GroupDef; -import org.opencds.cqf.fhir.cr.measure.common.InvalidMeasureDefinitionException; import org.opencds.cqf.fhir.cr.measure.common.MeasureDef; import org.opencds.cqf.fhir.cr.measure.common.MeasureDefBuilder; import org.opencds.cqf.fhir.cr.measure.common.MeasurePopulationType; @@ -54,6 +53,11 @@ @SuppressWarnings("squid:S1135") public class R4MeasureDefBuilder implements MeasureDefBuilder { + // Non-fatal structural issues collected during build (e.g. stratifiers with no + // criteria.expression and no components). Surfaced as MeasureReport contained + // OperationOutcomes so the rest of the measure still evaluates. + private final List buildErrors = new ArrayList<>(); + @Override public MeasureDef build(Measure measure) { checkId(measure); @@ -73,9 +77,11 @@ public MeasureDef build(Measure measure) { groups.add(groupDef); } - return new MeasureDef( + var measureDef = new MeasureDef( // We don't need either the version of the "Measure" qualifier here measure.getIdElement(), measure.getUrl(), measure.getVersion(), groups, getSdeDefs(measure)); + buildErrors.forEach(measureDef::addError); + return measureDef; } private GroupDef buildGroupDef( @@ -106,7 +112,9 @@ private GroupDef buildGroupDef( final Optional optPopulationDefDateOfCompliance = buildPopulationDefForDateOfCompliance( measure.getUrl(), group, populationsWithCriteriaReference, populationBasisDef); - // Stratifiers + // Stratifiers — empty stratifiers (no criteria.expression, no components) yield a + // StratifierDef with a null type that the evaluator and report builder skip. + // Errors collected in buildErrors are surfaced as contained OperationOutcomes. var stratifiers = group.getStratifier().stream() .map(mgsc -> buildStratifierDef(measure.getUrl(), mgsc, populationBasisDef)) .toList(); @@ -349,6 +357,21 @@ private StratifierDef buildStratifierDef( String measureUrl, MeasureGroupStratifierComponent mgsc, CodeDef populationBasisDef) { checkId(mgsc); + final boolean hasCriteriaExpression = + mgsc.hasCriteria() && mgsc.getCriteria().hasExpression(); + final boolean hasAnyComponentCriteria = + mgsc.getComponent().stream().anyMatch(MeasureGroupStratifierComponentComponent::hasCriteria); + + if (!hasCriteriaExpression && !hasAnyComponentCriteria) { + // Build an un-evaluable StratifierDef (null type) and record a non-fatal + // error so the rest of the measure still evaluates and the report contains + // an OperationOutcome. Keeping the StratifierDef in the list preserves the + // 1:1 ordering with Measure.group.stratifier expected by the report builder. + buildErrors.add("Stratifier '%s' has no criteria.expression and no components for measure: %s" + .formatted(mgsc.getId(), measureUrl)); + return new StratifierDef(mgsc.getId(), conceptToConceptDef(mgsc.getCode()), null, null, List.of()); + } + boolean isBooleanBasis = isBooleanPopulationBasis(populationBasisDef); // Components var components = new ArrayList(); @@ -393,15 +416,10 @@ private List getSdeDefs(Measure measure) { return sdes; } - @Nullable private static MeasureStratifierType getStratifierType( String measureUrl, MeasureGroupStratifierComponent measureGroupStratifierComponent, boolean isBooleanBasis) { - if (measureGroupStratifierComponent == null) { - return null; - } - final boolean hasCriteria = measureGroupStratifierComponent.hasCriteria() && measureGroupStratifierComponent.getCriteria().hasExpression(); @@ -414,12 +432,6 @@ private static MeasureStratifierType getStratifierType( .formatted(hasCriteria, hasAnyComponentCriteria, measureUrl)); } - if (!hasCriteria && !hasAnyComponentCriteria) { - throw new InvalidMeasureDefinitionException( - "Stratifier '%s' has no criteria.expression and no components for measure: %s" - .formatted(measureGroupStratifierComponent.getId(), measureUrl)); - } - if (hasCriteria) { return MeasureStratifierType.CRITERIA; } else if (!isBooleanBasis) { diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilderContext.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilderContext.java index 6e6a7edfa5..568fc7c859 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilderContext.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilderContext.java @@ -144,13 +144,16 @@ private void validateReference(String reference) { public void addOperationOutcomes() { var errorMsgs = this.measureDef.errors(); - for (var error : errorMsgs) { - addContained(createOperationOutcome(error)); + for (int i = 0; i < errorMsgs.size(); i++) { + // Distinct ids so addContained's putIfAbsent does not collapse + // multiple errors into a single contained OperationOutcome. + addContained(createOperationOutcome(errorMsgs.get(i), "operation-outcome-" + (i + 1))); } } - private OperationOutcome createOperationOutcome(String errorMsg) { + private OperationOutcome createOperationOutcome(String errorMsg, String id) { OperationOutcome op = new OperationOutcome(); + op.setId(id); op.addIssue() .setSeverity(OperationOutcome.IssueSeverity.ERROR) .setCode(IssueType.EXCEPTION) diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest.java index 8e683c6a35..6dcea30da5 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest.java @@ -8,7 +8,6 @@ import org.hl7.fhir.r4.model.CodeableConcept; import org.hl7.fhir.r4.model.MeasureReport.MeasureReportStatus; import org.junit.jupiter.api.Test; -import org.opencds.cqf.fhir.cr.measure.common.InvalidMeasureDefinitionException; import org.opencds.cqf.fhir.cr.measure.common.MeasurePopulationType; import org.opencds.cqf.fhir.cr.measure.r4.Measure.Given; import org.opencds.cqf.fhir.cr.measure.r4.Measure.When; @@ -28,20 +27,29 @@ class MeasureStratifierTest { Measure.given().repositoryFor("CriteriaBasedStratifiersComplex"); private static final Given GIVEN_SIMPLE = Measure.given().repositoryFor("MeasureTest"); + /** + * Stratifiers with no {@code criteria.expression} and no components are skipped and + * surfaced as one contained OperationOutcome per offending stratifier; the rest of + * the measure still evaluates. + */ @Test void emptyStratifier() { - final When when = GIVEN_MEASURE_STRATIFIER_TEST + GIVEN_MEASURE_STRATIFIER_TEST .when() .measureId("EmptyStratifier") - .evaluate(); - try { - when.then(); - fail("expected InvalidMeasureDefinitionException for stratifier without expression or components"); - } catch (InvalidMeasureDefinitionException e) { - assertEquals( - "Stratifier 'stratifier-1' has no criteria.expression and no components for measure: https://example.com/Measure/EmptyStratifier", - e.getMessage()); - } + .evaluate() + .then() + .report() + .logReportJson() + .hasContainedOperationOutcome() + .hasContainedOperationOutcomeMsg( + "Stratifier 'stratifier-1' has no criteria.expression and no components for measure: http://example.com/Measure/EmptyStratifier") + .hasContainedOperationOutcomeMsg( + "Stratifier 'stratifier-2' has no criteria.expression and no components for measure: http://example.com/Measure/EmptyStratifier") + .firstGroup() + .hasStratifierCount(2) + .population(MeasurePopulationType.INITIALPOPULATION) + .hasCount(10); } /** diff --git a/cqf-fhir-cr/src/test/resources/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest/input/cql/EmptyStratifier.cql b/cqf-fhir-cr/src/test/resources/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest/input/cql/EmptyStratifier.cql deleted file mode 100644 index d4b59d4eb7..0000000000 --- a/cqf-fhir-cr/src/test/resources/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest/input/cql/EmptyStratifier.cql +++ /dev/null @@ -1,41 +0,0 @@ -library EmptyStratifier - -using FHIR version '4.0.1' - -include FHIRHelpers version '4.0.1' called FHIRHelpers - -parameter "Measurement Period" Interval - default Interval[@2025-01-01T00:00:00.000Z, @2025-12-31T23:59:59.999Z] - -context Patient - -// Populations referenced by the Measure resource (boolean basis). -// Definitions are intentionally trivial — the test exercises the stratifier -// validation path, which fails before any evaluation result is consulted -// because the Measure declares stratifiers with a null criteria.expression. - -define "Initial Population": - true - -define "Denominator": - "Initial Population" - -define "Denominator Exclusions": - false - -define "Numerator": - "Initial Population" - -// Supplemental data elements referenced by Measure.supplementalData - -define "SDE Sex": - Patient.gender - -define "SDE Race": - null - -define "SDE Ethnicity": - null - -define "SDE Payer": - null diff --git a/cqf-fhir-cr/src/test/resources/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest/input/resources/library/EmptyStratifier.json b/cqf-fhir-cr/src/test/resources/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest/input/resources/library/EmptyStratifier.json deleted file mode 100644 index e966bcdad4..0000000000 --- a/cqf-fhir-cr/src/test/resources/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest/input/resources/library/EmptyStratifier.json +++ /dev/null @@ -1,26 +0,0 @@ -{ - "resourceType": "Library", - "id": "EmptyStratifier", - "url": "https://example.com/Library/EmptyStratifier", - "meta": { - "profile": [ - "http://hl7.org/fhir/us/cqfmeasures/StructureDefinition/computable-library-cqfm" - ] - }, - "name": "EmptyStratifier", - "status": "active", - "type": { - "coding": [ - { - "system": "http://terminology.hl7.org/CodeSystem/library-type", - "code": "logic-library" - } - ] - }, - "content": [ - { - "contentType": "text/cql", - "url": "../../cql/EmptyStratifier.cql" - } - ] -} diff --git a/cqf-fhir-cr/src/test/resources/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest/input/resources/measure/EmptyStratifier.json b/cqf-fhir-cr/src/test/resources/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest/input/resources/measure/EmptyStratifier.json index c9f27456f7..a593652c07 100644 --- a/cqf-fhir-cr/src/test/resources/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest/input/resources/measure/EmptyStratifier.json +++ b/cqf-fhir-cr/src/test/resources/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest/input/resources/measure/EmptyStratifier.json @@ -1,47 +1,30 @@ { - "resourceType": "Measure", "id": "EmptyStratifier", - "url": "https://example.com/Measure/EmptyStratifier", - "name": "EmptyStratifier", + "resourceType": "Measure", + "url": "http://example.com/Measure/EmptyStratifier", "library": [ - "https://example.com/Library/EmptyStratifier" + "http://example.com/Library/LibrarySimple" + ], + "extension": [ + { + "url": "http://hl7.org/fhir/us/cqfmeasures/StructureDefinition/cqfm-populationBasis", + "valueCode": "boolean" + } ], + "scoring": { + "coding": [ + { + "system": "http://hl7.org/fhir/measure-scoring", + "code": "cohort" + } + ] + }, "group": [ { "id": "group-1", - "extension": [ - { - "url": "http://hl7.org/fhir/us/cqfmeasures/StructureDefinition/cqfm-scoring", - "valueCodeableConcept": { - "coding": [ - { - "system": "http://terminology.hl7.org/CodeSystem/measure-scoring", - "code": "proportion", - "display": "Proportion" - } - ] - } - }, - { - "url": "http://hl7.org/fhir/us/cqfmeasures/StructureDefinition/cqfm-populationBasis", - "valueCode": "boolean" - }, - { - "url": "http://hl7.org/fhir/us/cqfmeasures/StructureDefinition/cqfm-improvementNotation", - "valueCodeableConcept": { - "coding": [ - { - "system": "http://terminology.hl7.org/CodeSystem/measure-improvement-notation", - "code": "decrease", - "display": "increase" - } - ] - } - } - ], "population": [ { - "id": "initial-population-1", + "id": "initial-population", "code": { "coding": [ { @@ -53,39 +36,7 @@ }, "criteria": { "language": "text/cql-identifier", - "expression": "Initial Population" - } - }, - { - "id": "denominator-1", - "code": { - "coding": [ - { - "system": "http://terminology.hl7.org/CodeSystem/measure-population", - "code": "denominator", - "display": "Denominator" - } - ] - }, - "criteria": { - "language": "text/cql-identifier", - "expression": "Denominator" - } - }, - { - "id": "numerator-1", - "code": { - "coding": [ - { - "system": "http://terminology.hl7.org/CodeSystem/measure-population", - "code": "numerator", - "display": "Numerator" - } - ] - }, - "criteria": { - "language": "text/cql-identifier", - "expression": "Numerator" + "expression": "Initial Population Boolean" } } ],