Make the application run: repair backend boot, frontend build, and 20 dead endpoints; add Android app - #952
Open
RohanExploit wants to merge 28 commits into
Open
Make the application run: repair backend boot, frontend build, and 20 dead endpoints; add Android app#952RohanExploit wants to merge 28 commits into
RohanExploit wants to merge 28 commits into
Conversation
The repository had no test, lint, or build workflow. All seven existing
workflows were bots. Two of them formed a loop that committed unreviewed
code to main every day:
jules-daily-auto-upgrade.yml ran on a daily cron and instructed Jules to
force a change no matter what ("You MUST append the current date to
daily_streak.log ... This ensures you always make at least one file
change"), with automationMode AUTO_CREATE_PR.
auto-merge-jules.yml then squash-merged the result. It gated on
wait-on-check-action, but with no CI checks defined that gate passed
vacuously, and it held contents: write on a pull_request_target trigger.
That loop is how backend/main_fixed.py (989 lines, zero importers) and a
frontend calling 15 endpoints the backend does not define both reached main.
Changes:
- jules-daily-auto-upgrade.yml: cron removed, manual dispatch only,
permissions reduced to contents: read.
- auto-merge-jules.yml deleted, replaced by label-bot-prs.yml which only
labels bot PRs for human review.
- ci.yml added: ruff lint + format check + pytest on Python 3.12, eslint +
jest + vite build on Node 20 and 22, plus a bandit / pip-audit / npm audit
security job. Build output is uploaded as an artifact.
- Dependencies pinned. backend/requirements.in holds the direct
dependencies; backend/requirements.txt is now a 99-package lock compiled
with uv for linux/py3.12. Previously every dependency was unpinned.
- pyproject.toml added with ruff and pytest configuration. Neither tool had
any configuration before.
- requirements-dev.txt added for CI tooling.
The remaining pull_request_target workflows were audited: none checks out
untrusted PR head, so they are label/comment only and safe.
backend.main could not be imported at all, so the API served nothing and Render's health check (healthCheckPath: /health) could never pass. Root cause: backend/main.py and backend/models.py used bare intra-project imports (from models import Base) while the other 20 backend modules used package imports (from backend.models import Base). Importing backend.main pulled in backend.bot, which imports backend.models, so Python loaded the same module twice under two names. Every SQLAlchemy table was then declared twice against one MetaData and import raised: InvalidRequestError: Table 'jurisdictions' is already defined for this MetaData instance. All 20 bare imports across main.py, models.py, flood_detection.py, hf_service.py and unified_detection_service.py are now fully qualified. render.yaml's PYTHONPATH moves from "backend" to "." accordingly, since having backend/ on sys.path is what allowed the double load. Also fixed, all of which ruff reported as F821 undefined names that would have raised NameError at request time: - main.py had two concatenated copies of its import block. It never imported timezone (used in /health), func (used in /api/stats), Image, asyncio, recent_issues_cache, get_detection_status, get_ai_services or DISTRICT_RANGES. /health, /api/stats and /api/ml-status all raised. - main.py never called initialize_ai_services, so get_ai_services() raised RuntimeError on /api/mh/rep-contacts. It is now initialized in the lifespan. - Three routes were registered twice. FastAPI serves the first match, so the later handler was dead in each case: GET /, GET /api/responsibility-map, and POST /api/detect-pothole. - The served detect-pothole handler had no return on its success path, so the endpoint answered null. The dead duplicate was the correct one. - The four api_detect_* handlers declared their upload as `file`, but all 25 frontend call sites post `image`, so detect-garbage, detect-vandalism and detect-flooding returned 422 on every request. Standardised on `image`. - bot.py referenced start_bot_thread, stop_bot_thread and _bot_thread, none of which existed. Implemented as a threaded runner, which also lets the bot run outside the FastAPI lifespan; polling inside the lifespan gives every uvicorn worker its own long-poll and Telegram rejects the extras with HTTP 409. - hf_service.py had unreachable code after a return that referenced two out-of-scope names. Removed. - schemas.py declared DetectionResponse twice. CORS previously combined allow_origins=["*"] with allow_credentials=True, which the Fetch spec forbids and browsers reject, and it ignored the CORS_ORIGINS variable render.yaml already declares. Origins are now read from CORS_ORIGINS, then FRONTEND_URL, then a localhost-only default, so a misconfigured deploy fails closed. tests/test_vandalism.py was rewritten: it declared four @patch decorators against three parameters and patched attributes that do not exist on backend.main, so it could never run. tests/test_bot_integration.py now imports through the package path. The suite previously failed at collection. It now collects 126 tests, of which 101 pass. The 25 failures are almost entirely assertions against the 15 endpoints the frontend calls but the backend does not yet define; those are the next piece of work.
vite build failed on main, so Netlify had no deployable artifact. src/App.jsx lazy-imported eleven detector components from ./features/detectors/, but src/features/ does not exist; all eleven components are at src/. Rollup halts on the first unresolved import, so the whole build died. The paths now point at the real locations. eslint.config.js applied only browser globals to every file, so Jest and Node globals (describe, it, expect, jest, global, process) were reported as no-undef in test, mock and config files. That accounted for 300 of the 381 reported problems and made the lint output useless as a signal. Added scoped config blocks for tests/mocks and for build tooling, and allowed underscore-prefixed unused args and caught errors. Build now succeeds: 77 modules, PWA service worker generated, 27 precache entries. Lint drops from 381 problems to 80, all of which are genuine. The 114 existing Jest tests still pass.
The frontend called 18 detector endpoints; backend/main.py declared 5. The
other 15 returned 404 to real users. Every service function they needed was
already implemented in backend/hf_api_service.py and
backend/local_ml_service.py -- none of it had ever been wired to a route.
Added: detect-fire, detect-illegal-parking, detect-street-light,
detect-stray-animal, detect-blocked-road, detect-tree-hazard, detect-pest,
detect-severity, detect-smart-scan, detect-waste, detect-civic-eye,
analyze-depth, detect-infrastructure, transcribe-audio, leaderboard, plus
generate-description, analyze-urgency and issues/{id}/verify.
tests/test_api_contract.py is the guard. It scans frontend/src for every
'/api/...' literal and asserts each resolves to a declared route, and
separately asserts no path is registered twice. It fails if either gap
reopens. It went from 15 failures to green.
The twelve CLIP-backed detectors are generated from a table rather than
copy-pasted; copy-paste is what produced four handlers with the wrong upload
field name. The service is stored as a name and resolved from the module at
request time so tests can monkeypatch backend.main.<name>.
MAX_UPLOAD_SIZE_MB is now enforced. It was declared in render.yaml and parsed
in config.py but no request path ever checked it.
A single httpx.AsyncClient is created in the lifespan and closed on shutdown,
rather than a client per request.
backend/maharashtra_locator.py: _load_maharashtra_pincode_map and
_load_maharashtra_mla_map re-keyed a dict that load_*_data() had already
keyed. Iterating a dict yields its string keys, so entry["pincode"] never
matched and both maps were always empty -- every constituency and MLA lookup
returned None, which is the representative-lookup feature the product is
built around. Pincode 411001 now resolves to Kasba Peth / Ravindra Dhangekar.
transcribe_audio() and generate_image_caption() return bare strings, so the
handlers wrap them as {"text": ...} and {"description": ...} instead of
serialising a naked JSON string.
Test suite: the same bare-vs-package import split that broke backend.main was
present throughout the tests. Sixteen files pushed backend/ onto sys.path and
six imported `main` directly, so tests patched backend.main while exercising a
separately-loaded `main`. A root conftest.py now puts the repo root on the
path and strips backend/ if anything re-adds it; all test imports and patch
targets are fully qualified.
Suite goes from 126 collected / 101 passing to 152 collected / 144 passing.
The 8 remaining failures are contract questions, not defects: two tests
disagree about what POST /api/issues/{id}/verify does, POST /api/issues
returns 200 where tests expect 201, /api/issues/nearby is unimplemented, and
three tests reference helper names that were never written.
The upvote button 404'd on every press. frontend/src/api/issues.js posted to
/api/issues/{id}/vote while the backend serves /api/issues/{id}/upvote.
tests/test_api_contract.py did not catch this, which was a hole in the guard
rather than an oversight: its path regex only accepted [A-Za-z0-9/_-], so any
URL built with a template-literal interpolation was skipped entirely. It now
collapses ${...} to a placeholder segment and strips query strings before
matching, so interpolated paths are checked like any other. Re-running it
immediately reproduced the /vote failure.
PWA installability: frontend/public/manifest.json and vite.config.js both
referenced /icon-192.png and /icon-512.png, and neither file existed. Chrome
requires a resolvable 192px and 512px icon before it will offer to install,
so the install prompt never appeared and Android had no launcher icon.
scripts/generate_icons.py now renders the set from frontend/public/logo.png:
96/192/512 standard, a 512 maskable variant with the larger safe-zone margin
Android needs when it crops to the launcher shape, plus apple-touch-icon and
a real favicon. The wordmark is light, so it is composed onto a #0D1117
ground rather than scaled on transparency.
index.html still shipped Vite's scaffold defaults: title "frontend" and
/vite.svg as the icon. It now carries the real title, description, icon and
apple-touch-icon links, an explicit manifest link, theme-color, and
viewport-fit=cover for notched devices.
Build stays green; precache goes from 27 to 31 entries. Frontend suite is
114 passing after updating the two assertions that encoded the broken /vote
path.
An adversarial pass over the previous commits found that the contract test
was proving less than it claimed, and that several live endpoints were still
broken behind it.
Contract test, rewritten. Three separate blind spots:
- The path regex required the literal to sit immediately after a quote or
backtick, so fetch(`${API_URL}/api/detect-fire`) -- the dominant call
pattern in this codebase -- was invisible. Broadening the scan took the
detected surface from 26 paths to 37 and exposed eight endpoints with no
backend implementation at all.
- _backend_routes() scanned app.routes flat, but include_router() inserts a
wrapper with no .path of its own, so routes from any mounted router were
reported as missing. It now reads app.openapi()["paths"], which is the
authoritative declared surface.
- A declared path was treated as a working path. Two new checks close that:
every /api/detect-* route must accept POST, and every one must accept the
upload field name its callers actually send. The second immediately caught
a live 500 (below).
Endpoints that did not exist, now implemented. All four services were already
written in backend/hf_api_service.py: detect-accessibility, detect-crowd,
detect-water-leak, and detect-audio (whose caller posts `file`, not `image`).
The grievance feature was entirely unmounted. grievance_service.py,
escalation_engine.py, routing_service.py and sla_config_service.py were all
implemented and frontend/src/api/grievances.js and views/GrievanceView.jsx
were written against them, but no router existed, so /api/grievances,
/api/grievances/{id}, /api/escalation-stats and
/api/grievances/{id}/escalate all 404'd. backend/grievance_routes.py wires
them up. GrievanceView reads escalation_history unconditionally, so it is
always serialised as a list.
/api/analyze-urgency was broken by the previous commit: ReportForm.jsx posts
{"description": ...} and the model required `text`, so every request from the
report form was rejected with 422 and the urgency panel silently never
populated. Both field names are now accepted.
POST /api/detect-vandalism returned 500 on every request. detect_vandalism is
a coroutine function, but the handler called it inside a sync closure passed
to run_in_threadpool, producing an un-awaited coroutine that failed
serialisation. The flooding handler awaited correctly but opened the image on
the event loop, and none of the four original handlers enforced
MAX_UPLOAD_SIZE_MB, so an oversized phone photo was accepted here while the
generated endpoints correctly rejected it. All four now share one path that
handles sync and async detectors, opens images off the loop, and applies the
size ceiling.
/api/issues/{id}/verify now returns confidence and question_asked.
VerifyView.jsx renders (result.confidence * 100).toFixed(1), which displayed
"NaN%", and interpolated an undefined question into its summary line.
Two tests were asserting contracts that no longer existed. The
"POST /verify with no body raises upvotes by 2" case collided with the real
AI verification feature and had no frontend caller; the coverage moved to
/upvote, which is what Home.jsx actually calls. test_main_imports_unified_service
demanded that main import two local detectors it does not use; it now checks
the callables the routes really resolve.
test_model_thread_safety passed alone and failed in a full run.
get_model() takes its fast path on _model_initialized, and the test only
reset _model, so a module left initialised by an earlier test returned
immediately and the load count came back 0. Both cases now call reset_model(),
and garbage_detection gained one to match pothole_detection.
Suite: 152 collected / 144 passing -> 211 collected / 206 passing. The five
remaining failures are the spatial-deduplication feature (/api/issues/nearby
is unimplemented), POST /api/issues returning 200 where two tests expect 201
with a backgrounded action plan, and two tests referencing helper names that
were never written.
Two things would have failed on the first device build.
Seven detector components called fetch('/api/...') with a relative path:
BlockedRoadDetector, FireDetector, IllegalParkingDetector, PestDetector,
StrayAnimalDetector, StreetLightDetector and TreeDetector. Those resolved
only because vite.config.js proxies /api in development and netlify.toml
rewrites /api/* in the web deployment. A Capacitor WebView serves the page
from capacitor://localhost or https://localhost, where neither mechanism
exists, so all seven would have had no backend to reach. They now go through
VITE_API_URL like SmartScanner and SeverityDetector already did.
CORS rejected the packaged app outright. Origins came only from CORS_ORIGINS
or FRONTEND_URL, which are written for the web domain and will never contain
the WebView origin, so every request from the app was blocked by the
browser's CORS check even with a correct absolute URL. The Capacitor origins
are now appended to whatever the environment configures instead of replacing
it; an arbitrary origin is still rejected.
Frontend build stays green, 114 tests still pass.
npm run lint reported 69 errors, so the CI lint job could never pass. views/ActionView.jsx called useEffect after an early `if (!actionPlan)` return, so the component invoked a different number of hooks depending on its props. The moment actionPlan went from null to set, React would throw "Rendered more hooks than during the previous render". The effect now runs before the bail-out and guards internally. react-hooks/rules-of-hooks stays an error precisely because it caught this. views/Landing.jsx imported `motion` from framer-motion, which is not a dependency of the package at all. It only avoided breaking the build because Landing is unreachable -- nothing routes to it. Import removed. The remaining 32 unused bindings were dead imports, unused destructured props, discarded error arguments and abandoned state. Removed or given the underscore prefix the config already allows. eslint-plugin-react-hooks v7 brings the React Compiler ruleset, whose immutability, exhaustive-deps, set-state-in-effect and static-components rules fire 47 times across the twenty detector components -- on ref access during render and on fetch-in-effect. These are compiler-readiness signals, not correctness failures, and clearing them means the component consolidation being deferred until after the first mobile release. They are set to `warn` with that reasoning recorded in the config, so they stay visible on every run without blocking the gate on work that is deliberately scheduled later. Lint: 69 errors -> 0 errors, 47 warnings, exit 0. Build green, 114 tests pass.
ruff reported 610 findings, so the CI backend job could never pass. 449 were cleared by safe autofixes (import ordering, PEP 585/604 annotations, timezone constants). The rest are below. backend/main_fixed.py is deleted. 1,010 lines, zero importers, 39 of the remaining lint findings, and 28 route definitions that never served a request. It is the artifact the daily auto-merge loop produced, and keeping it around was actively misleading -- it looked like the real application. Real defects found while clearing the list: - unified_detection_service._check_hf_available() read HF_TOKEN into a local and then set _hf_available = True unconditionally. An unconfigured deployment reported the hosted inference backend as available and never fell back to the local model, so requests failed at call time instead of routing around the gap. It now checks the token and warns when it is absent. - ai_service.generate_action_plan parsed an image path out of args and kwargs three different ways and never used it, because the prompt built by _generate_action_plan_with_retry is text-only. It read as though the image informed the plan. Removed, with the behaviour documented; image analysis lives in analyze_issue_image(). - hf_api_service.generate_image_caption base64-encoded the image and built a JSON payload that was then discarded -- the request posts the raw bytes. The swallowed-exception blocks are now observable. backend/init_db.py wrapped 22 migration statements in bare `except Exception: pass`, and the lifespan in main.py wrapped 4 more. Each statement is expected to fail once its column or index exists, but a failure for any other reason -- wrong dialect, locked table, missing permission -- was indistinguishable from that and left no trace anywhere. Both are now table-driven and log every applied and skipped statement. This is still not a migration system; adopting Alembic is tracked separately. Exceptions raised inside except blocks are chained, so a 502 in the logs can be traced to the failure underneath instead of appearing to come from nowhere. Test-only rules (assert, fixture tokens, non-cryptographic RNG, and the E402 that is unavoidable when environment variables or module mocks must be set before importing the app) are scoped to test paths in pyproject.toml rather than suppressed globally. Four tests in test_local_ml_service.py swallowed missing optional ML dependencies with a bare pass; they now skip with the reason attached. ruff format applied across 64 files. ruff check: 610 findings -> 0. ruff format --check: clean. Suite unchanged at 206 passing, same 5 known failures.
…lication POST /api/issues generated the action plan inline, so the request stayed open for the whole model call and submitting a report looked like a hang. It now returns 201 immediately with action_plan null and produces the plan in a background task. /api/issues/recent carries action_plan so the client poll can terminate -- views/ActionView.jsx polls that endpoint for exactly this field and, since it was never returned, the "Generating Action Plan..." spinner could never resolve. Issue.action_plan was declared as plain Text while models.py defines a JSONEncodedDict decorator for precisely this case, so plans round-tripped as raw JSON strings and every reader had to decode by hand. The column now uses it. Storage is still Text underneath, so existing rows are unaffected. GET /api/issues/nearby is new. backend/spatial_utils.py implemented the bounding-box pre-filter and haversine distance, and the frontend's duplicate check called the endpoint, but nothing ever exposed it. Results are sorted by distance and capped by an explicit radius and limit. POST /api/issues now accepts latitude, longitude and location, and reports possible duplicates within 50 metres as deduplication_info plus linked_issue_id. The image is optional -- a report pinned to a location is still a report. Creating an issue used to drop the recent-issues cache entirely. It now prepends the new row to the cached list and only invalidates when there is nothing to update. /api/detect-vandalism and /api/detect-infrastructure route through backend.unified_detection_service instead of calling one implementation directly. Vandalism went via backend.vandalism_detection, which reaches the module marked DEPRECATED at the top of hf_service.py, and infrastructure called the local model with no path to the hosted API at all. The unified service tries local first and falls back. Added validate_image_for_processing, a second seam that checks the decoded image. validate_uploaded_file caps the byte length, but a small payload can still decode to dimensions large enough to exhaust memory during inference. Backend suite: 202 passed, 4 skipped, 0 failed. ruff check and ruff format --check both clean. Frontend: 0 lint errors, 114 tests, build green.
The project had no mobile target at all: no Capacitor, no native project, no Android build. It shipped a PWA whose manifest pointed at icons that did not exist. Capacitor 8 now wraps the existing Vite build, so the web app and the Android app are the same bundle rather than a fork. capacitor.config.ts sets androidScheme to https, which keeps the WebView origin at https://localhost. That is the origin the backend's CORS allowlist admits, and it avoids Android's cleartext-traffic block, which rejects http:// by default from API 28 onward. allowMixedContent stays off. AndroidManifest declares CAMERA, the media-read permissions, and both fine and coarse location -- coarse because Android 12+ lets a user grant only approximate position. Camera and GPS are declared as optional features so a device without them can still install and file a location-only report. src/native.js is the platform bootstrap. It is a no-op on the web, so one bundle serves both targets. Inside a WebView three things do not happen by themselves: the splash screen never lifts, the status bar keeps system colours, and navigator.geolocation resolves only after the Android runtime permission is granted while offering no way to request it. views/ReportForm.jsx now takes its position through that helper, which asks for the permission natively and falls back to navigator.geolocation in the browser. Launcher icons, round icons, adaptive foreground/background layers and splash screens are generated for every density from the same source logo as the PWA icons, so web and app identity match. Release signing is driven entirely by environment variables; no keystore or password is committed, and *.keystore / *.jks are ignored. A release build without them falls back to debug signing, which Play Console rejects -- deliberately, so an unsigned artifact cannot be mistaken for a shippable one. minifyEnabled and shrinkResources are on for release. .github/workflows/android.yml builds a debug APK on pull requests so packaging breakage is caught before merge, and a signed AAB on tags and manual runs. It refuses to build unless VITE_API_URL is set and absolute https, because a packaged app has no dev proxy or Netlify redirect to resolve a relative /api path against. After bundling it verifies the artifact actually carries a release signature rather than trusting that Gradle used the right config. versionCode comes from the run number, which Play requires to increase on every upload. eslint now ignores android/: Capacitor copies the built bundle into android/app/src/main/assets/public on every sync, which otherwise produced several hundred errors against minified output. Verified locally: cap sync succeeds with all six plugins registered, web build green, 114 frontend tests pass, backend 202 passed / 4 skipped / 0 failed, ruff clean. The AAB itself is built in CI, since this machine has no Android SDK.
… web process Two settings existed on paper only. RATE_LIMIT_ENABLED and MAX_REQUESTS_PER_MINUTE were declared in render.yaml and parsed in backend/config.py, but no middleware ever read them, so every endpoint was unmetered. That is a billing exposure as much as an availability one: the detector, chat, caption and verification routes each call a paid inference API per request. slowapi now enforces a default bucket, with a tighter AI_REQUESTS_PER_MINUTE bucket on the eleven model-backed routes. /health is exempt, because a platform restarts a service whose health check starts failing under load. tests/test_rate_limiting.py fails if the enforcement is removed again. Counters are in-process, which is right for a single instance; RATE_LIMIT_STORAGE_URI points at Redis for more than one. The Telegram poller ran inside the FastAPI lifespan, so every uvicorn worker opened its own long-poll against Telegram. Telegram answers the second one with HTTP 409, which meant the API could never run more than one worker -- a hard ceiling on scaling that nothing in the repository documented. Polling now runs on the threaded runner in backend/bot.py and only in the process that sets RUN_TELEGRAM_BOT. Run the web service without it and one dedicated worker with it, and the API scales horizontally. render.yaml and .env.example document all of it, including why RUN_TELEGRAM_BOT must be set on exactly one process and why VITE_API_URL has to be an absolute https URL for the Android build. Backend: 205 passed, 4 skipped, 0 failed. ruff check and format clean.
Phase 0 killed auto-merge-jules.yml but missed this one. auto-deploy.yml ran
daily at 02:00 UTC and executed vishwaguru_pipeline.py, which squash-merges
open pull requests through the GitHub API.
It looked gated. It was not:
* quality check = PR title at least 5 characters, body at least 10
* security check = grep the diff for a keyword list
* "run tests" = `npm test` against the ROOT package.json, whose script is
`jest tests/`. That collects a single TypeScript file. It never ran the
backend pytest suite or the frontend Jest suite.
* "deploy and health check" = there is no docker-compose.yml and no
manage.py, so it fell through to `python -m http.server`, then confirmed
that static file server returned 200 and treated it as the application
being healthy.
So it merged to main every day on evidence that proved nothing. Between this
and auto-merge-jules.yml, two independent schedules were writing unreviewed
code to main daily, which is how the repository reached a state where the
backend could not import, the frontend could not build, and fifteen endpoints
the frontend called did not exist.
Now manual dispatch only. No scheduled workflow can merge to main any more;
ci.yml plus human review is the path.
The project had no container packaging: no Dockerfile, no compose file. The only reproducible path to a running backend was Render's buildCommand, which is not runnable locally and not testable in CI. Two stages, so the compilers needed to build psycopg2, Pillow and numpy wheels never reach the runtime image. Dependencies install from the committed uv lockfile rather than a resolver run, so an image built today and one built in six months contain the same packages. The runtime carries libmagic1, which python-magic needs to sniff upload types, and libpq5 for psycopg2 -- neither pulls in a toolchain. The service runs as an unprivileged user, not root. PYTHONPATH is the repo root, not backend/. Putting backend/ on the path is what let `models` and `backend.models` load as two separate modules and double-register every SQLAlchemy table, which is why backend.main could not be imported at all before this branch. HEALTHCHECK hits the same /health path render.yaml uses. That endpoint is exempt from rate limiting, so a busy service is never mistaken for an unhealthy one and cycled. CI now builds the image on every pull request, boots the container, and polls /health until it answers -- so the image is proven to run, not merely to compile. Build cache is shared through GitHub Actions cache. Also verified in this pass: all nine workflow files parse as valid YAML, and no scheduled trigger remains anywhere in .github/workflows after the two auto-merge crons were disabled.
Both were caught by producing real artifacts locally rather than trusting the
CI workflow to be correct. Neither would have been visible before the first
tagged release.
app/build.gradle declared its signing locals as keystorePath, keystorePassword,
keyAlias and keyPassword. Inside a signingConfigs block those last three are
DSL setter names, so `keyAlias keyAlias` parses as invoking the String as a
method and Gradle fails with:
No signature of method: java.lang.String.call() is applicable for
argument types: (String) values: [vishwaguru]
Configuration aborts before any task runs, so every release build would have
failed -- including the CI job on its first tag. The locals are now prefixed
and the clash is documented in place.
gradle/wrapper/gradle-wrapper.properties carried Capacitor's scaffolded
networkTimeout=10000. The wrapper applies that as a socket READ timeout while
fetching its own ~230MB distribution, so any stall longer than ten seconds
aborts the partial download and the next attempt starts from zero. On a slow
or bursty link the wrapper can never complete, which is a plausible failure on
a shared CI runner, not just a local-network quirk. Raised to ten minutes.
Verified locally against the real toolchain (Android SDK platform-36,
build-tools 36.0.0, Gradle 8.14.3, Temurin JDK 21.0.12):
app-debug.apk 8.7 MB com.vishwaguru.app 1.0.0, targetSdk 36
all seven declared permissions present
React bundle under assets/public
17 launcher icon entries
arm64-v8a, armeabi-v7a, x86
app-release.aab 3.8 MB META-INF/VISHWAGU.RSA present, SHA384withRSA
The release bundle was signed with a throwaway key generated only to prove the
path works; it has been deleted and was never committed. The point of the
exercise was to confirm signingConfig selection and the CI check that asserts
the artifact carries a release signature rather than silently falling back to
debug.
Capacitor's plugins pin a Java 21 toolchain, which JDK 23 does not satisfy --
the workflow already pins java-version 21, and that is now confirmed as
required rather than merely conventional.
…ffordances it needed Installed and driven on a Galaxy S20 FE 5G (SM-G781B, Android 13, SDK 33) over USB. The full stack answered: GET /api/issues/recent 200 POST /api/detect-pothole 200 (x7, from the live camera loop) That second endpoint is the one that returned null on main because its handler had no success return, and whose upload field was named `file` while every caller posts `image`. Both fixes are now confirmed against real camera frames on real hardware rather than only against the test suite. Also confirmed on device: the launcher icon and label, the splash screen dismissing via src/native.js, the camera runtime permission prompt appearing and being honoured, WebView navigation between routes, and the Capacitor Network plugin reporting connectivity. Two obstacles surfaced that only appear on a device, and both needed a dev-only affordance rather than a weakened shipping config: Android blocks cleartext HTTP from API 28 onward, so a debug build cannot reach an http:// dev server. app/src/debug/ now carries a manifest overlay and a network security config exempting loopback and private LAN ranges. It lives under src/debug, so it is merged into debug builds and cannot reach a release artifact, which keeps the platform default of HTTPS-only for anything shipped. Separately, the WebView serves the app from https://localhost, so an http:// API is refused as mixed content. That is a Chromium rule, distinct from the cleartext policy, and the network security config cannot waive it. allowMixedContent is now read from CAP_ALLOW_MIXED_CONTENT and defaults to false, so production -- where the API is HTTPS -- is unaffected, and a device test opts in explicitly: CAP_ALLOW_MIXED_CONTENT=true VITE_API_URL=http://127.0.0.1:8123 npm run mobile:sync Worth recording for whoever tests next: when the phone is acting as the hotspot, its own apps route through cellular rather than the hotspot subnet, so the laptop's LAN address is unreachable from the app even though adb shell ping reaches it. `adb reverse tcp:8123 tcp:8123` is the reliable path. Use 127.0.0.1 and not localhost in VITE_API_URL -- Capacitor's WebViewLocalServer intercepts the hostname `localhost`, so requests to it never leave the WebView. The packaged capacitor.config.json in this commit has allowMixedContent false.
🙏 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. |
❌ Deploy Preview for fixmybharat failed. Why did it fail? →
|
|
Important Review skippedToo many files! This PR contains 168 files, which is 68 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (40)
📒 Files selected for processing (168)
You can disable this status message by setting the 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 |
Android job: the workflow refuses to build unless VITE_API_URL is set and absolute HTTPS, and the repository variable did not exist. Set it to https://vishwaguru-backend.onrender.com, which is the same origin netlify.toml already proxies /api/* to. The guard behaved exactly as intended -- a packaged app has no dev proxy or Netlify redirect to resolve a relative path against. Security scan: bandit flagged B104 on backend/__main__.py, the 0.0.0.0 bind. That is required inside a container and on Render, where the platform routes external traffic to the published port, and HOST already allows a narrower bind. Suppressed with `# nosec B104` alongside the existing ruff `# noqa: S104` -- the two tools do not share a suppression syntax, so only ruff was silenced before. Bandit now reports no issues across 5,892 lines. npm advisories: GitHub reported seven high-severity alerts on the default branch, all transitive -- js-yaml, brace-expansion and nanoid. `npm audit fix` in both the root and frontend packages clears every high. Root goes to zero vulnerabilities; frontend keeps three moderates with no non-breaking fix available. Frontend build, lint and all 114 tests still pass on the updated tree. Backend suite unchanged.
🔍 Quality Reminder |
…records
Every endpoint in this service was publicly callable. For most that is
correct: anonymous reporting is a feature of a civic platform, and abuse is
bounded by the rate limiter added earlier. Two were not.
POST /api/grievances/{id}/escalate reassigns a grievance to a different
authority and writes an audit record. POST /api/issues/{id}/verify changes the
status that the public dashboard and officials treat as the record of whether a
problem was fixed. Both were callable by anyone who could reach the API.
backend/auth.py adds an X-API-Key guard for both, compared with
hmac.compare_digest so the check does not leak the key through timing.
It fails closed. If ADMIN_API_KEY is unset the endpoints answer 503, not 200 --
a missing secret must never read as "no authentication required", which is the
usual way an auth layer silently stops protecting anything. A key shorter than
32 characters is refused for the same reason: a placeholder that is accepted
looks like security without being any.
optional_user decodes a bearer token when one is present, for attribution.
Absent a token the request stays anonymous; a token that is present but invalid
is rejected rather than being silently treated as anonymous, which would hide
both client bugs and tampering. Tokens are decoded with an explicit algorithm
list, so an `alg: none` token is rejected -- there is a test for exactly that.
Deliberately small: no user table, no registration, no password handling.
Nothing in the product needs one yet -- issues carry user_email for attribution
only -- and a full identity system to protect two endpoints would open more
surface than it closes. optional_user exists so that adopting real identity
later does not require touching the routes again.
tests/test_auth.py covers a missing key, a wrong key, a weak key, an unset
secret, a correct key, and that the public read endpoints stayed public.
Suite: 221 passed, 4 skipped, 0 failed. ruff and bandit clean.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| # not that the endpoint is open. | ||
| logger.error( | ||
| "%s is not set; refusing privileged request rather than allowing it.", | ||
| ADMIN_API_KEY_ENV, |
| if len(expected) < MIN_API_KEY_LENGTH: | ||
| logger.error( | ||
| "%s is shorter than %d characters; refusing privileged request.", | ||
| ADMIN_API_KEY_ENV, |
| logger.error( | ||
| "%s is shorter than %d characters; refusing privileged request.", | ||
| ADMIN_API_KEY_ENV, | ||
| MIN_API_KEY_LENGTH, |
|
|
||
| secret = os.getenv(JWT_SECRET_ENV, "").strip() | ||
| if not secret: | ||
| logger.error("%s is not set; cannot verify the supplied token.", JWT_SECRET_ENV) |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
39 issues found and verified against the latest diff
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/Landing.jsx">
<violation number="1" location="frontend/src/views/Landing.jsx:3">
P0: Removing the `framer-motion` import leaves the `motion` identifier undefined, yet the JSX still uses `motion.div`, `motion.h1`, etc. in 40+ places. Since framer-motion is not otherwise imported or in package.json, rendering Landing throws `ReferenceError: motion is not defined` and the page goes blank. Restore the import (and add framer-motion as a dependency) or replace all `motion.*` elements with plain HTML elements.</violation>
</file>
<file name=".github/workflows/android.yml">
<violation number="1" location=".github/workflows/android.yml:96">
P1: The Android job cannot run either build because `frontend/android/gradlew` is not executable in Git. Mark the wrapper executable in the repository (preferred), or invoke `bash ./gradlew` in both build steps.</violation>
</file>
<file name="frontend/android/app/src/main/res/values/styles.xml">
<violation number="1" location="frontend/android/app/src/main/res/values/styles.xml:7">
P1: AppTheme references @color/colorPrimary, @color/colorPrimaryDark, and @color/colorAccent, but no colors.xml (or any definition of these colors) exists anywhere in the res tree. AAPT2 fails resource linking on undefined colors, breaking the Android build that this PR claims to have verified. Add a colors.xml under res/values defining the three colors.</violation>
</file>
<file name="backend/main.py">
<violation number="1" location="backend/main.py:200">
P2: A real migration failure is treated as an ordinary already-applied migration and only logged at DEBUG. Ignore only known duplicate/already-exists errors; log other failures at ERROR and fail startup or otherwise prevent serving with an incomplete schema.</violation>
<violation number="2" location="backend/main.py:353">
P1: When AI service initialization fails, `/health` still reports AI as initialized and the deployment remains healthy. Track initialization state and report the service as degraded or fail startup before accepting traffic.</violation>
<violation number="3" location="backend/main.py:553">
P2: `POST /api/issues` persists out-of-range coordinates, unlike the nearby query and issue schemas. Add `ge=-90, le=90` and `ge=-180, le=180` constraints to reject invalid location data.</violation>
<violation number="4" location="backend/main.py:1147">
P1: Every public leaderboard response exposes reporters' raw email addresses. Mask or omit `user_email` before returning leaderboard entries, matching the existing issue-list privacy behavior.</violation>
</file>
<file name="backend/init_db.py">
<violation number="1" location="backend/init_db.py:80">
P1: Any non-idempotency failure is still counted as `skipped` and logged only at DEBUG, so `migrate_db()` returns successfully and reports it as already present instead of failing. Distinguish already-existing schema objects from lock, permission, and missing-table errors, then re-raise the latter.</violation>
</file>
<file name=".github/workflows/auto-deploy.yml">
<violation number="1" location=".github/workflows/auto-deploy.yml:3">
P1: This workflow is kept "for manual dispatch only, for the deployment steps," but the retained `run: python vishwaguru_pipeline.py` step still executes the full auto-merge pipeline with `GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}`. That script (vishwaguru_pipeline.py) fetches every open PR and squash-merges it via the GitHub API, then reverts on failure. A manual dispatch therefore still auto-merges all open, human-unreviewed PRs to main, defeating this PR's purpose. The cron removal only removes the timer, not the merge behavior. Remove the `python vishwaguru_pipeline.py` step (or its PR-merge logic) and keep only the deployment steps.</violation>
</file>
<file name="Dockerfile">
<violation number="1" location="Dockerfile:73">
P1: When this image runs on a platform-assigned `PORT` other than 8000, Uvicorn listens on 8000 and the service fails routing or health checks. Read `PORT` with an 8000 fallback in the command.</violation>
</file>
<file name="backend/models.py">
<violation number="1" location="backend/models.py:19">
P1: When developers use the documented backend-directory launch command, importing `models` now requires a package path that is not available, so the API cannot start. Update the launch commands to run from the repository root with `PYTHONPATH=.` and `backend.main:app`, or otherwise make the documented invocation package-compatible.</violation>
<violation number="2" location="backend/models.py:164">
P2: Switching action_plan from plain Text to JSONEncodedDict moves JSON decoding into the type's process_result_value, which calls json.loads(value) with no exception handling. Rows holding a non-JSON value (e.g. anything written before this column change, which the comment claims are 'unaffected') will now raise a JSONDecodeError the moment that Issue row is loaded, breaking every endpoint that queries issues (such as GET /api/issues/recent) instead of returning None. Previously _coerce_action_plan in main.py guarded reads with a try/except and fell back to None. Guard the decode inside JSONEncodedDict.process_result_value so a single bad row cannot take down issue reads.</violation>
</file>
<file name="backend/unified_detection_service.py">
<violation number="1" location="backend/unified_detection_service.py:93">
P1: When the local model is unavailable, deployments without `HF_TOKEN` now raise `ServiceUnavailableException` instead of using the anonymous public-model path supported by `backend.hf_service`. With only `HUGGINGFACE_HUB_TOKEN`, this reports the backend ready, but both HTTP clients read only `HF_TOKEN`, so the selected credential is never sent. Match this check to the clients’ actual authentication behavior, or perform a health check using the same request headers before selecting the backend.</violation>
</file>
<file name="backend/bot.py">
<violation number="1" location="backend/bot.py:135">
P2: When the bot thread does not stop before `timeout`, this assignment forgets a still-running poller. A subsequent start can create a second Telegram long-poll and replace the event the old thread observes; retain the live thread/state until it exits and use per-thread shutdown state.</violation>
<violation number="2" location="backend/bot.py:246">
P2: When `TELEGRAM_BOT_TOKEN` is configured, `start_bot_thread()` runs a different `Application` from the exported `application`. Callers using `backend.main.application` therefore cannot control the running bot, and that instance is never shut down; reuse one application instance or remove the stale export.</violation>
<violation number="3" location="backend/bot.py:249">
P2: Because the tests set a fake `TELEGRAM_BOT_TOKEN` and call `start_bot_thread()`, `_run` builds a *real* python-telegram-bot Application via `_make_application()` and calls `start_polling()`, which makes outbound calls to api.telegram.org with an invalid token. The `MockApplication` is only used when no token is set, so these 'offline' tests still hit the live Telegram API and can flake/slow the suite. Consider using a mock token that routes to the MockApplication during tests, or monkeypatching `_make_application` in the test setup.</violation>
</file>
<file name="frontend/android/app/src/androidTest/java/com/getcapacitor/myapp/ExampleInstrumentedTest.java">
<violation number="1" location="frontend/android/app/src/androidTest/java/com/getcapacitor/myapp/ExampleInstrumentedTest.java:24">
P2: This instrumented test will always fail: it asserts the app package is "com.getcapacitor.app" (the default Capacitor template value), but the app's applicationId/namespace is "com.vishwaguru.app". Update the expected string to "com.vishwaguru.app", and ideally move the test into a matching package instead of the stale "com.getcapacitor.myapp" template package.</violation>
</file>
<file name="frontend/package.json">
<violation number="1" location="frontend/package.json:19">
P2: The mobile:apk and mobile:aab scripts hardcode `gradlew.bat`, which only runs on Windows. On macOS/Linux `./gradlew` (the shell script) is required, so these scripts break on non-Windows machines. Use a cross-platform invocation or document the Windows-only requirement.</violation>
<violation number="2" location="frontend/package.json:21">
P2: `npm run mobile:assets` fails with "command not found: capacitor-assets" because that binary comes from the `@capacitor/assets` package, which is not declared in dependencies or devDependencies. Add `@capacitor/assets` as a devDependency.</violation>
</file>
<file name="frontend/android/app/build.gradle">
<violation number="1" location="frontend/android/app/build.gradle:16">
P2: When the keystore file exists but any signing credential is missing, `hasReleaseSigning` remains true and the release build uses an incomplete signing config. Require all four signing inputs before enabling `signingConfigs.release`, so a partial local configuration follows the documented fallback and CI fails only at its explicit unsigned-artifact check.</violation>
</file>
<file name=".github/workflows/jules-daily-auto-upgrade.yml">
<violation number="1" location=".github/workflows/jules-daily-auto-upgrade.yml:38">
P2: When `JULES_API_KEY` is missing or Jules rejects the request, `curl -sS` still exits successfully because this call has no `--fail-with-body`, so the manual workflow reports success even though no PR was created. Make HTTP errors fail the step and validate the API key before sending the request.</violation>
</file>
<file name="frontend/android/app/src/debug/res/xml/network_security_config.xml">
<violation number="1" location="frontend/android/app/src/debug/res/xml/network_security_config.xml:21">
P2: The 192.168.0.0 domain entry does not exempt a LAN range. Android's network-security-config <domain> matches an exact hostname, not a CIDR prefix, so it only ever matches the literal name '192.168.0.0' and never a real device IP like 192.168.66.197. The stated intent (exempt private LAN ranges for a dev server) is therefore met only for the single hard-coded 192.168.66.197 host, and HTTP to any other LAN machine is still blocked in debug builds. Remove the misleading 192.168.0.0 entry and instead list each concrete dev-host IP you need, or document that each developer must add their own host.</violation>
</file>
<file name="tests/test_local_ml_service.py">
<violation number="1" location="tests/test_local_ml_service.py:291">
P2: For the vandalism route this guard tests the wrong function. /api/detect-vandalism dispatches detect_vandalism_unified (from backend.unified_detection_service), but the test asserts callable(detect_vandalism) (backend.vandalism_detection), which no route uses. A regression in the vandalism route's real wiring would therefore slip past the very check this test claims to add. Import detect_vandalism_unified instead of (or in addition to) detect_vandalism and assert that one, and drop the stale claim in the docstring.</violation>
</file>
<file name="tests/manual_integration_test.py">
<violation number="1" location="tests/manual_integration_test.py:11">
P2: Running this script the normal way, `python tests/manual_integration_test.py`, now fails with ModuleNotFoundError: No module named 'backend'. Python adds only the script's directory (`tests/`) to sys.path, so the `backend` package is only importable when the repo root is on the path (e.g. `python -m tests.manual_integration_test` from the root). The previous code explicitly inserted the backend path and ran from any directory. Restore a sys.path insertion (of the repo root, or backend) before the import so the standalone script keeps working.</violation>
</file>
<file name="tests/test_api_contract.py">
<violation number="1" location="tests/test_api_contract.py:153">
P2: The field-name test never runs for /api/transcribe-audio. _detector_paths() filters on the "/api/detect-" prefix, but transcribe-audio (which main.py documents as accepting `file`) does not match that prefix, so the AUDIO_FIELD_PATHS branch for it is dead and the audio transcription field contract is left unguarded.</violation>
<violation number="2" location="tests/test_api_contract.py:171">
P2: This contract test runs real detection for 4 routes it meant to only field-check. _detector_paths() includes /api/detect-pothole, /api/detect-garbage, /api/detect-vandalism and /api/detect-infrastructure, but their service functions aren't stubbed, so each PR triggers a real YOLO model load / hosted inference call even though the assertion only distinguishes 422/405. Also the detect_infrastructure_local stub is dead because the infrastructure handler now calls detect_infrastructure_unified. Stub every invoked service so the test stays fast and offline.</violation>
</file>
<file name="backend/grievance_routes.py">
<violation number="1" location="backend/grievance_routes.py:147">
P2: When a grievance has a NULL or non-active status, `escalation_stats` counts it as active because it uses `total - resolved` instead of the defined active-status set. Count rows with `Grievance.status.in_(_ACTIVE_STATUSES)` so the active tile matches its documented status definition.</violation>
</file>
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:143">
P2: The security scans can never fail the build: both `pip-audit --strict` and `npm audit --audit-level=high` are set to `continue-on-error: true`. The `--strict` flag on pip-audit is therefore dead — it exists to fail the job on any vulnerability, but the same step ignores the job's failure. This contradicts the PR description listing security audits as CI guards; vulnerabilities, including high-level npm audit findings, ship without breaking CI. Either drop `--strict`/the audit-level and treat these as informational, or remove `continue-on-error` so they actually gate merges.</violation>
</file>
<file name="frontend/index.html">
<violation number="1" location="frontend/index.html:12">
P2: This page ends up with two `<link rel="manifest">` declarations: the manual `/manifest.json` added here and the `manifest.webmanifest` that `vite-plugin-pwa` auto-injects because `manifest` is set in vite.config.js. The PWA manifest is now defined in two parallel places (static `public/manifest.json` and the vite.config manifest object) that must be kept in sync by hand. Pick one source of truth and drop the other; to rely on the plugin-injected manifest, remove this line (and the stray static `public/manifest.json`), or remove the `manifest` object from vite.config.js and keep only the static file.</violation>
</file>
<file name="frontend/android/app/src/main/res/xml/file_paths.xml">
<violation number="1" location="frontend/android/app/src/main/res/xml/file_paths.xml:3">
P2: The FileProvider here maps `external-path` to `.` , which grants URI access to the entire shared external storage directory (`Environment.getExternalStorageDirectory()`). This is the classic overly-broad FileProvider path: if any code or plugin later calls `FileProvider.getUriForFile()` for a user image (this manifest wires the provider and `grantUriPermissions="true"`), it can expose any file under shared storage rather than only the app's own image directory. Scope the path to a specific subdirectory.</violation>
</file>
<file name="frontend/android/app/src/main/res/drawable-v24/ic_launcher_foreground.xml">
<violation number="1" location="frontend/android/app/src/main/res/drawable-v24/ic_launcher_foreground.xml:1">
P3: This vector is never used: the adaptive-icon foreground is referenced as @mipmap/ic_launcher_foreground (resolved from the PNG density buckets), and nothing references @drawable/ic_launcher_foreground, so the file placed in drawable-v24 has no effect on the rendered icon. If the intent is to use this vector as the launcher foreground, move it to res/mipmap-anydpi-v26/ic_launcher_foreground.xml (or point the adaptive-icon <foreground> at @drawable/ic_launcher_foreground); otherwise it is dead code that should be removed.</violation>
</file>
<file name="pyproject.toml">
<violation number="1" location="pyproject.toml:44">
P3: `ignore::DeprecationWarning` is applied globally, silencing every deprecation warning across the entire test suite. That hides dependency-level and project-level deprecations that normally warn about upcoming breaking changes, so CI stays green until a dependency upgrade actually breaks the app. Scope the ignore to the specific warnings you know are noise (e.g. `ignore:module was already imported:DeprecationWarning`) instead of turning off the category everywhere.</violation>
</file>
<file name="backend/requirements.in">
<violation number="1" location="backend/requirements.in:15">
P3: backend/requirements.in declares firebase-functions, firebase-admin, a2wsgi, and pywebpush, but none of them is imported anywhere in backend/*.py. These belong to the separate `functions/` Firebase project (functions/main.py imports a2wsgi, and functions/requirements.txt already pins this set). Keeping them in the backend requirements adds a heavy unused transitive chain (flask, functions-framework, cloudevents, cryptography) to the backend runtime image and makes the two dependency manifests drift independently. Remove these lines from backend/requirements.in and re-run uv pip compile.</violation>
</file>
<file name=".github/workflows/label-bot-prs.yml">
<violation number="1" location=".github/workflows/label-bot-prs.yml:30">
P2: addLabels fails with a 422 (and the run shows as failed) when a label name does not already exist in the repository; the labels 'bot-generated' and 'needs-human-review' are not created by this call and are configured only in the repo UI, so there is no in-repo guarantee they exist. If either is missing, the whole point of the workflow is silently lost and no review-required label is applied to bot PRs. Create the labels when missing (issues.createLabel on 404) before addLabels, or wrap the call in try/catch so a missing label is visible instead of a red herring failure.</violation>
</file>
<file name="tests/test_mh_endpoint.py">
<violation number="1" location="tests/test_mh_endpoint.py:16">
P3: The retained `__main__` runner is now broken: running `python tests/test_mh_endpoint.py` directly adds only `tests/` to sys.path, so `from backend.main import app` raises ModuleNotFoundError. The repo-root conftest.py only makes `backend.*` importable under pytest. Add a repo-root sys.path insert (guarded by `__package__`) before the import so the direct-run path still works.</violation>
</file>
<file name="tests/test_model_thread_safety.py">
<violation number="1" location="tests/test_model_thread_safety.py:20">
P3: Both tests leave the detection module polluted after they finish: the reset runs only at the start, so _model stays bound to "mock_model" (and pothole leaves _model_initialized=True) in the shared module namespace. Any later test in the same pytest session that calls get_model() would silently get the stale mock instead of triggering a real load. The sibling test_pothole_detection_thread_safety.py uses a fixture that resets after every test; mirror that here by resetting the module after the threads join (or wrapping the body in try/finally) so no test leaks state into the next.</violation>
</file>
<file name="frontend/src/StreetLightDetector.jsx">
<violation number="1" location="frontend/src/StreetLightDetector.jsx:39">
P2: If VITE_API_URL is configured with a trailing slash, `${API_URL}/api/detect-street-light` builds `//api/detect-street-light`, which FastAPI/uvicorn serves as a distinct 404 route. Normalize the base URL by stripping trailing slashes so the concatenation is safe regardless of how the env var is set.</violation>
</file>
<file name="frontend/android/app/src/main/res/drawable/ic_launcher_background.xml">
<violation number="1" location="frontend/android/app/src/main/res/drawable/ic_launcher_background.xml:1">
P3: This new ic_launcher_background.xml vector drawable is never referenced. The adaptive launcher icons (mipmap-anydpi-v26/ic_launcher.xml, ic_launcher_round.xml) reference @color/ic_launcher_background (white #FFFFFF) instead of @drawable/ic_launcher_background, and nothing else uses this drawable. It ships unused in every APK and its teal-grid design is also not what the app actually shows. Either wire it into the adaptive-icon background or remove the file.</violation>
</file>
<file name="backend/requirements.txt">
<violation number="1" location="backend/requirements.txt:1">
P3: This lockfile resolves only Python 3.12 (header: `--python-version 3.12 --python-platform linux`), and its pinned packages (e.g. numpy==2.5.2, scipy==1.18.0) require newer interpreters. README.md badge and backend/README.md still tell users `pip install -r backend/requirements.txt` works on "Python 3.8+". A dev on 3.8-3.11 following the README will hit resolution/wheel failures. Since all deployment targets (Docker python:3.12-slim, render.yaml PYTHON_VERSION 3.12.0, CI matrix 3.12) are 3.12, update the README/badge to require Python 3.12 so the documented local path matches the lock.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -1,6 +1,5 @@ | |||
| import React from 'react'; | |||
| import { useNavigate } from 'react-router-dom'; | |||
| import { motion } from 'framer-motion'; | |||
Contributor
There was a problem hiding this comment.
P0: Removing the framer-motion import leaves the motion identifier undefined, yet the JSX still uses motion.div, motion.h1, etc. in 40+ places. Since framer-motion is not otherwise imported or in package.json, rendering Landing throws ReferenceError: motion is not defined and the page goes blank. Restore the import (and add framer-motion as a dependency) or replace all motion.* elements with plain HTML elements.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/views/Landing.jsx, line 3:
<comment>Removing the `framer-motion` import leaves the `motion` identifier undefined, yet the JSX still uses `motion.div`, `motion.h1`, etc. in 40+ places. Since framer-motion is not otherwise imported or in package.json, rendering Landing throws `ReferenceError: motion is not defined` and the page goes blank. Restore the import (and add framer-motion as a dependency) or replace all `motion.*` elements with plain HTML elements.</comment>
<file context>
@@ -1,6 +1,5 @@
import React from 'react';
import { useNavigate } from 'react-router-dom';
-import { motion } from 'framer-motion';
import {
Building2, MessageCircle, Users, Shield, Star, FileText,
Search, Lock, ShoppingCart, User, ArrowRight
</file context>
| - name: Build debug APK | ||
| if: github.event_name == 'pull_request' | ||
| working-directory: frontend/android | ||
| run: ./gradlew --no-daemon assembleDebug |
Contributor
There was a problem hiding this comment.
P1: The Android job cannot run either build because frontend/android/gradlew is not executable in Git. Mark the wrapper executable in the repository (preferred), or invoke bash ./gradlew in both build steps.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/android.yml, line 96:
<comment>The Android job cannot run either build because `frontend/android/gradlew` is not executable in Git. Mark the wrapper executable in the repository (preferred), or invoke `bash ./gradlew` in both build steps.</comment>
<file context>
@@ -0,0 +1,137 @@
+ - name: Build debug APK
+ if: github.event_name == 'pull_request'
+ working-directory: frontend/android
+ run: ./gradlew --no-daemon assembleDebug
+
+ - name: Build release bundle
</file context>
| <!-- Base application theme. --> | ||
| <style name="AppTheme" parent="Theme.AppCompat.Light.DarkActionBar"> | ||
| <!-- Customize your theme here. --> | ||
| <item name="colorPrimary">@color/colorPrimary</item> |
Contributor
There was a problem hiding this comment.
P1: AppTheme references @color/colorPrimary, @color/colorPrimaryDark, and @color/colorAccent, but no colors.xml (or any definition of these colors) exists anywhere in the res tree. AAPT2 fails resource linking on undefined colors, breaking the Android build that this PR claims to have verified. Add a colors.xml under res/values defining the three colors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/android/app/src/main/res/values/styles.xml, line 7:
<comment>AppTheme references @color/colorPrimary, @color/colorPrimaryDark, and @color/colorAccent, but no colors.xml (or any definition of these colors) exists anywhere in the res tree. AAPT2 fails resource linking on undefined colors, breaking the Android build that this PR claims to have verified. Add a colors.xml under res/values defining the three colors.</comment>
<file context>
@@ -0,0 +1,22 @@
+ <!-- Base application theme. -->
+ <style name="AppTheme" parent="Theme.AppCompat.Light.DarkActionBar">
+ <!-- Customize your theme here. -->
+ <item name="colorPrimary">@color/colorPrimary</item>
+ <item name="colorPrimaryDark">@color/colorPrimaryDark</item>
+ <item name="colorAccent">@color/colorAccent</item>
</file context>
| "leaderboard": [ | ||
| { | ||
| "rank": index, | ||
| "user_email": row.user_email, |
Contributor
There was a problem hiding this comment.
P1: Every public leaderboard response exposes reporters' raw email addresses. Mask or omit user_email before returning leaderboard entries, matching the existing issue-list privacy behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/main.py, line 1146:
<comment>Every public leaderboard response exposes reporters' raw email addresses. Mask or omit `user_email` before returning leaderboard entries, matching the existing issue-list privacy behavior.</comment>
<file context>
@@ -505,3 +881,322 @@ def upvote_issue(issue_id: int, db: Session = Depends(get_db)):
+ "leaderboard": [
+ {
+ "rank": index,
+ "user_email": row.user_email,
+ "reports_count": int(row.reports_count or 0),
+ "upvotes": int(row.upvotes or 0),
</file context>
Suggested change
| "user_email": row.user_email, | |
| "user_email": ( | |
| f"{row.user_email[:1]}***{row.user_email[row.user_email.index('@') :]}" | |
| if "@" in row.user_email | |
| else "***" | |
| ), |
| "database": "connected", | ||
| "ai_services": "initialized" | ||
| } | ||
| services={"database": "connected", "ai_services": "initialized"}, |
Contributor
There was a problem hiding this comment.
P1: When AI service initialization fails, /health still reports AI as initialized and the deployment remains healthy. Track initialization state and report the service as degraded or fail startup before accepting traffic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/main.py, line 352:
<comment>When AI service initialization fails, `/health` still reports AI as initialized and the deployment remains healthy. Track initialization state and report the service as degraded or fail startup before accepting traffic.</comment>
<file context>
@@ -124,70 +325,64 @@ def get_db():
- "database": "connected",
- "ai_services": "initialized"
- }
+ services={"database": "connected", "ai_services": "initialized"},
)
</file context>
| python-magic | ||
| pywebpush | ||
| Pillow | ||
| firebase-functions |
Contributor
There was a problem hiding this comment.
P3: backend/requirements.in declares firebase-functions, firebase-admin, a2wsgi, and pywebpush, but none of them is imported anywhere in backend/*.py. These belong to the separate functions/ Firebase project (functions/main.py imports a2wsgi, and functions/requirements.txt already pins this set). Keeping them in the backend requirements adds a heavy unused transitive chain (flask, functions-framework, cloudevents, cryptography) to the backend runtime image and makes the two dependency manifests drift independently. Remove these lines from backend/requirements.in and re-run uv pip compile.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/requirements.in, line 15:
<comment>backend/requirements.in declares firebase-functions, firebase-admin, a2wsgi, and pywebpush, but none of them is imported anywhere in backend/*.py. These belong to the separate `functions/` Firebase project (functions/main.py imports a2wsgi, and functions/requirements.txt already pins this set). Keeping them in the backend requirements adds a heavy unused transitive chain (flask, functions-framework, cloudevents, cryptography) to the backend runtime image and makes the two dependency manifests drift independently. Remove these lines from backend/requirements.in and re-run uv pip compile.</comment>
<file context>
@@ -0,0 +1,21 @@
+python-magic
+pywebpush
+Pillow
+firebase-functions
+firebase-admin
+a2wsgi
</file context>
| os.environ["GEMINI_API_KEY"] = "" # Test without Gemini | ||
|
|
||
| from main import app | ||
| from backend.main import app |
Contributor
There was a problem hiding this comment.
P3: The retained __main__ runner is now broken: running python tests/test_mh_endpoint.py directly adds only tests/ to sys.path, so from backend.main import app raises ModuleNotFoundError. The repo-root conftest.py only makes backend.* importable under pytest. Add a repo-root sys.path insert (guarded by __package__) before the import so the direct-run path still works.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_mh_endpoint.py, line 16:
<comment>The retained `__main__` runner is now broken: running `python tests/test_mh_endpoint.py` directly adds only `tests/` to sys.path, so `from backend.main import app` raises ModuleNotFoundError. The repo-root conftest.py only makes `backend.*` importable under pytest. Add a repo-root sys.path insert (guarded by `__package__`) before the import so the direct-run path still works.</comment>
<file context>
@@ -1,100 +1,101 @@
+os.environ["GEMINI_API_KEY"] = "" # Test without Gemini
-from main import app
+from backend.main import app
client = TestClient(app, raise_server_exceptions=False)
</file context>
Suggested change
| from backend.main import app | |
| # Allow running this file directly (``python tests/test_mh_endpoint.py``). | |
| if __package__ is None: | |
| sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) | |
| from backend.main import app |
| # initialised by an earlier test returns immediately and load_model is never | ||
| # called -- the load count comes back 0 and the test fails only when run | ||
| # after its neighbours. | ||
| garbage_detection.reset_model() |
Contributor
There was a problem hiding this comment.
P3: Both tests leave the detection module polluted after they finish: the reset runs only at the start, so _model stays bound to "mock_model" (and pothole leaves _model_initialized=True) in the shared module namespace. Any later test in the same pytest session that calls get_model() would silently get the stale mock instead of triggering a real load. The sibling test_pothole_detection_thread_safety.py uses a fixture that resets after every test; mirror that here by resetting the module after the threads join (or wrapping the body in try/finally) so no test leaks state into the next.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_model_thread_safety.py, line 20:
<comment>Both tests leave the detection module polluted after they finish: the reset runs only at the start, so _model stays bound to "mock_model" (and pothole leaves _model_initialized=True) in the shared module namespace. Any later test in the same pytest session that calls get_model() would silently get the stale mock instead of triggering a real load. The sibling test_pothole_detection_thread_safety.py uses a fixture that resets after every test; mirror that here by resetting the module after the threads join (or wrapping the body in try/finally) so no test leaks state into the next.</comment>
<file context>
@@ -2,144 +2,156 @@
+ # initialised by an earlier test returns immediately and load_model is never
+ # called -- the load count comes back 0 and the test fails only when run
+ # after its neighbours.
+ garbage_detection.reset_model()
+
# Track how many times the model was loaded and ensure sequential execution
</file context>
| @@ -0,0 +1,170 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
Contributor
There was a problem hiding this comment.
P3: This new ic_launcher_background.xml vector drawable is never referenced. The adaptive launcher icons (mipmap-anydpi-v26/ic_launcher.xml, ic_launcher_round.xml) reference @color/ic_launcher_background (white #FFFFFF) instead of @drawable/ic_launcher_background, and nothing else uses this drawable. It ships unused in every APK and its teal-grid design is also not what the app actually shows. Either wire it into the adaptive-icon background or remove the file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/android/app/src/main/res/drawable/ic_launcher_background.xml, line 1:
<comment>This new ic_launcher_background.xml vector drawable is never referenced. The adaptive launcher icons (mipmap-anydpi-v26/ic_launcher.xml, ic_launcher_round.xml) reference @color/ic_launcher_background (white #FFFFFF) instead of @drawable/ic_launcher_background, and nothing else uses this drawable. It ships unused in every APK and its teal-grid design is also not what the app actually shows. Either wire it into the adaptive-icon background or remove the file.</comment>
<file context>
@@ -0,0 +1,170 @@
+<?xml version="1.0" encoding="utf-8"?>
+<vector xmlns:android="http://schemas.android.com/apk/res/android"
+ android:width="108dp"
</file context>
| # Spatial deduplication dependencies | ||
| scikit-learn | ||
| numpy | ||
| # Locked with: uv pip compile backend/requirements.in --python-version 3.12 --python-platform linux -o backend/requirements.txt |
Contributor
There was a problem hiding this comment.
P3: This lockfile resolves only Python 3.12 (header: --python-version 3.12 --python-platform linux), and its pinned packages (e.g. numpy==2.5.2, scipy==1.18.0) require newer interpreters. README.md badge and backend/README.md still tell users pip install -r backend/requirements.txt works on "Python 3.8+". A dev on 3.8-3.11 following the README will hit resolution/wheel failures. Since all deployment targets (Docker python:3.12-slim, render.yaml PYTHON_VERSION 3.12.0, CI matrix 3.12) are 3.12, update the README/badge to require Python 3.12 so the documented local path matches the lock.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/requirements.txt, line 1:
<comment>This lockfile resolves only Python 3.12 (header: `--python-version 3.12 --python-platform linux`), and its pinned packages (e.g. numpy==2.5.2, scipy==1.18.0) require newer interpreters. README.md badge and backend/README.md still tell users `pip install -r backend/requirements.txt` works on "Python 3.8+". A dev on 3.8-3.11 following the README will hit resolution/wheel failures. Since all deployment targets (Docker python:3.12-slim, render.yaml PYTHON_VERSION 3.12.0, CI matrix 3.12) are 3.12, update the README/badge to require Python 3.12 so the documented local path matches the lock.</comment>
<file context>
@@ -1,20 +1,332 @@
-# Spatial deduplication dependencies
-scikit-learn
-numpy
+# Locked with: uv pip compile backend/requirements.in --python-version 3.12 --python-platform linux -o backend/requirements.txt
+# Do not edit by hand. Edit backend/requirements.in and re-run the command above.
+a2wsgi==1.10.10
</file context>
Contributor
There was a problem hiding this comment.
35 issues found and verified against the latest diff
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/package.json">
<violation number="1" location="frontend/package.json:19">
P2: The mobile:apk and mobile:aab scripts hardcode `gradlew.bat`, which only runs on Windows. On macOS/Linux `./gradlew` (the shell script) is required, so these scripts break on non-Windows machines. Use a cross-platform invocation or document the Windows-only requirement.</violation>
<violation number="2" location="frontend/package.json:21">
P2: `npm run mobile:assets` fails with "command not found: capacitor-assets" because that binary comes from the `@capacitor/assets` package, which is not declared in dependencies or devDependencies. Add `@capacitor/assets` as a devDependency.</violation>
</file>
<file name="frontend/android/app/src/main/res/values/styles.xml">
<violation number="1" location="frontend/android/app/src/main/res/values/styles.xml:7">
P1: AppTheme references @color/colorPrimary, @color/colorPrimaryDark, and @color/colorAccent, but no colors.xml (or any definition of these colors) exists anywhere in the res tree. AAPT2 fails resource linking on undefined colors, breaking the Android build that this PR claims to have verified. Add a colors.xml under res/values defining the three colors.</violation>
</file>
<file name="frontend/android/app/build.gradle">
<violation number="1" location="frontend/android/app/build.gradle:16">
P2: When the keystore file exists but any signing credential is missing, `hasReleaseSigning` remains true and the release build uses an incomplete signing config. Require all four signing inputs before enabling `signingConfigs.release`, so a partial local configuration follows the documented fallback and CI fails only at its explicit unsigned-artifact check.</violation>
</file>
<file name="frontend/android/app/src/debug/res/xml/network_security_config.xml">
<violation number="1" location="frontend/android/app/src/debug/res/xml/network_security_config.xml:21">
P2: The 192.168.0.0 domain entry does not exempt a LAN range. Android's network-security-config <domain> matches an exact hostname, not a CIDR prefix, so it only ever matches the literal name '192.168.0.0' and never a real device IP like 192.168.66.197. The stated intent (exempt private LAN ranges for a dev server) is therefore met only for the single hard-coded 192.168.66.197 host, and HTTP to any other LAN machine is still blocked in debug builds. Remove the misleading 192.168.0.0 entry and instead list each concrete dev-host IP you need, or document that each developer must add their own host.</violation>
</file>
<file name="frontend/android/app/src/main/res/xml/file_paths.xml">
<violation number="1" location="frontend/android/app/src/main/res/xml/file_paths.xml:3">
P2: The FileProvider here maps `external-path` to `.` , which grants URI access to the entire shared external storage directory (`Environment.getExternalStorageDirectory()`). This is the classic overly-broad FileProvider path: if any code or plugin later calls `FileProvider.getUriForFile()` for a user image (this manifest wires the provider and `grantUriPermissions="true"`), it can expose any file under shared storage rather than only the app's own image directory. Scope the path to a specific subdirectory.</violation>
</file>
<file name="frontend/android/app/src/main/res/drawable-v24/ic_launcher_foreground.xml">
<violation number="1" location="frontend/android/app/src/main/res/drawable-v24/ic_launcher_foreground.xml:1">
P3: This vector is never used: the adaptive-icon foreground is referenced as @mipmap/ic_launcher_foreground (resolved from the PNG density buckets), and nothing references @drawable/ic_launcher_foreground, so the file placed in drawable-v24 has no effect on the rendered icon. If the intent is to use this vector as the launcher foreground, move it to res/mipmap-anydpi-v26/ic_launcher_foreground.xml (or point the adaptive-icon <foreground> at @drawable/ic_launcher_foreground); otherwise it is dead code that should be removed.</violation>
</file>
<file name="backend/main.py">
<violation number="1" location="backend/main.py:552">
P2: `POST /api/issues` persists out-of-range coordinates, unlike the nearby query and issue schemas. Add `ge=-90, le=90` and `ge=-180, le=180` constraints to reject invalid location data.</violation>
<violation number="2" location="backend/main.py:1146">
P1: Every public leaderboard response exposes reporters' raw email addresses. Mask or omit `user_email` before returning leaderboard entries, matching the existing issue-list privacy behavior.</violation>
</file>
<file name="backend/requirements.in">
<violation number="1" location="backend/requirements.in:15">
P3: backend/requirements.in declares firebase-functions, firebase-admin, a2wsgi, and pywebpush, but none of them is imported anywhere in backend/*.py. These belong to the separate `functions/` Firebase project (functions/main.py imports a2wsgi, and functions/requirements.txt already pins this set). Keeping them in the backend requirements adds a heavy unused transitive chain (flask, functions-framework, cloudevents, cryptography) to the backend runtime image and makes the two dependency manifests drift independently. Remove these lines from backend/requirements.in and re-run uv pip compile.</violation>
</file>
<file name="tests/test_mh_endpoint.py">
<violation number="1" location="tests/test_mh_endpoint.py:16">
P3: The retained `__main__` runner is now broken: running `python tests/test_mh_endpoint.py` directly adds only `tests/` to sys.path, so `from backend.main import app` raises ModuleNotFoundError. The repo-root conftest.py only makes `backend.*` importable under pytest. Add a repo-root sys.path insert (guarded by `__package__`) before the import so the direct-run path still works.</violation>
</file>
<file name="backend/models.py">
<violation number="1" location="backend/models.py:19">
P1: When developers use the documented backend-directory launch command, importing `models` now requires a package path that is not available, so the API cannot start. Update the launch commands to run from the repository root with `PYTHONPATH=.` and `backend.main:app`, or otherwise make the documented invocation package-compatible.</violation>
<violation number="2" location="backend/models.py:164">
P2: Switching action_plan from plain Text to JSONEncodedDict moves JSON decoding into the type's process_result_value, which calls json.loads(value) with no exception handling. Rows holding a non-JSON value (e.g. anything written before this column change, which the comment claims are 'unaffected') will now raise a JSONDecodeError the moment that Issue row is loaded, breaking every endpoint that queries issues (such as GET /api/issues/recent) instead of returning None. Previously _coerce_action_plan in main.py guarded reads with a try/except and fell back to None. Guard the decode inside JSONEncodedDict.process_result_value so a single bad row cannot take down issue reads.</violation>
</file>
<file name="frontend/index.html">
<violation number="1" location="frontend/index.html:12">
P2: This page ends up with two `<link rel="manifest">` declarations: the manual `/manifest.json` added here and the `manifest.webmanifest` that `vite-plugin-pwa` auto-injects because `manifest` is set in vite.config.js. The PWA manifest is now defined in two parallel places (static `public/manifest.json` and the vite.config manifest object) that must be kept in sync by hand. Pick one source of truth and drop the other; to rely on the plugin-injected manifest, remove this line (and the stray static `public/manifest.json`), or remove the `manifest` object from vite.config.js and keep only the static file.</violation>
</file>
<file name="backend/bot.py">
<violation number="1" location="backend/bot.py:135">
P2: When the bot thread does not stop before `timeout`, this assignment forgets a still-running poller. A subsequent start can create a second Telegram long-poll and replace the event the old thread observes; retain the live thread/state until it exits and use per-thread shutdown state.</violation>
<violation number="2" location="backend/bot.py:246">
P2: When `TELEGRAM_BOT_TOKEN` is configured, `start_bot_thread()` runs a different `Application` from the exported `application`. Callers using `backend.main.application` therefore cannot control the running bot, and that instance is never shut down; reuse one application instance or remove the stale export.</violation>
<violation number="3" location="backend/bot.py:249">
P2: Because the tests set a fake `TELEGRAM_BOT_TOKEN` and call `start_bot_thread()`, `_run` builds a *real* python-telegram-bot Application via `_make_application()` and calls `start_polling()`, which makes outbound calls to api.telegram.org with an invalid token. The `MockApplication` is only used when no token is set, so these 'offline' tests still hit the live Telegram API and can flake/slow the suite. Consider using a mock token that routes to the MockApplication during tests, or monkeypatching `_make_application` in the test setup.</violation>
</file>
<file name="frontend/android/app/src/androidTest/java/com/getcapacitor/myapp/ExampleInstrumentedTest.java">
<violation number="1" location="frontend/android/app/src/androidTest/java/com/getcapacitor/myapp/ExampleInstrumentedTest.java:24">
P2: This instrumented test will always fail: it asserts the app package is "com.getcapacitor.app" (the default Capacitor template value), but the app's applicationId/namespace is "com.vishwaguru.app". Update the expected string to "com.vishwaguru.app", and ideally move the test into a matching package instead of the stale "com.getcapacitor.myapp" template package.</violation>
</file>
<file name="Dockerfile">
<violation number="1" location="Dockerfile:73">
P1: When this image runs on a platform-assigned `PORT` other than 8000, Uvicorn listens on 8000 and the service fails routing or health checks. Read `PORT` with an 8000 fallback in the command.</violation>
</file>
<file name="frontend/src/views/Landing.jsx">
<violation number="1" location="frontend/src/views/Landing.jsx:3">
P0: Removing the `framer-motion` import leaves the `motion` identifier undefined, yet the JSX still uses `motion.div`, `motion.h1`, etc. in 40+ places. Since framer-motion is not otherwise imported or in package.json, rendering Landing throws `ReferenceError: motion is not defined` and the page goes blank. Restore the import (and add framer-motion as a dependency) or replace all `motion.*` elements with plain HTML elements.</violation>
</file>
<file name=".github/workflows/android.yml">
<violation number="1" location=".github/workflows/android.yml:96">
P1: The Android job cannot run either build because `frontend/android/gradlew` is not executable in Git. Mark the wrapper executable in the repository (preferred), or invoke `bash ./gradlew` in both build steps.</violation>
</file>
<file name="backend/grievance_routes.py">
<violation number="1" location="backend/grievance_routes.py:146">
P2: When a grievance has a NULL or non-active status, `escalation_stats` counts it as active because it uses `total - resolved` instead of the defined active-status set. Count rows with `Grievance.status.in_(_ACTIVE_STATUSES)` so the active tile matches its documented status definition.</violation>
</file>
<file name="backend/unified_detection_service.py">
<violation number="1" location="backend/unified_detection_service.py:93">
P1: When the local model is unavailable, deployments without `HF_TOKEN` now raise `ServiceUnavailableException` instead of using the anonymous public-model path supported by `backend.hf_service`. With only `HUGGINGFACE_HUB_TOKEN`, this reports the backend ready, but both HTTP clients read only `HF_TOKEN`, so the selected credential is never sent. Match this check to the clients’ actual authentication behavior, or perform a health check using the same request headers before selecting the backend.</violation>
</file>
<file name=".github/workflows/auto-deploy.yml">
<violation number="1" location=".github/workflows/auto-deploy.yml:3">
P1: This workflow is kept "for manual dispatch only, for the deployment steps," but the retained `run: python vishwaguru_pipeline.py` step still executes the full auto-merge pipeline with `GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}`. That script (vishwaguru_pipeline.py) fetches every open PR and squash-merges it via the GitHub API, then reverts on failure. A manual dispatch therefore still auto-merges all open, human-unreviewed PRs to main, defeating this PR's purpose. The cron removal only removes the timer, not the merge behavior. Remove the `python vishwaguru_pipeline.py` step (or its PR-merge logic) and keep only the deployment steps.</violation>
</file>
<file name="tests/manual_integration_test.py">
<violation number="1" location="tests/manual_integration_test.py:11">
P2: Running this script the normal way, `python tests/manual_integration_test.py`, now fails with ModuleNotFoundError: No module named 'backend'. Python adds only the script's directory (`tests/`) to sys.path, so the `backend` package is only importable when the repo root is on the path (e.g. `python -m tests.manual_integration_test` from the root). The previous code explicitly inserted the backend path and ran from any directory. Restore a sys.path insertion (of the repo root, or backend) before the import so the standalone script keeps working.</violation>
</file>
<file name="frontend/android/app/src/main/res/drawable/ic_launcher_background.xml">
<violation number="1" location="frontend/android/app/src/main/res/drawable/ic_launcher_background.xml:1">
P3: This new ic_launcher_background.xml vector drawable is never referenced. The adaptive launcher icons (mipmap-anydpi-v26/ic_launcher.xml, ic_launcher_round.xml) reference @color/ic_launcher_background (white #FFFFFF) instead of @drawable/ic_launcher_background, and nothing else uses this drawable. It ships unused in every APK and its teal-grid design is also not what the app actually shows. Either wire it into the adaptive-icon background or remove the file.</violation>
</file>
<file name="tests/test_model_thread_safety.py">
<violation number="1" location="tests/test_model_thread_safety.py:20">
P3: Both tests leave the detection module polluted after they finish: the reset runs only at the start, so _model stays bound to "mock_model" (and pothole leaves _model_initialized=True) in the shared module namespace. Any later test in the same pytest session that calls get_model() would silently get the stale mock instead of triggering a real load. The sibling test_pothole_detection_thread_safety.py uses a fixture that resets after every test; mirror that here by resetting the module after the threads join (or wrapping the body in try/finally) so no test leaks state into the next.</violation>
</file>
<file name="pyproject.toml">
<violation number="1" location="pyproject.toml:44">
P3: `ignore::DeprecationWarning` is applied globally, silencing every deprecation warning across the entire test suite. That hides dependency-level and project-level deprecations that normally warn about upcoming breaking changes, so CI stays green until a dependency upgrade actually breaks the app. Scope the ignore to the specific warnings you know are noise (e.g. `ignore:module was already imported:DeprecationWarning`) instead of turning off the category everywhere.</violation>
</file>
<file name="tests/test_api_contract.py">
<violation number="1" location="tests/test_api_contract.py:153">
P2: The field-name test never runs for /api/transcribe-audio. _detector_paths() filters on the "/api/detect-" prefix, but transcribe-audio (which main.py documents as accepting `file`) does not match that prefix, so the AUDIO_FIELD_PATHS branch for it is dead and the audio transcription field contract is left unguarded.</violation>
</file>
<file name=".github/workflows/label-bot-prs.yml">
<violation number="1" location=".github/workflows/label-bot-prs.yml:30">
P2: addLabels fails with a 422 (and the run shows as failed) when a label name does not already exist in the repository; the labels 'bot-generated' and 'needs-human-review' are not created by this call and are configured only in the repo UI, so there is no in-repo guarantee they exist. If either is missing, the whole point of the workflow is silently lost and no review-required label is applied to bot PRs. Create the labels when missing (issues.createLabel on 404) before addLabels, or wrap the call in try/catch so a missing label is visible instead of a red herring failure.</violation>
</file>
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:143">
P2: The security scans can never fail the build: both `pip-audit --strict` and `npm audit --audit-level=high` are set to `continue-on-error: true`. The `--strict` flag on pip-audit is therefore dead — it exists to fail the job on any vulnerability, but the same step ignores the job's failure. This contradicts the PR description listing security audits as CI guards; vulnerabilities, including high-level npm audit findings, ship without breaking CI. Either drop `--strict`/the audit-level and treat these as informational, or remove `continue-on-error` so they actually gate merges.</violation>
</file>
<file name=".github/workflows/jules-daily-auto-upgrade.yml">
<violation number="1" location=".github/workflows/jules-daily-auto-upgrade.yml:38">
P2: When `JULES_API_KEY` is missing or Jules rejects the request, `curl -sS` still exits successfully because this call has no `--fail-with-body`, so the manual workflow reports success even though no PR was created. Make HTTP errors fail the step and validate the API key before sending the request.</violation>
</file>
<file name="frontend/src/StreetLightDetector.jsx">
<violation number="1" location="frontend/src/StreetLightDetector.jsx:39">
P2: If VITE_API_URL is configured with a trailing slash, `${API_URL}/api/detect-street-light` builds `//api/detect-street-light`, which FastAPI/uvicorn serves as a distinct 404 route. Normalize the base URL by stripping trailing slashes so the concatenation is safe regardless of how the env var is set.</violation>
</file>
<file name="backend/requirements.txt">
<violation number="1" location="backend/requirements.txt:1">
P3: This lockfile resolves only Python 3.12 (header: `--python-version 3.12 --python-platform linux`), and its pinned packages (e.g. numpy==2.5.2, scipy==1.18.0) require newer interpreters. README.md badge and backend/README.md still tell users `pip install -r backend/requirements.txt` works on "Python 3.8+". A dev on 3.8-3.11 following the README will hit resolution/wheel failures. Since all deployment targets (Docker python:3.12-slim, render.yaml PYTHON_VERSION 3.12.0, CI matrix 3.12) are 3.12, update the README/badge to require Python 3.12 so the documented local path matches the lock.</violation>
</file>
<file name="tests/test_local_ml_service.py">
<violation number="1" location="tests/test_local_ml_service.py:291">
P2: For the vandalism route this guard tests the wrong function. /api/detect-vandalism dispatches detect_vandalism_unified (from backend.unified_detection_service), but the test asserts callable(detect_vandalism) (backend.vandalism_detection), which no route uses. A regression in the vandalism route's real wiring would therefore slip past the very check this test claims to add. Import detect_vandalism_unified instead of (or in addition to) detect_vandalism and assert that one, and drop the stale claim in the docstring.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -1,6 +1,5 @@ | |||
| import React from 'react'; | |||
| import { useNavigate } from 'react-router-dom'; | |||
| import { motion } from 'framer-motion'; | |||
Contributor
There was a problem hiding this comment.
P0: Removing the framer-motion import leaves the motion identifier undefined, yet the JSX still uses motion.div, motion.h1, etc. in 40+ places. Since framer-motion is not otherwise imported or in package.json, rendering Landing throws ReferenceError: motion is not defined and the page goes blank. Restore the import (and add framer-motion as a dependency) or replace all motion.* elements with plain HTML elements.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/views/Landing.jsx, line 3:
<comment>Removing the `framer-motion` import leaves the `motion` identifier undefined, yet the JSX still uses `motion.div`, `motion.h1`, etc. in 40+ places. Since framer-motion is not otherwise imported or in package.json, rendering Landing throws `ReferenceError: motion is not defined` and the page goes blank. Restore the import (and add framer-motion as a dependency) or replace all `motion.*` elements with plain HTML elements.</comment>
<file context>
@@ -1,6 +1,5 @@
import React from 'react';
import { useNavigate } from 'react-router-dom';
-import { motion } from 'framer-motion';
import {
Building2, MessageCircle, Users, Shield, Star, FileText,
Search, Lock, ShoppingCart, User, ArrowRight
</file context>
| - name: Build debug APK | ||
| if: github.event_name == 'pull_request' | ||
| working-directory: frontend/android | ||
| run: ./gradlew --no-daemon assembleDebug |
Contributor
There was a problem hiding this comment.
P1: The Android job cannot run either build because frontend/android/gradlew is not executable in Git. Mark the wrapper executable in the repository (preferred), or invoke bash ./gradlew in both build steps.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/android.yml, line 96:
<comment>The Android job cannot run either build because `frontend/android/gradlew` is not executable in Git. Mark the wrapper executable in the repository (preferred), or invoke `bash ./gradlew` in both build steps.</comment>
<file context>
@@ -0,0 +1,137 @@
+ - name: Build debug APK
+ if: github.event_name == 'pull_request'
+ working-directory: frontend/android
+ run: ./gradlew --no-daemon assembleDebug
+
+ - name: Build release bundle
</file context>
| <!-- Base application theme. --> | ||
| <style name="AppTheme" parent="Theme.AppCompat.Light.DarkActionBar"> | ||
| <!-- Customize your theme here. --> | ||
| <item name="colorPrimary">@color/colorPrimary</item> |
Contributor
There was a problem hiding this comment.
P1: AppTheme references @color/colorPrimary, @color/colorPrimaryDark, and @color/colorAccent, but no colors.xml (or any definition of these colors) exists anywhere in the res tree. AAPT2 fails resource linking on undefined colors, breaking the Android build that this PR claims to have verified. Add a colors.xml under res/values defining the three colors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/android/app/src/main/res/values/styles.xml, line 7:
<comment>AppTheme references @color/colorPrimary, @color/colorPrimaryDark, and @color/colorAccent, but no colors.xml (or any definition of these colors) exists anywhere in the res tree. AAPT2 fails resource linking on undefined colors, breaking the Android build that this PR claims to have verified. Add a colors.xml under res/values defining the three colors.</comment>
<file context>
@@ -0,0 +1,22 @@
+ <!-- Base application theme. -->
+ <style name="AppTheme" parent="Theme.AppCompat.Light.DarkActionBar">
+ <!-- Customize your theme here. -->
+ <item name="colorPrimary">@color/colorPrimary</item>
+ <item name="colorPrimaryDark">@color/colorPrimaryDark</item>
+ <item name="colorAccent">@color/colorAccent</item>
</file context>
| "leaderboard": [ | ||
| { | ||
| "rank": index, | ||
| "user_email": row.user_email, |
Contributor
There was a problem hiding this comment.
P1: Every public leaderboard response exposes reporters' raw email addresses. Mask or omit user_email before returning leaderboard entries, matching the existing issue-list privacy behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/main.py, line 1146:
<comment>Every public leaderboard response exposes reporters' raw email addresses. Mask or omit `user_email` before returning leaderboard entries, matching the existing issue-list privacy behavior.</comment>
<file context>
@@ -505,3 +881,322 @@ def upvote_issue(issue_id: int, db: Session = Depends(get_db)):
+ "leaderboard": [
+ {
+ "rank": index,
+ "user_email": row.user_email,
+ "reports_count": int(row.reports_count or 0),
+ "upvotes": int(row.upvotes or 0),
</file context>
Suggested change
| "user_email": row.user_email, | |
| "user_email": ( | |
| f"{row.user_email[:1]}***{row.user_email[row.user_email.index('@') :]}" | |
| if "@" in row.user_email | |
| else "***" | |
| ), |
| python-magic | ||
| pywebpush | ||
| Pillow | ||
| firebase-functions |
Contributor
There was a problem hiding this comment.
P3: backend/requirements.in declares firebase-functions, firebase-admin, a2wsgi, and pywebpush, but none of them is imported anywhere in backend/*.py. These belong to the separate functions/ Firebase project (functions/main.py imports a2wsgi, and functions/requirements.txt already pins this set). Keeping them in the backend requirements adds a heavy unused transitive chain (flask, functions-framework, cloudevents, cryptography) to the backend runtime image and makes the two dependency manifests drift independently. Remove these lines from backend/requirements.in and re-run uv pip compile.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/requirements.in, line 15:
<comment>backend/requirements.in declares firebase-functions, firebase-admin, a2wsgi, and pywebpush, but none of them is imported anywhere in backend/*.py. These belong to the separate `functions/` Firebase project (functions/main.py imports a2wsgi, and functions/requirements.txt already pins this set). Keeping them in the backend requirements adds a heavy unused transitive chain (flask, functions-framework, cloudevents, cryptography) to the backend runtime image and makes the two dependency manifests drift independently. Remove these lines from backend/requirements.in and re-run uv pip compile.</comment>
<file context>
@@ -0,0 +1,21 @@
+python-magic
+pywebpush
+Pillow
+firebase-functions
+firebase-admin
+a2wsgi
</file context>
| os.environ["GEMINI_API_KEY"] = "" # Test without Gemini | ||
|
|
||
| from main import app | ||
| from backend.main import app |
Contributor
There was a problem hiding this comment.
P3: The retained __main__ runner is now broken: running python tests/test_mh_endpoint.py directly adds only tests/ to sys.path, so from backend.main import app raises ModuleNotFoundError. The repo-root conftest.py only makes backend.* importable under pytest. Add a repo-root sys.path insert (guarded by __package__) before the import so the direct-run path still works.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_mh_endpoint.py, line 16:
<comment>The retained `__main__` runner is now broken: running `python tests/test_mh_endpoint.py` directly adds only `tests/` to sys.path, so `from backend.main import app` raises ModuleNotFoundError. The repo-root conftest.py only makes `backend.*` importable under pytest. Add a repo-root sys.path insert (guarded by `__package__`) before the import so the direct-run path still works.</comment>
<file context>
@@ -1,100 +1,101 @@
+os.environ["GEMINI_API_KEY"] = "" # Test without Gemini
-from main import app
+from backend.main import app
client = TestClient(app, raise_server_exceptions=False)
</file context>
Suggested change
| from backend.main import app | |
| # Allow running this file directly (``python tests/test_mh_endpoint.py``). | |
| if __package__ is None: | |
| sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) | |
| from backend.main import app |
| # initialised by an earlier test returns immediately and load_model is never | ||
| # called -- the load count comes back 0 and the test fails only when run | ||
| # after its neighbours. | ||
| garbage_detection.reset_model() |
Contributor
There was a problem hiding this comment.
P3: Both tests leave the detection module polluted after they finish: the reset runs only at the start, so _model stays bound to "mock_model" (and pothole leaves _model_initialized=True) in the shared module namespace. Any later test in the same pytest session that calls get_model() would silently get the stale mock instead of triggering a real load. The sibling test_pothole_detection_thread_safety.py uses a fixture that resets after every test; mirror that here by resetting the module after the threads join (or wrapping the body in try/finally) so no test leaks state into the next.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_model_thread_safety.py, line 20:
<comment>Both tests leave the detection module polluted after they finish: the reset runs only at the start, so _model stays bound to "mock_model" (and pothole leaves _model_initialized=True) in the shared module namespace. Any later test in the same pytest session that calls get_model() would silently get the stale mock instead of triggering a real load. The sibling test_pothole_detection_thread_safety.py uses a fixture that resets after every test; mirror that here by resetting the module after the threads join (or wrapping the body in try/finally) so no test leaks state into the next.</comment>
<file context>
@@ -2,144 +2,156 @@
+ # initialised by an earlier test returns immediately and load_model is never
+ # called -- the load count comes back 0 and the test fails only when run
+ # after its neighbours.
+ garbage_detection.reset_model()
+
# Track how many times the model was loaded and ensure sequential execution
</file context>
| @@ -0,0 +1,170 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
Contributor
There was a problem hiding this comment.
P3: This new ic_launcher_background.xml vector drawable is never referenced. The adaptive launcher icons (mipmap-anydpi-v26/ic_launcher.xml, ic_launcher_round.xml) reference @color/ic_launcher_background (white #FFFFFF) instead of @drawable/ic_launcher_background, and nothing else uses this drawable. It ships unused in every APK and its teal-grid design is also not what the app actually shows. Either wire it into the adaptive-icon background or remove the file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/android/app/src/main/res/drawable/ic_launcher_background.xml, line 1:
<comment>This new ic_launcher_background.xml vector drawable is never referenced. The adaptive launcher icons (mipmap-anydpi-v26/ic_launcher.xml, ic_launcher_round.xml) reference @color/ic_launcher_background (white #FFFFFF) instead of @drawable/ic_launcher_background, and nothing else uses this drawable. It ships unused in every APK and its teal-grid design is also not what the app actually shows. Either wire it into the adaptive-icon background or remove the file.</comment>
<file context>
@@ -0,0 +1,170 @@
+<?xml version="1.0" encoding="utf-8"?>
+<vector xmlns:android="http://schemas.android.com/apk/res/android"
+ android:width="108dp"
</file context>
| # Spatial deduplication dependencies | ||
| scikit-learn | ||
| numpy | ||
| # Locked with: uv pip compile backend/requirements.in --python-version 3.12 --python-platform linux -o backend/requirements.txt |
Contributor
There was a problem hiding this comment.
P3: This lockfile resolves only Python 3.12 (header: --python-version 3.12 --python-platform linux), and its pinned packages (e.g. numpy==2.5.2, scipy==1.18.0) require newer interpreters. README.md badge and backend/README.md still tell users pip install -r backend/requirements.txt works on "Python 3.8+". A dev on 3.8-3.11 following the README will hit resolution/wheel failures. Since all deployment targets (Docker python:3.12-slim, render.yaml PYTHON_VERSION 3.12.0, CI matrix 3.12) are 3.12, update the README/badge to require Python 3.12 so the documented local path matches the lock.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/requirements.txt, line 1:
<comment>This lockfile resolves only Python 3.12 (header: `--python-version 3.12 --python-platform linux`), and its pinned packages (e.g. numpy==2.5.2, scipy==1.18.0) require newer interpreters. README.md badge and backend/README.md still tell users `pip install -r backend/requirements.txt` works on "Python 3.8+". A dev on 3.8-3.11 following the README will hit resolution/wheel failures. Since all deployment targets (Docker python:3.12-slim, render.yaml PYTHON_VERSION 3.12.0, CI matrix 3.12) are 3.12, update the README/badge to require Python 3.12 so the documented local path matches the lock.</comment>
<file context>
@@ -1,20 +1,332 @@
-# Spatial deduplication dependencies
-scikit-learn
-numpy
+# Locked with: uv pip compile backend/requirements.in --python-version 3.12 --python-platform linux -o backend/requirements.txt
+# Do not edit by hand. Edit backend/requirements.in and re-run the command above.
+a2wsgi==1.10.10
</file context>
Contributor
There was a problem hiding this comment.
2 issues found across 7 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/auth.py">
<violation number="1" location="backend/auth.py:90">
P1: The existing browser verification and escalation flows now fail because neither sends `X-API-Key`, and the administrative key cannot safely be shipped to public JavaScript. Route these actions through an authenticated server-side path or remove them from the public UI.</violation>
<violation number="2" location="backend/auth.py:90">
P2: hmac.compare_digest raises TypeError when either string contains non-ASCII characters, so a request whose X-API-Key header includes a non-ASCII byte (e.g. "é") makes require_api_key raise an uncaught TypeError and the endpoint responds 500 instead of 401. Encode both sides to bytes before comparing so the comparison is both timing-safe and robust to non-ASCII input.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| detail="This endpoint is not available: the configured administrative key is too weak.", | ||
| ) | ||
|
|
||
| if not x_api_key or not hmac.compare_digest(x_api_key, expected): |
Contributor
There was a problem hiding this comment.
P1: The existing browser verification and escalation flows now fail because neither sends X-API-Key, and the administrative key cannot safely be shipped to public JavaScript. Route these actions through an authenticated server-side path or remove them from the public UI.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/auth.py, line 90:
<comment>The existing browser verification and escalation flows now fail because neither sends `X-API-Key`, and the administrative key cannot safely be shipped to public JavaScript. Route these actions through an authenticated server-side path or remove them from the public UI.</comment>
<file context>
@@ -0,0 +1,144 @@
+ detail="This endpoint is not available: the configured administrative key is too weak.",
+ )
+
+ if not x_api_key or not hmac.compare_digest(x_api_key, expected):
+ raise HTTPException(
+ status_code=status.HTTP_401_UNAUTHORIZED,
</file context>
| detail="This endpoint is not available: the configured administrative key is too weak.", | ||
| ) | ||
|
|
||
| if not x_api_key or not hmac.compare_digest(x_api_key, expected): |
Contributor
There was a problem hiding this comment.
P2: hmac.compare_digest raises TypeError when either string contains non-ASCII characters, so a request whose X-API-Key header includes a non-ASCII byte (e.g. "é") makes require_api_key raise an uncaught TypeError and the endpoint responds 500 instead of 401. Encode both sides to bytes before comparing so the comparison is both timing-safe and robust to non-ASCII input.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/auth.py, line 90:
<comment>hmac.compare_digest raises TypeError when either string contains non-ASCII characters, so a request whose X-API-Key header includes a non-ASCII byte (e.g. "é") makes require_api_key raise an uncaught TypeError and the endpoint responds 500 instead of 401. Encode both sides to bytes before comparing so the comparison is both timing-safe and robust to non-ASCII input.</comment>
<file context>
@@ -0,0 +1,144 @@
+ detail="This endpoint is not available: the configured administrative key is too weak.",
+ )
+
+ if not x_api_key or not hmac.compare_digest(x_api_key, expected):
+ raise HTTPException(
+ status_code=status.HTTP_401_UNAUTHORIZED,
</file context>
Suggested change
| if not x_api_key or not hmac.compare_digest(x_api_key, expected): | |
| if not x_api_key or not hmac.compare_digest(x_api_key.encode(), expected.encode()): |
The schema was maintained by raw ALTER and CREATE INDEX statements executed on every boot -- four in the FastAPI lifespan, twenty-two more in backend/init_db.py -- each wrapped so its failure was ignored. That has no ordering, no down path, and no record of which revision a database is on. A statement that failed for a real reason was indistinguishable from one that was simply already applied. With more than one worker, every process raced to apply the same DDL. Alembic now owns the schema. backend/migrations/ holds a baseline revision generated from the models, verified to apply to an empty database, reverse cleanly, and re-apply. env.py takes the database URL from backend.database rather than alembic.ini, so migrations always target the database the service uses and there is no second place to keep in sync. render_as_batch is on because SQLite -- the local fallback -- cannot ALTER a column in place; without it any future migration that alters or drops a column would pass against PostgreSQL and fail locally. compare_type is on so column type changes are detected, which alembic ignores by default. script.py.mako imports backend.models. Autogenerate renders custom column types by their fully qualified name, so a migration touching Issue.action_plan (a JSONEncodedDict) raises NameError at upgrade time without it. The generated baseline hit exactly that before the template was fixed. Migrations run once per deploy, not at startup: render.yaml gains preDeployCommand, and the Dockerfile documents the equivalent one-shot command and ships alembic.ini. A container that cannot migrate should fail the deploy rather than boot against a half-changed schema. The lifespan block and backend/init_db.py are removed. Nothing imported init_db. tests/test_migrations.py asserts every model table and column exists after upgrade, that downgrade leaves nothing behind, and -- the guard that matters -- that `alembic check` reports no pending changes. Edit a model and forget the migration, and the suite fails instead of production. Suite: 225 passed, 4 skipped, 0 failed.
tests/test_migrations.py shells out to alembic to exercise upgrade, downgrade and drift detection the way a deploy actually runs it. S603/S607 flag that as untrusted input, but the argv is built from module constants and a tmp_path fixture, never from request data, and running alembic in-process would not test the command the deploy issues. Scoped to test paths in pyproject.toml rather than suppressed inline, matching how S101 and the other test-only rules are already handled. Backend code keeps both rules.
RohanExploit
force-pushed
the
enterprise-hardening
branch
from
August 19, 2026 13:49
ae4ba8c to
78e56ef
Compare
Twenty detector components exist. Eleven were routed. The other nine were written, styled and wired to an API client, but had no lazy import, no route and no entry in the home grid, so no user could reach any of them. Their backend endpoints did not exist either until earlier in this branch, so the gap was invisible from both ends. Eight are now routed and linked: accessibility, civic-eye, crowd, noise, pest, severity, waste and water-leak. Their icons were already imported in Home.jsx -- Bug, Volume2, Users, Waves, Recycle, Eye -- which suggests the cards were meant to be added and never were. The four that are not environmental get a new "Community & Access" group rather than being pushed into a category they do not belong to. Every one of these paths is now covered by tests/test_api_contract.py, which walks the frontend for `/api/...` literals: previously they were absent from the scan only because no routed component referenced them, so the contract test was passing for the wrong reason. SmartScanner stays unrouted on purpose. It imports @tensorflow/tfjs and @tensorflow-models/mobilenet, neither of which is a dependency of this package, so routing it as-is breaks the build. Adding them would put tens of megabytes of model and runtime into a bundle whose users are on low-end Android phones and metered connections, to duplicate work /api/detect-smart-scan already does server-side. The reasoning is recorded next to the import block so the next person does not have to rediscover it. Verified: build green, lint 0 errors, 114 frontend tests, 225 backend tests, and the rebuilt APK installs and launches on the S20 FE.
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/App.jsx">
<violation number="1" location="frontend/src/App.jsx:360">
P3: The new Civic Eye and Waste screens discard the `onBack` callback, so neither offers a direct in-screen way to return home. Render a Back button and invoke the supplied callback like the other detector screens.</violation>
<violation number="2" location="frontend/src/App.jsx:360">
P2: When users leave the newly reachable Civic Eye or Waste screen, its camera stream remains active. Both cleanup functions close over the initial `stream === null`; stop tracks through `videoRef.current.srcObject` or a stream ref during unmount.</violation>
<violation number="3" location="frontend/src/App.jsx:362">
P2: When Android users open the newly routed Noise detector, microphone capture fails because the app does not declare `RECORD_AUDIO`. Add the microphone permission and its runtime/WebView permission handling before exposing this route.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| <Route path="/accessibility" element={<AccessibilityDetector onBack={() => navigate('/')} />} /> | ||
| <Route path="/civic-eye" element={<CivicEyeDetector onBack={() => navigate('/')} />} /> | ||
| <Route path="/crowd" element={<CrowdDetector onBack={() => navigate('/')} />} /> | ||
| <Route path="/noise" element={<NoiseDetector onBack={() => navigate('/')} />} /> |
Contributor
There was a problem hiding this comment.
P2: When Android users open the newly routed Noise detector, microphone capture fails because the app does not declare RECORD_AUDIO. Add the microphone permission and its runtime/WebView permission handling before exposing this route.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/App.jsx, line 362:
<comment>When Android users open the newly routed Noise detector, microphone capture fails because the app does not declare `RECORD_AUDIO`. Add the microphone permission and its runtime/WebView permission handling before exposing this route.</comment>
<file context>
@@ -337,6 +356,14 @@ element={<ActionView actionPlan={actionPlan} setActionPlan={setActionPlan} setVi
+ <Route path="/accessibility" element={<AccessibilityDetector onBack={() => navigate('/')} />} />
+ <Route path="/civic-eye" element={<CivicEyeDetector onBack={() => navigate('/')} />} />
+ <Route path="/crowd" element={<CrowdDetector onBack={() => navigate('/')} />} />
+ <Route path="/noise" element={<NoiseDetector onBack={() => navigate('/')} />} />
+ <Route path="/pest" element={<PestDetector onBack={() => navigate('/')} />} />
+ <Route path="/severity" element={<SeverityDetector onBack={() => navigate('/')} />} />
</file context>
| <Route path="/blocked" element={<BlockedRoadDetector onBack={() => navigate('/')} />} /> | ||
| <Route path="/tree" element={<TreeDetector onBack={() => navigate('/')} />} /> | ||
| <Route path="/accessibility" element={<AccessibilityDetector onBack={() => navigate('/')} />} /> | ||
| <Route path="/civic-eye" element={<CivicEyeDetector onBack={() => navigate('/')} />} /> |
Contributor
There was a problem hiding this comment.
P2: When users leave the newly reachable Civic Eye or Waste screen, its camera stream remains active. Both cleanup functions close over the initial stream === null; stop tracks through videoRef.current.srcObject or a stream ref during unmount.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/App.jsx, line 360:
<comment>When users leave the newly reachable Civic Eye or Waste screen, its camera stream remains active. Both cleanup functions close over the initial `stream === null`; stop tracks through `videoRef.current.srcObject` or a stream ref during unmount.</comment>
<file context>
@@ -337,6 +356,14 @@ element={<ActionView actionPlan={actionPlan} setActionPlan={setActionPlan} setVi
<Route path="/blocked" element={<BlockedRoadDetector onBack={() => navigate('/')} />} />
<Route path="/tree" element={<TreeDetector onBack={() => navigate('/')} />} />
+ <Route path="/accessibility" element={<AccessibilityDetector onBack={() => navigate('/')} />} />
+ <Route path="/civic-eye" element={<CivicEyeDetector onBack={() => navigate('/')} />} />
+ <Route path="/crowd" element={<CrowdDetector onBack={() => navigate('/')} />} />
+ <Route path="/noise" element={<NoiseDetector onBack={() => navigate('/')} />} />
</file context>
| <Route path="/blocked" element={<BlockedRoadDetector onBack={() => navigate('/')} />} /> | ||
| <Route path="/tree" element={<TreeDetector onBack={() => navigate('/')} />} /> | ||
| <Route path="/accessibility" element={<AccessibilityDetector onBack={() => navigate('/')} />} /> | ||
| <Route path="/civic-eye" element={<CivicEyeDetector onBack={() => navigate('/')} />} /> |
Contributor
There was a problem hiding this comment.
P3: The new Civic Eye and Waste screens discard the onBack callback, so neither offers a direct in-screen way to return home. Render a Back button and invoke the supplied callback like the other detector screens.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/App.jsx, line 360:
<comment>The new Civic Eye and Waste screens discard the `onBack` callback, so neither offers a direct in-screen way to return home. Render a Back button and invoke the supplied callback like the other detector screens.</comment>
<file context>
@@ -337,6 +356,14 @@ element={<ActionView actionPlan={actionPlan} setActionPlan={setActionPlan} setVi
<Route path="/blocked" element={<BlockedRoadDetector onBack={() => navigate('/')} />} />
<Route path="/tree" element={<TreeDetector onBack={() => navigate('/')} />} />
+ <Route path="/accessibility" element={<AccessibilityDetector onBack={() => navigate('/')} />} />
+ <Route path="/civic-eye" element={<CivicEyeDetector onBack={() => navigate('/')} />} />
+ <Route path="/crowd" element={<CrowdDetector onBack={() => navigate('/')} />} />
+ <Route path="/noise" element={<NoiseDetector onBack={() => navigate('/')} />} />
</file context>
…handling
Found by running the app on a device and watching the server log rather than
by reading code.
POST /api/chat answered 422 to every message. ChatWidget.jsx posts
{"query": ...}; the model required "message". The widget swallowed the failure
into a console.error, so the assistant simply never replied and nothing
surfaced. Both names are accepted now, "message" canonical. This is the third
instance of the same class after /api/analyze-urgency and the detector upload
fields.
tests/test_json_payload_contract.py closes that gap. test_api_contract.py
proves a path exists, accepts the right method, and accepts the right upload
field -- it cannot see JSON bodies, which is why two live 422s hid behind a
green suite. Each case is the exact body the named component sends.
Writing those tests exposed a second defect: both handlers wrapped the payload
access in a try/except that caught everything, so a deliberate 422 for an empty
message came back as 500 or 502. A validation error reported as an upstream
outage sends whoever is debugging it to the wrong system. The payload is now
resolved before the try. /api/chat also stopped returning the raw exception
string to the caller, which leaked internals on any failure.
The Smart Scanner call to action -- the most prominent button on the home
screen, "AI-powered issue detection" -- called setView('pothole'). It opened
the pothole detector, and the actual Smart Scanner screen was unreachable from
anywhere in the app.
SmartScanner is now routed and the CTA points at it. Reaching that required
dropping @tensorflow/tfjs and @tensorflow-models/mobilenet, which were imported
but are not dependencies of this package, so the component could not have been
routed as it stood. MobileNet was used only as a client-side gate deciding
whether a frame was worth uploading. The file already does that with a
frame-difference check and a two-second cooldown, and the backend classifies
properly at /api/detect-smart-scan, so the model was shipping a runtime and
weights to a device on a metered connection to duplicate a decision made
server-side.
Verified on a Galaxy S20 FE this session: the R8-minified release APK installs
and runs (3.0 MB against 10.4 MB debug), all eight newly routed detectors
render, and POST /api/detect-waste returned 200 from a live camera frame --
an endpoint that did not exist and a screen that could not be reached when this
branch started.
Not verified on device: the Smart Scanner screen itself, and the report
submission flow. The phone was disconnected before either could be exercised.
Contributor
There was a problem hiding this comment.
3 issues found across 5 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/SmartScanner.jsx">
<violation number="1" location="frontend/src/SmartScanner.jsx:110">
P1: When scanning remains active, `previousFrame` stays `null` inside the interval closure, so this unconditional branch uploads every cooldown tick even for a stationary camera. Store the previous frame in a ref or recreate the interval when it changes before removing the MobileNet gate; otherwise the backend rate limit stops updates with 429 responses.</violation>
</file>
<file name="backend/main.py">
<violation number="1" location="backend/main.py:328">
P3: The generated `/api/chat` OpenAPI schema now marks both `message` and `query` optional, but the endpoint rejects requests containing neither field. Express the one-of requirement in the request model/schema so `/docs` and generated clients match runtime validation.</violation>
</file>
<file name="tests/test_json_payload_contract.py">
<violation number="1" location="tests/test_json_payload_contract.py:76">
P2: This file tests the rate-limited /api/analyze-urgency endpoint (AI_RATE_LIMIT, default 12/min) but never resets the in-process slowapi counters, unlike test_rate_limiting.py which documents that counters leak between tests. The blank/empty tests assert exact 422; if preceding requests in the same pytest process exhaust the AI limit (7 hits here plus any from test_api_contract.py), those endpoints return 429 and the ==422 assertions fail intermittently. Add an autouse fixture calling main_module.limiter.reset() before/after each test, as test_rate_limiting does.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // runtime to a device on a metered connection to decide whether to make | ||
| // a request the backend classifies properly anyway. | ||
| { | ||
| lastSentRef.current = now; |
Contributor
There was a problem hiding this comment.
P1: When scanning remains active, previousFrame stays null inside the interval closure, so this unconditional branch uploads every cooldown tick even for a stationary camera. Store the previous frame in a ref or recreate the interval when it changes before removing the MobileNet gate; otherwise the backend rate limit stops updates with 429 responses.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/SmartScanner.jsx, line 110:
<comment>When scanning remains active, `previousFrame` stays `null` inside the interval closure, so this unconditional branch uploads every cooldown tick even for a stationary camera. Store the previous frame in a ref or recreate the interval when it changes before removing the MobileNet gate; otherwise the backend rate limit stops updates with 429 responses.</comment>
<file context>
@@ -117,14 +101,13 @@ const SmartScanner = ({ onBack }) => {
+ // runtime to a device on a metered connection to decide whether to make
+ // a request the backend classifies properly anyway.
+ {
+ lastSentRef.current = now;
canvas.toBlob(async (blob) => {
if (!blob) return;
</file context>
| assert response.status_code != 422, f"{path} stopped accepting its canonical field: {body}" | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("path", ["/api/analyze-urgency", "/api/chat"]) |
Contributor
There was a problem hiding this comment.
P2: This file tests the rate-limited /api/analyze-urgency endpoint (AI_RATE_LIMIT, default 12/min) but never resets the in-process slowapi counters, unlike test_rate_limiting.py which documents that counters leak between tests. The blank/empty tests assert exact 422; if preceding requests in the same pytest process exhaust the AI limit (7 hits here plus any from test_api_contract.py), those endpoints return 429 and the ==422 assertions fail intermittently. Add an autouse fixture calling main_module.limiter.reset() before/after each test, as test_rate_limiting does.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_json_payload_contract.py, line 76:
<comment>This file tests the rate-limited /api/analyze-urgency endpoint (AI_RATE_LIMIT, default 12/min) but never resets the in-process slowapi counters, unlike test_rate_limiting.py which documents that counters leak between tests. The blank/empty tests assert exact 422; if preceding requests in the same pytest process exhaust the AI limit (7 hits here plus any from test_api_contract.py), those endpoints return 429 and the ==422 assertions fail intermittently. Add an autouse fixture calling main_module.limiter.reset() before/after each test, as test_rate_limiting does.</comment>
<file context>
@@ -0,0 +1,89 @@
+ assert response.status_code != 422, f"{path} stopped accepting its canonical field: {body}"
+
+
+@pytest.mark.parametrize("path", ["/api/analyze-urgency", "/api/chat"])
+def test_empty_body_is_rejected(client, path):
+ """Accepting both names must not mean accepting neither."""
</file context>
| canonical. | ||
| """ | ||
|
|
||
| message: str | None = None |
Contributor
There was a problem hiding this comment.
P3: The generated /api/chat OpenAPI schema now marks both message and query optional, but the endpoint rejects requests containing neither field. Express the one-of requirement in the request model/schema so /docs and generated clients match runtime validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/main.py, line 328:
<comment>The generated `/api/chat` OpenAPI schema now marks both `message` and `query` optional, but the endpoint rejects requests containing neither field. Express the one-of requirement in the request model/schema so `/docs` and generated clients match runtime validation.</comment>
<file context>
@@ -317,9 +317,28 @@ class PincodeRequest(BaseModel):
+ canonical.
+ """
+
+ message: str | None = None
+ query: str | None = None
history: list[dict] = []
</file context>
Two layout defects observed while driving the app on a device, plus the first component tests in this repository. The home grid's last row sat underneath the fixed buttons in the bottom-right corner. Four separate elements are pinned there -- quick actions at bottom-28, chat at bottom-8, the chat widget itself at bottom-4, scroll-to-top at bottom-44 -- and the scrolling content had no bottom padding to clear any of them. "Report Issue" could not be tapped on a 1080x2400 screen; the tap landed on the button stack. Added pb-40 to the routed content. EnhancedChatWidget is `fixed bottom-8 right-8` and renders ChatWidget, which was itself `fixed bottom-4 right-4`. The inner element escaped its wrapper, so the button sat 16px from the corner while the hover tooltip and the green status dot stayed anchored to the wrapper at 32px -- decorations floating detached from the control they describe. ChatWidget no longer positions itself; the wrapper owns placement. views/__tests__/ReportForm.test.jsx is new. ReportForm is 635 lines and is the reason the application exists, and it had no tests, so the field names it posts were only ever checked by hand. Given that /api/chat, /api/analyze-urgency and four detector endpoints all shipped with mismatched field names, that is exactly the code that needed a contract test. Eight cases cover the submitted FormData keys, that latitude and longitude are omitted rather than sent empty when no location was captured, failure handling, and the offline path -- queueing to IndexedDB instead of posting, registering a background sync, and still routing the user onward. Writing them exposed why the views had no component tests at all: jsdom does not implement TextEncoder or TextDecoder, and react-router v7 needs them at import time, so any test rendering a routed component died with a ReferenceError before its first assertion. setupTests.js now polyfills both from node:util, which unblocks component testing generally. Frontend: 122 tests across 7 suites, 0 lint errors, build green. Not verified on device -- the phone was disconnected. Both layout changes are CSS-only and reasoned from the measured offsets, but neither has been seen rendered.
…database
The deployed backend has been returning 500 from every database-backed endpoint
while reporting itself healthy. Diagnosed against the live service:
GET /health 200 {"status":"healthy"}
GET /api/stats 500 psycopg2.OperationalError: could not translate
host name "dpg-...-a" to address
The Postgres instance that DATABASE_URL points at no longer exists. That is an
infrastructure fact, not a code defect -- but three code defects are why it went
unnoticed and why the service could not explain itself.
/health returned a hard-coded {"database": "connected"} without ever opening a
connection. The platform health gate passed, so nothing surfaced the outage. It
now runs SELECT 1 and reports what it finds.
/health and readiness were the same endpoint, which forces a bad trade: return
503 and the platform restarts a process whose database has been deleted, or
return 200 and nothing alerts. They are now separate. /health is liveness and
stays 200 while the process can serve, reporting status "degraded" and the real
component state. /health/ready is readiness and returns 503 when the database
is unreachable -- that is what alerting and load-balancer membership should
watch.
Base.metadata.create_all(bind=engine) ran at import time in backend/main.py and
again in backend/bot.py, which backend/main.py imports. An unreachable database
was therefore a hard import failure: the process could not start at all, so it
could not serve the endpoint that would have explained why. Confirmed by
pointing DATABASE_URL at a dead host -- the import raised OperationalError
before any route was registered. Both are removed; Alembic owns the schema, and
the app now boots and reports "degraded" instead of failing to start.
render.yaml: migrations moved from preDeployCommand into startCommand. Render
only honours preDeployCommand on paid instance types and silently ignores it on
the free tier, so the migrations this branch added were never running. A deploy
that cannot migrate now fails visibly instead of serving 500s against a schema
that was never created. The comment records that preDeployCommand is the right
home on a paid plan.
The health response deliberately reports only the exception type, not the
driver's message: this is a public endpoint and psycopg2 errors carry the host
name and connection string. There is a test for that.
tests/test_health.py covers all of it, including a guard that fails if
create_all(bind=...) reappears in an import path.
Suite: 241 passed, 4 skipped. ruff and bandit clean.
Contributor
There was a problem hiding this comment.
6 issues found across 4 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/bot.py">
<violation number="1" location="backend/bot.py:27">
P2: When the backend uses the documented local or direct Docker startup path without a preceding migration, this removal leaves a fresh database with no tables. Run `alembic upgrade head` in every supported startup path, or retain a development-only schema bootstrap.</violation>
</file>
<file name="render.yaml">
<violation number="1" location="render.yaml:26">
P1: When Render starts multiple replicas during a deploy or scale-out, every replica runs Alembic against the same database. Serialize migrations in a one-time deploy job or use a database lock; do not run them in each web replica's `startCommand`.</violation>
<violation number="2" location="render.yaml:26">
P1: On any database created before Alembic, this command runs the baseline revision against existing tables and aborts the Render deployment with duplicate-table errors. Add a one-time adoption/stamp migration for existing databases before enabling `upgrade head` at startup.</violation>
</file>
<file name="backend/main.py">
<violation number="1" location="backend/main.py:395">
P2: When the database is unreachable but does not fail immediately, `/health` can time out because liveness synchronously opens a database connection on every probe. Keep liveness dependency-free or configure a short bounded connection timeout and use `/health/ready` for dependency checks.</violation>
<violation number="2" location="backend/main.py:418">
P2: When AI initialization fails but the database is reachable, `/health/ready` returns 200 with `degraded`, so load balancers keep sending traffic to AI-backed routes that cannot serve. Return 503 when either dependency is unavailable.</violation>
</file>
<file name="tests/test_health.py">
<violation number="1" location="tests/test_health.py:43">
P2: These two database-focused tests assert the response status is "healthy"/200, but the endpoint only returns "healthy" when the AI service is also initialized (status = "healthy" if db_ok and ai_ok). The lifespan wraps AI initialization in try/except and get_service_type() defaults to "gemini", so a missing/invalid GEMINI_API_KEY makes _ai_status() return "not initialized" and turns the status "degraded" even when the database is perfectly reachable, failing the tests in CI. Assert on body["services"]["database"] == "connected" instead of the aggregate "status" so the database-health assertions stay independent of the AI service.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| # silently ignores it on the free tier, so the migrations never ran at all. | ||
| # On a paid plan, move this to preDeployCommand -- it belongs there, and | ||
| # running it in startCommand means every replica races to apply it. | ||
| startCommand: "alembic upgrade head && python start-backend.py" |
Contributor
There was a problem hiding this comment.
P1: When Render starts multiple replicas during a deploy or scale-out, every replica runs Alembic against the same database. Serialize migrations in a one-time deploy job or use a database lock; do not run them in each web replica's startCommand.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At render.yaml, line 26:
<comment>When Render starts multiple replicas during a deploy or scale-out, every replica runs Alembic against the same database. Serialize migrations in a one-time deploy job or use a database lock; do not run them in each web replica's `startCommand`.</comment>
<file context>
@@ -15,11 +15,15 @@ services:
+ # silently ignores it on the free tier, so the migrations never ran at all.
+ # On a paid plan, move this to preDeployCommand -- it belongs there, and
+ # running it in startCommand means every replica races to apply it.
+ startCommand: "alembic upgrade head && python start-backend.py"
envVars:
- key: PYTHON_VERSION
</file context>
| # silently ignores it on the free tier, so the migrations never ran at all. | ||
| # On a paid plan, move this to preDeployCommand -- it belongs there, and | ||
| # running it in startCommand means every replica races to apply it. | ||
| startCommand: "alembic upgrade head && python start-backend.py" |
Contributor
There was a problem hiding this comment.
P1: On any database created before Alembic, this command runs the baseline revision against existing tables and aborts the Render deployment with duplicate-table errors. Add a one-time adoption/stamp migration for existing databases before enabling upgrade head at startup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At render.yaml, line 26:
<comment>On any database created before Alembic, this command runs the baseline revision against existing tables and aborts the Render deployment with duplicate-table errors. Add a one-time adoption/stamp migration for existing databases before enabling `upgrade head` at startup.</comment>
<file context>
@@ -15,11 +15,15 @@ services:
+ # silently ignores it on the free tier, so the migrations never ran at all.
+ # On a paid plan, move this to preDeployCommand -- it belongs there, and
+ # running it in startCommand means every replica races to apply it.
+ startCommand: "alembic upgrade head && python start-backend.py"
envVars:
- key: PYTHON_VERSION
</file context>
|
|
||
| # Initialize Database | ||
| Base.metadata.create_all(bind=engine) | ||
| # Schema creation is Alembic's job, not an import side effect. |
Contributor
There was a problem hiding this comment.
P2: When the backend uses the documented local or direct Docker startup path without a preceding migration, this removal leaves a fresh database with no tables. Run alembic upgrade head in every supported startup path, or retain a development-only schema bootstrap.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/bot.py, line 27:
<comment>When the backend uses the documented local or direct Docker startup path without a preceding migration, this removal leaves a fresh database with no tables. Run `alembic upgrade head` in every supported startup path, or retain a development-only schema bootstrap.</comment>
<file context>
@@ -24,8 +24,12 @@
-# Initialize Database
-Base.metadata.create_all(bind=engine)
+# Schema creation is Alembic's job, not an import side effect.
+#
+# This ran at import time, and backend.main imports this module, so an
</file context>
| kept answering "healthy" -- so the platform health gate passed and nothing | ||
| surfaced the outage. It now reports what it actually finds. | ||
| """ | ||
| db_ok, db_detail = _database_status() |
Contributor
There was a problem hiding this comment.
P2: When the database is unreachable but does not fail immediately, /health can time out because liveness synchronously opens a database connection on every probe. Keep liveness dependency-free or configure a short bounded connection timeout and use /health/ready for dependency checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/main.py, line 395:
<comment>When the database is unreachable but does not fail immediately, `/health` can time out because liveness synchronously opens a database connection on every probe. Keep liveness dependency-free or configure a short bounded connection timeout and use `/health/ready` for dependency checks.</comment>
<file context>
@@ -347,14 +358,71 @@ def root():
+ kept answering "healthy" -- so the platform health gate passed and nothing
+ surfaced the outage. It now reports what it actually finds.
+ """
+ db_ok, db_detail = _database_status()
+ ai_ok, ai_detail = _ai_status()
+
</file context>
| db_ok, db_detail = _database_status() | ||
| ai_ok, ai_detail = _ai_status() | ||
|
|
||
| if not db_ok: |
Contributor
There was a problem hiding this comment.
P2: When AI initialization fails but the database is reachable, /health/ready returns 200 with degraded, so load balancers keep sending traffic to AI-backed routes that cannot serve. Return 503 when either dependency is unavailable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/main.py, line 418:
<comment>When AI initialization fails but the database is reachable, `/health/ready` returns 200 with `degraded`, so load balancers keep sending traffic to AI-backed routes that cannot serve. Return 503 when either dependency is unavailable.</comment>
<file context>
@@ -347,14 +358,71 @@ def root():
+ db_ok, db_detail = _database_status()
+ ai_ok, ai_detail = _ai_status()
+
+ if not db_ok:
+ response.status_code = status.HTTP_503_SERVICE_UNAVAILABLE
+
</file context>
Suggested change
| if not db_ok: | |
| if not db_ok or not ai_ok: |
| def test_health_reports_a_reachable_database(client): | ||
| body = client.get("/health").json() | ||
| assert body["services"]["database"] == "connected" | ||
| assert body["status"] == "healthy" |
Contributor
There was a problem hiding this comment.
P2: These two database-focused tests assert the response status is "healthy"/200, but the endpoint only returns "healthy" when the AI service is also initialized (status = "healthy" if db_ok and ai_ok). The lifespan wraps AI initialization in try/except and get_service_type() defaults to "gemini", so a missing/invalid GEMINI_API_KEY makes _ai_status() return "not initialized" and turns the status "degraded" even when the database is perfectly reachable, failing the tests in CI. Assert on body["services"]["database"] == "connected" instead of the aggregate "status" so the database-health assertions stay independent of the AI service.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_health.py, line 43:
<comment>These two database-focused tests assert the response status is "healthy"/200, but the endpoint only returns "healthy" when the AI service is also initialized (status = "healthy" if db_ok and ai_ok). The lifespan wraps AI initialization in try/except and get_service_type() defaults to "gemini", so a missing/invalid GEMINI_API_KEY makes _ai_status() return "not initialized" and turns the status "degraded" even when the database is perfectly reachable, failing the tests in CI. Assert on body["services"]["database"] == "connected" instead of the aggregate "status" so the database-health assertions stay independent of the AI service.</comment>
<file context>
@@ -0,0 +1,120 @@
+def test_health_reports_a_reachable_database(client):
+ body = client.get("/health").json()
+ assert body["services"]["database"] == "connected"
+ assert body["status"] == "healthy"
+
+
</file context>
Measured against the live service: the first request after an idle period took
117 seconds, the next took 0.5. The deployment suspends when idle and takes
around two minutes to serve again.
Nothing in the client handled that. Every call was a bare fetch with no
timeout, no retry and no abort, so during that window requests hung
indefinitely, the UI showed a spinner with no explanation, and a detector
polling every two seconds stacked up dozens of pending requests against a
server that was still starting. That is the "everything is broken, extremely
poor performance" symptom, and it is not a code defect so much as an
unhandled deployment reality.
The client now bounds each attempt, retries transient failures with backoff
inside a budget long enough to cover a cold start, and exposes onServerWaking
so the UI can say "starting" rather than implying "broken". 4xx responses are
never retried -- the request itself is wrong, and on the AI endpoints retrying
costs real money. Callers that need a fresh answer or none, such as a live
detector frame, pass retry: false.
Writing the tests found a bug in that logic: the non-retriable throw happened
inside the try block and was caught by the function's own handler, so a 422 was
retried until the budget ran out. 28 requests for a payload the server had
already rejected. Non-retriable errors are now flagged and re-thrown.
Separately, api/__tests__/client.test.js was not testing the client. jest.config
mapped '^../client$' to src/__mocks__/client.js, so all eleven tests imported a
hand-written fixture and asserted its behaviour -- reading process.env, sending
a JSON Content-Type on GET -- neither of which the real client does. They could
not have failed if the client broke. The redirect is removed and the file
rewritten against the real module; it now also asserts that GET sends no
Content-Type, that postForm leaves it unset so fetch can add the multipart
boundary, and that every request carries an abort signal.
The four suites that genuinely want a stubbed client already call
jest.mock('../client', ...) with their own factory, so nothing depended on the
global redirect. src/__mocks__/client.js is deleted; import.meta.env is handled
by babel-plugin-transform-vite-meta-env.
Frontend: 132 tests across 8 suites, 0 lint errors, build green.
Contributor
There was a problem hiding this comment.
1 issue found across 5 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/api/client.js">
<violation number="1" location="frontend/src/api/client.js:96">
P2: Every successful response calls `notifyWaking(false)`, even when the first attempt succeeds without retry. This contradicts the test expectation 'stays quiet when the first attempt succeeds' and the comment 'Only announce after the first failure.' Track whether a retry occurred (e.g., add `let hasRetried = false;` before the loop, set it true at line 113, and conditionally call `notifyWaking(false)` only if `hasRetried`) so the UI only shows waking notifications when the server actually needed to wake.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| } | ||
| lastError = error; | ||
| } else { | ||
| notifyWaking(false); |
Contributor
There was a problem hiding this comment.
P2: Every successful response calls notifyWaking(false), even when the first attempt succeeds without retry. This contradicts the test expectation 'stays quiet when the first attempt succeeds' and the comment 'Only announce after the first failure.' Track whether a retry occurred (e.g., add let hasRetried = false; before the loop, set it true at line 113, and conditionally call notifyWaking(false) only if hasRetried) so the UI only shows waking notifications when the server actually needed to wake.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/api/client.js, line 96:
<comment>Every successful response calls `notifyWaking(false)`, even when the first attempt succeeds without retry. This contradicts the test expectation 'stays quiet when the first attempt succeeds' and the comment 'Only announce after the first failure.' Track whether a retry occurred (e.g., add `let hasRetried = false;` before the loop, set it true at line 113, and conditionally call `notifyWaking(false)` only if `hasRetried`) so the UI only shows waking notifications when the server actually needed to wake.</comment>
<file context>
@@ -1,37 +1,146 @@
+ }
+ lastError = error;
+ } else {
+ notifyWaking(false);
+ return response;
+ }
</file context>
Suggested change
| notifyWaking(false); | |
| if (attempt > 0) notifyWaking(false); |
The deployment's Postgres instance was deleted. Its hostname stopped resolving
and every database-backed endpoint returned 500:
psycopg2.OperationalError: could not translate host name
"dpg-d5h3qaali9vc73a7iqog-a" to address: Name or service not known
Repointing DATABASE_URL is the actual repair and needs access to the hosting
dashboard. This change makes the code stop being a second failure on top of the
first one: a civic reporting app that cannot accept a report is useless, and an
unreachable database is not a reason to refuse to run at all.
backend/database now probes the configured database at startup and falls back
to local SQLite when it cannot be reached. Verified against the exact dead
hostname from the live deployment:
/health 200 (was: process could not start)
/health/ready 503 correctly still failing
/api/issues/recent 200 was 500
/api/stats 200 was 500
The fallback is never silent. It logs at ERROR with the underlying reason,
/health reports "sqlite-fallback: configured database unreachable" and the
service reads "degraded", and /health/ready keeps returning 503 so alerting
still fires. A working service is not the same as a correctly configured one,
and the previous outage lasted as long as it did precisely because something
reported "healthy" without checking.
SQLITE_FALLBACK_ENABLED=false turns it off for deployments that would rather
fail hard than write somewhere unexpected. DB_CONNECT_TIMEOUT bounds the
startup probe, since a dead host otherwise hangs the boot until the OS gives
up; postgresql connections now carry connect_timeout and pool_pre_ping.
The migration chain follows automatically: alembic's env.py takes its URL from
backend.database, so `alembic upgrade head` migrates the fallback database.
Confirmed end to end -- with the dead host configured, alembic creates all six
tables in SQLite and the server starts against them. Without this the
startCommand's `alembic upgrade head &&` would abort and the service would
never boot at all.
This is a stopgap, and the docstring and .env.example both say so: on an
ephemeral filesystem the SQLite file does not survive a restart, so reports
collected while degraded can be lost.
Suite: 247 passed, 4 skipped. ruff clean.
…ract.py CI's ruff format --check failed on these two files; local formatting had drifted from what was committed. No behavior change.
Committed as mode 100644 (non-executable), likely lost on a Windows checkout. CI failed with 'Permission denied' running ./gradlew assembleDebug.
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.
Summary
maindid not run. This branch makes it run, then hardens it.The backend could not be imported, the frontend could not be built, and 20 endpoints the frontend calls did not exist. Two scheduled workflows were merging unreviewed code to
mainevery day, which is how it got there.Everything below was verified by building and running real artifacts, not only by tests.
The two schedules that caused this
jules-daily-auto-upgrade.ymlAUTO_CREATE_PR.auto-merge-jules.ymlwait-on-check-action, but no CI checks existed, so the gate passed vacuously. Heldcontents: writeonpull_request_target.auto-deploy.ymlnpm testagainst the rootpackage.json(collects one TypeScript file — never the backend or frontend suites), then "deploy and health check" fell through topython -m http.serverand confirmed that returned 200.Both merge paths are disabled. No scheduled workflow can write to
mainany more.Defects fixed
The app could not start.
backend/main.pyandmodels.pyused bare intra-project imports while the other 20 modules used package imports, somodelsandbackend.modelsloaded as two separate modules and every SQLAlchemy table registered twice.backend.mainraisedInvalidRequestErrorat import. Render's/healthgate could therefore never pass.22 undefined names in the backend, including
timezoneinside/healthitself,funcin/api/stats, andImagein the detector paths — allNameErrorat request time.The frontend build failed.
App.jsxlazy-imported eleven components from./features/detectors/, a directory that does not exist. Rollup halts on the first unresolved import, so Netlify had no artifact.20 endpoints returned 404. Every service function already existed in
hf_api_service.pyandlocal_ml_service.py; none was routed. The grievance feature —grievance_service.py,escalation_engine.py,routing_service.py,sla_config_service.py, plus its API client and view — was fully written and never mounted.Three routes were registered twice. FastAPI serves the first match, so the later handler was dead each time. The served
detect-potholehad no successreturnand answerednull; the dead duplicate was the correct one.Four detector handlers named their upload
filewhile all 25 frontend call sites postimage— 422 on every request.detect_vandalismis a coroutine function called inside a sync closure passed torun_in_threadpool, producing an un-awaited coroutine and a 500 on every request.Representative lookup always returned
None._load_maharashtra_pincode_mapre-keyed a dict that was already keyed; iterating a dict yields its string keys, so the map was always empty. This is the feature the product is built around. Pincode411001now resolves to Kasba Peth / Ravindra Dhangekar.The upvote button 404'd — frontend posted
/vote, backend served/upvote./api/analyze-urgencyrejected every request from the report form: it requiredtext, the form postsdescription._check_hf_available()readHF_TOKENthen returnedTrueregardless, so an unconfigured deployment reported the hosted backend as available and never fell back.ActionView.jsxcalleduseEffectafter an early return — hook count varied with props, so React throws "Rendered more hooks than during the previous render" as soon asactionPlanbecomes set.Rate limiting was declared and never enforced.
RATE_LIMIT_ENABLEDandMAX_REQUESTS_PER_MINUTEwere inrender.yamland parsed inconfig.py, but no middleware read them. The detector, chat, caption and verification routes each call a paid inference API per request, so this was a billing exposure as much as an availability one.The Telegram poller ran in the FastAPI lifespan, so every uvicorn worker opened its own long-poll and Telegram rejected the extras with 409 — a hard one-worker ceiling that nothing documented.
Guards added
tests/test_api_contract.pyscansfrontend/srcfor every/api/...literal and asserts each resolves to a declared route, that no path is registered twice, that every detector route accepts POST, and that each accepts the upload field name its callers actually send. It found the/votebug the moment its regex was widened to cover template literals.ci.yml: ruff lint + format + pytest on 3.12, eslint + jest + vite build on Node 20 and 22, a Docker job that builds the image and polls/healthinside the container, and bandit / pip-audit / npm audit.Android
Capacitor 8 wrapping the existing Vite build. Permissions declared, icons and splash at every density,
androidScheme: httpsto match the CORS allowlist and avoid the cleartext block, release signing from environment variables only, R8 on for release.android.ymlbuilds a debug APK on PRs and a signed AAB on tags, refuses to build unlessVITE_API_URLis absolute HTTPS, and verifies the bundle carries a release signature rather than trusting Gradle not to have fallen back to debug.Verified, not assumed
Built and driven on a Galaxy S20 FE 5G (SM-G781B, Android 13):
Building locally rather than trusting CI caught a defect nothing else would have:
def keyAliasshadowed the GradlesigningConfigDSL setter, sokeyAlias keyAliasparsed as invoking a String as a method. Configuration aborts before any task runs — every release build would have failed, including CI on its first tag.Also fixed:
networkTimeout=10000in the committed wrapper properties applies as a socket read timeout to Gradle's own ~230MB download, so any stall over ten seconds aborts it and the retry restarts from zero.Numbers
ruff checkbackend/main_fixed.py— 1,010 lines, zero importers, 28 routes that never served a request — is deleted.Known gaps
except: pass.Summary by cubic
Makes the app start and work end to end: fixes backend import/boot and the frontend build, wires missing APIs, enforces rate limits and an admin API key on privileged actions, moves schema to Alembic, adds an Android app, routes previously unreachable detectors (now including SmartScanner), and fixes chat payload handling.
Behavior changes
Highlights
backend.*imports (fixes double-registered tables), mount grievance routes and previously dead detector endpoints, add/api/issues/nearby, background action‑plan generation on issue create (201 then later fill), validate uploads (byte size and decoded image), standardize upload field toimage, route detectors through a unified service with local→hosted fallback, fix async handler bugs, return verification confidence/question, tighten CORS and append Capacitor origins, move Telegram polling off the web process, and enforceslowapirate limits (/healthexempt). Adds a JSON‑payload contract test to cover body shapes like/api/chat.alembicmigrations (baseline revision; batch mode for SQLite). Removeinit_db.py; run migrations once per deploy./healthcheck; CI runs ruff/pytest, eslint/jest/vite build,bandit,pip-audit,npm audit, and boots the image; lock Python deps; add contract tests that assert every frontend/api/*resolves with the right method, upload field, and JSON body.Rollout
alembic upgrade head. Remove any startup migration hooks.Written for commit 6e88b0a. Summary will update on new commits.