packages/core/src/external/ehr/athenahealth/command/write-back/lab.ts (1)

15-21: Minor: prefer safe destructuring for departmentId

Using array destructuring is a bit more idiomatic and guards against undefined arrays.

Apply this diff:

-  const athenaDepartmentId = secondaryMappings.departmentIds[0];
+  const [athenaDepartmentId] = secondaryMappings.departmentIds ?? [];

packages/core/src/external/ehr/shared.ts (2)

1009-1021: getValidCode may over-filter; consider allowing entries without display

Filtering out codings without a display can yield no titles even when valid codes exist. Suggest allowing codings with a code but missing display, and let the caller derive a fallback label.

Apply this diff:

-export function getValidCode(coding: Coding[] | undefined): Coding[] {
+export function getValidCode(coding: Coding[] | undefined): Coding[] {
   if (!coding) return [];
 
   return coding.filter(coding => {
     return (
       coding.code &&
       coding.code.toLowerCase().trim() !== UNK_CODE.toLowerCase() &&
-      coding.display &&
-      coding.display.toLowerCase().trim() !== UNKNOWN_DISPLAY
+      (coding.display ? coding.display.toLowerCase().trim() !== UNKNOWN_DISPLAY : true)
     );
   });
 }

---

1023-1041: Title normalization: broaden BP aliases and avoid mutating for non-S/D components

Current logic rewrites any title containing “blood pressure” to “Blood Pressure” and sets grouping = title. That’s fine for S/D components, but generic BP measurements (e.g., MAP) will later be dropped by the pairing routine. Keep grouping only for S/D, and let other BP-related observations preserve their title.

Apply this diff:

-  if (
-    title.toLowerCase().includes("blood pressure") ||
-    title.toLowerCase().includes("bp sys") ||
-    title.toLowerCase().includes("bp dias")
-  ) {
-    observationPoint.grouping = title;
-    updatedTitle = "Blood Pressure";
-  }
+  const lower = title.toLowerCase();
+  const isSystolic = lower.includes("systolic") || lower.includes("bp sys");
+  const isDiastolic = lower.includes("diastolic") || lower.includes("bp dias");
+  const isBloodPressure = lower.includes("blood pressure") || lower.startsWith("bp ");
+  if (isBloodPressure && (isSystolic || isDiastolic)) {
+    observationPoint.grouping = title;
+    updatedTitle = "Blood Pressure";
+  }

packages/core/src/external/ehr/command/write-back/grouped-vitals.ts (4)

70-72: Type guard: avoid 'in' per local guideline; use property presence check

Slight style tweak per guideline “Use truthy syntax instead of in”.

Apply this diff:

-export function isGroupedVitalsByCode(val: unknown): val is GroupedVitals {
-  return !!val && typeof val === "object" && "mostRecentObservation" in val;
-}
+export function isGroupedVitalsByCode(val: unknown): val is GroupedVitals {
+  const obj = val as Partial<GroupedVitals> | undefined;
+  return !!obj && typeof obj === "object" && obj.mostRecentObservation !== undefined;
+}

---

92-96: Ensure date-grouped output is sorted by date

Downstream consumers typically expect chronological order.

Apply this diff:

-  return Object.entries(groupedVitals).map(([chartDate, observations]) => [
-    buildDayjs(chartDate).toDate(),
-    observations,
-  ]);
+  return Object.entries(groupedVitals)
+    .sort((a, b) => buildDayjs(a[0]).valueOf() - buildDayjs(b[0]).valueOf())
+    .map(([chartDate, observations]) => [buildDayjs(chartDate).toDate(), observations]);

---

105-143: Use forEach instead of map for side effects

.map’s return value is unused; .forEach communicates intent and avoids creating an intermediate array.

Apply this diff:

-  observations.map(v => {
+  observations.forEach(v => {
     let title: string;
     const codings = getValidCode(v.code?.coding);
     const displays = codings.map(coding => coding.display);
     if (displays.length) {
       title = Array.from(new Set(displays)).join(", ");
     } else if (v.code?.text) {
       title = v.code.text;
     } else {
       results.push({ title: "-", mostRecentObservation: v });
       return;
     }
@@
-    const groupedObservation = observationMap.get(title);
-    if (groupedObservation) {
-      groupedObservation.push(observationPoint);
-      observationMap.set(title, groupedObservation);
-    } else {
-      observationMap.set(title, [observationPoint]);
-    }
-  });
+    const groupedObservation = observationMap.get(title);
+    if (groupedObservation) groupedObservation.push(observationPoint);
+    else observationMap.set(title, [observationPoint]);
+  });

---

148-164: Nit: prefer forEach over map when pushing to an array

The result of .map is not used; use .forEach for clarity.

Apply this diff:

-  Array.from(observationMap.entries()).map(([title, values]) => {
+  Array.from(observationMap.entries()).forEach(([title, values]) => {
     const sortedPoints = values.sort(
       (a, b) => new Date(a.date).getTime() - new Date(b.date).getTime()
     );
     const mostRecent = sortedPoints[sortedPoints.length - 1];
     if (!mostRecent) return;
     mainResults.push({
       title,
       mostRecentObservation: mostRecent.rawVital,
       sortedPoints: sortedPoints.map(p => ({
         value: p.observation,
         date: p.date,
         ...(p.unit && { unit: p.unit }),
         ...(p.bp && { bp: p.bp }),
       })),
     });
-  });
+  });

packages/core/src/external/ehr/athenahealth/command/write-back/condition.ts (2)

10-16: Department selection: picking the first ID is brittle when multiple are configured

If multiple departmentIds are present, always choosing index 0 may write to an unintended department. Consider allowing an explicit preferred department (if/when our mappings support it) or at least logging which one was chosen to aid traceability.

Example minimal improvement (retain behavior, add clarity):

-  const athenaDepartmentId = secondaryMappings.departmentIds[0];
+  const [athenaDepartmentId] = secondaryMappings.departmentIds ?? [];

Optionally, centralize resolution into a helper (see grouped-vitals comment) so all Athena write-backs share the same logic.

---

27-33: Optional: Pre-validate SNOMED code before calling Athena createProblem

Although our ehrWriteBackConditionPrimaryCodeMap correctly maps Athena to SNOMED_CODE, the Athena‐specific writeBackCondition command does not verify that the provided FHIR Condition actually contains a SNOMED coding. If it’s missing, Athena will return a generic 4xx—adding an explicit check will surface a clearer, early error.

• Location:
packages/core/src/external/ehr/athenahealth/command/write-back/condition.ts (around lines 27–33)

• Suggested tweak:

 import { BadRequestError } from "@metriport/shared";
+import { getConditionSnomedCode } from "../../../shared";
 
 export async function writeBackCondition(params: WriteBackConditionClientRequest): Promise<void> {
   const { cxId, practiceId, ehrPatientId, tokenId, condition } = params;
+
+  // Optional: ensure the Condition has a SNOMED coding
+  const snomedCode = getConditionSnomedCode(condition);
+  if (!snomedCode) {
+    throw new BadRequestError(
+      "Missing SNOMED code for condition",
+      undefined,
+      { ehr: EhrSources.athena, practiceId }
+    );
+  }
 
   const secondaryMappings = await getSecondaryMappings({ … });
   // …
   await client.createProblem({
     cxId,
     patientId: ehrPatientId,
     departmentId: athenaDepartmentId,
     condition,
   });
 }

This check is purely optional but will fail fast with a descriptive error if a SNOMED code is absent.

packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts (1)

639-642: getEhrGroupedVitals usage verified—please add guardrail test

I’ve confirmed that in packages/core/src/external/ehr/job/bundle/write-back-bundles:

• There are no remaining references to the old GroupedVitalsByDate shape or isGroupedVitalsByDate.
• All grouped‐vitals write-back paths now call getEhrGroupedVitals.

The switch is safe and EHR-aware. Please add or adjust a unit/integration test to cover both Athena (code-based) and Elation (date-based) grouping paths.

packages/core/src/external/ehr/athenahealth/command/write-back/grouped-vitals.ts (1)

23-34: Duplicate department resolution logic — extract a shared helper

This logic mirrors the pattern in the Athena condition write-back (and likely labs). Centralize in a single resolver to ensure consistent selection, logging, and future preferences (e.g., preferred department).

Apply this diff locally to use a helper:

-  const secondaryMappings = await getSecondaryMappings({
-    ehr: EhrSources.athena,
-    practiceId,
-    schema: athenaSecondaryMappingsSchema,
-  });
-  const athenaDepartmentId = secondaryMappings.departmentIds[0];
+  const secondaryMappings = await getSecondaryMappings({
+    ehr: EhrSources.athena,
+    practiceId,
+    schema: athenaSecondaryMappingsSchema,
+  });
+  const [athenaDepartmentId] = secondaryMappings.departmentIds ?? [];
   if (!athenaDepartmentId) {
     throw new BadRequestError("Department ID not found", undefined, {
       ehr: EhrSources.athena,
       practiceId,
     });
   }

And consider adding a shared resolver in packages/core/src/external/ehr/athenahealth/shared.ts:

// in athenahealth/shared.ts
export async function resolveAthenaDepartmentId(practiceId: string): Promise<string> {
  const mappings = await getSecondaryMappings({
    ehr: EhrSources.athena,
    practiceId,
    schema: athenaSecondaryMappingsSchema,
  });
  const [id] = mappings.departmentIds ?? [];
  if (!id) {
    throw new BadRequestError("Department ID not found", undefined, {
      ehr: EhrSources.athena,
      practiceId,
    });
  }
  return id;
}

Then replace local logic with const athenaDepartmentId = await resolveAthenaDepartmentId(practiceId);

If you want, I can prep a follow-up PR to introduce this helper and update all Athena write-backs for consistency.

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#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#4358
File: packages/core/src/external/ehr/command/write-back/shared.ts:21-21
Timestamp: 2025-08-12T17:28:06.150Z
Learning: In packages/core/src/external/ehr/command/write-back/shared.ts, the WriteBackResourceRequest.primaryResourceOrResources field deliberately uses the generic tuple [Date, Resource[]] instead of the more specific GroupedVitalsByDate type ([Date, Observation[]]) to keep the shared interface flexible for potentially grouping different resource types by date in the future.

Learnt from: thomasyopes
PR: metriport/metriport#4000
File: packages/core/src/external/ehr/athenahealth/index.ts:504-507
Timestamp: 2025-06-11T21:39:26.805Z
Learning: In AthenaHealth write-back (`packages/core/src/external/ehr/athenahealth/index.ts`), only the condition statuses “relapse” and “recurrence” are currently mapped to AthenaHealth problem statuses (“CHRONIC”); other FHIR clinicalStatus values (e.g., “active”, “resolved”, “inactive”, “remission”) are not yet supported.

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: keshavsaharia
PR: metriport/metriport#4370
File: packages/shared/src/domain/patient.ts:5-5
Timestamp: 2025-08-15T00:00:45.080Z
Learning: The patientSchema in packages/shared/src/domain/patient.ts is used in a limited subsystem scope ("SS") and is not the same as other Patient schemas referenced throughout the codebase, making additive optional field changes like externalId safe and non-breaking.

Learnt from: leite08
PR: metriport/metriport#3463
File: packages/api/src/command/medical/patient/settings/create-patient-settings.ts:174-175
Timestamp: 2025-03-19T13:44:54.126Z
Learning: In the Metriport codebase, Lodash is used extensively and preferred for array and object manipulations. When working with batches of data, use Lodash's `_.chunk()` instead of manual array slicing with `array.slice(i, i + batchSize)`. The codebase also commonly uses other Lodash functions like `cloneDeep`, `uniq`, `uniqBy`, `groupBy`, and `orderBy`. Prefer Lodash functions over native JavaScript implementations for better readability and consistency across the codebase.

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.

packages/core/src/external/ehr/shared.ts (1)

1087-1136: BP pairing loses data and is timestamp-fragile; normalize keys and preserve singletons

Pairs are done by raw date string; near-equal timestamps won’t match. Non S/D BP entries are dropped. Reuse the normalized-key strategy and keep unmatched entries.

Apply:

 export function handleBloodPressureMapping(obsMap: Map<string, GroupedObservation[]>) {
   const bloodPressure = obsMap.get("Blood Pressure");
   if (!bloodPressure) return;

-  const groupedBloodPressure: GroupedObservation[] = [];
-
-  const systolicMap = new Map<string, number>();
-  const diastolicMap = new Map<string, number>();
-
-  bloodPressure.forEach(bp => {
-    if (bp.grouping?.toLowerCase().includes("systolic")) {
-      systolicMap.set(bp.date, bp.observation);
-    } else if (bp.grouping?.toLowerCase().includes("diastolic")) {
-      diastolicMap.set(bp.date, bp.observation);
-    }
-  });
-
-  bloodPressure.forEach(bp => {
-    if (bp.grouping?.toLowerCase().includes("systolic")) {
-      const diastolicValue = diastolicMap.get(bp.date);
-      if (diastolicValue !== undefined) {
-        groupedBloodPressure.push({
-          ...bp,
-          bp: {
-            systolic: bp.observation,
-            diastolic: diastolicValue,
-          },
-        });
-      }
-    } else if (bp.grouping?.toLowerCase().includes("diastolic")) {
-      const systolicValue = systolicMap.get(bp.date);
-      if (systolicValue !== undefined) {
-        if (
-          !groupedBloodPressure.some(
-            gbp => gbp.date === bp.date && gbp.bp?.diastolic === bp.observation
-          )
-        ) {
-          groupedBloodPressure.push({
-            ...bp,
-            bp: {
-              systolic: systolicValue,
-              diastolic: bp.observation,
-            },
-          });
-        }
-      }
-    }
-  });
-
-  obsMap.set("Blood Pressure", groupedBloodPressure);
+  const groupedBloodPressure: GroupedObservation[] = [];
+  const systolicMap = new Map<string, number>();
+  const diastolicMap = new Map<string, number>();
+
+  const toKey = (d: string) => buildDayjs(d).toISOString();
+  bloodPressure.forEach(bp => {
+    const lower = bp.grouping?.toLowerCase() ?? "";
+    const key = toKey(bp.date);
+    if (lower.includes("systolic")) {
+      systolicMap.set(key, bp.observation);
+    } else if (lower.includes("diastolic")) {
+      diastolicMap.set(key, bp.observation);
+    }
+  });
+
+  bloodPressure.forEach(bp => {
+    const lower = bp.grouping?.toLowerCase() ?? "";
+    const key = toKey(bp.date);
+    if (lower.includes("systolic")) {
+      const diastolicValue = diastolicMap.get(key);
+      if (diastolicValue !== undefined) {
+        const candidate = { ...bp, bp: { systolic: bp.observation, diastolic: diastolicValue } };
+        const dup = groupedBloodPressure.some(
+          gbp =>
+            toKey(gbp.date) === key &&
+            gbp.bp?.systolic === candidate.bp!.systolic &&
+            gbp.bp?.diastolic === candidate.bp!.diastolic
+        );
+        if (!dup) groupedBloodPressure.push(candidate);
+      }
+    } else if (lower.includes("diastolic")) {
+      const systolicValue = systolicMap.get(key);
+      if (systolicValue !== undefined) {
+        const candidate = { ...bp, bp: { systolic: systolicValue, diastolic: bp.observation } };
+        const dup = groupedBloodPressure.some(
+          gbp =>
+            toKey(gbp.date) === key &&
+            gbp.bp?.systolic === candidate.bp!.systolic &&
+            gbp.bp?.diastolic === candidate.bp!.diastolic
+        );
+        if (!dup) groupedBloodPressure.push(candidate);
+      }
+    }
+  });
+
+  // Preserve non S/D BP observations and unpaired singletons
+  bloodPressure.forEach(bp => {
+    const lower = bp.grouping?.toLowerCase() ?? "";
+    const key = toKey(bp.date);
+    const isSD = lower.includes("systolic") || lower.includes("diastolic");
+    const hasPair = groupedBloodPressure.some(gbp => toKey(gbp.date) === key);
+    if (!isSD || !hasPair) groupedBloodPressure.push(bp);
+  });
+
+  obsMap.set("Blood Pressure", groupedBloodPressure);
 }

packages/core/src/external/ehr/command/write-back/grouped-vitals.ts (1)

98-167: Bug: numeric parsing drops decimals and treats 0 as missing

• parseInt truncates decimals (e.g., 98.6 → 98).
• if (!date || !observationValue) discards valid zero values.

Apply:

-    const observationValue = v.valueQuantity?.value ?? v.valueString;
+    const observationValue = v.valueQuantity?.value ?? v.valueString;
     const date = v.effectiveDateTime ?? v.effectivePeriod?.start;
-    if (!date || !observationValue) {
+    if (!date || observationValue == null) {
       results.push({ title, mostRecentObservation: v });
       return;
     }
 
     const observationUnit = v.valueQuantity?.unit?.replace(/[{()}]/g, "");
-    const observationPoint: GroupedObservation = {
-      rawVital: v,
-      date,
-      observation:
-        typeof observationValue === "number" ? observationValue : parseInt(observationValue),
-      ...(observationUnit && { unit: observationUnit }),
-    };
+    const parsedValue =
+      typeof observationValue === "number"
+        ? observationValue
+        : parseFloat(String(observationValue));
+    if (!Number.isFinite(parsedValue)) {
+      results.push({ title, mostRecentObservation: v });
+      return;
+    }
+    const observationPoint: GroupedObservation = {
+      rawVital: v,
+      date,
+      observation: parsedValue,
+      ...(observationUnit && { unit: observationUnit }),
+    };

packages/core/src/external/ehr/athenahealth/command/write-back/condition.ts (2)

15-21: Don’t pick the first department arbitrarily.

Selecting departmentIds[0] is non-deterministic across envs. Prefer an explicit selection strategy (e.g., configured default write-back department, per-resource override, or stable sort by ID) and log which department is used.

I can wire a selectDepartmentId(secondaryMappings) helper with clear precedence rules.

---

27-33: Add observability around the write-back.

Log at debug level before/after createProblem with cxId, practiceId, patientId, and departmentId (no PHI). This eases triage without noisy multi-line logs.

packages/core/src/external/ehr/shared.ts (2)

413-422: Clarify GroupedObservation.date semantics

Define whether date must be ISO 8601 (full datetime) or date-only. A branded type alias (e.g., type IsoDateString = string & { __isoDate: true }) or a short doc comment reduces accidental mixing.

---

1053-1065: getValidCode is fine; consider broader acceptance in future

Current filter requires both code and display (not UNK/unknown). If some EHRs omit display but have valid code, you might want to keep those and fall back to code. Not blocking.

packages/core/src/external/ehr/command/write-back/grouped-vitals.ts (2)

74-96: Group-by-date: consider stable output ordering

Optional: sort the returned array by date for deterministic behavior (useful for tests/logs).

-  return Object.entries(groupedVitals).map(([chartDate, observations]) => [
-    buildDayjs(chartDate).toDate(),
-    observations,
-  ]);
+  return Object.entries(groupedVitals)
+    .sort(([a], [b]) => buildDayjs(a).diff(buildDayjs(b)))
+    .map(([chartDate, observations]) => [buildDayjs(chartDate).toDate(), observations]);

---

169-185: Dispatch by EHR — good; add future-proofing note

Throwing on unknown EHR is fine. If more EHRs are added, extend writeBackGroupedVitalsEhrs consistently.

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#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#4000
File: packages/core/src/external/ehr/athenahealth/index.ts:504-507
Timestamp: 2025-06-11T21:39:26.805Z
Learning: In AthenaHealth write-back (`packages/core/src/external/ehr/athenahealth/index.ts`), only the condition statuses “relapse” and “recurrence” are currently mapped to AthenaHealth problem statuses (“CHRONIC”); other FHIR clinicalStatus values (e.g., “active”, “resolved”, “inactive”, “remission”) are not yet supported.

Learnt from: thomasyopes
PR: metriport/metriport#4358
File: packages/core/src/external/ehr/command/write-back/shared.ts:21-21
Timestamp: 2025-08-12T17:28:06.150Z
Learning: In packages/core/src/external/ehr/command/write-back/shared.ts, the WriteBackResourceRequest.primaryResourceOrResources field deliberately uses the generic tuple [Date, Resource[]] instead of the more specific GroupedVitalsByDate type ([Date, Observation[]]) to keep the shared interface flexible for potentially grouping different resource types by date in the future.

Learnt from: thomasyopes
PR: metriport/metriport#4451
File: packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts:143-151
Timestamp: 2025-08-25T17:29:25.387Z
Learning: In packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts, the filterObservations function with vital.latestOnly=true is designed to keep only the most recent observation per LOINC code across all dates, not per date+LOINC pair. This behavior intentionally collapses multiple days into a single latest reading per code.

Learnt from: thomasyopes
PR: metriport/metriport#4164
File: packages/core/src/external/ehr/healthie/index.ts:1031-1076
Timestamp: 2025-07-09T17:18:28.920Z
Learning: In packages/core/src/external/ehr/healthie/index.ts, the `interpretation` field in `labObservationResult` is defined as a required enum in the Zod schema (`z.enum(["NORMAL", "ABNORMAL", "CRITICAL", "UNKNOWN"])`), making null-safety checks unnecessary when calling `toTitleCase(labObservationResult.interpretation)`.

Learnt from: thomasyopes
PR: metriport/metriport#4164
File: packages/core/src/external/ehr/healthie/index.ts:1031-1076
Timestamp: 2025-07-09T17:18:28.920Z
Learning: In packages/core/src/external/ehr/healthie/index.ts, the `interpretation` field in `labObservationResult` is defined as a required enum in the Zod schema (`z.enum(["high", "low", "normal"])`), making null-safety checks unnecessary when calling `toTitleCase(labObservationResult.interpretation)`.

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: leite08
PR: metriport/metriport#3463
File: packages/api/src/command/medical/patient/settings/create-patient-settings.ts:174-175
Timestamp: 2025-03-19T13:44:54.126Z
Learning: In the Metriport codebase, Lodash is used extensively and preferred for array and object manipulations. When working with batches of data, use Lodash's `_.chunk()` instead of manual array slicing with `array.slice(i, i + batchSize)`. The codebase also commonly uses other Lodash functions like `cloneDeep`, `uniq`, `uniqBy`, `groupBy`, and `orderBy`. Prefer Lodash functions over native JavaScript implementations for better readability and consistency across the codebase.

packages/core/src/external/ehr/athenahealth/command/write-back/condition.ts (1)

9-10: Token auth fix acknowledged; looks correct now.

Destructuring tokenInfo and passing it to createAthenaHealthClient addresses the earlier tokenId issue. ✅

Also applies to: 22-26

packages/core/src/external/ehr/shared.ts (1)

1079-1129: BP pairing is fragile and drops data; normalize timestamps and preserve singletons.

Raw string date keys can miss S/D pairing when timestamps differ slightly; non S/D BP entries are dropped.

Apply:

 export function handleBloodPressureMapping(obsMap: Map<string, GroupedObservation[]>) {
   const bloodPressure = obsMap.get("Blood Pressure");
   if (!bloodPressure) return;

   const groupedBloodPressure: GroupedObservation[] = [];

-  const systolicMap = new Map<string, number>();
-  const diastolicMap = new Map<string, number>();
+  const systolicMap = new Map<string, number>();
+  const diastolicMap = new Map<string, number>();

-  bloodPressure.forEach(bp => {
-    if (bp.grouping?.toLowerCase().includes("systolic")) {
-      systolicMap.set(bp.date, bp.observation);
-    } else if (bp.grouping?.toLowerCase().includes("diastolic")) {
-      diastolicMap.set(bp.date, bp.observation);
-    }
-  });
+  const toKey = (d: string) => buildDayjs(d).toISOString();
+  bloodPressure.forEach(bp => {
+    const lower = bp.grouping?.toLowerCase() ?? "";
+    const key = toKey(bp.date);
+    if (lower.includes("systolic")) {
+      systolicMap.set(key, bp.observation);
+    } else if (lower.includes("diastolic")) {
+      diastolicMap.set(key, bp.observation);
+    }
+  });

-  bloodPressure.forEach(bp => {
-    if (bp.grouping?.toLowerCase().includes("systolic")) {
-      const diastolicValue = diastolicMap.get(bp.date);
-      if (diastolicValue !== undefined) {
-        groupedBloodPressure.push({
-          ...bp,
-          bp: {
-            systolic: bp.observation,
-            diastolic: diastolicValue,
-          },
-        });
-      }
-    } else if (bp.grouping?.toLowerCase().includes("diastolic")) {
-      const systolicValue = systolicMap.get(bp.date);
-      if (systolicValue !== undefined) {
-        if (
-          !groupedBloodPressure.some(
-            gbp => gbp.date === bp.date && gbp.bp?.diastolic === bp.observation
-          )
-        ) {
-          groupedBloodPressure.push({
-            ...bp,
-            bp: {
-              systolic: systolicValue,
-              diastolic: bp.observation,
-            },
-          });
-        }
-      }
-    }
-  });
+  bloodPressure.forEach(bp => {
+    const lower = bp.grouping?.toLowerCase() ?? "";
+    const key = toKey(bp.date);
+    if (lower.includes("systolic")) {
+      const diastolicValue = diastolicMap.get(key);
+      if (diastolicValue !== undefined) {
+        const candidate = { ...bp, bp: { systolic: bp.observation, diastolic: diastolicValue } };
+        const dup = groupedBloodPressure.some(
+          gbp =>
+            toKey(gbp.date) === key &&
+            gbp.bp?.systolic === candidate.bp!.systolic &&
+            gbp.bp?.diastolic === candidate.bp!.diastolic
+        );
+        if (!dup) groupedBloodPressure.push(candidate);
+      }
+    } else if (lower.includes("diastolic")) {
+      const systolicValue = systolicMap.get(key);
+      if (systolicValue !== undefined) {
+        const candidate = { ...bp, bp: { systolic: systolicValue, diastolic: bp.observation } };
+        const dup = groupedBloodPressure.some(
+          gbp =>
+            toKey(gbp.date) === key &&
+            gbp.bp?.systolic === candidate.bp!.systolic &&
+            gbp.bp?.diastolic === candidate.bp!.diastolic
+        );
+        if (!dup) groupedBloodPressure.push(candidate);
+      }
+    }
+  });

-  obsMap.set("Blood Pressure", groupedBloodPressure);
+  // Preserve non S/D BP observations and any singletons that couldn't be paired
+  bloodPressure.forEach(bp => {
+    const lower = bp.grouping?.toLowerCase() ?? "";
+    const isSD = lower.includes("systolic") || lower.includes("diastolic");
+    const hasPair = groupedBloodPressure.some(gbp => toKey(gbp.date) === toKey(bp.date));
+    if (!isSD || !hasPair) groupedBloodPressure.push(bp);
+  });
+
+  obsMap.set("Blood Pressure", groupedBloodPressure);
 }

packages/core/src/external/ehr/athenahealth/command/write-back/condition.ts (1)

17-20: Add write-back context to error metadata.

Include writeBackResource: "condition" in BadRequestError additionalInfo to improve observability.

Apply:

   if (!athenaDepartmentId) {
-    throw new BadRequestError("Department ID not found", undefined, {
-      ehr: EhrSources.athena,
-      practiceId,
-    });
+    throw new BadRequestError("Department ID not found", undefined, {
+      ehr: EhrSources.athena,
+      practiceId,
+      writeBackResource: "condition",
+    });
   }

packages/core/src/external/ehr/shared.ts (1)

1066-1077: Broaden BMI match; keep case handling consistent.

Match “body mass index” too and avoid mixing lower with a case-insensitive regex target.

Apply:

 export function handleTitleSpecialCases(
   title: string,
   observationPoint: GroupedObservation
 ): string {
-  const lower = title.toLowerCase();
-  if (/(blood pressure|bp sys|bp dias|systolic|diastolic)/i.test(title)) {
+  const lower = title.toLowerCase();
+  if (/(blood pressure|bp sys|bp dias|systolic|diastolic)/i.test(lower)) {
     observationPoint.grouping = title;
     return "Blood Pressure";
   }
-  if (lower.includes("bmi")) return "Body Mass Index (BMI)";
+  if (/(bmi|body\s*mass\s*index)/i.test(lower)) return "Body Mass Index (BMI)";
   return title;
 }

packages/core/src/external/ehr/athenahealth/command/write-back/grouped-vitals.ts (1)

35-45: Add write-back context to error metadata (consistency).

If department is missing, include writeBackResource: "grouped-vitals" for easier triage; otherwise the flow looks good.

Apply:

   if (!athenaDepartmentId) {
-    throw new BadRequestError("Department ID not found", undefined, {
-      ehr: EhrSources.athena,
-      practiceId,
-    });
+    throw new BadRequestError("Department ID not found", undefined, {
+      ehr: EhrSources.athena,
+      practiceId,
+      writeBackResource: "grouped-vitals",
+    });
   }

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#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#4358
File: packages/core/src/external/ehr/command/write-back/shared.ts:21-21
Timestamp: 2025-08-12T17:28:06.150Z
Learning: In packages/core/src/external/ehr/command/write-back/shared.ts, the WriteBackResourceRequest.primaryResourceOrResources field deliberately uses the generic tuple [Date, Resource[]] instead of the more specific GroupedVitalsByDate type ([Date, Observation[]]) to keep the shared interface flexible for potentially grouping different resource types by date in the future.

Learnt from: thomasyopes
PR: metriport/metriport#4000
File: packages/core/src/external/ehr/athenahealth/index.ts:504-507
Timestamp: 2025-06-11T21:39:26.805Z
Learning: In AthenaHealth write-back (`packages/core/src/external/ehr/athenahealth/index.ts`), only the condition statuses “relapse” and “recurrence” are currently mapped to AthenaHealth problem statuses (“CHRONIC”); other FHIR clinicalStatus values (e.g., “active”, “resolved”, “inactive”, “remission”) are not yet supported.

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#4478
File: packages/core/src/external/ehr/command/write-back/grouped-vitals.ts:10-10
Timestamp: 2025-08-27T23:13:51.351Z
Learning: In `packages/lambdas/src/shared/ehr.ts`, the `tokenId` field in `ehrCreateResourceDiffBundlesSchema` is intentionally kept as legacy support with a TODO comment referencing ENG-937, allowing existing queue messages to be processed during the tokenId → tokenInfo migration. This is not a missing update but intentional technical debt to be cleaned up once all queue messages have been processed.

Learnt from: leite08
PR: metriport/metriport#3942
File: packages/core/src/external/fhir/__tests__/location.ts:6-9
Timestamp: 2025-06-01T13:48:26.600Z
Learning: In TypeScript test code, don't flag deconstructed object parameters as needing simplification or refactoring. Test helper functions with nested optional object parameters (like `param?: { patient?: PatientWithId; location?: Partial<Location>; }`) are acceptable and should not be suggested for flattening.

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#4451
File: packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts:143-151
Timestamp: 2025-08-25T17:29:25.387Z
Learning: In packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts, the filterObservations function with vital.latestOnly=true is designed to keep only the most recent observation per LOINC code across all dates, not per date+LOINC pair. This behavior intentionally collapses multiple days into a single latest reading per code.

Learnt from: thomasyopes
PR: metriport/metriport#4164
File: packages/core/src/external/ehr/healthie/index.ts:1031-1076
Timestamp: 2025-07-09T17:18:28.920Z
Learning: In packages/core/src/external/ehr/healthie/index.ts, the `interpretation` field in `labObservationResult` is defined as a required enum in the Zod schema (`z.enum(["NORMAL", "ABNORMAL", "CRITICAL", "UNKNOWN"])`), making null-safety checks unnecessary when calling `toTitleCase(labObservationResult.interpretation)`.

Learnt from: thomasyopes
PR: metriport/metriport#4164
File: packages/core/src/external/ehr/healthie/index.ts:1031-1076
Timestamp: 2025-07-09T17:18:28.920Z
Learning: In packages/core/src/external/ehr/healthie/index.ts, the `interpretation` field in `labObservationResult` is defined as a required enum in the Zod schema (`z.enum(["high", "low", "normal"])`), making null-safety checks unnecessary when calling `toTitleCase(labObservationResult.interpretation)`.

Learnt from: leite08
PR: metriport/metriport#3463
File: packages/api/src/command/medical/patient/settings/create-patient-settings.ts:174-175
Timestamp: 2025-03-19T13:44:54.126Z
Learning: In the Metriport codebase, Lodash is used extensively and preferred for array and object manipulations. When working with batches of data, use Lodash's `_.chunk()` instead of manual array slicing with `array.slice(i, i + batchSize)`. The codebase also commonly uses other Lodash functions like `cloneDeep`, `uniq`, `uniqBy`, `groupBy`, and `orderBy`. Prefer Lodash functions over native JavaScript implementations for better readability and consistency across the codebase.

packages/core/src/external/ehr/elation/index.ts (2)

624-629: Tighten flatMap result type.

Return an array consistently to avoid union typing and improve readability.

-    const vitals = observations.flatMap(observation => {
-      const formattedVital = this.formatGroupedVital(observation);
-      if (!formattedVital) return [];
-      return formattedVital;
-    });
+    const vitals = observations.flatMap(observation => {
+      const formattedVital = this.formatGroupedVital(observation);
+      return formattedVital ? [formattedVital] : [];
+    });

---

533-534: Typo in additionalInfo key: diagnostricReportId → diagnosticReportId.

Fix the log/metadata field name.

-      diagnostricReportId: diagnosticReport.id,
+      diagnosticReportId: diagnosticReport.id,

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#4358
File: packages/core/src/external/ehr/command/write-back/shared.ts:21-21
Timestamp: 2025-08-12T17:28:06.150Z
Learning: In packages/core/src/external/ehr/command/write-back/shared.ts, the WriteBackResourceRequest.primaryResourceOrResources field deliberately uses the generic tuple [Date, Resource[]] instead of the more specific GroupedVitalsByDate type ([Date, Observation[]]) to keep the shared interface flexible for potentially grouping different resource types by date in the future.

packages/core/src/external/ehr/command/write-back/grouped-vitals.ts (1)

118-133: Bug: drops decimals and treats 0 as missing; also misses end-date fallback

Replace parseInt with parseFloat and use explicit null checks; use getObservationObservedDate for full fallback.

Apply:

-    const observationValue = v.valueQuantity?.value ?? v.valueString;
-    const date = v.effectiveDateTime ?? v.effectivePeriod?.start;
-    if (!date || !observationValue) {
+    const observationValue = v.valueQuantity?.value ?? v.valueString;
+    const date = getObservationObservedDate(v);
+    if (!date || observationValue == null) {
       results.push({ title, mostRecentObservation: v });
       return;
     }
 
-    const observationUnit = v.valueQuantity?.unit?.replace(/[{()}]/g, "");
-    const observationPoint: GroupedObservation = {
-      rawVital: v,
-      date,
-      observation:
-        typeof observationValue === "number" ? observationValue : parseInt(observationValue),
-      ...(observationUnit && { unit: observationUnit }),
-    };
+    const observationUnit = v.valueQuantity?.unit?.replace(/[{()}]/g, "")?.trim();
+    const parsedValue =
+      typeof observationValue === "number"
+        ? observationValue
+        : parseFloat(String(observationValue));
+    if (!Number.isFinite(parsedValue)) {
+      results.push({ title, mostRecentObservation: v });
+      return;
+    }
+    const observationPoint: GroupedObservation = {
+      rawVital: v,
+      date,
+      observation: parsedValue,
+      ...(observationUnit && { unit: observationUnit }),
+    };

packages/core/src/external/ehr/command/write-back/condition.ts (1)

5-6: Wrap long imports to respect the 100-char max line length.
packages/core/src/external/ehr/command/write-back/condition.ts:5–6

-import { writeBackCondition as writeBackConditionAthena } from "../../athenahealth/command/write-back/condition";
+import {
+  writeBackCondition as writeBackConditionAthena,
+} from "../../athenahealth/command/write-back/condition";

-import { writeBackCondition as writeBackConditionElation } from "../../elation/command/write-back/condition";
+import {
+  writeBackCondition as writeBackConditionElation,
+} from "../../elation/command/write-back/condition";

packages/core/src/external/ehr/command/write-back/grouped-vitals.ts (4)

105-144: Prefer forEach/for..of over map for side-effects

map’s return is unused here; forEach or a simple for..of is clearer.

Apply:

-  observations.map(v => {
+  observations.forEach(v => {
     // ...
-  });
+  });

---

125-132: Tiny robustness: trim unit and keep only printable ASCII

Some EHRs include stray whitespace or braces in units. You’re already stripping braces; trimming helps too (included in the earlier diff). Optionally, normalize multiple spaces.

---

98-143: Potential conflation: grouping key is display title, not LOINC

Different LOINC codes can share identical display text, merging unrelated series under one title. Consider grouping by LOINC when present and using the title just for labeling.

Would you like me to draft a minimal change that uses a grouping key of ${title}::${loincCode} when a LOINC is available while keeping title unchanged in the returned payload?

Also applies to: 147-165

---

35-37: Minor: avoid return await

Not harmful, but return handler({ ...params }) is equivalent and avoids an extra tick.

-  return await handler({ ...params });
+  return handler({ ...params });

packages/core/src/external/ehr/athenahealth/command/write-back/lab.ts (2)

15-15: Avoid implicit “first department” selection

If multiple departmentIds are configured, defaulting to index 0 may attach to the wrong department. Prefer an explicit default or document the convention inline for maintainers.

Minimal clarification:

-  const athenaDepartmentId = secondaryMappings.departmentIds[0];
+  const { departmentIds } = secondaryMappings;
+  // NOTE: if multiple departmentIds are configured, we default to the first by convention.
+  const athenaDepartmentId = departmentIds[0];

---

16-21: Enrich error context for triage

Include configured departmentIds in the error’s additionalInfo to speed up support/debugging.

-    throw new BadRequestError("Department ID not found", undefined, {
-      ehr: EhrSources.athena,
-      practiceId,
-    });
+    throw new BadRequestError("Department ID not found", undefined, {
+      ehr: EhrSources.athena,
+      practiceId,
+      departmentIds: secondaryMappings.departmentIds,
+    });

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#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#4000
File: packages/core/src/external/ehr/athenahealth/index.ts:504-507
Timestamp: 2025-06-11T21:39:26.805Z
Learning: In AthenaHealth write-back (`packages/core/src/external/ehr/athenahealth/index.ts`), only the condition statuses “relapse” and “recurrence” are currently mapped to AthenaHealth problem statuses (“CHRONIC”); other FHIR clinicalStatus values (e.g., “active”, “resolved”, “inactive”, “remission”) are not yet supported.

Learnt from: thomasyopes
PR: metriport/metriport#4451
File: packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts:143-151
Timestamp: 2025-08-25T17:29:25.387Z
Learning: In packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts, the filterObservations function with vital.latestOnly=true is designed to keep only the most recent observation per LOINC code across all dates, not per date+LOINC pair. This behavior intentionally collapses multiple days into a single latest reading per code.

Learnt from: thomasyopes
PR: metriport/metriport#4358
File: packages/core/src/external/ehr/command/write-back/shared.ts:21-21
Timestamp: 2025-08-12T17:28:06.150Z
Learning: In packages/core/src/external/ehr/command/write-back/shared.ts, the WriteBackResourceRequest.primaryResourceOrResources field deliberately uses the generic tuple [Date, Resource[]] instead of the more specific GroupedVitalsByDate type ([Date, Observation[]]) to keep the shared interface flexible for potentially grouping different resource types by date in the future.

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: thomasyopes
PR: metriport/metriport#4164
File: packages/core/src/external/ehr/healthie/index.ts:1031-1076
Timestamp: 2025-07-09T17:18:28.920Z
Learning: In packages/core/src/external/ehr/healthie/index.ts, the `interpretation` field in `labObservationResult` is defined as a required enum in the Zod schema (`z.enum(["NORMAL", "ABNORMAL", "CRITICAL", "UNKNOWN"])`), making null-safety checks unnecessary when calling `toTitleCase(labObservationResult.interpretation)`.

Learnt from: thomasyopes
PR: metriport/metriport#4164
File: packages/core/src/external/ehr/healthie/index.ts:1031-1076
Timestamp: 2025-07-09T17:18:28.920Z
Learning: In packages/core/src/external/ehr/healthie/index.ts, the `interpretation` field in `labObservationResult` is defined as a required enum in the Zod schema (`z.enum(["high", "low", "normal"])`), making null-safety checks unnecessary when calling `toTitleCase(labObservationResult.interpretation)`.

packages/api/src/sequelize/migrations/2025-08-29_00_add-secondary-mappings-to-patient-mapping.ts (4)

4-6: Prefer a more explicit constant name than “column”.

Improves grep-ability and avoids ambiguity when multiple columns are involved.

-const column = "secondary_mappings";
+const secondaryMappingsColumn = "secondary_mappings";
@@
-      tableName,
-      column,
+      tableName,
+      secondaryMappingsColumn,
@@
-    await queryInterface.removeColumn(tableName, column, { transaction });
+    await queryInterface.removeColumn(tableName, secondaryMappingsColumn, { transaction });

Also applies to: 11-12, 20-20

---

7-16: Up migration is safe and transactional. Consider adding a DB-level column comment.

Nullable JSONB with no default avoids table rewrite; good for production. Optional: add a COMMENT for future maintainers.

     await queryInterface.addColumn(
       tableName,
-      column,
+      secondaryMappingsColumn,
       { type: DataTypes.JSONB, allowNull: true },
       { transaction }
     );
+    await queryInterface.sequelize.query(
+      "COMMENT ON COLUMN patient_mapping.secondary_mappings IS " +
+        "'Secondary EHR-specific mappings used by write-back flows (e.g., Athena).'",
+      { transaction }
+    );

---

18-22: Optional guard in down() to be resilient if partially applied.

Not required, but avoids failures if the column was already removed in a partial rollback.

-export const down: Migration = ({ context: queryInterface }) => {
-  return queryInterface.sequelize.transaction(async transaction => {
-    await queryInterface.removeColumn(tableName, column, { transaction });
-  });
-};
+export const down: Migration = ({ context: queryInterface }) => {
+  return queryInterface.sequelize.transaction(async transaction => {
+    const table = await queryInterface.describeTable(tableName);
+    if (table.secondary_mappings) {
+      await queryInterface.removeColumn(tableName, secondaryMappingsColumn, { transaction });
+    }
+  });
+};

---

4-16: Future-readiness: index only if you’ll query inside JSON.

If lookups will filter by specific JSON paths (e.g., departmentId inside secondary_mappings), consider a GIN index or a functional index later. Skip for now if reads are by PK and the JSON is only fetched.

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: leite08
PR: metriport/metriport#3412
File: packages/api/src/models/medical/patient-import-mapping.ts:16-41
Timestamp: 2025-04-26T22:17:45.248Z
Learning: The patient_import_mapping table already has a unique constraint on the fields "job_id", "row_number", and "data_pipeline_request_id" defined in the migration file 2025-04-24_02_create-patient_import_mapping-table.ts, which ensures data integrity.

Learnt from: RamilGaripov
PR: metriport/metriport#3962
File: packages/api/src/models/medical/patient-cohort.ts:15-21
Timestamp: 2025-06-18T08:03:50.674Z
Learning: In the Metriport codebase, Sequelize models typically don't define `allowNull: false` or `references` metadata for foreign key columns. These constraints are handled exclusively in migration files, keeping models simpler and avoiding duplication. This is a consistent pattern across recent models like CohortModel, CoverageEnhancementModel, DocRefMappingModel, etc. The migration files properly define all FK constraints, NOT NULL requirements, and cascade behaviors.

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: leite08
PR: metriport/metriport#3749
File: packages/api/src/models/medical/patient-import-mapping.ts:16-41
Timestamp: 2025-04-28T22:58:19.669Z
Learning: For Sequelize models in this codebase, field constraints (such as `allowNull: false`) and field mappings (such as `field: "column_name"`) are defined in the migrations, not in the model definitions themselves.

Learnt from: leite08
PR: metriport/metriport#3624
File: packages/api/src/command/medical/patient/get-patient.ts:90-97
Timestamp: 2025-04-07T17:32:22.935Z
Learning: When dealing with raw SQL queries in Sequelize that return complex data structures (especially with joins and JSON aggregation), using `any` type assertions may be necessary since TypeScript cannot infer the types. Using constants for column aliases helps minimize maintenance issues in these cases.

Learnt from: RamilGaripov
PR: metriport/metriport#3962
File: packages/api/src/models/medical/patient-cohort.ts:15-21
Timestamp: 2025-06-18T08:03:50.674Z
Learning: In the Metriport codebase, Sequelize models typically don't define `allowNull: false` or `references` metadata for foreign key columns. These constraints are handled exclusively in migration files, keeping models simpler and avoiding duplication. This is a consistent pattern across recent models like CohortModel, CoverageEnhancementModel, DocRefMappingModel, etc.

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

1681-1693: By-ID short-circuit now forwards resourceId — good fix.

Prevents cache-key collisions for by-id lookups. Consider Set-based predicate here too for parity.

-    if (resourceType === "MedicationStatement") {
+    if (writeBackOnlyResources.has(resourceType as ResourceType)) {
       return await fetchEhrBundleUsingCache({
         ehr: EhrSources.athena,
         cxId,
         metriportPatientId,
         ehrPatientId: athenaPatientId,
         resourceType,
         resourceId,
         fetchResourcesFromEhr: () => Promise.resolve([]),
         useCachedBundle,
       });
     }

packages/core/src/external/ehr/command/write-back/grouped-vitals.ts (2)

149-151: Keep sort logic as-is for parity with internal repo.

Per prior guidance, do not change this comparator.

---

118-133: Vitals parsing bug: decimals dropped; zero treated as missing.

parseInt truncates; falsy check excludes 0. This corrupts values and skips valid observations.

Apply:

-    const observationValue = v.valueQuantity?.value ?? v.valueString;
+    const observationValue = v.valueQuantity?.value ?? v.valueString;
     const date = v.effectiveDateTime ?? v.effectivePeriod?.start;
-    if (!date || !observationValue) {
+    if (!date || observationValue == null) {
       results.push({ title, mostRecentObservation: v });
       return;
     }
-
-    const observationPoint: GroupedObservation = {
-      rawVital: v,
-      date,
-      observation:
-        typeof observationValue === "number" ? observationValue : parseInt(observationValue),
-      // TODO: Make sure all data points have the same unit
-      unit: v.valueQuantity?.unit?.replace(/[{()}]/g, ""),
-    };
+    const parsedValue =
+      typeof observationValue === "number"
+        ? observationValue
+        : parseFloat(String(observationValue));
+    if (!Number.isFinite(parsedValue)) {
+      results.push({ title, mostRecentObservation: v });
+      return;
+    }
+    const observationPoint: GroupedObservation = {
+      rawVital: v,
+      date,
+      observation: parsedValue,
+      // TODO: Make sure all data points have the same unit
+      unit: v.valueQuantity?.unit?.replace(/[{()}]/g, ""),
+    };

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

232-239: Centralize write-back–only resources to avoid stringly-typed checks.

Define a Set and reuse it across scopes and short-circuits, instead of hardcoding "MedicationStatement" in multiple places.

Apply the Set definition near resource lists:

 export const supportedAthenaHealthResources: ResourceType[] = [
   "AllergyIntolerance",
   "Condition",
   "DiagnosticReport",
   "Immunization",
   "MedicationRequest",
-  "MedicationStatement", // NOT REALLY SUPPORTED
+  "MedicationStatement", // write-back only (no read support)
   "Observation",
   "Procedure",
   "Encounter",
 ];
+
+// Resources we allow for write-back flows but do not fetch from Athena
+const writeBackOnlyResources = new Set<ResourceType>(["MedicationStatement"]);

---

254-258: Derive OAuth scopes from the Set (avoid drift).

Use the writeBackOnlyResources Set so the filtering logic stays consistent with future additions.

-export const scopes = [
-  ...supportedAthenaHealthResources,
-  ...supportedAthenaHealthReferenceResources,
-].filter(resource => resource !== "MedicationStatement");
+export const scopes = [
+  ...supportedAthenaHealthResources,
+  ...supportedAthenaHealthReferenceResources,
+].filter(resource => !writeBackOnlyResources.has(resource));

---

436-476: Include query params in additionalInfo for better observability.

Helps debug issues (e.g., department context).

-    const additionalInfo = { cxId, practiceId: this.practiceId, patientId };
+    const additionalInfo = {
+      cxId,
+      practiceId: this.practiceId,
+      patientId,
+      queryParams: queryParams.toString(),
+    };

---

1558-1569: Short-circuit for write-back–only types is correct; reuse the Set.

This returns a cached empty bundle as intended; switch to Set-based check for consistency.

-    if (resourceType === "MedicationStatement") {
+    if (writeBackOnlyResources.has(resourceType as ResourceType)) {
       return await fetchEhrBundleUsingCache({
         ehr: EhrSources.athena,
         cxId,
         metriportPatientId,
         ehrPatientId: athenaPatientId,
         resourceType,
         fetchResourcesFromEhr: () => Promise.resolve([]),
         useCachedBundle,
       });
     }

packages/core/src/external/ehr/command/write-back/shared.ts (2)

95-117: Lab-panel: empty secondary array passes validation. Confirm intent.

Array.every returns true for [], so an empty secondaries list will pass and write back with 0 observations.

If you want to require at least one Observation:

-    if (
-      !params.secondaryResourceOrResources ||
-      !params.secondaryResourceOrResources.every(isObservation)
-    ) {
+    if (
+      !Array.isArray(params.secondaryResourceOrResources) ||
+      params.secondaryResourceOrResources.length < 1 ||
+      !params.secondaryResourceOrResources.every(isObservation)
+    ) {

---

123-162: Medication-statement: unify error text and single guard for “no medications”.

Minor DX: duplicate error and a follow-up empty check.

Apply:

-    if (
-      !params.secondaryResourceOrResources ||
-      !params.secondaryResourceOrResources.every(isMedication)
-    ) {
-      throw new BadRequestError(
-        "MedicationStatement write back requires secondary resources",
-        undefined,
-        {
-          ehr: params.ehr,
-          writeBackResource: params.writeBackResource,
-        }
-      );
-    }
-    const medication = params.secondaryResourceOrResources[0];
-    if (!medication) {
+    if (
+      !Array.isArray(params.secondaryResourceOrResources) ||
+      params.secondaryResourceOrResources.length < 1 ||
+      !params.secondaryResourceOrResources.every(isMedication)
+    ) {
       throw new BadRequestError(
-        "MedicationStatement write back requires secondary resources",
+        "MedicationStatement write back requires at least one Medication in secondary resources",
         undefined,
         {
           ehr: params.ehr,
           writeBackResource: params.writeBackResource,
         }
       );
     }
+    const medication = params.secondaryResourceOrResources[0];

packages/core/src/external/ehr/command/write-back/grouped-vitals.ts (1)

105-116: Iteration nit: use forEach instead of map when not using return value.

Slight readability win, no behavior change.

-  observations.map(v => {
+  observations.forEach(v => {
     ...
-  });
+  });

Also applies to: 143-144

packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts (1)

304-330: Hydration is O(M×S); index statements by medication for O(M+S).

Improves scalability without changing behavior.

Apply:

-async function hydrateMedications({
-  medications,
-  statements,
-}: {
-  medications: Medication[];
-  statements: MedicationStatement[];
-}): Promise<{ medication: Medication; statements: MedicationStatement[] }[]> {
-  const hydratedMetriportOnlyResources: {
-    medication: Medication;
-    statements: MedicationStatement[];
-  }[] = [];
-  for (const medication of medications) {
-    if (!medication.id) {
-      hydratedMetriportOnlyResources.push({ medication, statements: [] });
-      continue;
-    }
-    const hydratedStatements: MedicationStatement[] = [];
-    for (const statement of statements) {
-      if (!statement.medicationReference || !statement.medicationReference.reference) continue;
-      const ref = statement.medicationReference.reference;
-      if (ref !== `Medication/${medication.id}`) continue;
-      hydratedStatements.push(statement);
-    }
-    hydratedMetriportOnlyResources.push({ medication, statements: hydratedStatements });
-  }
-  return hydratedMetriportOnlyResources;
-}
+async function hydrateMedications({
+  medications,
+  statements,
+}: {
+  medications: Medication[];
+  statements: MedicationStatement[];
+}): Promise<{ medication: Medication; statements: MedicationStatement[] }[]> {
+  const byMedicationId = new Map<string, MedicationStatement[]>();
+  for (const s of statements) {
+    const ref = s.medicationReference?.reference; // "Medication/{id}"
+    if (!ref?.startsWith("Medication/")) continue;
+    const id = ref.slice("Medication/".length);
+    if (!id) continue;
+    const arr = byMedicationId.get(id) ?? [];
+    arr.push(s);
+    byMedicationId.set(id, arr);
+  }
+  return medications.map(m => ({
+    medication: m,
+    statements: m.id ? byMedicationId.get(m.id) ?? [] : [],
+  }));
+}

packages/api/src/external/ehr/athenahealth/command/update-patient-mapping-department-id.ts (3)

5-10: Param type is clear; consider immutability.
Optional: mark fields as readonly to signal intent and help TS catch accidental mutation.

-export type UpdateAthenaPatientMappingDepartmentIdParams = {
-  cxId: string;
-  athenaPatientId: string;
-  athenaDepartmentId: string;
-};
+export type UpdateAthenaPatientMappingDepartmentIdParams = {
+  readonly cxId: string;
+  readonly athenaPatientId: string;
+  readonly athenaDepartmentId: string;
+};

---

11-15: Doc should clarify persistence vs. context-bound department usage.
Per retrieved learnings, departmentId is context-sensitive in Athena (patient can be opened from multiple departments). Please state in the docstring whether persisting departmentId in secondaryMappings is the intended long-term model, or if this is a transitional mechanism for write-back only.

---

21-26: Tighten typing by specializing the generic to Athena’s mappings.
Without binding T, parsedSecondaryMappings may be inferred as a broad union, weakening type-safety around departmentId. Import the Athena-specific type and specialize the call.

-import { EhrSources } from "@metriport/shared/interface/external/ehr/source";
+import { EhrSources } from "@metriport/shared/interface/external/ehr/source";
+import { AthenaPatientMappingSecondaryMappings } from "../shared";
@@
-  const { parsedSecondaryMappings, patientMapping } =
-    await getPatientMappingAndParsedSecondaryMappings({
+  const { parsedSecondaryMappings, patientMapping } =
+    await getPatientMappingAndParsedSecondaryMappings<AthenaPatientMappingSecondaryMappings>({
       ehr: EhrSources.athena,
       cxId,
       ehrPatientId: athenaPatientId,
     });