RELEASE cavnas diff part 2 (with jobs!) #3714
Conversation
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…port into 2850-canvas-diff-workflow Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…port into 2850-canvas-diff-workflow Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…port into 2850-canvas-diff-workflow Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…port into 2850-canvas-diff-workflow Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…port into 2850-canvas-diff-workflow Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…port into 2850-canvas-diff-workflow Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…port into 2850-canvas-diff-workflow Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…port into 2850-canvas-diff-workflow Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
WalkthroughThis set of changes introduces a comprehensive patient job management system and refactors the workflow for resource diff bundle creation and retrieval, particularly for Canvas EHR integrations. It adds a new database model and migration for tracking patient jobs, implements job lifecycle management (creation, initialization, status updates, completion), and exposes new internal API routes for job operations. The resource diff bundle endpoints are refactored to operate asynchronously via job tracking, with endpoints for job initiation and result retrieval. Supporting utilities, types, and schemas are added or updated across the shared, API, and core packages to enable robust job tracking, status validation, and S3 bundle management keyed by job IDs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant DB
participant S3
participant CanvasEHR
Client->>API: POST /ehr/canvas/patient/:id/resource/diff (start job)
API->>DB: Create PatientJob (status: waiting)
API-->>Client: { jobId }
Client->>API: GET /ehr/canvas/patient/:id/resource/diff/:jobId (poll for result)
API->>DB: Get PatientJob by jobId
alt Job status == completed
API->>CanvasEHR: Fetch pre-signed URLs for resource diff bundles
API-->>Client: { job metadata, preSignedUrls }
else Job not completed
API-->>Client: { job metadata }
end
Note over API,CanvasEHR: (Background worker)
API->>CanvasEHR: Start resource diff bundle job (async)
CanvasEHR->>S3: Store resource diff bundles (keyed by jobId)
CanvasEHR->>API: Update job entry statuses, mark job as completed
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (1)`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
🔇 Additional comments (6)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (3)
packages/core/src/external/ehr/shared.ts (1)
15-27:⚠️ Potential issueFixed missing dayjs plugin initialization.
The code was previously using
dayjs.duration()(lines 29 and 276) without properly importing and extending the duration plugin, which could lead to runtime errors.This fix properly imports and extends the dayjs duration plugin, aligning with the coding guidelines for date and time management:
- "Always use
buildDayjs()to createdayjsinstances"- "Prefer
dayjs.duration(...)to create duration consts"packages/api/src/external/ehr/canvas/command/bundle/fetch-ehr-bundle.ts (1)
60-73: 💡 Verification agent🧩 Analysis chain
Consider handling potential duplicates in merged bundles.
When merging entries from multiple resource bundles, there's no check for duplicate resources. This could potentially lead to the same resource appearing multiple times in the final bundle.
🏁 Script executed:
#!/bin/bash # Description: Check if the code handles duplicate resources when merging bundles # Look for any checks of duplicates in bundle entry merging rg -A 5 -B 5 "bundle.entry.push\(" --glob "**/*.ts" # Look for any distinct or filter operations on bundle entries rg -A 5 -B 5 "entry.*filter\(" --glob "**/*.ts"Length of output: 36908
Handle duplicate resources when merging bundles
The current loop in
packages/api/src/external/ehr/canvas/command/bundle/fetch-ehr-bundle.ts(lines 60–73) blindly pushes all entries from eachresourceBundleinto the finalbundle. If the same resource appears in more than oneresourceBundlecall, you’ll get duplicates in the result.Suggested fixes:
- After merging, filter
bundle.entryby a unique key (e.g.${resourceType}/${id}) to drop duplicates.- You can reuse the deduplication logic in
packages/utils/src/fhir-deduplication, or add:for (const resourceType of resourceTypes) { const resourceBundle = await canvasApi.getBundleByResourceType({ … }); bundle.entry.push(...resourceBundle.entry); } + // remove duplicate resources by resourceType/id + const seen = new Set<string>(); + bundle.entry = bundle.entry.filter(entry => { + const res = entry.resource; + if (!res?.resourceType || !res.id) return true; + const key = `${res.resourceType}/${res.id}`; + if (seen.has(key)) return false; + seen.add(key); + return true; + }); return { bundle, resourceTypes };• File: packages/api/src/external/ehr/canvas/command/bundle/fetch-ehr-bundle.ts
• Location: immediately after the loop that pushesresourceBundle.entrypackages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/start/ehr-start-resource-diff-bundles-local.ts (1)
36-104: 🛠️ Refactor suggestionAdd error handling for job operations.
The function lacks try-catch blocks for handling errors during job initialization, bundle creation, or job total updates. An error in any of these operations could cause the job to fail without updating its status.
async startResourceDiffBundlesMetriportOnly({ ehr, cxId, practiceId, metriportPatientId, ehrPatientId, jobId, }: StartResourceDiffBundlesRequest): Promise<void> { - await initializeJob({ cxId, jobId }); - const consolidated = await getConsolidated({ cxId, patientId: metriportPatientId }); - if (!consolidated || !consolidated.bundle?.entry || consolidated.bundle.entry.length < 1) { - return; - } + try { + await initializeJob({ cxId, jobId }); + const consolidated = await getConsolidated({ cxId, patientId: metriportPatientId }); + if (!consolidated || !consolidated.bundle?.entry || consolidated.bundle.entry.length < 1) { + return; + } // ... rest of function ... + const total = computeResourceDiffParamsWithExistingResources.length; + await updateJobTotal({ cxId, jobId, total }); + await this.next.computeResourceDiffBundlesMetriportOnly( + computeResourceDiffParamsWithExistingResources + ); + if (this.waitTimeInMillis > 0) await sleep(this.waitTimeInMillis); + } catch (error) { + // Import this at the top of the file + // import { setPatientJobEntryStatus } from "../../../../api/set-entry-status"; + await setPatientJobEntryStatus({ cxId, jobId, entryStatus: "failed" }) + .catch(e => { + const { log } = out("EhrStartResourceDiffBundlesLocal"); + log(`Failed to set job status to failed: ${errorToString(e)}`); + }); + throw error; + } }
🧹 Nitpick comments (21)
packages/lambdas/src/ehr-compute-resource-diff-bundles.ts (1)
30-32: Consider extracting the jobId for consistency and clarity.While the schema now requires
jobId, it's not being destructured from the parsed body. Consider adding it to the destructuring statement for clarity, even if it's only used internally by thecomputeResourceDiffBundlesMetriportOnlymethod.- const { ehr, cxId, practiceId, metriportPatientId, ehrPatientId, newResource } = parsedBody; + const { ehr, cxId, practiceId, metriportPatientId, ehrPatientId, newResource, jobId } = parsedBody;packages/core/src/external/ehr/api/fetch-bundle.ts (1)
38-38: Minor log prefix inconsistency with function nameThe log prefix has been changed from "Ehr fetchEhrBundle" to "Ehr fetchBundle", which doesn't match the function name
fetchEhrBundle. While this is a minor issue, consistent naming helps with log readability and debugging.- const { log, debug } = out(`Ehr fetchBundle - cxId ${cxId}`); + const { log, debug } = out(`Ehr fetchEhrBundle - cxId ${cxId}`);packages/api/src/command/job/patient/update-total.ts (2)
24-28: Consider making total field required in the job creation flowSince you validate that total must be greater than 0 here, it would be better to ensure the total is properly set during job creation rather than defaulting to 0 (as seen in create.ts). This would provide more consistency in the job lifecycle.
16-31: Unused parameter in type definitionThe
UpdateJobTotalParamstype includes aforceTotalUpdateparameter that isn't used in this function implementation. Consider either using this parameter or removing it from the type definition.packages/core/src/external/ehr/api/set-entry-status.ts (1)
26-26: Consider using path constants for API endpoint constructionInstead of hardcoding the path as a string, consider extracting it to a constant or using a more structured approach for API endpoint construction. This would make the code more maintainable if the API paths change.
+const PATIENT_JOB_BASE_PATH = '/internal/patient/job'; -const updateJobUrl = `/internal/patient/job/${jobId}/set-entry-status?${queryParams.toString()}`; +const updateJobUrl = `${PATIENT_JOB_BASE_PATH}/${jobId}/set-entry-status?${queryParams.toString()}`;packages/api/src/command/job/patient/create.ts (2)
13-52: Consider adding logging for job creationUnlike other job-related functions in this PR (such as complete.ts), this function doesn't include logging. Consider adding logging to track job creation for consistency and easier debugging.
+import { out } from "@metriport/core/util/log"; export async function createPatientJob({ cxId, patientId, jobType, jobGroupId, requestId, limitedToOneRunningJob = false, }: CreatePatientJobParams): Promise<PatientJob> { + const { log } = out(`createPatientJob - patientId ${patientId} cxId ${cxId} jobType ${jobType}`); if (limitedToOneRunningJob) { const runningJob = await getLatestPatientJob({ cxId, patientId, jobType, jobGroupId, status: ["waiting", "processing"], }); if (runningJob) { + log(`Found existing running job: ${runningJob.id}`); throw new BadRequestError("Only one job can be running at a time", undefined, { cxId, patientId, jobType, jobGroupId, runningJobId: runningJob.id, }); } } + log(`Creating new job with type ${jobType}`); const created = await PatientJobModel.create({ id: uuidv7(), cxId, patientId, jobType, jobGroupId, requestId, status: jobInitialStatus, total: 0, successful: 0, failed: 0, }); return created.dataValues; }
39-50: Consider adding initial timestampsThe job creation doesn't set a
createdAttimestamp. Although Sequelize might handle this automatically, it would be clearer to explicitly set it for consistency with other fields.packages/api/src/command/job/patient/complete.ts (2)
25-25: Misaligned logging contextThe log context is
updateJobTrackingbut the function is namedcompletePatientJob. Consider updating the logging context to match the function name for clarity and consistency.- const { log } = out(`updateJobTracking - jobId ${jobId} cxId ${cxId}`); + const { log } = out(`completePatientJob - jobId ${jobId} cxId ${cxId}`);
34-36: Consider adding statusReason to the updated fieldsThe function updates the status but doesn't provide a way to set a reason for the status change. Consider adding an optional
statusReasonparameter to provide more context about why the job was completed.-export async function completePatientJob({ - jobId, - cxId, - forceStatusUpdate = false, - onCompleted, -}: CompleteJobParams): Promise<PatientJob> { +export async function 8000 completePatientJob({ + jobId, + cxId, + forceStatusUpdate = false, + onCompleted, + statusReason, +}: CompleteJobParams & { statusReason?: string }): Promise<PatientJob> { // ...existing code... - const fieldsToUpdate: Partial<Pick<PatientJob, "status" | "finishedAt">> = { + const fieldsToUpdate: Partial<Pick<PatientJob, "status" | "statusReason" | "finishedAt">> = { status: newStatus, + ...(statusReason && { statusReason }), };packages/api/src/command/job/patient/set-entry-status.ts (1)
40-43: Consider a more type-safe approach for handling Sequelize raw resultsWhile the current approach with disabling ESLint and using
anyis explained in the comment, consider defining a type interface for the raw DB response to make the code more maintainable and type-safe in the long run.- // Using any because Sequelize doesn't map the columns to the model, even using mapToModel/model - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const updatedRaw = (updatedRows as unknown as any[] | undefined)?.[0]; + // Define a type for the raw DB response to improve type safety + type PatientJobRawRow = Record<keyof typeof patientJobRawColumnNames, unknown>; + const updatedRaw = (updatedRows as unknown as PatientJobRawRow[] | undefined)?.[0];packages/api/src/sequelize/migrations/2025-04-16_00_create-patient-job.ts (1)
79-92: Consider adding database-level validation for JSONB fields.While the JSONB fields are appropriate for flexible data storage, consider whether any validation constraints should be applied at the database level for consistency, especially for
paramsCxandparamsOpswhich have specific type definitions in the shared package.packages/core/src/external/ehr/bundle/commands/update-bundle.ts (1)
26-26: Make fallback behavior for jobId more explicit in the code.The JSDoc states "If not provided, the tag 'latest' will be used" for jobId, but this fallback logic isn't visible in this function. For clarity and maintainability, consider implementing this fallback explicitly in the function rather than relying on downstream implementations.
- const key = createKey({ ehr, cxId, metriportPatientId, ehrPatientId, resourceType, jobId }); + // If jobId is not provided, use 'latest' tag as default + const effectiveJobId = jobId ?? 'latest'; + const key = createKey({ ehr, cxId, metriportPatientId, ehrPatientId, resourceType, jobId: effectiveJobId });packages/api/src/models/patient-job.ts (1)
46-101: Consider adding database constraints in the model definition.While the database migration defines constraints like
allowNullanddefaultValue, these aren't reflected in the model definition. Consider adding these constraints to the model for consistency and to ensure the ORM enforces the same rules as the database.Example for the
totalfield:total: { type: DataTypes.INTEGER, + allowNull: false, + defaultValue: 0, },packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.ts (1)
33-46: Consider separating duplicate resource checking logic.The code for checking duplicate resources is quite complex and might benefit from being extracted into a separate function for better readability and maintainability.
- const existingResourcesToUse: FhirResource[] = - existingResources ?? - (await getExistingResourcesFromApi({ - ehr, - cxId, - practiceId, - patientId: ehrPatientId, - resourceType, - })); - const isDuplicate = resourceIsDuplicateOfExistingResources({ - existingResources: existingResourcesToUse, - newResource, - }); + const existingResourcesToUse: FhirResource[] = await getExistingResourcesOrUseProvided({ + existingResources, + ehr, + cxId, + practiceId, + patientId: ehrPatientId, + resourceType, + }); + const isDuplicate = resourceIsDuplicateOfExistingResources({ + existingResources: existingResourcesToUse, + newResource, + });Add this function at the bottom of the file:
async function getExistingResourcesOrUseProvided({ existingResources, ehr, cxId, practiceId, patientId, resourceType, }: { existingResources?: FhirResource[]; ehr: string; cxId: string; practiceId: string; patientId: string; resourceType: string; }): Promise<FhirResource[]> { if (existingResources) return existingResources; return getExistingResourcesFromApi({ ehr, cxId, practiceId, patientId, resourceType, }); }packages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/start-job.ts (1)
57-67: Consider documenting the asynchronous behavior in the function's JSDoc.The function initiates an asynchronous process but the JSDoc doesn't explicitly mention this behavior. This could lead to confusion for developers who might expect the function to complete all processing before returning.
/** * Starts the resource diff job to produce the resource type bundles containing * the resources in Metriport that are not in Canvas, or vice versa. + * + * Note: This function starts an asynchronous job and returns immediately with the job ID. + * The actual bundle creation happens in the background. * * @param cxId - The cxId of the patient. * @param canvasPracticeId - The canvas practice id of the patient. * @param canvasPatientId - The canvas patient id of the patient. * @param direction - The direction of the resource diff bundles to create. * @param requestId - The request id of the job. (optional, defaults to a new UUID) + * @returns A promise that resolves to the created job ID. */packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/start/ehr-start-resource-diff-bundles-local.ts (1)
67-87: Consider parallel bundle fetching for improved performance.The function fetches bundles sequentially for each resource type, which could be slow with many resource types. Consider using
Promise.allto fetch bundles concurrently.- for (const resourceType of resourceTypes) { - const [existingResourcesBundle] = await Promise.all([ - fetchEhrBundleFromApi({ - ehr, - cxId, - practiceId, - patientId: ehrPatientId, - resourceType, - useCachedBundle: false, - }), - createOrReplaceBundleOnS3({ - ehr, - cxId, - metriportPatientId, - ehrPatientId, - bundleType: BundleType.RESOURCE_DIFF_METRIPORT_ONLY, - bundle: getDefaultBundle(), - resourceType, - jobId, - }), - ]); - this.fetchedBundles.set(resourceType, existingResourcesBundle.bundle); - } + const fetchPromises = Array.from(resourceTypes).map(async resourceType => { + const [existingResourcesBundle] = await Promise.all([ + fetchEhrBundleFromApi({ + ehr, + cxId, + practiceId, + patientId: ehrPatientId, + resourceType, + useCachedBundle: false, + }), + createOrReplaceBundleOnS3({ + ehr, + cxId, + metriportPatientId, + ehrPatientId, + bundleType: BundleType.RESOURCE_DIFF_METRIPORT_ONLY, + bundle: getDefaultBundle(), + resourceType, + jobId, + }), + ]); + return { resourceType, bundle: existingResourcesBundle.bundle }; + }); + + const results = await Promise.all(fetchPromises); + results.forEach(({ resourceType, bundle }) => { + this.fetchedBundles.set(resourceType, bundle); + });packages/api/src/external/ehr/canvas/command/bundle/fetch-resource-diff-bundle-pre-signed-urls.ts (1)
55-56: Consider simplifying this variable assignment.This intermediate variable assignment doesn't add much value and could be simplified.
- const metriportPatientId = existingPatient.patientId; + const { patientId: metriportPatientId } = existingPatient;packages/api/src/external/ehr/create-resource-diff-bundles.ts (2)
39-62: Consider additional error handling for async job creation.The function uses
.catch(processAsyncError)to handle errors from the job creation, but doesn't provide a way to know if the job was successfully created. Since this function returnsvoid, the caller has no way to know if all jobs were successfully created.Consider returning a result that indicates whether the job creation was initiated successfully, such as:
-async function createCanvasResourceDiffBundlesJob({ +async function createCanvasResourceDiffBundlesJob({ cxId, canvasPatientId, direction, requestId, -}: { +}: { cxId: string; canvasPatientId: string; direction: ResourceDiffDirection; requestId: string; -}): Promise<void> { +}): Promise<boolean> { const cxMappings = await getCxMappingsByCustomer({ cxId, source: EhrSources.canvas }); const cxMapping = cxMappings[0]; if (!cxMapping) throw new MetriportError("Canvas CX mapping not found", undefined, { cxId }); if (cxMappings.length > 1) throw new MetriportError("Multiple Canvas CX mappings found", undefined, { cxId }); - createResourceDiffBundlesJob({ + return createResourceDiffBundlesJob({ cxId, canvasPracticeId: cxMapping.externalId, canvasPatientId, direction, requestId, - }).catch(processAsyncError(`createResourceDiffBundlesJob`)); + }) + .then(jobId => { + return true; + }) + .catch(err => { + processAsyncError(`createResourceDiffBundlesJob`)(err); + return false; + }); }
51-52: Add validation for empty array before accessing first element.The code directly accesses the first element of the
cxMappingsarray without first checking if the array is empty. While it throws an error afterward ifcxMappingis falsy, following the coding guidelines of building deterministic functions, it's better to explicitly check array length first.- const cxMapping = cxMappings[0]; - if (!cxMapping) throw new MetriportError("Canvas CX mapping not found", undefined, { cxId }); + if (cxMappings.length === 0) throw new MetriportError("Canvas CX mapping not found", undefined, { cxId }); + const cxMapping = cxMappings[0];packages/shared/src/domain/job/types.ts (1)
3-4: Consider using more specific type definitions.The
JobParamsCxandJobParamsOpstypes use very genericRecord<string, string | boolean>definitions. While this provides flexibility, it could lead to less type safety. Consider defining more specific interfaces if the structure of these parameters is known.If the parameters have a known structure, you could define them more specifically:
export type JobParamsCx = { customerId?: string; // other known fields [key: string]: string | boolean | undefined; }; export type JobParamsOps = { // known operational parameters [key: string]: string | boolean | undefined; };packages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/get-job-payload.ts (1)
58-99: Function leverages patient mapping to retrieve the latest job.The
getLatestResourceDiffBundlesJobPayloadfunction:
- Properly resolves the Metriport patient ID from the Canvas patient ID
- Uses the job type factory function for consistent job type identification
- Sets appropriate filtering parameters to find the latest relevant job
- Handles the case where no job exists by returning undefined
- Conditionally fetches pre-signed URLs based on job status
One minor optimization to consider: caching the patient mapping result if this function is called frequently, as the mapping lookup adds an extra database query.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (52)
packages/api/src/command/job/patient/complete.ts(1 hunks)packages/api/src/command/job/patient/create.ts(1 hunks)packages/api/src/command/job/patient/get.ts(1 hunks)packages/api/src/command/job/patient/initialize.ts(1 hunks)packages/api/src/command/job/patient/set-entry-status.ts(1 hunks)packages/api/src/command/job/patient/update-total.ts(1 hunks)packages/api/src/command/job/shared.ts(1 hunks)packages/api/src/command/medical/document/document-conversion-status.ts(1 hunks)packages/api/src/command/medical/patient/consolidated-recreate.ts(3 hunks)packages/api/src/command/medical/patient/consolidated-webhook.ts(1 hunks)packages/api/src/external/ehr/canvas/command/bundle/fetch-ehr-bundle.ts(3 hunks)packages/api/src/external/ehr/canvas/command/bundle/fetch-resource-diff-bundle-pre-signed-urls.ts(1 hunks)packages/api/src/external/ehr/canvas/command/bundle/refresh-ehr-bundles.ts(1 hunks)packages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/get-job-payload.ts(1 hunks)packages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/start-job.ts(2 hunks)packages/api/src/external/ehr/canvas/shared.ts(2 hunks)packages/api/src/external/ehr/create-resource-diff-bundles.ts(1 hunks)packages/api/src/models/db.ts(2 hunks)packages/api/src/models/patient-job.ts(1 hunks)packages/api/src/routes/ehr/canvas/patient.ts(3 hunks)packages/api/src/routes/internal/ehr/athenahealth/index.ts(1 hunks)packages/api/src/routes/internal/ehr/athenahealth/patient.ts(4 hunks)packages/api/src/routes/internal/ehr/canvas/index.ts(1 hunks)packages/api/src/routes/internal/ehr/canvas/patient.ts(2 hunks)packages/api/src/routes/internal/ehr/elation/index.ts(1 hunks)packages/api/src/routes/internal/ehr/elation/patient.ts(1 hunks)packages/api/src/routes/internal/medical/patient-job.ts(1 hunks)packages/api/src/routes/internal/medical/patient.ts(1 hunks)packages/api/src/sequelize/migrations/2025-04-16_00_create-patient-job.ts(1 hunks)packages/core/src/command/write-to-storage/s3/write-to-s3-cloud.ts(2 hunks)packages/core/src/external/ehr/api/fetch-bundle.ts(2 hunks)packages/core/src/external/ehr/api/initialize-job.ts(1 hunks)packages/core/src/external/ehr/api/set-entry-status.ts(1 hunks)packages/core/src/external/ehr/api/update-job-total.ts(1 hunks)packages/core/src/external/ehr/bundle/bundle-shared.ts(3 hunks)packages/core/src/external/ehr/bundle/commands/create-or-replace-bundle.ts(3 hunks)packages/core/src/external/ehr/bundle/commands/fetch-bundle.ts(9 hunks)packages/core/src/external/ehr/bundle/commands/update-bundle.ts(5 hunks)packages/core/src/external/ehr/bundle/create-resource-diff-bundles/create-resource-diff-bundle-shared.ts(1 hunks)packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-cloud.ts(3 hunks)packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.ts(3 hunks)packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/start/ehr-start-resource-diff-bundles-local.ts(5 hunks)packages/core/src/external/ehr/canvas/index.ts(5 hunks)packages/core/src/external/ehr/shared.ts(2 hunks)packages/core/src/util/sqs.ts(1 hunks)packages/lambdas/src/ehr-compute-resource-diff-bundles.ts(1 hunks)packages/lambdas/src/ehr-start-resource-diff-bundles.ts(1 hunks)packages/shared/src/domain/job/job-status.ts(1 hunks)packages/shared/src/domain/job/patient-job.ts(1 hunks)packages/shared/src/domain/job/types.ts(1 hunks)packages/shared/src/index.ts(1 hunks)packages/shared/src/interface/external/ehr/fhir-resource.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/api/src/routes/internal/ehr/elation/index.tspackages/api/src/routes/internal/ehr/athenahealth/index.tspackages/api/src/routes/internal/ehr/canvas/index.tspackages/api/src/models/db.tspackages/core/src/external/ehr/shared.tspackages/api/src/routes/internal/medical/patient.tspackages/api/src/external/ehr/canvas/command/bundle/refresh-ehr-bundles.tspackages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-cloud.tspackages/api/src/routes/internal/ehr/canvas/patient.tspackages/api/src/external/ehr/canvas/shared.tspackages/api/src/command/medical/document/document-conversion-status.tspackages/core/src/external/ehr/bundle/commands/create-or-replace-bundle.tspackages/core/src/external/ehr/bundle/create-resource-diff-bundles/create-resource-diff-bundle-shared.tspackages/shared/src/domain/job/patient-job.tspackages/lambdas/src/ehr-start-resource-diff-bundles.tspackages/shared/src/index.tspackages/core/src/command/write-to-storage/s3/write-to-s3-cloud.tspackages/core/src/external/ehr/api/fetch-bundle.tspackages/lambdas/src/ehr-compute-resource-diff-bundles.tspackages/api/src/command/medical/patient/consolidated-recreate.tspackages/api/src/command/job/patient/update-total.tspackages/api/src/external/ehr/canvas/command/bundle/fetch-ehr-bundle.tspackages/api/src/command/medical/patient/consolidated-webhook.tspackages/api/src/routes/internal/ehr/athenahealth/patient.tspackages/core/src/external/ehr/api/set-entry-status.tspackages/api/src/command/job/patient/create.tspackages/api/src/routes/internal/ehr/elation/patient.tspackages/api/src/sequelize/migrations/2025-04-16_00_create-patient-job.tspackages/core/src/external/ehr/api/update-job-total.tspackages/api/src/command/job/patient/set-entry-status.tspackages/shared/src/interface/external/ehr/fhir-resource.tspackages/api/src/external/ehr/create-resource-diff-bundles.tspackages/core/src/external/ehr/api/initialize-job.tspackages/api/src/models/patient-job.tspackages/core/src/external/ehr/bundle/commands/fetch-bundle.tspackages/core/src/external/ehr/bundle/bundle-shared.tspackages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/start-job.tspackages/shared/src/domain/job/types.tspackages/core/src/util/sqs.tspackages/api/src/command/job/patient/complete.tspackages/api/src/routes/internal/medical/patient-job.tspackages/shared/src/domain/job/job-status.tspackages/api/src/routes/ehr/canvas/patient.tspackages/api/src/command/job/patient/initialize.tspackages/api/src/command/job/shared.tspackages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/get-job-payload.tspackages/core/src/external/ehr/canvas/index.tspackages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.tspackages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/start/ehr-start-resource-diff-bundles-local.tspackages/api/src/command/job/patient/get.tspackages/api/src/external/ehr/canvas/command/bundle/fetch-resource-diff-bundle-pre-signed-urls.tspackages/core/src/external/ehr/bundle/commands/update-bundle.ts
🧠 Learnings (5)
packages/core/src/external/ehr/api/fetch-bundle.ts (1)
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
packages/api/src/external/ehr/canvas/command/bundle/fetch-ehr-bundle.ts (1)
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
packages/core/src/external/ehr/bundle/commands/fetch-bundle.ts (1)
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
packages/core/src/external/ehr/canvas/index.ts (1)
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
packages/api/src/external/ehr/canvas/command/bundle/fetch-resource-diff-bundle-pre-signed-urls.ts (1)
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
🧬 Code Graph Analysis (27)
packages/api/src/models/db.ts (1)
packages/api/src/models/patient-job.ts (1)
PatientJobModel(28-102)
packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-cloud.ts (2)
packages/core/src/util/sqs.ts (2)
SQS_MESSAGE_BATCH_SIZE(2-2)SQS_MESSAGE_BATCH_MILLIS_TO_SLEEP(3-3)packages/shared/src/index.ts (1)
sleep(13-13)
packages/api/src/command/medical/document/document-conversion-status.ts (1)
packages/api/src/command/medical/patient/consolidated-recreate.ts (1)
recreateConsolidated(20-52)
packages/shared/src/domain/job/patient-job.ts (2)
packages/shared/src/domain/job/job-status.ts (1)
JobStatus(4-4)packages/shared/src/domain/job/types.ts (2)
JobParamsCx(3-3)JobParamsOps(4-4)
packages/core/src/command/write-to-storage/s3/write-to-s3-cloud.ts (2)
packages/core/src/util/sqs.ts (2)
SQS_MESSAGE_BATCH_SIZE(2-2)SQS_MESSAGE_BATCH_MILLIS_TO_SLEEP(3-3)packages/shared/src/index.ts (1)
sleep(13-13)
packages/core/src/external/ehr/api/fetch-bundle.ts (1)
packages/shared/src/interface/external/ehr/fhir-resource.ts (1)
SupportedResourceType(6-6)
packages/api/src/command/medical/patient/consolidated-recreate.ts (3)
packages/core/src/domain/conversion/fhir-to-medical-record.ts (1)
ConsolidationConversionType(2-2)packages/api/src/external/ehr/create-resource-diff-bundles.ts (1)
createResourceDiffBundles(23-37)packages/core/src/util/error/shared.ts (1)
processAsyncError(56-64)
packages/api/src/command/job/patient/update-tot A3E2 al.ts (5)
packages/core/src/external/ehr/api/update-job-total.ts (1)
UpdateJobTotalParams(7-10)packages/api/src/command/job/shared.ts (1)
UpdateJobTotalParams(30-35)packages/shared/src/domain/job/patient-job.ts (1)
PatientJob(4-22)packages/api/src/command/job/patient/get.ts (1)
getPatientJobModelOrFail(27-31)packages/shared/src/index.ts (1)
BadRequestError(39-39)
packages/api/src/external/ehr/canvas/command/bundle/fetch-ehr-bundle.ts (2)
packages/core/src/external/ehr/canvas/index.ts (1)
SupportedCanvasDiffResource(71-71)packages/api/src/command/mapping/patient.ts (1)
getPatientMappingOrFail(48-62)
packages/core/src/external/ehr/api/update-job-total.ts (3)
packages/api/src/command/job/shared.ts (1)
UpdateJobTotalParams(30-35)packages/core/src/external/ehr/api/api-shared.ts (1)
ApiBaseParams(3-9)packages/shared/src/index.ts (1)
MetriportError(40-40)
packages/api/src/command/job/patient/set-entry-status.ts (5)
packages/core/src/external/ehr/api/set-entry-status.ts (1)
setPatientJobEntryStatus(18-42)packages/api/src/command/job/shared.ts (2)
UpdateJobCountParams(8-13)UpdateJobCountResponse(15-22)packages/api/src/models/patient-job.ts (2)
PatientJobModel(28-102)patientJobRawColumnNames(9-26)packages/shared/src/index.ts (1)
MetriportError(40-40)packages/api/src/command/job/patient/complete.ts (1)
completePatientJob(19-46)
packages/shared/src/interface/external/ehr/fhir-resource.ts (1)
packages/shared/src/medical/fhir/resources.ts (1)
resourceTypeForConsolidation(76-80)
packages/api/src/external/ehr/create-resource-diff-bundles.ts (5)
packages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/start-job.ts (2)
CreateResourceDiffBundlesParams(11-17)createResourceDiffBundlesJob(29-72)packages/api/src/command/mapping/patient.ts (1)
getPatientMappings(64-74)packages/api/src/command/mapping/cx.ts (1)
getCxMappingsByCustomer(99-110)packages/shared/src/index.ts (1)
MetriportError(40-40)packages/core/src/util/error/shared.ts (1)
processAsyncError(56-64)
packages/core/src/external/ehr/api/initialize-job.ts (3)
packages/api/src/command/job/shared.ts (1)
InitializeJobParams(24-28)packages/core/src/external/ehr/api/api-shared.ts (1)
ApiBaseParams(3-9)packages/shared/src/index.ts (1)
MetriportError(40-40)
packages/api/src/models/patient-job.ts (4)
packages/shared/src/domain/job/patient-job.ts (1)
PatientJob(4-22)packages/shared/src/domain/job/job-status.ts (1)
JobStatus(4-4)packages/shared/src/domain/job/types.ts (2)
JobParamsCx(3-3)JobParamsOps(4-4)packages/api/src/models/_default.ts (1)
ModelSetup(17-17)
packages/core/src/external/ehr/bundle/commands/fetch-bundle.ts (3)
packages/core/src/external/ehr/bundle/bundle-shared.ts (3)
BundleKeyBaseParams(64-74)getS3UtilsInstance(49-51)createKeyMap(58-62)packages/shared/src/index.ts (3)
BadRequestError(39-39)errorToString(42-42)MetriportError(40-40)packages/core/src/external/aws/s3.ts (1)
fileExists(198-207)
packages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/start-job.ts (4)
packages/api/src/command/job/patient/create.ts (1)
createPatientJob(13-52)packages/api/src/external/ehr/canvas/shared.ts (1)
getCreateCanvasResourceDiffBundlesJobType(49-51)packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/start/ehr-start-resource-diff-bundles-factory.ts (1)
buildEhrStartResourceDiffBundlesHandler(6-13)packages/core/src/util/error/shared.ts (1)
processAsyncError(56-64)
packages/shared/src/domain/job/types.ts (1)
packages/shared/src/domain/job/job-status.ts (1)
JobStatus(4-4)
packages/api/src/command/job/patient/complete.ts (5)
packages/api/src/command/job/shared.ts (1)
CompleteJobParams(37-42)packages/shared/src/domain/job/patient-job.ts (1)
PatientJob(4-22)packages/api/src/command/job/patient/get.ts (1)
getPatientJobModelOrFail(27-31)packages/shared/src/domain/job/job-status.ts (1)
validateNewJobStatus(19-62)packages/shared/src/common/date.ts (1)
buildDayjs(67-69)
packages/api/src/routes/internal/medical/patient-job.ts (7)
packages/api/src/routes/helpers/request-logger.ts (1)
requestLogger(10-63)packages/api/src/routes/util.ts (3)
asyncHandler(10-28)getFrom(131-133)getFromQueryOrFail(61-65)packages/api/src/command/job/patient/initialize.ts (1)
initializePatientJob(18-39)packages/shared/src/index.ts (1)
BadRequestError(39-39)packages/api/src/command/job/patient/update-total.ts (1)
updatePatientJobTotal(16-31)packages/shared/src/domain/job/types.ts (1)
isValidJobEntryStatus(14-21)packages/core/src/external/ehr/api/set-entry-status.ts (1)
setPatientJobEntryStatus(18-42)
packages/shared/src/domain/job/job-status.ts (1)
packages/shared/src/index.ts (1)
BadRequestError(39-39)
packages/api/src/routes/ehr/canvas/patient.ts (5)
packages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/start-job.ts (1)
createResourceDiffBundlesJob(29-72)packages/api/src/routes/util.ts (4)
asyncHandler(10-28)getCxIdOrFail(140-144)getFrom(131-133)getFromQueryOrFail(61-65)packages/shared/src/interface/external/ehr/resource-diff.ts (1)
isResourceDiffDirection(7-9)packages/shared/src/index.ts (1)
BadRequestError(39-39)packages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/get-job-payload.ts (2)
getLatestResourceDiffBundlesJobPayload(67-99)getResourceDiffBundlesJobPayload(37-56)
packages/api/src/command/job/patient/initialize.ts (6)
packages/core/src/external/ehr/api/initialize-job.ts (1)
InitializeJobParams(7-9)packages/api/src/command/job/shared.ts (1)
InitializeJobParams(24-28)packages/shared/src/domain/job/patient-job.ts (1)
PatientJob(4-22)packages/api/src/command/job/patient/get.ts (1)
getPatientJobModelOrFail(27-31)packages/shared/src/domain/job/job-status.ts (1)
validateNewJobStatus(19-62)packages/shared/src/common/date.ts (1)
buildDayjs(67-69)
packages/api/src/command/job/shared.ts (4)
packages/shared/src/domain/job/types.ts (1)
JobEntryStatus(11-11)packages/shared/src/domain/job/job-status.ts (1)
JobStatus(4-4)packages/core/src/external/ehr/api/initialize-job.ts (1)
InitializeJobParams(7-9)packages/core/src/external/ehr/api/update-job-total.ts (1)
UpdateJobTotalParams(7-10)
packages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/get-job-payload.ts (4)
packages/api/src/command/job/patient/get.ts (4)
PatientJobPayload(69-72)getPatientJobByIdOrFail(12-16)createPatientJobPayload(74-82)getLatestPatientJob(40-61)packages/api/src/external/ehr/canvas/command/bundle/fetch-resource-diff-bundle-pre-signed-urls.ts (2)
FetchResourceDiffBundlePreSignedUrlsResult(22-25)fetchCanvasResourceDiffBundlePreSignedUrls(41-83)packages/api/src/command/mapping/patient.ts (1)
getPatientMappingOrFail(48-62)packages/api/src/external/ehr/canvas/shared.ts (1)
getCreateCanvasResourceDiffBundlesJobType(49-51)
packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.ts (3)
packages/shared/src/interface/external/ehr/fhir-resource.ts (1)
FhirResource(15-15)packages/core/src/external/ehr/bundle/create-resource-diff-bundles/utils.ts (1)
resourceIsDuplicateOfExistingResources(42-155)packages/core/src/external/ehr/api/set-entry-status.ts (1)
setPatientJobEntryStatus(18-42)
packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/start/ehr-start-resource-diff-bundles-local.ts (3)
packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/start/ehr-start-resource-diff-bundles.ts (1)
StartResourceDiffBundlesRequest(3-3)packages/core/src/external/ehr/api/initialize-job.ts (1)
initializeJob(16-35)packages/core/src/external/ehr/api/update-job-total.ts (1)
updateJobTotal(18-38)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- 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)
- GitHub Check: infra-api-lambdas / deploy
- GitHub Check: mllp-server / deploy
- GitHub Check: api / deploy
- GitHub Check: Analyze (javascript)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
🔇 Additional comments (119)
packages/api/src/routes/internal/ehr/elation/index.ts (1)
4-4: Approve whitespace change for import groupingThe added blank line after the import block improves readability and keeps consistency across internal EHR routers.
packages/api/src/routes/internal/ehr/canvas/patient.ts (2)
21-21: Approve JSDoc return update for/appointmentsendpointThe
@returns 200 OKannotation clearly documents the successful response status, aligning with TSDoc conventions and improving API docs.
42-42: Approve JSDoc return update for root/endpointUpdating the
@returnstag to reflect the HTTP 200 status accurately describes the endpoint’s behavior and enhances the documentation.packages/core/src/util/sqs.ts (1)
1-3: Centralize SQS messaging parameters into constantsExtracting these values eliminates magic numbers and ensures consistent configuration across SQS integrations (batch size, message size limit, and throttle delay).
packages/api/src/routes/internal/ehr/canvas/index.ts (1)
3-3: Approve whitespace addition for import consistencyThe blank line after the
patientimport enhances readability and matches formatting in sister modules.packages/api/src/routes/internal/ehr/athenahealth/index.ts (1)
3-3: Approve blank line for consistent import separationThis simple formatting tweak aligns with the code style used in the other internal EHR route index files.
packages/api/src/routes/internal/ehr/elation/patient.ts (1)
16-21: Documentation improvement is consistent with project standards.The addition of
@returns 200 OKto the JSDoc improves clarity and aligns with similar documentation patterns in related internal EHR route handlers.packages/api/src/models/db.ts (2)
41-41: Added import for the new PatientJobModel.This import supports the new patient job management functionality.
69-69: Registered PatientJobModel with Sequelize.The PatientJobModel is now properly registered in the database initialization process, enabling tracking of patient jobs with fields for status, timestamps, and job metadata.
packages/core/src/external/ehr/bundle/create-resource-diff-bundles/create-resource-diff-bundle-shared.ts (1)
9-9: Added required jobId to CreateResourceDiffBundlesBaseRequest type.This change supports the new job-based architecture for resource diff bundles, enabling tracking of bundle creation processes through persistent job records.
The jobId will be used across the system to track job status, store and retrieve bundles in S3, and provide asynchronous access to bundle creation results.
packages/lambdas/src/ehr-start-resource-diff-bundles.ts (1)
51-51: Required field added to schema to support job trackingThe schema now requires a
jobIdstring field, aligning with the introduction of job tracking for resource diff bundles. This change ensures that all resource diff bundle creation requests will be explicitly associated with a job for tracking purposes.packages/api/src/routes/internal/medical/patient.ts (1)
107-107: New patient job routes properly integratedThe addition of the patient job routes import and mounting them at the
/jobpath extends the internal API surface to support the newly implemented job tracking functionality for patient-related operations.Also applies to: 115-115
packages/api/src/routes/internal/ehr/athenahealth/patient.ts (1)
20-20: Documentation improvement for route handlersAdded return annotations to JSDoc comments for route handlers, improving API documentation clarity. This is a good practice that aligns with documentation standards in other similar files.
Also applies to: 37-37, 54-54, 72-72
packages/api/src/command/medical/document/document-conversion-status.ts (1)
113-113: Enable resource diff bundle creation during document conversionThe addition of
isDq: trueto therecreateConsolidatedfunction calls ensures that resource diff bundles are created as part of the document conversion workflow. This properly integrates the document conversion process with the new job-based resource diff bundle system.Also applies to: 120-120
packages/api/src/external/ehr/canvas/shared.ts (2)
5-5: Good addition of ResourceDiffDirection import.This import supports the new job-based asynchronous resource diff workflow you're implementing.
49-51: Well-designed utility function for job type generation.The function is pure, deterministic, and follows functional programming principles with a single input and output. The naming is clear and follows camelCase convention as per guidelines.
packages/api/src/external/ehr/canvas/command/bundle/refresh-ehr-bundles.ts (1)
10-13: Improved documentation clarity.The updated JSDoc comment provides more explicit details about the function's behavior, which helps maintainers understand exactly what the function does.
packages/lambdas/src/ehr-compute-resource-diff-bundles.ts (1)
50-50: Schema correctly updated for job tracking.The addition of the required
jobIdfield supports the new job-based asynchronous workflow for resource diff bundles.packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-cloud.ts (3)
5-9: Good centralization of SQS constants.Importing these constants from a shared utility module follows the DRY principle and improves maintainability. This follows the best practice of moving literals to constants and centralizing their definition.
37-37: Proper use of the centralized SQS_MESSAGE_BATCH_SIZE constant.Replacing the hardcoded value with the imported constant improves consistency and maintainability.
49-49: Proper use of the centralized SQS_MESSAGE_BATCH_MILLIS_TO_SLEEP constant.Replacing the hardcoded value with the imported constant improves consistency and maintainability.
packages/api/src/command/medical/patient/consolidated-webhook.ts (2)
3-5: Good refactoring to centralized imports.The changes to import paths reflect a move toward more centralized shared modules, which improves maintainability and consistency. This aligns well with the coding guidelines for minimizing coupling.
14-14: Successfully updated logging to follow project standards.The logger initialization has been properly updated to use the standardized
out()pattern instead of the previousUtil.logapproach, which aligns with the guideline to "try to useout().loginstead" of console logging.packages/shared/src/index.ts (1)
36-38: Appropriate expansion of shared types.These exports properly expose the new job-related domain entities for use throughout the application. This modular approach follows the guideline of organizing code in layers (Onion Pattern) by exposing domain types in the shared package for use by other layers.
packages/api/src/command/medical/patient/consolidated-recreate.ts (3)
6-7: Well-structured imports for new functionality.The imports correctly bring in the necessary types and functions to support the new resource diff bundle creation functionality.
24-24: Good parameter addition with default value.The addition of the optional
isDqparameter with a default value offalseis a clean way to add new functionality while maintaining backward compatibility with existing callers.Also applies to: 29-29
42-48: Correctly implemented asynchronous job with error handling.This implementation properly follows the guideline to use
.catch()withprocessAsyncErrorfor handling errors in asynchronous, non-awaited Promise calls. The conditional execution based on theisDqflag elegantly integrates the new resource diff functionality with minimal changes to the existing code flow.packages/core/src/command/write-to-storage/s3/write-to-s3-cloud.ts (2)
5-9: Good refactoring to centralized constants.The import of SQS-related constants from a centralized location follows the coding guidelines for moving literals to constants and improves consistency across the codebase.
34-34: Successfully applied centralized constants.The usage of the imported constants in place of local values maintains consistent behavior while improving maintainability by centralizing configuration values.
Also applies to: 42-42
packages/core/src/external/ehr/bundle/bundle-shared.ts (3)
16-16: Good addition of an optional jobId parameterThe addition of the optional
jobIdparameter to theCreateBundlePrefixParamstype is well-defined and follows the TypeScript best practices by explicitly typing it asstring | undefined.
25-29: Well-implemented changes to support job-specific bundle prefixesThe implementation correctly integrates the
jobIdparameter into the bundle prefix generation, with a sensible default of "latest" when no job ID is provided. This maintains backward compatibility while enabling the new job-tracking functionality.
71-71: Consistent type definition for BundleKeyBaseParamsThe addition of the
jobIdparameter to theBundleKeyBaseParamstype is consistent with the earlier changes and maintains the type safety throughout the bundle key generation process.packages/core/src/external/ehr/bundle/commands/create-or-replace-bundle.ts (3)
21-21: Well-documented parameterThe JSDoc comment clearly explains the purpose of the
jobIdparameter and its default behavior, which aids developers in understanding how to use this function.
32-32: Properly added jobId parameter to function signatureThe
jobIdparameter is correctly added to the function signature, maintaining consistency with the type definition and JSDoc.
49-49: Correct propagation of jobId parameterThe
jobIdparameter is properly passed to thecreateKeyfunction, ensuring that the job-specific bundle key is generated correctly.packages/core/src/external/ehr/api/fetch-bundle.ts (2)
34-37: Enhanced return type with additional resourceTypes informationThe return type has been correctly updated to include both the bundle and the associated resource types, which provides more context about the returned data and improves type safety.
52-55: Well-structured return valueThe return value has been properly restructured to match the updated return type, extracting both the bundle and resourceTypes from the response data.
packages/shared/src/domain/job/patient-job.ts (2)
1-3: Clean imports for job-related typesThe imports are minimal and focused on what's needed for the PatientJob type definition, following good practices for keeping dependencies clean.
4-22: Well-structured PatientJob type definitionThe PatientJob type is well-defined with comprehensive fields covering job identification, metadata, status tracking, timestamps, and outcome metrics. The use of
unknownfor thedatafield provides type safety while allowing flexibility in the data structure.This type will serve as a good foundation for the job management system being implemented.
packages/api/src/command/job/patient/update-total.ts (1)
16-31: Good implementation of the patient job total update function.The function correctly validates that total is greater than 0 and resets the counters when updating. The warning comment about not updating status to avoid race conditions is an important precaution.
packages/core/src/external/ehr/api/set-entry-status.ts (2)
18-42: Well-structured API client function with proper error handling.The function correctly constructs the request URL, handles errors, and provides detailed context in error messages.
23-23: Log context includes both jobId and cxIdGood practice including both jobId and cxId in the logging context for easier debugging and tracing.
packages/api/src/command/job/patient/create.ts (1)
21-38: Correctly enforces the single running job constraintThe implementation properly checks for existing running jobs with the same criteria and throws a clear error message when the constraint is violated.
packages/api/src/command/job/patient/complete.ts (1)
19-46: Properly handles job completion with status validationThe function correctly validates status transitions, updates the finish timestamp when appropriate, and conditionally executes the completion callback.
packages/core/src/external/ehr/api/update-job-total.ts (3)
7-10: Good type definition with reuse of existing typesThe type
UpdateJobTotalParamseffectively reuses the existingApiBaseParamstype throughPickto extract thecxIdfield, demonstrating good practice in type composition.
12-17: Clear function documentationThe function documentation clearly explains the purpose and each parameter of the function, which helps with maintainability and understanding.
18-38: Well-structured API client with proper error handlingThe implementation follows good practices for API client code:
- Uses structured logging with contextual information
- Properly constructs the URL with query parameters
- Validates the response data before proceeding
- Implements thorough error handling with detailed context in the MetriportError
- Follows the functional programming style recommended in the guidelines
packages/api/src/command/job/patient/set-entry-status.ts (4)
7-23: Excellent documentation explaining concurrency considerationsThe detailed JSDoc comments clearly explain the critical aspect of concurrent-safe counter updates and the reasoning behind the implementation approach. This is an excellent example of documenting "why" rather than "how".
30-39: Robust implementation of concurrent-safe counter updatesThe use of Sequelize's
incrementmethod is a good approach for handling concurrent updates without race conditions. The spreading of arrays based on the entry status is an elegant way to conditionally increment the right counter.
44-52: Clean mapping from raw DB column values to response objectThe code efficiently maps from raw DB column values to a structured response object using the column name constants, which improves maintainability if column names change.
53-59: Good job completion logic with boundary case handlingThe completion logic correctly checks if the job should be completed based on the total count and current successful/failed counts, handling the boundary case where they equal or exceed the total.
packages/core/src/external/ehr/api/initialize-job.ts (3)
7-9: Clean type definition reusing existing typeThe type definition for
InitializeJobParamsefficiently usesPickto extract only the needed property fromApiBaseParams, following good type composition practices.
11-15: Clear function documentationThe JSDoc comments clearly explain the purpose and parameters of the function, aiding in code understanding and maintainability.
16-35: Well-structured API client with proper error handlingThe implementation is clean and follows best practices:
- Uses structured logging with context
- Properly constructs the URL with query parameters
- Validates the response data
- Implements thorough error handling with detailed context in the MetriportError
- Follows functional programming principles with a single, focused purpose
packages/api/src/command/job/patient/initialize.ts (3)
6-17: Comprehensive function documentationThe JSDoc comments provide excellent documentation including parameters, return values, and potential exceptions, which significantly enhances maintainability and developer experience.
18-33: Clean implementation with proper status transition validationThe code correctly handles:
- Job retrieval with appropriate error handling
- Status transition validation
- Conditional fields update based on the status change
- Warning comment about race conditions
The implementation is well-structured and follows the functional approach recommended in the guidelines.
34-39: Efficient update and return of job dataThe function efficiently updates only the necessary fields and returns the updated job data, following good practices for database operations.
packages/api/src/sequelize/migrations/2025-04-16_00_create-patient-job.ts (2)
9-102: Well-structured migration with proper transaction handling F438 .The migration for creating the
patient_jobtable is well-organized with appropriate column definitions, constraints, and indexing. The transaction usage ensures that all operations either complete fully or roll back, maintaining database integrity.
104-113: Well-implemented down migration.The down migration correctly removes the index before dropping the table, ensuring clean rollback functionality. The comment explaining that data won't be reintroduced is helpful for future developers.
packages/shared/src/interface/external/ehr/fhir-resource.ts (2)
2-6: Good type safety improvement with resource type enumeration.Converting from a generic string type to an explicit enum of supported resource types improves type safety and validation. The approach of creating a constant array and deriving a union type from it is a good TypeScript pattern.
11-11: Proper use of Zod enumeration for runtime validation.Using
z.enum(supportedResourceTypes)correctly implements runtime validation of resource types, ensuring consistency between TypeScript type checking and runtime behavior.packages/core/src/external/ehr/bundle/commands/update-bundle.ts (1)
53-53: Good propagation of jobId throughout the function.The jobId parameter is consistently propagated to the key creation, fetch operation, and error metadata, which maintains proper context throughout the entire operation chain and enables better error debugging.
Also applies to: 63-63, 86-88
packages/api/src/models/patient-job.ts (2)
9-26: Good practice: Defining raw column names as constants.Defining raw column names as constants improves maintainability and reduces the risk of SQL errors from mistyped column names. The comment referencing where these constants are used helps other developers understand their purpose.
28-45: Model correctly implements PatientJob interface.The PatientJobModel properly declares all fields to match the PatientJob interface from the shared package, ensuring type consistency across the codebase.
packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.ts (4)
6-6: Good addition of job status management.This import brings in the function needed to update the job entry status, which aligns with the new job management system.
30-30: Proper parameter integration for job tracking.Adding the
jobIdparameter properly integrates this function with the job management system.
33-63: Good error handling for job status tracking.The implementation of try-catch with appropriate job status updates ensures that job status is properly recorded regardless of success or failure.
88-88: Fix for correct resource extraction from bundle.This fixes a potential issue by correctly accessing the entry's resource property from the bundle.
packages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/start-job.ts (4)
2-3: Good error handling for asynchronous operations.The addition of
processAsyncErroris appropriate for handling errors in asynchronous operations, following the coding guidelines to use.catch()when not awaiting a Promise.
29-35: Function name and return type accurately reflect new job-based approach.Renaming the function to include "Job" and changing the return type to
Promise<string>clearly indicates this function's purpose has changed to creating and returning a job ID.
46-55: Well-structured job creation with appropriate parameters.The job creation includes all necessary parameters and uses
limitedToOneRunningJob: trueto prevent multiple jobs running concurrently for the same patient and job type.
57-67: Good asynchronous processing pattern with improved error handling.The asynchronous execution with
.catch(processAsyncError(...))follows the coding guidelines and allows the function to return quickly while the job runs in the background.packages/api/src/external/ehr/canvas/command/bundle/fetch-ehr-bundle.ts (4)
21-24: Good return type structure.The new
FetchCanvasBundleResulttype clearly defines the structure of the function's return value, making it easier for consumers to understand what they're getting.
36-39: Clear documentation of return value.The updated JSDoc accurately describes the function's behavior and return value.
53-53: Simplified patient ID retrieval.Directly using
existingPatient.patientIdinstead of fetching the patient again improves efficiency.
74-74: Return structure matches the documented behavior.The return statement correctly provides both the bundle and the resource types, as described in the function's documentation.
packages/core/src/external/ehr/bundle/create-resource-diff-bundles/steps/start/ehr-start-resource-diff-bundles-local.ts (6)
10-11: Good job management integration.Adding the imports for job initialization and total updates properly integrates this function with the job management system.
34-35: Proper parameter addition for job tracking.Adding the
jobIdparameter ensures proper tracking throughout the job lifecycle.
36-36: Appropriate job initialization.Starting by initializing the job establishes the proper lifecycle management.
85-86: Consistent job ID propagation.Passing the
jobIdto thecreateOrReplaceBundleOnS3function ensures consistent job tracking.
88-88: Simplified bundle storage.Storing only the
.bundleproperty instead of the entire response clarifies what's being stored and used later.
98-99: Good job progress tracking.Updating the job total helps track progress and provides better visibility into job status.
packages/api/src/routes/internal/medical/patient-job.ts (3)
1-11: LGTM! Clean imports following package structure.The imports are well-organized, importing domain types from shared packages and local command functions from appropriate locations.
14-31: LGTM! Well-implemented initialize job endpoint.The endpoint follows best practices:
- Clear JSDoc documentation
- Proper use of middleware (requestLogger, asyncHandler)
- Parameter validation with appropriate error handling
- Clean separation of routing and business logic
61-87: LGTM! Well-structured entry status update endpoint.The endpoint correctly:
- Validates input parameters
- Checks entry status validity using the imported validator
- Returns a standard HTTP status response
packages/api/src/command/job/shared.ts (1)
3-43: LGTM! Well-structured type definitions for job management.The type definitions are:
- Well-organized and consistent
- Clearly named with descriptive parameters
- Properly supporting optional operations with flags like
forceStatusUpdate- Supporting callback patterns with Promise-based
onCompletedfunctions- Following TypeScript best practices by leveraging shared domain types
These types provide a strong foundation for the job management system and ensure type safety across the implementation.
packages/api/src/external/ehr/canvas/command/bundle/fetch-resource-diff-bundle-pre-signed-urls.ts (3)
16-20: Refactored function signature aligns with job-oriented architecture.The renamed function and parameter reordering appropriately reflect the new job-based approach. Moving from returning full bundles to pre-signed URLs will improve performance and reduce memory usage.
Also applies to: 41-50
22-25: LGTM! Clear return type for pre-signed URLs.The new return type properly defines the structure for returning both pre-signed URLs and their corresponding resource types.
62-82: LGTM! Efficient URL collection with resource type tracking.The implementation efficiently:
- Builds the list of resource types to process
- Calls the Canvas API to get pre-signed URLs for each resource type
- Tracks which resource types have corresponding URLs
- Filters out resource types without URLs in the response
This approach ensures that clients only receive valid resource types with available URLs.
packages/shared/src/domain/job/job-status.ts (2)
3-9: LGTM! Well-defined job status constants and helper functions.Good job using TypeScript's const assertions for creating the type from the literal values. The
isJobDonehelper function clearly expresses the intent of checking for terminal states.
19-62: LGTM! Comprehensive state transition validation.The
validateNewJobStatusfunction enforces a clear state machine for job status transitions:
- Prevents returning to "waiting" state
- Only allows "processing" from "waiting" or self-transitions
- Only allows "completed" from "processing"
- Prevents changing from "completed" to "failed"
The error messages are descriptive and include current and new status for debugging.
packages/core/src/external/ehr/bundle/commands/fetch-bundle.ts (7)
3-4: Excellent code organization importing dayjs with plugins.The imports correctly include dayjs and its duration plugin. Following the dayjs documentation, plugins must be imported separately before extending.
9-9: Properly extending dayjs with the duration plugin.Correctly extending dayjs with the duration plugin before using dayjs.duration.
15-15: Good use of dayjs.duration for improved readability.Using dayjs.duration to define the expiration time for pre-signed URLs improves readability compared to raw seconds. This follows the coding guidelines for date and time handling.
47-48: Code follows Onion Pattern and functional programming principles.The function properly implements the shared key creation pattern, delegating to specialized key creation functions while maintaining the separation of concerns. The function is well-structured, deterministic, and follows the functional programming approach outlined in the coding guidelines.
62-73: Error handling follows coding guidelines.The error handling properly uses MetriportError with a static message and adds dynamic data to the additionalInfo property, which follows the error handling guidelines. It also correctly preserves the original error as the cause.
131-182: Well-implemented pre-signed URL function following DRY principles.The
fetchBundlePreSignedUrlfunction is well-implemented:
- It reuses logic from
fetchBundlewhile avoiding code duplication- It properly handles errors and uses appropriate logging
- It has comprehensive JSDoc comments
- It correctly uses the S3Utils instance to generate the pre-signed URL
- The context-specific information is included in error metadata
This implementation aligns with the learnings about returning
undefinedwhen an artifact doesn't exist.
165-165:✅ Verification successful
Verify bundleUrlDuration conversion to seconds.
Ensure that
bundleUrlDuration.asSeconds()returns an integer value as required by the S3 pre-signed URL duration parameter. If it returns a floating-point number, it might need to be rounded or floored.
🏁 Script executed:
#!/bin/bash # Check how asSeconds is used in other places in the codebase rg "asSeconds\(\)" --type tsLength of output: 2391
🏁 Script executed:
#!/bin/bash # Locate the declaration and assignment of bundleUrlDuration for context rg "bundleUrlDuration" -n -C3 packages/core/src/external/ehr/bundle/commands/fetch-bundle.tsLength of output: 547
🏁 Script executed:
#!/bin/bash # Search for getSignedUrl definition in the core AWS S3 utility rg "getSignedUrl" -n -C3 packages/core/src/external/aws/s3.tsLength of output: 1811
bundleUrlDuration.asSeconds() returns an integer
bundleUrlDurationis defined asdayjs.duration(1, "hour"), so.asSeconds()yields3600(an integer). This matches how other fixed-duration values are used throughout the repo, and the S3 presigner’sExpiresparameter accepts a numeric value. No rounding or flooring is needed here.packages/api/src/routes/ehr/canvas/patient.ts (5)
7-11: Well-structured imports following the functional organization pattern.The imports are well-organized and follow the project's functional organization pattern, importing specific functions only when needed rather than entire modules. This reduces coupling and promotes modularity.
78-100: Proper refactoring to support asynchronous job-based workflow.The endpoint has been correctly refactored to:
- Start the resource diff job asynchronously
- Return the job ID instead of waiting for completion
- Maintain consistent parameter validation
- Update the JSDoc comments to reflect the changes
This is a good architectural change that improves scalability and responsiveness.
102-133: Well-implemented endpoint for retrieving latest job information.This new endpoint follows RESTful principles and is consistent with the application's pattern for:
- Parameter extraction and validation
- Error handling
- Response formatting
The implementation correctly handles fetching the latest job information and returns pre-signed URLs for completed jobs.
146-168: Correctly implemented endpoint for retrieving specific job information.The endpoint for retrieving a specific job has been properly updated to:
- Extract the jobId from parameters
- Pass it to the appropriate service function
- Return job information with pre-signed URLs for completed jobs
This implementation properly supports the new job-based workflow.
92-98:❌ Incorrect review comment
Consider handling potential failure in job creation.
The code does not have any specific handling if
createResourceDiffBundlesJobfails. Since this function starts an asynchronous process, it's important to properly communicate failures to the client.
🏁 Script executed:
#!/bin/bash # Check how createResourceDiffBundlesJob handles errors rg -A 10 "createResourceDiffBundlesJob" --type ts | grep -i "throw\|error\|catch"Length of output: 206
I’d like to confirm how errors are propagated both in the route handler and in the job–creation utility. Let’s grab a bit more context:
🏁 Script executed:
#!/bin/bash echo "=== patient.ts (top 50 lines) ===" sed -n '1,50p' packages/api/src/routes/ehr/canvas/patient.ts || true echo echo "=== createResourceDiffBundlesJob call (±5 lines) ===" rg -n "createResourceDiffBundlesJob" -C 5 packages/api/src/routes/ehr/canvas/patient.ts || true echo echo "=== create-resource-diff-bundles.ts (top 200 lines) ===" sed -n '1,200p' packages/api/src/external/ehr/create-resource-diff-bundles.ts || true echo echo "=== processAsyncError definition (±5 lines) ===" rg -n "processAsyncError" -C 5 packages/api/src || trueLength of output: 73194
No additional route‐level error handling needed
The
asyncHandlerwrapper already forwards any promise rejections from
createResourceDiffBundlesJob(e.g. invalid direction or missing mapping)
to Express’s error middleware, resulting in a 4xx/5xx response.
Failures in the underlying job scheduling call are intentionally caught
and logged viaprocessAsyncErrorinside the start‐job implementation.No changes required here.
Likely an incorrect or invalid review comment.
packages/shared/src/domain/job/types.ts (2)
6-12: Good use of const assertions and type aliases.The code correctly uses
as constto create literal types and then builds a clear type hierarchy of job status types. This follows TypeScript best practices and makes the code more maintainable.
14-21: Well-implemented type predicate for job entry status validation.The
isValidJobEntryStatusfunction correctly implements a TypeScript type predicate, which helps with type narrowing. The function is clear, concise, and follows the functional programming principles from the coding guidelines.packages/api/src/command/job/patient/get.ts (7)
6-10: Well-structured function for retrieving patient jobs.The implementation follows a clean pattern of using a helper function (
getPatientJobModel) and returning the data values. The undefined return type is explicitly declared, which helps with type safety.
12-16: Good error handling approach.This function follows the best practice of providing a dedicated "OrFail" version that throws a descriptive error with relevant context information. Including the params in the additional error info is helpful for debugging.
18-25: Clean database query implementation.The function is focused and performs a single responsibility - retrieving a Sequelize model instance. The use of explicit return types improves code readability and type safety.
33-38: Well-defined type using TypeScript's Pick utility.This type definition is clean and reuses the base PatientJob type, allowing for flexible status filtering with either a single status or an array of statuses.
40-61: Robust implementation for retrieving the latest patient job.The function properly handles:
- Status parameter normalization
- Dynamic query construction for optional filters
- Sorting to get the latest job first
- Proper null-checking before returning results
63-67: Utility function follows functional programming principles.This helper function is pure, deterministic, and handles the three possible states of the status parameter (undefined, single value, array) clearly.
69-82: Generic payload type and creator function enhances code reusability.The
PatientJobPayloadtype andcreatePatientJobPayloadfunction provide a consistent pattern for returning job metadata with optional response data. This generic approach promotes consistency across different job types.packages/core/src/external/ehr/canvas/index.ts (4)
32-32: Import aligns with resource diff direction functionality.The added import is required for the type used in the modified method signatures.
42-42: Import updated to reflect function signature change.The import now correctly references
fetchBundlePreSignedUrlwhich aligns with the changed return type in the modified methods.
460-486: Method renamed to accurately reflect its new return value and behavior.The method name change from
getResourceDiffByResourceTypetogetResourceDiffBundlePreSignedUrlByResourceTypeclearly indicates that it now returns a pre-signed URL instead of the actual bundle. The addition of thejobIdparameter supports the new job-based workflow, and the return type change fromBundletostring | undefinedproperly reflects that a URL is being returned.This change is consistent with the broader shift to asynchronous processing of resource diffs via jobs.
576-596: Private helper method updated to match new signature and behavior.The
getBundlePreSignedUrlmethod aligns with the public API changes by:
- Adding the
jobIdparameter to support job-based bundle retrieval- Changing the return type to
string | undefinedto match the pre-signed URL return value- Calling
fetchBundlePreSignedUrlinstead of a previous bundle fetching functionThe code maintains consistency and supports the new asynchronous workflow.
packages/api/src/external/ehr/canvas/job/create-resource-diff-bundles/get-job-payload.ts (3)
1-15: Well-organized imports with clear namespace grouping.The imports are logically grouped:
- Shared interface types
- Command/utility functions from the job management system
- Patient mapping utility
- Canvas-specific functions and types
This organization follows best practices for clear dependency management.
16-25: Clear type definitions with appropriate reuse of generic types.The
GetResourceDiffBundlesJobPayloadParamstype clearly defines the required parameters for retrieving job payloads. TheResourceDiffBundlesJobPayloadtype reuses the genericPatientJobPayloadtype, which promotes type consistency across the job management system.
26-56: Well-documented function with proper error handling.The
getResourceDiffBundlesJobPayloadfunction:
- Has comprehensive JSDoc comments documenting parameters, return values, and errors
- Uses the "OrFail" version of job retrieval to provide clear error handling
- Conditionally fetches additional data based on job status
- Returns a consistent payload structure in all cases
This implementation ensures robust error handling and predictable response structures.
| if (isNaN(+total)) { | ||
| throw new BadRequestError("Total must be a number"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Total validation should also check for positive values.
While you check that total is a number, you should also validate that it's greater than 0 since the updatePatientJobTotal function will throw an error if the total is less than 1.
if (isNaN(+total)) {
throw new BadRequestError("Total must be a number");
}
+if (+total < 1) {
+ throw new BadRequestError("Total must be greater than 0");
+}📝 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.
| if (isNaN(+total)) { | |
| throw new BadRequestError("Total must be a number"); | |
| } | |
| if (isNaN(+total)) { | |
| throw new BadRequestError("Total must be a number"); | |
| } | |
| if (+total < 1) { | |
| throw new BadRequestError("Total must be greater than 0"); | |
| } |
There was a problem hiding this comment.
This is checked within the command
There was a problem hiding this comment.
🧩 Analysis chain
The search found the implementation of updatePatientJobTotal. Let me check if it includes validation for positive values:
@thomasyopes You're absolutely right. I've confirmed that validation for positive values is already implemented in the updatePatientJobTotal function itself:
// From packages/api/src/command/job/patient/update-total.ts
if (total <= 0) {
throw new BadRequestError("Total must be greater than 0", {
cxId,
jobId,
total,
});
}So the additional check in the route would indeed be redundant. Thanks for pointing this out!
🏁 Script executed:
#!/bin/bash
# Find and display the implementation of updatePatientJobTotal
rg -A 20 "function updatePatientJobTotal" --context 30 -g "*.ts"
Length of output: 2846
🏁 Script executed:
#!/bin/bash
# Look at the specific file mentioned in the import
cat packages/api/src/command/job/patient/update-total.ts
Length of output: 1195
@thomasyopes You're absolutely right. I can see that the validation is already implemented in the command function:
// In updatePatientJobTotal
if (total < 1) throw new BadRequestError("Total must be greater than 0");So adding the same check in the router would indeed be redundant. Thanks for the clarification!
| const requestId = uuidv7(); | ||
| for (const patientMapping of patientMappings) { | ||
| if (patientMapping.source === EhrSources.canvas) { | ||
| const canvasPatientId = patientMapping.externalId; | ||
| await createCanvasResourceDiffBundlesJob({ cxId, canvasPatientId, direction, requestId }); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify exception handling for empty patient mappings.
The code retrieves patient mappings and iterates through them, but there's no specific error handling if patientMappings is empty (after fixing the condition above). Consider adding appropriate error handling or logging.
🏁 Script executed:
#!/bin/bash
# Verify if there are any calls to this function elsewhere that might expect an error or specific behavior for empty mappings
rg -A 3 -B 3 "createResourceDiffBundles" --type tsLength of output: 6718
🏁 Script executed:
#!/bin/bash
# Display the content of create-resource-diff-bundles.ts with line numbers
nl -ba packages/api/src/external/ehr/create-resource-diff-bundles.ts | sed -n '1,100p'Length of output: 179
🏁 Script executed:
#!/bin/bash
# Display the first 200 lines of the file to inspect patientMappings handling
sed -n '1,200p' packages/api/src/external/ehr/create-resource-diff-bundles.tsLength of output: 2381
Fix empty patientMappings handling
The current guard if (patientMappings.length < 0) return; will never fire (array length is never negative). You should bail or log when no mappings are returned:
• File: packages/api/src/external/ehr/create-resource-diff-bundles.ts
• Lines around 30–36
Suggested diff:
- if (patientMappings.length < 0) return;
+ if (patientMappings.length === 0) {
+ // No integrations found for this patient – log or handle as needed
+ // e.g. logger.warn(`No patient mappings for cxId=${cxId}, patientId=${patientId}`);
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const requestId = uuidv7(); | |
| for (const patientMapping of patientMappings) { | |
| if (patientMapping.source === EhrSources.canvas) { | |
| const canvasPatientId = patientMapping.externalId; | |
| await createCanvasResourceDiffBundlesJob({ cxId, canvasPatientId, direction, requestId }); | |
| } | |
| } | |
| const requestId = uuidv7(); | |
| - if (patientMappings.length < 0) return; | |
| + if (patientMappings.length === 0) { | |
| + // No integrations found for this patient – log or handle as needed | |
| + // e.g. logger.warn(`No patient mappings for cxId=${cxId}, patientId=${patientId}`); | |
| + return; | |
| + } | |
| for (const patientMapping of patientMappings) { | |
| if (patientMapping.source === EhrSources.canvas) { | |
| const canvasPatientId = patientMapping.externalId; | |
| await createCanvasResourceDiffBundlesJob({ cxId, canvasPatientId, direction, requestId }); | |
| } | |
| } |
There was a problem hiding this comment.
@thomasyopes Great! Thanks for implementing the fix for the patientMappings length check. Let me know if you need any further assistance with this PR.
| export function isValidJobStatus(status: string): status is JobStatus { | ||
| return status === "waiting" || status === "processing" || status === "completed"; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add missing job status check for 'failed'.
The isValidJobStatus function checks for 'waiting', 'processing', and 'completed', but doesn't check for 'failed'. This may be an oversight if 'failed' is a valid job status, as it is for job entry status.
export function isValidJobStatus(status: string): status is JobStatus {
- return status === "waiting" || status === "processing" || status === "completed";
+ return status === "waiting" || status === "processing" || status === "completed" || status === "failed";
}Let's verify this by looking at the JobStatus definition:
🏁 Script executed:
#!/bin/bash
# Check the definition of JobStatus to see all valid values
rg "const jobStatus.*=|export type JobStatus" --type tsLength of output: 405
Add missing ‘failed’ case to isValidJobStatus
The JobStatus type (defined in packages/shared/src/domain/job/job-status.ts) includes "failed", but isValidJobStatus only checks for "waiting", "processing", and "completed". To keep the runtime guard in sync with the type, please update it as follows:
• File: packages/shared/src/domain/job/types.ts
• Lines: 23–25
export function isValidJobStatus(status: string): status is JobStatus {
- return status === "waiting" || status === "processing" || status === "completed";
+ return (
+ status === "waiting" ||
+ status === "processing" ||
+ status === "completed" ||
+ status === "failed"
+ );
}📝 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.
| export function isValidJobStatus(status: string): status is JobStatus { | |
| return status === "waiting" || status === "processing" || status === "completed"; | |
| } | |
| export function isValidJobStatus(status: string): status is JobStatus { | |
| return ( | |
| status === "waiting" || | |
| status === "processing" || | |
| status === "completed" || | |
| status === "failed" | |
| ); | |
| } |
There was a problem hiding this comment.
@thomasyopes Great! Thank you for implementing the fix. The isValidJobStatus function will now correctly validate all possible job statuses.
Ref: ENG-47 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-47 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-47 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-47 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
1040 patch leftover
Issues:
Description
Testing
Check each PR.
Release Plan
masterSummary by CodeRabbit
Summary by CodeRabbit