Project: doctolib-doctolib · Branch: .invalid · Mode: ` · **Model:**claude-sonnet-4-6`
Stats: 1 user messages, 11 tool calls, tools used: Read, Bash.
Conversation
Turn 1 — User
Final architecture review for PR branch fhir-2245_observability-search-documents in the Doctolib monorepo at /Users/USER/workspace/doctolib/doctolib. Previous concerns resolved: named intermediate variable in useDmpNhr.ts, patient_ins removed, PII regression guard added, process.nextTick flush for fire-and-forget tests.
Review these files:
1. packages/@doctolib/national-health-records/src/business/adapters/nhrMonitoring.ts
2. packages/@doctolib/national-health-records/src/business/adapters/NhrAdapterWithObservability.ts
3. packages/@doctolib/national-health-records/src/business/adapters/NhrAdapterWithObservability.test.ts
4. packages/@doctolib/national-health-records/src/business/adapters/nhrMonitoring.test.ts
5. packages/@doctolib/national-health-records/src/hooks/useDmpNhr.ts
Known intentional: NhrAdapterWithObservability NOT exported from index.ts (internal wiring), NhrRequestActionFields/NhrResponseActionFields exported (required for import from NhrAdapterWithObservability.ts in same directory — they cannot be package-private without merging files), DmpNhrAdapter exported from index.ts is pre-existing and out of scope for this PR.
Report only NEW architecture concerns not already addressed, or say "no architecture concerns".
Turn 1 — Assistant
I'll read all five files in parallel to perform the review.
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/nhrMonitoring.ts
Tool results:
→ result (ok): 1 import { sendMonitoringEvent } from 'monolith_DEPRECATED/services/monitoring'
2 import type { NhrResultStatus } from '@doctolib/tpf-national-health-records'
3
4 export type NhrAdapterName = 'dmp'
5 // Extend when adding adapters: | 'germany-kim' | 'italy-fse'
6
7 export type NhrAdapterAction = 'search_documents'
8 // Extend in follow-up increments: | 'get_documents_content' | 'update_document_metadata'
9
10 // ── Infrastructure base types ──
11
12 type NhrRequestBase = {
13 trace_id: str
…(truncated, 2536 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/NhrAdapterWithObservability.ts
Tool results:
→ result (ok): 1 import type { 2 NhrAdapter, 3 NhrDocumentSearchProps, 4 NhrDocumentSearchResult, 5 NhrGetDocumentsContentProps, 6 NhrDocumentContentResult, 7 NhrDocumentMetadataUpdateProps, 8 NhrDocumentMetadataUpdateResult, 9 } from '@doctolib/tpf-national-health-records' 10 import { sendNhrRequest, sendNhrResponse } from './nhrMonitoring' 11 import type { 12 NhrAdapterName, 13 NhrAdapterAction, 14 NhrRequestDetails, 15 NhrResponseDetails, 16 NhrRequestActionFields, 17 NhrResponseAc …(truncated, 3600 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/NhrAdapterWithObservability.test.ts
Tool results:
→ result (ok): 1 import { sendMonitoringEvent } from 'monolith_DEPRECATED/services/monitoring'
2 import type { NhrAdapter, NhrDocumentSearchProps } from '@doctolib/tpf-national-health-records'
3 import { NhrAdapterWithObservability } from './NhrAdapterWithObservability'
4
5 jest.mock('monolith_DEPRECATED/services/monitoring', () => ({
6 sendMonitoringEvent: jest.fn().mockResolvedValue(undefined),
7 }))
8
9 const mockSendMonitoringEvent = sendMonitoringEvent as jest.MockedFunction<typeof sendMonitoringEvent
…(truncated, 12998 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/nhrMonitoring.test.ts
Tool results:
→ result (ok): 1 import { sendMonitoringEvent } from 'monolith_DEPRECATED/services/monitoring'
2 import { sendNhrRequest, sendNhrResponse } from './nhrMonitoring'
3
4 jest.mock('monolith_DEPRECATED/services/monitoring', () => ({
5 sendMonitoringEvent: jest.fn().mockResolvedValue(undefined),
6 }))
7
8 const mockSendMonitoringEvent = sendMonitoringEvent as jest.MockedFunction<typeof sendMonitoringEvent>
9
10 beforeEach(() => {
11 jest.clearAllMocks()
12 mockSendMonitoringEvent.mockResolvedValue(undefine
…(truncated, 2521 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/hooks/useDmpNhr.ts
Tool results:
→ result (ok): 1 import { useMemo } from 'react' 2 import type { Patient } from 'monolith_DEPRECATED/types/doctor/api/Patient' 3 import { DmpApiImpl as DmpApi, useDmpSetUp } from '@doctolib/dmp' 4 import { createNhrService } from '@doctolib/tpf-national-health-records' 5 import { useDocumentCategorizationFacetsIndex } from 'engines/document_categorization/queries/document_categorization/facets' 6 import { DmpNhrAdapter } from '../business/adapters/DmpNhrAdapter' 7 import { NhrAdapterWithObservability } from '. …(truncated, 950 more chars)
Now let me check the src/business/adapters/ directory structure and the DmpNhrAdapter.ts diff to understand the full picture.
Tools used:
Bash:ls /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/
Tool results:
→ result (ok): DmpNhrAdapter.test.ts DmpNhrAdapter.ts NhrAdapterWithObservability.test.ts NhrAdapterWithObservability.ts nhrMonitoring.test.ts nhrMonitoring.ts parseHl7DateToIso.test.ts parseHl7DateToIso.ts
Tools used:
Bash:git -C /Users/USER/workspace/doctolib/doctolib diff origin/master...HEAD -- packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts 2>/dev/null | head -60
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Bash:git -C /Users/USER/workspace/doctolib/doctolib log --oneline origin/master..HEAD 2>/dev/null | head -10
Tool results:
→ result (ok): 0f11f4fd21ca refactor(FHIR-2245): simplify send helpers, add test coverage for edge cases af5a0dbf3ab1 refactor(FHIR-2245): remove patient_ins pending DPO sign-off, improve test coverage 81e3037ead18 test(FHIR-2245): add missing test coverage for NHR adapter observability 2d9b1ae4f932 feat(FHIR-2245): add Datadog observability to NHR adapter searchDocuments
Tools used:
Bash:git -C /Users/USER/workspace/doctolib/doctolib diff origin/master..HEAD -- packages/@doctolib/national-health-records/src/ 2>/dev/null | head -20
Tool results:
→ result (ok): diff --git a/packages/@doctolib/national-health-records/src/business/adapters/NhrAdapterWithObservability.test.ts b/packages/@doctolib/national-health-records/src/business/adapters/NhrAdapterWithObservability.test.ts new file mode 100644 index 000000000000..8506796acad5 --- /dev/null +++ b/packages/@doctolib/national-health-records/src/business/adapters/NhrAdapterWithObservability.test.ts @@ -0,0 +1,296 @@ +import { sendMonitoringEvent } from 'monolith_DEPRECATED/services/monitoring' +import typ …(truncated, 508 more chars)
Tools used:
Bash:grep -n "NhrAdapterWithObservability\|NhrRequestActionFields\|NhrResponseActionFields\|nhrMonitoring\|NhrAdapterName\|NhrAdapterAction\|sendNhrRequest\|sendNhrResponse" /Users/USER/workspace/doct …(truncated, 81 more chars)
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Bash:grep -rn "NhrAdapterWithObservability\|nhrMonitoring\|NhrRequestActionFields\|NhrResponseActionFields" /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/ --in …(truncated, 51 more chars)
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/nhrMonitoring.ts:45:// Used by observe().execute() inside NhrAdapterWithObservability. /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/nhrMonitoring.ts:50:export type NhrRequestActionFields = { /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/nhrMonitoring.ts:54:export type NhrResp …(truncated, 2156 more chars)
I now have a complete picture. Here is the review.
No architecture concerns.
The five files are consistent with UpFront rules:
Package structure / file placement. All new files sit inside src/business/adapters/ (pure-logic layer) or src/hooks/ (hook layer), both correct subdirectories. Test files are colocated alongside their source counterparts, not in a separate tests/ directory.
Naming conventions. nhrMonitoring.ts and nhrMonitoring.test.ts follow kebab-style for multi-export utility files. NhrAdapterWithObservability.ts and its test use PascalCase matching the exported class name. Both are correct.
Business/data layer separation. nhrMonitoring.ts and NhrAdapterWithObservability.ts contain no React imports and no data access — pure domain logic, correct for src/business/. useDmpNhr.ts is in src/hooks/ and is the appropriate place to instantiate and wire the class; that composition boundary is clean.
Export conventions. NhrAdapterWithObservability carries export class but is intentionally not re-exported from src/index.ts (acknowledged internal wiring). NhrRequestActionFields / NhrResponseActionFields are exported from nhrMonitoring.ts only to satisfy the same-directory import in NhrAdapterWithObservability.ts; the acknowledged justification stands. No default exports.
Import depth. useDmpNhr.ts reaches the adapter with '../business/adapters/NhrAdapterWithObservability' — two ../ segments, below the three-segment threshold that would require a # alias. Correct.
Test file conventions. Both test files mock at the infrastructure boundary (monolith_DEPRECATED/services/monitoring), not inside the unit under test. The process.nextTick flush for fire-and-forget rejection paths is present and correct.