Update source parsing for query filter parsing

[sarahmcdougall]

Summary

For measures authored using QI-Core, the sources array is unpopulated due to unhandled scenarios related to parsing query sources. Updates parseSources to drill into expressions on a given query that are not immediate retrieves.

New behavior

The sources arrays on the query info should now be populated appropriately. The approach to source parsing is as follows:

• When parsing sources, handle all sources whose expressions are retrieves (which is the current behavior)
• If the source expression is not a retrieve, check that it contains a resultTypeSpecifier. For the QI-Core-authored measures, the source is of the following format (for an example of an Encounter datatype and “Union” type):

“source”: [
  {
    “localId”: 2,
    “alias”: “MyAlias”,
    “resultTypeSpecifier”: {
      “type”: “ListTypeSpecifier”,
      “elementType”: {
        “name”: “{http//hl7.org/fhir}Encounter”,
        “type”: “NamedTypeSpecifier”
      }
    },
    “expression”: {
      “localId”: “1”,
      “type”: “Union”,
      “resultTypeSpecifier”: {
        “type”: “ListTypeSpecifier”,
        “elementType”: {
          “name”: “{http://hl7.org/fhir}Encounter”,
          “type”: “NamedTypeSpecifier”
        }
      }
    }
  }
]

Since there is no datatype on the expression, the “elementType” is parsed to retrieve the associated datatype.

• The relationship clauses are now also parsed for sources. The relationship clauses allow related sources to be used to restrict the elements included from another source in a query scope.

A potential side effect of the current query filter parsing implementation is that incorrect attribute paths are assigned to resource types during data requirements calculation. Note that this issue is not directly addressed in this pull request, as there are other issues involved, such as the “query stacking” approach that is implemented and how it handles the “Union” type that is seen in the ELM of QI-Core measures.

More information about the ELM structure is available at https://cql.hl7.org/04-logicalspecification.html.

Code changes

QueryFilterParser

• Update parseSources function to now extract sources from the resultTypeSpecifier if present (in the case that the type is not a Retrieve)
• Parse the relationship clauses in addition to parsing the sources
• Add function parseElementType to parse the elementType on the expression (this is an alternative to parsing the datatype) ELMTypes
• Adds resultTypeSpecifier as an option field on an ELM expression

Testing guidance

It is important to test against both QI-Core and non-QI-Core measures. Some potential steps that can be taken for testing:

• npm run test, npm run test:integration, etc. to ensure that no side effects were introduced. Pay specific attention to the query filter parsing unit tests
• Run data requirements against various measures with the debug option enabled.
  • Within debug/gaps.json, pay specific attention to the queryInfo output on the retrieves. Previously the sources array would be empty for some of the QI-Core measures (CMS161 is an example). Check that this is no longer the case, and that the sources array is appropriately populated by looking back at the ELM output for the measure being tested.
  • Note that the data requirements output may not be 100% accurate (i.e. the paths on the filters may be inaccurate due to the current query stacking approach)
  • Check that the gaps.json output has not changed for non-QI-Core measures, except in the case that the relationships array sources are not encompassed in the sources. The data requirements output is not expected to change. This can be done by generating the output on this branch and on the main branch, and using the ‘Select for Compare’ feature in VSCode.
  • Specific measures that can be tested: CMS161, CMS1028, CMS142, CMS177

Open questions to check for when testing

• Do we want to parse the relationships expressions alongside the source expressions? According to the logical specification, the relationship clauses “allow related sources to restrict the elements included from another source in a query scope…The elements of the related source are not included in the query scope.”
• Is it “valid” to pull information from the resultTypeSpecifier? The cql-execution library contains types for TypeSpecifiers, but little (if any) handling is done in fqm-execution for parsing the TypeSpecifiers.
• Right now, if an inner query exists, the inner query is parsed, and the original query’s sources are set to the inner query’s sources. Do we want to keep this logic intact? Using CMS161 as an example, there are instances where the inner query info sources contain a Procedure:

However, the original query info does not contain the Procedure and instead contains an Encounter:

When we set queryInfo.sources = innerQueryInfo.sources, we lose this Procedure source.

[github-actions[bot]]

Coverage report

St.:grey_question:	Category	Percentage	Covered / Total
🟢	Statements	86.33% (-0.25% 🔻)	2362/2736
🟡	Branches	73.64% (-0.19% 🔻)	2190/2974
🟢	Functions	88.89% (-0.16% 🔻)	424/477
🟢	Lines	86.67% (-0.27% 🔻)	2282/2633

Show files with reduced coverage 🔻

|

St.^{:grey_question:}

| File | Statements | Branches | Functions | Lines | | :----------------------------------------------------------------------------------------------------------------------------------------------------: | :------------------------------------------------------------------------ | :------------------------------------------------------------- | :------------------------------------------------------------- | :----------------------------------------------------------- | :------------------------------------------------------------- | | 🟢 |

`...` / GapsReportBuilder.ts

|

93.66% (-0.31% 🔻)

| 83.91% | 100% |

93.63% (-0.34% 🔻)

| | 🟢 |

`...` / QueryFilterParser.ts

|

85.31% (-1.56% 🔻)

|

78.57% (-1.86% 🔻)

|

97.06% (-2.94% 🔻)

|

85.07% (-1.58% 🔻)

|

Test suite run success

447 tests passing in 31 suites.

Report generated by 🧪jest coverage report action from bead5ad9aa02267d0fceaa6cbaaa5c0bbc276bc4

[sarahmcdougall]

Summary of changes made in newest commits:

1. Before adding filters to data requirement, GapsReportBuilder now checks that a queryInfo source exists whose alias matches the alias of the data type query. If no source exists, filters are not added to the data requirement.
2. Before adding filters to data requirement, GapsReportBuilder now checks that the alias of the queryInfo source matches the alias of the filter. If the aliases do not match, filters are not added to the data requirement. All the filter types in the function extend from AttributeFilter, which has alias as a required field. I did notice a few filters during testing that do not have alias populated but it seems that this is the case when the type is ‘unknown’ and ‘withError’ is populated.
3. In the QueryFilterParser, we now only copy over the first index of the inner query info source rather than the entire array of sources. While it usually is the case that there is only one source, it is possible for there to be multiple sources (like when we are working with relationships), and so we want to only replace the first index and copy over the rest of the sources so that we do not lose any information.
4. parseSources was updated to consolidate the logic between sources and relationships since we are essentially treating them the same. The function now parses sources and relationships in the same loop.
5. Type handling added for parseElementType.

Changes to the output: We no longer receive the “Procedure.period” error (or similar errors depending on the measure under test). This is due to the additional query alias handling that was added to GapsReportBuilder. The correct filter does not get added to the data requirements because we never actually parse the relationship during the query filter parsing. Is this still in scope of this task? It looks like the relationships do get parsed in RetrievesFinder but the suchThat expressions do not get parsed in the query parser. I believe that significant re-working of the parseQueryInfo function will be required to dig down into the relationship clauses.