Condition List Implementation

[mogoodrich (Mark Goodrich)]

Hello all…

I’ve been diving back into the Condition List functionality and have been remembering what I found confusing/frustrating about it previously and I had some questions for people… not sure who was on the initial design?

My main frustration, is that a “Condition” has an “onset date” and “end date”, but also has a “Clinical Status” enum that can be one of three values: ACTIVE, INACTIVE, and HISTORY_OF.

This could lead to funky, confusing states. Note that from the API level, an “active” condition is just a condition with end date == null, it isn’t dependent on the “clinicial status” at all:

openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/api/db/hibernate/HibernateConditionDAO.java#L101

In our current model it’s unclear to me what the “proper” model should be, even for simple use cases.

For instance, say a patient went to see a doctor on January 1, 2000 and was diagnosed with “Condition A” on that date and the doctor added it to their condition list. In the database, I’d expect to see the following row in the DB:

Condition: Condition A, Clinical Status: ACTIVE, Onset Date: 2000-01-01, End Date: Null

Now let’s say 10 years later on Feb 1, 2010, at a doctor decides that the condition is no longer active, and removes it from the patient’s condition list. The simplest thing to do would be to update the existing row to follows:

Condition: Condition A, Clinical Status: INACTIVE, Onset Date: 2000-01-01, End Date: 2010-02-01

However, it’s possible the intended modelling might be for two rows to end up the database:

Condition: Condition A, Clinical Status: ACTIVE, Onset Date: 2000-01-01, End Date: 2010-02-01 Condition: Condition A, Clinical Status: INACTIVE, Onset Date: 2010-02-01, End Date: null

Does anyone have options on which of these is the intended use? @jteich @burke @jdick @mksd? (Sorry Jonathan, I know I’ve asked you a similar question before!)

This is particularly relevant because there’s an existing Condition List widget in the reference app, and one is being built in the new SPA framework, and we’d want them to interpret the data in the same manner!

Right now, if the above scenario was entered using the existing Condition List Reference App UI, you’d get neither of the above in the underlying database, but the following 2 rows, which seems clearly wrong (and may be the source of my confusion):

Condition: Condition A, Clinical Status: ACTIVE, Onset Date: 2000-01-01, End Date: 2000-01-01 Condition: Condition A, Clinical Status: INACTIVE, Onset Date: 2000-01-01, End Date: 2010-02-01

Note that the UI only displays the most recent status, so in the UI it reads “INACTIVE”, onset: 2000-01-01, end date: 2010-02-01, which more closely aligns with the first of my two potential examples. Can we change the Condition List UI so that instead of adding a new row, it simply overwrites the existing row?

I feel like this is a case where we’ve overengineered thing, preventing us from getting working code out to users that meets the 95% use case.

fyi @ball @ddesimone @mseaton

Take care, Mark

[burke (Burke Mamlin)]

I would understand if the default behavior added onset date or end date when a condition is added or removed from the list of active conditions, this would often be inaccurate. For example, adding diabetes to the condition list for a new patient is rarely done because the patient was diagnosed with diabetes that day. Likewise, items are typically taken off the list during a visit long after the condition has resolved. So, it would make sense to me for the clinical status (whether something is on the list) change between ACTIVE (on list) or INACTIVE (off list) without setting onset or end dates; rather, allowing the provider to optionally enter those dates when the data are available.

@mogoodrich

Condition: Condition A, Clinical Status: ACTIVE, Onset Date: 2000-01-01, End Date: 2010-02-01 Condition: Condition A, Clinical Status: INACTIVE, Onset Date: 2010-02-01, End Date: null

This is not the intent. If the provider wanted to take an items off the condition list, I would expect the clinical status to be INACTIVE.

It helps to understand that “Problem Lists” (aka condition lists) have historically been a means for a provider to track clinically relevant conditions and not as a source of truth of a patient’s conditions over time. For example, consider a patient with ongoing acne who fully recovered from a stroke 2 years ago. Physiologically, she has “active” acne and an “inactive” stroke. As her primary care physician, I would want stroke (or history of stroke) on her condition list (since it’s a risk factor, effects medication decisions, etc.) and may not want/need to see acne in her condition list. So, “Clinical” status is not the same thing as “Physiologic” status (i.e., the actual state of the condition).

As a “clinical” status, I think condition status (ACTIVE, INACTIVE, HISTORY_OF) was meant to determine whether or not the condition should be listed in the active problem list – ACTIVE items would show on the condition list and INACTIVE conditions would be hidden by default; HISTORY_OF was added as a way to include entries in the condition list without having to label something physiologically inactive as “active.”

• ACTIVE/INACTIVE/HISTORY_OF were intended for list management, not physiologic status. So, the API is likely a mixture of this intent and people assuming they represent physiologic status.
• Being able to record physiologic details when available is great; however, there are a myriad of reasons why the EMR will be a poor representation of physiologic truth of condition (e.g., a patient may be not be seen for 2 years,;new patients present with longstanding problems with an onset of “when I was younger”, “a while ago”, “as long as I can remember”, or “I don’t know”; not every condition is address at every visit; etc.)
• Typically, nobody is responsible for keeping the condition list accurate, so things get added as needed (sometimes as duplicates are with variations on granularity), there are little to no workflows that depend on removing things from the list, and – just like a wiki – entropy takes over and people complain about the mess.

All that said, (1) we should move toward FHIR’s view of the world and (2) how a condition appears within a list for a provider is ideally provider-specific, so would be better handled in a user-specific way (e.g., exposed as FHIR extensions). For example, a cardiology will want to focus on different conditions than an ophthalmologist. Another provider might like the conditions to be listed in the same order as in her notes.

[mogoodrich (Mark Goodrich)]

Thanks @burke, very helpful! I think that leads me with a way forward to fix… let me think through things a bit and then I’ll confirm here.

[mogoodrich (Mark Goodrich)]

So I just had a call with @ball and @ddesimone internally about this.

Basically, we are looking for a relatively short-term work to get the Condition List UI in Ref App working properly. It’s been built for a couple years, but is buggy to the point of being unusable. The good news is based on your feedback @burke I think it should be relatively straightforward to get to place where it’s functional for the 95% use case. Generally, we think the UI is pretty good as long as the bugs are fixed (as least for basic use cases).

I think most of the issues come from the fact that the Condition List UI tries to maintain a “condition history”, that is storing multiple rows for a single condition to track it’s active and inactive states. Since per @burke this wasn’t the intent, and makes things more complicated, we were thinking of making the following changes:

• Rework so that it only maintains a single non-voided row in the DB per condition
• When updating a record, favor voiding and creating a new row vs updating the existing row to maintain an audit history in case it is ever needed
• Decouple ACTIVE and INACTIVE from onset date and end date, ie allow onset date and end date to be empty, regardless of ACTIVE/INACTIVE state, and don’t automatically set an end date when marking a condition as inactive; the only rule we’d enforce is you can’t have an end date for an ACTIVE condition
• Continue to ignore HISTORY_OF state at this point

We think this will allow us to get the Condition List UI usable in a reasonable amount of time, while still modelling the data behind the scenes that could be compatible with more advanced UIs that could be developed in the future. We are planning our FY Q1 (July to Sept) roadmap now, and we are hopeful this work will be included.

Take care, Mark

[mogoodrich (Mark Goodrich)]

Actually, @burke I didn’t realize that this logic is actually happening the API, which I think should be changed.

See here.

Specifically, lines 65 to 75. It appears that in these lines, if you change the clinical status of existing condition, it:

• Creates new copy of the condition
• Sets the “end date” of the existing condition to the onset date of the new condition
• Saves both conditions

So it does seem to be trying to create a “state history” of the condition, which it sounds was not the original intent, at least in the design.

I’d rather it simply void the existing condition (without changing the date or anything else), but this would of course be a non-trivial API change.

I’ve asked to have a design call about this, where hopefully we can discuss further.

Take care, Mark

[ibacher (Ian Bacher)]

@mogoodrich

Decouple ACTIVE and INACTIVE from onset date and end date, ie allow onset date and end date to be empty, regardless of ACTIVE/INACTIVE state, and don’t automatically set an end date when marking a condition as inactive; the only rule we’d enforce is you can’t have an end date for an ACTIVE condition

To align things better with FHIR, I would actually advocate getting rid of the onset and end dates for Conditions altogether. Their are corresponding fields in the FHIR resource, but it seems to me that these are intended to represent a structured way of capturing more nuanced data than just a date time. For example, I can encode through FHIR something like “I’ve had a back-pain since my late 40s”. Modelling this as a simple date time means that we’re losing expressiveness.

I’m totally in favour of decoupling active and inactive from these dates.

@mogoodrich

Continue to ignore HISTORY_OF state at this point

I would get rid of HISTORY_OF altogether. FHIR represents the clinical status of Conditions using a hierarchical value set where a Condition is either active or inactive. HISTORY_OF doesn’t seem to add new information beyond inactive, although the FHIR value of remission might be useful for highlighting conditions at a higher risk of recurrence.

@mogoodrich

I’d rather it simply void the existing condition (without changing the date or anything else), but this would of course be a non-trivial API change.

This seems like the correct choice to me.

The only other thing I’d note is that the FHIR Condition resource is slightly broader than our Condition class, mainly because Conditions are also how encounter / visit diagnoses are represented in FHIR.

@burke

(2) how a condition appears within a list for a provider is ideally provider-specific, so would be better handled in a user-specific way (e.g., exposed as FHIR extensions)

I would agree that we want the problem list to be (potentially) handleable in a user-specific way. I’m less certain that this information should be encoded in the FHIR API.

[jteich (Jonathan Teich)]

Basically agree with the discussion above, and it reflects similar discussions we had in around 2015 (search for Condition List in talk). The live discussion would be helpful to getting everyone fully on the same page. Fundamentally there were these use cases:

1. [The patient] has COPD: ACTIVE condition; you want to see it on the list.

2. Has a history of Tb, no longer active: INACTIVE medically, although you probably want to see it on the list. This is why we wanted to have HISTORY_OF. (FHIR also uses RESOLVED, but we think FHIR’s statuses are too complicated)

3. …had a sore throat. INACTIVE condition – if it ever got onto the condition list at all, because it’s not really long-standing or very important

4. Has cancer in remission. We probably want to see that on the list, most people would still consider it an ACTIVE condition (Again, FHIR has REMISSION, but that’s too complicated).

5. History of heart valve surgery. Medically, you might say this is an inactive case of valvular heart disease, but that’s not how docs think; procedures do get mixed into the condition list sometimes. Either way, this is a good case for HISTORY_OF – it’s important to see on the list, but not medically active.

As @burke noted, the dates aren’t that important and are not well kept. They should be secondary – a new problem could have today’s date listed as the default for onset date, or it could simply not have one.

[mogoodrich (Mark Goodrich)]

Thanks @ibacher @jteich… I will look up the old discussions on Talk.

It seems like we are generally on the same page, but does anybody know why the API was implemented as it is (where “saveCondition” really creates a “history” of a condition state via adding multiple rows for a single condition in the DB)? I’m 110% up for changing it, but I just want to make sure there isn’t anybody currently using it that way or expecting it to work that way… not sure if there’s anyone in particular we need to reach out to before changing?

[ibacher (Ian Bacher)]

@mogoodrich

It seems like we are generally on the same page, but does anybody know why the API was implemented as it is (where “saveCondition” really creates a “history” of a condition state via adding multiple rows for a single condition in the DB)? I’m 110% up for changing it, but I just want to make sure there isn’t anybody currently using it that way or expecting it to work that way… not sure if there’s anyone in particular we need to reach out to before changing?

It seems to have originated from this set of requirements and the ensuing discussion there. It seems to be kept for audit reasons (and, actually, that suggests that the date issue is really a simple bug).

I’m pretty agnostic as to whether we should keep this around for audit history (it seems to me that the snapshot of the condition list at the time a diagnosis or order is made may have some legal relevance), but if we do, we should probably have a previous_version pointer as we do for Obs.

[mogoodrich (Mark Goodrich)]

Thanks so much @ibacher for that thread, very helpful!

Would be great to pull in @darius or @akhilmalhotra1 for their thoughts.

So it is for audit history… I understand not wanting to use “voided” because it’s not really voided, but it makes it a difficult if I just want to “get the current status for all the conditions for the patient”.

And I don’t think it should be setting the date behind the scenes, unless the onset and end dates are just being used for audit purposes as well? I think this is what you were getting at @ibacher?

For example, say that I add a condition, then in the future mark it as INACTIVE, and then afterwards mark it back as ACTIVE, all without specifying dates:

• Create a condition with status=ACTIVE
• On 2020-02-01 I change that condition’s status to INACTIVE (but don’t modify the date fields or any other fields) and resave using the API.
• On 2020-03-01 I change that condition’s status to ACTIVE (but don’t modify the date fields or any other fields) and resave using the API.

Then in the database I believe end up with:

row=1, status=ACTIVE, onsetDate=null, endDate=2020-02-01
row=2, status=INACTIVE,onsetDate=2020-02-01, endDate=2020-03-01, prev=1
row=3, status=ACTIVE,onsetDate=2020-03-01, endDate=null, prev=2

If the “previous” is just used for audit purposed, but the onset date is really supposed to be an onset date, not an audit date, I’d expect to see:

row=1, status=ACTIVE, onsetDate=null, endDate=null
row=2, status=INACTIVE,onsetDate=null, endDate=null, prev=1
row=3, status=ACTIVE,onsetDate=null, endDate=null, prev=2

ie it shouldn’t be automatically setting dates behind the scenes.

Also, to get all the “conditions” for a patient, regardless of status, I’d need to do something like this, which is a little convoluted:

select * from condition where patient_id=abc and row not in (select prev from condition where patient_id=abc)

In the ConditionService saveCondition method could we change:

	condition = Condition.newInstance(condition);
	condition.setPreviousVersion(existingCondition);
	if (existingCondition.getClinicalStatus().equals(condition.getClinicalStatus())) {
		existingCondition.setVoided(true);
		conditionDAO.saveCondition(existingCondition);
		return conditionDAO.saveCondition(condition);
	}
	Date onSetDate = condition.getOnsetDate() != null ? condition.getOnsetDate() : new Date();
	existingCondition.setEndDate(onSetDate);
	conditionDAO.saveCondition(existingCondition);
	condition.setOnsetDate(onSetDate);

	return conditionDAO.saveCondition(condition);

to:

	condition = Condition.newInstance(condition);
	condition.setPreviousVersion(existingCondition);
	if (existingCondition.getClinicalStatus().equals(condition.getClinicalStatus())) {
		existingCondition.setVoided(true);
	}
	
	conditionDAO.saveCondition(existingCondition);
	return conditionDAO.saveCondition(condition);

I assume this would break some of the existing condition functionality in EMR-API though?

Take care, Mark

[ibacher (Ian Bacher)]

@mogoodrich

And I don’t think it should be setting the date behind the scenes, unless the onset and end dates are just being used for audit purposes as well? I think this is what you were getting at @ibacher?

Yes, that’s definitely what I was getting at.

@mogoodrich

If the “previous” is just used for audit purposed, but the onset date is really supposed to be an onset date, not an audit date, I’d expect to see:

That’s what I’d expect. After all, we’ve already got date_created, date_updated and dated_voided.

[akhilmalhotra1 (Akhil Malhotra)]

The History_of status was added to highlight something of clinical relevance but medically inactive as @jteich has summed up above and we all agreed at the time that FHIR’s statuses were complicated for our use and more inclined towards malignant conditions.

I’m not sure why multiple copies of the same condition are being created along with the dates that are clinically irrelevant and most probably inaccurate. As @burke and @jteich noted above, the dates can probably be gotten rid of.

[mogoodrich (Mark Goodrich)]

For reference, we’ve had a couple design discussions about this, and the conclusion was to simplify some of the business logic in the service layer:

TRUNK-5888

Then there would need to be some clean up of the Rep App Condition List UI to work with the new behavior.

However, there are some concerns about whether or not these service layer changes would break the Bahmni condition list implementation. Also, as a condition list is currently being worked on in the new 3.0 frontend, building out that new interface may be prioritized over fixing the old one.

Take care, Mark

[akhilmalhotra1 (Akhil Malhotra)]

Maybe @angshuonline can weigh-in on this.

@mogoodrich

However, there are some concerns about whether or not these service layer changes would break the Bahmni condition list implementation. Also, as a condition list is currently being worked on in the new 3.0 frontend, building out that new interface may be prioritized over fixing the old one.

[ball (Ellen Ball)]

The condition list remains buggy and needs to be fixed before it can be used in production. Does anyone use condition list in the RefApp?

I have reviewed tickets in the Condition List epic. Many are marked as “approved” but many serious problems are not fixed.

Time zone: When a condition is created, the onset date is set to yesterday at 20:00:00. Possibly fixed by this Pull Request: RA-1678 Problems for time zone but still a problem today.

Multiple rows in the conditions database table:

What creates a new row?

• A new condition? YES
• Changing a condition from Active to inactive? Maybe

Examples of multiple rows:

• Delete of condition (inactive or active) creates an additional row in the conditions table. The UI is fine, but the database is hairy. The initial row is updated with voided and date_changed, but not voided_by or date_voided. The new row has an end date and voided.

Dates:

1. Dates should not be required for active and inactive.
2. The UI forces an onset date for all active conditions (wrong)
3. The UI forces onset and end dates for inactive conditions. (wrong)
4. On edit of a condition, the UI defaults the onset date to today. (wrong) It should show the dates if they are already set, but can be modified by the UI.
5. If there’s an end date, it can’t be active.
6. error is partially described

UI change:

1. The condition should not be editable and not appear in a text box.
2. Page reload is required for edited inactive conditions (Is this right?)
3. The condition list should be configured (global property with uuid?) with a concept ConvSet
4. There should be a shortcut to select today or no date for onset and end dates.

Configuration The condition list should be configured (global property with uuid; coreapps.conditionListSet?) with a concept ConvSet. NOTE: There is a global property called “coreapps.conditionListClasses”. With that you can restrict the choice of condition list to diagnosis, finding, and/or symptom with a comma-separated list of uuids. See this ticket: [RA-1486] Conditions should be limited to diagnoses, findings, etc - OpenMRS Issues

Tech Notes

From this Talk thread, it seems that we want to change the model where it tries to preserve a “history” of a condition and make the following changes:

• Only have one non-voided row for per condition
• When updating a condition, void and recreate to preserve an audit history
• Removing a condition should just void it
• Active/Inactive not tied to onset/end date that is:
  • You can add a condition without an onset date
  • Making a condition inactive does not automatically set end date
  • You can have conditions without onset or end date, of either type (INACTIVE or ACTIVE)
  • Only restriction is active condition can’t have end date

NOTE: Much of this documentation from Mark Goodrich who did much of the analysis. Thanks to Mark.

FYI @burke @ibacher @mozzy @mseaton