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 frontend architecture of the changes in branch fhir-2235_expand-document-category-filter-type-codes in the doctolib monolith at /Users/USER/workspace/doctolib/doctolib. Changed files: 1. packages/@doctolib/national-health-records/src/variants/dmp/constants.ts 2. packages/@doctolib/national-health-records/src/variants/dmp/constants.test.ts The change replaces a 9-entry DMP_CLASS_CODES array + deprecated LOINC_CODES_TO_KIND runtime label resolution with a 108-entry static Record. Review for: 1. UpFront architecture compliance (separation of concerns, naming, structure) 2. TypeScript patterns — is Record the right type? Should any types be narrower/wider? 3. The legacy naming note (DmpClassCode/DMP_CLASS_CODES are now type codes not class codes) — is this handled correctly? 4. Any architectural concerns about putting 108 static entries in a single constants file vs splitting Read the files before reviewing. Turn 1 — Assistant I'll read both files before reviewing. Tools used: - Read: /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/constants.ts Tool results: → result (ok): 1 // All type codes from the PM-authoritative CSV mapping (JDV_J06/JDV_J07 ANS + LOINC + ASTM). 2 // NOTE: DMP_CLASS_CODES / DmpClassCode are legacy names — these are PM-authoritative type codes 3 // (typeCode), not DMP class codes (classCode). A follow-up rename to DMP_TYPE_CODES / DmpTypeCode 4 // is recommended once this change is merged and stable. 5 export const DMP_CLASS_CODES = [ 6 'ATTEST-ASS-COMPL', 7 'ATTEST-HEBGT', 8 '59284-0', 9 'ATTEST-DROITS-AM', 10 'ATTEST-RESID', 11 ' …(truncated, 7746 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/constants.test.ts Tool results: → result (ok): 1 import { DMP_CLASS_CODES, DMP_CLASS_CODE_LABELS } from './constants' 2 3 describe('DMP_CLASS_CODE_LABELS', () => { 4 it('has an entry for every code in DMP_CLASS_CODES', () => { 5 for (const code of DMP_CLASS_CODES) { 6 expect(DMP_CLASS_CODE_LABELS[code]).toBeDefined() 7 } 8 }) 9 10 it('has exactly 108 codes from the PM-authoritative CSV mapping', () => { 11 expect(DMP_CLASS_CODES).toHaveLength(108) 12 }) 13 14 it('has correct French labels for all codes', () => …(truncated, 1477 more chars) Now let me check the broader package structure to understand the context around this file. Tools used: - Bash: find /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src -type f | sort Tool results: → result (ok): /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/parseHl7DateToIso.test.ts /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/parseHl7DateToI …(truncated, 4697 more chars) Tools used: - Bash: ls /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/ Tool results: → result (ok): constants.test.ts constants.ts factories index.ts mappers useDmpDocumentListVariant.ts Tools used: - Read: /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/factories/dmpCategoryOptions.ts Tool results: → result (ok): 1 import type { Option } from '@doctolib/design-system/Option' 2 import { DMP_CLASS_CODES, DMP_CLASS_CODE_LABELS } from '../constants' 3 4 export const DMP_CATEGORY_OPTIONS: Option[] = DMP_CLASS_CODES.map((code) => ({ 5 value: code, 6 label: DMP_CLASS_CODE_LABELS[code], 7 })).sort((a, b) => a.label.localeCompare(b.label, 'fr')) 8 Tools used: - Read: /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/factories/filtersFactory.ts Tool results: → result (ok): 1 import { 2 type FilterConfig, 3 createNhrDateSelectionFilterConfig, 4 createNhrDocumentCategoryFilterConfig, 5 createNhrDocumentStatusFilterConfig, 6 createNhrDocumentVisibilityFilterConfig, 7 } from '@doctolib/tpf-national-health-records-ui' 8 import { DMP_CATEGORY_OPTIONS } from './dmpCategoryOptions' 9 10 export function filtersFactory(): FilterConfig[] { 11 return [ 12 createNhrDocumentCategoryFilterConfig({ 13 options: DMP_CATEGORY_OPTIONS, 14 label: 'Document …(truncated, 1218 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/index.ts Tool results: → result (ok): 1 export { useDmpDocumentListVariant } from './useDmpDocumentListVariant' 2 Tools used: - Read: /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/index.ts Tool results: → result (ok): 1 export { useNationalHealthRecords, type UseNhrOperations } from './hooks/useNationalHealthRecords' 2 export { useDmpNhr } from './hooks/useDmpNhr' 3 export { useNhrDocumentListVariant } from './variants/useNhrDocumentListVariant' 4 export type { DocumentSearchOptions, NhrDocumentListVariant } from './variants/types' 5 export { actionsBuilder, type NhrActionsBuilder } from './business/builders/actionsBuilder' 6 export { useGetDocumentsContent } from './hooks/useGetDocumentsContent' 7 Tools used: - Bash: git log --oneline -10 -- packages/@doctolib/national-health-records/src/variants/dmp/constants.ts packages/@doctolib/national-health-records/src/variants/dmp/constants.test.ts 2>/dev/null || echo "git …(truncated, 12 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/mappers/dmpDocumentHelpers.ts Tool results: → result (ok): 1 import type { NhrDocument } from '@doctolib/tpf-national-health-records' 2 import type { NhrVisibilityEntry } from '@doctolib/tpf-national-health-records-ui' 3 import { DOCUMENT_STATUS, DOCUMENT_VISIBILITY } from '@doctolib/dmp' 4 5 const CLASS_CODE_SYSTEM = 'dmp:classCode' 6 const TYPE_CODE_SYSTEM = 'dmp:typeCode' 7 const PATIENT_CODE = '90' // EX_3.1-1040: class code for patient-authored documents 8 9 // classification.codes may contain both a classCode (JDV_J06) and a typeCode (JDV_J07/LO …(truncated, 2792 more chars) Tool results: → result (ok): 2fed3650169e fix(fhir-2235): expand DMP category filter to PM-authoritative type code mapping 536fe4c90999 chore(FHIR-2208): populate Document Category filter with DMP LOINC options (#240065) 48101cac2cbf feat(FHIR-2189): translate DMP class codes to human-readable labels in document list (#239293) 8f3620f2ded2 refactor(fhir-2186): improve type safety and explicit mock count in useSearchDocuments (#239204) Tools used: - Bash: git diff HEAD~1 HEAD -- packages/@doctolib/national-health-records/src/variants/dmp/constants.ts packages/@doctolib/national-health-records/src/variants/dmp/constants.test.ts 2>/dev/null | head -80 Tool results: → result (ok): diff --git a/packages/@doctolib/national-health-records/src/variants/dmp/constants.test.ts b/packages/@doctolib/national-health-records/src/variants/dmp/constants.test.ts index 7a26f1fa6b26..02affd9aea0c 100644 --- a/packages/@doctolib/national-health-records/src/variants/dmp/constants.test.ts +++ b/packages/@doctolib/national-health-records/src/variants/dmp/constants.test.ts @@ -7,24 +7,33 @@ describe('DMP_CLASS_CODE_LABELS', () => { } }) - it('pulls French labels from LOINC_CODES_TO …(truncated, 3877 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/constants/cdaTypeCodeToFacetName.ts Tool results: → result (ok): 1 export const CDA_TYPE_CODE_TO_FACET_NAME: Record = { 2 'ATTEST-ASS-COMPL': 'Carte de Tiers payant', 3 'ATTEST-HEBGT': 'Autre', 4 '59284-0': 'Consentement du patient', 5 'ATTEST-DROITS-AM': 'Autre', 6 'ATTEST-RESID': 'Autre', 7 '86531-1': 'Autre', 8 'AUTORIS-SOINS': 'Consentement du patient', 9 EXPPAT_2: 'Autre', 10 '28617-9': 'Compte-rendu de Consultation', 11 '68817-6': 'Compte-rendu de Consultation', 12 '56445-0': 'Autre', 13 '34120-6': 'Autre', 14 ' …(truncated, 3502 more chars) I now have a complete picture. Let me also check the package.json for any path alias configuration and examine whether the variants/dmp/ directory is an established subdirectory pattern. Tools used: - Bash: cat /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/package.json Tool results: → result (ok): { "name": "@doctolib/national-health-records", "version": "1.0.0", "publishConfig": { "registry": "https://doctolib-580698825394.d.codeartifact.eu-west-1.amazonaws.com/npm/npm-internal/" }, "repository": { "type": "git", "url": "git+https://github.com/doctolib/doctolib.git" }, "keywords": [ "upfront", "modular frontend", "in-monolith-package" ], "engines": { "node": ">= 24.8.0", "npm": ">= 11.6.0" }, "author": "@doctolib/fire", "license": " …(truncated, 1014 more chars) I have everything I need. Here is the full review. --- Architecture Review: national-health-records — DMP constants expansion Summary of the change The PR replaces a 9-entry DMP_CLASS_CODES array with a 108-entry array, removes the runtime dependency on LOINC_CODES_TO_KIND from monolith_DEPRECATED/..., and introduces DMP_CLASS_CODE_LABELS: Record as a static compile-time map. The corresponding test file is updated to reflect the new surface area. --- Critical Issues (must fix) 1. Legacy naming is left unresolved, with a comment that normalises deferral /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/constants.ts, lines 1–4. The comment acknowledges that DMP_CLASS_CODES and DmpClassCode are misnomers — these are typeCode values (JDV_J07/LOINC), not classCode values (JDV_J06) — and defers the rename to a follow-up. The problem is that this PR is itself the semantic boundary event: it expands to the full PM-authoritative type-code set and removes the only caller that was treating these as class codes (LOINC_CODES_TO_KIND). Leaving the misnomer in place after that boundary event is crossed means every consumer introduced between now and the follow-up will anchor on the wrong name. The rename cost grows with each new callsite. The rename is a four-step mechanical operation that can be done in this PR: rename the constant, rename the type, update the two consumers (dmpCategoryOptions.ts, constants.test.ts), and update the re-exports. There is no functional risk. Recommendation: Rename DMP_CLASS_CODES → DMP_TYPE_CODES and DmpClassCode → DmpTypeCode in this PR. If a true follow-up is required for external package consumers, add a deprecated re-export shim rather than leaving the primary name wrong. 2. DMP_CLASS_CODE_LABELS is typed Record but string admits empty strings — the test catches this only at runtime /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/constants.ts, line 120. The type Record enforces key completeness (every code has an entry) but says nothing about value validity — an empty string "" satisfies it. The test at line 35 compensates by iterating and asserting .length > 0, but that is a runtime guard on a structure whose values are authored statically in the same file. A narrower type makes the constraint visible at the type level and removes the need for that test. Consider a branded or template-literal type: type NonEmptyString = string & { readonly __brand: 'NonEmptyString' } Or, at minimum, document why string is deliberately wide if wideness is intentional (e.g. to allow i18n substitution later). --- Warnings (should fix) 3. Mixed quotation styles on label values containing typographic apostrophes /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/constants.ts, lines 202–205. Four labels use a Unicode right-single-quotation-mark (', U+2019) inside regular straight-quote strings: 'PRESC-BIO': 'Prescription d'actes de biologie médicale', 'PRESC-KINE': 'Prescription d'actes de kinésithérapie', 'PRESC-PEDI': 'Prescription d'actes de pédicurie', 'PRESC-INF': 'Prescription d'actes infirmiers', The surrounding labels on lines 139–228 use a standard ASCII apostrophe (') escaped as part of a double-quoted string ("CR d'examens biologiques"). These four entries are inconsistent: they use a curly apostrophe to avoid escaping a straight-quote delimiter. This is an invisible source of bugs — a downstream string comparison against an API-sourced or i18n-sourced label would silently fail because ' ≠ '. All four should be normalised to the same encoding as the surrounding labels: 'PRESC-BIO': "Prescription d'actes de biologie médicale", 'PRESC-KINE': "Prescription d'actes de kinésithérapie", 'PRESC-PEDI': "Prescription d'actes de pédicurie", 'PRESC-INF': "Prescription d'actes infirmiers", The test does not cover these four entries explicitly, so the inconsistency has no test coverage. The "has a non-empty label for every code" test does not catch the character encoding difference. 4. The test for label correctness does not cover the four typographic-apostrophe entries /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/constants.test.ts, lines 14–24 and 26–32. The two it('has correct French labels...') blocks sample 14 codes between them but none of PRESC-BIO, PRESC-KINE, PRESC-PEDI, PRESC-INF. Adding at least one of these as an explicit assertion would have caught the typographic-apostrophe inconsistency in review or CI. 5. variants/dmp/ is a feature subdirectory, not a recognised UpFront layer — constants.ts may belong in src/business/constants/ /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/constants.ts. The UpFront architecture defines src/business/, src/data/, src/components/, src/hooks/, and src/helpers/ as the canonical layers. The variants/dmp/ directory is a feature-scoped subdirectory that does not map to any of these layers. This is not a violation introduced by this PR, but the PR adds to the surface area of a file that sits outside the canonical structure. An analogous constants file already exists at /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/constants/cdaTypeCodeToFacetName.ts. The DMP_CLASS_CODE_LABELS map is structurally identical: a static Record derived from the same PM-authoritative CSV. The two maps have 108 identical keys and diverge only in values (UI display label vs. facet name). Keeping them in different layer directories (variants/dmp/ vs. src/business/constants/) makes the relationship between the two maps invisible to a new contributor. This is worth raising as a follow-up structural question: should DMP_CLASS_CODE_LABELS (and DMP_CLASS_CODES) migrate to src/business/constants/ to sit alongside cdaTypeCodeToFacetName.ts? --- Suggestions (consider improving) 6. The removal of monolith_DEPRECATED is architecturally correct and should be noted explicitly The elimination of the import from monolith_DEPRECATED/models/document/fr/DocumentLoincCodesToKind is the most structurally significant aspect of this PR: it removes a runtime dependency on a deprecated monolith module and replaces it with a self-contained static map. This deserves a sentence in the PR description because it is a dependency hygiene improvement, not just a data expansion. 7. DMP_CATEGORY_OPTIONS in dmpCategoryOptions.ts now sorts 108 entries at module initialisation on every load /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/factories/dmpCategoryOptions.ts, line 4–7. The .sort((a, b) => a.label.localeCompare(b.label, 'fr')) call runs at module initialisation time and now operates over 108 entries rather than 9. The overhead is negligible in isolation, but it is worth noting that the sorted order could be baked directly into DMP_CLASS_CODE_LABELS or a separate ordered array, making the sort a no-op or eliminable. This is a minor point — do not change it unless profiling shows it matters. 8. The length assertion in the test is fragile as a single hardcoded number /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/dmp/constants.test.ts, line 11. expect(DMP_CLASS_CODES).toHaveLength(108) This test will fail the next time a code is added or removed from the PM CSV, requiring a manual update to the magic number. A comment explaining what "108" represents (the current PM CSV version/date) would help whoev …(truncated, 800 more chars)