2887 hl7 to fhir lambda logic #3675
Conversation
…only. also added api route stub refs. metriport/metriport-internal#2887 Signed-off-by: Ramil Garipov <ramil@metriport.com>
refs. metriport/metriport-internal#2887 Signed-off-by: Ramil Garipov <ramil@metriport.com>
WalkthroughThis update introduces new functionality for converting HL7v2 messages to FHIR bundles, uploading them to AWS S3, and notifying internal APIs via webhooks. It adds a conversion module, validation schemas, and a webhook endpoint for processing presigned S3 URLs. Supporting utilities and type definitions are included for S3 operations and HL7 message handling. The update also includes scripts for batch processing HL7 files and error handling improvements. Modifications were made to ensure proper type handling and conditional property inclusion in FHIR resource generation. Additionally, configuration accessors and package dependency adjustments were made. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HL7ToFhirConverter
participant S3
participant InternalAPI
participant WebhookEndpoint
Client->>HL7ToFhirConverter: Provide HL7v2 message, context, patient info
HL7ToFhirConverter->>HL7ToFhirConverter: Parse & convert HL7 to FHIR bundle
HL7ToFhirConverter->>S3: Upload FHIR bundle
HL7ToFhirConverter->>S3: Upload combined FHIR bundle (with deduplication)
HL7ToFhirConverter->>S3: Generate presigned URL for new bundle
HL7ToFhirConverter->>InternalAPI: POST presigned URL, cxId, patientId, triggerEvent
InternalAPI->>WebhookEndpoint: (via route) process presigned URL notification
WebhookEndpoint->>WebhookEndpoint: Validate and process notification
sequenceDiagram
participant ClientScript
participant FileSystem
participant HL7ToFhirConverter
participant S3
participant InternalAPI
ClientScript->>FileSystem: Read HL7 files from folder
loop For each HL7 message
ClientScript->>HL7ToFhirConverter: Convert HL7 to FHIR and upload
HL7ToFhirConverter->>S3: Upload FHIR bundle
HL7ToFhirConverter->>InternalAPI: Notify via presigned URL
end
ClientScript->>FileSystem: Write errors to error folder (if any)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (22)
packages/core/src/external/fhir/shared/datetime.ts (1)
4-14: Function looks good, with minor simplification opportunities.This utility function follows functional programming principles and handles edge cases appropriately.
You could simplify the parameter types by removing the redundant
| undefinedsince optional parameters already include undefined:export function buildPeriod( - start?: string | undefined, - end?: string | undefined + start?: string, + end?: string ): Period | undefined { // ... }packages/shared/src/common/__tests__/date.test.ts (1)
50-80: Comprehensive test suite for the new date formatting function.The tests cover various scenarios including compact dates with/without offsets, standard ISO format, and fallback behavior.
There's a small typo in the test description:
- it("correctly handless offset", () => { + it("correctly handles offset", () => {packages/mllp-server/src/app.ts (1)
21-22: Remove unused constants.These constants are no longer used after refactoring to use
getHl7MessageIdentifierOrFail. Their presence could cause confusion for future developers.-const IDENTIFIER_FIELD = 3; -const IDENTIFIER_COMPONENT = 1;packages/api/src/routes/internal/hl7/index.ts (1)
39-40: Add logging for webhook processing.Adding more detailed logging will help with debugging and monitoring.
await processHl7FhirBundleWebhook({ cxId, patientId, presignedUrl: validatedUrl }); + console.log(`Successfully processed HL7 FHIR bundle webhook for patient ${patientId}, customer ${cxId}`); return res.sendStatus(httpStatus.OK);packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/msh.ts (3)
9-10: Define constants for all field indices.Following the pattern for
MSH_9_MESSAGE_TYPE, add constants for all field indices used in this module for better readability and maintenance.const MSH_9_MESSAGE_TYPE = 9; +const MSH_7_MESSAGE_DATETIME = 7; + +// Component indices +const COMPONENT_1 = 1; +const COMPONENT_2 = 2; +const COMPONENT_3 = 3;
16-33: Improve error messages with context and add comments for fallback logic.The current implementation is solid, but could benefit from more descriptive error messages and explanation of the fallback logic for trigger event extraction.
export function getHl7MessageIdentifierOrFail(hl7Message: Hl7Message): Hl7MessageIdentifier { const mshSegment = getSegmentByNameOrFail(hl7Message, "MSH"); - const messageType = getOptionalValueFromSegment(mshSegment, MSH_9_MESSAGE_TYPE, 1); - const triggerEvent = getOptionalValueFromSegment(mshSegment, MSH_9_MESSAGE_TYPE, 2); - const messageStructure = getOptionalValueFromSegment(mshSegment, MSH_9_MESSAGE_TYPE, 3); + const messageType = getOptionalValueFromSegment(mshSegment, MSH_9_MESSAGE_TYPE, COMPONENT_1); + const triggerEvent = getOptionalValueFromSegment(mshSegment, MSH_9_MESSAGE_TYPE, COMPONENT_2); + const messageStructure = getOptionalValueFromSegment(mshSegment, MSH_9_MESSAGE_TYPE, COMPONENT_3); if (!messageType) { - throw new MetriportError("Message type not found in MSH segment"); + throw new MetriportError("Message type not found in MSH segment", undefined, { + segment: "MSH", + field: MSH_9_MESSAGE_TYPE, + component: COMPONENT_1 + }); } + // If triggerEvent is missing, try to derive it from messageStructure (e.g., "ADT_A01" -> "A01") const derivedTriggerEvent = triggerEvent ?? messageStructure?.split("_")[1]; if (!derivedTriggerEvent) { - throw new MetriportError("Trigger event not found in MSH segment"); + throw new MetriportError("Trigger event not found in MSH segment", undefined, { + segment: "MSH", + field: MSH_9_MESSAGE_TYPE, + messageType + }); } return { messageType, triggerEvent: derivedTriggerEvent }; }
35-37: Update getMessageDatetime to use constants.For consistency, use the newly defined constants in this function as well.
export function getMessageDatetime(msg: Hl7Message): string | undefined { - return getOptionalValueFromMessage(msg, "MSH", 7, 1); + return getOptionalValueFromMessage(msg, "MSH", MSH_7_MESSAGE_DATETIME, COMPONENT_1); }packages/utils/src/hl7v2-notifications/convert-adt-to-fhir-local-example.ts (1)
49-51: Simplify timestamp formatting.The timestamp formatting code is overly complex and hard to read. Using a library function would be more maintainable.
- console.log( - new Date().toISOString().split(".")[0].replace(/-/g, "").replace("T", "").replace(/:/g, "") - ); + // Format: YYYYMMDDHHmmss + const now = new Date(); + const formattedTime = now.toISOString().replace(/[-:T.Z]/g, "").substring(0, 14); + console.log(formattedTime);packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/location.ts (1)
11-24: Improve error handling for location data extraction.The function returns undefined if no facility name is found, but doesn't provide any information about why. Consider returning a more informative result.
export function getLocationFromAdt( adt: Hl7Message -): { location: Location; locationReference: EncounterLocation } | undefined { +): { location: Location; locationReference: EncounterLocation; } | { error: string } | undefined { const name = getFacilityName(adt); // TODO 2883: Parse the address, if possible - if (!name) return undefined; + if (!name) { + return { error: "No facility name found in ADT message" }; + } const location = buildLocation({ name }); return { location, locationReference: buildLocationReference({ resource: location }), }; }packages/core/src/external/fhir/shared/references.ts (1)
107-114: Use consistent error handling for invalid parameters.The function assumes the input is valid and doesn't handle the case where neither property is provided or both are provided. This is partly covered by the XOR type, but runtime validation would be more robust.
export function buildEncounterParticipant(input: EncounterParticipantInput): EncounterParticipant { + if (!input || (typeof input !== 'object')) { + throw new Error('EncounterParticipant input must be a valid object'); + } + if ("practitionerId" in input) { return { individual: { reference: `Practitioner/${input.practitionerId}` } }; } + if (!("resource" in input) || !input.resource) { + throw new Error('EncounterParticipant input must contain either practitionerId or resource'); + } + return { individual: { reference: buildEntryReference(input.resource) } }; }packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/index.ts (2)
12-17: Add optional validation or doc comments for parameters.
The structure ofHl7ToFhirParamslooks good. Consider adding optional doc comments or type validations (e.g., ensuringtimestampStringis a valid date/time string) to enhance clarity and type safety.
67-73: Preserve existing extensions instead of overwriting them.
Currently, any previousextensionarray on entries in the bundle might be replaced. If an entry already has extensions, those extensions would be lost.Apply this diff to append the new extension instead of overwriting:
-function addHl7SourceExtension( bundle: BundleWithEntry<Resource>, sourcePath: string ): Bundle<Resource> { const ext = buildDocIdFhirExtension(sourcePath); return { ...bundle, entry: bundle.entry.map(e => ({ ...e, extension: [ext] })) }; } +function addHl7SourceExtension( + bundle: BundleWithEntry<Resource>, + sourcePath: string +): Bundle<Resource> { + const ext = buildDocIdFhirExtension(sourcePath); + return { + ...bundle, + entry: bundle.entry.map(e => ({ + ...e, + extension: e.extension ? [...e.extension, ext] : [ext], + })), + }; +}packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/encounter.ts (1)
47-65: Extend coverage for additional ADT trigger events.
getPatientStatusonly covers A01 (in-progress) and A03 (finished). The TODO comment suggests supporting more events. Expanding this mapping or inferring from the ADT message directly can reduce reliance on defaults.Would you like suggestions on additional trigger event mappings or a reminder to open an issue for it?
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts (3)
22-24: Consider centralizing event type references.Currently, the
createMessageTypesandupdateMessageTypesarrays are defined at lines 23–24. Since more event types will be added, consider centralizing these into a configuration or a constant file to facilitate easy maintenance.Would you like me to open a follow-up ticket or propose a shared config approach?
133-138: Replace console.log with out().log for consistency.At line 136, there's a
console.logstatement. To align with logging guidelines, replace it without().log(...).- console.log("Need to retrieve and update existing entries"); + out().log("Need to retrieve and update existing entries");
144-150: Validate special characters in document name.
buildDocumentNameForHl7v2Rosterconcatenates string components (lines 148–149). IfhieNameor elements insubscriptionscontain unexpected or special characters, it might cause S3 path issues. Consider sanitizing these fields or encoding them if needed.packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/condition.ts (2)
45-62: Optional system inference improvement.
buildConditionCoding(lines 45–62) callsinferConditionSystem, which always returnsundefined. If you plan to support ICD-10, SNOMED, etc., consider implementing logic withininferConditionSystem.
76-81: Implement planned system detection.
inferConditionSystemis left unimplemented (line 79). Consider mapping known code prefixes to recognized systems (ICD-10, LOINC, SNOMED) if feasible.packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/practitioner.ts (1)
16-32: Support multiple practitioner types.Right now,
getParticipantsFromAdtfocuses on the attending doctor only. If your HL7 feed can include additional practitioners (consulting, referring, etc.), consider expanding your logic to collect them.packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/shared.ts (1)
20-20: Consider adding a descriptive comment for the crypto instance.The Base64Scrambler instance is initialized but lacks context on why it's needed and how it fits into the overall flow.
-const crypto = new Base64Scrambler(Config.getHl7Base64ScramblerSeed()); +// Initialize scrambler with a seed for consistent UUID obfuscation across application restarts +const crypto = new Base64Scrambler(Config.getHl7Base64ScramblerSeed());packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/utils.ts (2)
16-17: Consider using an enum for ConditionOffset.Using a type for the limited set of condition offsets (0 or 1) is good, but an enum would provide more clarity and type safety.
-type ConditionOffset = 0 | 1; +enum ConditionOffset { + Primary = 0, + Secondary = 1, +}
44-60: Consider refactoring to reduce nested conditionals.The function has multiple fallback options which could be simplified using a chain of operations or a more declarative approach.
-export function getFacilityName(adt: Hl7Message): string | undefined { - const evn = getSegmentByNameOrFail(adt, "EVN"); - const eventFacilityNamespace = getOptionalValueFromSegment(evn, 7, 1); - const eventFacilityUniversalId = getOptionalValueFromSegment(evn, 7, 2); - if (eventFacilityNamespace || eventFacilityUniversalId) { - return [eventFacilityNamespace, eventFacilityUniversalId].filter(Boolean).join(" - "); - } - - const pv1Segment = adt.getSegment("PV1"); - if (!pv1Segment) return undefined; - - const servicingFacilityName = getOptionalValueFromSegment(pv1Segment, 39, 1); - if (servicingFacilityName) return servicingFacilityName; - - const assignedPatientLocationFacility = getOptionalValueFromSegment(pv1Segment, 3, 4); - return assignedPatientLocationFacility ?? undefined; +export function getFacilityName(adt: Hl7Message): string | undefined { + // Try to get facility from EVN segment first + try { + const evn = getSegmentByNameOrFail(adt, "EVN"); + const eventFacilityNamespace = getOptionalValueFromSegment(evn, 7, 1); + const eventFacilityUniversalId = getOptionalValueFromSegment(evn, 7, 2); + if (eventFacilityNamespace || eventFacilityUniversalId) { + return [eventFacilityNamespace, eventFacilityUniversalId].filter(Boolean).join(" - "); + } + } catch (error) { + // EVN segment not found, continue to PV1 fallbacks + } + + // Fall back to PV1 segment + const pv1Segment = adt.getSegment("PV1"); + if (!pv1Segment) return undefined; + + // Try servicingFacilityName first, then assigned location + return ( + getOptionalValueFromSegment(pv1Segment, 39, 1) || + getOptionalValueFromSegment(pv1Segment, 3, 4) + ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (24)
packages/api/src/command/medical/patient/hl7-fhir-webhook.ts(1 hunks)packages/api/src/routes/internal/hl7/index.ts(1 hunks)packages/api/src/routes/internal/hl7/schemas.ts(1 hunks)packages/api/src/routes/internal/index.ts(2 hunks)packages/core/package.json(2 hunks)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/condition.ts(1 hunks)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/encounter.ts(1 hunks)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/location.ts(1 hunks)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/mappings.ts(1 hunks)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/practitioner.ts(1 hunks)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/utils.ts(1 hunks)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/index.ts(1 hunks)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/msh.ts(1 hunks)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/shared.ts(1 hunks)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts(1 hunks)packages/core/src/external/fhir/shared/bundle.ts(2 hunks)packages/core/src/external/fhir/shared/datetime.ts(1 hunks)packages/core/src/external/fhir/shared/references.ts(2 hunks)packages/mllp-server/src/app.ts(3 hunks)packages/shared/src/common/__tests__/date.test.ts(2 hunks)packages/shared/src/common/date.ts(1 hunks)packages/utils/package.json(2 hunks)packages/utils/src/hl7v2-notifications/convert-adt-to-fhir-local-example.ts(1 hunks)packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/api/src/routes/internal/index.tspackages/shared/src/common/__tests__/date.test.tspackages/api/src/command/medical/patient/hl7-fhir-webhook.tspackages/utils/src/hl7v2-notifications/convert-adt-to-fhir-local-example.tspackages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.tspackages/api/src/routes/internal/hl7/index.tspackages/api/src/routes/internal/hl7/schemas.tspackages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/msh.tspackages/shared/src/common/date.tspackages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/encounter.tspackages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/mappings.tspackages/mllp-server/src/app.tspackages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.tspackages/core/src/external/fhir/shared/datetime.tspackages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/practitioner.tspackages/core/src/external/fhir/shared/references.tspackages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/utils.tspackages/core/src/external/fhir/shared/bundle.tspackages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/shared.tspackages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/location.tspackages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/index.tspackages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/condition.ts
🧬 Code Graph Analysis (12)
packages/shared/src/common/__tests__/date.test.ts (1)
packages/shared/src/common/date.ts (1)
buildDayjsFromCompactDate(87-97)
packages/api/src/command/medical/patient/hl7-fhir-webhook.ts (1)
packages/core/src/util/log.ts (2)
out(29-34)log(9-22)
packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts (4)
packages/api/src/shared/config.ts (1)
getEnvVarOrFail(14-14)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/msh.ts (1)
getMessageDatetime(35-37)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/utils.ts (1)
getPatientIdsOrFail(123-127)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts (1)
convertHl7MessageToFhirAndUpload(43-142)
packages/api/src/routes/internal/hl7/index.ts (4)
packages/api/src/routes/helpers/request-logger.ts (1)
requestLogger(10-63)packages/api/src/routes/util.ts (1)
asyncHandler(10-28)packages/api/src/routes/internal/hl7/schemas.ts (1)
hl7WebhookParamsSchema(25-29)packages/api/src/command/medical/patient/hl7-fhir-webhook.ts (1)
processHl7FhirBundleWebhook(3-13)
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/msh.ts (2)
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/shared.ts (3)
getSegmentByNameOrFail(89-102)getOptionalValueFromSegment(104-110)getOptionalValueFromMessage(77-87)packages/shared/src/index.ts (1)
MetriportError(37-37)
packages/mllp-server/src/app.ts (1)
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/msh.ts (1)
getHl7MessageIdentifierOrFail(16-33)
packages/core/src/external/fhir/shared/datetime.ts (1)
packages/shared/src/common/date.ts (1)
buildDayjsFromCompactDate(87-97)
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/practitioner.ts (2)
packages/core/src/external/fhir/shared/references.ts (1)
buildEncounterParticipant(108-114)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/utils.ts (1)
getAttendingDoctorNameDetails(62-76)
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/utils.ts (3)
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/shared.ts (6)
getOptionalValueFromSegment(104-110)getOptionalValueFromMessage(77-87)getSegmentByNameOrFail(89-102)getOptionalValueFromField(112-117)mapHl7SystemNameToSystemUrl(123-140)unpackPidFieldOrFail(40-51)packages/core/src/external/fhir/shared/datetime.ts (1)
buildPeriod(4-14)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/condition.ts (1)
buildConditionCoding(45-62)
packages/core/src/external/fhir/shared/bundle.ts (1)
packages/api/src/routes/medical/schemas/fhir.ts (2)
Bundle(23-23)BundleEntry(22-22)
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/shared.ts (7)
packages/core/src/util/base64-scrambler.ts (1)
Base64Scrambler(7-87)packages/core/src/util/pack-uuid.ts (2)
unpackUuid(19-39)packUuid(8-14)packages/shared/src/index.ts (1)
MetriportError(37-37)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/utils.ts (1)
getPatientIdsOrFail(123-127)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/msh.ts (1)
getMessageDatetime(35-37)packages/core/src/util/log.ts (2)
out(29-34)log(9-22)packages/core/src/util/mime.ts (1)
JSON_FILE_EXTENSION(19-19)
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/index.ts (7)
packages/core/src/util/log.ts (2)
out(29-34)log(9-22)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/msh.ts (1)
getHl7MessageIdentifierOrFail(16-33)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/shared.ts (1)
buildHl7MessageFileKey(142-150)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/encounter.ts (1)
mapEncounterAndRelatedResources(11-45)packages/core/src/external/fhir/shared/bundle.ts (2)
buildBundleFromResources(161-174)BundleWithEntry(54-56)packages/shared/src/common/date.ts (1)
elapsedTimeFromNow(60-65)packages/shared/src/index.ts (1)
MetriportError(37-37)
🪛 GitHub Actions: PR Check - Core
packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts
[error] 6-6: TypeScript error TS2307: Cannot find module '@***/core/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/pid' or its corresponding type declarations.
🪛 GitHub Actions: PR Check - Utils
packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts
[error] 6-6: TypeScript error TS2307: Cannot find module '@***/core/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/pid' or its corresponding type declarations.
🪛 GitHub Actions: PR Check - API
packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts
[error] 6-6: TypeScript error TS2307: Cannot find module '@***/core/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/pid' or its corresponding type declarations.
🪛 GitHub Actions: PR Check - MLLP Server
packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts
[error] 6-6: TypeScript error TS2307: Cannot find module '@***/core/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/pid' or its corresponding type declarations.
🪛 GitHub Actions: PR Check - Shared
packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts
[error] 6-6: TypeScript error TS2307: Cannot find module '@***/core/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/pid' or its corresponding type declarations.
🔇 Additional comments (39)
packages/shared/src/common/date.ts (2)
71-85: Well-implemented date parsing utility!The
tryParseCompactDatefunction effectively parses HL7's compact date format using regex pattern matching. The implementation is clean and follows good functional programming practices.
87-97: Good integration with existing date handling system.The
buildDayjsFromCompactDatefunction maintains the same signature as the existingbuildDayjsfunction, making it a drop-in enhancement. It correctly attempts to parse the compact date format first before falling back to the standard parser, providing backward compatibility.packages/api/src/routes/internal/hl7/schemas.ts (2)
7-20: Good validation for S3 presigned URLs.The schema properly validates that URLs are from S3 endpoints while providing appropriate error messages. The error handling in the refinement is a good practice.
25-29:❓ Verification inconclusive
Verify webhook parameters are sufficient.
The schema correctly validates UUIDs for patient and connection identifiers along with the presigned URL. However, consider if additional parameters might be needed for future extensibility, such as message type or source information.
🏁 Script executed:
#!/bin/bash # Search for usages of this schema to verify if these parameters are sufficient fd hl7WebhookParamsSchema --type file --exec grep -A 10 -B 10 "hl7WebhookParamsSchema" {} \;Length of output: 95
Below is a new shell script to re-run the search (using
rgfor a broader scan) so that we can gain a clearer picture of where and howhl7WebhookParamsSchemais used. Once you run it, please verify if the schema is indeed only used as-is—or if there are any contexts suggesting additional parameters might be needed.Based on the output of this search, you can confirm whether the current fields (i.e., validating UUIDs for patient and connection IDs and the presigned URL) fully meet the requirements. If the search still returns no usage context, please perform a manual verification of how this schema is utilized across endpoints to decide if future extensibility (e.g., including message type or source information) is needed.
🏁 Script executed:
#!/bin/bash # Use ripgrep to search the entire repository for 'hl7WebhookParamsSchema' rg -i "hl7WebhookParamsSchema" .Length of output: 475
Action: Assess HL7 Schema Parameter Usage & Consider Future Extensibility
The current schema in
packages/api/src/routes/internal/hl7/schemas.tscorrectly validatespatientIdandcxIdas UUIDs and thepresignedUrlusingpresignedUrlSchema. However, the usage seen inpackages/api/src/routes/internal/hl7/index.tsonly destructures thepresignedUrl. Please verify that the inputs forpatientIdandcxIdare indeed required for downstream processing. Additionally, consider whether future scenarios might benefit from extra parameters (e.g., message type or source information) or if it would be clearer to remove the unused fields for now.packages/core/src/external/fhir/shared/bundle.ts (2)
54-56: Good type extension for bundle with entries.This type alias ensures that bundles created with this type will always have an entry array, which improves type safety when working with FHIR bundles.
161-174: Concise bundle creation utility.The
buildBundleFromResourcesfunction follows good functional programming principles by creating a new bundle without modifying inputs. It leverages existingbuildBundleEntryfunction for consistency and maintains the pattern of other bundle creation utilities in this file.packages/core/package.json (1)
151-151:❓ Verification inconclusive
Verify compatibility with existing Medplum dependencies.
Adding
@medplum/coreas a dev dependency is appropriate for the HL7-to-FHIR conversion functionality. However, ensure this version is compatible with the existing@medplum/fhirtypespackage (v2.2.10) as there might be version mismatches between these related packages.
🌐 Web query:
Are @medplum/core v3.2.33 and @medplum/fhirtypes v2.2.10 compatible versions?💡 Result:
Based on the versions provided—
@medplum/corev3.2.33 and@medplum/fhirtypesv2.2.10—compatibility between these two libraries cannot be definitively confirmed with the available information. There are no explicit compatibility matrices or documentation found in the search results that directly address whether these specific versions work seamlessly together.However, Medplum, as a platform, emphasizes backwards compatibility and type safety in their libraries, particularly those supporting FHIR standards. Additionally:
Semantic Versioning: Both libraries use semantic versioning. The
@medplum/corelibrary is at version 3.x, and@medplum/fhirtypesis at version 2.x. Given their minor patch updates (last digits), it is likely the updates remain backward-compatible. This suggests a low likelihood of breaking changes unless explicitly stated.Documentation Guidance: Medplum typically provides type definitions and developer-friendly tooling in
@medplum/fhirtypesto interface with FHIR resources. The@medplum/corelibrary is designed to integrate with these type definitions as part of the Medplum ecosystem.To verify compatibility, refer to Medplum’s version policy or test your application with these versions to ensure that they function together as expected.
Citations:
- 1: https://www.npmjs.com/package/@medplum/fhirtypes
- 2: https://github.com/medplum/medplum
- 3: https://www.vintasoftware.com/blog/fhir-healthcare-interoperability
- 4: https://www.medplum.com/docs/sdk/core
- 5: https://www.medplum.com/blog
- 6: https://www.medplum.com/docs/bots/hl7-into-fhir
- 7: https://www.medplum.com/docs/versions
- 8: https://www.medplum.com/blog/v4
- 9: https://www.medplum.com
- 10: https://www.medplum.com/docs/fhir-datastore/working-with-fhir
Action: Confirm Medplum Package Compatibility for HL7-to-FHIR Conversion
The dependency on
@medplum/corev3.2.33 appears well-suited for the HL7-to-FHIR conversion functionality. However, compatibility with the installed@medplum/fhirtypesv2.2.10 could not be definitively confirmed based on public information. Medplum’s commitment to semantic versioning and backwards compatibility suggests that issues are unlikely, but it's important to:
- Verify integration by consulting Medplum’s version policy and relevant documentation.
- Perform local integration tests to ensure there are no unexpected issues between these versions.
packages/api/src/routes/internal/index.ts (1)
45-45: LGTM: Clean integration of HL7 routes.The import and route registration follow the established patterns in the codebase.
Also applies to: 69-69
packages/core/src/external/fhir/shared/datetime.ts (2)
8-8: Good early return for edge case.Early returning undefined when both parameters are missing is a clean approach.
10-13: Clean conditional property construction.Using spread operators with conditional objects is a nice functional approach to conditionally include properties.
packages/shared/src/common/__tests__/date.test.ts (1)
75-79: Verify expected behavior for invalid date formats.This test suggests that invalid compact date formats like "20240226123000" (missing timezone) are still successfully parsed. Is this the intended behavior? If so, the test is correct, but it's worth confirming this fallback behavior aligns with your expectations.
packages/mllp-server/src/app.ts (3)
5-6: New import for HL7 message identifier extraction looks good.The addition of
getHl7MessageIdentifierOrFailproperly centralizes the HL7 message type and trigger event extraction logic.
46-52: Good centralization of HL7 message identifier extraction.Using the
getHl7MessageIdentifierOrFailfunction improves code maintainability by centralizing extraction logic and error handling. The Sentry context setting maintains the same behavior while leveraging the new utility.
65-66: Consistent usage of extracted message identifiers.The S3 key building correctly uses the values from the extracted message identifier, maintaining consistency with the Sentry context setting.
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/index.ts (1)
22-47: Solid core logic with appropriate error handling.
TheconvertHl7v2MessageToFhirfunction effectively handles ADT message conversions and logs relevant data. Error handling for unsupported message types is cleanly implemented.packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/mappings.ts (2)
1-6: Type guard and constants look appropriate.
The type guardisAdtPatientClassaccurately checks membership in theadtPatientClasstuple. This provides a clean and type-safe approach.
24-34: Mapping logic is well-structured.
TheadtToFhirEncounterClassMapcovers essential ADT codes with a fallback toDEFAULT_ENCOUNTER_CLASS. The code and display values appear correct.packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/encounter.ts (2)
11-45: Effective creation of Encounter and related resources.
mapEncounterAndRelatedResourcesneatly composes an Encounter resource along with relevant Condition, Practitioner, and Location resources. The optional chaining for each resource is clean and avoids unnecessary code repetition.
67-85: Mapping HL7 patient class code to FHIR is appropriate.
getEncounterClassproperly uses the fallback class if the HL7 patient class is missing or invalid. The code is succinct and follows best practices for returning typedCoding.packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts (3)
58-63: Validate HL7 parsing logic.The logic for parsing HL7 messages here looks straightforward. However, ensure that
convertHl7v2MessageToFhirhandles all ADT segments properly, including unexpected or missing segments. You might consider adding negative tests or verifying partial ADT messages.
65-72: Confirm downstream success for patient retrieval.You're correctly using
executeWithNetworkRetriesand structured error handling in the request to internal patient data. Confirm that thetoFhirPatientfunction successfully maps all essential fields for your FHIR Bundle, especially for edge cases (e.g., minimal or inconsistent data).
75-131: Good use of retries and structured logging.The block for handling “create” events is solid. The usage of
storeInS3WithRetriesand a nestedexecuteWithNetworkRetriesfor calling the internal endpoint ensures robust error handling. This is a good pattern to follow for other ADT trigger events.packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/condition.ts (2)
24-43: Check for partial or missing Condition codes.
getAdmitReasongracefully returnsundefinedif no codings are found. Confirm that downstream logic adequately handles this scenario. If certain ADT messages never contain a PV2 segment, you might need fallback logic to avoid missing resources.
64-74: Descriptive resource composition.
buildConditionis a clear pattern for generating minimal FHIR Condition resources. Great job leveraging a stable UUID for the resourceidbased on the coding object. This ensures deterministic references across the pipeline.packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/practitioner.ts (3)
34-47: Mapping HL7 to FHIR names.
getAttendingDoctorDetailsFromAdtextracts and maps HL7 fields to FHIRHumanName(line 42). The approach is concise, but keep an eye on potential data loss if HL7 includes more specific name segments (e.g., multiple prefixes or name types).
49-61: Concise name building.
buildHumanNameis well-structured; returning arrays only when needed aligns with FHIR’s approach to supporting multiple name components.
63-72: Deterministic ID creation.
buildPractitioneruses a stable UUID derived from thenameproperty when no explicitidis provided. This helps ensure consistent referencing for repeated ADT references with the same name data.packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/shared.ts (9)
1-11: Import organization looks good.Clean organization of imports with clear separation between external dependencies and internal utilities.
32-38: Good use of UUID compression with scrambling for security.The UUID compression/decompression functions provide a secure way to shorten UUIDs while maintaining uniqueness.
40-51: LGTM: Robust error handling in PID field unpacking.The function properly validates PID format and throws descriptive errors when invalid.
62-72: Error handling includes helpful context.Good practice adding patient IDs and datetime to error context for easier debugging and traceability.
104-117: Functions follow functional programming principles.These utility functions are small, deterministic, and have clear input/output relationships, following the recommended functional programming style.
123-140: Good warning logging for unmapped system names.The function properly handles missing system names and logs warnings when mappings fail, which helps with monitoring and debugging.
142-154: Well-structured file key generation for HL7 and FHIR artifacts.The functions create consistent file paths that maintain the relationship between original HL7 messages and their FHIR bundle counterparts.
22-30:❓ Verification inconclusive
Consider implementing validation for coding system mappings.
The TODO comment indicates uncertainty about the completeness of the HL7 coding system mappings. This could lead to unexpected behavior if unmapped systems are encountered.
🏁 Script executed:
#!/bin/bash # Check if there are any unmapped coding systems in the codebase echo "Searching for potential HL7 coding systems that might need to be added to the mapping..." rg -i "(?:coding|system)\s*[:=].*(?!SCT|L|I10|ICD-10|I9|ICD-9)[A-Z0-9-]+" --type ts --type jsLength of output: 577
Action Required: Validate and Enforce HL7 Coding System Mappings
It appears that the current mapping in
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/shared.tsis solely a static record without any active validation. To prevent potential issues when encountering unmapped HL7 coding systems, please consider the following actions:
- Implement Runtime/Validation Checks: Add a function or testing mechanism that verifies each HL7 coding system used in the codebase is present in the mapping. This could alert you at runtime or during tests if an unmapped system is encountered.
- Improve Regex-Based Searches: The initial script used to search for unmapped systems resulted in an error due to unsupported look-around syntax. When re-verifying, please ensure to run the search using PCRE2 by adding the
--pcre2flag (e.g., userg --pcre2 ...) or modify the regex to eliminate look-around constructs.- Manual Verification: Given the regex issue encountered, a manual review of HL7 system appearances in the repository may also be worthwhile to ensure the mapping remains comprehensive.
Please address these points to strengthen the robustness of our HL7-to-FHIR conversion logic.
53-75:✅ Verification successful
Consider handling potential circular dependency.
The function relies on
getPatientIdsOrFailwhich could create a circular dependency if that function (imported from./adt/utils.ts) also depends on functions from this module.
🏁 Script executed:
#!/bin/bash # Check for potential circular dependencies echo "Checking for imports of shared.ts in adt/utils.ts..." rg -A 10 "import.*from\s+['\"].*shared['\"]" packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/utils.ts echo "Checking which functions from shared.ts are used in utils.ts..." rg "getRequiredValueFromMessage|getOptionalValueFromMessage|getSegmentByNameOrFail" packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/utils.tsLength of output: 1040
No Circular Dependency Issue Identified
After verifying the dependency flow:
- The
shared.tsmodule callsgetPatientIdsOrFailfromadt/utils.ts.- Our search shows that
adt/utils.tsdoes not import or referenceshared.ts(despite using similarly named helper functions), so there isn’t a circular dependency occurring.No modifications are required at this time, but please remain mindful of future refactoring that might inadvertently bridge these modules.
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/adt/utils.ts (3)
27-38: LGTM: Good use of optional chaining and early returns.The function properly handles potentially missing segments and returns undefined when appropriate.
78-95: LGTM: Good use of offset for extracting multiple conditions.The function provides a flexible way to extract different conditions using an offset mechanism.
111-121: Deterministic encounter ID creation with good fallbacks.The function correctly creates deterministic IDs for encounters when possible, with a UUIDv7 fallback when needed.
packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts
Outdated
Show resolved
Hide resolved
packages/utils/src/hl7v2-notifications/convert-adt-to-fhir-local-example.ts
Outdated
Show resolved
Hide resolved
refs. metriport/metriport-internal#2887 Signed-off-by: Ramil Garipov <ramil@metriport.com>
|
deleted a bunch of comments cuz they were created for an unrelated PR that this branch was created off of |
…only. also added api route stub refs. metriport/metriport-internal#2887 Signed-off-by: Ramil Garipov <ramil@metriport.com>
refs. metriport/metriport-internal#2887 Signed-off-by: Ramil Garipov <ramil@metriport.com>
refs. metriport/metriport-internal#2887 Signed-off-by: Ramil Garipov <ramil@metriport.com>
refs. metriport/metriport-internal#2887 Signed-off-by: Ramil Garipov <ramil@metriport.com>
refs. metriport/metriport-internal#2887 Signed-off-by: Ramil Garipov <ramil@metriport.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts (1)
135-137: Consider throwing an error for unexpected message types.Currently, the function just logs a message for unexpected message types and returns. Consider either throwing an error or adding more detailed logging about what to do with these message types.
- log("Unexpected message type: ", msgIdentifier.triggerEvent); - return; + const errorMessage = `Unexpected message type: ${msgIdentifier.triggerEvent}`; + log(errorMessage); + // Either throw an error or return a specific status + return;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (1)
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts
🧬 Code Graph Analysis (1)
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts (7)
packages/core/src/external/aws/s3.ts (2)
S3Utils(99-471)storeInS3WithRetries(526-581)packages/core/src/util/log.ts (2)
out(29-34)log(9-22)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/index.ts (1)
convertHl7v2MessageToFhir(22-65)packages/core/src/external/fhir/shared/bundle.ts (2)
buildBundleEntry(184-190)buildBundle(151-159)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/msh.ts (1)
getHl7MessageIdentifierOrFail(16-33)packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-conversion/shared.ts (1)
buildHl7MessageFhirBundleFileKey(152-154)packages/core/src/util/mime.ts (1)
JSON_APP_MIME_TYPE(17-17)
🔇 Additional comments (5)
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts (5)
41-48: Function implements the HL7v2 to FHIR conversion logic as described in the PR objectives.The function signature matches the requirements and includes all necessary parameters for the conversion process.
73-82: LGTM! Good conditional handling based on message type.The code correctly identifies create messages by checking if the trigger event is in the createMessageTypes array. The file key generation looks correct as well.
83-100: LGTM! Good use of existing utilities with proper error configuration.Using
storeInS3WithRetrieswith comprehensive error configuration is a good practice. The error message and context are well defined.
108-126: LGTM! Good error handling for the API request.The code properly uses executeWithNetworkRetries for the API request and includes appropriate error handling with logging.
20-22:❓ Verification inconclusive
TODO comment needs to be addressed before merging.
The TODO comment indicates that additional event types need to be added to both arrays. Consider either implementing this before merging or creating a tracking issue to ensure this doesn't get forgotten.
🏁 Script executed:
#!/bin/bash # Check if there are other HL7 event types that should be included in these arrays echo "Searching for HL7 message types and trigger events in the codebase..." rg -A 3 "getHl7MessageIdentifierOrFail" | grep "triggerEvent"Length of output: 511
Action Required: Address the Pending TODO for HL7 Event Types
Before merging, please ensure that the pending TODO is resolved—either by extending the arrays with the complete set of HL7 event types needed by the logic or by creating a tracking issue to revisit this enhancement later. Our search confirms that the code checks
msgIdentifier.triggerEventagainst thecreateMessageTypesarray in the converter, suggesting there may be more event types that warrant handling.
- Location:
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts(lines 20-22)- Recommendation: Update the
createMessageTypesandupdateMessageTypesarrays with any missing HL7 event types or create an issue to track this pending work.
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts
Outdated
Show resolved
Hide resolved
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts
Outdated
Show resolved
Hide resolved
packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts
Outdated
Show resolved
Hide resolved
Part of ENG-89 Signed-off-by: Ramil Garipov <ramil@metriport.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts (2)
89-90: Consider making the script more modular.Directly invoking the function at the bottom of the file makes it difficult to import and use this module programmatically in other contexts.
-invokeLambdaLogic(); +// Only run the function if this file is being executed directly +if (require.main === module) { + invokeLambdaLogic() + .then(result => { + if (result.errors) { + process.exit(1); + } + process.exit(0); + }) + .catch(err => { + console.error("Fatal error:", errorToString(err)); + process.exit(1); + }); +} + +// Export the function for use in other modules +export { invokeLambdaLogic };
58-81: Consider processing messages in sequence rather than in parallel.The current implementation processes all messages in parallel using
forEach, which could lead to race conditions or resource exhaustion if there are many messages. Consider processing them sequentially with a for...of loop or using a batching approach.- chunks.forEach(message => { + // Process messages sequentially to avoid overwhelming resources + for (const message of chunks) { const hl7Message = Hl7Message.parse(message); const timestamp = getOrCreateMessageDatetime(hl7Message); const messageId = getMessageUniqueIdentifier(hl7Message); try { const { cxId, patientId } = getCxIdAndPatientIdOrFail(hl7Message); - convertHl7MessageToFhirAndUpload({ + await convertHl7MessageToFhirAndUpload({ cxId, patientId, message, messageId, messageReceivedTimestamp: timestamp, apiUrl, bucketName, }); } catch (err) { errors.push({ err, msg: errorToString(err), timestamp, }); } - }); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (1)
packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts
🔇 Additional comments (4)
packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts (4)
1-13: Imports look good, but consider reorganizing per guidelines.The imports are structured appropriately with dotenv configuration at the top, followed by external and internal dependencies. This follows good practices for script initialization.
14-46: Documentation is thorough and clear.The JSDoc provides excellent context about the purpose, inputs, process, outputs, required environment variables, and usage instructions for this utility script. This level of documentation is valuable for maintainability.
47-49: Environment variables are correctly validated.Using
getEnvVarOrFailensures the script will fail fast with a clear message if required configuration is missing.
54-56:✅ Verification successful
Consider more robust HL7 message parsing.
The current splitting logic assumes messages always start with "MSH|" at the beginning of a line. This might miss messages if they don't strictly follow this format or if the file has different formatting.
🏁 Script executed:
#!/bin/bash # Check if there are any common HL7 message formats that might not be captured by the current regex # Look for potential message segments that might indicate the start of an HL7 message rg -i "^[A-Z]{3}\|" ${filePath}/${fileName} || echo "No alternative segment headers found" # Check if all MSH segments are at the beginning of lines rg -i "MSH\|" ${filePath}/${fileName} | rg -v "^MSH\|" || echo "All MSH segments are at the beginning of lines"Length of output: 403
🏁 Script executed:
#!/bin/bash set -e # Files to verify files=( "packages/utils/src/hl7v2-notifications/hl7-to-fhir-converter-logic.ts" "packages/utils/src/hl7v2-notifications/convert-adt-to-fhir-local-example.ts" ) for file in "${files[@]}"; do echo "=== Checking $file ===" # 1. Look for any three-letter segment headers (e.g., PID|, EVN|) that might indicate alternative message starts echo "- Searching for any three-letter segment header at line start:" rg -n "^[A-Z]{3}\|" "$file" || echo " → No alternative segment headers found" # 2. Ensure all MSH| occurrences are anchored at the start of a line echo "- Verifying that all MSH| entries begin at column 1:" rg -n "MSH\|" "$file" | rg -n -v "^MSH\|" || echo " → All MSH| entries are anchored at line start" echo doneLength of output: 1796
Splitting logic is sufficient for current HL7 message patterns
We’ve verified across both
hl7-to-fhir-converter-logic.tsandconvert-adt-to-fhir-local-example.tsthat:
- No other three‑letter segment headers (e.g. “PID|”, “EVN|”) appear at the start of lines.
- Every
MSH|occurrence is already anchored to the beginning of a line.Given this, the existing
hl7Text.split(/(?=^MSH\|)/m)approach correctly captures each message in our current files. No changes are required here.
packages/utils/src/hl7v2-notifications/convert-adt-to-fhir-local-example.ts
Outdated
Show resolved
Hide resolved
packages/utils/src/hl7v2-notifications/convert-adt-to-fhir-local-example.ts
Show resolved
Hide resolved
packages/core/src/util/config.ts
Outdated
| static getOutgoingHl7NotificationBucketName(): string { | ||
| return getEnvVarOrFail("OUTGOING_HL7_NOTIFICATION_BUCKET_NAME"); | ||
| } |
There was a problem hiding this comment.
when you pull in upstream changes you'll want to swap this out for the other, similarly but differently named getHl7OutgoingMessageBucketName
There was a problem hiding this comment.
Ya, I was expecting there to be a mashup here haha
Updated!
Part of ENG-89 Signed-off-by: Ramil Garipov <ramil@metriport.com>
Part of ENG-89 Signed-off-by: Ramil Garipov <ramil@metriport.com>
Ticket: #metriport/metriport-internal#2887
Description
Testing
Release Plan
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores