fix: add missing endpoints, fix rollback, and resolve closure bugs#238
Open
Muneerali199 wants to merge 10 commits intoAOSSIE-Org:mainfrom
Open
fix: add missing endpoints, fix rollback, and resolve closure bugs#238Muneerali199 wants to merge 10 commits intoAOSSIE-Org:mainfrom
Muneerali199 wants to merge 10 commits intoAOSSIE-Org:mainfrom
Conversation
Fixed: - Critical: argparse.parse_args() called twice causing Flask crash - Critical: Index route returning Response object causing 500 error - Critical: Hardcoded relative DB paths breaking on wrong CWD - Critical: ChromaDB relative path crash at import - Critical: NameError in vector store except block - Critical: SymptomsScreen null entry crash - Critical: CalendarScreen broken inline styles - Critical: Schema missing mood and sleep tables Security: - UPDATE profile with no WHERE clause (data corruption) - Removed broken require_auth decorators (always 401) - Fixed logout deleting all user data - Removed mock profile data from schema - Added .env validation in model.jsx Missing Endpoints: - Created mood.py routes (log_mood, get_mood_entries, update_mood, delete_mood) - Created sleep.py routes (log_sleep, get_sleep_entries, update_sleep, delete_sleep) - Created analytics.py routes (get_analytics, get_weight_entries, get_mood_entries, get_sleep_entries) Wrong URLs: - Fixed ActionExecutor: /create_appointment -> /add_appointment - Fixed ActionExecutor: /medicine -> /set_medicine - Fixed RAGService: /get_weight -> /weight - Fixed RAGService: /get_all_medicine -> /get_medicine GitHub Issues: AOSSIE-Org#197-236
High Priority Fixes: - Added /agent/refresh endpoint for AgentContext - Added /undo_last_action endpoint for undo functionality - Created crud.py with all UPDATE/DELETE endpoints (medicine, BP, discharge, symptoms, weight) - Backend now returns IDs in create responses for rollback support URL Fixes: - Fixed ActionExecutor rollback: /appointments -> /delete_appointment - Fixed ActionExecutor rollback: POST to /appointments -> /add_appointment Logic Fixes: - Fixed useChatEngine stale closure bug using useRef - Fixed ConversationContext using process.env.BASE_URL (now imports from @env) Also updated: - appointments.py: returns id on create - medicine.py: returns id on create - weight.py: returns id on create
Contributor
📝 WalkthroughWalkthroughAdds mood/sleep tracking and analytics blueprints, a generic CRUD blueprint, agent endpoints, centralized BASE_DIR/CHROMADB_PATH with conditional Chroma init, DB path normalization, returns created record IDs, frontend endpoint/env adjustments, and schema additions for mood/sleep tables. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FlaskApp as App
participant DB
participant AgentCache as Agent
Client->>FlaskApp: POST /log_mood or /log_sleep (create)
FlaskApp->>DB: INSERT record
DB-->>FlaskApp: lastrowid
FlaskApp->>Agent: update cache (data_type, create, id)
Agent-->>FlaskApp: ack
FlaskApp-->>Client: 201 { status, message, id }
sequenceDiagram
participant Client
participant FlaskApp as App
participant VectorStore
participant ChromaFS as Chroma/FS
Client->>FlaskApp: POST /agent/refresh or guideline update
FlaskApp->>VectorStore: ensure CHROMADB_PATH, init client if available
VectorStore->>ChromaFS: read/write collections, compare/store guideline.hash
ChromaFS-->>VectorStore: success/error
VectorStore-->>FlaskApp: result
FlaskApp-->>Client: 200/500 status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
- Fixed RAGService: /get_symptoms -> /symptoms - Fixed RAGService: /get_blood_pressure -> /blood_pressure - Fixed AnalyticsService: correct endpoints for weight, symptoms, BP - Fixed ConversationContext: removed remaining process.env.BASE_URL - Fixed validate_bp_data: use is not None instead of truthiness - Fixed profile.py: MissingFieldError now lists only missing fields
- Fixed useChatEngine: import from '../model/model.jsx' explicitly - Fixed HomeScreen: dueDate check before calculating week numbers - Fixed HomeScreen: added onPress to floating AI button to navigate to Chat
- ProfileScreen: Edit Information now navigates to BasicDetails - SettingsScreen: Edit Profile now navigates to BasicDetails - SettingsScreen: Notification button shows alert (was non-functional) - SOSAlertScreen: Fixed tap logic - now triggers on 4th tap correctly
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 (4)
Frontend/src/services/ActionExecutor.js (3)
185-210:⚠️ Potential issue | 🔴 CriticalPost
appointment_locationto/add_appointment.The new backend endpoint requires
appointment_location, but this payload still sendslocation. Every create call will fail validation, so the success path and rollback id will never be reached.Suggested fix
const appointmentData = { title: payload.title, description: payload.description || '', - location: payload.location || '', + appointment_location: payload.location || '', appointment_date: this.formatDate(payload.startISO), appointment_time: this.formatTime(payload.startISO), appointment_status: 'scheduled', content: payload.description || '', week_number: userContext.current_week || 12 }; @@ return { success: true, - message: `✅ Appointment "${payload.title}" created successfully!\n\n📅 Date: ${appointmentData.appointment_date}\n⏰ Time: ${appointmentData.appointment_time}\n📍 Location: ${appointmentData.location}`, + message: `✅ Appointment "${payload.title}" created successfully!\n\n📅 Date: ${appointmentData.appointment_date}\n⏰ Time: ${appointmentData.appointment_time}\n📍 Location: ${appointmentData.appointment_location}`, data: result, actionType: 'create_appointment', appointmentId: result.id || result.appointment_id // Include ID for rollback };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frontend/src/services/ActionExecutor.js` around lines 185 - 210, The payload sent to the backend uses the wrong field name: update the appointmentData object (used in the POST to add_appointment within ActionExecutor.js) to include appointment_location instead of location (and keep appointment_date/appointment_time generation via this.formatDate and this.formatTime unchanged), then ensure the success return still reads appointmentId from result.id || result.appointment_id so rollback can use the returned id; verify the fetch to `${BASE_URL}/add_appointment` sends the corrected appointmentData.
236-256:⚠️ Potential issue | 🟠 MajorKeep the backend field names and rollback snapshot in the update result.
/update_appointment/<id>readscontent, notdescription, so description edits are dropped today. The success result also omitsappointmentIdandpreviousData, which meansrollbackUpdateAppointment()can never restore the prior values.Suggested fix
+ const currentResponse = await fetch(`${BASE_URL}/get_appointment/${payload.id}`); + if (!currentResponse.ok) { + throw new Error('Failed to load appointment before update'); + } + const previousData = await currentResponse.json(); + const updateData = {}; if (payload.title) updateData.title = payload.title; - if (payload.description) updateData.description = payload.description; + if (payload.description) updateData.content = payload.description; if (payload.location) updateData.appointment_location = payload.location; if (payload.startISO) { updateData.appointment_date = this.formatDate(payload.startISO); updateData.appointment_time = this.formatTime(payload.startISO); } @@ return { success: true, message: `✅ Appointment updated successfully!`, - actionType: 'update_appointment' + actionType: 'update_appointment', + appointmentId: payload.id, + previousData };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frontend/src/services/ActionExecutor.js` around lines 236 - 256, The update flow drops description and omits rollback metadata: ensure payload.description is mapped to the backend field content (set updateData.content = payload.description) alongside existing mappings (title, appointment_location, appointment_date, appointment_time using this.formatDate/formatTime) and capture a snapshot of the current appointment before the PATCH (e.g., previousData = { title, content, appointment_location, appointment_date, appointment_time }) so you can restore on failure; include appointmentId (payload.id) and previousData in the returned success object so rollbackUpdateAppointment can restore prior values.
272-291:⚠️ Potential issue | 🟠 MajorCapture the deleted appointment before calling DELETE.
rollbackDeleteAppointment()only knows how to restore fromresult.deletedAppointment, but this success path never includes it. Undoing a delete will still always fail.Suggested fix
+ const existingResponse = await fetch(`${BASE_URL}/get_appointment/${payload.id}`); + if (!existingResponse.ok) { + throw new Error('Failed to load appointment before delete'); + } + const deletedAppointment = await existingResponse.json(); + const response = await fetch(`${BASE_URL}/delete_appointment/${payload.id}`, { method: 'DELETE' }); if (response.ok) { return { success: true, message: `✅ Appointment deleted successfully!`, - actionType: 'delete_appointment' + actionType: 'delete_appointment', + deletedAppointment }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frontend/src/services/ActionExecutor.js` around lines 272 - 291, The deleteAppointment success path doesn't include the deleted appointment object that rollbackDeleteAppointment expects in result.deletedAppointment; before calling DELETE in deleteAppointment (or by parsing the DELETE response if the API returns the removed resource), retrieve and capture the appointment (e.g., GET `${BASE_URL}/appointments/${payload.id}`) into a deletedAppointment variable and include it in the returned success object (e.g., return { success: true, message: ..., actionType: 'delete_appointment', deletedAppointment }) so rollbackDeleteAppointment can restore it.Backend/routes/medicine.py (1)
65-148:⚠️ Potential issue | 🔴 CriticalThese medicine read/update/delete handlers are now public.
require_authis still defined in this module, but none of the handlers fromGET /get_medicinethroughDELETE /medicine/<id>apply it, andBackend/app.pydoes not install any app-wide auth middleware. That exposes medication history and allows unauthenticated edits/deletes.Suggested fix
`@medicine_bp.route`('/get_medicine', methods=['GET']) +@require_auth `@handle_db_errors` def get_all_medicine(): db = open_db() rows = db.execute('SELECT * FROM weekly_medicine').fetchall() return jsonify([dict(row) for row in rows]), 200 # Read by week `@medicine_bp.route`('/medicine/week/<int:week>', methods=['GET']) +@require_auth `@handle_db_errors` def get_week_medicine(week): db = open_db() rows = db.execute('SELECT * FROM weekly_medicine WHERE week_number = ?', (week,)).fetchall() return jsonify([dict(row) for row in rows]), 200 # Read by ID `@medicine_bp.route`('/medicine/<int:id>', methods=['GET']) +@require_auth `@handle_db_errors` def get_medicine(id): db = open_db() entry = db.execute('SELECT * FROM weekly_medicine WHERE id = ?', (id,)).fetchone() if not entry: raise NotFoundError(resource="Medicine entry", resource_id=id) return jsonify(dict(entry)), 200 # Update by ID `@medicine_bp.route`('/medicine/<int:id>', methods=['PATCH']) +@require_auth `@handle_db_errors` def update_medicine(id): db = open_db() data = request.get_json() entry = db.execute('SELECT * FROM weekly_medicine WHERE id = ?', (id,)).fetchone() @@ # Delete by ID `@medicine_bp.route`('/medicine/<int:id>', methods=['DELETE']) +@require_auth `@handle_db_errors` def delete_medicine(id): db = open_db() entry = db.execute('SELECT * FROM weekly_medicine WHERE id = ?', (id,)).fetchone()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Backend/routes/medicine.py` around lines 65 - 148, These endpoints are missing authentication: add the require_auth decorator to the five handlers (get_all_medicine, get_week_medicine, get_medicine, update_medicine, delete_medicine) so requests must be authenticated before DB access; place `@require_auth` immediately after the `@medicine_bp.route` decorator (and before `@handle_db_errors`) for each function; no new imports are needed since require_auth is already defined in the module.
🧹 Nitpick comments (1)
Frontend/src/model/model.jsx (1)
38-42: Remove unnecessaryMODEL_NAMEvalidation.
MODEL_NAMEis validated but never used withinfetchAvailableGGUFs. OnlyHF_TO_GGUFis used (line 43). Consider removing the dead validation to avoid confusion.🔧 Suggested fix
if (!HF_TO_GGUF) throw new Error("HF_TO_GGUF not configured in environment variables"); - if (!MODEL_NAME) - throw new Error("MODEL_NAME not configured in environment variables");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frontend/src/model/model.jsx` around lines 38 - 42, The check that throws when MODEL_NAME is missing is dead code for the fetchAvailableGGUFs flow; remove the MODEL_NAME validation (the throw referencing "MODEL_NAME not configured in environment variables") so only HF_TO_GGUF is validated before fetchAvailableGGUFs runs. Locate the validation block around HF_TO_GGUF and MODEL_NAME in the module (the if statements that throw Errors) and delete the MODEL_NAME check, leaving the HF_TO_GGUF check and any logic inside fetchAvailableGGUFs unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Backend/app.py`:
- Around line 265-287: The undo endpoint currently expects action_type values
like "add_weight" and a record_id, but callers send high-level types (e.g.,
"weight", "sleep", "appointment") and often omit record_id; update the handler
that reads action_type and record_id to accept those high-level types and map
them to the existing DB table actions (e.g., map "weight" -> DELETE FROM
weekly_weight, "sleep" -> DELETE FROM sleep_logs, "appointment" -> DELETE FROM
appointments, etc.), and when record_id is missing implement logic to delete the
most recent record for the current user from the mapped table (instead of
failing); adjust the code paths that currently branch on action_type (the
if/elif block handling action_type) to use this mapping and optional-record_id
fallback so frontend undo requests succeed.
- Around line 148-151: The handler reads user_id from the request into user_id
but then calls agent.update_cache(data_type="all", operation="refresh") without
using it; update the call to pass the user_id (e.g.,
agent.update_cache(data_type="all", operation="refresh", user_id=user_id) or the
expected parameter name used by update_cache) so the refresh applies to the
correct user context, referencing the user_id variable and the
agent.update_cache(...) call in the handler.
In `@Backend/routes/analytics.py`:
- Around line 55-65: Duplicate route handlers in analytics.py (get_mood_entries
and get_sleep_entries) conflict with existing routes in mood.py and sleep.py;
remove or rename them to avoid routing collisions. Either delete the functions
get_mood_entries and get_sleep_entries from Backend/routes/analytics.py so the
canonical handlers in Backend/routes/mood.py and Backend/routes/sleep.py are
used, or change the analytics endpoints to unique paths (e.g.,
/analytics/get_mood_entries, /analytics/get_sleep_entries) and update any
callers to the new paths; ensure you update or remove any imports/registration
related to these functions to prevent dead code.
In `@Backend/routes/crud.py`:
- Around line 13-42: The new PUT handler update_medicine bypasses the existing
partial-update validators and writes raw request values to SQLite, risking
persisting invalid data and then refreshing the agent cache; fix it by reusing
the existing validation/update logic from the canonical route (e.g., import and
call the validator or update function used in Backend/routes/medicine.py) or
invoke the same validation helpers before executing the DB UPDATE in
update_medicine; ensure you call the same validation functions for fields like
week_number/name/dose/time/note and only apply db.execute with the
validated/normalized values, and keep the agent cache refresh
(get_agent(...).update_cache(...)) after a successful validated update.
In `@Backend/routes/mood.py`:
- Around line 68-71: Handlers currently only accept payload key 'id' (mood_id =
data.get('id')) but the frontend sends date/time-based identifiers; update the
handlers (the mood update and delete handlers that reference mood_id and
data.get('id')) to accept the frontend's identifier key(s) (e.g., 'timestamp' or
'datetime') and/or support both formats. Change the input parsing to check
data.get('id') || data.get('timestamp') (or parse a datetime string), normalize
it to the DB lookup key (convert timestamp to the stored id or query by
datetime), and update the error message to reflect the accepted keys; apply the
same fix to the other handler block around the 97-105 area so both update and
delete use the same identifier contract.
In `@Backend/routes/profile.py`:
- Around line 94-96: The logout route currently only returns a JSON success
message but does not clear the server-side session; update the logout() handler
(profile_bp.route '/logout') to remove authentication state on the server by
clearing relevant session keys (e.g., session.clear() or session.pop('user_id',
None)) and, if using Flask-Login, call logout_user(); then return the same
response and explicitly remove the session cookie on the response so subsequent
requests (and checks like require_auth) no longer treat the user as logged in.
In `@Backend/routes/sleep.py`:
- Around line 22-35: The API uses the field name hours in the sleep create/read
handlers but the frontend expects duration; update the handlers in
Backend/routes/sleep.py to accept and produce duration while preserving
backward-compatibility with hours: read duration from request data if present
(fall back to hours), validate and store the canonical value (e.g., duration) in
the DB insert used by the code block around cursor = db.execute(...), and update
response payloads (the read/list handlers around lines 47-58) to return duration
(and optionally include hours for compatibility) so existing frontend callers
continue to work.
- Around line 69-71: The update/delete handlers currently abort if
data.get('id') is missing (sleep_id) but chat callers send date/time
identifiers; change the handlers (the code that sets sleep_id and the
update_sleep/delete_sleep functions) to accept either an explicit 'id' or a
date/time identifier pair (e.g., 'date' and 'start_time' or a single
'timestamp') — if 'id' is absent, normalize the incoming date/time fields into a
lookup key and query the DB for the matching sleep record to obtain its id
before proceeding; only return the 400 error when neither an 'id' nor valid
date/time fields are provided, and apply the same fix to the other occurrence of
this logic (the block covering lines ~104-113).
In `@Backend/utils.py`:
- Around line 4-11: The validator in validate_bp_data is skipping checks when
values are None, allowing create payloads with null required fields; add a new
boolean parameter require_non_null (default False) to validate_bp_data and, when
True (used by the CREATE endpoint in Backend/routes/blood_pressure.py), treat
keys present with value None as validation failures for required fields
(week_number, systolic, diastolic, time), returning appropriate errors instead
of skipping them; keep the default False so PATCH/partial updates retain the
current behavior and update the CREATE call to pass require_non_null=True.
In `@Frontend/src/hooks/useChatEngine.js`:
- Around line 104-105: The conversation ref is updated in a separate effect,
causing race conditions when sendMessage is called rapidly; instead, update
conversationRef.current inside the setConversation functional updater so the new
array is computed and assigned atomically (e.g., in sendMessage use
setConversation(prev => { const next = [...prev, userMessage];
conversationRef.current = next; return next; }) and mirror this pattern for the
other setConversation call at the second site; also add a re-entry guard in
sendMessage (check or set isGenerating at the start/end) to prevent concurrent
sends.
---
Outside diff comments:
In `@Backend/routes/medicine.py`:
- Around line 65-148: These endpoints are missing authentication: add the
require_auth decorator to the five handlers (get_all_medicine,
get_week_medicine, get_medicine, update_medicine, delete_medicine) so requests
must be authenticated before DB access; place `@require_auth` immediately after
the `@medicine_bp.route` decorator (and before `@handle_db_errors`) for each
function; no new imports are needed since require_auth is already defined in the
module.
In `@Frontend/src/services/ActionExecutor.js`:
- Around line 185-210: The payload sent to the backend uses the wrong field
name: update the appointmentData object (used in the POST to add_appointment
within ActionExecutor.js) to include appointment_location instead of location
(and keep appointment_date/appointment_time generation via this.formatDate and
this.formatTime unchanged), then ensure the success return still reads
appointmentId from result.id || result.appointment_id so rollback can use the
returned id; verify the fetch to `${BASE_URL}/add_appointment` sends the
corrected appointmentData.
- Around line 236-256: The update flow drops description and omits rollback
metadata: ensure payload.description is mapped to the backend field content (set
updateData.content = payload.description) alongside existing mappings (title,
appointment_location, appointment_date, appointment_time using
this.formatDate/formatTime) and capture a snapshot of the current appointment
before the PATCH (e.g., previousData = { title, content, appointment_location,
appointment_date, appointment_time }) so you can restore on failure; include
appointmentId (payload.id) and previousData in the returned success object so
rollbackUpdateAppointment can restore prior values.
- Around line 272-291: The deleteAppointment success path doesn't include the
deleted appointment object that rollbackDeleteAppointment expects in
result.deletedAppointment; before calling DELETE in deleteAppointment (or by
parsing the DELETE response if the API returns the removed resource), retrieve
and capture the appointment (e.g., GET `${BASE_URL}/appointments/${payload.id}`)
into a deletedAppointment variable and include it in the returned success object
(e.g., return { success: true, message: ..., actionType: 'delete_appointment',
deletedAppointment }) so rollbackDeleteAppointment can restore it.
---
Nitpick comments:
In `@Frontend/src/model/model.jsx`:
- Around line 38-42: The check that throws when MODEL_NAME is missing is dead
code for the fetchAvailableGGUFs flow; remove the MODEL_NAME validation (the
throw referencing "MODEL_NAME not configured in environment variables") so only
HF_TO_GGUF is validated before fetchAvailableGGUFs runs. Locate the validation
block around HF_TO_GGUF and MODEL_NAME in the module (the if statements that
throw Errors) and delete the MODEL_NAME check, leaving the HF_TO_GGUF check and
any logic inside fetchAvailableGGUFs unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 920229af-ec0d-4ec7-9828-15f99267a63c
📒 Files selected for processing (21)
Backend/agent/vector_store.pyBackend/app.pyBackend/db/db.pyBackend/routes/analytics.pyBackend/routes/appointments.pyBackend/routes/crud.pyBackend/routes/medicine.pyBackend/routes/mood.pyBackend/routes/profile.pyBackend/routes/sleep.pyBackend/routes/weight.pyBackend/schema.sqlBackend/utils.pyFrontend/src/Screens/CalendarScreen.jsxFrontend/src/Screens/SymptomsScreen.jsxFrontend/src/hooks/useChatEngine.jsFrontend/src/model/model.jsxFrontend/src/services/ActionExecutor.jsFrontend/src/services/AnalyticsService.jsFrontend/src/services/ConversationContext.jsFrontend/src/services/RAGService.js
💤 Files with no reviewable changes (1)
- Frontend/src/Screens/CalendarScreen.jsx
Comment on lines
+148
to
+151
| user_id = data.get("user_id", "default") | ||
|
|
||
| agent.update_cache(data_type="all", operation="refresh") | ||
|
|
Contributor
There was a problem hiding this comment.
/agent/refresh ignores the provided user_id.
Line 148 reads user_id, but Line 150 refreshes cache without passing it, so non-default users won’t get their context refreshed.
Suggested fix
- agent.update_cache(data_type="all", operation="refresh")
+ agent.update_cache(user_id=user_id, data_type="all", operation="update")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Backend/app.py` around lines 148 - 151, The handler reads user_id from the
request into user_id but then calls agent.update_cache(data_type="all",
operation="refresh") without using it; update the call to pass the user_id
(e.g., agent.update_cache(data_type="all", operation="refresh", user_id=user_id)
or the expected parameter name used by update_cache) so the refresh applies to
the correct user context, referencing the user_id variable and the
agent.update_cache(...) call in the handler.
Comment on lines
+265
to
+287
| if not action_type or not record_id: | ||
| return jsonify({"error": "action_type and record_id are required"}), 400 | ||
|
|
||
| db = open_db() | ||
|
|
||
| if action_type == "create_appointment": | ||
| db.execute("DELETE FROM appointments WHERE id = ?", (record_id,)) | ||
| elif action_type == "add_weight": | ||
| db.execute("DELETE FROM weekly_weight WHERE id = ?", (record_id,)) | ||
| elif action_type == "log_mood": | ||
| db.execute("DELETE FROM mood_logs WHERE id = ?", (record_id,)) | ||
| elif action_type == "log_sleep": | ||
| db.execute("DELETE FROM sleep_logs WHERE id = ?", (record_id,)) | ||
| elif action_type == "add_medicine": | ||
| db.execute("DELETE FROM weekly_medicine WHERE id = ?", (record_id,)) | ||
| elif action_type == "add_symptom": | ||
| db.execute("DELETE FROM weekly_symptoms WHERE id = ?", (record_id,)) | ||
| elif action_type == "log_bp": | ||
| db.execute("DELETE FROM blood_pressure_logs WHERE id = ?", (record_id,)) | ||
| elif action_type == "log_discharge": | ||
| db.execute("DELETE FROM discharge_logs WHERE id = ?", (record_id,)) | ||
| else: | ||
| return jsonify({"error": f"Unknown action type: {action_type}"}), 400 |
Contributor
There was a problem hiding this comment.
Undo endpoint contract is incompatible with current caller payloads.
/undo_last_action requires record_id and action names like add_weight, but Frontend/src/services/RAGService.js sends only high-level types (weight, sleep, appointment, etc.). This will fail most chat undo requests.
Suggested direction
- if not action_type or not record_id:
- return jsonify({"error": "action_type and record_id are required"}), 400
+ if not action_type:
+ return jsonify({"error": "action_type is required"}), 400
+
+ action_alias = {
+ "weight": "add_weight",
+ "appointment": "create_appointment",
+ "mood": "log_mood",
+ "sleep": "log_sleep",
+ "medicine": "add_medicine",
+ "symptom": "add_symptom",
+ "blood_pressure": "log_bp",
+ "discharge": "log_discharge",
+ }
+ action_type = action_alias.get(action_type, action_type)
+
+ # If record_id is omitted, resolve last action record id from action history
+ # (or return a clear error that includes required shape expected by callers).Also applies to: 291-296
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Backend/app.py` around lines 265 - 287, The undo endpoint currently expects
action_type values like "add_weight" and a record_id, but callers send
high-level types (e.g., "weight", "sleep", "appointment") and often omit
record_id; update the handler that reads action_type and record_id to accept
those high-level types and map them to the existing DB table actions (e.g., map
"weight" -> DELETE FROM weekly_weight, "sleep" -> DELETE FROM sleep_logs,
"appointment" -> DELETE FROM appointments, etc.), and when record_id is missing
implement logic to delete the most recent record for the current user from the
mapped table (instead of failing); adjust the code paths that currently branch
on action_type (the if/elif block handling action_type) to use this mapping and
optional-record_id fallback so frontend undo requests succeed.
Comment on lines
+55
to
+65
| @analytics_bp.route('/get_mood_entries', methods=['GET']) | ||
| def get_mood_entries(): | ||
| db = open_db() | ||
| week = request.args.get('week') | ||
|
|
||
| if week: | ||
| rows = db.execute('SELECT * FROM mood_logs WHERE week_number = ? ORDER BY week_number', (week,)).fetchall() | ||
| else: | ||
| rows = db.execute('SELECT * FROM mood_logs ORDER BY week_number').fetchall() | ||
|
|
||
| return jsonify([dict(row) for row in rows]), 200 |
Contributor
There was a problem hiding this comment.
Duplicate route paths conflict with existing mood/sleep blueprints.
/get_mood_entries and /get_sleep_entries are already defined in Backend/routes/mood.py and Backend/routes/sleep.py. Keeping duplicate handlers for identical paths/methods makes routing behavior order-dependent and brittle.
Also applies to: 68-78
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Backend/routes/analytics.py` around lines 55 - 65, Duplicate route handlers
in analytics.py (get_mood_entries and get_sleep_entries) conflict with existing
routes in mood.py and sleep.py; remove or rename them to avoid routing
collisions. Either delete the functions get_mood_entries and get_sleep_entries
from Backend/routes/analytics.py so the canonical handlers in
Backend/routes/mood.py and Backend/routes/sleep.py are used, or change the
analytics endpoints to unique paths (e.g., /analytics/get_mood_entries,
/analytics/get_sleep_entries) and update any callers to the new paths; ensure
you update or remove any imports/registration related to these functions to
prevent dead code.
Comment on lines
+13
to
+42
| @crud_bp.route('/update_medicine', methods=['PUT']) | ||
| @handle_db_errors | ||
| def update_medicine(): | ||
| db = open_db() | ||
| data = request.get_json() | ||
| if not data or 'id' not in data: | ||
| return jsonify({"error": "Medicine ID is required"}), 400 | ||
|
|
||
| entry = db.execute('SELECT * FROM weekly_medicine WHERE id = ?', (data['id'],)).fetchone() | ||
| if not entry: | ||
| raise NotFoundError(resource="Medicine entry", resource_id=data['id']) | ||
|
|
||
| db.execute( | ||
| 'UPDATE weekly_medicine SET week_number=?, name=?, dose=?, time=?, note=? WHERE id=?', | ||
| ( | ||
| data.get('week_number', entry['week_number']), | ||
| data.get('name', entry['name']), | ||
| data.get('dose', entry['dose']), | ||
| data.get('time', entry['time']), | ||
| data.get('note', entry['note']), | ||
| data['id'] | ||
| ) | ||
| ) | ||
| db.commit() | ||
|
|
||
| db_path = os.path.join(os.path.dirname(os.path.dirname(__file__)), "db", "database.db") | ||
| agent = get_agent(db_path) | ||
| agent.update_cache(data_type="medicine", operation="update") | ||
|
|
||
| return jsonify({"status": "success", "message": "Medicine updated"}), 200 |
Contributor
There was a problem hiding this comment.
Don't fork the update paths without the existing validators.
Backend/routes/medicine.py, Backend/routes/blood_pressure.py, Backend/routes/discharge.py, and Backend/routes/weight.py already validate partial updates before committing. These new /update_* handlers write request values straight to SQLite, so they can now persist values the canonical routes would reject and then refresh the cache from invalid data. Please reuse the same validators here or route these calls through the existing update paths.
Also applies to: 66-95, 119-148, 223-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Backend/routes/crud.py` around lines 13 - 42, The new PUT handler
update_medicine bypasses the existing partial-update validators and writes raw
request values to SQLite, risking persisting invalid data and then refreshing
the agent cache; fix it by reusing the existing validation/update logic from the
canonical route (e.g., import and call the validator or update function used in
Backend/routes/medicine.py) or invoke the same validation helpers before
executing the DB UPDATE in update_medicine; ensure you call the same validation
functions for fields like week_number/name/dose/time/note and only apply
db.execute with the validated/normalized values, and keep the agent cache
refresh (get_agent(...).update_cache(...)) after a successful validated update.
Comment on lines
+68
to
+71
| mood_id = data.get('id') | ||
| if not mood_id: | ||
| return jsonify({"error": "Mood ID is required"}), 400 | ||
|
|
Contributor
There was a problem hiding this comment.
Mood update/delete contract is misaligned with current frontend caller.
These handlers are id-only, but existing chat service calls send date/time-based identifiers. This mismatch will block mood edit/delete flows unless both sides are aligned.
Also applies to: 97-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Backend/routes/mood.py` around lines 68 - 71, Handlers currently only accept
payload key 'id' (mood_id = data.get('id')) but the frontend sends
date/time-based identifiers; update the handlers (the mood update and delete
handlers that reference mood_id and data.get('id')) to accept the frontend's
identifier key(s) (e.g., 'timestamp' or 'datetime') and/or support both formats.
Change the input parsing to check data.get('id') || data.get('timestamp') (or
parse a datetime string), normalize it to the DB lookup key (convert timestamp
to the stored id or query by datetime), and update the error message to reflect
the accepted keys; apply the same fix to the other handler block around the
97-105 area so both update and delete use the same identifier contract.
Comment on lines
+94
to
+96
| @profile_bp.route('/logout', methods=['POST']) | ||
| def logout(): | ||
| return jsonify({"status": "success", "message": "Logged out successfully. Local session cleared."}), 200 |
Contributor
There was a problem hiding this comment.
Actually clear the server-side session on logout.
This route only returns a success message right now. If the client still sends an existing session cookie, checks like require_auth will continue to treat the user as logged in.
Suggested fix
-from flask import Blueprint, jsonify, request
+from flask import Blueprint, jsonify, request, session
@@
`@profile_bp.route`('/logout', methods=['POST'])
def logout():
+ session.clear()
return jsonify({"status": "success", "message": "Logged out successfully. Local session cleared."}), 200📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @profile_bp.route('/logout', methods=['POST']) | |
| def logout(): | |
| return jsonify({"status": "success", "message": "Logged out successfully. Local session cleared."}), 200 | |
| from flask import Blueprint, jsonify, request, session |
Suggested change
| @profile_bp.route('/logout', methods=['POST']) | |
| def logout(): | |
| return jsonify({"status": "success", "message": "Logged out successfully. Local session cleared."}), 200 | |
| `@profile_bp.route`('/logout', methods=['POST']) | |
| def logout(): | |
| session.clear() | |
| return jsonify({"status": "success", "message": "Logged out successfully. Local session cleared."}), 200 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Backend/routes/profile.py` around lines 94 - 96, The logout route currently
only returns a JSON success message but does not clear the server-side session;
update the logout() handler (profile_bp.route '/logout') to remove
authentication state on the server by clearing relevant session keys (e.g.,
session.clear() or session.pop('user_id', None)) and, if using Flask-Login, call
logout_user(); then return the same response and explicitly remove the session
cookie on the response so subsequent requests (and checks like require_auth) no
longer treat the user as logged in.
Comment on lines
+22
to
+35
| week_number = data.get('week_number') | ||
| hours = data.get('hours') | ||
| quality = data.get('quality', '') | ||
| note = data.get('note', '') | ||
|
|
||
| if not week_number or hours is None: | ||
| raise MissingFieldError(['week_number', 'hours']) | ||
|
|
||
| if quality and quality.lower() not in VALID_QUALITY: | ||
| return jsonify({"error": f"Invalid quality. Must be one of: {', '.join(VALID_QUALITY)}"}), 400 | ||
|
|
||
| cursor = db.execute( | ||
| 'INSERT INTO sleep_logs (week_number, hours, quality, note) VALUES (?, ?, ?, ?)', | ||
| (week_number, hours, quality.lower() if quality else '', note) |
Contributor
There was a problem hiding this comment.
hours vs duration mismatch breaks current sleep API consumers.
Current create/read payloads use hours, while existing frontend callers use duration. This will cause failed creates and incorrect analytics parsing.
Suggested compatibility fix
- hours = data.get('hours')
+ hours = data.get('hours', data.get('duration'))
...
- return jsonify([dict(row) for row in rows]), 200
+ normalized = []
+ for row in rows:
+ item = dict(row)
+ item['duration'] = item.get('hours')
+ normalized.append(item)
+ return jsonify(normalized), 200Also applies to: 47-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Backend/routes/sleep.py` around lines 22 - 35, The API uses the field name
hours in the sleep create/read handlers but the frontend expects duration;
update the handlers in Backend/routes/sleep.py to accept and produce duration
while preserving backward-compatibility with hours: read duration from request
data if present (fall back to hours), validate and store the canonical value
(e.g., duration) in the DB insert used by the code block around cursor =
db.execute(...), and update response payloads (the read/list handlers around
lines 47-58) to return duration (and optionally include hours for compatibility)
so existing frontend callers continue to work.
Comment on lines
+69
to
+71
| sleep_id = data.get('id') | ||
| if not sleep_id: | ||
| return jsonify({"error": "Sleep ID is required"}), 400 |
Contributor
There was a problem hiding this comment.
Sleep update/delete identifier contract is inconsistent with chat caller payloads.
These endpoints require id, but current chat flows send date/time identifiers. Without alignment, update/delete operations will fail at runtime.
Also applies to: 104-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Backend/routes/sleep.py` around lines 69 - 71, The update/delete handlers
currently abort if data.get('id') is missing (sleep_id) but chat callers send
date/time identifiers; change the handlers (the code that sets sleep_id and the
update_sleep/delete_sleep functions) to accept either an explicit 'id' or a
date/time identifier pair (e.g., 'date' and 'start_time' or a single
'timestamp') — if 'id' is absent, normalize the incoming date/time fields into a
lookup key and query the DB for the matching sleep record to obtain its id
before proceeding; only return the 400 error when neither an 'id' nor valid
date/time fields are provided, and apply the same fix to the other occurrence of
this logic (the block covering lines ~104-113).
Comment on lines
+4
to
11
| if data.get('week_number') is not None and (not isinstance(data['week_number'], int) or data['week_number'] < 1): | ||
| errors["week_number"] = "week_number must be a positive integer." | ||
|
|
||
| if data.get('systolic') and (not isinstance(data['systolic'], int) or not (50 < data['systolic'] < 300)): | ||
| if data.get('systolic') is not None and (not isinstance(data['systolic'], int) or not (50 < data['systolic'] < 300)): | ||
| errors["systolic"] = "systolic must be an integer between 50 and 300." | ||
|
|
||
| if data.get('diastolic') and (not isinstance(data['diastolic'], int) or not (30 < data['diastolic'] < 200)): | ||
| if data.get('diastolic') is not None and (not isinstance(data['diastolic'], int) or not (30 < data['diastolic'] < 200)): | ||
| errors["diastolic"] = "diastolic must be an integer between 30 and 200." |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the Backend directory and understand the structure
find . -type f -name "utils.py" | grep -i backend
find . -type f -name "*blood_pressure*" | grep -i backendRepository: AOSSIE-Org/BabyNest
Length of output: 116
🏁 Script executed:
# List files to understand the repo structure
git ls-files | head -30Repository: AOSSIE-Org/BabyNest
Length of output: 892
🏁 Script executed:
# Check the size and read Backend/utils.py
wc -l Backend/utils.pyRepository: AOSSIE-Org/BabyNest
Length of output: 82
🏁 Script executed:
# Check the size and read Backend/routes/blood_pressure.py
wc -l Backend/routes/blood_pressure.pyRepository: AOSSIE-Org/BabyNest
Length of output: 99
🏁 Script executed:
# Read Backend/utils.py completely
cat -n Backend/utils.pyRepository: AOSSIE-Org/BabyNest
Length of output: 2526
🏁 Script executed:
# Read Backend/routes/blood_pressure.py completely
cat -n Backend/routes/blood_pressure.pyRepository: AOSSIE-Org/BabyNest
Length of output: 6317
Null values bypass required-field validation in create flow.
In Backend/routes/blood_pressure.py (line 20), the CREATE endpoint only checks key presence. The validation function in Backend/utils.py (lines 4-11) skips checks when values are None. Combined, payloads like {"week_number": null, "systolic": null, "diastolic": null, "time": "12:30"} pass both checks and reach the database.
The proposed fix is correct: add a require_non_null parameter to validate_bp_data(), use it in the CREATE endpoint to enforce non-null values for required fields, and keep the default False for the PATCH endpoint where partial updates are supported.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Backend/utils.py` around lines 4 - 11, The validator in validate_bp_data is
skipping checks when values are None, allowing create payloads with null
required fields; add a new boolean parameter require_non_null (default False) to
validate_bp_data and, when True (used by the CREATE endpoint in
Backend/routes/blood_pressure.py), treat keys present with value None as
validation failures for required fields (week_number, systolic, diastolic,
time), returning appropriate errors instead of skipping them; keep the default
False so PATCH/partial updates retain the current behavior and update the CREATE
call to pass require_non_null=True.
Comment on lines
+104
to
+105
| // Important: Use ref to get the latest conversation state | ||
| const updatedConversationForModel = [...conversationRef.current, userMessage]; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all writes to conversationRef and where updatedConversationForModel is built.
rg -n -C2 'conversationRef\.current\s*=' Frontend/src/hooks/useChatEngine.js
rg -n -C3 'updatedConversationForModel|setConversation\(\s*prev\s*=>\s*\{' Frontend/src/hooks/useChatEngine.jsRepository: AOSSIE-Org/BabyNest
Length of output: 1691
🏁 Script executed:
#!/bin/bash
# Find the sendMessage function and check for isGenerating guard
rg -n -B5 -A50 'const sendMessage|function sendMessage' Frontend/src/hooks/useChatEngine.js | head -100Repository: AOSSIE-Org/BabyNest
Length of output: 2013
🏁 Script executed:
#!/bin/bash
# Check lines 62-75 for re-entry guard
sed -n '62,75p' Frontend/src/hooks/useChatEngine.jsRepository: AOSSIE-Org/BabyNest
Length of output: 457
Potential history loss on rapid sends (ref updated too late).
At Line 105, updatedConversationForModel is built from conversationRef.current, but the ref is only synced in an effect after the state update. If sendMessage is triggered again before the effect runs, the second call reads stale ref content and constructs model history that skips the previous message.
With no re-entry guard (e.g., if (isGenerating) return), concurrent sends can occur. The sequence is:
- Call A: schedules state update, reads ref (still old), model receives [old + A]
- Call B: schedules state before effect A runs, reads ref (still old), model receives [old + B]
- Effect syncs ref after, but damage is done—model context missing prior turns
Suggest updating conversationRef atomically inside the setConversation functional update, not in a separate effect:
setConversation(prev => {
const newHistory = [...prev, userMessage];
conversationRef.current = newHistory;
saveChats(newHistory);
return newHistory;
});Apply the same pattern to other setConversation calls (line 183).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Frontend/src/hooks/useChatEngine.js` around lines 104 - 105, The conversation
ref is updated in a separate effect, causing race conditions when sendMessage is
called rapidly; instead, update conversationRef.current inside the
setConversation functional updater so the new array is computed and assigned
atomically (e.g., in sendMessage use setConversation(prev => { const next =
[...prev, userMessage]; conversationRef.current = next; return next; }) and
mirror this pattern for the other setConversation call at the second site; also
add a re-entry guard in sendMessage (check or set isGenerating at the start/end)
to prevent concurrent sends.
- EmergencyCallingScreen now uses Linking API to make real emergency calls - Shows confirmation dialog before calling 911 - Added proper UI with call button and emergency contacts placeholder - Updated styles for better UX Note: Video demonstration needed to show emergency call functionality working.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
Frontend/src/Screens/CalendarScreen.jsx (1)
332-333: Good refactor: Extracted magic number to named constant.The constant extraction improves readability. However, consider moving
SCHEDULE_OFFSET_MINUTESandtimeSlotHeightoutside the component since they are static values, avoiding recreation on each render.Additionally, the offset value of 370 (6:10 AM in minutes) seems inconsistent with the time grid starting at 8:00 AM (480 minutes). Verify this value produces correct vertical alignment for appointments.
♻️ Suggested refactor: move constants outside component
const months = [ 'January', ... 'December', ]; + +const timeSlotHeight = 80; +const SCHEDULE_OFFSET_MINUTES = 370; // Offset to position appointments in time grid + const generateWeekDates = startDate => {Then remove lines 332-333 from inside the component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frontend/src/Screens/CalendarScreen.jsx` around lines 332 - 333, Move the two static values out of the CalendarScreen component to module-level constants so they are not recreated on each render: lift timeSlotHeight and SCHEDULE_OFFSET_MINUTES to the top of the file (module scope) and remove their declarations from inside CalendarScreen; also verify and correct the offset value—if the time grid starts at 8:00 AM (480 minutes) then SCHEDULE_OFFSET_MINUTES should reflect that (e.g., 480) or be computed from an existing scheduleStartMinutes constant to ensure vertical alignment of appointments.Frontend/src/Screens/SettingsScreen.jsx (1)
1-1: Stale file path comment.The comment says
src/components/ProfileScreen.jsbut this file isSettingsScreen.jsx. Consider removing or correcting this comment to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frontend/src/Screens/SettingsScreen.jsx` at line 1, Remove or update the stale header comment referencing src/components/ProfileScreen.js in SettingsScreen.jsx; locate the top-of-file comment (the line containing "// src/components/ProfileScreen.js") and either delete it or replace it with an accurate comment such as the current file/component name (e.g., "// src/Screens/SettingsScreen.jsx" or "// SettingsScreen component") so the file header accurately reflects the file and component.Frontend/src/Screens/EmergencyCallingScreen.jsx (2)
2-2: Unused import:Image
Imageis imported but no longer used in the JSX after the refactor to placeholder contacts. The correspondingcontactImagestyle on line 97 is also unused.🧹 Suggested cleanup
-import { View, Text, TouchableOpacity, StyleSheet, Image, Alert, Linking, Platform } from "react-native"; +import { View, Text, TouchableOpacity, StyleSheet, Alert, Linking } from "react-native";And optionally remove the unused style:
- contactImage: { width: 50, height: 50, borderRadius: 25, marginHorizontal: 10 },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frontend/src/Screens/EmergencyCallingScreen.jsx` at line 2, Remove the unused Image import and the unused contactImage style: delete "Image" from the import list in EmergencyCallingScreen.jsx and remove the contactImage entry from the StyleSheet (or any references to it) so there are no dead imports or styles left (look for the imported symbol Image and the StyleSheet key contactImage).
2-2: Unused import:Platform
Platformis imported but not used anywhere in this file.🧹 Suggested fix
-import { View, Text, TouchableOpacity, StyleSheet, Image, Alert, Linking, Platform } from "react-native"; +import { View, Text, TouchableOpacity, StyleSheet, Image, Alert, Linking } from "react-native";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frontend/src/Screens/EmergencyCallingScreen.jsx` at line 2, The import list in EmergencyCallingScreen.jsx includes an unused symbol Platform; remove Platform from the import statement (the line importing View, Text, TouchableOpacity, StyleSheet, Image, Alert, Linking, Platform) so only used modules remain to eliminate the unused import warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Backend/db/db.py`:
- Line 24: Replace the two separate exists()+stat() checks with a single
race-safe helper (e.g., file_missing_or_empty(path) or
is_database_missing_or_empty(path)) that performs a single os.stat() call inside
a try/except FileNotFoundError and returns True if the file is missing or its
st_size == 0; then call that helper wherever the code currently uses
os.path.exists(DATABASE) or os.stat(DATABASE).st_size checks (reference the
DATABASE variable and replace both occurrences around the startup checks) so the
check is atomic and TOCTOU-safe.
- Line 3: The code currently imports fcntl (in Backend/db/db.py) which breaks on
Windows; either add a cross-platform fallback or document POSIX-only support:
implement a small compatibility layer that tries "import fcntl" and on
ImportError/import failure imports "msvcrt" and exposes the same file-locking
API (e.g., provide lock_file/unlock_file functions or a FileLock class used by
any existing callers) mapping fcntl.flock semantics to msvcrt.locking, or if you
choose POSIX-only, update the README/CI to state and enforce Linux/POSIX support
and raise a clear RuntimeError at import time if fcntl is unavailable. Ensure
the symbols referenced (fcntl, msvcrt, lock_file/unlock_file or FileLock) are
used consistently so callers in this module don't need to change.
In `@Frontend/src/Screens/EmergencyCallingScreen.jsx`:
- Line 86: In the callingText style object (callingText), fix the unterminated
string literal for textAlign by replacing the mismatched `"center'` with a
properly quoted string (e.g., "center" or 'center') so the style reads
textAlign: "center" and the file parses correctly.
- Around line 12-30: The handleEmergencyCall function leaves isCalling true when
Linking.openURL succeeds; update the success path so setIsCalling(false) is
called after a successful openURL (e.g., chain a .then(() =>
setIsCalling(false)) or use async/await with try/finally around
Linking.openURL(`tel:${EMERGENCY_NUMBER}`)) so both success and failure reset
isCalling; reference handleEmergencyCall, setIsCalling, Linking.openURL and
EMERGENCY_NUMBER when making the change.
In `@Frontend/src/Screens/SettingsScreen.jsx`:
- Around line 119-123: The IconButton currently ignores the onPress passed from
SettingsScreen, so update the IconButton component to accept an onPress prop and
use a touchable container (e.g., TouchableOpacity or Pressable) instead of a
plain View: add onPress to IconButton's props signature, wrap the rendered
content in TouchableOpacity/Pressable and pass the onPress handler through (also
import TouchableOpacity/Pressable), and preserve existing icon and label
rendering so the Notification button in SettingsScreen will respond to taps.
In `@Frontend/src/Screens/SOSAlertScreen.jsx`:
- Around line 11-15: The tap counting logic uses a stale closure value (tapCount
+ 1) which can miss increments under rapid presses; update the handler to use a
functional state updater with setTapCount(prev => ...) so each increment is
based on the latest state, compute newCount = prev + 1 inside that updater, call
navigation.navigate("EmergencyCalling") when newCount >= 4, and reset the state
to 0 there (otherwise return newCount); update references to
tapCount/setTapCount and keep the EmergencyCalling navigation call inside the
functional updater.
---
Nitpick comments:
In `@Frontend/src/Screens/CalendarScreen.jsx`:
- Around line 332-333: Move the two static values out of the CalendarScreen
component to module-level constants so they are not recreated on each render:
lift timeSlotHeight and SCHEDULE_OFFSET_MINUTES to the top of the file (module
scope) and remove their declarations from inside CalendarScreen; also verify and
correct the offset value—if the time grid starts at 8:00 AM (480 minutes) then
SCHEDULE_OFFSET_MINUTES should reflect that (e.g., 480) or be computed from an
existing scheduleStartMinutes constant to ensure vertical alignment of
appointments.
In `@Frontend/src/Screens/EmergencyCallingScreen.jsx`:
- Line 2: Remove the unused Image import and the unused contactImage style:
delete "Image" from the import list in EmergencyCallingScreen.jsx and remove the
contactImage entry from the StyleSheet (or any references to it) so there are no
dead imports or styles left (look for the imported symbol Image and the
StyleSheet key contactImage).
- Line 2: The import list in EmergencyCallingScreen.jsx includes an unused
symbol Platform; remove Platform from the import statement (the line importing
View, Text, TouchableOpacity, StyleSheet, Image, Alert, Linking, Platform) so
only used modules remain to eliminate the unused import warning.
In `@Frontend/src/Screens/SettingsScreen.jsx`:
- Line 1: Remove or update the stale header comment referencing
src/components/ProfileScreen.js in SettingsScreen.jsx; locate the top-of-file
comment (the line containing "// src/components/ProfileScreen.js") and either
delete it or replace it with an accurate comment such as the current
file/component name (e.g., "// src/Screens/SettingsScreen.jsx" or "//
SettingsScreen component") so the file header accurately reflects the file and
component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b90a2535-a0c6-4a0a-b164-753c3a75a071
📒 Files selected for processing (8)
Backend/db/db.pyFrontend/src/Screens/CalendarScreen.jsxFrontend/src/Screens/EmergencyCallingScreen.jsxFrontend/src/Screens/HomeScreen.jsxFrontend/src/Screens/ProfileScreen.jsxFrontend/src/Screens/SOSAlertScreen.jsxFrontend/src/Screens/SettingsScreen.jsxFrontend/src/hooks/useChatEngine.js
🚧 Files skipped from review as they are similar to previous changes (1)
- Frontend/src/hooks/useChatEngine.js
Backend/db/db.py
Outdated
| @@ -1,9 +1,12 @@ | |||
| import sqlite3 | |||
| import os | |||
| import fcntl | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm POSIX-only locking usage locations
rg -n -C2 '\bimport\s+fcntl\b|\bfcntl\.flock\s*\(' --type=py
# Check whether project runtime/docs/CI explicitly constrain OS to Linux
fd -HI 'Dockerfile|docker-compose|README|requirements|pyproject|workflows|\.yml|\.yaml' \
| xargs -r rg -n -C1 'linux|ubuntu|windows|macos|docker|container'Repository: AOSSIE-Org/BabyNest
Length of output: 5611
🏁 Script executed:
# Check the full context around fcntl usage in db.py
cat -n Backend/db/db.py | head -50Repository: AOSSIE-Org/BabyNest
Length of output: 1660
🏁 Script executed:
# Look for any platform checks or try/except around fcntl
rg -A5 -B5 'fcntl\.flock' Backend/db/db.pyRepository: AOSSIE-Org/BabyNest
Length of output: 824
🏁 Script executed:
# Search for README and documentation files
fd -HI 'README|CONTRIBUTING|INSTALL|SETUP' | head -20Repository: AOSSIE-Org/BabyNest
Length of output: 139
🏁 Script executed:
# Look for CI configuration files
fd -HI '\.github|\.gitlab-ci|\.circleci|\.travis|azure-pipelines|Jenkinsfile'Repository: AOSSIE-Org/BabyNest
Length of output: 71
🏁 Script executed:
# Read main README
cat README.mdRepository: AOSSIE-Org/BabyNest
Length of output: 5067
🏁 Script executed:
# Find and read CI workflow files
fd -HI '\.yml|\.yaml' .github/workflowsRepository: AOSSIE-Org/BabyNest
Length of output: 167
🏁 Script executed:
# Check for any sys.platform guards in the codebase
rg 'sys\.platform|sys\.version_info|os\.name' --type=py -nRepository: AOSSIE-Org/BabyNest
Length of output: 45
🏁 Script executed:
# Check if Windows is mentioned anywhere in project setup/docs
rg -i 'windows|win32' README.md Backend/ --type=py --type=mdRepository: AOSSIE-Org/BabyNest
Length of output: 45
🏁 Script executed:
# Double-check: are there any platform-specific imports or guards anywhere?
rg -n 'import.*os\.|import sys|try:|except.*ImportError|except.*AttributeError' Backend/db/ --type=py -A2 -B2Repository: AOSSIE-Org/BabyNest
Length of output: 417
🏁 Script executed:
# Check if there are any conditional imports or fallbacks for fcntl
rg -n 'fcntl|flock' Backend/ --type=pyRepository: AOSSIE-Org/BabyNest
Length of output: 260
Add cross-platform file locking or document Linux-only support.
The fcntl module will fail on Windows (AttributeError on import). Either implement a cross-platform locking fallback using msvcrt on Windows and fcntl on POSIX, or explicitly document and enforce Linux/POSIX-only support in the project README and CI configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Backend/db/db.py` at line 3, The code currently imports fcntl (in
Backend/db/db.py) which breaks on Windows; either add a cross-platform fallback
or document POSIX-only support: implement a small compatibility layer that tries
"import fcntl" and on ImportError/import failure imports "msvcrt" and exposes
the same file-locking API (e.g., provide lock_file/unlock_file functions or a
FileLock class used by any existing callers) mapping fcntl.flock semantics to
msvcrt.locking, or if you choose POSIX-only, update the README/CI to state and
enforce Linux/POSIX support and raise a clear RuntimeError at import time if
fcntl is unavailable. Ensure the symbols referenced (fcntl, msvcrt,
lock_file/unlock_file or FileLock) are used consistently so callers in this
module don't need to change.
| db.close() | ||
|
|
||
| def first_time_setup(): | ||
| if not os.path.exists(DATABASE) or os.stat(DATABASE).st_size == 0: |
Contributor
There was a problem hiding this comment.
Harden the database existence check against TOCTOU failures.
Line 24 and Line 31 use exists() + stat() separately; if the file disappears between those calls, startup can throw FileNotFoundError. Use a single race-safe helper and reuse it in both checks.
Proposed fix
+def _db_missing_or_empty(path):
+ try:
+ return os.stat(path).st_size == 0
+ except FileNotFoundError:
+ return True
+
def first_time_setup():
- if not os.path.exists(DATABASE) or os.stat(DATABASE).st_size == 0:
+ if _db_missing_or_empty(DATABASE):
# Use file lock to prevent race condition during setup
os.makedirs(os.path.dirname(SETUP_LOCK_FILE), exist_ok=True)
with open(SETUP_LOCK_FILE, 'w') as lock_file:
try:
fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX)
# Double-check after acquiring lock
- if not os.path.exists(DATABASE) or os.stat(DATABASE).st_size == 0:
+ if _db_missing_or_empty(DATABASE):
with sqlite3.connect(DATABASE) as db:
with open(SCHEMA_FILE,"r") as f:
db.executescript(f.read())
db.commit()
finally:
fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN)Also applies to: 31-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Backend/db/db.py` at line 24, Replace the two separate exists()+stat() checks
with a single race-safe helper (e.g., file_missing_or_empty(path) or
is_database_missing_or_empty(path)) that performs a single os.stat() call inside
a try/except FileNotFoundError and returns True if the file is missing or its
st_size == 0; then call that helper wherever the code currently uses
os.path.exists(DATABASE) or os.stat(DATABASE).st_size checks (reference the
DATABASE variable and replace both occurrences around the startup checks) so the
check is atomic and TOCTOU-safe.
Comment on lines
+12
to
+30
| const handleEmergencyCall = () => { | ||
| setIsCalling(true); | ||
| Alert.alert( | ||
| 'Emergency Call', | ||
| `Calling ${EMERGENCY_NUMBER}...`, | ||
| [ | ||
| { text: 'Cancel', style: 'cancel', onPress: () => setIsCalling(false) }, | ||
| { | ||
| text: 'Call', | ||
| onPress: () => { | ||
| Linking.openURL(`tel:${EMERGENCY_NUMBER}`).catch(err => { | ||
| Alert.alert('Error', 'Unable to make phone call'); | ||
| setIsCalling(false); | ||
| }); | ||
| } | ||
| } | ||
| ] | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
Missing state reset after successful call.
When Linking.openURL() succeeds, isCalling remains true, leaving the call button permanently disabled when the user returns to the app. Only error and cancel paths reset the state.
🔧 Proposed fix
text: 'Call',
onPress: () => {
- Linking.openURL(`tel:${EMERGENCY_NUMBER}`).catch(err => {
+ Linking.openURL(`tel:${EMERGENCY_NUMBER}`).then(() => {
+ setIsCalling(false);
+ }).catch(err => {
Alert.alert('Error', 'Unable to make phone call');
setIsCalling(false);
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleEmergencyCall = () => { | |
| setIsCalling(true); | |
| Alert.alert( | |
| 'Emergency Call', | |
| `Calling ${EMERGENCY_NUMBER}...`, | |
| [ | |
| { text: 'Cancel', style: 'cancel', onPress: () => setIsCalling(false) }, | |
| { | |
| text: 'Call', | |
| onPress: () => { | |
| Linking.openURL(`tel:${EMERGENCY_NUMBER}`).catch(err => { | |
| Alert.alert('Error', 'Unable to make phone call'); | |
| setIsCalling(false); | |
| }); | |
| } | |
| } | |
| ] | |
| ); | |
| }; | |
| const handleEmergencyCall = () => { | |
| setIsCalling(true); | |
| Alert.alert( | |
| 'Emergency Call', | |
| `Calling ${EMERGENCY_NUMBER}...`, | |
| [ | |
| { text: 'Cancel', style: 'cancel', onPress: () => setIsCalling(false) }, | |
| { | |
| text: 'Call', | |
| onPress: () => { | |
| Linking.openURL(`tel:${EMERGENCY_NUMBER}`).then(() => { | |
| setIsCalling(false); | |
| }).catch(err => { | |
| Alert.alert('Error', 'Unable to make phone call'); | |
| setIsCalling(false); | |
| }); | |
| } | |
| } | |
| ] | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Frontend/src/Screens/EmergencyCallingScreen.jsx` around lines 12 - 30, The
handleEmergencyCall function leaves isCalling true when Linking.openURL
succeeds; update the success path so setIsCalling(false) is called after a
successful openURL (e.g., chain a .then(() => setIsCalling(false)) or use
async/await with try/finally around Linking.openURL(`tel:${EMERGENCY_NUMBER}`))
so both success and failure reset isCalling; reference handleEmergencyCall,
setIsCalling, Linking.openURL and EMERGENCY_NUMBER when making the change.
| callingText: { fontSize: 20, color: "white", marginBottom: 20 }, | ||
| callingCircle: { width: 100, height: 100, borderRadius: 50, backgroundColor: "white", justifyContent: "center", alignItems: "center" }, | ||
| callingContainer: { flex: 1, justifyContent: "center", alignItems: "center", backgroundColor: "#FF6F61", padding: 20 }, | ||
| callingText: { fontSize: 22, color: "white", marginBottom: 30, fontWeight: "bold", textAlign: "center' }, |
Contributor
There was a problem hiding this comment.
Critical syntax error: unterminated string literal.
The string "center' has mismatched quotes (starts with ", ends with '), causing a parse error that will break the application.
🐛 Fix the mismatched quotes
- callingText: { fontSize: 22, color: "white", marginBottom: 30, fontWeight: "bold", textAlign: "center' },
+ callingText: { fontSize: 22, color: "white", marginBottom: 30, fontWeight: "bold", textAlign: "center" },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| callingText: { fontSize: 22, color: "white", marginBottom: 30, fontWeight: "bold", textAlign: "center' }, | |
| callingText: { fontSize: 22, color: "white", marginBottom: 30, fontWeight: "bold", textAlign: "center" }, |
🧰 Tools
🪛 Biome (2.4.6)
[error] 86-86: unterminated string literal
(parse)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Frontend/src/Screens/EmergencyCallingScreen.jsx` at line 86, In the
callingText style object (callingText), fix the unterminated string literal for
textAlign by replacing the mismatched `"center'` with a properly quoted string
(e.g., "center" or 'center') so the style reads textAlign: "center" and the file
parses correctly.
Comment on lines
+119
to
+123
| <IconButton | ||
| icon="🔔" | ||
| label="Notification" | ||
| onPress={() => Alert.alert('Notifications', 'Notification settings coming soon!')} | ||
| /> |
Contributor
There was a problem hiding this comment.
onPress prop is not handled by IconButton component.
The IconButton component (lines 19-30) only accepts { icon, label } props and renders a non-interactive View. The onPress handler passed here will be silently ignored, and the notification button won't respond to taps.
Either update IconButton to accept and use onPress, or use a TouchableOpacity wrapper directly.
🐛 Proposed fix: Update IconButton to support onPress
-const IconButton = ({ icon, label }) => {
+const IconButton = ({ icon, label, onPress }) => {
const { theme } = useTheme();
return (
- <View style={[styles.iconContainer, { backgroundColor: theme.iconBackground }]}>
+ <TouchableOpacity
+ style={[styles.iconContainer, { backgroundColor: theme.iconBackground }]}
+ onPress={onPress}
+ disabled={!onPress}
+ >
<View style={[styles.iconCircle, { backgroundColor: theme.primary }]}>
<Text style={styles.iconText}>{icon}</Text>
</View>
<Text style={[styles.iconLabel, { color: theme.iconText || theme.text}]}>{label}</Text>
- </View>
+ </TouchableOpacity>
);
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Frontend/src/Screens/SettingsScreen.jsx` around lines 119 - 123, The
IconButton currently ignores the onPress passed from SettingsScreen, so update
the IconButton component to accept an onPress prop and use a touchable container
(e.g., TouchableOpacity or Pressable) instead of a plain View: add onPress to
IconButton's props signature, wrap the rendered content in
TouchableOpacity/Pressable and pass the onPress handler through (also import
TouchableOpacity/Pressable), and preserve existing icon and label rendering so
the Notification button in SettingsScreen will respond to taps.
Comment on lines
+11
to
+15
| const newCount = tapCount + 1; | ||
| setTapCount(newCount); | ||
| if (newCount >= 4) { | ||
| navigation.navigate("EmergencyCalling"); | ||
| } else { | ||
| setTapCount(tapCount + 1); | ||
| setTapCount(0); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In React Native (React 18+), can onPress handlers lose increments when using setState(valueFromClosure) instead of setState(prev => next)? Please cite React docs on batched/asynchronous updates and functional state updates.
💡 Result:
Yes. In React 18+ (including React Native), an onPress handler can “lose” increments if you compute the next state from a stale value captured in the handler’s closure (e.g. setCount(count + 1)) instead of using a functional update (e.g. setCount(c => c + 1)).
Why it happens:
- State updates are not applied immediately inside the running event handler—calling a setter requests a re-render, but it doesn’t change the state variable value you already have in that handler/closure. So repeated
setCount(count + 1)calls in the same handler (or in async work started by the handler) can all enqueue the same “next value,” effectively dropping increments. React’suseStatedocs explicitly note that an update “does not affect the … variable in your already-running event handler.” [1] - React batches updates (and React 18 introduced broader/automatic batching), so multiple updates can be processed together rather than one-by-one immediately, increasing the chance that “value from closure” updates collapse into one. [2] React 18 also documents “automatic batching” more generally. [3]
- Functional updates are designed for this case: when you want to update based on the previous state, pass an updater function (
setCount(c => c + 1)), and React will apply queued updaters in order using the latest queued state. [2]
Rule of thumb for counters/toggles: if the next state depends on the previous state, use setState(prev => next) (functional update), especially in event handlers and any async callbacks triggered by them.
Sources (React docs):
- React
useStatereference (state value in an already-running handler doesn’t change immediately). [1] - “Queueing a Series of State Updates” (batching + functional updater to avoid lost updates). [2]
- React 18 blog post (automatic batching in React 18). [3]
[1] (react.dev)
[2] (react.dev)
[3] (react.dev)
Citations:
- 1: https://react.dev/reference/react/useState?utm_source=openai
- 2: https://react.dev/learn/queueing-a-series-of-state-updates?utm_source=openai
- 3: https://react.dev/blog/2022/03/29/react-v18?utm_source=openai
Use functional state updates to avoid missed taps in SOS trigger.
On lines 11–15, tapCount + 1 uses a closure value that can drop increments under rapid presses when React batches updates. In this emergency flow, that can make the 4-tap threshold unreliable. Use a functional updater so React applies state changes sequentially based on the previous state, not a stale closure value.
🔧 Proposed fix
const handleTap = () => {
- const newCount = tapCount + 1;
- setTapCount(newCount);
- if (newCount >= 4) {
- navigation.navigate("EmergencyCalling");
- setTapCount(0);
- }
+ setTapCount((prevCount) => {
+ const newCount = prevCount + 1;
+ if (newCount >= 4) {
+ navigation.navigate("EmergencyCalling");
+ return 0;
+ }
+ return newCount;
+ });
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Frontend/src/Screens/SOSAlertScreen.jsx` around lines 11 - 15, The tap
counting logic uses a stale closure value (tapCount + 1) which can miss
increments under rapid presses; update the handler to use a functional state
updater with setTapCount(prev => ...) so each increment is based on the latest
state, compute newCount = prev + 1 inside that updater, call
navigation.navigate("EmergencyCalling") when newCount >= 4, and reset the state
to 0 there (otherwise return newCount); update references to
tapCount/setTapCount and keep the EmergencyCalling navigation call inside the
functional updater.
- vector_store.py: chromadb is now optional, graceful fallback if not installed - db.py: simplified first_time_setup without file lock (Windows compatibility)
- SOSAlertScreen: removed unused Image import - EmergencyCallingScreen: removed unused Platform import
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Backend/agent/vector_store.py (3)
107-120:⚠️ Potential issue | 🟠 MajorMissing guard for chromadb availability will cause
AttributeError.Same issue as
update_guidelines_in_vector_store: when chromadb is unavailable,user_details_collectionisNoneand.upsert()will crash.Proposed fix
def update_user_details_in_vector_store(documents: list = None, ids: list = None, metadatas: list = None): """Update user details in the vector store.""" + if not CHROMADB_AVAILABLE: + print("ChromaDB not available, skipping user details update.") + return if not documents or not ids or not metadatas: print("No user detail documents to update.") return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Backend/agent/vector_store.py` around lines 107 - 120, The function update_user_details_in_vector_store currently calls user_details_collection.upsert without checking whether the Chroma client/collection exists, causing AttributeError when chromadb is unavailable; update update_user_details_in_vector_store to check that user_details_collection is not None (and/or that chroma is enabled) before calling upsert, return early and log a clear message if the collection is missing, and keep the existing try/except around the upsert call to handle runtime errors.
47-105:⚠️ Potential issue | 🟠 MajorMissing guard for chromadb availability will cause
AttributeError.When
CHROMADB_AVAILABLEisFalse,guidelines_collectionisNone. Calling.count()(line 72),.delete()(line 74), or.add()(line 94) onNonewill raiseAttributeError, crashing the function instead of gracefully degrading.The callers (
initialize_knowledge_basein context.py,refresh_cache_and_embeddingsin agent.py) don't guard against this either.Proposed fix
def update_guidelines_in_vector_store(): """Update the vector store with pregnancy guidelines.""" + if not CHROMADB_AVAILABLE: + print("ChromaDB not available, skipping vector store update.") + return False try: guidelines_file = os.path.join(os.path.dirname(__file__), "guidelines.json")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Backend/agent/vector_store.py` around lines 47 - 105, update_guidelines_in_vector_store currently assumes chromadb is available and calls methods on guidelines_collection which may be None when CHROMADB_AVAILABLE is False; add a guard at the start of update_guidelines_in_vector_store that checks CHROMADB_AVAILABLE and that guidelines_collection is not None and return False (or skip processing) if unavailable, and update callers initialize_knowledge_base and refresh_cache_and_embeddings to handle a False return gracefully; ensure all uses of guidelines_collection (count, delete, add) are only invoked after this guard so AttributeError is avoided.
122-136:⚠️ Potential issue | 🟠 MajorMissing guard for chromadb availability will cause
AttributeError.Same issue:
guidelines_collection.query()will crash when chromadb is unavailable.Proposed fix
def query_vector_store(query: str, n_results: int = 3): """Query the vector store for relevant guidelines.""" + if not CHROMADB_AVAILABLE: + print("ChromaDB not available, cannot query vector store.") + return [] try: results = guidelines_collection.query(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Backend/agent/vector_store.py` around lines 122 - 136, The function query_vector_store should guard against chromadb being unavailable by checking that guidelines_collection exists and has a callable query method before calling guidelines_collection.query; update query_vector_store to verify guidelines_collection is not None and hasattr(guidelines_collection, "query") and callable(guidelines_collection.query), log/print a clear message and return an empty list if the guard fails, then proceed to call guidelines_collection.query and keep the existing try/except to handle runtime errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Backend/db/db.py`:
- Line 3: Remove the unused top-level import of the sys module in
Backend/db/db.py (the "import sys" statement); delete that line so no unused
symbol "sys" remains imported and ensure no other references to sys exist in
functions or classes in this file (e.g., any uses in db-related functions or
classes).
---
Outside diff comments:
In `@Backend/agent/vector_store.py`:
- Around line 107-120: The function update_user_details_in_vector_store
currently calls user_details_collection.upsert without checking whether the
Chroma client/collection exists, causing AttributeError when chromadb is
unavailable; update update_user_details_in_vector_store to check that
user_details_collection is not None (and/or that chroma is enabled) before
calling upsert, return early and log a clear message if the collection is
missing, and keep the existing try/except around the upsert call to handle
runtime errors.
- Around line 47-105: update_guidelines_in_vector_store currently assumes
chromadb is available and calls methods on guidelines_collection which may be
None when CHROMADB_AVAILABLE is False; add a guard at the start of
update_guidelines_in_vector_store that checks CHROMADB_AVAILABLE and that
guidelines_collection is not None and return False (or skip processing) if
unavailable, and update callers initialize_knowledge_base and
refresh_cache_and_embeddings to handle a False return gracefully; ensure all
uses of guidelines_collection (count, delete, add) are only invoked after this
guard so AttributeError is avoided.
- Around line 122-136: The function query_vector_store should guard against
chromadb being unavailable by checking that guidelines_collection exists and has
a callable query method before calling guidelines_collection.query; update
query_vector_store to verify guidelines_collection is not None and
hasattr(guidelines_collection, "query") and
callable(guidelines_collection.query), log/print a clear message and return an
empty list if the guard fails, then proceed to call guidelines_collection.query
and keep the existing try/except to handle runtime errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 030858c6-0354-45b3-b8af-32d55e03efbc
📒 Files selected for processing (2)
Backend/agent/vector_store.pyBackend/db/db.py
| @@ -1,9 +1,11 @@ | |||
| import sqlite3 | |||
| import os | |||
| import sys | |||
Contributor
There was a problem hiding this comment.
Remove unused sys import.
The sys module is imported but never used in this file.
Proposed fix
import sqlite3
import os
-import sys
from flask import g🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Backend/db/db.py` at line 3, Remove the unused top-level import of the sys
module in Backend/db/db.py (the "import sys" statement); delete that line so no
unused symbol "sys" remains imported and ensure no other references to sys exist
in functions or classes in this file (e.g., any uses in db-related functions or
classes).
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
Frontend/src/Screens/EmergencyCallingScreen.jsx (2)
22-25:⚠️ Potential issue | 🟠 MajorReset
isCallingon successful dial open as well.Line [22] only resets state on failure. After a successful
Linking.openURL,isCallingcan staytrue, leaving the call control disabled when user returns. This repeats the previously reported issue.🔧 Proposed fix
{ text: 'Call', - onPress: () => { - Linking.openURL(`tel:${EMERGENCY_NUMBER}`).catch(err => { - Alert.alert('Error', 'Unable to make phone call'); - setIsCalling(false); - }); + onPress: async () => { + try { + await Linking.openURL(`tel:${EMERGENCY_NUMBER}`); + } catch (err) { + Alert.alert('Error', 'Unable to make phone call'); + } finally { + setIsCalling(false); + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frontend/src/Screens/EmergencyCallingScreen.jsx` around lines 22 - 25, The on-call opener only resets state on failure; ensure isCalling is reset after a successful dial as well by handling the Promise returned by Linking.openURL (for the call to `tel:${EMERGENCY_NUMBER}`) and invoking `setIsCalling(false)` on success (or in a `.finally()`/resolved handler) so the UI is re-enabled when the user returns; locate the existing `Linking.openURL(...).catch(...)` call and add the success/final handler to clear `isCalling`.
86-87:⚠️ Potential issue | 🔴 CriticalFix unterminated string literal in styles (parse blocker).
Line [86] has mismatched quotes in
textAlign, which breaks parsing and causes the Line [87] comma error downstream. This repeats the previously reported issue.🐛 Proposed fix
- callingText: { fontSize: 22, color: "white", marginBottom: 30, fontWeight: "bold", textAlign: "center' }, + callingText: { fontSize: 22, color: "white", marginBottom: 30, fontWeight: "bold", textAlign: "center" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frontend/src/Screens/EmergencyCallingScreen.jsx` around lines 86 - 87, The style object callingText has a mismatched quote in the textAlign property causing a parse error; open the styles where callingText and instructionText are defined and fix textAlign in callingText to use matching quotes (e.g., textAlign: "center") so the object literal is valid and the trailing comma after instructionText no longer triggers a downstream error.Frontend/src/Screens/SOSAlertScreen.jsx (1)
11-15:⚠️ Potential issue | 🟠 MajorUse functional state updates for the SOS tap counter.
Line [11] reads
tapCountfrom closure, which can miss increments under batched/rapid taps and make the 4-tap trigger unreliable. This repeats the previously reported issue.🔧 Proposed fix
const handleTap = () => { - const newCount = tapCount + 1; - setTapCount(newCount); - if (newCount >= 4) { - navigation.navigate("EmergencyCalling"); - setTapCount(0); - } + setTapCount((prevCount) => { + const newCount = prevCount + 1; + if (newCount >= 4) { + navigation.navigate("EmergencyCalling"); + return 0; + } + return newCount; + }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frontend/src/Screens/SOSAlertScreen.jsx` around lines 11 - 15, The tap counter uses the stale closure variable tapCount when calling setTapCount, causing missed increments under rapid taps; change to a functional state update with setTapCount(prev => { const newCount = prev + 1; if (newCount >= 4) { navigation.navigate("EmergencyCalling"); return 0; } return newCount; }) so increments are based on the latest state and the 4-tap trigger is reliable (refer to tapCount, setTapCount, and navigation.navigate("EmergencyCalling")).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Frontend/src/Screens/EmergencyCallingScreen.jsx`:
- Around line 22-25: The on-call opener only resets state on failure; ensure
isCalling is reset after a successful dial as well by handling the Promise
returned by Linking.openURL (for the call to `tel:${EMERGENCY_NUMBER}`) and
invoking `setIsCalling(false)` on success (or in a `.finally()`/resolved
handler) so the UI is re-enabled when the user returns; locate the existing
`Linking.openURL(...).catch(...)` call and add the success/final handler to
clear `isCalling`.
- Around line 86-87: The style object callingText has a mismatched quote in the
textAlign property causing a parse error; open the styles where callingText and
instructionText are defined and fix textAlign in callingText to use matching
quotes (e.g., textAlign: "center") so the object literal is valid and the
trailing comma after instructionText no longer triggers a downstream error.
In `@Frontend/src/Screens/SOSAlertScreen.jsx`:
- Around line 11-15: The tap counter uses the stale closure variable tapCount
when calling setTapCount, causing missed increments under rapid taps; change to
a functional state update with setTapCount(prev => { const newCount = prev + 1;
if (newCount >= 4) { navigation.navigate("EmergencyCalling"); return 0; } return
newCount; }) so increments are based on the latest state and the 4-tap trigger
is reliable (refer to tapCount, setTapCount, and
navigation.navigate("EmergencyCalling")).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f96072a-040f-45c5-87d9-b89c0ff610d6
📒 Files selected for processing (2)
Frontend/src/Screens/EmergencyCallingScreen.jsxFrontend/src/Screens/SOSAlertScreen.jsx
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.
Summary
High priority fixes for missing endpoints, broken URLs, and logic bugs
High Priority Fixes
/agent/refreshendpoint for AgentContext/undo_last_actionendpoint for undo functionalitycrud.pywith all UPDATE/DELETE endpoints:/update_medicine,/delete_medicine/update_blood_pressure,/delete_blood_pressure/update_discharge,/delete_discharge/update_symptoms,/delete_symptoms/update_weight,/delete_weightURL Fixes
/appointments/${id}->/delete_appointment/${id}/appointments->/add_appointmentLogic Fixes
useChatEnginestale closure bug usinguseRefto always get latest conversationConversationContextusing brokenprocess.env.BASE_URL- now imports from@envBackend Updates
appointments.py: returns{id: lastrowid}on successful createmedicine.py: returns{id: lastrowid}on successful createweight.py: returns{id: lastrowid}on successful createSummary by CodeRabbit
New Features
Improvements
Bug Fixes