Project: workspace-doctolib · Branch: HEAD · Mode: ` · **Model:**claude-sonnet-4-6`
Stats: 1 user messages, 11 tool calls, tools used: Bash, Read.
Conversation
Turn 1 — User
Analyze test coverage quality for PR https://github.com/doctolib/treatment-plan/pull/3085
Worktree: /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/
Changed test file:
- packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentVisibilityFilterConfig.test.tsx
Get the diff:
cd /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter && git diff main...HEAD
The feature being tested: createNhrDocumentVisibilityFilterConfig factory that wraps onChange with exclusive-checkbox logic. An isExclusive: true flag on a mode means:
- Checking it → all other modes become false
- Checking any other mode while exclusive is active → exclusive becomes false
- Unchecking it → only that mode becomes false
Evaluate: Are all meaningful state transitions covered? Are there edge cases missing?
Turn 1 — Assistant
I'll analyze the test coverage for this PR systematically. Let me start by getting the diff and reading the relevant files.
Tools used:
Bash:git diff main...HEAD
Tool results:
→ result (ok): <persisted-output> Output too large (476.7KB). Full output saved to: /Users/USER/.claude/projects/-Users-clertonra-workspace-doctolib/116b5e84-4b1c-4396-986a-6677b6aff27f/tool-results/bdaab902a.txt
Preview (first 2KB): diff --git a/.claude/skills/delegate/PROMPT_TEMPLATE.md b/.claude/skills/delegate/PROMPT_TEMPLATE.md index 9d33a5475..77c46999f 100644 --- a/.claude/skills/delegate/PROMPT_TEMPLATE.md +++ b/.claude/skills/delegate/PROMPT_TEMPLATE.md @@ -13,6 +13,13 @@ If your task references a JI …(truncated, 1718 more chars)
Tools used:
Bash:find /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/business/factories/filters -type f | sort
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDateSelectionFilterConfig.test.tsx /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDateSelectionFilterConfig.tsx /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter …(truncated, 1153 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentVisibilityFilterConfig.tsx
Tool results:
→ result (ok): 1 import { NhrDocumentVisibilityFilter } from '../../../components/NhrDocumentVisibilityFilter/NhrDocumentVisibilityFilter'
2 import type { FilterConfig, VisibilityOption } from '../../../types'
3
4 type VisibilityModeConfig = VisibilityOption & { isExclusive?: boolean }
5
6 type CreateNhrDocumentVisibilityFilterConfigOptions = {
7 groupLabel: string
8 visibilityModes: VisibilityModeConfig[]
9 }
10
11 export const createNhrDocumentVisibilityFilterConfig = ({
12 groupLabel,
13 visibili
…(truncated, 1247 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentVisibilityFilterConfig.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 { createNhrDocumentVisibilityFilterConfig } from './createNhrDocumentVisibilityFilterConfig'
7
8 const ctx = { patient: mockPatientInfo }
9
10 const defaultOptions = {
11 groupLabel: 'Document visibility',
12 visibilityModes: [
13 { filterKey: 'hiddenFromPatient', labe
…(truncated, 3915 more chars)
Now let me look at the related component to understand the full interface and see what other filters look like for comparison.
Tools used:
Bash:find /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/components/NhrDocumentVisibilityFilter -type f | sort
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/components/NhrDocumentVisibilityFilter/NhrDocumentVisibilityFilter.stories.tsx /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/components/NhrDocumentVisibilityFilter/NhrDocumentVisibilityFilter.test.tsx /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-vis …(truncated, 133 more chars)
Tools used:
Bash:find /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src -name "*.ts" -o -name "*.tsx" | xargs grep -l "Visibil …(truncated, 47 more chars)
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/types.ts /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/index.ts /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDateSelectionFilterConfig.tsx /Users/U …(truncated, 1443 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/components/NhrDocumentVisibilityFilter/NhrDocumentVisibilityFilter.tsx
Tool results:
→ result (ok): 1 import { Checkbox } from '@doctolib/design-system/Checkbox/v2'
2 import { Heading } from '@doctolib/design-system/Heading'
3
4 import type { VisibilityOption } from '../../types'
5
6 export type NhrDocumentVisibilityFilterProps = {
7 groupLabel: string
8 visibilityModes: VisibilityOption[]
9 filters: Record<string, boolean>
10 onChange: (filterKey: string, checked: boolean) => void
11 }
12
13 export const NhrDocumentVisibilityFilter = ({
14 groupLabel,
15 visibilityModes,
16 f
…(truncated, 600 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/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, NhrResultStatus } 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. Return
…(truncated, 3703 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentStatusFilterConfig.tsx
Tool results:
→ result (ok): 1 import { NhrDocumentStatusFilter } from '../../../components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocumentStatusFilter'
2 import type { FilterConfig, StatusOption } from '../../../types'
3
4 export const createNhrDocumentStatusFilterConfig = (statuses: StatusOption[], label: string): FilterConfig => ({
5 key: 'documentStatus',
6 render: (filters, onChange) => (
7 <NhrDocumentStatusFilter
8 label={label}
9 statuses={statuses}
10 value={statuses.filter((s) => fil
…(truncated, 158 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentStatusFilterConfig.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 { createNhrDocumentStatusFilterConfig } from './createNhrDocumentStatusFilterConfig'
7
8 // ctx is required by FilterConfig.render but unused by this filter
9 const ctx = { patient: mockPatientInfo }
10
11 const sampleStatuses = [
12 { filterKey: 'includeArchived', label: 'A
…(truncated, 1866 more chars)
Now I have a thorough understanding of the implementation and tests. Let me map every logical branch in the source against the tests to identify gaps.
Tools used:
Bash:grep -n "isExclusive" /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrD …(truncated, 33 more chars)
Tool results:
→ result (ok): 4:type VisibilityModeConfig = VisibilityOption & { isExclusive?: boolean } 19: visibilityModes={visibilityModes.map(({ isExclusive: _ignored, ...rest }) => rest)} 24: if (checked && tappedMode?.isExclusive) { 34: const exclusiveMode = visibilityModes.find((m) => m.isExclusive)
I have everything I need. Here is the full analysis.
Summary
The test suite covers the three primary exclusive-checkbox state transitions correctly and uses idiomatic RTL patterns. The tests are behavioral (they assert on onChange payload, not internal state), they use user.click rather than synthetic events, and the naming is clear. Coverage for the no-exclusive-mode baseline path is present.
However, there are four meaningful gaps and one moderate test-quality issue against the implementation's actual branching logic.
Critical Gaps
1. Checking a non-exclusive mode while exclusive is NOT active — unchecking while exclusive is inactive Criticality: 7/10
The else branch (line 31–42 of the implementation) contains a guard:
if (checked) {
const exclusiveMode = visibilityModes.find((m) => m.isExclusive)
if (exclusiveMode && (filters as Record<string, unknown>)[exclusiveMode.filterKey]) {
patch[exclusiveMode.filterKey] = false
}
}
The test "should call onChange with patch when checkbox is toggled" (line 44 in the test file) uses defaultOptions, which has no exclusive mode at all. It confirms the simple { hiddenFromPatient: true } patch, but that path never enters the exclusiveMode sub-guard at all because visibilityModes.find((m) => m.isExclusive) returns undefined.
The missing test is: check a non-exclusive mode when an exclusive mode exists but is currently false (not active). This is the ordinary "add one filter" case in a configuration that has an exclusive mode, and it should produce { hiddenFromPatient: true } with no visibleToAll key in the patch. If a future refactor accidentally always appended visibleToAll: false to every non-exclusive check, the existing tests would not catch it.
What a failure looks like: a refactor inadvertently sets { hiddenFromPatient: true, visibleToAll: false } even when visibleToAll was already false — silently double-writing the exclusive key on every toggle.
2. Unchecking a non-exclusive mode when exclusive is NOT active Criticality: 6/10
The else branch is also entered when checked === false. In that path, the inner if (checked) block is skipped entirely, so only { [filterKey]: false } is emitted. There is no test for unchecking a non-exclusive mode in any configuration (with or without an exclusive mode present). The "should call onChange with patch when checkbox is toggled" test only ever clicks an unchecked checkbox, so it only tests checked: true.
The test for the analogous createNhrDocumentStatusFilterConfig does cover the uncheck case ("should call onChange with false when a checked checkbox is clicked"). Parity is missing here.
What a failure looks like: any future change that accidentally emits { [filterKey]: true } regardless of checkbox direction would be undetected.
3. Checking an exclusive mode when ALL other modes are already false Criticality: 5/10
The "should uncheck all other modes when an exclusive mode is checked" test initialises filters as { hiddenFromPatient: true }. That means the patch will include hiddenFromPatient: false (explicitly set). The test does not exercise the case where the other modes start as false (or absent). In that case the patch would still be { visibleToAll: true, hiddenFromPatient: false } — which is correct — but no test verifies it. More importantly, with three or more modes, the patch must set all non-exclusive modes to false; only a two-mode config is tested.
What a failure looks like: a future version that only patches modes that were true in the previous state would pass the existing test but silently leave a third mode unchecked-but-unpatched.
4. Three-mode configuration with multiple non-exclusive modes active when exclusive is checked Criticality: 6/10
The implementation iterates all visibilityModes and sets every non-exclusive key to false. All tests use exactly two modes. A three-mode setup (visibleToAll, hiddenFromPatient, hiddenFromProfessionals) would produce a patch of { visibleToAll: true, hiddenFromPatient: false, hiddenFromProfessionals: false }. No test verifies that all three keys are patched, only that the two-mode case patches correctly.
What a failure looks like: a refactor that uses slice(1) instead of forEach would still pass all current tests but would miss the third mode's key in the patch.
Important Improvements
5. isExclusive is stripped before being passed to NhrDocumentVisibilityFilter
Criticality: 3/10
Line 19 strips isExclusive from each mode via destructuring before passing visibilityModes to the underlying component. There is no test that verifies the rendered component does not receive or expose isExclusive. If the stripping were accidentally removed, the VisibilityOption type mismatch would be caught at compile time, so this is low risk — but a test that asserts the rendered checkboxes have no unexpected attributes would provide a safety net for the runtime contract.
Test Quality Issues
6. The "unchecking exclusive" test name vs actual behaviour
The test named "should only send the exclusive key as false when it is unchecked" (line 53) correctly exercises unchecking the exclusive mode. However it does not assert that no other key appears in the patch. The assertion is:
expect(onChange).toHaveBeenCalledWith({ visibleToAll: false })
toHaveBeenCalledWith does equality-match the full argument, so extra keys would fail the assertion. The test is technically correct. No change needed, but it is worth noting the test name could be more specific: it says "only send the exclusive key" but the assertion already guarantees that implicitly.
Positive Observations
- All three primary exclusive-checkbox transitions (check exclusive, uncheck exclusive, check non-exclusive while exclusive is active) are covered with explicit assertions.
- Each test that introduces an exclusive mode creates a fresh
optionsobject inline rather than mutatingdefaultOptions, which avoids state bleed between tests. - Tests assert on the exact
onChangepayload shape, which will catch extra or missing keys introduced by future refactors. - The
setup()wrapper from@doctolib/test-helpersis used consistently, matching the project's testing conventions. - The test file is well under the 400-line warning threshold (103 lines), and the three exclusive-mode tests share a clean, locally-scoped options fixture pattern.
Prioritised list for the author
| # | Description | Criticality |
|---|---|---|
| 4 | Three-mode exclusive-check patches all non-exclusive keys | 6/10 |
| 2 | Unchecking a non-exclusive checkbox emits checked: false |
6/10 |
| 1 | Checking non-exclusive while exclusive exists but is inactive | 7/10 |
| 3 | Exclusive check from all-false starting state | 5/10 |
The highest-value addition is gap #1: it is the only scenario where the exclusiveMode sub-guard is entered but the exclusive mode is false, and it is easy to regress silently. Gaps #2 and #4 each catch distinct real failure modes and match the coverage standard set by the sibling createNhrDocumentStatusFilterConfig test file.
Relevant files:
- /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentVisibilityFilterConfig.tsx
- /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentVisibilityFilterConfig.test.tsx
- /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2222_add-visible-to-all-filter/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentStatusFilterConfig.test.tsx (sibling reference for uncheck pattern)