Improve handling of boundless and unknown intervals by cmoesel · Pull Request #365 · cqframework/cql-execution

cmoesel · 2026-05-29T16:39:13Z

Add special case logic for boundless intervals (e.g., Interval[null, null]) and unknown intervals (e.g., Interval(null, null)).

When we process an Interval that has both boundaries null, we can't infer the point type since we have no value to infer it from. This caused problems in the existing logic that needs to know the point type for some calculations. To address this, I've added special-case logic for these edge cases since we often don't really need to know the point type to arrive at the correct answer.

I initially tried implementing this by using the point type of the other argument in each operation, but this turned out to be kind of messy and still was not perfect (in particular, it failed tests like Interval[null, null] overlaps Interval[null, null]).

My CQL Long PR introduces the approach of using the resultType from the ELM when we can't easily infer types from the data. We should extend that to also work in these cases, but since we want to get this fix out ASAP, I decided to implement this approach first. We need this approach regardless since ELM does not always carry the resultType.

NOTE: The CI that runs npm audit currently fails, but the reported audit issues are currently not resolvable, so there's not much we can do.

Fixes #364.

Pull requests into cql-execution require the following.
Submitter and reviewer should ✔ when done.
For items that are not-applicable, mark "N/A" and ✔.

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

Add special case logic for boundless intervals (e.g., Interval[null, null]) and unknown intervals (e.g., Interval(null, null)

codecov-commenter · 2026-05-29T16:39:42Z

Codecov Report

❌ Patch coverage is 77.19298% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.56%. Comparing base (5bc96b7) to head (fd13dd2).

Files with missing lines	Patch %	Lines
src/datatypes/interval.ts	77.19%	0 Missing and 13 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #365      +/-   ##
==========================================
- Coverage   87.58%   87.56%   -0.02%     
==========================================
  Files          52       52              
  Lines        4606     4658      +52     
  Branches     1297     1331      +34     
==========================================
+ Hits         4034     4079      +45     
+ Misses        359      354       -5     
- Partials      213      225      +12

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

lmd59

Partway through. Just putting up some initial comments/questions now for discussion and avoiding overlap since @elsaperelli is also reviewing!

lmd59 · 2026-05-29T18:21:34Z

+  get isUnknownInterval() {
+    return this.low == null && !this.lowClosed && this.high == null && !this.highClosed;
+  }
+


In addition to these two cases, do we need to do any handling of mixed unknown/boundless intervals? i.e. Interval[null, null) and Interval(null, null]

So edge-casey! TBH, I wouldn't be surprised if some of these mixed ones (with closed null and open null boundaries) might not return correct results. But I also think it would complicate the logic a lot, so I'd prefer to wait until we implement a fuller solution (including using resultTypes).

lmd59 · 2026-05-29T19:18:37Z

@@ -1481,7 +1491,7 @@ describe('Ends', () => {
  });

  it('should calculate to null', async function () {


Suggested change

it('should calculate to null', async function () {

it('should calculate to false for bounded interval ends boundless interval', async function () {

lmd59 · 2026-05-29T19:36:47Z

 define OverlapsAfterRealIvl: Interval[1.345, 1.678] overlaps Interval[1.234, 1.567]
 define OverlapsBoundaryRealIvl: Interval[1.0, 1.234] overlaps Interval[1.234, 2.0]
 define NoOverlapsRealIvl: Interval[1.0, 1.23456789) overlaps Interval[1.23456789, 2.0]
+define OverlapsClosedNullIntervalLHS: (Interval[null, null] as Interval<Integer>) overlaps Interval[6, 10]


Could we add tests like this without the cast? Since the engine may not always receive fully/properly typed [null, null] intervals, it seems we should test with the untyped version as well.

It shouldn't make a difference because our runtime can't tell what it is (cast or no cast), but I can make that change in the other PR.

lmd59

Adding some more comments. Mostly done looking through with the exception of a close look at test/datatypes/interval-test.ts

lmd59 · 2026-05-29T21:18:57Z

  }

  sameOrBefore(other: any, precision?: any) {
+    if (this.isBoundlessInterval || other?.isBoundlessInterval) {


I can't wrap my head around this one. This returns true if both intervals are boundless. SameOrBefore says true if the ending point of the first interval is less than or equal to the starting point of the second interval, which would not be true for both intervals boundless.

lmd59 · 2026-05-29T21:20:22Z

  }

  sameOrAfter(other: any, precision?: any) {
+    if (this.isBoundlessInterval || other?.isBoundlessInterval) {


Same as above.

lmd59 · 2026-05-29T21:37:46Z

+    if (this.isBoundlessInterval) {
+      return other.isBoundlessInterval ? true : other.isUnknownInterval ? null : false;
+    } else if (other?.isBoundlessInterval) {
+      return this.isUnknownInterval ? null : false;


Should return true if the starting point of the first is equal to the starting point of the second interval and the ending point of the first interval is less than or equal to the ending point of the second interval. If other is boundless, then I believe we should return true if this interval starts at minimumValue (e.g. Interval[null, 0] or Interval[minValue, 0] ).

lmd59 · 2026-05-29T21:45:13Z

    return true;
  }

+  get isBoundlessInterval() {


This checks for null endpoints, but it doesn't check for the minValue and maxValue being specified explicitly (e.g. integer example [-2147483648, 2147483647]). The way this function is used sometimes relies on eliminating the possibility that the interval is boundless, so we might run into some logic trouble without allowing for a boundless interval to be specified explicitly.

lmd59 · 2026-05-29T21:47:57Z

+  }
+
+  get isUnknownInterval() {
+    return this.low == null && !this.lowClosed && this.high == null && !this.highClosed;


Similar to above, I think maybe one could explicitly create an interval with equivalent uncertainty that should be checked for by this definition? Something like Interval[uncertainty[-2147483648, 2147483647], [-2147483648, 2147483647]]?

lmd59 · 2026-05-29T21:51:25Z

+    if (this.isBoundlessInterval) {
+      return other.isBoundlessInterval ? true : other.isUnknownInterval ? null : false;
+    } else if (other?.isBoundlessInterval) {
+      return this.isUnknownInterval ? null : false;


Similar to above, if other is boundless, the I believe we should return true is this interval ends at the maximum value (e.g. Interval[0,null] or Interval[0, maxValue] ).

cmoesel added 2 commits May 29, 2026 12:28

Improve handling of boundless and unknown intervals

8b51d26

Add special case logic for boundless intervals (e.g., Interval[null, null]) and unknown intervals (e.g., Interval(null, null)

npm audit fix

fd13dd2

lmd59 self-requested a review May 29, 2026 16:56

lmd59 reviewed May 29, 2026

View reviewed changes

cmoesel marked this pull request as draft May 29, 2026 21:49

lmd59 reviewed May 29, 2026

View reviewed changes

		@@ -1481,7 +1491,7 @@ describe('Ends', () => {
		});

		it('should calculate to null', async function () {

	it('should calculate to null', async function () {
	it('should calculate to false for bounded interval ends boundless interval', async function () {

Conversation

cmoesel commented May 29, 2026

Uh oh!

codecov-commenter commented May 29, 2026

Codecov Report

Uh oh!

lmd59 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lmd59 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants