Project: workspace-doctolib · Branch: HEAD · Mode: ` · **Model:**claude-sonnet-4-6`
Stats: 1 user messages, 27 tool calls, tools used: ToolSearch, mcp__plugin_github_github__pull_request_read, Bash, Read.
Conversation
Turn 1 — User
Review the type design in the following GitHub PR:
PR URL: https://github.com/doctolib/doctolib/pull/241133 Repo: doctolib/doctolib
Use the GitHub MCP tools to read the PR diff. If you need to browse local code for context, the local worktree is at /Users/USER/workspace/doctolib/worktrees/doctolib/ (check subdirectories for the relevant branch).
Assess any new types/interfaces/enums introduced in this PR for: - Encapsulation quality - Invariant expression - Usefulness and clarity - Whether types enforce correct usage at compile time
Return a structured list of findings with file paths and line numbers.
Turn 1 — Assistant
I'll start by reading the PR diff using GitHub tools to understand what types are being introduced.
Tools used:
ToolSearch(inputs: query, max_results)
Tool results:
→ result (ok): No matching deferred tools found
Tools used:
ToolSearch(inputs: query, max_results)
Tool results:
→ result (ok)
Let me fetch the PR details and diff simultaneously.
Tools used:
mcp__plugin_github_github__pull_request_read(inputs: method, owner, repo, pullNumber)
Tool results:
→ result (ERROR): failed to get pull request: GET https://api.github.com/repos/doctolib/doctolib/pulls/241133: 404 Not Found []
Tools used:
mcp__plugin_github_github__pull_request_read(inputs: method, owner, repo, pullNumber, perPage)
Tool results:
→ result (ERROR): failed to get pull request files: GET https://api.github.com/repos/doctolib/doctolib/pulls/241133/files?page=1&per_page=100: 404 Not Found []
The GitHub API returns 404, which likely means the repository is private and the MCP token doesn't have access, or the PR number is incorrect. Let me try fetching the diff directly and also check the local worktree for relevant branches.
Tools used:
Bash:ls /Users/USER/workspace/doctolib/worktrees/doctolib/
Tool results:
→ result (ok): fhir-2089 FHIR-2091-nhr-document-list-pagination FHIR-2166-fix-nhr-mock-status-visibility fhir-2169 fhir-2181 fhir-2189_class-code-label-map fhir-2190_improve-filter-panel-ui fhir-2209_fix-patient-ins-env-transformation fhir-2216_fix-nhr-date-columns-and-document-type fhir-2221_display-last-dmp-access-date fhir-2222_add-visible-to-all-filter fhir-2223_fix-column-widths-dmp FHIR-2224
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git worktree list 2>/dev/null | head -20
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/doctolib e8b3e695da92 [fhir-2215_wire-dmp-errors-through-dmpnhradapter] /Users/USER/workspace/doctolib/doctolib/.claude/worktrees/crazy-volhard d2d21781dba4 [claude/crazy-volhard] /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2089 9c01fcf036d7 (detached HEAD) /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2091-nhr-document-li …(truncated, 1676 more chars)
The GitHub API returns 404 because the repo is private. I need to use gh CLI to access the PR. Let me try that, and also look for the branch associated with this PR number.
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && gh pr view 241133 --json title,headRefName,baseRefName,body 2>&1 | head -30
Tool results:
→ result (ok): {"baseRefName":"master","body":"## Why\n\nhttps://doctolib.atlassian.net/browse/FHIR-2224\n\nupdateDocumentStatus in DmpApi previously returned Promise<void> and threw on failure. Callers had no way to access structured DMP error information (error codes, descriptions) — they could only catch a plain Error with a message string. This PR makes the API return structured results, consistent with how search() returns SearchResult with an optional error: DmpApiErrorInfo.\n\n## How\n\n**
…(truncated, 2794 more chars)
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && gh pr diff 241133 2>&1
Tool results:
→ result (ok): diff --git a/packages/@doctolib/dmp/src/business/DmpApi.test.ts b/packages/@doctolib/dmp/src/business/DmpApi.test.ts index 2b146a28faf65..99769412b8646 100644 --- a/packages/@doctolib/dmp/src/business/DmpApi.test.ts +++ b/packages/@doctolib/dmp/src/business/DmpApi.test.ts @@ -1609,33 +1609,78 @@ describe('DmpApi', () => { }) })
- it('should throw when no cached session exists', async () => {
- it('should return success: true on success', async () => {
- getCachedDmpSessi …(truncated, 18145 more chars)
Now let me read the actual type definition files to get the full context and line numbers.
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git log --oneline FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api 2>/dev/null | head -5
Tools used:
Bash:ls /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/
Tool results:
→ result (ok): (Bash completed with no output)
Tool results:
→ result (ok): AGENTS.md app babel.config.js backend.dependencies.codeowners.yml bin catalog-info.yaml ci CLAUDE.md ⇒ AGENTS.md CODEOWNERS CODEOWNERS.local component_backlog.yml config config.ru CONTRIBUTING.md db dev-env-setup-wsl dev-tools doc docker docker-compose.linux.yml docker-compose.specmatic.yml docker-compose.yml docs engine_metrics.yml engines fixtures frontend-metrics.config.ts frontend.dependencies.codeowners.yml Gemfile Gemfile.experimental ⇒ Gemfile Gemfile.experimental.lock Gemfile.lock Gemfil …(truncated, 559 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/types/DmpApi.ts
Tool results:
→ result (ok): 1 import type { Patient } from 'monolith_DEPRECATED/types/doctor/api/Patient'
2 import type { ClinicalDocument } from 'engines/cdar2/types/ClinicalDocument'
3 import type { DmpDocumentToAdd, DmpDocumentToDelete } from '../business/FrDmpConfig'
4 import type { DmpWorkflow, BillingSituation, GetDocumentsContentParams, PracticeLocationOverride } from './FrDmpTypes'
5
6 export type PatientConsent = 'error' | 'authorized' | 'expired' | 'forbidden' | 'noAuthorization'
7
8 export type PatientConsentU
…(truncated, 5382 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts
Tool results:
→ result (ok): 1 / eslint-disable @typescript-eslint/no-non-null-assertion / 2 import type { Patient } from 'monolith_DEPRECATED/types/doctor/api/Patient' 3 import type { MedicalFolder } from '@doctolib/ehr-core' 4 import { getLoincCode } from 'monolith_DEPRECATED/models/document/document' 5 import { sendMonitoringEvent } from 'monolith_DEPRECATED/services/monitoring' 6 import { isMinor } from 'monolith_DEPRECATED/models/patient/patient' 7 import { isMedicalSynthesis } from 'monolith_DEPRECATED/components/do …(truncated, 38564 more chars)
I can see a critical discrepancy: the diff shows one version of DocumentStatusUpdateResult but the actual file in the worktree shows another. Let me read the adapter file and also check the diff's version of the type more carefully.
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts(200–280)
Tool results:
→ result (ok): 200 const typeCode = doc.cdaContent?.code?._code 201 const facet = findFacetForCDAType(typeCode, this.documentCategorizationFacets) 202 return toNhrDocumentContentResult(doc, facet) 203 }) 204 } 205 206 async updateDocumentMetadata(props: NhrDocumentMetadataUpdateProps): Promise<NhrDocumentMetadataUpdateResult> { 207 if (!props.patient?.nationalHealthIdentifier) { 208 throw new Error('patient_national_health_identifier_required') 209 } 210 211 const …(truncated, 2378 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/index.ts
Tool results:
→ result (ok): 1 export {
2 type DmpDocumentItem,
3 type DmpDocumentToAdd,
4 type DmpDocumentToDelete,
5 type FrDmpDocument,
6 ItemType,
7 type UploadedDmpDocumentItem,
8 } from './business/FrDmpConfig'
9 export { getErrorMessage, getWarningMessage } from './business/DmpI18nMessage'
10 export type {
11 DmpApi,
12 DmpAuthor,
13 DmpDocument,
14 DmpDocumentContent,
15 DmpSetUpProps,
16 ShareProps,
17 UnshareProps,
18 SearchProps,
19 SearchResult,
20 DocumentStatusUpdateProps,
21
…(truncated, 5772 more chars)
Now I have a clear picture. There is a critical discrepancy: the diff proposes DocumentStatusUpdateResult as a discriminated union ({ success: true } | { success: false; error: DmpApiErrorInfo }), but the actual code in the FHIR-2224 worktree implements it as { error?: DmpApiErrorInfo } (the old SearchResult-mirror pattern). The tests in the diff also reflect the discriminated union, while the implementation code uses the optional-error pattern. Let me verify this split and check the DmpApiError class.
Tools used:
Bash:find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src -name "DmpApiError.ts" 2>/dev/null
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/api/DmpApiError.ts
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git log --oneline origin/FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api 2>/dev/null | head -10
Tool results:
→ result (ok): 52de63a3ad5b FHIR-2224: Remove hcsettings d8c22c336a1d Merge branch 'master' into FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api d10c70241288 feat(VIBE-1524): Add native app conversion tracking events (#241151) 76355f378ad7 chore(P3C-7089): add Account scope rubocop rule and remove unused scopes (#241135) ad0c254c5e60 fix(LION-1350): use cache for member_ids lookup in agendas and messaging profiles (#241143) 4d362d823d3f feat(TAHC-1793): web landing page at /deeplink/health-com …(truncated, 424 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/api/DmpApiError.ts
Tool results:
→ result (ok): 1 import type { DmpApiErrorInfo } from '../types/DmpApi'
2
3 export class DmpApiError extends Error {
4 readonly apiErrorCode: string | undefined
5
6 readonly apiErrorType: string | undefined
7
8 readonly apiErrorContext: string | undefined
9
10 readonly apiErrorDescription: string | undefined
11
12 readonly apiErrorExtendedInformations: string | undefined
13
14 readonly apiErrorTypeDescription: string | undefined
15
16 readonly status: string
17
18 constructor(info: DmpA
…(truncated, 730 more chars)
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git show origin/FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api:packages/@doctolib/dmp/src/types/DmpApi.ts 2>/dev/null | grep -n "Do …(truncated, 26 more chars)
Tool results:
→ result (ok): 112: updateDocumentStatus: (props: DocumentStatusUpdateProps) => Promise<DocumentStatusUpdateResult> 186:export type DocumentStatusUpdateResult = { success: true } | { success: false; error: DmpApiErrorInfo }
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git show origin/FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api:packages/@doctolib/dmp/src/business/DmpApi.ts 2>/dev/null | grep -n …(truncated, 33 more chars)
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git show origin/FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api:packages/@doctolib/dmp/src/business/DmpApi.ts 2>/dev/null | sed -n ' …(truncated, 9 more chars)
Tool results:
→ result (ok): 36: updateDocumentStatus as updateDocumentStatusCommand, 346: async updateDocumentStatus(props: DocumentStatusUpdateProps): Promise<DocumentStatusUpdateResult> { 376: const response = await updateDocumentStatusCommand({
Tool results:
→ result (ok): const { sessionId } = cachedSession const response = await getDocumentsContent({ sessionId, ...props }) return response.Documents.map(this.toDocumentContent) }
async updateDocumentStatus(props: DocumentStatusUpdateProps): Promise<DocumentStatusUpdateResult> { const { ins, documentUniqueId: uniqueId, uuid, getTransactionFrames } = props if (!this.props.frDmpWorkflow) throw new Error('frDmpWorkflow is required to update document status') const healthCareSettings = this.pro …(truncated, 1961 more chars)
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git show origin/FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api:packages/@doctolib/national-health-records/src/business/adapters/Dmp …(truncated, 45 more chars)
Tool results:
→ result (ok): const typeCode = doc.cdaContent?.code?._code const facet = findFacetForCDAType(typeCode, this.documentCategorizationFacets) return toNhrDocumentContentResult(doc, facet) }) }
async updateDocumentMetadata(props: NhrDocumentMetadataUpdateProps): Promise<NhrDocumentMetadataUpdateResult> { if (!props.patient?.nationalHealthIdentifier) { throw new Error('patient_national_health_identifier_required') }
const ins = toEnvIns(props.patient.nationalHealthIdentifier)
…(truncated, 1499 more chars)
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git show origin/FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api:packages/@doctolib/dmp/src/types/DmpApi.ts 2>/dev/null | grep -n "Do …(truncated, 25 more chars)
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git show origin/FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api:packages/@doctolib/dmp/src/types/DmpApi.ts 2>/dev/null | sed -n '183 …(truncated, 6 more chars)
Tool results:
→ result (ok): 112: updateDocumentStatus: (props: DocumentStatusUpdateProps) => Promise<DocumentStatusUpdateResult> 190:export type DocumentStatusUpdateProps = {
Tool results:
→ result (ok): error?: DmpApiErrorInfo }
export type DocumentStatusUpdateResult = { success: true } | { success: false; error: DmpApiErrorInfo }
export type GetDocumentsContentProps = Omit<GetDocumentsContentParams, 'sessionId'>
export type DocumentStatusUpdateProps = { ins: string documentUniqueId: string uuid: string getTransactionFrames?: boolean }
Now I have the complete picture. The actual PR branch uses the discriminated union design. The worktree at /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/ is on a different (older) commit. Let me verify the DmpApiErrorInfo interface and also check for any structural inconsistency with the SearchResult type used elsewhere.
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git show origin/FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api:packages/@doctolib/dmp/src/types/DmpApi.ts 2>/dev/null | sed -n '170 …(truncated, 6 more chars)
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git show origin/FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api:packages/@doctolib/dmp/src/types/DmpApi.ts 2>/dev/null | sed -n '100 …(truncated, 6 more chars)
Tool results:
→ result (ok): export interface DmpApiErrorInfo { apiErrorCode?: string apiErrorType?: string apiErrorContext?: string apiErrorDescription?: string apiErrorExtendedInformations?: string apiErrorTypeDescription?: string status: string }
export interface SearchResult { documents: DmpDocument[] error?: DmpApiErrorInfo }
export type DocumentStatusUpdateResult = { success: true } | { success: false; error: DmpApiErrorInfo }
export type GetDocumentsContentProps = Omit<GetDocumentsContentParams, …(truncated, 146 more chars)
Tool results:
→ result (ok): // Parsed s_cdaContentInBase64 cdaContent?: ClinicalDocument cdaContentInBase64?: string }
export interface DmpApi { share: (props: ShareProps) => Promise<void> unshare: (props: UnshareProps) => Promise<void> search: (props: SearchProps) => Promise<SearchResult> getPatientConsent: () => Promise<PatientConsent> updatePatientConsent: (consent: PatientConsentUpdate) => Promise<boolean> getDocumentsContentByUniqueId: (props: GetDocumentsContentProps) => Promise<DmpDocumentContent[]> …(truncated, 303 more chars)
I now have the complete, authoritative picture from the actual PR branch head. Let me do one final check on the DmpApiError class usage and the healthCareSettings prop removal to confirm the full scope.
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git show origin/FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api:packages/@doctolib/dmp/src/api/DmpApiError.ts 2>/dev/null
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib && git show origin/FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api:packages/@doctolib/dmp/src/business/DmpApi.ts 2>/dev/null | sed -n ' …(truncated, 9 more chars)
Tool results:
→ result (ok): import type { DmpApiErrorInfo } from '../types/DmpApi'
export class DmpApiError extends Error { readonly apiErrorCode: string | undefined
readonly apiErrorType: string | undefined
readonly apiErrorContext: string | undefined
readonly apiErrorDescription: string | undefined
readonly apiErrorExtendedInformations: string | undefined
readonly apiErrorTypeDescription: string | undefined
readonly status: string
constructor(info: DmpApiErrorInfo) { const message = info …(truncated, 637 more chars)
Tool results:
→ result (ok): }
async updateDocumentStatus(props: DocumentStatusUpdateProps): Promise<DocumentStatusUpdateResult> { const { ins, documentUniqueId: uniqueId, uuid, getTransactionFrames } = props if (!this.props.frDmpWorkflow) throw new Error('frDmpWorkflow is required to update document status') const healthCareSettings = this.props.frDmpWorkflow.healthcareSettings
if (this.dmpService) {
const payload = buildUpdateDocumentStatusPayload({
I now have a complete, accurate picture of all types introduced in this PR. Here is the full structured analysis.
Type Design Review: PR #241133 — feat(FHIR-2224): return doc status update results from DmpApi
All file paths refer to the PR branch (FHIR-2224-monolith-return-doc-status-update-results-from-dmp-api).
Type: DocumentStatusUpdateResult
File: packages/@doctolib/dmp/src/types/DmpApi.ts, line 186
Definition:
export type DocumentStatusUpdateResult = { success: true } | { success: false; error: DmpApiErrorInfo }
Invariants Identified
- A result is either a success with no error payload, or a failure with a mandatory
DmpApiErrorInfo. - There is no representable state where
success: trueand an error co-exist. - There is no representable state where
success: falseexists without a concrete error.
Ratings
-
Encapsulation: 8/10 The type is exported from the package index and is properly opaque from the caller's perspective. The
DmpApiImplclass is the only producer, and callers can only consume via the discriminant. There are no mutable internals. A minor deduction:DmpApiErrorInfoitself has all-optional fields (exceptstatus), which means the error payload is structurally weak — but that is a separate, pre-existing concern inherited from the DMP API response shape. -
Invariant Expression: 9/10 The discriminated union on the
successboolean literal makes it a genuine tagged union. TypeScript narrows the type correctly when callers branch onresult.success. Accessingresult.errorwithout first checkingresult.success === falseis a compile-time error. This is the strongest possible expression of this invariant without introducing a nominal wrapper type. -
Invariant Usefulness: 9/10 The PR's own motivation is precisely that callers previously had no structured way to inspect DMP-specific error fields — they could only catch a
string-messagedError. The new type enforces at compile time that callers handle the error case and gives them access toapiErrorCode,apiErrorType,apiErrorDescription, andapiErrorExtendedInformations. This directly prevents the class of bug where a caller silently ignores DMP error detail. -
Invariant Enforcement: 8/10 The
DmpApiImplimplementation always returns one of the two union members — success returns{ success: true }, all error paths return{ success: false, error: ... }. The interface definition at line 112 ensures no conforming implementation can returnvoidor throw as the primary signal. One gap: on thedmpServicepath, onlyBusinessErroris caught and turned into a structured result; non-BusinessErrorexceptions still propagate as throws. This means callers interacting withDmpApiImplvia theDmpApiinterface cannot rely on the no-throw guarantee uniformly across both code paths.
Strengths
- The discriminated union design is clearly superior to the
SearchResultpattern ({ error?: DmpApiErrorInfo }) used elsewhere in the same file. TheSearchResultpattern requires callers to testif (result.error)but TypeScript does not enforce the check — a caller can readresult.documentsand silently ignoreresult.error. With the discriminated union, the TypeScript compiler forces callers to narrow the type before accessing error fields, so the error path cannot be silently skipped. - Removing
healthCareSettingsfromDocumentStatusUpdateProps(line 190–195) is a meaningful cohesion improvement. The implementation now readshealthcareSettingsfromthis.props.frDmpWorkflow, which is the canonical place for that value, eliminating a redundant input that callers could supply inconsistently. - The
DmpApiErrorclass (atpackages/@doctolib/dmp/src/api/DmpApiError.ts) provides a single, tested message-composition rule that correctly combinesapiErrorDescriptionandapiErrorExtendedInformationswith a:separator and falls back to'DMP API error'. By routing throughnew DmpApiError(result.error).messageinDmpNhrAdapter, the PR avoids duplicating that logic.
Concerns
-
Asymmetric throw behaviour between code paths. In
DmpApiImpl.updateDocumentStatus, thedmpServicepath wraps onlyBusinessErrorin a structured result and re-throws everything else (line ~363:throw e). The legacygetCachedDmpSession/updateDocumentStatusCommandpath wraps everything in a try/catch and never throws (lines ~370–405). A caller who switches between the two paths at runtime (which is the design — thedmpServicepath is behind a feature switch) gets different exception-vs-result semantics. TheDmpApiinterface signature alone cannot express this: it saysPromise<DocumentStatusUpdateResult>with no indication that non-BusinessErrormay still reject the promise. This is a latent correctness risk at the adapter layer (DmpNhrAdapterstill has a surrounding try/catch at line 249 in the worktree copy, masking the gap for now, but the PR version removes that safety net). -
DmpApiErrorInfofields are all optional exceptstatus. The error member of the failure branch is typed asDmpApiErrorInfo, in whichapiErrorDescription,apiErrorCode,apiErrorTypeetc. are allstring | undefined. This means a fully valid{ success: false, error: { status: 'ERROR' } }carries no human-readable message. Callers must defensively handle the undefined case. TheDmpApiErrorclass handles this correctly with its fallback to'DMP API error', but a caller who directly readsresult.error.apiErrorDescriptionwithout going throughDmpApiErrorwill getundefined. This is not a new issue introduced by the PR, but the new type makes it the caller's problem in a more visible way. -
Divergence from the
SearchResultpattern is not documented. The PR description correctly notes thatDocumentStatusUpdateResultmirrorsSearchResult. In fact it improves on it by using a discriminated union instead of an optional error, but the existingSearchResultinterface is left in the older, weaker form. A future developer reading both types in the same file will see two different idioms for the same concept (optional-error vs. discriminated union) without any comment explaining the intentional improvement. This invites regression to the weaker pattern for new result types. -
The
successfield carries no additional payload on the success branch.{ success: true }is a valid and simple design, but it means callers get no confirmation detail (no response timestamp, no document ID echo, nothing). Forsearch(),SearchResultincludesdocuments. ForupdateDocumentStatus, success is purely acknowledged by the absence of an error. This is appropriate for an update operation, but worth noting as a deliberate design choice since it means the type cannot be extended to carry success-side data without a breaking change to the union.
Recommended Improvements
-
Make the throw behaviour consistent across both code paths. The cleanest fix is to wrap the entire body of
updateDocumentStatus— both thedmpServicebranch and the legacy branch — in a single top-level try/catch that converts any exception into a{ success: false, error: { status: 'ERROR', apiErrorDescription: message } }result. This fulfils the implied promise of theDocumentStatusUpdateResultreturn type: that the function always resolves to a structured value and never rejects. This would require updating the two tests that currently assert re-throw behaviour on thedmpServicepath. -
Annotate the intentional departure from
SearchResult. A short comment at theDocumentStatusUpdateResultdeclaration stating why a discriminated union is used instead of theSearchResult-style optional error would help maintain consistency for future result types added to this interface. -
**Consider tightening
DmpApiErrorInfo.apiErrorDescriptiontostring…(truncated, 3938 more chars)