Refactor Vandalism to CLIP and Add Civic Eye UI#315
Refactor Vandalism to CLIP and Add Civic Eye UI#315RohanExploit wants to merge 4 commits intomainfrom
Conversation
- Backend: Replaced local ML-based vandalism detection with Hugging Face CLIP API (`detect_vandalism_clip`) to reduce resource usage. - Backend: Updated `/api/detect-vandalism` to use the new lightweight service. - Frontend: Added `CivicEyeDetector` component for safety/cleanliness scanning. - Frontend: Registered `/civic-eye` route. - Frontend: Enhanced `CameraCheckModal` with snapshot verification. - Tests: Updated `tests/test_vandalism.py` to mock the new API service. Co-authored-by: RohanExploit <178623867+RohanExploit@users.noreply.github.com>
|
👋 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. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a new HF-API CLIP vandalism detector and switches the backend endpoint to use it; introduces a CivicEyeDetector React component with camera capture/analysis flow; updates related tests; adds a Playwright UI verification script; and adds frontend security headers, redirects, and Netlify config changes. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend as "CivicEyeDetector (Frontend)"
participant Camera as "Browser Camera API"
participant Backend as "Backend Service"
participant HF as "HuggingFace API"
User->>Frontend: Click Capture
Frontend->>Camera: requestVideoStream()
Camera-->>Frontend: Video Stream
User->>Frontend: Click Analyze
Frontend->>Frontend: captureFrame() → blob
Frontend->>Backend: POST /analyze (image blob)
Backend->>HF: detect_vandalism_clip request
HF-->>Backend: classification results
Backend-->>Frontend: analysis results (scores)
Frontend->>Camera: stopCamera()
Frontend-->>User: Render results & actions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@verification/verify_ui.py`:
- Line 29: Replace the hardcoded absolute screenshot path in the page.screenshot
calls with a configurable, portable path: import os or pathlib at top of
verification/verify_ui.py, read a SCREENSHOT_DIR environment variable (fallback
to a relative directory like "verification" or Path(__file__).parent /
"verification"), ensure the directory exists, then build the screenshot
filenames (e.g., "civic_eye.png") via os.path.join or Path / operator and pass
that constructed path to page.screenshot; apply the same change to the other
page.screenshot usages referenced around the other calls (lines ~48 and ~60) so
all screenshots use the configurable relative/env-driven path.
🧹 Nitpick comments (2)
verification/verify_ui.py (2)
44-46: Prefer Playwright's waiting mechanisms overtime.sleep.Using
time.sleep(2)is fragile and can cause flaky tests. Consider using Playwright's built-in wait methods for more reliable timing.♻️ Suggested improvement
- # Wait for active status (fake media stream should be fast) - time.sleep(2) + # Wait for active status indicator + page.wait_for_selector("text=Camera is working correctly!", timeout=5000)
58-61: Consider adding context cleanup for completeness.While
browser.close()will clean up resources, explicitly closing the context first is good practice for completeness.♻️ Suggested improvement
except Exception as e: print(f"Error: {e}") - page.screenshot(path="/home/jules/verification/error.png") + page.screenshot(path=os.path.join(SCREENSHOT_DIR, "error.png")) finally: + context.close() browser.close()
|
|
||
| # Screenshot Civic Eye | ||
| print("Taking screenshot 1...") | ||
| page.screenshot(path="/home/jules/verification/civic_eye.png") |
There was a problem hiding this comment.
Hardcoded screenshot paths limit portability.
The absolute path /home/jules/verification/ will fail on other machines or CI environments. Consider using a relative path or environment variable.
🔧 Proposed fix using relative paths
+import os
+
+# At the top of the file or inside the function
+SCREENSHOT_DIR = os.environ.get("VERIFICATION_SCREENSHOT_DIR", "./verification_screenshots")
+os.makedirs(SCREENSHOT_DIR, exist_ok=True)
+
# Then update screenshot paths:
- page.screenshot(path="/home/jules/verification/civic_eye.png")
+ page.screenshot(path=os.path.join(SCREENSHOT_DIR, "civic_eye.png"))Apply similar changes to lines 48 and 60.
🤖 Prompt for AI Agents
In `@verification/verify_ui.py` at line 29, Replace the hardcoded absolute
screenshot path in the page.screenshot calls with a configurable, portable path:
import os or pathlib at top of verification/verify_ui.py, read a SCREENSHOT_DIR
environment variable (fallback to a relative directory like "verification" or
Path(__file__).parent / "verification"), ensure the directory exists, then build
the screenshot filenames (e.g., "civic_eye.png") via os.path.join or Path /
operator and pass that constructed path to page.screenshot; apply the same
change to the other page.screenshot usages referenced around the other calls
(lines ~48 and ~60) so all screenshots use the configurable relative/env-driven
path.
There was a problem hiding this comment.
Pull request overview
This PR shifts vandalism detection to a lightweight Hugging Face CLIP-based implementation and introduces a new “Civic Eye” scanning UI, along with an enhanced Camera Diagnostics modal and an automated UI verification script.
Changes:
- Backend: switch
/api/detect-vandalismto usedetect_vandalism_clip(CLIP/HF API) and adddetect_vandalism_cliphelper inhf_api_service.py. - Frontend: add “Civic Eye” detector route/UI and extend Camera Diagnostics with a “Test Snapshot” flow.
- Tooling/tests: add a Playwright UI verification script; update vandalism unit test mocks to target the new detector function.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| verification/verify_ui.py | Adds Playwright-based UI verification for Civic Eye navigation and Camera Diagnostics modal. |
| tests/test_vandalism.py | Updates mocks to patch the new CLIP-based vandalism detector. |
| frontend/src/views/Home.jsx | Enhances Camera Diagnostics with snapshot/retake UI and capture logic. |
| frontend/src/CivicEyeDetector.jsx | Implements the Civic Eye scanner UI and “Report” navigation flow. |
| frontend/src/App.jsx | Registers the new civic-eye detector route for lazy loading. |
| backend/main.py | Routes vandalism detection through the CLIP/HF API implementation. |
| backend/hf_api_service.py | Adds detect_vandalism_clip using the generic CLIP zero-shot helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| print("Waiting for Civic Eye scanner...") | ||
| # The title in DetectorWrapper is "Civic Eye Scanner" | ||
| expect(page.get_by_text("Civic Eye Scanner")).to_be_visible() | ||
|
|
There was a problem hiding this comment.
The UI wrapper renders the heading as "{title} Scanner" (currently title={path.replace('-', ' ')}), which results in "civic eye Scanner" (lowercase) for the new route. This script waits for "Civic Eye Scanner", so it will fail. Update the selector to be case-insensitive (or match the actual heading role/name), or update the app title generation to use Title Case consistently.
| # Screenshot Civic Eye | ||
| print("Taking screenshot 1...") | ||
| page.screenshot(path="/home/jules/verification/civic_eye.png") | ||
|
|
There was a problem hiding this comment.
These screenshots are written to hard-coded absolute paths under /home/jules/..., which will fail outside that specific environment (e.g., CI, other dev machines). Use a repo-relative output directory (e.g., under verification/), or make the output path configurable via env var/CLI arg and ensure the directory exists before writing.
| from playwright.sync_api import sync_playwright, expect | ||
| import time | ||
| import os |
There was a problem hiding this comment.
os is imported but never used in this script; please remove it (or use it to build portable screenshot paths as suggested).
| # Wait for active status (fake media stream should be fast) | ||
| time.sleep(2) | ||
|
|
There was a problem hiding this comment.
Using time.sleep(2) will make this verification flaky on slower machines/CI. Prefer waiting for a deterministic UI signal (e.g., the "Camera is working correctly!" text or the "Test Snapshot" button) with Playwright expect(...).to_be_visible() before taking the screenshot.
| const takeSnapshot = () => { | ||
| if (videoRef.current && canvasRef.current) { | ||
| const video = videoRef.current; | ||
| const canvas = canvasRef.current; | ||
| canvas.width = video.videoWidth; | ||
| canvas.height = video.videoHeight; | ||
| canvas.getContext('2d').drawImage(video, 0, 0); | ||
| setSnapshot(canvas.toDataURL('image/jpeg')); | ||
| } |
There was a problem hiding this comment.
takeSnapshot() can run before the video has loaded metadata, leaving video.videoWidth/videoHeight as 0 and producing a blank/0x0 snapshot. Other detectors in this repo gate capture on video.readyState === 4 before using videoWidth (e.g., frontend/src/PotholeDetector.jsx:64-66). Consider checking video.readyState and non-zero dimensions (or waiting for loadedmetadata) before enabling/capturing the snapshot.
| const captureAndAnalyze = async () => { | ||
| if (!videoRef.current || !canvasRef.current) return; | ||
|
|
||
| setAnalyzing(true); | ||
| setResult(null); | ||
| setError(null); | ||
|
|
||
| const video = videoRef.current; | ||
| const canvas = canvasRef.current; | ||
| const context = canvas.getContext('2d'); | ||
|
|
||
| canvas.width = video.videoWidth; | ||
| canvas.height = video.videoHeight; | ||
| context.drawImage(video, 0, 0); | ||
| 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); |
There was a problem hiding this comment.
captureAndAnalyze() does not verify the video is ready before reading video.videoWidth/videoHeight; if metadata isn't loaded yet, this can capture a blank/0x0 frame. Other detectors in this codebase early-return when video.readyState !== 4 before capturing (e.g., frontend/src/PotholeDetector.jsx:64-66). Add a similar readiness guard (and/or disable the capture button until ready).
| <button onClick={onBack} className="text-gray-600 font-medium"> | ||
| ← Back | ||
| </button> |
There was a problem hiding this comment.
The Back button calls onBack, but this component is mounted as <Component /> inside DetectorWrapper (no props passed), so onBack will be undefined and clicking Back will throw at runtime. Either remove this internal header and rely on DetectorWrapper's Back control, or pass an onBack prop from App.jsx (or use navigate('/') directly here).
| from backend.local_ml_service import ( | ||
| detect_infrastructure_local, | ||
| detect_flooding_local, | ||
| detect_vandalism_local, | ||
| get_detection_status | ||
| ) |
There was a problem hiding this comment.
backend/tests/test_detection_bytes.py patches backend.main.detect_vandalism_local, but this PR removes detect_vandalism_local from backend.main imports. That test will now error during patching. Either update the test to patch backend.main.detect_vandalism_clip, or re-export an alias in backend.main for backward-compatible mocking.
- Created `frontend/public/_redirects` for SPA routing. - Created `frontend/public/_headers` for security headers. - These files are required by Netlify CI/CD checks and ensure proper deployment. Co-authored-by: RohanExploit <178623867+RohanExploit@users.noreply.github.com>
- Updated `netlify.toml` to use `base = "frontend"` setting. - Simplified build command to `npm install && npm run build`. - Set publish directory to `dist` (relative to base). - This ensures Netlify correctly handles the frontend subdirectory structure and dependency caching. Co-authored-by: RohanExploit <178623867+RohanExploit@users.noreply.github.com>
🔍 Quality Reminder |
- Copied `_redirects` and `_headers` to the repository root. - This ensures Netlify detects the configuration files regardless of the `base` directory setting or build context issues. - Previously, these files were only in `frontend/public`, which might not have been correctly picked up by the Netlify build bot's initial scan in this specific monorepo setup. Co-authored-by: RohanExploit <178623867+RohanExploit@users.noreply.github.com>
This PR implements requested features to make the application lighter and more feature-rich:
PR created automatically by Jules for task 7106235104142574736 started by @RohanExploit
Summary by CodeRabbit
New Features
Improvements
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.