Day 4 — Patient Detail Page, Visit History & Appointments#4
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 40 minutes and 16 seconds. 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 (6)
📝 WalkthroughWalkthroughThis PR adds appointment booking and visit recording features to the Diacify patient management system. The backend introduces appointment and visit creation APIs with ML integration for risk prediction, the frontend provides modals for both interactions, and the patient detail page is refactored to display visit history and appointment data using a visit-based architecture. Documentation and infrastructure are updated throughout. ChangesAppointment and Visit Management
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
machine-learning/app.py (1)
232-237:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMedium-band scoring can leak into the high band (70).
40 + (prob_medium * 30)reaches70at full confidence, but70is classified ashigh. This contradicts the documented 40–69 medium range and can mislabel top medium predictions.Suggested fix
if predicted_class == 2: return 70 + (prob_high * 30) elif predicted_class == 1: - return 40 + (prob_medium * 30) + return 40 + (prob_medium * 29) else: return prob_low * 39🤖 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 `@machine-learning/app.py` around lines 232 - 237, The medium-band calculation can return 70 at full confidence, leaking into the high band; update the medium branch (where predicted_class == 1 using prob_medium) so its maximum is 69 — e.g., replace the multiplier 30 with 29 (use 40 + prob_medium * 29) or explicitly clamp the result to a maximum of 69; adjust only the medium branch (referencing predicted_class, prob_medium) and keep the high and low branches unchanged.frontend/src/pages/PatientDetailPage.jsx (1)
82-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid
new Date(iso)forYYYY-MM-DDdate-only fields—parse manually informatDate.
formatDatecurrently doesconst d = new Date(iso), butvisit_date/scheduled_dateare stored as SQLDATEand sent from<input type="date">asYYYY-MM-DD.new Date('YYYY-MM-DD')is interpreted as UTC midnight, so in US (negative offset) timezones it can display one calendar day early. Split the string (e.g.,const [yyyy, mm, dd] = iso.split('-')) and format from those parts instead.🤖 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/PatientDetailPage.jsx` around lines 82 - 90, The formatDate function currently uses new Date(iso) which can shift YYYY-MM-DD values across timezones; update formatDate to detect date-only strings (e.g., match /^\d{4}-\d{2}-\d{2}$/), split into [yyyy, mm, dd], and construct the formatted string using those parts (fall back to Date parsing only for non-date-only inputs). Modify the formatDate function to use the manual parse for date-only inputs (references: function name formatDate and usages like visit_date/scheduled_date) and keep existing validation behavior (return '—' for falsy or invalid inputs).
🧹 Nitpick comments (1)
backend/controllers/patientController.js (1)
416-451: ⚖️ Poor tradeoff
createVisitskips the zod schema validation used by create/update.
createPatientandupdatePatientrunpatientSchema.safeParseto enforce ranges/types, butcreateVisitonly checks presence. Out-of-range or non-numeric clinical values flow into the DB and the ML payload unchecked. Consider reusing a (possibly visit-scoped) schema for parity.🤖 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 `@backend/controllers/patientController.js` around lines 416 - 451, createVisit currently only checks presence and skips the zod validation used by createPatient/updatePatient; add a visit-scoped zod schema (or reuse an appropriate existing schema) and call visitSchema.safeParse(req.body) at the start of createVisit (the function named createVisit) to validate types/ranges for visit_date, age, bp_systolic, bp_diastolic, hba1c, bmi, rbs, cholesterol, triglycerides, hdl, ldl, vldl; if safeParse fails return res.status(400) with the validation errors, and use the parsed/parsedData values (not raw req.body) when inserting via db.execute so only validated, correctly-typed values are stored and sent to the ML payload.
🤖 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/appointmentController.js`:
- Around line 28-38: The appointmentDate validation currently uses new
Date(scheduled_date) but doesn't reject unparseable strings; check
appointmentDate.getTime() (or Number.isNaN) to detect an invalid date and return
a 400 before the future-date comparison. In the appointment controller where
appointmentDate is created, add an explicit guard: if
Number.isNaN(appointmentDate.getTime()) respond with res.status(400).json({
success: false, message: 'scheduled_date must be a valid date' }) then proceed
to the existing future-date check for appointmentDate <= today.
In `@backend/controllers/patientController.js`:
- Around line 421-451: The INSERT fails when optional binds are undefined; in
patientController.js before calling db.execute (the visitResult INSERT), coerce
each optional input (rbs, cholesterol, triglycerides, hdl, ldl, vldl) to null
(e.g. rbs = rbs ?? null) so no undefined values are passed to db.execute; then
use these sanitized variables in the bind array for the INSERT, leaving required
fields unchanged and ensuring db.execute never receives undefined bind
parameters.
In `@backend/database/migrations/008_run_migrations.sh`:
- Line 14: The mysql invocation in the migration loop doesn't specify the target
database, so migrations may run against no or the wrong schema; update the
command that currently uses mysql -u "$DB_USER" ${DB_PASS:+-p"$DB_PASS"} -h
"$DB_HOST" < ${file}_*.sql to explicitly pass the intended database (the
diacify_db variable) using the -D "$diacify_db" option (or equivalent DB_NAME
var) and ensure diacify_db is defined/exported earlier in the script so the
migrations always run against the expected database.
In `@frontend/src/components/AddVisitModal.jsx`:
- Around line 56-65: The prefill logic in getPrefilledAge only adds one year
when monthsDiff >= 12; update it to add the full elapsed years by computing
yearsElapsed = Math.floor(monthsDiff / 12) and returning String(latestVisit.age
+ yearsElapsed) (keep the initial guard if !latestVisit). Refer to
getPrefilledAge, latestVisit, monthsDiff when making this change.
- Around line 87-93: The modal resets submitting to false on open which allows
users to close/reopen and retrigger duplicate POSTs; remove or stop calling
setSubmitting(false) inside the useEffect that runs on isOpen/latestVisit and
instead only setSubmitting(false) when the POST completes or fails inside the
submit handler (e.g., in handleSubmit/async createVisit). Also make the close
flow defensive by disabling or short-circuiting the header close button and any
backdrop/escape-to-close logic when submitting is true (check submitting in
onClose/closeModal and return early or keep the close button disabled), and
ensure the primary action button remains disabled while submitting so duplicate
non-idempotent /visits inserts cannot be produced.
- Around line 104-117: The payload creation in AddVisitModal.jsx currently
coerces empty numeric inputs to 0 via Number(form.*) causing
missing-required-fields to be accepted by backend.createVisit; update the
payload building in the AddVisitModal component to only convert values to
numbers when the input is non-empty (e.g., leave '' or set null when form.age,
form.bp_systolic, form.bp_diastolic, form.hba1c, form.bmi or optional lab fields
are blank) and add a client-side validation step that prevents submit when any
required numeric field is blank (check form.age, form.bp_systolic,
form.bp_diastolic, form.hba1c, form.bmi) so you never send coerced 0s to the
backend or ML service.
In `@frontend/src/components/BookAppointmentModal.jsx`:
- Around line 35-43: The modal resets `submitting` every time it opens which
allows a user to close/reopen and resubmit while a POST is still in flight;
instead, stop resetting submitting in the useEffect inside BookAppointmentModal
(do not call setSubmitting(false) on open), ensure the submit handler
(handleSubmit / onSubmit) sets submitting = true immediately and early-returns
if submitting is already true, and disable/hide any dismiss controls
(CloseButton, CancelButton) and disable backdrop/escape closing while submitting
so onClose cannot be invoked during an in-flight non-idempotent POST; this
preserves the submitting state across close/open and prevents duplicate
submissions.
In `@frontend/src/pages/PatientDetailPage.jsx`:
- Around line 71-74: The code is coercing missing or pending ML outputs into
'low' risk and a 0 score; update the logic around computing category and risk
points (e.g., the useMemo that derives const category from
patient?.latest_visit?.risk_category and any places that read risk_score) to
preserve a distinct 'pending'/'unavailable' state instead of mapping
null/pending to 'low' or 0. Change the checks to explicitly detect
null/latest_visit===null or risk_category === 'pending' (and risk_score ===
null) and return a special value (e.g., 'pending' or null) so the summary card,
chart, and visit table can skip or render an unavailable state; apply the same
fix to the other occurrences noted (the blocks around lines 149-153, 217-245,
377-397 that read latest_visit, risk_category, or risk_score).
- Around line 94-101: Remove the temporary debug console.log statements that
print sensitive patient data from the useEffect block; specifically delete the
logs referencing patient, patient?.latest_visit?.top_factors, and the confidence
fields (confidence_low, confidence_medium, confidence_high) in the useEffect
tied to patient, or alternatively wrap them behind a strict development-only
guard (e.g., process.env.NODE_ENV === 'development') so they never run in
production builds.
In `@README.md`:
- Around line 313-325: The Backend env table is missing BACKEND_ORIGIN which
causes CORS mismatches during local setup; add a new row for `BACKEND_ORIGIN` in
the Backend environment variables section alongside `ML_SERVICE_URL` and the DB
vars, specifying its purpose (backend origin used for CORS configuration) and a
sensible default such as `http://localhost:3000` (note frontend runs at
`http://localhost:5173`) so developers can set the correct origin to avoid CORS
failures.
---
Outside diff comments:
In `@frontend/src/pages/PatientDetailPage.jsx`:
- Around line 82-90: The formatDate function currently uses new Date(iso) which
can shift YYYY-MM-DD values across timezones; update formatDate to detect
date-only strings (e.g., match /^\d{4}-\d{2}-\d{2}$/), split into [yyyy, mm,
dd], and construct the formatted string using those parts (fall back to Date
parsing only for non-date-only inputs). Modify the formatDate function to use
the manual parse for date-only inputs (references: function name formatDate and
usages like visit_date/scheduled_date) and keep existing validation behavior
(return '—' for falsy or invalid inputs).
In `@machine-learning/app.py`:
- Around line 232-237: The medium-band calculation can return 70 at full
confidence, leaking into the high band; update the medium branch (where
predicted_class == 1 using prob_medium) so its maximum is 69 — e.g., replace the
multiplier 30 with 29 (use 40 + prob_medium * 29) or explicitly clamp the result
to a maximum of 69; adjust only the medium branch (referencing predicted_class,
prob_medium) and keep the high and low branches unchanged.
---
Nitpick comments:
In `@backend/controllers/patientController.js`:
- Around line 416-451: createVisit currently only checks presence and skips the
zod validation used by createPatient/updatePatient; add a visit-scoped zod
schema (or reuse an appropriate existing schema) and call
visitSchema.safeParse(req.body) at the start of createVisit (the function named
createVisit) to validate types/ranges for visit_date, age, bp_systolic,
bp_diastolic, hba1c, bmi, rbs, cholesterol, triglycerides, hdl, ldl, vldl; if
safeParse fails return res.status(400) with the validation errors, and use the
parsed/parsedData values (not raw req.body) when inserting via db.execute so
only validated, correctly-typed values are stored and sent to the ML payload.
🪄 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: 0d032852-6b93-4188-81c7-33fd9f809326
⛔ Files ignored due to path filters (3)
assets/analytics.pngis excluded by!**/*.pngassets/dashboard.pngis excluded by!**/*.pngbackend/logs/error.logis excluded by!**/*.log
📒 Files selected for processing (13)
README.mdbackend/controllers/appointmentController.jsbackend/controllers/patientController.jsbackend/database/migrations/008_run_migrations.shbackend/database/migrations/README.mdbackend/routes/appointmentRoutes.jsbackend/routes/patients.jsbackend/server.jsfrontend/src/components/AddVisitModal.jsxfrontend/src/components/BookAppointmentModal.jsxfrontend/src/components/PriorityTable.jsxfrontend/src/pages/PatientDetailPage.jsxmachine-learning/app.py
Comment on lines
94
to
101
| // Debug: log the patient object to see the full structure | ||
| useEffect(() => { | ||
| if (patient) { | ||
| console.log('Patient object:', patient); | ||
| console.log('Top factors:', patient?.top_factors); | ||
| console.log('Confidence values:', { low: patient?.confidence_low, medium: patient?.confidence_medium, high: patient?.confidence_high }); | ||
| console.log('Top factors:', patient?.latest_visit?.top_factors); | ||
| console.log('Confidence values:', { low: patient?.latest_visit?.confidence_low, medium: patient?.latest_visit?.confidence_medium, high: patient?.latest_visit?.confidence_high }); | ||
| } | ||
| }, [patient]); |
There was a problem hiding this comment.
Remove patient debug logging before merge.
Lines 97-99 dump the full patient payload plus risk factors/confidence to the browser console. That's sensitive health data and shouldn't be emitted in normal builds.
🤖 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/PatientDetailPage.jsx` around lines 94 - 101, Remove the
temporary debug console.log statements that print sensitive patient data from
the useEffect block; specifically delete the logs referencing patient,
patient?.latest_visit?.top_factors, and the confidence fields (confidence_low,
confidence_medium, confidence_high) in the useEffect tied to patient, or
alternatively wrap them behind a strict development-only guard (e.g.,
process.env.NODE_ENV === 'development') so they never run in production builds.
Comment on lines
+313
to
+325
| ### Backend | ||
| | Variable | Description | | ||
| |----------|-------------| | ||
| | `PORT` | Server port (default 3300) | | ||
| | `DB_HOST` | MySQL host | | ||
| | `DB_USER` | MySQL user | | ||
| | `DB_PASSWORD` | MySQL password | | ||
| | `DB_NAME` | Database name (diacify_db) | | ||
| | `DB_PORT` | MySQL port (default 3306) | | ||
| | `ML_SERVICE_URL` | URL of the ML FastAPI service | | ||
| | `CLERK_SECRET_KEY` | Clerk backend secret key | | ||
| | `ML_INTERNAL_SECRET` | Shared secret for ML service authentication | | ||
|
|
There was a problem hiding this comment.
Add BACKEND_ORIGIN to backend env docs to prevent CORS setup failures.
The backend env reference omits BACKEND_ORIGIN, but the frontend runs on http://localhost:5173 and the ML service default origin is still http://localhost:3000. Fresh local setups can fail CORS unless this is explicitly configured.
Suggested doc patch
### Backend
| Variable | Description |
|----------|-------------|
| `PORT` | Server port (default 3300) |
| `DB_HOST` | MySQL host |
| `DB_USER` | MySQL user |
| `DB_PASSWORD` | MySQL password |
| `DB_NAME` | Database name (diacify_db) |
| `DB_PORT` | MySQL port (default 3306) |
| `ML_SERVICE_URL` | URL of the ML FastAPI service |
+| `BACKEND_ORIGIN` | Allowed frontend origin for ML CORS (e.g. `http://localhost:5173`) |
| `CLERK_SECRET_KEY` | Clerk backend secret key |
| `ML_INTERNAL_SECRET` | Shared secret for ML service authentication |🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 314-314: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 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 `@README.md` around lines 313 - 325, The Backend env table is missing
BACKEND_ORIGIN which causes CORS mismatches during local setup; add a new row
for `BACKEND_ORIGIN` in the Backend environment variables section alongside
`ML_SERVICE_URL` and the DB vars, specifying its purpose (backend origin used
for CORS configuration) and a sensible default such as `http://localhost:3000`
(note frontend runs at `http://localhost:5173`) so developers can set the
correct origin to avoid CORS failures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was built
createVisitcontroller with ML fallback to pending stateappointmentControllerandappointmentRoutesBugs fixed
prob_low * 40→prob_low * 39pprefix removed across PriorityTable and BookAppointmentModalNot yet merged to main
Analytics page broken, tests not written, CI/CD not configured — merge to main on Day 9.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation