[dehall]

Addresses #390 , where previously Query sorting was done primarily with the plain JS === and < operators, now sorting is done with our usual comparison functions equals, lessThan, greaterThan, and only falling back to < as a last resort.

Two specific problems that this addresses were with DateTimes with different timezones, and Decimals, where sorting was incorrect. (The Decimal sorting problem was really a regression with the big Decimal PR) See examples in the test cases.

Note that some of the spec-tests revealed a gap in the spec, where DateTimes of different precision compare as null, but the tests show that the DateTime of lower precision is consistently sorted as "lower". See cqframework/cql-tests#168
This is technically handled by the < fallback, but I've added logic that makes this more explicit. Since this is an open question, I'm ok with holding off on this PR until that's resolved.

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 check to run tests, lint, and prettier)
• All dependent libraries are appropriately updated or have a corresponding PR related to this change
  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]

Codecov Report

❌ Patch coverage is 70.58824% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.23%. Comparing base (e8096c6) to head (7f58f87).

Files with missing lines	Patch %	Lines
src/elm/query.ts	70.58%	3 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #392      +/-   ##
==========================================
+ Coverage   89.19%   89.23%   +0.04%     
==========================================
  Files          60       60              
  Lines        5064     5065       +1     
  Branches     1446     1444       -2     
==========================================
+ Hits         4517     4520       +3     
  Misses        315      315              
+ Partials      232      230       -2     

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

[cmoesel]

Regarding sorting less precise Dates and DateTimes, I am comfortable sorting less precise values first. I made an argument for that case on the cql-tests bug cited above here.

[cmoesel]

Thanks for fixing this. As noted in my earlier comment, I think it's OK to merge this with the behavior of sorting less precise Dates and DateTimes lower.

That said, I did find one issue that I think should be addressed.

[cmoesel]

Codex found a case where this sort is not always symmetrical:

[P2] Make mixed-precision DateTime ordering independent of operand order

For a = @2025-01-02T05-03:30 and b = @2025-01-02T08:15:00.000Z, the new comparator returns 1 for both (a, b) and (b, a). The comparison helpers normalize to the first operand's timezone, so one direction establishes an order while the other reaches the precision fallback. Consequently, ascending sorting produces opposite results when the input is reversed, including placing a before b even though its entire hour starts after b. Ensure timezone normalization and the precision fallback produce an antisymmetric comparator, and add a fractional-offset regression test.

I asked it for a failing unit test demonstrating this problem and it produced:

  it('should sort mixed-precision DateTimes with fractional timezone offsets independently of input order', async function () {
    this.ctx.getExecutionDateTime().timezoneOffset = Decimal.from(0);
    const earlier = DateTime.parse('2025-01-02T08:15:00.000Z');
    // 05-03:30 spans the hour starting at 08:30Z, entirely after 08:15Z.
    const later = DateTime.parse('2025-01-02T05-03:30');
    const sort = new SortClause({ by: [{ type: 'ByDirection', direction: 'asc' }] });
    const results = [
      await sort.sort(this.ctx, [earlier, later]),
      await sort.sort(this.ctx, [later, earlier])
    ];
    for (const result of results) {
      result.should.eql([earlier, later]);
    }
  });

NOTE: It was lazy and did not do the full CQL data fixture, so you have to import Sort for the above to work -- but hopefully this helps to at least demonstrate the problem.

[dehall]

This is a good catch, and it's relevant to more than just this sort. Our DateTime.before/after/... functions are asymmetric in this situation.

The issue is that 2025-01-02T05-03:30 has a timezone offset with greater precision (minutes) than the DateTime itself (minutes unspecified). As of now when we convert this to a timezone with 0-minute offset like UTC, it becomes 2025-01-02T08+00:00 which is not equivalent, and I don't think it would be correct to change it to 2025-01-02T08:30+00:00 since that's a finer precision than the original DateTime.
The way to get around this would be to treat the not-fully-specified DateTime as an uncertainty instead, as the spec does suggest here https://cql.hl7.org/02-authorsguide.html#comparing-dates-and-times .

In general, CQL treats Date and Time values that are only known to some specific precision as an uncertainty over the range at the first unspecified precision.

So then let's call the original value uncertainty[2025-01-02T05:00:00.000-03:30, 2025-01-02T05:59:59.999-03:30] which we can convert to another timezone just fine: uncertainty[2025-01-02T08:30:00.000+00:00, 2025-01-02T09:29:59.999+00:00] and then the usual uncertainty logic and DateTime comparisons work as expected.
We might even want to call this a gap in the spec because if we treat DateTimes this way for comparison, there's no longer any such thing as a DateTime with unspecified fields, as suggested in the descriptions for those comparison functions, eg https://cql.hl7.org/09-b-cqlreference.html#after

Unfortunately I think this requires another refactoring of the DateTime comparison logic (after we just refactored recently in #380). Let me know if you'd like me to do that here or break that out into a separate task.