feat: add GPS nearby locations endpoint with Overpass API

forgejo_admin commented

2026-03-16 19:15:43 +00:00

Contributor

Summary

Add GET /locations/nearby?lat=X&lng=Y&radius=5000 endpoint that queries OpenStreetMap's Overpass API for nearby McDonald's locations, calculates haversine distances, and matches results against the user's saved locations with slot availability info.

Changes

src/mcd_tracker_api/services/geo.py: New haversine distance formula (great-circle distance in metres)
src/mcd_tracker_api/services/overpass.py: Async Overpass API client using httpx, parses OSM nodes into OsmLocation dataclasses, raises typed exceptions on failure
src/mcd_tracker_api/routes/nearby.py: The endpoint -- validates lat/lng/radius, calls Overpass, proximity-matches against saved locations (100m threshold), enriches with slot counts, sorts by distance
src/mcd_tracker_api/schemas.py: Added NearbyLocation, NearbyResponse, SavedLocationMatch schemas
src/mcd_tracker_api/main.py: Registered nearby router before locations router so /locations/nearby matches before /locations/{id}
tests/test_geo.py: 5 unit tests for haversine function
tests/test_overpass.py: 14 tests for Overpass client (all HTTP mocked)
tests/test_nearby.py: 16 integration tests for the endpoint (Overpass mocked, DB for saved location matching)

Test Plan

All 35 new tests pass in CI (Postgres service container for DB tests)
All Overpass API calls are mocked -- no real HTTP in CI
ruff check and ruff format --check pass clean
Modules import correctly, routes registered in correct order

Review Checklist

Passed automated review-fix loop
No secrets committed
No unnecessary file changes
Commit messages are descriptive

Closes #10
Forgejo issue: forgejo_admin/mcd-tracker-api #10

## Summary Add `GET /locations/nearby?lat=X&lng=Y&radius=5000` endpoint that queries OpenStreetMap's Overpass API for nearby McDonald's locations, calculates haversine distances, and matches results against the user's saved locations with slot availability info. ## Changes - `src/mcd_tracker_api/services/geo.py`: New haversine distance formula (great-circle distance in metres) - `src/mcd_tracker_api/services/overpass.py`: Async Overpass API client using httpx, parses OSM nodes into OsmLocation dataclasses, raises typed exceptions on failure - `src/mcd_tracker_api/routes/nearby.py`: The endpoint -- validates lat/lng/radius, calls Overpass, proximity-matches against saved locations (100m threshold), enriches with slot counts, sorts by distance - `src/mcd_tracker_api/schemas.py`: Added NearbyLocation, NearbyResponse, SavedLocationMatch schemas - `src/mcd_tracker_api/main.py`: Registered nearby router before locations router so /locations/nearby matches before /locations/{id} - `tests/test_geo.py`: 5 unit tests for haversine function - `tests/test_overpass.py`: 14 tests for Overpass client (all HTTP mocked) - `tests/test_nearby.py`: 16 integration tests for the endpoint (Overpass mocked, DB for saved location matching) ## Test Plan - [ ] All 35 new tests pass in CI (Postgres service container for DB tests) - [ ] All Overpass API calls are mocked -- no real HTTP in CI - [ ] ruff check and ruff format --check pass clean - [ ] Modules import correctly, routes registered in correct order ## Review Checklist - [ ] Passed automated review-fix loop - [ ] No secrets committed - [ ] No unnecessary file changes - [ ] Commit messages are descriptive ## Related - Closes #10 - Forgejo issue: forgejo_admin/mcd-tracker-api #10

forgejo_admin added 1 commit

2026-03-16 19:15:43 +00:00

feat: add GPS nearby locations endpoint with Overpass API

ci/woodpecker/pr/woodpecker Pipeline failed

Details

b384b1d152

Add GET /locations/nearby?lat=X&lng=Y&radius=5000 that queries
OpenStreetMap Overpass API for McDonald's locations, calculates
haversine distances, and matches against user's saved locations
with slot availability.

New files:
- services/geo.py -- haversine distance calculation
- services/overpass.py -- async Overpass API client with error handling
- routes/nearby.py -- endpoint with query validation, proximity matching
- schemas: NearbyLocation, NearbyResponse, SavedLocationMatch

Tests mock Overpass API (never hit real API in CI). Covers basic
queries, validation, error handling, saved location matching with
slot counts, distance sorting, and missing address fields.

Closes #10

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin added 1 commit

2026-03-16 19:16:50 +00:00

fix: correct stale docstring and type hint from review

ci/woodpecker/pr/woodpecker Pipeline failed

Details

bdc24fe099

- overpass.py: docstring said "never raises" but function raises
  OverpassTimeoutError/OverpassError -- updated to match behavior
- nearby.py: _build_slot_info return type was tuple[int, int, None | str]
  but next_reopen is datetime | None -- fixed type hint

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-16 19:17:01 +00:00

Author

Contributor

Self-Review Findings

Two issues found and fixed in bdc24fe:

Stale docstring in overpass.py: query_nearby docstring said "Never raises -- errors are logged and swallowed" but the function actually raises OverpassTimeoutError and OverpassError. Updated to accurately describe the raise behavior.
Wrong type hint in nearby.py: _build_slot_info return type was tuple[int, int, None | str] but next_reopen is a datetime | None from the SQLAlchemy query. Fixed to tuple[int, int, datetime | None].

Both fixes are in the second commit. No logic changes.

## Self-Review Findings Two issues found and fixed in `bdc24fe`: 1. **Stale docstring in `overpass.py`**: `query_nearby` docstring said "Never raises -- errors are logged and swallowed" but the function actually raises `OverpassTimeoutError` and `OverpassError`. Updated to accurately describe the raise behavior. 2. **Wrong type hint in `nearby.py`**: `_build_slot_info` return type was `tuple[int, int, None | str]` but `next_reopen` is a `datetime | None` from the SQLAlchemy query. Fixed to `tuple[int, int, datetime | None]`. Both fixes are in the second commit. No logic changes.

forgejo_admin commented

2026-03-16 19:19:59 +00:00

Author

Contributor

PR #11 Review

DOMAIN REVIEW

Tech stack: Python 3.12 / FastAPI / SQLAlchemy / httpx / Pydantic / pytest. Applying Python/FastAPI/SQLAlchemy domain checks.

Haversine formula (services/geo.py): Mathematically correct. Uses the standard atan2(sqrt(a), sqrt(1-a)) form which is numerically stable for both small and large distances. Earth radius constant 6_371_000 is the accepted mean radius. Tests cover identity, known distance, short range, antipodal, and symmetry -- good coverage.

Overpass API client (services/overpass.py): Properly async via httpx.AsyncClient in an async with context manager. Timeout handling is correct -- the httpx timeout matches the Overpass QL [timeout:] directive. Error hierarchy (OverpassError > OverpassTimeoutError) is clean. The brand="McDonald's" filter is the correct OSM tag. Apostrophe escaping in the f-string is correct.

Saved location matching (routes/nearby.py): The _find_saved_match function correctly finds the closest match under the 100m threshold using < (not <=), and initializes best_distance to the threshold itself so only closer matches win. The null lat/lng guard on saved locations is present both in the DB query (lines 112-113) and in the matching loop (lines 40-41), which is belt-and-suspenders -- good.

User isolation (routes/nearby.py, line 111): The saved locations query filters by Location.keycloak_sub == user.sub, so user A's saved locations cannot leak into user B's results. The _build_slot_info function also passes user.sub through to the coupon usage query. The isolation is correct at the code level.

Route registration (main.py): The nearby router is registered before the locations router (line 41 vs 42), ensuring /locations/nearby matches before /locations/{id}. This is correct and the PR body explains the reasoning.

Input validation: FastAPI Query parameters enforce lat [-90, 90], lng [-180, 180], radius [100, 50000]. All user-supplied values validated before reaching Overpass or the DB.

Schemas (schemas.py): NearbyLocation, NearbyResponse, SavedLocationMatch are clean Pydantic models. No from_attributes needed since they are constructed manually (not from ORM instances).

BLOCKERS

1. anyio / pytest-anyio missing from dev dependencies -- async tests will not run in CI

tests/test_overpass.py uses @pytest.mark.anyio on 5 async tests (lines 78, 120, 137, 149, 166), but neither anyio nor pytest-anyio appears in pyproject.toml's [project.optional-dependencies] dev list. Without the pytest-anyio plugin installed, these markers will either:

Be silently treated as unknown markers (tests skipped without error if --strict-markers is off), or
Fail with "Unknown pytest.mark.anyio" if strict markers are enabled.

Either way, the 5 async Overpass client tests will not actually execute in CI. The CI step runs pip install .[dev] followed by pytest, and without the anyio plugin, those tests are dead code.

Fix: Add "anyio[trio]>=4.0" or "pytest-anyio>=0.0.1" (or alternatively "pytest-asyncio>=0.23" and switch to @pytest.mark.asyncio) to the dev dependencies in pyproject.toml.

This is a BLOCKER per the "new functionality with zero test coverage" criterion -- the Overpass client's async behavior is tested, but those tests will not actually run.

NITS

1. DRY: _build_slot_info duplicates _count_active_codes / _next_reopen from locations.py

nearby.py lines 50-76 reimplement the exact same two queries that exist as _count_active_codes (locations.py:74-84) and _next_reopen (locations.py:87-98). Consider extracting these into a shared module (e.g., services/slots.py) that both route files import. Not blocking since this is not an auth/security path, but it creates divergence risk if slot logic changes.

2. Overpass query only fetches node elements -- misses way and relation McDonald's

In OpenStreetMap, McDonald's locations can be mapped as nodes (points), ways (building outlines), or relations (complex buildings). The query node["brand"="McDonald's"] only returns nodes. A more complete query would use nwr (node/way/relation) and add out center; to get centroid coordinates for ways/relations. This could miss a meaningful percentage of McDonald's in dense urban areas. Non-blocking since the current approach still works -- just incomplete.

3. No user isolation test

While the code correctly filters by user.sub, there is no test that creates saved locations for a different user and verifies they do not appear in the results. A test like "user B's saved location near the OSM result should not match for user A" would strengthen confidence. The existing tests all use TEST_USER.sub.

4. OVERPASS_URL is hardcoded

The Overpass API URL (https://overpass-api.de/api/interpreter) is a module-level constant. For production resilience (and testing without mocking), consider making this configurable via Settings so it can be pointed at a different Overpass instance or a self-hosted one.

5. _build_slot_info slot count could go negative if data is inconsistent

If active > MAX_SLOTS due to data corruption or a race condition, MAX_SLOTS - active would be negative. The available_slots field in SavedLocationMatch has no ge=0 constraint. Consider max(0, MAX_SLOTS - active).

SOP COMPLIANCE

Branch named after issue (10-add-gps-nearby-locations-endpoint-with-o references issue #10)
PR body has: Summary, Changes, Test Plan, Related
Related section references plan slug -- PR body says "Closes #10" and "forgejo_admin/mcd-tracker-api #10" but does not reference plan-mcd-tracker
No secrets committed
No unnecessary file changes -- all 9 files are directly relevant
Commit messages are descriptive (PR title follows conventional commit format)

PROCESS OBSERVATIONS

Test count: 35 new tests (5 geo, 14 overpass, 16 nearby) is strong for 890 lines of new code. Test-to-code ratio is healthy.
Deployment frequency: This is a clean feature addition with no migrations or breaking changes. Low change failure risk once the anyio dependency is fixed.
Documentation gap: The PR body's Related section should reference the plan slug (plan-mcd-tracker) per SOP convention.
Risk: The anyio dependency gap means this PR has been developed with tests that never actually ran in CI. The dev agent should verify the tests pass with the plugin installed before re-submitting.

VERDICT: NOT APPROVED

One BLOCKER must be fixed: pytest-anyio (or equivalent) missing from dev dependencies means the 5 async Overpass client tests are not executing in CI. This effectively means the Overpass client has zero verified test coverage, which meets the "new functionality with zero test coverage" BLOCKER criterion.

## PR #11 Review ### DOMAIN REVIEW **Tech stack**: Python 3.12 / FastAPI / SQLAlchemy / httpx / Pydantic / pytest. Applying Python/FastAPI/SQLAlchemy domain checks. **Haversine formula** (`services/geo.py`): Mathematically correct. Uses the standard `atan2(sqrt(a), sqrt(1-a))` form which is numerically stable for both small and large distances. Earth radius constant `6_371_000` is the accepted mean radius. Tests cover identity, known distance, short range, antipodal, and symmetry -- good coverage. **Overpass API client** (`services/overpass.py`): Properly async via `httpx.AsyncClient` in an `async with` context manager. Timeout handling is correct -- the httpx timeout matches the Overpass QL `[timeout:]` directive. Error hierarchy (`OverpassError` > `OverpassTimeoutError`) is clean. The `brand="McDonald's"` filter is the correct OSM tag. Apostrophe escaping in the f-string is correct. **Saved location matching** (`routes/nearby.py`): The `_find_saved_match` function correctly finds the closest match under the 100m threshold using `<` (not `<=`), and initializes `best_distance` to the threshold itself so only closer matches win. The null lat/lng guard on saved locations is present both in the DB query (lines 112-113) and in the matching loop (lines 40-41), which is belt-and-suspenders -- good. **User isolation** (`routes/nearby.py`, line 111): The saved locations query filters by `Location.keycloak_sub == user.sub`, so user A's saved locations cannot leak into user B's results. The `_build_slot_info` function also passes `user.sub` through to the coupon usage query. The isolation is correct at the code level. **Route registration** (`main.py`): The nearby router is registered before the locations router (line 41 vs 42), ensuring `/locations/nearby` matches before `/locations/{id}`. This is correct and the PR body explains the reasoning. **Input validation**: FastAPI `Query` parameters enforce `lat` [-90, 90], `lng` [-180, 180], `radius` [100, 50000]. All user-supplied values validated before reaching Overpass or the DB. **Schemas** (`schemas.py`): `NearbyLocation`, `NearbyResponse`, `SavedLocationMatch` are clean Pydantic models. No `from_attributes` needed since they are constructed manually (not from ORM instances). ### BLOCKERS **1. `anyio` / `pytest-anyio` missing from dev dependencies -- async tests will not run in CI** `tests/test_overpass.py` uses `@pytest.mark.anyio` on 5 async tests (lines 78, 120, 137, 149, 166), but neither `anyio` nor `pytest-anyio` appears in `pyproject.toml`'s `[project.optional-dependencies] dev` list. Without the pytest-anyio plugin installed, these markers will either: - Be silently treated as unknown markers (tests skipped without error if `--strict-markers` is off), or - Fail with "Unknown pytest.mark.anyio" if strict markers are enabled. Either way, **the 5 async Overpass client tests will not actually execute in CI**. The CI step runs `pip install .[dev]` followed by `pytest`, and without the anyio plugin, those tests are dead code. **Fix**: Add `"anyio[trio]>=4.0"` or `"pytest-anyio>=0.0.1"` (or alternatively `"pytest-asyncio>=0.23"` and switch to `@pytest.mark.asyncio`) to the `dev` dependencies in `pyproject.toml`. This is a BLOCKER per the "new functionality with zero test coverage" criterion -- the Overpass client's async behavior is tested, but those tests will not actually run. ### NITS **1. DRY: `_build_slot_info` duplicates `_count_active_codes` / `_next_reopen` from `locations.py`** `nearby.py` lines 50-76 reimplement the exact same two queries that exist as `_count_active_codes` (locations.py:74-84) and `_next_reopen` (locations.py:87-98). Consider extracting these into a shared module (e.g., `services/slots.py`) that both route files import. Not blocking since this is not an auth/security path, but it creates divergence risk if slot logic changes. **2. Overpass query only fetches `node` elements -- misses `way` and `relation` McDonald's** In OpenStreetMap, McDonald's locations can be mapped as nodes (points), ways (building outlines), or relations (complex buildings). The query `node["brand"="McDonald's"]` only returns nodes. A more complete query would use `nwr` (node/way/relation) and add `out center;` to get centroid coordinates for ways/relations. This could miss a meaningful percentage of McDonald's in dense urban areas. Non-blocking since the current approach still works -- just incomplete. **3. No user isolation test** While the code correctly filters by `user.sub`, there is no test that creates saved locations for a different user and verifies they do not appear in the results. A test like "user B's saved location near the OSM result should not match for user A" would strengthen confidence. The existing tests all use `TEST_USER.sub`. **4. `OVERPASS_URL` is hardcoded** The Overpass API URL (`https://overpass-api.de/api/interpreter`) is a module-level constant. For production resilience (and testing without mocking), consider making this configurable via `Settings` so it can be pointed at a different Overpass instance or a self-hosted one. **5. `_build_slot_info` slot count could go negative if data is inconsistent** If `active > MAX_SLOTS` due to data corruption or a race condition, `MAX_SLOTS - active` would be negative. The `available_slots` field in `SavedLocationMatch` has no `ge=0` constraint. Consider `max(0, MAX_SLOTS - active)`. ### SOP COMPLIANCE - [x] Branch named after issue (`10-add-gps-nearby-locations-endpoint-with-o` references issue #10) - [x] PR body has: Summary, Changes, Test Plan, Related - [ ] Related section references plan slug -- PR body says "Closes #10" and "forgejo_admin/mcd-tracker-api #10" but does not reference `plan-mcd-tracker` - [x] No secrets committed - [x] No unnecessary file changes -- all 9 files are directly relevant - [x] Commit messages are descriptive (PR title follows conventional commit format) ### PROCESS OBSERVATIONS - **Test count**: 35 new tests (5 geo, 14 overpass, 16 nearby) is strong for 890 lines of new code. Test-to-code ratio is healthy. - **Deployment frequency**: This is a clean feature addition with no migrations or breaking changes. Low change failure risk once the anyio dependency is fixed. - **Documentation gap**: The PR body's Related section should reference the plan slug (`plan-mcd-tracker`) per SOP convention. - **Risk**: The anyio dependency gap means this PR has been developed with tests that never actually ran in CI. The dev agent should verify the tests pass with the plugin installed before re-submitting. ### VERDICT: NOT APPROVED One BLOCKER must be fixed: `pytest-anyio` (or equivalent) missing from dev dependencies means the 5 async Overpass client tests are not executing in CI. This effectively means the Overpass client has zero verified test coverage, which meets the "new functionality with zero test coverage" BLOCKER criterion.

forgejo_admin added 1 commit

2026-03-16 19:22:11 +00:00

fix: add anyio dev dependency so async overpass tests run in CI

ci/woodpecker/pr/woodpecker Pipeline failed

Details

e235b24827

The 5 async tests in test_overpass.py use @pytest.mark.anyio but the
anyio package (which provides the pytest plugin) was not listed in
dev dependencies. Without it, pytest silently skips the async tests.

Closes #10

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>