Project: workspace-doctolib · Branch: HEAD · Mode: ` · **Model:**claude-sonnet-4-6`
Stats: 1 user messages, 10 tool calls, tools used: Bash, Read.
Conversation
Turn 1 — User
Analyze test coverage for the monolith repo changes at /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2223_fix-column-widths-dmp (PR #241187).
Our changes (last 2 commits, HEAD~2..HEAD) touch exactly:
- packages/@doctolib/national-health-records/src/variants/dmp/factories/columnsFactory.tsx — assigns width: '20%' to category column and width: equalWidth to the remaining 6 columns (equal division of 80%)
- packages/@doctolib/national-health-records/src/variants/dmp/factories/columnsFactory.test.ts — tests the width assignments
Run git diff HEAD~2..HEAD from the worktree root. Check: are all new behaviors tested? Is the width computation (category=20%, remaining sum=80%) properly verified? Are there missing edge cases?
Turn 1 — Assistant
I'll analyze the test coverage for this PR by examining the changes and tests.
Tools used:
Bash:git diff HEAD~2..HEAD
Tool results:
→ result (ok): diff --git a/packages/@doctolib/national-health-records/src/variants/dmp/factories/columnsFactory.test.ts b/packages/@doctolib/national-health-records/src/variants/dmp/factories/columnsFactory.test.ts index f792b883ee37..825cfbc05b41 100644 --- a/packages/@doctolib/national-health-records/src/variants/dmp/factories/columnsFactory.test.ts +++ b/packages/@doctolib/national-health-records/src/variants/dmp/factories/columnsFactory.test.ts @@ -113,4 +113,28 @@ describe('columnsFactory', () => {
…(truncated, 2079 more chars)
Tools used:
Bash:git log --oneline HEAD~2..HEAD
Tool results:
→ result (ok): 91240c98feee fix(FHIR-2223): drop toFixed(2) so remaining widths sum to exactly 80% 4623894f46db fix(FHIR-2223): assign proportional widths in columnsFactory for DMP documents
Now let me read the full test file and the factory source to understand the complete context.
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2223_fix-column-widths-dmp/packages/@doctolib/national-health-records/src/variants/dmp/factories/columnsFactory.test.ts
Tool results:
→ result (ok): 1 import { createNhrDocumentCategoryColumn } from '@doctolib/tpf-national-health-records-ui'
2 import { DMP_CLASS_CODE_LABELS } from '../constants'
3 import { columnsFactory } from './columnsFactory'
4
5 jest.mock('@doctolib/dmp', () => ({
6 DOCUMENT_STATUS: {
7 validated: 'validated',
8 obsolete: 'obsolete',
9 archived: 'archived',
10 },
11 }))
12
13 jest.mock('../mappers/dmpDocumentHelpers', () => ({
14 getTypeCode: jest.fn(),
15 getProcedureDate: jest.fn(),
16 getShare
…(truncated, 4207 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2223_fix-column-widths-dmp/packages/@doctolib/national-health-records/src/variants/dmp/factories/columnsFactory.tsx
Tool results:
→ result (ok): 1 import type { ColumnConfig } from '@doctolib/tpf-national-health-records-ui'
2 import {
3 createNhrDocumentCategoryColumn,
4 createNhrDocumentNameColumn,
5 createNhrProcedureDateColumn,
6 createNhrShareDateColumn,
7 createNhrSharedByColumn,
8 createNhrStatusColumn,
9 createNhrVisibilityColumn,
10 } from '@doctolib/tpf-national-health-records-ui'
11 import { DOCUMENT_STATUS } from '@doctolib/dmp'
12 import type { DesignSystemIconComponent } from '@doctolib/design-system-icons'
13
…(truncated, 2785 more chars)
Now I have a complete picture of both the source and tests. Let me do a quick arithmetic check on the width computation to validate correctness.
Tools used:
Bash:node -e " …
Tool results:
→ result (ERROR): Exit code 127 zsh: command not found: node
Tools used:
Bash:python3 -c " …
Tool results:
→ result (ok): Total columns: 7 Remaining columns: 6 Equal width per column: 13.333333333333334 Formatted string: 13.333333333333334% Sum check: 80.0 Is exact integer? False Test parseFloat simulation: 13.333333333333334 Test sum result (expect 80): 80.0
What if columns count was 8 (6+1=7 remaining)? 5 columns -> remaining=4, width=20.0%, sum=80.0 6 columns -> remaining=5, width=16.0%, sum=80.0 7 columns -> remaining=6, width=13.333333333333334%, sum=80.0 8 columns -> remaining=7, width=11.42857142857 …(truncated, 65 more chars)
Tools used:
Bash:python3 -c " …
Tool results:
→ result (ok): 80/6 = 13.333333333333334 str(80/6) = 13.333333333333334 Ends with repeating decimal in string? True
Crucially: 6 * (80/6) in floating point: 80.0 == 80? True
Tools used:
Bash:python3 -c " …
Tool results:
→ result (ok): With toFixed(2): width per col = 13.33 Sum = 79.98 (expected 80, got 79.98 ) Diff = 0.01999999999999602
--- Identifying test gaps ---
Test 1 (category=20%): PASSES - directly asserts result.at(0).width == 20% Test 2 (equal widths after cat): PASSES - checks uniformity of remaining cols Test 3 (sum=80%): PASSES - validates the toFixed fix, but has a fragility: - uses result.at(1)?.width as base, which is undefined if only 1 column exists - parseFloat of undefined => NaN => NaN*0 => NaN != 8 …(truncated, 489 more chars)
Tools used:
Bash:python3 -c " …
Tool results:
→ result (ok): Test 2 has a potential false positive if equalWidth is undefined/empty: - result.at(1)?.width = undefined - forEach checks: col.width === undefined for each -> vacuously true for all - Test 2 PASSES even though widths are broken - Test 3 catches this: NaN != 80
This is not a critical gap because test 3 covers it, but test 2 is not as strong as it looks. A minor improvement: test 2 should also assert equalWidth is a non-empty string.
Tools used:
Bash:python3 -c " …
Tool results:
→ result (ok): <string>:33: SyntaxWarning: "\d" is an invalid escape sequence. Such sequences will not work in the future. Did you mean "\d"? A raw string is also an option. === Regression Analysis ===
CAUGHT by existing tests: 1. category column NOT having 20% width 2. remaining columns having different widths from each other 3. toFixed(2) re-introduction (79.98 != 80 -> test 3 fails) 4. width property completely absent on category column
NOT CAUGHT by existing tests: A. equalWidth being undefine …(truncated, 1465 more chars)
Here is the full analysis.
Summary
The PR adds three new tests to cover width assignments introduced in columnsFactory.tsx. Overall the coverage is solid for the primary behaviors, correctly verifies the fix that motivated this PR (the toFixed(2) rounding issue), and doesn't over-specify implementation details. There are two minor gaps worth noting.
Critical Gaps
None. No tests rated 8-10 are missing.
Important Improvements
Gap 1 — Test 2 has a silent false-positive scenario when equalWidth is undefined
Rating: 5/10
The "equal widths" test reads the reference value from result.at(1)?.width. If a future change caused equalWidth to be undefined (for example, a bad refactor of the division formula), every remaining column would have width: undefined. The forEach assertion would then be undefined === undefined for each entry, which vacuously passes. The test would produce a green result even though no widths are set.
Test 3 (the sum test) is the only safety net in that scenario — parseFloat(undefined ?? '') = NaN, and NaN !== 80 fails the assertion. So no regression actually escapes the suite today, but Test 2 gives a false confidence signal in isolation.
A simple hardening: add expect(equalWidth).toMatch(/^\d+(\.\d+)?%$/) before the forEach, or expect(typeof equalWidth).toBe('string'). This makes Test 2 self-sufficient rather than relying on Test 3 to catch the same problem with a less descriptive failure message.
Gap 2 — No test verifies that remaining column widths are formatted as % strings
Rating: 4/10
Test 1 hard-codes '20%' so the format is verified for the category column. For remaining columns, Test 3 uses parseFloat(result.at(1)?.width ?? ''), which silently strips any unit suffix — parseFloat('13.33px') returns 13.33 just as well as parseFloat('13.333...%'). A bug that used pixel values instead of percentages would pass all three tests. Since this is a UI layout concern, the risk is real but low-severity (it would be immediately visible visually). Asserting expect(result.at(1)?.width).toMatch(/^\d+(\.\d+)?%$/) in the equal-widths test would close this gap.
Test Quality Issues
Test 3's toBe(80) is fragile to floating-point accumulation in JS
The computation remainingCount * parseFloat(result.at(1)?.width ?? '') relies on 6 * (80/6) === 80.0 being true in JavaScript's IEEE 754 doubles. It is true today (the multiplication happens to be exact), but this is a coincidence of this specific divisor. If the column count ever changes to a value where 80/N produces a repeating binary fraction that doesn't round-trip, the test could fail spuriously. For example, 7 remaining columns gives 80/7 ≈ 11.4285714..., and 7 * 11.4285714... !== 80 in floating point. A future developer adding a column would see a mysterious test failure.
The idiomatic fix is to use toBeCloseTo(80, 5) or to assert sum >= 79.99 && sum <= 80.01 explicitly. However, because the current column count (6) does produce an exact result with the current formula, this is a latent rather than active issue.
Rating: 4/10 — not urgent, but worth noting before the column list grows.
Positive Observations
-
Test 1 is precise and direct. It asserts the exact string
'20%', covering both the numeric value and the%unit, for the exact position (index 0). This would catch any regression that reorders columns or changes the category width. -
Test 3 correctly validates the motivation for this PR. The second commit message ("drop
toFixed(2)so remaining widths sum to exactly 80%") indicates that a previous iteration rounded each width to two decimal places, producing6 × 13.33 = 79.98. Test 3 would fail on that code, confirming it is a meaningful regression guard for the bug that was just fixed. -
The mock setup is clean and behavioral. All seven column-creator mocks return objects with only
key,header,sortable, andrender— nowidthproperty — which means thewidthproperties seen in test assertions must come entirely from thecolumnsFactoryspread logic. The tests cannot accidentally pass because a mock pre-populated the width. -
The pre-existing tests (
getTypeCodebehavior) are unaffected and continue to test the column configuration logic orthogonally to the new width tests, keeping concerns well separated.
File Paths Relevant to This Analysis
/Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2223_fix-column-widths-dmp/packages/@doctolib/national-health-records/src/variants/dmp/factories/columnsFactory.tsx/Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2223_fix-column-widths-dmp/packages/@doctolib/national-health-records/src/variants/dmp/factories/columnsFactory.test.ts