-
Notifications
You must be signed in to change notification settings - Fork 61
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
overrideReasons for 1.1 #513
Conversation
I've read through the text here but I can't figure out what's going on. An end-to-end example might help. Question I have, that I hope this PR can address:
|
Co-Authored-By: Josh Mandel <[email protected]>
Josh, Your lack of understanding reflects poorly on my spec authoring. :( Particularly, if you've read the recently created Acknowledgement-reasons-for-1.1 wiki page. (Have you read this page?) Regardless, the spec needs to clearly set expectation for this feature.
Yes, an "acknowledgement" is only used to indicate why a card's advice was not taken. I was initially calling these reject reasons, we got feedback during the CDS Hooks breakout that a clinician would rather acknowledge cds than reject it. Also, acknowledge is an existing term and reject would be a new term. I added a new sentence to the ackReason row within the Card table to try to better explain this.
Yes, an acknowledgement rejects/ignores the whole card. I think that the card is the right level. While an acknowledgement reason almost behaves in the workflow as an alternate suggestion (i.e. the user can pick suggestion A, or suggestion B or the ackReason); it is not a suggestion. An ackReason is the rejection of the provided suggestions. (This design is influenced by existing art). I struggled to determine how much of the wiki page could be forced into the spec. Are you suggesting that an example should be in the spec?
Yes, I haven't written the PR content for the "feedback service" yet. Here's the draft wiki writeup: Feedback-endpoint-for-CDS-Hooks-1.1. Isaac |
Following conversation with @dennispatterson , @brettmarquard, we should:
|
Encourage standardization in the future.
per Bryn
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 great, just a few comments throughout
docs/specification/1.0.md
Outdated
|
||
#### OverrideReason | ||
|
||
An **OverrideReason** is described by the following attributes. This specification does not prescribe a standard set of override reasons; implementers are encouraged to submit suggestions for standardization. To enable analysis of a card's usage, the CDS service SHALL send the same `code`, `system`, and `label` across CDS Hooks requests for the same CDS client. |
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.
"...across CDS Hooks responses for the same..."
the CDS service SHALL send the same
code
,system
, andlabel
across CDS Hooks requests for the same CDS client.
I want to make sure this doesn't get read as each card must have the same code(s), but that the codes/systems/labels are from a set of codes/systems/labels that is consistently drawn from across replies. What about:
The CDS Service SHALL use a consistent set of override reasons across CDS Hooks responses for the same CDS Client
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 agree with the confusion; your proposed language here isn't helping me, though. I think we're trying to say:
- for a given CDS Client and label the CDS Service SHALL send the same
code
andsystem
- for a given CDS Client,
code
, andsystem
, the CDS Service SHALL send the samelabel
Is that right? i.e., both (1) and (2) are desired? Why is this important? What is the connection to analytics? Why not just do analytics based on the code/system? Also, why is all of thsi "for a given CDS Client"?
per Dennis
I recently had some e-mail conversations with Anna Dover at First Databank to get CDS Service perspective on this, which some comments bleeding into the territory of the feedback endpoint. I wanted to share what has been discussed so far:
As for the types of reasons, she shares some but also draws a distinction that not all reasons are override reasons, per se.
|
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.
Thanks -- my questions from up-thread have been addressed. I do it's still worth trying to nail down the language of what these things are called (@dennispatterson suggests some generic terms like response
). But with that caveat, from my perspective, this is good 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.
As Josh also mentions in the card.source.topic PR, can we update the overrideReason object to contain 'display' instead of 'label' to better match FHIR Coding?
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've suggested a minor change, but don't think it should block approval. Nice work!
for consistency with FHIR and card.topic
per Bryn, use: `"code":"reason-code-provided-by-service",` and not: `"code":"reason-id-provided-by-service",`
Add ackReason from https://github.com/cds-hooks/docs/wiki/Acknowledgement-reasons-for-1.1