Feat/real world market locations#77
Conversation
|
@Aharshi3614 is attempting to deploy a commit to the karan3431's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
🎉 Thank you for your Pull Request! We're thrilled to have your contribution to FreshScan AI. Before we review, please make sure you have:
A maintainer will review your code as soon as possible! |
|
Can you just upload a ss or video to show this feature in working state to make it merge shortly as i am tired it takes 4+ hr to debug and settle the issue of the onnx update today. Dont forget to pull the latest changes. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Here is the ss of it working!
|
|
pull the latest changes and try to fix the backend lint test, if not dont take the pressure much i will be checking it tonight. |
|
The failing job is the Lint step in the CI workflow. The error shows ESLint found 2 linting errors:
Or run this command locally to identify and fix the issues: npm run lint -- --fix |
|
Warning Review limit reached
More reviews will be available in 52 minutes and 5 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. Warning
|
| Layer / File(s) | Summary |
|---|---|
Markets API endpoint (Overpass integration) backend/markets.py |
FastAPI router at /api/v1/maps/markets with GET /live endpoint that accepts latitude, longitude, and radius; computes latitude-scaled bounding box; builds and executes Overpass QL query for shop=seafood and shop=fish nodes; normalizes response into structured market objects (name, coordinates, vendor count, address); returns JSON success payload on success or error details on timeout/failure. |
Vendors leaderboard endpoint backend/main.py |
New GET /api/v1/vendors/leaderboard endpoint that queries vendors table, orders by trust_score descending, returns top 10 vendors in leaderboard response, or HTTP 500 on error. |
Backend wiring, imports, and CORS configuration backend/main.py |
Import paths consolidated to backend.fusion, backend.router, backend.inference submodules; CORS allowlist refactored to parse ADDITIONAL_CORS_ORIGINS environment variable and include Vercel production origin alongside FRONTEND_URL; markets_router included in FastAPI app startup. |
Sequence Diagram
sequenceDiagram
participant Client
participant MarketsAPI as GET /api/v1/maps/markets/live
participant OverpassAPI as Overpass Interpreter
Client->>MarketsAPI: lat, lng, radius
MarketsAPI->>MarketsAPI: compute_bbox (latitude-scaled bounds)
MarketsAPI->>MarketsAPI: build Overpass QL query
MarketsAPI->>OverpassAPI: POST query (shop=seafood|fish)
alt Success
OverpassAPI-->>MarketsAPI: elements (nodes with tags)
MarketsAPI->>MarketsAPI: normalize_markets (extract fields)
MarketsAPI-->>Client: JSON {lat, lng, radius, markets:[...]}
else Timeout/Error
OverpassAPI-->>MarketsAPI: exception
MarketsAPI-->>Client: JSON {error, markets:[]}
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
🐰 A market-finder hops with glee,
Overpass queries run free,
Top vendors leap to the board,
Fresh data is freshly stored!
CORS routes are tidied up bright,
The backend is wired just right. 🥬
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check | Multiple out-of-scope changes are present: added /api/v1/vendors/leaderboard endpoint not in #75 requirements, modified CORS configuration logic, changed unrelated imports, and included accidental files (backend/notepad, notepad, backend/test_markets.py). |
Remove leaderboard endpoint addition, revert CORS/import changes unrelated to markets feature, delete accidental files, and ensure only markets-related code is included in this PR. | |
| Docstring Coverage | Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | Title 'Feat/real world market locations' directly describes the main change: addition of a new endpoint to fetch real-world fish market locations using OpenStreetMap Overpass API. |
| Linked Issues check | ✅ Passed | The PR implements all core requirements from issue #75: integrates OpenStreetMap Overpass API [#75], fetches real markets based on GPS location [#75], updates backend routes [#75], and standardizes response format [#75]. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
|
@jpdevhub fixed the issues now |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
backend/test_markets.py (1)
12-15: ⚡ Quick winAdd User-Agent header to match production implementation.
The production endpoint at
backend/markets.py:84includes aUser-Agentheader, which aligns with Overpass API usage recommendations. This test script should follow the same pattern for consistency and API etiquette.🔧 Add User-Agent header
r = requests.post( "https://overpass-api.de/api/interpreter", - data={"data": query} + data={"data": query}, + headers={"User-Agent": "FreshScanAI/1.0 (https://github.com/jpdevhub/FreshScanAi)"} )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/test_markets.py` around lines 12 - 15, The test's requests.post call in test_markets.py lacks the User-Agent header used in production; update the POST to include a headers dict with the same 'User-Agent' string used in backend/markets.py (the request to "https://overpass-api.de/api/interpreter") so the test mirrors production etiquette — locate the requests.post invocation and add headers={'User-Agent': '<same-value-as-backend/markets.py>'} when sending data={"data": query}.backend/markets.py (3)
91-99: 💤 Low valueResponse includes fields not in frontend contract.
The success response adds
source,count,lat,lng, andradius_mfields, but the frontendMarketsResponseinterface expects onlysuccessandmarkets. While extra fields won't break TypeScript consumers, they deviate from the documented API contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/markets.py` around lines 91 - 99, The response currently returns extra fields (source, count, lat, lng, radius_m) alongside success and markets; update the return value in the function that builds this dict (the block returning {"success": True, "source": ..., "count": ..., "lat": ..., "lng": ..., "radius_m": ..., "markets": markets}) to conform to the frontend MarketsResponse contract by returning only "success" and "markets" (use the existing markets variable) and remove the additional keys so the API matches the documented interface.
37-62: 💤 Low valueDocument magic numbers for fallback score and vendor count.
Lines 55 and 58 use hardcoded values (
fallback_score=70,vendors=1) without explanation. Adding brief comments would clarify why these defaults were chosen.📝 Suggested documentation
def _parse_overpass(elements: list, fallback_score: int = 70) -> list: """Normalize Overpass API elements into the MarketMapPage format.""" markets = [] for i, el in enumerate(elements): tags = el.get("tags", {}) name = ( tags.get("name") or tags.get("name:en") or tags.get("shop") or "Fish Market" ) lat = el.get("lat") lon = el.get("lon") if lat is None or lon is None: continue markets.append({ "id": el.get("id", i + 1), "name": name, - "score": fallback_score, + "score": fallback_score, # Default quality score when OSM has no rating data "lat": float(lat), "lng": float(lon), - "vendors": 1, + "vendors": 1, # OSM nodes represent single locations "source": "openstreetmap", "address": tags.get("addr:full") or tags.get("addr:street") or "", }) return markets🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/markets.py` around lines 37 - 62, The function _parse_overpass uses unexplained magic numbers (fallback_score=70 and vendors=1); replace these literals with clearly named module-level constants (e.g., FALLBACK_SCORE_DEFAULT and DEFAULT_VENDOR_COUNT), update the _parse_overpass signature to use FALLBACK_SCORE_DEFAULT as the default for fallback_score, use DEFAULT_VENDOR_COUNT for the "vendors" field, and add a one-line comment above each constant (or in the function docstring) explaining why those defaults were chosen or when they apply to make the intent clear.
25-34: ⚡ Quick winRemove unnecessary epsilon in longitude delta calculation.
Line 28 adds
+ 1e-9but no division occurs—this epsilon serves no purpose. The calculation is strictly multiplication, so even at the poles wherecos(lat) = 0, no division-by-zero can occur.♻️ Simplify the calculation
- delta_lng = radius_m / (111320 * abs(math.cos(math.radians(lat))) + 1e-9) + delta_lng = radius_m / (111320 * abs(math.cos(math.radians(lat))))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/markets.py` around lines 25 - 34, Remove the unnecessary epsilon in the longitude delta calculation inside _lat_lng_to_bbox: change the delta_lng computation to use radius_m / (111320 * abs(math.cos(math.radians(lat)))) (i.e., drop the "+ 1e-9") so the expression is simplified and clearer; update only the delta_lng expression.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/main.py`:
- Line 821: Update the import to use the package namespace: replace the
top-level import "from router import is_valid_fish_image" with the
module-qualified import "from backend.router import is_valid_fish_image" so it
matches the existing backend.* pattern in main.py and avoids ImportError when
importing is_valid_fish_image.
- Line 910: The import statement that currently reads "from markets import
router as markets_router" is missing the backend namespace; update that import
so it references the backend.markets module (i.e., prefix the module path with
"backend.") so it aligns with the other consolidated imports and prevents
ImportError at startup.
In `@backend/markets.py`:
- Around line 107-113: The except Exception as exc block currently returns the
raw exception via "error": str(exc); change this to return a generic,
non-sensitive message (e.g., "An internal error occurred") in production while
still recording the full exception to internal logs; update the except block
(the except Exception as exc handler and the returned dict with "error") to
check your environment/config (e.g., PRODUCTION or app.debug) and return a
sanitized message for clients, and call your logger (e.g., logger.exception or
logger.error with exc_info=True) to preserve the original exc for debugging.
---
Nitpick comments:
In `@backend/markets.py`:
- Around line 91-99: The response currently returns extra fields (source, count,
lat, lng, radius_m) alongside success and markets; update the return value in
the function that builds this dict (the block returning {"success": True,
"source": ..., "count": ..., "lat": ..., "lng": ..., "radius_m": ..., "markets":
markets}) to conform to the frontend MarketsResponse contract by returning only
"success" and "markets" (use the existing markets variable) and remove the
additional keys so the API matches the documented interface.
- Around line 37-62: The function _parse_overpass uses unexplained magic numbers
(fallback_score=70 and vendors=1); replace these literals with clearly named
module-level constants (e.g., FALLBACK_SCORE_DEFAULT and DEFAULT_VENDOR_COUNT),
update the _parse_overpass signature to use FALLBACK_SCORE_DEFAULT as the
default for fallback_score, use DEFAULT_VENDOR_COUNT for the "vendors" field,
and add a one-line comment above each constant (or in the function docstring)
explaining why those defaults were chosen or when they apply to make the intent
clear.
- Around line 25-34: Remove the unnecessary epsilon in the longitude delta
calculation inside _lat_lng_to_bbox: change the delta_lng computation to use
radius_m / (111320 * abs(math.cos(math.radians(lat)))) (i.e., drop the "+ 1e-9")
so the expression is simplified and clearer; update only the delta_lng
expression.
In `@backend/test_markets.py`:
- Around line 12-15: The test's requests.post call in test_markets.py lacks the
User-Agent header used in production; update the POST to include a headers dict
with the same 'User-Agent' string used in backend/markets.py (the request to
"https://overpass-api.de/api/interpreter") so the test mirrors production
etiquette — locate the requests.post invocation and add headers={'User-Agent':
'<same-value-as-backend/markets.py>'} when sending data={"data": query}.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3bd52db0-0a7c-444b-9526-50423c2e8a04
📒 Files selected for processing (5)
backend/main.pybackend/markets.pybackend/notepadbackend/test_markets.pysrc/pages/Leaderboard.tsx
| import numpy as np | ||
| from inference import stream_a_model, stream_a_transforms, device | ||
| from backend.inference import stream_a_model, stream_a_transforms, device | ||
| from router import is_valid_fish_image |
There was a problem hiding this comment.
Import path missing backend. prefix.
Line 821 imports from router but should use backend.router to match the pattern established at line 531 and throughout this PR. This inconsistency will cause an ImportError when the module structure enforces the backend.* namespace.
🐛 Fix the import path
- from router import is_valid_fish_image
+ from backend.router import is_valid_fish_image📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from router import is_valid_fish_image | |
| from backend.router import is_valid_fish_image |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/main.py` at line 821, Update the import to use the package namespace:
replace the top-level import "from router import is_valid_fish_image" with the
module-qualified import "from backend.router import is_valid_fish_image" so it
matches the existing backend.* pattern in main.py and avoids ImportError when
importing is_valid_fish_image.
| except Exception as exc: | ||
| return { | ||
| "success": False, | ||
| "source": "openstreetmap", | ||
| "error": str(exc), | ||
| "markets": [], | ||
| } |
There was a problem hiding this comment.
Sanitize exception messages to avoid information leakage.
Line 111 exposes the raw exception message via str(exc), which can reveal internal implementation details such as file paths, library versions, or database schema. Replace with a generic error message in production.
🛡️ Proposed sanitization
except Exception as exc:
+ # Log the full exception internally for debugging
+ print(f"Overpass API error: {exc}")
return {
"success": False,
"source": "openstreetmap",
- "error": str(exc),
+ "error": "Failed to fetch markets. Please try again later.",
"markets": [],
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except Exception as exc: | |
| return { | |
| "success": False, | |
| "source": "openstreetmap", | |
| "error": str(exc), | |
| "markets": [], | |
| } | |
| except Exception as exc: | |
| # Log the full exception internally for debugging | |
| print(f"Overpass API error: {exc}") | |
| return { | |
| "success": False, | |
| "source": "openstreetmap", | |
| "error": "Failed to fetch markets. Please try again later.", | |
| "markets": [], | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/markets.py` around lines 107 - 113, The except Exception as exc block
currently returns the raw exception via "error": str(exc); change this to return
a generic, non-sensitive message (e.g., "An internal error occurred") in
production while still recording the full exception to internal logs; update the
except block (the except Exception as exc handler and the returned dict with
"error") to check your environment/config (e.g., PRODUCTION or app.debug) and
return a sanitized message for clients, and call your logger (e.g.,
logger.exception or logger.error with exc_info=True) to preserve the original
exc for debugging.
|
The open street map logic is perfect but with that their are many flaws:
|
…eat/real-world-market-locations
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/main.py (1)
863-863:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winImport path missing
backend.prefix.Line 863 imports from
routerbut should usebackend.routerto match the pattern established at lines 31, 539, 862, 949, and 952. This inconsistency will cause anImportErrorwhen the module structure enforces thebackend.*namespace.🐛 Fix the import path
- from router import is_valid_fish_image + from backend.router import is_valid_fish_image🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/main.py` at line 863, Replace the incorrect module import by updating the import statement that currently reads "from router import is_valid_fish_image" to import from the package namespace instead (i.e., use "from backend.router import is_valid_fish_image"); locate the import near the top-level imports in main.py (the line importing is_valid_fish_image) and change the module path to backend.router so it matches the established backend.* pattern and prevents ImportError.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/main.py`:
- Around line 713-728: Delete the duplicate leaderboard endpoint implementation
in backend/main.py: remove the get_vendor_leaderboard async function that
defines the /api/v1/vendors/leaderboard route (the block ordering by
"trust_score") so the application uses the canonical vendors.py leaderboard
(which orders by avg_freshness_score and provides trust_badge/trend and
configurable limits); ensure no other references to get_vendor_leaderboard
remain and confirm register_routes(vendors_router, _db) is used to mount the
vendors.py routes.
---
Duplicate comments:
In `@backend/main.py`:
- Line 863: Replace the incorrect module import by updating the import statement
that currently reads "from router import is_valid_fish_image" to import from the
package namespace instead (i.e., use "from backend.router import
is_valid_fish_image"); locate the import near the top-level imports in main.py
(the line importing is_valid_fish_image) and change the module path to
backend.router so it matches the established backend.* pattern and prevents
ImportError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fe29f6f3-464f-4ec4-968f-8d229a9d6601
📒 Files selected for processing (2)
backend/main.pynotepad
|
Hey @Aharshi3614, you are completely right! I specifically asked you to restore that endpoint, and you followed my instructions perfectly. My apologies for the confusion there. Keep it exactly as you have it in main.py, and I'll handle any cleanup or duplication in a separate PR later. We are almost ready to merge! Just two quick fixes needed on your latest commit:
Everything else looks perfect. Let me know when you've pushed these last updates so we can get this merged! |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/main.py (2)
862-863:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winInconsistent import paths: line 862 uses
backend.prefix, line 863 does not.Line 862 will fail with a
ModuleNotFoundErroron Hugging Face Spaces (same issue as lines 31 and 539), while line 863 correctly uses a top-level import. Both imports should follow the same pattern established at lines 949 and 952.🐛 Fix the import path
- from backend.inference import stream_a_model, stream_a_transforms, device + from inference import stream_a_model, stream_a_transforms, device from router import is_valid_fish_image🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/main.py` around lines 862 - 863, The imports are inconsistent: change the backend-prefixed imports (stream_a_model, stream_a_transforms, device) to use the same top-level import style as is_valid_fish_image so they resolve on Hugging Face Spaces; locate the import statement referencing stream_a_model/stream_a_transforms/device and replace the "backend." prefix with the top-level module name used elsewhere so all four symbols (stream_a_model, stream_a_transforms, device, is_valid_fish_image) use the same import path pattern.
31-31:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winImport paths with
backend.prefix will fail on Hugging Face Spaces.Lines 31, 539, and 862 all use the
backend.prefix for internal imports (backend.fusion,backend.router,backend.inference). When the server runs from thebackend/directory (Hugging Face Spaces deployment), Python's module resolution starts frombackend/, so these imports will fail withModuleNotFoundErrorlooking forbackend/backend/.... The maintainer explicitly requested top-level imports (lines 949 and 952 demonstrate the correct pattern). Remove thebackend.prefix from all three imports to match the deployment environment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/main.py` at line 31, The import statements use a leading "backend." package prefix which breaks when running from the backend/ directory; update the imports so they import directly from the top-level modules: change "from backend.fusion import process_and_fuse" to "from fusion import process_and_fuse", change "from backend.router import router" to "from router import router", and change the import from the backend.inference module to import directly from "inference" (keeping the same symbol name used in code); ensure all three imports (the ones importing process_and_fuse, router, and the inference symbol) have the "backend." prefix removed so module resolution works in the deployment environment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@backend/main.py`:
- Around line 862-863: The imports are inconsistent: change the backend-prefixed
imports (stream_a_model, stream_a_transforms, device) to use the same top-level
import style as is_valid_fish_image so they resolve on Hugging Face Spaces;
locate the import statement referencing
stream_a_model/stream_a_transforms/device and replace the "backend." prefix with
the top-level module name used elsewhere so all four symbols (stream_a_model,
stream_a_transforms, device, is_valid_fish_image) use the same import path
pattern.
- Line 31: The import statements use a leading "backend." package prefix which
breaks when running from the backend/ directory; update the imports so they
import directly from the top-level modules: change "from backend.fusion import
process_and_fuse" to "from fusion import process_and_fuse", change "from
backend.router import router" to "from router import router", and change the
import from the backend.inference module to import directly from "inference"
(keeping the same symbol name used in code); ensure all three imports (the ones
importing process_and_fuse, router, and the inference symbol) have the
"backend." prefix removed so module resolution works in the deployment
environment.
|
@jpdevhub I have done both updates! |
2 participants
Closes #75
What this does
Adds a new
/api/v1/maps/markets/liveendpoint that queries the OpenStreetMapOverpass API to fetch real fish markets, seafood shops, and marketplaces near
a user's GPS location.
New endpoint
GET /api/v1/maps/markets/live?lat={lat}&lng={lng}&radius={meters}
Response:
{
"success": true,
"source": "openstreetmap",
"count": 3,
"markets": [
{ "id": 123, "name": "Fish Market", "lat": 12.34, "lng": 56.78, "vendors": 1, "source": "openstreetmap" }
]
}
Implementation
backend/markets.pywith Overpass API integrationWhy OpenStreetMap over Google Places
Summary by CodeRabbit
Release Notes