Skip to content

Fix care-plan review loop: route on decision, fail closed - #147

Merged
jaideepr97 merged 1 commit into
samschifman:mainfrom
khaledsulayman:fix-careplan-review-decision-field
Aug 24, 2026
Merged

Fix care-plan review loop: route on decision, fail closed#147
jaideepr97 merged 1 commit into
samschifman:mainfrom
khaledsulayman:fix-careplan-review-decision-field

Conversation

@khaledsulayman

Copy link
Copy Markdown
Collaborator

Sam flagged verifying the reject/feedback loop — it was broken, in the fail-open direction.

Bug

The UI + contract (ReviewAction) submit the review outcome as decision, but the workflow switch and BFF state derivation keyed on .action, which is never present:

  • Workflow CheckCarePlanReview (acp-writer-workflow.yaml) — .careplanReview.action == "approve" | "request_changes", with defaultCondition: WriteFHIR.
  • BFF sonataflow_client.pyinfer_current_state / map_to_run_detail read .get("action").

Since .action is always null, every review fell through to the default → WriteFHIR. So clicking “Request changes” (reject) silently wrote the plan as active instead of looping to regenerate, and previousFeedback/reviewIteration never reached the UI. Approve only “worked” because the default happened to also be WriteFHIR.

Not caught earlier because the mock BFF (#136) and MSW mocks key on the contract field decision — only the real SonataFlow path had the mismatch.

Fix

  • Workflow: route on .careplanReview.decision; change defaultCondition from WriteFHIR to ReviewCarePlan so an unrecognized/missing decision fails closed (never auto-writes) — ties to Harden clinical review pipeline to fail safe #47.
  • BFF: infer_current_state + map_to_run_detail read .decision for routing state and previousFeedback.
  • Tests: test_bff_review_mapping.py covers approve→WriteFHIR, request_changes→GenerateBundle loop, and previousFeedback on round 2. Verified failing on the old .action code, passing after.

The UI is already correct (sends decision per the contract); no UI change needed.

Deploy note

Workflow change is oc apply (SonataFlow dev profile, no build); the BFF change needs an acp-writer-bff image rebuild + rollout.

Generated by Claude Code under the supervision of Khaled Sulayman

The UI/contract (ReviewAction) submit the review outcome as `decision`, but
the workflow switch (CheckCarePlanReview) and the BFF state derivation keyed on
`.action`, which is never present. Every review fell through to the switch
default -> WriteFHIR, so clicking "Request changes" (reject) silently wrote
the plan as active instead of looping to regenerate, and previousFeedback never
reached the UI.

- workflow: route CheckCarePlanReview on .careplanReview.decision (approve /
  request_changes); change the defaultCondition from WriteFHIR to ReviewCarePlan
  so an unrecognized decision fails CLOSED (never auto-writes).
- bff: infer_current_state + map_to_run_detail read .decision (not .action) for
  routing state and previousFeedback.
- tests: cover approve->WriteFHIR, request_changes->GenerateBundle loop, and
  previousFeedback surfacing on the second round. (Fail on the old .action code.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants