From 5a921cf01dbcc947600bcb502737f77d547e276a Mon Sep 17 00:00:00 2001 From: Emi Griffith Date: Sun, 19 Jul 2026 11:25:55 -0700 Subject: [PATCH] Default climate pages to the city's own temperature unit (#189) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Search Console shows impressions skewing hard to °C countries — India, South Africa, New Zealand, the UK, Australia — while every climate page rendered °F into the HTML. Search snippets quote the server-rendered text, so a reader in Delhi saw a temperature they had to convert in their head before the page was worth a click. Climate pages now render temperatures in the city's country convention: °F for the US and the handful of other Fahrenheit countries, °C everywhere else. This happens server-side, so it holds with no JS and shows up in the snippet. data-temp-f keeps the raw Fahrenheit in every case — it stays the conversion source of truth, and climate.js repaints from it on toggle exactly as before. The visitor's stored choice still wins over everything; the page default only sits between that and the locale guess, so a first-time visitor stops seeing a °F→°C repaint on load for the majority of our traffic. The unit rides on a ContextVar rather than a parameter because _temp() is reached from both directions — templates call it as a Jinja global, and the context builders call it directly to pre-render the records tables into strings. Threading an argument would mean touching ~20 call sites across three levels of nesting, where missing one yields a silently wrong unit instead of an error. The scope wraps the context builder as well as the render, since the builder is evaluated as an argument and runs first, and it resets in a finally: the page handlers are sync `def`, so Starlette runs them on a recycled threadpool thread and a value left set would leak into the next request that thread serves. Conversion rounds half-up to match JS's Math.round rather than Python's round-half-to-even, so the server prints the number climate.js would and there is no flicker on load for a visitor whose unit already matched. Pages with no single city — the homepage strip, the hub, the interactive views — set no scope, emit no data-unit-default, and keep today's locale behaviour. Precipitation, wind and humidity stay imperial; that inconsistency is worth a follow-up but is outside this change. Two existing assertions were passing for the wrong reason: base.html.j2 mentions "°F/°C" in an HTML comment, so a bare `"°F" in html` check succeeded even on a page whose every temperature was Celsius. They now read the rendered spans. Claude-Session: https://claude.ai/code/session_013dRZmX9D3JEntfMKWMTWZ8 --- content.py | 100 +++++++++++++++++++++++++++++---- templates/base.html.j2 | 2 +- tests/test_content.py | 125 ++++++++++++++++++++++++++++++++++++----- 3 files changed, 202 insertions(+), 25 deletions(-) diff --git a/content.py b/content.py index 2c049ca..fc31e2d 100644 --- a/content.py +++ b/content.py @@ -5,10 +5,13 @@ These are the SEO surface: real URLs with the climate stats in the HTML, rendere with Jinja2 from the same builders the API uses, linking into the interactive tool. Registered on the app (via register()) BEFORE the StaticFiles mount so they win. """ +import contextlib +import contextvars import datetime import functools import hashlib import json +import math import os import polars as pl @@ -60,18 +63,78 @@ METRIC_LABELS = [ _TEMP_METRICS = {"tmax", "tmin", "feels"} +# Countries that actually use Fahrenheit. Mirrors F_REGIONS in frontend/units.js — +# the client applies it to the visitor's locale, we apply it to the city's country. +# A test asserts the two lists stay identical. +F_COUNTRIES = frozenset({"US", "PR", "GU", "VI", "AS", "MP", "UM", + "BS", "BZ", "KY", "PW", "FM", "MH", "LR"}) + +# The unit the current page renders temperatures in. None = °F, and also means +# "this page has no city", which is what keeps the interactive pages on the +# client-side locale default instead of being pinned server-side. +# +# A ContextVar rather than a parameter because _temp() is reached from both sides: +# templates call it as a Jinja global, and the context builders call it directly +# (the records tables are pre-rendered into strings in Python). Threading a `unit` +# argument would mean touching ~20 call sites across three levels of nesting, where +# forgetting one yields a silently wrong unit rather than an error. +_UNIT: contextvars.ContextVar[str | None] = contextvars.ContextVar("display_unit", default=None) + + +@contextlib.contextmanager +def _unit_scope(unit: str | None): + """Render temperatures in `unit` for the duration of the block. + + The reset is not optional: the page handlers are sync `def`, so Starlette runs + them on a recycled threadpool thread, and a value left set would leak into + whatever request that thread serves next — including pages with no city. + """ + token = _UNIT.set(unit) + try: + yield + finally: + _UNIT.reset(token) + + +def unit_for_country(code: str | None) -> str: + """The temperature unit a reader in this country expects.""" + return "F" if (code or "").upper() in F_COUNTRIES else "C" + + +def _round_half_up(x: float) -> int: + """Round like JS's Math.round, not like Python's round(). + + Python rounds halves to even, JS rounds them up. Where they disagree the + server would render one number and climate.js would repaint a different one — + a visible flicker for exactly the visitor whose unit already matched. + """ + return math.floor(x + 0.5) + + def _c(f: float) -> int: - return round((f - 32) * 5 / 9) + return _round_half_up((f - 32) * 5 / 9) + + +def _shown(f: float) -> int: + """The number to print, in the active unit.""" + return _c(f) if _UNIT.get() == "C" else _round_half_up(f) + + +def _letter() -> str: + return "C" if _UNIT.get() == "C" else "F" def _temp(f): """A Fahrenheit temperature as a client-convertible span, e.g. - '72°F'. Renders °F by default - (so crawlers and no-JS visitors see a real value); climate.js rewrites it to - the active unit on load and on toggle. Returns '—' for a missing value.""" + '72°F'. The text is rendered in + the page's unit (the city's country convention — see _unit_scope), so crawlers + and no-JS visitors get the local convention rather than a conversion they have + to do themselves; climate.js repaints on toggle. data-temp-f stays Fahrenheit + either way — it is the conversion source of truth. '—' for a missing value.""" if f is None: return "—" - return Markup('{}°F').format(f, round(f)) + return Markup('{}°{}').format( + f, _shown(f), _letter()) def _temp_bare(f): @@ -79,16 +142,18 @@ def _temp_bare(f): axis already implies the unit. Still carries data-temp-f so it converts.""" if f is None: return "—" - return Markup('{}°').format(f, round(f)) + return Markup('{}°').format( + f, _shown(f)) def _temp_text(f) -> str: """'72°F' as plain text. The span _temp() returns can't go in a , and a meta description is the one place a temperature is - never converted client-side — it is what the search snippet quotes.""" + never converted client-side — it is what the search snippet quotes, which is + why it has to be in the city's own unit at render time.""" if f is None: return "—" - return f"{round(f)}°F" + return f"{_shown(f)}°{_letter()}" def _month_year(date_s: str) -> str: @@ -203,6 +268,10 @@ def _breadcrumb_jsonld(o: str, breadcrumb: list[tuple[str, str | None]]) -> dict def _respond_html(request: Request, template: str, **ctx) -> Response: o = origin(request) + # Taken from the same ContextVar the numbers were rendered through, so the + # attribute units.js reads can't drift from what the page actually says. + # None on any page without a city -> base.html.j2 omits it entirely. + ctx.setdefault("unit_default", _UNIT.get()) html = _env.get_template(template).render(base=BASE, origin=o, base_url=f"{o}{BASE}", **ctx) etag = f'W/"{hashlib.sha1(html.encode()).hexdigest()[:20]}"' inm = request.headers.get("if-none-match") @@ -484,7 +553,12 @@ def _resolve_city(slug: str): def city_page(request: Request, slug: str) -> Response: city, cell, history = _resolve_city(slug) - return _respond_html(request, "city.html.j2", **_city_context(request, city, cell, history)) + # The scope has to cover the context builder too, not just the render: it is + # evaluated as an argument, so it runs first — and it is where most of the + # page's temperatures are formatted. + with _unit_scope(unit_for_country(city.get("country_code"))): + return _respond_html(request, "city.html.j2", + **_city_context(request, city, cell, history)) # --- month & records pages ---------------------------------------------------- @@ -576,7 +650,9 @@ def month_page(request: Request, slug: str, month: str) -> Response: if month not in MONTH_INDEX: raise HTTPException(status_code=404, detail="Unknown month.") city, cell, history = _resolve_city(slug) - return _respond_html(request, "month.html.j2", **_month_context(request, city, history, MONTH_INDEX[month])) + with _unit_scope(unit_for_country(city.get("country_code"))): + return _respond_html(request, "month.html.j2", + **_month_context(request, city, history, MONTH_INDEX[month])) def _records_context(request, city, history) -> dict: @@ -660,7 +736,9 @@ def _records_context(request, city, history) -> dict: def records_page(request: Request, slug: str) -> Response: city, cell, history = _resolve_city(slug) - return _respond_html(request, "records.html.j2", **_records_context(request, city, history)) + with _unit_scope(unit_for_country(city.get("country_code"))): + return _respond_html(request, "records.html.j2", + **_records_context(request, city, history)) # --- hub / glossary / about --------------------------------------------------- diff --git a/templates/base.html.j2 b/templates/base.html.j2 index 53da82b..229adbc 100644 --- a/templates/base.html.j2 +++ b/templates/base.html.j2 @@ -28,7 +28,7 @@ {% block jsonld %}{% endblock %} - +
diff --git a/tests/test_content.py b/tests/test_content.py index e24d563..9311328 100644 --- a/tests/test_content.py +++ b/tests/test_content.py @@ -1,6 +1,7 @@ """Tests for the crawlable SEO content pages: the city set, robots/sitemap, and that the server-rendered pages contain the stats in the HTML (with the weather layer faked, like test_api.py).""" +import math import re import pytest @@ -55,8 +56,8 @@ def test_city_page_renders_stats_in_html(client): assert 'rel="canonical"' in b and f"/climate/{SLUG}" in b assert '"@type":"Dataset"' in b assert "average temperatures by month" in b.lower() - # a real number rendered (°F appears in the normals/today text) - assert "°F" in b + # a real number rendered in the normals/today text (London is GB, so °C) + assert re.search(r"-?\d+°C", b) def _jsonld_breadcrumbs(html: str) -> list[dict]: @@ -150,10 +151,11 @@ def test_records_page_has_monthly_and_seasonal(client): assert "Northern Hemisphere" in b assert 'Winter (Dec–Feb)' in b assert 'Summer (Jun–Aug)' in b - # All-time table still present, plus structured data and real °F values. + # All-time table still present, plus structured data and real values. assert "All-time records" in b - assert '"@type":"Dataset"' in b and "°F" in b - # Title/description advertise the new coverage. + assert '"@type":"Dataset"' in b + assert re.search(r"-?\d+°C", b) + # The Dataset structured data still advertises the month/season coverage. assert "monthly" in b.lower() and "seasonal" in b.lower() @@ -244,8 +246,10 @@ def test_meta_descriptions_lead_with_real_numbers(client): b = client.get(path).text desc = re.search(r']*)>(-?\d+)°([CF]?)') + + +def _temp_spans(html): + """(fahrenheit_attr, shown_number, shown_letter) for every rendered temp span. + + Reading the spans rather than the raw document matters: base.html.j2 mentions + "°F/°C" in an HTML comment, so a bare `"°F" in html` assertion passes on a page + whose every temperature is Celsius. + """ + return [(float(f), int(n), letter) for f, _attrs, n, letter in _TEMP_SPAN.findall(html)] + + def test_climate_pages_carry_convertible_temps(client): - # Every temperature is a client-convertible span (default °F text so crawlers - # and no-JS visitors still see a real value); the old inline "(NN°C)" dual form - # is gone (the toggle now supplies Celsius). - import re + # Every temperature is a client-convertible span; the old inline "(NN°C)" dual + # form is gone (the toggle now supplies the other unit). for path in (f"{B}/climate/{SLUG}", f"{B}/climate/{SLUG}/july", f"{B}/climate/{SLUG}/records"): b = client.get(path).text - assert 'class="temp" data-temp-f=' in b - assert "°F" in b + spans = _temp_spans(b) + assert spans, f"no temperature spans on {path}" assert re.search(r"\d+°F \(-?\d+°C\)", b) is None # no leftover dual-unit strings +def test_units_default_to_the_citys_country(client): + # The search snippet shows whatever the server rendered, and impressions skew + # to °C countries — so a UK city page must say °C in the HTML itself, with no + # JS and no stored preference. data-temp-f stays Fahrenheit either way: it is + # what climate.js converts from. + gb = client.get(f"{B}/climate/{SLUG}").text # London, GB + us = client.get(f"{B}/climate/seattle-washington-us").text + + gb_spans, us_spans = _temp_spans(gb), _temp_spans(us) + assert gb_spans and us_spans + assert {letter for _f, _n, letter in gb_spans if letter} == {"C"} + assert {letter for _f, _n, letter in us_spans if letter} == {"F"} + + # The attribute is still Fahrenheit, and really does differ from the Celsius + # text — this is what breaks if someone "helpfully" converts the attribute too. + assert any(math.floor(f + 0.5) != n for f, n, _l in gb_spans), "data-temp-f was converted" + # On a °F page the attribute and the text agree by definition. Rounded half-up, + # not with Python's round(): the page renders the number JS would. + assert all(math.floor(f + 0.5) == n for f, n, _l in us_spans) + + assert 'data-unit-default="C"' in gb + assert 'data-unit-default="F"' in us + + +def test_unit_default_absent_without_a_city(client): + # No city -> no attribute -> units.js keeps today's locale behaviour. Pinning + # these to "F" would silently regress the interactive pages. + for path in (f"{B}/", f"{B}/climate", f"{B}/about", f"{B}/glossary"): + assert "data-unit-default" not in client.get(path).text, path + + +def test_unit_does_not_leak_between_requests(client): + # The page handlers are sync `def`, so Starlette runs them on a recycled + # threadpool thread: a unit left set would bleed into the next request served + # by that thread. Interleave and re-check rather than trusting the reset. + assert 'data-unit-default="C"' in client.get(f"{B}/climate/{SLUG}").text + about = client.get(f"{B}/about").text + assert "data-unit-default" not in about + # about.html.j2 renders an illustrative temperature; it must stay °F. + assert {l for _f, _n, l in _temp_spans(about) if l} == {"F"} + + us = client.get(f"{B}/climate/seattle-washington-us").text + assert {l for _f, _n, l in _temp_spans(us) if l} == {"F"} + gb_again = client.get(f"{B}/climate/{SLUG}").text + assert {l for _f, _n, l in _temp_spans(gb_again) if l} == {"C"} + + +def test_f_country_list_matches_the_frontend(): + # Two hand-maintained copies of the same 14 codes; nothing else stops them + # drifting apart, and drift means the server and the client disagree about + # what a first-time visitor should see. + import os + import content + units_js = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), + os.pardir, "frontend", "units.js") + with open(units_js, encoding="utf-8") as f: + src = f.read() + literal = re.search(r"F_REGIONS = new Set\(\[(.*?)\]\)", src, re.S).group(1) + assert set(re.findall(r'"([A-Z]{2})"', literal)) == set(content.F_COUNTRIES) + + +def test_celsius_rounding_matches_js(): + # Python's round() goes to even, JS's Math.round goes up. Where they disagree + # the server prints one number and climate.js repaints another — a flicker for + # the visitor whose unit already matched. + import content + assert content._round_half_up(0.5) == 1 # round(0.5) would give 0 + assert content._round_half_up(1.5) == 2 + assert content._round_half_up(2.5) == 3 # round(2.5) would give 2 + assert content._round_half_up(-0.5) == 0 # Math.round(-0.5) === 0 + # 31.1°F -> -0.5°C exactly: the case that actually shows up on a cold page. + assert content._c(31.1) == 0 + + +def test_city_page_etag_is_stable(client): + # The unit comes from the URL's city, never from a request header, so the + # response stays a pure function of the URL — no Vary, and the ETag still works. + r1 = client.get(f"{B}/climate/{SLUG}") + r2 = client.get(f"{B}/climate/{SLUG}") + assert r1.headers["etag"] == r2.headers["etag"] + r3 = client.get(f"{B}/climate/{SLUG}", headers={"If-None-Match": r1.headers["etag"]}) + assert r3.status_code == 304 + + def test_seo_pages_load_unit_and_account_scripts(client): # Header parity with the interactive pages: the °F/°C toggle, account/bell, and # the temperature converter are all loaded.