Session: review-the-pr-https-github-com-doctolib-shiny-axolotl-subagent-a38f4fcf — 2026-04-23

Project: treatment-plan · Branch: main · Mode: ` · **Model:**claude-opus-4-7`

Stats: 1 user messages, 13 tool calls, tools used: Bash, Read.

Conversation

Turn 1 — User

Review PR #3019 in the doctolib/treatment-plan repository for code quality, style, and adherence to project guidelines. Do NOT post any comments on the PR — report your findings only as text output.

PR context

• Title: feat(FHIR-2194): pass document to dropdown action callbacks and add isAvailable filter
• Package: @doctolib/tpf-national-health-records-ui
• Change type: Frontend feature (TypeScript/React)

What changed

• types.ts: DropdownActionOption.onClick changed from () => void to (doc: NhrDocument) => void | Promise<void>; added isAvailable?: (doc: NhrDocument) => boolean; added 'link-primary' to uiStyle union
• ActionButton.tsx: filters options by isAvailable, passes document to MoreActions
• MoreActions.tsx: receives document, injects into onClick at click time
• NhrDocumentListModal.tsx: threads document through loading-state wrapper
• Test files and stories updated accordingly

How to get the diff

gh pr diff 3019 --repo doctolib/treatment-plan

What to focus on

• TypeScript correctness (no any, no eslint suppressions)
• Naming conventions and consistency
• Potential backward-compatibility issues with the onClick signature change
• React patterns (hooks, props, memo, key usage)
• Any dead code or TODOs left in
• Adherence to project CLAUDE.md rules (no eslint-disable, no snapshot tests, etc.)

Report findings categorized by severity (blocking / suggestion / nit).

Turn 1 — Assistant

I'll review PR #3019 by fetching the diff and examining the changes.

Tools used:

• Bash: gh pr diff 3019 --repo doctolib/treatment-plan

Tool results:

→ result (ok): diff --git a/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.stories.tsx b/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.stories.tsx index ee5f3198b1..d4e7919d7d 100644 --- a/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.stories.tsx +++ b/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.stories.tsx @@ -157,7 +157,9 @@ const sharedBySampleR …(truncated, 26519 more chars)

I have the full diff. Let me examine the key source files in more depth to understand the context and verify correctness.

Tools used:

• Read: /Users/USER/workspace/doctolib/treatment-plan/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 } 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. Returns a string for lo …(truncated, 3393 more chars)

Tools used:

• Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/ActionButton.tsx

Tool results:

→ result (ok): 1 import type { NhrDocument } from '@doctolib/tpf-national-health-records' 2
3 import type { ActionConfig } from '../../../../types' 4 import { MoreActions } from './MoreActions' 5 import { SimpleAction } from './SimpleAction' 6
7 type ActionButtonProps = { 8 action: ActionConfig 9 row: NhrDocument 10 } 11
12 export const ActionButton = ({ action, row }: ActionButtonProps) => { 13 if (action.kind === 'dropdown') { 14 if (action.options.length === 0) return null 15
16 return <Mo …(truncated, 102 more chars)

Tools used:

• Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/MoreActions.tsx

Tool results:

→ result (ok): 1 import { Dropdown } from '@doctolib/design-system/Dropdown' 2 import { DropdownTrigger } from '@doctolib/design-system/DropdownTrigger' 3 import type { IconName } from '@doctolib/design-system/Icon' 4 import { IconButton } from '@doctolib/design-system/IconButton/v2' 5
6 import type { DropdownActionConfig, DropdownActionOption } from '../../../../types' 7
8 type DropdownOptionProps = { 9 option: DropdownActionOption 10 } 11
12 const DropdownOption = ({ option }: DropdownOptionProps) => {

…(truncated, 1356 more chars)

Tools used:

• Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx

Tool results:

→ result (ok): 1 import { Modal } from '@doctolib/design-system/Modal' 2 import { ModalBody } from '@doctolib/design-system/ModalBody' 3 import type { NhrDocument } from '@doctolib/tpf-national-health-records' 4 import { useEffect, useMemo, useState } from 'react' 5
6 import type { ActionConfig, ColumnConfig, FilterConfig, NhrPatientInfo, SearchDocumentsFn } from '../../types' 7 import { NhrDocumentList } from '../NhrDocumentList/NhrDocumentList' 8
9 export type NhrDocumentListModalProps = { 10 isOpen: boo …(truncated, 2809 more chars)

Tools used:

• Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/ActionButton.test.tsx

Tool results:

→ result (ok): 1 import type { DesignSystemIconComponent } from '@doctolib/design-system-icons' 2 import { setup } from '@doctolib/test-helpers' 3 import { screen } from '@testing-library/react' 4 import { describe, expect, it, vi } from 'vitest' 5
6 import { createMoreActionsAction } from '../../../../business/factories/actions/createMoreActionsAction' 7 import { nhrDocumentFixtureBuilder } from '../../../../test-helpers/fixtures/nhrDocumentFixtureBuilder' 8 import type { SimpleActionConfig } from '../../../ …(truncated, 1421 more chars)

Now let me check the createMoreActionsAction factory and examine the test descriptions for an issue I spotted.

Tools used:

• Bash: find /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui -type f -name "createMoreActionsAction*"

Tool results:

→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/dist/business/factories/actions/createMoreActionsAction.js.map /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/dist/business/factories/actions/createMoreActionsAction.d.ts.map /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/dist/business/factories/actions/createMoreActionsAction.js /Users/USER/workspace/doctolib/treatment-plan/packages/ …(truncated, 384 more chars)

Tools used:

• Bash: grep -rn "isAvailable\|shouldBeVisible" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui --include="*.ts" --include="*.tsx"

Tool results:

→ result (ok): (Bash completed with no output)

Tools used:

• Bash: grep -rn "DropdownActionOption\|onClick" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx

Tool results:

→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx:61: onClick: async () => { /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx:65: await option.onClick() /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/ …(truncated, 266 more chars)

Note: the grep output for the file on disk shows the pre-change onClick signature for dropdown options. Let me confirm this by looking at the current file; since the diff is unstaged/unmerged, the working copy must represent the PR state. Let me check the current branch and file state.

Tools used:

• Bash: git log --oneline -5 main && git branch --show-current

Tool results:

→ result (ok): 2b7ed9a59 chore(ATF-1657): remove MFR config from tpf-ldt-lab-integration (#2996) 1dd982255 feat(MOOSE-4134): add dev toolbar with scenario presets and Storybook integration [Stack 11/12] (#2958) cb850abc3 docs(SEAL-1733): strengthen Claude workflow routing in AGENTS.md (#3000) a995b1bdd feat(FHIR-2194)!: render dropdown for actions in DocList table (#2997) 0ef9d99fc feat(ACAI-4219): add GET /bundles/{bundle_id}/templates endpoint (#2972) main

Tools used:

• Bash: gh pr view 3019 --repo doctolib/treatment-plan --json headRefName,baseRefName,state,isDraft

Tool results:

→ result (ok): {"baseRefName":"main","headRefName":"fhir-2194-3","isDraft":true,"state":"OPEN"}

I'm on main. The working copy shows pre-PR state. My review is based on the diff from gh pr diff 3019 which I already have. Let me look at the createMoreActionsAction factory to ensure the PR works correctly.

Tools used:

• Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/actions/createMoreActionsAction.ts

Tool results:

→ result (ok): 1 import { EllipsisVerticalRegularIcon } from '@doctolib/design-system-icons/EllipsisVerticalRegularIcon' 2
3 import type { DropdownActionConfig, DropdownActionOption } from '../../../types' 4
5 export const createMoreActionsAction = (options: DropdownActionOption[]): DropdownActionConfig => ({ 6 kind: 'dropdown', 7 key: 'more', 8 tooltip: 'More actions', 9 iconComponent: EllipsisVerticalRegularIcon, 10 variant: 'transparent', 11 options, 12 }) 13

Tools used:

• Bash: gh pr view 3019 --repo doctolib/treatment-plan --json files --jq '.files[].path'

Tool results:

→ result (ok): packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.stories.tsx packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.test.tsx packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/ActionButton.test.tsx packages/tpf-national-health-records-ui/src/compon …(truncated, 737 more chars)

Now I have a complete picture. Let me compile the review findings.

PR Review: #3019 — feat(FHIR-2194): pass document to dropdown action callbacks and add isAvailable filter

Files reviewed (all absolute paths): - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/types.ts - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/ActionButton.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/ActionButton.test.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/MoreActions.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/MoreActions.test.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/NhrDocumentListTableBody.test.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.test.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.stories.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/NhrDocumentListTableBody.stories.tsx

Blocking (Critical: 90–100)

1. Breaking change on a public API not marked with ! in the commit — confidence 92

File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/types.ts lines 80 & 82

• uiStyle widened from 'link-danger' to 'link-primary' | 'link-danger' (non-breaking, additive).
• onClick signature changed from () => void | Promise<void> to (doc: NhrDocument) => void | Promise<void> (breaking — existing consumers whose callback has no parameter are still assignable via TS bivariance, but any consumer that relied on DropdownActionOption['onClick'] as a value-level () => void type will break at the type level, and any programmatic call-site that invoked it with no argument relies on doc being undefined — which is now a contract violation).

Per the repo's CLAUDE.md (.claude/rules/general.md "Commit Message Format" section), breaking public-API changes MUST either append ! after the type/scope or include a BREAKING CHANGE: footer. The PR title is feat(FHIR-2194): pass document to dropdown action callbacks... — no !. The prior PR #2997 that introduced this component was correctly flagged feat(FHIR-2194)!:. This PR changes the same public surface of @doctolib/tpf-national-health-records-ui (the DropdownActionOption type is exported via types.ts) and must also be marked breaking.

Fix: retitle commit / PR to feat(FHIR-2194)!: ... or add a BREAKING CHANGE: footer explaining the onClick signature change.

Important (80–89)

2. NhrDocumentListModal wrappedActions wrapper is type-incorrect post-change — confidence 88

File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx lines 58–71

After the diff:

options: action.options.map((option) => ({
  ...option,
  onClick: async (doc: NhrDocument) => {
    setIsActionLoading(true)
    try {
      await option.onClick(doc)
    } finally {
      setIsActionLoading(false)
    }
  },
})),

The inline parameter annotation doc: NhrDocument duplicates a type that is now inferable from the DropdownActionOption.onClick signature. That is purely stylistic. But more importantly: option.onClick(doc) is passed through correctly; however the wrapper discards the isAvailable guard entirely — isAvailable is declared at the type level but the wrapper ...option spread only preserves it, and NhrDocumentList downstream does use it. The isActionLoading wrapper is fine.

The real concern: the inline doc: NhrDocument annotation is redundant because the callback's parameter type is already inferred by the object literal being assigned to DropdownActionOption. Remove it for consistency (onClick: async (doc) => { ... }) — the surrounding code in the same file (line 76 for simple action) already uses the pattern async (doc: NhrDocument) => ..., so fine either way, but stay consistent.

This is a style-only nit — downgraded from "important" to suggestion. See Suggestion #3 below.

3. Test description references a renamed/removed prop, confidence 85

File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/ActionButton.test.tsx lines 48 and 64 (added hunks)

it('should render the option when shouldBeVisible returns true for the row', () => { ... })
it('should render nothing when shouldBeVisible returns false for all options', () => { ... })
it('should pass the row to shouldBeVisible', () => { ... })

The prop is named isAvailable (in types.ts and in ActionButton.tsx), not shouldBeVisible. Test names describe the behavior in terms of a prop name that does not exist in the source. The tests themselves set isAvailable: vi.fn()... correctly, so functionally they pass, but the it() descriptions are misleading. The third test even names the local variable const shouldBeVisible = vi.fn() and then passes it as isAvailable: shouldBeVisible — that is stale naming from an earlier draft.

Fix: replace shouldBeVisible with isAvailable in all three test descriptions and in the local variable on the third test, to match the actual public API.

Suggestions (Valid but lower impact: 70–79)

4. Grammar fix in test description is incomplete — confidence 75

File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/ActionButton.test.tsx lines 30, 39

The PR fixes "an DropdownActionConfig" → "a DropdownActionConfig" in two places, which is correct. No action needed — just noting it is good cleanup bundled with the feature.

5. MoreActions.tsx handleClick closure recreated on every render — confidence 70

File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/MoreActions.tsx line 15

const handleClick = () => option.onClick(doc)

Acceptable since DropdownOption is a very small presentational leaf, but if the dropdown list becomes long this could be memoized with useCallback. Not a blocker; Oxygen Dropdown.Item / Dropdown.Button don't appear to be memoized consumers, so there is no measurable win. Skip unless perf profiling shows a problem.

6. NhrDocumentListModal.tsx inline NhrDocument type annotation is redundant — confidence 72

File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx line 61

The explicit doc: NhrDocument annotation is already inferable from DropdownActionOption['onClick']. Removing it (and keeping the import if still used elsewhere) shortens the closure and keeps the callback aligned with the type system:

onClick: async (doc) 

…(truncated, 3836 more chars)