Add and leverage a wrapper class for CQL ExpressionResult raw values.#1020
Draft
lukedegruchy wants to merge 15 commits into
Draft
Add and leverage a wrapper class for CQL ExpressionResult raw values.#1020lukedegruchy wants to merge 15 commits into
lukedegruchy wants to merge 15 commits into
Conversation
Introduce CqlExpressionValue, a wrapper around the raw Object returned by ExpressionResult.getValue(), to consolidate the null / Boolean / Iterable / scalar normalization that was previously duplicated across measure-evaluation call sites. Phase 1 adopts the wrapper at three boundaries: - MeasureEvaluator.evaluatePopulationCriteria / evaluateSupportingCriteria - FunctionEvaluationHandler.getResultIterable - PopulationBasisValidator's two-stage null checks The wrapper's internal raw field is intentionally typed as Object so a future migration to the upstream sealed CQL Value type touches only this class, not its callers. CqlExpressionValueException (plain RuntimeException) replaces a HAPI InternalErrorException previously thrown on the subject-context lookup path. Out of scope for Phase 1: collapsing CriteriaResult and StratumValueWrapper, and pushing the wrapper through the Def-class signatures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 2 of the wrapper migration. Adds valueAsSet() and nonNullValues() to CqlExpressionValue (matching the existing CriteriaResult contract, including HashSetForFhirResourcesAndCqlTypes identity semantics for valueAsSet) and switches the per-subject result maps on SdeDef, StratifierDef, and StratifierComponentDef from CriteriaResult to CqlExpressionValue. Updates the only external consumers (MeasureMultiSubjectEvaluator and Dstu3MeasureReportBuilder) to call raw() / valueAsSet() / nonNullValues() on the new wrapper. The CriteriaResult class is now redundant and is removed; its unused NULL_VALUE / EMPTY_RESULT sentinels go with it. The public StratifierDef.getAllCriteriaResultValues() method retains its name to avoid disturbing call sites; only its return type semantics (now backed by CqlExpressionValue) change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Formatting check succeeded! |
…ed-expression-results-wrapper-class
Extends CqlExpressionValue with isMap() / asMap() and replaces every remaining instanceof Map / Map.class::isInstance / unsafe-cast site that handles MEASUREOBSERVATION accumulators (Map<inputResource, outputValue>) so the lone unchecked cast lives in one tested place. Also fixes a latent bug in MeasureEvaluator.retainObservationResources InPopulation: the previous loop modified the same Set it was iterating over (working only by reference-aliasing accident). Replaced with the collect-then-removeAll pattern. Sites migrated: - MeasureEvaluator: retainObservationSubjectResourcesInPopulation, retainObservationResourcesInPopulation (+ bug fix), removeObservatorySubjectResource - MeasureMultiSubjectEvaluator: collectFunctionRowKeys, mapToListOfTableEntries, stratifierResultAsIntersectionSet, getPopulationResourceKeySet - MeasureReportDefScorer: getResultsForStratumByResourceIds - PopulationDef: countObservations, removeExcludedMeasureObservation Resource PopulationDef.subjectResources field type is unchanged; that's a larger architectural change reserved for a future phase. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A handful of small post-Phase-3a refinements that all match the wrapper-migration theme and get the remaining instanceof Map / raw Object passthroughs out of the way: - MeasureEvaluator.retainObservationResourcesInPopulation: switch from collect-then-removeAll(List) to Set.removeIf, silencing a static analyzer warning about Set.removeAll(List) being O(n*m) in the worst case and lining this method up with its sibling. - MeasureObservationHandler.removeObservationResourcesInPopulation: drop instanceof Map in favour of CqlExpressionValue.asMap(). - MeasureScoreCalculator.collectQuantities: replace the filter(instanceof Map).map(cast) chain with the wrapper's asMap() flatMap. Method signature unchanged. - StratifierUtils.extractClassesFromSingleOrListResult: change the parameter from Object to CqlExpressionValue and dispatch via isNull/isIterable/asIterable. The two callers in PopulationBasisValidator drop their .raw() passthrough. - StratifierDef: delete the private toSet helper and route through CqlExpressionValue.valueAsSet() directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tidies up the satellite call sites that did the same null / Iterable /
Map dispatch as the wrapper but in their own raw form. No behaviour
change; consistent vocabulary across the package.
- EvaluationResultFormatter.formatValue / printValue: wrapper-based
isIterable / asMap dispatch. The {empty} sentinel for empty Maps
is preserved.
- StratumValueWrapper: delete the private isEmptyCollection helper
(it was a near-duplicate of wrapper.isEmpty()) and route its three
callers through the wrapper. Drops the java.util.Collection and
java.util.Map imports along with it.
- R4SupportingEvidenceExtension.classifyValue / collectLeavesInto:
wrapper-based dispatch for the recursive evidence-flattening paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switches MeasureScoreCalculator.collectQuantities from Collection<Object> to Collection<CqlExpressionValue>. The body simplifies to a single stream chain over CqlExpressionValue::asMap, and the only production caller (MeasureReportDefScorer.calculate ContinuousVariableAggregateQuantity) wraps at the boundary using CqlExpressionValue.ofRaw. Tests gain a small wrap(...) helper. The boundary wrap is a transient shim: when PopulationDef.subject Resources eventually moves to typed storage, the wrap goes away (captured in a MIGRATION-NOTE at that boundary). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds MIGRATION-NOTE comments at the major call sites that will need to change when PopulationDef.subjectResources moves from Map<String, Set<Object>> to a typed container (Set<CqlExpressionValue> or a new PopulationResultSet). Each note captures: what changes at that site, the equality-semantics consideration where relevant, and test-focus areas for that flavour of measure. Sites covered: - PopulationDef: the field declaration (central design note) and addResource (single insertion point) - MeasureEvaluator: three observation methods that walk the Set - MeasureMultiSubjectEvaluator: getPopulationResourceKeySet (three branches by population basis) and stratifierResultAsIntersectionSet (load-bearing for Sets.intersection equality) - MeasureObservationHandler: HashSetForFhirResourcesAndCqlTypes copy - R4MeasureReportBuilder / Dstu3MeasureReportBuilder: the FHIR-build consumers - HashSetForFhirResourcesAndCqlTypes: javadoc note on the two options for wrapper identity Searchable via grep "MIGRATION-NOTE (typed-subjectResources)". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Promotes PopulationDef.subjectResources from Map<String, Set<Object>> to Map<String, Set<CqlExpressionValue>>, eliminating the last raw-Object storage in the population-results pipeline. Equality strategy: a new sister type HashSetForCqlExpressionValues mirrors HashSetForFhirResourcesAndCqlTypes but unwraps each element via .raw() before applying FHIR-resource / CQL-type identity rules. The wrapper itself stays generic (no equals/hashCode override), and per-subject sets remain small enough that linear-time identity checks are acceptable. Migrated: - PopulationDef accessors (addResource, getResourcesForSubject, getAllSubjectResources, getSubjectResources, countObservations, removeExcludedMeasureObservationResource) all speak in CqlExpressionValue. addResource is the single wrap point. - MeasureEvaluator's three observation methods (retainObservation SubjectResourcesInPopulation, retainObservationResourcesInPopulation, removeObservatorySubjectResource) take typed parameters and drop per-item ofRaw() wraps and the legacy Set<Object> casts. - MeasureMultiSubjectEvaluator: getPopulationResourceKeySet, the resourceIds builder, and calculateCriteriaStratifierIntersection (now manually intersects via raw() rather than Sets.intersection across mismatched element types). - MeasureReportDefScorer: calculateContinuousVariableAggregateQuantity drops the boundary-wrap shim; getResultsForStratum and getResultsForStratumByResourceIds return wrapped collections. - MeasureObservationHandler: copy uses HashSetForCqlExpressionValues; exclusion lookup unwraps to raw via wrapper.raw() at one site. - R4MeasureReportBuilder + Dstu3MeasureReportBuilder: builder consumers unwrap via .raw() at the FHIR-resource-id boundary. - EvaluationResultFormatter.printSubjectResources: unwraps wrappers before formatting. Tests: PopulationDefTest's helper unwraps wrappers; the assertions that used to call getAllSubjectResources().contains(rawValue) route through the FHIR-identity helper. MeasureObservationHandlerTest switches from direct subjectResources.put() to the public addResource() API everywhere. All MIGRATION-NOTE breadcrumbs left for this phase are now removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After the typed-subjectResources migration, several spots dereferenced .raw() or called wrapper methods without first guarding against null wrappers (which the underlying HashSet permits in principle). This adds defensive null filters / null-checks at the iteration boundaries. Spots tightened: - MeasureEvaluator: removeIf lambdas in retainObservation SubjectResourcesInPopulation, retainObservationResourcesInPopulation, and removeObservatorySubjectResource skip null wrappers; the removeObservatorySubjectResource null-then-isMap check no longer NPEs on a null first element. - MeasureMultiSubjectEvaluator: stream chains over Set<CqlExpression Value> add Objects::nonNull filters before .map(CqlExpressionValue:: raw / asMap), and the criteria-stratifier intersection skips null wrappers / null raw values explicitly. - MeasureObservationHandler: removeMatchingKeysFromObservationMap skips null exclusion wrappers. - MeasureReportDefScorer: getResultsForStratumByResourceIds adds Objects::nonNull after the flatMap before consuming wrappers. - PopulationDef: removeExcludedMeasureObservationResource null-guards the forEach and removeIf elements. - R4MeasureReportBuilder: getPopulationResourceIds and the populationSet filter null-guard the wrapper before calling .raw(); the date-of- compliance pull splits the iterator chain so the wrapper itself can be null-checked separately. - Dstu3MeasureReportBuilder: stratifier groupingBy null-guards the wrapper looked up via Map.get before calling .raw(). - EvaluationResultFormatter: printSubjectResources filters null wrappers before mapping to .raw(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lukedegruchy
changed the title
The MEASUREOBSERVATION population accumulator was a Map<Object, Object> with implicit semantics (FHIR-identity keys, QuantityDef values). Replace it with two records: ObservationEntry(inputResource, observation) and ObservationAccumulator wrapping a List<ObservationEntry>. The accumulator wraps the list in a non-Iterable record so the upstream asIterable() path doesn't unroll it into individual entries. CqlExpressionValue gains asObservationAccumulator() alongside asMap(); generic asMap() is kept because supporting-evidence and formatting code still consumes arbitrary CQL Maps that are not observation accumulators. This is commit A of three. Commit B will give the same treatment to the stratifier function-result Map; commit C will delete HashMapForFhirResourcesAndCqlTypes once both are migrated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the previous commit for non-subject-value stratifier function results. FunctionEvaluationHandler.processNonSubValueStratifier now produces a FunctionResultAccumulator wrapping List<FunctionResultEntry(input, output)> instead of Map<Object, Object>. The accumulator is a non-Iterable record so the upstream asIterable() path doesn't unroll it. CqlExpressionValue gains asFunctionResultAccumulator() alongside asObservationAccumulator(). MeasureMultiSubjectEvaluator's three function- result consumers (collectFunctionRowKeys, mapToListOfTableEntries via addFunctionResultRows, stratifierResultAsIntersectionSet) read entries through the typed accessor. asMap() / isMap() are kept since R4SupportingEvidenceExtension and EvaluationResultFormatter still process arbitrary CQL Map values that are not function-result accumulators. After this commit HashMapForFhirResourcesAndCqlTypes is unused by any production code; commit C will remove it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ed-expression-results-wrapper-class
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch centralizes the handling of the raw
Objectvalues that flow out of the CQL engine throughExpressionResult.getValue()into a single wrapper class, and replaces the two anonymousMap<Object, Object>accumulators that the measure-evaluation pipeline used internally with typed records. Most call sites that previously had to do their owninstanceof Map/instanceof Iterable/instanceof Booleanreasoning now go through narrow, well-named accessors. The pipeline's internal contract for "value associated with a population result" is now expressed in code rather than implied by convention. There are zero changes to integration-style tests; behaviour parity is guaranteed by the existing measure end-to-end suite.CqlExpressionValuewrapper around the rawObjectreturned fromExpressionResult.getValue(). Centralizes null/Boolean/Iterable/Map/scalar normalization and exposes typed accessors (asBoolean,asMap,asObservationAccumulator,asFunctionResultAccumulator,asIterable,valueAsSet,nonNullValues,resolveForPopulation). Replaces the previous pattern of scatteredinstanceofcasts and absorbs the responsibilities of the deletedCriteriaResultclass.PopulationDef.subjectResourcesfield type tightened fromMap<String, Set<Object>>toMap<String, Set<CqlExpressionValue>>, backed by a newHashSetForCqlExpressionValueswhose membership semantics useFhirResourceAndCqlTypeUtils.areObjectsEqualon the wrapped raw value (FHIR resource identity for resources, CQL-type equality otherwise). The single insertion pointaddResource(String, Object)is the only place that wraps raw values intoCqlExpressionValue.MEASUREOBSERVATIONaccumulator produced byFunctionEvaluationHandler.processMeasureObservationis nowObservationAccumulator(List<ObservationEntry(inputResource, observation)>)instead of an opaqueMap<Object, Object>. Theobservationfield is statically typed asQuantityDef, eliminating downstreamQuantityDef::isInstancefiltering. The accumulator is wrapped in a non-Iterable record so the upstreamasIterable()path does not unroll it into individual entries.FunctionEvaluationHandler.processNonSubValueStratifieris nowFunctionResultAccumulator(List<FunctionResultEntry(input, output)>)instead ofMap<Object, Object>, with the same non-Iterable record pattern. Consumers inMeasureMultiSubjectEvaluator(collectFunctionRowKeys,mapToListOfTableEntries/addFunctionResultRows,stratifierResultAsIntersectionSet,getPopulationResourceKeySet) read entries through the typed accessor.MeasureObservationHandler.removeObservationResourcesInPopulation(cancelled-encounter exclusions for continuous-variable observations) is now expressed by iterating typedObservationEntryrows and rebuilding accumulators when entries are filtered out, replacing in-placeMap.removemutation. Empty accumulators continue to be purged so subject counts behave the same way.