Project: doctolib-doctolib · Branch: .invalid · Mode: ` · **Model:**claude-opus-4-7`

Stats: 1 user messages, 21 tool calls, tools used: Bash, Read.

Conversation

Turn 1 — User

Review PR #241133 (FHIR-2224) in the doctolib/doctolib monorepo. This PR changes DmpApi.updateDocumentStatus from Promise<void> (throwing on failure) to Promise<DocumentStatusUpdateResult> (returning structured errors), following the existing SearchResult pattern.

Changed files (all in /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/):

1. packages/@doctolib/dmp/src/types/DmpApi.ts — adds DocumentStatusUpdateResult interface { error?: DmpApiErrorInfo }, changes DmpApi interface signature
2. packages/@doctolib/dmp/src/business/DmpApi.ts — updated DmpApiImpl.updateDocumentStatus implementation (lines ~346–400)
3. packages/@doctolib/dmp/src/index.ts — exports DocumentStatusUpdateResult
4. packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts — uses structured result + DmpApiError for error messages (lines ~218–258)
5. packages/@doctolib/dmp/src/business/DmpApi.test.ts — updated tests
6. packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts — updated tests

Key concerns to analyze:

1. Asymmetric error handling between the two code paths in DmpApiImpl.updateDocumentStatus: - Legacy path (!this.dmpService): On non-OK status, maps response.s_status ?? 'ERROR' — the null-coalescing on s_status is suspicious since s_status !== STATUS_OK just passed, so s_status is always truthy here. - dmpService path: catches BusinessError and returns { error: { status: 'ERROR', apiErrorDescription: e.message } } — no apiErrorCode, apiErrorType, etc., so the error object is sparse. Other exceptions still throw (non-BusinessError). This means updateDocumentStatus can still throw in some cases — callers in DmpNhrAdapter must handle both structured errors AND thrown exceptions.

2. Mock duplication in tests: DmpNhrAdapter.test.ts inlines a replica of DmpApiError constructor logic inside jest.mock('@doctolib/dmp'). This creates a maintenance risk — if DmpApiError's message format changes, the inline mock diverges silently.

3. DocumentStatusUpdateProps cast removed: The original code had } as DocumentStatusUpdateProps) cast that was removed. This should be validated that the TypeScript inference now works correctly without the cast.

4. return statement cleanup: Original code had return at the end of the dmpService path (after the try-catch). The PR moves return {} inside the try block, which changes flow slightly. Verify no edge cases are missed.

5. DmpApiError is only used for .message extraction in DmpNhrAdapter.ts — the full typed error object is instantiated just to read .message. This might be over-engineering vs. a simpler helper.

Please review the actual files at the worktree path and report findings in Conventional Comments format, with file:line references.

Turn 1 — Assistant

I'll review the PR changes by examining the actual files at the worktree path.

Tools used:

• Bash: ls /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/ 2>&1 | head -20

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

I'll review the actual files in the worktree to verify the concerns raised.

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)

Tools used:

• Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts

Tool results:

→ result (ok): 1 import type { 2 DmpApi, 3 DmpAuthor, 4 DmpDocument, 5 DmpDocumentContent, 6 SearchProps, 7 SearchResult as DmpSearchResult, 8 } from '@doctolib/dmp' 9 import { DmpApiError, DOCUMENT_STATUS, DOCUMENT_VISIBILITY, getPatientInsForDMP } from '@doctolib/dmp' 10 import { isUnstructured } from '@doctolib/cda/types/UnstructuredBody' 11 import { isStructured } from '@doctolib/cda/types/StructuredBody' 12 import type { Patient } from 'monolith_DEPRECATED/types/doctor/api/Patient' 13 import t …(truncated, 9948 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 let's examine the DmpApiError class and the test files to understand the mock duplication concern.

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:

• Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts

Tool results:

→ result (ok): 1 import { getPatientInsForDMP } from '@doctolib/dmp' 2 import type { DmpApi, DmpDocumentContent, SearchResult as DmpSearchResult } from '@doctolib/dmp' 3 import type { NhrDocumentSearchProps } from '@doctolib/tpf-national-health-records' 4 import type { DocumentCategorizationConcept, DocumentCategorizationFacet } from '@doctolib/documents-ui/types/Document' 5 import { DmpNhrAdapter } from './DmpNhrAdapter' 6 import { findFacetForCDAType } from '../utils/findFacetForCDAType' 7
8 jest.mock('../u …(truncated, 26876 more chars)

Now let me check the DmpApi.test.ts file and verify the FrDmpCommunication updateDocumentStatus return shape.

Tools used:

• Bash: grep -n "s_status\|i_apiErrorCode\|i_apiErrorType\|s_apiErrorDescription" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/FrDmpCommunication.ts | h …(truncated, 7 more chars)

Tool results:

→ result (ok): 77: response: { s_status, i_apiErrorCode, i_apiErrorType } = {}, 85: const has_unexpected_error = s_commandName !== 'hl_readCpxCard' && s_status !== STATUS_OK 98: s_status, 99: i_apiErrorCode, 100: i_apiErrorType,

Tools used:

• Bash: grep -n "updateDocumentStatus\|s_status" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts | head -40

Tool results:

→ result (ok): 19: updateDocumentStatus as updateDocumentStatusCommand, 54: updateDocumentStatus: jest.fn(), 117:const updateDocumentStatusCommandMocked = jest.mocked(updateDocumentStatusCommand) 197: deleteDocumentMocked.mockResolvedValue({ s_status: STATUS_OK }) 198: setConnectionConfidentialityLevelMocked.mockResolvedValue({ s_status: STATUS_OK }) 199: getDmpMajorityAgeMocked.mockResolvedValue({ s_status: STATUS_OK, i_majorityAge: 18 }) 635: s_status: STATUS_OK, 888: s …(truncated, 2651 more chars)

Tools used:

• Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts (1589–1789)

Tool results:

→ result (ok): 1589 describe('updateDocumentStatus ', () => { 1590 it('should call updateDocumentStatusCommand with session and props', async () => { 1591 getCachedDmpSessionMocked.mockResolvedValue({ sessionId: 'session-abc', healthcareSettings: 'SA07' }) 1592 updateDocumentStatusCommandMocked.mockResolvedValue({ s_status: STATUS_OK }) 1593
1594 const dmpApi = new DmpApi(mockProps) 1595 await dmpApi.updateDocumentStatus({ 1596 ins: '123456789012345678901', 1597 d …(truncated, 7539 more chars)

Let me also confirm the dmpService updateDocumentStatus tests for the BusinessError path:

Tools used:

• Bash: grep -n "UpdateDocumentStatus\|updateDocumentStatus" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts | head -30

Tool results:

→ result (ok): 19: updateDocumentStatus as updateDocumentStatusCommand, 54: updateDocumentStatus: jest.fn(), 117:const updateDocumentStatusCommandMocked = jest.mocked(updateDocumentStatusCommand) 1589: describe('updateDocumentStatus ', () => { 1590: it('should call updateDocumentStatusCommand with session and props', async () => { 1592: updateDocumentStatusCommandMocked.mockResolvedValue({ s_status: STATUS_OK }) 1595: await dmpApi.updateDocumentStatus({ 1602: expect(updateDocumentStatusCom …(truncated, 763 more chars)

Tools used:

• Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts (2010–2110)

Tool results:

→ result (ok): 2010 describe('updateDocumentStatus (UpdateDocumentStatus)', () => { 2011 it('should call dmpService.call with UpdateDocumentStatus action and correct payload', async () => { 2012 mockDmpService.call.mockResolvedValue({}) 2013
2014 const dmpApi = createDmpApiWithService() 2015 await dmpApi.updateDocumentStatus({ 2016 ins: '123456789012345678901', 2017 documentUniqueId: 'unique-doc-123', 2018 uuid: 'uuid-doc-456', 2019 hea …(truncated, 3793 more chars)

Now let me also check the FrDmpCommunication.ts response shape for updateDocumentStatus to verify whether response.s_status could be undefined.

Tools used:

• Bash: grep -n "updateDocumentStatus\|UpdateDocumentStatusByUniqueIdResponse" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/FrDmpCommunication.ts | head …(truncated, 4 more chars)

Tool results:

→ result (ok): 45: UpdateDocumentStatusByUniqueIdResponse, 408:export const updateDocumentStatus = ({ 415:}: UpdateDocumentStatusByUniqueIdParams): Promise<UpdateDocumentStatusByUniqueIdResponse> => { 417: s_commandName: 'hl_updateDocumentStatusByUniqueId', 421: return frDmpSend<UpdateDocumentStatusByUniqueIdRequest, UpdateDocumentStatusByUniqueIdResponse>(payload)

Tools used:

• Bash: grep -n "UpdateDocumentStatusByUniqueIdResponse" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/types/FrDmpTypes.ts

Tool results:

→ result (ok): 606:export interface UpdateDocumentStatusByUniqueIdResponse extends DmpResponse {}

Tools used:

• Bash: grep -n "interface DmpResponse\|^export interface DmpResponse\|s_status" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/types/FrDmpTypes.ts | head -10

Tool results:

→ result (ok): 192:export interface DmpResponse extends DmpResult { 194: s_status?: string 208: s_status: string 231: s_status: string 670:// "s_status":"OK"

Tools used:

• Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/types/FrDmpTypes.ts (185–225)

Tool results:

→ result (ok): 185 / Event codes to filter by. Returns documents matching at least one event code. */ 186 EventCodes?: EventCode[] 187 / Enable transaction frame debugging (0 or 1). Development only. */ 188 i_getTransactionFrames?: number 189 } 190 191 // DMP Responses 192 export interface DmpResponse extends DmpResult { 193 // Value "OK" for success. 194 s_status?: string 195 i_apiErrorCode?: string 196 i_apiErrorType?: string 197 s_apiErrorContext?: string 198 s_apiErrorDescription?: …(truncated, 655 more chars)

Important finding. s_status on DmpResponse is typed as string | undefined (optional). So the response.s_status ?? 'ERROR' fallback in DmpApi.ts line 394 is NOT redundant - it correctly handles a missing s_status typed as undefined. The narrowing from response.s_status !== STATUS_OK only confirms it's not equal to 'OK', but it could still be undefined. Let me verify.

Actually let me re-read the code more carefully:

Tools used:

• Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts (385–405)

Tool results:

→ result (ok): 385 if (response.s_status !== STATUS_OK) { 386 return { 387 error: { 388 apiErrorCode: response.i_apiErrorCode, 389 apiErrorType: response.i_apiErrorType, 390 apiErrorContext: response.s_apiErrorContext, 391 apiErrorDescription: response.s_apiErrorDescription, 392 apiErrorExtendedInformations: response.s_apiErrorExtendedInformations, 393 apiErrorTypeDescription: response.s_apiErrorTypeDescription, 394 status: …(truncated, 307 more chars)

Good. Since s_status is string | undefined, s_status !== STATUS_OK does NOT narrow it to truthy - it still could be undefined. So the ?? 'ERROR' fallback is correct and necessary.

Now let me check the getTransactionFrames mismatch — the test passes getTransactionFrames: undefined but DocumentStatusUpdateProps accepts boolean, while updateDocumentStatusCommand may expect a different shape.

Tools used:

• Bash: grep -n "getTransactionFrames\|i_getTransactionFrames\|UpdateDocumentStatusByUniqueIdParams" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/FrDmpC …(truncated, 26 more chars)

Tool results:

→ result (ok): 43: UpdateDocumentStatusByUniqueIdParams, 286: payload.i_getTransactionFrames = 1 384: getTransactionFrames, 402: getTransactionFrames, 414: getTransactionFrames, 415:}: UpdateDocumentStatusByUniqueIdParams): Promise<UpdateDocumentStatusByUniqueIdResponse> => { 419: ...buildUpdateDocumentStatusPayload({ ins, uniqueId, uuid, healthCareSettings, getTransactionFrames }), 431: getTransactionFrames, 442: getTransactionFrames, 472: * @param getTransactionFrames - Enable transaction …(truncated, 529 more chars)

Tools used:

• Bash: grep -n "UpdateDocumentStatusByUniqueIdParams\|getTransactionFrames" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/types/FrDmpTypes.ts | head -10

Tool results:

→ result (ok): 80: i_getTransactionFrames?: number 188: i_getTransactionFrames?: number 551: getTransactionFrames?: boolean 568: i_getTransactionFrames?: number 588:export interface UpdateDocumentStatusByUniqueIdParams { 594: getTransactionFrames?: boolean 603: i_getTransactionFrames?: number 614: getTransactionFrames?: boolean 624: i_getTransactionFrames?: number 636: getTransactionFrames?: boolean

Type alignment is fine. Let me check the test for the partial-success edge case - line 625 in the adapter test does mockResolvedValueOnce({}).mockRejectedValueOnce(new Error('fail')). That means one returns {} (success) and one throws (caught by catch block). Expected to be partial success. But the call ordering in Promise.all may not be deterministic; let me confirm:

Promise.all preserves the order of inputs, but mockResolvedValueOnce queues up calls in invocation order. Both payloads call updateDocumentStatus synchronously before any await resolves, so the first call would get the resolved {}, second call would get the rejection. That works.

Let me check if there's anywhere else that calls dmpApi.updateDocumentStatus to verify the API change is complete:

Tools used:

• Bash: grep -rn "updateDocumentStatus" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/ --include="*.ts" --include="*.tsx" | grep -v "\.test\.\|FrDmpCommunication\|frDmpPa …(truncated, 49 more chars)

Tool results:

→ result (ok): /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/index.ts:106: updateDocumentStatus, /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/CallDmpDelegate.ts:12: [DmpAction.UpdateDocumentStatus]: 'hl_updateDocumentStatusByUniqueId', /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts:221: const result = await this.d …(truncated, 1572 more chars)

Only DmpNhrAdapter.ts consumes the high-level dmpApi.updateDocumentStatus. Good. The lower-level command updateDocumentStatus (from FrDmpCommunication) is unchanged.

Let me verify one more concern - whether the try/catch in DmpNhrAdapter.updateDocumentMetadata (line 219-257) actually catches non-BusinessError exceptions that propagate from the legacy path. Looking at the legacy path code (lines 370-399): it can throw 'No DMP session found' if no session, and updateDocumentStatusCommand could fail. Both will propagate. The catch in the adapter handles e instanceof Error - good.

Let me verify the getTransactionFrames field is preserved through the adapter - actually the adapter never passes getTransactionFrames. That seems intentional since it's a debug field.

Let me also verify the partial success test concern more carefully:

Tools used:

• Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts (623–653)

Tool results:

→ result (ok): 623 it('should return partial success when one payload succeeds and one fails', async () => { 624 const mockDmpApi = createMockDmpApi() 625 jest.mocked(mockDmpApi.updateDocumentStatus).mockResolvedValueOnce({}).mockRejectedValueOnce(new Error('fail')) 626 const adapter = new DmpNhrAdapter(mockDmpApi, undefined) 627 628 const result = await adapter.updateDocumentMetadata({ 629 ...updateProps, 630 params: { 631 payloads: [ 632 { id …(truncated, 834 more chars)

I have enough context to provide the review. Let me now provide my findings.

PR #241133 Review (FHIR-2224)

Files reviewed

• /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/types/DmpApi.ts
• /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts
• /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/index.ts
• /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts
• /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts
• /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts
• /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/api/DmpApiError.ts (referenced for mock comparison)
• /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/types/FrDmpTypes.ts (referenced to confirm DmpResponse.s_status is string | undefined)

Verdict on flagged concerns

After reading the actual code, several of the concerns raised are not bugs. Below I confirm each one explicitly so reviewers don't waste cycles, and add the genuine high‑confidence findings.

Concern 1 — response.s_status ?? 'ERROR' "always truthy" (DmpApi.ts:394)

Not a bug. Confidence: 95.

In FrDmpTypes.ts:194, DmpResponse.s_status is typed string | undefined (optional). The check response.s_status !== STATUS_OK only excludes the literal 'OK'; it does not narrow undefined away. So when the iCanopée adapter returns a response with no s_status field, the branch is entered with response.s_status === undefined, and ?? 'ERROR' is required to satisfy DmpApiErrorInfo.status: string (non‑optional). The same idiom is used in toSearchResult at DmpApi.ts:591. No change recommended.

Concern 3 — Removed as DocumentStatusUpdateProps cast

Not a bug. Confidence: 90. The argument shape passed to updateDocumentStatusCommand (lines 376–383) matches UpdateDocumentStatusByUniqueIdParams field‑by‑field (verified via FrDmpTypes.ts:588). Inference works — removing the cast is an improvement.

Concern 4 — return {} moved inside the try block

Not a behavior change. Confidence: 92. The previous shape had await ... ; return after the catch; the new shape returns {} after the awaited call inside the try. If the awaited call throws and the catch returns, the post‑try statement was unreachable in the original anyway. Flow is equivalent. No change recommended.

Critical (90–100)

None.

Important (80–89)

issue (confidence 86) — DmpApiError mock duplicates production constructor logic

File: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts:24-33

The jest.mock('@doctolib/dmp', ...) factory inlines a hand‑rolled DmpApiError whose message‑formatting branches mirror the production class at packages/@doctolib/dmp/src/api/DmpApiError.ts:18-32. Three tests ('should use DMP API error fallback message...', 'should combine apiErrorDescription and...', 'should return error result when updateDocumentStatus returns a structured error') assert specific message strings — 'DMP API error', 'Access denied: Quota exceeded', 'Document not found' — that depend on this duplicated logic. If the production DmpApiError message format ever changes (e.g. add a prefix, change separator, add status to the message), the mock will silently keep the old format and the tests will pass while the adapter emits a different message in production.

Suggested fix: export a small, dependency‑free helper from @doctolib/dmp that builds the message (e.g. formatDmpApiErrorMessage(info)) and have both DmpApiError and the test mock call it; or move DmpApiError to its own subpath that does not pull @doctolib/tpf-consent (the reason given for inlining in the comment at line 23) and stop mocking @doctolib/dmp wholesale. Alternatively, reference the real DmpApiError class via jest.requireActual for that single export and keep the rest mocked.

issue (confidence 82) — dmpService error path drops most DmpApiErrorInfo fields

File: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts:362-364

if (e instanceof BusinessError) {
  return { error: { status: 'ERROR', apiErrorDescription: e.message } }
}

The legacy iCanopée path (lines 386–396) populates the full structured error (apiErrorCode, apiErrorType, apiErrorContext, apiErrorDescription, apiErrorExtendedInformations, apiErrorTypeDescription, status). The dmpService path returns only { status: 'ERROR', apiErrorDescription }. This asymmetry leaks into DmpApiError.message in DmpNhrAdapter.ts:234, where the message format is apiErrorDescription (since apiErrorExtendedInformations is always absent here), and into any future caller that inspects apiErrorCode/apiErrorType for branching or telemetry. The same asymmetry already exists for findDocuments (lines 670–677 returns an empty result, no error) but updateDocumentStatus chose a different strategy, so the inconsistency is now within this same method.

Suggested fix: if BusinessError carries structured fields (code, type, etc. — worth checking @doctolib/tpf-french-teleservices), surface them to DmpApiErrorInfo. If not, document with a comment that the dmpService path intentionally provides only a description. At minimum, set apiErrorCode / apiErrorType to a constant like 'BUSINESS_ERROR' so callers can disambiguate the two backends.

issue (confidence 80) — Non‑BusinessError exceptions still throw, contradicting the new contract

File: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts:366 and /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts:373

The new return type Promise<DocumentStatusUpdateResult> and the PR description ("returning structured errors, following the existing SearchResult pattern") suggest callers should be able to rely on a result object. But two paths still throw:

1. dmpService path: any non‑BusinessError propagates (throw e at line 366).
2. Legacy path: throw new Error('No DMP session found') at line 373.

This mixed contract is exactly what DmpNhrAdapter.ts:219-257 defends against with a wrapping try/catch, but other future consumers may reasonably read the new signature and skip the try/catch. The SearchResult precedent in find (lines 662–689) has the same dual nature ("No DMP session found" still throws), so this PR is not regressing that precedent — but it is also not improving it.

Suggested fix: either (a) make the contract symmetric by also returning { error: { status: 'NO_SESSION', ... } } for the no‑session case, or (b) document the throw points in the JSDoc on DmpApi.updateDocumentStatus so callers know they must still wrap in try/catch for non‑business failures. Given the PR explicitly invokes SearchResult as the model, option (a) would also retroactively improve search and yield a genuinely uniform error model.

Minor / not reported (below threshold)

• DmpApiError instantiated only for .message (DmpNhrAdapter.ts:234) — confidence 55. Slight over‑engineering, but it is forward‑compatible (callers may later read apiErrorCode for telemetry without rework) and keeps formatting in one place. Not worth a change unless the DmpApiErrorInfo plumbing in concern 2 above is fixed.
• Test coverage for partial‑success ordering (DmpNhrAdapter.test.ts:623) …(truncated, 712 more chars)