-
Notifications
You must be signed in to change notification settings - Fork 35
feat: add Emotion Detector and fix tests #592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
4a66046
74f0fcb
f4ff508
e0ebf97
973c078
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,256 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import React, { useState, useRef, useCallback } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Camera, Image as ImageIcon, Loader2, ArrowLeft, RefreshCw, Smile } from 'lucide-react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { motion, AnimatePresence } from 'framer-motion'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import Webcam from 'react-webcam'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { detectorsApi } from '../api'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const EmotionDetector = ({ onBack }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [image, setImage] = useState(null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [previewUrl, setPreviewUrl] = useState(null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [isAnalyzing, setIsAnalyzing] = useState(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [result, setResult] = useState(null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [error, setError] = useState(null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [showWebcam, setShowWebcam] = useState(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const webcamRef = useRef(null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+16
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add null guard before accessing If Proposed fix const captureWebcam = useCallback(() => {
+ if (!webcamRef.current) return;
const imageSrc = webcamRef.current.getScreenshot();
if (imageSrc) {🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+17
to
+31
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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).
Check failure
Code scanning / CodeQL
DOM text reinterpreted as HTML High
DOM text
Copilot Autofix
AI 26 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 toReact.lazylike the other routes to avoid increasing the initial bundle size.