-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
appliesResult to StratifierResult in detailed results output #308
Conversation
Coverage report
Show files with reduced coverage 🔻
Test suite run success456 tests passing in 31 suites. Report generated by 🧪jest coverage report action from 8fd71a3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The place where the appliedResult is created needs to be moved to have accurate results.
if (appliesToExtension) { | ||
const popCode = appliesToExtension.valueCodeableConcept?.coding?.[0].code; | ||
if (popCode) { | ||
popValue = patientResults[popCode]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't the right place to get the results for population as the patientResults
is the raw data from the engine. The define statement that provides the results might not match the code. Should look back at the populationResults
that have already been collected and find by populationType
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally the populationResults will not be accurate at this point and should happen after the call to handlePoulationValues
on line 35.
const result = isStatementValueTruthy(value); | ||
const appliesResult = isStatementValueTruthy(value && popValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call to isStatementValueTruthy
shouldn't be needed as the &&
should be done on the result
and the boolean result found in the populationResults
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A handful of things in the new function.
It should also be run on the individual episode results after handlePopulationValues
at https://github.com/projecttacoma/fqm-execution/blob/master/src/calculation/DetailedResultsBuilder.ts#L439.
The code in the measure report builder that does appliesTo logic could be removed and the appliedResult
could be used instead of the result
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the Stratification addition to the README. This looks ready to go!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made one optional test description clarification
Co-authored-by: lmd59 <[email protected]>
Summary
This PR adds an optional attribute to the
StratifierResult
interface so that we can report the result of a stratifier that includes anappliesTo
extension for Patient-based Measures only.New behavior
The
appliesTo
extension indicates which population a stratifier should apply to. If it is included on a Measure.group.stratifier, we now report the result of the stratifier AND the result of the population it applies to on theappliesResult
attribute on the StratifierResult. The previousresult
attribute remains as it was: simply the result of the stratifier. The DetailedResults now contain the raw stratifier result in result (like before) as well as the result of the stratifier with the population result it applies to taken into consideration inappliesResult
.Code changes
src/calculation/DetailedResultsBuilder.ts
- add AND logic of population result and stratifier result to appliesResult attribute of StratifierResults for patient based measuressrc/types/Calculator.ts
- add optional appliesResult attribute toStratifierResult
interfacetest/unit/DetailedResultsBuilder.test.ts
- unit tests for described changesTesting guidance
npm run cli -- detailed -m <path to mb> --p <path to patient bundle (I think the file they provide is in an array so just make sure to get rid of that)> -s 2025-01-01 -e 2025-12-31 --trust-meta-profile true -o --debug