fix: add weather location fallbacks

[gzhang33]

Summary

• add a frontend location fallback that uses network-based approximate coordinates when browser geolocation fails
• keep approximate coordinates unsaved until the user explicitly saves them in settings
• geocode location_name on the backend when weather endpoints are called without stored coordinates

Testing

• npm test -- --run tests/location.test.ts
• npx tsc --noEmit
• docker compose exec -T -e TEST_DATABASE_URL=postgresql+asyncpg://wardrobe:4f5a5cb732900d2d7890d3bcb215f05268b2dce7e6188c2d56810cd8e9ccba91@postgres:5432/wardrobe_test backend pytest tests/test_weather_api.py
• manual verification in local containers for the weather flow

Notes

• .gitignore has local changes in the worktree and is intentionally excluded from this PR.

[Copilot]

Pull request overview

Adds fallback mechanisms so the weather flow can still work when precise geolocation (or stored coordinates) aren’t available, by using network-based approximation on the frontend and on-demand geocoding on the backend.

Changes:

• Frontend: introduce shared location helper utilities (+ tests) and update Settings to fall back to IP-based approximate coordinates when browser geolocation fails.
• Backend: add WeatherService.geocode_location_name() and update weather endpoints to geocode location_name when coordinates are missing (plus API tests).
• UX: improve user-facing messaging when exact geolocation fails and approximate data is used.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
frontend/tests/location.test.ts	Adds unit coverage for new location helper utilities.
frontend/lib/location.ts	Introduces helpers for resolving network location responses, reverse-geocode formatting, and geolocation error messaging.
frontend/app/dashboard/settings/page.tsx	Adds network-location fallback and refactors reverse geocoding + messaging in Settings.
backend/tests/test_weather_api.py	Adds API-level tests for geocoding fallback behavior and missing-location errors.
backend/app/services/weather_service.py	Adds a Nominatim geocoding helper for resolving location_name to coordinates.
backend/app/api/weather.py	Uses geocoding fallback in /weather/current and /weather/forecast when coordinates aren’t stored/provided.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gzhang33]

Validation update for latest branch push:

• Reviewed all Copilot review threads in the PR UI/API: 5/5 are resolved; no remaining unresolved review comment requests found.
• Cleaned up the geocoding JSON error handling so HTTP failures and invalid JSON responses are handled distinctly.
• Fixed the Settings page lint warning for the unitSystem dependency without resetting unsaved measurement edits on unit toggles.
• Backend local test environment prepared with Python 3.11.15 in backend/.venv and project test dependencies installed.
• Backend full suite: 292 passed via pytest -q.
• Backend coverage validation: 292 passed via pytest --cov=app --cov-report=xml.
• Frontend tests: 61 passed via npm test.
• Frontend lint: npm run lint passes; the PR-touched Settings warning is gone. Remaining warnings are pre-existing in unrelated files.

Note: agent.md remains untracked locally and was not added to the PR.

[Anyesh]

Nice direction — falling back to network-based coords + server-side geocoding fixes a real gap. A few things need to change before this goes in, and a few I'd push back on for design reasons.

Must fix before merge

1. Geocoding has no cache and gets called on every weather request

weather_service.geocode_location_name runs on every /weather/current and /weather/forecast call for any user who hasn't saved coords. That's an unbounded number of Nominatim hits per user session, and Nominatim's usage policy caps you at ~1 req/sec with a strong preference for caching results. The rest of this service already uses Redis (CACHE_PREFIX, _cache_key) — geocode results should sit in the same cache, keyed by normalized location_name, TTL on the order of days. Right now you're one popular user away from getting the deployment IP-blocked from Nominatim.

2. Frontend calls Nominatim directly for reverse geocoding

frontend/app/dashboard/settings/page.tsx still hits nominatim.openstreetmap.org/reverse from the browser. The previous code at least set a User-Agent header; this PR drops it (browsers strip that header anyway, so the old code was already non-compliant, but the new code makes it worse by removing the intent). Nominatim's ToS requires an identifying UA — browser default UA does not qualify. Move reverse geocoding behind the backend the same way you did forward geocoding, so the UA from settings.get_geocoding_user_agent() is actually applied and you have one chokepoint for rate-limiting/caching.

3. ipapi.co fallback is a silent third-party data leak

The geolocation-denied path now sends the user's IP to ipapi.co without any disclosure in the UI or docs. For an open-source self-hosted wardrobe app this is a noticeable privacy regression. Either:

• proxy this through the backend too (the user's IP already goes there), or
• gate it behind an explicit "use approximate location" button so the user opts in.

At minimum: document the default in the settings page next to the button, and in the env var description.

Strongly suggest

4. Coord-resolution logic is duplicated

The 15-line block resolving lat/lon then geocoding is copy-pasted across get_current_weather and get_weather_forecast. Pull it into a single helper (_resolve_user_coords(user, weather_service, lat, lon) -> tuple[float, float]) that raises the 400/503 itself. Two endpoints today, more later, drift is guaranteed.

5. geocode_location_name returns display_name but every caller discards it

if geocoded:
    lat, lon, _ = geocoded

Either drop display_name from the return signature, or actually use it to canonicalize user.location_name on first successful geocode. Returning a value nobody reads is a footgun for the next person who calls this.

6. Silent None return on malformed Nominatim response

except (KeyError, TypeError, ValueError):
    return None

A bad first hit gets swallowed as "no match", caller raises 400 "Location not set" — which is misleading when the user has set a name. Log a warning at minimum so this is debuggable in production.

Smaller stuff

• WeatherService() is now constructed even when both lat/lon are missing and no location_name exists. Cheap, but move it after the early-return for clarity.
• unitSystemRef indirection in settings/page.tsx silences exhaustive-deps but reads stale unit-system if the user toggles units before userProfile loads. Narrow window, but a one-line comment explaining the ref pattern would help the next reader.
• test_weather_api.py builds the fake Weather object via type("Weather", (), {...})(). You have a real WeatherData dataclass — use it.
• frontend/tests/location.test.ts manually saves/restores process.env in each test. vi.stubEnv + vi.unstubAllEnvs in afterEach is cleaner.
• from None in the API exception handlers strips the cause chain; you log it explicitly so this is fine, but from e is more conventional and shows up in /health debug traces.
• geocoding_user_agent default of "Wardrowbe/1.0" should include a contact URL (Nominatim ToS asks for one), e.g. "Wardrowbe/1.0 (+https://github.com/Anyesh/wardrowbe)".

Nits

• Removed blank line between third-party and first-party imports in weather.py. Will ruff format --check flag this? If CI is green, ignore.
• PR body says .gitignore is intentionally excluded — fine, just call out in commit what changed locally so it doesn't get lost.

Happy to look again once geocode caching + the Nominatim-via-backend changes land. The structure of this is right, it just needs the caching layer to be safe to deploy.

[Anyesh]

Thank you for your PR 🙂 claude did the first pass so lmk how if anything is not relevant and I'll review next.
Thanks again!