From b836d2a8b187f9f87b590151a23c268235b0093d Mon Sep 17 00:00:00 2001 From: smunini Date: Mon, 4 May 2026 21:47:15 -0400 Subject: [PATCH] feat(hts): synthesise parent/child/inactive properties + definition in $lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The HL7 tx-ecosystem `simple-cases/simple-lookup-1` and `simple-lookup-2` fixtures send `property=*` and expect $lookup to surface several "well-known" concept properties that aren't stored directly in `concept_properties`: * `parent` — derived from `concept_hierarchy.parent_code WHERE child_code = req.code` * `child` — derived from `concept_hierarchy.child_code WHERE parent_code = req.code` * `inactive` — boolean derived from a `status` property in the FHIR inactive set (retired/deprecated/withdrawn/inactive); skipped when the concept already has an explicit `inactive` row to avoid duplicates * `definition` — top-level Parameters entry sourced from `concepts.definition` Each synthesised parent/child entry carries the related concept's display in `description`, matching the IG fixtures' optional `description` parts. The wildcard handling and explicit-property filter both honour synthesised entries: `property=*` includes them all alongside stored properties; `property=parent` returns only the synthesised parent. Both SQLite and PostgreSQL backends are updated. `LookupResponse` gains a `definition: Option` field, surfaced by `process_lookup` as a top-level `definition` parameter. Adds 8 unit tests (parent/child/inactive synthesis, no-status fallback, explicit-inactive non-duplication, definition echo, filtered selection, wildcard inclusion) and 1 HTTP-level integration test asserting the IG response shape end-to-end. All 499 helios-hts lib tests pass. --- .../hts/src/backends/postgres/code_system.rs | 111 +++++- crates/hts/src/backends/sqlite/code_system.rs | 374 +++++++++++++++++- crates/hts/src/operations/lookup.rs | 120 ++++++ crates/hts/src/types.rs | 6 +- 4 files changed, 599 insertions(+), 12 deletions(-) diff --git a/crates/hts/src/backends/postgres/code_system.rs b/crates/hts/src/backends/postgres/code_system.rs index 5481aa191..c0b85ad28 100644 --- a/crates/hts/src/backends/postgres/code_system.rs +++ b/crates/hts/src/backends/postgres/code_system.rs @@ -43,17 +43,30 @@ impl CodeSystemOperations for PostgresTerminologyBackend { ) .await?; - let (concept_id, display, _definition) = + let (concept_id, display, definition) = find_concept(&client, &system_id, &req.code).await?; - let all_props = fetch_properties(&client, concept_id).await?; - let properties = if req.properties.is_empty() { - all_props + let stored_props = fetch_properties(&client, concept_id).await?; + // Per FHIR spec, property="*" is the wildcard meaning "include + // every property the concept has". + let want_all = req.properties.is_empty() || req.properties.iter().any(|p| p == "*"); + let synth_props = + fetch_synthesised_properties(&client, &system_id, &req.code, &stored_props).await?; + let properties = if want_all { + let mut out = stored_props; + out.extend(synth_props); + out } else { - all_props + let mut out: Vec = stored_props .into_iter() .filter(|p| req.properties.contains(&p.code)) - .collect() + .collect(); + out.extend( + synth_props + .into_iter() + .filter(|p| req.properties.contains(&p.code)), + ); + out }; let all_designations = fetch_designations(&client, concept_id).await?; @@ -81,6 +94,7 @@ impl CodeSystemOperations for PostgresTerminologyBackend { name: cs_name, version: cs_version, display, + definition, properties, designations, }) @@ -510,6 +524,91 @@ async fn fetch_properties( .collect()) } +/// Synthesise hierarchy- and status-derived properties for `$lookup`. +/// +/// Mirrors the SQLite backend implementation — see +/// [`super::super::sqlite::code_system::fetch_synthesised_properties`] for +/// rationale. +async fn fetch_synthesised_properties( + client: &tokio_postgres::Client, + system_id: &str, + code: &str, + stored: &[PropertyValue], +) -> Result, HtsError> { + let mut out = Vec::new(); + + // Parents. + let parent_rows = client + .query( + "SELECT h.parent_code, c.display + FROM concept_hierarchy h + LEFT JOIN concepts c + ON c.system_id = h.system_id AND c.code = h.parent_code + WHERE h.system_id = $1 AND h.child_code = $2 + ORDER BY h.parent_code", + &[&system_id, &code], + ) + .await + .map_err(|e| HtsError::StorageError(e.to_string()))?; + for row in parent_rows { + out.push(PropertyValue { + code: "parent".into(), + value_type: "code".into(), + value: row.get(0), + description: row.get(1), + }); + } + + // Children. + let child_rows = client + .query( + "SELECT h.child_code, c.display + FROM concept_hierarchy h + LEFT JOIN concepts c + ON c.system_id = h.system_id AND c.code = h.child_code + WHERE h.system_id = $1 AND h.parent_code = $2 + ORDER BY h.child_code", + &[&system_id, &code], + ) + .await + .map_err(|e| HtsError::StorageError(e.to_string()))?; + for row in child_rows { + out.push(PropertyValue { + code: "child".into(), + value_type: "code".into(), + value: row.get(0), + description: row.get(1), + }); + } + + // Inactive flag (only when not already stored explicitly). + if !stored.iter().any(|p| p.code == "inactive") { + let row = client + .query_one( + "SELECT EXISTS ( + SELECT 1 FROM concept_properties cp + JOIN concepts c ON c.id = cp.concept_id + WHERE c.system_id = $1 + AND c.code = $2 + AND cp.property = 'status' + AND cp.value IN ('retired', 'deprecated', 'withdrawn', 'inactive') + )", + &[&system_id, &code], + ) + .await + .map_err(|e| HtsError::StorageError(e.to_string()))?; + let inactive: bool = row.get(0); + out.push(PropertyValue { + code: "inactive".into(), + value_type: "boolean".into(), + value: inactive.to_string(), + description: None, + }); + } + + Ok(out) +} + /// Fetch all designations for a concept. async fn fetch_designations( client: &tokio_postgres::Client, diff --git a/crates/hts/src/backends/sqlite/code_system.rs b/crates/hts/src/backends/sqlite/code_system.rs index 81cd151fc..6c1a36475 100644 --- a/crates/hts/src/backends/sqlite/code_system.rs +++ b/crates/hts/src/backends/sqlite/code_system.rs @@ -59,20 +59,37 @@ impl CodeSystemOperations for SqliteTerminologyBackend { req.date.as_deref(), )?; - let (concept_id, display, _definition) = find_concept(&conn, &system_id, &req.code)?; + let (concept_id, display, definition) = find_concept(&conn, &system_id, &req.code)?; - let all_props = fetch_properties(&conn, concept_id)?; + let stored_props = fetch_properties(&conn, concept_id)?; // Per FHIR spec, property="*" is the wildcard meaning "include // every property the concept has". Treat any "*" entry as // equivalent to omitting the filter. let want_all = req.properties.is_empty() || req.properties.iter().any(|p| p == "*"); + + // Synthesised properties (parent/child/inactive) are derived from + // the hierarchy and status tables rather than concept_properties. + // Most callers (and the tx-ecosystem IG fixtures) expect these to + // appear alongside the stored properties when property=* or any + // explicit filter names them. + let synth_props = + fetch_synthesised_properties(&conn, &system_id, &req.code, &stored_props)?; + let properties = if want_all { - all_props + let mut out = stored_props; + out.extend(synth_props); + out } else { - all_props + let mut out: Vec = stored_props .into_iter() .filter(|p| req.properties.contains(&p.code)) - .collect() + .collect(); + out.extend( + synth_props + .into_iter() + .filter(|p| req.properties.contains(&p.code)), + ); + out }; let all_designations = fetch_designations(&conn, concept_id)?; @@ -103,6 +120,7 @@ impl CodeSystemOperations for SqliteTerminologyBackend { name: cs_name, version: cs_version, display, + definition, properties, designations, }) @@ -821,6 +839,116 @@ fn fetch_properties( .collect() } +/// Synthesise hierarchy- and status-derived properties for `$lookup`. +/// +/// FHIR defines several "well-known" concept properties whose values are not +/// stored in `concept_properties` directly but are inferred from other tables: +/// +/// - `parent` / `child` — derived from `concept_hierarchy`. Each row carries +/// the parent/child code in `value` and the parent/child display in +/// `description`. +/// - `inactive` — boolean derived from a `status` property in the inactive +/// set (retired/deprecated/withdrawn/inactive). Skipped when the concept +/// already has an explicitly-stored `inactive` property to avoid duplicates. +/// +/// Returned properties carry `description` populated from the related +/// concept's display so the response includes human-readable context. +fn fetch_synthesised_properties( + conn: &rusqlite::Connection, + system_id: &str, + code: &str, + stored: &[PropertyValue], +) -> Result, HtsError> { + let mut out = Vec::new(); + + // Parents — `concept_hierarchy.parent_code WHERE child_code = code`. + { + let mut stmt = conn + .prepare_cached( + "SELECT h.parent_code, c.display + FROM concept_hierarchy h + LEFT JOIN concepts c + ON c.system_id = h.system_id AND c.code = h.parent_code + WHERE h.system_id = ?1 AND h.child_code = ?2 + ORDER BY h.parent_code", + ) + .map_err(|e| HtsError::StorageError(e.to_string()))?; + let rows = stmt + .query_map(rusqlite::params![system_id, code], |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, Option>(1)?)) + }) + .map_err(|e| HtsError::StorageError(e.to_string()))?; + for r in rows { + let (parent_code, parent_display) = + r.map_err(|e| HtsError::StorageError(e.to_string()))?; + out.push(PropertyValue { + code: "parent".into(), + value_type: "code".into(), + value: parent_code, + description: parent_display, + }); + } + } + + // Children — `concept_hierarchy.child_code WHERE parent_code = code`. + { + let mut stmt = conn + .prepare_cached( + "SELECT h.child_code, c.display + FROM concept_hierarchy h + LEFT JOIN concepts c + ON c.system_id = h.system_id AND c.code = h.child_code + WHERE h.system_id = ?1 AND h.parent_code = ?2 + ORDER BY h.child_code", + ) + .map_err(|e| HtsError::StorageError(e.to_string()))?; + let rows = stmt + .query_map(rusqlite::params![system_id, code], |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, Option>(1)?)) + }) + .map_err(|e| HtsError::StorageError(e.to_string()))?; + for r in rows { + let (child_code, child_display) = + r.map_err(|e| HtsError::StorageError(e.to_string()))?; + out.push(PropertyValue { + code: "child".into(), + value_type: "code".into(), + value: child_code, + description: child_display, + }); + } + } + + // Inactive flag — synthesise from the `status` property when the concept + // doesn't already carry an explicit `inactive` row. `inactive=true` when + // status is in the FHIR inactive set, otherwise `false` (so the response + // always communicates the active/inactive state). + if !stored.iter().any(|p| p.code == "inactive") { + let inactive: bool = conn + .query_row( + "SELECT EXISTS ( + SELECT 1 FROM concept_properties cp + JOIN concepts c ON c.id = cp.concept_id + WHERE c.system_id = ?1 + AND c.code = ?2 + AND cp.property = 'status' + AND cp.value IN ('retired', 'deprecated', 'withdrawn', 'inactive') + )", + rusqlite::params![system_id, code], + |row| row.get::<_, bool>(0), + ) + .map_err(|e| HtsError::StorageError(e.to_string()))?; + out.push(PropertyValue { + code: "inactive".into(), + value_type: "boolean".into(), + value: inactive.to_string(), + description: None, + }); + } + + Ok(out) +} + /// Fetch all designations for a concept. fn fetch_designations( conn: &rusqlite::Connection, @@ -1529,4 +1657,240 @@ mod tests { // Default display is unchanged. assert_eq!(resp.display, Some("Term (English default)".into())); } + + // ── Synthesised properties (parent / child / inactive / definition) ─────── + + /// Seed a three-concept hierarchy used by the synthesis tests below. + /// + /// PARENT + /// └── MIDDLE (status=retired → inactive) + /// ├── CHILD_A + /// └── CHILD_B + fn seed_synth(b: &SqliteTerminologyBackend) { + let conn = b.pool().get().unwrap(); + conn.execute_batch( + "INSERT INTO code_systems + (id, url, version, name, status, content, created_at, updated_at) + VALUES ('cs-syn', 'http://example.org/syn', '1.0', 'Synth CS', + 'active', 'complete', '2024-01-01', '2024-01-01'); + + INSERT INTO concepts (id, system_id, code, display, definition) + VALUES (200, 'cs-syn', 'PARENT', 'Parent display', NULL), + (201, 'cs-syn', 'MIDDLE', 'Middle display', 'Middle defn'), + (202, 'cs-syn', 'CHILD_A', 'Child A display', NULL), + (203, 'cs-syn', 'CHILD_B', 'Child B display', NULL); + + INSERT INTO concept_hierarchy (system_id, parent_code, child_code) + VALUES ('cs-syn', 'PARENT', 'MIDDLE'), + ('cs-syn', 'MIDDLE', 'CHILD_A'), + ('cs-syn', 'MIDDLE', 'CHILD_B'); + + INSERT INTO concept_properties (concept_id, property, value_type, value) + VALUES (201, 'status', 'code', 'retired');", + ) + .unwrap(); + } + + #[tokio::test] + async fn lookup_synthesises_parent_and_child_properties() { + let b = backend(); + seed_synth(&b); + + let resp = b + .lookup( + &ctx(), + LookupRequest { + system: "http://example.org/syn".into(), + code: "MIDDLE".into(), + ..Default::default() + }, + ) + .await + .unwrap(); + + let parents: Vec<_> = resp + .properties + .iter() + .filter(|p| p.code == "parent") + .collect(); + assert_eq!(parents.len(), 1); + assert_eq!(parents[0].value, "PARENT"); + assert_eq!(parents[0].description.as_deref(), Some("Parent display")); + + let children: Vec<_> = resp + .properties + .iter() + .filter(|p| p.code == "child") + .collect(); + assert_eq!(children.len(), 2); + // Children are ORDER BY child_code → CHILD_A then CHILD_B. + assert_eq!(children[0].value, "CHILD_A"); + assert_eq!(children[0].description.as_deref(), Some("Child A display")); + assert_eq!(children[1].value, "CHILD_B"); + } + + #[tokio::test] + async fn lookup_synthesises_inactive_from_status_property() { + let b = backend(); + seed_synth(&b); + + let resp = b + .lookup( + &ctx(), + LookupRequest { + system: "http://example.org/syn".into(), + code: "MIDDLE".into(), + ..Default::default() + }, + ) + .await + .unwrap(); + + // status=retired → inactive=true, surfaced even though concept_properties + // has no explicit `inactive` row. + let inactive: Vec<_> = resp + .properties + .iter() + .filter(|p| p.code == "inactive") + .collect(); + assert_eq!(inactive.len(), 1); + assert_eq!(inactive[0].value, "true"); + assert_eq!(inactive[0].value_type, "boolean"); + } + + #[tokio::test] + async fn lookup_synthesises_inactive_false_when_no_status() { + let b = backend(); + seed_synth(&b); + + let resp = b + .lookup( + &ctx(), + LookupRequest { + system: "http://example.org/syn".into(), + code: "PARENT".into(), + ..Default::default() + }, + ) + .await + .unwrap(); + + let inactive = resp + .properties + .iter() + .find(|p| p.code == "inactive") + .unwrap(); + assert_eq!(inactive.value, "false"); + } + + #[tokio::test] + async fn lookup_does_not_duplicate_explicit_inactive() { + let b = backend(); + let conn = b.pool().get().unwrap(); + conn.execute_batch( + "INSERT INTO code_systems + (id, url, version, name, status, content, created_at, updated_at) + VALUES ('cs-i', 'http://example.org/i', '1.0', 'I CS', + 'active', 'complete', '2024-01-01', '2024-01-01'); + INSERT INTO concepts (id, system_id, code, display) + VALUES (300, 'cs-i', 'X', 'X display'); + INSERT INTO concept_properties (concept_id, property, value_type, value) + VALUES (300, 'inactive', 'boolean', 'false');", + ) + .unwrap(); + drop(conn); + + let resp = b + .lookup( + &ctx(), + LookupRequest { + system: "http://example.org/i".into(), + code: "X".into(), + ..Default::default() + }, + ) + .await + .unwrap(); + + // Exactly one inactive property — synthesis is skipped because the + // concept already has an explicit `inactive` row. + let inactive: Vec<_> = resp + .properties + .iter() + .filter(|p| p.code == "inactive") + .collect(); + assert_eq!(inactive.len(), 1); + assert_eq!(inactive[0].value, "false"); + } + + #[tokio::test] + async fn lookup_returns_definition_field() { + let b = backend(); + seed_synth(&b); + + let resp = b + .lookup( + &ctx(), + LookupRequest { + system: "http://example.org/syn".into(), + code: "MIDDLE".into(), + ..Default::default() + }, + ) + .await + .unwrap(); + + assert_eq!(resp.definition.as_deref(), Some("Middle defn")); + } + + #[tokio::test] + async fn lookup_property_filter_includes_synthesised_codes() { + let b = backend(); + seed_synth(&b); + + // Asking only for `parent` should return just the synthesised parent — + // no children, no inactive, even though those would surface under `*`. + let resp = b + .lookup( + &ctx(), + LookupRequest { + system: "http://example.org/syn".into(), + code: "MIDDLE".into(), + properties: vec!["parent".into()], + ..Default::default() + }, + ) + .await + .unwrap(); + + assert_eq!(resp.properties.len(), 1); + assert_eq!(resp.properties[0].code, "parent"); + assert_eq!(resp.properties[0].value, "PARENT"); + } + + #[tokio::test] + async fn lookup_wildcard_includes_synthesised_and_stored() { + let b = backend(); + seed_synth(&b); + + let resp = b + .lookup( + &ctx(), + LookupRequest { + system: "http://example.org/syn".into(), + code: "MIDDLE".into(), + properties: vec!["*".into()], + ..Default::default() + }, + ) + .await + .unwrap(); + + // Stored: status. Synthesised: parent, child x2, inactive. + let codes: Vec<_> = resp.properties.iter().map(|p| p.code.as_str()).collect(); + assert!(codes.contains(&"status")); + assert!(codes.contains(&"parent")); + assert!(codes.contains(&"child")); + assert!(codes.contains(&"inactive")); + } } diff --git a/crates/hts/src/operations/lookup.rs b/crates/hts/src/operations/lookup.rs index 1f3ea1e06..88165242c 100644 --- a/crates/hts/src/operations/lookup.rs +++ b/crates/hts/src/operations/lookup.rs @@ -116,6 +116,11 @@ async fn process_lookup( parameter.push(json!({"name": "display", "valueString": display})); } + // Top-level concept definition (free-form text from concepts.definition). + if let Some(def) = resp.definition { + parameter.push(json!({"name": "definition", "valueString": def})); + } + // Echo back the system + code so the IG fixtures can confirm what we // looked up; also surface `abstract` from the notSelectable property // when set. @@ -477,4 +482,119 @@ mod tests { let resp = post_json(app, "/CodeSystem/$lookup", body).await; assert_eq!(resp.status(), 400); } + + /// Build an app with a small hierarchy so we can exercise the synthesised + /// `parent` / `child` properties and the top-level `definition` parameter + /// emitted by `process_lookup` for `property=*` requests (mirroring the + /// IG simple-lookup fixture shape). + fn make_hierarchical_app() -> Router { + let backend = SqliteTerminologyBackend::in_memory().unwrap(); + { + let conn = backend.pool().get().unwrap(); + conn.execute_batch( + "INSERT INTO code_systems + (id, url, version, name, status, content, created_at, updated_at) + VALUES ('cs-h', 'http://example.org/h', '0.1.0', 'HierCS', + 'active', 'complete', '2024-01-01', '2024-01-01'); + + INSERT INTO concepts (id, system_id, code, display, definition) + VALUES (10, 'cs-h', 'top', 'Top display', NULL), + (11, 'cs-h', 'mid', 'Middle display', 'Middle definition'), + (12, 'cs-h', 'leaf', 'Leaf display', NULL); + + INSERT INTO concept_hierarchy (system_id, parent_code, child_code) + VALUES ('cs-h', 'top', 'mid'), + ('cs-h', 'mid', 'leaf');", + ) + .unwrap(); + } + let state = AppState::new(backend); + Router::new() + .route( + "/CodeSystem/$lookup", + post(lookup_handler::), + ) + .with_state(state) + } + + #[tokio::test] + async fn lookup_wildcard_emits_definition_parent_child_inactive() { + let app = make_hierarchical_app(); + let body = json!({ + "resourceType": "Parameters", + "parameter": [ + {"name": "system", "valueUri": "http://example.org/h"}, + {"name": "code", "valueCode": "mid"}, + {"name": "property", "valueCode": "*"} + ] + }); + + let resp = post_json(app, "/CodeSystem/$lookup", body).await; + assert_eq!(resp.status(), 200); + + let json = body_json(resp).await; + let params = json["parameter"].as_array().unwrap(); + + // Top-level `definition` from concepts.definition. + let def = params.iter().find(|p| p["name"] == "definition").unwrap(); + assert_eq!(def["valueString"], "Middle definition"); + + // Synthesised parent property pointing at "top" with description. + let parent = params + .iter() + .find(|p| { + p["name"] == "property" + && p["part"] + .as_array() + .map(|parts| parts.iter().any(|x| x["valueCode"] == "parent")) + .unwrap_or(false) + }) + .expect("synthesised parent property should be present"); + let parent_parts = parent["part"].as_array().unwrap(); + let parent_value = parent_parts.iter().find(|x| x["name"] == "value").unwrap(); + assert_eq!(parent_value["valueCode"], "top"); + let parent_desc = parent_parts + .iter() + .find(|x| x["name"] == "description") + .unwrap(); + assert_eq!(parent_desc["valueString"], "Top display"); + + // Synthesised child property pointing at "leaf". + let child = params + .iter() + .find(|p| { + p["name"] == "property" + && p["part"] + .as_array() + .map(|parts| parts.iter().any(|x| x["valueCode"] == "child")) + .unwrap_or(false) + }) + .expect("synthesised child property should be present"); + let child_value = child["part"] + .as_array() + .unwrap() + .iter() + .find(|x| x["name"] == "value") + .unwrap(); + assert_eq!(child_value["valueCode"], "leaf"); + + // Synthesised inactive=false (no status property on `mid`). + let inactive = params + .iter() + .find(|p| { + p["name"] == "property" + && p["part"] + .as_array() + .map(|parts| parts.iter().any(|x| x["valueCode"] == "inactive")) + .unwrap_or(false) + }) + .expect("synthesised inactive property should be present"); + let inactive_value = inactive["part"] + .as_array() + .unwrap() + .iter() + .find(|x| x["name"] == "value") + .unwrap(); + assert_eq!(inactive_value["valueBoolean"], false); + } } diff --git a/crates/hts/src/types.rs b/crates/hts/src/types.rs index 03a28112c..e0ffa0d51 100644 --- a/crates/hts/src/types.rs +++ b/crates/hts/src/types.rs @@ -55,12 +55,16 @@ pub struct LookupRequest { } /// Response from `CodeSystem/$lookup`. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Default)] pub struct LookupResponse { /// The canonical name of the code system. pub name: String, pub version: Option, pub display: Option, + /// Optional concept definition text — surfaced as a top-level + /// `definition` parameter in the FHIR Parameters response. + #[serde(default)] + pub definition: Option, pub properties: Vec, pub designations: Vec, }