Cache function ref

[rdingwell]

This is primarily a performance enhancement. When function use is defined within a Query, every iteration of elements in the where clause caused a re-evaluation of finding the correct FunctionDef to use. When there are a large number of items to go through this becomes a very expensive process, mainly due to the type comparisons that are used to perform the FunctionDef lookups.

This PR implements caching of the FunctionDef look ups with a FunctionRef the first time it is executed.

Note that this PR also contains updates to 3 node modules that were not passing audit tests
Pull requests into cql-execution require the following.
Submitter and reviewer should ✔ when done.
For items that are not-applicable, mark "N/A" and ✔.

This is primarily a performance enhancement. When function use is defined within a Query, every iteration of elements in the where clause caused a re-evaluation of finding the correct FunctionDef to use. When there are a large number of items to go through this becomes a very expensive process, mainly due to the type comparisons that are used to perform the FunctionDef lookups.

This PR implements caching of the FunctionDef look ups with a FunctionRef the first time it is executed.

Note that this PR also contains updates to 3 node modules that were not passing audit tests
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]

Codecov Report

Attention: Patch coverage is 77.77778% with 4 lines in your changes missing coverage. Please review.

Project coverage is 87.06%. Comparing base (12a8ce0) to head (bc0b6e4).

Files with missing lines	Patch %	Lines
src/elm/reusable.ts	77.77%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #326      +/-   ##
==========================================
+ Coverage   87.04%   87.06%   +0.02%     
==========================================
  Files          52       52              
  Lines        4517     4524       +7     
  Branches     1273     1275       +2     
==========================================
+ Hits         3932     3939       +7     
  Misses        377      377              
  Partials      208      208              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cmoesel]

My only concern with this approach was caching the resolved function for reuse across iterations, but I can't think of a use case where different iterations of the same FunctionRef would result in different function resolutions. I tried to force it with CQL like this:

define function ValueAsString(value FHIR.CodeableConcept):
  Coalesce(value.text.value, value.coding[0].display.value, value.coding[0].code.value, 'unknown')

define function ValueAsString(value FHIR.Quantity):
  ToString(value.value.value) + ' ' + Coalesce(value.unit.value, value.code.value)

define PatienObservationsAsStrings:
  [Observation] O return ValueAsString(O.value as Choice<FHIR.CodeableConcept, FHIR.Quantity>)

but the CQL-to-ELM compiler doesn't allow it because the function invocation is ambiguous. You have to do something like this, which results in two independency FunctionRefs:

define PatienObservationsAsStrings:
  [Observation] O return
    case
      when O.value is FHIR.CodeableConcept then ValueAsString(O.value as FHIR.CodeableConcept)
      when O.value is FHIR.Quantity then ValueAsString(O.value as FHIR.Quantity)
      else 'unsupported'
    end

All that to say, I think this is safe and it appears to work. Still, I'd like for the FLAME team to have a chance to review and possibly regress it against their measures. @hossenlopp - do you agree that your team should review and regress this?

[cmoesel]

I'll also note that the npm audit is unresolvable because the vulnerable module has not yet released a patch.

[cmoesel]

@rdingwell - I got to run a FQM regression on this using the updated FQM regression script in fqm-regression#321. The changes in this branch did fail regression. They don't affect the "statement results" but do affect the "clause results" -- and it seems to all be related to calls to FHIRHelpers.ToInterval. I'm not familiar enough w/ FQM or the regression script to make much sense of it or determine if it really is a problem or not, but... I thought I'd let you know.

If you're interested, here is an example:

• Measure CQL: EXM125-7.3.000.cql
• Measure Library: library-EXM125-7.3.000.json
• Test Patients: EXM125-7.3.000 tests
• Results w/ master: results-tests-denom-EXM125-bundle.json, results-tests-numer-EXM125-bundle.json
• Results w/ cache_function_ref: results-tests-denom-EXM125-bundle.json, results-tests-numer-EXM125-bundle.json
• Diff of results: EXM125-results-diff.html.zip (zipped because GitHub doesn't support attaching .html files)

I know you said you were going to rework this PR -- so let me know if/when you want me to run regression again.