Interval Operator Overhaul by cmoesel · Pull Request #373 · cqframework/cql-execution

cmoesel · 2026-07-21T15:04:45Z

This PR replaces #365 by aligning (almost) all interval operators more closely with the exact language and algorithms prescribed in the spec and using the Start and End operators to determine interval boundaries. In addition, interval point types are detected more accurately (using resultType where possible) to ensure accurate interval representations when both boundaries are null. As a result, handling of closed null and open null boundaries has been greatly improved.

Other tangentially related improvements have been made throughout to address additional issues that arose in implementation and testing of the interval operators, especially in regard to arithmetic functions, precision, and data type limits (min/max).

Note:

Interval expand and collapse are quite complex. Refactoring these operators should be addressed in a future PR.
The spec has diverging definitions of how properly contains should work. I've refactored it to work how I think it should work, but I've done it in a separate commit in case we need to back that out. Check the jira issue and zulip conversation for updates.

Submitter:

This pull request describes why these changes were made
Code diff has been done and been reviewed (it does not contain: additional white space, not applicable code changes, debug statements, etc.)
Tests are included and test edge cases
Tests have been run locally and pass
Code coverage has not gone down and all code touched or added is covered.
Code passes lint and prettier (hint: use npm run test:plus to run tests, lint, and prettier)
All dependent libraries are appropriately updated or have a corresponding PR related to this change
cql4browsers.js built with npm run build:browserify if source changed.

Reviewer:

Name:

Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
The tests appropriately test the new code, including edge cases
You have tried to break the code

cmoesel · 2026-07-21T15:08:16Z

@lmd59 - I added you as a reviewer since you reviewed #365 (which this replaces), but if you don't have time, then I think anyone else on the team would be fine. @dehall will also review, but this one is kind of big, so two reviewers might be good -- especially if someone from your team can regress it with fqm.

codecov-commenter · 2026-07-21T15:12:28Z

Codecov Report

❌ Patch coverage is 90.95238% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.57%. Comparing base (e8dddb1) to head (dd09d09).

Files with missing lines	Patch %	Lines
src/datatypes/interval.ts	90.23%	16 Missing and 9 partials ⚠️
src/util/math.ts	83.78%	6 Missing and 6 partials ⚠️
src/elm/interval.ts	92.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #373      +/-   ##
==========================================
+ Coverage   88.27%   89.57%   +1.30%     
==========================================
  Files          54       55       +1     
  Lines        4877     4855      -22     
  Branches     1374     1391      +17     
==========================================
+ Hits         4305     4349      +44     
+ Misses        377      322      -55     
+ Partials      195      184      -11

☔ View full report in Codecov by Harness.
📢 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.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dehall

I haven't had time to fully finish reviewing but overall it looks good. Submitting the few comments I do have now, since I'll be out Monday

…tors - Redefine Start and End operators based on clarified definitions in upcoming CQL 2.0 - Update interval operators to follow CQL spec implementation language as closely as possible - Preserve interval point types and improve arithmetic, precision, and limit handling - Expand unit, integration, and specification test coverage - Reorganize some exports and imports to avoid circular imports - NOTE: expand and collapse implementations have not yet been updated as they are considerably more complex

The logical definition and reference definition for proper contains/includes disagree. The reference definition seems more consistent to me, so I've implemented that while we ask the CQL community.

- Move pointFrom to be with other spec-defined operations - Add documentation links and quotes to pointFrom

…daysInMonth

- Add Jira link to comment about keeping quantity unit for min/max values - Use quantityInstance more consistently for min/max values - Remove unused maxValueForInstance and minValueForInstance functions

cmoesel · 2026-07-27T20:18:51Z

@dehall and @lmd59 - this is ready for re-review. I also updated all of the dependencies that could be updated and documented the others in DEPENDENCY-NOTES.md.

dehall

I found just a couple more places where a call to .low or .high needs to consider uncertainties to prevent errors, but overall I think this is really close. After those are addressed I'll be good to approve. Two other comments are just on comments, and maybe those are already how we want them/as good as they can be at this point.

cmoesel · 2026-07-28T21:26:26Z

Thanks for the review, @dehall. Good finds. I've addressed them now, so this is ready for re-review (again!).

dehall

Looks good! Per the last comment, there's one thing I'd be ok with removing but since it's there and tested I'm also fine with leaving it in.

The custom equalInterval assertion did not work correctly for Intervals w/ uncertain boundaries. Now it does!

cmoesel · 2026-07-29T18:30:30Z

I don't know why the CI failed, but I re-ran it (without changing anything) and it passed. I thought that would update the UI here, but I'm not seeing it. Here are the updated results: https://github.com/cqframework/cql-execution/actions/runs/30477306092?pr=373

lmd59

Regression for fqm-execution looks good to me!

cmoesel requested a review from dehall July 21, 2026 15:05

cmoesel mentioned this pull request Jul 21, 2026

Improve handling of boundless and unknown intervals #365

Closed

11 tasks

cmoesel requested a review from lmd59 July 21, 2026 15:06

dehall reviewed Jul 24, 2026

View reviewed changes

Comment thread src/datatypes/datetime.ts Outdated

Comment thread test/spec-tests/skip-list.txt

Comment thread test/datatypes/date-test.ts

Comment thread src/util/math.ts Outdated

cmoesel added 3 commits July 27, 2026 08:34

Implement CQL reference definition for proper contains

5cf3405

The logical definition and reference definition for proper contains/includes disagree. The reference definition seems more consistent to me, so I've implemented that while we ask the CQL community.

Post-Rebase Tweaks

b75ad37

- Move pointFrom to be with other spec-defined operations - Add documentation links and quotes to pointFrom

cmoesel force-pushed the interval-operator-overhaul branch from 85b5f9a to b75ad37 Compare July 27, 2026 13:06

cmoesel added 7 commits July 27, 2026 10:16

Fix browserify

efb9127

Remove now-unused Date/Time methods: getFieldFloor, getFieldCieling, …

7d2f99d

…daysInMonth

Skip slice tests

d8d8cdc

Tweaks related to quantity instance for min/max values

11111d5

- Add Jira link to comment about keeping quantity unit for min/max values - Use quantityInstance more consistently for min/max values - Remove unused maxValueForInstance and minValueForInstance functions

npm audit fix

412a07a

Update dependencies where possible

c5e9ae2

Upgrade TypeScript to v6

0feb777

cmoesel requested a review from dehall July 27, 2026 20:18

dehall requested changes Jul 28, 2026

View reviewed changes

Comment thread src/datatypes/interval.ts Outdated

Comment thread src/datatypes/interval.ts

Comment thread src/datatypes/interval.ts Outdated

Comment thread src/datatypes/interval.ts Outdated

Improve handling of null / uncertain boundaries

af4da33

cmoesel requested a review from dehall July 28, 2026 21:25

browserify

dba9632

dehall approved these changes Jul 29, 2026

View reviewed changes

Comment thread src/datatypes/interval.ts Outdated

Fix equalInterval should.js assertion

0a06b37

The custom equalInterval assertion did not work correctly for Intervals w/ uncertain boundaries. Now it does!

lmd59 approved these changes Jul 30, 2026

View reviewed changes

Add more context to comment for normalizeInterval

dd09d09

cmoesel merged commit 214a25b into master Jul 31, 2026
14 checks passed

cmoesel deleted the interval-operator-overhaul branch July 31, 2026 17:41

Uh oh!

Conversation

cmoesel commented Jul 21, 2026

Uh oh!

cmoesel commented Jul 21, 2026

Uh oh!

codecov-commenter commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dehall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cmoesel commented Jul 27, 2026

Uh oh!

dehall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cmoesel commented Jul 28, 2026

Uh oh!

dehall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cmoesel commented Jul 29, 2026

Uh oh!

lmd59 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Jul 21, 2026 •

edited

Loading