bug: Clinical Rules - logic issues in filter and target evaluations; divide by zero bug

[hanksterr7]

Entering this issue as part of pull request

I will create individual issues for these items in order to facilitate discussion, with more details in each issue. This is a summary issue for purposes of the PR

I am providing updated code to these files, in order to address several bugs and logic issues in
library\clinical_rules.php

Topics addressed:
-- divide by zero error in calculate_percentage()
-- Reminder Details page showing status of "not due" or "past due" but not "due soon" or "due".
-- Logic deficiencies in evaluating filters and targets:
-- -- logic does not properly handle results of criteria evaluation across categories of data, such as filters on both diagnoses and gender
-- -- logic inappropriately modified right-side boundary for target intervals when determining rule status, resulting in inappropriate rule conclusions
-- -- logic inappropriately applied date intervals during filter evaluation, and unnecessarily performs filter evaluation multiple times

[bradymiller]

hi @hanksterr7 ,

Thanks for the detailed work. Are these logic failures happening in the rules that are built into OpenEMR (ie. the rules that are preinstalled) or the rules that are created with the editor or both?

I ask this because the CDR engine was originally built by myself without a editor gui and the elements needed to be manually added to the database at that time (here is an example of them at https://www.open-emr.org/wiki/index.php/CDR_Rules_tables, although is outdated and most current sets are in the database.sql file in OpenEMR). Can check this page to get an idea of what was happening when the editor gui was then built by a different developer: https://www.open-emr.org/wiki/index.php/Clinical_Decision_Rules

[bradymiller]

Been awhile since I have looked at this code (plan to re-educate myself while reviewing your PR :) ), but pretty sure the test_filter() date is needed when running the rules on historical data (for example, setting a target date when running report in OpenEMR at Report->Clinic->Standard Measures)

[hanksterr7]

Hi Brady. I have not tested the pre-installed rules. Just those created by the editor gui. In general though, I don't know of a valid use case for testing demographic filters against a date range. I would think demographic filters should be tested against the "patient", not against the patient's status during some time frame. It certainly makes sense to test rule targets against the three "intervals" related to the "due soon", "due" and "past due" time frames. If you can provide an example of a demographic filter that should be tested against a date range, please share your thoughts on that. test_filter() is used only when processing demographic filters, so I don't know of a use case that would require it to evaluate against a date range. Thanks

[hanksterr7]

Note, I have 7 topics that my code addresses (maybe 8 if I work on one more, but it is not yet in my updated code). Please advise as to the appropriate way for me to document this in github. Should I create 7 new issues (one for each topic), but link each (somehow) to the single PR that contains the code that fixes all of them? This issue #7153 is going to get very busy if all 7 are discussed in a single thread. Should I create separate PR's for each topic, with each PR containing the code that fixes each topic? Something else? I'm new to submitting PRs. Want to do this in the most helpful manner. Thanks

[hanksterr7]

I am going to use this issue #7153 to discuss my topic #7. This is the most complicated of my topics. I can create other issues for the other topics if that would be helpful.

So here goes

My overall topic list (#1 and #2 completed in separate PR):
3 divide by zero bug with calculate_percent() if have same # of pass and exclude results
4 Issue when have 2 targets in a target/action group - the more recent target instance can get missed when looking for the "due" interval
5 filters evaluated against multiple target-associated dates ($target_dates instead of $dateTarget)
6 missing/incorrect rule status labels in reminder details and reminder widget
7 logic for optional filters when defined across data domains (and adding detailed logging)
8 age and gender filters don't check for required/optional
9 custom data entry form could be taller, so can see and edit previous entries without scrolling

My topic #7
Demographic filter evaluation happens in test_filter(), which is called from test_rules_clinic() in clinical_rules.php.
In doing its work, this function can evaluate a variety of different filter types: age, gender, db lookups (lifestyle, custom forms, custom table lookups), lists (diagnoses, meds, surgeries, allergies, etc) and procedures.
Multiple instances of a filter type can exist in a rule (i.e. there can be several "lists" "diagnosis" filters, each looking for a different ICD code).
A rule can also contain filters of different types (e.g. a rule can have any or all of: age, gender, diagnosis, meds and lifestyle filters).
In a rule, each filter is flagged as either "required" or "optional", and as either "inclusion" or "exclusion".
The rule entry gui allows any filter type to be flagged as "exclusion", but clinical_rules.php evaluates only "lists" filters as exclusion criteria
All inclusion filters are evaluated first. If inclusion evaluation succeeds, the exclusion filters are then evaluated.
In a rule, each filter type can have a mix of required and optional filters (e.g. there can be a mix of required and optional diagnosis filters, and a mix of required and optional lifestyle filters). However, if any filters for a filter type are "required", the optional ones have no meaning and are ignored.
Similarly, if the filters for one filter type contain a required filter, and the filters for a second filter type are all optional, the filters for the second type have no meaning and are ignored.
Example: suppose a rule has:
-- diagnosis filters requiring ICD codes 1 and 2 and also listing ICD code 3 as optional. There is no purpose to the specification of ICD code 3. If ICD codes 1 and 2 are required, then it does not matter if ICD code 3 is found or not.
-- similarly, if the diagnosis filters are required, and the lifestyle filters are optional, there is no purpose to defining lifestyle filters. If the diagnosis filters are found in a patient's data, it does not matter if the lifestyle filters are found or not.
The logic currently in clinical_rules.php does not handle this required/optional logic properly. I suspect it has had this flaw for a long time.
It treats each filter type as "required".
When evaluating a filter type, if the evaluation of the type returns "false", the overall evaluation of all filters in a rule returns "false" and the rule is deemed not applicable to a patient.
The logic DOES consider the required/optional flag when evaluating multiple filters for a filter type, for lists, db and procedure filter types (but not for age or gender filters). E.g. if the diagnosis filters (which are in the lists filter type) for a rule are all optional, each mentioning a different ICD code, then if ANY of the ICD codes are found in a patient's data, the evaluation of the diagnosis filters will return true.
But suppose the rule also mentions an optional lifestyle filter (which is a db filter type).
If the diagnosis filters evaluate to false, processing will stop, and the lifestyle filter will not be examined.
My code changes fix this deficiency, allowing filter type evaluation to return true, false, or "continue" instead of just true or false.
It returns "true" when all filters in a filter type are required and evaluate to true, OR, if all filters in a filter type are optional and at least one evaluates to true.
It returns "continue" when all filters for a filter type are optional and all evaluate to false.
It returns "false" if any required filters in a filter type evaluate to false
If no filter types evaluate to false, and at least one filter type evaluates to true, the overall evaluation is declared true and the rule is deemed applicable to a patient (pending evaluation of exclusions, which also adds this same "continue" logic)

Example:
define a rule with two demographic filters:
-- One for lifestyle:tobacco, match: "current", inclusion: yes
-- One for allergy to aspirin, inclusion: yes

Specify a patient as having an allergy to aspirin (as a text string, not a term lookup), and being a current tobacco user

Create a custom target and action for the rule, and mention custom input: yes in the action definition, so you can see the status of rule evaluation against the patient

Observe, using the released code base:
In Rule Detail display, cause both filters to show as Required.

Modify the filter criteria to create four tests: (1) neither, (2) the tobacco, (3) the allergy, or (4) both evaluate to true. This modification can be done by toggling between "aspirin" and "aspirinn" as the allergy filter value, and between "current" and "zcurrent" as the tobacco filter value. (note that "current" and "currentt" both look for a status of current tobacco user, but "zcurrent" does not)
Observe the status of your rule in the Clinical Reminders widget in a patient's dashboard
-- neither true -> NA (i.e. the rule target is not listed). Both true: due. tobacco only: NA. allergy only: NA
Now modify the filters so they both show as optional in Rule Detail display
-- neither true -> NA. Both true: due. smoking only: NA. allergy only: NA
==> so, behavior is the same regardless of required/optional flag. Both filters are treated as required

my version:
Cause both filters to show as required: neither true -> NA. Both true: past due. smoking only: NA. allergy only: NA
Cause both filters to show as optional: neither true -> NA. Both true: past due. smoking only: past due. allergy only: past due

Note that, in the current code base, the rule is showing as "Due", even though no instance of the target has been created. This is an incorrect status. It should show as "past due", with this status being modifiable if you create an instance of the action and vary the date of the instance. This status display is fixed in my topic #6

[hanksterr7]

I created #7157 to continue the discussion of filter dates. Not sure how to link 7157 to the PR

[stephenwaite]

hi @hanksterr7, you can edit the PR and update what issue is fixed by the Fixes # magic

[hanksterr7]

I believe I can provide a simplified description of the problem with the current code:
-- the "check" functions (database_check, lists_check, and procedure_check) return false if all filters they are examining are optional, and none succeed
-- test_filters returns false is any of the "check" functions return false
So, if a rule has only optional filters, and these filters require evaluation by more than one "check" function, then after evaluation by the first "check" function, test_filters currently will return false without testing the second "check" function to see if it succeeds. This is not appropriate. All "check" functions need to be evaluated in this scenario to see if the optional filters reviewed by any "check" function succeeds. My allowing the "check" functions to return "continue" in addition to true and false solves this deficiency.

[sjpadgett]

Has any thought been put on how, if any, effect this will have on legacy?

[bradymiller]

Still going through this, but don't see any negative impact on legacy rules at this time.

[hanksterr7]

Yes, I tried to ensure my changes would have no impact on existing rules (other than to maybe make them work properly, if any had been working incorrectly for certain patient-data situations). I don't believe my changes will cause any rules that were producing correct results to suddenly start producing incorrect or unexpected solutions.
Out of curiosity, I know you have some automated testing that is part of your system. Are there automated test of clinical rules? If so, I'd be happy to review, and add scenarios that my changes are addressing.

[bradymiller]

@hanksterr7 , No clinical rules testing in the automated testing. Would be awesome to incorporate CDR engine functions in the automated testing, although not simple task since need to get data into the database for the testing (just makes it a little more difficult). Can get an idea of the automated testing in the tests/Tests/ directory (and can see how they are run by checking out the functions in the easy dev docker).