Add documentation for Patients with Billable Charge Item and Encounter/Appointment Details query

[sonzsara]

No description provided.

[Copilot]

Pull request overview

Adds a new Markdown documentation page describing the “Patients with Billable Charge Item and Encounter/Appointment Details” SSMM query so analysts can understand intent, parameters, and the exact SQL used.

Changes:

• Introduces a new SSMM query documentation file with Purpose/Parameters/Notes sections.
• Documents the SQL query that returns encounters with associated token bookings and billable charge items.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Analytics SQL Review — billable charge item + encounter/appointment report (ENG-910)

Ticket ask: report billable charge items with associated patient, encounter, and appointment details. The query delivers the right shape (patient → encounter → token booking → charge item, filtered to status = 'billable' and total_price > 0) and column set matches the ask, so requirement fidelity is fine at the grain level.

Numbers are not yet trustworthy — two correctness issues found:

1. Critical — no facility scoping. Despite the _ssmm filename, no join or WHERE clause constrains to a single facility. This returns billable charge items across every facility, not just SSMM.
2. High — no entered_in_error/deleted exclusion on emr_encounter, emr_tokenbooking, or emr_patientidentifier. Only the charge item's own status is checked; a mistakenly-created encounter/booking can still surface with a real charge item attached.

Documentation: Purpose/Notes/Parameters are accurate to the SQL and the earlier reviewer's "Purpose vs actual columns" comment was addressed. One gap remains: the template's ### Output column table is missing entirely (Medium).

Hygiene: correct domain folder, correct filename convention with _ssmm suffix, one file per PR, magic config_id = 21 documented in Notes — all fine.

Verdict: not yet safe to publish — fix the facility scope (Critical) and add status/deleted filters on the joined tables (High) before this feeds the dashboard. Output table doc gap is a quick follow-up, not a blocker for correctness.

Generated by Analytics SQL Reviewer for #147 · auto · 37.3 AIC · ⌖ 3.2 AIC · ⊞ 14.7K

[github-actions]

[Critical] No facility scoping — the _ssmm suffix promises a single-facility query, but nothing in the SQL enforces it.

Facility boundaries are normally enforced by the application, not by Postgres — a raw query has to add it explicitly (care-sql-code-review skill, inversion 2). This query has no facility_id predicate anywhere: not on emr_encounter, not on emr_tokenbooking, not on emr_chargeitem. As written it returns billable charge items across every facility in the deployment, not just SSMM, which is both a wrong-number bug for a per-facility dashboard and a cross-tenant data exposure.

Fix — scope through the encounter (or another table that carries facility_id):

WHERE emr_chargeitem.status = 'billable'
  AND emr_chargeitem.total_price > 0
  AND emr_encounter.facility_id = <ssmm_facility_id>
  ...

If the intent is genuinely "all facilities, but named _ssmm for another reason," please say so in ## Notes — as it stands the filename and the SQL disagree.

[github-actions]

[High] No entered_in_error / deleted exclusion on any joined table.

emr_chargeitem.status = 'billable' pins the charge item to a valid state, but emr_encounter, emr_tokenbooking, and emr_patientidentifier are joined with no status or deleted filter at all. A mistakenly-created encounter or token booking (status = 'entered_in_error') — or a soft-deleted row (deleted = TRUE) on any of these tables — will still surface in this report with a real billable charge item attached, since the charge item itself can be fine even when the encounter/booking it's linked to is not.

Fix — add per table:

FROM emr_encounter
JOIN emr_patient
	ON emr_patient.id = emr_encounter.patient_id
	AND emr_patient.deleted = FALSE
LEFT JOIN emr_patientidentifier
	ON emr_patientidentifier.patient_id = emr_patient.id
	AND emr_patientidentifier.config_id = 21
	AND emr_patientidentifier.deleted = FALSE
JOIN emr_tokenbooking
	ON emr_tokenbooking.associated_encounter_id = emr_encounter.id
	AND emr_tokenbooking.status != 'entered_in_error'
	AND emr_tokenbooking.deleted = FALSE
JOIN emr_chargeitem
	ON emr_chargeitem.id = emr_tokenbooking.charge_item_id
	AND emr_chargeitem.deleted = FALSE
WHERE emr_chargeitem.status = 'billable'
  AND emr_chargeitem.total_price > 0
  AND emr_encounter.status != 'entered_in_error'
  AND emr_encounter.deleted = FALSE
  ...

(Confirm the exact entered_in_error handling is appropriate for emr_tokenbooking's status enum before applying — worth a quick check against care/emr/resources/scheduling/spec.py if unsure.)

[sonzsara]

There is already filter for encounter status in the query

[github-actions]

That filter is different from what this finding is about. --[[AND {{encounter_status}}]] is a commented-out optional Metabase parameter — it only fires when a dashboard user picks a specific encounter_status value (e.g. 'in-progress'), and the query runs standalone with it disabled by default. It does not default-exclude entered_in_error, and even when supplied it constrains to one status the user chose, not "not entered_in_error." It also only covers emr_encounter — emr_tokenbooking and emr_patientidentifier still have no status/deleted filter at all, so a mistakenly-created token booking or a soft-deleted identifier can still surface here with a real billable charge item attached.

Suggested fix, alongside the existing optional param:

WHERE emr_chargeitem.status = 'billable'
  AND emr_chargeitem.total_price > 0
  AND emr_encounter.status != 'entered_in_error'
  AND emr_tokenbooking.status != 'entered_in_error'
  --[[AND {{date_filter}}]]
  --[[AND {{encounter_class}}]]
  --[[AND {{encounter_status}}]]
  --[[AND emr_patientidentifier.value = {{ssmm_id}}]]

Leaving this open — the optional param alone doesn't cover it (it's opt-in, single-table, and not an entered_in_error exclusion).

Generated by Analytics SQL Reviewer for #147 · auto · 28.5 AIC · ⌖ 10.2 AIC · ⊞ 14.7K

[sonzsara]

commenting out the filters were intentional

[github-actions]

To be clear — I'm not asking you to uncomment --[[AND {{encounter_status}}]] or any of the other optional Metabase params; leaving those commented is the correct repo convention and I'm not flagging it.

The finding is about a different, always-on exclusion that isn't in the query at all: a hardcoded AND emr_encounter.status != 'entered_in_error' (and the equivalent for emr_tokenbooking), which should apply unconditionally regardless of whatever the dashboard user picks for the optional encounter_status filter. Right now nothing excludes entered_in_error rows by default — a mistakenly-created encounter or token booking with a real billable charge item attached will still show up in this report.

If that's an acceptable tradeoff for this report (e.g. entered_in_error rows are rare/negligible at SSMM), that's a reasonable call to make — just flagging so it's a conscious decision rather than an oversight. Leaving this open for your call; happy to resolve if you confirm it's intentional.

Generated by Analytics SQL Reviewer for #147 · auto · 27.4 AIC · ⌖ 3.58 AIC · ⊞ 14.7K

[sonzsara]

yes didnt add that check because the user wanted to filter using the encounter status filter, it was intentional

[github-actions]

Understood — thanks for confirming. If the intent is to let dashboard users scope out entered_in_error via the optional encounter_status filter rather than hardcoding an always-on exclusion, that's a reasonable, conscious tradeoff (and only leaves emr_tokenbooking uncovered as a known limitation). Resolving.

Generated by Analytics SQL Reviewer for #147 · auto · 24.7 AIC · ⌖ 2.38 AIC · ⊞ 14.7K

[github-actions]

[Medium] Missing ### Output section required by TEMPLATE.md.

The template calls for an ### Output table (column/type/description) directly under ## Query, documenting the 8 selected columns for the dashboard consumer. This doc jumps straight from the SQL fence to ## Notes — please add the output table before merging.