From b2b56bdc1a8337a48d221886d9bd7f3dc43022d6 Mon Sep 17 00:00:00 2001 From: Emi Griffith Date: Sat, 25 Jul 2026 11:42:16 -0700 Subject: [PATCH] frontend: fix the three inconsistencies the onboarding guide found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. CLAUDE.md and README.md described the superseded Python service. Both now describe server/ (Go), say plainly that the Python files at that level are the original the port was made from, and drop CLAUDE.md's claim that `make test-unit` is "the tier CI runs" — CI's only frontend check is the Dockerfile builder stage's gofmt + vet + go test. 2. static/units.js's F_REGIONS was guarded by nothing, despite three source comments claiming "a test asserts all three stay identical": the only check compared the Go set against the backend's Python. TestFCountriesMatchesUnitsJS now diffs the browser copy both directions. That backend cross-check also skips in CI — the image build context is frontend/, so backend/ is unreachable from the builder stage, which is the only place CI runs these tests. static/ IS in the context, so the Dockerfile copies units.js into the builder and the new assertion runs during the image build. Verified by mutating units.js and confirming the build fails. 3. Both docker-compose.test.yml files defaulted to the retired emi/thermograph-backend/app path, and the frontend harness pinned the split-era v0.0.2-split-ci tag. Path corrected in both. Rather than swap one hardcoded pin for another, backend-for-tests.sh now derives the tag from the checkout — sha-<12hex of `git log -1 -- backend/`>, the same domain-keyed rule build-push.yml and deploy.yml use — and compose requires the variable so a stale pin cannot creep back in. Verified: backend 429 passed/8 skipped; frontend go vet clean and all packages ok; frontend image builds; `make backend-up` pulls and serves on the derived tag; shellcheck zero findings across the tree. Unrelated pre-existing issue noted in the docs, not fixed here: `make test-integration` fails 7/16 with 503 against a cold throwaway backend (empty database, nothing warm). Reproduced identically on the old image, so it predates this change. Claude-Session: https://claude.ai/code/session_01AfXqHrxCJLs2D7hpQkiUiJ --- backend/docker-compose.test.yml | 6 +- docs/onboarding/02-setup.md | 10 + docs/onboarding/05-frontend.md | 10 +- docs/onboarding/06-contracts.md | 25 ++- docs/onboarding/11-traps.md | 103 +++++----- frontend/CLAUDE.md | 108 ++++++++--- frontend/Dockerfile | 13 ++ frontend/README.md | 182 +++++++++--------- frontend/docker-compose.test.yml | 13 +- frontend/scripts/backend-for-tests.sh | 38 +++- .../server/internal/format/format_test.go | 83 ++++++-- 11 files changed, 393 insertions(+), 198 deletions(-) diff --git a/backend/docker-compose.test.yml b/backend/docker-compose.test.yml index 2b1addb..d5ff99e 100644 --- a/backend/docker-compose.test.yml +++ b/backend/docker-compose.test.yml @@ -22,7 +22,11 @@ services: backend: # Defaults to a locally-built `:local` image (scripts/smoke.sh builds it). In CI, # set BACKEND_IMAGE_TAG=sha-<12hex> to smoke the exact published image. - image: ${REGISTRY_HOST:-git.thermograph.org}/${BACKEND_IMAGE_PATH:-emi/thermograph-backend/app}:${BACKEND_IMAGE_TAG:-local} + # The path must match what build-push.yml publishes -- emi/thermograph/backend. + # It read emi/thermograph-backend/app (the retired split-era path) until this + # was corrected; harmless for the default `:local` build, wrong the moment + # BACKEND_IMAGE_TAG names a real published tag. + image: ${REGISTRY_HOST:-git.thermograph.org}/${BACKEND_IMAGE_PATH:-emi/thermograph/backend}:${BACKEND_IMAGE_TAG:-local} depends_on: db: condition: service_healthy diff --git a/docs/onboarding/02-setup.md b/docs/onboarding/02-setup.md index ba79e75..52aa099 100644 --- a/docs/onboarding/02-setup.md +++ b/docs/onboarding/02-setup.md @@ -139,9 +139,19 @@ make backend-up # pulls + runs the published backend image + throwaway db make backend-down ``` +The image tag is **derived from your checkout** — `sha-<12hex of git log -1 -- +backend/>`, the same domain-keyed rule `build-push.yml` and `deploy.yml` use — +so the harness follows the tree. If those backend commits are still local-only, +no image exists yet and the script says so; pin a published build with +`THERMOGRAPH_BACKEND_TEST_TAG=sha-<12hex>`. + `make test-integration` runs the Python integration tier against that. CI does **not** run it — it needs a live backend container. +> Heads-up: against a freshly-booted throwaway backend, that tier currently +> fails 7 of 16 with `503` — the database is empty, so nothing is warm. +> Pre-existing, and unrelated to which image tag you use. + ## The daemon ```bash diff --git a/docs/onboarding/05-frontend.md b/docs/onboarding/05-frontend.md index 8241b54..c1dd17e 100644 --- a/docs/onboarding/05-frontend.md +++ b/docs/onboarding/05-frontend.md @@ -8,11 +8,11 @@ HTTP. > **The live service is Go.** `frontend/server/` builds, tests, ships and runs. > The Python files one directory up (`app.py`, `content.py`, `api_client.py`, > `format.py`, `content_loader.py`, `paths.py`, `templates/*.html.j2`) are the -> superseded original implementation, kept in-tree as the reference the port -> was made from. `frontend/CLAUDE.md` and `frontend/README.md` still describe -> the Python service — see [traps](11-traps.md). Everything in those docs about -> **contracts, env vars and design** is still correct; only "which language and -> which file" changed. +> superseded original implementation, kept in-tree as the reference the port was +> made from. Nothing deploys them, and they hold duplicate copies of a couple of +> contracts (`api_client.py`'s `API_VERSION` pin, `format.py`'s `F_COUNTRIES`) — +> keep those in step if you run them locally. Whether to delete them is an open +> question; raise it rather than doing it in passing. ## Shape of the Go service diff --git a/docs/onboarding/06-contracts.md b/docs/onboarding/06-contracts.md index dad33a4..a0020b3 100644 --- a/docs/onboarding/06-contracts.md +++ b/docs/onboarding/06-contracts.md @@ -104,16 +104,23 @@ Fourteen codes: `US PR GU VI AS MP UM BS BZ KY PW FM MH LR`. | browser | `frontend/static/units.js::F_REGIONS` | | Python SSR *(not deployed)* | `frontend/format.py::F_COUNTRIES` | -`format_test.go::TestFCountriesMatchesBackend` re-parses the backend's Python -source and asserts the Go copy matches — a genuinely good guard. +Two tests in `frontend/server/internal/format/format_test.go` guard this: -> ⚠️ **Verified gap.** Several source comments claim "a test asserts all three -> stay identical." As of this writing that is not true: no test reads -> `static/units.js`. All four copies currently *are* identical (checked), but -> the browser copy is held in step by convention alone. If you touch this set, -> update `units.js` by hand and diff it yourself. Extending the Go test to -> parse `units.js` too would close this properly and is a good first -> contribution. +- `TestFCountriesMatchesBackend` re-parses the backend's Python and asserts the + Go copy matches. **This one skips in CI** — the frontend image's build context + is `frontend/`, so `backend/` is unreachable from the builder stage, and the + builder stage is the only place CI runs these tests. Treat it as a + checkout-only guard: it fires on your machine and in a full-checkout run, not + during the image build. +- `TestFCountriesMatchesUnitsJS` parses `static/units.js` and diffs both + directions. This one *does* run in the image build — `frontend/Dockerfile` + copies that single file into the builder stage for exactly that reason — so a + divergence fails the build. + +The browser copy was unguarded until that second test was added; several source +comments had long claimed "a test asserts all three stay identical" when only +the Go↔backend pair was checked. The Python `frontend/format.py` copy remains +guarded only by the local Python tier, which is fine — it isn't deployed. ## 6. Backend boot must survive an unreachable frontend, and vice versa diff --git a/docs/onboarding/11-traps.md b/docs/onboarding/11-traps.md index d3e4583..4604b5d 100644 --- a/docs/onboarding/11-traps.md +++ b/docs/onboarding/11-traps.md @@ -12,27 +12,23 @@ Everything below was checked against the tree at the time of writing. ## Stale docs in this repo -### `frontend/CLAUDE.md` and `frontend/README.md` describe a service that is not deployed +### ~~`frontend/CLAUDE.md` and `frontend/README.md` describe a service that is not deployed~~ — fixed -**The live frontend is Go** (`frontend/server/`, module `thermograph/frontend`). -Both documents describe the Python FastAPI/Jinja implementation -(`frontend/app.py`, `content.py`, `api_client.py`, `format.py`, -`templates/*.html.j2`), which was superseded by the Go port and is kept in-tree -as the reference the port was made from. +**Fixed.** Both now describe the Go service (`frontend/server/`), state plainly +that the Python files at that level are the superseded original, and correct +`CLAUDE.md`'s claim that `make test-unit` is *"the tier CI runs"* — it isn't. +CI builds the frontend image, and `frontend/Dockerfile`'s builder stage runs +`gofmt -l` + `go vet` + `go test ./...` before compiling, so a failing **Go** +test is what fails CI. The Python tiers are local-only. -What is still **correct** in them: every contract (API-version pinning, -`pctOrd`, the `/cell` + ETag relationship, the Fahrenheit set, boot-must-survive- -an-unreachable-backend), every environment variable name and default, and all of -`DESIGN.md`. +Still worth knowing: the superseded Python files carry **duplicate copies of +some contracts** — `api_client.py` has a third `API_VERSION` pin and +`format.py` a fourth `F_COUNTRIES`. Nothing deploys them, but if you run them +locally, keep them in step or you'll debug a phantom. -What is **wrong**: the language, the file paths, and — notably — -`frontend/CLAUDE.md`'s claim that `make test-unit` is *"the tier CI runs."* It -isn't any more. CI builds the frontend image, and `frontend/Dockerfile`'s -builder stage runs `gofmt -l` + `go vet` + `go test ./...` before compiling, so -a failing **Go** test is what fails CI. The Python tiers are local-only now. - -Also note `api_client.py` carries a **third** copy of the `API_VERSION` pin. -It isn't deployed, but keep it in step or you'll debug a phantom locally. +Whether to delete them outright is an open question, not a settled one: the +Python test tiers still run, against fixtures the Go tests share. Raise it +rather than doing it in passing. ### `infra/DEPLOY.md` describes the pre-Swarm, pre-monorepo deploy @@ -108,34 +104,49 @@ at the monorepo, that's the only path. Flagged again in ## Live traps -### The `docker-compose.test.yml` files still pull retired image paths +### ~~The `docker-compose.test.yml` files pull retired image paths~~ — fixed -Both `backend/docker-compose.test.yml` and `frontend/docker-compose.test.yml` -default `BACKEND_IMAGE_PATH` to **`emi/thermograph-backend/app`** — the -split-era path. The current image is `emi/thermograph/backend`. +**Fixed.** Both now use `emi/thermograph/backend`, the path `build-push.yml` +actually publishes. -- `backend/make smoke` is fine: it builds `:local` and never pulls. -- `frontend/make backend-up` **does** pull, and pins - `THERMOGRAPH_BACKEND_TEST_TAG` to `v0.0.2-split-ci`. The old packages were - deliberately left in the registry for rollback and GC'd only after burn-in, so - this may work, may serve a very old backend, or may 404 depending on when you - read this. If `make backend-up` fails or behaves oddly, set the path and tag - explicitly: - ```bash - BACKEND_IMAGE_PATH=emi/thermograph/backend \ - THERMOGRAPH_BACKEND_TEST_TAG=sha-<12hex> make backend-up - ``` +The frontend harness had the worse half of this: it also pinned +`THERMOGRAPH_BACKEND_TEST_TAG` to the split-era `v0.0.2-split-ci`. Rather than +swap one hardcoded pin for another, `scripts/backend-for-tests.sh` now +**derives** the tag from the checkout — `sha-<12hex of git log -1 -- backend/>`, +the same domain-keyed rule `build-push.yml` and `deploy.yml` both use — so the +harness follows the tree instead of drifting behind a pin. `docker-compose.test.yml` +requires the variable (`:?`) rather than defaulting it, so a stale pin can't +creep back in silently. -### `static/units.js`'s `F_REGIONS` is guarded by nothing +If the derived tag has no published image (backend commits that are still +local-only), the script says so and tells you to pin one: -Source comments in three files claim *"a test asserts all three stay -identical."* The only automated assertion is -`format_test.go::TestFCountriesMatchesBackend`, which re-parses the backend's -Python and compares the **Go** copy. Nothing reads `units.js`. +```bash +THERMOGRAPH_BACKEND_TEST_TAG=sha-<12hex> make backend-up +``` -All four copies currently *are* identical (14 codes, checked). But the browser -copy is held in step by convention alone. Extending the Go test to parse -`units.js` too is a good first contribution. +> Unrelated but adjacent: `make test-integration` against a freshly-booted +> throwaway backend currently fails 7 of 16 tests with `503`, because nothing +> in that empty database is warm. Verified identical on the old image and the +> new one, so it predates these changes and is **not** caused by them. Not +> investigated further here. + +### ~~`static/units.js`'s `F_REGIONS` is guarded by nothing~~ — fixed + +**Fixed.** `format_test.go::TestFCountriesMatchesUnitsJS` now parses +`static/units.js` and diffs it against the Go set in both directions. + +The subtlety worth knowing: the *existing* backend cross-check +(`TestFCountriesMatchesBackend`) **skips in CI**. The frontend image's build +context is `frontend/`, so `backend/` is structurally unreachable from the +builder stage — and the builder stage is the only place CI runs these tests. It +is a checkout-only guard and always was. + +The new `units.js` check does not have that problem, because `static/` *is* +inside the context: `frontend/Dockerfile` copies that one file into the builder +stage specifically so the assertion runs during the image build. Verified by +mutating `units.js` and confirming the **image build fails**, not just the local +test run. ### The compose project name is load-bearing @@ -259,10 +270,10 @@ is one of Starlette's shared sync-threadpool threads. ## If you fix one of these -Docs are cheap to correct and the root `CLAUDE.md` already asks you to fix -repo-era wording as you touch files near it. If you're in `frontend/` anyway, -bringing `frontend/CLAUDE.md` in line with the Go service would be the single -highest-value documentation change available — it's the file an agent reads -*before* touching that domain. +Docs are cheap to correct, and the root `CLAUDE.md` already asks you to fix +repo-era wording as you touch files near it. The three entries struck through +above were fixed in the same change that added this guide; the rest are still +open, and `infra/DEPLOY.md` is probably the highest-value one left — it +describes a deploy model that has since changed twice. Back to the [index](README.md). diff --git a/frontend/CLAUDE.md b/frontend/CLAUDE.md index 5542457..4804185 100644 --- a/frontend/CLAUDE.md +++ b/frontend/CLAUDE.md @@ -7,65 +7,117 @@ no compute — everything comes from `backend/`'s content API over HTTP. Read the root `CLAUDE.md` first for the branch model, deploy contract and image names. +## The live service is Go — `server/` + +`server/` (module `thermograph/frontend`, Go 1.26) is what builds, tests, ships +and runs. It is the only thing in the `emi/thermograph/frontend` image. + +The Python files at this level — `app.py`, `content.py`, `api_client.py`, +`format.py`, `content_loader.py`, `paths.py`, `templates/*.html.j2` — are the +**superseded original implementation**, kept as the reference the port was made +from. Nothing deploys them. Don't add features there; don't take their file +paths as current. (They still carry duplicate copies of some contracts below — +notably `api_client.py`'s `API_VERSION` pin and `format.py`'s `F_COUNTRIES`. If +you use them locally, keep them in step or you will debug a phantom.) + +Deleting them is a live question, not a settled one — the Python test tiers +still run against the fixtures the Go tests share. Raise it rather than doing it +in passing. + ## Build & verify -- `make test` — whole suite (`scripts/test.sh`). -- `make test-unit` — hermetic unit tier: SSR rendering fed committed fixtures, - no Docker. **This is the tier CI runs.** -- `make test-integration` — pulls and runs the real backend image and tests the - live contract. Local only; CI does not run it. +- `cd server && go build ./... && go vet ./... && go test ./...` — **the suite + that matters.** +- **CI runs it inside `Dockerfile`'s builder stage**, which does `gofmt -l` + + `go vet` + `go test ./...` before compiling. A failing Go test therefore fails + the image build — that is the whole frontend check. There is no separate + frontend test step in any workflow. +- `make test-unit` / `make test-integration` — the **Python** tiers. Local only; + CI runs neither. The unit tier is hermetic (SSR rendering fed committed + fixtures); the integration tier pulls and runs a real backend image. - `make backend-up` / `make backend-down` — a local backend container for dev. + The image tag is derived from this checkout (last commit touching `backend/`, + the same domain-keyed rule build-push and deploy use); override with + `THERMOGRAPH_BACKEND_TEST_TAG=sha-<12hex>`. - `make capture-fixtures` — refresh `tests/fixtures/*.json` from a live backend. + **These fixtures feed the Go tests too**, not just the Python ones. ## Running it -`THERMOGRAPH_API_BASE_INTERNAL` is **required** — `api_client.py` raises at -import if unset. +`THERMOGRAPH_API_BASE_INTERNAL` is **required** — the boot fails loudly without +it, as the Python raised at import. + +Build in `server/`, run from `frontend/`: `static/` and `content/` resolve +relative to the working directory, while templates are embedded in the binary. ```bash +cd server && go build -o thermograph-frontend . +cd .. THERMOGRAPH_API_BASE_INTERNAL=http://127.0.0.1:8137 THERMOGRAPH_BASE=/thermograph \ - .venv/bin/uvicorn app:app --host 0.0.0.0 --port 8080 + ./server/thermograph-frontend ``` Other env: `THERMOGRAPH_BASE` (default `/thermograph`; the Dockerfile sets `/`), `THERMOGRAPH_API_VERSION` (default `v2`), `THERMOGRAPH_API_BASE_PUBLIC` (browser-facing backend origin when cross-origin; empty = same-origin, today's -default), `THERMOGRAPH_SSR_CACHE_TTL` (default 600s). +default), `THERMOGRAPH_SSR_CACHE_TTL` (default 600s), +`THERMOGRAPH_GOOGLE_VERIFY` / `THERMOGRAPH_BING_VERIFY`, `PORT` (default 8080). + +Boot is fail-loud on bad **config or content**: a missing backend URL, a garbage +TTL, or malformed `content/*.yaml` all stop the process rather than 500 on the +first request. ## Layout -- **`content.py`** — SSR routes (climate hub, city, month, records, glossary, - about, privacy) + `robots.txt` + `sitemap.xml`. Each fetches from the backend - via `api_client.py`; the backend's `content_payloads.py` owns `page_title` / - `canonical_path` / `breadcrumb` / `jsonld`, not this domain. +- **`server/internal/content/`** — the SSR page handlers (climate hub, city, + month, records, glossary, about, privacy) + `robots.txt` + `sitemap.xml`, + plus the template funcmap and SEO helpers. The backend's + `content_payloads.py` owns `page_title` / `canonical_path` / `breadcrumb` / + `jsonld`, **not this domain**. +- **`server/internal/contentapi/`** — the backend `/content/*` client: TTL + cache, bounded LRU, per-key single-flight, origin forwarding. All four + properties are load-bearing; read the file before changing caching. +- **`server/internal/render/`** + `render/templates/*.tmpl` — `html/template` + over an `embed.FS`, plus the ETag helpers. Templates are **embedded**, so a + template change needs a rebuild. +- **`server/internal/{config,format,contentdata,handlers}/`** — env parsing, + unit-aware °C/°F formatting and band names, the glossary/pages loader, and + the SPA-shell + static handlers. - **`static/*.js`** — `app.js` (map/search/results + inline SVG chart), `calendar.js`/`day.js`/`score.js`/`compare.js` (SPA shells), `account.js` (auth), `cache.js` (IndexedDB cache + `/cell` bundle prefetch), `shared.js`. - **`static/style.css`** — the single hand-written stylesheet; design tokens live here, documented in `DESIGN.md`. -- **`templates/*.html.j2`**, **`content/*.yaml`** (structured SSR copy, loaded by - `content_loader.py`). +- **`content/*.yaml`** — structured SSR copy, loaded by + `server/internal/contentdata`. ## Contracts with the backend -- **API version is pinned in exactly two places** — `api_client.py`'s - `API_VERSION` (Python) and `static/account.js`'s exported `API_VERSION` plus the - `uv(path)` helper (JS). Never hardcode `api/v2/...` anywhere else. Bump both in - one PR, and only after the target backend's `/api/version` confirms it serves - that version and its `min_frontend` doesn't exclude the one you're leaving. +- **API version is pinned in exactly two places** — `server/internal/config`'s + `THERMOGRAPH_API_VERSION` (Go) and `static/account.js`'s exported + `API_VERSION` plus the `uv(path)` helper (JS). Never hardcode `api/v2/...` + anywhere else. Bump both in one PR, and only after the target backend's + `/api/version` confirms it serves that version and its `min_frontend` doesn't + exclude the one you're leaving. - **`shared.js::pctOrd()` must mirror `backend`'s `data/grading.py::pct_ordinal()`** - in behaviour — floor into `1..99`, never 0 or 100. Every percentile on every - surface goes through one of the two; if they diverge, the same reading says two + in behaviour — floor into `1..99`, never 0 or 100 — and so must + `server/internal/format`'s `PctOrdinal`. Every percentile on every surface + goes through one of the three; if they diverge, the same reading says two different things in two places. - **`/cell` bundle + ETag** — `cache.js` fetches `/api/v2/cell` once per view-set and slices it, revalidating with `If-None-Match`. The slice→view map must track the backend's payload shape. -- **Fahrenheit country set** — `format.py::F_COUNTRIES` and `static/units.js`'s - `F_REGIONS` must stay identical to the backend's `F_COUNTRIES`. -- **Boot must survive an unreachable backend.** `content.py`'s `register()` tries - the IndexNow-key fetch once, catches failure, and falls back to a lazy - per-request lookup. Asynchronous FE/BE deploys depend on this — don't make it - fatal. +- **Fahrenheit country set** — `server/internal/format`'s `FCountries` and + `static/units.js`'s `F_REGIONS` must stay identical to the backend's + `F_COUNTRIES`. Both are now asserted by tests in + `server/internal/format/format_test.go`: the backend cross-check is a + checkout-only guard (the image build context is `frontend/`, so `backend/` is + unreachable there), while the `units.js` check runs in the image build too — + `Dockerfile` copies that one file into the builder stage for exactly that + reason. +- **Boot must survive an unreachable backend.** The IndexNow-key fetch is tried + once at boot, caught, and falls back to a lazy per-request lookup. + Asynchronous FE/BE deploys depend on this — don't make it fatal. ## Commits & PRs diff --git a/frontend/Dockerfile b/frontend/Dockerfile index 692ecb9..920c545 100644 --- a/frontend/Dockerfile +++ b/frontend/Dockerfile @@ -35,6 +35,19 @@ COPY server/ ./ COPY tests/fixtures /tests/fixtures COPY content /content +# static/units.js is copied for ONE test: internal/format's +# TestFCountriesMatchesUnitsJS, which asserts the browser's F_REGIONS still +# matches the Go/backend Fahrenheit country set. The builder stage is the only +# place CI ever executes these tests, so without this the check would skip in CI +# and only ever run on a developer's checkout. Same three-levels-up relationship +# the test expects (/src/internal/format -> /static/units.js), anchored the same +# way the two directories above are. +# +# The sibling backend cross-check cannot be wired up this way: the build context +# is frontend/, so backend/ is structurally unreachable and that test stays a +# checkout-only guard. +COPY static/units.js /static/units.js + RUN test -z "$(gofmt -l .)" && go vet ./... && go test ./... # Static binary: CGO off (no libc dependency on Alpine), -trimpath for diff --git a/frontend/README.md b/frontend/README.md index ccc40d5..c22d8e4 100644 --- a/frontend/README.md +++ b/frontend/README.md @@ -2,107 +2,92 @@ The server-rendered content pages, the interactive-tool SPA shells, and every static asset for [Thermograph](https://thermograph.org) — grades recent local -weather against ~45 years of climate history. This repo holds **no climate -data and does no compute**; it renders pages and serves the JS/CSS/image -assets that call the backend's API from the browser. It was split out of the -`emi/thermograph` monorepo (`frontend_ssr/` + `frontend/` there) so it can -build its own image and deploy independently of the backend. See -[`CLAUDE.md`](CLAUDE.md) for the full agent-facing detail on the split -topology and deploy contract, and -[`thermograph-docs`](../thermograph-docs) (sibling repo) for the -architecture decision record behind the split. +weather against ~45 years of climate history. This domain holds **no climate +data and does no compute**; it renders pages and serves the JS/CSS/image assets +that call the backend's API from the browser. It builds its own image +(`emi/thermograph/frontend`) and deploys independently of the backend. -## Layout +See [`CLAUDE.md`](CLAUDE.md) for the agent-facing detail on the deploy contract +and the cross-service contracts, and [`DESIGN.md`](DESIGN.md) for the visual +system. + +## The service is Go + +`server/` is the live implementation. The Python files at this level (`app.py`, +`content.py`, `api_client.py`, `format.py`, `content_loader.py`, `paths.py`, +`templates/*.html.j2`) are the **superseded original**, kept as the reference +the port was made from — nothing deploys them. See `server/README.md` for the +port's own notes. ``` -api_client.py HTTP client for the backend's content API (api/content_routes.py - on the backend). Fails loud at import if - THERMOGRAPH_API_BASE_INTERNAL is unset. TTL-cached (10 min - default) in front of every call. -app.py FastAPI app: /healthz, the interactive tool's SPA-shell - routes (calendar/day/score/compare/legend/alerts), and the - StaticFiles mount for everything else. Calls - content.register(app) for the SSR routes below. -content.py Server-rendered pages: climate hub, per-city, month, - records, glossary, about, privacy, robots.txt, sitemap.xml. -content_loader.py Loads content/glossary.yaml + content/pages.yaml (SSR copy), - validated fail-loud at load time. -format.py Unit-aware (°C/°F) formatting for the SSR templates, - ContextVar-scoped active unit. -paths.py Canonical filesystem locations (STATIC_DIR, TEMPLATES_DIR, - CONTENT_DIR), resolved from the repo root. -templates/ Jinja templates content.py renders (base, home, city, month, - hub, records, glossary, glossary_term, about, privacy). -static/ Every static asset: the interactive tool's JS - (app.js, calendar.js, day.js, score.js, compare.js, - account.js, cache.js, shared.js, units.js, mappicker.js, …), - the SPA-shell HTML pages, style.css (all design tokens — - see DESIGN.md), icons/favicons, manifest.webmanifest, sw.js. -content/ Structured SSR copy: glossary.yaml, pages.yaml. -tests/ pytest suite — see the KNOWN GAP note below before relying on it. -tools/ shoot.py — the make-shots visual-verify screenshot tool - (see DESIGN.md). -Dockerfile Builds this repo's own image (python:3.12-slim, - uvicorn app:app), independent of the backend's. +server/ the service (module thermograph/frontend, Go 1.26) + main.go config + mux + static + graceful shutdown + internal/config/ every env var it reads + internal/contentapi/ backend /content/* client: TTL cache, bounded LRU, + per-key single-flight, origin forwarding + internal/content/ SSR page handlers, template funcmap, SEO + internal/contentdata/ glossary.yaml / pages.yaml loader (fail-loud) + internal/format/ unit-aware °C/°F formatting + band names + internal/handlers/ SPA shells + static serving + internal/render/ html/template over embed.FS + ETag helpers + internal/render/templates/ the page templates (embedded, *.tmpl) +static/ every static asset: the interactive tool's JS + (app.js, calendar.js, day.js, score.js, compare.js, + account.js, cache.js, shared.js, units.js, + mappicker.js, …), the SPA-shell HTML pages, + style.css (all design tokens — see DESIGN.md), + icons/favicons, manifest.webmanifest, sw.js +content/ structured SSR copy: glossary.yaml, pages.yaml +tests/fixtures/ golden payloads — consumed by the GO tests as well + as the Python tiers +tests/{unit,integration}/ the Python tiers (local only; CI runs neither) +tools/shoot.py the screenshot sweep for visual verification +Dockerfile golang builder (gofmt + vet + test) → alpine runtime ``` -## How it fits the split - -Four sibling repos replace the old monorepo: - -- **`thermograph-backend`** — the FastAPI API, grading/scoring/grid compute, - and DB. This repo's only runtime dependency. -- **`thermograph-frontend`** (this repo). -- **`thermograph-infra`** — Terraform, compose files, `deploy/deploy.sh` (the - script both repos' deploy workflows SSH into), the secrets vault. -- **`thermograph-docs`** — architecture/decision docs and runbooks. - -Each app repo now builds and publishes its **own** image -(`git.thermograph.org/emi/thermograph-frontend/app`, tagged by git SHA and -semver) instead of the old shared `emi/thermograph/app` image, and deploys via -its own `.forgejo/workflows/deploy.yml` (main → beta) and `deploy-prod.yml` -(release → prod), each rolling only the `frontend` compose service on the -target VPS. +Dependencies are **stdlib plus `gopkg.in/yaml.v3`** — the committed SSR copy in +`content/*.yaml` uses block/folded scalars, so a real YAML parser is required. ## Talking to the backend -All data comes from the backend's content API over HTTP -(`api_client.py`) — this process holds nothing in-process. Two env vars -control the topology: +All data comes from the backend's content API over HTTP — this process holds +nothing. Two env vars control the topology: - **`THERMOGRAPH_API_BASE_INTERNAL`** (required) — where this process reaches the backend server-side, e.g. `http://backend:8137` in compose, or - `http://127.0.0.1:8137` for a local backend checkout. `api_client.py` raises - at import if this is unset — a missing backend URL should break boot, not - silently 500 the first request. -- **`THERMOGRAPH_API_BASE_PUBLIC`** (optional) — the backend's - browser-facing origin, used only when frontend and backend are served - cross-origin (e.g. a genuinely separate LAN-dev topology). Left empty - (the default), asset/API references stay relative and same-origin, which is - today's real deployed topology. + `http://127.0.0.1:8137` for a local backend. Boot **fails loudly** if unset: + a missing backend URL should break the boot, not silently 500 on the first + request. +- **`THERMOGRAPH_API_BASE_PUBLIC`** (optional) — the backend's browser-facing + origin, used only when frontend and backend are served cross-origin. Left + empty (the default), asset/API references stay relative and same-origin, + which is today's real deployed topology. + +`THERMOGRAPH_BASE` (default `/thermograph`) must be set identically on both +frontend and backend in every real deployment — it's how both processes agree on +the shared URL prefix (or the deployed clean-root `/`, which the Dockerfile +sets). **Same-origin cookie-auth caveat:** the interactive tool's auth (`static/account.js`) is an HttpOnly session cookie. It uses `credentials: "include"` (not the fetch default) specifically so the cookie -still rides along if this script is ever loaded cross-origin, but the backend -must actually be configured to accept credentialed cross-origin requests -(CORS + `SameSite`) for that case to work — in the default same-origin -deployment this is a non-issue. Don't assume a fully decoupled, independently- -hosted frontend "just works" for logged-in features without checking that -cookie/CORS configuration first. - -`THERMOGRAPH_BASE` (default `/thermograph`) must be set identically on both -frontend and backend in every real deployment — it's how both processes agree -on the shared URL prefix (or the deployed clean-root `/`, which the Dockerfile -sets by default). +still rides along if this script is ever loaded cross-origin — but the backend +must actually be configured to accept credentialed cross-origin requests (CORS + +`SameSite`) for that case to work. In the default same-origin deployment this is +a non-issue. Don't assume a fully decoupled, independently-hosted frontend "just +works" for logged-in features without checking that configuration first. ## Build & run +Build in `server/`, run from here — `static/` and `content/` resolve relative to +the working directory, while templates are embedded in the binary: + ```bash -python3 -m venv .venv && .venv/bin/pip install -r requirements.txt +cd server && go build -o thermograph-frontend . +cd .. THERMOGRAPH_API_BASE_INTERNAL=http://127.0.0.1:8137 \ THERMOGRAPH_BASE=/thermograph \ - .venv/bin/uvicorn app:app --host 0.0.0.0 --port 8080 + ./server/thermograph-frontend ``` Or build the image directly: @@ -115,12 +100,37 @@ docker run -p 8080:8080 -e THERMOGRAPH_API_BASE_INTERNAL=http://backend:8137 the The image healthchecks `GET /healthz` (liveness only — it does no I/O, so it stays cheap; it does not prove the backend is reachable). +## Test + +```bash +cd server && go build ./... && go vet ./... && go test ./... +``` + +`Dockerfile`'s builder stage runs `gofmt -l` + `go vet` + `go test ./...` before +compiling, so **a failing Go test fails the image build** — and that is the +whole frontend check in CI. There is no separate frontend test step in any +workflow. + +The Python tiers are local-only: + +```bash +make test-unit # hermetic: SSR rendering fed committed fixtures +make test-integration # pulls + runs a real backend image +make backend-up # just the backend container, for dev +make capture-fixtures # refresh tests/fixtures/*.json from a live backend +``` + +`make backend-up` derives the backend image tag from this checkout (the last +commit touching `backend/` — the same domain-keyed rule build-push and deploy +use); pin one with `THERMOGRAPH_BACKEND_TEST_TAG=sha-<12hex>`. + ## More detail -- [`CLAUDE.md`](CLAUDE.md) — agent-facing instructions: deploy contract, - API-version pinning, the cross-repo `/cell`/ETag and `pctOrd()` contracts, - and the current known test-suite gap. +- [`CLAUDE.md`](CLAUDE.md) — agent-facing instructions: build/verify, layout, + API-version pinning, and the `/cell`/ETag, `pctOrd()` and Fahrenheit-set + contracts. +- [`server/README.md`](server/README.md) — the Go port's own notes. - [`DESIGN.md`](DESIGN.md) — the visual system (tokens, components, - breakpoints) and `make shots` visual verification. -- [`thermograph-docs`](../thermograph-docs) — the repo-topology decision - record this split implements. + breakpoints) and `tools/shoot.py` visual verification. +- [`docs/onboarding/`](../docs/onboarding/README.md) — the developer onboarding + path for the whole monorepo. diff --git a/frontend/docker-compose.test.yml b/frontend/docker-compose.test.yml index 83b5d33..234fa2d 100644 --- a/frontend/docker-compose.test.yml +++ b/frontend/docker-compose.test.yml @@ -20,9 +20,16 @@ services: retries: 20 backend: - # The published backend image is pulled (no local build). Default a published tag; - # override THERMOGRAPH_BACKEND_TEST_TAG to pin a specific backend build (e.g. a sha). - image: ${REGISTRY_HOST:-git.thermograph.org}/${BACKEND_IMAGE_PATH:-emi/thermograph-backend/app}:${THERMOGRAPH_BACKEND_TEST_TAG:-v0.0.2-split-ci} + # The published backend image is pulled (no local build). + # + # No default tag on purpose: scripts/backend-for-tests.sh computes one from + # THIS checkout -- sha-<12hex of `git log -1 -- backend/`>, the same + # domain-keyed rule build-push.yml and deploy.yml both use -- so the harness + # tracks the tree instead of drifting behind a hardcoded pin. (It was pinned + # to the split-era v0.0.2-split-ci, under the retired + # emi/thermograph-backend/app path, long after both were superseded.) + # Override THERMOGRAPH_BACKEND_TEST_TAG to pin a specific build. + image: ${REGISTRY_HOST:-git.thermograph.org}/${BACKEND_IMAGE_PATH:-emi/thermograph/backend}:${THERMOGRAPH_BACKEND_TEST_TAG:?set it, or run via scripts/backend-for-tests.sh which derives it} depends_on: db: condition: service_healthy diff --git a/frontend/scripts/backend-for-tests.sh b/frontend/scripts/backend-for-tests.sh index 0770a96..1f48ab4 100755 --- a/frontend/scripts/backend-for-tests.sh +++ b/frontend/scripts/backend-for-tests.sh @@ -7,8 +7,9 @@ # scripts/backend-for-tests.sh down # stop + remove (incl. the throwaway db volume) # scripts/backend-for-tests.sh url # print the base URL (no lifecycle change) # -# Which backend build: THERMOGRAPH_BACKEND_TEST_TAG (default a published tag; set a -# sha-<12hex> to pin one). Host port: BACKEND_TEST_HOST_PORT (default 18137). +# Which backend build: THERMOGRAPH_BACKEND_TEST_TAG. Unset, it is DERIVED from this +# checkout (see below) rather than defaulted to a hardcoded tag -- set a sha-<12hex> +# to pin one. Host port: BACKEND_TEST_HOST_PORT (default 18137). set -euo pipefail cd "$(dirname "$0")/.." @@ -17,10 +18,39 @@ export BACKEND_TEST_HOST_PORT="${BACKEND_TEST_HOST_PORT:-18137}" BASE_URL="http://127.0.0.1:${BACKEND_TEST_HOST_PORT}" COMPOSE=(docker compose -f docker-compose.test.yml) +# Derive the backend image tag the SAME way build-push.yml and deploy.yml do: +# keyed to the last commit that touched backend/, not the branch tip. That is the +# image actually published for this checkout's backend tree, so the harness follows +# the tree instead of drifting behind a pin (this file previously defaulted to the +# split-era v0.0.2-split-ci tag under a retired image path). +# +# Exported before ANY compose call -- docker-compose.test.yml requires the variable +# (`:?`), so `down` and `url` need it set too, not just `up`. +# +# `:/backend/` is a repo-ROOT-relative pathspec: a bare `backend/` would resolve +# against this script's cwd (frontend/) and silently match nothing, yielding an +# empty sha and a confusing error instead of the right tag. +if [ -z "${THERMOGRAPH_BACKEND_TEST_TAG:-}" ]; then + domain_sha="$(git log -1 --format=%H -- ':/backend/' 2>/dev/null || true)" + if [ -z "$domain_sha" ]; then + echo "!! could not derive a backend image tag from git (no backend/ history here?)." >&2 + echo " Set THERMOGRAPH_BACKEND_TEST_TAG=sha-<12hex> explicitly." >&2 + exit 2 + fi + THERMOGRAPH_BACKEND_TEST_TAG="sha-${domain_sha:0:12}" +fi +export THERMOGRAPH_BACKEND_TEST_TAG + case "${1:-up}" in up) - echo "==> pulling backend image (${THERMOGRAPH_BACKEND_TEST_TAG:-v0.0.2-split-ci})" >&2 - "${COMPOSE[@]}" pull backend >&2 + echo "==> pulling backend image (${THERMOGRAPH_BACKEND_TEST_TAG})" >&2 + if ! "${COMPOSE[@]}" pull backend >&2; then + echo "!! no published backend image for ${THERMOGRAPH_BACKEND_TEST_TAG}." >&2 + echo " That tag is keyed to the last commit touching backend/ -- if those" >&2 + echo " commits are local-only, build-push.yml has not published it yet." >&2 + echo " Pin a published build: THERMOGRAPH_BACKEND_TEST_TAG=sha-<12hex> $0 up" >&2 + exit 1 + fi echo "==> starting backend + db" >&2 "${COMPOSE[@]}" up -d >&2 echo "==> waiting for backend /healthz (up to 90s)" >&2 diff --git a/frontend/server/internal/format/format_test.go b/frontend/server/internal/format/format_test.go index defa4b5..699f999 100644 --- a/frontend/server/internal/format/format_test.go +++ b/frontend/server/internal/format/format_test.go @@ -45,42 +45,93 @@ func TestUnitForCountry(t *testing.T) { } } -// TestFCountriesMatchesBackend re-parses the backend's F_COUNTRIES (the -// source of truth per both CLAUDE.md files) and asserts our set is identical. -// Skips when the monorepo backend checkout isn't present. -func TestFCountriesMatchesBackend(t *testing.T) { - path := filepath.Join("..", "..", "..", "..", "backend", "api", "content_payloads.py") +// parseCountryCodes pulls a set of ISO-3166 alpha-2 codes out of `path` by +// matching `outer` (which must capture the literal's body in group 1) and then +// scanning that body for quoted codes. Returns nil when the file isn't present, +// so a caller can skip rather than fail in a checkout/build context that does +// not carry it. +func parseCountryCodes(t *testing.T, path string, outer *regexp.Regexp) map[string]bool { + t.Helper() raw, err := os.ReadFile(path) if err != nil { - t.Skipf("backend checkout not present (%v) — parity asserted only against the committed copy", err) + return nil } - m := regexp.MustCompile(`(?s)F_COUNTRIES\s*=\s*frozenset\(\{(.*?)\}\)`).FindSubmatch(raw) + m := outer.FindSubmatch(raw) if m == nil { - t.Fatalf("could not find F_COUNTRIES in %s", path) + t.Fatalf("could not find the country-set literal in %s", path) } codes := regexp.MustCompile(`"([A-Z]{2})"`).FindAllSubmatch(m[1], -1) if len(codes) == 0 { t.Fatalf("no codes parsed from %s", path) } - backend := map[string]bool{} + set := map[string]bool{} for _, c := range codes { - backend[string(c[1])] = true + set[string(c[1])] = true } - if len(backend) != len(FCountries) { - t.Errorf("F_COUNTRIES size mismatch: backend %d vs ours %d", len(backend), len(FCountries)) + return set +} + +// assertMatchesFCountries diffs a parsed copy against ours, both directions. +func assertMatchesFCountries(t *testing.T, name string, other map[string]bool) { + t.Helper() + if len(other) != len(FCountries) { + t.Errorf("size mismatch: %s %d vs ours %d", name, len(other), len(FCountries)) } - for code := range backend { + for code := range other { if !FCountries[code] { - t.Errorf("backend has %q, ours does not", code) + t.Errorf("%s has %q, ours does not", name, code) } } for code := range FCountries { - if !backend[code] { - t.Errorf("ours has %q, backend does not", code) + if !other[code] { + t.Errorf("ours has %q, %s does not", code, name) } } } +// TestFCountriesMatchesBackend re-parses the backend's F_COUNTRIES (the +// source of truth per both CLAUDE.md files) and asserts our set is identical. +// Skips when the monorepo backend checkout isn't present — notably inside +// frontend/Dockerfile's builder stage, whose build context is frontend/ only, +// so backend/ is structurally unreachable there. This check is therefore a +// local/CI-checkout guard, not an image-build one. +func TestFCountriesMatchesBackend(t *testing.T) { + path := filepath.Join("..", "..", "..", "..", "backend", "api", "content_payloads.py") + backend := parseCountryCodes(t, path, regexp.MustCompile(`(?s)F_COUNTRIES\s*=\s*frozenset\(\{(.*?)\}\)`)) + if backend == nil { + t.Skip("backend checkout not present — parity asserted only against the committed copy") + } + assertMatchesFCountries(t, "backend", backend) +} + +// TestFCountriesMatchesUnitsJS asserts the BROWSER copy — static/units.js's +// F_REGIONS — matches too. +// +// This copy was previously guarded by nothing at all, despite comments in +// backend/api/content_payloads.py, frontend/format.py and format.go each +// claiming "a test asserts all three stay identical": the only check compared +// Go against the backend's Python, and nothing read units.js. The sets happened +// to agree, held in step by convention alone. Client-side unit selection +// disagreeing with server-rendered unit selection means the same page shows °C +// in SSR and °F after hydration — a silent, per-country split. +// +// Unlike the backend check above, static/ IS inside the frontend build context, +// and frontend/Dockerfile copies units.js into the builder stage precisely so +// this runs during the image build — the only place CI executes these tests. +func TestFCountriesMatchesUnitsJS(t *testing.T) { + for _, path := range []string{ + filepath.Join("..", "..", "..", "static", "units.js"), // repo checkout + filepath.Join("/", "static", "units.js"), // Dockerfile builder stage + } { + js := parseCountryCodes(t, path, regexp.MustCompile(`(?s)F_REGIONS\s*=\s*new Set\(\[(.*?)\]\)`)) + if js != nil { + assertMatchesFCountries(t, "static/units.js", js) + return + } + } + t.Skip("static/units.js not reachable from this working directory") +} + func TestTempSpans(t *testing.T) { // Exact rendered bytes — the golden-diff contract. if got := Temp("F", fp(71.1)); got != `71°F` {