Add Western-style detectors (Graffiti, Traffic Sign, Abandoned Vehicle) to UI#472
Add Western-style detectors (Graffiti, Traffic Sign, Abandoned Vehicle) to UI#472RohanExploit wants to merge 4 commits intomainfrom
Conversation
|
👋 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. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 12 files
Prompt for AI agents (all 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/Home.jsx">
<violation number="1" location="frontend/src/views/Home.jsx:103">
P2: The new graffiti item hardcodes an English label instead of using i18n like the other items, so it won’t be localized.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| { id: 'crowd', label: t('home.issues.crowd'), icon: <Users size={24} />, color: 'text-red-500', bg: 'bg-red-50' }, | ||
| { id: 'water-leak', label: t('home.issues.waterLeak'), icon: <Waves size={24} />, color: 'text-blue-500', bg: 'bg-blue-50' }, | ||
| { id: 'report', label: t('home.issues.waste'), icon: <Recycle size={24} />, color: 'text-emerald-600', bg: 'bg-emerald-50' }, | ||
| { id: 'graffiti', label: 'Graffiti', icon: <Brush size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, |
There was a problem hiding this comment.
P2: The new graffiti item hardcodes an English label instead of using i18n like the other items, so it won’t be localized.
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 103:
<comment>The new graffiti item hardcodes an English label instead of using i18n like the other items, so it won’t be localized.</comment>
<file context>
@@ -97,9 +97,10 @@ const Home = ({ setView, fetchResponsibilityMap, recentIssues, handleUpvote, loa
+ { id: 'crowd', label: t('home.issues.crowd'), icon: <Users size={24} />, color: 'text-red-500', bg: 'bg-red-50' },
+ { id: 'water-leak', label: t('home.issues.waterLeak'), icon: <Waves size={24} />, color: 'text-blue-500', bg: 'bg-blue-50' },
{ id: 'report', label: t('home.issues.waste'), icon: <Recycle size={24} />, color: 'text-emerald-600', bg: 'bg-emerald-50' },
+ { id: 'graffiti', label: 'Graffiti', icon: <Brush size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' },
]
},
</file context>
| { id: 'graffiti', label: 'Graffiti', icon: <Brush size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, | |
| { id: 'graffiti', label: t('home.issues.graffiti'), icon: <Brush size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, |
There was a problem hiding this comment.
Pull request overview
This PR adds three new Western-style civic issue detectors (Graffiti, Traffic Sign, and Abandoned Vehicle) along with support for Water Leak and Crowd detection. It introduces a new BaseDetector component pattern for simpler static image detection, updates the Home page UI with new issue categories, and fixes import paths in the unified detection service.
Changes:
- Added new
BaseDetectorcomponent for reusable static image detection UI - Created three new detector components using BaseDetector (Traffic Sign, Abandoned Vehicle, Graffiti)
- Fixed duplicate
id: 'report'entries in Home.jsx issue categories with proper unique IDs - Updated backend to support HF token configuration and fixed import paths from
hf_servicetobackend.hf_api_service - Deleted temporary verification scripts that are no longer needed
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| verification/verify_ui_buttons.py | Deleted temporary verification script |
| verification/verify_detection_import.py | Deleted temporary verification script |
| frontend/src/views/Home.jsx | Fixed duplicate issue IDs and added graffiti category |
| frontend/src/components/BaseDetector.jsx | New reusable component for static image detectors |
| frontend/src/api/detectors.js | Added API client functions for new detectors |
| frontend/src/TrafficSignDetector.jsx | New Traffic Sign detector using BaseDetector |
| frontend/src/GraffitiDetector.jsx | New Graffiti detector using BaseDetector |
| frontend/src/App.jsx | Added routing for new detector views |
| frontend/src/AbandonedVehicleDetector.jsx | New Abandoned Vehicle detector using BaseDetector |
| backend/unified_detection_service.py | Fixed import paths from hf_service to backend.hf_api_service |
| backend/hf_api_service.py | Added detection functions and integrated HF token from config |
| backend/config.py | Added HF token configuration support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { id: 'crowd', label: t('home.issues.crowd'), icon: <Users size={24} />, color: 'text-red-500', bg: 'bg-red-50' }, | ||
| { id: 'water-leak', label: t('home.issues.waterLeak'), icon: <Waves size={24} />, color: 'text-blue-500', bg: 'bg-blue-50' }, | ||
| { id: 'report', label: t('home.issues.waste'), icon: <Recycle size={24} />, color: 'text-emerald-600', bg: 'bg-emerald-50' }, | ||
| { id: 'graffiti', label: 'Graffiti', icon: <Brush size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, |
There was a problem hiding this comment.
The label for the Graffiti button is hardcoded as 'Graffiti' instead of using the translation key like other buttons. This breaks internationalization. It should use t('home.issues.graffiti') to be consistent with other issue buttons in the same list and support multiple languages.
| { id: 'graffiti', label: 'Graffiti', icon: <Brush size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, | |
| { id: 'graffiti', label: t('home.issues.graffiti'), icon: <Brush size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, |
| async def detect_vandalism_clip(image: Union[Image.Image, bytes], client: httpx.AsyncClient = None): | ||
| """ | ||
| Detects vandalism/graffiti. | ||
| """ | ||
| labels = ["graffiti", "vandalism", "spray paint", "street art", "clean wall", "public property", "normal street"] | ||
| targets = ["graffiti", "vandalism", "spray paint"] | ||
| return await _detect_clip_generic(image, labels, targets, client) | ||
|
|
||
| async def detect_infrastructure_clip(image: Union[Image.Image, bytes], client: httpx.AsyncClient = None): | ||
| """ | ||
| Detects general infrastructure damage. | ||
| """ | ||
| labels = ["broken streetlight", "damaged traffic sign", "fallen tree", "damaged fence", "pothole", "clean street", "normal infrastructure"] | ||
| targets = ["broken streetlight", "damaged traffic sign", "fallen tree", "damaged fence"] | ||
| return await _detect_clip_generic(image, labels, targets, client) | ||
|
|
||
| async def detect_flooding_clip(image: Union[Image.Image, bytes], client: httpx.AsyncClient = None): | ||
| """ | ||
| Detects flooding/waterlogging (outdoor). | ||
| """ | ||
| labels = ["flooded street", "waterlogging", "blocked drain", "heavy rain", "dry street", "normal road"] | ||
| targets = ["flooded street", "waterlogging", "blocked drain", "heavy rain"] | ||
| return await _detect_clip_generic(image, labels, targets, client) |
There was a problem hiding this comment.
These three detection functions (detect_vandalism_clip, detect_infrastructure_clip, detect_flooding_clip) are added to support the unified_detection_service but appear to duplicate existing functionality. The detect_vandalism_clip function seems to overlap with the existing detect_graffiti_art_clip function (line 437-443). Similarly, infrastructure and flooding detection functions may duplicate existing endpoints. Consider whether these are necessary or if the existing functions could be reused instead.
| const capture = useCallback(() => { | ||
| const imageSrc = webcamRef.current.getScreenshot(); | ||
| setImgSrc(imageSrc); | ||
| setError(null); | ||
| }, [webcamRef]); |
There was a problem hiding this comment.
The capture function calls webcamRef.current.getScreenshot() without checking if webcamRef.current is not null. If the webcam fails to initialize or is not ready, this could result in a runtime error. Consider adding a null check before calling getScreenshot(), e.g., if (!webcamRef.current) return; at the beginning of the function.
📝 WalkthroughWalkthroughAdds optional Hugging Face token support to backend config, wires it into HF-based detectors, introduces three new zero-shot CLIP detectors, adds five new frontend detector components plus a shared BaseDetector camera UI, updates routes and API endpoints, and removes two verification scripts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Frontend as Frontend (Browser)
participant Backend as Backend API
participant HF as Hugging Face API
Frontend->>Backend: POST /api/detect-<type> with image/form-data
Backend->>Backend: config.get_hf_token() -> token?
alt token present
Backend->>HF: POST inference request (Authorization: Bearer <token>)
else no token
Backend->>HF: POST inference request (no Authorization header)
end
HF-->>Backend: inference results (labels/confidences)
Backend-->>Frontend: normalized detection response (labels, confidence, targets)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
frontend/src/components/BaseDetector.jsx (1)
113-125: Prefer a stable key over array index for detection items.
key={idx}is fragile if detections are ever sorted, filtered, or partially updated. Most detection shapes include alabelfield; using a combination of label and index is more stable.♻️ Proposed change
- {detections.map((det, idx) => ( - <div key={idx} className="bg-white ..."> + {detections.map((det, idx) => ( + <div key={`${det.label ?? det.waste_type ?? 'det'}-${idx}`} className="bg-white ...">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/BaseDetector.jsx` around lines 113 - 125, The map rendering in BaseDetector.jsx uses key={idx} which is fragile; update the key in the detections.map(...) callback to use a stable identifier from each detection (e.g., det.id if present) or a composite like `${det.label || det.waste_type}-${idx}` to reduce reordering bugs; modify the JSX where detections.map((det, idx) => ...) and replace key={idx} accordingly while keeping the rest of the rendering code unchanged.frontend/src/App.jsx (1)
77-77: ExtractvalidViewsto a module-level constant.The allowlist now has 30+ entries and must be kept in sync with the
<Route>definitions below. Inlining it insideuseCallbackmakes it easy to miss when adding future routes.♻️ Proposed change
Above
AppContent, define once:+const VALID_VIEWS = new Set([ + 'home', 'map', 'report', 'action', 'mh-rep', 'pothole', 'garbage', + 'vandalism', 'flood', 'infrastructure', 'parking', 'streetlight', 'fire', + 'animal', 'blocked', 'tree', 'pest', 'smart-scan', 'grievance-analysis', + 'noise', 'safety-check', 'insight', 'my-reports', 'grievance', 'login', + 'signup', 'traffic-sign', 'abandoned-vehicle', 'graffiti', 'water-leak', 'crowd', +]);Then in the callback:
const navigateToView = useCallback((view) => { - const validViews = ['home', 'map', /* ... long array ... */ 'crowd']; - if (validViews.includes(view)) { + if (VALID_VIEWS.has(view)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/App.jsx` at line 77, Move the inlined allowlist out of the useCallback by extracting the const validViews array to a module-level constant defined above the AppContent component; update the callback that currently defines validViews inline to reference this new top-level validViews constant (preserve the same values/order) so route additions are maintained in one place and stay in sync with the <Route> definitions.
🤖 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/config.py`:
- Around line 284-286: get_hf_token() currently calls get_config() which may
abort the process if unrelated required keys are missing; change get_hf_token to
avoid invoking get_config() and instead fetch the Hugging Face token directly
(e.g., read from environment variables or a lightweight config accessor) so
HF-only code paths don't trigger full-app validation/exit—update the function
get_hf_token() to return the HF token from os.environ (or a safe partial config
read) and ensure it never calls get_config() or any routine that can sys.exit()
on missing keys.
In `@backend/hf_api_service.py`:
- Around line 13-14: The module currently captures get_hf_token() into
module-level variables `token` and `headers`, which can become stale; change
auth header construction to be lazy by adding/using a helper `_auth_headers()`
that calls `get_hf_token()` and returns {"Authorization": f"Bearer {token}"} or
{} per request, then replace all uses of the module-level `headers` and
`headers_bin` with calls to `_auth_headers()` (including in binary upload paths)
so each request computes fresh auth headers.
In `@frontend/src/components/BaseDetector.jsx`:
- Around line 46-53: The last catch-all branch in BaseDetector.jsx that sets
results = [data] will treat any non-empty object (including API error payloads)
as a detection; remove or tighten that branch so only known detection shapes are
accepted (e.g., objects with keys like "detections", "label",
"bbox"/"confidence"/"class" or whatever your detection schema uses). Instead,
add explicit guards to ignore objects that look like errors (e.g., contain
"error" or "message") and route them to error handling/logging; keep the earlier
branches that handle Array, data.detections, and single-label data and ensure
the variable results is only populated when the object matches a valid detection
schema.
- Around line 80-99: When onUserMediaError runs it only calls setError, leaving
the <Webcam> (and its webcamRef) mounted so the Capture button can call
getScreenshot() on a broken stream; add a boolean cameraError state (e.g., const
[cameraError, setCameraError] = useState(false)) and setCameraError(true) inside
the onUserMediaError handler, then use cameraError to (1) prevent rendering the
<Webcam> component and instead show a fallback UI/message, and (2) disable or
hide the Capture button/handler that calls webcamRef.current.getScreenshot() so
getScreenshot() is never invoked when cameraError is true. Ensure any existing
setError calls remain to show the error message.
- Around line 19-23: The capture function should guard against a null ref and a
null screenshot: inside capture (the function using webcamRef, setImgSrc,
setError) first verify webcamRef.current exists and then call getScreenshot();
if getScreenshot() returns null call setError with a descriptive message and do
not call setImgSrc, otherwise call setImgSrc with the non-null image and clear
any previous error via setError(null); also remove webcamRef from the
useCallback dependency array since the ref is stable.
In `@frontend/src/views/Home.jsx`:
- Line 103: The Graffiti menu item currently hardcodes its label; update it to
use the app's translation helper the neighboring items use (e.g., t('...') or
i18n.t('...')) instead of the literal 'Graffiti' so it participates in
localization. Locate the menu entry with id 'graffiti' in Home.jsx and replace
the label with a translated key (for example t('nav.graffiti') or
i18n.t('nav.graffiti')), and add the corresponding 'nav.graffiti' string to your
locale resource files.
---
Nitpick comments:
In `@frontend/src/App.jsx`:
- Line 77: Move the inlined allowlist out of the useCallback by extracting the
const validViews array to a module-level constant defined above the AppContent
component; update the callback that currently defines validViews inline to
reference this new top-level validViews constant (preserve the same
values/order) so route additions are maintained in one place and stay in sync
with the <Route> definitions.
In `@frontend/src/components/BaseDetector.jsx`:
- Around line 113-125: The map rendering in BaseDetector.jsx uses key={idx}
which is fragile; update the key in the detections.map(...) callback to use a
stable identifier from each detection (e.g., det.id if present) or a composite
like `${det.label || det.waste_type}-${idx}` to reduce reordering bugs; modify
the JSX where detections.map((det, idx) => ...) and replace key={idx}
accordingly while keeping the rest of the rendering code unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
backend/config.pybackend/hf_api_service.pybackend/unified_detection_service.pyfrontend/src/AbandonedVehicleDetector.jsxfrontend/src/App.jsxfrontend/src/GraffitiDetector.jsxfrontend/src/TrafficSignDetector.jsxfrontend/src/api/detectors.jsfrontend/src/components/BaseDetector.jsxfrontend/src/views/Home.jsxverification/verify_detection_import.pyverification/verify_ui_buttons.py
💤 Files with no reviewable changes (2)
- verification/verify_detection_import.py
- verification/verify_ui_buttons.py
| def get_hf_token() -> Optional[str]: | ||
| """Get Hugging Face token from config.""" | ||
| return get_config().hf_token |
There was a problem hiding this comment.
Avoid forcing full app config load when only HF token is needed.
get_hf_token() calls get_config(), which can exit the process if unrelated required keys are missing. That makes HF-only code paths brittle.
Suggested fix
def get_hf_token() -> Optional[str]:
"""Get Hugging Face token from config."""
- return get_config().hf_token
+ global _config
+ if _config is not None:
+ return _config.hf_token
+ # Allow HF token lookup without forcing full Config bootstrap
+ return os.getenv("HF_TOKEN")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/config.py` around lines 284 - 286, get_hf_token() currently calls
get_config() which may abort the process if unrelated required keys are missing;
change get_hf_token to avoid invoking get_config() and instead fetch the Hugging
Face token directly (e.g., read from environment variables or a lightweight
config accessor) so HF-only code paths don't trigger full-app
validation/exit—update the function get_hf_token() to return the HF token from
os.environ (or a safe partial config read) and ensure it never calls
get_config() or any routine that can sys.exit() on missing keys.
| token = get_hf_token() | ||
| headers = {"Authorization": f"Bearer {token}"} if token else {} |
There was a problem hiding this comment.
Compute auth headers at request time, not module import time.
Capturing token/headers once at import can become stale and couples request auth to import order. Build headers lazily per call.
Suggested fix
-token = get_hf_token()
-headers = {"Authorization": f"Bearer {token}"} if token else {}
+def _auth_headers() -> Dict[str, str]:
+ token = get_hf_token()
+ return {"Authorization": f"Bearer {token}"} if token else {}
@@
- response = await client.post(url, headers=headers, json=payload, timeout=20.0)
+ response = await client.post(url, headers=_auth_headers(), json=payload, timeout=20.0)Apply the same _auth_headers() helper in the binary upload paths currently using headers_bin.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/hf_api_service.py` around lines 13 - 14, The module currently
captures get_hf_token() into module-level variables `token` and `headers`, which
can become stale; change auth header construction to be lazy by adding/using a
helper `_auth_headers()` that calls `get_hf_token()` and returns
{"Authorization": f"Bearer {token}"} or {} per request, then replace all uses of
the module-level `headers` and `headers_bin` with calls to `_auth_headers()`
(including in binary upload paths) so each request computes fresh auth headers.
| const capture = useCallback(() => { | ||
| const imageSrc = webcamRef.current.getScreenshot(); | ||
| setImgSrc(imageSrc); | ||
| setError(null); | ||
| }, [webcamRef]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and inspect the BaseDetector.jsx file
find . -name "BaseDetector.jsx" -type fRepository: RohanExploit/VishwaGuru
Length of output: 109
🌐 Web query:
react-webcam getScreenshot returns null documentation
💡 Result:
react-webcam’s getScreenshot() is typed/implemented to return string | null—so null is an expected outcome when a screenshot can’t be produced yet. The main documented/observable reasons are:
- No camera stream yet (or permission denied): the method returns
nullif the component doesn’t have user media (hasUserMediais false). [1] - Video not “ready” (no frame size yet): internally it also returns
nullif the underlying<video>has novideoHeightyet (common if you call it immediately on mount). [1] - Insecure context / camera unavailable:
getUserMedia()(which the component relies on) only works in secure contexts (HTTPS) and can fail for various reasons, preventing any frame from being available. [2]
What to do in practice:
- Call
getScreenshot()only after the webcam is actually streaming (e.g., afteronUserMediafires, or after the video has loaded metadata), and ensure the<Webcam />is rendered and yourref.currentis non-null. [3]
Sources:
[1] getScreenshot() / getCanvas() logic showing return null when !hasUserMedia or !video.videoHeight (react-webcam source snippet). (gist.github.com)
[2] MDN: MediaDevices.getUserMedia() requires secure context (HTTPS) and can fail, preventing a stream. (developer.mozilla.org)
[3] react-webcam docs: getScreenshot returns a base64 string (implying it can be absent if capture isn’t possible yet). (npmjs.com)
Citations:
- 1: https://gist.github.com/32892f47751a39aeb2f98a763f293a30?utm_source=openai
- 2: https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getUserMedia.?utm_source=openai
- 3: https://www.npmjs.com/package/react-webcam/v/5.0.1?activeTab=versions&utm_source=openai
🏁 Script executed:
cat -n ./frontend/src/components/BaseDetector.jsx | head -50Repository: RohanExploit/VishwaGuru
Length of output: 2136
🏁 Script executed:
cat -n ./frontend/src/components/BaseDetector.jsx | tail -n +50 | head -100Repository: RohanExploit/VishwaGuru
Length of output: 5425
Guard against null webcam ref and null screenshot before calling setImgSrc.
webcamRef.current can be null if the Webcam component unmounts unexpectedly. More critically, react-webcam's getScreenshot() returns null when the video stream isn't fully ready (e.g., still negotiating permissions, frame not yet captured), which is common even after the component mounts. Calling setImgSrc(null) on line 21 causes silent failure—the UI stays on the live-camera view with no error feedback or visual change, leaving the user confused about whether the capture worked.
Fix by checking both the ref and the returned screenshot:
Suggested fix
const capture = useCallback(() => {
+ if (!webcamRef.current) return;
const imageSrc = webcamRef.current.getScreenshot();
+ if (!imageSrc) {
+ setError("Could not capture image. Please ensure the camera is ready.");
+ return;
+ }
setImgSrc(imageSrc);
setError(null);
- }, [webcamRef]);
+ }, []);Also remove webcamRef from the dependency array—it's a stable ref and doesn't need to be included.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/BaseDetector.jsx` around lines 19 - 23, The capture
function should guard against a null ref and a null screenshot: inside capture
(the function using webcamRef, setImgSrc, setError) first verify
webcamRef.current exists and then call getScreenshot(); if getScreenshot()
returns null call setError with a descriptive message and do not call setImgSrc,
otherwise call setImgSrc with the non-null image and clear any previous error
via setError(null); also remove webcamRef from the useCallback dependency array
since the ref is stable.
| let results = []; | ||
| if (Array.isArray(data)) results = data; | ||
| else if (data && data.detections) results = data.detections; | ||
| else if (data && data.label) results = [data]; // Single result like waste | ||
| else if (data && typeof data === 'object' && Object.keys(data).length > 0) { | ||
| // Maybe it's a dict of results? | ||
| results = [data]; | ||
| } |
There was a problem hiding this comment.
Catch-all normalization branch will silently display API error objects as detections.
The else if (data && typeof data === 'object' && Object.keys(data).length > 0) branch wraps any non-empty object — including error payloads like { message: "Unauthorized" } or { error: "Model loading" } — as a detection result. This renders in the results panel with label undefined → shown as "Detected" at 0% confidence, giving users a false positive while hiding the real failure.
🛡️ Proposed fix
Drop the catch-all or tighten the guard to only accept objects that carry a known detection field:
let results = [];
if (Array.isArray(data)) results = data;
else if (data && data.detections) results = data.detections;
else if (data && data.label) results = [data];
- else if (data && typeof data === 'object' && Object.keys(data).length > 0) {
- // Maybe it's a dict of results?
- results = [data];
- }
+ // Unexpected shape — treat as no results and let the empty-results message surface.If the backend legitimately returns other shapes, add explicit branches for each known schema rather than a generic fallback.
📝 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.
| let results = []; | |
| if (Array.isArray(data)) results = data; | |
| else if (data && data.detections) results = data.detections; | |
| else if (data && data.label) results = [data]; // Single result like waste | |
| else if (data && typeof data === 'object' && Object.keys(data).length > 0) { | |
| // Maybe it's a dict of results? | |
| results = [data]; | |
| } | |
| let results = []; | |
| if (Array.isArray(data)) results = data; | |
| else if (data && data.detections) results = data.detections; | |
| else if (data && data.label) results = [data]; // Single result like waste | |
| // Unexpected shape — treat as no results and let the empty-results message surface. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/BaseDetector.jsx` around lines 46 - 53, The last
catch-all branch in BaseDetector.jsx that sets results = [data] will treat any
non-empty object (including API error payloads) as a detection; remove or
tighten that branch so only known detection shapes are accepted (e.g., objects
with keys like "detections", "label", "bbox"/"confidence"/"class" or whatever
your detection schema uses). Instead, add explicit guards to ignore objects that
look like errors (e.g., contain "error" or "message") and route them to error
handling/logging; keep the earlier branches that handle Array, data.detections,
and single-label data and ensure the variable results is only populated when the
object matches a valid detection schema.
| {!imgSrc ? ( | ||
| <Webcam | ||
| audio={false} | ||
| ref={webcamRef} | ||
| screenshotFormat="image/jpeg" | ||
| className="w-full h-full object-cover" | ||
| videoConstraints={{ facingMode: 'environment' }} | ||
| onUserMediaError={(e) => setError("Camera access denied. Please check permissions.")} | ||
| /> | ||
| ) : ( | ||
| <img src={imgSrc} alt="Captured" className="w-full h-full object-cover" /> | ||
| )} | ||
|
|
||
| {loading && ( | ||
| <div className="absolute inset-0 bg-black/60 flex flex-col items-center justify-center backdrop-blur-sm text-white"> | ||
| <RefreshCw size={48} className="animate-spin mb-4" /> | ||
| <p className="font-bold text-lg">Analyzing...</p> | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Camera-error state doesn't disable Capture or unmount the broken <Webcam>.
When onUserMediaError fires, only error state is set. The <Webcam> component stays mounted (showing a blank feed), and the Capture Photo button remains fully active. The user can still press Capture, triggering getScreenshot() on a failed stream and hitting the null-ref crash described above.
🛡️ Proposed fix
Add a cameraError boolean state and use it to gate both the Webcam render and the Capture button:
const [error, setError] = useState(null);
+ const [cameraError, setCameraError] = useState(false);
const navigate = useNavigate(); {!imgSrc ? (
- <Webcam
- audio={false}
- ref={webcamRef}
- screenshotFormat="image/jpeg"
- className="w-full h-full object-cover"
- videoConstraints={{ facingMode: 'environment' }}
- onUserMediaError={(e) => setError("Camera access denied. Please check permissions.")}
- />
+ cameraError ? (
+ <div className="w-full h-full flex items-center justify-center text-white text-center p-4">
+ <p>Camera unavailable. Please check permissions and reload.</p>
+ </div>
+ ) : (
+ <Webcam
+ audio={false}
+ ref={webcamRef}
+ screenshotFormat="image/jpeg"
+ className="w-full h-full object-cover"
+ videoConstraints={{ facingMode: 'environment' }}
+ onUserMediaError={() => {
+ setCameraError(true);
+ setError("Camera access denied. Please check permissions.");
+ }}
+ />
+ )
) : ( {!imgSrc ? (
<button
onClick={capture}
+ disabled={cameraError}
className="w-full bg-blue-600 text-white py-4 rounded-2xl font-bold text-lg shadow-xl hover:bg-blue-700 active:scale-95 transition-all flex items-center justify-center gap-2"
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/BaseDetector.jsx` around lines 80 - 99, When
onUserMediaError runs it only calls setError, leaving the <Webcam> (and its
webcamRef) mounted so the Capture button can call getScreenshot() on a broken
stream; add a boolean cameraError state (e.g., const [cameraError,
setCameraError] = useState(false)) and setCameraError(true) inside the
onUserMediaError handler, then use cameraError to (1) prevent rendering the
<Webcam> component and instead show a fallback UI/message, and (2) disable or
hide the Capture button/handler that calls webcamRef.current.getScreenshot() so
getScreenshot() is never invoked when cameraError is true. Ensure any existing
setError calls remain to show the error message.
| { id: 'crowd', label: t('home.issues.crowd'), icon: <Users size={24} />, color: 'text-red-500', bg: 'bg-red-50' }, | ||
| { id: 'water-leak', label: t('home.issues.waterLeak'), icon: <Waves size={24} />, color: 'text-blue-500', bg: 'bg-blue-50' }, | ||
| { id: 'report', label: t('home.issues.waste'), icon: <Recycle size={24} />, color: 'text-emerald-600', bg: 'bg-emerald-50' }, | ||
| { id: 'graffiti', label: 'Graffiti', icon: <Brush size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, |
There was a problem hiding this comment.
Localize the new Graffiti label.
This is currently hardcoded and bypasses translation handling used by neighboring items.
Suggested fix
- { id: 'graffiti', label: 'Graffiti', icon: <Brush size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' },
+ { id: 'graffiti', label: t('home.issues.graffiti'), icon: <Brush 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: 'graffiti', label: 'Graffiti', icon: <Brush size={24} />, color: 'text-pink-600', bg: 'bg-pink-50' }, | |
| { id: 'graffiti', label: t('home.issues.graffiti'), icon: <Brush 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 103, The Graffiti menu item currently
hardcodes its label; update it to use the app's translation helper the
neighboring items use (e.g., t('...') or i18n.t('...')) instead of the literal
'Graffiti' so it participates in localization. Locate the menu entry with id
'graffiti' in Home.jsx and replace the label with a translated key (for example
t('nav.graffiti') or i18n.t('nav.graffiti')), and add the corresponding
'nav.graffiti' string to your locale resource files.
🔍 Quality Reminder |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/components/VoiceInput.jsx (1)
42-45:⚠️ Potential issue | 🟠 Major
onTranscriptin the dependency array causes the recognition instance to be recreated on every render if the parent doesn't memoize the callback.
onTranscriptis listed as a dependency on line 63. Any parent that passes an inline function (e.g.onTranscript={(t) => setValue(t)}) will produce a new function reference every render, causing this effect to re-fire, abort any in-progress recognition session, and recreate the instance. This is a silent, hard-to-debug breakage during active dictation.The idiomatic fix is to store the callback in a ref so the effect captures a stable reference, removing
onTranscriptfrom the dependency array:🔧 Proposed fix — stabilize `onTranscript` via a ref
const VoiceInput = ({ onTranscript, language = 'en' }) => { const [isListening, setIsListening] = useState(false); const recognitionRef = useRef(null); + const onTranscriptRef = useRef(onTranscript); const [error, setError] = useState(null); + // Keep the ref in sync with the latest prop without re-creating the instance + useEffect(() => { + onTranscriptRef.current = onTranscript; + }); // ...inside the setup useEffect: recognitionInstance.onresult = (event) => { const transcript = event.results[0][0].transcript; - onTranscript(transcript); + onTranscriptRef.current(transcript); }; - }, [language, onTranscript, isSupported]); + }, [language, isSupported]);Also applies to: 63-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/VoiceInput.jsx` around lines 42 - 45, The effect that creates recognitionInstance should not depend on the onTranscript prop directly; capture onTranscript in a ref instead so the SpeechRecognition handler uses a stable reference — create a ref (e.g., transcriptRef), update transcriptRef.current whenever the onTranscript prop changes, change recognitionInstance.onresult to call transcriptRef.current(transcript) and remove onTranscript from the useEffect dependency array that creates/cleans up recognitionInstance; keep other cleanup logic (abort/stop and event teardown) intact.frontend/src/NoiseDetector.jsx (1)
43-44:⚠️ Potential issue | 🟠 MajorRace condition: stream acquired after
stopLoopruns is never released
startLoopisasyncand awaitsgetUserMediabefore assigningstreamRef.current(line 44). If the user clicks Stop (or the component unmounts) between the call tostartLoop()and thatawaitresolving:
stopLoopfires —streamRef.currentis stillnullat that point, so thegetTracks().forEach(stop)call is a no-op.getUserMediaresolves →streamRef.current = stream(line 44) overwrites thenullset bystopLoop.recordAndSend()is called immediately (line 82) and the 5-second interval is started (line 84), beginning a recording loop even though the component has been stopped.The acquired media tracks are now orphaned — they are never stopped, and no subsequent call will clean them up.
🔒 Proposed fix — guard after the await
const startLoop = async () => { setError(null); setStatus('Initializing...'); try { const stream = await navigator.mediaDevices.getUserMedia({ audio: true }); + + // Component was stopped while we were awaiting mic permission + if (!isRecordingRef.current) { + stream.getTracks().forEach(track => track.stop()); + return; + } + streamRef.current = stream;You'll need to mirror
isRecordinginto a ref so the async callback can read the latest value:const intervalRef = useRef(null); const streamRef = useRef(null); +const isRecordingRef = useRef(false);And keep it in sync:
useEffect(() => { if (isRecording) { + isRecordingRef.current = true; startLoop(); } else { + isRecordingRef.current = false; stopLoop(); } // eslint-disable-next-line react-hooks/exhaustive-deps }, [isRecording]);Also applies to: 82-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/NoiseDetector.jsx` around lines 43 - 44, There’s a race where startLoop awaits navigator.mediaDevices.getUserMedia and may assign streamRef.current after stopLoop ran, leaving tracks orphaned; update startLoop to check a latest-recording flag before assigning the stream and starting timers: mirror isRecording into a mutable ref (e.g., isRecordingRef) and keep it in sync in startLoop/stopLoop, then after getUserMedia resolves verify isRecordingRef.current is still true before setting streamRef.current and calling recordAndSend / starting the interval; also apply the same post-await guard for the logic around recordAndSend and interval setup so no media or timers are started when recording was stopped while awaiting permission.frontend/src/CrowdDetector.jsx (1)
71-90:⚠️ Potential issue | 🟡 MinorIn-flight
toBlobcallback can draw stale detections after stop.
canvas.toBlob(async callback)is asynchronous. If the user stops detection while a callback is mid-flight (e.g., still awaiting thefetchon line 78), the effect cleanup clears the interval and the canvas, but the callback resumes and callsdrawDetections— overwriting the cleared canvas with stale detections.Introduce a cancellation ref to guard the async callback:
🛡️ Proposed fix — cancellation guard
+ const cancelledRef = useRef(false); async function detectFrame() { if (!videoRef.current || !canvasRef.current || !isDetecting) return; ... canvas.toBlob(async (blob) => { if (!blob) return; const formData = new FormData(); formData.append('image', blob, 'frame.jpg'); try { const response = await fetch(`${API_URL}/api/detect-crowd`, { method: 'POST', body: formData }); - if (response.ok) { + if (response.ok && !cancelledRef.current) { const data = await response.json(); drawDetections(data.detections, context); } } catch (err) { console.error("Detection error:", err); } }, 'image/jpeg', 0.8); } useEffect(() => { + cancelledRef.current = false; let interval; if (isDetecting) { startCamera(); interval = setInterval(detectFrame, 2000); } else { stopCamera(); ... } return () => { + cancelledRef.current = true; stopCamera(); if (interval) clearInterval(interval); }; }, [isDetecting]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/CrowdDetector.jsx` around lines 71 - 90, The canvas.toBlob async callback can resume after detection is stopped and overwrite the cleared canvas; add a cancellation ref (e.g., detectionCancelledRef) in the CrowdDetector component, set it true in your stop/cleanup logic, and check it at the start of the toBlob callback and again after awaiting fetch (before calling drawDetections) to bail out if cancelled; ensure the ref is reset when starting detection so only in-flight callbacks get ignored.
🧹 Nitpick comments (7)
frontend/eslint.config.js (1)
8-8: Keep test files linted; relax rules instead of globally ignoring them.Global test ignores reduce static safety on test code. Prefer a test-specific override with lighter rules.
Suggested adjustment
export default defineConfig([ - globalIgnores(['dist', 'src/setupTests.js', '**/*.test.js', '**/__tests__/**']), + globalIgnores(['dist']), { files: ['**/*.{js,jsx}'], ignores: ['**/__mocks__/**'], @@ }, + { + files: ['src/setupTests.js', '**/*.{test,spec}.{js,jsx}', '**/__tests__/**/*.{js,jsx}'], + rules: { + 'react-refresh/only-export-components': 'off', + }, + }, ])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/eslint.config.js` at line 8, Remove test patterns from the globalIgnores call (globalIgnores(['dist', 'src/setupTests.js', '**/*.test.js', '**/__tests__/**']) -> keep only true global ignores) and instead add an ESLint override for tests (use the overrides array with a pattern like '**/*.test.js' and '**/__tests__/**') that relaxes or adjusts rules for test files; reference the existing globalIgnores symbol to find where to change and add an overrides entry to lighten rules for test files rather than globally ignoring them.frontend/src/components/VoiceInput.jsx (2)
28-30: RedundantSpeechRecognitionnull-check is dead code.Line 25 already returns early when
!isSupported, andisSupportedistrueonly whenSpeechRecognitionis non-null. The re-assignment on line 28 and the guard on line 30 can never be reached withSpeechRecognition === undefined.🧹 Proposed cleanup
useEffect(() => { if (!isSupported) return; - // Check if browser supports SpeechRecognition const SpeechRecognition = window.SpeechRecognition || window.webkitSpeechRecognition; - - if (!SpeechRecognition) return; const recognitionInstance = new SpeechRecognition();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/VoiceInput.jsx` around lines 28 - 30, The re-assignment and null-check of SpeechRecognition in VoiceInput.jsx are redundant because isSupported already ensures a non-null SpeechRecognition; remove the lines "const SpeechRecognition = window.SpeechRecognition || window.webkitSpeechRecognition;" and the subsequent "if (!SpeechRecognition) return;" (or consolidate to use the existing isSupported-derived variable) so the component relies on the prior isSupported check and avoids unreachable dead code.
58-62:if (recognitionInstance)guard in cleanup is always truthy.
recognitionInstanceis unconditionally assigned on line 32 and is in scope for the closure; it can never benullorundefinedhere.🧹 Proposed cleanup
return () => { - if (recognitionInstance) { - recognitionInstance.stop(); - } + recognitionInstance.stop(); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/VoiceInput.jsx` around lines 58 - 62, The cleanup closure's conditional guard is redundant because recognitionInstance is always assigned in the outer scope; remove the if check in the return cleanup and directly call recognitionInstance.stop() (or use recognitionInstance.stop?.() if you prefer a null-safe call for future changes) so the cleanup simply stops the recognitionInstance without the always-true if block.frontend/src/NoiseDetector.jsx (1)
110-116: Prefer callingstopLoop()directly in the unmount cleanup
stopRecording()calls bothsetIsRecording(false)(a state update on an unmounting component) andstopLoop(). React 19 silently ignores state updates on unmounted components, but thesetIsRecordingcall here is unnecessary — onlystopLoop()performs the actual resource cleanup (clearing the interval and stopping mic tracks). Calling it directly avoids a superfluous (and potentially confusing) state mutation at teardown.♻️ Proposed change
useEffect(() => { - // Cleanup on unmount return () => { - stopRecording(); + stopLoop(); // release mic and interval; no state update needed at unmount }; // eslint-disable-next-line react-hooks/exhaustive-deps }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/NoiseDetector.jsx` around lines 110 - 116, The useEffect unmount cleanup currently calls stopRecording(), which invokes setIsRecording(false) and causes a state update on an unmounting component; change the cleanup to call stopLoop() directly to perform the actual resource teardown (clearing intervals and stopping mic tracks) and avoid the unnecessary setIsRecording call, or alternatively guard/remove the setIsRecording(false) inside stopRecording so the unmount path only triggers stopLoop(); locate the cleanup in the useEffect where stopRecording is returned and replace or adjust it to reference stopLoop and leave state updates out of the unmount flow.frontend/src/components/ResolutionProofCapture.jsx (1)
99-104: ReplaceuseState+useEffectwithuseMemofor derived geofence data.
geofenceStatusis computed entirely fromtokenandgpsPositionand only read thereafter. UsinguseMemoeliminates the extra render cycle and removes the need for the lint suppression.Proposed refactor
-import React, { useState, useRef, useCallback, useEffect } from 'react'; +import React, { useState, useRef, useCallback, useEffect, useMemo } from 'react'; -const [geofenceStatus, setGeofenceStatus] = useState(null); +const geofenceStatus = useMemo(() => { + if (!token || !gpsPosition) return null; + + const R = 6371000; + const lat1 = (token.geofence_latitude * Math.PI) / 180; + const lat2 = (gpsPosition.latitude * Math.PI) / 180; + const dLat = ((gpsPosition.latitude - token.geofence_latitude) * Math.PI) / 180; + const dLon = ((gpsPosition.longitude - token.geofence_longitude) * Math.PI) / 180; + + const a = + Math.sin(dLat / 2) ** 2 + + Math.cos(lat1) * Math.cos(lat2) * Math.sin(dLon / 2) ** 2; + const c = 2 * Math.asin(Math.sqrt(a)); + const distance = R * c; + + return { + distance: Math.round(distance), + isInside: distance <= token.geofence_radius_meters, + radius: token.geofence_radius_meters, + }; +}, [token, gpsPosition]); -// Check geofence distance -useEffect(() => { - if (!token || !gpsPosition) return; - ... - // eslint-disable-next-line react-hooks/set-state-in-effect - setGeofenceStatus({...}); -}, [token, gpsPosition]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ResolutionProofCapture.jsx` around lines 99 - 104, geofenceStatus is currently derived state set via setGeofenceStatus inside an effect; replace this pattern by computing geofenceStatus with useMemo from token and gpsPosition to avoid the extra render and remove the eslint suppression — locate the setGeofenceStatus usage and the geofenceStatus state in the ResolutionProofCapture component, remove the useState and related useEffect that computes distance and calls setGeofenceStatus, and instead add a useMemo that returns { distance: Math.round(distance), isInside: distance <= token.geofence_radius_meters, radius: token.geofence_radius_meters } where distance is computed from token and gpsPosition so the memo depends on [token, gpsPosition]; ensure any consumers read the memoized geofenceStatus.frontend/src/WaterLeakDetector.jsx (2)
62-98: In-flight async work can draw on canvas after detection is stopped.
detectFramekicks offtoBlob→fetch→drawDetections, all asynchronous. When the user stops detection, the interval is cleared and the canvas is wiped, but any already-in-flight blob/fetch callback can still resolve and calldrawDetections, briefly re-painting results on a "stopped" canvas.A lightweight guard using an
AbortController(or a boolean ref likeisMountedRef) that is toggled in the cleanup function would let thetoBlobcallback bail out if detection was stopped mid-flight.Sketch: guard with a ref
useEffect(() => { let interval; + let active = true; if (isDetecting) { startCamera(); interval = setInterval(detectFrame, 2000); } else { /* … */ } return () => { + active = false; stopCamera(); if (interval) clearInterval(interval); }; }, [isDetecting]);Then inside the
toBlobcallback:canvas.toBlob(async (blob) => { + if (!active || !blob) return; - if (!blob) return;(This requires
detectFrameto close overactive, e.g. by inlining or passing it.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/WaterLeakDetector.jsx` around lines 62 - 98, The async canvas pipeline in detectFrame (canvas.toBlob → fetch → drawDetections) can repaint after detection is stopped; add a lightweight guard (either an AbortController or a boolean ref like isActiveRef) that detectFrame closes over and that the toBlob callback, the fetch response handling, and before calling drawDetections check and bail if not active; ensure the stop/cleanup logic toggles this guard (e.g., set isActiveRef.current = false or call controller.abort()) when clearing the interval so any in-flight blob/fetch paths return early and never call drawDetections on a cleared canvas (references: detectFrame, canvas.toBlob callback, drawDetections, isDetecting, videoRef, canvasRef).
62-98: Migrate WaterLeakDetector to use centralized detectorsApi and BaseDetector pattern.WaterLeakDetector hand-rolls the camera lifecycle and uses raw
fetchwith a hard-coded path (/api/detect-water-leak), whiledetectorsApi.waterLeakexists infrontend/src/api/detectors.jsandBaseDetectoris already used by other detectors (e.g., TrafficSignDetector, GraffitiDetector, AbandonedVehicleDetector). Migrating to the shared abstractions would improve consistency across detector components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/WaterLeakDetector.jsx` around lines 62 - 98, The detectFrame function is manually managing camera/canvas lifecycle and calling fetch('/api/detect-water-leak'); refactor WaterLeakDetector to use the shared BaseDetector pattern and detectorsApi.waterLeak: remove direct fetch and API_URL usage inside detectFrame (or the component) and instead call detectorsApi.waterLeak with the captured Blob/FormData (or the API helper’s expected payload), reuse BaseDetector’s camera start/stop and frame scheduling utilities (e.g., startCamera/stopCamera or the lifecycle methods used by TrafficSignDetector/GraffitiDetector) and keep drawDetections for rendering; update references to videoRef/canvasRef usage to match how other detectors obtain frames from BaseDetector and ensure any error handling matches the standardized detectorsApi error flow.
🤖 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/CrowdDetector.jsx`:
- Line 33: The forEach callback currently uses an expression body
(tracks.forEach(track => track.stop())) which implicitly returns the result of
track.stop() and triggers the static analyser; change the callback to use a
block body so it performs the side effect without returning a value (update the
tracks.forEach callback in CrowdDetector.jsx to use a block-style arrow function
that calls track.stop(); inside the block).
- Around line 93-111: The useEffect in CrowdDetector.jsx is silencing real
dependency warnings because startCamera, stopCamera, and detectFrame are
recreated each render; stabilize them by wrapping each function in useCallback
(e.g., const startCamera = useCallback(..., [/* its real deps */]), const
stopCamera = useCallback(..., [/* deps */]), const detectFrame =
useCallback(..., [/* deps */])) so you can safely include them in the effect
dependency array and remove the eslint-disable comment; ensure the callbacks
declare correct dependencies (or move detectFrame entirely inside the useEffect
if it uses only effect-scoped values) and update the useEffect signature to
depend on isDetecting plus those callbacks.
In `@frontend/src/NoiseDetector.jsx`:
- Around line 31-36: The non-OK branch and catch block currently only call
console.error, leaving the component state (status) stuck at "Analyzing..." and
the recording interval running; update both branches to call setStatus with an
error/idle value (e.g., setStatus("Error") or setStatus("Idle")), stop the
ongoing recordings by clearing the recording interval and/or calling the
component's stopRecording/clearInterval routine (reference the recordingInterval
or stopRecording function used in NoiseDetector.jsx), and surface the error
message to the user (e.g., store the error string in an error state or pass it
to the existing UI error handler). Ensure you reference and use the existing
setStatus and stopRecording/recordingInterval identifiers so the UI resets and
the interval stops after a failed request.
In `@frontend/src/WaterLeakDetector.jsx`:
- Around line 100-118: The interval callback captures a stale detectFrame
function; change the effect to use a ref that always points to the latest
detectFrame (e.g., latestDetectRef.current = detectFrame whenever detectFrame
changes), have setInterval call () => latestDetectRef.current(), and keep
videoRef/canvasRef usage unchanged; also remove the dead clearInterval call in
the else branch (interval is only set when isDetecting is true) and ensure the
cleanup still clears the interval and stops the camera in the returned cleanup
function of the useEffect that currently wraps isDetecting.
---
Outside diff comments:
In `@frontend/src/components/VoiceInput.jsx`:
- Around line 42-45: The effect that creates recognitionInstance should not
depend on the onTranscript prop directly; capture onTranscript in a ref instead
so the SpeechRecognition handler uses a stable reference — create a ref (e.g.,
transcriptRef), update transcriptRef.current whenever the onTranscript prop
changes, change recognitionInstance.onresult to call
transcriptRef.current(transcript) and remove onTranscript from the useEffect
dependency array that creates/cleans up recognitionInstance; keep other cleanup
logic (abort/stop and event teardown) intact.
In `@frontend/src/CrowdDetector.jsx`:
- Around line 71-90: The canvas.toBlob async callback can resume after detection
is stopped and overwrite the cleared canvas; add a cancellation ref (e.g.,
detectionCancelledRef) in the CrowdDetector component, set it true in your
stop/cleanup logic, and check it at the start of the toBlob callback and again
after awaiting fetch (before calling drawDetections) to bail out if cancelled;
ensure the ref is reset when starting detection so only in-flight callbacks get
ignored.
In `@frontend/src/NoiseDetector.jsx`:
- Around line 43-44: There’s a race where startLoop awaits
navigator.mediaDevices.getUserMedia and may assign streamRef.current after
stopLoop ran, leaving tracks orphaned; update startLoop to check a
latest-recording flag before assigning the stream and starting timers: mirror
isRecording into a mutable ref (e.g., isRecordingRef) and keep it in sync in
startLoop/stopLoop, then after getUserMedia resolves verify
isRecordingRef.current is still true before setting streamRef.current and
calling recordAndSend / starting the interval; also apply the same post-await
guard for the logic around recordAndSend and interval setup so no media or
timers are started when recording was stopped while awaiting permission.
---
Nitpick comments:
In `@frontend/eslint.config.js`:
- Line 8: Remove test patterns from the globalIgnores call
(globalIgnores(['dist', 'src/setupTests.js', '**/*.test.js', '**/__tests__/**'])
-> keep only true global ignores) and instead add an ESLint override for tests
(use the overrides array with a pattern like '**/*.test.js' and
'**/__tests__/**') that relaxes or adjusts rules for test files; reference the
existing globalIgnores symbol to find where to change and add an overrides entry
to lighten rules for test files rather than globally ignoring them.
In `@frontend/src/components/ResolutionProofCapture.jsx`:
- Around line 99-104: geofenceStatus is currently derived state set via
setGeofenceStatus inside an effect; replace this pattern by computing
geofenceStatus with useMemo from token and gpsPosition to avoid the extra render
and remove the eslint suppression — locate the setGeofenceStatus usage and the
geofenceStatus state in the ResolutionProofCapture component, remove the
useState and related useEffect that computes distance and calls
setGeofenceStatus, and instead add a useMemo that returns { distance:
Math.round(distance), isInside: distance <= token.geofence_radius_meters,
radius: token.geofence_radius_meters } where distance is computed from token and
gpsPosition so the memo depends on [token, gpsPosition]; ensure any consumers
read the memoized geofenceStatus.
In `@frontend/src/components/VoiceInput.jsx`:
- Around line 28-30: The re-assignment and null-check of SpeechRecognition in
VoiceInput.jsx are redundant because isSupported already ensures a non-null
SpeechRecognition; remove the lines "const SpeechRecognition =
window.SpeechRecognition || window.webkitSpeechRecognition;" and the subsequent
"if (!SpeechRecognition) return;" (or consolidate to use the existing
isSupported-derived variable) so the component relies on the prior isSupported
check and avoids unreachable dead code.
- Around line 58-62: The cleanup closure's conditional guard is redundant
because recognitionInstance is always assigned in the outer scope; remove the if
check in the return cleanup and directly call recognitionInstance.stop() (or use
recognitionInstance.stop?.() if you prefer a null-safe call for future changes)
so the cleanup simply stops the recognitionInstance without the always-true if
block.
In `@frontend/src/NoiseDetector.jsx`:
- Around line 110-116: The useEffect unmount cleanup currently calls
stopRecording(), which invokes setIsRecording(false) and causes a state update
on an unmounting component; change the cleanup to call stopLoop() directly to
perform the actual resource teardown (clearing intervals and stopping mic
tracks) and avoid the unnecessary setIsRecording call, or alternatively
guard/remove the setIsRecording(false) inside stopRecording so the unmount path
only triggers stopLoop(); locate the cleanup in the useEffect where
stopRecording is returned and replace or adjust it to reference stopLoop and
leave state updates out of the unmount flow.
In `@frontend/src/WaterLeakDetector.jsx`:
- Around line 62-98: The async canvas pipeline in detectFrame (canvas.toBlob →
fetch → drawDetections) can repaint after detection is stopped; add a
lightweight guard (either an AbortController or a boolean ref like isActiveRef)
that detectFrame closes over and that the toBlob callback, the fetch response
handling, and before calling drawDetections check and bail if not active; ensure
the stop/cleanup logic toggles this guard (e.g., set isActiveRef.current = false
or call controller.abort()) when clearing the interval so any in-flight
blob/fetch paths return early and never call drawDetections on a cleared canvas
(references: detectFrame, canvas.toBlob callback, drawDetections, isDetecting,
videoRef, canvasRef).
- Around line 62-98: The detectFrame function is manually managing camera/canvas
lifecycle and calling fetch('/api/detect-water-leak'); refactor
WaterLeakDetector to use the shared BaseDetector pattern and
detectorsApi.waterLeak: remove direct fetch and API_URL usage inside detectFrame
(or the component) and instead call detectorsApi.waterLeak with the captured
Blob/FormData (or the API helper’s expected payload), reuse BaseDetector’s
camera start/stop and frame scheduling utilities (e.g., startCamera/stopCamera
or the lifecycle methods used by TrafficSignDetector/GraffitiDetector) and keep
drawDetections for rendering; update references to videoRef/canvasRef usage to
match how other detectors obtain frames from BaseDetector and ensure any error
handling matches the standardized detectorsApi error flow.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
frontend/eslint.config.jsfrontend/lint_output.txtfrontend/src/CrowdDetector.jsxfrontend/src/NoiseDetector.jsxfrontend/src/WaterLeakDetector.jsxfrontend/src/components/ResolutionProofCapture.jsxfrontend/src/components/SupabaseExample.jsxfrontend/src/components/VoiceInput.jsxfrontend/src/contexts/AuthContext.jsxnetlify.toml
💤 Files with no reviewable changes (2)
- frontend/src/components/SupabaseExample.jsx
- frontend/lint_output.txt
✅ Files skipped from review due to trivial changes (1)
- frontend/src/contexts/AuthContext.jsx
| function stopCamera() { | ||
| if (videoRef.current && videoRef.current.srcObject) { | ||
| const tracks = videoRef.current.srcObject.getTracks(); | ||
| tracks.forEach(track => track.stop()); |
There was a problem hiding this comment.
Fix the forEach callback to use a block body.
track => track.stop() implicitly returns the result of track.stop(). While harmless today, it is flagged by the static analyser because returning values from forEach callbacks is considered suspicious and may mask unintended behavior in the future.
🐛 Proposed fix
- tracks.forEach(track => track.stop());
+ tracks.forEach(track => { track.stop(); });📝 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.
| tracks.forEach(track => track.stop()); | |
| tracks.forEach(track => { track.stop(); }); |
🧰 Tools
🪛 Biome (2.4.4)
[error] 33-33: This callback passed to forEach() iterable method should not return a value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/CrowdDetector.jsx` at line 33, The forEach callback currently
uses an expression body (tracks.forEach(track => track.stop())) which implicitly
returns the result of track.stop() and triggers the static analyser; change the
callback to use a block body so it performs the side effect without returning a
value (update the tracks.forEach callback in CrowdDetector.jsx to use a
block-style arrow function that calls track.stop(); inside the block).
| useEffect(() => { | ||
| let interval; | ||
| if (isDetecting) { | ||
| startCamera(); | ||
| interval = setInterval(detectFrame, 2000); | ||
| } else { | ||
| stopCamera(); | ||
| if (interval) clearInterval(interval); | ||
| if (canvasRef.current) { | ||
| const ctx = canvasRef.current.getContext('2d'); | ||
| ctx.clearRect(0, 0, canvasRef.current.width, canvasRef.current.height); | ||
| } | ||
| } | ||
| return () => { | ||
| stopCamera(); | ||
| if (interval) clearInterval(interval); | ||
| }; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [isDetecting]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
eslint-disable is masking real stale-closure dependencies — stabilise with useCallback.
startCamera, stopCamera, and detectFrame are plain function declarations redefined on every render. The // eslint-disable-next-line react-hooks/exhaustive-deps comment suppresses a legitimate warning: all three are used inside the effect but absent from its dependency array. This is fragile — a future refactor that adds state reads inside those functions will silently capture stale values.
The idiomatic fix is to wrap each function with useCallback (or move detectFrame fully inside the effect). The adjacent AccessibilityDetector.jsx has the same pattern and would benefit from the same treatment.
♻️ Proposed refactor — stabilise all three functions with useCallback
- async function startCamera() {
+ const startCamera = useCallback(async () => {
setError(null);
try {
const stream = await navigator.mediaDevices.getUserMedia({ ... });
if (videoRef.current) {
videoRef.current.srcObject = stream;
}
} catch (err) {
setError("Could not access camera: " + err.message);
setIsDetecting(false);
}
- }
+ }, []);
- function stopCamera() {
+ const stopCamera = useCallback(() => {
if (videoRef.current && videoRef.current.srcObject) {
const tracks = videoRef.current.srcObject.getTracks();
tracks.forEach(track => { track.stop(); });
videoRef.current.srcObject = null;
}
- }
+ }, []);
- async function detectFrame() {
+ const detectFrame = useCallback(async () => {
...
- }
+ }, [isDetecting]); // isDetecting is the only non-ref dependency
useEffect(() => {
let interval;
if (isDetecting) {
startCamera();
interval = setInterval(detectFrame, 2000);
} else {
stopCamera();
if (interval) clearInterval(interval);
if (canvasRef.current) {
const ctx = canvasRef.current.getContext('2d');
ctx.clearRect(0, 0, canvasRef.current.width, canvasRef.current.height);
}
}
return () => {
stopCamera();
if (interval) clearInterval(interval);
};
- // eslint-disable-next-line react-hooks/exhaustive-deps
}, [isDetecting, startCamera, stopCamera, detectFrame]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/CrowdDetector.jsx` around lines 93 - 111, The useEffect in
CrowdDetector.jsx is silencing real dependency warnings because startCamera,
stopCamera, and detectFrame are recreated each render; stabilize them by
wrapping each function in useCallback (e.g., const startCamera =
useCallback(..., [/* its real deps */]), const stopCamera = useCallback(..., [/*
deps */]), const detectFrame = useCallback(..., [/* deps */])) so you can safely
include them in the effect dependency array and remove the eslint-disable
comment; ensure the callbacks declare correct dependencies (or move detectFrame
entirely inside the useEffect if it uses only effect-scoped values) and update
the useEffect signature to depend on isDetecting plus those callbacks.
| } else { | ||
| console.error("Audio API error"); | ||
| } | ||
| } catch (err) { | ||
| console.error("Audio network error", err); | ||
| } |
There was a problem hiding this comment.
Silent failures leave status stuck at 'Analyzing...' indefinitely
Both the non-OK branch (line 32) and the catch block (line 35) only console.error without resetting status or surfacing the error to the user. After a failed request, status remains 'Analyzing...' for the lifetime of that recording session — while the interval keeps firing new recordings — giving no user-visible feedback.
🐛 Proposed fix
if (response.ok) {
const data = await response.json();
if (data.detections) {
setDetections(data.detections);
}
setStatus('Listening...');
} else {
- console.error("Audio API error");
+ console.error("Audio API error", response.status);
+ setStatus('Listening...');
+ setError(`Detection service error (${response.status}). Retrying...`);
}
} catch (err) {
- console.error("Audio network error", err);
+ console.error("Audio network error", err);
+ setStatus('Listening...');
+ setError("Network error. Check your connection.");
}📝 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.
| } else { | |
| console.error("Audio API error"); | |
| } | |
| } catch (err) { | |
| console.error("Audio network error", err); | |
| } | |
| } else { | |
| console.error("Audio API error", response.status); | |
| setStatus('Listening...'); | |
| setError(`Detection service error (${response.status}). Retrying...`); | |
| } | |
| } catch (err) { | |
| console.error("Audio network error", err); | |
| setStatus('Listening...'); | |
| setError("Network error. Check your connection."); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/NoiseDetector.jsx` around lines 31 - 36, The non-OK branch and
catch block currently only call console.error, leaving the component state
(status) stuck at "Analyzing..." and the recording interval running; update both
branches to call setStatus with an error/idle value (e.g., setStatus("Error") or
setStatus("Idle")), stop the ongoing recordings by clearing the recording
interval and/or calling the component's stopRecording/clearInterval routine
(reference the recordingInterval or stopRecording function used in
NoiseDetector.jsx), and surface the error message to the user (e.g., store the
error string in an error state or pass it to the existing UI error handler).
Ensure you reference and use the existing setStatus and
stopRecording/recordingInterval identifiers so the UI resets and the interval
stops after a failed request.
| useEffect(() => { | ||
| let interval; | ||
| if (isDetecting) { | ||
| startCamera(); | ||
| interval = setInterval(detectFrame, 2000); | ||
| } else { | ||
| stopCamera(); | ||
| if (interval) clearInterval(interval); | ||
| if (canvasRef.current) { | ||
| const ctx = canvasRef.current.getContext('2d'); | ||
| ctx.clearRect(0, 0, canvasRef.current.width, canvasRef.current.height); | ||
| } | ||
| }); | ||
| }; | ||
| } | ||
| return () => { | ||
| stopCamera(); | ||
| if (interval) clearInterval(interval); | ||
| }; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [isDetecting]); |
There was a problem hiding this comment.
Stale closure: setInterval captures a snapshot of detectFrame that never updates.
detectFrame is re-created every render, but setInterval on line 104 captures the version from the render where isDetecting became true. Because detectFrame reads component state via closure (line 63), any future state captured inside it will be stale for the lifetime of that interval. Today this is masked because:
isDetectingis alwaystruein the captured closure (interval is cleared before it could befalse).- Refs (
videoRef,canvasRef) are mutable objects, so they stay current.
But this is fragile — adding any additional state read inside detectFrame will silently stale-close. A useRef-based approach (storing the latest detectFrame in a ref and calling it from the interval) is the idiomatic fix.
Suggested fix using a ref to always call the latest detectFrame
+ const detectFrameRef = useRef(detectFrame);
+ useEffect(() => {
+ detectFrameRef.current = detectFrame;
+ });
+
useEffect(() => {
let interval;
if (isDetecting) {
startCamera();
- interval = setInterval(detectFrame, 2000);
+ interval = setInterval(() => detectFrameRef.current(), 2000);
} else {
stopCamera();
- if (interval) clearInterval(interval);
if (canvasRef.current) {
const ctx = canvasRef.current.getContext('2d');
ctx.clearRect(0, 0, canvasRef.current.width, canvasRef.current.height);
}
}
return () => {
stopCamera();
if (interval) clearInterval(interval);
};
- // eslint-disable-next-line react-hooks/exhaustive-deps
}, [isDetecting]);Also, line 107 (if (interval) clearInterval(interval)) is dead code — interval is only assigned inside the if (isDetecting) branch, so it's always undefined in the else branch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/WaterLeakDetector.jsx` around lines 100 - 118, The interval
callback captures a stale detectFrame function; change the effect to use a ref
that always points to the latest detectFrame (e.g., latestDetectRef.current =
detectFrame whenever detectFrame changes), have setInterval call () =>
latestDetectRef.current(), and keep videoRef/canvasRef usage unchanged; also
remove the dead clearInterval call in the else branch (interval is only set when
isDetecting is true) and ensure the cleanup still clears the interval and stops
the camera in the returned cleanup function of the useEffect that currently
wraps isDetecting.
Added new detectors for Traffic Signs, Abandoned Vehicles, and Graffiti using Hugging Face API integration. Updated backend to support HF token from config and fixed unified detection service to avoid deprecated imports. Updated frontend routing and Home page to include these new features.
PR created automatically by Jules for task 9487458731636491438 started by @RohanExploit
Summary by cubic
Adds Traffic Sign, Abandoned Vehicle, and Graffiti detectors with a shared, camera-first UI wired to backend endpoints. Also fixes Netlify deploy and standardizes Hugging Face token via config.
New Features
Bug Fixes
Written for commit 9b8b2ef. Summary will update on new commits.
Summary by CodeRabbit
New Features
Chores