Project: workspace-doctolib · Branch: HEAD · Mode: ` · **Model:**claude-opus-4-7`
Stats: 1 user messages, 22 tool calls, tools used: Bash, Read.
Conversation
Turn 1 — User
Review the code changes in PR https://github.com/doctolib/treatment-plan/pull/3129
Context: This PR fixes a pub/sub event name mismatch. Three TPF toast helper files dispatched 'toasts:add' via GLOBAL_PUBSUB_REGISTRY, but the monolith's Toaster component subscribes to 'toaster:add'. The fix is a one-line change per file, plus new unit tests.
Changed files (all in /Users/USER/workspace/doctolib/treatment-plan on branch fhir-2233-fix-toast-event-name): 1. packages/tpf-national-health-records-ui/src/business/toast.ts 2. packages/tpf-national-health-records-ui/src/business/toast.test.ts 3. packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.ts 4. packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.test.ts 5. packages/tpf-prescription-fulfillments/src/business/eprescription/toast.ts 6. packages/tpf-prescription-fulfillments/src/business/eprescription/toast.test.ts
Review for: correctness, style compliance, CLAUDE.md guidelines, testing quality. Focus on any issues with the test approach (mocking GLOBAL_PUBSUB_REGISTRY) and whether the fix is complete.
Turn 1 — Assistant
I'll review the PR changes for the toast event name fix. Let me start by exploring the repository structure and the changed files.
Tools used:
Bash:git status && git branch --show-current
Tool results:
→ result (ok): On branch fhir-2233-fix-toast-event-name Your branch is up to date with 'origin/fhir-2233-fix-toast-event-name'.
nothing to commit, working tree clean fhir-2233-fix-toast-event-name
I'll review the PR by examining all changed files. Since plan mode is active, I'll only perform read-only operations.
Tools used:
Bash:git log --oneline -10 && echo "---" && git diff main...HEAD --stat
Tool results:
→ result (ok): 9e0bfd841 fix(fhir-2233): align toast pub/sub event name with monolith toaster eacc8d489 chore(module-federation): sync versions after release 70f4577de chore: release main (#3114) cd8f9aed7 feat(CAPY-1516): backend scaffold + acute template + frontend playground (1/4) (#3079) 77e07cda1 chore: Sync i18n translations from Phrase (#3126) 9ab4ea8a4 chore(deps): update dependency @stoplight/spectral-cli to ^6.15.1 (#3125) 49a7cf4c6 chore(deps): update dependency @rsbuild/plugin-sass to ^1.5.2 (#3124 …(truncated, 752 more chars)
Tools used:
Bash:git diff main...HEAD
Tool results:
→ result (ok): diff --git a/packages/tpf-national-health-records-ui/src/business/toast.test.ts b/packages/tpf-national-health-records-ui/src/business/toast.test.ts new file mode 100644 index 000000000..0da6c1b6e --- /dev/null +++ b/packages/tpf-national-health-records-ui/src/business/toast.test.ts @@ -0,0 +1,44 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { addToast } from './toast' + +vi.mock('@doctolib/upfront-core', () => ({ + GLOBAL_PUBSUB_REGISTRY: { + dispatch: vi.fn(),
…(truncated, 5159 more chars)
Now let me check the project structure, CLAUDE.md guidelines, and the actual files for context.
Tools used:
Bash:find /Users/USER/workspace/doctolib/treatment-plan -name "CLAUDE.md" -not -path "*/node_modules/*" 2>/dev/null | head -20
Tools used:
Bash:ls /Users/USER/workspace/doctolib/treatment-plan/
Tool results:
→ result (ok): AGENTS.md backend bin build catalog-info.yaml CLAUDE.md CODEOWNERS deployment dist doc docker docker-compose.yml Dockerfile eslint.config.js mise.toml mkdocs.yml module-federation.json mvnw mvnw.cmd node_modules nx.json openspec package-lock.json package.json packages pom.xml postcss.config.js prettier.config.js README.md release-please-config.json renovate.json5 service.datadog.yaml sonar-project.properties src tailwind.config.js target tests tpf.code-workspace tsconfig-base.json tsconfig-desig …(truncated, 144 more chars)
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/CLAUDE.md /Users/USER/workspace/doctolib/treatment-plan/backend/cda-validator/CLAUDE.md /Users/USER/workspace/doctolib/treatment-plan/.claude/worktrees/stupefied-lewin/CLAUDE.md /Users/USER/workspace/doctolib/treatment-plan/.claude/worktrees/friendly-lalande/CLAUDE.md
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/CLAUDE.md
Tool results:
→ result (ok): 1 # CLAUDE.md
2
3 This is a full-stack healthcare treatment plan application (Kotlin/Spring Boot backend + React/TypeScript frontend) built as a modular monolith using the "hive" pattern with hexagonal architecture. It handles prescription creation, document management, and clinical workflows for the French and German markets.
4
5 Refer to @AGENTS.md for complete project overview, architecture, tech stack, and development guidelines.
6
7 ## Language Rule
8
9 All Claude stack files (C
…(truncated, 664 more chars)
Now let me examine the actual files in the PR to verify the fix.
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 import type { ReactNode } from 'react'
3
4 type ToastProps = {
5 message: ReactNode
6 title: string
7 uiStyle: 'info' | 'success' | 'error' | 'warning'
8 action?: { message: string; onClick: () => void }
9 }
10
11 export function addToast(props: ToastProps) {
12 GLOBAL_PUBSUB_REGISTRY.dispatch({ type: 'toaster:add', payload: props })
13 }
14
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/toast.test.ts
Tool results:
→ result (ok): 1 import { afterEach, describe, expect, it, vi } from 'vitest'
2
3 import { addToast } from './toast'
4
5 vi.mock('@doctolib/upfront-core', () => ({
6 GLOBAL_PUBSUB_REGISTRY: {
7 dispatch: vi.fn(),
8 },
9 }))
10
11 describe('addToast', () => {
12 afterEach(() => {
13 vi.clearAllMocks()
14 })
15
16 it('dispatches toaster:add event with the given props', async () => {
17 const { GLOBAL_PUBSUB_REGISTRY } = await import('@doctolib/upfront-core')
18 const props = { title
…(truncated, 773 more chars)
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 import { GLOBAL_PUBSUB_REGISTRY } from '@doctolib/upfront-core'
2
3 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 function addToast(props: ToastProps) {
11 GLOBAL_PUBSUB_REGISTRY.dispatch({ type: 'toaster:add', payload: props })
12 }
13
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.test.ts
Tool results:
→ result (ok): 1 import { afterEach, describe, expect, it, vi } from 'vitest'
2
3 import { addToast } from './toast'
4
5 vi.mock('@doctolib/upfront-core', () => ({
6 GLOBAL_PUBSUB_REGISTRY: {
7 dispatch: vi.fn(),
8 },
9 }))
10
11 describe('addToast', () => {
12 afterEach(() => {
13 vi.clearAllMocks()
14 })
15
16 it('dispatches toaster:add event with the given props', async () => {
17 const { GLOBAL_PUBSUB_REGISTRY } = await import('@doctolib/upfront-core')
18 const props = { title
…(truncated, 254 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: 'toaster:add', payload: props })
12 }
13
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-prescription-fulfillments/src/business/eprescription/toast.test.ts
Tool results:
→ result (ok): 1 import { afterEach, describe, expect, it, vi } from 'vitest'
2
3 import { addToast } from './toast'
4
5 vi.mock('@doctolib/upfront-core', () => ({
6 GLOBAL_PUBSUB_REGISTRY: {
7 dispatch: vi.fn(),
8 },
9 }))
10
11 describe('addToast', () => {
12 afterEach(() => {
13 vi.clearAllMocks()
14 })
15
16 it('dispatches toaster:add event with the given props', async () => {
17 const { GLOBAL_PUBSUB_REGISTRY } = await import('@doctolib/upfront-core')
18 const props = { title
…(truncated, 254 more chars)
Now let me check if any other places in the codebase still use the old event name 'toasts:add' or other related event types that may need fixing.
Tools used:
Bash:grep -r "toasts:add" /Users/USER/workspace/doctolib/treatment-plan/packages /Users/USER/workspace/doctolib/treatment-plan/src 2>/dev/null --include="*.ts" --include="*.tsx" | head -20
Tools used:
Bash:grep -r "toaster:add\|toasts:add" /Users/USER/workspace/doctolib/treatment-plan/packages /Users/USER/workspace/doctolib/treatment-plan/src 2>/dev/null --include="*.ts" --include="*.tsx" | he …(truncated, 6 more chars)
Tools used:
Bash:grep -r "GLOBAL_PUBSUB_REGISTRY" /Users/USER/workspace/doctolib/treatment-plan/packages /Users/USER/workspace/doctolib/treatment-plan/src 2>/dev/null --include="*.ts" --include="*.tsx" | hea …(truncated, 5 more chars)
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/.storybook/preview.tsx: const unsubscribe = GLOBAL_PUBSUB_REGISTRY.subscribe('toasts:add', (event) => {
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/.storybook/preview.tsx: const unsubscribe = GLOBAL_PUBSUB_REGISTRY.subscribe('toasts:add', (event) => { /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.ts: GLOBAL_PUBSUB_REGISTRY.dispatch({ type: 'toaster:add', payload: props }) /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.test.ts: it('d …(truncated, 1601 more chars)
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/.storybook/preview.tsx:import { GLOBAL_PUBSUB_REGISTRY } from '@doctolib/upfront-core' /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/.storybook/preview.tsx: const unsubscribe = GLOBAL_PUBSUB_REGISTRY.subscribe('toasts:add', (event) => { /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.ts:import { GLOBAL_PUBSUB_REGISTRY } from '@doctolib/upfr …(truncated, 5020 more chars)
Important finding. There's a Storybook preview file still using the old 'toasts:add' event name. Also the tpf-french-teleservices may have another dispatcher. Let me check those.
Tools used:
Bash:grep -A 3 "GLOBAL_PUBSUB_REGISTRY.dispatch" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-french-teleservices/src/cps/authenticate-dmp-cps.ts | head -20
Tool results:
→ result (ok): GLOBAL_PUBSUB_REGISTRY.dispatch({ type: CPS_PIN_REQUIRED_EVENT, payload: { validate, resolve, reject }, })
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/.storybook/preview.tsx
Tool results:
→ result (ok): 1 import '@doctolib/design-system/design-system.scss'
2 import { GLOBAL_PUBSUB_REGISTRY } from '@doctolib/upfront-core'
3 import { action } from '@storybook/addon-actions'
4 import type { Preview } from '@storybook/react'
5 import { useEffect } from 'react'
6
7 import { DmpPatientFileContextProvider } from '../src/hooks'
8 import { getInstance } from '../src/i18n'
9
10 const i18n = getInstance()
11
12 let isI18nInitialized = false
13
14 const i18nInitLoader = async () => {
15 if (!isI18nIn
…(truncated, 1221 more chars)
Found the critical issue. The Storybook preview file in tpf-nhr-fr is still subscribing to 'toasts:add', but the toast.ts in the same package now dispatches 'toaster:add'. This means Storybook previews will silently break — the ToastListener will no longer log toasts to the Actions panel because it's listening to the old event name.
Let me also verify the file with import type { ReactNode } vs React.ReactNode discrepancy.
Tools used:
Bash:grep -n "React" /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 …(truncated, 66 more chars)
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-prescription-fulfillments/src/business/eprescription/toast.ts:4: message: React.ReactNode /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.ts:4: message: React.ReactNode
Tools used:
Bash:grep -l "jsx" packages/tpf-nhr-fr/tsconfig.json packages/tpf-prescription-fulfillments/tsconfig.json 2>/dev/null
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Bash:cat /Users/USER/workspace/doctolib/treatment-plan/tsconfig-base.json | head -40
Tool results:
→ result (ok): { "extends": ["@doctolib/upfront-dev/tsconfig.json"], "compilerOptions": { // Configuration for TS project references "rootDir": ".", "outDir": "build", "composite": true, "noEmit": false, "declaration": true, // need to set this back to false to allow noEmit: false "allowImportingTsExtensions": false, // was needed for webpack build "jsx": "react-jsx",
// ignore errors due to design system
"exactOptionalPropertyTypes": false,
"types": ["
…(truncated, 653 more chars)
Note: React.ReactNode is used without an explicit import * as React or import type { ReactNode } from 'react'. With jsx: "react-jsx", React doesn't need to be imported for JSX, but using React.ReactNode as a value reference does require the import. This was a pre-existing issue (not introduced by this PR), since the diff shows those lines were already using React.ReactNode.
Let me also confirm by looking at the original toast.ts content (before diff) to verify what was pre-existing.
Tools used:
Bash:git show main:packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.ts 2>&1 | head -20
Tool results:
→ result (ok): /* * Toast utility for the DMP consultation consent View. * * Dispatches toast notifications via the global pub/sub registry. * The monolith listens for 'toasts:add' events and renders the UI. / import { GLOBAL_PUBSUB_REGISTRY } from '@doctolib/upfront-core'
type ToastProps = { message: React.ReactNode title: string uiStyle: 'info' | 'success' | 'error' | 'warning' action?: { message: string; onClick: () => void } }
export function addToast(props: ToastProps) { GLOBAL_PUBSUB_R …(truncated, 58 more chars)
Tools used:
Bash:git show main:packages/tpf-prescription-fulfillments/src/business/eprescription/toast.ts 2>&1 | head -20
Tool results:
→ result (ok): import { GLOBAL_PUBSUB_REGISTRY } from '@doctolib/upfront-core'
export type ToastProps = { message: React.ReactNode title: string uiStyle: 'info' | 'success' | 'error' | 'warning' action?: { message: string; onClick: () => void } }
export const addToast = (props: ToastProps) => { GLOBAL_PUBSUB_REGISTRY.dispatch({ type: 'toasts:add', payload: props }) }
Confirmed: the React.ReactNode usage was pre-existing. The PR did not introduce this issue, so I won't flag it.
Now let me check the test props to ensure they typecheck against the ToastProps type. Looking at the test, it passes { title, message, uiStyle } but ToastProps requires message: ReactNode | React.ReactNode. A string is a valid ReactNode, so that's fine. Let me also verify the comment removal is clean.
Now let me write up my review. Here are my findings:
Code Review: PR #3129 — fix(fhir-2233): align toast pub/sub event name with monolith toaster
Files reviewed:
- /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/business/toast.test.ts
- /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-nhr-fr/src/components/dmp-consultation-consent-view/toast.test.ts
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-prescription-fulfillments/src/business/eprescription/toast.ts
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-prescription-fulfillments/src/business/eprescription/toast.test.ts
Critical (90-100)
1. Storybook listener still subscribes to old event name — preview broken in tpf-nhr-fr — Confidence: 95
- File:
/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/.storybook/preview.tsx:31 - Issue: The fix in
packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.tsnow dispatches'toaster:add', but the colocated Storybook preview decorator is still subscribing to'toasts:add': ```tsx const unsubscribe = GLOBAL_PUBSUB_REGISTRY.subscribe('toasts:add', (event) => { logToast(event.payload) })
The `ToastListener` decorator was specifically designed to catch what the monolith would catch (per its inline comment). Now Storybook will silently log nothing in the Actions panel when toasts are dispatched — DevEx regression and the design intent of mirroring the monolith subscription is lost. Update the subscribe call to `'toaster:add'` so the preview stays aligned with the new dispatch type and with the monolith.
### Important (80-89)
**2. Stale comment removed without explanation, while new dispatch type is now undocumented** — Confidence: 80
- File: `/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.ts:1-6` (before diff)
- The PR removes the JSDoc block that explicitly stated which event the monolith subscribes to. That comment was the load-bearing piece of context that made the original bug discoverable. Suggest replacing it with an updated version, e.g.:
```ts
/**
* Toast utility for the DMP consultation consent View.
*
* Dispatches toast notifications via the global pub/sub registry.
* The monolith Toaster component subscribes to 'toaster:add'.
*/
This both prevents the same regression (someone changing the event name back) and gives reviewers an obvious cross-reference to validate against the monolith.
3. Tests assert a coupling to a magic string but never centralize/share it — Confidence: 82
- Files:
/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/toast.test.ts:23,40/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.test.ts:23/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-prescription-fulfillments/src/business/eprescription/toast.test.ts:23- The string
'toaster:add'is now duplicated in 6 places (3 source + 3 test files). The whole bug being fixed here is "string mismatch between producer and consumer." Tests as written only validate the producer matches itself — they will never catch a future drift from the monolith side. Consider: - extracting
const TOASTER_ADD_EVENT = 'toaster:add'(shared intpf-commonsor a single source-of-truth module that the monolith can also consume), or - at minimum, defining the constant once per module and importing it into the test (so the test still asserts the literal string but not by re-typing it). This is the more elegant solution called for by the project's "Demand Elegance (Balanced)" principle when a fix touches a non-trivial coupling.
4. Test props in tpf-prescription-fulfillments and tpf-nhr-fr cover only one scenario and miss the optional-action branch — Confidence: 80
- Files:
/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.test.ts/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-prescription-fulfillments/src/business/eprescription/toast.test.ts- Only
tpf-national-health-records-uihas the second test covering the optionalactionprop. The other two packages have the sameToastPropsshape (includingaction?), but their tests stop at the simple case. Per.claude/rules/testing/devex.md, the goal is meaningful behavior coverage, not test count — but here the three helpers are the same code with different homes, so the coverage should be uniform. Either: - mirror the second test (
with optional action prop) into the other two files, or - collapse the three duplicates into one shared helper + one shared test (preferred — eliminates duplication entirely).
Notes (informational, not blocking)
- The fix itself (
'toasts:add'→'toaster:add') is correct, minimal, and the right change per the PR description. - All three test files use a clean
vi.mockpattern withafterEach(() => vi.clearAllMocks())— consistent with.claude/rules/frontend/react/test/testing-patterns.md. import type { ReactNode }is used intpf-national-health-records-ui/src/business/toast.ts(good), while the other two files still useReact.ReactNodewithout importing React. That latter pattern is pre-existing onmain, not introduced by this PR — out of scope here, but worth a follow-up.- No CLAUDE.md violations (no
eslint-disable, noany, no snapshot tests, no arbitrary waits, no CSS selectors). - Conventional Commits format on the commit message is correct:
fix(fhir-2233): ....
Recommendation
Request changes for items 1 (definitely) and 3 (strongly preferred). The other two are nice-to-haves that materially improve maintenance characteristics. Item 1 is the only one that breaks behavior in a real environment (Storybook), and it should be fixed in this same PR since both the dispatch and the subscribe live in the same package.