-
Notifications
You must be signed in to change notification settings - Fork 72
Eng 000 backmerge #4417
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
Eng 000 backmerge #4417
Conversation
RELEASE - ENG-861
RELEASE ENG-484: Surescripts updates and improvements
Ref: ENG-000 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Mac.attlocal.net>
…tribution PATCH RELEASE feat(ehr): diable healthie via flag
Ref: ENG-000 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Mac.attlocal.net>
…tribution PATCH RELEASE feat(ehr): fix dist
…backmerge Signed-off-by: Thomas Yopes <thomasyopes@Mac.attlocal.net>
WalkthroughAdds Healthie-specific pre-flight validation to contributeResourceDiffBundle, fetching and validating CX mappings and honoring a new contributionDisabled flag. Introduces new schema field contributionDisabled in Healthie secondary mappings. Adds error paths for invalid/missing mappings and early-return when contribution is disabled. No public function signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant Bundle as contributeResourceDiffBundle
participant CX as getCxMappingsByCustomer
participant Schema as healthieSecondaryMappingsSchema
Caller->>Bundle: contributeResourceDiffBundle(ehr, ctx, diff)
alt ehr == Healthie
Bundle->>CX: fetch CX mappings by customer
CX-->>Bundle: mappings[]
alt mappings.length == 0
Bundle-->>Caller: throw MetriportError (missing mapping)
else mappings.length > 1
Bundle-->>Caller: throw MetriportError (multiple mappings)
else exactly 1
Bundle->>Schema: parse secondaryMappings
alt secondaryMappings missing/invalid
Bundle-->>Caller: throw MetriportError (missing/invalid secondary mappings)
else valid
alt secondaryMappings.contributionDisabled == true
Note over Bundle: Early return for Healthie
Bundle-->>Caller: return (no further processing)
else
Note over Bundle: Continue existing contribution flow
Bundle-->>Caller: proceed to standard logic
end
end
end
else Other EHRs
Note over Bundle: Execute existing contribution flow
Bundle-->>Caller: proceed to standard logic
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 (
|
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts
Did you find this useful? React with a 👍 or 👎 |
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: 0
🧹 Nitpick comments (1)
packages/shared/src/interface/external/ehr/healthie/cx-mapping.ts (1)
21-22
: Backmerge checklist failing: source branch isn’t master and PR description lacks linked PR(s).Per our retrieved learnings for backmerge PRs at metriport/metriport, we only verify:
- Source branch is master and destination is develop.
- Description includes a link to at least one related PR (usually the patch PR).
Findings for this PR:
- Source branch is eng-000-backmerge → target develop (not master → develop).
- Description does not include links to related PR(s).
Please confirm whether this backmerge intentionally deviates from the standard flow. If not, retarget from master → develop and add links to the relevant PR(s) in the description.
I can propose a short description template and checklist to include the linked PRs if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts
(2 hunks)packages/shared/src/interface/external/ehr/healthie/cx-mapping.ts
(1 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/shared/src/interface/external/ehr/healthie/cx-mapping.ts
packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Use types whenever possible
Files:
packages/shared/src/interface/external/ehr/healthie/cx-mapping.ts
packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.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/shared/src/interface/external/ehr/healthie/cx-mapping.ts
packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts
🧠 Learnings (17)
📓 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: 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.
📚 Learning: 2025-08-13T21:36:10.069Z
Learnt from: thomasyopes
PR: metriport/metriport#4346
File: packages/core/src/external/ehr/athenahealth/command/get-bundle-by-resource-type.ts:36-41
Timestamp: 2025-08-13T21:36:10.069Z
Learning: For contributionEncounterSummariesEnabled in Athena secondary mappings, strict boolean checking (=== true) is unnecessary because Zod schema validation ensures the value is boolean or undefined, making truthy checks sufficient.
Applied to files:
packages/shared/src/interface/external/ehr/healthie/cx-mapping.ts
📚 Learning: 2025-08-15T00:00:45.080Z
Learnt from: keshavsaharia
PR: metriport/metriport#4370
File: packages/shared/src/domain/patient.ts:5-5
Timestamp: 2025-08-15T00:00:45.080Z
Learning: The patientSchema in packages/shared/src/domain/patient.ts is used in a limited subsystem scope ("SS") and is not the same as other Patient schemas referenced throughout the codebase, making additive optional field changes like externalId safe and non-breaking.
Applied to files:
packages/shared/src/interface/external/ehr/healthie/cx-mapping.ts
📚 Learning: 2025-07-09T17:18:16.731Z
Learnt from: thomasyopes
PR: metriport/metriport#4164
File: packages/core/src/external/ehr/healthie/index.ts:939-953
Timestamp: 2025-07-09T17:18:16.731Z
Learning: In packages/core/src/external/ehr/healthie/index.ts, the immunization.cvx_code field is required and non-nullable according to the zod schema validation (z.coerce.string()), so null checks are not needed in the convertImmunizationToFhir method.
Applied to files:
packages/shared/src/interface/external/ehr/healthie/cx-mapping.ts
📚 Learning: 2025-08-13T21:37:33.680Z
Learnt from: thomasyopes
PR: metriport/metriport#4346
File: packages/core/src/external/ehr/athenahealth/command/get-bundle-by-resource-type.ts:25-29
Timestamp: 2025-08-13T21:37:33.680Z
Learning: In Athena EHR integration (packages/core/src/external/ehr/athenahealth/command/get-bundle-by-resource-type.ts), thomasyopes prefers hard-failing when secondary mappings fetch fails rather than graceful degradation, because these mappings control critical behaviors like contributionEncounterAppointmentTypesBlacklist and contributionEncounterSummariesEnabled. Running with default values when mappings are unavailable could lead to incorrect processing.
Applied to files:
packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.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/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.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/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.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/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.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/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.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/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts
📚 Learning: 2025-08-11T23:36:57.573Z
Learnt from: thomasyopes
PR: metriport/metriport#4315
File: packages/core/src/external/ehr/healthie/index.ts:622-657
Timestamp: 2025-08-11T23:36:57.573Z
Learning: In the Healthie EHR integration's `getResourceBundleByResourceId` method, returning an empty bundle when a resource doesn't exist (by using `fetchResourcesFromEhr: () => Promise.resolve([])`) is intentional design. Empty bundles are an acceptable state for non-existent resources rather than throwing NotFoundError.
Applied to files:
packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.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/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.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/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.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/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.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/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts
📚 Learning: 2025-05-27T16:10:48.223Z
Learnt from: lucasdellabella
PR: metriport/metriport#3866
File: packages/core/src/command/hl7v2-subscriptions/hl7v2-roster-generator.ts:142-151
Timestamp: 2025-05-27T16:10:48.223Z
Learning: In packages/core/src/command/hl7v2-subscriptions/hl7v2-roster-generator.ts, the user prefers to let axios errors bubble up naturally rather than adding try-catch blocks that re-throw with MetriportError wrappers, especially when the calling code already has retry mechanisms in place via simpleExecuteWithRetries.
Applied to files:
packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.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/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts
🧬 Code Graph Analysis (1)
packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts (2)
packages/api/src/command/mapping/cx.ts (1)
getCxMappingsByCustomer
(99-110)packages/shared/src/interface/external/ehr/healthie/cx-mapping.ts (1)
healthieSecondaryMappingsSchema
(10-23)
⏰ 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 (2)
packages/shared/src/interface/external/ehr/healthie/cx-mapping.ts (1)
21-22
: Skipping code-level review per backmerge policy.Given this is a backmerge and the checklist above isn’t satisfied, I’m intentionally not performing a detailed code review for this change.
packages/api/src/external/ehr/shared/command/bundle/contribute-resource-diff-bundle.ts (1)
21-25
: Skipping code-level review per backmerge policy until PR-level checklist is satisfied.Once the backmerge checklist is addressed (source branch and linked PRs in the description), I can proceed if you still want a targeted review of the Healthie pre-flight logic.
Also applies to: 51-74
Description
Release Plan
Summary by CodeRabbit
New Features
Bug Fixes
Chores
No changes to public APIs or signatures.