Eng 296 athena writeback #3936
Eng 296 athena writeback #3936
Conversation
Ref: ENG-296 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-296 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-296 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-296 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
WalkthroughThis update introduces new "write-back" functionality for Athenahealth EHR integration, enabling the system to record procedures, immunizations, allergies, lab results, medications, vitals, and clinical notes via newly added API endpoints and command functions. The AthenaHealthApi class is extended with methods for creating and searching clinical data, supported by new FHIR coding extraction utilities and expanded Zod schemas for data validation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Router
participant WriteBackCommand
participant AthenaHealthApi
participant AthenahealthEHR
Client->>API_Router: POST /ehr/athenahealth/chart/:id/(procedure|immunization|allergy|lab|note|medication)
API_Router->>WriteBackCommand: write*ToChart({ids, data})
WriteBackCommand->>AthenaHealthApi: create*(patientId, departmentId, data)
AthenaHealthApi->>AthenahealthEHR: [API call] Create resource
AthenahealthEHR-->>AthenaHealthApi: Creation result
AthenaHealthApi-->>WriteBackCommand: Created*Success
WriteBackCommand-->>API_Router: Created*Success
API_Router-->>Client: HTTP 200 + Created*Success
sequenceDiagram
participant AthenaHealthApi
participant ExternalAPI
AthenaHealthApi->>ExternalAPI: Search for medication/allergen (parallel requests)
ExternalAPI-->>AthenaHealthApi: List of matches
AthenaHealthApi->>AthenaHealthApi: Select best match using Jaro-Winkler similarity
AthenaHealthApi-->>AthenaHealthApi: Return best match or error
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error code ERR_SSL_WRONG_VERSION_NUMBER 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
🧠 Learnings (1)packages/core/src/external/ehr/athenahealth/index.ts (2)🧬 Code Graph Analysis (1)packages/core/src/external/ehr/athenahealth/index.ts (9)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (14)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Ref: ENG-296 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
… eng-296-athena-writeback Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-296 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-296 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
There was a problem hiding this comment.
"allergry" mispelled.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/api/src/external/ehr/athenahealth/command/write-back/allergry.ts (1)
1-1: Fix the filename typo.The filename contains a spelling error - "allergry" should be "allergy".
🧹 Nitpick comments (9)
packages/core/src/external/ehr/shared.ts (3)
338-347:parseFloatsilently mis-parses scientific notation
parseFloat("5e-3")returns5, not0.005.
Consider a more robust numeric coercion to avoid wrong lab values:- const parsedNumber = parseFloat(observation.valueString); + const parsedNumber = Number(observation.valueString);
Number()understands scientific notation and fails withNaNfor pure text, preserving current behaviour while increasing accuracy.
446-481: Remove unreachable code ingetObservationInterpretationAfter the early
returnwhenexplicitInterpretationis truthy, the later
check at lines 479-480 can never execute with a truthy value, so the branch is
dead code:- if (highInterpretations.includes(explicitInterpretation?.toLowerCase() ?? "")) return "high"; - return undefined; + return undefined;Eliminating it clarifies intent and avoids confusion when maintaining the
function.
525-533: Variable naming mismatch (loincCodingvs CPT)Inside
getProcedureCptCoding, the local variable is namedloincCoding
although we are searching for CPT codes. Rename to convey the correct medical
terminology and improve readability:- const loincCoding = code?.coding?.find(coding => { + const cptCoding = code?.coding?.find(coding => { ... - if (!loincCoding) return undefined; - return loincCoding; + if (!cptCoding) return undefined; + return cptCoding;packages/api/src/external/ehr/athenahealth/command/write-back/immunization.ts (1)
18-25: Consider adding error handling and validation.The implementation is clean and follows the async/await pattern. However, consider enhancing it with:
- Input validation: Validate required parameters before proceeding
- Error handling: Add try-catch to handle potential errors from the API client
- Error context: Ensure errors include sufficient context for debugging
export async function writeImmunizationToChart({ cxId, athenaPatientId, athenaPracticeId, athenaDepartmentId, immunization, }: { cxId: string; athenaPatientId: string; athenaPracticeId: string; athenaDepartmentId: string; immunization: Immunization; }): Promise<CreatedVaccinesSuccess> { + if (!cxId || !athenaPatientId || !athenaPracticeId || !athenaDepartmentId) { + throw new Error("Missing required parameters for immunization write-back"); + } + + try { const api = await createAthenaClient({ cxId, practiceId: athenaPracticeId }); return await api.createVaccine({ cxId, patientId: athenaPatientId, departmentId: athenaDepartmentId, immunization, }); + } catch (error) { + throw new Error("Failed to write immunization to chart", { cause: error }); + } }packages/api/src/external/ehr/athenahealth/command/write-back/lab.ts (1)
1-26: Consider refactoring to reduce code duplication.All four write-back functions (
writeAllergyToChart,writeNoteToChart,writeProcedureToChart,writeLabToChart) follow an identical pattern:
- Create Athena client
- Call specific API method
- Return result
Consider creating a generic helper function to reduce duplication:
async function writeToChart<TInput, TOutput>( params: { cxId: string; athenaPatientId: string; athenaPracticeId: string; athenaDepartmentId: string; }, data: TInput, apiMethod: (client: AthenaHealthApi, params: any) => Promise<TOutput> ): Promise<TOutput> { const api = await createAthenaClient({ cxId: params.cxId, practiceId: params.athenaPracticeId }); return await apiMethod(api, { ...params, patientId: params.athenaPatientId, departmentId: params.athenaDepartmentId, ...data }); }This would align with the DRY principle mentioned in the coding guidelines.
packages/api/src/routes/ehr/athenahealth/chart.ts (1)
236-243: Strengthen Zod validation fornoteSchema
z.string()alone accepts anything – including empty strings or malformed dates. A small upgrade guards the downstreamformatDate()call from throwing:-const noteSchema = z.object({ - date: z.string(), - encounterText: z.string(), -}); +const noteSchema = z.object({ + date: z.string().datetime(), // ISO-8601 check + encounterText: z.string().min(1), // forbid empty notes +});Alternatively
z.coerce.date()can yield aDatedirectly and avoid manual formatting later.Also applies to: 255-268
packages/core/src/external/ehr/athenahealth/index.ts (3)
1097-1177:searchForMedication/searchForAllergenmay return an arbitrary match when similarity ties
calculateJaroWinklerSimilarityreturns the first string that matches the current max score.
When two options have an identical similarity (common for short drug names), the function silently keeps the earliest one, which might be incorrect.Consider:
// after loop, gather all with maxSimilarity and decide deterministically const best = options .map(o => ({ o, score: jaroWinkler(target, o) })) .reduce(/* choose strictly highest, or pick lowest medicationid, etc. */);At minimum, document the tie-break behaviour so downstream consumers know the limitations.
1629-1645: Guard against emptysearchValues– current BadRequestError unreachableThe split/filter logic guarantees
searchValuesWithAtLeastTwoParts.length≥ 0, never negative; so theifcheck is functionally “length === 0”.
That is fine, but the error message “No search values with at least two parts” is misleading when the real cause is usually a one-word display string.Add a unit-test and clarify wording:
-if (searchValuesWithAtLeastTwoParts.length < 1) { - throw new BadRequestError("No search values with at least two parts", …); +if (searchValuesWithAtLeastTwoParts.length === 0) { + throw new BadRequestError("Display value too short to derive search terms", …); }
140-145: Hard-coded document IDs risk future breakage
labResultDocumentId = "386265"andclinicalNoteDocumentId = "423482"are practice-specific and can change without notice.
Prefer moving these to configuration (env vars or DB) or fetch them dynamically via/reference/documentsto avoid silently failing write-backs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
packages/api/src/external/ehr/athenahealth/command/write-back/allergry.ts(1 hunks)packages/api/src/external/ehr/athenahealth/command/write-back/candition.ts(1 hunks)packages/api/src/external/ehr/athenahealth/command/write-back/immunization.ts(1 hunks)packages/api/src/external/ehr/athenahealth/command/write-back/lab.ts(1 hunks)packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts(1 hunks)packages/api/src/external/ehr/athenahealth/command/write-back/note.ts(1 hunks)packages/api/src/external/ehr/athenahealth/command/write-back/procedure.ts(1 hunks)packages/api/src/external/ehr/athenahealth/command/write-back/vitals.ts(1 hunks)packages/api/src/routes/ehr/athenahealth/chart.ts(2 hunks)packages/core/src/external/ehr/athenahealth/index.ts(18 hunks)packages/core/src/external/ehr/shared.ts(7 hunks)packages/shared/src/interface/external/ehr/athenahealth/allergy.ts(1 hunks)packages/shared/src/interface/external/ehr/athenahealth/clinical-document.ts(1 hunks)packages/shared/src/interface/external/ehr/athenahealth/index.ts(1 hunks)packages/shared/src/interface/external/ehr/athenahealth/lab-result.ts(1 hunks)packages/shared/src/interface/external/ehr/athenahealth/medication.ts(1 hunks)packages/shared/src/interface/external/ehr/athenahealth/surgical-history.ts(1 hunks)packages/shared/src/interface/external/ehr/athenahealth/vaccine.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/api/src/external/ehr/athenahealth/command/write-back/vitals.tspackages/api/src/external/ehr/athenahealth/command/write-back/candition.tspackages/shared/src/interface/external/ehr/athenahealth/index.tspackages/shared/src/interface/external/ehr/athenahealth/medication.tspackages/api/src/external/ehr/athenahealth/command/write-back/note.tspackages/shared/src/interface/external/ehr/athenahealth/clinical-document.tspackages/api/src/external/ehr/athenahealth/command/write-back/allergry.tspackages/shared/src/interface/external/ehr/athenahealth/vaccine.tspackages/api/src/external/ehr/athenahealth/command/write-back/lab.tspackages/api/src/external/ehr/athenahealth/command/write-back/procedure.tspackages/shared/src/interface/external/ehr/athenahealth/lab-result.tspackages/api/src/external/ehr/athenahealth/command/write-back/immunization.tspackages/shared/src/interface/external/ehr/athenahealth/surgical-history.tspackages/api/src/external/ehr/athenahealth/command/write-back/medication.tspackages/api/src/routes/ehr/athenahealth/chart.tspackages/shared/src/interface/external/ehr/athenahealth/allergy.tspackages/core/src/external/ehr/shared.tspackages/core/src/external/ehr/athenahealth/index.ts
🧬 Code Graph Analysis (5)
packages/api/src/external/ehr/athenahealth/command/write-back/allergry.ts (2)
packages/shared/src/interface/external/ehr/athenahealth/allergy.ts (1)
CreatedAllergySuccess(11-11)packages/api/src/external/ehr/athenahealth/shared.ts (1)
createAthenaClient(31-40)
packages/api/src/external/ehr/athenahealth/command/write-back/lab.ts (2)
packages/shared/src/interface/external/ehr/athenahealth/lab-result.ts (1)
CreatedLabResultSuccess(13-13)packages/api/src/external/ehr/athenahealth/shared.ts (1)
createAthenaClient(31-40)
packages/api/src/external/ehr/athenahealth/command/write-back/immunization.ts (2)
packages/shared/src/interface/external/ehr/athenahealth/vaccine.ts (1)
CreatedVaccinesSuccess(10-10)packages/api/src/external/ehr/athenahealth/shared.ts (1)
createAthenaClient(31-40)
packages/api/src/routes/ehr/athenahealth/chart.ts (6)
packages/terminology/src/util.ts (1)
asyncHandler(6-22)packages/api/src/external/ehr/athenahealth/command/write-back/procedure.ts (1)
writeProcedureToChart(5-25)packages/api/src/external/ehr/athenahealth/command/write-back/immunization.ts (1)
writeImmunizationToChart(5-25)packages/api/src/external/ehr/athenahealth/command/write-back/allergry.ts (1)
writeAllergyToChart(5-25)packages/api/src/external/ehr/athenahealth/command/write-back/lab.ts (1)
writeLabToChart(5-25)packages/api/src/external/ehr/athenahealth/command/write-back/note.ts (1)
writeNoteToChart(4-27)
packages/core/src/external/ehr/shared.ts (3)
packages/core/src/fhir-deduplication/shared.ts (1)
fetchCodingCodeOrDisplayOrSystem(461-484)packages/shared/src/common/date.ts (1)
buildDayjs(70-72)packages/shared/src/index.ts (1)
toTitleCase(15-15)
🔇 Additional comments (30)
packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts (1)
1-4: Import path adjustment looks correctThe relative import update aligns with neighbouring modules and resolves the
module correctly. No further action needed.packages/api/src/external/ehr/athenahealth/command/write-back/vitals.ts (1)
3-3: LGTM: Import path correction.The import path update from
"../shared"to"../../shared"correctly adjusts the relative path to the shared module, which aligns with the file structure reorganization mentioned in the summary.packages/shared/src/interface/external/ehr/athenahealth/medication.ts (1)
17-17:❓ Verification inconclusive
Verify the type change is consistent with API and consuming code.
The change from
z.coerce.number()toz.coerce.string()formedicationidalters the type contract. Ensure that:
- This aligns with the actual Athenahealth API response format
- All code consuming
MedicationReferencecan handle the string type- The coercion from potential numeric values to strings is the intended behavior
🏁 Script executed:
#!/bin/bash # Description: Search for usage of medicationid field to verify compatibility with string type # Expected: Find all usages and check if they expect string or number type rg -A 3 -B 3 "medicationid" --type tsLength of output: 1539
🏁 Script executed:
#!/bin/bash # Inspect surrounding context of medicationid usage in core integration rg -C5 "medicationid:" packages/core/src/external/ehr/athenahealth/index.tsLength of output: 532
Verify medicationid type change for Athenahealth API
Please confirm that changing
medicationidfrom a number to a string is valid across both schema and API integration:
- In
packages/shared/src/interface/external/ehr/athenahealth/medication.ts,medicationidnow usesz.coerce.string()instead ofz.coerce.number().- In
packages/core/src/external/ehr/athenahealth/index.ts, we forwardmedicationReference.medicationidin the request payload.- Ensure the Athenahealth API endpoint accepts string IDs (or coerces them back to numbers) and that no downstream code assumes a numeric type.
If the API expects a numeric ID, revert to
z.coerce.number()or update any callers/docs to handle string IDs.packages/shared/src/interface/external/ehr/athenahealth/index.ts (1)
10-14:✅ Verification successful
LGTM: New exports follow established patterns.
The new exports for clinical data modules follow the existing barrel export pattern and kebab-case naming convention. These exports provide a clean public interface for the new write-back capabilities.
🏁 Script executed:
#!/bin/bash # Description: Verify that all newly exported modules exist # Expected: All module files should exist and be valid TypeScript modules for module in "lab-result" "vaccine" "clinical-document" "allergy" "surgical-history"; do echo "Checking module: $module" fd "${module}.ts" packages/shared/src/interface/external/ehr/athenahealth/ doneLength of output: 1522
LGTM: Verified new exports & modules exist.
The new exports for
lab-result,vaccine,clinical-document,allergy, andsurgical-historyfollow the established barrel-export pattern and kebab-case naming convention. All corresponding files were found underpackages/shared/src/interface/external/ehr/athenahealth/.packages/api/src/external/ehr/athenahealth/command/write-back/immunization.ts (2)
1-4: LGTM: Clean imports following conventions.The imports are well-organized and follow the established patterns:
- FHIR types from @medplum
- Shared interface types from @metriport
- Local shared utilities
5-17: LGTM: Function signature follows coding guidelines.The function signature demonstrates good practices:
- Uses destructured parameters to avoid multiple parameters of the same type
- Meaningful parameter names that convey their purpose
- Proper TypeScript typing throughout
- Returns a properly typed Promise
packages/api/src/external/ehr/athenahealth/command/write-back/allergry.ts (1)
5-25: LGTM! Clean abstraction layer implementation.The function follows coding guidelines well:
- Uses destructured parameters as recommended
- Proper async/await usage
- Meaningful naming conventions
- Clean delegation to the API client
- Appropriate TypeScript typing
The implementation provides a clean abstraction for allergy write-back operations.
packages/api/src/external/ehr/athenahealth/command/write-back/note.ts (1)
4-27: LGTM! Consistent implementation following established pattern.The function implementation is solid:
- Follows the same clean pattern as other write-back functions
- Proper destructured parameters and async/await usage
- Meaningful parameter names and return type
- Clean delegation to
createClinicalDocumentAPI methodThe string type for the
dateparameter is appropriate for this use case.packages/api/src/external/ehr/athenahealth/command/write-back/procedure.ts (1)
5-25: LGTM! Well-structured procedure write-back implementation.The function implementation maintains excellent consistency with the established write-back pattern:
- Clear, descriptive function name
- Proper use of FHIR
Proceduretype- Clean delegation to
createSurgicalHistoryAPI method- Follows all coding guidelines for destructured parameters and async/await
The mapping of procedure data to surgical history is semantically appropriate.
packages/api/src/external/ehr/athenahealth/command/write-back/lab.ts (1)
5-25: LGTM! Consistent lab result write-back implementation.The function follows the established pattern perfectly:
- Appropriate use of FHIR
Observationtype for lab data- Clean delegation to
createLabResultDocumentAPI method- Proper async/await and parameter destructuring
- Meaningful naming and return type
packages/shared/src/interface/external/ehr/athenahealth/vaccine.ts (3)
1-2: Import Zod for schema definitions
Importingzfrom Zod is correct and provides both runtime validation and compile-time typings.
3-6: DefinecreatedVaccinesSchemaandCreatedVaccinestype
The optional coerced string array forvaccineidsaccurately models the API response when no vaccines are created. The inferred type ensures strong typing downstream.
7-10: DefinecreatedVaccinesSuccessSchemaandCreatedVaccinesSuccesstype
Requiring.min(1)guarantees at least one vaccine ID on success. The schema and its inferred type align with expected positive responses.packages/shared/src/interface/external/ehr/athenahealth/lab-result.ts (3)
1-2: Import Zod for schema definitions
The import brings in theznamespace needed for defining validation schemas.
3-8: DefinecreatedLabResultSchemaandCreatedLabResulttype
Modelingsuccess, optionalerrormessage, and coercedlabresultidcorrectly mirrors the API’s general response. The inferred type supports type safety.
9-13: DefinecreatedLabResultSuccessSchemaandCreatedLabResultSuccesstype
Usingz.literal(true)forsuccessand requiring a coercedlabresultidenforces a valid success payload.packages/shared/src/interface/external/ehr/athenahealth/surgical-history.ts (3)
1-2: Import Zod for schema definitions
Thezimport from Zod is necessary for building the validation objects.
3-8: DefinecreatedSurgicalHistorySchemaandCreatedSurgicalHistorytype
Capturingsuccess, optionalerrormessage, and optionalprocedureidsmatches the API’s general create-response format. The inferred type is accurate.
9-13: DefinecreatedSurgicalHistorySuccessSchemaandCreatedSurgicalHistorySuccesstype
Enforcingsuccess: trueandprocedureids.array().min(1)ensures that successful calls always return at least one ID.packages/shared/src/interface/external/ehr/athenahealth/clinical-document.ts (3)
1-2: Import Zod for schema definitions
Bringing inzfrom Zod is correct for defining the runtime schemas.
3-8: DefinecreatedClinicalDocumentSchemaandCreatedClinicalDocumenttype
Modeling the standard response withsuccess, optionalerrormessage, and optionalclinicaldocumentidaligns with the API’s contract.
9-13: DefinecreatedClinicalDocumentSuccessSchemaandCreatedClinicalDocumentSuccesstype
Usingz.literal(true)forsuccessand requiringclinicaldocumentidenforces a valid success payload.packages/shared/src/interface/external/ehr/athenahealth/allergy.ts (7)
1-2: Import Zod for schema definitions
Thezimport is correctly used for creating all following validation schemas.
3-7: DefinecreatedAllergySchemaandCreatedAllergytype
Capturingsuccessand optionalerrormessagematches the general response format. The inferred type is accurate.
8-11: DefinecreatedAllergySuccessSchemaandCreatedAllergySuccesstype
Usingz.literal(true)forsuccessguarantees the flag is always true in a success scenario.
13-29: Define core allergy data schemas (Allergy&Allergies)
The nested object forreactionsand rootallergiesarray correctly represent the domain model. Inferred types provide strong typing for downstream usage.
31-37: Define allergen reference schemas (AllergenReference&AllergenReferences)
The simple reference schema and its array form are properly defined and typed.
39-45: Define allergy reaction reference schemas (AllergyReactionReference&AllergyReactionReferences)
The reference schema for reactions is clear and the array wrapper aligns with usage patterns.
47-53: Define allergy severity reference schemas (AllergySeverityReference&AllergySeverityReferences)
Severity references are properly modeled, coerced to string, and typed for consistency.packages/core/src/external/ehr/athenahealth/index.ts (1)
1595-1605: Temperature conversion mixes Celsius and Fahrenheit units
convertCelciusToFahrenheitmultiplies by(9/5)then adds 32 – correct – but the upstreamifbranch triggers for"cel"&"c"units, converting from Celsius to Fahrenheit.
Yet the Athena mapping earlier only expects conversion when the target unit must be grams or centimetres. Double-check that Athena really wants Fahrenheit here; otherwise you may store temperatures in the wrong scale.If left as-is, at least rename the function to
celciusToFahrenheitand document why Fahrenheit is required.
| import { Condition } from "@medplum/fhirtypes"; | ||
| import { CreatedProblemSuccess } from "@metriport/shared/interface/external/ehr/athenahealth/problem"; | ||
| import { createAthenaClient } from "../shared"; | ||
| import { createAthenaClient } from "../../shared"; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix file & import spelling (candition → condition)
The file name and all related imports use the misspelling “candition”.
Renaming to condition.ts prevents future confusion and eases discovery by
search tools.
No functional change is required besides the rename and corresponding import
updates elsewhere in the project.
🤖 Prompt for AI Agents
In packages/api/src/external/ehr/athenahealth/command/write-back/candition.ts
lines 1 to 4, the file name and imports use the incorrect spelling "candition".
Rename the file to condition.ts and update all import statements throughout the
project that reference this file to use the correct spelling "condition" to
ensure consistency and avoid confusion.
There was a problem hiding this comment.
@thomasyopes Perfect! Thank you for fixing the spelling from "candition" to "condition". The corrected filename will make the code much more discoverable and prevent confusion for future developers.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/core/src/external/ehr/athenahealth/index.ts (1)
1097-1259: 🛠️ Refactor suggestionEliminate code duplication between search methods.
The
searchForMedicationandsearchForAllergenmethods have nearly identical implementations. Consider extracting common logic into a generic search method:+private async searchForReference<T>({ + cxId, + patientId, + coding, + referenceUrl, + schema, + s3PathPrefix, + nameExtractor, + context, +}: { + cxId: string; + patientId: string; + coding: Coding; + referenceUrl: string; + schema: z.ZodSchema<T[]>; + s3PathPrefix: string; + nameExtractor: (item: T) => string; + context: string; +}): Promise<T | undefined> { + // Common search logic here +} async searchForMedication({...}): Promise<MedicationReference | undefined> { + return this.searchForReference({ + ...params, + referenceUrl: "/reference/medications", + schema: medicationReferencesSchema, + s3PathPrefix: "reference/medications", + nameExtractor: (m) => m.medication, + context: "medication", + }); } async searchForAllergen({...}): Promise<AllergenReference | undefined> { + return this.searchForReference({ + ...params, + referenceUrl: "/reference/allergies", + schema: allergenReferencesSchema, + s3PathPrefix: "reference/allergies", + nameExtractor: (a) => a.allergenname, + context: "allergen", + }); }
🧹 Nitpick comments (1)
packages/core/src/external/ehr/athenahealth/index.ts (1)
142-144: Consider making document IDs configurable.The hardcoded document IDs might be better managed as configuration values if they vary across different Athenahealth environments or practices.
-const labResultDocumentId = "386265"; -const clinicalNoteDocumentSubclass = "CLINICALDOCUMENT"; -const clinicalNoteDocumentId = "423482"; +// Consider moving these to configuration +const labResultDocumentId = config.labResultDocumentId ?? "386265"; +const clinicalNoteDocumentSubclass = config.clinicalNoteDocumentSubclass ?? "CLINICALDOCUMENT"; +const clinicalNoteDocumentId = config.clinicalNoteDocumentId ?? "423482";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/api/src/external/ehr/athenahealth/command/write-back/allergy.ts(1 hunks)packages/api/src/external/ehr/athenahealth/command/write-back/condition.ts(1 hunks)packages/api/src/routes/ehr/athenahealth/chart.ts(2 hunks)packages/core/src/external/ehr/athenahealth/index.ts(18 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/api/src/external/ehr/athenahealth/command/write-back/condition.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/api/src/routes/ehr/athenahealth/chart.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/api/src/external/ehr/athenahealth/command/write-back/allergy.tspackages/core/src/external/ehr/athenahealth/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/api/src/external/ehr/athenahealth/command/write-back/allergy.ts (1)
1-25: LGTM! Clean implementation following established patterns.The write-back command function is well-implemented:
- Proper use of destructuring parameters
- Clean async/await pattern
- Appropriate delegation to the API client
- Follows TypeScript and naming conventions
packages/core/src/external/ehr/athenahealth/index.ts (3)
1629-1666: Well-designed helper methods with clear responsibilities.The helper methods are well-implemented:
getSearchvaluesFromCodinghas focused responsibility for search term extractioncalculateJaroWinklerSimilarityis a clean utility for finding the most similar option- Both methods follow good separation of concerns and coding guidelines
569-848: Excellent consistency in error handling and validation patterns.The new creation methods (
createVaccine,createSurgicalHistory,createLabResultDocument,createClinicalDocument) all follow consistent patterns:
- Proper validation of required fields with meaningful error messages
- Consistent use of BadRequestError for validation failures
- Schema validation with Zod
- Success validation before returning parsed results
This consistency enhances maintainability and reliability.
1-128: Appropriate imports for the new functionality.The new imports are well-organized and necessary for the added functionality:
- FHIR types for clinical data handling
- Jaro-Winkler for intelligent string similarity matching
- Comprehensive schema imports for validation
- Utility functions for data extraction from FHIR resources
| if (!reaction || reaction.length < 1) { | ||
| throw new BadRequestError("No reactions found for allergy", undefined, additionalInfo); | ||
| } | ||
| const snomedCodingsPairs: [Coding, Coding, string | undefined][] = reaction.flatMap(r => { | ||
| const substanceSnomedCoding = getAllergyIntoleranceSubstanceSnomedCoding(r); | ||
| if (!substanceSnomedCoding) return []; | ||
| const manifestationSnomedCoding = getAllergyIntoleranceManifestationSnomedCoding(r); | ||
| if (!manifestationSnomedCoding) return []; | ||
| return [[substanceSnomedCoding, manifestationSnomedCoding, r.severity]]; | ||
| }); | ||
| const snomedCodings = snomedCodingsPairs[0]; | ||
| if (!snomedCodings) { | ||
| throw new BadRequestError( | ||
| "No SNOMED codes found for allergy reaction", | ||
| undefined, | ||
| additionalInfo | ||
| ); | ||
| } | ||
| const [substanceSnomedCoding, manifestationSnomedCoding, severity] = snomedCodings; | ||
| const allergenReference = await this.searchForAllergen({ | ||
| cxId, | ||
| patientId, | ||
| coding: substanceSnomedCoding, | ||
| }); | ||
| if (!allergenReference) { | ||
| throw new BadRequestError("No allergen options found via search", undefined, additionalInfo); | ||
| } | ||
| const existingAllergy = await this.getAllergyForPatient({ | ||
| cxId, | ||
| patientId, | ||
| departmentId, | ||
| allergenId: allergenReference.allergenid, | ||
| }); | ||
| const existingReactions = existingAllergy?.reactions ?? []; | ||
| const possibleReactions = await this.getCompleteAllergyReactions({ cxId }); | ||
| const reactionReference = possibleReactions.find( | ||
| r => r.snomedcode === manifestationSnomedCoding.code | ||
| ); | ||
| if (!reactionReference) { | ||
| throw new BadRequestError( | ||
| "No reaction reference found for allergy reaction manifestation", | ||
| undefined, | ||
| additionalInfo | ||
| ); | ||
| } | ||
| const possibleSeverities = await this.getCompleteAllergySeverities({ cxId }); | ||
| const severityReference = severity | ||
| ? possibleSeverities.find(s => s.severity.toLowerCase() === severity.toLowerCase()) | ||
| : undefined; | ||
| const newReaction = { | ||
| reactionname: reactionReference.reactionname, | ||
| snomedcode: reactionReference.snomedcode, | ||
| ...(severityReference | ||
| ? { | ||
| severity: severityReference.severity, | ||
| severitysnomedcode: severityReference.snomedcode, | ||
| } | ||
| : {}), | ||
| }; | ||
| const allReactions = uniqBy([...existingReactions, newReaction], "snomedcode"); | ||
| const onsetDate = getAllergyIntoleranceOnsetDate(allergyIntolerance); | ||
| if (!onsetDate) { | ||
| throw new BadRequestError("No onset date found for allergy", undefined, additionalInfo); | ||
| } | ||
| const criticality = allergyIntolerance.criticality; | ||
| const allergies = [ | ||
| { | ||
| allergenid: allergenReference.allergenid, | ||
| allergenname: allergenReference.allergenname, | ||
| criticality, | ||
| note: "Added via Metriport App", | ||
| onsetdate: this.formatDate(onsetDate), | ||
| reactions: allReactions, | ||
| }, | ||
| ]; | ||
| const data = { | ||
| departmentid: this.stripDepartmentId(departmentId), | ||
| allergies, | ||
| THIRDPARTYUSERNAME: undefined, | ||
| PATIENTFACINGCALL: undefined, | ||
| }; | ||
| const createdAllergy = await this.makeRequest<CreatedAllergy>({ | ||
| cxId, | ||
| patientId, | ||
| s3Path: `chart/allergy/${additionalInfo.allergyIntoleranceId ?? "unknown"}`, | ||
| method: "PUT", | ||
| data, | ||
| url: chartAllergyUrl, | ||
| schema: createdAllergySchema, | ||
| additionalInfo, | ||
| debug, | ||
| }); | ||
| if (!createdAllergy.success) { | ||
| throw new MetriportError("Allergy creation failed", undefined, { | ||
| ...additionalInfo, | ||
| error: createdAllergy.errormessage, | ||
| }); | ||
| } | ||
| return createdAllergySuccessSchema.parse(createdAllergy); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Break down complex method into smaller functions.
The createAllergy method is doing too many things and violates the single responsibility principle. Consider refactoring into smaller, focused methods:
async createAllergy({
cxId,
patientId,
departmentId,
allergyIntolerance,
}: {
cxId: string;
patientId: string;
departmentId: string;
allergyIntolerance: AllergyIntolerance;
}): Promise<CreatedAllergySuccess> {
+ const allergyData = await this.prepareAllergyData({
+ cxId,
+ patientId,
+ departmentId,
+ allergyIntolerance,
+ });
+
+ return await this.submitAllergyData({
+ cxId,
+ patientId,
+ allergyData,
+ allergyIntoleranceId: allergyIntolerance.id,
+ });
+}
+
+private async prepareAllergyData({...}) {
+ // Extract SNOMED codes, search for allergen, merge reactions
+}
+
+private async submitAllergyData({...}) {
+ // Handle the actual API submission
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/core/src/external/ehr/athenahealth/index.ts from lines 850 to 972,
the createAllergy method is too large and handles multiple responsibilities.
Refactor by extracting logical parts into smaller helper methods, such as
validating reactions, extracting SNOMED codes, searching for allergen
references, handling existing allergies and reactions, and preparing the allergy
data payload. Each helper should perform a single focused task to improve
readability and maintainability.
… eng-296-athena-writeback Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/core/src/external/ehr/athenahealth/index.ts (2)
1630-1653:getSearchvaluesFromCodingmay yield duplicates and single-letter tokens.
- No
Set– repeated tokens cause redundant API calls.- Tokens of length 2 are filtered in, but common lab codes like “A1c” (length 3 inc. digit) survive while anything shorter (e.g. “B” brand names) is dropped even if meaningful.
Consider:
-const searchValues = codingDisplay - .split("/") - .flatMap(v => v.split(" ")) - .map(v => v.toLowerCase()) - .map(v => v.endsWith("s") ? v.slice(0, -1) : v); -const searchValuesWithAtLeastTwoParts = searchValues.filter( - searchValue => searchValue.length >= 2 -); -return searchValuesWithAtLeastTwoParts; +const searchValues = [...new Set( + codingDisplay + .replace(/[^\w\s/]/g, "") // strip punctuation + .split(/[\/\s]+/) + .map(w => w.toLowerCase()) + .map(w => w.endsWith("s") ? w.slice(0, -1) : w) + .filter(w => w.length >= 2) +)]; +if (searchValues.length === 0) { + throw new BadRequestError("No valid search tokens", undefined, additionalInfo); +} +return searchValues;
1594-1607: Temperature conversion helper is misspelled and inconsistent.
convertCelciusToFahrenheitcontains a typo (“Celcius”) – minor but leaks into public stack traces and harms readability. Rename toconvertCelsiusToFahrenheit. Also,units === "cel"comparison is lowercase whereas UCUM uses"Cel". Normalise case before comparison:-private convertCelciusToFahrenheit(value: number): number { +private convertCelsiusToFahrenheit(value: number): number { return value * (9 / 5) + 32; }and in
convertValue:- if (units === "cel" || units === "c" || units.includes("celsius")) { - return this.convertCelciusToFahrenheit(value); + const u = units.toLowerCase(); + if (u === "cel" || u === "c" || u.includes("celsius")) { + return this.convertCelsiusToFahrenheit(value); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/external/ehr/athenahealth/index.ts(18 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/core/src/external/ehr/athenahealth/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/core/src/external/ehr/athenahealth/index.ts (1)
1059-1064: Race condition when mutatingallCreatedVitalsinside parallel loop.
allCreatedVitals.pushis executed concurrently byexecuteAsynchronously. Although Node’s single-threaded nature makes data races unlikely, JS isn’t guaranteed to keep array mutation atomic. UseallCreatedVitals[index] = ...with the same index you derived for params, or gather results in a local array andpushonce outside the parallel loop.
| const substanceSnomedCoding = getAllergyIntoleranceSubstanceSnomedCoding(r); | ||
| if (!substanceSnomedCoding) return []; | ||
| const manifestationSnomedCoding = getAllergyIntoleranceManifestationSnomedCoding(r); | ||
| if (!manifestationSnomedCoding) return []; | ||
| return [[substanceSnomedCoding, manifestationSnomedCoding, r.severity]]; | ||
| }); | ||
| const snomedCodings = snomedCodingsPairs[0]; | ||
| if (!snomedCodings) { |
There was a problem hiding this comment.
Only the first reaction is honoured – remaining reactions are dropped.
const snomedCodings = snomedCodingsPairs[0]; means patients with multiple manifestations/severities will have everything except the first silently ignored.
-const snomedCodings = snomedCodingsPairs[0];
-if (!snomedCodings) {
+if (snomedCodingsPairs.length === 0) {
throw new BadRequestError(
"No SNOMED codes found for allergy reaction",
undefined,
additionalInfo
);
}
-const [substanceSnomedCoding, manifestationSnomedCoding, severity] = snomedCodings;
+const reactionsPayload = snomedCodingsPairs.map(([substanceSnomedCoding,
+ manifestationSnomedCoding,
+ severity]) => ({
+ substanceSnomedCoding,
+ manifestationSnomedCoding,
+ severity,
+}));Then iterate over reactionsPayload when composing allergies / allReactions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const substanceSnomedCoding = getAllergyIntoleranceSubstanceSnomedCoding(r); | |
| if (!substanceSnomedCoding) return []; | |
| const manifestationSnomedCoding = getAllergyIntoleranceManifestationSnomedCoding(r); | |
| if (!manifestationSnomedCoding) return []; | |
| return [[substanceSnomedCoding, manifestationSnomedCoding, r.severity]]; | |
| }); | |
| const snomedCodings = snomedCodingsPairs[0]; | |
| if (!snomedCodings) { | |
| }); | |
| // Ensure we have at least one reaction | |
| if (snomedCodingsPairs.length === 0) { | |
| throw new BadRequestError( | |
| "No SNOMED codes found for allergy reaction", | |
| undefined, | |
| additionalInfo | |
| ); | |
| } | |
| // Build a payload for all reactions, not just the first | |
| const reactionsPayload = snomedCodingsPairs.map( | |
| ([substanceSnomedCoding, manifestationSnomedCoding, severity]) => ({ | |
| substanceSnomedCoding, | |
| manifestationSnomedCoding, | |
| severity, | |
| }) | |
| ); |
🤖 Prompt for AI Agents
In packages/core/src/external/ehr/athenahealth/index.ts around lines 878 to 885,
the code only processes the first reaction by assigning snomedCodingsPairs[0] to
snomedCodings, ignoring other reactions. To fix this, modify the code to iterate
over all elements in snomedCodingsPairs, processing each reaction's substance,
manifestation, and severity, ensuring all reactions are included when composing
allergies or allReactions.
There was a problem hiding this comment.
We will only write the first reaction
There was a problem hiding this comment.
@thomasyopes, understood! Thank you for clarifying that this is an intentional design decision to only process the first reaction.
Would you like to add a comment to the code to make this intention clear for future developers? Something like:
// Intentionally process only the first reaction
const snomedCodings = snomedCodingsPairs[0];This would help prevent future confusion about the behavior.
∩___∩
( ◕ ◕ )
\_ o _/ Understanding the design!
U
✏️ Learnings added
Learnt from: thomasyopes
PR: metriport/metriport#3936
File: packages/core/src/external/ehr/athenahealth/index.ts:936-943
Timestamp: 2025-06-06T13:01:10.230Z
Learning: In the AthenaHealth allergy creation code, only the first reaction is processed intentionally - this is a design decision, not a bug. The code `const snomedCodings = snomedCodingsPairs[0];` deliberately takes only the first reaction from multiple possible reactions.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
… eng-296-athena-writeback Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/core/src/external/ehr/athenahealth/index.ts (3)
1690-1696: Fix similarity threshold logic per past review.The
calculateJaroWinklerSimilaritymethod throwsBadRequestErrorwhen similarity is below threshold, but callers expectundefinedfor graceful fallback handling. This was flagged in past reviews but not addressed.
908-916: Only first reaction processed per past review.The code only processes
snomedCodingsPairs[0], ignoring additional reactions. This was flagged in past reviews but remains unaddressed.
143-146: Hard-coded document IDs per past review.The document ID constants are hard-coded and should be moved to configuration as noted in past reviews.
🧹 Nitpick comments (4)
packages/core/src/external/ehr/shared.ts (1)
335-358: Consider extracting blacklisted values to a constant.The
blacklistedValuesarray is well-defined but could be moved to the top-level constants section for better maintainability and reusability across the codebase.+const OBSERVATION_BLACKLISTED_VALUES = ["see below", "see text", "see comments", "see note"]; + -const blacklistedValues = ["see below", "see text", "see comments", "see note"]; - if (blacklistedValues.includes(value?.toString().toLowerCase().trim())) value = undefined; + if (OBSERVATION_BLACKLISTED_VALUES.includes(value?.toString().toLowerCase().trim())) value = undefined;packages/core/src/external/ehr/athenahealth/index.ts (3)
459-478: Simplify nested date processing logic.The nested
flatMapand conditional logic for extracting dates from medication statements is complex and hard to follow. Consider extracting this into a separate helper method.+ private extractMedicationDates(statements: MedicationStatement[]): Array<{startdate: string, stopdate: string}> { + return statements.flatMap(statement => { + const startdate = this.formatDate(getMedicationStatementStartDate(statement)); + if (!startdate) return []; + const stopdate = this.formatDate(statement.effectivePeriod?.end); + if (!stopdate) return []; + return { startdate, stopdate }; + }); + } + - const dates = medication.statement.flatMap(statement => { - const startdate = this.formatDate(getMedicationStatementStartDate(statement)); - if (!startdate) return []; - const stopdate = this.formatDate(statement.effectivePeriod?.end); - if (!stopdate) return []; - return { - startdate, - stopdate, - }; - }); + const dates = this.extractMedicationDates(medication.statement);
507-528: Consider using Promise.all for parallel medication creation.The sequential loop for creating multiple medications could be optimized to run in parallel, especially since each creation is independent.
- const createdMedications: CreatedMedicationSuccess[] = []; - for (const date of dates) { - const data = { ...sharedData, ...date }; - const createdMedication = await this.makeRequest<CreatedMedication>({ - cxId, - patientId, - s3Path: `chart/medication/${additionalInfo.medicationId ?? "unknown"}`, - method: "POST", - data, - url: chartMedicationUrl, - schema: createdMedicationSchema, - additionalInfo, - debug, - }); - if (!createdMedication.success || !createdMedication.medicationentryid) { - throw new MetriportError("Medication creation failed", undefined, { - ...additionalInfo, - error: createdMedication.errormessage, - }); - } - createdMedications.push(createdMedicationSuccessSchema.parse(createdMedication)); - } + const createdMedications = await Promise.all( + dates.map(async (date) => { + const data = { ...sharedData, ...date }; + const createdMedication = await this.makeRequest<CreatedMedication>({ + cxId, + patientId, + s3Path: `chart/medication/${additionalInfo.medicationId ?? "unknown"}`, + method: "POST", + data, + url: chartMedicationUrl, + schema: createdMedicationSchema, + additionalInfo, + debug, + }); + if (!createdMedication.success || !createdMedication.medicationentryid) { + throw new MetriportError("Medication creation failed", undefined, { + ...additionalInfo, + error: createdMedication.errormessage, + }); + } + return createdMedicationSuccessSchema.parse(createdMedication); + }) + );
1083-1087: Inconsistent error handling condition formatting.The condition check spans multiple lines inconsistently with the pattern used elsewhere in the file. Consider consolidating for better readability.
- if ( - !createdVitals.success || - !createdVitals.vitalids || - createdVitals.vitalids.length < 1 - ) { + if (!createdVitals.success || !createdVitals.vitalids || createdVitals.vitalids.length < 1) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/external/ehr/athenahealth/index.ts(18 hunks)packages/core/src/external/ehr/shared.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/core/src/external/ehr/shared.tspackages/core/src/external/ehr/athenahealth/index.ts
🧬 Code Graph Analysis (2)
packages/core/src/external/ehr/shared.ts (3)
packages/core/src/fhir-deduplication/shared.ts (1)
fetchCodingCodeOrDisplayOrSystem(461-484)packages/shared/src/common/date.ts (1)
buildDayjs(70-72)packages/shared/src/index.ts (1)
toTitleCase(15-15)
packages/core/src/external/ehr/athenahealth/index.ts (10)
packages/shared/src/interface/external/ehr/athenahealth/allergy.ts (16)
Allergy(25-25)Allergies(29-29)allergiesSchema(26-28)CreatedAllergySuccess(11-11)CreatedAllergy(7-7)createdAllergySchema(3-6)createdAllergySuccessSchema(8-10)AllergenReference(35-35)AllergenReferences(37-37)allergenReferencesSchema(36-36)AllergyReactionReference(43-43)AllergyReactionReferences(45-45)allergyReactionReferencesSchema(44-44)AllergySeverityReference(51-51)AllergySeverityReferences(53-53)allergySeverityReferencesSchema(52-52)packages/shared/src/interface/external/ehr/athenahealth/medication.ts (5)
CreatedMedicationSuccess(13-13)CreatedMedication(8-8)createdMedicationSchema(3-7)createdMedicationSuccessSchema(9-12)MedicationReference(19-19)packages/shared/src/index.ts (2)
BadRequestError(40-40)NotFoundError(42-42)packages/core/src/external/ehr/shared.ts (13)
getMedicationStatementStartDate(260-264)getMedicationRxnormCoding(250-258)getImmunizationCvxCode(319-323)getImmunizationAdministerDate(325-333)getObservationLoincCoding(406-414)getObservationUnitAndValue(416-424)getObservationReferenceRange(426-441)getObservationResultStatus(443-447)getObservationObservedDate(449-451)getObservationInterpretation(453-488)getAllergyIntoleranceSubstanceSnomedCoding(504-516)getAllergyIntoleranceManifestationSnomedCoding(490-502)getAllergyIntoleranceOnsetDate(526-530)packages/shared/src/interface/external/ehr/athenahealth/vaccine.ts (4)
CreatedVaccinesSuccess(10-10)CreatedVaccines(6-6)createdVaccinesSchema(3-5)createdVaccinesSuccessSchema(7-9)packages/shared/src/interface/external/ehr/athenahealth/surgical-history.ts (4)
CreatedSurgicalHistorySuccess(13-13)CreatedSurgicalHistory(8-8)createdSurgicalHistorySchema(3-7)createdSurgicalHistorySuccessSchema(9-12)packages/shared/src/interface/external/ehr/athenahealth/lab-result.ts (4)
CreatedLabResultSuccess(13-13)CreatedLabResult(8-8)createdLabResultSchema(3-7)createdLabResultSuccessSchema(9-12)packages/shared/src/interface/external/ehr/athenahealth/clinical-document.ts (4)
CreatedClinicalDocumentSuccess(13-13)CreatedClinicalDocument(8-8)createdClinicalDocumentSchema(3-7)createdClinicalDocumentSuccessSchema(9-12)packages/core/src/util/concurrency.ts (1)
executeAsynchronously(83-130)packages/lambdas/src/shared/capture.ts (1)
capture(18-103)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/core/src/external/ehr/shared.ts (2)
250-258: LGTM! Good consistent pattern.The function follows the same pattern as existing coding extraction functions and properly handles the case where no RXNORM coding is found.
453-488: Complex but well-structured interpretation logic.The
getObservationInterpretationfunction handles multiple interpretation scenarios effectively. The logic for determining normal/abnormal values based on reference ranges and explicit interpretations is comprehensive and appears correct.
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/core/src/external/ehr/athenahealth/index.ts (1)
439-561: 🛠️ Refactor suggestionBreak down complex medication creation method.
The
createMedicationmethod has grown significantly in complexity with the addition of asynchronous batch processing. Consider extracting the data preparation and batch execution logic into separate methods.Apply this refactor to improve readability and maintainability:
async createMedication({ cxId, patientId, departmentId, medication, }: { cxId: string; patientId: string; departmentId: string; medication: MedicationWithRefs; }): Promise<CreatedMedicationSuccess[]> { const { log, debug } = out( `AthenaHealth createMedication - cxId ${cxId} practiceId ${this.practiceId} patientId ${patientId} departmentId ${departmentId}` ); + const additionalInfo = this.buildMedicationAdditionalInfo({ + cxId, + patientId, + departmentId, + medicationId: medication.medication.id, + }); + + const medicationReference = await this.findMedicationReference({ + cxId, + patientId, + medication, + additionalInfo, + }); + + const createMedicationArgs = this.prepareMedicationCreateParams({ + medication, + medicationReference, + departmentId, + additionalInfo, + }); + + return await this.executeMedicationCreation({ + cxId, + patientId, + createMedicationArgs, + chartMedicationUrl: `/chart/${this.stripPatientId(patientId)}/medications`, + additionalInfo, + log, + debug, + }); +} + +private buildMedicationAdditionalInfo({...}) {...} +private async findMedicationReference({...}) {...} +private prepareMedicationCreateParams({...}) {...} +private async executeMedicationCreation({...}) {...}
♻️ Duplicate comments (4)
packages/core/src/external/ehr/athenahealth/index.ts (4)
144-147: 🛠️ Refactor suggestionAvoid magic numbers – promote configurability for document IDs.
Move these hard-coded document IDs to a configuration object to improve maintainability and allow for tenant-specific overrides without code changes.
934-941:⚠️ Potential issueOnly the first reaction is honoured – remaining reactions are dropped.
The current implementation only processes the first reaction in the allergy, silently ignoring all other reactions. This could lead to incomplete medical records.
907-1029: 🛠️ Refactor suggestionBreak down complex method into smaller functions.
The
createAllergymethod is handling too many responsibilities. Extract the reaction processing, allergen search, and data preparation logic into separate methods.
1711-1729:⚠️ Potential issueThrowing on low similarity blocks fallback logic.
Replace the throw with
return undefinedto allow callers to handle the no-match scenario gracefully.
🧹 Nitpick comments (4)
packages/core/src/external/ehr/athenahealth/index.ts (4)
463-475: Improve error messages for better debugging.The error messages could be more specific about which medication statement is missing data.
Apply this improvement:
- const dates = medication.statement.flatMap(statement => { + const dates = medication.statement.flatMap((statement, index) => { const startdate = this.formatDate(getMedicationStatementStartDate(statement)); - if (!startdate) return []; + if (!startdate) { + log(`Skipping medication statement at index ${index} - no start date found`); + return []; + } const stopdate = this.formatDate(statement.effectivePeriod?.end); return { startdate, stopdate }; }); if (dates.length < 1) { throw new BadRequestError( - "No start dates found for medication statements", + `No valid start dates found in ${medication.statement.length} medication statements`, undefined, additionalInfo ); }
813-814: Simplify interpretation handling.The
getObservationInterpretationfunction appears to already handle the uppercase conversion internally based on its usage pattern.- const interpretation = getObservationInterpretation(observation, value); const analytes = [ { - abnormalflag: interpretation?.toUpperCase(), + abnormalflag: getObservationInterpretation(observation, value),
1686-1709: Add input validation for robustness.The method should validate the input to prevent unexpected behavior with empty or malformed strings.
Apply this improvement:
private getSearchvaluesFromCoding( codingDisplay: string, additionalInfo: Record<string, string | undefined> ): string[] { + if (!codingDisplay || codingDisplay.trim().length === 0) { + throw new BadRequestError( + "Coding display is empty or invalid", + undefined, + additionalInfo + ); + } const searchValues = codingDisplay .split("/") .flatMap(v => v.split(" ")) + .filter(v => v.length > 0) // Remove empty strings .map(v => v.toLowerCase()) .map(v => { if (v.endsWith("s")) return v.slice(0, -1); return v; });
878-886: Extract magic strings to constants.The document data fields contain hard-coded strings that should be extracted to constants for better maintainability.
+const CLINICAL_NOTE_INTERNAL_NOTE = "Note Added via Metriport App"; +const CLINICAL_NOTE_AUTO_CLOSE = "true"; const data = { departmentid: this.stripDepartmentId(departmentId), documentdata: encounterText, documentsubclass: clinicalNoteDocumentSubclass, documenttypeid: clinicalNoteDocumentId, - internalnote: "Note Added via Metriport App", + internalnote: CLINICAL_NOTE_INTERNAL_NOTE, observationdate: this.formatDate(date), - autoclose: "true", + autoclose: CLINICAL_NOTE_AUTO_CLOSE, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/external/ehr/athenahealth/index.ts(19 hunks)packages/shared/src/interface/external/ehr/athenahealth/medication.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared/src/interface/external/ehr/athenahealth/medication.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/core/src/external/ehr/athenahealth/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: fern-check
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/core/src/external/ehr/athenahealth/index.ts (2)
1226-1234: Handle edge case when no medication references are found.The current implementation returns
undefinedwhen no references are found, but the error handling could be more explicit about this being an expected case.The implementation correctly handles the no-match scenario by returning
undefined, allowing the caller to decide how to handle it.
717-723:❓ Verification inconclusive
Validate procedures array structure.
The method creates a single-element array for procedures, but the API might support multiple procedures in a single call.
Run the following script to check if the Athena API documentation or other usage patterns indicate support for multiple procedures:
🏁 Script executed:
#!/bin/bash # Description: Check for existing patterns of surgical history creation with multiple procedures # Search for other surgical history creation patterns rg -A 10 "procedures.*\[" --glob "*.ts" --glob "*.js" # Search for any documentation or comments about surgical history API rg -B 2 -A 2 "surgical.*history.*procedure" --glob "*.md" --glob "*.ts"Length of output: 31318
Confirm multiple procedures support in Athenahealth Surgical History API
I haven’t found any existing usage patterns or in-repo documentation showing more than one procedure in a single call. Our code currently hard-codes a one-element
proceduresarray:const procedures = [ { note: "Added via Metriport App", procedurecode: cptCode, proceduredate: this.formatDate(performedDate), }, ];• Please verify against the Athenahealth API docs (or with your API owner) whether the
proceduresfield supports batching multiple entries.
• If it does, consider changing this method to accept and forward an array of procedures rather than always sending exactly one.
• If the endpoint only accepts one procedure per request, add a comment to clarify and simplify the implementation accordingly.
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
… eng-296-athena-writeback Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
… eng-296-athena-writeback Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-296 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/core/src/external/ehr/athenahealth/index.ts (1)
147-150: Hard-coded document IDs remain – consider configurability
These IDs (labResultDocumentId,clinicalNoteDocumentId, …) are still baked into the source. If Athena rolls new IDs per tenant the SDK will break silently. Loading them from a config or env keeps you future-proof.
🧹 Nitpick comments (1)
packages/core/src/external/ehr/athenahealth/index.ts (1)
1703-1711: Minor: variable name vs. intent & unnecessary token churn
searchValuesWithAtLeastTwoPartsactually filters by character length ≥ 2, not “parts”.
Also, stripping a trailingsnaïvely turns “as” ➔ “a”, “us” ➔ “u”, inflating search volume.
Not urgent, but a tightened splitter (e.g., keep ≥ 3 chars; smarter plural trim) will save redundant API calls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/external/ehr/athenahealth/index.ts(19 hunks)packages/core/src/external/ehr/shared.ts(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/external/ehr/shared.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/core/src/external/ehr/athenahealth/index.ts
🧠 Learnings (1)
packages/core/src/external/ehr/athenahealth/index.ts (1)
Learnt from: thomasyopes
PR: metriport/metriport#3936
File: packages/core/src/external/ehr/athenahealth/index.ts:936-943
Timestamp: 2025-06-06T13:01:10.230Z
Learning: In the AthenaHealth allergy creation code, only the first reaction is processed intentionally - this is a design decision, not a bug. The code `const snomedCodings = snomedCodingsPairs[0];` deliberately takes only the first reaction from multiple possible reactions.
🧬 Code Graph Analysis (1)
packages/core/src/external/ehr/athenahealth/index.ts (9)
packages/shared/src/interface/external/ehr/athenahealth/allergy.ts (14)
Allergy(25-25)CreatedAllergySuccess(11-11)CreatedAllergy(7-7)createdAllergySchema(3-6)createdAllergySuccessSchema(8-10)AllergenReference(35-35)AllergenReferences(37-37)allergenReferencesSchema(36-36)AllergyReactionReference(43-43)AllergyReactionReferences(45-45)allergyReactionReferencesSchema(44-44)AllergySeverityReference(51-51)AllergySeverityReferences(53-53)allergySeverityReferencesSchema(52-52)packages/shared/src/interface/external/ehr/athenahealth/medication.ts (5)
CreatedMedicationSuccess(13-13)MedicationCreateParams(23-35)CreatedMedication(8-8)createdMedicationSchema(3-7)createdMedicationSuccessSchema(9-12)packages/shared/src/index.ts (3)
BadRequestError(40-40)NotFoundError(42-42)toTitleCase(15-15)packages/core/src/external/ehr/shared.ts (15)
getMedicationStatementStartDate(304-308)getMedicationRxnormCoding(294-302)getImmunizationCvxCode(363-367)getImmunizationAdministerDate(369-377)getProcedureCptCode(577-581)getProcedurePerformedDate(583-585)getObservationLoincCoding(450-458)getObservationUnitAndValue(460-468)getObservationReferenceRange(470-485)getObservationResultStatus(487-491)getObservationObservedDate(493-495)getObservationInterpretation(497-532)getAllergyIntoleranceSubstanceRxnormCoding(534-545)getAllergyIntoleranceManifestationSnomedCoding(547-559)getAllergyIntoleranceOnsetDate(561-565)packages/lambdas/src/shared/capture.ts (1)
capture(18-103)packages/shared/src/interface/external/ehr/athenahealth/vaccine.ts (4)
CreatedVaccinesSuccess(10-10)CreatedVaccines(6-6)createdVaccinesSchema(3-5)createdVaccinesSuccessSchema(7-9)packages/shared/src/interface/external/ehr/athenahealth/surgical-history.ts (4)
CreatedSurgicalHistorySuccess(13-13)CreatedSurgicalHistory(8-8)createdSurgicalHistorySchema(3-7)createdSurgicalHistorySuccessSchema(9-12)packages/shared/src/interface/external/ehr/athenahealth/lab-result.ts (4)
CreatedLabResultSuccess(13-13)CreatedLabResult(8-8)createdLabResultSchema(3-7)createdLabResultSuccessSchema(9-12)packages/shared/src/interface/external/ehr/athenahealth/clinical-document.ts (4)
CreatedClinicalDocumentSuccess(13-13)CreatedClinicalDocument(8-8)createdClinicalDocumentSchema(3-7)createdClinicalDocumentSuccessSchema(9-12)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
| throw new BadRequestError("No medication statements found", undefined, additionalInfo); | ||
| } | ||
| const dates = medicationWithRefs.statement.flatMap(statement => { | ||
| const startdate = getMedicationStatementStartDate(statement); | ||
| if (!startdate) return []; | ||
| const stopdate = statement.effectivePeriod?.end; | ||
| return { startdate, stopdate }; | ||
| }); | ||
| if (dates.length < 1) { | ||
| throw new BadRequestError( | ||
| "No start dates found for medication statements", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Date extraction may create duplicate medication rows
flatMap pulls every startdate/stopdate pair across statements, but later calls are issued blindly for each pair.
If multiple statements share the same startdate, Athena will receive duplicate creates and respond with “duplicate entry” errors.
Refactor to collapse identical (startdate, stopdate) combos before firing requests.
-const dates = medicationWithRefs.statement.flatMap(statement => {
+const dates = uniqBy(
+ medicationWithRefs.statement.flatMap(statement => {
const startdate = getMedicationStatementStartDate(statement);
if (!startdate) return [];
const stopdate = statement.effectivePeriod?.end;
return { startdate, stopdate };
-});
+ }),
+ d => `${d.startdate}|${d.stopdate ?? ""}`
+);Also applies to: 520-525
🤖 Prompt for AI Agents
In packages/core/src/external/ehr/athenahealth/index.ts around lines 472 to 482,
the code uses flatMap to extract startdate and stopdate pairs from medication
statements, which can lead to duplicate date pairs and cause duplicate entry
errors when creating medication rows. Refactor this section to first collect all
(startdate, stopdate) pairs, then remove duplicates by collapsing identical
pairs before proceeding with any requests. Apply the same deduplication logic to
the similar code block around lines 520 to 525.
There was a problem hiding this comment.
There may be relevant differences beyond dates for the medications so will leave as is.
There was a problem hiding this comment.
@thomasyopes, understood! You're absolutely right that medication statements can have important differences beyond just the dates - dosage, administration routes, prescriber information, and other properties that would make each statement unique and worth preserving separately. Thanks for clarifying the business logic behind keeping all statements intact.
✏️ Learnings added
Learnt from: thomasyopes
PR: metriport/metriport#3936
File: packages/core/src/external/ehr/athenahealth/index.ts:472-482
Timestamp: 2025-06-06T15:37:44.529Z
Learning: In AthenaHealth medication creation, even if medication statements have the same start/stop dates, they should not be deduplicated because there may be relevant differences beyond dates (dosage, administration routes, prescriber info, etc.) that make each statement unique and worth creating separately.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| const analytes = [ | ||
| { | ||
| abnormalflag: interpretation?.toUpperCase(), | ||
| analytename: loincCoding.display, | ||
| loinc: loincCoding.code, | ||
| referencerange: referenceRange, | ||
| units: unit, | ||
| value: value.toString(), | ||
| resultstatus: toTitleCase(resultStatus.toLowerCase()), | ||
| note: "Added via Metriport App", | ||
| }, |
There was a problem hiding this comment.
Guard against missing display in LOINC coding
loincCoding.display is optional in FHIR. When it is undefined, the current payload will send "analytename": undefined, which Athena may reject or, worse, coerce to the string "undefined". Fall back to the code or throw early.
- analytename: loincCoding.display,
+ analytename: loincCoding.display ?? loincCoding.code,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const analytes = [ | |
| { | |
| abnormalflag: interpretation?.toUpperCase(), | |
| analytename: loincCoding.display, | |
| loinc: loincCoding.code, | |
| referencerange: referenceRange, | |
| units: unit, | |
| value: value.toString(), | |
| resultstatus: toTitleCase(resultStatus.toLowerCase()), | |
| note: "Added via Metriport App", | |
| }, | |
| const analytes = [ | |
| { | |
| abnormalflag: interpretation?.toUpperCase(), | |
| - analytename: loincCoding.display, | |
| + analytename: loincCoding.display ?? loincCoding.code, | |
| loinc: loincCoding.code, | |
| referencerange: referenceRange, | |
| units: unit, | |
| value: value.toString(), | |
| resultstatus: toTitleCase(resultStatus.toLowerCase()), | |
| note: "Added via Metriport App", | |
| }, |
🤖 Prompt for AI Agents
In packages/core/src/external/ehr/athenahealth/index.ts around lines 826 to 836,
the analytename field uses loincCoding.display which can be undefined. To fix
this, add a guard to check if loincCoding.display is present; if not, use
loincCoding.code as a fallback or throw an error early to prevent sending
undefined or invalid values in the payload.
Ref: ENG-296 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/api/src/routes/ehr/athenahealth/chart.ts (1)
112-141: Consider HTTP 201 Created status for resource creation.The new routes follow the established pattern but use
200 OKstatus. Since these endpoints create new resources in the patient's chart,201 Createdwould be more semantically correct according to HTTP standards.- return res.status(httpStatus.OK).json(procedureDetails); + return res.status(httpStatus.CREATED).json(procedureDetails);Apply similar changes to all new write-back routes.
Also applies to: 143-172, 174-203, 205-234
🧹 Nitpick comments (1)
packages/api/src/routes/ehr/athenahealth/chart.ts (1)
236-239: Move schema definition closer to usage or make it reusable.The
noteSchemais defined at the file level but only used in one route. Consider either moving it inside the route handler or making it part of a shared validation module if it will be reused.// Option 1: Move inside route handler router.post("/:id/note", ..., asyncHandler(async (req, res) => { const noteSchema = z.object({ date: z.string(), encounterText: z.string(), }); // ... })); // Option 2: Create shared validation module // packages/api/src/routes/ehr/athenahealth/schemas.ts
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/api/src/external/ehr/athenahealth/command/write-back/allergy.ts(1 hunks)packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts(1 hunks)packages/api/src/routes/ehr/athenahealth/chart.ts(4 hunks)packages/shared/src/interface/external/ehr/athenahealth/allergy.ts(1 hunks)packages/shared/src/interface/external/ehr/athenahealth/medication.ts(1 hunks)packages/shared/src/interface/external/ehr/athenahealth/vitals.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/shared/src/interface/external/ehr/athenahealth/allergy.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts
- packages/api/src/external/ehr/athenahealth/command/write-back/allergy.ts
- packages/shared/src/interface/external/ehr/athenahealth/medication.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/shared/src/interface/external/ehr/athenahealth/vitals.tspackages/api/src/routes/ehr/athenahealth/chart.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/shared/src/interface/external/ehr/athenahealth/vitals.ts (2)
12-15: LGTM! Improved validation logic for success responses.The refactored
createdVitalsSuccessSchemaenhances type safety and validation by:
- Using
z.literal(true)for more precise success validation- Requiring at least one vital ID with
.min(1)constraint- Ensuring successful creation responses always include non-empty vital IDs
This aligns with the broader standardization across clinical data domains mentioned in the AI summary.
14-14: Good addition of minimum array length validation.The
.min(1)constraint appropriately enforces that successful vital creation must return at least one vital ID, preventing edge cases where success is true but no IDs are provided.packages/api/src/routes/ehr/athenahealth/chart.ts (2)
1-16: Imports look good with proper organization.The import statements are well-organized and all paths appear correct. The addition of Zod for validation support is appropriate.
26-26: Parameter name change aligns with API evolution.The change from
medicationtomedicationWithRefsin both the comment and parameter usage correctly reflects the updated API interface that supports batch medication creation.Also applies to: 44-44
| const noteSchema = z.object({ | ||
| date: z.string(), | ||
| encounterText: z.string(), | ||
| }); | ||
|
|
||
| /** | ||
| * POST /ehr/athenahealth/chart/:id/note | ||
| * | ||
| * Writes the note to the patient's chart | ||
| * @param req.params.id The ID of AthenaHealth Patient. | ||
| * @param req.query.practiceId The ID of AthenaHealth Practice. | ||
| * @param req.query.departmentId The ID of AthenaHealth Department. | ||
| * @param req.body The FHIR Resource payload | ||
| * @returns Athena API response | ||
| */ | ||
| router.post( | ||
| "/:id/note", | ||
| handleParams, | ||
| requestLogger, | ||
| asyncHandler(async (req: Request, res: Response) => { | ||
| const cxId = getCxIdOrFail(req); | ||
| const athenaPatientId = getFrom("params").orFail("id", req); | ||
| const athenaPracticeId = getFromQueryOrFail("practiceId", req); | ||
| const athenaDepartmentId = getFromQueryOrFail("departmentId", req); | ||
| const note = noteSchema.parse(req.body); | ||
| const noteDetails = await writeNoteToChart({ | ||
| cxId, | ||
| athenaPatientId, | ||
| athenaPracticeId, | ||
| athenaDepartmentId, | ||
| encounterText: note.encounterText, | ||
| date: note.date, | ||
| }); | ||
| return res.status(httpStatus.OK).json(noteDetails); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Input validation inconsistency across routes.
The note route uniquely implements Zod validation while all other routes have TODO comments about body parsing. This creates inconsistency in the API's input validation approach.
Consider either:
- Adding validation schemas for all routes, or
- Moving validation to a higher level (middleware or command layer)
// Example: Add validation for other routes
const procedureSchema = z.object({
// Define expected procedure fields
});
// Or implement at middleware level for consistency🤖 Prompt for AI Agents
In packages/api/src/routes/ehr/athenahealth/chart.ts between lines 236 and 271,
the POST /:id/note route uses Zod for input validation while other routes lack
consistent validation, causing inconsistency. To fix this, either define and
apply Zod validation schemas for all routes in this file to standardize input
validation or refactor the validation logic into a reusable middleware or
command layer that all routes can use, ensuring uniform validation handling
across the API.
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
… eng-296-athena-writeback Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Issues:
Dependencies
Description
Testing
Release Plan
Summary by CodeRabbit
New Features
Bug Fixes