fix: add input validation and zero-score guard to evaluate-assessments Edge Function#172
Conversation
…nts Edge Function (AOSSIE-Org#171)
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Changes
Sequence DiagramsequenceDiagram
actor Client
participant Function as evaluate-assessments
participant DB as Database
Client->>Function: POST request body
rect rgba(200, 100, 100, 0.5)
Function->>Function: Parse & validate body
alt Missing/Invalid fields
Function-->>Client: 400 Bad Request
end
end
rect rgba(100, 150, 200, 0.5)
Function->>DB: Fetch assessment data
DB-->>Function: Assessment details
end
rect rgba(100, 200, 100, 0.5)
Function->>Function: Score questions<br/>(track scoredCount)
alt scoredCount === 0
Function-->>Client: 422 Unprocessable Entity
end
end
Function->>DB: Insert assessment_results
DB-->>Function: Confirm
Function-->>Client: 200 OK + assessment_score
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes two bugs in the evaluate-assessments Supabase Edge Function identified in issue #171: (1) a crash on missing/malformed input that produced an unhelpful 500 error, and (2) a silent zero-score false negative when submitted IDs didn't match assessment data.
Changes:
- Added early input validation via
req.json().catch(() => null)to return400 Bad Requestbefore any processing whenpatient_id,assessment_id, orquestionsare missing/malformed. - Added a
scoredCounttracker in the scoring loop and a422 Unprocessable Entityguard when zero questions are successfully scored, preventing a clinically dangerous false-negative result from being persisted and returned. - Removed blank lines throughout the file (whitespace-only cleanup).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| if (scoredCount === 0) { | ||
| return new Response( | ||
| JSON.stringify({ error: "No questions could be scored. Check that question_id and answer_id values match the assessment." }), | ||
| { status: 422, headers: { "Content-Type": "application/json" } } | ||
| ); |
There was a problem hiding this comment.
The scoredCount === 0 guard only catches the case where all submitted questions fail to match. If even one question matches (e.g. 1 out of 10), the guard passes and a score based on a single matched question is silently accepted and stored as the final result. This can produce a clinically misleading score — for example, a score of 0 from 1 matched question while 9 questions were silently skipped.
Consider also checking whether scoredCount is significantly less than the number of submitted questions (e.g. scoredCount < answered_questions.questions.length) and returning a warning or error when only a fraction of questions could be scored.
| } | |
| if (scoredCount === 0) { | |
| return new Response( | |
| JSON.stringify({ error: "No questions could be scored. Check that question_id and answer_id values match the assessment." }), | |
| { status: 422, headers: { "Content-Type": "application/json" } } | |
| ); | |
| } | |
| const submittedCount = answered_questions.questions.length; | |
| if (scoredCount === 0) { | |
| return new Response( | |
| JSON.stringify({ error: "No questions could be scored. Check that question_id and answer_id values match the assessment." }), | |
| { status: 422, headers: { "Content-Type": "application/json" } } | |
| ); | |
| } else if (scoredCount < submittedCount) { | |
| return new Response( | |
| JSON.stringify({ | |
| error: "Only a subset of submitted questions could be scored. Check that question_id and answer_id values match the assessment.", | |
| scored_questions: scoredCount, | |
| submitted_questions: submittedCount, | |
| }), | |
| { status: 422, headers: { "Content-Type": "application/json" } } | |
| ); |
| if ( | ||
| !body || | ||
| !body.patient_id || | ||
| !body.assessment_id || | ||
| !Array.isArray(body.questions) || | ||
| body.questions.length === 0 | ||
| ) { | ||
| return new Response( | ||
| JSON.stringify({ error: "Missing or invalid request body. Required fields: patient_id, assessment_id, questions (non-empty array)." }), | ||
| { status: 400, headers: { "Content-Type": "application/json" } } | ||
| ); | ||
| } |
There was a problem hiding this comment.
The !body.patient_id and !body.assessment_id checks use JavaScript's truthiness, which does not distinguish between a missing field, null, false, 0, or an empty string "". Since these fields are expected to be non-empty UUID strings, using typeof body.patient_id !== 'string' || body.patient_id.trim() === '' (and similarly for assessment_id) would be a more explicit and robust validation. With the current check, a caller passing patient_id: "" gets a 400, which is correct, but the error message does not indicate which specific field is missing or invalid, making debugging harder for API consumers.
| if ( | |
| !body || | |
| !body.patient_id || | |
| !body.assessment_id || | |
| !Array.isArray(body.questions) || | |
| body.questions.length === 0 | |
| ) { | |
| return new Response( | |
| JSON.stringify({ error: "Missing or invalid request body. Required fields: patient_id, assessment_id, questions (non-empty array)." }), | |
| { status: 400, headers: { "Content-Type": "application/json" } } | |
| ); | |
| } | |
| if (!body || typeof body !== "object") { | |
| return new Response( | |
| JSON.stringify({ error: "Missing or invalid request body. Expected JSON object with fields: patient_id, assessment_id, questions (non-empty array)." }), | |
| { status: 400, headers: { "Content-Type": "application/json" } } | |
| ); | |
| } | |
| const invalidFields: string[] = []; | |
| if (typeof body.patient_id !== "string" || body.patient_id.trim() === "") { | |
| invalidFields.push("patient_id"); | |
| } | |
| if (typeof body.assessment_id !== "string" || body.assessment_id.trim() === "") { | |
| invalidFields.push("assessment_id"); | |
| } | |
| if (!Array.isArray(body.questions) || body.questions.length === 0) { | |
| invalidFields.push("questions (non-empty array)"); | |
| } | |
| if (invalidFields.length > 0) { | |
| return new Response( | |
| JSON.stringify({ error: `Missing or invalid fields: ${invalidFields.join(", ")}.` }), | |
| { status: 400, headers: { "Content-Type": "application/json" } } | |
| ); | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/functions/evaluate-assessments/index.ts`:
- Around line 25-31: The request body validation currently only checks that
body.questions is a non-empty array, but individual items can be null or missing
required fields and later crash when mapped; update the validation in index.ts
to ensure every item in body.questions is an object with required keys (e.g.,
question_id and answer_id) and valid types before proceeding (the same stricter
check should also be applied to the secondary validation block around the
mapping at the area referenced by the comment for lines 52-55); if any item
fails validation, return the existing 400/invalid request response rather than
allowing a runtime error during the questions.map handling.
Closes #171
📝 Description
The
evaluate-assessmentsEdge Function had two bugs:patient_id,assessment_id, orquestionscaused
questions.map()to crash withTypeError: Cannot read properties of undefined, returning an unhelpful 500 error.the assessment data, the scoring loop silently skipped every question,
totalScorestayed 0, and the patient was incorrectly told they are notautistic — a clinically dangerous false negative.
🔧 Changes Made
supabase/functions/evaluate-assessments/index.ts:req.json().catch(() => null)—returns
400 Bad Requestifpatient_id,assessment_id, orquestionsare missing or malformed before any processing begins.
scoredCounttracker in the scoring loop — returns422 Unprocessable Entityif 0 out of N questions were successfully scored, preventing asilent false negative result from being saved and returned to the patient.
📷 Screenshots or Visual Changes (if applicable)
N/A — Edge Function logic fix, no visual changes.
🤝 Collaboration
Collaborated with: N/A
✅ Checklist
Summary by CodeRabbit
New Features
Bug Fixes