projecttacoma / fqm-execution

fqm-execution is a library that allows users to calculate FHIR-based electronic Clinical Quality Measures (eCQMs) and retrieve the results in a variety of formats
https://projecttacoma.github.io/fqm-execution/
Apache License 2.0
18 stars 6 forks source link

Add workaround for nested scopes within Property and FunctionRef #255

Closed mgramigna closed 1 year ago

mgramigna commented 1 year ago

Summary

Fixes #254

Specifically, fixes the following cases:

  1. Aliases sometimes don't highlight on subsequent uses within CQL clauses
  2. Calls to fluent functions (or regular functions) sometimes don't highlight the alias when it is part of an argument

These two cases are from the same root issue, where the ELM annotation is using a localId that never appears in the actual expression, but is 1 less than the localId that actually does appear in the expression. We already have some handling for this in ClauseResultsHelper, but this branch of logic only handles if the .scope attribute is present on the statement and is itself a reference to the original alias.

In QI-Core authored measures, the way that alias references show up can be different:

// ELM generated from FHIR-based CQL
{
  "localId": "9",
  "path": "status",
  "scope": "MyEncounter",
  "type": "Property"
}
// ELM generated from QICore-based CQL
{
  "localId": "9",
  "path": "value",
  "type": "Property",
  "source": {
    "path": "status",
    "scope": "MyEncounter",
    "type": "Property"
  }
}

Note the presence of .source which actually contains the scope. Importantly, for both of these examples, the localId used in the annotation for this alias is actually 8:

{
  "r": "9",
  "s": [
    {
      "r": "8", // localId 8 does not appear in the actual ELM.
      "s": [
        {
          "value": [
            "MyEncounter"
          ]
        }
      ]
    },
    {
      "value": [
        "."
      ]
    },
    {
      "r": "9",
      "s": [
        {
          "value": [
            "status"
          ]
        }
      ]
    }
  ]
}

This PR adds the same "subtract by one logic" to the recursive function that grabs all the localIds and ensures that ELM of the above format maps the use of that off-by-one localId on to the original alias in the CQL when defined.

New behavior

Fixes highlighting of aliases in the above cases:

master

qicore-simple-alias-broken

This PR

qicore-simple-alias-fixed

master

functionref-alias-broken

This PR

functionref-alias-fixed

Code changes

Testing guidance

Attached is the measure bundles used to generate the above screenshots, with the patients. Instructions to run it and replicate are in DESCRIPTION.md. In all cases, look at debug/html/clause-coverage-*.html to view the highlighting.

If you can think of any other alias highlighting cases that are broken on master that you believe aren't covered by this PR, we can work to create a test case for it. These are the only two I was able to find.

coverage-aliases.tar.gz

github-actions[bot] commented 1 year ago

Coverage report

St.:grey_question:
Category Percentage Covered / Total
🟒 Statements
86.32% (+0.07% πŸ”Ό)
2271/2631
🟑 Branches
75.5% (+0.11% πŸ”Ό)
2099/2780
🟒 Functions
89.43% (+0.02% πŸ”Ό)
423/473
🟒 Lines
86.64% (+0.07% πŸ”Ό)
2191/2529

Test suite run success

426 tests passing in 31 suites.

Report generated by πŸ§ͺjest coverage report action from f974cc46863295f06eafc8394a394c0d2787b878