Project: workspace-doctolib · Branch: HEAD · Mode: ` · **Model:**claude-sonnet-4-6`
Stats: 1 user messages, 6 tool calls, tools used: Read.
Conversation
Turn 1 — User
Re-analyze test coverage for NhrDocumentCategoryFilter. This is a second pass — the previous pass's findings have all been addressed: - option.disabled tests added (disabled renders as disabled, disabled does not call onToggle) - Diacritic normalization tests added (French accents, German umlauts) - Hook behavior tests added (Escape from unrelated element, second-click on trigger, searchQuery reset on reopen) - aria-controls tests added (absent when closed, present when open) - Custom label factory test added - afterEach now calls both vi.clearAllMocks() and vi.restoreAllMocks() - "Unknown values" display test strengthened to toHaveDisplayValue('')
Test files: 1. /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.test.tsx 2. /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.test.tsx
Source files: 1. /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx 2. /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.hooks.ts 3. /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.helpers.ts 4. /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx
Report ONLY new gaps not already addressed. If coverage is satisfactory, say so explicitly.
Turn 1 — Assistant
I'll read all the relevant source and test files in parallel to do a thorough second-pass analysis.
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 { value: 'C', label: 'Report C' },
11 ]
12
13 describe('NhrDocumentCategoryFilter', () => {
14 afterEach(() => {
15 vi.clearAllMocks()
16
…(truncated, 13514 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 { 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', label: 'Report
…(truncated, 4317 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 { 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 './NhrDocumentCategoryFilter.helpers'
8 import { useNhrDocumentCategoryClose } from './NhrDocumentCategoryFilter.hook
…(truncated, 3927 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.hooks.ts
Tool results:
→ result (ok): 1 import { type RefObject, useEffect, useRef } from 'react'
2
3 export const useNhrDocumentCategoryClose = (
4 containerRef: RefObject<HTMLDivElement | null>,
5 onClose: () => void,
6 ) => {
7 const onCloseRef = useRef(onClose)
8 useEffect(() => {
9 onCloseRef.current = onClose
10 })
11
12 useEffect(() => {
13 const handlePointerDown = (e: PointerEvent) => {
14 if (containerRef.current && !containerRef.current.contains(e.target as Node)) {
15 onCloseRef.curren
…(truncated, 476 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.helpers.ts
Tool results:
→ result (ok): 1 export const normalize = (s: string) => 2 s.normalize('NFD').replace(/\p{Diacritic}/gu, '').toLowerCase() 3
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 type { Option } from '@doctolib/design-system/Option'
2
3 import { NhrDocumentCategoryFilter } from '../../../components/NhrDocumentList/NhrDocumentCategoryFilter'
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 render:
…(truncated, 631 more chars)
Here is the second-pass analysis with fresh eyes on the current state of the test files.
Summary
The test suite for NhrDocumentCategoryFilter is in very good shape. All gaps from the first pass are addressed. The tests cover rendering, interaction, accessibility attributes, search/filter behavior (including diacritics), disabled options, hook edge cases, and the factory config logic. The afterEach teardown is correct.
I identified three remaining gaps — one important and two minor — none of which were listed as addressed in the prompt.
Critical Gaps
None.
Important Improvements
1. The values prop in createNhrDocumentCategoryFilterConfig accepts a non-array typeCodes at runtime — untested defensive path (Criticality: 6)
In /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx, the render function does:
const selectedValues = (filters.typeCodes as string[] | undefined) ?? []
The filters parameter is typed as Record<string, unknown>, so typeCodes could arrive as null, a string, or any other non-array type at runtime. The nullish coalescing only guards against undefined — it does not guard against null (which is falsy but not nullish-falsy — actually null is nullish so ?? [] does catch it). However a non-null, non-array value such as typeCodes: 'A' or typeCodes: 42 would bypass the guard and be passed as values to NhrDocumentCategoryFilter, where values.includes(...) and values.length would then behave unexpectedly or throw.
The existing test for "should show empty trigger when selected typeCode does not match any option" uses { typeCodes: ['UNKNOWN'] } — a valid array. There is no test for a malformed typeCodes.
Missing test:
it('should treat a non-array typeCodes as no selection', () => {
renderFilter(sampleOptions, { typeCodes: 'A' as unknown })
expect(screen.queryByDisplayValue('Report A')).not.toBeInTheDocument()
})
This would catch a regression if the guard were removed or narrowed. Criticality: 6 — not a likely production path but filters arrives from a generic host, and the type cast makes it a real risk.
2. The displayLabel memoization uses values.length but only checks firstLabel to decide the format — one partially-matched edge case is untested (Criticality: 5)
In NhrDocumentCategoryFilter.tsx:
const firstLabel = options.find((o) => values.includes(o.value))?.label ?? ''
if (values.length === 0 || firstLabel === '') return ''
return values.length === 1 ? firstLabel : `${firstLabel}, +${values.length - 1}`
When values contains multiple entries but the first matching option is not the first item in values (i.e., values[0] has no matching option but values[1] does), the label will still render as "Report B, +1" rather than an empty string — because firstLabel is derived from options.find() (first option that has a match in values), not from values[0].
The existing "unknown values" test (values: ['UNKNOWN_A', 'UNKNOWN_B']) verifies a total miss. But the partial case — values: ['UNKNOWN', 'A'] where the first value in the array is unmatched but another is — is not tested. The component shows "Report A, +1" in this case (since there are 2 values entries and firstLabel finds 'A'). Whether that is correct behavior or a bug depends on product intent, but the behavior is at least worth pinning with a test.
Missing test:
it('should show matching label with count when only some selected values match options', () => {
renderComponent({ values: ['UNKNOWN', 'A', 'B'] })
// first matching option is 'A'; 3 values total so +2
expect(screen.getByDisplayValue('Report A, +2')).toBeInTheDocument()
})
Criticality: 5 — the behavior should be intentional and pinned so a future change to the label logic does not silently alter it.
Test Quality Issues
The createNhrDocumentCategoryFilterConfig factory test file does not import afterEach / call mock cleanup
/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.test.tsx has no afterEach at all. The component test file correctly calls vi.clearAllMocks() and vi.restoreAllMocks(), but the factory test omits it. The onChange callbacks created with vi.fn() are local to each test here, so this does not cause failures in the current suite. However it is inconsistent with the pattern established in the component test and project rules, and could become a problem if shared spies are introduced later.
The factory test file should include:
import { afterEach, describe, expect, it, vi } from 'vitest'
afterEach(() => {
vi.clearAllMocks()
vi.restoreAllMocks()
})
This is a low-priority style issue (Criticality: 3) but is flagged for consistency with project standards.
Positive Observations
- The "searchQuery reset on reopen" test correctly exercises close-then-reopen and verifies all options are visible — a subtle state leak that many suites miss.
- The "second click on open trigger does not close" test directly validates the
if (!isOpen)guard inhandleInputClick, protecting against a behavior regression. - The "Escape from unrelated element" test is exactly right for the global
keydownlistener inuseNhrDocumentCategoryClose— it fires even when focus is outside the component. - The
aria-controlsabsent-when-closed / present-when-open pair is well-formed and tests the ternaryisOpen ? popupId : undefinedbranch directly. - The factory config tests cleanly separate toggle-adds, toggle-removes, append-order, and onClear — all four branches of the
onToggleclosure are exercised. toHaveDisplayValue('')for the unknown-values case is the correct matcher (verifies the input's display value, not just DOM text presence).