Merge pull request 'Merge main into dev: absorb responsiveness hardening; migrate its new tests to the unit tier' (#6) from reconcile-dev-with-main into dev

Dockerfile

@@ -20,6 +20,7 @@ USER thermograph
 WORKDIR /app
 
 ENV PORT=8080 \
+    WORKERS=1 \
     THERMOGRAPH_BASE=/ \
     PYTHONUNBUFFERED=1
 
@@ -28,4 +29,12 @@ EXPOSE 8080
 HEALTHCHECK --interval=30s --timeout=5s --start-period=40s --retries=3 \
     CMD curl -fsS http://127.0.0.1:${PORT}/healthz || exit 1
 
-CMD uvicorn app:app --host 0.0.0.0 --port ${PORT}
+# Worker count is env-driven (mirrors thermograph-backend's Dockerfile/
+# entrypoint WORKERS pattern) so infra can raise it later with no code
+# change. Default of 1 preserves today's exact behavior (no --workers was
+# ever passed before). api_client's TTL cache and register()'s IndexNow-key
+# handling are both in-process only -- multiple workers just each keep their
+# own independent cache, and the IndexNow key is fetched fresh FROM the
+# backend by every worker's own boot, so there's no cross-worker state to
+# diverge -- safe to raise whenever infra wants the throughput.
+CMD uvicorn app:app --host 0.0.0.0 --port ${PORT} --workers ${WORKERS:-1}

api_client.py

@@ -12,7 +12,9 @@ backend's own derived-store/ETag caching -- this cache saves the network hop
 entirely; the backend's saves the recompute.
 """
 import os
+import threading
 import time
+from collections import OrderedDict
 
 import httpx
 
@@ -35,14 +37,60 @@ API_VERSION = os.environ.get("THERMOGRAPH_API_VERSION", "v2")
 _BASE = os.environ.get("THERMOGRAPH_BASE", "/thermograph").strip("/")
 _BASE_PREFIX = f"/{_BASE}" if _BASE else ""
 
+# One pooled client shared by every call, instead of the old top-level
+# httpx.get() (a brand-new connection -- fresh TCP, no keep-alive -- per
+# request). Mirrors thermograph-backend's web/app.py _frontend_client.
+_client = httpx.Client(base_url=API_BASE, timeout=10.0,
+                        limits=httpx.Limits(max_connections=100, max_keepalive_connections=20))
+
 _TTL = float(os.environ.get("THERMOGRAPH_SSR_CACHE_TTL", "600") or 600)  # 10 min default
-_cache: dict[str, tuple[float, object]] = {}
+
+# Bounded LRU: city()/city_records() fold the browser-facing `origin` (client-
+# controlled via Host/X-Forwarded-Host, see content.py's _origin) into the
+# cache key, so an unbounded dict here is a cheap memory-exhaustion vector --
+# spoof enough distinct Host values and the cache grows forever. An
+# OrderedDict is a full LRU with no extra dependency: move_to_end() on every
+# hit/write, popitem(last=False) to evict the oldest when over the cap.
+_CACHE_MAX_ENTRIES = 2048
+_cache: "OrderedDict[str, tuple[float, object]]" = OrderedDict()
+_cache_lock = threading.Lock()
+
+# Per-key single-flight: without this, N concurrent requests that miss the
+# same cache key (cold start, or right after TTL expiry on a hot city) each
+# fire their own backend call -- a thundering herd. One lock per in-flight
+# key makes every caller but the first block on, then reuse, that first
+# caller's result instead of duplicating the request.
+_inflight: dict[str, threading.Lock] = {}
+_inflight_lock = threading.Lock()
+
+
+def _cache_get(key: str) -> object | None:
+    with _cache_lock:
+        hit = _cache.get(key)
+        if hit is None:
+            return None
+        ts, data = hit
+        if time.time() - ts >= _TTL:
+            del _cache[key]
+            return None
+        _cache.move_to_end(key)
+        return data
+
+
+def _cache_put(key: str, data: object) -> None:
+    with _cache_lock:
+        _cache[key] = (time.time(), data)
+        _cache.move_to_end(key)
+        while len(_cache) > _CACHE_MAX_ENTRIES:
+            _cache.popitem(last=False)  # evict least-recently-used
 
 
 def _get(path: str, *, origin: str | None = None) -> object:
     """GET {API_BASE}{path}, cached for _TTL seconds. Raises httpx.HTTPStatusError
     on a non-2xx response (including 404/503) so callers can map it the same
-    way the old in-process code raised HTTPException.
+    way the old in-process code raised HTTPException; raises httpx.TransportError
+    (ConnectError, TimeoutException, ...) when the backend can't be reached at
+    all -- callers map that to a 503 too (see content.py's _raise_api_error).
 
     `origin` (e.g. "https://thermograph.org") is the ORIGINAL browser-facing
     request's origin, not this internal call's -- forwarded as Host/
@@ -53,19 +101,32 @@ def _get(path: str, *, origin: str | None = None) -> object:
     default same-domain prod topology), and correct when there's more than
     one (the genuinely-cross-origin capability Stage 5 proves out)."""
     cache_key = f"{origin}|{path}" if origin else path
-    now = time.time()
-    hit = _cache.get(cache_key)
-    if hit and now - hit[0] < _TTL:
-        return hit[1]
+    hit = _cache_get(cache_key)
+    if hit is not None:
+        return hit
+
+    with _inflight_lock:
+        lock = _inflight.get(cache_key)
+        if lock is None:
+            lock = threading.Lock()
+            _inflight[cache_key] = lock
+    with lock:
+        hit = _cache_get(cache_key)  # someone else may have filled it while we waited
+        if hit is not None:
+            return hit
+        try:
             headers = {}
             if origin:
                 proto, _, host = origin.partition("://")
                 headers = {"host": host, "x-forwarded-proto": proto}
-    resp = httpx.get(f"{API_BASE}{_BASE_PREFIX}{path}", timeout=10.0, headers=headers)
+            resp = _client.get(f"{_BASE_PREFIX}{path}", headers=headers)
             resp.raise_for_status()
             data = resp.json()
-    _cache[cache_key] = (now, data)
+            _cache_put(cache_key, data)
             return data
+        finally:
+            with _inflight_lock:
+                _inflight.pop(cache_key, None)
 
 
 def hub() -> dict:

app.py

@@ -19,6 +19,22 @@ from fastapi.staticfiles import StaticFiles
 
 import content
 
+# Static assets aren't content-hashed in their filenames, so this has to stay
+# short -- it's a revalidation window, not a long-lived immutable cache. Still
+# saves a full body re-transfer (the mount's weak ETag otherwise forces a
+# conditional GET on every navigation) for anything fetched again within it.
+_STATIC_CACHE_CONTROL = "public, max-age=300"
+
+
+class _CachedStaticFiles(StaticFiles):
+    """StaticFiles with a Cache-Control header stamped onto every response,
+    including 304s -- see _STATIC_CACHE_CONTROL above."""
+
+    def file_response(self, *args, **kwargs) -> Response:
+        response = super().file_response(*args, **kwargs)
+        response.headers["Cache-Control"] = _STATIC_CACHE_CONTROL
+        return response
+
 # content's own `paths` (not a fresh `import paths` here) -- in the test
 # environment, conftest.py's sys.modules swap-then-restore trick means a
 # top-level `import paths` in THIS module, executed later than content.py's,
@@ -58,19 +74,40 @@ def _page(name):
     reached both directly (Caddy) and through backend's proxy fallback, and
     only that version prefers X-Forwarded-Host over Host (repo-split Stage 5)
     -- required for the proxied case to resolve the real browser-facing host
-    instead of this internal hop's own address."""
-    path = os.path.join(paths.STATIC_DIR, name)
+    instead of this internal hop's own address.
 
-    def route(request: Request):
+    The shell file itself (and the verification <meta> tags, also constant
+    for the process's lifetime) is read and prepped once, not on every
+    request -- only the __ORIGIN__ substitution actually varies per request,
+    and even that repeats across requests (one canonical origin in the
+    common topology), so the substituted HTML + its ETag are memoized per
+    origin instead of re-read-and-re-sha1'd every time."""
+    path = os.path.join(paths.STATIC_DIR, name)
+    _template: list[str] = []  # lazy singleton cell, populated on first request
+    _by_origin: dict[str, tuple[str, str]] = {}  # origin_prefix -> (html, etag)
+
+    def _load_template() -> str:
+        if not _template:
             with open(path, encoding="utf-8") as f:
                 html = f.read()
-        html = html.replace("__ORIGIN__", f"{content._origin(request)}{content.BASE}")
-        # Search-engine verification <meta> tags (same source as the SSR content
-        # pages), so the interactive tool's own pages carry them too.
+            # Search-engine verification <meta> tags (same source as the SSR
+            # content pages), so the interactive tool's own pages carry them
+            # too. Also constant for the process's life -- folded in here.
             verify = content.head_verify_html()
             if verify:
                 html = html.replace("<head>", f"<head>\n  {verify}", 1)
+            _template.append(html)
+        return _template[0]
+
+    def route(request: Request):
+        origin_prefix = f"{content._origin(request)}{content.BASE}"
+        cached = _by_origin.get(origin_prefix)
+        if cached is None:
+            html = _load_template().replace("__ORIGIN__", origin_prefix)
             etag = f'W/"{hashlib.sha1(html.encode()).hexdigest()[:20]}"'
+            cached = (html, etag)
+            _by_origin[origin_prefix] = cached
+        html, etag = cached
         if _not_modified(request, etag):
             return Response(status_code=304, headers={"ETag": etag})
         return Response(html, media_type="text/html", headers={"ETag": etag})
@@ -91,4 +128,4 @@ app.add_api_route(f"{content.BASE}/alerts", _page("subscriptions.html"), methods
 # is a static asset. Registered last so the explicit routes above win. Mounts
 # must start with "/", so at the root (BASE == "") mount at "/" -- the
 # earlier routes still win because Starlette matches in registration order.
-app.mount(content.BASE or "/", StaticFiles(directory=paths.STATIC_DIR), name="static")
+app.mount(content.BASE or "/", _CachedStaticFiles(directory=paths.STATIC_DIR), name="static")

content.py

@@ -41,6 +41,10 @@ _env = Environment(
     autoescape=select_autoescape(["html", "xml", "j2"]),
     trim_blocks=True,
     lstrip_blocks=True,
+    # Templates only change on a redeploy (a fresh process), not while this
+    # one is running -- the implicit default (True) stats every template file
+    # on every single render just to notice a change that can never happen.
+    auto_reload=False,
 )
 
 _env.globals["temp_class"] = fmt.temp_class
@@ -89,10 +93,15 @@ def _breadcrumb_tuples(breadcrumb: list[dict]) -> list[tuple]:
     return [(c["name"], c["href"]) for c in breadcrumb]
 
 
-def _raise_api_error(e: httpx.HTTPStatusError):
+def _raise_api_error(e: httpx.HTTPStatusError | httpx.TransportError):
     """The backend's 404 (unknown city/month) and 503 (warming) map straight
-    through -- same status codes _resolve_city raised in-process before."""
+    through -- same status codes _resolve_city raised in-process before. A
+    TransportError (ConnectError, TimeoutException, ...) never got a response
+    at all -- a backend blip or restart -- so it becomes a 503 too, rather
+    than escaping this handler and surfacing as a raw framework 500."""
+    if isinstance(e, httpx.HTTPStatusError):
         raise HTTPException(status_code=e.response.status_code, detail=e.response.text) from e
+    raise HTTPException(status_code=503, detail="backend unavailable") from e
 
 
 # --- robots.txt & sitemap.xml ---------------------------------------------
@@ -115,7 +124,10 @@ _BOOT_DATE = datetime.date.today().isoformat()
 
 def sitemap_xml(request: Request) -> Response:
     base_url = f"{_origin(request)}{BASE}"
+    try:
         entries = api_client.sitemap()
+    except (httpx.HTTPStatusError, httpx.TransportError) as e:
+        _raise_api_error(e)
     parts = ['<?xml version="1.0" encoding="UTF-8"?>',
              '<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">']
     for e in entries:
@@ -124,7 +136,11 @@ def sitemap_xml(request: Request) -> Response:
             f"<changefreq>{e['changefreq']}</changefreq><priority>{e['priority']}</priority></url>"
         )
     parts.append("</urlset>")
-    return Response("\n".join(parts), media_type="application/xml")
+    # 2.35 MB and crawled repeatedly (~0.5-1s per hit) -- a short cache saves
+    # every crawler hit within the window a full api_client.sitemap() call
+    # plus the string build above, on top of the response body itself.
+    return Response("\n".join(parts), media_type="application/xml",
+                     headers={"Cache-Control": "public, max-age=300"})
 
 
 def head_verify_html() -> Markup:
@@ -159,7 +175,7 @@ def _fmt_months(months: list[dict]) -> list[dict]:
 def city_page(request: Request, slug: str) -> Response:
     try:
         j = api_client.city(slug, origin=_origin(request))
-    except httpx.HTTPStatusError as e:
+    except (httpx.HTTPStatusError, httpx.TransportError) as e:
         _raise_api_error(e)
 
     with fmt.unit_scope(j["default_unit"]):
@@ -203,7 +219,7 @@ def month_page(request: Request, slug: str, month: str) -> Response:
     try:
         j = api_client.city_month(slug, month)
         city = api_client.city(slug)["city"]
-    except httpx.HTTPStatusError as e:
+    except (httpx.HTTPStatusError, httpx.TransportError) as e:
         _raise_api_error(e)
 
     with fmt.unit_scope(fmt.unit_for_country(city.get("country_code"))):
@@ -261,7 +277,7 @@ def records_page(request: Request, slug: str) -> Response:
     try:
         j = api_client.city_records(slug, origin=_origin(request))
         city = api_client.city(slug)["city"]
-    except httpx.HTTPStatusError as e:
+    except (httpx.HTTPStatusError, httpx.TransportError) as e:
         _raise_api_error(e)
 
     with fmt.unit_scope(fmt.unit_for_country(city.get("country_code"))):
@@ -302,7 +318,10 @@ def records_page(request: Request, slug: str) -> Response:
 
 # --- hub / glossary / about -------------------------------------------------
 def hub_page(request: Request) -> Response:
+    try:
         h = api_client.hub()
+    except (httpx.HTTPStatusError, httpx.TransportError) as e:
+        _raise_api_error(e)
     groups = {c["country"]: c["cities"] for c in h["countries"]}
     ctx = {
         "section": "climate",
@@ -386,7 +405,10 @@ _HOME_JSONLD = {
 
 
 def home_page(request: Request) -> Response:
+    try:
         h = api_client.home()
+    except (httpx.HTTPStatusError, httpx.TransportError) as e:
+        _raise_api_error(e)
     ctx = {
         "section": "home",
         "brand_tag": "p",

static/account.js

@@ -219,6 +219,14 @@ const BELL_IC = `<svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stro
 let notifications = [];
 let unreadCount = 0;
 let notifTimer = null;
+let notifPolling = false;
+
+// Backoff: a sustained outage shouldn't keep hitting the backend every 2 min
+// forever. Doubles on each consecutive failure, capped, resets on success.
+const NOTIF_POLL_BASE_MS = 120000;   // refresh unread every 2 min when healthy
+const NOTIF_POLL_MAX_MS = 960000;    // cap at 16 min
+let notifPollDelay = NOTIF_POLL_BASE_MS;
+let notifFailStreak = 0;
 
 function timeAgo(sec) {
   const d = Date.now() / 1000 - sec;
@@ -231,13 +239,19 @@ function timeAgo(sec) {
 async function loadNotifications() {
   try {
     const res = await apiFetch(uv("notifications?limit=50"));
-    if (!res.ok) return;
+    if (!res.ok) throw new Error(`HTTP ${res.status}`);
     const data = await res.json();
     notifications = data.notifications || [];
     unreadCount = data.unread_count || 0;
     paintBadge();
     paintNotifList();
-  } catch (e) { /* offline — leave prior state */ }
+    notifFailStreak = 0;
+    notifPollDelay = NOTIF_POLL_BASE_MS;
+  } catch (e) {
+    // offline / backend blip — leave prior state, back off the poll interval
+    notifFailStreak++;
+    notifPollDelay = Math.min(NOTIF_POLL_BASE_MS * 2 ** notifFailStreak, NOTIF_POLL_MAX_MS);
+  }
 }
 
 function paintBadge() {
@@ -290,13 +304,23 @@ async function markAllRead() {
 }
 
 function startNotifPolling() {
-  loadNotifications();
-  if (notifTimer) return;
-  notifTimer = setInterval(loadNotifications, 120000);   // refresh unread every 2 min
+  if (notifPolling) return;
+  notifPolling = true;
+  // Recursive setTimeout, not setInterval -- the delay is variable (backoff),
+  // so the next tick has to be scheduled after each fetch settles rather than
+  // fixed up front.
+  const tick = async () => {
+    await loadNotifications();
+    if (!notifPolling) return;   // stopped while the fetch was in flight
+    notifTimer = setTimeout(tick, notifPollDelay);
+  };
+  tick();
 }
 function stopNotifPolling() {
-  if (notifTimer) { clearInterval(notifTimer); notifTimer = null; }
+  notifPolling = false;
+  if (notifTimer) { clearTimeout(notifTimer); notifTimer = null; }
   notifications = []; unreadCount = 0;
+  notifFailStreak = 0; notifPollDelay = NOTIF_POLL_BASE_MS;
 }
 
 // --- header entry ------------------------------------------------------------

tests/unit/test_api_client.py

@@ -0,0 +1,92 @@
+"""Unit tests for api_client.py's own plumbing (cache eviction, single-flight)
+-- as opposed to test_content.py, which exercises it indirectly through the
+rendered pages via the `client` fixture's monkeypatched api_client functions.
+"""
+import threading
+import time
+import types
+
+import api_client
+
+
+def setup_function(_):
+    api_client._cache.clear()
+    api_client._inflight.clear()
+
+
+def test_cache_eviction_is_lru(monkeypatch):
+    monkeypatch.setattr(api_client, "_CACHE_MAX_ENTRIES", 3)
+    for i in range(3):
+        api_client._cache_put(f"k{i}", i)
+    # Touch k0 so it's most-recently-used; k1 is now the least-recently-used
+    # entry and should be the one evicted when the cap is exceeded.
+    api_client._cache_get("k0")
+    api_client._cache_put("k3", 3)
+
+    assert api_client._cache_get("k1") is None
+    assert api_client._cache_get("k0") == 0
+    assert api_client._cache_get("k2") == 2
+    assert api_client._cache_get("k3") == 3
+    assert len(api_client._cache) == 3
+
+
+def test_cache_entries_expire_after_ttl(monkeypatch):
+    monkeypatch.setattr(api_client, "_TTL", 0.01)
+    api_client._cache_put("k", "v")
+    assert api_client._cache_get("k") == "v"
+    time.sleep(0.02)
+    assert api_client._cache_get("k") is None
+
+
+def test_single_flight_dedupes_concurrent_misses(monkeypatch):
+    """N concurrent misses on the same key must reach the backend once, not
+    N times -- every caller gets the same result either way."""
+    calls = []
+    release = threading.Event()
+
+    class _FakeResp:
+        def raise_for_status(self):
+            pass
+
+        def json(self):
+            return {"ok": True}
+
+    def fake_get(path, headers=None):
+        calls.append(path)
+        release.wait(timeout=2)
+        return _FakeResp()
+
+    monkeypatch.setattr(api_client, "_client", types.SimpleNamespace(get=fake_get))
+
+    results = []
+    results_lock = threading.Lock()
+
+    def worker():
+        r = api_client._get("/thing")
+        with results_lock:
+            results.append(r)
+
+    threads = [threading.Thread(target=worker) for _ in range(5)]
+    for t in threads:
+        t.start()
+    time.sleep(0.1)  # let every thread reach (and block behind) the first fetch
+    release.set()
+    for t in threads:
+        t.join(timeout=2)
+
+    assert len(calls) == 1  # exactly one backend call for 5 concurrent misses
+    assert results == [{"ok": True}] * 5
+    assert api_client._cache_get("/thing") == {"ok": True}
+
+
+def test_single_flight_clears_inflight_entry_after_completion(monkeypatch):
+    class _FakeResp:
+        def raise_for_status(self):
+            pass
+
+        def json(self):
+            return {"n": 1}
+
+    monkeypatch.setattr(api_client, "_client", types.SimpleNamespace(get=lambda path, headers=None: _FakeResp()))
+    api_client._get("/thing")
+    assert "/thing" not in api_client._inflight

tests/unit/test_rendering.py

@@ -66,3 +66,29 @@ def test_unknown_city_is_404(client):
 
 def test_unknown_month_is_404(client):
     assert client.get(f"{B}/climate/{SLUG}/nonemonth").status_code == 404
+
+
+# --- backend-down resilience (migrated from main's hardening of the old
+# tests/test_content.py, which the unit-tier restructure deleted) ------------
+def _unreachable(*_a, **_k):
+    import httpx
+    req = httpx.Request("GET", "http://backend.invalid/x")
+    raise httpx.ConnectError("connection refused", request=req)
+
+
+@pytest.mark.parametrize("path, attr", [
+    (f"/climate/{SLUG}", "city"),
+    (f"/climate/{SLUG}/july", "city_month"),
+    (f"/climate/{SLUG}/records", "city_records"),
+    ("/climate", "hub"),
+    ("/", "home"),
+    ("/sitemap.xml", "sitemap"),
+])
+def test_backend_unreachable_maps_to_503(client, monkeypatch, path, attr):
+    """A backend that's down/timing out (httpx.ConnectError, TimeoutException,
+    ...) never raises httpx.HTTPStatusError -- it doesn't even get a response
+    -- so it must be caught separately from the 404/503 status-code passthrough
+    or it escapes as a raw framework 500 instead of a clean 503."""
+    import api_client
+    monkeypatch.setattr(api_client, attr, _unreachable)
+    assert client.get(f"{B}{path}").status_code == 503