Skip to content

feat(invoicing): TAM-6900: inpatient bed fee#10142

Open
tcaiger wants to merge 16 commits into
epic-fsm-invoicingfrom
feature/tam-6900-featinvoicing-inpatient-encounter-fees
Open

feat(invoicing): TAM-6900: inpatient bed fee#10142
tcaiger wants to merge 16 commits into
epic-fsm-invoicingfrom
feature/tam-6900-featinvoicing-inpatient-encounter-fees

Conversation

@tcaiger

@tcaiger tcaiger commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Changes

Implements the inpatient bed fee (TAM-6900). Stacked on #10141 (TAM-6901) → #10137 (TAM-6898) — review/merge those first (base is the 6901 branch). No DB migration — BED_FEE is a category string and Location-as-product reuses InvoiceProduct.sourceRecordId.

  • New BED_FEE invoice category with each Location as a priceable product (InvoiceProduct.sourceLocationRecord); rate via price lists.
  • Facility setting invoicing.bedFee.overnightChargeTime (default 02:00, facility-local).
  • Pure computeBedFeeChargeInstants helper (facility-local night counting, unit-tested) + Invoice.recalculateBedFee — reconciles one bed-fee line per location (quantity = nights, attributed to the location at each overnight check; placeholder wards without a product are skipped; recompute sets quantity and won't resurrect a cashier-removed line).
  • Night 1 charged on admission (encounter create); the nightly BedFeeCharger accrues later nights for all admitted patients. The recompute is idempotent and computes facility-local timing internally, so the job just runs hourly — no per-facility cron needed.

Verified: @tamanu/database + @tamanu/central-server build pass, lint clean, night-counting unit tests (5/5). Open items (see specs/invoicing/inpatient-encounter-fees-plan.md): the PRD worked-example looks off by a day — confirm the night-counting rule (an N-night stay bills N nights); discharge-day finalisation has a narrow edge for non-hour-aligned check times; integration tests (location attribution, batching) not yet run.

Auto-Deploy

  • Deploy
Options
  • Artillery load test
  • Seed from closest snapshot
  • Generate fake data
  • More data (20Gi)
  • No facility servers (central-only)
  • No sync (facility tasks scaled to zero)
  • AMD64 architecture (default is arm64)
  • Skip mobile build
  • Always build mobile
  • Stay up for 8 hours
  • Stay up for 24 hours
  • Stay up (no TTL)
  • Build images only (don't deploy)
  • Pause this deploy

Tests

  • Run E2E tests
  • Run DAST scan

Review Hero

  • Run Review Hero
  • Auto-fix review suggestions Wait for Review Hero to finish, resolve any comments you disagree with or want to fix manually, then check this to auto-fix the rest.
  • Auto-fix CI failures Check this to auto-fix lint errors, test failures, and other CI issues.
  • Auto-merge upstream Check this to merge the base branch into this PR, with AI conflict resolution if needed.
  • Save suppressions Check this to capture 👎 reactions on Review Hero comments as suppression rules in .github/review-hero/suppressions.yml. Also runs automatically at the end of any auto-fix run.

Remember to...

  • ...write or update tests
  • ...add UI screenshots and testing notes to the Linear issue
  • ...add any manual upgrade steps to the Linear issue
  • ...update the config reference, settings reference, or any relevant runbook(s)
  • ...call out additions or changes to config files for the deployment team to take note of

@tcaiger tcaiger requested a review from a team as a code owner June 24, 2026 23:46

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 9188a62. Configure here.

Comment thread packages/central-server/app/tasks/BedFeeCharger.js
Comment thread packages/database/src/models/Invoice/Invoice.ts
Comment thread packages/central-server/app/tasks/BedFeeCharger.js
Comment thread packages/database/src/models/Invoice/Invoice.ts
Comment thread packages/database/src/models/Invoice/Invoice.ts Outdated
Comment thread packages/database/src/models/Invoice/Invoice.ts
Comment thread packages/database/src/models/Invoice/Invoice.ts
Comment thread packages/database/src/models/Invoice/Invoice.ts
@review-hero

review-hero Bot commented Jun 24, 2026

Copy link
Copy Markdown

🦸 Review Hero Summary
15 agents reviewed this PR | 2 critical | 3 suggestions | 1 nitpick | Filtering: consensus 3 voters, 8 below threshold

Below consensus threshold (8 unique issues not confirmed by majority)
Location Agent Severity Comment
packages/central-server/app/tasks/BedFeeCharger.js:43 Performance suggestion The Encounter.count on line 43 and the subsequent Encounter.findAll with offset pagination on line 60 hit the same table twice on every run. The count is only used to compute batchCount and l...
packages/central-server/app/tasks/BedFeeCharger.js:54 Bugs & Correctness suggestion Offset pagination over live endDate IS NULL data is not stable. An encounter discharged (endDate set) between batch iterations removes a row from the result set, shifting all subsequent rows by o...
packages/central-server/app/tasks/BedFeeCharger.js:60 Performance suggestion Offset-based pagination becomes increasingly expensive as the offset grows: the database must scan and discard all preceding rows to reach each batch. For 10 000 admitted patients with batchSize=10...
packages/database/src/models/Invoice/Invoice.ts:410 BES Requirements nitpick encounter.endDate || getCurrentDateTimeString() uses || instead of ??. The project prefers ?? over ||.
packages/database/src/models/Invoice/Invoice.ts:411 Bugs & Correctness suggestion (await settings.get('invoicing.bedFee.overnightChargeTime')) as string silences the type but doesn't add a runtime guard. If the setting is missing or the ReadSettings instance hasn't been seeded...
packages/database/src/models/Invoice/Invoice.ts:434 Performance suggestion Sequential item.destroy() calls inside a loop: each triggers an individual DELETE (plus audit trigger). Collect the ids of items to remove and issue a single `InvoiceItem.destroy({ where: { id: {...
packages/database/src/models/Invoice/Invoice.ts:480 BES Requirements suggestion item.destroy() on a paranoid model soft-deletes the record, placing it in the same state as a cashier-manually-removed line. Later, the existing?.deletedAt guard (line ~472) skips resurrection ...
packages/utils/src/invoice/bedFee.ts:47 Bugs & Correctness suggestion overnightChargeTime.split(':').map(Number) with default destructuring ([hour = 0, minute = 0]) silently falls back to midnight (00:00) if the time string is malformed (e.g. '2:00' without zer...

Nitpicks

File Line Agent Comment
packages/utils/src/invoice/bedFee.ts 35 BES Requirements facilityTimeZone || primaryTimeZone uses || where the project prefers ?? (see important-project-rules.md). facilityTimeZone is typed string | null | undefined, so ?? is correct here — it falls back only on null/undefined, not on an empty string. Same issue on line 55: `primaryTimeZone...
Local fix prompt (copy to your coding agent)

Fix these issues identified on the pull request. One commit per issue fixed.


packages/database/src/models/Invoice/Invoice.ts:392: recalculateBedFee performs multiple reads and writes (findAll, destroy, findOne, update/create) without a transaction. Concurrent runs — e.g. the hourly BedFeeCharger and an admission trigger firing simultaneously for the same encounter — can race, producing duplicate invoice items or leaving items in an inconsistent state. Wrap the entire body after the early-return guards in await this.sequelize.transaction(async () => { ... }) per the project's managed-transaction pattern (no need to pass the transaction object thanks to CLS).


packages/database/src/models/Invoice/Invoice.ts:468: orderDate: new Date() passes a JavaScript Date object to a column declared as a string (dateType). The project convention is to store datetimes as ISO 9075 strings via getCurrentDateTimeString() (already imported on line 27). Use orderDate: getCurrentDateTimeString() instead.


packages/utils/src/invoice/bedFee.ts:35: facilityTimeZone || primaryTimeZone uses || where the project prefers ?? (see important-project-rules.md). facilityTimeZone is typed string | null | undefined, so ?? is correct here — it falls back only on null/undefined, not on an empty string. Same issue on line 55: primaryTimeZone || displayTimeZone should be primaryTimeZone ?? displayTimeZone.


packages/database/src/models/Invoice/Invoice.ts:418: N+1 query pattern in the per-instant loop: locationIdAtInstant issues one EncounterHistory.findOne per charge instant (one per night). For a 90-day admission that's 90 separate DB queries just for location lookups, and this runs hourly for every admitted patient in the system. Fix: fetch all EncounterHistory rows for the encounter in a single query (WHERE encounterId = ? AND date <= endDateTime ORDER BY date ASC), then resolve each instant's location in-memory by binary-searching or walking the sorted result set.


packages/database/src/models/Invoice/Invoice.ts:441: N+1 query pattern: InvoiceProduct.findOne is called once per distinct location in nightsByLocation. For an encounter that moved through many locations this multiplies round-trips. Fix: replace with a single InvoiceProduct.findAll({ where: { category: BED_FEE, sourceRecordId: { [Op.in]: [...nightsByLocation.keys()] } } }) and build a Map keyed by sourceRecordId before the loop.


packages/database/src/models/Invoice/Invoice.ts:448: N+1 query pattern: InvoiceItem.findOne is issued once per location. Combined with the InvoiceProduct.findOne loop above, each location causes 2 sequential round-trips. Fix: batch-load all existing BED_FEE InvoiceItems for the invoice in a single query (the existingItems fetch on line 431 already does this but only for the destroy pass — reuse it, keyed by sourceRecordId, for the upsert pass too).

@tcaiger tcaiger force-pushed the feature/tam-6901-featinvoicing-inpatient-fee-inclusionsexclusions branch from 708c381 to 4eb05b7 Compare June 25, 2026 02:43
@tcaiger tcaiger force-pushed the feature/tam-6900-featinvoicing-inpatient-encounter-fees branch from 5627937 to 8dbaea4 Compare June 25, 2026 02:43
@tcaiger tcaiger force-pushed the feature/tam-6901-featinvoicing-inpatient-fee-inclusionsexclusions branch from 4eb05b7 to c4a1071 Compare June 25, 2026 03:46
@tcaiger tcaiger force-pushed the feature/tam-6900-featinvoicing-inpatient-encounter-fees branch 3 times, most recently from fff379e to 199e6e8 Compare June 25, 2026 05:04
@tcaiger tcaiger force-pushed the feature/tam-6901-featinvoicing-inpatient-fee-inclusionsexclusions branch from 5426da1 to 9cbd524 Compare June 25, 2026 05:17
@tcaiger tcaiger force-pushed the feature/tam-6900-featinvoicing-inpatient-encounter-fees branch from 199e6e8 to 70633c3 Compare June 25, 2026 05:17
tcaiger and others added 3 commits June 26, 2026 17:07
…usions

Add a facility setting (invoicing.inpatientFee.bundledCategories) and an
isInpatientFeeBundled helper, then gate the clinical-item auto-add paths: for
admission encounters at a facility that bundles a category, lab/imaging items
don't auto-add and the administered (MAR) medication portion is excluded.
Discharge meds always bill; outpatient/ER and procedures are unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ons (lab)

Lab items don't auto-add for an admission encounter when the facility bundles lab into
the admission fee, but still auto-add for a non-admission (clinic) encounter.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion split

Add a medication case to the inclusions suite: for an admission encounter at a facility
that bundles medications, the administered (MAR) portion is excluded from the invoice while
discharge dispensing is still billed (quantity = dispensed only).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tcaiger tcaiger force-pushed the feature/tam-6901-featinvoicing-inpatient-fee-inclusionsexclusions branch from 9cbd524 to 47b62ad Compare June 26, 2026 05:10
@tcaiger tcaiger force-pushed the feature/tam-6900-featinvoicing-inpatient-encounter-fees branch from 70633c3 to 4f4ad90 Compare June 26, 2026 05:12
tcaiger and others added 8 commits June 26, 2026 17:15
…elper

- Type the category arg as the InpatientBundledCategory union (was string), so a
  typo at a call site fails at compile time instead of silently returning false.
- Skip the bundling lookup when there's no MAR quantity to exclude (marQty === 0),
  avoiding a wasted DB round-trip on pure discharge dispenses.
- Reduce the helper's comment to one line.

Kept the encounterId-based fetch (rather than passing the encounter object) because
one call site passes an association-loaded encounter whose locationId isn't
guaranteed; the self-sufficient re-fetch keeps bundling correct there.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the BED_FEE invoice category with each Location as a priceable product
(InvoiceProduct.sourceLocationRecord), the facility overnight-charge-time setting,
a pure computeBedFeeChargeInstants helper (facility-local night counting, unit-tested),
and Invoice.recalculateBedFee which reconciles one bed-fee line per location
(quantity = nights, location at each overnight check, soft-delete-aware). Wiring to
the admission trigger and nightly job follows.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the BedFeeCharger scheduled task (hourly; recomputes bed fees for all currently-
admitted patients, relying on the idempotent per-facility-local recompute) plus its
config, and charge the admission night immediately when an admission encounter is created.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ischarge-day bed fee

Decision A: a same-day overnight check (early-hours admission) counts — charge the night
admitted in (pinned by a unit test). And the BedFeeCharger now also recomputes recently-
discharged admission encounters (endDate within ~25h), so the final discharge-day night
lands even for off-hour check times and death discharges.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drive Invoice.recalculateBedFee against the DB: one night for a same-day admission, one
night per overnight check for a multi-night stay, and no charge for a location without a
bed-fee product (placeholder ward).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… a ward move

Add a case where the patient moves wards mid-stay: each overnight check is attributed to
the location occupied at that time, so the invoice carries one line per location with the
right night counts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n bed-fee helper

Mirror the encounter-fee selector cleanup (Review Hero nit) in computeBedFeeChargeInstants:
use ?? for the timezone fallback and drop the dead primaryTimeZone ternary.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tcaiger tcaiger force-pushed the feature/tam-6900-featinvoicing-inpatient-encounter-fees branch from 4f4ad90 to c6a386a Compare June 26, 2026 05:22
Base automatically changed from feature/tam-6901-featinvoicing-inpatient-fee-inclusionsexclusions to epic-fsm-invoicing June 26, 2026 05:24
tcaiger and others added 3 commits June 26, 2026 17:26
…ne string

orderDate is a dateType (string) column storing ISO 9075 in the primary
timezone; passing a JS Date serialised to UTC ISO-8601. Use
getCurrentDateTimeString() to match the datetime storage convention.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
recalculateBedFee issued one EncounterHistory query per charge instant (a query
per night, hourly, per admitted patient). Load the encounter's location history
once and walk the sorted rows in memory instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tcaiger and others added 2 commits June 26, 2026 17:41
recalculateBedFee ran only on admission creation and in the hourly job for
still-admitted/recently-discharged encounters. With no discharge-time recompute,
nights accrued between the last hourly run and discharge were absent from the
invoice until the next run — and lost permanently if the invoice was finalised
in that window (recalc only touches in-progress invoices). Recompute on the
encounter update path when endDate or locationId changes.

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.

1 participant