Skip to content

Index billing records in the QueryStore for chart search (optional 2.9 integration) - #186

Open
dkayiwa wants to merge 9 commits into
mainfrom
querystore-integration
Open

Index billing records in the QueryStore for chart search (optional 2.9 integration)#186
dkayiwa wants to merge 9 commits into
mainfrom
querystore-integration

Conversation

@dkayiwa

@dkayiwa dkayiwa commented Jul 4, 2026

Copy link
Copy Markdown
Member

What

Lets the billing module contribute its patient records to the OpenMRS QueryStore so a clinician-facing semantic / keyword chart search (e.g. chartsearchai) can retrieve them. Ships as a new, optional billing-querystore omod.

📐 The architectural decisions behind this PR are recorded in docs/adr.md.

The key realization

The QueryStore integration path is "register a serializer," not "publish events." QueryStore's steady‑state sync (CoreServiceEventListener) consumes the *ServiceEvents that core openmrs-core#6084 (TRUNK‑6429, merged to 2.9.x) publishes for every OpenmrsService method named save* / void* / unvoid* / retire* / unretire* / purge* whose first arg is an OpenmrsObject, and projects any entity for which a ClinicalRecordSerializer is registered.

That covers billing_bill (its BillService is an OpenmrsService). BillDiscountService / BillRefundService are plain interfaces, so #6084's service-event advice never fires for them — billing_discount / billing_refund live-sync instead through a small BillChildDbEventListener in this submodule that consumes core's other, non-AOP half: the Hibernate EventInterceptor's SaveDbEvent / DeleteDbEvent, which fire for every persisted entity. No base-billing change; all three types were verified live on a 2.9 standalone (see verification comments).

Billing's services already fit that shape — saveBill / voidBill / unvoidBill / purgeBill, saveBillDiscount, saveBillRefund — and are wired the standard OpenMRS way (TransactionProxyFactoryBean + core serviceInterceptors). So live indexing comes for free once a serializer is registered; the base billing services are not touched.

What gets indexed — the "useful to a clinician reading the chart" lens

Three patient‑scoped resource types (all BaseOpenmrsData, each with its own save* events):

Resource type Why a clinician cares
billing_bill The anchor. Its text folds in the billed services / drugs / tests (line items), the raw total, amount paid, status and cash point — i.e. what care the patient was charged for. (The discount-adjusted balance is intentionally not folded in — see Hardening below.)
billing_discount Fee waivers / discounts — often a marker of a subsidized programme (HIV, TB, under‑5, indigent) or financial hardship.
billing_refund Refunds — weakest clinical signal, but completes the record and is nearly free to add.

Folded into billing_bill (they cascade with the bill and have no independent save events): line items, payments. Excluded (not patient‑scoped or pure config, and QueryStore only projects OpenmrsData anyway): timesheets, cash points, payment modes, billable‑service catalog, item prices, exemption rules, sequence models.

How it's built

  • New querystore/ submodule → per type a ClinicalRecordSerializer (AbstractRecordSerializer subclass), a HibernateTypeBootstrapper (backfill), and a ResourceTypeProvider (discovered by QueryStore via Context.getRegisteredComponents).
  • Discount/refund bootstrappers scope through the parent bill (e.bill.patient.uuid) and cursor on e.dateCreated (they're JPA @Entitys that don't map dateChanged).
  • Backfill of pre‑existing records is admin‑triggered (POST /ws/rest/v1/querystore/reindex {"scope":"all"} or BootstrapService), not run on startup.

Why it's a separate, optional omod

QueryStore + the #6084 event API require the (currently unreleased) platform 2.9 plus an Elasticsearch/Lucene/ONNX backend. Baking that into the base module would force every billing deployment onto 2.9 + QueryStore. Instead this is a standalone billing-querystore omod, built only behind a Maven profile:

mvn -Pquerystore clean install
# → querystore/target/billing-querystore-*.omod  (install alongside billing only where QueryStore runs)

The default build, pom.xml <modules>, and the base billing.omod are unchanged and still target 2.7.8. The only edits outside querystore/ are the profile in the root pom.xml and a README section.

Verification

  • ✅ Compiles against querystore-api:1.0.0-SNAPSHOT + openmrs-api:2.9.0-SNAPSHOT.
  • ✅ 8 serializer unit tests pass (bill with items/payments/status, voided‑item skipping, discount & refund patient‑scoping via the bill, patient‑less skip, discount‑independent raw total, percentage discount, empty‑line‑items).
  • billing-querystore-*.omod packages.
  • Verified live on a 2.9 standalonefull results in this comment. A bill created via saveBill with no manual reindex is projected by core #6084 into a billing_bill document and returned by a chartsearchai chart search within ~25s; the bootstrapper backfill path works too. So the #6084-for-free path does hold for this module's service (the isAopProxy / parent-child-context concern doesn't block it), and the fallback publisher aspect is not needed.

Hardening (second commit)

An iterative review/polish pass tightened the slice by tracing its integration boundaries:

  • Discount‑balance staleness (correctness). BillDiscountService.saveBillDiscount persists the discount alone and does not re‑save the parent bill, so a billing_bill document that folded in an "amount after discount" / outstanding balance would silently overstate what the patient owes until the bill's next save. billing_bill now exposes only bill‑aggregate‑derived fields that are refreshed on every saveBill (raw total, amount paid, status); discount detail lives on billing_discount. (saveBillRefund, by contrast, does reconcile + re‑save the bill, so refunds stay fresh.)
  • Classloader (integration). The bill serializer references StockItem/Drug, so stockmanagement was added to require_modules — transitive visibility through billing isn't guaranteed.
  • Live/backfill parity. Patient‑less records are now skipped on the live path too (they were already filtered by the backfill's patient IS NOT NULL scan). Unreachable for persisted entities (NOT‑NULL FKs), but keeps the two paths symmetric.
  • Metadata clarity. discount_value (which conflated a percentage with a currency amount under one key) became discount_percent + an always‑currency discount_amount; line_item_countbilled_service_count (it counts billed‑service labels, not raw line items).
  • Polish. Extracted AbstractBillChildRecordSerializer for the discount/refund shared logic, centralized metadata keys in BillingQueryFields, and reused the SPI's trimToNull / QueryStoreConstants.FIELD_VISIT_UUID instead of re‑implementing them.

Verified as non‑issues along the way: the bootstrapper HQL is valid for the JPA @Entity children (BaseOpenmrsData is a @MappedSuperclass), and discount/refund status updates re‑index correctly even without a dateChanged bump because querystore's upsert freshness guard is >=.

Follow‑ups

  • Verified live end-to-end on a 2.9 + QueryStore (Lucene) + chartsearchai standalone (see verification comments): all three types live-sync and backfill — billing_bill via core #6084 SaveServiceEvent, and billing_discount / billing_refund via the SaveDbEvent listener. Elasticsearch backend not exercised (Lucene is what's deployed; the serializers are backend-agnostic).
  • Optionally tune billing_bill text once we see real chart‑search queries.

🤖 Generated with Claude Code

Contributes bills, discounts and refunds to the OpenMRS QueryStore via its
ResourceTypeProvider SPI so a clinician-facing semantic/keyword chart search
(e.g. chartsearchai) can retrieve them. Three patient-scoped resource types are
added: billing_bill (folding in line items, totals, payment status and balance),
billing_discount and billing_refund.

Steady-state indexing rides on core #6084's *ServiceEvent advice, which already
fires for billing's save*/void*/purge* service methods, so no change is needed to
the base billing services. Backfill uses HibernateTypeBootstrapper subclasses.

Because QueryStore and the #6084 event API require the (unreleased) platform 2.9
plus an Elasticsearch/Lucene backend, this ships as a separate, optional
billing-querystore omod built behind the 'querystore' Maven profile. The default
build and the base billing.omod are unchanged and still target 2.7.8.

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

codecov-commenter commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 34.45%. Comparing base (f4d8151) to head (1c5a7b3).

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #186   +/-   ##
=========================================
  Coverage     34.45%   34.45%           
  Complexity      809      809           
=========================================
  Files           213      213           
  Lines          5274     5274           
  Branches        646      646           
=========================================
  Hits           1817     1817           
  Misses         3252     3252           
  Partials        205      205           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dkayiwa and others added 3 commits July 5, 2026 01:20
Structural (Phase 1):
- billing_bill no longer folds the discount-adjusted total/balance. Approving a
  fee waiver goes through saveBillDiscount, which does not re-save the bill, so a
  denormalized after-discount figure would silently go stale; expose the raw
  line-item total + amount_paid + status (all refreshed on every saveBill) instead,
  and leave discount detail to billing_discount.
- Add stockmanagement to require_modules: the bill serializer references
  StockItem/Drug, so it must be visible to this module's classloader.

Polish (Phase 2):
- Extract AbstractBillChildRecordSerializer for the discount/refund serializers
  (shared patient-scope/uuid/date/bill-reference logic).
- Add BillingQueryFields as the single source of truth for metadata key names.
- Reuse AbstractRecordSerializer.trimToNull and QueryStoreConstants.FIELD_VISIT_UUID
  instead of re-implementing them; simplify getDate.
- Split the unit-ambiguous discount_value into discount_percent (percentage only)
  plus an always-currency discount_amount; rename line_item_count to
  billed_service_count.
- Skip patient-less records so the live path matches the backfill's patient filter.

Tests: 8 serializer cases (added percentage-discount, empty-line-items,
discount-independent total, patient-less skip). Build + package green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- S6213 (restricted identifier): rename the 'record' parameter to 'entity' in
  AbstractBillChildRecordSerializer ('record' is a restricted identifier since Java 14).
- S135 (multiple continue): billedItemLabels now uses a single continue, nesting the
  label handling instead.

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

Found deploying to a live 2.9.0-SNAPSHOT standalone: billingquerystore failed to
start with 'requires querystore 1.0, billing 2.4'. OpenMRS ranks 'X.Y.Z-SNAPSHOT'
below a bare 'X.Y.Z', so the bare require_version 2.9.0 and require_module 1.0/2.4
were left unsatisfied by the running -SNAPSHOT builds. Filter the requirements from
this module's properties (openmrsPlatformVersion / project.parent.version /
querystoreVersion) so they match the exact versions built against, mirroring how
chartsearchai requires querystore (1.0.0-SNAPSHOT).

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

dkayiwa commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

✅ Verified end-to-end on a live OpenMRS 2.9 standalone

Deployed and tested against a referenceapplication-standalone-3.7.0-rc.2 instance — platform 2.9.0-SNAPSHOT, with querystore 1.0.0-SNAPSHOT, chartsearchai 1.0.0-SNAPSHOT, stockmanagement 3.0.0, event 4.0.0, fhir2 4.0.0 installed. querystore backend = Lucene.

Deploy. Built billing-2.4.0-SNAPSHOT.omod + billing-querystore-2.4.0-SNAPSHOT.omod (mvn -Pquerystore clean package), replaced the bundled billing-2.3.0, restarted. All four modules start:

billing            2.4.0-SNAPSHOT   started=True
billingquerystore  2.4.0-SNAPSHOT   started=True
querystore         1.0.0-SNAPSHOT   started=True
chartsearchai      1.0.0-SNAPSHOT   started=True

Fix found during deploy (68baa62): billingquerystore first failed with requires querystore 1.0, billing 2.4. OpenMRS ranks X.Y.Z-SNAPSHOT below a bare X.Y.Z, so require_version 2.9.0 / require_module 1.0/2.4 were unsatisfied by the running -SNAPSHOT builds. Requirements now filter to the exact versions built against (matching chartsearchai's require querystore 1.0.0-SNAPSHOT).

1. Live projection via core #6084 — the PR's central assumption ✅

Created a bill through the normal service path and did not reindex:

POST /ws/rest/v1/billing/bill   → Joshua Johnson, receipt 0002-6, Orthopedic Service, 750, PENDING

25 s later, a clinician chart search returned it:

POST /ws/rest/v1/chartsearchai/search  {"question":"What has this patient been billed for?","patient":"<uuid>"}
{ "answer": "The patient has been billed for Orthopedic Service [1].",
  "references": [ { "resourceType": "billing_bill" } ] }

So saveBill emits a core #6084 SaveServiceEvent<Bill> that querystore's CoreServiceEventListener projects live into a billing_bill document — with no billing-side event publishing, exactly as the SPI intends. The isAopProxy / parent-child-context concern flagged in the description does not block it for this module's service.

2. Backfill via the bootstrapper ✅

POST /ws/rest/v1/querystore/reindex  {"patient":"<Betty uuid>"}   → {"documentsIndexed":228}
POST /ws/rest/v1/chartsearchai/search {"question":"What has this patient been billed for, and what is the outstanding balance?","patient":"<Betty uuid>"}
{ "answer": "The patient has been billed for Antenatal care [1]. The outstanding balance is 500.00 [1].",
  "references": [ { "resourceType": "billing_bill", "date": "2026-07-04" } ] }

BillBootstrapper + BillRecordSerializer project a pre-existing bill, and the LLM correctly derived the 500.00 outstanding from the emitted Total: 500, paid: 0 — validating the hardening decision to expose raw total + amount paid rather than a denormalized (staleable) balance.

Verdict

A clinician's chart search retrieves a patient's bills as billing_bill documents — both live-on-save (#6084) and via backfill — on a real 2.9 + querystore + chartsearchai deployment.

Not covered: billing_discount / billing_refund were not separately exercised (no discount/refund created in this run); querystore's Elasticsearch backend was not tested (this instance uses Lucene). The instance's Debezium CDC pipeline is misconfigured (binlog_format ≠ ROW), but that path is irrelevant here — #6084 is in-process and worked regardless.

* waiver is patient-relevant because it often signals a subsidized programme (HIV, TB, under-5,
* indigent) or financial hardship. It is indexed as its own type - rather than folded into the bill
* - because its lifecycle (PENDING -> APPROVED/REJECTED) is driven through its own
* {@code saveBillDiscount} service call, so its own save events keep the projection current without

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the load-bearing assumption for both the discount and refund types, and I don't think it holds. saveBillDiscount / saveBillRefund never emit a *ServiceEvent, so nothing projects billing_discount / billing_refund on save.

Core #6084's advice (OpenmrsServiceEventAdvice) only intercepts save*/void*/purge* when the target is an OpenmrsService:

@Around("(execution(* *.save*(..)) || execution(* *.create*(..))) && target(org.openmrs.api.OpenmrsService)")

BillServiceImpl extends BaseOpenmrsService implements BillService (and BillService extends OpenmrsService), which is exactly why saveBill fired and billing_bill indexed live in your run. But BillDiscountServiceImpl / BillRefundServiceImpl only implements BillDiscountService / BillRefundService, and neither interface extends OpenmrsService, so the pointcut never matches their save* methods. There is no billing-side publisher either, and the cascade doesn't rescue it: SerializerRegistry.resolve keys on entity type, so the SaveServiceEvent<Bill> that saveBillRefund's internal saveBill re-save produces only ever resolves to the bill serializer, never the refund one.

The effect is silent (no exception): a discount or refund saved through the service is never projected live, it only appears after an admin reindex. Status is worse: a discount approved (PENDING to APPROVED), or a refund completed, after the initial backfill keeps its stale status in the index until the next manual reindex, since the "live sync handles later status changes" assumption in BillDiscountBootstrapper never fires. That is also why these two were the paths your live verification didn't cover. billing_bill is genuinely fine.

So this blocks the discount/refund half of the feature as documented (this javadoc, the refund serializer's, the bootstrapper's, and the README all assert live indexing works). Two options, I'd pick the first:

  1. Put these two services on the same path BillService already uses: BillDiscountService / BillRefundService extends OpenmrsService, with the impls extends BaseOpenmrsService. Identical wiring, so the isAopProxy guard behaves just like saveBill; it's harmless on platforms without the advice or without querystore installed. It does mean a small base-module change, so the "only edits are the profile + README" framing stops being true.
  2. Keep the base module untouched and add the small publisher aspect the README mentions, scoped to these two services, here in the submodule.

Quick way to confirm, same shape as your bill check: save a discount without reindexing and search for it. It should be missing (and after approval its status should lag) until this is addressed.

Deploying to a live 2.9 standalone showed billing_discount/billing_refund never
live-projected (only backfill did): BillDiscountService/BillRefundService are plain
interfaces, not OpenmrsServices, so core #6084's service-event AOP advice
(target(OpenmrsService)) never fires for saveBillDiscount/saveBillRefund. billing_bill
was unaffected because BillService extends OpenmrsService.

Rather than change those base-billing services, consume core's non-AOP path: the
Hibernate EventInterceptor publishes a SaveDbEvent/DeleteDbEvent for every entity
change, in-transaction on the flush thread. BillChildDbEventListener listens for those,
filters to BillDiscount/BillRefund (Bill still syncs via its SaveServiceEvent), and drives
the same querystore projection pipeline (RecordProjector + the reachable querystore.sync.*
beans) the service consumer uses. No base-billing change.

Verified live on a 2.9 standalone: a newly created discount (50) and refund (75) - with
no reindex - are returned by a chartsearchai chart search within ~25s as billing_discount
/ billing_refund. Serializer javadocs corrected to describe this mechanism.

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

dkayiwa commented Jul 5, 2026

Copy link
Copy Markdown
Member Author

Follow-up: billing_discount / billing_refund live-sync — a gap found and fixed

Exercising discounts/refunds on the 2.9 standalone surfaced a real bug and produced a fix (commit a4c6bc9).

What I found. billing_bill live-syncs because BillService extends OpenmrsService, so core #6084's AOP advice (… && target(org.openmrs.api.OpenmrsService)) fires for saveBill. But BillDiscountService / BillRefundService are plain interfaces — so #6084's service-event advice never fires for saveBillDiscount / saveBillRefund, and those two types reached the store only via backfill/reindex, not live. That contradicted the original claim (and serializer javadocs) that they live-sync "via their own save events." Confirmed empirically: before reindex the discount/refund queries returned only billing_bill; after reindexPatient they returned billing_discount / billing_refund.

The fix — use core's non-AOP event path, no base-billing change. openmrs-core #6084 has two halves: the AOP OpenmrsServiceEventAdvice (*ServiceEvent, OpenmrsService-only) and the Hibernate EventInterceptor, which publishes SaveDbEvent / DeleteDbEvent for every persisted entity, in-transaction on the flush thread (session open). Added BillChildDbEventListener in the submodule: it consumes those DB events, filters to BillDiscount / BillRefund, and drives the same querystore projection pipeline the service consumer uses (RecordProjector + the reachable querystore.sync.* beans — exactly the "provider contributes its own event listener" fallback the SPI documents). Bill is deliberately ignored there (it already live-syncs via its SaveServiceEvent).

Verified live — fresh patient, brand-new discount + refund, no reindex:

POST /billing/billDiscount  → FIXED_AMOUNT 50
UPDATE cashier_bill … PAID  (refund eligibility; the paid-bill-with-payment REST path hits a
                             core RequiredDataAdvice reflection bug on Payment's generic type)
POST /billing/billRefund    → 75, "Duplicate charge"
# ~25s later, no reindex:
POST /ws/rest/v1/chartsearchai/search {"question":"…discount…","patient":"…"}
  → "a bill discount of 50 … and a bill refund of 75"   refs [billing_discount, billing_refund]
POST /ws/rest/v1/chartsearchai/search {"question":"…refund…","patient":"…"}
  → "a bill refund was requested for bill 0004-2 in the amount of 75. The reason is Duplicate charge"
    refs [billing_bill, billing_refund]

So all three types now live-sync: billing_bill via SaveServiceEvent (AOP), billing_discount / billing_refund via SaveDbEvent (Hibernate). Backfill (reindex) also verified for all three. Serializer javadocs corrected to describe the real mechanism.

Elasticsearch gap: closed as not-applicable to this module — verified on the deployed Lucene backend, and the serializers are backend-agnostic (structured fields go into querystore's metadata_json blob, not per-field ES mappings), with the cross-backend freshness guard (>= / external_gte) already code-verified. Spinning up ES would exercise querystore's infra, not this integration.

dkayiwa and others added 4 commits July 6, 2026 22:14
Records, in docs/adr.md (mirroring querystore's ADR format), the six decisions behind
the billing-querystore integration: SPI-based integration, the optional profile-gated
omod, indexed-type scope, billing_bill folding only bill-fresh fields, the two live-sync
paths (AOP service events for bills, non-AOP Hibernate DB events for discounts/refunds),
and the exact-SNAPSHOT version requirements. Links it from the README.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Decision 3 recorded the selection criteria (clinician-chart lens, patient-scoped,
OpenmrsData-only) but the per-resource clinical justification lived only in the serializer
javadocs. Pull it into the ADR: the two hard filters + a marginal-value note; a per-type
'what it tells a clinician' for bill/discount/refund; and grouped, reasoned exclusions
(not patient-scoped / facility config / policy-not-record / metadata).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
List natural-language questions a clinician can ask chart search for each of billing_bill,
billing_discount, and billing_refund, marking the ones actually exercised during verification,
and noting that retrieval is semantic (phrasing varies) and one question may cite multiple types.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 1 (structural):
- BillChildDbEventListener: move the querystore bean lookups (registry/indexer/dispatcher)
  inside the try, so an infra lookup failure is swallowed instead of propagating out of the
  SaveDbEvent handler and rolling back the clinical saveBillDiscount/saveBillRefund transaction
  (honoring the best-effort contract the class documents).
- Correct BillingQuerystoreActivator javadoc: it claimed steady-state indexing rides only on
  #6084 *ServiceEvents and omitted the listener — now describes both paths (Bill via
  SaveServiceEvent, discount/refund via the SaveDbEvent listener) and lists the bean.
- Document that a PERCENTAGE billing_discount's discount_amount is a projection-time snapshot
  that can lag a later bill-total change until the discount is next saved (discount_percent is
  always current).

Phase 2 (polish, from 4-agent review):
- Reword the listener javadoc: drop the unsubstantiated claim that a generic-typed @eventlistener
  would miss Hibernate proxies (it wouldn't); keep raw SaveDbEvent<?> + instanceof, justified by
  mirroring querystore's own consumer and proxy-robustness.
- Document the flush-thread constraint on AbstractBillChildRecordSerializer: populate() may run on
  the Hibernate flush thread, so it must navigate only id-loadable to-one proxies (no queries /
  lazy collections) to avoid a flush-inside-flush.
- Source the stockmanagement require version from ${stockmanagementVersion} (matching the sibling
  requires and the base billing module), rather than a hardcoded 1.4.

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

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

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.

2 participants