Widen AbstractIndexingAdvice type bound to admit BaseOpenmrsMetadata

[dkayiwa]

Problem

AbstractIndexingAdvice<T extends BaseOpenmrsData> (querystore-api org.openmrs.module.querystore.bridge.AbstractIndexingAdvice) constrains the generic type parameter to BaseOpenmrsData. This rules out indexing any entity that extends BaseOpenmrsMetadata or BaseChangeableOpenmrsMetadata — i.e., catalog and reference data.

In practice this shuts out a broad class of legitimate search targets:

• BillableService (the billing module's service catalog — concrete pain point we hit; we had to drop catalog indexing entirely)
• PaymentMode, CashPoint, BillableServicePrice
• Drug, OrderType, LocationTag, and presumably similar catalog types in other modules

The serializer SPI (AbstractRecordSerializer<T>) has no such bound. Only the advice does, and only because it relies on BaseOpenmrsData.getVoided() to route voided entities through the delete path (the per-node voided policy).

What we tried

In the billing module's querystore slice we built a full BillableServiceRecordSerializer + BillableServiceIndexingAdvice + BillableServiceResourceTypeProvider, then deleted them when the type bound surfaced at compile time. Catalog search ("find every Lab-department service") remains unaddressed and there's no workaround within the existing SPI.

Suggested approach

Two reasonable shapes:

1. Split the abstract. AbstractDataIndexingAdvice<T extends BaseOpenmrsData> for the current behavior, plus AbstractMetadataIndexingAdvice<T extends BaseOpenmrsMetadata> that routes retired entities through the delete path instead of voided. Slight code duplication, clean type system.

2. Widen to OpenmrsObject. Drop the bound, replace entity.getVoided() with a strategy hook (protected boolean isInactive(T entity)) that data subclasses implement as getVoided() and metadata subclasses implement as getRetired(). Less duplication, slightly more contract for subclasses.

Either way, the document writes / SPI surface / consumer queries don't change — only the advice's type constraint.

Failure mode prevented

Without this, every module with catalog search needs (billing, stockmanagement, FHIR, formentry…) hand-rolls its own index outside the querystore, fragmenting the search surface that querystore exists to centralize.

Surfaced during a billing module slice that adds Bill / BillRefund / BillDiscount / Timesheet to the querystore. See openmrs/openmrs-module-billing#176 (the ADR) for the full context.

[dkayiwa]

Some additional BaseOpenmrsMetadata / BaseChangeableOpenmrsMetadata types where indexing would have real search value, grouped by likely payoff:

High-value clinical catalogs

• Drug (already cited) — formulary search ("find amoxicillin 500mg suspension")
• Form — "which intake form covers postpartum follow-up?"
• Location — facility/ward/clinic search, especially with LocationTag and LocationAttributes
• Provider — find clinicians by name/role/specialty; pairs with ProviderAttributeType
• Program / ProgramWorkflow / ProgramWorkflowState — care-program catalog (HIV, TB, MCH)
• EncounterType — "what kinds of visits exist?"
• VisitType
• OrderType (already cited) + OrderFrequency, CareSetting

Person/identity metadata

• PatientIdentifierType, PersonAttributeType, RelationshipType — small sets but clinically referenced and locale-translated, so worth normalizing into the same surface

Module-contributed catalogs (the real motivator)

• Item, Stockroom, Party, StockOperationType (stockmanagement) — direct parallel to billing's BillableService case
• AppointmentType, AppointmentServiceDefinition (appointments / bahmni-appointments)
• HtmlForm (htmlformentry) — form-template search
• ReportDefinition, CohortDefinition (reporting) — large free-text descriptions, very embedding-friendly
• IdentifierSource (idgen)
• AddressHierarchyEntry / AddressHierarchyLevel — sheer volume + name-search demand

Probably not worth indexing
Small, locale-invariant lookup tables: ConceptClass, ConceptDatatype, ConceptSource, FieldType, Privilege, Role, EncounterRole. They're enum-ish — better fetched from core directly than denormalized.

One wrinkle worth calling out: Concept is the highest-value clinical catalog in OpenMRS, but it extends BaseOpenmrsObject directly (with its own retired flag), not BaseOpenmrsMetadata. So widening to BaseOpenmrsMetadata still leaves Concept outside — which argues for shape #2 in the suggested approach (widen to OpenmrsObject + isInactive(T) strategy hook) over shape #1, since concept-dictionary search is exactly the sort of thing querystore should centralize.

[dkayiwa]

Worth addressing a fair pushback: aren't these metadata fields already denormalized onto data documents (per Decision 9) — concept_name on obs, location_name on encounters, drug_name on orders, etc.?

Half-right. Denormalization covers one class of query, not the other:

What denormalization already covers: "find data records mentioning X." querystore_obs carrying concept_name/concept_uuid is enough to answer "obs about amoxicillin" without a Concept index.

What it doesn't cover — and why a metadata index is still resourceful:

1. Catalog browse, including unreferenced entries. A drug newly added to the formulary, a form not yet filled, a BillableService that hasn't been billed yet — none appear in any data document. The chartsearchai-equivalent use case isn't "find obs about amoxicillin," it's "what drugs exist in the formulary matching 'amox'?" That requires the catalog itself indexed. This is exactly the billing pain point: you can't search BillableService via Bill records because the unbilled ones never surface.
2. Richer text per metadata record than fits on a data record. A Concept has preferred name + synonyms across multiple locales + description + mappings to ICD/SNOMED/LOINC. A Form has a description and field labels. Denormalizing only the preferred-locale name onto an obs throws away the rest — search recall for "diabetes mellitus type 2" vs. "T2DM" vs. "DM-2" only works if the Concept document carries all of them.
3. Metadata-only attributes. BillableService has price/department; Location has tags + attributes; Provider has specialty. These don't make sense to copy onto every referencing data record, but they're the natural search facets for the catalog.
4. Cardinality inversion. "Which locations have no encounters this year?" is answerable from a location index joined to obs counts; it isn't answerable from obs alone.

Rule of thumb: denormalized names answer data → metadata queries; metadata indexes answer catalog browse / catalog → data queries. The widening in this issue is for the second class.

[dkayiwa]

One clarification worth pinning, since "metadata indexing" can read as "index the whole concept dictionary":

Widening the type bound is not the same as projecting the OpenMRS knowledge base. This issue is a type-system fix — it removes the compile-time block so a module can contribute a metadata serializer if it wants to. Nothing gets indexed unless a ResourceTypeProvider is actually registered. The billing module can ship BillableServiceResourceTypeProvider without anyone projecting CIEL.

The ADR has already taken a position on direct metadata indices for the dictionary: Decision 11 point 4 puts querystore_concepts / querystore_drugs / querystore_locations out of v1 scope, and the Knowledge-base and reference-data projection open question gates re-opening on concrete consumer demand (chartsearchai evals or a downstream ask).

If/when concept indexing does land, "full CIEL" is the wrong default. CIEL is ~50K+ concepts; most are housekeeping (ConvSet, Question, Misc, LabSet, MedSet) that exist to structure forms and answer lists, not to be searched for. The clinically-search-relevant subset (Diagnosis, Symptom, Finding, Drug, Procedure, Test, Anatomy) is a few thousand, not fifty. And the cost question is real — embedding 50K concepts at backfill plus re-embedding on every CIEL dump refresh is a meaningful operational hammer the ADR hasn't sized yet.

The cleanest landing for it, when the time comes, is as a Decision 13 SPI contribution (e.g., a querystore-concepts companion module) with its own scoping rules for which classes to project — not as a core querystore concern. So this issue's scope is the bound widening; knowledge-base projection stays a separate, downstream decision.

[dkayiwa]

Correction to my earlier comment: point #4 ("cardinality inversion — which locations have no encounters this year?") was a poorly chosen example and I'm walking it back. That's a structured analytics query — aggregation + negation — and its natural home is a LEFT JOIN against the transactional DB, the Reporting module, or a downstream analytics layer. The querystore is CQRS for semantic and text-shaped retrieval (embeddings, kNN, locale-aware fuzzy matching); forcing SQL-shaped aggregations onto an OpenSearch/Lucene index is the wrong tool.

The case for metadata indexing rests on the first three motivators only:

1. Catalog browse of unreferenced entries — BillableService never billed, Drug never prescribed, Form never filled. Invisible from data documents.
2. Richer text than fits on a data record — Concept synonyms across locales, mappings to ICD/SNOMED/LOINC, descriptions. Denormalizing only the preferred-locale name onto an obs loses the rest.
3. Metadata-only attributes — price, tags, specialty. Don't make sense to copy onto every referencing data record but are the natural search facets for the catalog.

All three are retrieval-shaped, not analytics-shaped. Apologies for the noise from the original #4.

[dkayiwa]

Consolidating this thread — earlier comments overshot, and the honest position is narrower. This supersedes the example lists above.

The rule

A metadata type warrants a querystore_* index if and only if it carries free-text content where embedding/semantic recall delivers materially better results than:

• structured SQL queries against the transactional DB (filters, joins, "never referenced"), or
• the denormalized *_uuid + *_name already present on clinical-data documents per Decision 9.

Two corollaries fall out:

• Missing-data queries are not querystore's job. "Drugs never prescribed," "BillableService never billed," "forms not yet used," "locations with no encounters this year" are negation queries answered by LEFT JOIN ... IS NULL or NOT IN (...) against the transactional DB. Embedding similarity does not help find absences.
• Clinical-content discovery in actively-used catalogs is often covered by obs. "Which forms collect FP method?" → query obs by concept, group by encounter.form_id. No catalog index needed.

Applied to the original list

Genuinely passes the rule (rich free text + queries that fail under SQL LIKE / obs-denormalization):

• Concept — synonyms across locales, descriptions, code-system mappings. Highest payoff. Explicitly deferred by the knowledge-base open question; arrives as an SPI contribution when it does.
• Drug — only if the formulary populates indications/descriptions richly and clinicians search by indication ("drugs for hypertension"). Many deployments don't, in which case it collapses to name search → SQL.
• ReportDefinition / CohortDefinition — only if descriptions are populated and users discover reports by description rather than browsing the Reporting UI by category.

Does not pass (no free-text payoff, or covered by obs/SQL):

• Form, HtmlForm — clinical-content discovery covered by obs; form-design discovery is too admin-oriented and too small-volume to warrant an embedding pipeline.
• BillableService, PaymentMode, CashPoint, BillableServicePrice
• OrderType, OrderFrequency, CareSetting
• EncounterType, EncounterRole, VisitType
• PatientIdentifierType, PersonAttributeType, RelationshipType, LocationTag
• ProviderAttributeType, VisitAttributeType
• IdentifierSource, StockOperationType, AppointmentType, Item, Stockroom, Party

Borderline — deployment-dependent: Provider, Location, AddressHierarchyEntry. Justified only when the deployment populates description / qualification / hierarchy text richly enough that fuzzy SQL LIKE materially under-recalls.

Implication for this issue

The bound widening is still worth doing — it removes a real compile-time obstacle and the change is small — but the demand for metadata indexing is thinner than the original framing implied. Modules should not reach for querystore as a default "search this catalog" hammer. The rule above is what to check first.

The strongest case (Concept) is already gated by the knowledge-base open question per Decision 11 point 4; the rest hinge on deployment-specific data richness. Whatever lands first should arrive as an SPI contribution per Decision 13, not as core querystore concern.

[dkayiwa]

Follow-up narrowing on the previous consolidating comment — even the Concept case overstated the querystore-specific value. The terminology dimension of those questions belongs upstream, not in querystore.

Terminology questions are not querystore's job either

Most of the Concept example queries I gave (synonym lookup across locales, definition retrieval, code-system mapping, set/answer enumeration) are exactly what terminology services are designed for, and OpenMRS deployments already have access to them:

• OCL / OCL Online — searchable, synonym-aware, mapping-aware access to CIEL and other published dictionaries. Canonical source.
• FHIR terminology operations via the FHIR2 module — $lookup (definitions + designations), $translate (cross-system mappings), $expand (value-set members).
• CIEL itself, browsable directly.

Re-implementing this inside querystore — embedding ~50K CIEL concepts per deployment, refreshing on every dump, maintaining locale-aware synonym indexes — duplicates work that OCL already does centrally, once, for everyone. The ADR's knowledge-base open question already flags this trade-off explicitly: "whether querystore re-projects a local snapshot of these … or proxies live to the terminology service. The latter avoids divergence but couples query latency to an external service."

What's genuinely deployment-specific (the residual)

If anything justifies concept content in querystore at all, it's the slice OCL can't answer because it doesn't see local state:

1. The subset of CIEL this deployment actually loaded — not the universal dictionary, but "concepts in our dictionary."
2. Local custom concepts that don't exist in CIEL/OCL upstream.
3. Concepts intersected with this deployment's clinical data — "concepts that have obs/orders against them here." Join between dictionary and patient data that only this deployment can resolve.
4. Co-mingled retrieval against clinical data — a single semantic query that ranks the concept's dictionary entry alongside obs/encounters/orders referencing it. The ADR's open question explicitly flags this as "not yet thought through… may produce nonsensical orderings without per-type weighting."

Items 1–3 are largely answerable with SQL against concept filtered to deployment scope. Only item 4 requires concept content co-located with clinical data in the same embedding space — and that's the case the open question is gating on.

Cleanest architectural position

• OCL / FHIR terminology services handle dictionary lookup, synonym/locale resolution, code-system translation, value-set expansion. Universal content, indexed once, served to all deployments.
• Querystore stays focused on patient-scoped clinical retrieval over this deployment's data.
• concept_uuid is the join key between them: querystore documents carry it (per Decision 9), consumers crosswalk to OCL/FHIR for dictionary detail when needed.

This reinforces the ADR's existing position — knowledge-base projection stays out of v1, re-opened only when a concrete consumer use case requires co-mingled retrieval that UUID-crosswalk can't serve.

Net for issue #16

The bound-widening is still a worthwhile cleanup, but the demand it unlocks is even thinner than the prior comment suggested: the strongest candidate (Concept) is largely a terminology-service job, and the rest of the list mostly fails the rule. The widening should land as a small, low-cost SPI tidy-up rather than as enablement for an imminent wave of metadata indexing.

[dkayiwa]

Folded the conclusions from this thread back into the ADR (commits edd5d3c and 304e9c6) so the position doesn't live only in this comment chain:

• Decision 13 → Criteria for contributing a metadata resource type — new Conventions subsection capturing the iff rule and the two corollaries (missing-data queries aren't querystore's job; clinical-content discovery in actively-used catalogs is often covered by obs).
• Knowledge-base and reference-data projection open question — now records the OCL / FHIR $lookup+$translate+$expand delegation default and the four-item deployment-specific residual (CIEL subset loaded, local custom concepts, dictionary∩data intersection, co-mingled embedding-space retrieval).
• Decision 11 point 4 — cross-link added so the deferred direct-metadata-indices position carries the concrete criterion rather than just the deferral.
• Decision 1 Context — now frames querystore and flattened analytical layers (e.g., MambaETL where deployed) as co-equal CQRS read-side projections each built for a distinct query shape.
• README → Scope — new section with the "what this is / what this is not for" table that condenses the ADR position for module authors who don't read the full ADR.

Net effect for this issue: the bound widening is still a worthwhile cleanup, scoped narrowly to "let metadata types compile through the SPI." Whether any specific metadata type should actually be indexed is now governed by the iff rule under Decision 13, not by this issue.

[dkayiwa]

One more honest correction to the earlier "worthwhile cleanup" framing on this thread.

Walking through the candidate set after the criterion landed in Decision 13:

• The original motivator — BillableService — fails the iff rule.
• Concept (the strongest case) is deferred behind the OCL/FHIR delegation default in the knowledge-base open question; the recommended architecture is to delegate dictionary lookups to OCL/FHIR rather than index in querystore.
• Drug / ReportDefinition / CohortDefinition — deployment-dependent on whether free-text descriptions are populated richly. No consumer in flight asking for them.
• The remaining catalog types (OrderType, EncounterType, VisitType, PaymentMode, LocationTag, …) — fail the rule.

So there is no concrete consumer in flight for the bound widening today. It's speculative infrastructure work that removes a compile-time obstacle for use cases the criterion says shouldn't be common. The earlier "small worthwhile cleanup" framing on this thread overstated things — it's only a cleanup if there's something to be cleaned up for.

Suggested disposition:

• Leave this issue open as a tracking ticket — the criterion in the ADR points back here for context.
• Don't implement the widening (split-the-abstract or widen-to-OpenmrsObject + isInactive strategy hook) until a concrete consumer arrives that passes the iff rule.
• If/when one does — most likely Concept re-opening via the knowledge-base open question — bundle the bound widening with the actual provider work, so the strategy hook shape can be verified against a real use case rather than guessed.

This is consistent with the broader CLAUDE.md guidance against introducing abstractions beyond what the task requires and against designing for hypothetical future requirements. Closing this issue now would also be reasonable since the disposition has crystallized; leaving it open is the lighter-touch option.