Add new detectors and update UI for public grievances #371
Conversation
…ed Vehicle detectors - Added `detect_public_facilities_clip` and `detect_construction_safety_clip` to backend HF service. - Added `/api/detect-public-facilities` and `/api/detect-construction-safety` endpoints. - Created frontend components: `TrafficSignDetector`, `AbandonedVehicleDetector`, `PublicFacilitiesDetector`, `ConstructionSafetyDetector`. - Updated `App.jsx` to route new detectors and expose existing ones (`Accessibility`, `WaterLeak`, `Crowd`, `Waste`). - Updated `Home.jsx` to include buttons for new categories in "Road & Traffic", "Environment & Safety", and "Management". Co-authored-by: RohanExploit <[email protected]>
|
👋 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 two backend CLIP detectors (public facilities, construction safety) with matching API endpoints, and four new frontend live-camera detector components (Traffic Sign, Abandoned Vehicle, Public Facilities, Construction Safety) plus routing and Home navigation entries; also adds a simple import test and removes a Netlify redirect line. Changes
Sequence DiagramsequenceDiagram
actor User
participant Browser
participant VideoCanvas as "Video + Canvas"
participant API as "Backend API"
participant CLIP as "HF CLIP Service"
User->>Browser: Click Start Live Detection
Browser->>VideoCanvas: request camera & start stream
loop every ~2s (while live)
Browser->>VideoCanvas: capture frame -> JPEG blob
Browser->>API: POST /api/detect-[type] (form-data image)
API->>CLIP: detect_[type]_clip(image)
CLIP-->>API: detections JSON
API-->>Browser: detections JSON
Browser->>VideoCanvas: draw detection overlays
end
User->>Browser: Click Stop Detection
Browser->>VideoCanvas: stop stream & cleanup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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: 5
🤖 Fix all issues with AI agents
In `@backend/routers/detection.py`:
- Around line 442-472: The new CLIP endpoints
(detect_public_facilities_endpoint, detect_construction_safety_endpoint,
detect_traffic_sign_endpoint, detect_abandoned_vehicle_endpoint) are reading raw
bytes via await image.read() and bypassing validation; update each to call
process_uploaded_image(image) (the same helper used by
detect_illegal_parking_endpoint and detect_street_light_endpoint) to perform
MIME/type, size and format checks and return validated/optimized bytes, then
pass those bytes into detect_public_facilities_clip /
detect_construction_safety_clip / detect_traffic_sign_clip /
detect_abandoned_vehicle_clip respectively; preserve the existing try/except
error handling pattern (catch invalid upload -> log and raise HTTPException 400,
other errors -> log and raise HTTPException 500).
In `@frontend/src/AbandonedVehicleDetector.jsx`:
- Around line 20-23: When assigning the camera stream returned from
navigator.mediaDevices.getUserMedia, handle the case where videoRef.current is
null to avoid leaking tracks: if videoRef.current is truthy set
videoRef.current.srcObject = stream, otherwise immediately stop all tracks on
the stream (stream.getTracks().forEach(t => t.stop())). Also ensure the
component's cleanup/unmount logic (the effect that calls getUserMedia) stops the
stream if it's still active so no tracks remain running; update the code around
videoRef and the getUserMedia promise to stop tracks in the null-branch and
during cleanup.
- Around line 46-60: The labels are all drawn at fixed coordinates inside the
detections.forEach loop, causing overlap; change the loop to use the item index
(e.g., detections.forEach((det, i) => { ... })) and compute a per-item y offset
like y = 30 + (i * 35) (mirror ConstructionSafetyDetector's approach) so each
label's y position is incremented; keep using label, textWidth, and the same
fillRect/fillText logic but replace the constant y with the indexed y to avoid
overlap.
In `@frontend/src/TrafficSignDetector.jsx`:
- Around line 1-189: The component files (TrafficSignDetector,
PublicFacilitiesDetector, ConstructionSafetyDetector, AbandonedVehicleDetector)
duplicate camera lifecycle, frame capture, canvas overlay and UI; extract a
reusable hook and/or wrapper component to remove duplication: create a hook
named useDetectorCamera that encapsulates startCamera, stopCamera, detectFrame,
canvas/video refs, interval management and error state, and a generic
LiveDetector component that accepts props (endpointUrl, overlayColor, title,
description, drawDetectionsOverride) to handle the UI and call drawDetections;
refactor TrafficSignDetector to use useDetectorCamera/LiveDetector and pass its
specific API route (`/api/detect-traffic-sign`), color and strings, and keep any
detector-specific drawing logic via a drawDetections or drawLabel prop so all
four detectors reuse the shared logic.
In `@frontend/src/views/Home.jsx`:
- Line 87: Replace the hardcoded English labels with i18n calls so translations
are used: update the items that currently use "Accessibility", "Construction",
and "Public Facilities" (the objects with id values 'accessibility',
'construction', 'public_facilities' in the array inside Home.jsx) to call the
translation function (t) with appropriate keys such as
t('home.issues.accessibility'), t('home.issues.construction'),
t('home.issues.public_facilities') respectively; also replace the other
hardcoded labels noted nearby (the items around those entries) to use the
existing t('home.issues.…') keys so all menu/issue labels are consistently
localized.
🧹 Nitpick comments (6)
frontend/src/TrafficSignDetector.jsx (2)
118-136: Stale closure:detectFramecaptured once but invoked repeatedly bysetInterval.
detectFrameis defined during render and closes over the currentisDetectingvalue. Because it's passed tosetIntervalinside the effect, it never picks up subsequent state changes. It works here only becauseisDetectingistruewhen the interval starts and the guard at line 78 happens to pass. However, if additional state (e.g. a detection-result accumulator) is added later, this pattern will silently break.Additionally,
startCamera()is called withoutawait, so the first fewsetIntervalticks fire before the camera stream is ready. ThereadyStateguard (line 82) saves it, but a cleaner approach is to start the interval after the videoloadeddataevent.Sketch: use refs for the detection loop and wait for camera readiness
+ const isDetectingRef = useRef(false); + const intervalRef = useRef(null); + useEffect(() => { - let interval; + isDetectingRef.current = isDetecting; if (isDetecting) { - startCamera(); - interval = setInterval(detectFrame, 2000); + startCamera().then(() => { + intervalRef.current = setInterval(() => { + if (isDetectingRef.current) detectFrame(); + }, 2000); + }); } else { stopCamera(); - if (interval) clearInterval(interval); + if (intervalRef.current) clearInterval(intervalRef.current); ... } return () => { stopCamera(); - if (interval) clearInterval(interval); + if (intervalRef.current) clearInterval(intervalRef.current); }; }, [isDetecting]);
77-116: Concurrent API calls:toBlobcallback can overlap with the next interval tick.
detectFramefires every 2 seconds, but the asyncfetchinsidetoBlobmay take longer. Multiple in-flight requests will race to draw on the same canvas context, causing visual flickering. Consider guarding with a "request-in-flight" ref to skip a frame when the previous one hasn't completed.backend/routers/detection.py (1)
448-448: Consider chaining exceptions withraise … from e.Per Ruff B904,
raise HTTPException(…)insideexceptblocks should useraise … from e(orfrom None) to preserve the exception chain for debugging. This matches best practice, though the existing endpoints in this file follow the same pattern.Also applies to: 456-456, 464-464, 472-472
test_detection_import.py (1)
1-28: Consider converting to a proper pytest test.This script uses
sys.exit(1)instead of assertions, and manipulatessys.pathmanually. If the project uses pytest (which the existingbackend/tests/test_new_detectors.pysuggests), this would integrate better as a standard test:Sketch: pytest-style import test
-import sys -import os - -# Add the current directory to sys.path -sys.path.append(os.getcwd()) - -try: - from backend.routers.detection import ( - detect_public_facilities_endpoint, - detect_construction_safety_endpoint - ) - print("Successfully imported new endpoints from backend.routers.detection") -except ImportError as e: - print(f"Failed to import new endpoints: {e}") - sys.exit(1) -except Exception as e: - print(f"An error occurred during import: {e}") - sys.exit(1) - -try: - from backend.hf_api_service import ( - detect_public_facilities_clip, - detect_construction_safety_clip - ) - print("Successfully imported new service functions from backend.hf_api_service") -except ImportError as e: - print(f"Failed to import new service functions: {e}") - sys.exit(1) +from backend.routers.detection import ( + detect_public_facilities_endpoint, + detect_construction_safety_endpoint, +) +from backend.hf_api_service import ( + detect_public_facilities_clip, + detect_construction_safety_clip, +) + + +def test_new_endpoints_are_importable(): + assert callable(detect_public_facilities_endpoint) + assert callable(detect_construction_safety_endpoint) + + +def test_new_clip_functions_are_importable(): + assert callable(detect_public_facilities_clip) + assert callable(detect_construction_safety_clip)Also note: the second
tryblock (line 20) lacks the broadexcept Exceptionhandler that the first block has, making error handling asymmetric.frontend/src/AbandonedVehicleDetector.jsx (2)
63-102: No guard against overlapping in-flight requests.
setIntervalfires every 2 seconds regardless of whether the previousfetchhas completed. Under slow network conditions, requests can pile up and race, potentially drawing stale detections over newer ones. Consider using a flag or replacingsetIntervalwith a chainedsetTimeoutthat schedules the next call only after the previous one resolves.
104-122: Stale closure risk hidden by the eslint-disable.
setInterval(detectFrame, 2000)capturesdetectFramefrom a single render. The current code works because the interval is torn down wheneverisDetectingchanges, but any future state/prop added todetectFramewould silently go stale. WrappingdetectFrame(and the other helpers) inuseCallback, or switching to asetTimeout-chain pattern inside a ref, would make this more robust and allow removing the lint suppression.Also, line 111 (
if (interval) clearInterval(interval)) in theelsebranch is dead code —intervalis only assigned in theifbranch.
| @router.post("/api/detect-public-facilities") | ||
| async def detect_public_facilities_endpoint(request: Request, image: UploadFile = File(...)): | ||
| try: | ||
| image_bytes = await image.read() | ||
| except Exception as e: | ||
| logger.error(f"Invalid image file: {e}", exc_info=True) | ||
| raise HTTPException(status_code=400, detail="Invalid image file") | ||
|
|
||
| try: | ||
| client = get_http_client(request) | ||
| detections = await detect_public_facilities_clip(image_bytes, client=client) | ||
| return {"detections": detections} | ||
| except Exception as e: | ||
| logger.error(f"Public facilities detection error: {e}", exc_info=True) | ||
| raise HTTPException(status_code=500, detail="Internal server error") | ||
|
|
||
| @router.post("/api/detect-construction-safety") | ||
| async def detect_construction_safety_endpoint(request: Request, image: UploadFile = File(...)): | ||
| try: | ||
| image_bytes = await image.read() | ||
| except Exception as e: | ||
| logger.error(f"Invalid image file: {e}", exc_info=True) | ||
| raise HTTPException(status_code=400, detail="Invalid image file") | ||
|
|
||
| try: | ||
| client = get_http_client(request) | ||
| detections = await detect_construction_safety_clip(image_bytes, client=client) | ||
| return {"detections": detections} | ||
| except Exception as e: | ||
| logger.error(f"Construction safety detection error: {e}", exc_info=True) | ||
| raise HTTPException(status_code=500, detail="Internal server error") |
There was a problem hiding this comment.
New endpoints skip image validation that other CLIP endpoints perform.
The existing CLIP-based endpoints (e.g., detect_illegal_parking_endpoint at line 122, detect_street_light_endpoint at line 135) call process_uploaded_image(image) which validates and optimizes the upload. The new endpoints (and the traffic-sign / abandoned-vehicle ones added in this PR) read raw bytes with await image.read(), bypassing MIME-type checks, size limits, and format validation.
This creates an inconsistency and means these endpoints will accept arbitrarily large or malformed payloads that would be rejected elsewhere.
Proposed fix: use process_uploaded_image consistently
`@router.post`("/api/detect-public-facilities")
async def detect_public_facilities_endpoint(request: Request, image: UploadFile = File(...)):
- try:
- image_bytes = await image.read()
- except Exception as e:
- logger.error(f"Invalid image file: {e}", exc_info=True)
- raise HTTPException(status_code=400, detail="Invalid image file")
+ _, image_bytes = await process_uploaded_image(image)
try:
client = get_http_client(request)
detections = await detect_public_facilities_clip(image_bytes, client=client)
return {"detections": detections}
except Exception as e:
logger.error(f"Public facilities detection error: {e}", exc_info=True)
raise HTTPException(status_code=500, detail="Internal server error")
`@router.post`("/api/detect-construction-safety")
async def detect_construction_safety_endpoint(request: Request, image: UploadFile = File(...)):
- try:
- image_bytes = await image.read()
- except Exception as e:
- logger.error(f"Invalid image file: {e}", exc_info=True)
- raise HTTPException(status_code=400, detail="Invalid image file")
+ _, image_bytes = await process_uploaded_image(image)
try:
client = get_http_client(request)
detections = await detect_construction_safety_clip(image_bytes, client=client)
return {"detections": detections}
except Exception as e:
logger.error(f"Construction safety detection error: {e}", exc_info=True)
raise HTTPException(status_code=500, detail="Internal server error")The same issue applies to detect_traffic_sign_endpoint (line 409) and detect_abandoned_vehicle_endpoint (line 426).
📝 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.
| @router.post("/api/detect-public-facilities") | |
| async def detect_public_facilities_endpoint(request: Request, image: UploadFile = File(...)): | |
| try: | |
| image_bytes = await image.read() | |
| except Exception as e: | |
| logger.error(f"Invalid image file: {e}", exc_info=True) | |
| raise HTTPException(status_code=400, detail="Invalid image file") | |
| try: | |
| client = get_http_client(request) | |
| detections = await detect_public_facilities_clip(image_bytes, client=client) | |
| return {"detections": detections} | |
| except Exception as e: | |
| logger.error(f"Public facilities detection error: {e}", exc_info=True) | |
| raise HTTPException(status_code=500, detail="Internal server error") | |
| @router.post("/api/detect-construction-safety") | |
| async def detect_construction_safety_endpoint(request: Request, image: UploadFile = File(...)): | |
| try: | |
| image_bytes = await image.read() | |
| except Exception as e: | |
| logger.error(f"Invalid image file: {e}", exc_info=True) | |
| raise HTTPException(status_code=400, detail="Invalid image file") | |
| try: | |
| client = get_http_client(request) | |
| detections = await detect_construction_safety_clip(image_bytes, client=client) | |
| return {"detections": detections} | |
| except Exception as e: | |
| logger.error(f"Construction safety detection error: {e}", exc_info=True) | |
| raise HTTPException(status_code=500, detail="Internal server error") | |
| `@router.post`("/api/detect-public-facilities") | |
| async def detect_public_facilities_endpoint(request: Request, image: UploadFile = File(...)): | |
| _, image_bytes = await process_uploaded_image(image) | |
| try: | |
| client = get_http_client(request) | |
| detections = await detect_public_facilities_clip(image_bytes, client=client) | |
| return {"detections": detections} | |
| except Exception as e: | |
| logger.error(f"Public facilities detection error: {e}", exc_info=True) | |
| raise HTTPException(status_code=500, detail="Internal server error") | |
| `@router.post`("/api/detect-construction-safety") | |
| async def detect_construction_safety_endpoint(request: Request, image: UploadFile = File(...)): | |
| _, image_bytes = await process_uploaded_image(image) | |
| try: | |
| client = get_http_client(request) | |
| detections = await detect_construction_safety_clip(image_bytes, client=client) | |
| return {"detections": detections} | |
| except Exception as e: | |
| logger.error(f"Construction safety detection error: {e}", exc_info=True) | |
| raise HTTPException(status_code=500, detail="Internal server error") |
🧰 Tools
🪛 Ruff (0.15.0)
[warning] 443-443: Do not perform function call File in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 448-448: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
[warning] 453-453: Consider moving this statement to an else block
(TRY300)
[warning] 456-456: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
[warning] 459-459: Do not perform function call File in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 464-464: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
[warning] 469-469: Consider moving this statement to an else block
(TRY300)
[warning] 472-472: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
In `@backend/routers/detection.py` around lines 442 - 472, The new CLIP endpoints
(detect_public_facilities_endpoint, detect_construction_safety_endpoint,
detect_traffic_sign_endpoint, detect_abandoned_vehicle_endpoint) are reading raw
bytes via await image.read() and bypassing validation; update each to call
process_uploaded_image(image) (the same helper used by
detect_illegal_parking_endpoint and detect_street_light_endpoint) to perform
MIME/type, size and format checks and return validated/optimized bytes, then
pass those bytes into detect_public_facilities_clip /
detect_construction_safety_clip / detect_traffic_sign_clip /
detect_abandoned_vehicle_clip respectively; preserve the existing try/except
error handling pattern (catch invalid upload -> log and raise HTTPException 400,
other errors -> log and raise HTTPException 500).
| }); | ||
| if (videoRef.current) { | ||
| videoRef.current.srcObject = stream; | ||
| } |
There was a problem hiding this comment.
Potential camera stream leak if ref is null.
If the component unmounts between the getUserMedia resolution and line 22, videoRef.current will be null and the acquired stream's tracks will never be stopped. Consider stopping the stream in the else path.
Proposed fix
- if (videoRef.current) {
- videoRef.current.srcObject = stream;
- }
+ if (videoRef.current) {
+ videoRef.current.srcObject = stream;
+ } else {
+ stream.getTracks().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.
| }); | |
| if (videoRef.current) { | |
| videoRef.current.srcObject = stream; | |
| } | |
| }); | |
| if (videoRef.current) { | |
| videoRef.current.srcObject = stream; | |
| } else { | |
| stream.getTracks().forEach(track => track.stop()); | |
| } |
🤖 Prompt for AI Agents
In `@frontend/src/AbandonedVehicleDetector.jsx` around lines 20 - 23, When
assigning the camera stream returned from navigator.mediaDevices.getUserMedia,
handle the case where videoRef.current is null to avoid leaking tracks: if
videoRef.current is truthy set videoRef.current.srcObject = stream, otherwise
immediately stop all tracks on the stream (stream.getTracks().forEach(t =>
t.stop())). Also ensure the component's cleanup/unmount logic (the effect that
calls getUserMedia) stops the stream if it's still active so no tracks remain
running; update the code around videoRef and the getUserMedia promise to stop
tracks in the null-branch and during cleanup.
| detections.forEach(det => { | ||
| // Basic drawing logic | ||
| const label = `${det.label} ${(det.confidence * 100).toFixed(0)}%`; | ||
| const textWidth = context.measureText(label).width; | ||
|
|
||
| // Top left corner for label | ||
| const x = 10; | ||
| const y = 30; | ||
|
|
||
| context.fillStyle = 'rgba(0,0,0,0.5)'; | ||
| context.fillRect(x, y - 20, textWidth + 10, 30); | ||
|
|
||
| context.fillStyle = '#FF0000'; | ||
| context.fillText(label, x + 5, y); | ||
| }); |
There was a problem hiding this comment.
Bug: All detection labels are drawn at the same position and overlap.
Unlike ConstructionSafetyDetector which offsets each label with y = 30 + (i * 35), this component renders every detection at a fixed (10, 30). When multiple detections are returned, each label overwrites the previous one — only the last is visible.
Proposed fix
- detections.forEach(det => {
- // Basic drawing logic
- const label = `${det.label} ${(det.confidence * 100).toFixed(0)}%`;
- const textWidth = context.measureText(label).width;
-
- // Top left corner for label
- const x = 10;
- const y = 30;
+ detections.forEach((det, i) => {
+ const label = `${det.label} ${(det.confidence * 100).toFixed(0)}%`;
+ const textWidth = context.measureText(label).width;
+
+ // Stack detections vertically
+ const x = 10;
+ const y = 30 + (i * 35);📝 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.
| detections.forEach(det => { | |
| // Basic drawing logic | |
| const label = `${det.label} ${(det.confidence * 100).toFixed(0)}%`; | |
| const textWidth = context.measureText(label).width; | |
| // Top left corner for label | |
| const x = 10; | |
| const y = 30; | |
| context.fillStyle = 'rgba(0,0,0,0.5)'; | |
| context.fillRect(x, y - 20, textWidth + 10, 30); | |
| context.fillStyle = '#FF0000'; | |
| context.fillText(label, x + 5, y); | |
| }); | |
| detections.forEach((det, i) => { | |
| const label = `${det.label} ${(det.confidence * 100).toFixed(0)}%`; | |
| const textWidth = context.measureText(label).width; | |
| // Stack detections vertically | |
| const x = 10; | |
| const y = 30 + (i * 35); | |
| context.fillStyle = 'rgba(0,0,0,0.5)'; | |
| context.fillRect(x, y - 20, textWidth + 10, 30); | |
| context.fillStyle = '#FF0000'; | |
| context.fillText(label, x + 5, y); | |
| }); |
🤖 Prompt for AI Agents
In `@frontend/src/AbandonedVehicleDetector.jsx` around lines 46 - 60, The labels
are all drawn at fixed coordinates inside the detections.forEach loop, causing
overlap; change the loop to use the item index (e.g., detections.forEach((det,
i) => { ... })) and compute a per-item y offset like y = 30 + (i * 35) (mirror
ConstructionSafetyDetector's approach) so each label's y position is
incremented; keep using label, textWidth, and the same fillRect/fillText logic
but replace the constant y with the indexed y to avoid overlap.
| import React, { useRef, useState, useEffect } from 'react'; | ||
|
|
||
| const API_URL = import.meta.env.VITE_API_URL || ''; | ||
|
|
||
| const TrafficSignDetector = ({ onBack }) => { | ||
| const videoRef = useRef(null); | ||
| const canvasRef = useRef(null); | ||
| const [isDetecting, setIsDetecting] = useState(false); | ||
| const [error, setError] = useState(null); | ||
|
|
||
| const startCamera = async () => { | ||
| setError(null); | ||
| try { | ||
| const stream = await navigator.mediaDevices.getUserMedia({ | ||
| video: { | ||
| facingMode: 'environment', | ||
| width: { ideal: 640 }, | ||
| height: { ideal: 480 } | ||
| } | ||
| }); | ||
| if (videoRef.current) { | ||
| videoRef.current.srcObject = stream; | ||
| } | ||
| } catch (err) { | ||
| setError("Could not access camera: " + err.message); | ||
| setIsDetecting(false); | ||
| } | ||
| }; | ||
|
|
||
| const stopCamera = () => { | ||
| if (videoRef.current && videoRef.current.srcObject) { | ||
| const tracks = videoRef.current.srcObject.getTracks(); | ||
| tracks.forEach(track => track.stop()); | ||
| videoRef.current.srcObject = null; | ||
| } | ||
| }; | ||
|
|
||
| const drawDetections = (detections, context) => { | ||
| context.clearRect(0, 0, context.canvas.width, context.canvas.height); | ||
|
|
||
| context.strokeStyle = '#00FF00'; // Green | ||
| context.lineWidth = 4; | ||
| context.font = 'bold 18px Arial'; | ||
| context.fillStyle = '#00FF00'; | ||
|
|
||
| detections.forEach(det => { | ||
| // CLIP often returns empty box [] if it's purely classification, | ||
| // but the backend format might ensure a box or we skip drawing box if empty. | ||
| // If the box is empty, we just show text at top left or something. | ||
| // Assuming unified format returns box, or we handle empty box. | ||
|
|
||
| let x1 = 10, y1 = 30, x2 = 10, y2 = 30; | ||
| if (det.box && det.box.length === 4) { | ||
| [x1, y1, x2, y2] = det.box; | ||
| context.strokeRect(x1, y1, x2 - x1, y2 - y1); | ||
| } | ||
|
|
||
| // Draw label background | ||
| const label = `${det.label} ${(det.confidence * 100).toFixed(0)}%`; | ||
| const textWidth = context.measureText(label).width; | ||
|
|
||
| if (det.box && det.box.length === 4) { | ||
| context.fillStyle = 'rgba(0,0,0,0.5)'; | ||
| context.fillRect(x1, y1 > 20 ? y1 - 25 : y1, textWidth + 10, 25); | ||
| context.fillStyle = '#00FF00'; | ||
| context.fillText(label, x1 + 5, y1 > 20 ? y1 - 7 : y1 + 18); | ||
| } else { | ||
| // Fallback for classification without box | ||
| context.fillStyle = 'rgba(0,0,0,0.5)'; | ||
| context.fillRect(10, 10, textWidth + 10, 25); | ||
| context.fillStyle = '#00FF00'; | ||
| context.fillText(label, 15, 30); | ||
| } | ||
| }); | ||
| }; | ||
|
|
||
| const detectFrame = async () => { | ||
| if (!videoRef.current || !canvasRef.current || !isDetecting) return; | ||
|
|
||
| const video = videoRef.current; | ||
|
|
||
| if (video.readyState !== 4) return; | ||
|
|
||
| const canvas = canvasRef.current; | ||
| const context = canvas.getContext('2d'); | ||
|
|
||
| if (canvas.width !== video.videoWidth || canvas.height !== video.videoHeight) { | ||
| canvas.width = video.videoWidth; | ||
| canvas.height = video.videoHeight; | ||
| } | ||
|
|
||
| context.drawImage(video, 0, 0, canvas.width, canvas.height); | ||
|
|
||
| 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-traffic-sign`, { | ||
| method: 'POST', | ||
| body: formData | ||
| }); | ||
|
|
||
| if (response.ok) { | ||
| const data = await response.json(); | ||
| if (data.detections) { | ||
| drawDetections(data.detections, context); | ||
| } | ||
| } | ||
| } catch (err) { | ||
| console.error("Detection error:", err); | ||
| } | ||
| }, 'image/jpeg', 0.8); | ||
| }; | ||
|
|
||
| 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]); | ||
|
|
||
| return ( | ||
| <div className="mt-6 flex flex-col items-center w-full"> | ||
| <h2 className="text-xl font-semibold mb-4 text-center">Traffic Sign Detector</h2> | ||
|
|
||
| {error && <div className="bg-red-100 border border-red-400 text-red-700 px-4 py-3 rounded mb-4">{error}</div>} | ||
|
|
||
| <div className="relative w-full max-w-md bg-black rounded-lg overflow-hidden shadow-lg mb-6"> | ||
| <div className="relative"> | ||
| <video | ||
| ref={videoRef} | ||
| autoPlay | ||
| playsInline | ||
| muted | ||
| className="w-full h-auto block" | ||
| style={{ opacity: isDetecting ? 1 : 0.5 }} | ||
| /> | ||
| <canvas | ||
| ref={canvasRef} | ||
| className="absolute top-0 left-0 w-full h-full pointer-events-none" | ||
| /> | ||
| {!isDetecting && ( | ||
| <div className="absolute inset-0 flex items-center justify-center"> | ||
| <p className="text-white font-medium bg-black bg-opacity-50 px-4 py-2 rounded"> | ||
| Camera Paused | ||
| </p> | ||
| </div> | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| <button | ||
| onClick={() => setIsDetecting(!isDetecting)} | ||
| className={`w-full max-w-md py-3 px-4 rounded-lg text-white font-medium shadow-md transition transform active:scale-95 ${isDetecting ? 'bg-red-600 hover:bg-red-700' : 'bg-blue-600 hover:bg-blue-700'}`} | ||
| > | ||
| {isDetecting ? 'Stop Detection' : 'Start Live Detection'} | ||
| </button> | ||
|
|
||
| <p className="text-sm text-gray-500 mt-2 text-center max-w-md"> | ||
| Point camera at traffic signs to detect issues. | ||
| </p> | ||
|
|
||
| <button | ||
| onClick={onBack} | ||
| className="mt-6 text-gray-600 hover:text-gray-900 underline" | ||
| > | ||
| Back to Home | ||
| </button> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default TrafficSignDetector; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Heavy code duplication across all detector components.
TrafficSignDetector, PublicFacilitiesDetector, ConstructionSafetyDetector, and AbandonedVehicleDetector share ~95% identical code (camera lifecycle, frame capture, canvas overlay, UI shell). The only differences are the API endpoint URL, overlay color, title/description strings, and minor draw logic.
Extract a shared useDetectorCamera hook (or a generic <LiveDetector> component) parameterized by endpoint, color, and title. This eliminates the maintenance burden of keeping four near-identical files in sync.
🧰 Tools
🪛 Biome (2.3.14)
[error] 33-33: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@frontend/src/TrafficSignDetector.jsx` around lines 1 - 189, The component
files (TrafficSignDetector, PublicFacilitiesDetector,
ConstructionSafetyDetector, AbandonedVehicleDetector) duplicate camera
lifecycle, frame capture, canvas overlay and UI; extract a reusable hook and/or
wrapper component to remove duplication: create a hook named useDetectorCamera
that encapsulates startCamera, stopCamera, detectFrame, canvas/video refs,
interval management and error state, and a generic LiveDetector component that
accepts props (endpointUrl, overlayColor, title, description,
drawDetectionsOverride) to handle the UI and call drawDetections; refactor
TrafficSignDetector to use useDetectorCamera/LiveDetector and pass its specific
API route (`/api/detect-traffic-sign`), color and strings, and keep any
detector-specific drawing logic via a drawDetections or drawLabel prop so all
four detectors reuse the shared logic.
| { id: 'streetlight', label: t('home.issues.darkStreet'), icon: <Lightbulb size={24} />, color: 'text-slate-600', bg: 'bg-slate-50' }, | ||
| { id: 'traffic-sign', label: t('home.issues.trafficSign'), icon: <Signpost size={24} />, color: 'text-yellow-600', bg: 'bg-yellow-50' }, | ||
| { id: 'abandoned-vehicle', label: t('home.issues.abandonedVehicle'), icon: <Car size={24} />, color: 'text-gray-600', bg: 'bg-gray-50' }, | ||
| { id: 'accessibility', label: "Accessibility", icon: <Accessibility size={24} />, color: 'text-blue-600', bg: 'bg-blue-50' }, |
There was a problem hiding this comment.
Hardcoded labels bypass i18n.
The new items use hardcoded English strings ("Accessibility", "Construction", "Public Facilities") while adjacent items use t('home.issues.…'). This breaks translations for non-English locales.
This pattern also applies to previously added items (lines 100–103), so it may be a broader cleanup, but ideally new code should not add more debt.
Also applies to: 104-104, 111-111
🤖 Prompt for AI Agents
In `@frontend/src/views/Home.jsx` at line 87, Replace the hardcoded English labels
with i18n calls so translations are used: update the items that currently use
"Accessibility", "Construction", and "Public Facilities" (the objects with id
values 'accessibility', 'construction', 'public_facilities' in the array inside
Home.jsx) to call the translation function (t) with appropriate keys such as
t('home.issues.accessibility'), t('home.issues.construction'),
t('home.issues.public_facilities') respectively; also replace the other
hardcoded labels noted nearby (the items around those entries) to use the
existing t('home.issues.…') keys so all menu/issue labels are consistently
localized.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
13 issues found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="test_detection_import.py">
<violation number="1" location="test_detection_import.py:15">
P2: The second `try/except` block is missing the general `Exception` handler that the first block has. If a non-`ImportError` exception occurs during the `hf_api_service` import (e.g., a `SyntaxError` or runtime error in a transitive dependency), it will produce an unhandled traceback instead of a clean error message. Add the missing handler for consistency and robustness.</violation>
</file>
<file name="frontend/src/PublicFacilitiesDetector.jsx">
<violation number="1" location="frontend/src/PublicFacilitiesDetector.jsx:77">
P2: The overlay canvas is reused for video frame capture (`drawImage` + `toBlob`), which paints a visible still frame over the live `<video>` element every detection cycle. This creates a perceptible flash/flicker until `drawDetections` clears the canvas asynchronously. Use a separate offscreen canvas for frame capture to keep the display overlay clean.</violation>
<violation number="2" location="frontend/src/PublicFacilitiesDetector.jsx:79">
P2: Race condition: overlapping detection requests can corrupt canvas state. `setInterval` fires every 2s, but `detectFrame` starts an async chain (`toBlob` → `fetch` → `drawDetections`) that may take longer than 2s. Without a guard, multiple requests can be in-flight simultaneously and their `drawDetections` callbacks race on the shared canvas context, causing flickering or stale results overwriting newer ones. Consider adding a ref-based cooldown or a busy flag (similar to `lastSentRef` in `SmartScanner.jsx`) to skip sending a new request while one is still pending.</violation>
</file>
<file name="frontend/src/views/Home.jsx">
<violation number="1" location="frontend/src/views/Home.jsx:87">
P2: Hardcoded label string bypasses i18n. All sibling items in this category use `t()` for their labels (e.g., `t('home.issues.trafficSign')`). This item (and the other two new items: `construction-safety`, `public-facilities`) will not be translatable. Use a translation key like `t('home.issues.accessibility')` instead.</violation>
<violation number="2" location="frontend/src/views/Home.jsx:104">
P2: This item uses the same icon (`AlertTriangle`) and color scheme (`text-orange-600`/`bg-orange-50`) as the existing `grievance` item, making them visually indistinguishable in the UI. Consider using a distinct icon (e.g., `HardHat` or `Construction` from lucide-react) or a different color to differentiate construction safety from grievance management.</violation>
<violation number="3" location="frontend/src/views/Home.jsx:104">
P2: Hardcoded label string bypasses i18n. Use a translation key like `t('home.issues.constructionSafety')` for multilingual support.</violation>
<violation number="4" location="frontend/src/views/Home.jsx:111">
P2: Hardcoded label string bypasses i18n. Use a translation key like `t('home.issues.publicFacilities')` for multilingual support.</violation>
</file>
<file name="frontend/src/ConstructionSafetyDetector.jsx">
<violation number="1" location="frontend/src/ConstructionSafetyDetector.jsx:77">
P2: Using the visible overlay canvas for both frame capture and detection display causes a visible flicker every 2 seconds. `drawImage` renders an opaque video frame on the overlay canvas, which remains visible until the async fetch completes and `drawDetections` clears it. Use a separate offscreen canvas for frame capture to avoid the flash.</violation>
<violation number="2" location="frontend/src/ConstructionSafetyDetector.jsx:107">
P2: `setInterval` fires every 2 seconds regardless of whether the previous detection request has completed. If the backend is slow, overlapping requests can cause out-of-order responses and stale detection labels. Consider using `setTimeout` chaining (schedule the next call only after the current one completes) to prevent overlapping requests.</violation>
</file>
<file name="frontend/src/AbandonedVehicleDetector.jsx">
<violation number="1" location="frontend/src/AbandonedVehicleDetector.jsx:53">
P1: Bug: All detection labels are drawn at the same fixed position (`x=10, y=30`). When the API returns multiple detections, they render on top of each other and only the last one is visible. The `y` coordinate should be offset for each detection (e.g., incremented by the label height per iteration).</violation>
<violation number="2" location="frontend/src/AbandonedVehicleDetector.jsx:108">
P2: Race condition: `setInterval` fires `detectFrame` every 2s regardless of whether the previous async detection (network request + canvas draw) has completed. If a request takes >2s, the next tick's `context.drawImage` will overwrite the canvas before the prior response's `drawDetections` runs, and out-of-order responses will show stale results. Consider using a flag or `setTimeout` chain to ensure the next frame is only requested after the current one completes.</violation>
</file>
<file name="backend/routers/detection.py">
<violation number="1" location="backend/routers/detection.py:445">
P1: Missing input validation and image optimization. This endpoint reads raw bytes via `image.read()` instead of using `process_uploaded_image(image)`, skipping file size limits, MIME type validation, image resizing, and EXIF stripping that all other CLIP detection endpoints use. This opens the endpoint to oversized uploads, non-image files, and unstripped EXIF metadata (which may contain GPS coordinates).</violation>
</file>
<file name="frontend/src/TrafficSignDetector.jsx">
<violation number="1" location="frontend/src/TrafficSignDetector.jsx:121">
P1: Camera stream leak: `startCamera` is async but not cancellable. If `isDetecting` is toggled off before `getUserMedia` resolves, the returned stream is assigned to the video element after cleanup has already run, leaving an orphaned media stream (camera stays on). Use an `AbortController` or a ref-based flag to discard the stream if the effect has been cleaned up.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| // Top left corner for label | ||
| const x = 10; | ||
| const y = 30; |
There was a problem hiding this comment.
P1: Bug: All detection labels are drawn at the same fixed position (x=10, y=30). When the API returns multiple detections, they render on top of each other and only the last one is visible. The y coordinate should be offset for each detection (e.g., incremented by the label height per iteration).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/AbandonedVehicleDetector.jsx, line 53:
<comment>Bug: All detection labels are drawn at the same fixed position (`x=10, y=30`). When the API returns multiple detections, they render on top of each other and only the last one is visible. The `y` coordinate should be offset for each detection (e.g., incremented by the label height per iteration).</comment>
<file context>
@@ -0,0 +1,175 @@
+
+ // Top left corner for label
+ const x = 10;
+ const y = 30;
+
+ context.fillStyle = 'rgba(0,0,0,0.5)';
</file context>
| @router.post("/api/detect-public-facilities") | ||
| async def detect_public_facilities_endpoint(request: Request, image: UploadFile = File(...)): | ||
| try: | ||
| image_bytes = await image.read() |
There was a problem hiding this comment.
P1: Missing input validation and image optimization. This endpoint reads raw bytes via image.read() instead of using process_uploaded_image(image), skipping file size limits, MIME type validation, image resizing, and EXIF stripping that all other CLIP detection endpoints use. This opens the endpoint to oversized uploads, non-image files, and unstripped EXIF metadata (which may contain GPS coordinates).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/routers/detection.py, line 445:
<comment>Missing input validation and image optimization. This endpoint reads raw bytes via `image.read()` instead of using `process_uploaded_image(image)`, skipping file size limits, MIME type validation, image resizing, and EXIF stripping that all other CLIP detection endpoints use. This opens the endpoint to oversized uploads, non-image files, and unstripped EXIF metadata (which may contain GPS coordinates).</comment>
<file context>
@@ -436,3 +438,35 @@ async def detect_abandoned_vehicle_endpoint(request: Request, image: UploadFile
[email protected]("/api/detect-public-facilities")
+async def detect_public_facilities_endpoint(request: Request, image: UploadFile = File(...)):
+ try:
+ image_bytes = await image.read()
+ except Exception as e:
+ logger.error(f"Invalid image file: {e}", exc_info=True)
</file context>
| useEffect(() => { | ||
| let interval; | ||
| if (isDetecting) { | ||
| startCamera(); |
There was a problem hiding this comment.
P1: Camera stream leak: startCamera is async but not cancellable. If isDetecting is toggled off before getUserMedia resolves, the returned stream is assigned to the video element after cleanup has already run, leaving an orphaned media stream (camera stays on). Use an AbortController or a ref-based flag to discard the stream if the effect has been cleaned up.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/TrafficSignDetector.jsx, line 121:
<comment>Camera stream leak: `startCamera` is async but not cancellable. If `isDetecting` is toggled off before `getUserMedia` resolves, the returned stream is assigned to the video element after cleanup has already run, leaving an orphaned media stream (camera stays on). Use an `AbortController` or a ref-based flag to discard the stream if the effect has been cleaned up.</comment>
<file context>
@@ -0,0 +1,189 @@
+ useEffect(() => {
+ let interval;
+ if (isDetecting) {
+ startCamera();
+ interval = setInterval(detectFrame, 2000);
+ } else {
</file context>
| print("Successfully imported new endpoints from backend.routers.detection") | ||
| except ImportError as e: | ||
| print(f"Failed to import new endpoints: {e}") | ||
| sys.exit(1) |
There was a problem hiding this comment.
P2: The second try/except block is missing the general Exception handler that the first block has. If a non-ImportError exception occurs during the hf_api_service import (e.g., a SyntaxError or runtime error in a transitive dependency), it will produce an unhandled traceback instead of a clean error message. Add the missing handler for consistency and robustness.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test_detection_import.py, line 15:
<comment>The second `try/except` block is missing the general `Exception` handler that the first block has. If a non-`ImportError` exception occurs during the `hf_api_service` import (e.g., a `SyntaxError` or runtime error in a transitive dependency), it will produce an unhandled traceback instead of a clean error message. Add the missing handler for consistency and robustness.</comment>
<file context>
@@ -0,0 +1,28 @@
+ print("Successfully imported new endpoints from backend.routers.detection")
+except ImportError as e:
+ print(f"Failed to import new endpoints: {e}")
+ sys.exit(1)
+except Exception as e:
+ print(f"An error occurred during import: {e}")
</file context>
| canvas.height = video.videoHeight; | ||
| } | ||
|
|
||
| context.drawImage(video, 0, 0, canvas.width, canvas.height); |
There was a problem hiding this comment.
P2: The overlay canvas is reused for video frame capture (drawImage + toBlob), which paints a visible still frame over the live <video> element every detection cycle. This creates a perceptible flash/flicker until drawDetections clears the canvas asynchronously. Use a separate offscreen canvas for frame capture to keep the display overlay clean.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/PublicFacilitiesDetector.jsx, line 77:
<comment>The overlay canvas is reused for video frame capture (`drawImage` + `toBlob`), which paints a visible still frame over the live `<video>` element every detection cycle. This creates a perceptible flash/flicker until `drawDetections` clears the canvas asynchronously. Use a separate offscreen canvas for frame capture to keep the display overlay clean.</comment>
<file context>
@@ -0,0 +1,174 @@
+ canvas.height = video.videoHeight;
+ }
+
+ context.drawImage(video, 0, 0, canvas.width, canvas.height);
+
+ canvas.toBlob(async (blob) => {
</file context>
| { id: 'crowd', label: "Crowd", icon: <Users size={24} />, color: 'text-red-500', bg: 'bg-red-50' }, | ||
| { id: 'water-leak', label: "Water Leak", icon: <Waves size={24} />, color: 'text-blue-500', bg: 'bg-blue-50' }, | ||
| { id: 'waste', label: "Waste Sorter", icon: <Recycle size={24} />, color: 'text-emerald-600', bg: 'bg-emerald-50' }, | ||
| { id: 'construction-safety', label: "Construction", icon: <AlertTriangle size={24} />, color: 'text-orange-600', bg: 'bg-orange-50' }, |
There was a problem hiding this comment.
P2: Hardcoded label string bypasses i18n. Use a translation key like t('home.issues.constructionSafety') for multilingual support.
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 104:
<comment>Hardcoded label string bypasses i18n. Use a translation key like `t('home.issues.constructionSafety')` for multilingual support.</comment>
<file context>
@@ -100,12 +101,14 @@ const Home = ({ setView, fetchResponsibilityMap, recentIssues, handleUpvote, loa
{ id: 'crowd', label: "Crowd", icon: <Users size={24} />, color: 'text-red-500', bg: 'bg-red-50' },
{ id: 'water-leak', label: "Water Leak", icon: <Waves size={24} />, color: 'text-blue-500', bg: 'bg-blue-50' },
{ id: 'waste', label: "Waste Sorter", icon: <Recycle size={24} />, color: 'text-emerald-600', bg: 'bg-emerald-50' },
+ { id: 'construction-safety', label: "Construction", icon: <AlertTriangle size={24} />, color: 'text-orange-600', bg: 'bg-orange-50' },
]
},
</file context>
| { id: 'construction-safety', label: "Construction", icon: <AlertTriangle size={24} />, color: 'text-orange-600', bg: 'bg-orange-50' }, | |
| { id: 'construction-safety', label: t('home.issues.constructionSafety'), icon: <AlertTriangle size={24} />, color: 'text-orange-600', bg: 'bg-orange-50' }, |
| { id: 'streetlight', label: t('home.issues.darkStreet'), icon: <Lightbulb size={24} />, color: 'text-slate-600', bg: 'bg-slate-50' }, | ||
| { id: 'traffic-sign', label: t('home.issues.trafficSign'), icon: <Signpost size={24} />, color: 'text-yellow-600', bg: 'bg-yellow-50' }, | ||
| { id: 'abandoned-vehicle', label: t('home.issues.abandonedVehicle'), icon: <Car size={24} />, color: 'text-gray-600', bg: 'bg-gray-50' }, | ||
| { id: 'accessibility', label: "Accessibility", icon: <Accessibility size={24} />, color: 'text-blue-600', bg: 'bg-blue-50' }, |
There was a problem hiding this comment.
P2: Hardcoded label string bypasses i18n. All sibling items in this category use t() for their labels (e.g., t('home.issues.trafficSign')). This item (and the other two new items: construction-safety, public-facilities) will not be translatable. Use a translation key like t('home.issues.accessibility') instead.
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 87:
<comment>Hardcoded label string bypasses i18n. All sibling items in this category use `t()` for their labels (e.g., `t('home.issues.trafficSign')`). This item (and the other two new items: `construction-safety`, `public-facilities`) will not be translatable. Use a translation key like `t('home.issues.accessibility')` instead.</comment>
<file context>
@@ -84,6 +84,7 @@ const Home = ({ setView, fetchResponsibilityMap, recentIssues, handleUpvote, loa
{ id: 'streetlight', label: t('home.issues.darkStreet'), icon: <Lightbulb size={24} />, color: 'text-slate-600', bg: 'bg-slate-50' },
{ id: 'traffic-sign', label: t('home.issues.trafficSign'), icon: <Signpost size={24} />, color: 'text-yellow-600', bg: 'bg-yellow-50' },
{ id: 'abandoned-vehicle', label: t('home.issues.abandonedVehicle'), icon: <Car size={24} />, color: 'text-gray-600', bg: 'bg-gray-50' },
+ { id: 'accessibility', label: "Accessibility", icon: <Accessibility size={24} />, color: 'text-blue-600', bg: 'bg-blue-50' },
]
},
</file context>
| { id: 'accessibility', label: "Accessibility", icon: <Accessibility size={24} />, color: 'text-blue-600', bg: 'bg-blue-50' }, | |
| { id: 'accessibility', label: t('home.issues.accessibility'), icon: <Accessibility size={24} />, color: 'text-blue-600', bg: 'bg-blue-50' }, |
| let interval; | ||
| if (isDetecting) { | ||
| startCamera(); | ||
| interval = setInterval(detectFrame, 2000); |
There was a problem hiding this comment.
P2: setInterval fires every 2 seconds regardless of whether the previous detection request has completed. If the backend is slow, overlapping requests can cause out-of-order responses and stale detection labels. Consider using setTimeout chaining (schedule the next call only after the current one completes) to prevent overlapping requests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/ConstructionSafetyDetector.jsx, line 107:
<comment>`setInterval` fires every 2 seconds regardless of whether the previous detection request has completed. If the backend is slow, overlapping requests can cause out-of-order responses and stale detection labels. Consider using `setTimeout` chaining (schedule the next call only after the current one completes) to prevent overlapping requests.</comment>
<file context>
@@ -0,0 +1,174 @@
+ let interval;
+ if (isDetecting) {
+ startCamera();
+ interval = setInterval(detectFrame, 2000);
+ } else {
+ stopCamera();
</file context>
| canvas.height = video.videoHeight; | ||
| } | ||
|
|
||
| context.drawImage(video, 0, 0, canvas.width, canvas.height); |
There was a problem hiding this comment.
P2: Using the visible overlay canvas for both frame capture and detection display causes a visible flicker every 2 seconds. drawImage renders an opaque video frame on the overlay canvas, which remains visible until the async fetch completes and drawDetections clears it. Use a separate offscreen canvas for frame capture to avoid the flash.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/ConstructionSafetyDetector.jsx, line 77:
<comment>Using the visible overlay canvas for both frame capture and detection display causes a visible flicker every 2 seconds. `drawImage` renders an opaque video frame on the overlay canvas, which remains visible until the async fetch completes and `drawDetections` clears it. Use a separate offscreen canvas for frame capture to avoid the flash.</comment>
<file context>
@@ -0,0 +1,174 @@
+ canvas.height = video.videoHeight;
+ }
+
+ context.drawImage(video, 0, 0, canvas.width, canvas.height);
+
+ canvas.toBlob(async (blob) => {
</file context>
| let interval; | ||
| if (isDetecting) { | ||
| startCamera(); | ||
| interval = setInterval(detectFrame, 2000); |
There was a problem hiding this comment.
P2: Race condition: setInterval fires detectFrame every 2s regardless of whether the previous async detection (network request + canvas draw) has completed. If a request takes >2s, the next tick's context.drawImage will overwrite the canvas before the prior response's drawDetections runs, and out-of-order responses will show stale results. Consider using a flag or setTimeout chain to ensure the next frame is only requested after the current one completes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/AbandonedVehicleDetector.jsx, line 108:
<comment>Race condition: `setInterval` fires `detectFrame` every 2s regardless of whether the previous async detection (network request + canvas draw) has completed. If a request takes >2s, the next tick's `context.drawImage` will overwrite the canvas before the prior response's `drawDetections` runs, and out-of-order responses will show stale results. Consider using a flag or `setTimeout` chain to ensure the next frame is only requested after the current one completes.</comment>
<file context>
@@ -0,0 +1,175 @@
+ let interval;
+ if (isDetecting) {
+ startCamera();
+ interval = setInterval(detectFrame, 2000);
+ } else {
+ stopCamera();
</file context>
There was a problem hiding this comment.
Pull request overview
This PR expands the civic “detectors” feature set by adding new CLIP-based backend detectors and exposing multiple (previously missing/hidden) detector flows in the frontend UI and routing.
Changes:
- Added backend CLIP detectors + FastAPI endpoints for “Public Facilities” and “Construction Safety”.
- Added/connected frontend detector screens and routes for Traffic Sign, Abandoned Vehicle, Public Facilities, Construction Safety, and exposed Accessibility/Water Leak/Crowd/Waste in routing.
- Updated the Home view to show the new detector options in category grids.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| test_detection_import.py | Adds a script intended to validate imports for new detector endpoints/services. |
| frontend/src/views/Home.jsx | Adds new detector entries to the Home category grid. |
| frontend/src/App.jsx | Registers new detector routes and expands the navigation allow-list. |
| frontend/src/TrafficSignDetector.jsx | New live camera detector screen calling /api/detect-traffic-sign. |
| frontend/src/AbandonedVehicleDetector.jsx | New live camera detector screen calling /api/detect-abandoned-vehicle. |
| frontend/src/PublicFacilitiesDetector.jsx | New live camera detector screen calling /api/detect-public-facilities. |
| frontend/src/ConstructionSafetyDetector.jsx | New live camera detector screen calling /api/detect-construction-safety. |
| backend/routers/detection.py | Adds imports and new FastAPI endpoints for the new detectors. |
| backend/hf_api_service.py | Adds CLIP label/target definitions for public facilities and construction safety. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| useEffect(() => { | ||
| let interval; | ||
| if (isDetecting) { | ||
| startCamera(); | ||
| interval = setInterval(detectFrame, 2000); | ||
| } else { |
There was a problem hiding this comment.
Live detection is triggered on a fixed setInterval without guarding against an in-flight request. If a request takes >2s, multiple overlapping uploads can pile up, increasing backend load and causing out-of-order rendering. Add an inFlight flag / abort controller to skip starting a new detection while the previous one is still running (or schedule the next tick after completion).
| useEffect(() => { | ||
| let interval; | ||
| if (isDetecting) { | ||
| startCamera(); | ||
| interval = setInterval(detectFrame, 2000); | ||
| } else { | ||
| stopCamera(); | ||
| if (interval) clearInterval(interval); |
There was a problem hiding this comment.
Same interval-driven live detection issue: without an in-flight guard, slow network/backend responses can cause overlapping requests and unnecessary load. Add an inFlight/abort mechanism so only one detection runs at a time.
| useEffect(() => { | ||
| let interval; | ||
| if (isDetecting) { | ||
| startCamera(); | ||
| interval = setInterval(detectFrame, 2000); | ||
| } else { | ||
| stopCamera(); |
There was a problem hiding this comment.
Same interval-driven live detection issue: without an in-flight guard, slow responses can cause overlapping requests and unnecessary load. Add an inFlight/abort mechanism so only one detection runs at a time.
| async def detect_public_facilities_endpoint(request: Request, image: UploadFile = File(...)): | ||
| try: | ||
| image_bytes = await image.read() | ||
| except Exception as e: | ||
| logger.error(f"Invalid image file: {e}", exc_info=True) | ||
| raise HTTPException(status_code=400, detail="Invalid image file") |
There was a problem hiding this comment.
These new endpoints read raw bytes via await image.read() and skip the repo’s existing process_uploaded_image(...) validation/optimization used by most detection endpoints. This can allow very large uploads, increases memory pressure, and bypasses consistent image verification. Use process_uploaded_image(image) here (and return the optimized image_bytes) to align with the rest of the router.
| async def detect_construction_safety_endpoint(request: Request, image: UploadFile = File(...)): | ||
| try: | ||
| image_bytes = await image.read() | ||
| except Exception as e: | ||
| logger.error(f"Invalid image file: {e}", exc_info=True) | ||
| raise HTTPException(status_code=400, detail="Invalid image file") | ||
|
|
There was a problem hiding this comment.
Same as the public-facilities endpoint: reading raw bytes and skipping process_uploaded_image(...) bypasses centralized validation/optimization and size controls. Prefer process_uploaded_image(image) for consistent handling of uploads.
| interval = setInterval(detectFrame, 2000); | ||
| } else { | ||
| stopCamera(); | ||
| if (interval) clearInterval(interval); |
There was a problem hiding this comment.
This use of variable 'interval' always evaluates to false.
| if (interval) clearInterval(interval); |
| interval = setInterval(detectFrame, 2000); | ||
| } else { | ||
| stopCamera(); | ||
| if (interval) clearInterval(interval); |
There was a problem hiding this comment.
This use of variable 'interval' always evaluates to false.
| if (interval) clearInterval(interval); |
| interval = setInterval(detectFrame, 2000); | ||
| } else { | ||
| stopCamera(); | ||
| if (interval) clearInterval(interval); |
There was a problem hiding this comment.
This use of variable 'interval' always evaluates to false.
| if (interval) clearInterval(interval); |
| from backend.routers.detection import ( | ||
| detect_public_facilities_endpoint, | ||
| detect_construction_safety_endpoint | ||
| ) |
There was a problem hiding this comment.
Import of 'detect_public_facilities_endpoint' is not used.
Import of 'detect_construction_safety_endpoint' is not used.
| from backend.hf_api_service import ( | ||
| detect_public_facilities_clip, | ||
| detect_construction_safety_clip | ||
| ) |
There was a problem hiding this comment.
Import of 'detect_public_facilities_clip' is not used.
Import of 'detect_construction_safety_clip' is not used.
| ) | |
| ) | |
| _imported_clip_functions = [ | |
| detect_public_facilities_clip, | |
| detect_construction_safety_clip, | |
| ] |
- Removed `frontend/public/_redirects` which was conflicting with `netlify.toml` and causing deployment verification failures. - Verified that required components (`TrafficSignDetector`, `AbandonedVehicleDetector`, etc.) exist and are correctly imported in `App.jsx`. - Verified that `frontend/src/views/Home.jsx` has all necessary icon imports. - Confirmed local build passes successfully with `npm run build`. Co-authored-by: RohanExploit <[email protected]>
- Created `frontend/netlify.toml` to support deployments where the Base Directory is set to `frontend`. - This ensures that if the root `netlify.toml` is ignored, the frontend build command and publish directory are still correctly defined. - Verified that all required detector components (`TrafficSign`, `AbandonedVehicle`, `PublicFacilities`, `ConstructionSafety`, etc.) exist and are imported correctly. - Confirmed that `frontend/public/_redirects` is removed to avoid conflicts. - Verified local build with `npm run build` in `frontend/`. Co-authored-by: RohanExploit <[email protected]>
🔍 Quality Reminder |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="frontend/netlify.toml">
<violation number="1" location="frontend/netlify.toml:24">
P2: `X-XSS-Protection: 1; mode=block` is deprecated and can actually *introduce* XSS vulnerabilities in otherwise safe websites (per OWASP and MDN). The XSS Auditor has been removed from all major browsers. Either remove this header entirely or explicitly disable it with `"0"`, and consider adding a `Content-Security-Policy` header instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Removed `frontend/netlify.toml` to avoid configuration ambiguity. - Updated root `netlify.toml` to change `X-Frame-Options` to `SAMEORIGIN` to fix "Header rules" CI failure. - Restored `frontend/public/_redirects` and `frontend/public/_headers` as file-based fallbacks, ensuring robust deployment even if toml config is partial. - Confirmed all referenced frontend components exist and backend endpoints are verified. Co-authored-by: RohanExploit <[email protected]>
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="netlify.toml">
<violation number="1" location="netlify.toml:22">
P2: Security header `X-Frame-Options` relaxed from `DENY` to `SAMEORIGIN` without apparent justification. This PR adds detection features and UI routes — none of which typically require same-origin framing. If the app doesn't embed itself in iframes, keep `DENY` for stronger clickjacking protection. If framing is needed (e.g., for an internal iframe-based component), please document the reason.</violation>
</file>
<file name="frontend/public/_headers">
<violation number="1" location="frontend/public/_headers:4">
P2: `X-XSS-Protection: 1; mode=block` is deprecated and can paradoxically introduce XSS vulnerabilities in otherwise safe websites (per MDN and OWASP). Modern browsers have removed their XSS Auditor. Set this to `0` and rely on a `Content-Security-Policy` header instead for XSS protection.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| for = "/*" | ||
| [headers.values] | ||
| X-Frame-Options = "DENY" | ||
| X-Frame-Options = "SAMEORIGIN" |
There was a problem hiding this comment.
P2: Security header X-Frame-Options relaxed from DENY to SAMEORIGIN without apparent justification. This PR adds detection features and UI routes — none of which typically require same-origin framing. If the app doesn't embed itself in iframes, keep DENY for stronger clickjacking protection. If framing is needed (e.g., for an internal iframe-based component), please document the reason.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At netlify.toml, line 22:
<comment>Security header `X-Frame-Options` relaxed from `DENY` to `SAMEORIGIN` without apparent justification. This PR adds detection features and UI routes — none of which typically require same-origin framing. If the app doesn't embed itself in iframes, keep `DENY` for stronger clickjacking protection. If framing is needed (e.g., for an internal iframe-based component), please document the reason.</comment>
<file context>
@@ -22,7 +19,7 @@
for = "/*"
[headers.values]
- X-Frame-Options = "DENY"
+ X-Frame-Options = "SAMEORIGIN"
X-Content-Type-Options = "nosniff"
X-XSS-Protection = "1; mode=block"
</file context>
| X-Frame-Options = "SAMEORIGIN" | |
| X-Frame-Options = "DENY" |
| /* | ||
| X-Frame-Options: SAMEORIGIN | ||
| X-Content-Type-Options: nosniff | ||
| X-XSS-Protection: 1; mode=block |
There was a problem hiding this comment.
P2: X-XSS-Protection: 1; mode=block is deprecated and can paradoxically introduce XSS vulnerabilities in otherwise safe websites (per MDN and OWASP). Modern browsers have removed their XSS Auditor. Set this to 0 and rely on a Content-Security-Policy header instead for XSS protection.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/public/_headers, line 4:
<comment>`X-XSS-Protection: 1; mode=block` is deprecated and can paradoxically introduce XSS vulnerabilities in otherwise safe websites (per MDN and OWASP). Modern browsers have removed their XSS Auditor. Set this to `0` and rely on a `Content-Security-Policy` header instead for XSS protection.</comment>
<file context>
@@ -0,0 +1,5 @@
+/*
+ X-Frame-Options: SAMEORIGIN
+ X-Content-Type-Options: nosniff
+ X-XSS-Protection: 1; mode=block
+ Referrer-Policy: strict-origin-when-cross-origin
</file context>
1 participant
Implemented new detection features inspired by western public grievance apps.
PR created automatically by Jules for task 13211897059197172537 started by @RohanExploit
Summary by cubic
Adds live CLIP detectors for Public Facilities, Construction Safety, Traffic Signs, and Abandoned Vehicles, with new routes and home entries. Simplifies Netlify config and relaxes security headers to fix deployment.
New Features
Bug Fixes
Written for commit 65b517c. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests