Implement Interval PointFrom operator by dehall · Pull Request #371 · cqframework/cql-execution

dehall · 2026-07-16T21:50:09Z

This PR implements the point from interval operator. (closes #163)

define PointFromUnit2Interval: point from Interval[2,2]

If the given interval is a "unit interval", aka it only contains a single point, this operator returns that point. Otherwise, it throws a runtime error.

To get that single point, it just checks if start() is equal to end(). If both bounds of the interval are null, it returns null, hence checking with both == and equals.

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

codecov-commenter · 2026-07-16T21:51:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.27%. Comparing base (64cfe9b) to head (e4ff973).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #371      +/-   ##
==========================================
+ Coverage   88.24%   88.27%   +0.02%     
==========================================
  Files          54       54              
  Lines        4867     4877      +10     
  Branches     1372     1374       +2     
==========================================
+ Hits         4295     4305      +10     
  Misses        377      377              
  Partials      195      195

☔ 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.

lmd59

Generally, this seems to work well, test well, and cover edge cases well. I had one question about a comparison of edge cases that I would think would have the same output but do not. Interested in thoughts on that!

lmd59 · 2026-07-17T19:11:10Z

-    skipped: 'PointFrom not implemented'
-    /*
    expression: point from Interval[null, null],
    output: null


I see that this is a spec test, but do we think it's inconsistent that this output is null, whereas output point from Interval[1, null] is error? Is this worth a clarification to make sure the spec test is correct (or to make sure the Interval[1, null] case is correct)?

So it looks like the latest build version of the spec has a new line in it that was not in the 2.0.0-ballot version (nor in the current 1.5.3):

https://build.fhir.org/ig/HL7/cql/09-b-cqlreference.html#point-from (latest)

The point from operator extracts the single point from a unit interval. If the argument is not a unit interval, a run-time error is thrown. In particular, attempting to extract a point from the maximum interval (Interval[null, null]) will result in an error.

https://cql.hl7.org/2025Sep/09-b-cqlreference.html#point-from

The point from operator extracts the single point from a unit interval. If the argument is not a unit interval, a run-time error is thrown.

I'm not sure when this was added, I can do more looking, but this would take away that inconsistency and make anything with a null be an error. But, that's only a build version so I'm not sure if we want to implement to that version yet or not. I would read that as a "clarification" not a "change" and so it feels ok to me to implement that now, but I think we should double check with @cmoesel .

Great find! Would also welcome thoughts from @cmoesel!

I agree with @lmd59's hunch that the spec test for point from Interval[null, null] should result in an error. I think the new language that @dehall found is just a clarification; it doesn't actually change anything because Interval[null, null] is definitely not a unit interval, so it should throw an error anyway. That said, the new language makes it very difficult for anyone to defend that test's current behavior (so I guess that's good).

Updated in the latest, note this specific cql-test is now skipped again. There is an open PR that updates that (# 128 over there) but it's one of many that says "add and update tests as suggested by Claude" so I'm not sure what the status of those is

cmoesel

Thanks for working on this. It's looking good, but I've added one suggestion and one question (that might require clarification from the CQL community). Depending on the answer to the question, further changes may be needed.

cmoesel · 2026-07-20T18:48:08Z

+    const start = interval.start();
+    const end = interval.end();
+
+    if (start == end || equals(start, end)) {
+      return start;
+    }
+
+    throw new Error(
+      'PointFrom operator may only be used on an interval containing a single point.'
+    );


We've implemented almost all of the other operators in the src/datatypes/interval.ts Interval class (with the exception of expand / collapse, for some reason). I'd suggest we do the same for this one and keep the ELM layer light.

cmoesel

I'm approving this because I'd accept it as-is, but I also noted one additional test for you to consider adding.

cmoesel · 2026-07-23T19:53:59Z

+    it('should throw for an interval with null high bound', function () {
+      const ivl = new Interval(DateTime.parse('2012-01-01T00:00:00.0+00'), null);
+      should(() => ivl.pointFrom()).throw(Error);
+    });


It might be good to test this for both closed null (max value) and open null (unknown value).

I'd be willing to accept the PR without this, but if you have time, then it might be good to add it just for completeness.

Added a few extra tests in 2545bd7 and then realized that wasn't actually what you wanted so added a couple more that hopefully are in 6dad96d

cmoesel · 2026-07-23T19:55:04Z

+    it('should throw for an interval with undefined width', function () {
+      const ivl = new Interval(0, 0, false, false);
+      should(() => ivl.pointFrom()).throw(Error);
+    });


I'm not sure if this is even a valid interval, but throwing seems like the right thing to do.

lmd59

Looks like this is reflecting spec intention and expected future changes to spec tests!

lmd59 self-requested a review July 17, 2026 15:28

lmd59 reviewed Jul 17, 2026

View reviewed changes

cmoesel requested changes Jul 20, 2026

View reviewed changes

dehall requested review from cmoesel and lmd59 July 23, 2026 14:47

cmoesel approved these changes Jul 23, 2026

View reviewed changes

lmd59 approved these changes Jul 24, 2026

View reviewed changes

dehall added 8 commits July 24, 2026 18:33

Implement Interval PointFrom operator

4d1d9f7

Migrate implementation of interval pointFrom from elm layer to datatype

16de73e

add extra tests

50afee8

Skip incorrect TestPointFromNull test

6df77e1

Regenerate cql4browsers

5f505e9

a couple extra tests

a2b8277

Add a few more tests

6025a3b

Add even more tests

e4ff973

dehall force-pushed the pointfrom branch from 6dad96d to e4ff973 Compare July 24, 2026 22:34

dehall merged commit e8dddb1 into master Jul 24, 2026
14 checks passed

dehall deleted the pointfrom branch July 24, 2026 22:37

Uh oh!

Conversation

dehall commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

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!

cmoesel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cmoesel 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!

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

dehall commented Jul 16, 2026 •

edited

Loading

codecov-commenter commented Jul 16, 2026 •

edited

Loading