fixes: CDR engine fixes for duplicate use of actions across rules in addition to an optimization

[bradymiller]

fixes #7266
Fixes #7153
Fixes #7157
Fixes #7161

CDR engine fixes for duplicate use of actions across rules in addition to an optimization

The optimization both clarifies the code regarding which $reminder_due status is chosen and also minimizes the calls to reminder_results_integrate, which could get resource intensive if overuse it.

Also brought in fixes from #7160

[bradymiller]

hi @hanksterr7 , Interestingly this fix/optimization amplifies that filter bug (where iterating through the filter on all the dates rather than just the target date), which you have fixed in your PR. Because of this, I think it may be best to get your PR into the codebase before this PR.

[hanksterr7]

@bradymiller : Interesting. Ok, I've reviewed the first part of what you've changed.

Previously, in lines 1037 to 1041, $reminder_due would be set to the status the rule will have if its target does NOT pass for the current value of $dateFocus

You have now changed $reminder_due to be the status the rule WILL have if its target DOES pass for the current $dateFocus. Ok. that is cleaner. Was very confusing previously.

Note that passing during iteration #1 means the rule status is "not due". If it does not pass during iteration #1 but passes during iteration #2, its status is "due soon". If it does not pass until iteration #3, its status is "due". And if it does not pass during any of iterations #1, #2 or #3, the status is "past due".

Note also that the status is "past due" if EITHER the target data was found, but its date was prior to today - target interval - past due interval, OR the target data was not found at all. There does not seem to be any effort to record a distinction between these two possibilities

In lines 1068 through 1106, you have adjusted the logic to match the new value of $reminder_due that is set for each $dateFocus value (i.e. for reach pass through the loop of $target_dates values)

I don't understand the meaning of the different possible values of $mode. If you could add some comments to the code explaining what these values are, and how and why the code should behave differently for each of them, that would help. In particular, explaining what the difference is between $mode = "reminders-all" vs "report", vs any other values $mode could have.

-- edit: I see you do describe these $mode values in your re-working of reminder_results_integrate(). Perhaps adjusting your comments in test_rules_clinic() to alert the reader to review the code/comments in reminder_results_integrate() for the meaining of $mode would help :)

Lines 1077 - 1080 determine if reminder_results_integrate() should get called if the target passes for the current value of $dateFocus.

LInes 1095 and 1096 determine if reminder_results_integrate() should get called when the target does NOT pass for the current value of $dateFocus

I'm going to describe the behavior that I believe the code is executing. Please confirm this is what you want.

The code behaves differently depending on whether or not $mode is "reminders-all". I will describe each option separately

First, when $mode is "reminders-all":

If the target passes during pass #1 ($reminder_due = "not_due"), reminder_results_integrate() gets called in line 1089 with due_status of "not_due". Further processing of $target_dates ceases, due to the "break" in 1093.

If it doesn't pass during pass #1, reminder_results_integrate() does not get called in either lines 1089 or 1103, and pass #2 begins

If the target passes during pass #2 ($reminder_due = "soon_due"), reminder_results_integrate() gets called in line 1089 with due_status of "soon_due". Further processing of $target_dates ceases due to the "break" in 1093.

If it does not pass during pass #2, again reminder_results_integrate() does not get called in either lines 1089 or 1103, and pass #3 begins

If the target passes during pass #3 ($reminder_due = "due"), reminder_results_ingrate() gets called in line 1089 with due_status of "due". Further processing of $target_dates ceases due to the "break" in 1093.

If it does not pass during pass #3, reminder_results_integrate() now gets called in line 1103 and sets due_status to "past_due"

The above behavior makes sense

Now, if $mode is not "report" AND $mode is not "reminders-all":

If target passes during pass #1: reminder_results_integrate() does NOT get called in either 1089 or 1103, and processing stops. You have the comment: "note this is skipped if not_due and not in reminders-all mode", so the behavior is consistent with this comment. It would help if your comment would explain WHY you do not want to call reminder_results_integrate() in this case. I don't understand why you want this behavior, but I also don't understand what it means for $mode to not be "report" and not be "reminders-all"

If target does not pass during pass #1 but passes during pass #2, reminder_results_integrate() is called in 1089 and with due_status of "soon_due" and processing stops

If target does not pass during passes #1 or #2 but passes during pass #3, reminder_results_integrate() is called in 1089 and with due_status of "due" and processing stops

If target does not pass during passes #1, #2 or #3, reminder_results_integrate() gets called in line 1103 and sets due_status to "past_due"

Given your comment of "note this is skipped if not_due and not in reminders-all mode", I guess the behavior for this $mode choice is also consistent with your desires, so I guess the code is good to go

But I also agree it would be good if this were merged with my code that moves the filter processing outside the looping through $target_dates values

I will now review your changes to reminder_results_integrate(). I won't have thoughts on that until tomorrow or Tuesday

Thanks
-- Hank

[bradymiller]

hi @hanksterr7 ,
Thanks for the detailed analysis. I'll clarify my comments in the code in the unclear places especially regarding mode.
There are 3 modes:

• 'report' mode is for the report at Reports->Clinic->Standard Measures. This mode only tests 1 datetarget.
• 'reminders-due' is the main mode which runs the passive-alerts/active-alerts/patient reminders (explained these three things below, which are set for each rule in Admin->Practice->Alerts). Goal of this mode is to just show the user what is due (or going to be due soon).
  • Passive Alerts are in the Clinical Reminders widget in the patient summary screen
  • Patient Reminders are in the Patient Reminders widget in the patient summary screen
  • Active Alerts will show up in a popup when open up the patient summary screen
• 'reminders-all' mode is used in the advanced part of the Clinical Reminders widget in the patient summary screen (click the edit icon at the top right of that widget). The special thing in this mode is that 'Not Due' reminders are shown.

[bradymiller]

to help with the review on this, I just removed this function since it was not being used.

[bradymiller]

hi @hanksterr7 .
I incorporated your filter fix in this PR, so this code would work correctly. Gave you authorship to the commit that has the filter fix code, which i followed with another commit that had some minor changes in your code. So far testing well. Plan to repeat extensive testing before bringing this in. Let me know if you have thoughts/issues.

[hanksterr7]

Thanks Brady. Sorry for being off the grid on this for a while. I'll be done with work projects soon and can dive back in on this-- Hank On Sunday, March 24, 2024 at 05:30:35 AM EDT, Brady Miller ***@***.***> wrote: hi @hanksterr7 . I incorporated your filter fix in this PR, so this code would work correctly. Gave you authorship to the commit that has the filter fix code, which i followed with another commit that had some minor changes in your code. So far testing well. Plan to repeat extensive testing before bringing this in. Let me know if you have thoughts/issues. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[hanksterr7]

Hi @bradymiller
Finally got to reviewing your latest commit. Looks good! (and I like the additional comments in the code)
Comments:
-- of course, still need to get the rest of my changes into the code, for better managing multiple targets and filters (including adjusting the target interval being tested to end on $dateTarget instead of $dateFocus + one of the three interval duration criteria), and better handling of optional vs required targets/filters
-- In your line 1261, you have $action = $actionArray[0];. If $actionArray is not set, this cause an error to be thrown. I recall this happening to me, but I don't recall what rule/data config would repro that. But wrapping the code block in "if ($actionArray) {" is harmless. I suggest adding that.
-- I also note that $pass_target is again getting incremented any time a target passes (for rule statuses of not_due, due_soon or due), which I think is the desired behavior. At one point, when you had wrapped the "if ($passTarget)" section in a test for ($dateCounter == 1), which was causing $passTarget to get updated only if rule status was "not_due", and not if "due_soon" or "due". $pass_target is used in calculate_percentage(). You might want to add a comment that $percentage, as calculated by calculate_percentage(), should be updated even if the rule status is "due", and not just "not_due" or "due_soon". If a rule is "due", that means the patient is no longer meeting criteria, so I'm unclear whether the percentage being reported should should consider the rule as successful when its status is "due". Certainly, both "not_due" and "due soon" suggest the patient is compliant with rule criteria.
Thanks!
-- Hank

[bradymiller]

@hanksterr7 ,
I just bundled all your work from your PR into a commit (with your authorship) and put it into this PR. All that's left now is some more testing before this all gets into the codebase (and just in time for the first 7.0.2 patch :) ). Plan to bring this in as a merge so authorship of commits are maintained (ie. you get duly deserved credit for all your work and fixes in this myriad maze of code that i created :) ).

[bradymiller]

hi @hanksterr7 ,
That percentage value is only used for report mode. Not sure why i calculate it for every mode and will look into migrating it into a block of code only run during report mode. Note report mode only tests one target date.

[hanksterr7]

Hi @bradymiller
I have compared the code in in this PR with what was in #7160 and all looks good! Nice to see all the #7160 changes incorporated into #7268
I don't have any additional comments
Looking forward to seeing the results of your additional testing, and hopefully merging this in soon
Thanks for all the review you have done of my suggested changes
-- Hank

[stephenwaite]

testing well with the demo data!

great job!

[hanksterr7]

@stephenwaite, if you can, try creating some complicated rules that have multiple filters and targets, where the different filters and targets are of different category types (e.g. age, vs vitals, vs diagnoses, vs social hx, vs appointments, vs meds, etc), and where you have a mix of inclusion and exclusion filters/targets, and where the dates of data the targets are looking at varies relative to the due soon/due/past due intervals (such as having one target, such a diagnosis, entered a week ago, and a blood pressure entered one year less a week ago, for a target interval of 1 year, and a warning interval of one month, and a past due interval of 2 months. That scenario is what my changes tried to make work better. Compare behavior of that scenario in current release vs what this PR is doing. Thanks

[hanksterr7]

Hi @bradymiller and @stephenwaite

To help you do some testing of of the changes in my code, I offer what is below

These will show how behavior changes when using the code in this PR compared to the code in the main branch. Note that the main branch has already been patched to cause the required/optional labels in the Rules UI to now be correct

This could be helpful to include in release notes

Preparation:
-- create Rule #1 with configuration:
-- -- passive
-- -- warning 1 month, past due 2 months
-- -- demographic filters: none
-- -- target: custom: assessment: alcohol, completed: yes, frequency: >= 1, optional: yes, inclusion: yes, interval: 1 day
-- -- action: custom: assessment: alcohol, custom input: yes
The above rule allows Assessment: Alcohol to show in the reminders widget in patient dashboard, giving easy access to modifying the date and completion choice for instances of this this custom item

-- create Rule #2, with configuration:
-- -- passive
-- -- warning 1 month, past due 2 months
This is the rule that will be modified for the tests below. The rest of the configuration depends on the test being done

-- create a patient with age: 25, gender: female, Medical Problem: ICD10 diagnosis: Chronic Obstructive Pulmonary Disease, J44.9 (or any problem of your choosing), begin date; blank, end date: blank (modify date will be set to the date you created the problem)

issues being tested/fixed:
-- Issue #1: age/gender filters treated as required, even though UI lets these be declared as optional
Test by modifying Rule #2 to have
-- demographic filters:
-- -- age min 30 years, optional: yes, inclusion: yes
-- -- age max 40 years, optional: yes, inclusion: yes
-- target: custom: education: blood pressure, completed: yes, frequency: >= 1, optional: yes or no (doesn't matter), inclusion: yes, interval: 6 months
-- action: custom: education: blood pressure, custom input: yes
Using main branch code for library\clinical_rules.php:
Refresh dashboard. Education: blood pressure will not be in the list, because the age min filter does not succeed, and the age max filter does not get tested, even though both are optional: yes
Change the filters to age min: 20 years, age max: 30 years. Education: blood pressure now shows (with status: past due, since no instance of Education: Blood Pressure has been created), since both filters succeed.
Change the filters to age min: 20 years, age max: 22 years. Education: blood pressure does not show. So both filters are being treated as required, even though they were declared as optional
Using PR code for library\clinical_rules.php, observe results matching expectations for optional filters

-- Issue #2: optional filter failure in one category doesn't allow checking optional or required filter in next category
Order in which filter categories are checked:
-- Inclusions
-- -- age min
-- -- age max
-- -- gender
-- -- db (custom, lifestyle, custom table)
-- -- lists (diagnoses, meds, surgeries, allergies)
-- -- procedures
-- Exclusions:
-- -- lists
Test by modifying Rule #2 to have
-- demographic filters:
-- -- custom: Assessment: Alcohol, completed: yes, frequency: >= 1, optional: yes, inclusion: yes
-- -- diagnosis: ICD10 diagnosis: Chronic Obstructive Pulmonary Disease, J44.9, optional: yes, inclusion: yes
-- target: custom: education: blood pressure, completed: yes, frequency: >= 1, optional: yes or no (doesn't matter), inclusion: yes, interval: 6 months
-- action: custom: education: blood pressure, custom input: yes
Using main branch code:
Refresh dashboard. Observe that Education: Blood Pressure does not show in reminder widget
This is because the optional Assessment: Alcohol filter does not succeed (since no instance of this custom item has been created yet), and, inappropriately, the diagnosis filter does not get checked (which would have succeeded), so the rule is considered not applicable to the patient.
Create an instance of Assessment: Alcohol, completed: yes, date: T-3M. Refresh dashboard. Education: Blood Pressure will show, with status: "past due" (since no instance of Education: Blood Pressure has been created)
If modify the Assessment: Alcohol instance to have date: T-1W, the status will inappropriately change to "due", for reasons explained in the next issue.
If modify the Assessment: Alcohol instance to have completed: no, Education: Blood Pressure will again not show since the Assessment: Alcohol filter never succeeds and the diagnosis filter inappropriately again does not get checked.
Using PR code, Education: Blood Pressure will appropriately show as "past due", regardless of existence, date, or completed status of the Assessment: Alcohol instance

-- Issue #3: rule gets status of "due" instead of "past due" if filter data has date between T-PastDueInterval and T.
$dateFocus is used when checking filters (db, lists, procedure). sql includes <= $dateFocus (or <= $dateFocus and >= $dateFocus). If data for filter has start date between T-PastDueInterval and T, and data for target is in past due range, rule gets status of due instead of past due since processing for 3rd pass stops after filter failure, with status remaining "due" inappropriately
This is since rule status is set at end of a $dateFocus pass to state rule has if filter passes and target does not pass.
pass 1: -> due soon
pass 2: -> due
pass 3: -> past due
If filter and target succeed during a $dateFocus pass, the previously set state persists unchanged.
Test by modifying Rule #2 to have:
-- demographic filter:
-- -- Custom: Assessment: Alcohol, completed: yes, frequency: >= 1, optional: yes or no (doesn't matter), inclusion: yes
-- target: custom: education: blood pressure, completed: yes, frequency: >= 1, optional: yes or no (doesn't matter), inclusion: yes, interval: 6 months
-- action: custom: education: blood pressure, custom input: yes
Using Main Branch code
Refresh dashboard.
Find the Assessment: Alcohol item and set it to have: completed: yes, date: T-1W
Refresh dashboard
Observe Education: Blood Pressure shows, but inappropriately with status "due" instead of "past due". Should be "past due" since no instance of Education: Blood Pressure exists.
Click "Education: Blood Pressure" in reminder widget and create an instance of this custom item, with completed: yes, date: T-9M (so is in "past due" range). Dashboard continues to show status as "due" instead of "past due". Change date to T-7M. Status remains "due". Change date to one week sooner than T-6M. Status appropriately changes to "due soon". Change date to T-1W. Education: Blood Pressure disappears from reminder widget but shows in reminder details with status: "not due" (or maybe blank, depending if fix for reminder details status values has been applied to main branch).
The above happens because, on the third pass of checking filters/targets, $dateFocus is T-2M, so the Assessment: Alcohol instance is not found (its date is sooner than T-2M), the filter is deemed not applicable for pass #3, and the status of "Due", set in pass #2, persists.
Change criteria for Assessment: Alcohol filter to be completed: no
Refresh dashboard. Education: Blood Pressure now does not show. This happens because Assessment: Alcohol filter never succeeds
Change criteria for Assessment: Alcohol filter to be completed: yes, date: T-3M.
Refresh the dashboard. Education: Blood Pressure shows with appropriate status as the date of the Education: Blood Pressure instance is changed between T-9M, T-7M, one week sooner than T-6M, and T-3M
Using PR code, Education: Blood Pressure status matches the date of the Education: Blood Pressure instance, when Assessment: Alcohol date is T-1W

-- Issue #4: if have two required targets, one with data date between T-PastDueInterval and T, and the other in "due" range, rule shows as past due instead of due, since on no $dateFocus pass do both targets succeed.
Happens because during the three $dateFocus passes, the intervals checked are:
T-targetInterval+warningInterval -> T+warningInterval
T-targetInterval -> T
T-targetInterval-pastDueInterval -> T-pastDueInterval
This is fixed in the PR by using these intervals instead:
T-targetInterval+warningInterval -> T
T-targetInterval -> T
T-targetInterval-pastDueInterval -> T
Test by modifying Rule #2 to have a filter that always succeeds (e.g. age or gender), and two targets in a target/action group, one for Assessment: Alcohol, and the other for Education: Blood Pressure. Both: frequency: >= 1, optional: no, inclusion: yes, interval: 6 months.
action: custom: education: blood pressure, custom input: yes
Set the dates for instances of the two targets as described above and observe status
Using PR code, rule status will correspond to the date of the older target instance

-- Issue #5: if have more than one target in a target/action group, and if optional ones in a category do not succeed, subsequent categories are not checked. Reasons are same as for Issue #2.
Also, only targets with inclusion: yes are checked. Any with inclusion: no are ignored.
Order in which target categories are checked:
-- Inclusion:
-- -- db
-- -- procedure
-- -- appointment
Test using the guidelines described in the various issues above, but would need to have a target in two of the three categories listed to perform an appropriate test
PR code handles tests of optional targets across target categories properly, but does not add support for exclusion targets.

[bradymiller]

fyi, getting close on this. Just a bit more testing (guessing will get into codebase over the weekend)

[stephenwaite]

setup an up for grabs demo

[hanksterr7]

@stephenwaite : what is the "up for grabs demo"? Is my code included in it? Is there anything I need to do to help make this happen?
Thanks
-- Hank

[hanksterr7]

@stephenwaite and @bradymiller
I compared the behavior of rules in the alpha demo vs the standard (release) demo
I ran the tests listed below, following the instructions I listed in my prior message, and can confirm that all rule functions work properly in the alpha version, and do not work properly in the release version
-- age + gender optional demographic filters
-- asmt: appointment + medication: lisinopril, both optional demographic filters
-- asmt: appointment demog (instance 3 days ago), education bp target. rule never shows as past due in release
-- demog filter: age min > 2 years. targets: asmt: appointment (instance 3 days ago) and educ: bp (instance 7 months ago). rule shows as past due in release. shows as due in alpha

The alpha system behaves as I would expect, based on my fixes
Thanks

[bradymiller]

hi @hanksterr7 ,
Thanks for the testing. My testing also working well. So bringing this in. Will bring it into rel-702 (so will be in next patch) this evening. Thanks for the awesome work on this!

[stephenwaite]

way to go @hanksterr7 @bradymiller !!!

[hanksterr7]

Great to see all my changes now in the master branch! Thanks everyone for all the effort and support