Project: treatment-plan · Branch: main · Mode: ` · **Model:**claude-sonnet-4-6`
Stats: 1 user messages, 26 tool calls, tools used: Bash, ToolSearch, mcp__plugin_github_github__get_file_contents.
Conversation
Turn 1 — User
Analyze the test coverage in PR https://github.com/doctolib/doctolib/pull/240556 (branch fhir-2216_fix-nhr-date-columns-and-document-type).
This PR fixes NHR Document List rendering bugs:
1. DmpNhrAdapter.ts: Added parseHl7DateToIso (HL7 DTM → ISO 8601) and dual-code classification (both dmp:classCode and dmp:typeCode)
2. dmpDocumentHelpers.ts: Updated getTypeCode to prefer dmp:typeCode, fixed date helper return types, split DMP_SYSTEM constant
New/updated test files:
- DmpNhrAdapter.test.ts: 17 tests including 5 new HL7 date-parsing cases
- dmpDocumentHelpers.test.ts: 22 tests including all new helper behavior
Key acceptance criteria to verify coverage for:
- HL7-8 (YYYYMMDD) and HL7-14 (YYYYMMDDHHMMSS) dates parse to correct ISO strings
- Empty/undefined dates → undefined (not "Invalid Date")
- getTypeCode returns dmp:typeCode when present, falls back to dmp:classCode
- isPatientAuthored still checks classCode '90' (not typeCode)
- When doc.typeCode is absent/empty, only one code entry in classification
Report any critical coverage gaps.
Turn 1 — Assistant
I'll analyze the test coverage for this PR by examining the changed files and their tests.
Tools used:
Bash:cd /Users/USER/workspace/doctolib && git worktree list 2>/dev/null | grep fhir-2216
Tools used:
Bash:gh pr view 240556 --repo doctolib/doctolib --json headRefName,baseRefName,files 2>/dev/null | head -100
Tool results:
→ result (ok): (Bash completed with no output)
Tool results:
→ result (ok): {"baseRefName":"master","files":[{"path":"packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts","additions":47,"deletions":4,"changeType":"MODIFIED"},{"path":"packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts","additions":30,"deletions":7,"changeType":"MODIFIED"},{"path":"packages/@doctolib/national-health-records/src/mocks/nhrMockedDocuments.ts","additions":115,"deletions":90,"changeType":"MODIFIED"},{"path":"packages/@doctolib/n …(truncated, 351 more chars)
Now let me fetch the actual file contents from the PR.
Tools used:
ToolSearch(inputs: query, max_results)
Tool results:
→ result (ok)
Let me fetch all the relevant files in parallel.
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, ref)
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, ref)
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, ref)
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, ref)
Tool results:
→ result (ERROR): failed to resolve git reference: failed to get final reference for "refs/heads/fhir-2216_fix-nhr-date-columns-and-document-type": GET https://api.github.com/repos/doctolib/doctolib/git/ref/heads/fhir-2216_fix-nhr-date-columns-and-document-type: 404 Not Found []
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, ref)
Tool results:
→ result (ERROR): failed to resolve git reference: failed to get final reference for "refs/heads/fhir-2216_fix-nhr-date-columns-and-document-type": GET https://api.github.com/repos/doctolib/doctolib/git/ref/heads/fhir-2216_fix-nhr-date-columns-and-document-type: 404 Not Found []
Tool results:
→ result (ERROR): failed to resolve git reference: failed to get final reference for "refs/heads/fhir-2216_fix-nhr-date-columns-and-document-type": GET https://api.github.com/repos/doctolib/doctolib/git/ref/heads/fhir-2216_fix-nhr-date-columns-and-document-type: 404 Not Found []
Tool results:
→ result (ERROR): failed to resolve git reference: failed to get final reference for "refs/heads/fhir-2216_fix-nhr-date-columns-and-document-type": GET https://api.github.com/repos/doctolib/doctolib/git/ref/heads/fhir-2216_fix-nhr-date-columns-and-document-type: 404 Not Found []
Tool results:
→ result (ERROR): failed to resolve git reference: failed to get final reference for "refs/heads/fhir-2216_fix-nhr-date-columns-and-document-type": GET https://api.github.com/repos/doctolib/doctolib/git/ref/heads/fhir-2216_fix-nhr-date-columns-and-document-type: 404 Not Found []
Let me try the PR head ref instead.
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, ref)
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, ref)
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, ref)
Tool results:
→ result (ERROR): failed to resolve git reference: failed to get final reference for "refs/pull/240556/head": GET https://api.github.com/repos/doctolib/doctolib/git/ref/pull/240556/head: 404 Not Found []
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, ref)
Tool results:
→ result (ERROR): failed to resolve git reference: failed to get final reference for "refs/pull/240556/head": GET https://api.github.com/repos/doctolib/doctolib/git/ref/pull/240556/head: 404 Not Found []
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, ref)
Tool results:
→ result (ERROR): failed to resolve git reference: failed to get final reference for "refs/pull/240556/head": GET https://api.github.com/repos/doctolib/doctolib/git/ref/pull/240556/head: 404 Not Found []
Tool results:
→ result (ERROR): failed to resolve git reference: failed to get final reference for "refs/pull/240556/head": GET https://api.github.com/repos/doctolib/doctolib/git/ref/pull/240556/head: 404 Not Found []
Tool results:
→ result (ERROR): failed to resolve git reference: failed to get final reference for "refs/pull/240556/head": GET https://api.github.com/repos/doctolib/doctolib/git/ref/pull/240556/head: 404 Not Found []
The GitHub API can't reach the branch via ref. Let me try to get it via the PR's commit SHA, and also check if there's a local worktree.
Tools used:
Bash:cd /Users/USER/workspace/doctolib && git worktree list 2>/dev/null
Tools used:
Bash:gh pr view 240556 --repo doctolib/doctolib --json headRefOid,headRepository 2>/dev/null
Tool results:
→ result (ERROR): Exit code 128
Tool results:
→ result (ERROR): <tool_use_error>Cancelled: parallel tool call Bash(cd /Users/USER/workspace/doctolib &…) errored</tool_use_error>
Tools used:
Bash:gh pr view 240556 --repo doctolib/doctolib --json headRefOid,headRepository 2>/dev/null
Tool results:
→ result (ok): {"headRefOid":"7202a79098b2b88e54accc2ca5e5846f797625a3","headRepository":{"id":"MDEwOlJlcG9zaXRvcnkxNDAyMjQyMQ==","name":"doctolib","nameWithOwner":"doctolib/doctolib"}}
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, sha)
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, sha)
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, sha)
Tool results:
→ result (ERROR): failed to get git tree: GET https://api.github.com/repos/doctolib/doctolib/git/trees/7202a79098b2b88e54accc2ca5e5846f797625a3?recursive=1: 404 Not Found []
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, sha)
Tool results:
→ result (ERROR): failed to get git tree: GET https://api.github.com/repos/doctolib/doctolib/git/trees/7202a79098b2b88e54accc2ca5e5846f797625a3?recursive=1: 404 Not Found []
Tools used:
mcp__plugin_github_github__get_file_contents(inputs: owner, repo, path, sha)
Tool results:
→ result (ERROR): failed to get git tree: GET https://api.github.com/repos/doctolib/doctolib/git/trees/7202a79098b2b88e54accc2ca5e5846f797625a3?recursive=1: 404 Not Found []
Tool results:
→ result (ERROR): failed to get git tree: GET https://api.github.com/repos/doctolib/doctolib/git/trees/7202a79098b2b88e54accc2ca5e5846f797625a3?recursive=1: 404 Not Found []
Tool results:
→ result (ERROR): failed to get git tree: GET https://api.github.com/repos/doctolib/doctolib/git/trees/7202a79098b2b88e54accc2ca5e5846f797625a3?recursive=1: 404 Not Found []
The GitHub MCP tool doesn't have access to this private repo's tree. Let me check for a local worktree and use the gh CLI to pull the diff directly.
Tools used:
Bash:ls /Users/USER/workspace/doctolib/worktrees/ 2>/dev/null
Tools used:
Bash:gh pr diff 240556 --repo doctolib/doctolib 2>/dev/null | head -500
Tool results:
→ result (ok): cleanup-merged-worktrees.sh doctolib treatment-plan
Tool results:
→ result (ok): diff --git a/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts b/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts index c053604527bfd..e90706214ee04 100644 --- a/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts +++ b/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts @@ -49,6 +49,7 @@ const dmpSearchResult: DmpSearchResult = { classCode: '114 …(truncated, 20787 more chars)
Tools used:
Bash:gh pr diff 240556 --repo doctolib/doctolib 2>/dev/null | tail -600
Tool results:
→ result (ok): format: 'application/pdf', serviceInformation: { startDate: '2024-10-13' }, authors: [{ id: 'doc-28', firstName: 'Éric', lastName: 'Sanchez', role: 'healthcare_professional' }], - classification: { codes: [{ system: 'dmp:classCode', code: 'Pathology report' }] }, + classification: { codes: [{ system: 'dmp:classCode', code: '34874-8' }] }, }, { id: 'mock-043', title: 'Bone marrow biopsy — haematology', creationDate: '2024-05-09', submissionDate: undefined …(truncated, 24974 more chars)
I now have all the information I need. Let me do a thorough analysis of the implementation against the tests.
Tools used:
Bash:gh pr diff 240556 --repo doctolib/doctolib 2>/dev/null | grep "^+" | grep -E "parseHl7DateToIso|serviceStopDate|submissionDate" | head -40
Tool results:
→ result (ok): + submissionDate: '20240320142530', + submissionDate: '2024-03-20T14:25:30', +function parseHl7DateToIso(hl7Date: string | undefined): string | undefined { + creationDate: parseHl7DateToIso(doc.creationDate), + submissionDate: parseHl7DateToIso(doc.submissionDate), + startDate: parseHl7DateToIso(doc.serviceStartDate), + endDate: parseHl7DateToIso(doc.serviceStopDate), + it('returns the ISO date string from submissionDate unchanged', () => { + const d = doc({ s …(truncated, 185 more chars)
Tools used:
Bash:gh pr diff 240556 --repo doctolib/doctolib 2>/dev/null | grep -A5 -B5 "serviceStopDate\|endDate" | head -80
Tool results:
→ result (ok): classCode: '11488-4',
typeCode: 'some-type',
creationDate: '20240315',
+ submissionDate: '20240320142530',
serviceStartDate: '',
serviceStopDate: '',
healthCareFacilityTypeCode: '',
@@ -128,9 +129,9 @@ describe('DmpNhrAdapter', () => {
title: 'Compte rendu',
description: '',
identifiers: { uniqueId: 'doc-123', uuid: 'uuid-456' },
- creationDate: '20240315',
- submissionDate: undefined,
-
…(truncated, 2771 more chars)
Now I have a complete picture. Let me compile my analysis.
Summary
The PR delivers solid coverage for the three main behavior changes: HL7 date parsing, dual-code classification, and the getTypeCode fallback chain. All acceptance criteria from the ticket description have at least one test. There are no critical data-loss or security gaps. There are, however, a handful of missing edge cases and one meaningful behavioral gap worth raising.
Critical Gaps
None rated 9–10. No security concerns or data-loss paths are uncovered.
Important Improvements
1. parseHl7DateToIso with an unrecognized length returns undefined silently — no test for this path (rating: 7)
The implementation returns undefined for any input that is neither 8 nor 14 characters (e.g., a 6-character partial date '202403', a 12-character timestamp, or a non-empty garbage string from the DMP API). A future caller receiving undefined where a date was expected could display a blank column without any error signal, making it an invisible regression. The function itself is not exported for direct unit testing, so the only way to cover this is through the DmpNhrAdapter.test.ts integration path.
Suggested test to add inside the HL7 date conversion block in /packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts:
it('returns undefined for an unrecognised HL7 date length', async () => {
const result = await new DmpNhrAdapter(
createMockDmpApi({ documents: [{ ...baseDoc, creationDate: '202403' }] })
).searchDocuments(searchProps)
expect(result.documents[0]?.creationDate).toBeUndefined()
})
This would catch a future change that adds a fallback like new Date(hl7Date) (which produces "Invalid Date") or a naive substring that returns a malformed string.
2. serviceStopDate (endDate) has no explicit non-empty HL7 conversion test (rating: 6)
The existing baseline test at line 129 of the adapter test confirms that empty serviceStopDate: '' maps to endDate: undefined. But there is no test confirming that a valid serviceStopDate: '20231201' is converted to '2023-12-01'. The startDate equivalent is tested explicitly. Since both go through the same parseHl7DateToIso call, the risk of regression here is low, but if the mapping were ever refactored to handle start/stop differently the gap would be silent.
Suggested test to add:
it('converts HL7-8 serviceStopDate to ISO date string', async () => {
const result = await new DmpNhrAdapter(
createMockDmpApi({ documents: [{ ...baseDoc, serviceStopDate: '20231231' }] })
).searchDocuments(searchProps)
expect(result.documents[0]?.serviceInformation?.endDate).toBe('2023-12-31')
})
3. getTypeCode with only a dmp:typeCode entry (no classCode at all) is not tested (rating: 5)
The three tested shapes are: both present (returns typeCode), only classCode (returns classCode), neither (returns undefined). There is no test for only dmp:typeCode present with no classCode entry. This is an unlikely API shape but is a valid code path since find on the classCode would return undefined and the nullish coalesce would fall through to the typeCode result — which is actually the correct behaviour. Worth pinning so a refactor can't accidentally short-circuit it.
it('returns the typeCode when only dmp:typeCode entry is present', () => {
const d = doc({ classification: { codes: [{ system: 'dmp:typeCode', code: '11488-4' }] } })
expect(getTypeCode(d)).toBe('11488-4')
})
Test Quality Issues
getAuthorName with a null/undefined first or last name is untested
The test covers two cases: one author present (returns "Jean Dupont") and empty authors list (returns ''). If firstName or lastName on an author object is ever undefined (which is possible if the DMP API omits optional fields), the result would be "undefined Dupont" or similar. This is a quality observation rather than a critical gap since NhrDocumentAuthor likely types both fields as string, but it's worth noting for data-defensiveness.
getProcedureDate and getShareDate tests pass through the ISO string unchanged
The tests confirm pass-through of a pre-formed ISO string, but the helpers used to call new Date(...).toLocaleDateString(). The PR correctly removes that locale-sensitive conversion. The tests adequately capture the new contract (identity function returning string | undefined). No issue here — just noting the tests are testing the right thing.
Positive Observations
- The HL7-8 and HL7-14 date parsing tests are specific about the exact output format (
'2023-09-07'vs'2023-09-07T08:30:15'), which is exactly right — they would catch any ISO 8601 formatting mistake. - The
isPatientAuthoredtest that explicitly verifiesdmp:typeCode '90'returnsfalse(not just a truthy classCode check) is excellent. It pins the security-relevant EX_3.1-1040 rule against future confusion of the two systems and would catch any accidental broadening of the check. - The absence-of-typeCode test in the adapter ("omits dmp:typeCode entry when typeCode is absent on the document") correctly exercises the
if (doc.typeCode)guard on an empty string, covering the JS truthiness edge thatundefinedand''both skip the push. - The baseline
searchDocumentsintegration test was updated atomically with the newsubmissionDatefixture, so the "full document round-trip" snapshot stays in sync with all four new conversions at once. - The
dmpDocumentHelpers.test.tsrefactor from a single-argdoc()helper to aPartial<NhrDocument>override pattern is a meaningful improvement — it makes future test additions much less brittle and is a good DAMP example.