cds-hooks / docs

CDS Hooks website & specification
http://cds-hooks.org
Apache License 2.0
166 stars 61 forks source link

How to support pre-fetch tokens for objects in context? #377

Open isaacvetter opened 6 years ago

isaacvetter commented 6 years ago

As part of the specification of each hook, each field defined in context is explicitly labeled whether it can be used as a Prefetch Token.

Prefetch tokens are important because they are how a CDS service specifies any specific FHIR queries to be run as part of its pre-fetch during the request. A CDS service's discovery endpoint specifies it's pre-fetch as a pre-fetch template.

Currently, our pre-fetch token syntax is nice and simple - it only supports top-level elements. For example, a service responding to the [order-review](https://cds-hooks.org/hooks/order-review/) hook can request pre-fetch data about the patient, like so:

{
  "prefetch": {
    "p": "Patient/{{context.patientId}}"
  }
}

But the service has no way to request pre-fetch data that's based upon the orders object which is also sent in context. Unlike the simple, valued {{context.patientId}}, relating a prefetch template to the orders being sent would necessitate a prefetch key such as: {{context.orders.MedicationRequest.id[0]}} or something even more complicated using FHIRPath.

This inability to refer to objects in context is a gap.

Is there a simple, targeted method to enable this? What about something as straighforward as context...id returning a simple comma-delimited string of FHIR identifiers that could be subsequently used in a search?

lmckenzi commented 6 years ago

What Isaac actually proposed where it says context...id before Github intervened was context.<prefetch key>.<FHIR resource name>.id

I'd be in favor of this. The reason for putting resource names in is to allow you to search by each resource because the ids aren't unique across resource types. The comma-separated approach allows you to say something like DeviceRequest?_id={{context.orders.medicationRequest.id}}&_include=DeviceRequest.device

Though that does mean that your prefetch space is going to contain both device requests and devices, the former being redundant with what's already in the hook invocation.

kpshek commented 6 years ago

I'd be in favor of this. The reason for putting resource names in is to allow you to search by each resource because the ids aren't unique across resource types.

So is context.<prefetch key>.<FHIR resource name>.id only important in the case where the context value can multiple FHIR resource types?

In the hooks I've seen prototyped today, if the context field value relates to a FHIR resource, there is just a single resource in use so using context.<prefetch key>.id as the prefetch token is not ambiguous.

lmckenzi commented 6 years ago

Right. If the context element is limited to a single resource type, no need to include the resource name in the path (unless there was some reason we wanted consistency in context strings, perhaps to allow for future evolution of the hook?)

kpshek commented 6 years ago

Thanks for the clarification @lmckenzi.

There is no limitation of context fields to always be associated to a single FHIR resource type, but I would expect that to be the overwhelming majority of use cases. I think a context field relating to disparate FHIR resources is definitely an edge case.

As we wrap up 1.0, I don't see a big motivation in trying to further enhance prefetch semantics at this point. After 1.0, if we see a lot of implementer experience/use out of prefetch, then I think this is something worth looking out.

Personally, I don't think this is a problem space that warrants the additional complexity. I think most CDS Service implementers are looking at prefetch as a method to remove the need to make their own FHIR queries for convenience rather than on better merits. There are limited cases where prefetch is truly a benefit and any additional complexity we add to will further lessen the use cases in which prefetch was designed for (primarily performance considerations of in-memory or shared data access across services).

lmckenzi commented 6 years ago

I'm not sure "remove the need to make their own FHIR queries" is necessarily an illegitimate requirement. It may be that EHRs will choose not to satisfy the prefetch, but at least if the prefetch is computably expressible, an intermediary could do it for them. My concern is that if prefetch isn't possible, DaVinci is likely to push to drop CDS Hooks and proceed with a custom operation instead where they can demand inclusion of whatever data they like (and still have the flexibility to subsequently query through the inclusion of a hook-like token they can exercise). I don't think that would be a good outcome.

kpshek commented 6 years ago

Note the last part of my statement which was left out: "remove the need to make their own FHIR queries for convenience rather than on better merits". This critical part of my argument is what I'm pushing back against here (which is what I'm hearing is the reasoning behind this request). Please correct my understanding of the situation if I am not understanding this correctly.

My concern is that if prefetch isn't possible, DaVinci is likely to push to drop CDS Hooks and proceed with a custom operation instead where they can demand inclusion of whatever data they like (and still have the flexibility to subsequently query through the inclusion of a hook-like token they can exercise).

I may be further misunderstanding you, but these statements seem like an argument for introduce complexity and promoting a design/approach that I think does not stand on its own merits due to implementers that are prioritizing their convenience above good API/standards design. I know you don't agree with that as well so there must be a disconnect here.

lmckenzi commented 6 years ago

It's not totally clear to me that convenience doesn't itself have merit or that doing so necessarily constitutes bad API design, though I'm open to that perspective. One challenge that I have is that there isn't anyone who can convincingly make the case that prefetch for convenience/simplification for payers is a bad idea. (Isaac doesn't speak with as strong an opinion on this as you do :>) If I have to weigh the cost of enabling (but not mandating support for) more complex prefetch vs. DaVinci mandating that EHRs implement a custom operation instead, I know where I'd land. If we can convince payers they don't need prefetch, that'd be even better. However, I'm not confident of my ability to make that pitch given my own ambivilence on the question and the mindset they bring to the table from prior interfaces.

eedrummer commented 6 years ago

I'll try to add some more context from Da Vinci to see if that helps justify the request or see if we are on the right path.

In our group, @lmckenzi has advocated that we stick with hooks that have been included in the specification, such as order-review. Given that, with the currently defined context and current capabilities of prefetch, we will be unable to send all of the information we know a payer will need to properly respond to a request.

That appears to leave us with two options if we want to be able to send all of the information necessary in a single request:

  1. Define a new hook that has a context with all of the information we need
  2. Enhance prefetch capabilities so that all of the desired information can be sent in the request

As I mentioned before, we have taken Option 1 off of the table to avoid hook proliferation and try to leverage current EHR implementation. We are pursuing Option 2 because payers in our group did not respond well to the suggestion of querying the EHR system.

I understand the benefits that querying the EHR can convey. It gives the payers future opportunities to make decisions on even more data than they may use today. However, our base use case with order-review forces us to query for information that is necessary to all CDS Services.

lmckenzi commented 6 years ago

The base usecase for prefetch - wanting to provide optimal access to data that may already be in memory, minimizing queries when providing data to multiple hook services, etc. all seems to elements associated to resources provided in context, not just those passed as an identifier in the context. The payer's current preference for prefetch/context data rather than querying their own is not a driver for this architectural change. The key is that the decision to include a full resource rather than just an identifier in context should not drive whether prefetch is possible. The decision of identifier vs. full resource should be driven only by whether we believe almost all consumers of the hook will need the full resource rather than just the id. Ensuring that prefetch is possible with both approaches is the best way of separating these concerns.

Whether some systems rely on prefetch or not (and whether they should) is a separate architectural/cultural question that will need to be addressed separately from the "Is it reasonable to enable prefetch at all here?" question.

brynrhodes commented 6 years ago

Proposed Disposition: Persuasive, the limitation on prefetch token references within prefetch templates will be relaxed to allow for context..[].id Moved: Isaac Vetter, Second: Ken Kawamoto Motion tabled to allow for more discussion

MedicationRequest?id={{context.orders.MedicationRequest.id}} &_include=MedicationRequest:patient &_include=MedicationRequest:intended-dispenser &_include=MedicationRequest:requester:Practitioner &_include=MedicationRequest:medication &_include=MedicationRequest:on-behalf &_include=MedicationRequest:insurance:Coverage

lmckenzi commented 5 years ago

I haven't seen more discussion - how do we move this forward?

bvdh commented 5 years ago

I think we should.

lmckenzi commented 2 years ago

Poking on this. There was a proposed disposition, but it never moved forward. @brynrhodes @isaacvetter - can we take this up at the Thur. Q4 session?

brynrhodes commented 2 years ago

Proposed disposition: Support the use of Simple FHIRPath as the context selector in prefetch template processing, option 2 below.

Requires the implementer to

  1. Retrieve the "value" from the context based on the context.X reference
  2. Process the remainder of the path as a Simple FHIRPath focused on that "value"
  3. If the result of that process is a collection, "stringify" it as a comma-separated list to put it in the URL

Motion: Lloyd McKenzie/Bob Dieterle: Further discussion: Propose additional discussion, motion tabled

Propose to take this up on the next CDS WG call on 1/26.

  1. "prescriber": "MedicationRequest?_id={{context.draftOrders.MedicationRequest.id}}&_include=MedicationRequest:author" Shorthand, requires implementers to perform the expansion and recognize when it's required
  2. "Simple FHIRPath": {{context.draftOrders.entry.resource.ofType(MedicationRequest).id}} Requires a limited FHIRPath implementation, supports most, if not all the use cases required, implementers would need to concatenate expressions, constrained FHIRPath: https://www.hl7.org/fhir/fhirpath.html#simple
  3. "Full FHIRPath": {{context.draftOrders.entry.resource.ofType(MedicationRequest).id.aggregate($total + ', ' & $this)}}

context.fooResource.id

hstrasbe commented 2 years ago

It would be helpful to see an example for another use case. Consider a MedicationRequest in the draft orders that includes a medicationReference. The details of this Medication resource are often very important for CDS. What would the syntax look like to indicate that we would like the system to prefetch this Medication resource?

Similarly, if there are other MedicationRequest resources that are sent as part of the prefetch, but if they too include medicationReference elements, how could we ask the system to prefetch these Medication resources as well?

lmckenzi commented 2 years ago

It would be the same, just the include would change. i.e. "&_include=MedicationRequest.medication". If somehow a hook had a context that could be a mixture of MedicationRequest and MedicationStatement instances, then you'd have two separate contexts - one for MedRequest and one for MedStatement.

brynrhodes commented 2 years ago

For retrieving Medications as part of pre-fetch, would need to be:

MedicationRequest?_id={{context.draftOrders.entries.ofType('MedicationRequest').id}}&_include=MedicationRequest.medication

Note this is re-requesting the draftOrders which may not be in the FHIR server

brynrhodes commented 2 years ago

{{context.draftOrders.entries.ofType('MedicationRequest').medication.reference}}

{{context.draftOrders.entries.ofType('MedicationRequest').requester.reference}}?_include=PractitionerRole.organization&_include=PractitionerRole.location

Can we propose a shorthand "include" for prefetches?

Support EvenSimplerFHIRPath (with path navigation and ofType() only), plus the additional semantics that if the result of the FHIRPath is a reference, it is transformed into a search by id so that it can be used as the basis for a prefetch with additional includes. In addition, the requirement that the prefetch would always be evaluated against the FHIR server (so would not be expected to be able to access in memory content).

Alternatively, add an option to the prefetch template to support specifying additionalReferenceTypes, so you could say, with this prefetch, also include any references to resources of type "Medication", "Location", and "Organization", recursively.