Project: workspace-doctolib · Branch: HEAD · Mode: · Model: claude-opus-4-7 Stats: 1 user messages, 7 tool calls, tools used: Bash, Read. Conversation Turn 1 — User Review the latest changes on the doctolib monolith PR https://github.com/doctolib/doctolib/pull/241176 Worktree: /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2222_add-visible-to-all-filter/ Get the full diff: cd /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2222_add-visible-to-all-filter && git diff HEAD~2...HEAD The latest commit added: - packages/@doctolib/national-health-records/src/mocks/filterMockedDocuments.ts: new visibleToAll branch in matchesVisibility, mirroring the real API's early-return for sharedWithAll - packages/@doctolib/national-health-records/src/mocks/filterMockedDocuments.test.ts: 3 new tests for visibleToAll - packages/@doctolib/national-health-records/src/variants/dmp/mappers/mapFiltersToSearchDocumentsParams.test.ts: one new test for visibleToAll: false The previous commit (HEAD~1) is the original feature work that was already reviewed and found clean. Focus on the new additions only. Report only issues above 80-confidence. Turn 1 — Assistant I'll review the latest changes on the PR. Let me start by examining the diff and relevant files. Tools used: - Bash: git log --oneline -5 Tool results: → result (ok): 080c4457547a fix(fhir-2222): handle visibleToAll in mock filter and close test gaps cb4ab8289db2 fix(fhir-2222): add visibleToAll filter to DMP NHR document visibility 401f9e11e68b fix(DITTO-4504): bump deduper child-job timeout to 15h to unblock 175k+ patient bases (#241136) 4227d3b98c6f feat(DIAL-1149): add phone_assistant_multi_location feature switch (#241097) e996c480f200 feat(PACE-3073): add Doctolib Classic FR + Psychologue offer + Bobby fixtures (#239808) Tools used: - Bash: git diff HEAD~1...HEAD --stat Tool results: → result (ok): .../src/mocks/filterMockedDocuments.test.ts | 26 ++++++++++++++++++++++ .../src/mocks/filterMockedDocuments.ts | 5 +++++ .../mapFiltersToSearchDocumentsParams.test.ts | 5 +++++ 3 files changed, 36 insertions(+) Tools used: - Bash: git diff HEAD~1...HEAD Tool results: → result (ok): diff --git a/packages/@doctolib/national-health-records/src/mocks/filterMockedDocuments.test.ts b/packages/@doctolib/national-health-records/src/mocks/filterMockedDocuments.test.ts index 3acd94b74fa6..208c047b267e 100644 --- a/packages/@doctolib/national-health-records/src/mocks/filterMockedDocuments.test.ts +++ b/packages/@doctolib/national-health-records/src/mocks/filterMockedDocuments.test.ts @@ -153,6 +153,32 @@ describe('filterMockedDocuments', () => { // undefined visibility treated …(truncated, 3476 more chars) Now let me examine the relevant source files for context. Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2222_add-visible-to-all-filter/packages/@doctolib/national-health-records/src/mocks/filterMockedDocuments.ts Tool results: → result (ok): 1 import type { NhrDocument } from '@doctolib/tpf-national-health-records' 2 import { DOCUMENT_STATUS, DOCUMENT_VISIBILITY } from '@doctolib/dmp' 3 4 import type { ServerFilters } from '../variants/types' 5 6 / 7 Applies ServerFilters to a list of mock NhrDocuments, simulating DMP server-side filtering. 8 Used so that the mock path in useSearchDocuments respects filter changes the same way the real API does. 9 / 10 export function filterMockedDocuments(docs: NhrDocument[], filters: S …(truncated, 3227 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2222_add-visible-to-all-filter/packages/@doctolib/national-health-records/src/mocks/filterMockedDocuments.test.ts Tool results: → result (ok): 1 import type { NhrDocument } from '@doctolib/tpf-national-health-records' 2 import { filterMockedDocuments } from './filterMockedDocuments' 3 4 const makeDoc = (overrides: Partial = {}): NhrDocument => ({ 5 id: 'doc-1', 6 title: 'Test document', 7 ...overrides, 8 }) 9 10 describe('filterMockedDocuments', () => { 11 describe('category filter', () => { 12 it('returns all documents when no filters are set', () => { 13 const docs = [makeDoc({ id: 'a' }), makeDoc({ i …(truncated, 10862 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2222_add-visible-to-all-filter/packages/@doctolib/national-health-records/src/variants/dmp/mappers/mapFiltersToSearchDocumentsParams.test.ts Tool results: → result (ok): 1 import { mapFiltersToSearchDocumentsParams } from './mapFiltersToSearchDocumentsParams' 2 3 jest.mock('@doctolib/dmp', () => ({ 4 DOCUMENT_STATUS: { validated: 1, obsolete: 2, archived: 4 }, 5 DOCUMENT_VISIBILITY: { 6 sharedWithAll: 1, 7 notSharedWithPatient: 2, 8 notSharedWithPractitioner: 4, 9 notSharedWithLegalRepresentative: 8, 10 }, 11 })) 12 13 describe('mapFiltersToSearchDocumentsParams', () => { 14 describe('categories', () => { 15 it('maps typeCodes to ca …(truncated, 7202 more chars) Let me check the real API mapper to confirm the mock matches the real API behavior described. Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2222_add-visible-to-all-filter/packages/@doctolib/national-health-records/src/variants/dmp/mappers/mapFiltersToSearchDocumentsParams.ts Tool results: → result (ok): 1 / eslint-disable no-bitwise */ 2 import type { NhrDocumentSearchParams } from '@doctolib/tpf-national-health-records' 3 import { DOCUMENT_STATUS, DOCUMENT_VISIBILITY } from '@doctolib/dmp' 4 import type { ServerFilters } from '../../types' 5 6 // Maps UI-level ServerFilters to the NhrDocumentSearchParams format expected by the NHR API. 7 // Each ServerFilters field needs an explicit mapping — field names do not match NhrDocumentSearchParams. 8 export function mapFiltersToSearchDocumentsParam …(truncated, 2888 more chars) Now I have a complete view of the changes. Let me analyze them carefully. Review Summary I'm reviewing the latest commit (080c4457547a) on PR #241176, which adds: 1. A new visibleToAll early-return branch in matchesVisibility (mock filter) 2. 3 new tests in filterMockedDocuments.test.ts for visibleToAll 3. 1 new test in mapFiltersToSearchDocumentsParams.test.ts for visibleToAll: false Analysis Mock implementation (filterMockedDocuments.ts lines 57-60): - Mirrors the real API mapper (mapVisibilityFilter line 58) which uses if (filters.visibleToAll) return String(...). Both use a truthy guard on visibleToAll, so they're consistent. A visibleToAll: false correctly falls through to the existing logic in both files. - The fallback ?? 1 matches the existing pattern used elsewhere in the file (line 76). - The String() wrapping is correct — doc.visibility is typed as string | undefined (per the existing parseInt on line 76). Tests (filterMockedDocuments.test.ts lines 157-181): - Test 1 (line 157-165) verifies visibleToAll: true returns only visibility=1 docs and excludes compound visibilities like '12' — correct. - Test 2 (line 167-170) verifies undefined visibility defaults to sharedWithAll — correct, matches the implementation's ?? fallback. - Test 3 (line 172-181) verifies visibleToAll: true takes priority over other flags — correct. Test (mapFiltersToSearchDocumentsParams.test.ts lines 143-146): - Verifies visibleToAll: false doesn't suppress other flags — correct, matches the truthy-guard implementation. Cross-file consistency check The mock and real mapper agree on: - visibleToAll: true → keep only visibility=1 docs / produce '1' - visibleToAll: false or undefined → fall through to bitmask logic - Both use the ?? 1 fallback for DOCUMENT_VISIBILITY.sharedWithAll Findings No issues with confidence ≥ 80. The changes meet project standards: - The mock implementation correctly mirrors the real API mapper's truthy guard semantics, including identical fallback handling (?? 1). - The 4 new tests provide good coverage of the new branch (true case, undefined-visibility default, priority over other flags, and the false-value fallthrough in the mapper). - TypeScript types are respected; no any casts or eslint-disable directives were introduced. - No bitwise concerns since the new branch is a straight equality check. - Test naming follows the existing kebab/sentence convention in the file. The latest commit is clean and ready. Relevant files: - /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2222_add-visible-to-all-filter/packages/@doctolib/national-health-records/src/mocks/filterMockedDocuments.ts - /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2222_add-visible-to-all-filter/packages/@doctolib/national-health-records/src/mocks/filterMockedDocuments.test.ts - /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2222_add-visible-to-all-filter/packages/@doctolib/national-health-records/src/variants/dmp/mappers/mapFiltersToSearchDocumentsParams.test.ts - /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2222_add-visible-to-all-filter/packages/@doctolib/national-health-records/src/variants/dmp/mappers/mapFiltersToSearchDocumentsParams.ts (reference for cross-checking)