Day 5:deterioration alert banner, HbA1c chart reference lines, priori…#5
Conversation
…ty table alignment
|
Warning Review limit reached
More reviews will be available in 49 minutes. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR implements a patient deterioration alert by adding sparkline risk score aggregation in the backend, tracking worsening patient trends in the Dashboard frontend, and rendering a dismissible alert when deterioration is detected. Supporting styling adjustments are made to the PriorityTable component. ChangesPatient Deterioration Alert Feature
Sequence Diagram(s)sequenceDiagram
participant getAllPatients as getAllPatients Query
participant Database
participant ResultMapping as Result Mapping
participant Dashboard
participant AlertBanner as Alert Banner UI
getAllPatients->>Database: SELECT with JSON_ARRAYAGG(risk_score)
Database-->>ResultMapping: row.sparkline JSON string
ResultMapping-->>Dashboard: parsed sparkline array
Dashboard->>Dashboard: compute counts.worsening
Dashboard->>AlertBanner: render if worsening > 0 && !dismissed
AlertBanner->>Dashboard: dismiss button clicked
Dashboard-->>AlertBanner: hide until next refresh
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/src/pages/Dashboard.jsx (1)
23-26: ⚡ Quick winAlert re-appears on every data refresh, even if user dismissed it.
The
useEffectdepends on[patients], so the alert will re-appear whenever the patients array reference changes—even if the worsening count hasn't increased or the same patients are still worsening. This creates poor UX: a user dismisses the alert, then the list auto-refreshes (or they add a non-worsening patient), and the alert pops back up.Consider tracking dismissed state across refreshes unless the worsening count increases or specific worsening patient IDs change.
♻️ Proposed improvement to reset only when worsening count increases
+ const prevWorseningRef = useRef(0); + - // Reset dismissal whenever the patients list refreshes so the banner reappears if still relevant useEffect(() => { - setAlertDismissed(false); - }, [patients]); + // Only reset if worsening count increased + const currentWorsening = patients.filter(p => p.trend === 'worsening').length; + if (currentWorsening > prevWorseningRef.current) { + setAlertDismissed(false); + } + prevWorseningRef.current = currentWorsening; + }, [patients]);Alternatively, track a Set of worsening patient IDs to detect when new patients deteriorate.
🤖 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 `@frontend/src/pages/Dashboard.jsx` around lines 23 - 26, The effect that resets alert dismissal (useEffect that calls setAlertDismissed(false)) runs on any change to the patients array reference, causing the alert to reappear on refresh; change the dependency and logic so dismissal is only reset when worsening state actually increases or new worsening patients appear—compute a derived value like worseningCount or a Set of worsening patient IDs (from patients) and depend on that (e.g., worseningCount or worseningIds) in the useEffect, or compare previous worseningIds to current and only call setAlertDismissed(false) when the count increases or new IDs are present; update the effect that currently depends on patients to use that derived worsening indicator instead.
🤖 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 `@backend/controllers/patientController.js`:
- Around line 198-208: The sparkline SQL subquery currently selects the 5 oldest
visits (ORDER BY visit_date ASC LIMIT 5) instead of the 5 most recent; modify
the subquery used for the sparkline aggregation so it first selects the most
recent 5 rows (ORDER BY visit_date DESC LIMIT 5) for patient_id = p.patient_id
and then, if you need the array ordered oldest→newest, wrap that limited set in
a nested query that re-orders by visit_date ASC before JSON_ARRAYAGG; key
symbols: sparkline, visits, risk_score, visit_date, patient_id / p.patient_id.
In `@frontend/src/pages/Dashboard.jsx`:
- Around line 69-79: The counts computation in the useMemo (const counts)
assumes patient.trend always exists; update the patients.forEach block to
defensively handle missing/null trend by normalizing it before comparison (e.g.,
derive a local const trend = (patient.trend || '').toLowerCase() or use optional
chaining) and then test trend === 'worsening'; ensure you reference the same
pattern used for risk_category to avoid silent misses in worsening counts.
---
Nitpick comments:
In `@frontend/src/pages/Dashboard.jsx`:
- Around line 23-26: The effect that resets alert dismissal (useEffect that
calls setAlertDismissed(false)) runs on any change to the patients array
reference, causing the alert to reappear on refresh; change the dependency and
logic so dismissal is only reset when worsening state actually increases or new
worsening patients appear—compute a derived value like worseningCount or a Set
of worsening patient IDs (from patients) and depend on that (e.g.,
worseningCount or worseningIds) in the useEffect, or compare previous
worseningIds to current and only call setAlertDismissed(false) when the count
increases or new IDs are present; update the effect that currently depends on
patients to use that derived worsening indicator instead.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff694d16-7136-4e23-8a69-dc4f2f72e057
⛔ Files ignored due to path filters (1)
backend/logs/error.logis excluded by!**/*.log
📒 Files selected for processing (3)
backend/controllers/patientController.jsfrontend/src/components/PriorityTable.jsxfrontend/src/pages/Dashboard.jsx
Changes
backend/controllers/patientController.js
subquery for MySQL 8 compatibility
visits per patient as a JSON array (oldest→newest) for future use
frontend/src/pages/Dashboard.jsx
have moved to a higher risk category since their previous visit
API call
frontend/src/components/PriorityTable.jsx
TableCell base padding
Summary by CodeRabbit