Skip to content

Commit d340296

Browse files
lukedegruchyclaude
andauthored
Pre-validate measure libraries before per-subject CQL evaluation (#1024)
Measures whose CQL referenced a missing ValueSet (or had other fatal compile errors) failed in two awkward ways. With a function-based component stratifier, FunctionEvaluationHandler.processNonSubValueStratifier observed missing expression results when the IP failed and threw "Expression result: Initial Population is missing" — masking the underlying CqlException. In multi-measure evaluations, that masking exception then escaped the per-library loop in MeasureEvaluationResultHandler and was caught by the outer subject-scoped try/catch, which wrote the error to every measure def — poisoning unrelated libraries that would otherwise have evaluated cleanly. Two complementary layers: - MeasureLibraryPreValidator resolves each library before the per-subject loop, collects fatal compile errors via LibraryManager.resolveLibrary's error-collecting overload, and walks the compiled ELM ValueSet refs against the terminology provider. Failed libraries drop from the evaluation set with the engine-level diagnostic ("Unable to locate ValueSet …", or the original compile message) recorded against their measure defs. Hard library-resolution failures still propagate so existing throw-based contracts in InvalidMeasureTest are preserved. - MeasureEvaluationResultHandler.getEvaluationResults now checks getExceptionFor(libraryId) before invoking cqlFunctionEvaluation, so any per-subject CqlException that slips past pre-validation is surfaced through the existing per-subject error path instead of being masked by the function-evaluation handler. Tests: - MeasureLibraryPreValidatorTest exercises compile-error, missing- ValueSet, fall-through, short-circuit, and multi-library isolation cases against real MultiLibraryIdMeasureEngineDetails / MeasureDef / CompositeEvaluationResultsPerMeasure instances. - MultiMeasureIsolationTest evaluates three measures (clean / broken / clean) and asserts the broken one's failure does not poison the siblings' MeasureReports. - Four cohortEncounterMissingValueSet* integration tests live in InvalidMeasureTest covering no-stratifier, scalar, value-component, and function-component stratifier shapes — all surfacing the same "Unable to locate ValueSet" diagnostic. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c7896f2 commit d340296

20 files changed

Lines changed: 1202 additions & 20 deletions

cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluationResultHandler.java

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import java.time.ZonedDateTime;
88
import java.util.List;
99
import java.util.Map;
10-
import java.util.Optional;
1110
import org.apache.commons.lang3.tuple.Pair;
1211
import org.hl7.elm.r1.VersionedIdentifier;
1312
import org.opencds.cqf.cql.engine.execution.CqlEngine;
@@ -111,15 +110,31 @@ public static CompositeEvaluationResultsPerMeasure getEvaluationResults(
111110
// measure -> subject -> results
112111
var resultsBuilder = CompositeEvaluationResultsPerMeasure.builder();
113112

113+
// Pre-validate ValueSet references against the terminology provider before any
114+
// per-subject evaluation runs. Libraries with unresolvable ValueSets are recorded with
115+
// an error and excluded from the per-subject loop entirely — keeping the original
116+
// diagnostic ("Unable to locate ValueSet …") and avoiding wasted per-subject work.
117+
var preValidationFailedLibraries =
118+
new MeasureLibraryPreValidator(context).validate(multiLibraryIdMeasureEngineDetails, resultsBuilder);
119+
120+
final List<VersionedIdentifier> activeLibraryIdentifiers =
121+
multiLibraryIdMeasureEngineDetails.getLibraryIdentifiers().stream()
122+
.filter(id -> !preValidationFailedLibraries.contains(id))
123+
.toList();
124+
114125
// Library $evaluate each subject
115126
// The goal here is to do each measure/library evaluation within the context of a single subject.
116127
// This means that we will not switch between subject contexts while evaluating measures.
117128
// Once we've switched to a different subject context, the previous expression cache is dropped.
118129

119-
final List<String> libraryIdentIds = multiLibraryIdMeasureEngineDetails.getLibraryIdentifiers().stream()
130+
final List<String> libraryIdentIds = activeLibraryIdentifiers.stream()
120131
.map(VersionedIdentifier::getId)
121132
.toList();
122133

134+
if (activeLibraryIdentifiers.isEmpty()) {
135+
return resultsBuilder.build();
136+
}
137+
123138
logger.atDebug()
124139
.setMessage(
125140
"START: Evaluate measure for library idents: (count:{}): [{}], and subjects (count={}): [{}]")
@@ -150,7 +165,6 @@ public static CompositeEvaluationResultsPerMeasure getEvaluationResults(
150165
String subjectIdPart = subjectInfo.getRight();
151166
context.getState().setContextValue(subjectTypePart, subjectIdPart);
152167
try {
153-
var libraryIdentifiers = multiLibraryIdMeasureEngineDetails.getLibraryIdentifiers();
154168

155169
final long startPerLibraryPerSubject = System.currentTimeMillis();
156170
throttledDebug(shouldLog)
@@ -161,7 +175,7 @@ public static CompositeEvaluationResultsPerMeasure getEvaluationResults(
161175
var evaluationResultsForMultiLib = multiLibraryIdMeasureEngineDetails
162176
.getLibraryEngine()
163177
.getEvaluationResult(
164-
libraryIdentifiers,
178+
activeLibraryIdentifiers,
165179
subjectId,
166180
null,
167181
parametersMap,
@@ -177,7 +191,7 @@ public static CompositeEvaluationResultsPerMeasure getEvaluationResults(
177191
.addArgument(() -> showSubsetOfTotal(libraryIdentIds))
178192
.log();
179193

180-
for (var libraryVersionedIdentifier : libraryIdentifiers) {
194+
for (var libraryVersionedIdentifier : activeLibraryIdentifiers) {
181195
validateEvaluationResultExistsForIdentifier(
182196
libraryVersionedIdentifier, evaluationResultsForMultiLib);
183197

@@ -186,24 +200,28 @@ public static CompositeEvaluationResultsPerMeasure getEvaluationResults(
186200
var measureDefs =
187201
multiLibraryIdMeasureEngineDetails.getMeasureDefsForLibrary(libraryVersionedIdentifier);
188202

189-
// function evaluation
190-
final List<EvaluationResult> functionEvaluationResults =
191-
FunctionEvaluationHandler.cqlFunctionEvaluation(
192-
context,
193-
measureDefs,
194-
libraryVersionedIdentifier,
195-
evaluationResult,
196-
subjectTypePart);
203+
final var libraryException =
204+
evaluationResultsForMultiLib.getExceptionFor(libraryVersionedIdentifier);
205+
206+
// Skip function evaluation when the base CQL eval already failed for this
207+
// library/subject. Otherwise the function path (e.g. processNonSubValueStratifier)
208+
// observes missing expression results and throws a higher-level
209+
// "Expression result: <pop> is missing" error that masks the underlying
210+
// CqlException ("Unable to locate ValueSet", etc.) and — by escaping the inner
211+
// loop into the outer catch — pollutes sibling libraries' measure defs.
212+
final List<EvaluationResult> functionEvaluationResults = (libraryException == null)
213+
? FunctionEvaluationHandler.cqlFunctionEvaluation(
214+
context, measureDefs, libraryVersionedIdentifier, evaluationResult, subjectTypePart)
215+
: List.of();
197216

198217
resultsBuilder.addResults(measureDefs, subjectId, evaluationResult, functionEvaluationResults);
199218

200-
Optional.ofNullable(evaluationResultsForMultiLib.getExceptionFor(libraryVersionedIdentifier))
201-
.ifPresent(exception -> {
202-
var error = EXCEPTION_FOR_SUBJECT_ID_MESSAGE_TEMPLATE.formatted(
203-
subjectId, exception.getMessage());
204-
resultsBuilder.addErrors(measureDefs, error);
205-
logger.error(error, exception);
206-
});
219+
if (libraryException != null) {
220+
var error = EXCEPTION_FOR_SUBJECT_ID_MESSAGE_TEMPLATE.formatted(
221+
subjectId, libraryException.getMessage());
222+
resultsBuilder.addErrors(measureDefs, error);
223+
logger.error(error, libraryException);
224+
}
207225
}
208226

209227
} catch (Exception e) {
Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
package org.opencds.cqf.fhir.cr.measure.common;
2+
3+
import java.util.ArrayList;
4+
import java.util.HashSet;
5+
import java.util.List;
6+
import java.util.Optional;
7+
import java.util.Set;
8+
import org.cqframework.cql.cql2elm.CqlCompilerException;
9+
import org.cqframework.cql.cql2elm.LibraryManager;
10+
import org.cqframework.cql.cql2elm.model.CompiledLibrary;
11+
import org.hl7.elm.r1.ValueSetDef;
12+
import org.hl7.elm.r1.VersionedIdentifier;
13+
import org.opencds.cqf.cql.engine.execution.CqlEngine;
14+
import org.opencds.cqf.cql.engine.terminology.TerminologyProvider;
15+
import org.opencds.cqf.cql.engine.terminology.ValueSetInfo;
16+
import org.slf4j.Logger;
17+
import org.slf4j.LoggerFactory;
18+
19+
/**
20+
* Static pre-validation pass for measure libraries. Resolves each library, captures fatal compile
21+
* errors, and walks the compiled ELM ValueSet references to confirm they exist in the terminology
22+
* provider — all before any per-subject CQL evaluation runs.
23+
*
24+
* <p>Libraries with fatal compile errors or unresolvable ValueSet references are recorded as
25+
* errors against their bound measure defs and returned in the failed-set so the caller can
26+
* exclude them from per-subject CQL evaluation. This avoids two failure modes that would
27+
* otherwise occur at runtime:
28+
* <ul>
29+
* <li><b>Diagnostic masking</b>: function-based stratifier evaluation observes missing
30+
* expression results when the IP fails and throws a higher-level
31+
* "Expression result: &lt;pop&gt; is missing" error that hides the underlying cause.</li>
32+
* <li><b>Multi-measure pollution</b>: the masked exception escapes the inner library loop in
33+
* {@link MeasureEvaluationResultHandler} and is caught by the outer subject-scoped
34+
* try/catch, which writes the error to <em>every</em> measure def — including unrelated
35+
* libraries that compiled and would have evaluated cleanly.</li>
36+
* </ul>
37+
*
38+
* <p>Hard library-resolution failures (e.g. {@code CqlIncludeException} for a missing CQL source)
39+
* still propagate to the runtime CQL path so existing throw-based contracts in
40+
* {@code InvalidMeasureTest} are preserved.
41+
*/
42+
public class MeasureLibraryPreValidator {
43+
44+
private static final Logger logger = LoggerFactory.getLogger(MeasureLibraryPreValidator.class);
45+
private static final String EXCEPTION_FOR_LIBRARY_TEMPLATE = "Exception for library: %s, Message: %s";
46+
47+
private final CqlEngine engine;
48+
49+
public MeasureLibraryPreValidator(CqlEngine engine) {
50+
this.engine = engine;
51+
}
52+
53+
/**
54+
* Validate compile-time integrity for every library bound to the given measure defs.
55+
*
56+
* @param details measure-engine details holding the library-to-measure binding
57+
* @param resultsBuilder builder to record per-library validation errors against measure defs
58+
* @return library identifiers that failed validation (caller should exclude them from
59+
* per-subject CQL evaluation)
60+
*/
61+
public Set<VersionedIdentifier> validate(
62+
MultiLibraryIdMeasureEngineDetails details, CompositeEvaluationResultsPerMeasure.Builder resultsBuilder) {
63+
64+
var libraryManager = engine.getEnvironment().getLibraryManager();
65+
var terminologyProvider = engine.getEnvironment().getTerminologyProvider();
66+
var failed = new HashSet<VersionedIdentifier>();
67+
68+
for (var libraryId : details.getLibraryIdentifiers()) {
69+
findFirstFailure(libraryManager, terminologyProvider, libraryId)
70+
.ifPresent(error -> recordLibraryFailure(details, resultsBuilder, libraryId, error, failed));
71+
}
72+
return failed;
73+
}
74+
75+
private static Optional<RuntimeException> findFirstFailure(
76+
LibraryManager libraryManager, TerminologyProvider terminologyProvider, VersionedIdentifier libraryId) {
77+
78+
var compileErrors = new ArrayList<CqlCompilerException>();
79+
CompiledLibrary compiled;
80+
try {
81+
compiled = libraryManager.resolveLibrary(libraryId, compileErrors);
82+
} catch (RuntimeException exception) {
83+
// Hard failures (missing library source, etc.) preserve the existing throw-at-runtime
84+
// contract for callers that rely on the exception type.
85+
return Optional.empty();
86+
}
87+
88+
var firstCompileError = firstFatalCompileError(compileErrors);
89+
if (firstCompileError.isPresent()) {
90+
return firstCompileError;
91+
}
92+
93+
return findFirstUnresolvableValueSet(terminologyProvider, compiled);
94+
}
95+
96+
private static Optional<RuntimeException> firstFatalCompileError(List<CqlCompilerException> errors) {
97+
return errors.stream()
98+
.filter(exception -> exception.getSeverity() == CqlCompilerException.ErrorSeverity.Error)
99+
.map(RuntimeException.class::cast)
100+
.findFirst();
101+
}
102+
103+
private static Optional<RuntimeException> findFirstUnresolvableValueSet(
104+
TerminologyProvider terminologyProvider, CompiledLibrary compiled) {
105+
106+
for (var vsDef : valueSetDefs(compiled)) {
107+
var info = toValueSetInfo(vsDef);
108+
if (info != null) {
109+
try {
110+
terminologyProvider.expand(info);
111+
} catch (RuntimeException e) {
112+
return Optional.of(e);
113+
}
114+
}
115+
}
116+
return Optional.empty();
117+
}
118+
119+
private static List<ValueSetDef> valueSetDefs(CompiledLibrary compiled) {
120+
if (compiled == null || compiled.getLibrary() == null) {
121+
return List.of();
122+
}
123+
var valueSets = compiled.getLibrary().getValueSets();
124+
return valueSets == null ? List.of() : valueSets.getDef();
125+
}
126+
127+
private static ValueSetInfo toValueSetInfo(ValueSetDef vsDef) {
128+
if (vsDef.getId() == null) {
129+
return null;
130+
}
131+
var info = new ValueSetInfo().withId(vsDef.getId());
132+
if (vsDef.getVersion() != null) {
133+
info.setVersion(vsDef.getVersion());
134+
}
135+
return info;
136+
}
137+
138+
private static void recordLibraryFailure(
139+
MultiLibraryIdMeasureEngineDetails details,
140+
CompositeEvaluationResultsPerMeasure.Builder resultsBuilder,
141+
VersionedIdentifier libraryId,
142+
RuntimeException error,
143+
Set<VersionedIdentifier> failed) {
144+
var measureDefs = details.getMeasureDefsForLibrary(libraryId);
145+
var formatted = EXCEPTION_FOR_LIBRARY_TEMPLATE.formatted(libraryId.getId(), error.getMessage());
146+
resultsBuilder.addErrors(measureDefs, formatted);
147+
logger.error(formatted, error);
148+
failed.add(libraryId);
149+
}
150+
}

0 commit comments

Comments
 (0)