packages/core/src/external/ehr/document/command/create-or-replace-document.ts (1)

53-55: Guard-with-braces change is fine; minor nit on log message earlier

The added braces improve readability. Minor nit: the log prefix on Line 49 still says "createOrReplaceCcda" while this function handles generic documents. Consider updating it for clarity.

packages/core/src/external/ehr/bundle/command/fetch-bundle.ts (1)

164-166: Nit: error message says 'Ehr' but the operation is S3

For consistency with fetchBundle(), prefer "S3" in the message.

Apply:

-    const msg = "Failure while fetching bundle pre-signed URL @ Ehr";
+    const msg = "Failure while fetching bundle pre-signed URL @ S3";

packages/core/src/external/ehr/document/command/fetch-document.ts (4)

52-52: Fix log prefix copy/paste: 'fetchBundle' -> 'fetchDocument'

The log label is misleading for this function.

-  const { log } = out(`Ehr fetchBundle - ehr ${ehr} cxId ${cxId} ehrPatientId ${ehrPatientId}`);
+  const { log } = out(
+    `Ehr fetchDocument - ehr ${ehr} cxId ${cxId} ehrPatientId ${ehrPatientId}`
+  );

---

81-95: Add documentType to error context for better diagnostics

Parity with fetchDocumentPreSignedUrl and easier triage for invalid/misrouted document types.

     throw new MetriportError(msg, error, {
       ehr,
       cxId,
       metriportPatientId,
       ehrPatientId,
       resourceType,
       jobId,
       key,
       s3BucketName,
       extension: createKeyAndExtension.extension,
+      documentType,
       context: "ehr.fetchDocument",
     });

---

152-152: Nit: error message says 'Ehr' but this is an S3 operation

Align wording for clarity.

-    const msg = "Failure while fetching document pre-signed URL @ Ehr";
+    const msg = "Failure while fetching document pre-signed URL @ S3";

---

154-165: Include extension in error context for observability parity

Pre-signed URL failures benefit from knowing the desired content type.

     throw new MetriportError(msg, error, {
       ehr,
       cxId,
       metriportPatientId,
       ehrPatientId,
       documentType,
       resourceType,
       jobId,
       key,
       s3BucketName,
+      extension: createKeyAndExtension.extension,
       context: "ehr.fetchDocumentPreSignedUrl",
     });

packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts (2)

221-272: Consider increasing parallel executions for better performance.

The function is set to execute with numberOfParallelExecutions: 1, which means uploads happen sequentially. Consider increasing this value for better performance, especially when dealing with multiple encounter summaries.

 await executeAsynchronously(
   encounterSummaries,
   async encounterSummary => {
     // ... existing logic ...
   },
   {
-    numberOfParallelExecutions: 1,
+    numberOfParallelExecutions: 5, // Or use a constant like parallelRequests
   }
 );

---

64-91: Consider extracting Athena-specific logic to a separate function.

The Athena-specific logic is growing complex within the main contribution function. Consider extracting it to improve readability and testability.

+async function processAthenaContribution({
+  bundle,
+  cxId,
+  metriportPatientId,
+  ehrPatientId,
+  ehr,
+  athenaPracticeId,
+}: {
+  bundle: Bundle;
+  cxId: string;
+  metriportPatientId: string;
+  ehrPatientId: string;
+  ehr: EhrSources;
+  athenaPracticeId: string;
+}): Promise<Bundle> {
+  const cxMappingLookupParams = { externalId: athenaPracticeId, source: EhrSources.athena };
+  const cxMapping = await getCxMappingOrFail(cxMappingLookupParams);
+  if (!cxMapping.secondaryMappings) {
+    throw new MetriportError("Athena secondary mappings not found", undefined, {
+      externalId: athenaPracticeId,
+      source: EhrSources.athena,
+    });
+  }
+  const secondaryMappings = athenaSecondaryMappingsSchema.parse(cxMapping.secondaryMappings);
+  let processedBundle = bundle;
+  if (secondaryMappings.contributionEncounterAppointmentTypesBlacklist) {
+    processedBundle = await removeEncounterEntriesWithBlacklistedAppointmentType({
+      bundle: processedBundle,
+      blacklistedAppointmentTypes:
+        secondaryMappings.contributionEncounterAppointmentTypesBlacklist,
+    });
+  }
+  if (secondaryMappings.contributionEncounterSummariesEnabled) {
+    await uploadEncounterSummaries({
+      ehr,
+      bundle: processedBundle,
+      cxId,
+      metriportPatientId,
+      ehrPatientId,
+    });
+  }
+  return processedBundle;
+}

 export async function contributeResourceDiffBundle({
   // ... parameters ...
 }): Promise<void> {
   // ... existing code up to line 63 ...
   if (!bundle?.bundle.entry || bundle.bundle.entry.length < 1) return;
   if (await isAthenaCustomFieldsEnabledForCx(cxId)) {
     const athenaPracticeId = getAthenaPracticeIdFromPatientId(patientMapping.externalId);
-    const cxMappingLookupParams = { externalId: athenaPracticeId, source: EhrSources.athena };
-    const cxMapping = await getCxMappingOrFail(cxMappingLookupParams);
-    // ... rest of the Athena-specific logic ...
+    bundle.bundle = await processAthenaContribution({
+      bundle: bundle.bundle,
+      cxId,
+      metriportPatientId,
+      ehrPatientId,
+      ehr,
+      athenaPracticeId,
+    });
   }
   await handleDataContribution({
     // ... existing parameters ...
   });
 }

packages/core/src/external/ehr/athenahealth/index.ts (1)

1939-2053: Consider refactoring the complex nested conditionals for better readability.

The method has grown complex with nested conditionals for handling appointment types and encounter summaries. Consider extracting the logic into separate helper methods.

+  private async attachAppointmentTypeToEncounter(
+    entry: EhrStrictFhirResourceBundleEntry,
+    cxId: string,
+    athenaPatientId: string,
+    encounterId: string
+  ): Promise<Encounter> {
+    const encounter = await this.getEncounter({
+      cxId,
+      patientId: athenaPatientId,
+      encounterId,
+    });
+    const appointment = await this.getAppointment({
+      cxId,
+      patientId: athenaPatientId,
+      appointmentId: encounter.appointmentid,
+    });
+    entry.resource.extension = [
+      ...(entry.resource.extension ?? []),
+      {
+        url: encounterAppointmentExtensionUrl,
+        valueString: appointment.appointmenttypeid,
+      },
+    ];
+    return encounter;
+  }
+
+  private async processEncounterSummary(
+    encounter: Encounter,
+    cxId: string,
+    metriportPatientId: string,
+    athenaPatientId: string,
+    encounterId: string
+  ): Promise<void> {
+    if (encounter.status !== athenaClosedStatus) return;
+    
+    const existingEncounterSummary = await fetchDocument({
+      ehr: EhrSources.athena,
+      cxId,
+      metriportPatientId,
+      ehrPatientId: athenaPatientId,
+      documentType: DocumentType.HTML,
+      resourceType: "Encounter",
+      resourceId: encounterId,
+    });
+    if (existingEncounterSummary) return;
+    
+    const encounterSummary = await this.getEncounterSummary({
+      cxId,
+      patientId: athenaPatientId,
+      encounterId,
+    });
+    await createOrReplaceDocument({
+      ehr: EhrSources.athena,
+      cxId,
+      metriportPatientId,
+      ehrPatientId: athenaPatientId,
+      documentType: DocumentType.HTML,
+      payload: encounterSummary,
+      resourceType: "Encounter",
+      resourceId: encounterId,
+    });
+  }

   private async dangerouslyAdjustEncountersInBundle({
     // ... parameters ...
   }): Promise<void> {
     // ... existing code up to line 1966 ...
         const encounterId = entry.resource.id;
         try {
           let encounter: Encounter | undefined;
           if (attachAppointmentType) {
-            encounter = await this.getEncounter({
-              cxId,
-              patientId: athenaPatientId,
-              encounterId,
-            });
-            const appointment = await this.getAppointment({
-              cxId,
-              patientId: athenaPatientId,
-              appointmentId: encounter.appointmentid,
-            });
-            entry.resource.extension = [
-              ...(entry.resource.extension ?? []),
-              {
-                url: encounterAppointmentExtensionUrl,
-                valueString: appointment.appointmenttypeid,
-              },
-            ];
+            encounter = await this.attachAppointmentTypeToEncounter(
+              entry,
+              cxId,
+              athenaPatientId,
+              encounterId
+            );
           }
           if (fetchEncounterSummary) {
             encounter =
               encounter ??
               (await this.getEncounter({
                 cxId,
                 patientId: athenaPatientId,
                 encounterId,
               }));
-            if (encounter.status === athenaClosedStatus) {
-              // ... rest of the summary processing logic ...
-            }
+            await this.processEncounterSummary(
+              encounter,
+              cxId,
+              metriportPatientId,
+              athenaPatientId,
+              encounterId
+            );
           }
         } catch (error) {
           // ... error handling ...
         }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: thomasyopes
PR: metriport/metriport#3970
File: packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts:17-17
Timestamp: 2025-06-06T16:45:31.832Z
Learning: The writeMedicationToChart function in packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts returns a response that is not currently used by any consumers, so changes to its return type are not breaking changes in practice.

Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:27-49
Timestamp: 2025-05-28T19:22:09.281Z
Learning: In packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.

Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.

Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:0-0
Timestamp: 2025-05-28T19:20:47.442Z
Learning: In packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.

Learnt from: thomasyopes
PR: metriport/metriport#4315
File: packages/core/src/external/ehr/healthie/index.ts:622-657
Timestamp: 2025-08-11T23:36:57.573Z
Learning: In the Healthie EHR integration's `getResourceBundleByResourceId` method, returning an empty bundle when a resource doesn't exist (by using `fetchResourcesFromEhr: () => Promise.resolve([])`) is intentional design. Empty bundles are an acceptable state for non-existent resources rather than throwing NotFoundError.

Learnt from: leite08
PR: metriport/metriport#3814
File: packages/api/src/routes/internal/medical/patient-consolidated.ts:141-174
Timestamp: 2025-05-20T21:26:26.804Z
Learning: The functionality introduced in packages/api/src/routes/internal/medical/patient-consolidated.ts is planned to be refactored in downstream PR #3857, including improvements to error handling and validation.

Learnt from: thomasyopes
PR: metriport/metriport#4061
File: packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/run-job.ts:34-47
Timestamp: 2025-06-19T22:44:49.393Z
Learning: In packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/run-job.ts, the team prefers to keep refreshEhrBundles operations grouped using fire-and-forget pattern rather than awaiting all operations with Promise.allSettled(). This allows individual refresh operations to run independently without blocking the job runner.

Learnt from: lucasdellabella
PR: metriport/metriport#3907
File: packages/core/src/external/fhir/adt-encounters.ts:118-143
Timestamp: 2025-05-28T02:32:27.527Z
Learning: In packages/core/src/external/fhir/adt-encounters.ts, conversion bundles stored by saveAdtConversionBundle do not require version ID assertions or versioning, unlike the sourced encounter data stored by putAdtSourcedEncounter which does require versionId validation.

Learnt from: lucasdellabella
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/resource-diff/steps/start/ehr-start-resource-diff-local.ts:0-0
Timestamp: 2025-04-21T03:47:54.332Z
Learning: In the EHR resource diff implementation, bundles should always have an `entry` property, and code should be allowed to throw if `entry` is missing rather than using null-coalescing operators, as this represents an unexpected state that should be caught and addressed.

Learnt from: leite08
PR: metriport/metriport#4255
File: packages/core/src/command/analytics-platform/fhir-to-csv/command/fhir-to-csv/fhir-to-csv-direct.ts:19-20
Timestamp: 2025-07-27T00:40:32.149Z
Learning: User leite08 acknowledged the duplicate inputBundle spread issue in packages/core/src/command/analytics-platform/fhir-to-csv/command/fhir-to-csv/fhir-to-csv-direct.ts and will fix it in a follow-up PR rather than in the current patch release PR #4255.

Learnt from: thomasyopes
PR: metriport/metriport#3709
File: packages/api/src/external/ehr/canvas/command/bundle/fetch-resource-diff-bundle.ts:62-73
Timestamp: 2025-04-23T21:19:08.443Z
Learning: In the metriport codebase, factory functions like `fetchResourceDiffBundle` intentionally let errors bubble up to the caller rather than handling them at the factory function level. This is a consistent pattern across factory implementations.

Learnt from: thomasyopes
PR: metriport/metriport#3788
File: packages/api/src/external/ehr/shared/utils/bundle.ts:83-93
Timestamp: 2025-05-08T19:41:36.533Z
Learning: In the Metriport codebase, the team prefers to let errors bubble up naturally in some cases rather than adding explicit error handling at every layer, as demonstrated in the refreshEhrBundle function in the bundle.ts file.

Learnt from: thomasyopes
PR: metriport/metriport#4090
File: packages/core/src/command/conversion-fhir/conversion-fhir-cloud.ts:30-30
Timestamp: 2025-06-25T01:55:42.627Z
Learning: In packages/core/src/command/conversion-fhir/conversion-fhir-cloud.ts, the FHIR converter Lambda endpoint is controlled by the team and guaranteed to return valid JSON in the expected Bundle<Resource> format, so JSON.parse() error handling and type validation are not necessary for this specific endpoint.

Learnt from: thomasyopes
PR: metriport/metriport#3997
File: packages/api/src/routes/internal/medical/patient-job.ts:28-29
Timestamp: 2025-06-13T20:24:16.668Z
Learning: In packages/api/src/routes/internal/medical/patient-job.ts, mounting patientJobsRouter at the root path before other route handlers is intentional behavior, even though it could potentially consume requests before later handlers.

Learnt from: RamilGaripov
PR: metriport/metriport#3976
File: packages/api/src/routes/medical/patient.ts:541-543
Timestamp: 2025-06-18T21:05:22.256Z
Learning: In packages/api/src/routes/medical/patient.ts, inline schema definitions like cohortIdSchema are acceptable and don't need to be moved to separate schema files when the user prefers to keep them inline.

Learnt from: lucasdellabella
PR: metriport/metriport#3866
File: packages/api/src/routes/internal/medical/organization.ts:95-98
Timestamp: 2025-05-27T15:54:47.774Z
Learning: The conversion of the GET `/internal/organization` endpoint from returning a single organization to returning an array of organizations (batch retrieval) in `packages/api/src/routes/internal/medical/organization.ts` was an intentional breaking change planned by the team.

packages/api/src/command/mapping/patient.ts (1)

88-93: Strengthen typing and prevent accidental mutation of defaultSources

Make the defaults immutable and typed to the enum. This improves safety (no in-place mutations downstream) and keeps types tight across the API surface.

Apply this patch:

-export const defaultSources = [
-  EhrSources.athena,
-  EhrSources.canvas,
-  EhrSources.elation,
-  EhrSources.healthie,
-];
+export const defaultSources = Object.freeze([
+  EhrSources.athena,
+  EhrSources.canvas,
+  EhrSources.elation,
+  EhrSources.healthie,
+] as const);

And update the getSourceMapForPatient signature to accept a readonly typed list:

-  sources?: string[];
+  sources?: readonly EhrSources[];

This keeps the default value assignable and ensures callers pass valid EHR sources without mutating the shared default.

Also applies to: 99-101

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled
• Linear integration is disabled

You can enable these settings in your CodeRabbit configuration.

Learnt from: leite08
PR: metriport/metriport#3814
File: packages/api/src/routes/internal/medical/patient-consolidated.ts:141-174
Timestamp: 2025-05-20T21:26:26.804Z
Learning: The functionality introduced in packages/api/src/routes/internal/medical/patient-consolidated.ts is planned to be refactored in downstream PR #3857, including improvements to error handling and validation.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled
• Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

Learnt from: thomasyopes
PR: metriport/metriport#4346
File: packages/core/src/external/ehr/athenahealth/command/get-bundle-by-resource-type.ts:25-29
Timestamp: 2025-08-13T21:37:33.618Z
Learning: In Athena EHR integration (packages/core/src/external/ehr/athenahealth/command/get-bundle-by-resource-type.ts), thomasyopes prefers hard-failing when secondary mappings fetch fails rather than graceful degradation, because these mappings control critical behaviors like contributionEncounterAppointmentTypesBlacklist and contributionEncounterSummariesEnabled. Running with default values when mappings are unavailable could lead to incorrect processing.

Learnt from: thomasyopes
PR: metriport/metriport#4346
File: packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts:221-233
Timestamp: 2025-08-14T20:43:26.255Z
Learning: In Athena EHR integration, encounter references are housed specifically in the 'encounter' and 'context' fields as checked by the doesResourceReferToEncounter function in packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts. No broadening of encounter reference detection is needed since the current implementation matches Athena's exact data structure.

Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:27-49
Timestamp: 2025-05-28T19:22:09.281Z
Learning: In packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.

Learnt from: thomasyopes
PR: metriport/metriport#4315
File: packages/core/src/external/ehr/healthie/index.ts:622-657
Timestamp: 2025-08-11T23:36:57.573Z
Learning: In the Healthie EHR integration's `getResourceBundleByResourceId` method, returning an empty bundle when a resource doesn't exist (by using `fetchResourcesFromEhr: () => Promise.resolve([])`) is intentional design. Empty bundles are an acceptable state for non-existent resources rather than throwing NotFoundError.

Learnt from: thomasyopes
PR: metriport/metriport#3970
File: packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts:17-17
Timestamp: 2025-06-06T16:45:31.832Z
Learning: The writeMedicationToChart function in packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts returns a response that is not currently used by any consumers, so changes to its return type are not breaking changes in practice.

Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.

Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:0-0
Timestamp: 2025-05-28T19:20:47.442Z
Learning: In packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.

packages/core/src/external/ehr/athenahealth/index.ts (1)

1977-1996: Skip missing appointment IDs and make extension write idempotent

Two improvements:

• Guard encounter.appointmentid to avoid an unnecessary API call with an undefined ID.
• Update or insert the extension (and skip when appointmenttypeid is falsy) to avoid duplicate entries.

Apply this diff:

-          if (attachAppointmentType) {
-            encounter = await this.getEncounter({
+          if (attachAppointmentType) {
+            encounter = await this.getEncounter({
               cxId,
               patientId: athenaPatientId,
               encounterId,
             });
-            const appointment = await this.getAppointment({
-              cxId,
-              patientId: athenaPatientId,
-              appointmentId: encounter.appointmentid,
-            });
-            entry.resource.extension = [
-              ...(entry.resource.extension ?? []),
-              {
-                url: encounterAppointmentExtensionUrl,
-                valueString: appointment.appointmenttypeid,
-              },
-            ];
+            const appointmentId = encounter.appointmentid;
+            if (appointmentId) {
+              const appointment = await this.getAppointment({
+                cxId,
+                patientId: athenaPatientId,
+                appointmentId,
+              });
+              const appointmentTypeId = appointment.appointmenttypeid;
+              if (appointmentTypeId) {
+                const extensions = entry.resource.extension ?? [];
+                const idx = extensions.findIndex(ext => ext.url === encounterAppointmentExtensionUrl);
+                if (idx >= 0) {
+                  extensions[idx] = {
+                    url: encounterAppointmentExtensionUrl,
+                    valueString: appointmentTypeId,
+                  };
+                } else {
+                  extensions.push({
+                    url: encounterAppointmentExtensionUrl,
+                    valueString: appointmentTypeId,
+                  });
+                }
+                entry.resource.extension = extensions;
+              }
+            }
           }

Note: This revisits a prior suggestion to avoid duplicate extensions and skip undefined values; surfacing it here only because the current code still appends unconditionally.

packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts (1)

205-220: Prefer truthy property checks over 'in' and keep detection scope narrow

Per repo style, prefer truthy checks rather than 'in'. The current scope (examining encounter.reference and context.reference) matches Athena’s data shape per prior guidance; keep it that way.

Apply this minimal refactor:

-function doesResourceReferenceEncounter(
-  resource: Resource,
-  encounterReferences: string[]
-): boolean {
-  if ("encounter" in resource) {
-    const encounter = resource.encounter;
-    if (!encounter || !encounter.reference) return false;
-    return encounterReferences.includes(encounter.reference);
-  }
-  if ("context" in resource) {
-    const context = resource.context;
-    if (!context || !("reference" in context) || !context.reference) return false;
-    return encounterReferences.includes(context.reference);
-  }
-  return false;
-}
+function doesResourceReferenceEncounter(resource: Resource, encounterReferences: string[]): boolean {
+  const encRef = (resource as any)?.encounter?.reference as string | undefined;
+  if (encRef && encounterReferences.includes(encRef)) return true;
+  const ctxRef = (resource as any)?.context?.reference as string | undefined;
+  return Boolean(ctxRef && encounterReferences.includes(ctxRef));
+}

packages/core/src/external/fhir/shared/index.ts (2)

300-305: Avoid producing Type/undefined for entries missing id

When an entry’s resource lacks an id, buildResourceReference will yield "<type>/undefined", which is not a valid FHIR reference and can cause subtle mismatches. Guard on id before constructing the reference.

Apply this diff:

   const entry = bundle.entry.find(entry => {
-    const entryReference = entry.resource ? buildResourceReference(entry.resource) : undefined;
+    const entryReference = entry.resource?.id
+      ? buildResourceReference(entry.resource)
+      : undefined;
     return entryReference === reference;
   });

---

307-309: Tighten buildResourceReference signature and update all call sites to guarantee an id

Changing the signature to

export function buildResourceReference<T extends Resource & { id: string }>(resource: T): string

will break existing calls unless they only pass resources with a known id. You’ll need to update these sites:

• packages/core/src/external/fhir/shared/index.ts (around line 301)
– Guard on entry.resource?.id before calling.
• packages/core/src/external/fhir/shared/references.ts (around lines 113, 122, 131)
– Change the input types to require { resource: Resource & { id: string } }.

Apply a diff like this:

--- a/packages/core/src/external/fhir/shared/index.ts
+++ b/packages/core/src/external/fhir/shared/index.ts
@@
-export function buildResourceReference(resource: Resource): string {
+export function buildResourceReference<T extends Resource & { id: string }>(
+  resource: T
+): string {
   return `${resource.resourceType}/${resource.id}`;
 }

 # Before:
-const entryReference = entry.resource ? buildResourceReference(entry.resource) : undefined;
+// After: only call when we know `id` is present
+const entryReference =
+  entry.resource?.id
+    ? buildResourceReference(entry.resource)
+    : undefined;

--- a/packages/core/src/external/fhir/shared/references.ts
+++ b/packages/core/src/external/fhir/shared/references.ts
@@
-type EncounterLocationInput = XOR<{ locationId: string }, { resource: Resource }>;
+type EncounterLocationInput = XOR<
+  { locationId: string },
+  { resource: Resource & { id: string } }
>;

@@
-type EncounterDiagnosisInput = XOR<{ conditionId: string }, { resource: Resource }>;
+type EncounterDiagnosisInput = XOR<
+  { conditionId: string },
+  { resource: Resource & { id: string } }
>;

Review any other buildResourceReference calls to ensure you only pass in resources with a guaranteed id.

packages/core/src/external/fhir/shared/references.ts (1)

113-114: Require resource.id for reference-building inputs

These constructors always create typed references from a full resource reference string ${type}/${id}. Strengthen the XOR input types to ensure id is present at compile time and avoid emitting Type/undefined.

Proposed type refinements (outside the selected lines):

// Prefer requiring id where a reference string is constructed
type EncounterParticipantInput = XOR<{ practitionerId: string }, { resource: Resource & { id: string } }>;
type EncounterLocationInput = XOR<{ locationId: string }, { resource: Resource & { id: string } }>;
type EncounterDiagnosisInput = XOR<{ conditionId: string }, { resource: Resource & { id: string } }>;

No runtime changes needed in the return lines; this is purely a safety improvement.

Also applies to: 122-123, 131-132

packages/core/src/external/ehr/athenahealth/index.ts (1)

1997-2031: Encounter-summary upload path is robust; consider early-continue for non-closed status

The summary fetch/write is correctly gated and idempotent via the S3 presence check. Minor readability improvement: return early when encounter.status !== athenaClosedStatus to reduce nesting.

Example:

-            if (encounter.status === athenaClosedStatus) {
+            if (encounter.status !== athenaClosedStatus) return;
+            {
               const existingEncounterSummary = await fetchDocument({
                 ehr: EhrSources.athena,
                 cxId,
                 metriportPatientId,
                 ehrPatientId: athenaPatientId,
                 documentType: DocumentType.HTML,
                 resourceType: "Encounter",
                 resourceId: encounterId,
               });
               if (existingEncounterSummary) return;
               const encounterSummary = await this.getEncounterSummary({
                 cxId,
                 patientId: athenaPatientId,
                 encounterId,
               });
               await createOrReplaceDocument({
                 ehr: EhrSources.athena,
                 cxId,
                 metriportPatientId,
                 ehrPatientId: athenaPatientId,
                 documentType: DocumentType.HTML,
                 payload: encounterSummary,
                 resourceType: "Encounter",
                 resourceId: encounterId,
               });
-            }
+            }

packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts (1)

331-370: DocumentReference draft is sensible; set Content-Type explicitly (good) and consider optional subject

The draft includes an appropriate LOINC type and ties to the Encounter via context.encounter. If available from the upstream utility, adding subject: { reference: "Patient/{id}" } can help downstream consumers; otherwise, current shape is acceptable.

Would you like me to wire in subject using metriportPatientId, assuming your getUploadUrlAndCreateDocRef supports it?

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled
• Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

Learnt from: keshavsaharia
PR: metriport/metriport#4045
File: packages/core/src/external/surescripts/fhir/shared.ts:87-93
Timestamp: 2025-06-18T18:50:40.968Z
Learning: The `getResourceFromResourceMap` function in `packages/core/src/external/surescripts/fhir/shared.ts` works correctly as implemented. The comparison `resourceValue === resourceMap[key]` where `resourceValue = resource[key]` is intentional and functions as designed, despite appearing to compare different types.

Learnt from: thomasyopes
PR: metriport/metriport#4346
File: packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts:221-233
Timestamp: 2025-08-14T20:43:26.281Z
Learning: In Athena EHR integration, encounter references are housed specifically in the 'encounter' and 'context' fields as checked by the doesResourceReferToEncounter function in packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts. No broadening of encounter reference detection is needed since the current implementation matches Athena's exact data structure.

Learnt from: thomasyopes
PR: metriport/metriport#4346
File: packages/core/src/external/ehr/athenahealth/command/get-bundle-by-resource-type.ts:25-29
Timestamp: 2025-08-13T21:37:33.680Z
Learning: In Athena EHR integration (packages/core/src/external/ehr/athenahealth/command/get-bundle-by-resource-type.ts), thomasyopes prefers hard-failing when secondary mappings fetch fails rather than graceful degradation, because these mappings control critical behaviors like contributionEncounterAppointmentTypesBlacklist and contributionEncounterSummariesEnabled. Running with default values when mappings are unavailable could lead to incorrect processing.

Learnt from: thomasyopes
PR: metriport/metriport#4061
File: packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/run-job.ts:34-47
Timestamp: 2025-06-19T22:44:49.393Z
Learning: In packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/run-job.ts, the team prefers to keep refreshEhrBundles operations grouped using fire-and-forget pattern rather than awaiting all operations with Promise.allSettled(). This allows individual refresh operations to run independently without blocking the job runner.

Learnt from: leite08
PR: metriport/metriport#3814
File: packages/api/src/routes/internal/medical/patient-consolidated.ts:141-174
Timestamp: 2025-05-20T21:26:26.804Z
Learning: The functionality introduced in packages/api/src/routes/internal/medical/patient-consolidated.ts is planned to be refactored in downstream PR #3857, including improvements to error handling and validation.

Learnt from: thomasyopes
PR: metriport/metriport#4346
File: packages/core/src/external/ehr/athenahealth/command/get-bundle-by-resource-type.ts:36-41
Timestamp: 2025-08-13T21:36:10.069Z
Learning: For contributionEncounterSummariesEnabled in Athena secondary mappings, strict boolean checking (=== true) is unnecessary because Zod schema validation ensures the value is boolean or undefined, making truthy checks sufficient.

Learnt from: lucasdellabella
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/resource-diff/steps/start/ehr-start-resource-diff-local.ts:0-0
Timestamp: 2025-04-21T03:47:54.332Z
Learning: In the EHR resource diff implementation, bundles should always have an `entry` property, and code should be allowed to throw if `entry` is missing rather than using null-coalescing operators, as this represents an unexpected state that should be caught and addressed.

Learnt from: thomasyopes
PR: metriport/metriport#4346
File: packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts:187-284
Timestamp: 2025-08-13T21:38:43.394Z
Learning: In the Metriport system, orphaned DocumentReferences (where the DocumentReference is created but the associated file upload fails) are acceptable and don't require cleanup logic. The system is designed to handle these gracefully.

Learnt from: leite08
PR: metriport/metriport#4255
File: packages/core/src/command/analytics-platform/fhir-to-csv/command/fhir-to-csv/fhir-to-csv-direct.ts:19-20
Timestamp: 2025-07-27T00:40:32.149Z
Learning: User leite08 acknowledged the duplicate inputBundle spread issue in packages/core/src/command/analytics-platform/fhir-to-csv/command/fhir-to-csv/fhir-to-csv-direct.ts and will fix it in a follow-up PR rather than in the current patch release PR #4255.

Learnt from: thomasyopes
PR: metriport/metriport#4346
File: packages/core/src/external/ehr/athenahealth/command/get-bundle-by-resource-type.ts:36-41
Timestamp: 2025-08-13T21:36:10.069Z
Learning: In Athena EHR integration, when contributionEncounterAppointmentTypesBlacklist is present (even if empty), attachAppointmentType should be enabled to start tracking appointment type IDs for future use, rather than only enabling when the array has items.

Learnt from: lucasdellabella
PR: metriport/metriport#4020
File: packages/fhir-sdk/src/index.ts:308-308
Timestamp: 2025-08-04T17:59:05.674Z
Learning: In packages/fhir-sdk/src/index.ts, the FhirBundleSdk constructor intentionally mutates the input bundle's total property to ensure data consistency between bundle.total and bundle.entry.length. This is an accepted exception to the "avoid modifying objects received as parameter" guideline for cleanup purposes.

Learnt from: thomasyopes
PR: metriport/metriport#3870
File: packages/core/src/external/ehr/bundle/job/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.ts:55-91
Timestamp: 2025-05-19T18:24:41.632Z
Learning: In EHR bundles related to resource diff computations, bundle creation operations that are for auditing purposes should not stop the main job flow if they fail. These operations typically use try/catch blocks that log errors without rethrowing them, which is the intended behavior.

Learnt from: leite08
PR: metriport/metriport#4043
File: packages/core/src/command/consolidated/consolidated-create.ts:0-0
Timestamp: 2025-06-18T00:18:59.144Z
Learning: In the consolidated bundle creation process (packages/core/src/command/consolidated/consolidated-create.ts), the team prefers strict failure behavior where if any component (including pharmacy resources) fails to be retrieved, the entire consolidation should fail rather than gracefully degrading. This ensures data consistency and prevents incomplete bundles.

Learnt from: thomasyopes
PR: metriport/metriport#3870
File: packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/start-job.ts:26-57
Timestamp: 2025-05-19T18:25:32.688Z
Learning: In the startCreateResourceDiffBundlesJob function (and similar functions in the codebase), the preferred approach is to let errors bubble up naturally rather than adding explicit try/catch blocks at multiple levels of the call stack.

Learnt from: thomasyopes
PR: metriport/metriport#3788
File: packages/api/src/external/ehr/shared/utils/bundle.ts:83-93
Timestamp: 2025-05-08T19:41:36.533Z
Learning: In the Metriport codebase, the team prefers to let errors bubble up naturally in some cases rather than adding explicit error handling at every layer, as demonstrated in the refreshEhrBundle function in the bundle.ts file.

Learnt from: leite08
PR: metriport/metriport#3940
File: packages/core/src/command/consolidated/search/fhir-resource/search-consolidated.ts:82-86
Timestamp: 2025-05-31T21:58:28.502Z
Learning: In packages/core/src/command/consolidated/search/fhir-resource/search-consolidated.ts, the mutable array operations using push() on resourcesMutable and hydratedMutable (lines 82-86 and 100-103) have been explicitly accepted as exceptions to the immutability guidelines after previous discussion.

Learnt from: lucasdellabella
PR: metriport/metriport#3907
File: packages/core/src/external/fhir/adt-encounters.ts:118-143
Timestamp: 2025-05-28T02:32:27.527Z
Learning: In packages/core/src/external/fhir/adt-encounters.ts, conversion bundles stored by saveAdtConversionBundle do not require version ID assertions or versioning, unlike the sourced encounter data stored by putAdtSourcedEncounter which does require versionId validation.

Learnt from: thomasyopes
PR: metriport/metriport#3970
File: packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts:17-17
Timestamp: 2025-06-06T16:45:31.832Z
Learning: The writeMedicationToChart function in packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts returns a response that is not currently used by any consumers, so changes to its return type are not breaking changes in practice.

Learnt from: leite08
PR: metriport/metriport#4193
File: packages/carequality-sdk/src/client/carequality-fhir.ts:198-208
Timestamp: 2025-07-16T00:54:56.156Z
Learning: User leite08 accepted the risk of parameter mutation in packages/carequality-sdk/src/client/carequality-fhir.ts when modifying org.active = false directly on input parameters, choosing to proceed despite the violation of the "Avoid modifying objects received as parameter" coding guideline.

Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:27-49
Timestamp: 2025-05-28T19:22:09.281Z
Learning: In packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.

Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:0-0
Timestamp: 2025-05-28T19:20:47.442Z
Learning: In packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.

Learnt from: lucasdellabella
PR: metriport/metriport#4382
File: packages/utils/src/hl7v2-notifications/reprocess-adt-conversion-bundles/common.ts:36-48
Timestamp: 2025-08-15T18:57:28.999Z
Learning: In utility scripts like packages/utils/src/hl7v2-notifications/reprocess-adt-conversion-bundles/common.ts, user lucasdellabella prefers to let bundle parsing errors fail fast rather than adding error handling, as it provides immediate visibility into data issues during script execution.

Learnt from: lucasdellabella
PR: metriport/metriport#4020
File: packages/fhir-sdk/src/index.ts:312-326
Timestamp: 2025-08-04T17:59:02.571Z
Learning: In packages/fhir-sdk/src/index.ts, the inconsistent error handling between the `total` getter (which throws an error) and the `entry` getter (which returns an empty array) when bundle.entry is undefined is intentional design by user lucasdellabella, serving different purposes where some operations need to fail fast while others can gracefully degrade.

Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.

Learnt from: thomasyopes
PR: metriport/metriport#3709
File: packages/api/src/external/ehr/canvas/command/bundle/fetch-ehr-bundle.ts:63-72
Timestamp: 2025-04-23T21:19:02.589Z
Learning: In the metriport codebase, factory functions like `fetchCanvasBundle` in the Canvas EHR subsystem intentionally let errors bubble up to the caller rather than handling them at the factory function level.

Learnt from: thomasyopes
PR: metriport/metriport#3960
File: packages/core/src/external/ehr/command/get-appointments/ehr-get-appointments-direct.ts:11-19
Timestamp: 2025-06-05T16:28:33.728Z
Learning: In the EhrGetAppointmentsDirect class in packages/core/src/external/ehr/command/get-appointments/ehr-get-appointments-direct.ts, type casting `as T[]` for the handler return value is considered safe and doesn't require additional runtime validation by the user thomasyopes.

packages/core/src/external/ehr/unit-conversion.ts (2)

200-209: Consider covering a few additional common variants ("beats per minute", singular forms, "resp/min").

These show up frequently in vendor feeds. Low-risk to include and keeps logic consistent with existing equality checks.

Apply this diff to broaden coverage:

 function isBpm(units: string): boolean {
   return (
     units === "bpm" ||
     units === "/min" ||
     units === "beats/min" ||
+    units === "beats per minute" ||
+    units === "beat/min" ||
     units === "per minute" ||
     units === "br/min" ||
+    units === "breath/min" ||
     units === "breaths/min" ||
-    units === "breaths per minute"
+    units === "breaths per minute" ||
+    units === "resp/min"
   );
 }

If you'd like, I can also refactor this to use a Set of canonical synonyms to improve readability and reduce future edit noise.

---

49-61: Minor: remove redundant re-computation of valueNumber inside the "lb_av" branch.

You already computed valueNumber at function scope (Line 23). Re-declaring it here shadows the outer binding for no gain.

Apply this diff:

   if (targetUnits === "lb_av") {
     // https://hl7.org/fhir/R4/valueset-ucum-bodyweight.html
-    const valueNumber = convertValueToNumber(baseValue);
     if (isLbs(baseInputUnits)) {
       return { ...baseParams, value: valueNumber };
     }
     if (isKg(baseInputUnits)) {
       return { ...baseParams, value: convertKgToLbs(valueNumber) };
     }
     if (isG(baseInputUnits)) {
       return { ...baseParams, value: convertGramsToLbs(valueNumber) };
     }
   }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled
• Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

packages/core/src/external/ehr/athenahealth/index.ts (1)

1989-2000: Make extension write idempotent and skip undefined appointment type IDs

Repeated runs can duplicate the appointment-type extension; also avoid writing when appointmenttypeid is missing.

Apply this diff:

-            const appointment = await this.getAppointment({
+            const appointment = await this.getAppointment({
               cxId,
-              patientId: athenaPatientId,
-              appointmentId: encounter.appointmentid,
+              patientId: athenaPatientId,
+              appointmentId: encounter.appointmentid,
             });
-            entry.resource.extension = [
-              ...(entry.resource.extension ?? []),
-              {
-                url: encounterAppointmentExtensionUrl,
-                valueString: appointment.appointmenttypeid,
-              },
-            ];
+            const appointmentTypeId = appointment.appointmenttypeid;
+            if (appointmentTypeId) {
+              const existing = entry.resource.extension ?? [];
+              const withoutExisting = existing.filter(
+                ext => ext.url !== encounterAppointmentExtensionUrl
+              );
+              entry.resource.extension = [
+                ...withoutExisting,
+                {
+                  url: encounterAppointmentExtensionUrl,
+                  valueString: appointmentTypeId,
+                },
+              ];
+            }

packages/core/src/external/ehr/athenahealth/index.ts (1)

1989-1992: Guard against missing encounter.appointmentid to avoid 404 churn

A quick guard prevents unnecessary network calls when an encounter has no appointment association.

Apply this diff:

-            const appointment = await this.getAppointment({
+            if (!encounter.appointmentid) return;
+            const appointment = await this.getAppointment({
               cxId,
               patientId: athenaPatientId,
               appointmentId: encounter.appointmentid,
             });

packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts (2)

205-220: Prefer truthy property checks over 'in' and keep scope narrow

Switch to truthy checks per guidelines without broadening supported shapes.

Apply this diff:

-function doesResourceReferenceEncounter(
-  resource: Resource,
-  encounterReferences: string[]
-): boolean {
-  if ("encounter" in resource) {
-    const encounter = resource.encounter;
-    if (!encounter || !encounter.reference) return false;
-    return encounterReferences.includes(encounter.reference);
-  }
-  if ("context" in resource) {
-    const context = resource.context;
-    if (!context || !("reference" in context) || !context.reference) return false;
-    return encounterReferences.includes(context.reference);
-  }
-  return false;
-}
+function doesResourceReferenceEncounter(
+  resource: Resource,
+  encounterReferences: string[]
+): boolean {
+  const encRef = (resource as any)?.encounter?.reference as string | undefined;
+  if (encRef && encounterReferences.includes(encRef)) return true;
+  const ctxRef = (resource as any)?.context?.reference as string | undefined;
+  if (ctxRef && encounterReferences.includes(ctxRef)) return true;
+  return false;
+}

---

298-327: Align Encounter HTML document keys between Core and API

The core Athena integration stores Encounter HTML under the Encounter’s current id, whereas this bundle flow always uses the predecessor extension value for both fetching and writing. If those differ, you’ll miss existing summaries (skipping uploads) or overwrite the wrong key.

• File: packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts
– Lines ~298–327: fetchDocument({ resourceId: predecessor })
– Lines ~371–383: createOrReplaceDocument({ resourceId: predecessor })

Optional refactor: introduce a sourceResourceId, attempt a fallback fetch with the current entry.resource.id, and use that resolved ID when creating the document.

--- a/packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts
+++ b/packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts
@@ -298,8 +298,12 @@ export async function contributeResourceDiffBundle(...) {
-  const encounterSummariesToUpload: { encounter: Encounter; summary: string }[] = [];
+  const encounterSummariesToUpload: {
+    encounter: Encounter;
+    summary: string;
+    sourceResourceId: string;
+  }[] = [];

   for (const entry of bundle.entry) {
     // ...
     const predecessor = getPredecessorExtensionValue(entry.resource);
     if (!predecessor) continue;
-    const uploadedSummary = await fetchDocument({ /* … resourceId: predecessor, jobId */ });
+    const uploadedSummary = await fetchDocument({
+      /* … resourceId: predecessor, jobId */
+    });
     if (uploadedSummary) continue;
 
-    const summary = await fetchDocument({ /* … resourceId: predecessor */ });
-    if (!summary) continue;
-    encounterSummariesToUpload.push({ encounter: entry.resource, summary: summary.file });
+    // Try predecessor, then fall back to current encounter ID
+    let sourceResourceId = predecessor;
+    let summary = await fetchDocument({ /* … resourceId: predecessor */ });
+    if (!summary) {
+      sourceResourceId = entry.resource.id!;
+      summary = await fetchDocument({ /* … resourceId: sourceResourceId */ });
+      if (!summary) continue;
+    }
+    encounterSummariesToUpload.push({
+      encounter: entry.resource,
+      summary: summary.file,
+      sourceResourceId,
+    });
 
     // …
@@ -371,7 +384,7 @@ export async function contributeResourceDiffBundle(...) {
       await createOrReplaceDocument({
         // …
-        resourceId: predecessor,
+        resourceId: encounterSummary.sourceResourceId,
         jobId: uploadJobId,
       });

This ensures you fetch and write against the same key where the HTML actually lives.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration: