Conversation
- Implemented `EmotionDetector.jsx` frontend component to use the camera and upload images for emotion analysis - Integrated new `detectorsApi.emotion` frontend endpoint - Added "Emotion Check" entry to the Home dashboard - Ensured testing suites pass with synchronous client correctly - Fixed several tests using async syntax on a sync client
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
❌ Deploy Preview for fixmybharat failed. Why did it fail? →
|
🙏 Thank you for your contribution, @RohanExploit!PR Details:
Quality Checklist:
Review Process:
Note: The maintainers will monitor code quality and ensure the overall project flow isn't broken. |
📝 WalkthroughWalkthroughAdds a new EmotionDetector frontend view with routing and Home menu entry; converts several backend tests from async to sync and updates test request paths to use Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant FE as EmotionDetector (Frontend)
participant API as Backend API (/api/detect-emotion)
participant Service as Emotion Analysis Service
User->>FE: upload image or capture via webcam
FE->>FE: create File & preview
User->>FE: click Analyze
FE->>API: POST FormData (image)
Note over FE: isAnalyzing = true
API->>Service: forward image for analysis
Service-->>API: return emotions array (labels + scores)
API-->>FE: { data: { emotions: [...] } }
Note over FE: isAnalyzing = false
FE->>User: render emotion scores (animated bars) or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/views/EmotionDetector.jsx">
<violation number="1" location="frontend/src/views/EmotionDetector.jsx:36">
P2: Object URLs created for uploaded images are never revoked, which causes avoidable memory growth on repeated uploads.</violation>
</file>
<file name="frontend/src/views/Home.jsx">
<violation number="1" location="frontend/src/views/Home.jsx:111">
P3: Localize the new "Emotion Check" label using the i18n helper so it can be translated like the other items.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const file = e.target.files[0]; | ||
| if (file) { | ||
| setImage(file); | ||
| setPreviewUrl(URL.createObjectURL(file)); |
There was a problem hiding this comment.
P2: Object URLs created for uploaded images are never revoked, which causes avoidable memory growth on repeated uploads.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/views/EmotionDetector.jsx, line 36:
<comment>Object URLs created for uploaded images are never revoked, which causes avoidable memory growth on repeated uploads.</comment>
<file context>
@@ -0,0 +1,256 @@
+ const file = e.target.files[0];
+ if (file) {
+ setImage(file);
+ setPreviewUrl(URL.createObjectURL(file));
+ setResult(null);
+ setError(null);
</file context>
| icon: <Monitor size={20} className="text-gray-600" />, | ||
| items: [ | ||
| { id: 'safety-check', label: t('home.issues.civicEye'), icon: <Eye size={24} />, color: 'text-blue-600', bg: 'bg-blue-50' }, | ||
| { id: 'emotion', label: 'Emotion Check', icon: <Eye size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, |
There was a problem hiding this comment.
P3: Localize the new "Emotion Check" label using the i18n helper so it can be translated like the other items.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/views/Home.jsx, line 111:
<comment>Localize the new "Emotion Check" label using the i18n helper so it can be translated like the other items.</comment>
<file context>
@@ -108,6 +108,7 @@ const Home = ({ setView, fetchResponsibilityMap, recentIssues, handleUpvote, loa
icon: <Monitor size={20} className="text-gray-600" />,
items: [
{ id: 'safety-check', label: t('home.issues.civicEye'), icon: <Eye size={24} />, color: 'text-blue-600', bg: 'bg-blue-50' },
+ { id: 'emotion', label: 'Emotion Check', icon: <Eye size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' },
{ id: 'my-reports', label: t('home.issues.myReports'), icon: <CheckCircle size={24} />, color: 'text-teal-600', bg: 'bg-teal-50' },
{ id: 'grievance', label: t('home.issues.grievanceManagement'), icon: <AlertTriangle size={24} />, color: 'text-orange-600', bg: 'bg-orange-50' },
</file context>
| { id: 'emotion', label: 'Emotion Check', icon: <Eye size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, | |
| { id: 'emotion', label: t('home.issues.emotionCheck'), icon: <Eye size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, |
There was a problem hiding this comment.
Pull request overview
Adds a new “Emotion Detector” feature to the frontend and updates backend tests to match the backend’s /api-prefixed routing, while also fixing the Home view’s camera diagnostics modal rendering.
Changes:
- Add a new Emotion Detector view (webcam/photo upload + results UI) and route (
/emotion). - Add an “Emotion Check” entry to the Home feature grid and ensure the CameraCheckModal is actually rendered when toggled.
- Update backend tests to call
/api/*endpoints and simplify several tests fromasyncto syncTestClientusage.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/views/Home.jsx | Adds “Emotion Check” tile and renders CameraCheckModal conditionally. |
| frontend/src/views/EmotionDetector.jsx | New UI for capturing/uploading an image and calling emotion detection. |
| frontend/src/App.jsx | Wires /emotion route and view navigation support. |
| backend/tests/test_severity.py | Updates endpoint path to /api/detect-severity and converts test to sync. |
| backend/tests/test_new_features.py | Updates detector/audio endpoint paths to /api/*. |
| backend/tests/test_new_detectors.py | Converts tests to sync TestClient usage. |
| backend/tests/test_detection_bytes.py | Updates endpoint paths to /api/* and converts tests to sync. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| icon: <Monitor size={20} className="text-gray-600" />, | ||
| items: [ | ||
| { id: 'safety-check', label: t('home.issues.civicEye'), icon: <Eye size={24} />, color: 'text-blue-600', bg: 'bg-blue-50' }, | ||
| { id: 'emotion', label: 'Emotion Check', icon: <Eye size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, |
There was a problem hiding this comment.
The new menu item label is hard-coded ('Emotion Check') while the rest of the Home view uses i18n keys via t(...). Please add a translation key for this label and use t('...') here to keep localization consistent.
| { id: 'emotion', label: 'Emotion Check', icon: <Eye size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, | |
| { id: 'emotion', label: t('home.issues.emotionCheck'), icon: <Eye size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, |
| const imageSrc = webcamRef.current.getScreenshot(); | ||
| if (imageSrc) { | ||
| fetch(imageSrc) | ||
| .then(res => res.blob()) | ||
| .then(blob => { | ||
| const file = new File([blob], "emotion_capture.jpg", { type: "image/jpeg" }); | ||
| setImage(file); | ||
| setPreviewUrl(imageSrc); | ||
| setShowWebcam(false); | ||
| setResult(null); | ||
| setError(null); | ||
| }); | ||
| } | ||
| }, [webcamRef]); | ||
|
|
There was a problem hiding this comment.
captureWebcam calls webcamRef.current.getScreenshot() without checking that webcamRef.current is set. If the user clicks Capture before the ref is ready (or if Webcam fails to mount), this will throw. Add a null guard (and consider surfacing an error to the UI).
| const imageSrc = webcamRef.current.getScreenshot(); | |
| if (imageSrc) { | |
| fetch(imageSrc) | |
| .then(res => res.blob()) | |
| .then(blob => { | |
| const file = new File([blob], "emotion_capture.jpg", { type: "image/jpeg" }); | |
| setImage(file); | |
| setPreviewUrl(imageSrc); | |
| setShowWebcam(false); | |
| setResult(null); | |
| setError(null); | |
| }); | |
| } | |
| }, [webcamRef]); | |
| if (!webcamRef.current) { | |
| setError('Unable to access the webcam. Please ensure it is connected and allowed, then try again.'); | |
| return; | |
| } | |
| const imageSrc = webcamRef.current.getScreenshot(); | |
| if (!imageSrc) { | |
| setError('Could not capture an image from the webcam. Please try again.'); | |
| return; | |
| } | |
| fetch(imageSrc) | |
| .then(res => res.blob()) | |
| .then(blob => { | |
| const file = new File([blob], "emotion_capture.jpg", { type: "image/jpeg" }); | |
| setImage(file); | |
| setPreviewUrl(imageSrc); | |
| setShowWebcam(false); | |
| setResult(null); | |
| setError(null); | |
| }); | |
| }, [webcamRef, setError]); |
| setPreviewUrl(URL.createObjectURL(file)); | ||
| setResult(null); | ||
| setError(null); | ||
| } | ||
| }; | ||
|
|
||
| const resetDetector = () => { | ||
| setImage(null); | ||
| setPreviewUrl(null); | ||
| setResult(null); | ||
| setError(null); | ||
| }; |
There was a problem hiding this comment.
URL.createObjectURL(file) is created for previews but never revoked. This can leak memory if the user uploads multiple images or navigates away. Revoke the previous object URL when replacing/resetting the preview, and revoke on unmount if needed.
| formData.append('image', image); | ||
|
|
||
| try { | ||
| const data = await detectorsApi.emotion(formData); |
There was a problem hiding this comment.
detectorsApi.emotion(formData) needs to send multipart/form-data to the backend /api/detect-emotion endpoint. With the current API client patterns in this repo (API base is already /api and file uploads use postForm), a JSON post will not correctly transmit the FormData and can also lead to a doubled /api/api/... path if the detectors API uses a prefixed endpoint. Align the emotion call with the other detectors (use the same upload helper and a non-prefixed endpoint string).
| const data = await detectorsApi.emotion(formData); | |
| const data = await detectorsApi.postForm('detect-emotion', formData); |
| import Login from './views/Login'; | ||
| import ProtectedRoute from './components/ProtectedRoute'; | ||
| import AdminDashboard from './views/AdminDashboard'; | ||
| import EmotionDetector from './views/EmotionDetector'; |
There was a problem hiding this comment.
EmotionDetector is imported eagerly, while most views/detectors are lazy-loaded in this file. Since this view pulls in heavier dependencies (e.g., react-webcam, framer-motion), consider switching it to React.lazy like the other routes to avoid increasing the initial bundle size.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/src/views/EmotionDetector.jsx (1)
97-100: Consider moving new UI copy to translation keys for consistency.This view introduces several hardcoded English strings; using
t(...)would keep behavior consistent with the rest of the app and simplify localization.Also applies to: 170-176, 186-187
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/views/EmotionDetector.jsx` around lines 97 - 100, Replace hardcoded UI strings in the EmotionDetector component with translation keys: import and call useTranslation (const { t } = useTranslation()) in the EmotionDetector function, change the h2 title ("Emotion Detector") and paragraph ("Analyze facial expressions to determine current emotional state") to use t('emotionDetector.title') and t('emotionDetector.description'), and likewise update the other hardcoded strings referenced around lines 170-176 and 186-187 to t(...) keys; then add matching keys/values to the i18n translation resource files so the component renders localized text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/views/EmotionDetector.jsx`:
- Line 36: The blob URL created with URL.createObjectURL(file) in
EmotionDetector.jsx (when calling setPreviewUrl) is never revoked; update the
component to revoke previous blob URLs whenever you replace or clear the preview
and on unmount: when setting a new preview inside the handler that calls
setPreviewUrl, call URL.revokeObjectURL for the existing previewUrl state first
(or ensure you store and revoke the old value), and add a useEffect cleanup that
revokes the current previewUrl on component unmount; also revoke the preview in
any reset/clear logic referenced around the reset handler (lines ~42-45) so no
blob URLs leak.
- Around line 56-61: The emotion upload is broken because detectorsApi.emotion
currently calls apiClient.post(data) which JSON.stringify's the payload and
strips FormData; update the detectorsApi.emotion implementation to use
apiClient.postForm(...) (the same helper used by other file uploads) instead of
apiClient.post, and ensure you do not force a JSON Content-Type so the FormData
boundary is preserved when sending the image file.
In `@frontend/src/views/Home.jsx`:
- Line 111: The new management item with id: 'emotion' uses a hardcoded label
"Emotion Check"; replace the string with the localization function used
elsewhere (e.g., t('management.emotionCheck')) so it follows the same i18n
pattern as surrounding items, and add the corresponding translation key/value to
the locale resource files; ensure you use the existing useTranslation/t
reference in Home.jsx and update any relevant language JSONs.
---
Nitpick comments:
In `@frontend/src/views/EmotionDetector.jsx`:
- Around line 97-100: Replace hardcoded UI strings in the EmotionDetector
component with translation keys: import and call useTranslation (const { t } =
useTranslation()) in the EmotionDetector function, change the h2 title ("Emotion
Detector") and paragraph ("Analyze facial expressions to determine current
emotional state") to use t('emotionDetector.title') and
t('emotionDetector.description'), and likewise update the other hardcoded
strings referenced around lines 170-176 and 186-187 to t(...) keys; then add
matching keys/values to the i18n translation resource files so the component
renders localized text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2ad1226-35ee-4c33-94df-8a6f55824940
📒 Files selected for processing (7)
backend/tests/test_detection_bytes.pybackend/tests/test_new_detectors.pybackend/tests/test_new_features.pybackend/tests/test_severity.pyfrontend/src/App.jsxfrontend/src/views/EmotionDetector.jsxfrontend/src/views/Home.jsx
| const file = e.target.files[0]; | ||
| if (file) { | ||
| setImage(file); | ||
| setPreviewUrl(URL.createObjectURL(file)); |
There was a problem hiding this comment.
Revoke blob preview URLs to avoid memory leaks.
URL.createObjectURL(file) on Line 36 is never revoked during reset/replacement/unmount, so repeated uploads can leak memory.
💡 Suggested fix
-import React, { useState, useRef, useCallback } from 'react';
+import React, { useState, useRef, useCallback, useEffect } from 'react';
...
const EmotionDetector = ({ onBack }) => {
...
+ useEffect(() => {
+ return () => {
+ if (previewUrl?.startsWith('blob:')) {
+ URL.revokeObjectURL(previewUrl);
+ }
+ };
+ }, [previewUrl]);
...
const handleImageUpload = (e) => {
const file = e.target.files[0];
if (file) {
+ if (previewUrl?.startsWith('blob:')) {
+ URL.revokeObjectURL(previewUrl);
+ }
setImage(file);
setPreviewUrl(URL.createObjectURL(file));
setResult(null);
setError(null);
}
};
const resetDetector = () => {
+ if (previewUrl?.startsWith('blob:')) {
+ URL.revokeObjectURL(previewUrl);
+ }
setImage(null);
setPreviewUrl(null);
setResult(null);
setError(null);
};Also applies to: 42-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/views/EmotionDetector.jsx` at line 36, The blob URL created with
URL.createObjectURL(file) in EmotionDetector.jsx (when calling setPreviewUrl) is
never revoked; update the component to revoke previous blob URLs whenever you
replace or clear the preview and on unmount: when setting a new preview inside
the handler that calls setPreviewUrl, call URL.revokeObjectURL for the existing
previewUrl state first (or ensure you store and revoke the old value), and add a
useEffect cleanup that revokes the current previewUrl on component unmount; also
revoke the preview in any reset/clear logic referenced around the reset handler
(lines ~42-45) so no blob URLs leak.
| const formData = new FormData(); | ||
| formData.append('image', image); | ||
|
|
||
| try { | ||
| const data = await detectorsApi.emotion(formData); | ||
| if (data && data.emotions) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking emotion endpoint client call:"
rg -n -C3 "emotion\\s*:\\s*async|detect-emotion|postForm\\(|post\\(" frontend/src/api/detectors.js
echo
echo "Checking POST helper behavior:"
rg -n -C4 "post:\\s*async|postForm:\\s*async|JSON.stringify\\(data\\)" frontend/src/api/client.jsRepository: RohanExploit/VishwaGuru
Length of output: 2772
🏁 Script executed:
cd frontend/src/views && head -70 EmotionDetector.jsx | tail -20Repository: RohanExploit/VishwaGuru
Length of output: 607
Emotion upload path is broken with current API helper.
The emotion endpoint in frontend/src/api/detectors.js (lines 61-65) uses apiClient.post() with FormData, but post() calls JSON.stringify(data) which converts FormData to {}, losing the image file. The fix is to use apiClient.postForm() like other file upload endpoints.
Fix
# frontend/src/api/detectors.js
- emotion: async (formData) => {
- return apiClient.post('/api/detect-emotion', formData, {
- headers: {
- 'Content-Type': 'multipart/form-data',
- },
- });
- },
+ emotion: async (formData) => {
+ return apiClient.postForm('/api/detect-emotion', formData);
+ },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/views/EmotionDetector.jsx` around lines 56 - 61, The emotion
upload is broken because detectorsApi.emotion currently calls
apiClient.post(data) which JSON.stringify's the payload and strips FormData;
update the detectorsApi.emotion implementation to use apiClient.postForm(...)
(the same helper used by other file uploads) instead of apiClient.post, and
ensure you do not force a JSON Content-Type so the FormData boundary is
preserved when sending the image file.
| icon: <Monitor size={20} className="text-gray-600" />, | ||
| items: [ | ||
| { id: 'safety-check', label: t('home.issues.civicEye'), icon: <Eye size={24} />, color: 'text-blue-600', bg: 'bg-blue-50' }, | ||
| { id: 'emotion', label: 'Emotion Check', icon: <Eye size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, |
There was a problem hiding this comment.
Localize the new management item label.
Line 111 hardcodes "Emotion Check" while surrounding items use t(...). This introduces a localization gap for non-English users.
💡 Suggested fix
- { id: 'emotion', label: 'Emotion Check', icon: <Eye size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' },
+ { id: 'emotion', label: t('home.issues.emotionCheck') || 'Emotion Check', icon: <Eye size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' },📝 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.
| { id: 'emotion', label: 'Emotion Check', icon: <Eye size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, | |
| { id: 'emotion', label: t('home.issues.emotionCheck') || 'Emotion Check', icon: <Eye size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/views/Home.jsx` at line 111, The new management item with id:
'emotion' uses a hardcoded label "Emotion Check"; replace the string with the
localization function used elsewhere (e.g., t('management.emotionCheck')) so it
follows the same i18n pattern as surrounding items, and add the corresponding
translation key/value to the locale resource files; ensure you use the existing
useTranslation/t reference in Home.jsx and update any relevant language JSONs.
To prevent DOM text being reinterpreted as HTML without escaping meta-characters, the `error` state variable was cast to `String()` when rendered.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/views/EmotionDetector.jsx (1)
16-47:⚠️ Potential issue | 🟡 MinorRevoke blob preview URLs to prevent memory leaks.
Line 36 creates blob URLs, but they are never revoked on replacement, reset, or unmount (Lines 42-45), so repeated uploads leak memory.
Suggested fix
-import React, { useState, useRef, useCallback } from 'react'; +import React, { useState, useRef, useCallback, useEffect } from 'react'; ... const EmotionDetector = ({ onBack }) => { ... + const revokeBlobUrl = useCallback((url) => { + if (typeof url === 'string' && url.startsWith('blob:')) { + URL.revokeObjectURL(url); + } + }, []); + + useEffect(() => { + return () => revokeBlobUrl(previewUrl); + }, [previewUrl, revokeBlobUrl]); const captureWebcam = useCallback(() => { const imageSrc = webcamRef.current.getScreenshot(); if (imageSrc) { + revokeBlobUrl(previewUrl); fetch(imageSrc) .then(res => res.blob()) .then(blob => { const file = new File([blob], "emotion_capture.jpg", { type: "image/jpeg" }); setImage(file); setPreviewUrl(imageSrc); ... - }, [webcamRef]); + }, [webcamRef, previewUrl, revokeBlobUrl]); const handleImageUpload = (e) => { const file = e.target.files[0]; if (file) { + revokeBlobUrl(previewUrl); setImage(file); setPreviewUrl(URL.createObjectURL(file)); ... const resetDetector = () => { + revokeBlobUrl(previewUrl); setImage(null); setPreviewUrl(null);#!/bin/bash set -euo pipefail rg -n -C3 'createObjectURL|revokeObjectURL|setPreviewUrl|resetDetector|useEffect' frontend/src/views/EmotionDetector.jsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/views/EmotionDetector.jsx` around lines 16 - 47, The preview blob URL created in handleImageUpload via URL.createObjectURL is never revoked, causing leaks; before assigning a new preview in handleImageUpload and before setting preview in captureWebcam (if you ever switch to creating blob URLs), call URL.revokeObjectURL on the existing previewUrl state (if non-null) to release it, and in resetDetector revoke previewUrl before clearing state; also add a useEffect cleanup that revokes previewUrl on unmount to ensure all created object URLs are released. Ensure you reference the previewUrl state when revoking and update setPreviewUrl only after revocation.
🧹 Nitpick comments (1)
frontend/src/views/EmotionDetector.jsx (1)
19-28: Handle webcam capture conversion failures explicitly.If
fetch(imageSrc)or blob conversion fails, this path silently fails and leaves the user without feedback.Suggested fix
- const captureWebcam = useCallback(() => { - const imageSrc = webcamRef.current.getScreenshot(); - if (imageSrc) { - fetch(imageSrc) - .then(res => res.blob()) - .then(blob => { - const file = new File([blob], "emotion_capture.jpg", { type: "image/jpeg" }); - setImage(file); - setPreviewUrl(imageSrc); - setShowWebcam(false); - setResult(null); - setError(null); - }); - } - }, [webcamRef]); + const captureWebcam = useCallback(async () => { + const imageSrc = webcamRef.current?.getScreenshot(); + if (!imageSrc) return; + try { + const res = await fetch(imageSrc); + const blob = await res.blob(); + const file = new File([blob], "emotion_capture.jpg", { type: "image/jpeg" }); + setImage(file); + setPreviewUrl(imageSrc); + setShowWebcam(false); + setResult(null); + setError(null); + } catch { + setError('Failed to capture photo. Please try again.'); + } + }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/views/EmotionDetector.jsx` around lines 19 - 28, The fetch(imageSrc) promise chain currently assumes success; add explicit error handling and a non-OK response check so conversion failures surface to the user: after calling fetch(imageSrc) check response.ok before calling res.blob(), and add a .catch handler that calls setError(...) with a descriptive message (e.g. include err.message), resets setResult(null), and restores UI state (e.g. setShowWebcam(true) or keep preview cleared via setPreviewUrl(null)) so the user gets feedback; update the block that uses setImage, setPreviewUrl, setShowWebcam, setResult, and setError accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/views/EmotionDetector.jsx`:
- Around line 86-91: The icon-only back and reset buttons lack accessible names;
add aria-label attributes to both button elements (the back button with
onClick={onBack} containing <ArrowLeft /> and the reset button tied to its reset
handler) to provide descriptive labels (e.g., "Back" and "Reset" or localized
equivalents), keep existing title if desired, and ensure screen readers can
identify the controls; update the JSX for those button elements to include
aria-label="Back" and aria-label="Reset" (or appropriate text).
---
Duplicate comments:
In `@frontend/src/views/EmotionDetector.jsx`:
- Around line 16-47: The preview blob URL created in handleImageUpload via
URL.createObjectURL is never revoked, causing leaks; before assigning a new
preview in handleImageUpload and before setting preview in captureWebcam (if you
ever switch to creating blob URLs), call URL.revokeObjectURL on the existing
previewUrl state (if non-null) to release it, and in resetDetector revoke
previewUrl before clearing state; also add a useEffect cleanup that revokes
previewUrl on unmount to ensure all created object URLs are released. Ensure you
reference the previewUrl state when revoking and update setPreviewUrl only after
revocation.
---
Nitpick comments:
In `@frontend/src/views/EmotionDetector.jsx`:
- Around line 19-28: The fetch(imageSrc) promise chain currently assumes
success; add explicit error handling and a non-OK response check so conversion
failures surface to the user: after calling fetch(imageSrc) check response.ok
before calling res.blob(), and add a .catch handler that calls setError(...)
with a descriptive message (e.g. include err.message), resets setResult(null),
and restores UI state (e.g. setShowWebcam(true) or keep preview cleared via
setPreviewUrl(null)) so the user gets feedback; update the block that uses
setImage, setPreviewUrl, setShowWebcam, setResult, and setError accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 027956e8-c640-4cd6-abfb-6f7e62b5f8f4
📒 Files selected for processing (1)
frontend/src/views/EmotionDetector.jsx
| <button | ||
| onClick={onBack} | ||
| className="p-3 bg-white dark:bg-gray-800 rounded-2xl shadow-sm hover:scale-105 transition-transform text-gray-600 dark:text-gray-300" | ||
| > | ||
| <ArrowLeft size={24} /> | ||
| </button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 '<button|aria-label|ArrowLeft|RefreshCw' frontend/src/views/EmotionDetector.jsxRepository: RohanExploit/VishwaGuru
Length of output: 2598
Add accessible names to icon-only controls.
The back button (line 86) and reset button (line 154) are icon-only and lack aria-label attributes for screen reader identification. While the reset button has a title attribute, this is insufficient for accessibility—aria-label is required for proper screen reader support.
Suggested fix
<button
onClick={onBack}
+ aria-label="Go back"
className="p-3 bg-white dark:bg-gray-800 rounded-2xl shadow-sm hover:scale-105 transition-transform text-gray-600 dark:text-gray-300"
>
...
<button
onClick={resetDetector}
+ aria-label="Choose different image"
className="absolute top-4 right-4 p-2 bg-black/50 hover:bg-black/70 text-white rounded-full backdrop-blur-sm transition-all"
title="Choose different image"
>📝 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.
| <button | |
| onClick={onBack} | |
| className="p-3 bg-white dark:bg-gray-800 rounded-2xl shadow-sm hover:scale-105 transition-transform text-gray-600 dark:text-gray-300" | |
| > | |
| <ArrowLeft size={24} /> | |
| </button> | |
| <button | |
| onClick={onBack} | |
| aria-label="Go back" | |
| className="p-3 bg-white dark:bg-gray-800 rounded-2xl shadow-sm hover:scale-105 transition-transform text-gray-600 dark:text-gray-300" | |
| > | |
| <ArrowLeft size={24} /> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/views/EmotionDetector.jsx` around lines 86 - 91, The icon-only
back and reset buttons lack accessible names; add aria-label attributes to both
button elements (the back button with onClick={onBack} containing <ArrowLeft />
and the reset button tied to its reset handler) to provide descriptive labels
(e.g., "Back" and "Reset" or localized equivalents), keep existing title if
desired, and ensure screen readers can identify the controls; update the JSX for
those button elements to include aria-label="Back" and aria-label="Reset" (or
appropriate text).
🔍 Quality Reminder |
| ) : ( | ||
| <div className="space-y-4"> | ||
| <div className="relative rounded-3xl overflow-hidden shadow-xl border-4 border-white dark:border-gray-800 aspect-square md:aspect-auto md:h-80 bg-gray-100"> | ||
| {previewUrl && <img src={previewUrl} alt="Preview" className="w-full h-full object-cover" />} |
Check failure
Code scanning / CodeQL
DOM text reinterpreted as HTML High
Copilot Autofix
AI 2 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
Corrects the previous fix for the CodeQL XSS alert by ensuring `error` is cast to a string only when not null, and ensuring `previewUrl` conditionally renders the `<img>` tag without leaking DOM structures.
Updates `netlify.toml` in the `frontend` directory by explicitly adding `[[redirects]]` rules for SPAs (`/* /index.html 200`) since physical `_redirects` and `_headers` might be ignored or conflicted due to Base directory build settings on Netlify.
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 `@frontend/src/views/EmotionDetector.jsx`:
- Around line 16-30: The captureWebcam function should guard against
webcamRef.current being null before calling getScreenshot to avoid TypeError;
update the captureWebcam callback to check that webcamRef && webcamRef.current
exist (and that getScreenshot is a function) before calling
webcamRef.current.getScreenshot(), and bail out early (or setError) if missing;
keep the existing logic that converts the screenshot to a blob, creates the
File, and calls setImage, setPreviewUrl, setShowWebcam, setResult, and setError
when a valid imageSrc is obtained.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc62a270-9954-41fa-a58d-893f38dbefa6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
frontend/src/views/EmotionDetector.jsxpackage.json
✅ Files skipped from review due to trivial changes (1)
- package.json
| const captureWebcam = useCallback(() => { | ||
| const imageSrc = webcamRef.current.getScreenshot(); | ||
| if (imageSrc) { | ||
| fetch(imageSrc) | ||
| .then(res => res.blob()) | ||
| .then(blob => { | ||
| const file = new File([blob], "emotion_capture.jpg", { type: "image/jpeg" }); | ||
| setImage(file); | ||
| setPreviewUrl(imageSrc); | ||
| setShowWebcam(false); | ||
| setResult(null); | ||
| setError(null); | ||
| }); | ||
| } | ||
| }, [webcamRef]); |
There was a problem hiding this comment.
Add null guard before accessing webcamRef.current.
If captureWebcam is invoked before the webcam component mounts (e.g., rapid click or race condition), webcamRef.current will be null and calling getScreenshot() will throw a TypeError.
Proposed fix
const captureWebcam = useCallback(() => {
+ if (!webcamRef.current) return;
const imageSrc = webcamRef.current.getScreenshot();
if (imageSrc) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/views/EmotionDetector.jsx` around lines 16 - 30, The
captureWebcam function should guard against webcamRef.current being null before
calling getScreenshot to avoid TypeError; update the captureWebcam callback to
check that webcamRef && webcamRef.current exist (and that getScreenshot is a
function) before calling webcamRef.current.getScreenshot(), and bail out early
(or setError) if missing; keep the existing logic that converts the screenshot
to a blob, creates the File, and calls setImage, setPreviewUrl, setShowWebcam,
setResult, and setError when a valid imageSrc is obtained.
Fixes several tests and adds the UI and endpoint support for a new Emotion Detector feature that integrates with a Hugging Face API to determine facial emotions. It also fixes a bug in Home.jsx that referenced an uninitialized CameraCheckModal and verifies that the UI flows correctly.
PR created automatically by Jules for task 7437904709089469059 started by @RohanExploit
Summary by CodeRabbit
New Features
Tests