Skip to content

Support expression evaluation for multiple libraries at once#1584

Merged
JPercival merged 56 commits intomasterfrom
ld_20250709_lib_eval_multiple_keep_cache
Aug 11, 2025
Merged

Support expression evaluation for multiple libraries at once#1584
JPercival merged 56 commits intomasterfrom
ld_20250709_lib_eval_multiple_keep_cache

Conversation

@lukedegruchy
Copy link
Copy Markdown
Contributor

@lukedegruchy lukedegruchy commented Jul 9, 2025 •
edited
Loading

  • The goal of this PR is to optimize CQL library expression evaluation, in particular, for multiple libraries that leverage expressions from the same common libraries, ensuring that where we would otherwise re-evaluate a given common library expression across multiple targeted libraries, we are now retrieving from the expression cache when encountering that common expression at subsequent points for subsequent top-level libraries
  • Enhance LibraryManager to resolve and compile multiple libraries at once
  • Enhance error handling to support partial results, namely, some libraries resolving successfully and others resulting in errors
  • Support backward compatibility with single library behaviour as much as possible
  • Ensure we can retrieve libraries from a combination of cache retrieval and new library compile for multiple libraries
  • Split up logic used in initializing single libraries to support multiple libraries without resetting the expression cache
  • Once libraries are initialized, traverse them in reverse order to evaluate the expression set for each library
  • When evaluating an expression, ensure we only set the context value once, ensuring we don't prematurely clear the expression cache, which is a crucial aspect of this PR's optimization
  • Once all expressions are evaluated for a given library, move to the next current library in the stack, as well as the next set of evaluated resources
  • Introduce new multi-library, multi-error supporting results objects to simplify the contract for internal and external client code for interacting with the results of library compilation, library loading, and library evaluation.
  • Lots and lots of new tests.

@lukedegruchy
First attempt to evaluate mulitple libraries without a cache refresh,…
0e93085 
… namely, by initializing them in order and then evaluating their expressions in reverse order.
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 9, 2025 •
edited
Loading

Formatting check succeeded!

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 9, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 82.80702% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.27%. Comparing base (9ad86f0) to head (a8aa38a).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...rg/opencds/cqf/cql/engine/execution/CqlEngine.java 81.92% 11 Missing and 4 partials ⚠️
...va/org/cqframework/cql/cql2elm/LibraryManager.java 83.09% 7 Missing and 5 partials ⚠️
...engine/execution/EvaluationResultsForMultiLib.java 83.33% 4 Missing and 4 partials ⚠️
...s/cqf/cql/engine/execution/LoadMultiLibResult.java 80.64% 4 Missing and 2 partials ⚠️
...cqframework/cql/cql2elm/CompiledLibraryResult.java 20.00% 4 Missing ⚠️
...ework/cql/cql2elm/CompiledLibraryMultiResults.java 93.54% 1 Missing and 1 partial ⚠️
...va/org/opencds/cqf/cql/engine/execution/State.java 84.61% 0 Missing and 2 partials ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##             master    #1584      +/-   ##
============================================
+ Coverage     65.07%   65.27%   +0.19%     
- Complexity     1976     1977       +1     
============================================
  Files           499      503       +4     
  Lines         28828    29046     +218     
  Branches       5671     5686      +15     
============================================
+ Hits          18761    18959     +198     
- Misses         7743     7757      +14     
- Partials       2324     2330       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

lukedegruchy added 27 commits July 9, 2025 17:10
@lukedegruchy
Remove duplicate initializeState and setParameters that was wrongly r…
f32a546 
…esulting in correct results. Ensure that when evaluating expressions that we pop the current library off the deque but not too soon, so the reverse order evaluation actually works. Add more TODOs.
@lukedegruchy
Make use of the new evaluate measure code from at least one single li…
7dc0749 
…brary call. Fixed invalid error handling before. All tests pass.
@lukedegruchy
Leave the existing single library evaluation the same as before. Ensu…
13aff67 
…re that the new multi-library fails on library ID and CQL ID mismatches.
@lukedegruchy
Pull changes for versioned identifier validating branch.
3a7692f
@lukedegruchy
Spotless.
90fa6dc
@lukedegruchy
Start merging the old and new code bases for compiling and validating…
0e8774e 
… libraries in LibraryManager. Ensure all tests pass.
@lukedegruchy
Improve matching Libraries with queried identifiers and solidifying v…
12af5d1 
…ersionless identifier searching.
@lukedegruchy
Add logging and TODOs, which may be temporary.
5092587
@lukedegruchy
Support multi-measure error handling and partial error handling thoug…
c624ce0 
…ht a series of bespoke combined result and error classes.
@lukedegruchy
Merge remote-tracking branch 'origin/master' into ld_20250709_lib_eva…
be9c632 
…l_multiple_keep_cache
@lukedegruchy
Fix bad merge from master.
d183311
@lukedegruchy
Fix partial error handling scenario.
1eff9f2
@lukedegruchy
Spotless.
0055b7d
@lukedegruchy
Clear out the evaluatedResources stack after evaluating each set of e…
fc77638 
…xpressions for each library in the multi-lib scenario. Start writing tests to ensure it all behaves as expected, for now only covering
@lukedegruchy
Enhance tests and rename them to make it clear they're for linear mul…
bc38565 
…tiple deps. Add convenience methods to result class. Fix bug with partially cached multiple libs. Ensure we handle the non Cache.READ_WRITE case.
@lukedegruchy
Spotless.
561cc7c
@lukedegruchy
Add failing test for mismatched libraries due to caching order.
4158d5f
@lukedegruchy
Fix bug and spotless.
0f6dc91
@lukedegruchy
First steps to setting up multi-layer CQL tests.
df98855
@lukedegruchy
Improve testing for complex library dependencies but the work is stil…
b40831e 
…l not done.
@lukedegruchy
More improvements to testing but more work to be done on complex scen…
a25b1b1 
…arios.
@lukedegruchy
All tests pass for complex scenario. Pave the way for making CQL load…
5701a1d 
…ing more efficient.
@lukedegruchy
Add TODOs
7a15bc3
@lukedegruchy
Add small tweaks to error handling and bump to a new version higher t…
4f2403b 
…han master.
@lukedegruchy
Start cleaning up cruft and TODOs and move inner classes to top level.
fbf5f20
@lukedegruchy
More refactoring and better use of dedicated classes instead of ad-ho…
7525ad4 
…c collections everywhere.
@lukedegruchy
Get rid of ExpressionCacheTest.java since it's no longer relevant in …
b4489c3 
…the multi-lib world.
@lukedegruchy
Fix TODOs.
1ebc3b6
@lukedegruchy
Fix more TODOs and get rid of more cruft.
e60df8d
@lukedegruchy
Resolve more TODOs regarding immutability.
56a4f6a
@lukedegruchy
More unit testing. Address TODOs.
7e4467a
@lukedegruchy
Resolve some TODOs. Add javadoc.
484c636
@lukedegruchy
Add counters for expression def evaluation.
06af1b3
@lukedegruchy
Resolve some TODOs, add some logging, and optimize the compile librar…
a0e6767 
…y workflow some more.
@lukedegruchy
Optimize library compile code to enhance reuse and not to pass around…
efd192e 
… collections.
@lukedegruchy
Add more tests. Refactor library resolution and compilation to be mor…
16bfd95 
…e consistent, introducing a CompiledLibraryMultiResults class.
@lukedegruchy
Implement all tests. Get rid of tons of logs. Use Collections.unmodif…
0c63d59 
…iableList() whenever possible. Revert temp change with stubbed version.
@lukedegruchy
Merge remote-tracking branch 'origin/master' into ld_20250709_lib_eva…
13aebdc 
…l_multiple_keep_cache
@lukedegruchy lukedegruchy changed the title First attempt to evaluate mulitple libraries without a cache refresh,… Support expression evaluation for multiple libraries at once Aug 8, 2025
@lukedegruchy lukedegruchy marked this pull request as ready for review August 8, 2025 16:00
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Aug 8, 2025

Quality Gate Passed Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
81.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@JPercival JPercival merged commit dd4e509 into master Aug 11, 2025
7 checks passed
@JPercival JPercival deleted the ld_20250709_lib_eval_multiple_keep_cache branch August 11, 2025 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JPercival JPercival JPercival approved these changes

+1 more reviewer

@Capt-Mac Capt-Mac Capt-Mac approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lukedegruchy @JPercival @Capt-Mac