Conversation
There was a problem hiding this comment.
Pull request overview
This pull request includes bug fixes and enhancements to improve system reliability and user experience. The changes address an assessment scoring bug, add a DynamoDB placeholder mechanism to prevent race conditions, and implement polling timeout logic for the chatbot interface.
- Fixed assessment answer validation by removing incorrect modulo calculation
- Added DynamoDB placeholder record creation to prevent 404 errors during async chatbot processing
- Implemented polling timeout mechanism with 3-minute limit for chatbot responses
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| services/course-service/src/controllers/course_controller.py | Fixed bug in assessment answer validation by directly using db_answer instead of applying modulo operation |
| services/course-service/src/controllers/chatbot_controller.py | Added immediate DynamoDB placeholder record creation to prevent 404 errors while Lambda processes the request asynchronously |
| frontend/src/components/user/chatbot/LessonChatbot.tsx | Implemented polling timeout logic with pollStartTime state to handle long-running chatbot requests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const pollAnswer = async () => { | ||
| try { | ||
| // Check timeout | ||
| if (pollStartTime && Date.now() - pollStartTime > POLLING_TIMEOUT) { |
There was a problem hiding this comment.
The pollStartTime is initialized to null but never set to Date.now() when polling actually starts. This means the timeout check on line 82 will never trigger because pollStartTime remains null. You should set setPollStartTime(Date.now()) when polling begins, such as right after calling setPendingChatId in the handleSendMessage function (around line 182).
|
|
||
| return () => { | ||
| if (pollingIntervalRef.current) { | ||
| setPollStartTime(Date.now()); // Start timeout timer |
There was a problem hiding this comment.
The pollStartTime is being set inside the cleanup function, which runs when the effect is torn down (when pendingChatId changes or component unmounts). This is backwards - the timeout timer should be started when polling begins, not when it ends. The setPollStartTime call should be moved to line 143 (after pollAnswer is called initially) to properly track when polling started.
|
|
||
| return () => { | ||
| if (pollingIntervalRef.current) { | ||
| setPollStartTime(Date.now()); // Start timeout timer |
There was a problem hiding this comment.
The indentation of this line is incorrect. It should be aligned with the clearInterval call on line 148, not nested inside the if block condition.
| setPollStartTime(Date.now()); // Start timeout timer | |
| setPollStartTime(Date.now()); // Start timeout timer |
No description provided.