Project: workspace-doctolib · Branch: HEAD · Mode: ` · **Model:**claude-sonnet-4-6`
Stats: 1 user messages, 13 tool calls, tools used: Read, Bash.
Conversation
Turn 1 — User
Review the following files in the treatment-plan repo for UpFront architecture compliance. Focus on separation of concerns, naming conventions, component structure, and anti-patterns.
Files to review: 1. /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/MultiSelectDropdown.tsx 2. /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx 3. /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/hooks.ts 4. /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/helpers.ts 5. /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx
Context: This is a multi-select dropdown for filtering document categories in the French DMP (national health records). It was recently refactored to extract the MultiSelectDropdown into its own file and rename helpers/hooks files to drop the component name prefix. The component uses Oxygen design system Checkbox/v2 inside a custom fieldset dropdown, with a custom hook for click-outside and Escape-key handling.
Turn 1 — Assistant
I'll read all five files in parallel to begin the review.
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/MultiSelectDropdown.tsx
Tool results:
→ result (ok): 1 import { ChevronDownRegularIcon } from '@doctolib/design-system-icons/ChevronDownRegularIcon'
2 import { Checkbox } from '@doctolib/design-system/Checkbox/v2'
3 import { IconButton } from '@doctolib/design-system/IconButton/v2'
4 import type { Option } from '@doctolib/design-system/Option'
5 import { useId, useMemo, useRef, useState } from 'react'
6
7 import { normalize } from './helpers'
8 import { useDropdownClose } from './hooks'
9
10 export type MultiSelectDropdownProps = {
11 label: s
…(truncated, 4058 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx
Tool results:
→ result (ok): 1 import type { ComponentProps } from 'react'
2
3 import { MultiSelectDropdown } from './MultiSelectDropdown'
4
5 export type NhrDocumentCategoryFilterProps = ComponentProps<typeof MultiSelectDropdown>
6
7 export const NhrDocumentCategoryFilter = (props: NhrDocumentCategoryFilterProps) => <MultiSelectDropdown {...props} />
8
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/hooks.ts
Tool results:
→ result (ok): 1 import { type RefObject, useEffect, useRef } from 'react'
2
3 export const useDropdownClose = (
4 containerRef: RefObject<HTMLDivElement | null>,
5 onClose: () => void,
6 isOpen: boolean
7 ) => {
8 const onCloseRef = useRef(onClose)
9 // Sync ref so the stable event listeners always call the latest onClose.
10 useEffect(() => {
11 onCloseRef.current = onClose
12 }, [onClose])
13
14 useEffect(() => {
15 if (!isOpen) return undefined
16
17 const handlePointerDown =
…(truncated, 625 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/helpers.ts
Tool results:
→ result (ok): 1 export const normalize = (s: string) => 2 s 3 .normalize('NFD') 4 .replace(/\p{Diacritic}/gu, '') 5 .toLowerCase() 6
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx
Tool results:
→ result (ok): 1 import { NhrDocumentCategoryFilter } from '#components/NhrDocumentList/NhrDocumentCategoryFilter'
2 import type { Option } from '@doctolib/design-system/Option'
3
4 import type { FilterConfig } from '../../../types'
5
6 export const createNhrDocumentCategoryFilterConfig = ({
7 options = [],
8 label = 'Document Category',
9 placeholder,
10 }: {
11 options?: Option[]
12 label?: string
13 placeholder?: string
14 } = {}): FilterConfig => ({
15 key: 'typeCodes',
16 // ctx is requi
…(truncated, 718 more chars)
Now let me check for the presence of an index.ts in the component directory, look at the broader directory structure, and verify if there are test files colocated.
Tools used:
Bash:find /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter -type f | sort
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/helpers.test.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/helpers.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/hooks.test.tsx /Users/USER/workspace/doctoli …(truncated, 1145 more chars)
Tools used:
Bash:find /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories -type f | sort
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/actions/createMoreActionsAction.test.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/actions/createMoreActionsAction.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/actions/createViewDocumentAction.test.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tp …(truncated, 3474 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/index.ts
Tool results:
→ result (ok): 1 export * from './NhrDocumentCategoryFilter' 2
Tools used:
Bash:find /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList -maxdepth 1 -type f | sort && find /Users/USER/workspace/doctolib/tr …(truncated, 110 more chars)
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.stories.tsx /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.test.tsx /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.tsx /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-r …(truncated, 1273 more chars)
Now let me check the NhrDocumentCategoryFilter component's index.ts re-export target and look at a sibling component for comparison, plus check if MultiSelectDropdown is exported from the package's index.ts.
Tools used:
Bash:grep -rn "MultiSelectDropdown" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src --include="*.ts" --include="*.tsx" | grep -v "\.test\." | grep -v "\.stori …(truncated, 5 more chars)
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx:3:import { MultiSelectDropdown } from './MultiSelectDropdown' /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx:5:export type NhrDocumentCategoryFilterProps = ComponentProps<typeof MultiSelectDropdown> /U …(truncated, 914 more chars)
Tools used:
Bash:find /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentStatusFilter -type f | sort
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocumentStatusFilter.stories.tsx /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocumentStatusFilter.test.tsx /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocum …(truncated, 19 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.test.tsx
Tool results:
→ result (ok): 1 import { setup } from '@doctolib/test-helpers'
2 import { screen } from '@testing-library/react'
3 import { afterEach, describe, expect, it, vi } from 'vitest'
4
5 import { NhrDocumentCategoryFilter } from './NhrDocumentCategoryFilter'
6
7 const sampleOptions = [
8 { value: 'A', label: 'Report A' },
9 { value: 'B', label: 'Report B' },
10 ]
11
12 describe('NhrDocumentCategoryFilter', () => {
13 afterEach(() => {
14 vi.clearAllMocks()
15 vi.restoreAllMocks()
16 })
17
18 co
…(truncated, 1920 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.test.tsx
Tool results:
→ result (ok): 1 import { setup } from '@doctolib/test-helpers'
2 import { screen } from '@testing-library/react'
3 import { afterEach, describe, expect, it, vi } from 'vitest'
4
5 import { mockPatientInfo } from '../../../test-helpers'
6 import { createNhrDocumentCategoryFilterConfig } from './createNhrDocumentCategoryFilterConfig'
7
8 const ctx = { patient: mockPatientInfo }
9
10 const sampleOptions = [
11 { value: 'A', label: 'Report A' },
12 { value: 'B', label: 'Report B' },
13 { value: 'C', labe
…(truncated, 4677 more chars)
I have a complete picture of all five files. Here is the review.
Architecture Review: NhrDocumentCategoryFilter
Critical Issues (must fix)
1. createNhrDocumentCategoryFilterConfig.tsx has a React import in src/business/
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx
The src/business/ layer must contain pure functions only — no React imports, no JSX. This file uses JSX directly in the factory function's render callback, which requires a .tsx extension and a React runtime. This is a hard business/data layer rule violation: the business layer is rendering UI.
The render method inside FilterConfig is a UI concern. A factory that composes a component into a renderable callback belongs in the component tree or in a dedicated UI-factory layer, not in src/business/.
The correct fix is to move this file out of src/business/. Given the pattern (it wires a component into a FilterConfig shape), it belongs either:
- Adjacent to the component it wraps, inside src/components/NhrDocumentList/NhrDocumentCategoryFilter/, as a component-level helper, or
- In a src/components/-level factory if other filter configs follow the same pattern and are colocated there.
The sibling filter factories (createNhrDateSelectionFilterConfig.tsx, createNhrDocumentStatusFilterConfig.tsx, createNhrDocumentVisibilityFilterConfig.tsx) all live under src/business/factories/filters/ as well — this is a systemic misplacement of the entire filters/ subdirectory, but the scope of this review is the five listed files. Flag the broader src/business/factories/filters/ as requiring the same treatment.
Warnings (should fix)
2. NhrDocumentCategoryFilter.tsx is a pure pass-through wrapper with no purpose
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx
export type NhrDocumentCategoryFilterProps = ComponentProps<typeof MultiSelectDropdown>
export const NhrDocumentCategoryFilter = (props: NhrDocumentCategoryFilterProps) => <MultiSelectDropdown {...props} />
This component adds zero behavior. It re-wraps MultiSelectDropdown with a domain name, but since the type alias is just ComponentProps<typeof MultiSelectDropdown>, callers get no additional abstraction — any prop change to MultiSelectDropdown propagates identically.
There are two legitimate reasons to have this indirection:
- The public API should be stable while the internal implementation (MultiSelectDropdown) is free to change shape.
- The NhrDocumentCategoryFilter should add domain-specific defaults, prop adaptation, or context wiring.
Neither currently applies. If the intent is only the first reason (stable public name), the index.ts re-exporter already achieves that boundary. The wrapper should either be removed (export MultiSelectDropdown directly from index.ts under the domain name, or rename the file) or given real responsibility (e.g. providing domain-specific default label or connecting to a context).
3. Hooks and helpers file naming after the refactor
Files:
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/hooks.ts
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/helpers.ts
The naming convention for component-local files is ComponentName.hooks.ts and ComponentName.helpers.ts (mantissa pattern from the rules). The refactor dropped the component name prefix. Since MultiSelectDropdown is now the actual component consuming both files, the correct names are MultiSelectDropdown.hooks.ts and MultiSelectDropdown.helpers.ts. Bare hooks.ts / helpers.ts are ambiguous and deviate from the established pattern across the codebase.
This also means the test files should follow:
- hooks.test.tsx → MultiSelectDropdown.hooks.test.tsx
- helpers.test.ts → MultiSelectDropdown.helpers.test.ts
4. index.ts re-exports only NhrDocumentCategoryFilter, not MultiSelectDropdownProps
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/index.ts
export * from './NhrDocumentCategoryFilter'
MultiSelectDropdownProps is exported from MultiSelectDropdown.tsx but not re-exported via index.ts. Consumers who want to type props for the filter will grab NhrDocumentCategoryFilterProps from the barrel, which is correct, but MultiSelectDropdownProps is left as a dangling public export without a clear visibility contract. Either:
- Don't export MultiSelectDropdownProps from MultiSelectDropdown.tsx at all (make it internal), or
- Add it to the barrel intentionally if it is part of the public API.
Currently it is exported from the file but not from the barrel — this inconsistency should be resolved deliberately.
Suggestions (consider improving)
5. useDropdownClose is a reusable platform-level UX hook — consider migrating to src/hooks/
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/hooks.ts
useDropdownClose handles pointer-outside and Escape-key dismissal. This pattern is entirely generic — it has no domain knowledge, no component-specific state, and no dependency on any NHR-specific type. If any other component in this package ever needs the same behavior (another dropdown, a popover, a modal), the hook will need to be duplicated or the import path will be a deep relative reference crossing component boundaries.
Per the hook placement rules: hooks that are reused across two or more components belong in src/hooks/. The hook is not there yet, but its shape makes it a natural candidate. Flag it now so it is moved preemptively rather than duplicated.
6. normalize in helpers.ts — same reuse argument applies
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/helpers.ts
Diacritic-stripping and lowercasing for search normalization is a generic text utility with no domain coupling. If a second search filter appears in this package, this will be duplicated. Per the rules, src/helpers/ is for formatters and small UI transforms used across the package. This function fits there exactly. Leave it component-local only if certain it will never be needed elsewhere.
7. createNhrDocumentCategoryFilterConfig.tsx ignores the ctx argument with a comment
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx
// ctx is required by FilterConfig.render but unused by this filter
render: (filters, onChange) => {
The comment acknowledges the parameter is dropped. This is fine behavior-wise, but the TypeScript signature of FilterConfig.render presumably includes ctx as a required third argument. Dropping it entirely from the destructure could silently hide future usages. The cleaner approach is to use _ctx (underscore-prefix convention for intentionally unused parameters) to make the intent explicit and satisfy lint rules without a comment.