Add Construction Safety and Playground Damage Detection Features#483
Open
RohanExploit wants to merge 4 commits intomainfrom
Open
Add Construction Safety and Playground Damage Detection Features#483RohanExploit wants to merge 4 commits intomainfrom
RohanExploit wants to merge 4 commits intomainfrom
Conversation
- Added `detect_construction_safety_clip` and `detect_playground_damage_clip` to `backend/hf_api_service.py` using Hugging Face CLIP inference. - Exposed new endpoints `/api/detect-construction-safety` and `/api/detect-playground-damage` in `backend/routers/detection.py`. - Created frontend components `ConstructionSafetyDetector.jsx` and `PlaygroundDetector.jsx`. - Integrated new detectors into `frontend/src/api/detectors.js` and `frontend/src/App.jsx`. - Added new "Public Safety" category to `frontend/src/views/Home.jsx` with the new detection features. - Added backend tests in `backend/tests/test_new_detectors.py`.
Contributor
|
👋 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 canceled.
|
🙏 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 CLIP-based detectors (construction safety and playground damage) with backend detection functions, cached router endpoints, frontend detector views and API bindings, tests, Playwright verification, and dependency updates for translation. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Browser)
participant UI as Detector Component
participant API as Backend Router
participant Cache as Cache Layer
participant HFService as HF API Service
participant HFClip as HuggingFace CLIP
User->>UI: Capture image (webcam)
UI->>API: POST /api/detect-{type} (FormData)
API->>API: Validate & extract image bytes
API->>Cache: lookup(image_hash)
alt cache hit
Cache-->>API: cached detections
else cache miss
API->>HFService: detect_{type}_clip(image_bytes)
HFService->>HFClip: run CLIP inference
HFClip-->>HFService: labels & scores
HFService-->>API: detections
API->>Cache: store(image_hash, detections)
end
API-->>UI: detections JSON
UI->>User: render overlay (labels & confidence)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/tests/test_new_detectors.py">
<violation number="1" location="backend/tests/test_new_detectors.py:65">
P2: Fix the indentation on the nested `with TestClient(app)` block; the extra leading space will cause an IndentationError.</violation>
</file>
<file name="frontend/src/views/Home.jsx">
<violation number="1" location="frontend/src/views/Home.jsx:110">
P3: Localize the new item labels with translation keys to keep this section consistent with the rest of the UI.</violation>
</file>
<file name="frontend/src/PlaygroundDetector.jsx">
<violation number="1" location="frontend/src/PlaygroundDetector.jsx:21">
P2: Guard against webcamRef.current being null before calling getScreenshot to avoid a runtime crash when capture is pressed too early.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
backend/tests/test_new_detectors.py
Outdated
| mock_client = AsyncMock() | ||
| # Patch get_http_client in detection router to return our mock | ||
| with patch("backend.routers.detection.get_http_client", return_value=mock_client): | ||
| with TestClient(app) as c: |
Contributor
There was a problem hiding this comment.
P2: Fix the indentation on the nested with TestClient(app) block; the extra leading space will cause an IndentationError.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/tests/test_new_detectors.py, line 65:
<comment>Fix the indentation on the nested `with TestClient(app)` block; the extra leading space will cause an IndentationError.</comment>
<file context>
@@ -32,21 +32,39 @@
+ mock_client = AsyncMock()
+ # Patch get_http_client in detection router to return our mock
+ with patch("backend.routers.detection.get_http_client", return_value=mock_client):
+ with TestClient(app) as c:
+ c.app.state.http_client = mock_client
+ yield c
</file context>
Suggested change
| with TestClient(app) as c: | |
| with TestClient(app) as c: |
- Added missing dependencies to `backend/requirements-render.txt` and `backend/requirements.txt`:
- `pywebpush`: Required for push notifications in `backend/tasks.py`.
- `SpeechRecognition`, `pydub`, `langdetect`: Required for voice features in `backend/voice_service.py`.
- `deep-translator`: Added as a replacement for `googletrans`.
- Switched from `googletrans` to `deep-translator` in `backend/voice_service.py` to resolve version conflicts with `httpx` and `httpcore` (which clashed with `python-telegram-bot`).
- Verified frontend build passes locally.
- Verified backend code imports successfully.
![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
🧹 Nitpick comments (3)
frontend_verification/verify_detectors.py (1)
17-20: Avoid hardcoded credentials in verification automation.Move login credentials to environment variables so the script is portable and safer for CI/local runs.
Suggested patch
from playwright.sync_api import sync_playwright import time +import os @@ - if "login" in page.url: - page.fill('input[type="email"]', 'test@example.com') - page.fill('input[type="password"]', 'password') - page.click('button:has-text("Login")') - time.sleep(2) + if "login" in page.url: + email = os.getenv("E2E_EMAIL") + password = os.getenv("E2E_PASSWORD") + if email and password: + page.fill('input[type="email"]', email) + page.fill('input[type="password"]', password) + page.click('button:has-text("Login")') + time.sleep(2)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend_verification/verify_detectors.py` around lines 17 - 20, Replace hardcoded credentials used in the Playwright steps (the page.fill calls for 'input[type="email"]' and 'input[type="password"]' before page.click('button:has-text("Login")')) with values read from environment variables (e.g., via os.environ.get or dotenv) and fail-fast or raise a clear error if they are missing; update any setup or test harness to document the required env vars and remove the literal 'test@example.com' and 'password' strings so CI/local runs can supply credentials securely.frontend/src/ConstructionSafetyDetector.jsx (2)
94-99: Consider adding defensive checks for detection data.If the API response contains malformed detection objects (missing
labelorconfidence), the rendering could fail or display "NaN%". A defensive check ensures graceful degradation.🛡️ Proposed fix
{detections.map((d, idx) => ( - <li key={idx} className="flex justify-between text-sm font-medium"> + <li key={d.label ?? idx} className="flex justify-between text-sm font-medium"> <span>{d.label}</span> - <span className="text-gray-400">{(d.confidence * 100).toFixed(0)}%</span> + <span className="text-gray-400"> + {typeof d.confidence === 'number' ? `${(d.confidence * 100).toFixed(0)}%` : 'N/A'} + </span> </li> ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/ConstructionSafetyDetector.jsx` around lines 94 - 99, The detections rendering in ConstructionSafetyDetector.jsx currently assumes every detection has valid label and confidence; update the code that maps over detections (the detections.map callback) to defensively handle malformed objects by filtering out or skipping items missing a string label, and normalizing confidence via Number(d.confidence) with a fallback (e.g., 0) before multiplying/formatting so you never render "NaN%"; ensure you still display a sensible placeholder or skip the item if label is missing and keep the existing key usage (idx) intact.
48-55: Consider replacingalert()with inline UI feedback.Using browser
alert()blocks the UI thread and provides a jarring experience. Consider using inline state-based messaging or a toast notification system for a more polished UX.💡 Example approach using inline state
+ const [message, setMessage] = useState(null); // In detectSafety: if (data.detections && data.detections.length > 0) { setDetections(data.detections); + setMessage(null); } else { - alert("No safety violations detected."); + setMessage({ type: 'info', text: 'No safety violations detected.' }); } // ... } catch (error) { console.error("Error:", error); - alert("An error occurred during detection."); + setMessage({ type: 'error', text: 'An error occurred during detection.' }); }Then render
messageinline in the JSX when present.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/ConstructionSafetyDetector.jsx` around lines 48 - 55, Replace blocking alert() calls in the ConstructionSafetyDetector component with state-driven inline UI feedback: add a message state (e.g., message and setMessage via useState in the ConstructionSafetyDetector component), call setMessage("No safety violations detected.") instead of alert in the else branch that checks data.detections, and call setMessage("An error occurred during detection.") (or include error details) inside the catch block instead of alert; then render the message in the component JSX (or hook into your toast/notification component) so feedback is non-blocking and part of the UI while keeping existing setDetections logic intact.
🤖 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/tests/test_new_detectors.py`:
- Around line 49-67: The fixture `client` currently only patches
`backend.routers.detection.get_http_client`, but `_get_cached_result` uses the
global `backend.dependencies.SHARED_HTTP_CLIENT` and tests can leak cached
state; update the fixture to also patch
`backend.dependencies.SHARED_HTTP_CLIENT` to return the same `mock_client` (or
set `c.app.state.http_client` from that mock) and ensure any in-memory cache
used by the cached detector endpoints is cleared/reset before yielding (e.g.,
reset the shared cache or call its clear/reset helper) so each test gets a fresh
client and no cross-test cache leakage.
In `@frontend/src/ConstructionSafetyDetector.jsx`:
- Line 85: The onUserMediaError handler currently swallows the actual error;
update the onUserMediaError callback (where setCameraError is called) to include
the original error information — e.g., stringify or append err.message/err.name
— so the state contains the descriptive error and/or also call
console.error(err) for debugging; locate the onUserMediaError prop passed to the
camera/video component and change the inline arrow function to include the err
details when calling setCameraError and/or logging.
- Around line 20-23: The capture callback currently calls
webcamRef.current.getScreenshot() without ensuring webcamRef.current exists;
update the capture function to null-check webcamRef (webcamRef.current) before
calling getScreenshot() and only call setImgSrc(imageSrc) when imageSrc is
non-null, returning early or handling the error case if webcamRef.current is
falsy. Locate the capture useCallback and add a guard like if
(!webcamRef?.current) return (or handle error) and ensure imageSrc is validated
before calling setImgSrc.
In `@frontend/src/PlaygroundDetector.jsx`:
- Around line 109-114: The capture button in PlaygroundDetector.jsx is icon-only
and missing an accessible name; update the button element (the one with
onClick={capture}) to include an accessible label such as aria-label="Capture
photo" or add visually-hidden text inside the button (e.g., a span with
screen-reader-only class) so assistive technologies can announce its purpose;
ensure the label is descriptive and matches the capture handler name/behavior.
In `@frontend/src/views/Home.jsx`:
- Around line 107-111: The Public Safety block in Home.jsx uses hard-coded label
strings and a non-pattern title; update the i18n usage so the title and item
labels use translation keys instead of literals. Replace title: t('Public
Safety') with a namespaced key such as t('publicSafety.title') and change the
item labels for ids 'construction-safety' and 'playground-damage' to use
t('publicSafety.constructionSafety') and t('publicSafety.playgroundDamage')
(keep the id, icon, color and bg fields unchanged) so the block follows the
existing i18n key pattern.
---
Nitpick comments:
In `@frontend_verification/verify_detectors.py`:
- Around line 17-20: Replace hardcoded credentials used in the Playwright steps
(the page.fill calls for 'input[type="email"]' and 'input[type="password"]'
before page.click('button:has-text("Login")')) with values read from environment
variables (e.g., via os.environ.get or dotenv) and fail-fast or raise a clear
error if they are missing; update any setup or test harness to document the
required env vars and remove the literal 'test@example.com' and 'password'
strings so CI/local runs can supply credentials securely.
In `@frontend/src/ConstructionSafetyDetector.jsx`:
- Around line 94-99: The detections rendering in ConstructionSafetyDetector.jsx
currently assumes every detection has valid label and confidence; update the
code that maps over detections (the detections.map callback) to defensively
handle malformed objects by filtering out or skipping items missing a string
label, and normalizing confidence via Number(d.confidence) with a fallback
(e.g., 0) before multiplying/formatting so you never render "NaN%"; ensure you
still display a sensible placeholder or skip the item if label is missing and
keep the existing key usage (idx) intact.
- Around line 48-55: Replace blocking alert() calls in the
ConstructionSafetyDetector component with state-driven inline UI feedback: add a
message state (e.g., message and setMessage via useState in the
ConstructionSafetyDetector component), call setMessage("No safety violations
detected.") instead of alert in the else branch that checks data.detections, and
call setMessage("An error occurred during detection.") (or include error
details) inside the catch block instead of alert; then render the message in the
component JSX (or hook into your toast/notification component) so feedback is
non-blocking and part of the UI while keeping existing setDetections logic
intact.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
frontend_verification/construction_safety.pngis excluded by!**/*.pngfrontend_verification/home_categories.pngis excluded by!**/*.pngfrontend_verification/playground_damage.pngis excluded by!**/*.png
📒 Files selected for processing (9)
backend/hf_api_service.pybackend/routers/detection.pybackend/tests/test_new_detectors.pyfrontend/src/App.jsxfrontend/src/ConstructionSafetyDetector.jsxfrontend/src/PlaygroundDetector.jsxfrontend/src/api/detectors.jsfrontend/src/views/Home.jsxfrontend_verification/verify_detectors.py
backend/tests/test_new_detectors.py
Outdated
Comment on lines
+49
to
+67
| @pytest.fixture | ||
| def client(): | ||
| mock_client = AsyncMock() | ||
| # Patch get_http_client in detection router to return our mock | ||
| with patch("backend.routers.detection.get_http_client", return_value=mock_client): | ||
| with TestClient(app) as c: | ||
| c.app.state.http_client = mock_client | ||
| yield c | ||
| # We need to successfully import backend.main to get 'app' | ||
| # We will wrap the import in a patch context to mock side effects | ||
|
|
||
| with patch("backend.ai_factory.create_all_ai_services") as mock_create: | ||
| mock_action = AsyncMock() | ||
| mock_chat = AsyncMock() | ||
| mock_summary = AsyncMock() | ||
| mock_create.return_value = (mock_action, mock_chat, mock_summary) | ||
|
|
||
| from backend.main import app | ||
|
|
||
| mock_client = AsyncMock() | ||
| # Patch get_http_client in detection router to return our mock | ||
| with patch("backend.routers.detection.get_http_client", return_value=mock_client): | ||
| with TestClient(app) as c: | ||
| c.app.state.http_client = mock_client | ||
| yield c |
There was a problem hiding this comment.
Fixture does not reliably mock the client path used by cached detector endpoints.
These endpoint tests go through _get_cached_result, which uses backend.dependencies.SHARED_HTTP_CLIENT. Patching only get_http_client can leave calls unmocked, and the global cache can leak state across tests.
Suggested patch
`@pytest.fixture`
def client():
@@
- mock_client = AsyncMock()
- # Patch get_http_client in detection router to return our mock
- with patch("backend.routers.detection.get_http_client", return_value=mock_client):
- with TestClient(app) as c:
- c.app.state.http_client = mock_client
- yield c
+ from backend import dependencies as deps
+ from backend.routers import detection as detection_router
+
+ mock_client = AsyncMock()
+ detection_router._cache_store.clear()
+
+ with patch.object(deps, "SHARED_HTTP_CLIENT", mock_client), \
+ patch("backend.routers.detection.get_http_client", return_value=mock_client):
+ with TestClient(app) as c:
+ c.app.state.http_client = mock_client
+ yield c
+
+ detection_router._cache_store.clear()📝 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.
Suggested change
| @pytest.fixture | |
| def client(): | |
| mock_client = AsyncMock() | |
| # Patch get_http_client in detection router to return our mock | |
| with patch("backend.routers.detection.get_http_client", return_value=mock_client): | |
| with TestClient(app) as c: | |
| c.app.state.http_client = mock_client | |
| yield c | |
| # We need to successfully import backend.main to get 'app' | |
| # We will wrap the import in a patch context to mock side effects | |
| with patch("backend.ai_factory.create_all_ai_services") as mock_create: | |
| mock_action = AsyncMock() | |
| mock_chat = AsyncMock() | |
| mock_summary = AsyncMock() | |
| mock_create.return_value = (mock_action, mock_chat, mock_summary) | |
| from backend.main import app | |
| mock_client = AsyncMock() | |
| # Patch get_http_client in detection router to return our mock | |
| with patch("backend.routers.detection.get_http_client", return_value=mock_client): | |
| with TestClient(app) as c: | |
| c.app.state.http_client = mock_client | |
| yield c | |
| `@pytest.fixture` | |
| def client(): | |
| # We need to successfully import backend.main to get 'app' | |
| # We will wrap the import in a patch context to mock side effects | |
| with patch("backend.ai_factory.create_all_ai_services") as mock_create: | |
| mock_action = AsyncMock() | |
| mock_chat = AsyncMock() | |
| mock_summary = AsyncMock() | |
| mock_create.return_value = (mock_action, mock_chat, mock_summary) | |
| from backend.main import app | |
| from backend import dependencies as deps | |
| from backend.routers import detection as detection_router | |
| mock_client = AsyncMock() | |
| detection_router._cache_store.clear() | |
| with patch.object(deps, "SHARED_HTTP_CLIENT", mock_client), \ | |
| patch("backend.routers.detection.get_http_client", return_value=mock_client): | |
| with TestClient(app) as c: | |
| c.app.state.http_client = mock_client | |
| yield c | |
| detection_router._cache_store.clear() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/tests/test_new_detectors.py` around lines 49 - 67, The fixture
`client` currently only patches `backend.routers.detection.get_http_client`, but
`_get_cached_result` uses the global `backend.dependencies.SHARED_HTTP_CLIENT`
and tests can leak cached state; update the fixture to also patch
`backend.dependencies.SHARED_HTTP_CLIENT` to return the same `mock_client` (or
set `c.app.state.http_client` from that mock) and ensure any in-memory cache
used by the cached detector endpoints is cleared/reset before yielding (e.g.,
reset the shared cache or call its clear/reset helper) so each test gets a fresh
client and no cross-test cache leakage.
Comment on lines
+20
to
+23
| const capture = useCallback(() => { | ||
| const imageSrc = webcamRef.current.getScreenshot(); | ||
| setImgSrc(imageSrc); | ||
| }, [webcamRef]); |
There was a problem hiding this comment.
Add a null check before accessing webcamRef.current.
If the webcam hasn't mounted yet or is in an error state, webcamRef.current will be null, causing a TypeError when calling getScreenshot().
🛡️ Proposed fix
const capture = useCallback(() => {
+ if (!webcamRef.current) return;
const imageSrc = webcamRef.current.getScreenshot();
setImgSrc(imageSrc);
- }, [webcamRef]);
+ }, []);📝 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.
Suggested change
| const capture = useCallback(() => { | |
| const imageSrc = webcamRef.current.getScreenshot(); | |
| setImgSrc(imageSrc); | |
| }, [webcamRef]); | |
| const capture = useCallback(() => { | |
| if (!webcamRef.current) return; | |
| const imageSrc = webcamRef.current.getScreenshot(); | |
| setImgSrc(imageSrc); | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/ConstructionSafetyDetector.jsx` around lines 20 - 23, The
capture callback currently calls webcamRef.current.getScreenshot() without
ensuring webcamRef.current exists; update the capture function to null-check
webcamRef (webcamRef.current) before calling getScreenshot() and only call
setImgSrc(imageSrc) when imageSrc is non-null, returning early or handling the
error case if webcamRef.current is falsy. Locate the capture useCallback and add
a guard like if (!webcamRef?.current) return (or handle error) and ensure
imageSrc is validated before calling setImgSrc.
| screenshotFormat="image/jpeg" | ||
| videoConstraints={videoConstraints} | ||
| className="absolute inset-0 w-full h-full object-cover" | ||
| onUserMediaError={(err) => setCameraError("Camera access denied. Please check permissions.")} |
There was a problem hiding this comment.
Preserve original error information in camera error handling.
The onUserMediaError callback receives the actual error object but discards it, always showing a generic message. Logging or displaying the real error helps with debugging.
🔧 Proposed fix
- onUserMediaError={(err) => setCameraError("Camera access denied. Please check permissions.")}
+ onUserMediaError={(err) => {
+ console.error("Camera error:", err);
+ const message = err?.message || "Camera access denied. Please check permissions.";
+ setCameraError(message);
+ }}📝 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.
Suggested change
| onUserMediaError={(err) => setCameraError("Camera access denied. Please check permissions.")} | |
| onUserMediaError={(err) => { | |
| console.error("Camera error:", err); | |
| const message = err?.message || "Camera access denied. Please check permissions."; | |
| setCameraError(message); | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/ConstructionSafetyDetector.jsx` at line 85, The onUserMediaError
handler currently swallows the actual error; update the onUserMediaError
callback (where setCameraError is called) to include the original error
information — e.g., stringify or append err.message/err.name — so the state
contains the descriptive error and/or also call console.error(err) for
debugging; locate the onUserMediaError prop passed to the camera/video component
and change the inline arrow function to include the err details when calling
setCameraError and/or logging.
frontend/src/PlaygroundDetector.jsx
Outdated
Comment on lines
+109
to
+114
| <button | ||
| onClick={capture} | ||
| className="w-20 h-20 rounded-full bg-white border-4 border-gray-200 shadow-lg flex items-center justify-center active:scale-95 transition-transform" | ||
| > | ||
| <div className="w-16 h-16 rounded-full bg-green-600"></div> | ||
| </button> |
There was a problem hiding this comment.
Add an accessible name to the capture button.
The icon-only control lacks an accessible label, which makes it unclear for assistive technologies.
Suggested patch
- <button
- onClick={capture}
+ <button
+ type="button"
+ aria-label="Capture playground image"
+ onClick={capture}
className="w-20 h-20 rounded-full bg-white border-4 border-gray-200 shadow-lg flex items-center justify-center active:scale-95 transition-transform"
>📝 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.
Suggested change
| <button | |
| onClick={capture} | |
| className="w-20 h-20 rounded-full bg-white border-4 border-gray-200 shadow-lg flex items-center justify-center active:scale-95 transition-transform" | |
| > | |
| <div className="w-16 h-16 rounded-full bg-green-600"></div> | |
| </button> | |
| <button | |
| type="button" | |
| aria-label="Capture playground image" | |
| onClick={capture} | |
| className="w-20 h-20 rounded-full bg-white border-4 border-gray-200 shadow-lg flex items-center justify-center active:scale-95 transition-transform" | |
| > | |
| <div className="w-16 h-16 rounded-full bg-green-600"></div> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/PlaygroundDetector.jsx` around lines 109 - 114, The capture
button in PlaygroundDetector.jsx is icon-only and missing an accessible name;
update the button element (the one with onClick={capture}) to include an
accessible label such as aria-label="Capture photo" or add visually-hidden text
inside the button (e.g., a span with screen-reader-only class) so assistive
technologies can announce its purpose; ensure the label is descriptive and
matches the capture handler name/behavior.
frontend/src/views/Home.jsx
Outdated
Comment on lines
+107
to
+111
| title: t('Public Safety'), | ||
| icon: <Construction size={20} className="text-orange-600" />, | ||
| items: [ | ||
| { id: 'construction-safety', label: "Construction Safety", icon: <HardHat size={24} />, color: 'text-orange-600', bg: 'bg-orange-50' }, | ||
| { id: 'playground-damage', label: "Playground Damage", icon: <Trophy size={24} />, color: 'text-green-600', bg: 'bg-green-50' }, |
There was a problem hiding this comment.
Use translation keys for the new Public Safety title/items.
This segment bypasses the existing i18n key pattern, which can leave these labels untranslated in non-English locales.
Suggested patch
- title: t('Public Safety'),
+ title: t('home.categories.publicSafety'),
@@
- { id: 'construction-safety', label: "Construction Safety", icon: <HardHat size={24} />, color: 'text-orange-600', bg: 'bg-orange-50' },
- { id: 'playground-damage', label: "Playground Damage", icon: <Trophy size={24} />, color: 'text-green-600', bg: 'bg-green-50' },
+ { id: 'construction-safety', label: t('home.issues.constructionSafety'), icon: <HardHat size={24} />, color: 'text-orange-600', bg: 'bg-orange-50' },
+ { id: 'playground-damage', label: t('home.issues.playgroundDamage'), icon: <Trophy size={24} />, color: 'text-green-600', bg: 'bg-green-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.
Suggested change
| title: t('Public Safety'), | |
| icon: <Construction size={20} className="text-orange-600" />, | |
| items: [ | |
| { id: 'construction-safety', label: "Construction Safety", icon: <HardHat size={24} />, color: 'text-orange-600', bg: 'bg-orange-50' }, | |
| { id: 'playground-damage', label: "Playground Damage", icon: <Trophy size={24} />, color: 'text-green-600', bg: 'bg-green-50' }, | |
| title: t('home.categories.publicSafety'), | |
| icon: <Construction size={20} className="text-orange-600" />, | |
| items: [ | |
| { id: 'construction-safety', label: t('home.issues.constructionSafety'), icon: <HardHat size={24} />, color: 'text-orange-600', bg: 'bg-orange-50' }, | |
| { id: 'playground-damage', label: t('home.issues.playgroundDamage'), icon: <Trophy size={24} />, color: 'text-green-600', bg: 'bg-green-50' }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/views/Home.jsx` around lines 107 - 111, The Public Safety block
in Home.jsx uses hard-coded label strings and a non-pattern title; update the
i18n usage so the title and item labels use translation keys instead of
literals. Replace title: t('Public Safety') with a namespaced key such as
t('publicSafety.title') and change the item labels for ids 'construction-safety'
and 'playground-damage' to use t('publicSafety.constructionSafety') and
t('publicSafety.playgroundDamage') (keep the id, icon, color and bg fields
unchanged) so the block follows the existing i18n key pattern.
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/voice_service.py">
<violation number="1" location="backend/voice_service.py:233">
P2: When `source_language` is `'auto'`, the response now reports `'auto'` instead of the detected language, so clients lose accurate source-language info. Detect the language for the response when auto mode is used.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Updated `render-build.sh` to use `--no-cache-dir` to prevent OOM errors and removed redundant frontend build steps for the backend service. - Updated `render.yaml` to ensure it uses `./render-build.sh` as the build command. - Re-verified and fixed `backend/requirements-render.txt` to include `deep-translator` (replacing `googletrans`), `pywebpush`, `SpeechRecognition`, `pydub`, and `langdetect` to resolve runtime import errors and dependency conflicts.
🔍 Quality Reminder |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds two new AI-powered detection features for construction safety and playground damage using the Hugging Face CLIP model. The implementation follows the existing detector pattern in the codebase, leveraging zero-shot image classification to identify safety violations and equipment damage without requiring heavy local models.
Changes:
- Added backend detection endpoints and CLIP-based detection functions for construction safety and playground damage
- Implemented frontend detector components with camera capture and real-time detection UI
- Added comprehensive unit tests with mocked HF API responses
- Integrated new features into the Home view with a "Public Safety" category
- Included verification script for manual frontend testing
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/hf_api_service.py | Added detect_construction_safety_clip and detect_playground_damage_clip functions using the _detect_clip_generic helper pattern |
| backend/routers/detection.py | Added /api/detect-construction-safety and /api/detect-playground-damage endpoints with caching support |
| backend/tests/test_new_detectors.py | Added unit tests for both new endpoints with HF API mocking |
| frontend/src/ConstructionSafetyDetector.jsx | New detector component for construction safety violations with camera capture and detection UI |
| frontend/src/PlaygroundDetector.jsx | New detector component for playground equipment damage with camera capture and detection UI |
| frontend/src/views/Home.jsx | Added "Public Safety" category section with links to new detectors and Camera Diagnostics Modal |
| frontend/src/App.jsx | Added routes for new detectors and updated validViews list |
| frontend/src/api/detectors.js | Added API client methods for new detection endpoints |
| frontend_verification/verify_detectors.py | Playwright-based verification script for manual frontend testing |
| frontend_verification/playground_damage.png | Screenshot artifact from verification (binary file) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
34
to
47
There was a problem hiding this comment.
The comments on lines 35-47 are outdated implementation notes about debugging the test setup. These verbose comments explain the thought process for fixing a previous issue but are no longer relevant since the solution has been implemented. Consider removing or condensing these comments to just explain the final approach: "Mock create_all_ai_services from ai_factory before importing main to avoid initializing AI services during tests."
Comment on lines
107
to
111
There was a problem hiding this comment.
The new "Public Safety" section uses hardcoded English strings instead of internationalization (i18n) keys. All other categories use t() function for translations (e.g., line 79: t('home.categories.roadTraffic')). For consistency, the title should use t('home.categories.publicSafety') and the labels should use t('home.issues.constructionSafety') and t('home.issues.playgroundDamage') with corresponding entries added to the locale files.
Suggested change
| title: t('home.categories.publicSafety'), | |
| icon: <Construction size={20} className="text-orange-600" />, | |
| items: [ | |
| { id: 'construction-safety', label: t('home.issues.constructionSafety'), icon: <HardHat size={24} />, color: 'text-orange-600', bg: 'bg-orange-50' }, | |
| { id: 'playground-damage', label: t('home.issues.playgroundDamage'), icon: <Trophy size={24} />, color: 'text-green-600', bg: 'bg-green-50' }, |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/voice_service.py (1)
225-227: Redundant ternary expression.Line 227 is effectively a no-op:
source_language if source_language != 'auto' else 'auto'always returnssource_language.Simplify the assignment
- # Use deep-translator (synchronous, robust) - # Handle 'auto' source - src_lang = source_language if source_language != 'auto' else 'auto' + # Use deep-translator (synchronous, robust) + src_lang = source_language🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/voice_service.py` around lines 225 - 227, The assignment to src_lang in voice_service.py is redundant: replace the ternary expression "source_language if source_language != 'auto' else 'auto'" with a direct assignment of source_language (i.e., set src_lang = source_language) to simplify the code and remove the no-op conditional that references source_language and src_lang.
🤖 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/voice_service.py`:
- Around line 231-243: When source_language == 'auto' we currently return 'auto'
and lose the real detected language; before calling the translator (or
immediately after receiving translated), run langdetect.detect(text) (handle
exceptions and fallback to 'unknown' or original source_language) and assign it
to detected_src, use detected_src for source_language and source_language_name
(via SUPPORTED_LANGUAGES.get(detected_src, 'Unknown')), and update logger.info
to log detected_src (replace src_lang) so the returned dict contains the actual
detected language instead of 'auto'.
---
Nitpick comments:
In `@backend/voice_service.py`:
- Around line 225-227: The assignment to src_lang in voice_service.py is
redundant: replace the ternary expression "source_language if source_language !=
'auto' else 'auto'" with a direct assignment of source_language (i.e., set
src_lang = source_language) to simplify the code and remove the no-op
conditional that references source_language and src_lang.
backend/voice_service.py
Outdated
Comment on lines
231
to
243
| # deep-translator doesn't return source language detection in the same way, | ||
| # so we trust the input or what we detected earlier | ||
| detected_src = source_language | ||
|
|
||
| logger.info(f"Successfully translated text: {translation.src} -> {translation.dest}") | ||
| logger.info(f"Successfully translated text: {src_lang} -> {target_language}") | ||
|
|
||
| return { | ||
| 'translated_text': translation.text, | ||
| 'source_language': translation.src, | ||
| 'source_language_name': SUPPORTED_LANGUAGES.get(translation.src, 'Unknown'), | ||
| 'target_language': translation.dest, | ||
| 'target_language_name': SUPPORTED_LANGUAGES.get(translation.dest, 'Unknown'), | ||
| 'translated_text': translated, | ||
| 'source_language': detected_src, | ||
| 'source_language_name': SUPPORTED_LANGUAGES.get(detected_src, 'Unknown'), | ||
| 'target_language': target_language, | ||
| 'target_language_name': SUPPORTED_LANGUAGES.get(target_language, 'Unknown'), | ||
| 'original_text': text, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
deep-translator GoogleTranslator source auto detected language
💡 Result:
In deep-translator (Python), GoogleTranslator supports automatic source-language detection—just set source="auto".
from deep_translator import GoogleTranslator
text = "bonjour la vie"
translated = GoogleTranslator(source="auto", target="en").translate(text)
print(translated)This uses Google Translate’s auto-detect for the source language. [1]
If you instead want to detect the language without translating, deep-translator provides single_detection / batch_detection, but they require your own API key (from detectlanguage.com). [1]
from deep_translator import single_detection
lang = single_detection("bonjour la vie", api_key="YOUR_KEY")
print(lang) # e.g., "fr"Note: GoogleTranslator’s source default is "auto" (so you can omit it if you want auto-detect). [2]
Sources: [1] deep-translator docs, README (Language Detection + GoogleTranslator source='auto') [2] deep-translator GoogleTranslator reference (default source="auto")
🏁 Script executed:
head -50 backend/voice_service.py | cat -nRepository: RohanExploit/VishwaGuru
Length of output: 1712
🏁 Script executed:
wc -l backend/voice_service.pyRepository: RohanExploit/VishwaGuru
Length of output: 95
🏁 Script executed:
sed -n '1,20p' backend/voice_service.py | cat -nRepository: RohanExploit/VishwaGuru
Length of output: 776
🏁 Script executed:
sed -n '220,250p' backend/voice_service.py | cat -nRepository: RohanExploit/VishwaGuru
Length of output: 1678
Source language detection is lost when source_language='auto'.
When the caller passes source_language='auto', deep-translator's GoogleTranslator performs auto-detection internally but doesn't expose the detected language code. The response will return source_language='auto' and source_language_name='Unknown', losing the information about which language was actually detected.
Since langdetect is already imported, detect the source language before translation when 'auto' is specified:
Use langdetect for source language detection
# Use deep-translator (synchronous, robust)
- # Handle 'auto' source
- src_lang = source_language if source_language != 'auto' else 'auto'
+ src_lang = source_language
+ detected_src = source_language
+
+ # Detect source language if 'auto' is specified
+ if source_language == 'auto':
+ try:
+ detected_src = detect(text)
+ except LangDetectException:
+ detected_src = 'auto' # Fallback if detection fails
translated = GoogleTranslator(source=src_lang, target=target_language).translate(text)
- # deep-translator doesn't return source language detection in the same way,
- # so we trust the input or what we detected earlier
- detected_src = source_language
-
logger.info(f"Successfully translated text: {src_lang} -> {target_language}")
return {
'translated_text': translated,
'source_language': detected_src,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/voice_service.py` around lines 231 - 243, When source_language ==
'auto' we currently return 'auto' and lose the real detected language; before
calling the translator (or immediately after receiving translated), run
langdetect.detect(text) (handle exceptions and fallback to 'unknown' or original
source_language) and assign it to detected_src, use detected_src for
source_language and source_language_name (via
SUPPORTED_LANGUAGES.get(detected_src, 'Unknown')), and update logger.info to log
detected_src (replace src_lang) so the returned dict contains the actual
detected language instead of 'auto'.
- Updated `render.yaml` to explicitly run `chmod +x ./render-build.sh` before executing it. - This resolves potential "Permission denied" errors during the Render build process.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces two new AI-powered detection features: Construction Safety and Playground Damage. These features utilize the Hugging Face Inference API (CLIP model) to provide lightweight, resource-efficient detection without heavy local models.
Key Changes:
Backend:
backend/hf_api_service.py: Addeddetect_construction_safety_clipanddetect_playground_damage_clipfunctions.backend/routers/detection.py: Added corresponding API endpoints.backend/tests/test_new_detectors.py: Added unit tests for the new endpoints (mocking HF API).Frontend:
frontend/src/ConstructionSafetyDetector.jsx: New camera-based UI for construction safety.frontend/src/PlaygroundDetector.jsx: New camera-based UI for playground damage.frontend/src/views/Home.jsx: Added a "Public Safety" section.frontend/src/App.jsx: Added routes for the new detectors.frontend/src/api/detectors.js: Updated API client.Verification:
pytest.verify_detectors.pyscript.render.yamland environment variables are correctly set up for deployment.PR created automatically by Jules for task 6191078611621407715 started by @RohanExploit
Summary by cubic
Adds construction safety and playground damage detection with CLIP, plus camera UIs and backend endpoints. Optimizes Render deployment with a lightweight build script (skips frontend build, uses --no-cache-dir) and ensures it runs by setting execute permissions.
New Features
detect_construction_safety_clipanddetect_playground_damage_clip, with cached endpoints/api/detect-construction-safetyand/api/detect-playground-damage.ConstructionSafetyDetector.jsxandPlaygroundDetector.jsx, routes/construction-safetyand/playground-damage, API methodsconstructionSafetyandplaygroundDamage, and a “Public Safety” section on Home.Dependencies
googletranswithdeep-translator; updated backend requirements to resolvehttpx/httpcoreconflicts.render.yamlnow runschmod +x ./render-build.shthen executes it; script uses--no-cache-dirand skips frontend build to reduce memory and prevent permission errors.Written for commit a95464e. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Chores