ENG-513: CW v2 API migration - organizations & facilities #4412
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Part of ENG-200 Signed-off-by: RamilGaripov <ramil@metriport.com>
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Part of ENG-200 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
WalkthroughIntroduces CommonWell v2 APIs and feature-flagged routing, adds v2 organization/facility sync and document downloading, centralizes organization typing onto shared TreatmentType, updates imports from commonwell-v1 to commonwell where applicable, adjusts config with CW_MEMBER_ID, enhances CommonWell SDK models and error handling, and updates API-SDK exports and versions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as Internal API Routes
participant FF as Feature Flags
participant CWv2 as CW v2 Commands
participant CWv1 as CW v1 Commands
Client->>API: PUT /internal/medical/organization
API->>FF: isCommonwellV2EnabledForCx(cxId)
alt v2 enabled
API->>CWv2: createOrUpdateCWOrganizationV2({ oid, data, active, isInitiatorAndResponder:true })
CWv2-->>API: void
else v2 disabled
API->>CWv1: createOrUpdateCWOrganization({ oid, data, active, isObo:false })
CWv1-->>API: void
end
API-->>Client: 200 OK
sequenceDiagram
autonumber
actor Client
participant API as Internal HIE Routes
participant FF as Feature Flags
participant OrgV2 as getParsedOrgOrFailV2
participant OrgV1 as getParsedOrgOrFail
Client->>API: GET /internal/commonwell/ops/organization/:oid
API->>FF: isCommonwellV2EnabledForCx(cxId)
alt v2 enabled
API->>OrgV2: getParsedOrgOrFailV2(oid)
OrgV2-->>API: CwOrgOrFacility
else v2 disabled
API->>OrgV1: getParsedOrgOrFail(oid)
OrgV1-->>API: CWOrganization (v1)
end
API-->>Client: JSON
sequenceDiagram
autonumber
actor Client
participant API as Internal HIE Routes
participant PD as runOrScheduleCwPatientDiscovery
participant Pat as getPatientOrFail
Client->>API: POST /internal/commonwell/patient-discovery/:patientId
API->>Pat: getPatientOrFail({ id, cxId })
Pat-->>API: patient
API->>PD: runOrScheduleCwPatientDiscovery({ patient, facilityId, requestId, ... })
PD-->>API: void
API-->>Client: { requestId }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (10)
✨ 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 (
|
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-200 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
1 task
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/commonwell-sdk/src/client/commonwell-member.ts (1)
211-230: Interim undefined on non-404 statuses: verify all call sites guard against undefined (ENG-668)Acknowledging the temporary behavior documented in ENG-668, returning undefined for non-404 errors can mask real failures. Please ensure all callers treat undefined as “not found or failed” and don’t dereference fields unguarded.
Run this script to list call sites and spot risky dereferences:
#!/bin/bash set -euo pipefail echo "Call sites of getOneOrg(...):" rg -nP '\bgetOneOrg\s*\(' -C2 packages echo echo "Heuristic: places that may dereference without guarding:" rg -nP '(getOneOrg\s*\(.*\).*\n.*\b\w+\.\w+)' -C1 packages || trueIf you find any, consider either: (a) adding explicit guards, or (b) temporarily routing through a helper that normalizes undefined into a NotFoundError for the specific flows that expect fail-fast semantics.
🧹 Nitpick comments (5)
packages/api/src/external/commonwell-v2/command/organization/organization.ts (5)
217-229: Avoidanyin catch; use axios type guard to keep strong typingThe explicit
anybypasses type safety and may hide non-Axios errors. Prefer an axios guard to check response status.- } catch (error: any) { + } catch (error: unknown) { const cwRef = commonWell.lastTransactionId; const extra = { orgOid: org.oid, cwReference: cwRef, context: `cw.org.update`, commonwellOrg: sdkOrg, error, }; - if (error.response?.status === 404) { + if (isAxiosError(error) && error.response?.status === 404) { capture.message("Got 404 while updating Org @ CW, creating it", { extra }); await create(cxId, org); return; }Note: add
import { isAxiosError } from "axios";at the top of this file.
63-91: Specify units fordefaultSearchRadiusto avoid ambiguityThe constant’s unit isn’t obvious. Consider encoding the unit in the name to avoid future misuse.
-const defaultSearchRadius = 150; +const defaultSearchRadiusMiles = 150; ... - searchRadius: defaultSearchRadius, + searchRadius: defaultSearchRadiusMiles,
120-139: Redundant override oftechnicalContacts
technicalContactsis already provided incwOrgBase; repeating it increases drift risk without benefit.- const cwOrg: CwSdkOrganizationWithOrgId = { - ...cwOrgBase, - securityTokenKeyType: "", // Must be an empty string for initatorOnly orgs - isActive: org.active, - technicalContacts: [technicalContact], + const cwOrg: CwSdkOrganizationWithOrgId = { + ...cwOrgBase, + securityTokenKeyType: "", // Must be an empty string for initatorOnly orgs + isActive: org.active, gateways: [], networks: [
158-179: Method name suggests throwing on failure, but it returns undefined on 404/other masked errors
getOrFailcurrently returnsundefinedwhen the SDK returnsundefined. That’s surprising for “orFail.” Either ensure it throws NotFoundError when undefined, or rename to reflect behavior.Option A — make it fail when undefined:
-export async function getOrFail(orgOid: string): Promise<CwSdkOrganization | undefined> { +export async function getOrFail(orgOid: string): Promise<CwSdkOrganization> { const { log, debug } = out(`CW.v2 get Org or fail - CW Org OID ${orgOid}`); const commonWell = makeCommonWellMemberAPI(); try { const resp = await commonWell.getOneOrg(orgOid); debug(`resp getOneOrgOrFail: `, JSON.stringify(resp)); - return resp; + if (!resp) throw new NotFoundError("Organization not found", undefined, { orgOid: orgOid }); + return resp;Then simplify
getParsedOrgOrFailV2accordingly.
270-288: Be careful lowercasing CW enum values: verify actual values or normalize mapping onceIf
CwTreatmentTypevalues aren’t lowercase (e.g., "ACUTE_CARE" or "AcuteCare"),type.toLowerCase()will break lookups against a map keyed by the original enum values.Safer approach:
-const CW_TO_TREATMENT_TYPE_MAP: Record<string, TreatmentType> = { +const CW_TO_TREATMENT_TYPE_MAP: Record<string, TreatmentType> = { [CwTreatmentType.acuteCare]: TreatmentType.acuteCare, [CwTreatmentType.ambulatory]: TreatmentType.ambulatory, [CwTreatmentType.hospital]: TreatmentType.hospital, [CwTreatmentType.labSystems]: TreatmentType.labSystems, [CwTreatmentType.pharmacy]: TreatmentType.pharmacy, [CwTreatmentType.postAcuteCare]: TreatmentType.postAcuteCare, } as const; -export function mapCwTypeToTreatmentType(type: string): TreatmentType { - const treatmentType = CW_TO_TREATMENT_TYPE_MAP[type.toLowerCase().trim()]; +// Normalize keys once, preserving robustness regardless of enum casing +const CW_TO_TREATMENT_TYPE_MAP_NORM: Record<string, TreatmentType> = Object.fromEntries( + Object.entries(CW_TO_TREATMENT_TYPE_MAP).map(([k, v]) => [k.toLowerCase(), v]) +) as Record<string, TreatmentType>; + +export function mapCwTypeToTreatmentType(type: string): TreatmentType { + const treatmentType = CW_TO_TREATMENT_TYPE_MAP_NORM[type.toLowerCase().trim()]; if (!treatmentType) { const msg = `Invalid CW treatment type: ${type}`; capture.error(msg, { extra: { type } }); throw new MetriportError(msg, undefined, { type }); } return treatmentType; }This keeps the existing public API but guards against casing differences in upstream data.
📜 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 ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
packages/api/src/external/commonwell-v2/command/organization/organization.ts(1 hunks)packages/commonwell-sdk/package.json(2 hunks)packages/commonwell-sdk/src/client/commonwell-member.ts(9 hunks)packages/commonwell-sdk/src/models/organization.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/commonwell-sdk/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/commonwell-sdk/src/models/organization.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
Useconstwhenever possible
Useasync/awaitinstead 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/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use types whenever possible
Files:
packages/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.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
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- U...
Files:
packages/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.ts
🧠 Learnings (24)
📓 Common learnings
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:296-320
Timestamp: 2025-07-17T21:24:37.077Z
Learning: RamilGaripov prefers to maintain consistency with existing patterns in a file rather than making isolated fixes to coding guideline violations like var vs const declarations, when the pattern is already established throughout the codebase.
📚 Learning: 2025-08-25T17:52:19.907Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/commonwell-sdk/src/client/commonwell-member.ts:231-233
Timestamp: 2025-08-25T17:52:19.907Z
Learning: In packages/commonwell-sdk/src/client/commonwell-member.ts, the getOneOrg method currently returns undefined for all non-404 HTTP errors (instead of throwing MetriportError) as a temporary workaround while waiting for the service provider to fix issues on their end. This is tracked in TODO ENG-668 and will be reverted later once the external dependency is resolved.
Applied to files:
packages/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.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/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-18T17:26:58.012Z
Learnt from: CR
PR: metriport/metriport#0
File: .cursorrules:0-0
Timestamp: 2025-07-18T17:26:58.012Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : 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.
Applied to files:
packages/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.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/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-25T17:26:51.678Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/api/src/external/commonwell-v2/command/organization/organization.ts:175-187
Timestamp: 2025-08-25T17:26:51.678Z
Learning: In the Metriport codebase, the established error handling pattern is to use capture.error(msg, { extra: { error } }) where the message is passed as the first parameter and the actual error object is included in the extra metadata. This pattern is intentionally used throughout the codebase and should not be changed to capture.error(error, { extra: {...} }).
Applied to files:
packages/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-19T12:28:30.160Z
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/api/src/external/ehr/canvas/command/resource-diff/start-resource-diff.ts:20-43
Timestamp: 2025-04-19T12:28:30.160Z
Learning: In the Metriport codebase, non-entry point commands (like internal service functions) should typically bubble up errors rather than handling them locally with try/catch blocks. Error handling is generally consolidated at entry points like API endpoints.
Applied to files:
packages/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.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/commonwell-sdk/src/client/commonwell-member.ts
📚 Learning: 2025-04-24T02:03:04.234Z
Learnt from: leite08
PR: metriport/metriport#3412
File: packages/api/src/command/medical/patient/patient-import/mapping/create.ts:0-0
Timestamp: 2025-04-24T02:03:04.234Z
Learning: For the Metriport codebase, errors should be allowed to bubble up instead of being caught and wrapped at each function level. This is their established error handling pattern.
Applied to files:
packages/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-05-08T19:41:19.510Z
Learnt from: thomasyopes
PR: metriport/metriport#3788
File: packages/api/src/external/ehr/shared/utils/bundle.ts:73-82
Timestamp: 2025-05-08T19:41:19.510Z
Learning: For the Metriport codebase, the team prefers letting errors bubble up the call stack naturally rather than adding explicit try-catch blocks at lower levels of abstraction.
Applied to files:
packages/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-17T21:24:37.077Z
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:296-320
Timestamp: 2025-07-17T21:24:37.077Z
Learning: RamilGaripov prefers to maintain consistency with existing patterns in a file rather than making isolated fixes to coding guideline violations like var vs const declarations, when the pattern is already established throughout the codebase.
Applied to files:
packages/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-25T17:52:11.617Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/core/src/external/commonwell-v2/document/document-downloader-local-v2.ts:49-58
Timestamp: 2025-08-25T17:52:11.617Z
Learning: RamilGaripov prefers to maintain consistency between v1 and v2 CommonWell implementations rather than addressing individual technical issues in isolation, even when the existing pattern has potential issues like binary data handling via string concatenation.
Applied to files:
packages/commonwell-sdk/src/client/commonwell-member.tspackages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-25T17:52:19.380Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/api/src/external/commonwell-v2/command/organization/organization.ts:89-98
Timestamp: 2025-08-25T17:52:19.380Z
Learning: In packages/api/src/external/commonwell-v2/command/organization/organization.ts, when setting organizationId, homeCommunityId, and patientIdAssignAuthority fields for CommonWell v2 SDK, raw OID values (org.oid) can be used directly without the "urn:oid:" prefix - the SDK handles any necessary formatting internally.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-05-27T15:54:47.774Z
Learnt from: lucasdellabella
PR: metriport/metriport#3866
File: packages/api/src/routes/internal/medical/organization.ts:95-98
Timestamp: 2025-05-27T15:54:47.774Z
Learning: The conversion of the GET `/internal/organization` endpoint from returning a single organization to returning an array of organizations (batch retrieval) in `packages/api/src/routes/internal/medical/organization.ts` was an intentional breaking change planned by the team.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-18T22:03:40.578Z
Learnt from: keshavsaharia
PR: metriport/metriport#4404
File: packages/core/src/external/surescripts/fhir/medication-request.ts:75-94
Timestamp: 2025-08-18T22:03:40.578Z
Learning: In packages/core/src/external/surescripts/fhir/medication-request.ts, there's a discrepancy between standard FHIR R4 specification (which recommends assigning authority URIs in Identifier.system) and specific HL7 guidance that user keshavsaharia references, which may support using code system URIs like "http://terminology.hl7.org/CodeSystem/v2-0203" in Identifier.system for Surescripts integration. This needs to be addressed in future PR with proper HL7 implementation guide references.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-06-26T06:28:31.490Z
Learnt from: keshavsaharia
PR: metriport/metriport#4099
File: packages/core/src/external/surescripts/fhir/coverage.ts:37-42
Timestamp: 2025-06-26T06:28:31.490Z
Learning: In packages/core/src/external/surescripts/fhir/coverage.ts, the getInsuranceOrganizationReference function doesn't need ID validation because the Organization resources are always created by getInsuranceOrganization which assigns id: uuidv7(), guaranteeing a valid ID is present.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-03-19T13:41:19.957Z
Learnt from: leite08
PR: metriport/metriport#3463
File: packages/api/src/command/medical/patient/settings/create-patient-settings.ts:202-204
Timestamp: 2025-03-19T13:41:19.957Z
Learning: In the Metriport codebase, when using `capture.error` or `capture.message` functions, the order of properties in the `extra` parameter is important because Sentry may trim data beyond its payload limit. The most critical information (usually `cxId`) should be placed first, while less important data (like `facilityId`) should be placed last.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-18T05:40:08.493Z
Learnt from: lucasdellabella
PR: metriport/metriport#3675
File: packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts:167-171
Timestamp: 2025-04-18T05:40:08.493Z
Learning: The team prefers to let errors bubble up to be caught by a Sentry root error handler rather than adding explicit try/catch blocks for error handling in Promise operations.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-03-29T00:01:59.998Z
Learnt from: thomasyopes
PR: metriport/metriport#3545
File: packages/core/src/command/write-to-storage/s3/write-to-s3-cloud.ts:12-20
Timestamp: 2025-03-29T00:01:59.998Z
Learning: In the Metriport codebase, error handling is often managed at higher levels in the call stack ("further up the chain") rather than within individual methods.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-15T18:57:29.030Z
Learnt from: lucasdellabella
PR: metriport/metriport#4382
File: packages/utils/src/hl7v2-notifications/reprocess-adt-conversion-bundles/common.ts:36-48
Timestamp: 2025-08-15T18:57:29.030Z
Learning: In utility scripts like packages/utils/src/hl7v2-notifications/reprocess-adt-conversion-bundles/common.ts, user lucasdellabella prefers to let bundle parsing errors fail fast rather than adding error handling, as it provides immediate visibility into data issues during script execution.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.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/api/src/external/commonwell-v2/command/organization/organization.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/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-17T21:24:30.319Z
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:255-260
Timestamp: 2025-07-17T21:24:30.319Z
Learning: In packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js, RamilGaripov prefers to maintain consistency with existing var declaration patterns throughout the file rather than changing individual functions to const, even when coding guidelines suggest using const whenever possible.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-18T22:03:40.578Z
Learnt from: keshavsaharia
PR: metriport/metriport#4404
File: packages/core/src/external/surescripts/fhir/medication-request.ts:75-94
Timestamp: 2025-08-18T22:03:40.578Z
Learning: User keshavsaharia indicated that HL7 guidance contradicts the standard FHIR R4 specification regarding Identifier.system usage in packages/core/src/external/surescripts/fhir/medication-request.ts, specifically that using HL7_CODE_SYSTEM_URL in Identifier.system may be correct according to HL7 guidance despite appearing to violate standard FHIR patterns.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
🧬 Code graph analysis (2)
packages/commonwell-sdk/src/client/commonwell-member.ts (2)
packages/commonwell-sdk/src/models/organization.ts (2)
organizationSchema(112-115)organizationListSchema(118-123)packages/shared/src/index.ts (2)
BadRequestError(42-42)MetriportError(43-43)
packages/api/src/external/commonwell-v2/command/organization/organization.ts (10)
packages/shared/src/common/env-var.ts (1)
getEnvVarOrFail(14-20)packages/core/src/domain/organization.ts (1)
OrganizationData(13-18)packages/commonwell-sdk/src/models/organization.ts (3)
OrganizationWithNetworkInfo(105-105)OrganizationBase(60-60)isOrgInitiatorAndResponder(136-138)packages/api/src/shared/config.ts (1)
Config(4-318)packages/core/src/util/log.ts (1)
out(30-35)packages/api/src/external/commonwell-v2/api.ts (2)
makeCommonWellMemberAPI(25-40)getCertificate(79-102)packages/core/src/util/error/shared.ts (1)
errorToString(15-23)packages/lambdas/src/shared/capture.ts (1)
capture(19-118)packages/shared/src/index.ts (2)
MetriportError(43-43)NotFoundError(44-44)packages/api-sdk/src/index.ts (1)
USState(27-27)
🔇 Additional comments (4)
packages/commonwell-sdk/src/client/commonwell-member.ts (1)
95-111: Good use of response interceptor to persist CW x-trace-id for both success and error pathsThe interceptor reliably captures x-trace-id via postRequest on both branches, which enables downstream correlation. No action needed.
packages/api/src/external/commonwell-v2/command/organization/organization.ts (3)
106-108: OAuth scopes likely not compliant; confirm and switch to SMART-on-FHIR v2-style resource scopesUsing "fhir/document" for DocumentReference and Binary is atypical. Most SMART v2 gateways expect system/DocumentReference.r[s] and system/Binary.r[s]. Please confirm with your CW v2 gateway configuration and update accordingly.
Apply this diff if confirmed:
- documentReferenceScope: "fhir/document", - binaryScope: "fhir/document", + documentReferenceScope: "system/DocumentReference.r", + binaryScope: "system/Binary.r",Optionally use ".rs" if search permission is required.
58-66: Type aliases to enforceorganizationIdpresence are solidThese aliases make create/update payloads type-safe by requiring organizationId at compile time. Nice.
238-262: Parsing CW org: guard and mapping look good
- Defensive check for missing locations with a meaningful MetriportError is good.
- OID extraction via OID_PREFIX is consistent with the v2 decision to use raw OIDs elsewhere.
leite08
reviewed
Aug 25, 2025
packages/api/src/external/commonwell-v2/command/facility/create-or-update-cw-facility.ts
Outdated
Show resolved
Hide resolved
| org: { | ||
| oid: facility.cwOboOid ?? facility.oid, | ||
| data: { | ||
| name: orgName, |
There was a problem hiding this comment.
WRT name, should we already move to the new DOA flow where we don't have the ... -OBO- <oid> on the name, or keep that around?
The code I did might have assumed we're doing the new DOA now, but that's not necessarily true.
There was a problem hiding this comment.
I think let's just use the new DOA now, right? Kill 2 birds with 1 stone?
packages/api/src/external/commonwell-v2/command/organization/organization.ts
Outdated
Show resolved
Hide resolved
| return cwOrg; | ||
| } | ||
|
|
||
| export async function get(orgOid: string): Promise<CwSdkOrganization | undefined> { |
There was a problem hiding this comment.
Do we need the get or can we only have getOrFail - which I think should just be get?
Reason being, this is not "get and return undefined if not found", this is swallowing any errors.
There was a problem hiding this comment.
Yeah, the SDK already accounts for the weird CW behavior, so our commands can be clean and expect the API returns undefined when not found, the Org when found, and throw an error when something else.
| @@ -91,21 +103,36 @@ router.put( | |||
| asyncHandler(async (req: Request, res: Response) => { | |||
| const cxId = getUUIDFrom("query", req, "cxId").orFail(); | |||
| const facilityId = getFrom("query").orFail("facilityId", req); | |||
| // TODO this should be refactored: (1) the oid is already in the facility; (2) we should have a fn for org | |||
There was a problem hiding this comment.
The comment about logic/command, totally.
The one about OID worries me, as one could send the wrong OID that doesn't match the facility, right? Can't we remove that param and just use the facility's? Or do intentionally want to be able to pass a diff OID for exceptional cases?
|
|
||
| if (error.response?.status === httpStatus.NOT_FOUND) { | ||
| const data = error.response?.data; | ||
| throw new MetriportError(title, undefined, { extra: JSON.stringify(data) }); |
There was a problem hiding this comment.
Why is a 404 being turned into a 500?
| ); | ||
| return organizationSchema.parse(resp.data); | ||
| } catch (error) { | ||
| this.rethrowDescriptiveError(error, "Failed to create CW organization"); |
There was a problem hiding this comment.
Can we make the function rethrowDescriptiveError to return the Error object instead of throwing, so we throw it here?
- cleaner, as the function becomes a mapping/converting function only
- the stack trace won't have
rethrowDescriptiveError, cleaner too
Part of ENG-513 Signed-off-by: RamilGaripov <ramil@metriport.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/api/src/external/commonwell-v1/organization.ts (2)
168-192: Fix 404 update fallback: create-then-return; don’t throw afterwardWhen updateOrg returns 404, you correctly fall back to create(), but you still log an error and rethrow after that path. That defeats the fallback and likely emits duplicate Sentry events, breaking “upsert”-like behavior.
[suggested fix below]
} catch (error: any) { const cwRef = commonWell.lastReferenceHeader; const extra = { orgOid: org.oid, cwReference: cwRef, context: `cw.org.update`, commonwellOrg, error: errorToString(error), }; if (error.response?.status === 404) { capture.message("Got 404 while updating Org @ CW, creating it", { extra }); await create(cxId, org, isObo); - } - const msg = `Failure while updating org @ CW`; - log(`${msg}. Org OID: ${org.oid}. Cause: ${errorToString(error)}. CW Reference: ${cwRef}`); - capture.error(msg, { extra }); - throw error; + return; // stop error propagation after successful fallback creation + } + const msg = `Failure while updating org @ CW`; + log(`${msg}. Org OID: ${org.oid}. Cause: ${errorToString(error)}. CW Reference: ${cwRef}`); + capture.error(msg, { extra }); + throw error; }
242-246: Differentiate upstream failures from genuine “not found” in getParsedOrgOrFailThe v1 SDK’s
getOneOrg(and by extensionget) currently returnsundefinednot only on 404s but also on any non-2xx status as a temporary ENG-668 workaround. Consequently, yourgetParsedOrgOrFailwill turn all upstream errors intoNotFoundError, masking transient or server faults as “not found.”Please update this handler to distinguish a true 404 from other failures until the SDK re-throws non-404s:
• File:
packages/api/src/external/commonwell-v1/organization.ts
Lines: ~242–246Suggested diff (minimal signal improvement):
export async function getParsedOrgOrFail(oid: string): Promise<CWOrganization> { const resp = await get(oid); - if (!resp) throw new NotFoundError("Organization not found"); + if (!resp) { + // ENG-668: undefined may indicate a non-404 failure upstream. + // Use a distinct error so callers can retry or handle differently. + throw new MetriportError("CWOrgUnavailable", { oid }); + // Or, at minimum: + // throw new NotFoundError("Organization not found or temporarily unavailable"); + } return parseCWEntry(resp); }Implementing this guard will prevent masking server or network issues as “not found” and allow calling layers to react appropriately.
♻️ Duplicate comments (5)
packages/api/src/domain/medical/facility.ts (2)
48-53: Deprecation of isOboFacility is fine; verify zero external usages remainGood to keep as a shim during the migration. Please confirm there are no remaining call sites outside this file so we don’t carry mixed semantics forward.
Run from repo root:
#!/bin/bash # Find any lingering usages of the deprecated helper (excluding this file and tests) rg -nP '\bisOboFacility\s*\(' packages \ -g '!**/__tests__/**' \ -g '!packages/api/src/domain/medical/facility.ts' || echo "No external usages found."
62-67: Overloads look good; consider a small resolver to reduce duplicationThe runtime narrowing via typeof === "string" is fine. For readability and to keep both helpers consistent, consider extracting a tiny resolver to avoid repeating the ternary here and below.
You can keep the change local by replacing the local const and adding a helper outside this range:
-export function isInitiatorAndResponder(facilityOrType: Facility | FacilityType): boolean { - const facilityType = typeof facilityOrType === "string" ? facilityOrType : facilityOrType.cwType; - return facilityType === FacilityType.initiatorAndResponder; -} +export function isInitiatorAndResponder(facilityOrType: Facility | FacilityType): boolean { + const facilityType = resolveFacilityType(facilityOrType); + return facilityType === FacilityType.initiatorAndResponder; +}Place this helper once in the file (e.g., above these functions):
function resolveFacilityType(input: Facility | FacilityType): FacilityType | undefined { return typeof input === "string" ? input : input.cwType; }Also, per earlier discussion, please add unit tests covering both overload forms (Facility and FacilityType).
packages/api/src/external/commonwell-v2/command/organization/organization.ts (3)
148-149: Use lazy debug serialization to avoid unnecessary JSON.stringify workPass a function as the second param so stringify only runs when debug logging is enabled (as already requested “here and throughout”).
- debug(`resp getOneOrg: `, JSON.stringify(resp)); + debug(`resp getOneOrg: `, () => JSON.stringify(resp)); @@ - debug(`resp getOneOrgOrFail: `, JSON.stringify(resp)); + debug(`resp getOneOrgOrFail: `, () => JSON.stringify(resp)); @@ - debug(`resp createOrg: `, JSON.stringify(respCreate)); + debug(`resp createOrg: `, () => JSON.stringify(respCreate)); - debug(`resp addCertificateToOrg: `, JSON.stringify(respAddCert)); + debug(`resp addCertificateToOrg: `, () => JSON.stringify(respAddCert)); @@ - debug(`resp updateOrg: `, JSON.stringify(resp)); + debug(`resp updateOrg: `, () => JSON.stringify(resp));Also applies to: 163-164, 188-190, 215-215
106-108: Confirm OAuth scopes; “fhir/document” looks non-SMART — likely should be resource scopesThe gateway scopes are set to
"fhir/document". SMART v2-style scopes are typicallysystem/DocumentReference.r[s]andsystem/Binary.r[s]. If CW v2 expects SMART scopes, update accordingly; otherwise, leave as-is if the CW gateway mandates the current values.Proposed change:
- documentReferenceScope: "fhir/document", - binaryScope: "fhir/document", + documentReferenceScope: "system/DocumentReference.r", + binaryScope: "system/Binary.r",Optionally, source these from Config to allow environment overrides.
143-156: “getOrFail” may still swallow errors due to SDK’s temporary behavior; naming/contract mismatchGiven the SDK’s current workaround (ENG-668) where
getOneOrgcan returnundefinedon non-404 errors,getOrFailcan still returnundefinedand only later be treated as “not found” ingetParsedOrgOrFailV2, masking real errors.Suggested mitigations:
- Short term: add a TODO referencing ENG-668 here and in
getParsedOrgOrFailV2, clarifying thatundefinedmight represent non-404 failures.- Medium term: once SDK reverts to throwing on non-404, update
getOrFailto rely on the throwing variant or inspect status to differentiate 404 vs other errors, ensuringgetOrFailnever resolvesundefined.Also applies to: 158-179, 264-267
🧹 Nitpick comments (4)
packages/api/src/external/commonwell-v2/command/organization/organization.ts (2)
122-122: Typo in comment: “initatorOnly” → “initiatorOnly”- securityTokenKeyType: "", // Must be an empty string for initatorOnly orgs + securityTokenKeyType: "", // Must be an empty string for initiatorOnly orgs
123-125: Redundant properties already set incwOrgBase
isActiveandtechnicalContactsare already provided bycwOrgBase. Safe to remove duplicates to reduce noise.const cwOrg: CwSdkOrganizationWithOrgId = { ...cwOrgBase, securityTokenKeyType: "", // Must be an empty string for initiatorOnly orgs - isActive: org.active, - technicalContacts: [technicalContact], gateways: [], networks: [packages/api/src/external/commonwell-v1/organization.ts (2)
235-236: Add runtime validation for TreatmentType castingWe’ve confirmed that
TreatmentTypeis defined as an enum inpackages/shared/src/domain/organization.ts(values include"acuteCare","ambulatory", etc.). To guard against unexpectedorg.typevalues from the CommonWell SDK, introduce a lightweight type guard and log any mismatches before falling back to the cast.• Canonical enum source:
packages/shared/src/domain/organization.ts:3–5–export enum TreatmentType { acuteCare = "acuteCare", ambulatory = "ambulatory", // … }• Suggested guard implementation in
packages/api/src/external/commonwell-v1/organization.ts:import { TreatmentType } from "@metriport/shared"; import { capture } from "<your-logging-lib>"; function isTreatmentType(v: unknown): v is TreatmentType { return ( typeof v === "string" && Object.values(TreatmentType).includes(v as TreatmentType) ); }• Replace the direct cast with:
- type: org.type as TreatmentType, + type: ((): TreatmentType => { + const t = org.type; + if (isTreatmentType(t)) { + return t; + } + capture.message("Unknown CW org type received", { + extra: { received: t }, + }); + return t as TreatmentType; // fallback to preserve existing behavior + })(),This ensures only valid enum values propagate and provides observability for any unexpected inputs.
132-140: Apply lazy debug for getOneOrg/updateOrg & confirm raw OID usage– I verified that in packages/api/src/external/commonwell-v1/organization.ts
• Line 107:debug('resp getOneOrg: ', JSON.stringify(resp));
• Line 173:debug('resp updateOrg: ', JSON.stringify(resp));
both are eager and should use the same lazy pattern you’ve applied to createOrg/addCertificate.
– The v1 SDK signature foraddCertificateToOrg(certificate, id, options?)injectsiddirectly into the URL, so passing the raworg.oid(without aurn:oid:prefix) is correct—no change needed there.Suggested diff:
packages/api/src/external/commonwell-v1/organization.ts @@ -106,7 +106,7 @@ - debug(`resp getOneOrg: `, JSON.stringify(resp)); + debug(`resp getOneOrg: `, () => JSON.stringify(resp)); @@ -172,7 +172,7 @@ - debug(`resp updateOrg: `, JSON.stringify(resp)); + debug(`resp updateOrg: `, () => JSON.stringify(resp));If inline lambdas are discouraged, you can introduce a helper:
+function lazyStringify(value: unknown): () => string { + return () => JSON.stringify(value); +} @@ - debug(`resp getOneOrg: `, () => JSON.stringify(resp)); + debug(`resp getOneOrg: `, lazyStringify(resp)); @@ - debug(`resp updateOrg: `, () => JSON.stringify(resp)); + debug(`resp updateOrg: `, lazyStringify(resp));
📜 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 (4)
packages/api/src/domain/medical/facility.ts(1 hunks)packages/api/src/external/commonwell-v1/organization.ts(7 hunks)packages/api/src/external/commonwell-v2/command/facility/create-or-update-cw-facility.ts(1 hunks)packages/api/src/external/commonwell-v2/command/organization/organization.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/api/src/external/commonwell-v2/command/facility/create-or-update-cw-facility.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
Useconstwhenever possible
Useasync/awaitinstead 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/api/src/external/commonwell-v2/command/organization/organization.tspackages/api/src/domain/medical/facility.tspackages/api/src/external/commonwell-v1/organization.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use types whenever possible
Files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/api/src/domain/medical/facility.tspackages/api/src/external/commonwell-v1/organization.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
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- U...
Files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/api/src/domain/medical/facility.tspackages/api/src/external/commonwell-v1/organization.ts
🧠 Learnings (32)
📓 Common learnings
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/core/src/external/commonwell-v2/document/document-downloader-local-v2.ts:275-288
Timestamp: 2025-08-25T17:52:21.580Z
Learning: RamilGaripov prefers to maintain consistency between CommonWell v1 and v2 implementations during the CW v2 API migration, keeping the new v2 files nearly identical to existing v1 files rather than making isolated improvements.
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:296-320
Timestamp: 2025-07-17T21:24:37.077Z
Learning: RamilGaripov prefers to maintain consistency with existing patterns in a file rather than making isolated fixes to coding guideline violations like var vs const declarations, when the pattern is already established throughout the codebase.
📚 Learning: 2025-08-25T17:52:19.380Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/api/src/external/commonwell-v2/command/organization/organization.ts:89-98
Timestamp: 2025-08-25T17:52:19.380Z
Learning: In packages/api/src/external/commonwell-v2/command/organization/organization.ts, when setting organizationId, homeCommunityId, and patientIdAssignAuthority fields for CommonWell v2 SDK, raw OID values (org.oid) can be used directly without the "urn:oid:" prefix - the SDK handles any necessary formatting internally.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/api/src/external/commonwell-v1/organization.ts
📚 Learning: 2025-05-27T15:54:47.774Z
Learnt from: lucasdellabella
PR: metriport/metriport#3866
File: packages/api/src/routes/internal/medical/organization.ts:95-98
Timestamp: 2025-05-27T15:54:47.774Z
Learning: The conversion of the GET `/internal/organization` endpoint from returning a single organization to returning an array of organizations (batch retrieval) in `packages/api/src/routes/internal/medical/organization.ts` was an intentional breaking change planned by the team.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/api/src/external/commonwell-v1/organization.ts
📚 Learning: 2025-08-18T22:03:40.578Z
Learnt from: keshavsaharia
PR: metriport/metriport#4404
File: packages/core/src/external/surescripts/fhir/medication-request.ts:75-94
Timestamp: 2025-08-18T22:03:40.578Z
Learning: In packages/core/src/external/surescripts/fhir/medication-request.ts, there's a discrepancy between standard FHIR R4 specification (which recommends assigning authority URIs in Identifier.system) and specific HL7 guidance that user keshavsaharia references, which may support using code system URIs like "http://terminology.hl7.org/CodeSystem/v2-0203" in Identifier.system for Surescripts integration. This needs to be addressed in future PR with proper HL7 implementation guide references.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-06-26T06:28:31.490Z
Learnt from: keshavsaharia
PR: metriport/metriport#4099
File: packages/core/src/external/surescripts/fhir/coverage.ts:37-42
Timestamp: 2025-06-26T06:28:31.490Z
Learning: In packages/core/src/external/surescripts/fhir/coverage.ts, the getInsuranceOrganizationReference function doesn't need ID validation because the Organization resources are always created by getInsuranceOrganization which assigns id: uuidv7(), guaranteeing a valid ID is present.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-25T17:52:19.907Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/commonwell-sdk/src/client/commonwell-member.ts:231-233
Timestamp: 2025-08-25T17:52:19.907Z
Learning: In packages/commonwell-sdk/src/client/commonwell-member.ts, the getOneOrg method currently returns undefined for all non-404 HTTP errors (instead of throwing MetriportError) as a temporary workaround while waiting for the service provider to fix issues on their end. This is tracked in TODO ENG-668 and will be reverted later once the external dependency is resolved.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/api/src/external/commonwell-v1/organization.ts
📚 Learning: 2025-08-25T17:26:51.678Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/api/src/external/commonwell-v2/command/organization/organization.ts:175-187
Timestamp: 2025-08-25T17:26:51.678Z
Learning: In the Metriport codebase, the established error handling pattern is to use capture.error(msg, { extra: { error } }) where the message is passed as the first parameter and the actual error object is included in the extra metadata. This pattern is intentionally used throughout the codebase and should not be changed to capture.error(error, { extra: {...} }).
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-03-19T13:41:19.957Z
Learnt from: leite08
PR: metriport/metriport#3463
File: packages/api/src/command/medical/patient/settings/create-patient-settings.ts:202-204
Timestamp: 2025-03-19T13:41:19.957Z
Learning: In the Metriport codebase, when using `capture.error` or `capture.message` functions, the order of properties in the `extra` parameter is important because Sentry may trim data beyond its payload limit. The most critical information (usually `cxId`) should be placed first, while less important data (like `facilityId`) should be placed last.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-18T17:26:58.012Z
Learnt from: CR
PR: metriport/metriport#0
File: .cursorrules:0-0
Timestamp: 2025-07-18T17:26:58.012Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : 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.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-19T12:28:30.160Z
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/api/src/external/ehr/canvas/command/resource-diff/start-resource-diff.ts:20-43
Timestamp: 2025-04-19T12:28:30.160Z
Learning: In the Metriport codebase, non-entry point commands (like internal service functions) should typically bubble up errors rather than handling them locally with try/catch blocks. Error handling is generally consolidated at entry points like API endpoints.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-18T05:40:08.493Z
Learnt from: lucasdellabella
PR: metriport/metriport#3675
File: packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts:167-171
Timestamp: 2025-04-18T05:40:08.493Z
Learning: The team prefers to let errors bubble up to be caught by a Sentry root error handler rather than adding explicit try/catch blocks for error handling in Promise operations.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-24T02:03:04.234Z
Learnt from: leite08
PR: metriport/metriport#3412
File: packages/api/src/command/medical/patient/patient-import/mapping/create.ts:0-0
Timestamp: 2025-04-24T02:03:04.234Z
Learning: For the Metriport codebase, errors should be allowed to bubble up instead of being caught and wrapped at each function level. This is their established error handling pattern.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-05-08T19:41:19.510Z
Learnt from: thomasyopes
PR: metriport/metriport#3788
File: packages/api/src/external/ehr/shared/utils/bundle.ts:73-82
Timestamp: 2025-05-08T19:41:19.510Z
Learning: For the Metriport codebase, the team prefers letting errors bubble up the call stack naturally rather than adding explicit try-catch blocks at lower levels of abstraction.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.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/commonwell-v2/command/organization/organization.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/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-03-29T00:01:59.998Z
Learnt from: thomasyopes
PR: metriport/metriport#3545
File: packages/core/src/command/write-to-storage/s3/write-to-s3-cloud.ts:12-20
Timestamp: 2025-03-29T00:01:59.998Z
Learning: In the Metriport codebase, error handling is often managed at higher levels in the call stack ("further up the chain") rather than within individual methods.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-17T21:24:37.077Z
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:296-320
Timestamp: 2025-07-17T21:24:37.077Z
Learning: RamilGaripov prefers to maintain consistency with existing patterns in a file rather than making isolated fixes to coding guideline violations like var vs const declarations, when the pattern is already established throughout the codebase.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/api/src/domain/medical/facility.ts
📚 Learning: 2025-08-15T18:57:29.030Z
Learnt from: lucasdellabella
PR: metriport/metriport#4382
File: packages/utils/src/hl7v2-notifications/reprocess-adt-conversion-bundles/common.ts:36-48
Timestamp: 2025-08-15T18:57:29.030Z
Learning: In utility scripts like packages/utils/src/hl7v2-notifications/reprocess-adt-conversion-bundles/common.ts, user lucasdellabella prefers to let bundle parsing errors fail fast rather than adding error handling, as it provides immediate visibility into data issues during script execution.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-26T02:33:26.756Z
Learnt from: leite08
PR: metriport/metriport#4469
File: packages/fhir-converter/src/lib/workers/worker.js:75-87
Timestamp: 2025-08-26T02:33:26.756Z
Learning: User leite08 confirmed that the debug logging approach in packages/fhir-converter/src/lib/workers/worker.js was already reviewed and approved in the feature PR, even though it logs both error messages and stack traces.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.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/api/src/external/commonwell-v2/command/organization/organization.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/commonwell-v2/command/organization/organization.tspackages/api/src/domain/medical/facility.ts
📚 Learning: 2025-07-17T21:24:30.319Z
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:255-260
Timestamp: 2025-07-17T21:24:30.319Z
Learning: In packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js, RamilGaripov prefers to maintain consistency with existing var declaration patterns throughout the file rather than changing individual functions to const, even when coding guidelines suggest using const whenever possible.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-18T22:03:40.578Z
Learnt from: keshavsaharia
PR: metriport/metriport#4404
File: packages/core/src/external/surescripts/fhir/medication-request.ts:75-94
Timestamp: 2025-08-18T22:03:40.578Z
Learning: User keshavsaharia indicated that HL7 guidance contradicts the standard FHIR R4 specification regarding Identifier.system usage in packages/core/src/external/surescripts/fhir/medication-request.ts, specifically that using HL7_CODE_SYSTEM_URL in Identifier.system may be correct according to HL7 guidance despite appearing to violate standard FHIR patterns.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-25T17:52:11.617Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/core/src/external/commonwell-v2/document/document-downloader-local-v2.ts:49-58
Timestamp: 2025-08-25T17:52:11.617Z
Learning: RamilGaripov prefers to maintain consistency between v1 and v2 CommonWell implementations rather than addressing individual technical issues in isolation, even when the existing pattern has potential issues like binary data handling via string concatenation.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.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/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-28T22:52:54.904Z
Learnt from: leite08
PR: metriport/metriport#3749
File: packages/core/src/command/patient-import/steps/query/patient-import-query-cloud.ts:12-26
Timestamp: 2025-04-28T22:52:54.904Z
Learning: The team prefers to let errors bubble up to higher-level handlers rather than adding error handling at each individual function level. Only suggest adding error handling for functions that are called asynchronously and don't have error handling on the clients.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-05-22T20:28:26.589Z
Learnt from: leite08
PR: metriport/metriport#3887
File: packages/infra/lib/surescripts-stack.ts:121-134
Timestamp: 2025-05-22T20:28:26.589Z
Learning: For configuration functions, returning `undefined` is preferred over empty objects when configuration is missing, as it makes the absence of configuration more explicit and easier to check.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-28T23:10:42.561Z
Learnt from: leite08
PR: metriport/metriport#3749
File: packages/api/src/command/medical/patient/patient-import/get.ts:70-74
Timestamp: 2025-04-28T23:10:42.561Z
Learning: Error messages should have static messages. Dynamic data like IDs should be added to MetriportError's `additionalInfo` property rather than interpolated into the error message string.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-06-18T04:51:44.941Z
Learnt from: leite08
PR: metriport/metriport#4044
File: packages/api/src/command/medical/tcm-encounter/update-tcm-encounter.ts:23-23
Timestamp: 2025-06-18T04:51:44.941Z
Learning: Error messages should be fixed, stable, and non-dynamic. Dynamic data like IDs should be moved to the `additionalInfo` property of MetriportError instead of being interpolated into the error message string.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-16T01:38:14.080Z
Learnt from: leite08
PR: metriport/metriport#4194
File: packages/api/src/command/medical/patient/get-patient-facilities.ts:36-44
Timestamp: 2025-07-16T01:38:14.080Z
Learning: In packages/api/src/command/medical/patient/get-patient-facilities.ts, the patient.facilityIds field is strongly typed as array of strings (non-nullable), so null/undefined checks are not needed when accessing this property.
Applied to files:
packages/api/src/domain/medical/facility.ts
📚 Learning: 2025-05-30T01:11:35.300Z
Learnt from: keshavsaharia
PR: metriport/metriport#3885
File: packages/utils/src/surescripts/generate-patient-load-file.ts:92-94
Timestamp: 2025-05-30T01:11:35.300Z
Learning: In V0 of the Surescripts implementation (packages/utils/src/surescripts/generate-patient-load-file.ts), there is always only 1 facility ID, so the current facility_ids parsing logic with substring operations is sufficient and additional validation is not necessary.
Applied to files:
packages/api/src/domain/medical/facility.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/api/src/domain/medical/facility.ts
🧬 Code graph analysis (2)
packages/api/src/external/commonwell-v2/command/organization/organization.ts (10)
packages/shared/src/common/env-var.ts (1)
getEnvVarOrFail(14-20)packages/core/src/domain/organization.ts (1)
OrganizationData(13-18)packages/commonwell-sdk/src/models/organization.ts (3)
OrganizationWithNetworkInfo(105-105)OrganizationBase(60-60)isOrgInitiatorAndResponder(136-138)packages/api/src/shared/config.ts (1)
Config(4-318)packages/core/src/util/log.ts (1)
out(30-35)packages/api/src/external/commonwell-v2/api.ts (2)
makeCommonWellMemberAPI(25-40)getCertificate(79-102)packages/core/src/util/error/shared.ts (1)
errorToString(15-23)packages/lambdas/src/shared/capture.ts (1)
capture(19-118)packages/shared/src/index.ts (2)
MetriportError(43-43)NotFoundError(44-44)packages/api-sdk/src/index.ts (1)
USState(27-27)
packages/api/src/external/commonwell-v1/organization.ts (1)
packages/api/src/external/commonwell-v1/api.ts (1)
metriportQueryMeta(78-78)
⏰ 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). (15)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (8)
packages/api/src/domain/medical/facility.ts (1)
55-60: Safe to remove deprecatedisNonOboFacilityNo occurrences of
isNonOboFacilitywere found outside its own definition (excluding tests), so you can safely remove this deprecated alias and switch any remaining callers toisInitiatorAndResponder.packages/api/src/external/commonwell-v2/command/organization/organization.ts (2)
95-95: ConfirmsecurityTokenKeyType: "JWT"is the v2-required valuePrevious versions used “Bearer” in places; confirm CW v2 expects
"JWT"here and ensure consistency across v2 org creation/update.
81-84: OID assignment looks correct for CW v2 SDK expectationsUsing raw
org.oidfororganizationId,homeCommunityId, andpatientIdAssignAuthorityaligns with the v2 SDK handling, which applies any needed formatting internally.packages/api/src/external/commonwell-v1/organization.ts (5)
104-123: Normalize error logging with errorToString in get()Good improvement replacing raw error prints with errorToString and including CW reference in logs and Sentry extras. Keeps messages parseable and avoids losing stack/context.
146-157: Create path: improved error serializationUsing errorToString in capture.error extra is a solid improvement for observability.
210-219: CQ include list: improved error serializationSame here—moving to errorToString avoids “[object Object]” logs and keeps errors consistent across code paths.
4-4: Import Organization for CWOrganization Omit is appropriateUsing the domain Organization type to derive CWOrganization keeps shapes aligned and reduces drift.
8-14: Consolidated imports from shared (TreatmentType, USState, errorToString) look goodTypes and utilities are sourced from the right place and align with the broader migration away from OrgType.
packages/api/src/external/commonwell-v2/command/organization/organization.ts
Outdated
Show resolved
Hide resolved
packages/api/src/external/commonwell-v2/command/organization/organization.ts
Show resolved
Hide resolved
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/api/src/__tests__/e2e/mapi/parts/organization.ts (1)
96-98: Replaceintype check per guidelines; use a type guard.Coding guideline: “Use truthy syntax instead of
in.” Replace theinoperator with a narrow via a small type guard.- if (`oid` in orgToCompare) { - expect(org?.identifier.value).toEqual(orgToCompare.oid); - } + if (hasOid(orgToCompare)) { + expect(org?.identifier.value).toEqual(orgToCompare.oid); + }Add this helper near the top of the file:
function hasOid(x: OrganizationCreate | Organization): x is Organization { return (x as Organization).oid !== undefined; }
♻️ Duplicate comments (1)
packages/api/src/external/commonwell-v2/command/organization/organization.ts (1)
148-148: Make debug JSON.stringify lazy to avoid unnecessary work.Pass a function so stringify only runs when debug logging is enabled. This matches the established pattern.
- debug(`resp getOneOrg: `, JSON.stringify(resp)); + debug(`resp getOneOrg: `, () => JSON.stringify(resp)); @@ - debug(`resp createOrg: `, JSON.stringify(respCreate)); + debug(`resp createOrg: `, () => JSON.stringify(respCreate)); - debug(`resp addCertificateToOrg: `, JSON.stringify(respAddCert)); + debug(`resp addCertificateToOrg: `, () => JSON.stringify(respAddCert)); @@ - debug(`resp updateOrg: `, JSON.stringify(resp)); + debug(`resp updateOrg: `, () => JSON.stringify(resp));Also applies to: 173-175, 200-200
🧹 Nitpick comments (7)
packages/api/src/__tests__/e2e/mapi/parts/organization.ts (2)
6-32: Validator refactor to named function: good change; keep assertions strict.LGTM. Minor: consider asserting specific shapes (e.g., zip format) if these tests gate contracts.
34-57: FHIR validator refactor: LGTM; avoid brittle cardinality when possible.If upstream may add multiple addresses, prefer “>= 1” to reduce brittleness.
Apply this diff if appropriate:
- expect(org.address?.length).toEqual(1); + expect((org.address ?? []).length).toBeGreaterThanOrEqual(1);packages/api-sdk/src/index.ts (1)
68-69: Provide deprecation shims for OrgType and orgTypeSchema
Removal of OrgType and orgTypeSchema breaks SDK consumers (used in Python SDK, Java SDK, docs). Add one-release aliases in packages/api-sdk/src/index.ts:export { Organization, OrganizationCreate, organizationCreateSchema, organizationSchema, treatmentTypeSchema, } from "./medical/models/organization"; + +/** @deprecated Use TreatmentType instead. Will be removed in the next minor release. */ +export { TreatmentType as OrgType } from "./medical/models/organization"; +/** @deprecated Use treatmentTypeSchema instead. Will be removed in the next minor release. */ +export { treatmentTypeSchema as orgTypeSchema } from "./medical/models/organization";packages/api-sdk/src/medical/models/organization.ts (1)
10-12: Schema field update to treatmentTypeSchema: LGTM. Consider a temporary alias for downstreams.If you want to offer a transitional path, you can export deprecated aliases here too.
export const organizationCreateSchema = z.object({ name: z.string(), type: treatmentTypeSchema, location: addressSchema, }); + +/** @deprecated Use treatmentTypeSchema */ +export const orgTypeSchema = treatmentTypeSchema; +/** @deprecated Use TreatmentType */ +export type OrgType = TreatmentType;packages/commonwell-sdk/src/client/commonwell-member.ts (2)
242-262: Unify error semantics for certificate operations (optional).For consistency with create/update/getAllOrgs, wrap these calls and rethrow using getDescriptiveError so callers get the same error shapes and preserved causes.
@@ - const resp = await this.api.post( + try { + const resp = await this.api.post( `${CommonWellMember.MEMBER_ENDPOINT}/${this.memberId}/org/${id}/certificate`, payload, { headers, } - ); - return certificateRespSchema.parse(resp.data); + ); + return certificateRespSchema.parse(resp.data); + } catch (error) { + throw this.getDescriptiveError(error, "Failed to add certificate to CW organization"); + } @@ - const resp = await this.api.put( + try { + const resp = await this.api.put( `${CommonWellMember.MEMBER_ENDPOINT}/${this.memberId}/org/${id}/certificate`, payload, { headers, } - ); - return certificateRespSchema.parse(resp.data); + ); + return certificateRespSchema.parse(resp.data); + } catch (error) { + throw this.getDescriptiveError( + error, + "Failed to replace certificate for CW organization" + ); + }Also applies to: 274-294
422-437: Narrow return type of getDescriptiveError for type-safety (optional).Returning Error (instead of unknown) tightens call sites and avoids throws of non-Error values.
- private getDescriptiveError(error: unknown, title: string): unknown { + private getDescriptiveError(error: unknown, title: string): Error { if (isAxiosError(error)) { const status = error.response?.status; const data = error.response?.data; const cwReference = this.lastTransactionId; @@ - return new MetriportError(title, error, { status, cwReference, data }); + return new MetriportError(title, error, { status, cwReference, data }); } - return error; + if (error instanceof Error) return error; + return new MetriportError(title, undefined, { data: { original: error } }); }packages/api/src/external/commonwell-v2/command/organization/organization.ts (1)
122-122: Typo in comment.“initatorOnly” → “initiatorOnly”.
- securityTokenKeyType: "", // Must be an empty string for initatorOnly orgs + securityTokenKeyType: "", // Must be an empty string for initiatorOnly orgs
📜 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 (7)
packages/api-sdk/src/index.ts(2 hunks)packages/api-sdk/src/medical/models/organization.ts(1 hunks)packages/api/src/__tests__/e2e/mapi/parts/organization.ts(3 hunks)packages/api/src/domain/medical/facility.ts(1 hunks)packages/api/src/external/commonwell-v2/command/organization/organization.ts(1 hunks)packages/commonwell-sdk/src/client/commonwell-member.ts(9 hunks)packages/commonwell-sdk/src/models/organization.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/commonwell-sdk/src/models/organization.ts
- packages/api/src/domain/medical/facility.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
Useconstwhenever possible
Useasync/awaitinstead 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/api-sdk/src/medical/models/organization.tspackages/api-sdk/src/index.tspackages/api/src/__tests__/e2e/mapi/parts/organization.tspackages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use types whenever possible
Files:
packages/api-sdk/src/medical/models/organization.tspackages/api-sdk/src/index.tspackages/api/src/__tests__/e2e/mapi/parts/organization.tspackages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.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
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- U...
Files:
packages/api-sdk/src/medical/models/organization.tspackages/api-sdk/src/index.tspackages/api/src/__tests__/e2e/mapi/parts/organization.tspackages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.ts
🧠 Learnings (41)
📓 Common learnings
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:296-320
Timestamp: 2025-07-17T21:24:37.077Z
Learning: RamilGaripov prefers to maintain consistency with existing patterns in a file rather than making isolated fixes to coding guideline violations like var vs const declarations, when the pattern is already established throughout the codebase.
📚 Learning: 2025-05-27T15:54:47.774Z
Learnt from: lucasdellabella
PR: metriport/metriport#3866
File: packages/api/src/routes/internal/medical/organization.ts:95-98
Timestamp: 2025-05-27T15:54:47.774Z
Learning: The conversion of the GET `/internal/organization` endpoint from returning a single organization to returning an array of organizations (batch retrieval) in `packages/api/src/routes/internal/medical/organization.ts` was an intentional breaking change planned by the team.
Applied to files:
packages/api-sdk/src/medical/models/organization.tspackages/api-sdk/src/index.tspackages/api/src/__tests__/e2e/mapi/parts/organization.tspackages/api/src/external/commonwell-v2/command/organization/organization.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/api-sdk/src/medical/models/organization.tspackages/api-sdk/src/index.ts
📚 Learning: 2025-08-25T17:52:19.445Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/api/src/external/commonwell-v2/command/organization/organization.ts:89-98
Timestamp: 2025-08-25T17:52:19.445Z
Learning: In packages/api/src/external/commonwell-v2/command/organization/organization.ts, when setting organizationId, homeCommunityId, and patientIdAssignAuthority fields for CommonWell v2 SDK, raw OID values (org.oid) can be used directly without the "urn:oid:" prefix - the SDK handles any necessary formatting internally.
Applied to files:
packages/api-sdk/src/medical/models/organization.tspackages/api/src/__tests__/e2e/mapi/parts/organization.tspackages/api/src/external/commonwell-v2/command/organization/organization.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/api-sdk/src/medical/models/organization.tspackages/api-sdk/src/index.ts
📚 Learning: 2025-08-14T23:29:53.606Z
Learnt from: keshavsaharia
PR: metriport/metriport#4370
File: packages/core/src/external/quest/file/file-generator.ts:55-57
Timestamp: 2025-08-14T23:29:53.606Z
Learning: In packages/shared/src/domain/patient.ts, the Patient schema defines address as z.array(...) without .optional(), meaning the address field is always present as an array. Optional chaining is not needed when accessing patient.address[0], though checking for undefined after indexing is still necessary in case the array is empty.
Applied to files:
packages/api-sdk/src/medical/models/organization.tspackages/api-sdk/src/index.ts
📚 Learning: 2025-08-14T23:29:53.606Z
Learnt from: keshavsaharia
PR: metriport/metriport#4370
File: packages/core/src/external/quest/file/file-generator.ts:55-57
Timestamp: 2025-08-14T23:29:53.606Z
Learning: In packages/shared/src/domain/patient.ts, the Patient type's address field is required (not optional) and is defined as z.array(addressSchema), guaranteeing it's always present as an array, so optional chaining is not needed when accessing patient.address[0].
Applied to files:
packages/api-sdk/src/medical/models/organization.tspackages/api-sdk/src/index.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/api-sdk/src/index.ts
📚 Learning: 2025-06-26T06:28:31.490Z
Learnt from: keshavsaharia
PR: metriport/metriport#4099
File: packages/core/src/external/surescripts/fhir/coverage.ts:37-42
Timestamp: 2025-06-26T06:28:31.490Z
Learning: In packages/core/src/external/surescripts/fhir/coverage.ts, the getInsuranceOrganizationReference function doesn't need ID validation because the Organization resources are always created by getInsuranceOrganization which assigns id: uuidv7(), guaranteeing a valid ID is present.
Applied to files:
packages/api/src/__tests__/e2e/mapi/parts/organization.tspackages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-18T22:03:40.578Z
Learnt from: keshavsaharia
PR: metriport/metriport#4404
File: packages/core/src/external/surescripts/fhir/medication-request.ts:75-94
Timestamp: 2025-08-18T22:03:40.578Z
Learning: In packages/core/src/external/surescripts/fhir/medication-request.ts, there's a discrepancy between standard FHIR R4 specification (which recommends assigning authority URIs in Identifier.system) and specific HL7 guidance that user keshavsaharia references, which may support using code system URIs like "http://terminology.hl7.org/CodeSystem/v2-0203" in Identifier.system for Surescripts integration. This needs to be addressed in future PR with proper HL7 implementation guide references.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-28T20:21:29.231Z
Learnt from: leite08
PR: metriport/metriport#4412
File: packages/api/src/external/commonwell-v2/command/organization/organization.ts:0-0
Timestamp: 2025-08-28T20:21:29.231Z
Learning: CommonWell v2 API uses its own OAuth scope format like "fhir/document" for documentReferenceScope and binaryScope in gateway authorization configuration, which is different from SMART on FHIR v2 scope conventions. This is part of CommonWell's proprietary API implementation, not standard SMART on FHIR.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-25T17:52:19.940Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/commonwell-sdk/src/client/commonwell-member.ts:231-233
Timestamp: 2025-08-25T17:52:19.940Z
Learning: In packages/commonwell-sdk/src/client/commonwell-member.ts, the getOneOrg method currently returns undefined for all non-404 HTTP errors (instead of throwing MetriportError) as a temporary workaround while waiting for the service provider to fix issues on their end. This is tracked in TODO ENG-668 and will be reverted later once the external dependency is resolved.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.ts
📚 Learning: 2025-08-25T17:26:51.717Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/api/src/external/commonwell-v2/command/organization/organization.ts:175-187
Timestamp: 2025-08-25T17:26:51.717Z
Learning: In the Metriport codebase, the established error handling pattern is to use capture.error(msg, { extra: { error } }) where the message is passed as the first parameter and the actual error object is included in the extra metadata. This pattern is intentionally used throughout the codebase and should not be changed to capture.error(error, { extra: {...} }).
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.ts
📚 Learning: 2025-03-19T13:41:19.957Z
Learnt from: leite08
PR: metriport/metriport#3463
File: packages/api/src/command/medical/patient/settings/create-patient-settings.ts:202-204
Timestamp: 2025-03-19T13:41:19.957Z
Learning: In the Metriport codebase, when using `capture.error` or `capture.message` functions, the order of properties in the `extra` parameter is important because Sentry may trim data beyond its payload limit. The most critical information (usually `cxId`) should be placed first, while less important data (like `facilityId`) should be placed last.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-18T17:26:58.012Z
Learnt from: CR
PR: metriport/metriport#0
File: .cursorrules:0-0
Timestamp: 2025-07-18T17:26:58.012Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : 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.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.ts
📚 Learning: 2025-04-19T12:28:30.160Z
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/api/src/external/ehr/canvas/command/resource-diff/start-resource-diff.ts:20-43
Timestamp: 2025-04-19T12:28:30.160Z
Learning: In the Metriport codebase, non-entry point commands (like internal service functions) should typically bubble up errors rather than handling them locally with try/catch blocks. Error handling is generally consolidated at entry points like API endpoints.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.ts
📚 Learning: 2025-04-18T05:40:08.493Z
Learnt from: lucasdellabella
PR: metriport/metriport#3675
File: packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts:167-171
Timestamp: 2025-04-18T05:40:08.493Z
Learning: The team prefers to let errors bubble up to be caught by a Sentry root error handler rather than adding explicit try/catch blocks for error handling in Promise operations.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-24T02:03:04.234Z
Learnt from: leite08
PR: metriport/metriport#3412
File: packages/api/src/command/medical/patient/patient-import/mapping/create.ts:0-0
Timestamp: 2025-04-24T02:03:04.234Z
Learning: For the Metriport codebase, errors should be allowed to bubble up instead of being caught and wrapped at each function level. This is their established error handling pattern.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.ts
📚 Learning: 2025-05-08T19:41:19.510Z
Learnt from: thomasyopes
PR: metriport/metriport#3788
File: packages/api/src/external/ehr/shared/utils/bundle.ts:73-82
Timestamp: 2025-05-08T19:41:19.510Z
Learning: For the Metriport codebase, the team prefers letting errors bubble up the call stack naturally rather than adding explicit try-catch blocks at lower levels of abstraction.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.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/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.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/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.ts
📚 Learning: 2025-03-29T00:01:59.998Z
Learnt from: thomasyopes
PR: metriport/metriport#3545
File: packages/core/src/command/write-to-storage/s3/write-to-s3-cloud.ts:12-20
Timestamp: 2025-03-29T00:01:59.998Z
Learning: In the Metriport codebase, error handling is often managed at higher levels in the call stack ("further up the chain") rather than within individual methods.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-17T21:24:37.077Z
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:296-320
Timestamp: 2025-07-17T21:24:37.077Z
Learning: RamilGaripov prefers to maintain consistency with existing patterns in a file rather than making isolated fixes to coding guideline violations like var vs const declarations, when the pattern is already established throughout the codebase.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.ts
📚 Learning: 2025-08-27T23:12:23.954Z
Learnt from: thomasyopes
PR: metriport/metriport#4478
File: packages/core/src/external/ehr/api/job/start-contribute-bundles-job.ts:44-47
Timestamp: 2025-08-27T23:12:23.954Z
Learning: In packages/core/src/external/ehr/api/job/start-contribute-bundles-job.ts, the intentional swallowing of HTTP 400 errors in startContributeBundlesJob via early return is by design, as confirmed by thomasyopes. This pattern should not be flagged in future reviews.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.ts
📚 Learning: 2025-08-15T18:57:29.030Z
Learnt from: lucasdellabella
PR: metriport/metriport#4382
File: packages/utils/src/hl7v2-notifications/reprocess-adt-conversion-bundles/common.ts:36-48
Timestamp: 2025-08-15T18:57:29.030Z
Learning: In utility scripts like packages/utils/src/hl7v2-notifications/reprocess-adt-conversion-bundles/common.ts, user lucasdellabella prefers to let bundle parsing errors fail fast rather than adding error handling, as it provides immediate visibility into data issues during script execution.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.ts
📚 Learning: 2025-08-26T02:33:26.777Z
Learnt from: leite08
PR: metriport/metriport#4469
File: packages/fhir-converter/src/lib/workers/worker.js:75-87
Timestamp: 2025-08-26T02:33:26.777Z
Learning: User leite08 confirmed that the debug logging approach in packages/fhir-converter/src/lib/workers/worker.js was already reviewed and approved in the feature PR, even though it logs both error messages and stack traces.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.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/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-17T21:24:30.319Z
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:255-260
Timestamp: 2025-07-17T21:24:30.319Z
Learning: In packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js, RamilGaripov prefers to maintain consistency with existing var declaration patterns throughout the file rather than changing individual functions to const, even when coding guidelines suggest using const whenever possible.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.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/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.ts
📚 Learning: 2025-08-18T22:03:40.578Z
Learnt from: keshavsaharia
PR: metriport/metriport#4404
File: packages/core/src/external/surescripts/fhir/medication-request.ts:75-94
Timestamp: 2025-08-18T22:03:40.578Z
Learning: User keshavsaharia indicated that HL7 guidance contradicts the standard FHIR R4 specification regarding Identifier.system usage in packages/core/src/external/surescripts/fhir/medication-request.ts, specifically that using HL7_CODE_SYSTEM_URL in Identifier.system may be correct according to HL7 guidance despite appearing to violate standard FHIR patterns.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-25T17:52:11.649Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/core/src/external/commonwell-v2/document/document-downloader-local-v2.ts:49-58
Timestamp: 2025-08-25T17:52:11.649Z
Learning: RamilGaripov prefers to maintain consistency between v1 and v2 CommonWell implementations rather than addressing individual technical issues in isolation, even when the existing pattern has potential issues like binary data handling via string concatenation.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.tspackages/commonwell-sdk/src/client/commonwell-member.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/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-28T22:52:54.904Z
Learnt from: leite08
PR: metriport/metriport#3749
File: packages/core/src/command/patient-import/steps/query/patient-import-query-cloud.ts:12-26
Timestamp: 2025-04-28T22:52:54.904Z
Learning: The team prefers to let errors bubble up to higher-level handlers rather than adding error handling at each individual function level. Only suggest adding error handling for functions that are called asynchronously and don't have error handling on the clients.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-05-22T20:28:26.589Z
Learnt from: leite08
PR: metriport/metriport#3887
File: packages/infra/lib/surescripts-stack.ts:121-134
Timestamp: 2025-05-22T20:28:26.589Z
Learning: For configuration functions, returning `undefined` is preferred over empty objects when configuration is missing, as it makes the absence of configuration more explicit and easier to check.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-28T23:10:42.561Z
Learnt from: leite08
PR: metriport/metriport#3749
File: packages/api/src/command/medical/patient/patient-import/get.ts:70-74
Timestamp: 2025-04-28T23:10:42.561Z
Learning: Error messages should have static messages. Dynamic data like IDs should be added to MetriportError's `additionalInfo` property rather than interpolated into the error message string.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-06-18T04:51:44.941Z
Learnt from: leite08
PR: metriport/metriport#4044
File: packages/api/src/command/medical/tcm-encounter/update-tcm-encounter.ts:23-23
Timestamp: 2025-06-18T04:51:44.941Z
Learning: Error messages should be fixed, stable, and non-dynamic. Dynamic data like IDs should be moved to the `additionalInfo` property of MetriportError instead of being interpolated into the error message string.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-21T15:53:34.154Z
Learnt from: leite08
PR: metriport/metriport#4427
File: packages/commonwell-cert-runner/src/flows/contribution/document-reference.ts:23-24
Timestamp: 2025-08-21T15:53:34.154Z
Learning: CommonWell uses their own version/profile of FHIR R4 that may have different validation rules than pure FHIR R4, including potentially allowing different formats for resource IDs and references that wouldn't be valid in standard FHIR R4.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-09T17:18:28.920Z
Learnt from: thomasyopes
PR: metriport/metriport#4164
File: packages/core/src/external/ehr/healthie/index.ts:1031-1076
Timestamp: 2025-07-09T17:18:28.920Z
Learning: In packages/core/src/external/ehr/healthie/index.ts, the `interpretation` field in `labObservationResult` is defined as a required enum in the Zod schema (`z.enum(["high", "low", "normal"])`), making null-safety checks unnecessary when calling `toTitleCase(labObservationResult.interpretation)`.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.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/commonwell-v2/command/organization/organization.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/commonwell-sdk/src/client/commonwell-member.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/commonwell-sdk/src/client/commonwell-member.ts
🧬 Code graph analysis (4)
packages/api-sdk/src/medical/models/organization.ts (1)
packages/api-sdk/src/index.ts (3)
treatmentTypeSchema(68-68)TreatmentType(81-81)organizationCreateSchema(66-66)
packages/api/src/__tests__/e2e/mapi/parts/organization.ts (3)
packages/api-sdk/src/index.ts (2)
Organization(64-64)OrganizationCreate(65-65)packages/api-sdk/src/medical/models/organization.ts (2)
Organization(21-21)OrganizationCreate(13-13)packages/commonwell-sdk/src/models/organization.ts (1)
Organization(116-116)
packages/api/src/external/commonwell-v2/command/organization/organization.ts (9)
packages/shared/src/common/env-var.ts (1)
getEnvVarOrFail(14-20)packages/core/src/domain/organization.ts (1)
OrganizationData(13-18)packages/commonwell-sdk/src/models/organization.ts (1)
isOrgInitiatorAndResponder(140-142)packages/api/src/shared/config.ts (1)
Config(4-318)packages/core/src/util/log.ts (3)
out(30-35)debug(25-28)log(10-23)packages/api/src/external/commonwell-v2/api.ts (2)
makeCommonWellMemberAPI(25-40)getCertificate(79-102)packages/core/src/util/error/shared.ts (1)
errorToString(15-23)packages/lambdas/src/shared/capture.ts (1)
capture(19-118)packages/shared/src/index.ts (2)
MetriportError(43-43)NotFoundError(44-44)
packages/commonwell-sdk/src/client/commonwell-member.ts (2)
packages/commonwell-sdk/src/models/organization.ts (2)
organizationSchema(112-115)organizationListSchema(118-123)packages/shared/src/index.ts (3)
BadRequestError(42-42)NotFoundError(44-44)MetriportError(43-43)
⏰ 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). (15)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (9)
packages/api/src/__tests__/e2e/mapi/parts/organization.ts (2)
2-2: SDK import change looks good.Using Organization and OrganizationCreate from the SDK aligns with the OrgType → TreatmentType migration.
6-32: Heads-up: CW v1 type in this file; ensure v2 path is covered by tests.validateCwOrg still targets @metriport/commonwell-sdk-v1. Given v2 is behind a flag, confirm we have equivalent v2 validator/tests or intentionally limit E2E here.
packages/api-sdk/src/index.ts (1)
81-81: Re-exporting TreatmentType: confirm single source of truth and avoid circulars.LGTM. Verify no duplicate/conflicting re-exports elsewhere and document the breaking change in SDK release notes.
packages/api-sdk/src/medical/models/organization.ts (1)
4-7: z.nativeEnum usage is correct for string enums
TreatmentType is declared as a string enum in packages/shared/src/domain/organization.ts, so z.nativeEnum(TreatmentType) already restricts to its string values. No change needed.packages/commonwell-sdk/src/client/commonwell-member.ts (4)
129-140: Robust Axios-to-domain error mapping — nice.Catches map Axios errors to domain errors with cause and structured context. Consistent with guidelines and recent decisions.
155-166: Update flow error handling is consistent and preserves cause.Good use of getDescriptiveError with structured extra fields.
191-199: List retrieval error handling is consistent.LGTM on mapping and zod parsing.
215-230: Temporary undefined on non-404 responses acknowledged.The ENG-668 TODO is present. No change requested; just ensure callers expect undefined in these edge cases until ENG-668 is resolved.
packages/api/src/external/commonwell-v2/command/organization/organization.ts (1)
63-141: SDK shape mapping looks solid.
- Correctly sets IDs, locations, gateways, and auth info.
- Keeps legacy bridge enabled with a TODO ticket.
- Initiator-only branch uses empty securityTokenKeyType as required.
packages/api/src/external/commonwell-v2/command/organization/organization.ts
Outdated
Show resolved
Hide resolved
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/api/src/external/commonwell-v2/command/organization/organization.ts (4)
121-140: Remove redundant overrides and fix comment typo
isActiveandtechnicalContactsalready come fromcwOrgBase. Also fix “initatorOnly” → “initiatorOnly”.const cwOrg: CwSdkOrganizationWithOrgId = { ...cwOrgBase, - securityTokenKeyType: "", // Must be an empty string for initatorOnly orgs - isActive: org.active, - technicalContacts: [technicalContact], + securityTokenKeyType: "", // Must be an empty string for initiatorOnly orgs gateways: [], networks: [
149-149: Defer JSON.stringify in debug logs to avoid unnecessary workPass a function so stringify runs only when debug is enabled. Matches established logging pattern.
- debug(`resp getOneOrg: `, JSON.stringify(resp)); + debug(`resp getOneOrg: `, () => JSON.stringify(resp)); - debug(`resp createOrg: `, JSON.stringify(respCreate)); + debug(`resp createOrg: `, () => JSON.stringify(respCreate)); - debug(`resp addCertificateToOrg: `, JSON.stringify(respAddCert)); + debug(`resp addCertificateToOrg: `, () => JSON.stringify(respAddCert)); - debug(`resp updateOrg: `, JSON.stringify(resp)); + debug(`resp updateOrg: `, () => JSON.stringify(resp));Also applies to: 175-175, 179-179, 204-204
246-246: Trim OID prefix safelyUse prefix-aware trimming to avoid accidental mid-string replacements.
- oid: org.organizationId.replace(OID_PREFIX, ""), + oid: + org.organizationId.startsWith(OID_PREFIX) + ? org.organizationId.slice(OID_PREFIX.length) + : org.organizationId,
24-31: Hoist magic strings to constantsMinor readability/typo-proofing: move repeated literals to named constants per guidelines.
const defaultSearchRadius = 150; + +const CW_NETWORK_TYPE = "CommonWell" as const; +const GATEWAY_SERVICE_TYPE_R4_BASE = "R4_Base" as const; +const GATEWAY_TYPE_FHIR = "FHIR" as const; +const PURPOSE_OF_USE_TREATMENT = "TREATMENT" as const; @@ - networks: [ + networks: [ { - type: "CommonWell", + type: CW_NETWORK_TYPE, purposeOfUse: [], }, ], @@ - { - serviceType: "R4_Base", - gatewayType: "FHIR", + { + serviceType: GATEWAY_SERVICE_TYPE_R4_BASE, + gatewayType: GATEWAY_TYPE_FHIR, endpointLocation: Config.getGatewayEndpoint(), }, ], @@ - { - type: "CommonWell", + { + type: CW_NETWORK_TYPE, purposeOfUse: [ { - id: "TREATMENT", + id: PURPOSE_OF_USE_TREATMENT, queryInitiatorOnly: !org.isInitiatorAndResponder, queryInitiator: org.isInitiatorAndResponder, queryResponder: org.isInitiatorAndResponder, }, ], },Also applies to: 93-140
📜 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 (1)
packages/api/src/external/commonwell-v2/command/organization/organization.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
Useconstwhenever possible
Useasync/awaitinstead 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/api/src/external/commonwell-v2/command/organization/organization.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use types whenever possible
Files:
packages/api/src/external/commonwell-v2/command/organization/organization.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
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- U...
Files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
🧠 Learnings (34)
📓 Common learnings
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:296-320
Timestamp: 2025-07-17T21:24:37.077Z
Learning: RamilGaripov prefers to maintain consistency with existing patterns in a file rather than making isolated fixes to coding guideline violations like var vs const declarations, when the pattern is already established throughout the codebase.
📚 Learning: 2025-08-25T17:52:19.445Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/api/src/external/commonwell-v2/command/organization/organization.ts:89-98
Timestamp: 2025-08-25T17:52:19.445Z
Learning: In packages/api/src/external/commonwell-v2/command/organization/organization.ts, when setting organizationId, homeCommunityId, and patientIdAssignAuthority fields for CommonWell v2 SDK, raw OID values (org.oid) can be used directly without the "urn:oid:" prefix - the SDK handles any necessary formatting internally.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-05-27T15:54:47.774Z
Learnt from: lucasdellabella
PR: metriport/metriport#3866
File: packages/api/src/routes/internal/medical/organization.ts:95-98
Timestamp: 2025-05-27T15:54:47.774Z
Learning: The conversion of the GET `/internal/organization` endpoint from returning a single organization to returning an array of organizations (batch retrieval) in `packages/api/src/routes/internal/medical/organization.ts` was an intentional breaking change planned by the team.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-18T22:03:40.578Z
Learnt from: keshavsaharia
PR: metriport/metriport#4404
File: packages/core/src/external/surescripts/fhir/medication-request.ts:75-94
Timestamp: 2025-08-18T22:03:40.578Z
Learning: In packages/core/src/external/surescripts/fhir/medication-request.ts, there's a discrepancy between standard FHIR R4 specification (which recommends assigning authority URIs in Identifier.system) and specific HL7 guidance that user keshavsaharia references, which may support using code system URIs like "http://terminology.hl7.org/CodeSystem/v2-0203" in Identifier.system for Surescripts integration. This needs to be addressed in future PR with proper HL7 implementation guide references.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-06-26T06:28:31.490Z
Learnt from: keshavsaharia
PR: metriport/metriport#4099
File: packages/core/src/external/surescripts/fhir/coverage.ts:37-42
Timestamp: 2025-06-26T06:28:31.490Z
Learning: In packages/core/src/external/surescripts/fhir/coverage.ts, the getInsuranceOrganizationReference function doesn't need ID validation because the Organization resources are always created by getInsuranceOrganization which assigns id: uuidv7(), guaranteeing a valid ID is present.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-28T20:21:29.231Z
Learnt from: leite08
PR: metriport/metriport#4412
File: packages/api/src/external/commonwell-v2/command/organization/organization.ts:0-0
Timestamp: 2025-08-28T20:21:29.231Z
Learning: CommonWell v2 API uses its own OAuth scope format like "fhir/document" for documentReferenceScope and binaryScope in gateway authorization configuration, which is different from SMART on FHIR v2 scope conventions. This is part of CommonWell's proprietary API implementation, not standard SMART on FHIR.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-25T17:52:19.940Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/commonwell-sdk/src/client/commonwell-member.ts:231-233
Timestamp: 2025-08-25T17:52:19.940Z
Learning: In packages/commonwell-sdk/src/client/commonwell-member.ts, the getOneOrg method currently returns undefined for all non-404 HTTP errors (instead of throwing MetriportError) as a temporary workaround while waiting for the service provider to fix issues on their end. This is tracked in TODO ENG-668 and will be reverted later once the external dependency is resolved.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-25T17:26:51.717Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/api/src/external/commonwell-v2/command/organization/organization.ts:175-187
Timestamp: 2025-08-25T17:26:51.717Z
Learning: In the Metriport codebase, the established error handling pattern is to use capture.error(msg, { extra: { error } }) where the message is passed as the first parameter and the actual error object is included in the extra metadata. This pattern is intentionally used throughout the codebase and should not be changed to capture.error(error, { extra: {...} }).
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-03-19T13:41:19.957Z
Learnt from: leite08
PR: metriport/metriport#3463
File: packages/api/src/command/medical/patient/settings/create-patient-settings.ts:202-204
Timestamp: 2025-03-19T13:41:19.957Z
Learning: In the Metriport codebase, when using `capture.error` or `capture.message` functions, the order of properties in the `extra` parameter is important because Sentry may trim data beyond its payload limit. The most critical information (usually `cxId`) should be placed first, while less important data (like `facilityId`) should be placed last.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-18T17:26:58.012Z
Learnt from: CR
PR: metriport/metriport#0
File: .cursorrules:0-0
Timestamp: 2025-07-18T17:26:58.012Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : 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.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-19T12:28:30.160Z
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/api/src/external/ehr/canvas/command/resource-diff/start-resource-diff.ts:20-43
Timestamp: 2025-04-19T12:28:30.160Z
Learning: In the Metriport codebase, non-entry point commands (like internal service functions) should typically bubble up errors rather than handling them locally with try/catch blocks. Error handling is generally consolidated at entry points like API endpoints.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-18T05:40:08.493Z
Learnt from: lucasdellabella
PR: metriport/metriport#3675
File: packages/core/src/command/hl7v2-subscriptions/hl7v2-to-fhir-converter.ts:167-171
Timestamp: 2025-04-18T05:40:08.493Z
Learning: The team prefers to let errors bubble up to be caught by a Sentry root error handler rather than adding explicit try/catch blocks for error handling in Promise operations.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-24T02:03:04.234Z
Learnt from: leite08
PR: metriport/metriport#3412
File: packages/api/src/command/medical/patient/patient-import/mapping/create.ts:0-0
Timestamp: 2025-04-24T02:03:04.234Z
Learning: For the Metriport codebase, errors should be allowed to bubble up instead of being caught and wrapped at each function level. This is their established error handling pattern.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-05-08T19:41:19.510Z
Learnt from: thomasyopes
PR: metriport/metriport#3788
File: packages/api/src/external/ehr/shared/utils/bundle.ts:73-82
Timestamp: 2025-05-08T19:41:19.510Z
Learning: For the Metriport codebase, the team prefers letting errors bubble up the call stack naturally rather than adding explicit try-catch blocks at lower levels of abstraction.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.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/commonwell-v2/command/organization/organization.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/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-03-29T00:01:59.998Z
Learnt from: thomasyopes
PR: metriport/metriport#3545
File: packages/core/src/command/write-to-storage/s3/write-to-s3-cloud.ts:12-20
Timestamp: 2025-03-29T00:01:59.998Z
Learning: In the Metriport codebase, error handling is often managed at higher levels in the call stack ("further up the chain") rather than within individual methods.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-17T21:24:37.077Z
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:296-320
Timestamp: 2025-07-17T21:24:37.077Z
Learning: RamilGaripov prefers to maintain consistency with existing patterns in a file rather than making isolated fixes to coding guideline violations like var vs const declarations, when the pattern is already established throughout the codebase.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-27T23:12:23.954Z
Learnt from: thomasyopes
PR: metriport/metriport#4478
File: packages/core/src/external/ehr/api/job/start-contribute-bundles-job.ts:44-47
Timestamp: 2025-08-27T23:12:23.954Z
Learning: In packages/core/src/external/ehr/api/job/start-contribute-bundles-job.ts, the intentional swallowing of HTTP 400 errors in startContributeBundlesJob via early return is by design, as confirmed by thomasyopes. This pattern should not be flagged in future reviews.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-15T18:57:29.030Z
Learnt from: lucasdellabella
PR: metriport/metriport#4382
File: packages/utils/src/hl7v2-notifications/reprocess-adt-conversion-bundles/common.ts:36-48
Timestamp: 2025-08-15T18:57:29.030Z
Learning: In utility scripts like packages/utils/src/hl7v2-notifications/reprocess-adt-conversion-bundles/common.ts, user lucasdellabella prefers to let bundle parsing errors fail fast rather than adding error handling, as it provides immediate visibility into data issues during script execution.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-26T02:33:26.777Z
Learnt from: leite08
PR: metriport/metriport#4469
File: packages/fhir-converter/src/lib/workers/worker.js:75-87
Timestamp: 2025-08-26T02:33:26.777Z
Learning: User leite08 confirmed that the debug logging approach in packages/fhir-converter/src/lib/workers/worker.js was already reviewed and approved in the feature PR, even though it logs both error messages and stack traces.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.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/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-17T21:24:30.319Z
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:255-260
Timestamp: 2025-07-17T21:24:30.319Z
Learning: In packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js, RamilGaripov prefers to maintain consistency with existing var declaration patterns throughout the file rather than changing individual functions to const, even when coding guidelines suggest using const whenever possible.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.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/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-18T22:03:40.578Z
Learnt from: keshavsaharia
PR: metriport/metriport#4404
File: packages/core/src/external/surescripts/fhir/medication-request.ts:75-94
Timestamp: 2025-08-18T22:03:40.578Z
Learning: User keshavsaharia indicated that HL7 guidance contradicts the standard FHIR R4 specification regarding Identifier.system usage in packages/core/src/external/surescripts/fhir/medication-request.ts, specifically that using HL7_CODE_SYSTEM_URL in Identifier.system may be correct according to HL7 guidance despite appearing to violate standard FHIR patterns.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-25T17:52:11.649Z
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/core/src/external/commonwell-v2/document/document-downloader-local-v2.ts:49-58
Timestamp: 2025-08-25T17:52:11.649Z
Learning: RamilGaripov prefers to maintain consistency between v1 and v2 CommonWell implementations rather than addressing individual technical issues in isolation, even when the existing pattern has potential issues like binary data handling via string concatenation.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.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/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-28T22:52:54.904Z
Learnt from: leite08
PR: metriport/metriport#3749
File: packages/core/src/command/patient-import/steps/query/patient-import-query-cloud.ts:12-26
Timestamp: 2025-04-28T22:52:54.904Z
Learning: The team prefers to let errors bubble up to higher-level handlers rather than adding error handling at each individual function level. Only suggest adding error handling for functions that are called asynchronously and don't have error handling on the clients.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-05-22T20:28:26.589Z
Learnt from: leite08
PR: metriport/metriport#3887
File: packages/infra/lib/surescripts-stack.ts:121-134
Timestamp: 2025-05-22T20:28:26.589Z
Learning: For configuration functions, returning `undefined` is preferred over empty objects when configuration is missing, as it makes the absence of configuration more explicit and easier to check.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-04-28T23:10:42.561Z
Learnt from: leite08
PR: metriport/metriport#3749
File: packages/api/src/command/medical/patient/patient-import/get.ts:70-74
Timestamp: 2025-04-28T23:10:42.561Z
Learning: Error messages should have static messages. Dynamic data like IDs should be added to MetriportError's `additionalInfo` property rather than interpolated into the error message string.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-06-18T04:51:44.941Z
Learnt from: leite08
PR: metriport/metriport#4044
File: packages/api/src/command/medical/tcm-encounter/update-tcm-encounter.ts:23-23
Timestamp: 2025-06-18T04:51:44.941Z
Learning: Error messages should be fixed, stable, and non-dynamic. Dynamic data like IDs should be moved to the `additionalInfo` property of MetriportError instead of being interpolated into the error message string.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-08-21T15:53:34.154Z
Learnt from: leite08
PR: metriport/metriport#4427
File: packages/commonwell-cert-runner/src/flows/contribution/document-reference.ts:23-24
Timestamp: 2025-08-21T15:53:34.154Z
Learning: CommonWell uses their own version/profile of FHIR R4 that may have different validation rules than pure FHIR R4, including potentially allowing different formats for resource IDs and references that wouldn't be valid in standard FHIR R4.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.ts
📚 Learning: 2025-07-09T17:18:28.920Z
Learnt from: thomasyopes
PR: metriport/metriport#4164
File: packages/core/src/external/ehr/healthie/index.ts:1031-1076
Timestamp: 2025-07-09T17:18:28.920Z
Learning: In packages/core/src/external/ehr/healthie/index.ts, the `interpretation` field in `labObservationResult` is defined as a required enum in the Zod schema (`z.enum(["high", "low", "normal"])`), making null-safety checks unnecessary when calling `toTitleCase(labObservationResult.interpretation)`.
Applied to files:
packages/api/src/external/commonwell-v2/command/organization/organization.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/commonwell-v2/command/organization/organization.ts
🧬 Code graph analysis (1)
packages/api/src/external/commonwell-v2/command/organization/organization.ts (9)
packages/shared/src/common/env-var.ts (1)
getEnvVarOrFail(14-20)packages/core/src/domain/organization.ts (1)
OrganizationData(13-18)packages/commonwell-sdk/src/models/organization.ts (3)
OrganizationWithNetworkInfo(105-105)OrganizationBase(60-60)isOrgInitiatorAndResponder(140-142)packages/api/src/shared/config.ts (1)
Config(4-318)packages/core/src/util/log.ts (3)
out(30-35)debug(25-28)log(10-23)packages/api/src/external/commonwell-v2/api.ts (2)
makeCommonWellMemberAPI(25-40)getCertificate(79-102)packages/shared/src/net/retry.ts (1)
executeWithNetworkRetries(111-138)packages/core/src/util/error/shared.ts (1)
errorToString(15-23)packages/shared/src/index.ts (2)
NotFoundError(44-44)MetriportError(43-43)
⏰ 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). (15)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
packages/api/src/external/commonwell-v2/command/organization/organization.ts (6)
82-85: LGTM on raw OID usage for CW v2 identifiersUsing raw OIDs for organizationId/homeCommunityId/patientIdAssignAuthority is correct for the v2 SDK here.
93-109: JWT key type and CommonWell-specific scopes look correctSecurity token type "JWT" and the proprietary "fhir/document" scopes align with CW v2 behavior.
170-180: No retries on create + retries on addCertificate is appropriateAvoiding duplicate org creation while retrying cert association is the right tradeoff.
203-219: 404 → create fallback with early return is correctHandling NotFoundError and returning after successful create avoids masking success.
226-233: Good: static error message with additionalInfoThe parse guard follows the team’s error conventions and bubbles up cleanly.
268-276: Case-normalized mapping is acceptable with documented enum casingLookup with
.toLowerCase().trim()is fine given the enum casing guarantee.
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref: #000 - api@1.29.0-alpha.0 - @metriport/api-sdk@18.0.1-alpha.0 - @metriport/carequality-cert-runner@1.20.0-alpha.0 - @metriport/carequality-sdk@1.9.0-alpha.0 - @metriport/commonwell-cert-runner@2.0.3-alpha.0 - @metriport/commonwell-jwt-maker@1.26.2-alpha.0 - @metriport/commonwell-sdk@6.2.3-alpha.0 - @metriport/core@1.25.2-alpha.0 - @metriport/eslint-plugin-eslint-rules@1.1.2-alpha.0 - @metriport/fhir-sdk@1.1.2-alpha.0 - @metriport/ihe-gateway-sdk@0.20.2-alpha.0 - infrastructure@1.23.2-alpha.0 - mllp-server@0.4.2-alpha.0 - @metriport/shared@0.25.2-alpha.0 - utils@1.26.2-alpha.0 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref: #000 - api@1.29.0-alpha.1 - @metriport/api-sdk@18.0.1-alpha.1 - @metriport/carequality-cert-runner@1.20.0-alpha.1 - @metriport/carequality-sdk@1.9.0-alpha.1 - @metriport/commonwell-cert-runner@2.0.3-alpha.1 - @metriport/commonwell-jwt-maker@1.26.2-alpha.1 - @metriport/commonwell-sdk@6.2.3-alpha.1 - @metriport/core@1.25.2-alpha.1 - @metriport/eslint-plugin-eslint-rules@1.1.2-alpha.1 - @metriport/fhir-sdk@1.1.2-alpha.1 - @metriport/ihe-gateway-sdk@0.20.2-alpha.1 - infrastructure@1.23.2-alpha.1 - mllp-server@0.4.2-alpha.1 - @metriport/shared@0.25.2-alpha.1 - utils@1.26.2-alpha.1 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
packages/api-sdk/package.json(2 hunks)packages/api/package.json(1 hunks)packages/carequality-cert-runner/package.json(2 hunks)packages/carequality-sdk/package.json(1 hunks)packages/commonwell-cert-runner/package.json(1 hunks)packages/commonwell-jwt-maker/package.json(1 hunks)packages/commonwell-sdk/package.json(2 hunks)packages/core/package.json(1 hunks)packages/eslint-rules/package.json(1 hunks)packages/fhir-sdk/package.json(1 hunks)packages/ihe-gateway-sdk/package.json(2 hunks)packages/infra/package.json(1 hunks)packages/mllp-server/package.json(1 hunks)packages/shared/package.json(1 hunks)packages/utils/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- packages/infra/package.json
- packages/commonwell-jwt-maker/package.json
- packages/fhir-sdk/package.json
- packages/eslint-rules/package.json
- packages/carequality-sdk/package.json
- packages/mllp-server/package.json
- packages/commonwell-sdk/package.json
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/ihe-gateway-sdk/package.json
- packages/commonwell-cert-runner/package.json
- packages/api/package.json
- packages/core/package.json
- packages/shared/package.json
- packages/utils/package.json
- packages/carequality-cert-runner/package.json
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: RamilGaripov
PR: metriport/metriport#4412
File: packages/core/src/external/commonwell-v2/document/document-downloader-local-v2.ts:275-288
Timestamp: 2025-08-25T17:52:21.619Z
Learning: RamilGaripov prefers to maintain consistency between CommonWell v1 and v2 implementations during the CW v2 API migration, keeping the new v2 files nearly identical to existing v1 files rather than making isolated improvements.
Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:296-320
Timestamp: 2025-07-17T21:24:37.077Z
Learning: RamilGaripov prefers to maintain consistency with existing patterns in a file rather than making isolated fixes to coding guideline violations like var vs const declarations, when the pattern is already established throughout the codebase.
📚 Learning: 2025-03-19T13:58:17.253Z
Learnt from: leite08
PR: metriport/metriport#3463
File: packages/api-sdk/src/medical/models/patient.ts:1-1
Timestamp: 2025-03-19T13:58:17.253Z
Learning: When changes are made to SDK packages (`api-sdk`, `commonwell-sdk`, `carequality-sdk`) or `packages/shared` in the Metriport codebase, alpha versions need to be published to NPM before merging the PR.
Applied to files:
packages/api-sdk/package.json
📚 Learning: 2025-08-20T22:19:04.553Z
Learnt from: leite08
PR: metriport/metriport#0
File: :0-0
Timestamp: 2025-08-20T22:19:04.553Z
Learning: In the Metriport codebase, during CommonWell SDK major version upgrades (like v5 to v6), it's expected and intentional to maintain both versions simultaneously - the new version (metriport/commonwell-sdk) and an aliased v1 version (metriport/commonwell-sdk-v1) for backward compatibility during migration periods.
Applied to files:
packages/api-sdk/package.json
📚 Learning: 2025-08-13T16:08:03.706Z
Learnt from: RadmirGari
PR: metriport/metriport#4324
File: packages/core/src/domain/address.ts:7-7
Timestamp: 2025-08-13T16:08:03.706Z
Learning: In the Metriport codebase, packages/core legitimately depends on metriport/api-sdk and imports from it in multiple places. This is an established architectural pattern and is properly declared as a dependency in core's package.json.
Applied to files:
packages/api-sdk/package.json
📚 Learning: 2025-07-15T19:00:17.445Z
Learnt from: leite08
PR: metriport/metriport#4190
File: packages/shared/package.json:3-3
Timestamp: 2025-07-15T19:00:17.445Z
Learning: The connect-widget package in the Metriport codebase is no longer being updated, so alpha version references in its package.json can be left as-is during release processes.
Applied to files:
packages/api-sdk/package.json
📚 Learning: 2025-08-20T22:28:47.373Z
Learnt from: leite08
PR: metriport/metriport#0
File: :0-0
Timestamp: 2025-08-20T22:28:47.373Z
Learning: In the Metriport codebase, form-data dependencies with version ">=4.0.4" are intentionally added to package.json files (even when not directly used) to force NPM to use secure versions and address Dependabot security alerts for transitive dependencies.
Applied to files:
packages/api-sdk/package.json
⏰ 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). (15)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/api-sdk/package.json (1)
3-3: Add publishConfig.tag and align internal references
- In packages/api-sdk/package.json, add:
"publishConfig": { "access": "public", "tag": "alpha" }- Update any downstream references of @metriport/api-sdk to the new prerelease (18.0.1-alpha.1):
• packages/lambdas/package.json — change"@metriport/api-sdk": "^7.12.7"to"^18.0.1-alpha.1"
• packages/connect-widget/package.json — change"^7.12.0-alpha.1"to"^18.0.1-alpha.1"⛔ Skipped due to learnings
Learnt from: leite08 PR: metriport/metriport#3463 File: packages/api-sdk/src/medical/models/patient.ts:1-1 Timestamp: 2025-03-19T13:58:17.253Z Learning: When changes are made to SDK packages (`api-sdk`, `commonwell-sdk`, `carequality-sdk`) or `packages/shared` in the Metriport codebase, alpha versions need to be published to NPM before merging the PR.Learnt from: leite08 PR: metriport/metriport#4190 File: packages/shared/package.json:3-3 Timestamp: 2025-07-15T19:00:17.445Z Learning: The connect-widget package in the Metriport codebase is no longer being updated, so alpha version references in its package.json can be left as-is during release processes.
packages/api-sdk/package.json
Outdated
| @@ -58,8 +58,7 @@ | |||
| }, | |||
| "dependencies": { | |||
| "@medplum/fhirtypes": "^2.0.32", | |||
| "@metriport/commonwell-sdk": "^5.9.20", | |||
| "@metriport/shared": "^0.25.0", | |||
| "@metriport/shared": "^0.25.2-alpha.1", | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Pin prerelease dependency to avoid unintended drift
For prereleases, caret ranges can jump to newer alpha builds unexpectedly. Prefer exact (or tilde) to keep the SDK reproducible during the migration.
Apply:
- "@metriport/shared": "^0.25.2-alpha.1",
+ "@metriport/shared": "0.25.2-alpha.1",Also confirm:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Shared alpha is published
npm view @metriport/shared versions --json | jq -r '.[]' | rg '^0\.25\.2-alpha\.1$' || {
echo "@metriport/shared 0.25.2-alpha.1 not found on npm"; exit 1; }
# 2) No lingering imports of the removed CW v1 SDK inside api-sdk
rg -nP '@metriport/commonwell-sdk' packages/api-sdk/src || echo "OK: no references found"Length of output: 233
Pin @metriport/shared prerelease dependency
Change the caret (^) range to an exact version to prevent automatic alpha upgrades in packages/api-sdk/package.json:
- "@metriport/shared": "^0.25.2-alpha.1",
+ "@metriport/shared": "0.25.2-alpha.1",Verified that version 0.25.2-alpha.1 is published on npm and there are no lingering @metriport/commonwell-sdk imports in api-sdk.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "@metriport/shared": "^0.25.2-alpha.1", | |
| ++ b/packages/api-sdk/package.json | |
| @@ lines 61 | |
| "@metriport/shared": "0.25.2-alpha.1", |
🤖 Prompt for AI Agents
In packages/api-sdk/package.json around line 61, the prerelease dependency
"@metriport/shared" is specified with a caret range ("^0.25.2-alpha.1") which
allows automatic alpha upgrades; change it to the exact version "0.25.2-alpha.1"
(remove the caret) to pin the prerelease, then run the package manager to update
lockfile (npm/yarn/pnpm install) and commit the package.json and lockfile
changes.
RamilGaripov
commented
Aug 29, 2025
packages/api/src/external/commonwell-v2/command/organization/organization.ts
Outdated
Show resolved
Hide resolved
| org: { | ||
| oid: facility.cwOboOid ?? facility.oid, | ||
| data: { | ||
| name: orgName, |
There was a problem hiding this comment.
I think let's just use the new DOA now, right? Kill 2 birds with 1 stone?
| throw new BadRequestError(title, error, { status, cwReference, data }); | ||
| } | ||
| if (status === httpStatus.NOT_FOUND) { | ||
| throw new MetriportError(title, error, { status, cwReference, data }); |
There was a problem hiding this comment.
I've run into issues with this, where data is an object and ends up printing without me seeing details.. like:
error: {
data: [array]
}
or something like that..
Comment on lines
+125
to
+128
| /** | ||
| * Keep these in lowercase since the code that uses it to map the CW type to the Metriport type | ||
| * expects the keys to be lowercase. | ||
| */ |
| } | ||
| throw error as never; | ||
| return error; |
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
leite08
approved these changes
Aug 29, 2025
Ref eng-513 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
4 tasks
Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of ENG-513
Issues:
Dependencies
Description
Testing
Release Plan
Summary by CodeRabbit
New Features
Breaking Changes
Improvements
Documentation
Chores