Project: doctolib-doctolib · Branch: .invalid · Mode: ` · **Model:**claude-sonnet-4-6`
Stats: 1 user messages, 28 tool calls, tools used: Bash, Read.
Conversation
Turn 1 — User
Review the TypeScript/architecture patterns in PR branch fhir-2214_nhr-error-type-extension-and-modal-toast in /Users/USER/workspace/doctolib/treatment-plan.
Run git diff main...fhir-2214_nhr-error-type-extension-and-modal-toast to see all changes.
Key changed files:
- packages/tpf-national-health-records/src/business/types/api.ts — NhrDocumentSearchResult extended with error?: { message: string; code?: string }
- packages/tpf-national-health-records-ui/src/business/toast.ts (new) — follows tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.ts pattern
- packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx — adds try/catch with addToast and t() from #i18n
Reference patterns are at:
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.ts
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-prescription-fulfillments/src/business/eprescription/toast.ts
Please review for: TypeScript typing quality, naming conventions, separation of concerns, component structure, and any Doctolib UpFront architecture pattern violations.
Turn 1 — Assistant
I'll start by examining the git diff and reference files in parallel.
Tools used:
Bash:cd /Users/USER/workspace/doctolib/treatment-plan && git diff main...fhir-2214_nhr-error-type-extension-and-modal-toast 2>/dev/null || git diff origin/main...fhir-2214_nhr-error-type-extension-and …(truncated, 24 more chars)
Tool results:
→ result (ok): diff --git a/packages/tpf-national-health-records-ui/src/business/toast.ts b/packages/tpf-national-health-records-ui/src/business/toast.ts new file mode 100644 index 000000000..15715cdee --- /dev/null +++ b/packages/tpf-national-health-records-ui/src/business/toast.ts @@ -0,0 +1,11 @@ +import { GLOBAL_PUBSUB_REGISTRY } from "@doctolib/upfront-core"; + +type ToastProps = { + message: string; + title: string; + uiStyle: "info" | "success" | "error" | "warning"; +}; + +export function addToast(p …(truncated, 28512 more chars)
Now let me examine the reference files and the broader context of the changed files.
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.ts
Tool results:
→ result (ok): 1 /*
2 * Toast utility for the DMP consultation consent View.
3 *
4 * Dispatches toast notifications via the global pub/sub registry.
5 * The monolith listens for 'toasts:add' events and renders the UI.
6 /
7 import { GLOBAL_PUBSUB_REGISTRY } from '@doctolib/upfront-core'
8
9 type ToastProps = {
10 message: React.ReactNode
11 title: string
12 uiStyle: 'info' | 'success' | 'error' | 'warning'
13 action?: { message: string; onClick: () => void }
14 }
15
16 export function addToast
…(truncated, 106 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-prescription-fulfillments/src/business/eprescription/toast.ts
Tool results:
→ result (ok): 1 import { GLOBAL_PUBSUB_REGISTRY } from '@doctolib/upfront-core'
2
3 export type ToastProps = {
4 message: React.ReactNode
5 title: string
6 uiStyle: 'info' | 'success' | 'error' | 'warning'
7 action?: { message: string; onClick: () => void }
8 }
9
10 export const addToast = (props: ToastProps) => {
11 GLOBAL_PUBSUB_REGISTRY.dispatch({ type: 'toasts:add', payload: props })
12 }
13
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/toast.ts
Tool results:
→ result (ok): 1 import { GLOBAL_PUBSUB_REGISTRY } from "@doctolib/upfront-core";
2
3 type ToastProps = {
4 message: string;
5 title: string;
6 uiStyle: "info" | "success" | "error" | "warning";
7 };
8
9 export function addToast(props: ToastProps) {
10 GLOBAL_PUBSUB_REGISTRY.dispatch({ type: "toasts:add", payload: props });
11 }
12
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business
Tool results:
→ result (ERROR): EISDIR: illegal operation on a directory, read '/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business'
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/business/types/api.ts
Tool results:
→ result (ok): 1 import type {
2 DateRange,
3 NhrDocument,
4 NhrDocumentFile,
5 } from "#business/types/document";
6 import type { NhrPatientInformation } from "#business/types/participant";
7
8 export type NhrResultStatus =
9 | "success"
10 | "skipped"
11 | "error"
12 | "partial success";
13
14 type NhrResult = {
15 status: NhrResultStatus;
16 message?: string;
17 };
18
19 // Document Search
20 export type NhrDocumentSearchParams = {
21 formats?: string[];
22 categories?: string[];
23
…(truncated, 2099 more chars)
Tools used:
Bash:ls /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/
Tool results:
→ result (ok): factories formatters toast.ts
Tools used:
Bash:ls /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/
Tool results:
→ result (ok): tests business components dev entry.ts i18n.test.ts i18n.ts index.ts locales test-helpers types.ts utils
Tools used:
Bash:ls /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/business/types/
Tool results:
→ result (ok): api.ts document.ts index.ts participant.ts service.ts
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/index.ts
Tool results:
→ result (ok): 1 export { NhrDocumentList } from './components/NhrDocumentList/NhrDocumentList' 2 export { NhrDocumentListFilterPanel } from './components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel' 3 export type { 4 ActionConfig, 5 ActionsFactory, 6 ColumnConfig, 7 FilterConfig, 8 FilterRenderContext, 9 NhrPatientInfo, 10 NhrVisibilityEntry, 11 StatusOption, 12 NhrDocumentStatusFilterProps, 13 VisibilityOption, 14 SearchDocumentsFn, 15 } from './types' 16 export { NhrEntryPoin …(truncated, 2130 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/types.ts
Tool results:
→ result (ok): 1 import type { DesignSystemIconComponent } from '@doctolib/design-system-icons'
2 import type { NhrDocument } from '@doctolib/tpf-national-health-records'
3 import type { ReactNode } from 'react'
4
5 export type NhrPatientInfo = {
6 fullName: string
7 gender: 'male' | 'female' | 'other'
8 birthDate: string
9 age: number
10 }
11
12 export type ColumnConfig = {
13 key: string
14 header: string
15 sortable: boolean
16 /** Only required when sortable: true. Returns a string for lo
…(truncated, 2274 more chars)
Tools used:
Bash:ls /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/
Tool results:
→ result (ok): NhrDocumentListHeader NhrDocumentListModal.stories.tsx NhrDocumentListModal.test.tsx NhrDocumentListModal.tsx NhrDocumentListTable
Tools used:
Bash:ls /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/
Tool results:
→ result (ok): factories formatters toast.ts
Tools used:
Bash:ls /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/
Tool results:
→ result (ok): actions columns filters
Tools used:
Bash:grep -r "addToast\|toast" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/ --include="*.ts" --include="*.tsx" -l
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/toast.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.test.tsx /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx
Tools used:
Bash:grep -r "NhrDocumentSearchResult" /Users/USER/workspace/doctolib/treatment-plan/packages/ --include="*.ts" --include="*.tsx" -l
Tools used:
Bash:grep -r "NhrDocumentSearchResult" /Users/USER/workspace/doctolib/treatment-plan/packages/ --include="*.ts" --include="*.tsx" -n
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/dist/index.d.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/dist/business/types/index.d.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/dist/business/types/service.d.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/dist/business/types/document.d.ts /Users/USER/workspace/doctolib/treatment-plan/packages …(truncated, 792 more chars)
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/dist/index.d.ts:3:export type { NhrDocumentSearchProps, NhrDocumentSearchResult, NhrDocument, NhrServiceInformation, NhrDocumentSearchParams, NhrDocumentClassification, DateRange, Code, } from '#business/types/document'; /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/dist/business/types/document.d.ts:33:export type NhrDocumentSearchResult = { /Users/USER/workspace/doctolib/treat …(truncated, 3314 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/business/types/service.ts
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/business/systems/dmp/services/dmp-nhr-service.ts
Tool results:
→ result (ok): 1 import type {
2 NhrDocumentContentResult,
3 NhrDocumentMetadataUpdateProps,
4 NhrDocumentMetadataUpdateResult,
5 NhrDocumentSearchProps,
6 NhrDocumentSearchResult,
7 NhrGetDocumentsContentProps,
8 } from '#business/types/api'
9 import type { NhrAdapter, NhrService } from '#business/types/service'
10
11 export class DmpNhrService implements NhrService {
12 readonly country = 'fr'
13
14 constructor(readonly nhrAdapter: NhrAdapter) {}
15
16 searchDocuments = (props: NhrDocume
…(truncated, 435 more chars)
Tool results:
→ result (ok): 1 import type {
2 NhrDocumentContentResult,
3 NhrDocumentMetadataUpdateProps,
4 NhrDocumentMetadataUpdateResult,
5 NhrDocumentSearchProps,
6 NhrDocumentSearchResult,
7 NhrGetDocumentsContentProps,
8 } from '#business/types/api'
9
10 export type CountryCode = 'fr' | 'mock'
11
12 export type NhrService = {
13 searchDocuments(props: NhrDocumentSearchProps): Promise<NhrDocumentSearchResult>
14 getDocumentsContent(props: NhrGetDocumentsContentProps): Promise<NhrDocumentContentResult
…(truncated, 452 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/tests/business/systems/dmp/services/dmp-nhr-service.test.ts
Tool results:
→ result (ok): 1 import { DmpNhrService } from '#business/systems/dmp/services/dmp-nhr-service'
2 import type {
3 NhrDocumentContentResult,
4 NhrDocumentMetadataUpdateProps,
5 NhrDocumentMetadataUpdateResult,
6 NhrDocumentSearchProps,
7 NhrDocumentSearchResult,
8 NhrGetDocumentsContentProps,
9 } from '#business/types/index'
10 import type { NhrAdapter } from '#business/types/service'
11 import { describe, expect, it, vi } from 'vitest'
12
13 const createMockAdapter = (): NhrAdapter => ({
14 sear
…(truncated, 4833 more chars)
Tools used:
Bash:grep -r "error" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/ --include="*.ts" -n | grep -v "//\|/*\|*/" | head -40
Tool results:
→ result (ok): ugrep: error: error at position 12 (?m)//|/|/ ___empty (sub)expression
Tools used:
Bash:grep -rn "\.error" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/ --include="*.ts" | head -20
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/business/types/index.ts
Tool results:
→ result (ok): 1 export type { 2 NhrDocument, 3 DateRange, 4 NhrDocumentClassification, 5 NhrServiceInformation, 6 Code, 7 NhrDocumentFile, 8 } from './document' 9 export type { 10 NhrResultStatus, 11 NhrDocumentSearchParams, 12 NhrDocumentSearchProps, 13 NhrDocumentSearchResult, 14 NhrGetDocumentsContentParams, 15 NhrGetDocumentsContentProps, 16 NhrDocumentContentResult, 17 NhrDocumentMetadataUpdatePayload, 18 NhrDocumentMetadataUpdateParams, 19 NhrDocumentMetadataUpdateAdditio …(truncated, 391 more chars)
Tools used:
Bash:grep -rn "NhrDocumentSearchResult\|searchDocuments" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/ --include="*.ts" --include="*.tsx" | head -30
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Bash:find /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src -name "*.ts" -o -name "*.tsx" | head -20
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/i18n.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/index.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/locales/loadNamespace.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/hooks/project-if-changed.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/hooks/use-dmp-consultation-consent.test.tsx /Users/USER/workspace/doctolib/treat …(truncated, 1654 more chars)
Tools used:
Bash:grep -rn "error\b" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/business/ --include="*.ts" | grep -v "test\|spec" | head -30
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/business/types/api.ts:11: | "error" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/business/types/api.ts:38: error?: {
Tools used:
Bash:grep -rn "\.error\b" /Users/USER/workspace/doctolib/treatment-plan/ --include="*.ts" --include="*.tsx" | grep -v "dist\|node_modules\|__snapshots__" | head -20
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/.claude/worktrees/stupefied-lewin/.claude/rules/testing/templates/hook-test.tsx:68: expect(result.current.error).toEqual(new Error('Not found')) /Users/USER/workspace/doctolib/treatment-plan/.claude/worktrees/stupefied-lewin/packages/tpf-muster-forms/src/v2/DocumentFiller.tsx:145: onError: console.error, /Users/USER/workspace/doctolib/treatment-plan/.claude/worktrees/stupefied-lewin/packages/tpf-muster-forms/src/v2/hooks/useMusterPrint. …(truncated, 3658 more chars)
Tools used:
Bash:cat /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx
Tool results:
→ result (ok): import { Modal } from "@doctolib/design-system/Modal"; import { ModalBody } from "@doctolib/design-system/ModalBody"; import type { NhrDocument } from "@doctolib/tpf-national-health-records"; import { useEffect, useMemo, useState } from "react";
import { addToast } from "../../business/toast"; import type { ActionConfig, ColumnConfig, FilterConfig, NhrPatientInfo, SearchDocumentsFn, } from "../../types"; import { NhrDocumentList } from "../NhrDocumentList/NhrDocumentList"; import { t …(truncated, 2581 more chars)
Now I have a thorough picture of all the changes. Let me compile the review.
Architecture Review: fhir-2214_nhr-error-type-extension-and-modal-toast
Critical Issues (must fix)
1. toast.ts is missing the action prop — type is narrower than the established contract
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/toast.ts
Both reference implementations define an optional action field on ToastProps:
// tpf-nhr-fr and tpf-prescription-fulfillments both have:
action?: { message: string; onClick: () => void }
The new toast.ts omits it entirely. This means any future caller in this package that needs an actionable toast will silently produce a type error or be forced to cast. The ToastProps type should match the shape the pub/sub consumer expects. At minimum, add the optional field to keep the door open without breaking the current call site.
2. message: string instead of message: React.ReactNode breaks the established contract
Same file. Both reference patterns type message as React.ReactNode, which is what the upstream toasts:add consumer renders. Narrowing to string means rich-content toasts cannot be dispatched from this package even if needed later, and creates a divergence from the platform contract. Use React.ReactNode to match.
3. import { t } from "#i18n" is placed after a relative import — violates import ordering
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx
The #i18n alias import appears after the relative ../../business/toast and ../NhrDocumentList/NhrDocumentList imports. ESM alias imports (# prefix) are still module-level imports and must follow the project's ESLint import ordering rules (external packages → path aliases → relative imports). The current order is:
import { addToast } from "../../business/toast"; // relative — should be last group
import { ... } from "../../types"; // relative
import { NhrDocumentList } from "../NhrDocumentList/NhrDocumentList"; // relative
import { t } from "#i18n"; // alias — should come before relative
This will trigger the import/order ESLint rule. The alias import must move above the relative imports.
Warnings (should fix)
4. NhrDocumentSearchResult.error is never consumed by any caller
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/business/types/api.ts
The error field is added to NhrDocumentSearchResult, but the component catches thrown exceptions, not a resolved-but-errored result shape. No code in this PR, nor in dmp-nhr-service.ts, reads result.error. The field is orphaned at this point — it exists on the type but is never populated or checked.
This creates two competing error models for the same operation: throw-on-error (used by NhrDocumentListModal) and error-in-result (the new field). Mixing these two patterns for the same operation is a maintenance hazard. If the intent is to support partial-success scenarios where a result is returned alongside an error (e.g., the DMP returned documents but also signalled a degraded state), that semantic distinction needs to be documented with a JSDoc comment and the field should be consumed somewhere. If the intent was simply to convey thrown errors to the UI, the field should be removed — thrown exceptions already handle that case.
5. ToastProps type is not exported from toast.ts
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/toast.ts
The tpf-prescription-fulfillments reference exports ToastProps (export type ToastProps). The tpf-nhr-fr reference keeps it private. Given this module lives in a shared src/business/ layer that may be called from multiple components in the same package, exporting ToastProps keeps callers from re-declaring the shape if they need to build toast payloads programmatically. This is a judgment call, but consistency with the more explicitly typed reference pattern (tpf-prescription-fulfillments) is preferable.
6. Error fallback in NhrDocumentListModal uses errorTitle as the message when the thrown value is not an Error
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx, lines 70–77.
message: error instanceof Error ? error.message : errorTitle,
When the thrown value is not an Error instance, the toast title and message fields become identical, which is redundant and likely not the intended UX. A more appropriate fallback would be an empty string, a generic "An unexpected error occurred." string (ideally i18n-keyed), or simply omitting message if the ToastProps type allows it. Repeating the title in the message slot is a UX defect.
Suggestions (consider improving)
7. total: 0 on error is a semantic mis-statement
In the catch block:
setDocumentsState({ rows: [], total: 0, isLoading: false });
Setting total to 0 implies zero results were found, which is different from "we do not know the result count because the request failed". The existing initial state uses undefined for this reason (total: undefined). Reset to undefined on error to be consistent with the "unknown" semantic.
8. fetchDocuments can be extracted into a useCallback or a named hook
The useEffect in NhrDocumentListModal is doing three things: setting loading state, fetching, and dispatching a toast side effect. This is the kind of logic that, as it grows, should live in a NhrDocumentListModal.hooks.ts file per the Josh Comeau component structure. This is not a blocker for the PR, but the component is approaching the threshold where extraction becomes warranted.
9. Quote style normalization is unrelated to the feature
The diff includes wholesale conversion of single-quoted strings to double-quoted strings across NhrDocumentListModal.test.tsx. This is a legitimate formatter change (Prettier enforcement), but it makes the diff significantly harder to read. Such formatting-only changes are better landed as a separate commit or PR to keep the feature-related signal clean.