Open
Conversation
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 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughAdds Hugging Face token support to config and implements zero-shot text classification for civic issue categorization. Introduces Changes
Sequence DiagramsequenceDiagram
actor User
participant Frontend as Frontend (ReportForm)
participant BackendAPI as Backend API
participant HFService as HF Zero-Shot Service
User->>Frontend: Enters issue description
User->>Frontend: Triggers onBlur (leaves text field)
Frontend->>Frontend: analyzeTextCategory() called
Frontend->>BackendAPI: POST /api/suggest-category-text<br/>{text: description}
BackendAPI->>HFService: classify_text_category(text)
HFService-->>BackendAPI: {category: "label", confidence: 0.95,...}
BackendAPI-->>Frontend: {category, confidence, all_scores}
Frontend->>Frontend: Update suggestedTextCategory state
Frontend->>Frontend: Render AI TEXT SUGGESTION card
User->>Frontend: Clicks suggestion card
Frontend->>Frontend: Apply category mapping
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 4
🧹 Nitpick comments (1)
frontend/src/views/ReportForm.jsx (1)
575-575: Consider debouncing text analysis trigger.Running
analyzeTextCategory()on every blur can cause avoidable API calls and noisy UX; a small debounce or “only when changed” guard would help.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/views/ReportForm.jsx` at line 575, The onBlur currently calls analyzeUrgency() and analyzeTextCategory() unconditionally which can trigger unnecessary API calls; modify the ReportForm component to debounce analyzeTextCategory (e.g., wrap it with a debounce utility or use useDebouncedCallback) or add a “only when changed” guard that compares the current text field value to a stored lastAnalyzedText before calling analyzeTextCategory(); ensure analyzeUrgency() can remain immediate or likewise debounced if needed, and reference the existing analyzeUrgency and analyzeTextCategory functions and the onBlur handler to apply the change.
🤖 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/hf_api_service.py`:
- Around line 487-490: The code currently assumes result['labels'][0] and
result['scores'][0] exist; guard against empty lists by checking that result is
a dict and that result.get('labels') and result.get('scores') are non-empty
(e.g. isinstance(...), and len(result['labels'])>0 and len(result['scores'])>0)
before indexing; if either list is empty, set top_label/top_score to a safe
default (None or a sentinel) or return/raise a controlled error path so callers
don’t get an unhandled IndexError — update the block that assigns top_label and
top_score to perform these checks and handle the empty-case consistently.
In `@backend/routers/analysis.py`:
- Around line 57-59: The except block in backend/routers/analysis.py currently
raises HTTPException with the raw exception text (str(e)), which can leak
internal details; instead log the full exception internally (e.g.,
logger.exception or capture traceback) and raise HTTPException with a generic,
non-sensitive message such as "Classification service unavailable" (or include a
short error ID if you want tracking), replacing the use of str(e) in the raise
HTTPException call while keeping the internal logging of e for diagnostics.
- Around line 50-55: The length check currently uses the raw body.text so
whitespace-only inputs bypass validation; update the handler to trim the input
(e.g., text_trimmed = body.text.strip()) before validating length and raise
HTTPException if len(text_trimmed) < 5, then pass the trimmed text into
classify_text_category; reference the existing variables/functions
get_http_client, classify_text_category, body.text and the HTTPException raise
to locate where to replace the checks and the argument passed to the classifier.
In `@frontend/src/views/ReportForm.jsx`:
- Around line 86-93: The code passes text-classifier output into the image-label
mapper mapSmartScanToCategory, causing incorrect fallbacks; replace the call to
mapSmartScanToCategory with the text-specific mapper (e.g.,
mapSmartTextToCategory) when handling the response from
analysisApi.suggestCategoryText in the block that sets setSuggestedTextCategory
(use the text mapping function for data.category and keep passing
data.confidence and original label).
---
Nitpick comments:
In `@frontend/src/views/ReportForm.jsx`:
- Line 575: The onBlur currently calls analyzeUrgency() and
analyzeTextCategory() unconditionally which can trigger unnecessary API calls;
modify the ReportForm component to debounce analyzeTextCategory (e.g., wrap it
with a debounce utility or use useDebouncedCallback) or add a “only when
changed” guard that compares the current text field value to a stored
lastAnalyzedText before calling analyzeTextCategory(); ensure analyzeUrgency()
can remain immediate or likewise debounced if needed, and reference the existing
analyzeUrgency and analyzeTextCategory functions and the onBlur handler to apply
the change.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/config.pybackend/hf_api_service.pybackend/routers/analysis.pybackend/schemas.pyfrontend/src/api/analysis.jsfrontend/src/api/index.jsfrontend/src/views/ReportForm.jsx
Comment on lines
+487
to
+490
| if isinstance(result, dict) and 'labels' in result and 'scores' in result: | ||
| top_label = result['labels'][0] | ||
| top_score = result['scores'][0] | ||
|
|
There was a problem hiding this comment.
Guard against empty labels / scores before indexing.
At Line 488 and Line 489, direct [0] access can raise on valid-but-empty API responses, turning recoverable cases into 500s.
💡 Suggested fix
- if isinstance(result, dict) and 'labels' in result and 'scores' in result:
- top_label = result['labels'][0]
- top_score = result['scores'][0]
+ if (
+ isinstance(result, dict)
+ and isinstance(result.get("labels"), list)
+ and isinstance(result.get("scores"), list)
+ and len(result["labels"]) > 0
+ and len(result["scores"]) > 0
+ ):
+ top_label = result["labels"][0]
+ top_score = result["scores"][0]📝 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
| if isinstance(result, dict) and 'labels' in result and 'scores' in result: | |
| top_label = result['labels'][0] | |
| top_score = result['scores'][0] | |
| if ( | |
| isinstance(result, dict) | |
| and isinstance(result.get("labels"), list) | |
| and isinstance(result.get("scores"), list) | |
| and len(result["labels"]) > 0 | |
| and len(result["scores"]) > 0 | |
| ): | |
| top_label = result["labels"][0] | |
| top_score = result["scores"][0] | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/hf_api_service.py` around lines 487 - 490, The code currently assumes
result['labels'][0] and result['scores'][0] exist; guard against empty lists by
checking that result is a dict and that result.get('labels') and
result.get('scores') are non-empty (e.g. isinstance(...), and
len(result['labels'])>0 and len(result['scores'])>0) before indexing; if either
list is empty, set top_label/top_score to a safe default (None or a sentinel) or
return/raise a controlled error path so callers don’t get an unhandled
IndexError — update the block that assigns top_label and top_score to perform
these checks and handle the empty-case consistently.
Comment on lines
+50
to
+55
| if not body.text or len(body.text) < 5: | ||
| raise HTTPException(status_code=400, detail="Text must be at least 5 characters long") | ||
|
|
||
| try: | ||
| client = get_http_client(request) | ||
| result = await classify_text_category(body.text, client=client) |
There was a problem hiding this comment.
Normalize input before length validation.
Line 50 validates untrimmed text, so whitespace-only payloads can pass and still hit the classifier.
💡 Suggested fix
- if not body.text or len(body.text) < 5:
+ cleaned_text = body.text.strip()
+ if len(cleaned_text) < 5:
raise HTTPException(status_code=400, detail="Text must be at least 5 characters long")
@@
- result = await classify_text_category(body.text, client=client)
+ result = await classify_text_category(cleaned_text, client=client)📝 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
| if not body.text or len(body.text) < 5: | |
| raise HTTPException(status_code=400, detail="Text must be at least 5 characters long") | |
| try: | |
| client = get_http_client(request) | |
| result = await classify_text_category(body.text, client=client) | |
| if not body.text: | |
| raise HTTPException(status_code=400, detail="Text must be at least 5 characters long") | |
| cleaned_text = body.text.strip() | |
| if len(cleaned_text) < 5: | |
| raise HTTPException(status_code=400, detail="Text must be at least 5 characters long") | |
| try: | |
| client = get_http_client(request) | |
| result = await classify_text_category(cleaned_text, client=client) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/routers/analysis.py` around lines 50 - 55, The length check currently
uses the raw body.text so whitespace-only inputs bypass validation; update the
handler to trim the input (e.g., text_trimmed = body.text.strip()) before
validating length and raise HTTPException if len(text_trimmed) < 5, then pass
the trimmed text into classify_text_category; reference the existing
variables/functions get_http_client, classify_text_category, body.text and the
HTTPException raise to locate where to replace the checks and the argument
passed to the classifier.
Comment on lines
+57
to
+59
| except Exception as e: | ||
| # Fallback or error | ||
| raise HTTPException(status_code=500, detail=f"Classification service unavailable: {str(e)}") |
There was a problem hiding this comment.
Do not expose raw exception text in API responses.
Line 59 returns str(e) to clients, which can leak internal/service details.
💡 Suggested fix
- except Exception as e:
- # Fallback or error
- raise HTTPException(status_code=500, detail=f"Classification service unavailable: {str(e)}")
+ except Exception as e:
+ # log e server-side
+ raise HTTPException(status_code=500, detail="Classification service unavailable") from e🧰 Tools
🪛 Ruff (0.15.2)
[warning] 57-57: Do not catch blind exception: Exception
(BLE001)
[warning] 59-59: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
[warning] 59-59: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/routers/analysis.py` around lines 57 - 59, The except block in
backend/routers/analysis.py currently raises HTTPException with the raw
exception text (str(e)), which can leak internal details; instead log the full
exception internally (e.g., logger.exception or capture traceback) and raise
HTTPException with a generic, non-sensitive message such as "Classification
service unavailable" (or include a short error ID if you want tracking),
replacing the use of str(e) in the raise HTTPException call while keeping the
internal logging of e for diagnostics.
Comment on lines
+86
to
+93
| const data = await analysisApi.suggestCategoryText(formData.description); | ||
| if (data && data.category && data.category !== 'unknown') { | ||
| const mappedCategory = mapSmartScanToCategory(data.category); | ||
| setSuggestedTextCategory({ | ||
| original: data.category, | ||
| mapped: mappedCategory, | ||
| confidence: data.confidence | ||
| }); |
There was a problem hiding this comment.
Text suggestions are mapped through the wrong label map.
At Line 88, text-classifier output is passed to mapSmartScanToCategory, but that map targets image smart-scan labels. This will mis-map many valid text labels to the fallback category.
💡 Suggested fix
+ const mapTextCategoryToCategory = (label) => {
+ const map = {
+ 'pothole': 'road',
+ 'garbage dump': 'garbage',
+ 'water leak': 'water',
+ 'broken street light': 'streetlight',
+ 'broken infrastructure': 'college_infra',
+ 'traffic congestion': 'road',
+ 'fire accident': 'road',
+ 'stray animal threat': 'road',
+ 'fallen tree hazard': 'road',
+ 'pest infestation': 'garbage',
+ 'clean area': 'road',
+ 'noise pollution': 'women_safety',
+ };
+ return map[label] || 'road';
+ };
@@
- const mappedCategory = mapSmartScanToCategory(data.category);
+ const mappedCategory = mapTextCategoryToCategory(data.category);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/views/ReportForm.jsx` around lines 86 - 93, The code passes
text-classifier output into the image-label mapper mapSmartScanToCategory,
causing incorrect fallbacks; replace the call to mapSmartScanToCategory with the
text-specific mapper (e.g., mapSmartTextToCategory) when handling the response
from analysisApi.suggestCategoryText in the block that sets
setSuggestedTextCategory (use the text mapping function for data.category and
keep passing data.confidence and original label).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 7 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/hf_api_service.py">
<violation number="1" location="backend/hf_api_service.py:487">
P2: Use `.get()` to safely ensure lists are non-empty before accessing index 0.</violation>
</file>
<file name="backend/routers/analysis.py">
<violation number="1" location="backend/routers/analysis.py:59">
P2: Avoid returning raw exception details in the HTTP response; return a generic error message instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| result = await _make_request(new_client, ZERO_SHOT_TEXT_API_URL, payload) | ||
|
|
||
| # Result format: {'sequence': '...', 'labels': ['pothole', ...], 'scores': [0.9, ...]} | ||
| if isinstance(result, dict) and 'labels' in result and 'scores' in result: |
Contributor
There was a problem hiding this comment.
P2: Use .get() to safely ensure lists are non-empty before accessing index 0.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/hf_api_service.py, line 487:
<comment>Use `.get()` to safely ensure lists are non-empty before accessing index 0.</comment>
<file context>
@@ -456,3 +459,40 @@ async def detect_abandoned_vehicle_clip(image: Union[Image.Image, bytes], client
+ result = await _make_request(new_client, ZERO_SHOT_TEXT_API_URL, payload)
+
+ # Result format: {'sequence': '...', 'labels': ['pothole', ...], 'scores': [0.9, ...]}
+ if isinstance(result, dict) and 'labels' in result and 'scores' in result:
+ top_label = result['labels'][0]
+ top_score = result['scores'][0]
</file context>
Suggested change
| if isinstance(result, dict) and 'labels' in result and 'scores' in result: | |
| if isinstance(result, dict) and result.get('labels') and result.get('scores'): |
| return result | ||
| except Exception as e: | ||
| # Fallback or error | ||
| raise HTTPException(status_code=500, detail=f"Classification service unavailable: {str(e)}") |
Contributor
There was a problem hiding this comment.
P2: Avoid returning raw exception details in the HTTP response; return a generic error message instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/routers/analysis.py, line 59:
<comment>Avoid returning raw exception details in the HTTP response; return a generic error message instead.</comment>
<file context>
@@ -36,3 +38,22 @@ def analyze_issue(request: AnalyzeIssueRequest):
+ return result
+ except Exception as e:
+ # Fallback or error
+ raise HTTPException(status_code=500, detail=f"Classification service unavailable: {str(e)}")
</file context>
Suggested change
| raise HTTPException(status_code=500, detail=f"Classification service unavailable: {str(e)}") | |
| raise HTTPException(status_code=500, detail="Classification service unavailable") |
🔍 Quality Reminder |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/views/ReportForm.jsx (1)
707-711:⚠️ Potential issue | 🟡 MinorObject URLs not revoked — potential memory leak.
URL.createObjectURLis called for image previews without correspondingURL.revokeObjectURLcalls. Repeated image uploads will accumulate blob URLs in memory.🧹 Suggested approach
Track the object URL and revoke it when the image changes or component unmounts:
+ const imageUrlRef = useRef(null); + + useEffect(() => { + if (formData.image) { + // Revoke previous URL + if (imageUrlRef.current) { + URL.revokeObjectURL(imageUrlRef.current); + } + imageUrlRef.current = URL.createObjectURL(formData.image); + } + return () => { + if (imageUrlRef.current) { + URL.revokeObjectURL(imageUrlRef.current); + } + }; + }, [formData.image]);Then use
imageUrlRef.currentinstead of callingURL.createObjectURLinline in JSX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/views/ReportForm.jsx` around lines 707 - 711, The JSX calls URL.createObjectURL(formData.image) inline (seen in ReportForm.jsx where the preview imgs are rendered), which leaks blob URLs; create and store the object URL in a ref or state (e.g., imageUrlRef or imagePreview state) when formData.image changes, replace inline createObjectURL usages with that stored value, and in a useEffect watch for formData.image to revoke the previous URL via URL.revokeObjectURL(imageUrlRef.current) before creating a new one and also revoke on component unmount in the cleanup to avoid memory leaks.
🧹 Nitpick comments (3)
frontend/src/setupTests.js (1)
7-10: UseglobalThisfor a more portable cross-runtime global reference.The test environment is configured to use jsdom (from
frontend/jest.config.js), where bothglobalandwindoware available. However, usingglobalThisaligns with ES2020 standards and provides better portability across different runtime environments.Proposed refactor
-// Define global if not available (for some test environments) -if (typeof global === 'undefined') { - window.global = window; -} +// Define global if not available (for some test environments) +if (typeof globalThis.global === 'undefined') { + globalThis.global = globalThis; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/setupTests.js` around lines 7 - 10, Replace runtime-specific checks for global/window in setupTests.js with the ES2020 standard globalThis: change the conditional that inspects typeof global and assigns window.global to instead reference globalThis so all environments use the portable global object; update any usages or initializations that relied on window.global to use globalThis (refer to the existing check using "global" and the assignment to "window.global" in this file).frontend/src/components/SupabaseExample.jsx (2)
54-57: Avoid timer-deferred state sync in this effect.Line 55 schedules an async state update for data you already have in the effect; this can cause stale writes and unnecessary timing complexity. Prefer direct update (or derive directly from
fetchedReportsand remove mirrored state).♻️ Suggested change
useEffect(() => { if (fetchedReports) { - // Defer update - setTimeout(() => setReports(fetchedReports), 0); + setReports(fetchedReports); } }, [fetchedReports]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/SupabaseExample.jsx` around lines 54 - 57, The effect is deferring state sync with setTimeout which can introduce staleness; remove the timer and update state directly (or eliminate the mirrored state altogether). Replace the setTimeout(() => setReports(fetchedReports), 0) call inside the useEffect that depends on fetchedReports with a direct setReports(fetchedReports) (or refactor components to derive UI from fetchedReports instead of maintaining reports state) and ensure the effect only reads fetchedReports; keep the dependency array as [fetchedReports].
58-69: Remove exploratory inline comments before merge.Lines 58-69 read like debugging notes and make the component harder to maintain. Please remove or replace with one concise rationale comment if needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/SupabaseExample.jsx` around lines 58 - 69, Remove the exploratory debug comments between lines 58–69 in SupabaseExample.jsx; replace them with a single concise comment explaining the chosen approach for synchronously calling setReports from the useEffect that depends on fetchedReports (e.g., "Handle fetchedReports → setReports here; ensure useSupabaseQuery memoizes results to avoid render loops" or note a deliberate lint suppression). Update the comment near the useEffect (in the SupabaseExample component) to reference setReports, fetchedReports, and useSupabaseQuery so future readers know why the implementation is safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/ResolutionProofCapture.jsx`:
- Around line 99-106: Remove the setTimeout(0) wrapper and update geofence state
synchronously: replace the deferred call with a direct call to
setGeofenceStatus({ distance: Math.round(distance), isInside: distance <=
token.geofence_radius_meters, radius: token.geofence_radius_meters }); so
handleSubmit reads the fresh geofenceStatus immediately; locate the code around
setGeofenceStatus in ResolutionProofCapture.jsx and ensure no other code relies
on the deferred update (if synchronous state-in-effect warnings appear, consider
switching this effect to useLayoutEffect instead of introducing a timeout).
In `@frontend/src/components/VoiceInput.jsx`:
- Line 64: The effect in VoiceInput.jsx currently lists onTranscript in its
dependency array (seen in the useEffect that depends on [language, onTranscript,
isSupported]) which causes SpeechRecognition to stop/recreate whenever the
parent passes a new function reference; stabilize onTranscript by creating a ref
(e.g., transcriptRef) inside VoiceInput and assign transcriptRef.current =
onTranscript in an effect, then remove onTranscript from the dependency array
and call transcriptRef.current(...) where the code currently calls onTranscript;
alternatively document that the parent should wrap its callback in useCallback
(e.g., in ReportForm.jsx) so the reference is stable.
In `@frontend/src/contexts/AuthContext.jsx`:
- Around line 31-43: The effect in AuthContext.jsx is incorrectly depending on
loading which causes a redundant authApi.me() call after setLoading(false);
update the useEffect dependency array to only include token (remove loading),
and directly call setLoading(false) inside the effect branches (or after the
authApi.me() promise) without adding loading to dependencies; locate the
useEffect that calls authApi.me(), the token and loading variables, and the
setLoading call and remove loading from the dependency list so the effect only
re-runs when token changes.
---
Outside diff comments:
In `@frontend/src/views/ReportForm.jsx`:
- Around line 707-711: The JSX calls URL.createObjectURL(formData.image) inline
(seen in ReportForm.jsx where the preview imgs are rendered), which leaks blob
URLs; create and store the object URL in a ref or state (e.g., imageUrlRef or
imagePreview state) when formData.image changes, replace inline createObjectURL
usages with that stored value, and in a useEffect watch for formData.image to
revoke the previous URL via URL.revokeObjectURL(imageUrlRef.current) before
creating a new one and also revoke on component unmount in the cleanup to avoid
memory leaks.
---
Nitpick comments:
In `@frontend/src/components/SupabaseExample.jsx`:
- Around line 54-57: The effect is deferring state sync with setTimeout which
can introduce staleness; remove the timer and update state directly (or
eliminate the mirrored state altogether). Replace the setTimeout(() =>
setReports(fetchedReports), 0) call inside the useEffect that depends on
fetchedReports with a direct setReports(fetchedReports) (or refactor components
to derive UI from fetchedReports instead of maintaining reports state) and
ensure the effect only reads fetchedReports; keep the dependency array as
[fetchedReports].
- Around line 58-69: Remove the exploratory debug comments between lines 58–69
in SupabaseExample.jsx; replace them with a single concise comment explaining
the chosen approach for synchronously calling setReports from the useEffect that
depends on fetchedReports (e.g., "Handle fetchedReports → setReports here;
ensure useSupabaseQuery memoizes results to avoid render loops" or note a
deliberate lint suppression). Update the comment near the useEffect (in the
SupabaseExample component) to reference setReports, fetchedReports, and
useSupabaseQuery so future readers know why the implementation is safe.
In `@frontend/src/setupTests.js`:
- Around line 7-10: Replace runtime-specific checks for global/window in
setupTests.js with the ES2020 standard globalThis: change the conditional that
inspects typeof global and assigns window.global to instead reference globalThis
so all environments use the portable global object; update any usages or
initializations that relied on window.global to use globalThis (refer to the
existing check using "global" and the assignment to "window.global" in this
file).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
frontend/src/AccessibilityDetector.jsxfrontend/src/BlockedRoadDetector.jsxfrontend/src/CivicEyeDetector.jsxfrontend/src/CrowdDetector.jsxfrontend/src/FireDetector.jsxfrontend/src/FloodDetector.jsxfrontend/src/GarbageDetector.jsxfrontend/src/IllegalParkingDetector.jsxfrontend/src/InfrastructureDetector.jsxfrontend/src/NoiseDetector.jsxfrontend/src/PestDetector.jsxfrontend/src/PotholeDetector.jsxfrontend/src/SeverityDetector.jsxfrontend/src/StrayAnimalDetector.jsxfrontend/src/StreetLightDetector.jsxfrontend/src/TreeDetector.jsxfrontend/src/VandalismDetector.jsxfrontend/src/WasteDetector.jsxfrontend/src/WaterLeakDetector.jsxfrontend/src/__mocks__/client.jsfrontend/src/__mocks__/location.jsfrontend/src/api/__tests__/client.test.jsfrontend/src/api/__tests__/detectors.test.jsfrontend/src/api/__tests__/index.test.jsfrontend/src/api/__tests__/issues.test.jsfrontend/src/api/__tests__/location.test.jsfrontend/src/api/__tests__/misc.test.jsfrontend/src/components/ResolutionProofCapture.jsxfrontend/src/components/SupabaseExample.jsxfrontend/src/components/VoiceInput.jsxfrontend/src/contexts/AuthContext.jsxfrontend/src/setupTests.jsfrontend/src/views/Home.jsxfrontend/src/views/ReportForm.jsx
💤 Files with no reviewable changes (28)
- frontend/src/TreeDetector.jsx
- frontend/src/IllegalParkingDetector.jsx
- frontend/src/WasteDetector.jsx
- frontend/src/WaterLeakDetector.jsx
- frontend/src/BlockedRoadDetector.jsx
- frontend/src/api/tests/issues.test.js
- frontend/src/GarbageDetector.jsx
- frontend/src/AccessibilityDetector.jsx
- frontend/src/FloodDetector.jsx
- frontend/src/NoiseDetector.jsx
- frontend/src/StreetLightDetector.jsx
- frontend/src/PotholeDetector.jsx
- frontend/src/mocks/client.js
- frontend/src/SeverityDetector.jsx
- frontend/src/views/Home.jsx
- frontend/src/FireDetector.jsx
- frontend/src/VandalismDetector.jsx
- frontend/src/api/tests/detectors.test.js
- frontend/src/mocks/location.js
- frontend/src/StrayAnimalDetector.jsx
- frontend/src/api/tests/misc.test.js
- frontend/src/api/tests/index.test.js
- frontend/src/PestDetector.jsx
- frontend/src/api/tests/client.test.js
- frontend/src/api/tests/location.test.js
- frontend/src/InfrastructureDetector.jsx
- frontend/src/CrowdDetector.jsx
- frontend/src/CivicEyeDetector.jsx
Comment on lines
+99
to
+106
| // Defer state update to avoid synchronous setState inside effect warning | ||
| setTimeout(() => { | ||
| setGeofenceStatus({ | ||
| distance: Math.round(distance), | ||
| isInside: distance <= token.geofence_radius_meters, | ||
| radius: token.geofence_radius_meters, | ||
| }); | ||
| }, 0); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find frontend/src/components -name "ResolutionProofCapture.jsx" -type fRepository: RohanExploit/VishwaGuru
Length of output: 117
🏁 Script executed:
cd frontend/src/components && wc -l ResolutionProofCapture.jsxRepository: RohanExploit/VishwaGuru
Length of output: 97
🏁 Script executed:
cd frontend/src/components && cat -n ResolutionProofCapture.jsx | head -150Repository: RohanExploit/VishwaGuru
Length of output: 6489
🏁 Script executed:
cd frontend/src/components && cat -n ResolutionProofCapture.jsx | sed -n '80,120p'Repository: RohanExploit/VishwaGuru
Length of output: 1942
🏁 Script executed:
cd frontend/src/components && grep -n "geofenceStatus" ResolutionProofCapture.jsxRepository: RohanExploit/VishwaGuru
Length of output: 906
🏁 Script executed:
cd frontend/src/components && cat -n ResolutionProofCapture.jsx | sed -n '150,250p'Repository: RohanExploit/VishwaGuru
Length of output: 5034
🏁 Script executed:
cd frontend && cat -n package.json | grep -A 5 -B 5 "react"Repository: RohanExploit/VishwaGuru
Length of output: 1502
🏁 Script executed:
cd frontend/src/components && cat -n ResolutionProofCapture.jsx | sed -n '150,180p'Repository: RohanExploit/VishwaGuru
Length of output: 1172
🏁 Script executed:
cd frontend && grep -r "synchronous setState" . --include="*.js" --include="*.jsx" --include="*.ts" --include="*.tsx" 2>/dev/null | head -20Repository: RohanExploit/VishwaGuru
Length of output: 331
🏁 Script executed:
cd frontend/src/components && cat -n ResolutionProofCapture.jsx | sed -n '300,320p'Repository: RohanExploit/VishwaGuru
Length of output: 1161
🏁 Script executed:
cd frontend/src/components && sed -n '300,365p' ResolutionProofCapture.jsxRepository: RohanExploit/VishwaGuru
Length of output: 3080
🏁 Script executed:
cd frontend && git log --oneline -p frontend/src/components/ResolutionProofCapture.jsx 2>/dev/null | grep -A 10 -B 10 "Defer state update" | head -30Repository: RohanExploit/VishwaGuru
Length of output: 49
🏁 Script executed:
cd frontend && git log --all --oneline -- frontend/src/components/ResolutionProofCapture.jsx 2>/dev/null | head -10Repository: RohanExploit/VishwaGuru
Length of output: 49
🏁 Script executed:
cd frontend/src/components && python3 << 'EOF'
# Analyze the code flow for race conditions
code = """
// Line 84-107: Effect
useEffect(() => {
if (!token || !gpsPosition) return;
const distance = R * c; // Line 97: computed synchronously
// Line 100-106: Deferred via setTimeout(0)
setTimeout(() => {
setGeofenceStatus({...});
}, 0);
}, [token, gpsPosition]);
// Line 168-177: Submit handler
const handleSubmit = async () => {
if (!token || !evidenceHash || !gpsPosition) return;
// Line 171: Uses geofenceStatus
if (geofenceStatus && !geofenceStatus.isInside) {
setError(...);
return;
}
// Can continue to submit
};
// Line 311: Button disabled state
disabled={geofenceStatus && !geofenceStatus.isInside}
"""
analysis = """
RACE CONDITION WINDOW:
1. gpsPosition updates → effect runs
2. Distance calculated synchronously in effect body
3. setTimeout(0) queues state update (not executed immediately)
4. User clicks submit button BEFORE setTimeout callback fires
5. handleSubmit executes with stale/old geofenceStatus
6. Validation at line 171 uses stale data
VULNERABILITY:
- If geofenceStatus is null: validation skipped (line 171 && short-circuits)
- If geofenceStatus is old value: validation uses outdated distance/isInside
- Button disable state (line 311) won't help if user manages to click before setTimeout
CORRECT BEHAVIOR WOULD BE:
- setGeofenceStatus immediately after distance calculation
- No race condition window
"""
print(analysis)
EOFRepository: RohanExploit/VishwaGuru
Length of output: 775
Remove setTimeout deferral from geofence status update—stale validation risk.
The setTimeout(0) creates a race condition where handleSubmit validation (line 171) can execute with outdated geofenceStatus values before the deferred state update completes. This allows a user to click submit between effect execution and callback, bypassing geofence validation with stale data. Set state immediately after computing distance.
🐛 Suggested fix
-// Defer state update to avoid synchronous setState inside effect warning
-setTimeout(() => {
- setGeofenceStatus({
- distance: Math.round(distance),
- isInside: distance <= token.geofence_radius_meters,
- radius: token.geofence_radius_meters,
- });
-}, 0);
+setGeofenceStatus({
+ distance: Math.round(distance),
+ isInside: distance <= token.geofence_radius_meters,
+ radius: token.geofence_radius_meters,
+});📝 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
| // Defer state update to avoid synchronous setState inside effect warning | |
| setTimeout(() => { | |
| setGeofenceStatus({ | |
| distance: Math.round(distance), | |
| isInside: distance <= token.geofence_radius_meters, | |
| radius: token.geofence_radius_meters, | |
| }); | |
| }, 0); | |
| setGeofenceStatus({ | |
| distance: Math.round(distance), | |
| isInside: distance <= token.geofence_radius_meters, | |
| radius: token.geofence_radius_meters, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/ResolutionProofCapture.jsx` around lines 99 - 106,
Remove the setTimeout(0) wrapper and update geofence state synchronously:
replace the deferred call with a direct call to setGeofenceStatus({ distance:
Math.round(distance), isInside: distance <= token.geofence_radius_meters,
radius: token.geofence_radius_meters }); so handleSubmit reads the fresh
geofenceStatus immediately; locate the code around setGeofenceStatus in
ResolutionProofCapture.jsx and ensure no other code relies on the deferred
update (if synchronous state-in-effect warnings appear, consider switching this
effect to useLayoutEffect instead of introducing a timeout).
| @@ -65,12 +64,12 @@ const VoiceInput = ({ onTranscript, language = 'en' }) => { | |||
| }, [language, onTranscript, isSupported]); | |||
There was a problem hiding this comment.
onTranscript in dependency array causes recognition reset on parent re-renders.
If the parent component passes an inline callback (as ReportForm.jsx does on line 580), every parent re-render creates a new function reference, triggering this effect. This stops and recreates the SpeechRecognition instance mid-listen, breaking voice input.
Consider using useCallback in the parent, or wrap onTranscript in a ref here to stabilize it:
🛠️ Suggested fix using a ref to stabilize the callback
import React, { useState, useEffect, useRef } from 'react';
import { Mic, MicOff } from 'lucide-react';
const VoiceInput = ({ onTranscript, language = 'en' }) => {
const [isListening, setIsListening] = useState(false);
const recognitionRef = useRef(null);
+ const onTranscriptRef = useRef(onTranscript);
const [error, setError] = useState(null);
+
+ // Keep ref in sync without triggering effect
+ useEffect(() => {
+ onTranscriptRef.current = onTranscript;
+ });
// ... existing code ...
recognitionInstance.onresult = (event) => {
const transcript = event.results[0][0].transcript;
- onTranscript(transcript);
+ onTranscriptRef.current(transcript);
};
// ... existing code ...
- }, [language, onTranscript, isSupported]);
+ }, [language, isSupported]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/VoiceInput.jsx` at line 64, The effect in
VoiceInput.jsx currently lists onTranscript in its dependency array (seen in the
useEffect that depends on [language, onTranscript, isSupported]) which causes
SpeechRecognition to stop/recreate whenever the parent passes a new function
reference; stabilize onTranscript by creating a ref (e.g., transcriptRef) inside
VoiceInput and assign transcriptRef.current = onTranscript in an effect, then
remove onTranscript from the dependency array and call
transcriptRef.current(...) where the code currently calls onTranscript;
alternatively document that the parent should wrap its callback in useCallback
(e.g., in ReportForm.jsx) so the reference is stable.
Comment on lines
+31
to
+43
| // Avoid setting state if already false, or use a ref if needed to track mounted status. | ||
| // However, this is inside useEffect, setting state is standard. | ||
| // The lint error might be due to unconditional set or dependency cycle? | ||
| // "Calling setState synchronously within an effect" usually means it's not wrapped in a condition or async? | ||
| // No, it usually happens if the effect runs immediately and sets state. | ||
| // Let's wrap in a check or setTimeout if strictly necessary, but standard auth flows often do this. | ||
| // Actually, let's just make sure we don't loop. | ||
| if (loading) { | ||
| // Defer state update to avoid "bad setState" warning/error | ||
| setTimeout(() => setLoading(false), 0); | ||
| } | ||
| } | ||
| }, [token]); | ||
| }, [token, loading]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find frontend/src -name "AuthContext.jsx" -type fRepository: RohanExploit/VishwaGuru
Length of output: 104
🏁 Script executed:
cat -n frontend/src/contexts/AuthContext.jsxRepository: RohanExploit/VishwaGuru
Length of output: 3211
Remove loading from effect dependencies to prevent redundant authApi.me() calls.
The current dependency array [token, loading] causes the effect to run again after setLoading(false) completes in the token branch, triggering a second authApi.me() request with the same token. Only token should trigger the effect; loading can be set directly without wrapping or timeout.
Suggested change
} else {
apiClient.removeToken();
- // Avoid setting state if already false, or use a ref if needed to track mounted status.
- // However, this is inside useEffect, setting state is standard.
- // The lint error might be due to unconditional set or dependency cycle?
- // "Calling setState synchronously within an effect" usually means it's not wrapped in a condition or async?
- // No, it usually happens if the effect runs immediately and sets state.
- // Let's wrap in a check or setTimeout if strictly necessary, but standard auth flows often do this.
- // Actually, let's just make sure we don't loop.
- if (loading) {
- // Defer state update to avoid "bad setState" warning/error
- setTimeout(() => setLoading(false), 0);
- }
+ setLoading(false);
}
- }, [token, loading]);
+ }, [token]);📝 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
| // Avoid setting state if already false, or use a ref if needed to track mounted status. | |
| // However, this is inside useEffect, setting state is standard. | |
| // The lint error might be due to unconditional set or dependency cycle? | |
| // "Calling setState synchronously within an effect" usually means it's not wrapped in a condition or async? | |
| // No, it usually happens if the effect runs immediately and sets state. | |
| // Let's wrap in a check or setTimeout if strictly necessary, but standard auth flows often do this. | |
| // Actually, let's just make sure we don't loop. | |
| if (loading) { | |
| // Defer state update to avoid "bad setState" warning/error | |
| setTimeout(() => setLoading(false), 0); | |
| } | |
| } | |
| }, [token]); | |
| }, [token, loading]); | |
| } else { | |
| apiClient.removeToken(); | |
| setLoading(false); | |
| } | |
| } | |
| }, [token]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/contexts/AuthContext.jsx` around lines 31 - 43, The effect in
AuthContext.jsx is incorrectly depending on loading which causes a redundant
authApi.me() call after setLoading(false); update the useEffect dependency array
to only include token (remove loading), and directly call setLoading(false)
inside the effect branches (or after the authApi.me() promise) without adding
loading to dependencies; locate the useEffect that calls authApi.me(), the token
and loading variables, and the setLoading call and remove loading from the
dependency list so the effect only re-runs when token changes.
Contributor
There was a problem hiding this comment.
3 issues found across 2 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="frontend/src/views/Home.jsx">
<violation number="1">
P2: Using `item.id` as the list key causes duplicate key warnings because multiple items share the id `'report'`. Consider combining the id with the index to ensure uniqueness.</violation>
<violation number="2">
P1: The `CameraCheckModal` is never rendered in the JSX. Add the conditionally rendered modal to the output to enable the feature.</violation>
<violation number="3">
P1: Missing imports for `Loader2` and `ChevronDown` which will cause a `ReferenceError` at runtime.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
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.
Implemented AI-powered text category suggestion for the Grievance Reporting form.
backend/config.pyto includeHF_TOKEN.classify_text_categorytobackend/hf_api_service.pyusingfacebook/bart-large-mnli./api/suggest-category-textendpoint inbackend/routers/analysis.py.frontend/src/api/analysis.jsand updatedReportForm.jsxto use it.ChatRequestschema validation inbackend/schemas.py.PR created automatically by Jules for task 16191297328419180687 started by @RohanExploit
Summary by cubic
Adds text category suggestions to the Grievance Reporting form using zero-shot text classification, with a new backend endpoint and lightweight UI hooks. Also fixes the frontend build by cleaning up deleted imports and restoring the Home view, and unblocks deployment.
New Features
Bug Fixes
Written for commit 171f555. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Changes