-
Notifications
You must be signed in to change notification settings - Fork 72
Eng 559 elation write back fixes #4315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Ref: ENG-559 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Mac.attlocal.net>
…-elation-write-back-fixes Signed-off-by: Thomas Yopes <thomasyopes@Mac.attlocal.net>
WalkthroughAdds per‑EHR primary condition coding mapping and accessor; converts grouped vitals to a [Date, Observation[]] tuple with a runtime guard; updates Elation client and write‑back handlers to accept grouped vitals and apply latestOnly filtering/grouping for vitals and conditions; renames schema key Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WriteBackBundle
participant FilterUtils
participant EhrApi
Client->>WriteBackBundle: submit resources + writeBackFilters
WriteBackBundle->>FilterUtils: filterAndGroupObservations(vitals, filters)
FilterUtils-->>WriteBackBundle: groupedVitals[] ([Date, Observation[]])
loop per date group
WriteBackBundle->>EhrApi: createGroupedVitals(groupedVitals)
end
WriteBackBundle->>FilterUtils: filterConditions(conditions, ehr, filters)
FilterUtils-->>WriteBackBundle: filteredConditions[]
WriteBackBundle->>EhrApi: write conditions and remaining resources
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
...rc/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
Show resolved
Hide resolved
...rc/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
Show resolved
Hide resolved
...rc/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
Outdated
Show resolved
Hide resolved
...rc/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
packages/shared/src/interface/external/ehr/shared.ts (1)
27-34
: Fix write-back filter tests to use singularproblem
keyThe interface rename to
problem
isn’t reflected in the write-back filter tests, soWriteBackFilters.problem
is never populated. Update the following file:• packages/core/src/external/ehr/job/bundle/write-back-bundles/tests/consts.ts
– Change each exported constant’sproblems:
block toproblem:
– E.g.:-export const chronicityFilterChronic = { - problems: { +export const chronicityFilterChronic = { + problem: { chronicityFilter: "chronic", }, };Apply the same update for
chronicityFilterNonChronic
,chronicityFilterAll
, andchronicityFilterUndefined
.
🧹 Nitpick comments (4)
packages/core/src/external/ehr/command/write-back/condition.ts (1)
48-56
: Primary code map: prefer immutability and exhaustiveness checksMap is good. Minor refinements:
- Make it immutable with
as const
.- Optionally use
satisfies
for compile-time exhaustiveness againstRecord<EhrSource, CodingSystem | undefined>
.Apply:
- export const ehrWriteBackConditionPrimaryCodeMap: Record<EhrSource, CodingSystem | undefined> = { - [EhrSources.elation]: SNOMED_CODE, - [EhrSources.athena]: SNOMED_CODE, - [EhrSources.canvas]: ICD_10_CODE, - [EhrSources.healthie]: ICD_10_CODE, - [EhrSources.eclinicalworks]: undefined, - }; + export const ehrWriteBackConditionPrimaryCodeMap = { + [EhrSources.elation]: SNOMED_CODE, + [EhrSources.athena]: SNOMED_CODE, + [EhrSources.canvas]: ICD_10_CODE, + [EhrSources.healthie]: ICD_10_CODE, + [EhrSources.eclinicalworks]: undefined, + } as const satisfies Record<EhrSource, CodingSystem | undefined>;Note: when we add eClinicalWorks support, update this map accordingly.
packages/core/src/external/ehr/elation/index.ts (2)
620-626
:flatMap
is unnecessary overhead here
formatGroupedVital
returns a single object orundefined
, so:const vitals = observations .map(o => this.formatGroupedVital(o)) .filter(Boolean) as ElationGroupedVitalData[];is both clearer and avoids an intermediate empty array allocation for every skipped observation.
1248-1257
: Dead code –formattedChartDate
is computed but never usedLines 1255-1257 build
formattedChartDate
solely to gate the branch.
Since the date is already validated earlier increateGroupedVitals
, this check is redundant and can be removed to reduce noise.packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts (1)
756-775
:latestOnly
filtering reimplements a common pattern – prefer a utilityThe reduce-into-map logic appears twice (conditions & vitals). Extracting it:
function keepLatestBy<T>(items: T[], keyFn: (i:T)=>string, dateFn:(i:T)=>string|undefined) {...}would cut duplication and potential future inconsistencies.
packages/core/src/external/ehr/command/write-back/grouped-vitals.ts
Outdated
Show resolved
Hide resolved
packages/core/src/external/ehr/command/write-back/grouped-vitals.ts
Outdated
Show resolved
Hide resolved
...rc/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts (1)
641-646
: Logging still dumps full resource
This is the same issue flagged in an earlier review:JSON.stringify(resource)
can explode CloudWatch logs whenobservations
contains large payloads. A concise fingerprint (date + count) was recommended.
🧹 Nitpick comments (1)
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts (1)
561-570
: Typo:skipVitalLoinCode
should beskipVitalLoincCode
Both the function name and its internal variable use “Loinc”; keeping the missing “c” only here is inconsistent and harms discoverability.
-export function skipVitalLoinCode( +export function skipVitalLoincCode(Remember to update the sole call site on Line 398.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/core/src/external/ehr/command/write-back/condition.ts
(2 hunks)packages/core/src/external/ehr/command/write-back/grouped-vitals.ts
(1 hunks)packages/core/src/external/ehr/command/write-back/shared.ts
(3 hunks)packages/core/src/external/ehr/elation/index.ts
(6 hunks)packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/core/src/external/ehr/command/write-back/shared.ts
- packages/core/src/external/ehr/command/write-back/grouped-vitals.ts
- packages/core/src/external/ehr/command/write-back/condition.ts
- packages/core/src/external/ehr/elation/index.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.{js,jsx,ts,tsx}
: Don’t use null inside the app, only on code interacting with external interfaces/services, like DB and HTTP; convert to undefined before sending inwards into the code
Useconst
whenever possible
Useasync/await
instead of.then()
Naming: classes, enums: PascalCase
Naming: constants, variables, functions: camelCase
Naming: file names: kebab-case
Naming: Don’t use negative names, likenotEnabled
, preferisDisabled
If possible, use decomposing objects for function parameters
Prefer Nullish Coalesce (??) than the OR operator (||) when you want to provide a default value
Avoid creating arrow functions
Use truthy syntax instead ofin
- i.e.,if (data.link)
notif ('link' in data)
While handling errors, keep the stack trace around: if you create a new Error (e.g., MetriportError), make sure to pass the original error as the new one’s cause so the stack trace is available upstream.
max column length is 100 chars
multi-line comments use/** */
top-level comments go after the import (save pre-import to basic file header, like license)
move literals to constants declared after imports when possible
Files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Use types whenever possible
Files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
**/*.ts
⚙️ CodeRabbit Configuration File
**/*.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
const
instead oflet
- Avoid
any
and casting fromany
to 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
null
inside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefined
before sending inwards into the code- Use
async/await
instead 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 (seeprocessAsyncError
andemptyFunction
depending on the case)- Date and Time
- Always use
buildDayjs()
to createdayjs
instances- 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
- U...
Files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
🧠 Learnings (44)
📓 Common learnings
Learnt from: leite08
PR: metriport/metriport#0
File: :0-0
Timestamp: 2025-03-05T18:43:30.389Z
Learning: For backmerge PRs at metriport/metriport, only verify two things: (1) that the source branch is `master` and destination branch is `develop`, and (2) that there's a link to at least one PR (usually a "patch" PR) in the description. No need for detailed review comments or updates to the PR description unless there's an issue with these criteria.
Learnt from: leite08
PR: metriport/metriport#3814
File: packages/api/src/routes/internal/medical/patient-consolidated.ts:141-174
Timestamp: 2025-05-20T21:26:26.804Z
Learning: The functionality introduced in packages/api/src/routes/internal/medical/patient-consolidated.ts is planned to be refactored in downstream PR #3857, including improvements to error handling and validation.
Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/command/get-bundle-by-resource-type.ts:31-37
Timestamp: 2025-06-05T14:09:59.683Z
Learning: When EHR handlers in mapping objects are set to `undefined` and there are downstream PRs mentioned in the PR objectives, this typically indicates a staged rollout approach where the core structure is implemented first and specific handler implementations follow in subsequent PRs.
Learnt from: thomasyopes
PR: metriport/metriport#3466
File: packages/api/src/routes/ehr/elation/chart.ts:20-37
Timestamp: 2025-03-17T15:30:34.647Z
Learning: The request body parsing in packages/api/src/routes/ehr/elation/chart.ts is tracked with TODO referencing issue #2170 and will be handled in a separate PR, not in PR #3466.
Learnt from: leite08
PR: metriport/metriport#4255
File: packages/core/src/command/analytics-platform/fhir-to-csv/command/fhir-to-csv/fhir-to-csv-direct.ts:19-20
Timestamp: 2025-07-27T00:40:32.149Z
Learning: User leite08 acknowledged the duplicate inputBundle spread issue in packages/core/src/command/analytics-platform/fhir-to-csv/command/fhir-to-csv/fhir-to-csv-direct.ts and will fix it in a follow-up PR rather than in the current patch release PR #4255.
Learnt from: thomasyopes
PR: metriport/metriport#3484
File: packages/api/src/routes/internal/medical/organization.ts:7-15
Timestamp: 2025-03-18T23:49:35.819Z
Learning: Typo fixes and other minor code quality improvements should not be addressed in release PRs to minimize risk and maintain stability.
Learnt from: RamilGaripov
PR: metriport/metriport#4187
File: packages/api/src/command/medical/patient/map-patient.ts:41-49
Timestamp: 2025-07-30T14:51:29.865Z
Learning: In the syncElationPatientIntoMetriport and syncHealthiePatientIntoMetriport functions, the patientId parameter is intended to receive the Metriport patient ID for demographic validation purposes via confirmPatientMatch, not the external system patient ID. The external patient ID should be passed through the specific external ID parameters (elationPatientId, healthiePatientId, etc.).
Learnt from: lucasdellabella
PR: metriport/metriport#3907
File: packages/terminology/src/seed/seed-ndc-from-fda.ts:91-113
Timestamp: 2025-05-28T05:28:26.896Z
Learning: On PRs with "release" in the title, only provide very high priority feedback and avoid lower priority suggestions like refactoring or optimization improvements.
Learnt from: leite08
PR: metriport/metriport#4215
File: packages/core/src/command/analytics-platform/config.ts:4-13
Timestamp: 2025-07-20T01:18:28.093Z
Learning: User leite08 prefers that code improvement suggestions (like consolidating duplicated schemas) be made on feature PRs rather than release PRs. Such improvements should be deferred to follow-up PRs during the release cycle.
Learnt from: thomasyopes
PR: metriport/metriport#3970
File: packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts:17-17
Timestamp: 2025-06-06T16:45:31.832Z
Learning: The writeMedicationToChart function in packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts returns a response that is not currently used by any consumers, so changes to its return type are not breaking changes in practice.
Learnt from: thomasyopes
PR: metriport/metriport#4000
File: packages/core/src/external/ehr/athenahealth/index.ts:504-507
Timestamp: 2025-06-11T21:39:26.805Z
Learning: In AthenaHealth write-back (`packages/core/src/external/ehr/athenahealth/index.ts`), only the condition statuses “relapse” and “recurrence” are currently mapped to AthenaHealth problem statuses (“CHRONIC”); other FHIR clinicalStatus values (e.g., “active”, “resolved”, “inactive”, “remission”) are not yet supported.
Learnt from: thomasyopes
PR: metriport/metriport#3981
File: packages/core/src/external/ehr/canvas/index.ts:650-657
Timestamp: 2025-06-11T16:56:22.035Z
Learning: In Canvas write-back (`createMedicationStatements`), the intended business logic is to submit only the most recent medication statement; older statements should be ignored.
Learnt from: leite08
PR: metriport/metriport#3940
File: packages/core/src/command/consolidated/search/fhir-resource/search-consolidated.ts:82-86
Timestamp: 2025-05-31T21:58:28.502Z
Learning: In packages/core/src/command/consolidated/search/fhir-resource/search-consolidated.ts, the mutable array operations using push() on resourcesMutable and hydratedMutable (lines 82-86 and 100-103) have been explicitly accepted as exceptions to the immutability guidelines after previous discussion.
Learnt from: thomasyopes
PR: metriport/metriport#3970
File: packages/shared/src/interface/external/ehr/athenahealth/vaccine.ts:4-4
Timestamp: 2025-06-06T16:45:21.766Z
Learning: In Athenahealth EHR integration schemas, property names like "vaccineids" are intentionally preserved to match the external API response format since responses are returned directly without internal transformation or usage.
📚 Learning: 2025-06-19T22:44:49.393Z
Learnt from: thomasyopes
PR: metriport/metriport#4061
File: packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/run-job.ts:34-47
Timestamp: 2025-06-19T22:44:49.393Z
Learning: In packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/run-job.ts, the team prefers to keep refreshEhrBundles operations grouped using fire-and-forget pattern rather than awaiting all operations with Promise.allSettled(). This allows individual refresh operations to run independently without blocking the job runner.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-06T16:45:31.832Z
Learnt from: thomasyopes
PR: metriport/metriport#3970
File: packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts:17-17
Timestamp: 2025-06-06T16:45:31.832Z
Learning: The writeMedicationToChart function in packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts returns a response that is not currently used by any consumers, so changes to its return type are not breaking changes in practice.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-20T21:26:26.804Z
Learnt from: leite08
PR: metriport/metriport#3814
File: packages/api/src/routes/internal/medical/patient-consolidated.ts:141-174
Timestamp: 2025-05-20T21:26:26.804Z
Learning: The functionality introduced in packages/api/src/routes/internal/medical/patient-consolidated.ts is planned to be refactored in downstream PR #3857, including improvements to error handling and validation.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-31T21:58:28.502Z
Learnt from: leite08
PR: metriport/metriport#3940
File: packages/core/src/command/consolidated/search/fhir-resource/search-consolidated.ts:82-86
Timestamp: 2025-05-31T21:58:28.502Z
Learning: In packages/core/src/command/consolidated/search/fhir-resource/search-consolidated.ts, the mutable array operations using push() on resourcesMutable and hydratedMutable (lines 82-86 and 100-103) have been explicitly accepted as exceptions to the immutability guidelines after previous discussion.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-04-21T03:47:54.332Z
Learnt from: lucasdellabella
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/resource-diff/steps/start/ehr-start-resource-diff-local.ts:0-0
Timestamp: 2025-04-21T03:47:54.332Z
Learning: In the EHR resource diff implementation, bundles should always have an `entry` property, and code should be allowed to throw if `entry` is missing rather than using null-coalescing operators, as this represents an unexpected state that should be caught and addressed.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-19T18:24:41.632Z
Learnt from: thomasyopes
PR: metriport/metriport#3870
File: packages/core/src/external/ehr/bundle/job/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.ts:55-91
Timestamp: 2025-05-19T18:24:41.632Z
Learning: In EHR bundles related to resource diff computations, bundle creation operations that are for auditing purposes should not stop the main job flow if they fail. These operations typically use try/catch blocks that log errors without rethrowing them, which is the intended behavior.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-07-27T00:40:32.149Z
Learnt from: leite08
PR: metriport/metriport#4255
File: packages/core/src/command/analytics-platform/fhir-to-csv/command/fhir-to-csv/fhir-to-csv-direct.ts:19-20
Timestamp: 2025-07-27T00:40:32.149Z
Learning: User leite08 acknowledged the duplicate inputBundle spread issue in packages/core/src/command/analytics-platform/fhir-to-csv/command/fhir-to-csv/fhir-to-csv-direct.ts and will fix it in a follow-up PR rather than in the current patch release PR #4255.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-19T18:25:32.688Z
Learnt from: thomasyopes
PR: metriport/metriport#3870
File: packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/start-job.ts:26-57
Timestamp: 2025-05-19T18:25:32.688Z
Learning: In the startCreateResourceDiffBundlesJob function (and similar functions in the codebase), the preferred approach is to let errors bubble up naturally rather than adding explicit try/catch blocks at multiple levels of the call stack.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-18T18:34:10.489Z
Learnt from: keshavsaharia
PR: metriport/metriport#4045
File: packages/core/src/external/surescripts/fhir/coverage.ts:0-0
Timestamp: 2025-06-18T18:34:10.489Z
Learning: Coverage resources in Surescripts FHIR conversion are currently excluded from bundles to prevent skewing data lift metrics. The team plans to examine available insurance data thoroughly before including properly structured Coverage resources with mandatory FHIR R4 elements like beneficiary and payor references.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-08-04T17:59:05.674Z
Learnt from: lucasdellabella
PR: metriport/metriport#4020
File: packages/fhir-sdk/src/index.ts:308-308
Timestamp: 2025-08-04T17:59:05.674Z
Learning: In packages/fhir-sdk/src/index.ts, the FhirBundleSdk constructor intentionally mutates the input bundle's total property to ensure data consistency between bundle.total and bundle.entry.length. This is an accepted exception to the "avoid modifying objects received as parameter" guideline for cleanup purposes.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-11T16:56:22.035Z
Learnt from: thomasyopes
PR: metriport/metriport#3981
File: packages/core/src/external/ehr/canvas/index.ts:650-657
Timestamp: 2025-06-11T16:56:22.035Z
Learning: In Canvas write-back (`createMedicationStatements`), the intended business logic is to submit only the most recent medication statement; older statements should be ignored.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-03-19T13:43:19.104Z
Learnt from: leite08
PR: metriport/metriport#3463
File: packages/api/src/command/medical/patient/settings/create-patient-settings.ts:199-199
Timestamp: 2025-03-19T13:43:19.104Z
Learning: In the Metriport codebase, avoid sending objects directly to string templates in logs without first converting them to strings. Use JSON.stringify() for objects or array.join(', ') for arrays of primitives to ensure proper formatting and readability of logs.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-18T21:03:16.752Z
Learnt from: RamilGaripov
PR: metriport/metriport#3976
File: packages/api/src/command/medical/cohort/update-cohort.ts:26-26
Timestamp: 2025-06-18T21:03:16.752Z
Learning: In the Metriport codebase, logging cohort dataValues objects doesn't cause multiline logs in their AWS environment, so JSON.stringify() on cohort objects is acceptable.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-04-19T13:19:25.570Z
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/lambdas/src/ehr-compute-resource-diff.ts:29-30
Timestamp: 2025-04-19T13:19:25.570Z
Learning: Avoid logging raw SQS message bodies in EHR-related lambdas as they may contain PHI (Protected Health Information) like lists of healthcare resources that shouldn't be exposed in CloudWatch logs.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-27T16:10:53.952Z
Learnt from: lucasdellabella
PR: metriport/metriport#3866
File: packages/core/src/command/hl7v2-subscriptions/hl7v2-roster-generator.ts:63-63
Timestamp: 2025-05-27T16:10:53.952Z
Learning: User lucasdellabella is okay with using JSON.stringify in log statements in TypeScript files, even though the coding guidelines suggest avoiding multi-line logs with JSON.stringify. They prefer to keep such logging as is when it serves the purpose effectively.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-28T02:51:35.779Z
Learnt from: leite08
PR: metriport/metriport#3857
File: packages/core/src/command/consolidated/search/fhir-resource/search-lexical.ts:67-82
Timestamp: 2025-05-28T02:51:35.779Z
Learning: In the search-lexical.ts file, the user prefers to bubble up JSON parsing errors rather than catching and logging them. When processing FHIR resources from OpenSearch results, errors should be thrown and allowed to propagate up the call stack instead of being caught and silently ignored.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-28T01:12:48.280Z
Learnt from: leite08
PR: metriport/metriport#3857
File: packages/core/src/external/opensearch/fhir-ingestor.ts:122-126
Timestamp: 2025-05-28T01:12:48.280Z
Learning: When analyzing logging code in TypeScript, distinguish between code formatting (which can span multiple lines for readability) and the actual log output. A log statement formatted across multiple lines in code can still produce single-line log output, which complies with the "avoid multi-line logs" guideline.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-28T19:22:09.281Z
Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:27-49
Timestamp: 2025-05-28T19:22:09.281Z
Learning: In packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-28T19:20:47.442Z
Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:0-0
Timestamp: 2025-05-28T19:20:47.442Z
Learning: In packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-16T02:10:01.792Z
Learnt from: lucasdellabella
PR: metriport/metriport#3855
File: packages/core/src/command/hl7-notification/hl7-notification-webhook-sender-direct.ts:88-104
Timestamp: 2025-05-16T02:10:01.792Z
Learning: When logging file paths or other strings that might contain patient identifiers like patientId or cxId, these should be masked or redacted (e.g., using regex replacement) to avoid logging Protected Health Information (PHI) to CloudWatch or error tracking systems like Sentry, ensuring HIPAA compliance.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-04-23T23:57:11.544Z
Learnt from: husainsyed
PR: metriport/metriport#3663
File: packages/shared/package.json:89-89
Timestamp: 2025-04-23T23:57:11.544Z
Learning: In the Metriport codebase, both Day.js and Luxon date libraries are used intentionally for different purposes: Luxon specifically for ISO date validation (where Day.js has limitations) and Day.js for other date operations. There is no plan to fully migrate from Day.js to Luxon.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-30T01:09:55.013Z
Learnt from: keshavsaharia
PR: metriport/metriport#3885
File: packages/core/src/external/surescripts/client.ts:0-0
Timestamp: 2025-05-30T01:09:55.013Z
Learning: In packages/core/src/external/surescripts/client.ts, the Surescripts date formatting should use local time according to their documentation ("message-date-must-be-local"), which means buildDayjs() cannot be used since it defaults to UTC. The specific use case for transmission date formatting requires dayjs to be used directly to preserve local time behavior.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-03-19T13:44:54.126Z
Learnt from: leite08
PR: metriport/metriport#3463
File: packages/api/src/command/medical/patient/settings/create-patient-settings.ts:174-175
Timestamp: 2025-03-19T13:44:54.126Z
Learning: In the Metriport codebase, Lodash is used extensively and preferred for array and object manipulations. When working with batches of data, use Lodash's `_.chunk()` instead of manual array slicing with `array.slice(i, i + batchSize)`. The codebase also commonly uses other Lodash functions like `cloneDeep`, `uniq`, `uniqBy`, `groupBy`, and `orderBy`. Prefer Lodash functions over native JavaScript implementations for better readability and consistency across the codebase.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-18T17:24:31.650Z
Learnt from: keshavsaharia
PR: metriport/metriport#4045
File: packages/core/src/external/surescripts/__tests__/file-names.test.ts:21-21
Timestamp: 2025-06-18T17:24:31.650Z
Learning: The team does all testing and deploying in PST local time, which helps avoid issues with date-dependent code in tests like buildRequestFileName that use dayjs().format('YYYYMMDD').
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-18T21:05:22.256Z
Learnt from: RamilGaripov
PR: metriport/metriport#3976
File: packages/api/src/routes/medical/patient.ts:541-543
Timestamp: 2025-06-18T21:05:22.256Z
Learning: In packages/api/src/routes/medical/patient.ts, inline schema definitions like cohortIdSchema are acceptable and don't need to be moved to separate schema files when the user prefers to keep them inline.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-05T16:28:12.424Z
Learnt from: thomasyopes
PR: metriport/metriport#3960
File: packages/lambdas/src/ehr/get-appointments.ts:36-44
Timestamp: 2025-06-05T16:28:12.424Z
Learning: In packages/lambdas/src/ehr/get-appointments.ts, date validation is not required for the convertToGetAppointmentsRequest function because the input is controlled and dates will always be valid.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-03-19T13:44:54.126Z
Learnt from: leite08
PR: metriport/metriport#3463
File: packages/api/src/command/medical/patient/settings/create-patient-settings.ts:174-175
Timestamp: 2025-03-19T13:44:54.126Z
Learning: In the Metriport codebase, Lodash is already imported and used throughout the project. When implementing array batch processing, it's preferred to use Lodash's `.chunk()` method instead of manual array slicing with `array.slice(i, i + batchSize)` for better readability and maintainability. The team encourages identifying more opportunities to leverage Lodash's utility functions since the library is already a dependency.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-01T16:17:02.236Z
Learnt from: thomasyopes
PR: metriport/metriport#3771
File: packages/api/src/external/ehr/healthie/command/sync-patient.ts:158-164
Timestamp: 2025-05-01T16:17:02.236Z
Learning: In the Metriport codebase, passing dayjs Duration objects directly to the add() method works in practice, even though the dayjs documentation suggests using numeric values with units instead (e.g., add(duration.asHours(), "hour")).
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-26T16:24:10.245Z
Learnt from: leite08
PR: metriport/metriport#3859
File: packages/utils/src/consolidated/check-patient-consolidated.ts:28-32
Timestamp: 2025-05-26T16:24:10.245Z
Learning: For utility scripts in the packages/utils directory, developers are expected to update the main constants at the top of the file (like bundleFilePath, cxId, patientId) rather than implementing command-line argument parsing or environment variable reading. This is the preferred pattern for configuring utility scripts in this codebase.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-18T00:18:59.144Z
Learnt from: leite08
PR: metriport/metriport#4043
File: packages/core/src/command/consolidated/consolidated-create.ts:0-0
Timestamp: 2025-06-18T00:18:59.144Z
Learning: In the consolidated bundle creation process (packages/core/src/command/consolidated/consolidated-create.ts), the team prefers strict failure behavior where if any component (including pharmacy resources) fails to be retrieved, the entire consolidation should fail rather than gracefully degrading. This ensures data consistency and prevents incomplete bundles.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-04-21T17:07:30.574Z
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-08T19:41:36.533Z
Learnt from: thomasyopes
PR: metriport/metriport#3788
File: packages/api/src/external/ehr/shared/utils/bundle.ts:83-93
Timestamp: 2025-05-08T19:41:36.533Z
Learning: In the Metriport codebase, the team prefers to let errors bubble up naturally in some cases rather than adding explicit error handling at every layer, as demonstrated in the refreshEhrBundle function in the bundle.ts file.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-25T01:55:42.627Z
Learnt from: thomasyopes
PR: metriport/metriport#4090
File: packages/core/src/command/conversion-fhir/conversion-fhir-cloud.ts:30-30
Timestamp: 2025-06-25T01:55:42.627Z
Learning: In packages/core/src/command/conversion-fhir/conversion-fhir-cloud.ts, the FHIR converter Lambda endpoint is controlled by the team and guaranteed to return valid JSON in the expected Bundle<Resource> format, so JSON.parse() error handling and type validation are not necessary for this specific endpoint.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-12T23:25:08.139Z
Learnt from: thomasyopes
PR: metriport/metriport#3997
File: packages/api/src/command/job/patient/status/cancel.ts:30-34
Timestamp: 2025-06-12T23:25:08.139Z
Learning: `cancelPatientJob` is intentionally non-idempotent: if the current job status is already `cancelled`, the function must throw a `BadRequestError` rather than silently succeeding.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-04-17T21:27:11.591Z
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/resource-diff/utils.ts:0-0
Timestamp: 2025-04-17T21:27:11.591Z
Learning: For the function `getSupportedResources` in resource-diff utils, returning an empty array for unsupported EHR sources is preferred over throwing an error, as it allows the code to gracefully handle unsupported sources without disruption.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-18T18:50:40.968Z
Learnt from: keshavsaharia
PR: metriport/metriport#4045
File: packages/core/src/external/surescripts/fhir/shared.ts:87-93
Timestamp: 2025-06-18T18:50:40.968Z
Learning: The `getResourceFromResourceMap` function in `packages/core/src/external/surescripts/fhir/shared.ts` works correctly as implemented. The comparison `resourceValue === resourceMap[key]` where `resourceValue = resource[key]` is intentional and functions as designed, despite appearing to compare different types.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-20T15:35:00.546Z
Learnt from: thomasyopes
PR: metriport/metriport#3891
File: packages/api/src/routes/internal/ehr/patient.ts:142-142
Timestamp: 2025-06-20T15:35:00.546Z
Learning: In the EHR resource diff contribution system, there are no "invalid" resource types. The system is designed to gracefully handle any resource type string by attempting to find a corresponding bundle, and if no bundle exists for that resource type, it will simply exit early rather than throwing an error.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-11T21:39:26.805Z
Learnt from: thomasyopes
PR: metriport/metriport#4000
File: packages/core/src/external/ehr/athenahealth/index.ts:504-507
Timestamp: 2025-06-11T21:39:26.805Z
Learning: In AthenaHealth write-back (`packages/core/src/external/ehr/athenahealth/index.ts`), only the condition statuses “relapse” and “recurrence” are currently mapped to AthenaHealth problem statuses (“CHRONIC”); other FHIR clinicalStatus values (e.g., “active”, “resolved”, “inactive”, “remission”) are not yet supported.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-27T01:50:14.227Z
Learnt from: lucasdellabella
PR: metriport/metriport#4098
File: packages/api/src/routes/medical/dtos/tcm-encounter-dto.ts:10-22
Timestamp: 2025-06-27T01:50:14.227Z
Learning: In packages/api/src/routes/medical/dtos/tcm-encounter-dto.ts, the patient fields patientData.firstName, patientData.lastName, and patientData.dob are guaranteed to be non-nullable values, so defensive null/undefined checks are not needed when accessing these fields.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-19T13:53:09.828Z
Learnt from: leite08
PR: metriport/metriport#3814
File: packages/core/src/command/consolidated/search/fhir-resource/search-semantic.ts:36-36
Timestamp: 2025-05-19T13:53:09.828Z
Learning: In the `getConsolidatedPatientData` function from `metriport/core/command/consolidated/consolidated-get`, only the `patient` parameter is required. Other parameters like `requestId`, `resources`, `dateFrom`, `dateTo`, `fromDashboard`, and `forceDataFromFhir` are all optional.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-07-18T09:33:29.581Z
Learnt from: thomasyopes
PR: metriport/metriport#4165
File: packages/api/src/routes/internal/analytics-platform/index.ts:28-28
Timestamp: 2025-07-18T09:33:29.581Z
Learning: In packages/api/src/routes/internal/analytics-platform/index.ts, the patientId parameter handling is intentionally inconsistent: the `/fhir-to-csv` and `/fhir-to-csv/transform` endpoints require patientId (using getFromQueryOrFail) for single patient processing, while the `/fhir-to-csv/batch` endpoint treats patientId as optional (using getFromQuery) because it's designed to run over all patients when no specific patient ID is provided.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-16T17:03:23.069Z
Learnt from: leite08
PR: metriport/metriport#4034
File: packages/core/src/command/patient-import/patient-or-record-failed.ts:19-30
Timestamp: 2025-06-16T17:03:23.069Z
Learning: In patient import error handling functions like setPatientOrRecordFailed, the team prefers Promise.all over Promise.allSettled to ensure atomic all-or-nothing behavior - if either the job update or patient record update fails, the entire operation should fail rather than allowing partial success.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-07-17T00:19:09.217Z
Learnt from: lucasdellabella
PR: metriport/metriport#4181
File: packages/api/src/command/medical/patient/batch-utils.ts:63-78
Timestamp: 2025-07-17T00:19:09.217Z
Learning: In batch processing utilities like packages/api/src/command/medical/patient/batch-utils.ts, user lucasdellabella prefers to keep the utility focused on its core responsibility (batching) and delegate retry logic to the calling code, rather than implementing retry mechanisms within the utility itself.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- 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/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts (1)
312-315
: Labs silently skipped – please confirm intent
if (!writeBackResourceType || writeBackResourceType === "lab") continue;
This drops every lab observation from write-back even when
writeBackFilters.lab?.disabled
is false.
If removal of lab write-back is not part of this PR’s scope, this line will prevent any lab data from ever reaching the EHR. Please double-check that this behaviour is desired.
...rc/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but update the release plan to update existing CX's mappings to use the latestOnly
flag if they need it
Good call! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts (2)
608-653
: Avoid JSON.stringify on grouped vitals in error logs (size + PHI risk)Logging the full GroupedVitalsByDate (Date + many Observations) can bloat logs and leak PHI.
Use a concise fingerprint instead:
} catch (error) { if (error instanceof BadRequestError || error instanceof NotFoundError) return; - const resourceToString = JSON.stringify(resource); - log(`Failed to write back resource ${resourceToString}. Cause: ${errorToString(error)}`); - writeBackErrors.push({ error, resource: resourceToString }); + const [chartDate, observations] = resource as GroupedVitalsByDate; + const fingerprint = `GroupedVitals[${chartDate.toISOString()}, count=${observations.length}]`; + log(`Failed to write back grouped vitals ${fingerprint}. Cause: ${errorToString(error)}`); + writeBackErrors.push({ error, resource: fingerprint }); }
792-795
: Timezone-safe construction of chart date from YYYY-MM-DDnew Date("YYYY-MM-DD") normalizes to UTC midnight and can shift the calendar date in local timezones. Prefer buildDayjs and attach Z explicitly.
- return Object.entries(groupedVitals).map(([chartDate, observations]) => [ - new Date(chartDate), - observations, - ]); + return Object.entries(groupedVitals).map(([chartDate, observations]) => [ + buildDayjs(`${chartDate}T00:00:00Z`).toDate(), + observations, + ]);Optionally sort groups by chartDate descending for determinism before returning.
🧹 Nitpick comments (1)
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts (1)
561-570
: Naming consistency: rename skipVitalLoinCode → skipVitalLoincCodeMatches the lab variant (skipLabLoincCode) and the field name loincCodes.
Apply within this hunk:
-export function skipVitalLoinCode( +export function skipVitalLoincCode( observation: Observation, writeBackFilters: WriteBackFiltersPerResourceType ): boolean { const loincCodes = writeBackFilters?.vital?.loincCodes; if (!loincCodes) return false; const loincCode = getObservationLoincCode(observation); if (!loincCode) return true; return !loincCodes.includes(loincCode); }Additionally update its call site at Line 398:
// Line 398 - if (skipVitalLoinCode(observation, writeBackFilters)) return false; + if (skipVitalLoincCode(observation, writeBackFilters)) return false;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/external/ehr/job/bundle/write-back-bundles/__tests__/consts.ts
(1 hunks)packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
(9 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.{js,jsx,ts,tsx}
: Don’t use null inside the app, only on code interacting with external interfaces/services, like DB and HTTP; convert to undefined before sending inwards into the code
Useconst
whenever possible
Useasync/await
instead of.then()
Naming: classes, enums: PascalCase
Naming: constants, variables, functions: camelCase
Naming: file names: kebab-case
Naming: Don’t use negative names, likenotEnabled
, preferisDisabled
If possible, use decomposing objects for function parameters
Prefer Nullish Coalesce (??) than the OR operator (||) when you want to provide a default value
Avoid creating arrow functions
Use truthy syntax instead ofin
- i.e.,if (data.link)
notif ('link' in data)
While handling errors, keep the stack trace around: if you create a new Error (e.g., MetriportError), make sure to pass the original error as the new one’s cause so the stack trace is available upstream.
max column length is 100 chars
multi-line comments use/** */
top-level comments go after the import (save pre-import to basic file header, like license)
move literals to constants declared after imports when possible
Files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/__tests__/consts.ts
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Use types whenever possible
Files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/__tests__/consts.ts
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
**/*.ts
⚙️ CodeRabbit Configuration File
**/*.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
const
instead oflet
- Avoid
any
and casting fromany
to 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
null
inside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefined
before sending inwards into the code- Use
async/await
instead 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 (seeprocessAsyncError
andemptyFunction
depending on the case)- Date and Time
- Always use
buildDayjs()
to createdayjs
instances- 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
- U...
Files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/__tests__/consts.ts
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
🧠 Learnings (16)
📚 Learning: 2025-06-11T21:39:26.805Z
Learnt from: thomasyopes
PR: metriport/metriport#4000
File: packages/core/src/external/ehr/athenahealth/index.ts:504-507
Timestamp: 2025-06-11T21:39:26.805Z
Learning: In AthenaHealth write-back (`packages/core/src/external/ehr/athenahealth/index.ts`), only the condition statuses “relapse” and “recurrence” are currently mapped to AthenaHealth problem statuses (“CHRONIC”); other FHIR clinicalStatus values (e.g., “active”, “resolved”, “inactive”, “remission”) are not yet supported.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/__tests__/consts.ts
📚 Learning: 2025-05-20T21:26:26.804Z
Learnt from: leite08
PR: metriport/metriport#3814
File: packages/api/src/routes/internal/medical/patient-consolidated.ts:141-174
Timestamp: 2025-05-20T21:26:26.804Z
Learning: The functionality introduced in packages/api/src/routes/internal/medical/patient-consolidated.ts is planned to be refactored in downstream PR #3857, including improvements to error handling and validation.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-19T22:44:49.393Z
Learnt from: thomasyopes
PR: metriport/metriport#4061
File: packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/run-job.ts:34-47
Timestamp: 2025-06-19T22:44:49.393Z
Learning: In packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/run-job.ts, the team prefers to keep refreshEhrBundles operations grouped using fire-and-forget pattern rather than awaiting all operations with Promise.allSettled(). This allows individual refresh operations to run independently without blocking the job runner.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-03-19T13:43:19.104Z
Learnt from: leite08
PR: metriport/metriport#3463
File: packages/api/src/command/medical/patient/settings/create-patient-settings.ts:199-199
Timestamp: 2025-03-19T13:43:19.104Z
Learning: In the Metriport codebase, avoid sending objects directly to string templates in logs without first converting them to strings. Use JSON.stringify() for objects or array.join(', ') for arrays of primitives to ensure proper formatting and readability of logs.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-18T21:03:16.752Z
Learnt from: RamilGaripov
PR: metriport/metriport#3976
File: packages/api/src/command/medical/cohort/update-cohort.ts:26-26
Timestamp: 2025-06-18T21:03:16.752Z
Learning: In the Metriport codebase, logging cohort dataValues objects doesn't cause multiline logs in their AWS environment, so JSON.stringify() on cohort objects is acceptable.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-07-20T01:18:28.093Z
Learnt from: leite08
PR: metriport/metriport#4215
File: packages/core/src/command/analytics-platform/config.ts:4-13
Timestamp: 2025-07-20T01:18:28.093Z
Learning: User leite08 prefers that code improvement suggestions (like consolidating duplicated schemas) be made on feature PRs rather than release PRs. Such improvements should be deferred to follow-up PRs during the release cycle.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-07-27T00:40:32.149Z
Learnt from: leite08
PR: metriport/metriport#4255
File: packages/core/src/command/analytics-platform/fhir-to-csv/command/fhir-to-csv/fhir-to-csv-direct.ts:19-20
Timestamp: 2025-07-27T00:40:32.149Z
Learning: User leite08 acknowledged the duplicate inputBundle spread issue in packages/core/src/command/analytics-platform/fhir-to-csv/command/fhir-to-csv/fhir-to-csv-direct.ts and will fix it in a follow-up PR rather than in the current patch release PR #4255.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-28T05:28:26.896Z
Learnt from: lucasdellabella
PR: metriport/metriport#3907
File: packages/terminology/src/seed/seed-ndc-from-fda.ts:91-113
Timestamp: 2025-05-28T05:28:26.896Z
Learning: On PRs with "release" in the title, only provide very high priority feedback and avoid lower priority suggestions like refactoring or optimization improvements.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-05T14:09:59.683Z
Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/command/get-bundle-by-resource-type.ts:31-37
Timestamp: 2025-06-05T14:09:59.683Z
Learning: When EHR handlers in mapping objects are set to `undefined` and there are downstream PRs mentioned in the PR objectives, this typically indicates a staged rollout approach where the core structure is implemented first and specific handler implementations follow in subsequent PRs.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-07-16T00:54:56.156Z
Learnt from: leite08
PR: metriport/metriport#4193
File: packages/carequality-sdk/src/client/carequality-fhir.ts:198-208
Timestamp: 2025-07-16T00:54:56.156Z
Learning: User leite08 accepted the risk of parameter mutation in packages/carequality-sdk/src/client/carequality-fhir.ts when modifying org.active = false directly on input parameters, choosing to proceed despite the violation of the "Avoid modifying objects received as parameter" coding guideline.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-03-17T15:30:34.647Z
Learnt from: thomasyopes
PR: metriport/metriport#3466
File: packages/api/src/routes/ehr/elation/chart.ts:20-37
Timestamp: 2025-03-17T15:30:34.647Z
Learning: The request body parsing in packages/api/src/routes/ehr/elation/chart.ts is tracked with TODO referencing issue #2170 and will be handled in a separate PR, not in PR #3466.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-30T01:09:55.013Z
Learnt from: keshavsaharia
PR: metriport/metriport#3885
File: packages/core/src/external/surescripts/client.ts:0-0
Timestamp: 2025-05-30T01:09:55.013Z
Learning: In packages/core/src/external/surescripts/client.ts, the Surescripts date formatting should use local time according to their documentation ("message-date-must-be-local"), which means buildDayjs() cannot be used since it defaults to UTC. The specific use case for transmission date formatting requires dayjs to be used directly to preserve local time behavior.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-06-18T17:24:31.650Z
Learnt from: keshavsaharia
PR: metriport/metriport#4045
File: packages/core/src/external/surescripts/__tests__/file-names.test.ts:21-21
Timestamp: 2025-06-18T17:24:31.650Z
Learning: The team does all testing and deploying in PST local time, which helps avoid issues with date-dependent code in tests like buildRequestFileName that use dayjs().format('YYYYMMDD').
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-04-23T23:57:11.544Z
Learnt from: husainsyed
PR: metriport/metriport#3663
File: packages/shared/package.json:89-89
Timestamp: 2025-04-23T23:57:11.544Z
Learning: In the Metriport codebase, both Day.js and Luxon date libraries are used intentionally for different purposes: Luxon specifically for ISO date validation (where Day.js has limitations) and Day.js for other date operations. There is no plan to fully migrate from Day.js to Luxon.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-08-11T21:33:38.272Z
Learnt from: thomasyopes
PR: metriport/metriport#4328
File: packages/core/src/external/ehr/healthie/index.ts:915-919
Timestamp: 2025-08-11T21:33:38.272Z
Learning: In packages/shared/src/interface/external/ehr/healthie/medication.ts, the Medication schema defines end_date as z.string().nullable(), which means the field can only be string or null, never undefined. Therefore, checking medication.end_date !== null is sufficient to guard against null values before using buildDayjs().
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
📚 Learning: 2025-05-28T19:22:09.281Z
Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:27-49
Timestamp: 2025-05-28T19:22:09.281Z
Learning: In packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.
Applied to files:
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
🧬 Code Graph Analysis (1)
packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts (7)
packages/shared/src/interface/external/ehr/shared.ts (1)
WriteBackFiltersPerResourceType
(44-44)packages/fhir-sdk/src/index.ts (1)
Observation
(52-52)packages/core/src/external/ehr/command/write-back/shared.ts (1)
writeBackResource
(28-61)packages/shared/src/index.ts (1)
BadRequestError
(42-42)packages/core/src/external/ehr/command/write-back/condition.ts (1)
getEhrWriteBackConditionPrimaryCode
(58-66)packages/core/src/external/ehr/shared.ts (4)
getConditionSnomedCode
(447-451)getConditionIcd10Code
(431-435)getConditionStartDate
(453-455)formatDate
(95-101)packages/core/src/external/ehr/command/write-back/grouped-vitals.ts (1)
GroupedVitalsByDate
(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/core/src/external/ehr/job/bundle/write-back-bundles/__tests__/consts.ts (1)
250-271
: No stale “problems” references foundA search for
writeBackFilters?.problems
and property keysproblems:
only returned commented examples in thecomplex_dead_resource_counter.py
script (lines 73 and 75), not active code. All test constants use the singularproblem
wrapper as intended—no further changes are needed.packages/core/src/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts (3)
18-18
: Imports align with new filtering logic — LGTM
- SNOMED_CODE used to pick primary code system for conditions.
- GroupedVitalsByDate/isWriteBackGroupedVitalsEhr wired for grouped vitals path.
- Condition helpers (ICD-10/SNOMED/start date) correctly imported.
Also applies to: 31-35, 39-43
314-314
: Confirm intentional exclusion of standalone lab ObservationsYou now skip write-back for Observations with type "lab" as primaries. This relies on DiagnosticReport write-back to include their Observations as secondary resources. Is it acceptable to drop standalone lab Observations that aren’t referenced by a DiagnosticReport?
If needed, we can add a fallback path for orphan lab Observations.
367-367
: problems → problem wrapper update — consistent usageBoth the disabled check and chronicityFilter read from problem now. Looks consistent with schema/type changes.
Also applies to: 410-411
...rc/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
Outdated
Show resolved
Hide resolved
...rc/external/ehr/job/bundle/write-back-bundles/ehr-write-back-resource-diff-bundles-direct.ts
Outdated
Show resolved
Hide resolved
…-elation-write-back-fixes Signed-off-by: Thomas Yopes <thomasyopes@Mac.attlocal.net>
Ref: ENG-559 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Mac.attlocal.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/core/src/external/ehr/healthie/command/get-resource-bundle-by-resource-id.ts (1)
22-26
: Optional: be explicit with tokenId inclusionCurrent spread is fine. If you want stricter intent, pass
tokenId
explicitly and rely on the client to ignore undefined.- const client = await createHealthieClient({ - cxId, - practiceId, - ...(tokenId && { tokenId }), - }); + const client = await createHealthieClient({ + cxId, + practiceId, + tokenId, + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/core/src/external/ehr/command/get-resource-bundle-by-resource-id.ts
(2 hunks)packages/core/src/external/ehr/healthie/command/get-resource-bundle-by-resource-id.ts
(1 hunks)packages/core/src/external/ehr/healthie/index.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.{js,jsx,ts,tsx}
: Don’t use null inside the app, only on code interacting with external interfaces/services, like DB and HTTP; convert to undefined before sending inwards into the code
Useconst
whenever possible
Useasync/await
instead of.then()
Naming: classes, enums: PascalCase
Naming: constants, variables, functions: camelCase
Naming: file names: kebab-case
Naming: Don’t use negative names, likenotEnabled
, preferisDisabled
If possible, use decomposing objects for function parameters
Prefer Nullish Coalesce (??) than the OR operator (||) when you want to provide a default value
Avoid creating arrow functions
Use truthy syntax instead ofin
- i.e.,if (data.link)
notif ('link' in data)
While handling errors, keep the stack trace around: if you create a new Error (e.g., MetriportError), make sure to pass the original error as the new one’s cause so the stack trace is available upstream.
max column length is 100 chars
multi-line comments use/** */
top-level comments go after the import (save pre-import to basic file header, like license)
move literals to constants declared after imports when possible
Files:
packages/core/src/external/ehr/command/get-resource-bundle-by-resource-id.ts
packages/core/src/external/ehr/healthie/command/get-resource-bundle-by-resource-id.ts
packages/core/src/external/ehr/healthie/index.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Use types whenever possible
Files:
packages/core/src/external/ehr/command/get-resource-bundle-by-resource-id.ts
packages/core/src/external/ehr/healthie/command/get-resource-bundle-by-resource-id.ts
packages/core/src/external/ehr/healthie/index.ts
**/*.ts
⚙️ CodeRabbit Configuration File
**/*.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
const
instead oflet
- Avoid
any
and casting fromany
to 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
null
inside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefined
before sending inwards into the code- Use
async/await
instead 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 (seeprocessAsyncError
andemptyFunction
depending on the case)- Date and Time
- Always use
buildDayjs()
to createdayjs
instances- 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
- U...
Files:
packages/core/src/external/ehr/command/get-resource-bundle-by-resource-id.ts
packages/core/src/external/ehr/healthie/command/get-resource-bundle-by-resource-id.ts
packages/core/src/external/ehr/healthie/index.ts
🧠 Learnings (5)
📚 Learning: 2025-05-28T19:22:09.281Z
Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:27-49
Timestamp: 2025-05-28T19:22:09.281Z
Learning: In packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.
Applied to files:
packages/core/src/external/ehr/command/get-resource-bundle-by-resource-id.ts
packages/core/src/external/ehr/healthie/command/get-resource-bundle-by-resource-id.ts
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-05-28T19:20:47.442Z
Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:0-0
Timestamp: 2025-05-28T19:20:47.442Z
Learning: In packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.
Applied to files:
packages/core/src/external/ehr/command/get-resource-bundle-by-resource-id.ts
packages/core/src/external/ehr/healthie/command/get-resource-bundle-by-resource-id.ts
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-04-21T17:07:30.574Z
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
Applied to files:
packages/core/src/external/ehr/command/get-resource-bundle-by-resource-id.ts
packages/core/src/external/ehr/healthie/command/get-resource-bundle-by-resource-id.ts
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-05-20T21:26:26.804Z
Learnt from: leite08
PR: metriport/metriport#3814
File: packages/api/src/routes/internal/medical/patient-consolidated.ts:141-174
Timestamp: 2025-05-20T21:26:26.804Z
Learning: The functionality introduced in packages/api/src/routes/internal/medical/patient-consolidated.ts is planned to be refactored in downstream PR #3857, including improvements to error handling and validation.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-06-06T16:45:31.832Z
Learnt from: thomasyopes
PR: metriport/metriport#3970
File: packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts:17-17
Timestamp: 2025-06-06T16:45:31.832Z
Learning: The writeMedicationToChart function in packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts returns a response that is not currently used by any consumers, so changes to its return type are not breaking changes in practice.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
🧬 Code Graph Analysis (2)
packages/core/src/external/ehr/healthie/command/get-resource-bundle-by-resource-id.ts (4)
packages/core/src/external/ehr/command/get-resource-bundle-by-resource-id.ts (2)
getResourceBundleByResourceId
(26-32)GetResourceBundleByResourceIdClientRequest
(21-24)packages/core/src/external/ehr/healthie/index.ts (1)
getResourceBundleByResourceId
(622-656)packages/fhir-sdk/src/index.ts (1)
Bundle
(37-37)packages/shared/src/index.ts (1)
BadRequestError
(42-42)
packages/core/src/external/ehr/healthie/index.ts (3)
packages/fhir-sdk/src/index.ts (1)
Bundle
(37-37)packages/shared/src/index.ts (1)
BadRequestError
(42-42)packages/core/src/external/ehr/shared.ts (1)
fetchEhrBundleUsingCache
(793-813)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: check-branch
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/core/src/external/ehr/command/get-resource-bundle-by-resource-id.ts (2)
7-7
: LGTM: Healthie handler import addedImport path and aliasing are correct.
47-47
: LGTM: Wire Healthie into the handler mapEnables Healthie support for resource-bundle lookup by resourceId via the new command.
packages/core/src/external/ehr/healthie/command/get-resource-bundle-by-resource-id.ts (2)
27-34
: LGTM: Delegation to client API is clear and typedStraightforward call; parameters align with the Healthie client’s API.
19-21
: No callers useuseCachedBundle: false
; cache-only enforcement is safeVerified via a repo-wide search that no call sites pass
useCachedBundle: false
togetResourceBundleByResourceId
. Explicitly throwing in that case makes the contract clear without breaking existing usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/core/src/external/ehr/healthie/index.ts (2)
85-86
: Non-blocking: Prefer const tuple for precise union typing (not re-opening)This was previously discussed and intentionally kept as
ResourceType[]
. Re-flagging only for traceability.
94-100
: Non-blocking: Type guard can avoid widening via includes signature (not re-opening)Prior feedback suggested casting the array to
readonly string[]
beforeincludes
to preserve literal unions; acknowledged as intentionally kept as-is.
🧹 Nitpick comments (1)
packages/core/src/external/ehr/healthie/index.ts (1)
622-657
: Document intentional empty-bundle caching on cache-miss for resourceIdGiven the retrieved learnings indicate empty bundles are an acceptable state for non-existent resources in Healthie, consider adding a short JSDoc to prevent future “optimize the fetch” changes from regressing this behavior.
Apply:
+ /** + * Fetch a bundle by resourceId using cache-only semantics. + * Intentionally returns (and persists) an empty bundle when the resource does not exist. + * This is the desired behavior for Healthie (see ENG-559 / PR #4315). + */ async getResourceBundleByResourceId({ cxId, metriportPatientId, healthiePatientId, resourceType, resourceId, }: { cxId: string; metriportPatientId: string; healthiePatientId: string; resourceType: string; resourceId: string; }): Promise<Bundle> { if ( !isSupportedHealthieResource(resourceType) && !isSupportedHealthieReferenceResource(resourceType) ) { throw new BadRequestError("Invalid resource type", undefined, { healthiePatientId, resourceId, resourceType, }); } const bundle = await fetchEhrBundleUsingCache({ ehr: EhrSources.healthie, cxId, metriportPatientId, ehrPatientId: healthiePatientId, resourceType, resourceId, fetchResourcesFromEhr: () => Promise.resolve([]), useCachedBundle: true, }); return bundle; }Note: This aligns with the documented intent in the retrieved learnings for this file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/external/ehr/healthie/index.ts
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.{js,jsx,ts,tsx}
: Don’t use null inside the app, only on code interacting with external interfaces/services, like DB and HTTP; convert to undefined before sending inwards into the code
Useconst
whenever possible
Useasync/await
instead of.then()
Naming: classes, enums: PascalCase
Naming: constants, variables, functions: camelCase
Naming: file names: kebab-case
Naming: Don’t use negative names, likenotEnabled
, preferisDisabled
If possible, use decomposing objects for function parameters
Prefer Nullish Coalesce (??) than the OR operator (||) when you want to provide a default value
Avoid creating arrow functions
Use truthy syntax instead ofin
- i.e.,if (data.link)
notif ('link' in data)
While handling errors, keep the stack trace around: if you create a new Error (e.g., MetriportError), make sure to pass the original error as the new one’s cause so the stack trace is available upstream.
max column length is 100 chars
multi-line comments use/** */
top-level comments go after the import (save pre-import to basic file header, like license)
move literals to constants declared after imports when possible
Files:
packages/core/src/external/ehr/healthie/index.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Use types whenever possible
Files:
packages/core/src/external/ehr/healthie/index.ts
**/*.ts
⚙️ CodeRabbit Configuration File
**/*.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
const
instead oflet
- Avoid
any
and casting fromany
to 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
null
inside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefined
before sending inwards into the code- Use
async/await
instead 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 (seeprocessAsyncError
andemptyFunction
depending on the case)- Date and Time
- Always use
buildDayjs()
to createdayjs
instances- 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
- U...
Files:
packages/core/src/external/ehr/healthie/index.ts
🧠 Learnings (22)
📚 Learning: 2025-05-20T21:26:26.804Z
Learnt from: leite08
PR: metriport/metriport#3814
File: packages/api/src/routes/internal/medical/patient-consolidated.ts:141-174
Timestamp: 2025-05-20T21:26:26.804Z
Learning: The functionality introduced in packages/api/src/routes/internal/medical/patient-consolidated.ts is planned to be refactored in downstream PR #3857, including improvements to error handling and validation.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-06-06T16:45:31.832Z
Learnt from: thomasyopes
PR: metriport/metriport#3970
File: packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts:17-17
Timestamp: 2025-06-06T16:45:31.832Z
Learning: The writeMedicationToChart function in packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts returns a response that is not currently used by any consumers, so changes to its return type are not breaking changes in practice.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-08-11T23:36:57.551Z
Learnt from: thomasyopes
PR: metriport/metriport#4315
File: packages/core/src/external/ehr/healthie/index.ts:622-657
Timestamp: 2025-08-11T23:36:57.551Z
Learning: In the Healthie EHR integration's `getResourceBundleByResourceId` method, returning an empty bundle when a resource doesn't exist (by using `fetchResourcesFromEhr: () => Promise.resolve([])`) is intentional design. Empty bundles are an acceptable state for non-existent resources rather than throwing NotFoundError.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-05-31T21:58:28.502Z
Learnt from: leite08
PR: metriport/metriport#3940
File: packages/core/src/command/consolidated/search/fhir-resource/search-consolidated.ts:82-86
Timestamp: 2025-05-31T21:58:28.502Z
Learning: In packages/core/src/command/consolidated/search/fhir-resource/search-consolidated.ts, the mutable array operations using push() on resourcesMutable and hydratedMutable (lines 82-86 and 100-103) have been explicitly accepted as exceptions to the immutability guidelines after previous discussion.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-06-18T18:50:40.968Z
Learnt from: keshavsaharia
PR: metriport/metriport#4045
File: packages/core/src/external/surescripts/fhir/shared.ts:87-93
Timestamp: 2025-06-18T18:50:40.968Z
Learning: The `getResourceFromResourceMap` function in `packages/core/src/external/surescripts/fhir/shared.ts` works correctly as implemented. The comparison `resourceValue === resourceMap[key]` where `resourceValue = resource[key]` is intentional and functions as designed, despite appearing to compare different types.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-04-21T03:47:54.332Z
Learnt from: lucasdellabella
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/resource-diff/steps/start/ehr-start-resource-diff-local.ts:0-0
Timestamp: 2025-04-21T03:47:54.332Z
Learning: In the EHR resource diff implementation, bundles should always have an `entry` property, and code should be allowed to throw if `entry` is missing rather than using null-coalescing operators, as this represents an unexpected state that should be caught and addressed.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-06-19T22:44:49.393Z
Learnt from: thomasyopes
PR: metriport/metriport#4061
File: packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/run-job.ts:34-47
Timestamp: 2025-06-19T22:44:49.393Z
Learning: In packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/run-job.ts, the team prefers to keep refreshEhrBundles operations grouped using fire-and-forget pattern rather than awaiting all operations with Promise.allSettled(). This allows individual refresh operations to run independently without blocking the job runner.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-04-17T21:27:11.591Z
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/resource-diff/utils.ts:0-0
Timestamp: 2025-04-17T21:27:11.591Z
Learning: For the function `getSupportedResources` in resource-diff utils, returning an empty array for unsupported EHR sources is preferred over throwing an error, as it allows the code to gracefully handle unsupported sources without disruption.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-05-19T18:24:41.632Z
Learnt from: thomasyopes
PR: metriport/metriport#3870
File: packages/core/src/external/ehr/bundle/job/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.ts:55-91
Timestamp: 2025-05-19T18:24:41.632Z
Learning: In EHR bundles related to resource diff computations, bundle creation operations that are for auditing purposes should not stop the main job flow if they fail. These operations typically use try/catch blocks that log errors without rethrowing them, which is the intended behavior.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-07-27T00:40:32.149Z
Learnt from: leite08
PR: metriport/metriport#4255
File: packages/core/src/command/analytics-platform/fhir-to-csv/command/fhir-to-csv/fhir-to-csv-direct.ts:19-20
Timestamp: 2025-07-27T00:40:32.149Z
Learning: User leite08 acknowledged the duplicate inputBundle spread issue in packages/core/src/command/analytics-platform/fhir-to-csv/command/fhir-to-csv/fhir-to-csv-direct.ts and will fix it in a follow-up PR rather than in the current patch release PR #4255.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-08-04T17:59:02.571Z
Learnt from: lucasdellabella
PR: metriport/metriport#4020
File: packages/fhir-sdk/src/index.ts:312-326
Timestamp: 2025-08-04T17:59:02.571Z
Learning: In packages/fhir-sdk/src/index.ts, the inconsistent error handling between the `total` getter (which throws an error) and the `entry` getter (which returns an empty array) when bundle.entry is undefined is intentional design by user lucasdellabella, serving different purposes where some operations need to fail fast while others can gracefully degrade.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-06-20T15:35:00.546Z
Learnt from: thomasyopes
PR: metriport/metriport#3891
File: packages/api/src/routes/internal/ehr/patient.ts:142-142
Timestamp: 2025-06-20T15:35:00.546Z
Learning: In the EHR resource diff contribution system, there are no "invalid" resource types. The system is designed to gracefully handle any resource type string by attempting to find a corresponding bundle, and if no bundle exists for that resource type, it will simply exit early rather than throwing an error.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-05-08T19:41:36.533Z
Learnt from: thomasyopes
PR: metriport/metriport#3788
File: packages/api/src/external/ehr/shared/utils/bundle.ts:83-93
Timestamp: 2025-05-08T19:41:36.533Z
Learning: In the Metriport codebase, the team prefers to let errors bubble up naturally in some cases rather than adding explicit error handling at every layer, as demonstrated in the refreshEhrBundle function in the bundle.ts file.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-05-28T19:22:09.281Z
Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:27-49
Timestamp: 2025-05-28T19:22:09.281Z
Learning: In packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-05-28T19:20:47.442Z
Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:0-0
Timestamp: 2025-05-28T19:20:47.442Z
Learning: In packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-04-23T21:19:08.443Z
Learnt from: thomasyopes
PR: metriport/metriport#3709
File: packages/api/src/external/ehr/canvas/command/bundle/fetch-resource-diff-bundle.ts:62-73
Timestamp: 2025-04-23T21:19:08.443Z
Learning: In the metriport codebase, factory functions like `fetchResourceDiffBundle` intentionally let errors bubble up to the caller rather than handling them at the factory function level. This is a consistent pattern across factory implementations.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-04-21T17:07:30.574Z
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-06-18T00:18:59.144Z
Learnt from: leite08
PR: metriport/metriport#4043
File: packages/core/src/command/consolidated/consolidated-create.ts:0-0
Timestamp: 2025-06-18T00:18:59.144Z
Learning: In the consolidated bundle creation process (packages/core/src/command/consolidated/consolidated-create.ts), the team prefers strict failure behavior where if any component (including pharmacy resources) fails to be retrieved, the entire consolidation should fail rather than gracefully degrading. This ensures data consistency and prevents incomplete bundles.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-06-01T02:28:19.913Z
Learnt from: leite08
PR: metriport/metriport#3944
File: packages/core/src/command/consolidated/search/fhir-resource/ingest-lexical.ts:53-53
Timestamp: 2025-06-01T02:28:19.913Z
Learning: In packages/core/src/command/consolidated/search/fhir-resource/ingest-lexical.ts, the processErrors function intentionally throws MetriportError to bubble errors up the call stack rather than handling them locally. This is by design - errors from ingestPatientConsolidated should propagate upward rather than being caught at immediate calling locations.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-06-25T01:56:08.732Z
Learnt from: thomasyopes
PR: metriport/metriport#4090
File: packages/core/src/command/conversion-fhir/conversion-fhir-direct.ts:18-25
Timestamp: 2025-06-25T01:56:08.732Z
Learning: In packages/core/src/command/conversion-fhir/conversion-fhir-direct.ts, the ConversionFhirDirect class is designed for local testing purposes and does not require comprehensive error handling for HTTP requests, as confirmed by the user thomasyopes.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-07-16T00:54:56.156Z
Learnt from: leite08
PR: metriport/metriport#4193
File: packages/carequality-sdk/src/client/carequality-fhir.ts:198-208
Timestamp: 2025-07-16T00:54:56.156Z
Learning: User leite08 accepted the risk of parameter mutation in packages/carequality-sdk/src/client/carequality-fhir.ts when modifying org.active = false directly on input parameters, choosing to proceed despite the violation of the "Avoid modifying objects received as parameter" coding guideline.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
📚 Learning: 2025-06-05T16:28:33.728Z
Learnt from: thomasyopes
PR: metriport/metriport#3960
File: packages/core/src/external/ehr/command/get-appointments/ehr-get-appointments-direct.ts:11-19
Timestamp: 2025-06-05T16:28:33.728Z
Learning: In the EhrGetAppointmentsDirect class in packages/core/src/external/ehr/command/get-appointments/ehr-get-appointments-direct.ts, type casting `as T[]` for the handler return value is considered safe and doesn't require additional runtime validation by the user thomasyopes.
Applied to files:
packages/core/src/external/ehr/healthie/index.ts
🧬 Code Graph Analysis (1)
packages/core/src/external/ehr/healthie/index.ts (4)
packages/fhir-sdk/src/index.ts (1)
Bundle
(37-37)packages/shared/src/index.ts (1)
BadRequestError
(42-42)packages/core/src/external/ehr/shared.ts (1)
fetchEhrBundleUsingCache
(793-813)packages/shared/src/common/date.ts (1)
buildDayjs
(93-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (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)
Ref: ENG-559
Description
Testing
Release Plan
Summary by CodeRabbit
New Features
Bug Fixes
Refactor