Overhaul medication top-4 boxes with a smart content-fit rule#663
Conversation
The four top boxes on the medication detail page (and several search cells) crammed full clinical sentences into a format built for short, glanceable values. Introduce one shared rule — constrained surfaces show curated crisp tokens; full content stays one tap away in the sections/reference — and apply it across the detail page and prescribing search without removing any content. Top-4 boxes: replace the verbose derived tiles (medicationDetailTiles) with medicationHeroMetrics, driven by each record's already-curated ~4-item stats array (Max Dose · half-life · drug-specific risk/caution flags). Max Dose leads and renders as the primary/clinical tile (its flag:"hi" marks ceiling importance, not danger) so red stays reserved for genuine safety signals; tone comes from medicationStatTone. A clamped one-line indication subtitle under the header preserves the "what it treats" orientation. Content-fit rule: add a shared shortValue() helper (first clinical clause + word-boundary cap, generalising the formulationShortLabel idiom); crisp medicationIndication to a single clause; clamp the verbose search cells (indication, prescribing action, dose) so rows stay uniform; and collapse long quick-reference rows with a chevron using the differential clamp-on-collapse pattern (line-clamp-2 + group-open:line-clamp-none). Retire the now-duplicate sidebar "Key stats" card. Tests cover shortValue capping/clause preservation and medicationHeroMetrics ordering, capping, tone, and backfill. Verified via lint, typecheck, vitest, prettier, type/icon-scale + brand checks, and Chromium screenshots across five drug classes in light/dark/mobile (no horizontal overflow). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBemWnYhqdGXBzwJUTQAqe
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMedication helpers now produce concise indications, doses, and hero metrics. The medication record page renders tone-driven metrics, clamped indications, and expandable quick-reference values. Prescribing results constrain indication and action text across desktop and mobile layouts. ChangesMedication UI presentation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MedicationRecordDetail
participant medicationHeroMetrics
participant DetailTile
MedicationRecordDetail->>medicationHeroMetrics: provide medication record
medicationHeroMetrics-->>MedicationRecordDetail: return ordered hero metrics
MedicationRecordDetail->>DetailTile: render metric label, value, and tone
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f3c6e594d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
Routing medicationIndication through firstClinicalSentence made it treat a single-letter clinical abbreviation followed by a period as a sentence end, so Clarithromycin's "Atypical CAP, H. pylori eradication, severe MAC complex in HIV." rendered as "Atypical CAP, H" — dropping the H. pylori indication — in the detail header and search/cross-mode subtitles. Skip the split when the token before the period is a lone letter (genus abbreviations like H. pylori, E. coli, C. difficile). Adds unit coverage plus a Clarithromycin snapshot regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBemWnYhqdGXBzwJUTQAqe
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f3c6e594d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/medications.ts (1)
2-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the max-dose label check into a shared helper
src/lib/medications.tsnow importsmedicationStatTonefromsrc/lib/medication-badges.ts, whilesrc/lib/medication-badges.tsimportsMedicationRecord/MedicationStatfromsrc/lib/medications.ts, creating a circular dependency./^(?:max\s+)?dose$/iis duplicated inheroMetricTone()andmedicationHeroMetrics(); a smallisMaxDoseLabel()helper would keep the tone and ordering logic aligned.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/medications.ts` around lines 2 - 3, Extract the duplicated max-dose label matching into a shared isMaxDoseLabel helper, then reuse it from heroMetricTone() and medicationHeroMetrics() for tone and ordering decisions. Move the helper to a dependency-neutral module or otherwise remove the medications.ts ↔ medication-badges.ts circular import while preserving the existing case-insensitive “dose”/“max dose” behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/clinical-dashboard/medication-prescribing-workspace.tsx`:
- Around line 619-621: Apply the same single-line clamping used for the mobile
indication and action to the mobile dose rendering of result.dose, preserving
its existing styling while adding the appropriate line-clamp class so long
values cannot expand the card.
---
Nitpick comments:
In `@src/lib/medications.ts`:
- Around line 2-3: Extract the duplicated max-dose label matching into a shared
isMaxDoseLabel helper, then reuse it from heroMetricTone() and
medicationHeroMetrics() for tone and ordering decisions. Move the helper to a
dependency-neutral module or otherwise remove the medications.ts ↔
medication-badges.ts circular import while preserving the existing
case-insensitive “dose”/“max dose” behavior.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e86a216-7f30-49bb-ab01-d08fff69d2dc
📒 Files selected for processing (4)
src/components/clinical-dashboard/medication-prescribing-workspace.tsxsrc/components/clinical-dashboard/medication-record-page.tsxsrc/lib/medications.tstests/medications.test.ts
Address CodeRabbit review on #663: - The mobile prescribing-result dose value was unclamped while indication and action were bounded, so a long dose could still expand the card. Add line-clamp-2 to match the desktop dose cell and the fixed-line content-fit goal. - Extract the duplicated /^(?:max\s+)?dose$/i test into a shared isMaxDoseLabel helper reused by heroMetricTone (tone) and medicationHeroMetrics (ordering) so the two stay aligned. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBemWnYhqdGXBzwJUTQAqe
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. The agent ran but didn't make any changes. The issues may already be fixed or require manual intervention. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91edfdd49d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Some records repeat a stat label (adrenaline-epinephrine has two "Route"
stats), so keying the hero tiles by label alone produced duplicate React keys
— risking tile reuse on client updates and a duplicate-key warning. Key by
`${label}-${index}` instead. Adds a snapshot test asserting the two Route
metrics both survive.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JBemWnYhqdGXBzwJUTQAqe
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efbda6f18f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The earlier genus fix skipped the sentence split after ANY lone letter before a period, which over-merged legitimate single-letter sentence ends across ~62 snapshot rows (e.g. "Pregnancy Category D. ...", "Vitamin C. ...", "Penicillin G. ..."). Restrict the skip to genus-species shape — a lone letter followed by a lowercase species (H. pylori, E. coli, C. difficile) — by also checking the next word starts lowercase. Adds counter-case tests that must still split. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBemWnYhqdGXBzwJUTQAqe
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/medications.ts (2)
366-370: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSchedule backfill bypasses the shared truncation, unlike its sibling entries.
Usual doseandCategorybackfills both go throughshortValue(...)to enforce the hero-tile content-fit contract, butScheduleusesrecord.scheduledirectly. A long schedule string will render untruncated in the constrained tile, defeating the purpose of this PR's condensed-value rule for that one field.✏️ Proposed fix
- record.schedule ? { label: "Schedule", value: record.schedule, tone: "neutral" } : null, + record.schedule ? { label: "Schedule", value: shortValue(record.schedule), tone: "neutral" } : null,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/medications.ts` around lines 366 - 370, The Schedule backfill in the `backfills` array bypasses the shared truncation applied to the other medication metrics. Update its value to pass `record.schedule` through `shortValue(...)`, while preserving the existing conditional inclusion and label/tone.
175-191: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSingle-letter abbreviation guard is too broad.
It skips any single-letter token before a period, soVitamin D. Monitor levels.andHepatitis B. Screening advised.can run past the real sentence boundary and return the wrong first sentence. Restrict this branch to genus-style cases by also requiring the next word to start lowercase, e.g.H. pylori.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/medications.ts` around lines 175 - 191, Update firstClinicalSentence’s single-letter abbreviation guard to require that the text following the period begins with a lowercase word, while preserving the existing genus-abbreviation handling for cases such as “H. pylori.” This must allow sentence boundaries after uppercase tokens like “Vitamin D.” and “Hepatitis B.” to be returned correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/lib/medications.ts`:
- Around line 366-370: The Schedule backfill in the `backfills` array bypasses
the shared truncation applied to the other medication metrics. Update its value
to pass `record.schedule` through `shortValue(...)`, while preserving the
existing conditional inclusion and label/tone.
- Around line 175-191: Update firstClinicalSentence’s single-letter abbreviation
guard to require that the text following the period begins with a lowercase
word, while preserving the existing genus-abbreviation handling for cases such
as “H. pylori.” This must allow sentence boundaries after uppercase tokens like
“Vitamin D.” and “Hepatitis B.” to be returned correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a1ea6ca-75f8-4222-8a62-7c141ffc7742
📒 Files selected for processing (4)
src/components/clinical-dashboard/medication-prescribing-workspace.tsxsrc/components/clinical-dashboard/medication-record-page.tsxsrc/lib/medications.tstests/medications.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/medications.test.ts
- src/components/clinical-dashboard/medication-prescribing-workspace.tsx
- src/components/clinical-dashboard/medication-record-page.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1ec8b35be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two review fixes on the hero metrics: - P1: `shortValue` truncation could drop clinically relevant dose variants from the Max Dose tile (e.g. buprenorphine-sl-depot "Buvidal 160 mg/month; Sublocade 300 mg/month" → "Buvidal 160 mg/month…", nicotine-gum's 4 mg variant). With the Key-stats sidebar removed, the full ceiling lived nowhere else. Never truncate the ceiling — only the two long values in the snapshot are Max Dose, and the short non-ceiling stats keep shortValue as a safety net. - Minor: the Schedule backfill now also passes through shortValue, matching the Usual dose / Category backfills. Adds snapshot + synthetic tests locking the ceiling-stays-complete behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBemWnYhqdGXBzwJUTQAqe
Summary
medicationDetailTiles) withmedicationHeroMetrics, driven by each record's already-curated ~4-itemstatsarray (Max Dose · half-life · drug-specific risk/caution flags; median value length ~8 chars). Max Dose leads and renders as the primary/clinical tile — itsflag:"hi"marks ceiling importance, not danger — so red stays reserved for genuine safety signals (contraindications, toxicity, teratogenicity); tone otherwise comes frommedicationStatTone. A clamped one-line indication subtitle under the header preserves the "what it treats" orientation the verbose tile used to carry.shortValue()(first clinical clause viafirstClinicalSentence, strip markdown, word-boundary cap with ellipsis), generalising theformulationShortLabelcap idiom into one source of truth for constrained surfaces.medicationIndicationto a single clause and clamped the verbose search cells — indication (line-clamp-1), prescribing action (line-clamp-2), dose (shortValue+line-clamp-2) — so rows stay uniform and compact.line-clamp-2+group-open:line-clamp-noneinside<details>). Nothing is dropped, just tucked away. Retired the now-duplicate sidebar "Key stats" card (stats now lead the page).Verification
npm run verify:pr-localnpm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changedRan individually (all pass):
npm run lint,npm run typecheck(changed files clean; pre-existing errors only in test files needing uninstalled optional deps@testing-library/react/@axe-core/playwright),vitestfull non-DOM suite (2311 passed) incl. newtests/medications.test.ts(28 passed),prettier --check,check:type-scale,check:icon-scale,brand:check.verify:uias a full gate needs Playwright specs whose deps aren't installed in this environment; instead verified with Chromium screenshots across five drug classes (acamprosate, cefazolin, sertraline, methadone, metformin) in light / dark / mobile — zero horizontal overflow, confirming the four boxes are crisp and tone-correct and the search rows no longer spill.Clinical Governance Preflight
Not applicable — this is a presentation-only change (trimming/relocating existing content and adjusting tone/layout). It does not touch ingestion, answer generation, search/ranking logic, source rendering data, document access, privacy, production env, or clinical content. No clinical claims, source metadata, or Supabase targets were altered; all relocated text remains visible in the detail sections / quick reference.
Notes
medicationDetailTilesand the sidebar "Key stats" card are retired (single consumers), replaced bymedicationHeroMetrics.Generated by Claude Code
Summary by CodeRabbit