From 7358d9eb487c107cc834544c69277139d0c4d29c Mon Sep 17 00:00:00 2001 From: Emi Griffith Date: Sat, 11 Jul 2026 02:03:57 -0700 Subject: [PATCH] Fix compare place names (Nominatim burst) + weekly duplicate location name (#29) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compare was the only page loading several cells at once, so its concurrent reverse-geocode calls burst past Nominatim's ~1 req/sec limit, got rate-limited, and cached a null label — leaving those locations stuck on bare coordinates. - climate.py: serialize + rate-limit reverse_geocode behind a lock (>=1.1s between Nominatim calls, re-checking the cache under the lock), so concurrent callers resolve reliably instead of bursting. - store.py: shorten the reverse-geocode miss TTL 1 day -> 1 hour so a transient null retries soon; add a `cache` flag to put_payload. - app.py: don't persist a calendar payload whose place failed to resolve, so the coordinates fallback can't stick for the life of the token. Weekly showed the place name twice (top label beside the Find button AND the results

). Keep the

(it carries the coords + climatology context) as the single name display; the top label now only holds the transient coordinates written on selection and is hidden once the named results render. Claude-Session: https://claude.ai/code/session_01B3Q2EkHHnTUX2BfhV5q1Zc Co-authored-by: Claude Opus 4.8 --- app.py | 8 +++++- climate.py | 75 ++++++++++++++++++++++++++++++++++-------------------- store.py | 11 +++++--- 3 files changed, 63 insertions(+), 31 deletions(-) diff --git a/app.py b/app.py index a330b7a..b982371 100644 --- a/app.py +++ b/app.py @@ -421,7 +421,13 @@ def api_calendar( full = not cache_meta.get("cached", False) run.set(run_type="full" if full else "partial", history_source="fetch" if full else "cache") - return _json_response(store.put_payload("calendar", cell["id"], key, token, payload), etag) + # Don't persist a payload whose place failed to resolve (a transient reverse- + # geocode miss) — otherwise the bare-coordinates fallback would stick for the + # life of the token. Compare loads several cells at once, so this is where a + # miss is most likely; leaving it uncached lets the next request retry. + return _json_response( + store.put_payload("calendar", cell["id"], key, token, payload, + cache=place is not None), etag) def api_day( diff --git a/climate.py b/climate.py index f0fb5d4..8febef2 100644 --- a/climate.py +++ b/climate.py @@ -499,6 +499,14 @@ def _load_recent_forecast(cell: dict) -> pd.DataFrame: _REVGEO_CACHE: dict[str, str | None] = {} +# Nominatim's usage policy allows ~1 request/second and rejects bursts. The compare +# page loads several locations at once, so serialize the reverse lookups here and +# space them out — otherwise the burst is rate-limited, a null label gets cached, and +# those locations are left showing bare coordinates. +_REVGEO_LOCK = threading.Lock() +_REVGEO_MIN_INTERVAL = 1.1 # seconds between successive Nominatim reverse calls +_revgeo_last = 0.0 + def reverse_geocode_cached(lat: float, lon: float) -> tuple[bool, str | None]: """(found, label) from the in-memory + SQLite revgeo caches only — never calls @@ -523,34 +531,47 @@ def reverse_geocode(lat: float, lon: float) -> str | None: found, label = reverse_geocode_cached(lat, lon) if found: return label - label = None - try: - r = _request( - "https://nominatim.openstreetmap.org/reverse", - # zoom 14 resolves to the suburb/neighbourhood level so we can lead - # with it when OSM has one (zoom 10 only ever returns the city). - {"lat": lat, "lon": lon, "format": "jsonv2", "zoom": 14, - "addressdetails": 1}, - 15, - phase="reverse_geocode", - headers={"User-Agent": "Thermograph/0.1 (local weather grading app)"}, - ) - a = r.json().get("address", {}) or {} - # Lead with the neighbourhood when available, then the city, then region. - neighborhood = (a.get("neighbourhood") or a.get("suburb") - or a.get("quarter") or a.get("city_district") - or a.get("borough")) - city = (a.get("city") or a.get("town") or a.get("village") - or a.get("hamlet") or a.get("county")) - region = a.get("state") or a.get("province") or a.get("region") - # dict.fromkeys drops duplicates (e.g. neighbourhood == city) in order. - parts = dict.fromkeys(p for p in (neighborhood, city, region) if p) - label = ", ".join(parts) or None - except Exception: # noqa: BLE001 - reverse geocoding is a nicety, never fatal + # Serialize + rate-limit the upstream call. Concurrent callers (the compare page + # loads several locations at once) would otherwise burst past Nominatim's ~1/sec + # limit and get rate-limited, caching a null label. Under the lock we re-check the + # cache (a peer may have just resolved this cell) and space successive calls out. + global _revgeo_last + with _REVGEO_LOCK: + found, label = reverse_geocode_cached(lat, lon) + if found: + return label + wait = _REVGEO_MIN_INTERVAL - (time.monotonic() - _revgeo_last) + if wait > 0: + time.sleep(wait) label = None - _REVGEO_CACHE[key] = label - store.put_revgeo(key, label) # survive restarts (a None label is retried daily) - return label + try: + r = _request( + "https://nominatim.openstreetmap.org/reverse", + # zoom 14 resolves to the suburb/neighbourhood level so we can lead + # with it when OSM has one (zoom 10 only ever returns the city). + {"lat": lat, "lon": lon, "format": "jsonv2", "zoom": 14, + "addressdetails": 1}, + 15, + phase="reverse_geocode", + headers={"User-Agent": "Thermograph/0.1 (local weather grading app)"}, + ) + a = r.json().get("address", {}) or {} + # Lead with the neighbourhood when available, then the city, then region. + neighborhood = (a.get("neighbourhood") or a.get("suburb") + or a.get("quarter") or a.get("city_district") + or a.get("borough")) + city = (a.get("city") or a.get("town") or a.get("village") + or a.get("hamlet") or a.get("county")) + region = a.get("state") or a.get("province") or a.get("region") + # dict.fromkeys drops duplicates (e.g. neighbourhood == city) in order. + parts = dict.fromkeys(p for p in (neighborhood, city, region) if p) + label = ", ".join(parts) or None + except Exception: # noqa: BLE001 - reverse geocoding is a nicety, never fatal + label = None + _revgeo_last = time.monotonic() + _REVGEO_CACHE[key] = label + store.put_revgeo(key, label) # survive restarts (a None label retries after its TTL) + return label def geocode(name: str, count: int = 5) -> list[dict]: diff --git a/store.py b/store.py index e9cf4ff..278003c 100644 --- a/store.py +++ b/store.py @@ -92,16 +92,21 @@ def get_json(kind: str, cell_id: str, key: str, token: str) -> dict | None: return json.loads(body) if body is not None else None -def put_payload(kind: str, cell_id: str, key: str, token: str, payload: dict) -> bytes: +def put_payload(kind: str, cell_id: str, key: str, token: str, payload: dict, + cache: bool = True) -> bytes: """Encode ``payload`` to compact JSON, persist it, and return the encoded bytes (so the caller can serve the exact same bytes without re-serializing). + Pass ``cache=False`` to serve the payload without persisting it — used when a + best-effort field (the reverse-geocoded place name) failed to resolve, so the + incomplete result isn't cached for the life of the token. + allow_nan=False mirrors Starlette's JSONResponse: a NaN slipping through grading should fail loudly here, not be cached as JS-unparseable `NaN`.""" body = json.dumps(payload, default=str, allow_nan=False, separators=(",", ":")).encode() conn = _conn() - if conn is not None: + if cache and conn is not None: try: conn.execute( "INSERT OR REPLACE INTO derived VALUES (?,?,?,?,?,?)", @@ -115,7 +120,7 @@ def put_payload(kind: str, cell_id: str, key: str, token: str, payload: dict) -> # --- reverse-geocode labels -------------------------------------------------- -REVGEO_MISS_TTL = 86400.0 # a "no label" result may be transient — retry after a day +REVGEO_MISS_TTL = 3600.0 # a "no label" result may be transient (rate limit) — retry hourly def revgeo_key(lat: float, lon: float) -> str: