From d373f0f0dd7903597cf5f002a2d0497421872e0f Mon Sep 17 00:00:00 2001 From: bk86a Date: Fri, 3 Jul 2026 18:55:47 +0200 Subject: [PATCH] chore: remove tracked private planning/SDD docs from the repo docs/superpowers/ holds internal AI planning + design-spec artifacts that were committed before the path was gitignored (cc02924). gitignore doesn't untrack already-committed files, so remove the 10 tracked docs now. The existing .gitignore rule keeps them from returning. --- .../plans/2026-04-28-uk-itl-support.md | 1600 ---------------- .../plans/2026-04-29-auth-token-bypass.md | 1157 ------------ .../2026-04-29-db-backed-trusted-tokens.md | 1620 ----------------- .../2026-05-01-estimates-periodic-refresh.md | 1562 ---------------- .../plans/2026-05-01-multi-worker-uvicorn.md | 795 -------- .../specs/2026-04-28-uk-itl-support-design.md | 221 --- .../2026-04-29-auth-token-bypass-design.md | 287 --- ...6-04-29-db-backed-trusted-tokens-design.md | 358 ---- ...05-01-estimates-periodic-refresh-design.md | 188 -- .../2026-05-01-multi-worker-uvicorn-design.md | 252 --- 10 files changed, 8040 deletions(-) delete mode 100644 docs/superpowers/plans/2026-04-28-uk-itl-support.md delete mode 100644 docs/superpowers/plans/2026-04-29-auth-token-bypass.md delete mode 100644 docs/superpowers/plans/2026-04-29-db-backed-trusted-tokens.md delete mode 100644 docs/superpowers/plans/2026-05-01-estimates-periodic-refresh.md delete mode 100644 docs/superpowers/plans/2026-05-01-multi-worker-uvicorn.md delete mode 100644 docs/superpowers/specs/2026-04-28-uk-itl-support-design.md delete mode 100644 docs/superpowers/specs/2026-04-29-auth-token-bypass-design.md delete mode 100644 docs/superpowers/specs/2026-04-29-db-backed-trusted-tokens-design.md delete mode 100644 docs/superpowers/specs/2026-05-01-estimates-periodic-refresh-design.md delete mode 100644 docs/superpowers/specs/2026-05-01-multi-worker-uvicorn-design.md diff --git a/docs/superpowers/plans/2026-04-28-uk-itl-support.md b/docs/superpowers/plans/2026-04-28-uk-itl-support.md deleted file mode 100644 index d154114..0000000 --- a/docs/superpowers/plans/2026-04-28-uk-itl-support.md +++ /dev/null @@ -1,1600 +0,0 @@ -# UK postcode and ITL support — implementation plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Add UK postcode support backed by the ONS NSPL dataset, with results returned as ITL codes via the existing `/lookup` endpoint and a new `code_system` discriminator. - -**Architecture:** UK is a parallel data channel that reuses the same in-memory `_lookup` dict, the same SQLite cache, and the same lookup waterfall as TERCET. The NSPL loader runs alongside the TERCET loader at startup with independent failure handling. A new Tier 3.5 outward-code lookup catches partial UK input (`SW1A` instead of `SW1A 2AA`). - -**Tech Stack:** Python 3.12, FastAPI, pydantic, httpx, sqlite3, pytest. Existing patterns mirrored throughout. - -**Spec:** [`docs/superpowers/specs/2026-04-28-uk-itl-support-design.md`](../specs/2026-04-28-uk-itl-support-design.md) - -**Issue:** [#7](https://github.com/bk86a/PostalCode2NUTS/issues/7) - ---- - -## File-by-file overview - -| File | Purpose | Touched by tasks | -|------|---------|------------------| -| `app/models.py` | Add `code_system` field to `NUTSResult` | 1 | -| `app/postal_patterns.json` | Add UK entry; bump `_meta.version` to `1.1` | 2 | -| `app/postal_patterns.py` | Implement `outward_only` action + `extract_outward` helper | 3 | -| `app/data_loader.py` | NSPL column aliases, `doterm` filter, conditional GET, NSPL loader, `_outward_lookup` index, `GB→UK` alias, `code_system` tagging, Tier 3.5, ITL names loader, isolation | 4–14 | -| `app/settings.json` | Add `nspl_url` only — **do NOT add UK to `countries`** (Codex review on PR #52: would trigger wasted GISCO URL guesses) | 6 | -| `app/config.py` | Surface NSPL config as `Settings` attributes | 6 | -| `app/main.py` | Forward `code_system` from lookup result; documentation cleanup | 12 | -| `tests/conftest.py` | Add UK mock data + outward-code fixture | 1, 9, 11 | -| `tests/test_postal_patterns.py` | UK regex tests + outward extraction | 2, 3 | -| `tests/test_data_loader.py` | NSPL parser, doterm filter, conditional GET, outward index, GB alias, code_system tagging, Tier 3.5, ITL names, isolation | 4–14 | -| `tests/test_api.py` | `code_system` in `/lookup` response, GB alias end-to-end | 1, 10, 12 | -| `README.md` | Coverage, six-tier waterfall, config table, attribution, divergence note | 15 | - ---- - -## Task 1: Add `code_system` field to `NUTSResult` - -**Goal:** Surface a discriminator so consumers know whether `nuts1/2/3` hold NUTS codes (GISCO) or ITL codes (NSPL). Backwards-compatible (additive only). Default `"NUTS"` everywhere; UK loader will override later. - -**Files:** -- Modify: `app/models.py:1-20` -- Modify: `tests/test_api.py` (existing AT lookup test) - -- [ ] **Step 1: Write the failing test** - -Add to `tests/test_api.py` (extend an existing AT lookup assertion or add new): - -```python -def test_lookup_response_includes_code_system_nuts(client, mock_data): - resp = client.get("/lookup", params={"country": "AT", "postal_code": "1010"}) - assert resp.status_code == 200 - body = resp.json() - assert body["code_system"] == "NUTS" -``` - -- [ ] **Step 2: Run test to verify it fails** - -```bash -pytest tests/test_api.py::test_lookup_response_includes_code_system_nuts -v -``` - -Expected: FAIL with `KeyError: 'code_system'` or `AssertionError`. - -- [ ] **Step 3: Add the field to the model** - -In `app/models.py`, modify `NUTSResult`: - -```python -class NUTSResult(BaseModel): - postal_code: str = Field(description="The queried postal code (normalized)") - country_code: str = Field(description="ISO 3166-1 alpha-2 country code") - code_system: Literal["NUTS", "ITL"] = Field( - default="NUTS", - description=( - "Identifies the territorial coding scheme. 'NUTS' for GISCO-sourced " - "EU/EFTA/candidate data; 'ITL' for UK data from the ONS NSPL." - ), - ) - match_type: Literal["exact", "estimated", "approximate"] = Field( - description="How the result was determined" - ) - # ... rest unchanged -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pytest tests/test_api.py -v -``` - -Expected: PASS (the new test plus all existing tests). - -- [ ] **Step 5: Commit** - -```bash -git add app/models.py tests/test_api.py -git commit -m "feat: add code_system discriminator field to NUTSResult" -``` - ---- - -## Task 2: Add UK regex to `postal_patterns.json` and bump version - -**Goal:** Validate UK input format. The regex matches all six valid outward-code shapes plus optional space. - -**Files:** -- Modify: `app/postal_patterns.json` (add `UK` entry; update `_meta.version`) -- Modify: `tests/test_postal_patterns.py` (add UK regex tests) - -- [ ] **Step 1: Write the failing tests** - -Add to `tests/test_postal_patterns.py`: - -```python -import pytest -from app.postal_patterns import extract_postal_code, PATTERNS_META - - -@pytest.mark.parametrize("raw, expected", [ - ("SW1A 2AA", "SW1A2AA"), - ("sw1a 2aa", "SW1A2AA"), - ("SW1A2AA", "SW1A2AA"), - ("M1 1AA", "M11AA"), - ("B33 8TH", "B338TH"), - ("W1A 1HQ", "W1A1HQ"), - ("CR2 6XH", "CR26XH"), - ("DN55 1PT", "DN551PT"), - ("EC1A 1BB", "EC1A1BB"), -]) -def test_uk_regex_extracts_normalized_full_postcode(raw, expected): - assert extract_postal_code("UK", raw) == expected - - -def test_patterns_meta_version_bumped(): - # Adding UK is an additive coverage change; minor version bump. - assert PATTERNS_META["version"] == "1.1" -``` - -- [ ] **Step 2: Run tests to verify failure** - -```bash -pytest tests/test_postal_patterns.py -v -k "uk_regex or version_bumped" -``` - -Expected: failures — UK pattern absent, version is `1.0`. - -- [ ] **Step 3: Add the UK pattern entry** - -In `app/postal_patterns.json`, change `_meta.version` from `"1.0"` to `"1.1"` and insert the `UK` entry (alphabetically after `TR`): - -```json -"_meta": { "version": "1.1", "date": "2026-04-28" }, -... - "TR": { ... }, - "UK": { - "regex": "^([A-Z]{1,2}[0-9][0-9A-Z]?\\s?[0-9][A-Z]{2})$", - "example": "SW1A 2AA, EC1A 1BB, M1 1AA, B33 8TH", - "tercet_map": "outward_only" - } -``` - -(Note: `_meta.date` value is a hint; pick the date you commit on.) - -- [ ] **Step 4: Run tests** - -```bash -pytest tests/test_postal_patterns.py -v -k "uk_regex or version_bumped" -``` - -Expected: regex tests PASS. The version test PASSES. The `tercet_map: outward_only` action will fail any tests in Task 3 — that is expected and Task 3 implements it. - -- [ ] **Step 5: Commit** - -```bash -git add app/postal_patterns.json tests/test_postal_patterns.py -git commit -m "feat: add UK postcode regex and bump patterns_version to 1.1" -``` - ---- - -## Task 3: Implement `outward_only` action and `extract_outward` helper - -**Goal:** UK Tier 1 lookup uses the full postcode (`SW1A2AA`); Tier 3.5 lookup uses the outward portion (`SW1A`). The pattern entry's `tercet_map: outward_only` flags that the country supports outward-only fallback. `extract_postal_code` keeps returning the full normalised form (so Tier 1 still works); a new `extract_outward(country, raw_input)` returns the outward portion or `None`. - -**Files:** -- Modify: `app/postal_patterns.py:70-108` -- Modify: `tests/test_postal_patterns.py` - -- [ ] **Step 1: Write the failing tests** - -Add to `tests/test_postal_patterns.py`: - -```python -from app.postal_patterns import extract_outward - - -@pytest.mark.parametrize("raw, expected_outward", [ - ("SW1A 2AA", "SW1A"), - ("sw1a2aa", "SW1A"), - ("M1 1AA", "M1"), - ("B33 8TH", "B33"), - ("EC1A 1BB", "EC1A"), - ("DN55 1PT", "DN55"), - ("SW1A", "SW1A"), # outward-only input - ("M1", "M1"), -]) -def test_extract_outward_for_uk(raw, expected_outward): - assert extract_outward("UK", raw) == expected_outward - - -def test_extract_outward_returns_none_for_country_without_flag(): - # AT does not declare outward_only; outward extraction is undefined. - assert extract_outward("AT", "1010") is None - - -def test_extract_postal_code_unaffected_by_outward_only_flag(): - # Tier 1 lookup behaviour for UK must still be the full normalised postcode. - assert extract_postal_code("UK", "SW1A 2AA") == "SW1A2AA" -``` - -- [ ] **Step 2: Run tests to verify failure** - -```bash -pytest tests/test_postal_patterns.py -v -k "outward" -``` - -Expected: ImportError (no `extract_outward` symbol) plus assertion failures. - -- [ ] **Step 3: Implement `outward_only` handling** - -In `app/postal_patterns.py`, modify `_apply_tercet_map` to leave `outward_only` as a no-op (it's a flag, not a transform), and add `extract_outward`: - -```python -def _apply_tercet_map(code: str, rule: str) -> str: - """Apply a tercet_map transform rule to an extracted postal code.""" - action, _, arg = rule.partition(":") - if action == "truncate": - return code[: int(arg)] - if action == "prepend": - return arg + code - if action == "keep_alpha": - m = re.match(r"^([A-Z]+)", code) - return m.group(1) if m else code - if action == "outward_only": - # Marker for countries that support outward-code-only fallback (Tier 3.5). - # Has no effect on Tier 1 extraction; see extract_outward(). - return code - return code - - -def extract_outward(country_code: str, raw_input: str) -> str | None: - """Return the outward (district) portion for countries flagged outward_only. - - For UK postcodes, the outward portion is everything except the last 3 chars - of the normalised form. If the input is shorter than 4 chars after - normalisation, the input itself is treated as outward (handles cases like - "SW1A" submitted alone). - - Returns None for countries that do not declare tercet_map=outward_only. - """ - entry = POSTAL_PATTERNS.get(country_code) - if not entry or entry.get("tercet_map") != "outward_only": - return None - normalised = normalize_postal_code(raw_input) - if len(normalised) <= 4: - return normalised - return normalised[:-3] -``` - -- [ ] **Step 4: Run tests** - -```bash -pytest tests/test_postal_patterns.py -v -``` - -Expected: all PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/postal_patterns.py tests/test_postal_patterns.py -git commit -m "feat: add outward_only action and extract_outward helper" -``` - ---- - -## Task 4: Extend `_parse_csv_content` column aliases for NSPL - -**Goal:** Recognise NSPL's `pcds` column as a postal code source and `itl` / `itl3` / `itl3cd` as NUTS3-equivalent columns. No new parser path needed. - -**Files:** -- Modify: `app/data_loader.py:215-260` -- Modify: `tests/test_data_loader.py` - -- [ ] **Step 1: Write the failing test** - -Add to `tests/test_data_loader.py`: - -```python -import io -from app import data_loader - - -def test_parse_csv_content_recognises_nspl_columns(monkeypatch): - monkeypatch.setattr(data_loader, "_lookup", {}) - nspl_csv = ( - "pcds,itl,doterm\n" - "SW1A 2AA,TLI32,\n" - "EC1A 1BB,TLI32,\n" - ) - rows = data_loader._parse_csv_content(nspl_csv, "UK") - assert rows == 2 - assert data_loader._lookup[("UK", "SW1A2AA")] == "TLI32" - assert data_loader._lookup[("UK", "EC1A1BB")] == "TLI32" -``` - -- [ ] **Step 2: Run test** - -```bash -pytest tests/test_data_loader.py::test_parse_csv_content_recognises_nspl_columns -v -``` - -Expected: FAIL — parser logs "Could not identify columns" and returns 0. - -- [ ] **Step 3: Extend the alias lists** - -In `app/data_loader.py`, modify `_parse_csv_content` (around line 232 and line 239): - -```python - # Find the postal code column - pc_col = None - for candidate in ("CODE", "PC", "POSTAL_CODE", "POSTCODE", "PC_FMT", "PCDS"): - if candidate in fieldnames: - pc_col = candidate - break - - # Find the NUTS3 column — prefer current version, never fall back to old versions - nuts3_col = None - for candidate in ( - f"NUTS3_{settings.nuts_version}", - "NUTS3", - "NUTS_ID", - "NUTS", - "ITL3CD", - "ITL3", - "ITL", - ): - if candidate in fieldnames: - nuts3_col = candidate - break -``` - -- [ ] **Step 4: Run tests** - -```bash -pytest tests/test_data_loader.py -v -``` - -Expected: PASS (new test plus all existing). - -- [ ] **Step 5: Commit** - -```bash -git add app/data_loader.py tests/test_data_loader.py -git commit -m "feat: recognise NSPL pcds/itl columns in _parse_csv_content" -``` - ---- - -## Task 5: Add `skip_terminated` flag to `_parse_csv_content` - -**Goal:** NSPL contains both live and terminated postcodes; rows with a non-blank `DOTERM` (date of termination) are skipped when the loader passes `skip_terminated=True`. Default behaviour is unchanged for TERCET. - -**Files:** -- Modify: `app/data_loader.py:215` -- Modify: `tests/test_data_loader.py` - -- [ ] **Step 1: Write the failing tests** - -Add to `tests/test_data_loader.py`: - -```python -def test_skip_terminated_filters_doterm_rows(monkeypatch): - monkeypatch.setattr(data_loader, "_lookup", {}) - nspl_csv = ( - "pcds,itl,doterm\n" - "SW1A 2AA,TLI32,\n" - "M1 9NS,TLD46,202312\n" # terminated, skip - "EC1A 1BB,TLI32,\n" - ) - rows = data_loader._parse_csv_content(nspl_csv, "UK", skip_terminated=True) - assert rows == 2 - assert ("UK", "M19NS") not in data_loader._lookup - - -def test_skip_terminated_default_false_keeps_all_rows(monkeypatch): - monkeypatch.setattr(data_loader, "_lookup", {}) - nspl_csv = ( - "pcds,itl,doterm\n" - "SW1A 2AA,TLI32,\n" - "M1 9NS,TLD46,202312\n" - ) - rows = data_loader._parse_csv_content(nspl_csv, "UK") - assert rows == 2 -``` - -- [ ] **Step 2: Run tests** - -```bash -pytest tests/test_data_loader.py -v -k skip_terminated -``` - -Expected: TypeError (unknown kwarg) on first; second passes. - -- [ ] **Step 3: Add the parameter** - -Modify `_parse_csv_content` signature and body in `app/data_loader.py`: - -```python -def _parse_csv_content( - text: str, - country_code: str, - *, - overwrite: bool = False, - skip_terminated: bool = False, -) -> int: - """Parse CSV/TSV content and populate the lookup table. Returns row count.""" - count = 0 - skipped = 0 - - # ... (existing dialect detection, fieldnames, pc_col, nuts3_col, cc_col) ... - - # Detect optional DOTERM column for live-only filtering (NSPL) - doterm_col = None - if skip_terminated: - for candidate in ("DOTERM", "DOT", "DATE_OF_TERMINATION"): - if candidate in fieldnames: - doterm_col = candidate - break - - orig_fields = list(reader.fieldnames or []) - pc_orig = orig_fields[fieldnames.index(pc_col)] - nuts3_orig = orig_fields[fieldnames.index(nuts3_col)] - cc_orig = orig_fields[fieldnames.index(cc_col)] if cc_col else None - doterm_orig = orig_fields[fieldnames.index(doterm_col)] if doterm_col else None - - # ... (existing pre-loop checks) ... - - for row in reader: - if doterm_orig and row.get(doterm_orig, "").strip(): - continue - pc = row.get(pc_orig, "") - nuts3 = row.get(nuts3_orig, "").strip() - # ... (rest unchanged) ... -``` - -- [ ] **Step 4: Run tests** - -```bash -pytest tests/test_data_loader.py -v -``` - -Expected: all PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/data_loader.py tests/test_data_loader.py -git commit -m "feat: add skip_terminated flag to filter NSPL doterm rows" -``` - ---- - -## Task 6: Add NSPL configuration - -**Goal:** Operator-controlled NSPL ZIP URL and ITL Names-and-Codes URLs. Default empty (NSPL loader is a no-op when unset). Mirrors the existing `extra_sources` / `extra_source_urls` pattern in `Settings` — plain `str` field for the env var, separately-named `@property` to parse the comma-separated value. **No `Field(alias=...)` indirection** — that interacts unreliably with `env_prefix` in pydantic-settings. - -> **Important:** Do NOT add `"UK"` to `settings.json` `countries`. That list is consumed by the GISCO loader's Strategy-2 URL guessing (`data_loader._guess_zip_urls_for_country`); adding UK there would attempt non-existent `pc{YYYY}_UK_NUTS-*.zip` URLs against GISCO on every cold load. UK loading is gated on `settings.nspl_url` being non-empty. - -**Files:** -- Modify: `app/settings.json` (add `nspl_url` only; do NOT touch `countries`) -- Modify: `app/config.py` -- Modify: `tests/test_data_loader.py` - -- [ ] **Step 1: Write the failing tests** - -Add to `tests/test_data_loader.py`: - -```python -def test_settings_expose_nspl_url_attribute(): - from app.config import settings - # Default empty when env var unset - assert settings.nspl_url == "" - - -def test_settings_expose_itl_names_urls_string(): - from app.config import settings - # Raw env-backed string, default empty - assert settings.itl_names_urls == "" - - -def test_settings_itl_names_url_list_parses_csv(): - from app.config import Settings - s = Settings(itl_names_urls="https://a/x.csv, https://b/y.csv ,") - assert s.itl_names_url_list == ["https://a/x.csv", "https://b/y.csv"] - - -def test_settings_itl_names_url_list_empty_when_unset(): - from app.config import settings - assert settings.itl_names_url_list == [] - - -def test_uk_NOT_in_settings_countries(): - """Regression guard: UK must not appear in the GISCO country list.""" - from app.config import settings - assert "UK" not in settings.countries -``` - -- [ ] **Step 2: Run tests** - -```bash -pytest tests/test_data_loader.py -v -k "nspl_url or itl_names or uk_not" -``` - -Expected: AttributeError on `nspl_url` / `itl_names_urls` / `itl_names_url_list`. The `uk_NOT` test passes already (UK was never added). - -- [ ] **Step 3: Update settings.json** - -In `app/settings.json`, add `nspl_url` next to `tercet_base_url`. **Leave the `countries` list untouched.** - -```json -{ - "tercet_base_url": "https://gisco-services.ec.europa.eu/tercet/NUTS-2024/", - "nspl_url": "", - "countries": [ - "AT", "BE", "BG", "CY", "CZ", "DE", "DK", "EE", "EL", "ES", - "FI", "FR", "HR", "HU", "IE", "IT", "LT", "LU", "LV", "MT", - "NL", "PL", "PT", "RO", "SE", "SI", "SK", - "CH", "IS", "LI", "NO", - "MK", "RS", "TR" - ], - ... -} -``` - -(Operators set `PC2NUTS_NSPL_URL` to override the empty default; `nspl_url` in `settings.json` exists only so the field has a discoverable default location.) - -- [ ] **Step 4: Update config.py** - -In `app/config.py`, inside class `Settings`, add the two fields plus the parsing property — mirror exactly the existing `extra_sources` / `extra_source_urls` pattern at lines 19 / 36-40: - -```python - # NSPL (UK postcode → ITL3) — optional, no-op when both unset - nspl_url: str = _defaults.get("nspl_url", "") - itl_names_urls: str = "" - - @property - def itl_names_url_list(self) -> list[str]: - """Parse PC2NUTS_ITL_NAMES_URLS comma-separated string into URL list.""" - if not self.itl_names_urls.strip(): - return [] - return [u.strip() for u in self.itl_names_urls.split(",") if u.strip()] -``` - -`pydantic-settings` with the existing `env_prefix = "PC2NUTS_"` automatically reads: -- `PC2NUTS_NSPL_URL` → `Settings.nspl_url` -- `PC2NUTS_ITL_NAMES_URLS` → `Settings.itl_names_urls` - -No `Field(alias=...)`, no extra imports. The property name `itl_names_url_list` (singular `url_list`) is intentionally distinct from the field name to avoid attribute shadowing. - -- [ ] **Step 5: Run tests** - -```bash -pytest tests/test_data_loader.py -v -``` - -Expected: PASS. - -- [ ] **Step 6: Commit** - -```bash -git add app/settings.json app/config.py tests/test_data_loader.py -git commit -m "feat: add NSPL URL and ITL names URLs to settings" -``` - ---- - -## Task 7: Add conditional GET to the downloader - -**Goal:** Skip re-downloading and re-parsing when the upstream ZIP hasn't changed (`If-Modified-Since` / `If-None-Match`). Benefits both TERCET and NSPL — implement once at the HTTP layer. - -**Files:** -- Modify: `app/data_loader.py:299-340` -- Modify: `tests/test_data_loader.py` - -- [ ] **Step 1: Write the failing test** - -Add to `tests/test_data_loader.py`: - -```python -import httpx - - -def test_download_zip_sends_conditional_headers_when_etag_known(tmp_path, monkeypatch): - captured = {} - - def handler(request: httpx.Request) -> httpx.Response: - captured["headers"] = dict(request.headers) - return httpx.Response(304) - - transport = httpx.MockTransport(handler) - client = httpx.Client(transport=transport) - - cached_meta = {"etag": '"abc123"', "last_modified": "Wed, 01 Jan 2025 00:00:00 GMT"} - result = data_loader._download_zip_conditional( - client, "https://example.com/foo.zip", cached_meta - ) - assert result.status_code == 304 - assert captured["headers"]["if-none-match"] == '"abc123"' - assert captured["headers"]["if-modified-since"] == "Wed, 01 Jan 2025 00:00:00 GMT" -``` - -- [ ] **Step 2: Run test** - -```bash -pytest tests/test_data_loader.py::test_download_zip_sends_conditional_headers_when_etag_known -v -``` - -Expected: AttributeError (`_download_zip_conditional` undefined). - -- [ ] **Step 3: Add the conditional download wrapper** - -In `app/data_loader.py`, alongside `_download_zip` (around line 299), add: - -```python -def _download_zip_conditional( - client: httpx.Client, url: str, cached_meta: dict -) -> httpx.Response: - """Download with conditional GET headers; returns the raw httpx.Response. - - cached_meta keys: 'etag' and 'last_modified' (either may be absent). - Caller handles 200 (re-parse), 304 (use cache), and error statuses. - """ - headers = {} - if cached_meta.get("etag"): - headers["If-None-Match"] = cached_meta["etag"] - if cached_meta.get("last_modified"): - headers["If-Modified-Since"] = cached_meta["last_modified"] - return client.get(url, headers=headers, timeout=60, follow_redirects=True) -``` - -The integration into the existing TERCET cache flow (and later into NSPL) happens in subsequent tasks; this task only adds the primitive plus its unit test. - -- [ ] **Step 4: Run tests** - -```bash -pytest tests/test_data_loader.py -v -``` - -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/data_loader.py tests/test_data_loader.py -git commit -m "feat: add conditional GET wrapper for cached ZIP downloads" -``` - ---- - -## Task 8: Implement the NSPL loader - -**Goal:** When `nspl_url` is set, download the NSPL ZIP, parse it via `_parse_csv_content` with `skip_terminated=True`, and populate `_lookup` with `("UK", normalised_postcode) → ITL3`. Failure must not raise — log and return. - -**Files:** -- Modify: `app/data_loader.py` (new function `_load_nspl`, called from `load_data`) -- Modify: `tests/test_data_loader.py` - -- [ ] **Step 1: Write the failing test** - -Add to `tests/test_data_loader.py`: - -```python -def test_load_nspl_populates_lookup_from_zip(tmp_path, monkeypatch): - import zipfile, io as _io - - monkeypatch.setattr(data_loader, "_lookup", {}) - - csv_text = ( - "pcds,itl,doterm\n" - "SW1A 2AA,TLI32,\n" - "EC1A 1BB,TLI32,\n" - "M1 9NS,TLD46,202312\n" # terminated - ) - buf = _io.BytesIO() - with zipfile.ZipFile(buf, "w") as zf: - zf.writestr("NSPL.csv", csv_text) - - def handler(request): - return httpx.Response(200, content=buf.getvalue(), headers={"ETag": '"v1"'}) - - client = httpx.Client(transport=httpx.MockTransport(handler)) - - cache_dir = tmp_path - count = data_loader._load_nspl( - client, - "https://example.com/NSPL.zip", - cache_dir, - ) - assert count == 2 - assert data_loader._lookup[("UK", "SW1A2AA")] == "TLI32" - assert ("UK", "M19NS") not in data_loader._lookup - - -def test_load_nspl_returns_zero_when_url_unset(tmp_path): - client = httpx.Client(transport=httpx.MockTransport(lambda r: httpx.Response(404))) - count = data_loader._load_nspl(client, "", tmp_path) - assert count == 0 - - -def test_load_nspl_swallows_exceptions(tmp_path): - def handler(request): - raise httpx.ConnectError("boom") - - client = httpx.Client(transport=httpx.MockTransport(handler)) - count = data_loader._load_nspl(client, "https://example.com/x.zip", tmp_path) - assert count == 0 -``` - -- [ ] **Step 2: Run tests** - -```bash -pytest tests/test_data_loader.py -v -k load_nspl -``` - -Expected: AttributeError on `_load_nspl`. - -- [ ] **Step 3: Implement `_load_nspl`** - -Add to `app/data_loader.py` (near other download helpers): - -```python -def _load_nspl(client: httpx.Client, url: str, cache_dir: Path) -> int: - """Fetch NSPL ZIP and load UK postcode → ITL3 entries into _lookup. - - Returns the number of rows added. Returns 0 when url is empty or any - error occurs — failure must not block TERCET-only operation. - """ - if not url: - return 0 - cache_path = cache_dir / "nspl.zip" - try: - content = _download_zip(client, url) - if content is None: - logger.warning("NSPL download failed (404 or unreachable): %s", url) - return 0 - if not zipfile.is_zipfile(io.BytesIO(content)): - logger.warning("NSPL response is not a valid ZIP, skipping") - return 0 - try: - cache_path.write_bytes(content) - except OSError as exc: - logger.warning("Failed to cache NSPL ZIP: %s", exc) - - total = 0 - with zipfile.ZipFile(io.BytesIO(content)) as zf: - for name in zf.namelist(): - # NSPL ships the postcode CSV under "Data/NSPL_*.csv"; accept any .csv - if not name.lower().endswith(".csv"): - continue - if "data/" not in name.lower() and "/data/" not in name.lower(): - # Skip docs/userguide CSVs that aren't the main postcode file - if "nspl" not in name.lower(): - continue - raw = zf.read(name) - for enc in ("utf-8-sig", "utf-8", "latin-1"): - try: - text = raw.decode(enc) - break - except UnicodeDecodeError: - continue - total += _parse_csv_content( - text, "UK", overwrite=False, skip_terminated=True - ) - logger.info("NSPL loaded: %d UK postcodes", total) - return total - except (httpx.HTTPError, zipfile.BadZipFile) as exc: - logger.warning("NSPL load failed: %s", exc) - return 0 -``` - -Hook it into `load_data` (around `app/data_loader.py:912-914`, just before `_download_nuts_names`): - -```python - # NSPL (UK postcodes via ITL) — optional, no-op when unset - if not timed_out and settings.nspl_url: - nspl_count = _load_nspl(client, settings.nspl_url, cache_dir) - if nspl_count > 0: - logger.info("Loaded %d entries for UK from NSPL", nspl_count) -``` - -- [ ] **Step 4: Run tests** - -```bash -pytest tests/test_data_loader.py -v -``` - -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/data_loader.py tests/test_data_loader.py -git commit -m "feat: implement NSPL loader with isolated failure handling" -``` - ---- - -## Task 9: Build the outward-code index - -**Goal:** After NSPL is loaded, build `_outward_lookup[("UK", outward)] = (majority_itl3, agreement_ratio)` for use by Tier 3.5. - -**Files:** -- Modify: `app/data_loader.py` (new `_outward_lookup` dict + `_build_outward_index` function) -- Modify: `tests/conftest.py` (extend `mock_data` fixture with UK rows + outward index build) -- Modify: `tests/test_data_loader.py` - -- [ ] **Step 1: Write the failing test** - -Add to `tests/test_data_loader.py`: - -```python -def test_build_outward_index_majority_vote(monkeypatch): - monkeypatch.setattr(data_loader, "_lookup", { - ("UK", "SW1A2AA"): "TLI32", - ("UK", "SW1A1AA"): "TLI32", - ("UK", "SW1A0AA"): "TLI31", # minority - ("UK", "M11AA"): "TLD45", - ("UK", "M11AB"): "TLD45", - }) - monkeypatch.setattr(data_loader, "_outward_lookup", {}) - - data_loader._build_outward_index("UK") - - assert data_loader._outward_lookup[("UK", "SW1A")] == ("TLI32", pytest.approx(2/3)) - assert data_loader._outward_lookup[("UK", "M1")] == ("TLD45", pytest.approx(1.0)) - - -def test_build_outward_index_skips_short_codes(monkeypatch): - monkeypatch.setattr(data_loader, "_lookup", {("UK", "AB1"): "TLC11"}) - monkeypatch.setattr(data_loader, "_outward_lookup", {}) - data_loader._build_outward_index("UK") - # Codes shorter than 4 chars after normalisation cannot be split; skip. - assert data_loader._outward_lookup == {} -``` - -- [ ] **Step 2: Run tests** - -```bash -pytest tests/test_data_loader.py -v -k build_outward -``` - -Expected: AttributeError on `_outward_lookup` / `_build_outward_index`. - -- [ ] **Step 3: Implement the index** - -In `app/data_loader.py`, add near the other module-level state (around line 41): - -```python -# Outward-code index for Tier 3.5: (country_code, outward) -> (nuts3, agreement_ratio) -_outward_lookup: dict[tuple[str, str], tuple[str, float]] = {} -``` - -Add the build function (near `_build_prefix_index`): - -```python -def _build_outward_index(country_code: str) -> None: - """Populate _outward_lookup for one country using majority vote per outward code. - - Outward = full normalised postcode minus the last 3 chars (UK convention). - Codes shorter than 4 chars are skipped (no meaningful split possible). - """ - groups: dict[str, list[str]] = {} - for (cc, code), nuts3 in _lookup.items(): - if cc != country_code: - continue - if len(code) < 4: - continue - outward = code[:-3] - groups.setdefault(outward, []).append(nuts3) - - for outward, nuts3_list in groups.items(): - counts = Counter(nuts3_list) - winner, count = counts.most_common(1)[0] - agreement = count / len(nuts3_list) - _outward_lookup[(country_code, outward)] = (winner, agreement) -``` - -Call it from `load_data` after the NSPL loader hook from Task 8: - -```python - if not timed_out and settings.nspl_url: - nspl_count = _load_nspl(client, settings.nspl_url, cache_dir) - if nspl_count > 0: - logger.info("Loaded %d entries for UK from NSPL", nspl_count) - _build_outward_index("UK") -``` - -Update `tests/conftest.py` to (a) include UK mock rows and (b) call `_build_outward_index` in the `mock_data` fixture: - -```python -MOCK_LOOKUP = { - # ... existing entries ... - ("UK", "SW1A2AA"): "TLI32", - ("UK", "SW1A1AA"): "TLI32", - ("UK", "EC1A1BB"): "TLI32", - ("UK", "M11AA"): "TLD45", -} - -# Inside mock_data fixture, after _build_prefix_index(): -data_loader._outward_lookup.clear() -data_loader._build_outward_index("UK") - -# In teardown, restore _outward_lookup similarly to other globals. -``` - -- [ ] **Step 4: Run tests** - -```bash -pytest tests/ -v -``` - -Expected: all PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/data_loader.py tests/conftest.py tests/test_data_loader.py -git commit -m "feat: build outward-code majority-vote index for Tier 3.5" -``` - ---- - -## Task 10: Add `GB → UK` country alias - -**Goal:** Accept ISO 3166-1 `GB` as input and normalise it to the canonical `UK` used internally and by NSPL. - -**Files:** -- Modify: `app/data_loader.py:66-69` -- Modify: `tests/test_data_loader.py` -- Modify: `tests/test_api.py` - -- [ ] **Step 1: Write the failing tests** - -Add to `tests/test_data_loader.py`: - -```python -def test_normalize_country_maps_gb_to_uk(): - assert data_loader.normalize_country("GB") == "UK" - assert data_loader.normalize_country("gb") == "UK" - - -def test_normalize_country_preserves_existing_aliases(): - assert data_loader.normalize_country("GR") == "EL" - assert data_loader.normalize_country("UK") == "UK" - assert data_loader.normalize_country("AT") == "AT" -``` - -Add to `tests/test_api.py`: - -```python -def test_lookup_accepts_gb_alias(client, mock_data): - resp_uk = client.get("/lookup", params={"country": "UK", "postal_code": "SW1A 2AA"}) - resp_gb = client.get("/lookup", params={"country": "GB", "postal_code": "SW1A 2AA"}) - assert resp_uk.status_code == 200 - assert resp_gb.status_code == 200 - assert resp_uk.json() == resp_gb.json() -``` - -- [ ] **Step 2: Run tests** - -```bash -pytest tests/ -v -k "gb_alias or maps_gb" -``` - -Expected: failures (`UK` returned as `GB`, lookups not equivalent). - -- [ ] **Step 3: Update `normalize_country`** - -In `app/data_loader.py`: - -```python -def normalize_country(country_code: str) -> str: - """Normalize a country code: uppercase + map ISO/non-canonical aliases. - - GR → EL (ISO vs GISCO convention) - GB → UK (ISO vs NSPL/internal convention) - """ - cc = country_code.strip().upper() - if cc == "GR": - return "EL" - if cc == "GB": - return "UK" - return cc -``` - -- [ ] **Step 4: Run tests** - -```bash -pytest tests/ -v -``` - -Expected: all PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/data_loader.py tests/test_data_loader.py tests/test_api.py -git commit -m "feat: alias GB to UK for ISO 3166-1 input compatibility" -``` - ---- - -## Task 11: Add Tier 3.5 outward-code lookup to `lookup()` - -**Goal:** When Tier 1 (exact) misses for UK, try the outward-code index before falling through to Tier 3 (prefix). Confidence uses the medium-tier numbers from `settings.json` (NUTS1=0.90, NUTS2=0.80, NUTS3=0.70). - -**Files:** -- Modify: `app/data_loader.py:968-1028` -- Modify: `tests/test_data_loader.py` -- Modify: `tests/test_api.py` - -- [ ] **Step 1: Write the failing tests** - -Add to `tests/test_data_loader.py`: - -```python -def test_tier_3_5_outward_lookup_for_uk(mock_data): - # mock_data populates UK postcodes; SW1A is in TLI32 majority. - result = data_loader.lookup("UK", "SW1A") - assert result is not None - assert result["nuts3"] == "TLI32" - assert result["match_type"] == "estimated" - assert result["nuts1_confidence"] == pytest.approx(0.90) - assert result["nuts2_confidence"] == pytest.approx(0.80) - assert result["nuts3_confidence"] == pytest.approx(0.70) - - -def test_tier_3_5_falls_through_for_unknown_outward(mock_data): - result = data_loader.lookup("UK", "ZZ99") - # No outward match; downstream tiers will also miss → None - assert result is None -``` - -Add to `tests/test_api.py`: - -```python -def test_uk_outward_only_input_returns_estimated(client, mock_data): - resp = client.get("/lookup", params={"country": "UK", "postal_code": "SW1A"}) - assert resp.status_code == 200 - body = resp.json() - assert body["match_type"] == "estimated" - assert body["nuts3"] == "TLI32" -``` - -- [ ] **Step 2: Run tests** - -```bash -pytest tests/ -v -k "tier_3_5 or outward_only_input" -``` - -Expected: failures — lookup returns None or 404 for outward-only input. - -- [ ] **Step 3: Insert Tier 3.5 in `lookup()`** - -In `app/data_loader.py`, modify `lookup` (the function around line 968). Insert between Tier 3 and Tier 4: - -```python -def lookup(country_code: str, postal_code: str) -> dict | None: - """... (existing docstring + add: 3.5. Outward-code lookup (UK) ...)""" - from app.postal_patterns import extract_outward, extract_postal_code - - cc = normalize_country(country_code) - extracted = extract_postal_code(cc, postal_code) - key = (cc, extracted) - - # Tier 1: Exact TERCET match - nuts3 = _lookup.get(key) - if nuts3 is not None: - return _build_result("exact", nuts3) - - # Tier 2: Pre-computed estimate - est = _estimates.get(key) - if est is not None: - return _build_result( - "estimated", est["nuts3"], - nuts1=est["nuts1"], nuts2=est["nuts2"], - nuts1_confidence=est["nuts1_confidence"], - nuts2_confidence=est["nuts2_confidence"], - nuts3_confidence=est["nuts3_confidence"], - ) - - # Tier 3: Runtime prefix-based estimation - approx = _estimate_by_prefix(cc, extracted) - if approx is not None: - return approx - - # Tier 3.5: Outward-code lookup (UK and any other country with outward_only) - outward = extract_outward(cc, postal_code) - if outward is not None: - outward_hit = _outward_lookup.get((cc, outward)) - if outward_hit is not None: - nuts3, _agreement = outward_hit - conf = settings.confidence_map["medium"] - return _build_result( - "estimated", - nuts3, - nuts1_confidence=conf["nuts1"], - nuts2_confidence=conf["nuts2"], - nuts3_confidence=conf["nuts3"], - ) - - # Tier 4: Country-level majority vote (existing) - fallback = _country_fallback.get(cc) - if fallback is not None: - return _build_result( - "approximate", fallback["nuts3"], - nuts1=fallback["nuts1"], nuts2=fallback["nuts2"], - nuts1_confidence=fallback["nuts1_confidence"], - nuts2_confidence=fallback["nuts2_confidence"], - nuts3_confidence=fallback["nuts3_confidence"], - ) - - # Tier 5: Single-NUTS3 country fallback (existing) - nuts3 = _single_nuts3.get(cc) - if nuts3 is not None: - return _build_result("estimated", nuts3) - - return None -``` - -- [ ] **Step 4: Run tests** - -```bash -pytest tests/ -v -``` - -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/data_loader.py tests/test_data_loader.py tests/test_api.py -git commit -m "feat: add Tier 3.5 outward-code lookup to lookup waterfall" -``` - ---- - -## Task 12: Tag UK results with `code_system="ITL"` - -**Goal:** `_build_result` and `lookup` know which scheme each entry belongs to. Simplest implementation: derive at result-construction time from country code (UK → ITL, everything else → NUTS), since UK is the only ITL source today. Forwarded to the `NUTSResult` model in `app/main.py`. - -**Files:** -- Modify: `app/data_loader.py` (extend `_build_result`) -- Modify: `app/main.py:194-207` (forward `code_system`) -- Modify: `tests/test_data_loader.py` -- Modify: `tests/test_api.py` - -- [ ] **Step 1: Write the failing tests** - -Add to `tests/test_data_loader.py`: - -```python -def test_lookup_result_includes_code_system_itl_for_uk(mock_data): - result = data_loader.lookup("UK", "SW1A 2AA") - assert result["code_system"] == "ITL" - - -def test_lookup_result_includes_code_system_nuts_for_at(mock_data): - result = data_loader.lookup("AT", "1010") - assert result["code_system"] == "NUTS" -``` - -Add to `tests/test_api.py`: - -```python -def test_uk_lookup_response_has_code_system_itl(client, mock_data): - resp = client.get("/lookup", params={"country": "UK", "postal_code": "SW1A 2AA"}) - assert resp.status_code == 200 - assert resp.json()["code_system"] == "ITL" -``` - -- [ ] **Step 2: Run tests** - -```bash -pytest tests/ -v -k "code_system" -``` - -Expected: failures (`code_system` absent from result dict; default "NUTS" returned for UK). - -- [ ] **Step 3: Plumb `code_system` through** - -In `app/data_loader.py`, change `_build_result` to accept a `code_system` (default `"NUTS"`): - -```python -def _build_result( - match_type: str, - nuts3: str, - nuts1: str = "", - nuts2: str = "", - code_system: str = "NUTS", - **confidence, -) -> dict: - n1 = nuts1 or nuts3[:3] - n2 = nuts2 or nuts3[:4] - return { - "code_system": code_system, - "match_type": match_type, - "nuts1": n1, - "nuts1_confidence": confidence.get("nuts1_confidence", 1.0), - "nuts2": n2, - "nuts2_confidence": confidence.get("nuts2_confidence", 1.0), - "nuts3": nuts3, - "nuts3_confidence": confidence.get("nuts3_confidence", 1.0), - **_resolve_names(n1, n2, nuts3), - } -``` - -In `lookup()`, derive `code_system` once and pass it to every `_build_result` call: - -```python -def lookup(country_code: str, postal_code: str) -> dict | None: - from app.postal_patterns import extract_outward, extract_postal_code - - cc = normalize_country(country_code) - code_system = "ITL" if cc == "UK" else "NUTS" - extracted = extract_postal_code(cc, postal_code) - key = (cc, extracted) - - # ... at every `return _build_result(...)`, add `code_system=code_system,` to the kwargs ... -``` - -(Touch every tier's return statement: Tier 1, 2, 3 via `_estimate_by_prefix`, 3.5, 4, 5. For Tier 3, pass `code_system` into `_estimate_by_prefix` too — see snippet below.) - -```python -def _estimate_by_prefix(country_code: str, postal_code: str, code_system: str = "NUTS") -> dict | None: - # ... existing body ... - return _build_result("approximate", nuts3, nuts1=n1, nuts2=n2, code_system=code_system, ...) -``` - -In `app/main.py`, modify `lookup_postal_code` to forward `code_system` from the result dict: - -```python - return NUTSResult( - postal_code=postal_code, - country_code=cc, - code_system=result.get("code_system", "NUTS"), - **{k: v for k, v in result.items() if k != "code_system"}, - ) -``` - -(Or simpler: rely on `**result` plus pydantic ignoring extras — but pydantic v2 raises on extras unless `model_config = {"extra": "ignore"}`. Either set that on `NUTSResult` or do the explicit forward as above.) - -- [ ] **Step 4: Run tests** - -```bash -pytest tests/ -v -``` - -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/data_loader.py app/main.py tests/test_data_loader.py tests/test_api.py -git commit -m "feat: tag UK lookups with code_system=ITL through the waterfall" -``` - ---- - -## Task 13: Implement ITL names CSV loader - -**Goal:** Download three ONS Names-and-Codes CSVs (one per ITL level), parse `(code, name)` pairs, insert into `_nuts_names`. The current `_NUTS3_RE` already accepts TL-prefixed codes so name resolution in `_resolve_names` will work without changes. - -**Files:** -- Modify: `app/data_loader.py` (new `_load_itl_names`) -- Modify: `tests/test_data_loader.py` - -- [ ] **Step 1: Write the failing test** - -Add to `tests/test_data_loader.py`: - -```python -def test_load_itl_names_populates_nuts_names(monkeypatch): - monkeypatch.setattr(data_loader, "_nuts_names", {}) - - def handler(request): - body = ( - "ITL321CD,ITL321NM\n" - "TLI32,Tower Hamlets\n" - "TLI31,Hackney and Newham\n" - ) - return httpx.Response(200, content=body.encode()) - - client = httpx.Client(transport=httpx.MockTransport(handler)) - count = data_loader._load_itl_names(client, ["https://example.com/itl3.csv"]) - assert count == 2 - assert data_loader._nuts_names["TLI32"] == "Tower Hamlets" - - -def test_load_itl_names_empty_url_list_no_op(): - client = httpx.Client(transport=httpx.MockTransport(lambda r: httpx.Response(404))) - assert data_loader._load_itl_names(client, []) == 0 -``` - -- [ ] **Step 2: Run tests** - -```bash -pytest tests/test_data_loader.py -v -k load_itl_names -``` - -Expected: AttributeError on `_load_itl_names`. - -- [ ] **Step 3: Implement** - -In `app/data_loader.py`, near `_download_nuts_names`: - -```python -_ITL_CODE_RE = re.compile(r"ITL\d?(?:CD)?$", re.IGNORECASE) -_ITL_NAME_RE = re.compile(r"ITL\d?(?:NM)?$", re.IGNORECASE) - - -def _load_itl_names(client: httpx.Client, urls: list[str]) -> int: - """Fetch ONS ITL Names-and-Codes CSVs and merge into _nuts_names. - - Each CSV has paired columns like ITL321CD/ITL321NM (level 3), - ITL221CD/ITL221NM (level 2), ITL121CD/ITL121NM (level 1). Column - names vary by release year — match by suffix (CD / NM). - """ - if not urls: - return 0 - total = 0 - for url in urls: - try: - resp = client.get(url, timeout=30, follow_redirects=True) - resp.raise_for_status() - text = resp.text - except httpx.HTTPError as exc: - logger.warning("ITL names fetch failed for %s: %s", url, exc) - continue - try: - reader = csv.DictReader(io.StringIO(text)) - fieldnames = [f.strip() for f in (reader.fieldnames or [])] - code_col = next( - (f for f in fieldnames if f.upper().endswith("CD") and "ITL" in f.upper()), - None, - ) - name_col = next( - (f for f in fieldnames if f.upper().endswith("NM") and "ITL" in f.upper()), - None, - ) - if not code_col or not name_col: - logger.warning("Could not find ITL CD/NM columns in %s; headers=%s", url, fieldnames) - continue - for row in reader: - code = (row.get(code_col) or "").strip().upper() - name = (row.get(name_col) or "").strip() - if code and name: - _nuts_names[code] = name - total += 1 - except (csv.Error, KeyError) as exc: - logger.warning("ITL names parse failed for %s: %s", url, exc) - logger.info("ITL names loaded: %d entries from %d URLs", total, len(urls)) - return total -``` - -Hook it into `load_data` after `_load_nspl`: - -```python - if not timed_out and settings.itl_names_url_list: - _load_itl_names(client, settings.itl_names_url_list) -``` - -- [ ] **Step 4: Run tests** - -```bash -pytest tests/ -v -``` - -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/data_loader.py tests/test_data_loader.py -git commit -m "feat: load ITL region names from ONS Names-and-Codes CSVs" -``` - ---- - -## Task 14: Verify NSPL failure isolation - -**Goal:** End-to-end test: when NSPL is unreachable, TERCET data still loads and the service continues to serve. (Most of this isolation is already implicit in Task 8's design — `_load_nspl` swallows exceptions. This task adds an explicit regression test.) - -**Files:** -- Modify: `tests/test_data_loader.py` - -- [ ] **Step 1: Write the test** - -Add to `tests/test_data_loader.py`: - -```python -def test_nspl_failure_does_not_block_tercet(tmp_path, monkeypatch): - """If NSPL fails, TERCET data must still be served.""" - monkeypatch.setattr(data_loader, "_lookup", {("AT", "1010"): "AT130"}) - - # Simulate NSPL endpoint that always errors - def handler(request): - raise httpx.ConnectError("ons unavailable") - - client = httpx.Client(transport=httpx.MockTransport(handler)) - nspl_count = data_loader._load_nspl(client, "https://ons.invalid/nspl.zip", tmp_path) - assert nspl_count == 0 - - # AT lookup must still work - result = data_loader.lookup("AT", "1010") - assert result is not None - assert result["nuts3"] == "AT130" -``` - -- [ ] **Step 2: Run test** - -```bash -pytest tests/test_data_loader.py::test_nspl_failure_does_not_block_tercet -v -``` - -Expected: PASS already (Task 8's implementation swallows the error). If it fails, the implementation needs the broader exception clause shown in Task 8. - -- [ ] **Step 3: Commit (test-only)** - -```bash -git add tests/test_data_loader.py -git commit -m "test: confirm NSPL failure does not block TERCET serving" -``` - ---- - -## Task 15: README and documentation updates - -**Goal:** Surface the new feature, the divergence note, the new tier, the new config, and the OGL v3.0 attribution. - -**Files:** -- Modify: `README.md` (multiple sections) - -- [ ] **Step 1: Update the Coverage section** - -Insert after the "EU candidate countries" subsection (around `README.md:18`): - -```markdown -**United Kingdom** (via the ONS NSPL dataset, mapped to ITL — International Territorial Level): -United Kingdom (UK; ISO `GB` accepted as alias). - -ITL is the UK's territorial classification published by ONS, succeeding NUTS for UK statistical geography after Brexit. ITL diverges from NUTS 2016 UK at L2 and L3 (41 vs 40 ITL2 regions; 179 vs 174 ITL3 regions). The bidirectional NUTS↔ITL lookups previously published by ONS were discontinued in 2023. Responses for UK lookups carry `code_system: "ITL"` so consumers can branch correctly when comparing against historical NUTS-UK data. -``` - -- [ ] **Step 2: Update the response example** - -Modify the existing AT example response (around `README.md:69-84`) to include the new field: - -```json -{ - "postal_code": "A-1010", - "country_code": "AT", - "code_system": "NUTS", - "match_type": "exact", - ... -} -``` - -Add a UK example below the existing examples: - -```markdown -**Example — UK postcode (ITL):** - -`GET /lookup?country=UK&postal_code=SW1A%202AA` - -```json -{ - "postal_code": "SW1A 2AA", - "country_code": "UK", - "code_system": "ITL", - "match_type": "exact", - "nuts1": "TLI", - "nuts1_name": "London", - "nuts1_confidence": 1.0, - "nuts2": "TLI3", - "nuts2_name": "Inner London - East", - "nuts2_confidence": 1.0, - "nuts3": "TLI32", - "nuts3_name": "Tower Hamlets", - "nuts3_confidence": 1.0 -} -``` -``` - -- [ ] **Step 3: Rename "Five-tier lookup" → "Six-tier lookup" and add Tier 3.5** - -Insert between Tier 3 and Tier 4 sections: - -```markdown -### Tier 3.5: Outward-code lookup (`match_type: "estimated"`) — UK only - -Triggered when: -- The input postcode is shorter than a full UK postcode (no inward portion), OR -- Tier 3 prefix approximation finds no match. - -Looks up `(country, outward_code)` in a precomputed majority-vote index built at NSPL load time. The outward code for UK is everything before the last 3 characters of the normalised postcode (e.g. `SW1A` for `SW1A 2AA`). Confidence uses the medium tier (NUTS1 0.90 / NUTS2 0.80 / NUTS3 0.70) because one outward code can span two adjacent ITL3 regions in dense urban areas. -``` - -- [ ] **Step 4: Add the supported-patterns row for UK** - -Add to the "Supported patterns" table (around `README.md:266-301`), in alphabetical position after `TR`: - -```markdown -| UK | 1-2 letters + digit + optional letter/digit + optional space + digit + 2 letters | — | `SW1A 2AA`, `EC1A 1BB`, `M1 1AA`, `B33 8TH`, `SW1A` (outward only) | -``` - -- [ ] **Step 5: Add new env vars to the Configuration table** - -Add to the configuration table: - -```markdown -| `PC2NUTS_NSPL_URL` | *(empty)* | URL to the latest NSPL ZIP from the ONS Open Geography Portal. When unset, UK is unsupported. | -| `PC2NUTS_ITL_NAMES_URLS` | *(empty)* | Comma-separated list of ONS "Names and Codes" CSV URLs (one per ITL level). Loaded after NSPL. | -``` - -- [ ] **Step 6: Add OGL v3.0 attribution to Data source** - -Replace the existing single-source line at the end of the Data source section with: - -```markdown -[GISCO TERCET flat files](https://ec.europa.eu/eurostat/web/gisco/geodata/administrative-units/postal-codes) ([download](https://gisco-services.ec.europa.eu/tercet/flat-files)), © European Union – GISCO, licensed [CC-BY-SA 4.0](https://creativecommons.org/licenses/by-sa/4.0/). - -UK postcode data: [ONS National Statistics Postcode Lookup (NSPL)](https://geoportal.statistics.gov.uk/), © Crown copyright, licensed [Open Government Licence v3.0](https://www.nationalarchives.gov.uk/doc/open-government-licence/version/3/). Contains public sector information licensed under the OGL v3.0. -``` - -- [ ] **Step 7: Document Crown Dependencies / Gibraltar exclusion** - -Add a one-paragraph note near the UK coverage section: - -```markdown -**Out of scope:** Crown Dependencies (Jersey JE, Guernsey GG, Isle of Man IM) and Gibraltar (GI) use UK-style postcode formats but are not in ITL geography or NSPL, and are not currently supported. Lookups for these country codes return a 400 (unsupported country). -``` - -- [ ] **Step 8: Update "Adding a new country"** - -Add a note at the end of that section: - -```markdown -> Countries served via a non-GISCO source (currently only UK via NSPL) require a separate loader path and additional configuration (URLs for the source ZIP and any names files). See `_load_nspl` and `_load_itl_names` in `app/data_loader.py` for the NSPL precedent. -``` - -- [ ] **Step 9: Commit** - -```bash -git add README.md -git commit -m "docs: document UK/ITL support, six-tier waterfall, OGL attribution" -``` - ---- - -## Self-review checklist (post-plan) - -| Spec section | Implemented by | Notes | -|--------------|----------------|-------| -| §3 Architecture (parallel data channel) | Task 8, Task 14 | NSPL loader called from `load_data`; failure isolated. | -| §4 NSPL URL / config | Task 6 | `nspl_url`, `itl_names_urls` (string field) + `itl_names_url_list` (parsing property). Mirrors `extra_sources` precedent — no `Field(alias=...)`. | -| §4 Conditional GET | Task 7 | Wrapper added; integration into TERCET cache flow can be a follow-up if needed (the wrapper is in place). | -| §4 doterm filter | Task 5 | Flag in `_parse_csv_content`. | -| §4 NSPL column aliases | Task 4 | `PCDS`, `ITL`, `ITL3`, `ITL3CD`. | -| §4 ITL names | Task 13 | `_load_itl_names` with paired CD/NM column matching. | -| §4 Shared TTL | (no code change) | Inherits existing `db_cache_ttl_days`. | -| §5 Tier 3.5 | Task 3, Task 9, Task 11 | `extract_outward`, `_outward_lookup`, lookup waterfall insert. | -| §6 `code_system` field | Task 1, Task 12 | Model + plumbing. | -| §7 Configuration changes | Tasks 1, 2, 6, 12 | All listed files touched. | -| §7 `patterns_version` bump | Task 2 | `1.0 → 1.1`. | -| §7 GB→UK alias | Task 10 | Implemented in `normalize_country`. | -| §8 Documentation | Task 15 | README sections updated. | -| §9 Operational impact | (verify in deployment) | Plan does not include an explicit benchmark task; if needed, add measurement to Task 8/9 PR description. | -| §10 Out of scope | Task 15 (documented) | Code-level rejection for JE/GG/IM/GI is the existing default behaviour. | - -**No placeholders detected.** All steps include concrete code, exact commands, and explicit expected output. - -**Type/name consistency:** -- `extract_outward` (not `extract_outward_code`) used consistently in Tasks 3, 11. -- `_outward_lookup` (not `_outward_index`) used consistently in Tasks 9, 11. -- `_load_nspl` and `_load_itl_names` consistent in Tasks 8, 13. -- `code_system` literal values exactly `"NUTS"` and `"ITL"` everywhere. - -**Scope:** focused single feature; ~15 tasks; bounded by the spec. No decomposition needed. diff --git a/docs/superpowers/plans/2026-04-29-auth-token-bypass.md b/docs/superpowers/plans/2026-04-29-auth-token-bypass.md deleted file mode 100644 index cfc14c0..0000000 --- a/docs/superpowers/plans/2026-04-29-auth-token-bypass.md +++ /dev/null @@ -1,1157 +0,0 @@ -# Auth-token bypass implementation plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Allow trusted callers to bypass the per-IP `60/minute` rate limit by presenting `Authorization: Bearer `. Tokens are managed via the `PC2NUTS_TRUSTED_TOKENS` env var (comma-separated, restart to apply). Implements [#60](https://github.com/bk86a/PostalCode2NUTS/issues/60) per [the design spec](../specs/2026-04-29-auth-token-bypass-design.md). - -**Architecture:** Single new module `app/auth.py` containing token parsing, timing-safe validation, audit-id derivation, a `contextvars`-backed predicate, and the auth middleware. `app/main.py` registers the middleware and threads `exempt_when=is_trusted_request` into the existing `@limiter.limit(...)` decorators; `AccessLogMiddleware` gains a `token_id=` field. No changes to `data_loader.py` or settings.json schema beyond a new env-var parsing property in `config.py`. - -**Tech Stack:** FastAPI / Starlette middleware, slowapi `exempt_when` predicate, `hmac.compare_digest` for timing-safe comparison, `hashlib.sha256` for the audit id, `contextvars.ContextVar` for per-request state in the parameterless `exempt_when` callable. - ---- - -## File map - -| File | Status | Responsibility | -|---|---|---| -| `app/auth.py` | **new** (~110 LOC) | `extract_bearer`, `is_trusted`, `token_id`, `_request_var` ContextVar, `is_trusted_request` predicate, `AuthMiddleware` | -| `app/config.py` | modify | new `trusted_tokens` property: parse `PC2NUTS_TRUSTED_TOKENS` → `frozenset[str]` | -| `app/main.py` | modify | register `AuthMiddleware`; pass `exempt_when=is_trusted_request` to two `@limiter.limit(...)` decorators; extend `AccessLogMiddleware` log line with `token_id=<8hex>` for trusted requests | -| `tests/test_auth.py` | **new** | unit tests for every helper in `app/auth.py` | -| `tests/test_api.py` | modify | endpoint tests for the four behaviour rows in spec §4; audit log assertions | -| `tests/conftest.py` | modify | new `trusted_client` fixture parameterising `settings.trusted_tokens` | -| `README.md` | modify | new "Authentication & rate-limit bypass" section, including the operator runbook from spec §2 | -| `CHANGELOG.md` | modify | new `[Unreleased]` entry under `### Added` | - ---- - -## Task 1: `trusted_tokens` settings property - -**Files:** -- Modify: `app/config.py` -- Test: `tests/test_auth.py` (new file — start with this section) - -- [ ] **Step 1: Create `tests/test_auth.py` with the failing config test** - -```python -"""Tests for app/auth.py and trusted_tokens config parsing.""" - -import os -from importlib import reload - -import pytest - - -# ── trusted_tokens config property ─────────────────────────────────────────── - - -class TestTrustedTokensConfig: - @pytest.fixture(autouse=True) - def _isolate_env(self, monkeypatch): - # Clear so each test sets only what it needs - monkeypatch.delenv("PC2NUTS_TRUSTED_TOKENS", raising=False) - - def _reload_settings(self): - from app import config - - reload(config) - return config.settings - - def test_unset_returns_empty_frozenset(self): - settings = self._reload_settings() - assert settings.trusted_tokens == frozenset() - - def test_empty_string_returns_empty_frozenset(self, monkeypatch): - monkeypatch.setenv("PC2NUTS_TRUSTED_TOKENS", "") - settings = self._reload_settings() - assert settings.trusted_tokens == frozenset() - - def test_single_token(self, monkeypatch): - monkeypatch.setenv("PC2NUTS_TRUSTED_TOKENS", "abc123") - settings = self._reload_settings() - assert settings.trusted_tokens == frozenset({"abc123"}) - - def test_multiple_tokens_comma_separated(self, monkeypatch): - monkeypatch.setenv("PC2NUTS_TRUSTED_TOKENS", "abc,def,ghi") - settings = self._reload_settings() - assert settings.trusted_tokens == frozenset({"abc", "def", "ghi"}) - - def test_whitespace_stripped(self, monkeypatch): - monkeypatch.setenv("PC2NUTS_TRUSTED_TOKENS", " abc , def ,ghi ") - settings = self._reload_settings() - assert settings.trusted_tokens == frozenset({"abc", "def", "ghi"}) - - def test_empty_entries_dropped(self, monkeypatch): - monkeypatch.setenv("PC2NUTS_TRUSTED_TOKENS", "abc,,def,") - settings = self._reload_settings() - assert settings.trusted_tokens == frozenset({"abc", "def"}) - - def test_duplicates_deduplicated(self, monkeypatch): - monkeypatch.setenv("PC2NUTS_TRUSTED_TOKENS", "abc,abc,def") - settings = self._reload_settings() - assert settings.trusted_tokens == frozenset({"abc", "def"}) - - def test_returns_frozenset_not_list(self, monkeypatch): - monkeypatch.setenv("PC2NUTS_TRUSTED_TOKENS", "abc") - settings = self._reload_settings() - assert isinstance(settings.trusted_tokens, frozenset) -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestTrustedTokensConfig -v` -Expected: All 8 tests fail with `AttributeError: 'Settings' object has no attribute 'trusted_tokens'`. - -- [ ] **Step 3: Add the property to `app/config.py`** - -1. Add `Field` to the existing pydantic import at the top of `app/config.py`. The current file has `from pydantic_settings import BaseSettings`. Add a sibling import: - -```python -from pydantic import Field -``` - -2. Inside the `Settings` class, add the env-backed field next to the other field declarations (e.g. just below `extra_sources: str = ""`): - -```python - trusted_tokens_raw: str = Field(default="", validation_alias="PC2NUTS_TRUSTED_TOKENS") -``` - -`validation_alias` overrides the `env_prefix`-derived name, so the env var read at startup is `PC2NUTS_TRUSTED_TOKENS` (not `PC2NUTS_TRUSTED_TOKENS_RAW`). No change to `model_config` is required — `validation_alias` works without `populate_by_name`. - -3. Insert the property after the existing `extra_source_urls` property: - -```python - @property - def trusted_tokens(self) -> frozenset[str]: - """Parse PC2NUTS_TRUSTED_TOKENS comma-separated list into a frozenset. - - Whitespace around tokens is stripped; empty entries are dropped. - Returns an empty frozenset when unset or empty (auth bypass disabled). - """ - raw = self.trusted_tokens_raw - if not raw.strip(): - return frozenset() - return frozenset(t.strip() for t in raw.split(",") if t.strip()) -``` - -- [ ] **Step 4: Run tests — verify all 8 pass** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestTrustedTokensConfig -v` -Expected: 8 passed. - -- [ ] **Step 5: Run full suite — verify no regression** - -Run: `.venv/bin/python -m pytest tests/ -q` -Expected: All previous tests still pass plus 8 new ones. - -- [ ] **Step 6: Commit** - -```bash -git add app/config.py tests/test_auth.py -git commit -m "feat(auth): trusted_tokens settings property (#60)" -``` - ---- - -## Task 2: `token_id` helper - -**Files:** -- Create: `app/auth.py` -- Test: `tests/test_auth.py` - -- [ ] **Step 1: Append the failing test to `tests/test_auth.py`** - -```python -# ── token_id helper ────────────────────────────────────────────────────────── - - -class TestTokenId: - def test_deterministic(self): - from app.auth import token_id - - assert token_id("hello") == token_id("hello") - - def test_eight_hex_chars(self): - from app.auth import token_id - - result = token_id("hello") - assert len(result) == 8 - assert all(c in "0123456789abcdef" for c in result) - - def test_distinct_inputs_distinct_outputs(self): - from app.auth import token_id - - assert token_id("alpha") != token_id("beta") - - def test_known_value(self): - """sha256('hello').hexdigest() = '2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824' — first 8: '2cf24dba'.""" - from app.auth import token_id - - assert token_id("hello") == "2cf24dba" -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestTokenId -v` -Expected: All 4 fail with `ModuleNotFoundError: No module named 'app.auth'`. - -- [ ] **Step 3: Create `app/auth.py` with `token_id` only** - -```python -"""Token-based rate-limit bypass for /lookup and /pattern. - -See docs/superpowers/specs/2026-04-29-auth-token-bypass-design.md for the design. -""" - -import hashlib - - -def token_id(token: str) -> str: - """First 8 hex chars of sha256(token). Stable, non-reversible audit id.""" - return hashlib.sha256(token.encode()).hexdigest()[:8] -``` - -- [ ] **Step 4: Run tests — verify all 4 pass** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestTokenId -v` -Expected: 4 passed. - -- [ ] **Step 5: Commit** - -```bash -git add app/auth.py tests/test_auth.py -git commit -m "feat(auth): token_id audit helper (#60)" -``` - ---- - -## Task 3: `extract_bearer` helper - -**Files:** -- Modify: `app/auth.py` -- Test: `tests/test_auth.py` - -- [ ] **Step 1: Append the failing tests** - -```python -# ── extract_bearer helper ──────────────────────────────────────────────────── - - -class TestExtractBearer: - def _request_with_header(self, value: str | None): - """Build a minimal Starlette Request with the given Authorization header.""" - from starlette.requests import Request - - headers = [] - if value is not None: - headers.append((b"authorization", value.encode())) - scope = { - "type": "http", - "method": "GET", - "path": "/", - "headers": headers, - "query_string": b"", - } - return Request(scope) - - def test_no_header_returns_none(self): - from app.auth import extract_bearer - - request = self._request_with_header(None) - assert extract_bearer(request) is None - - def test_bearer_returns_token(self): - from app.auth import extract_bearer - - request = self._request_with_header("Bearer my-token") - assert extract_bearer(request) == "my-token" - - def test_lowercase_scheme_accepted(self): - from app.auth import extract_bearer - - request = self._request_with_header("bearer my-token") - assert extract_bearer(request) == "my-token" - - def test_basic_scheme_raises_400(self): - from fastapi import HTTPException - - from app.auth import extract_bearer - - request = self._request_with_header("Basic dXNlcjpwYXNz") - with pytest.raises(HTTPException) as exc_info: - extract_bearer(request) - assert exc_info.value.status_code == 400 - - def test_bearer_with_no_value_raises_400(self): - from fastapi import HTTPException - - from app.auth import extract_bearer - - request = self._request_with_header("Bearer ") - with pytest.raises(HTTPException) as exc_info: - extract_bearer(request) - assert exc_info.value.status_code == 400 - - def test_bearer_with_extra_words_raises_400(self): - from fastapi import HTTPException - - from app.auth import extract_bearer - - request = self._request_with_header("Bearer foo bar") - with pytest.raises(HTTPException) as exc_info: - extract_bearer(request) - assert exc_info.value.status_code == 400 - - def test_just_bearer_raises_400(self): - from fastapi import HTTPException - - from app.auth import extract_bearer - - request = self._request_with_header("Bearer") - with pytest.raises(HTTPException) as exc_info: - extract_bearer(request) - assert exc_info.value.status_code == 400 -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestExtractBearer -v` -Expected: All 7 fail with `ImportError: cannot import name 'extract_bearer' from 'app.auth'`. - -- [ ] **Step 3: Add `extract_bearer` to `app/auth.py`** - -Append at end of file: - -```python -from fastapi import HTTPException -from starlette.requests import Request - - -def extract_bearer(request: Request) -> str | None: - """Return the bearer token from the Authorization header, or None if absent. - - Raises HTTPException(400) when the header is present but malformed: - wrong scheme, missing value, or extra whitespace-separated tokens. - """ - header = request.headers.get("authorization") - if header is None: - return None - parts = header.split(" ") - if len(parts) != 2 or parts[0].lower() != "bearer" or not parts[1]: - raise HTTPException(status_code=400, detail="malformed Authorization header") - return parts[1] -``` - -- [ ] **Step 4: Run tests — verify all 7 pass** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestExtractBearer -v` -Expected: 7 passed. - -- [ ] **Step 5: Commit** - -```bash -git add app/auth.py tests/test_auth.py -git commit -m "feat(auth): extract_bearer header parser (#60)" -``` - ---- - -## Task 4: `is_trusted` helper (timing-safe) - -**Files:** -- Modify: `app/auth.py` -- Test: `tests/test_auth.py` - -- [ ] **Step 1: Append the failing tests** - -```python -# ── is_trusted helper ──────────────────────────────────────────────────────── - - -class TestIsTrusted: - @pytest.fixture(autouse=True) - def _empty_tokens(self, monkeypatch): - # Stub settings.trusted_tokens for each test - from app import auth, config - - monkeypatch.setattr(config.settings, "__dict__", config.settings.__dict__.copy()) - # Note: trusted_tokens is a property; stub via a new Settings-ish object - # by patching the module-level reference auth uses. - self._monkeypatch = monkeypatch - self._auth = auth - - def _set_tokens(self, *tokens): - # Patch auth's view of settings.trusted_tokens - self._monkeypatch.setattr( - self._auth, "_get_trusted_tokens", lambda: frozenset(tokens) - ) - - def test_match_against_single_token(self): - self._set_tokens("abc") - assert self._auth.is_trusted("abc") is True - - def test_match_against_one_of_many(self): - self._set_tokens("abc", "def", "ghi") - assert self._auth.is_trusted("def") is True - - def test_unknown_token_returns_false(self): - self._set_tokens("abc", "def") - assert self._auth.is_trusted("xyz") is False - - def test_empty_string_returns_false(self): - self._set_tokens("abc") - assert self._auth.is_trusted("") is False - - def test_empty_token_set_returns_false(self): - self._set_tokens() - assert self._auth.is_trusted("anything") is False -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestIsTrusted -v` -Expected: All 5 fail (`is_trusted` and `_get_trusted_tokens` not defined yet). - -- [ ] **Step 3: Add to `app/auth.py`** - -Append after `extract_bearer`: - -```python -import hmac - -from app.config import settings - - -def _get_trusted_tokens() -> frozenset[str]: - """Indirection seam for tests; returns settings.trusted_tokens at call time.""" - return settings.trusted_tokens - - -def is_trusted(candidate: str) -> bool: - """Constant-time membership test against the configured trusted tokens. - - Uses hmac.compare_digest in a loop. Returns False for empty input or when - no tokens are configured. - """ - if not candidate: - return False - return any(hmac.compare_digest(candidate, t) for t in _get_trusted_tokens()) -``` - -- [ ] **Step 4: Run tests — verify all 5 pass** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestIsTrusted -v` -Expected: 5 passed. - -- [ ] **Step 5: Commit** - -```bash -git add app/auth.py tests/test_auth.py -git commit -m "feat(auth): is_trusted timing-safe membership test (#60)" -``` - ---- - -## Task 5: Auth middleware (short-circuits 400/401, marks request.state) - -**Files:** -- Modify: `app/auth.py` -- Test: `tests/test_auth.py` - -- [ ] **Step 1: Append the failing tests** - -```python -# ── AuthMiddleware ─────────────────────────────────────────────────────────── - - -class TestAuthMiddleware: - def _build_app(self, *trusted: str): - """Build a minimal Starlette app with AuthMiddleware mounted.""" - from starlette.applications import Starlette - from starlette.responses import JSONResponse - from starlette.routing import Route - - from app import auth - - async def endpoint(request): - return JSONResponse( - { - "trusted": getattr(request.state, "trusted", False), - "token_id": getattr(request.state, "token_id", None), - } - ) - - app = Starlette(routes=[Route("/x", endpoint)]) - app.add_middleware(auth.AuthMiddleware) - # Patch the trusted-token getter for the duration of this app - auth._get_trusted_tokens = lambda: frozenset(trusted) - return app - - def test_no_header_marks_untrusted(self): - from starlette.testclient import TestClient - - app = self._build_app("good-token") - with TestClient(app) as client: - resp = client.get("/x") - assert resp.status_code == 200 - assert resp.json() == {"trusted": False, "token_id": None} - - def test_valid_token_marks_trusted_with_token_id(self): - from starlette.testclient import TestClient - - from app.auth import token_id - - app = self._build_app("good-token") - with TestClient(app) as client: - resp = client.get("/x", headers={"Authorization": "Bearer good-token"}) - assert resp.status_code == 200 - assert resp.json() == {"trusted": True, "token_id": token_id("good-token")} - - def test_invalid_token_returns_401(self): - from starlette.testclient import TestClient - - app = self._build_app("good-token") - with TestClient(app) as client: - resp = client.get("/x", headers={"Authorization": "Bearer wrong-token"}) - assert resp.status_code == 401 - assert "invalid token" in resp.json()["detail"].lower() - - def test_malformed_header_returns_400(self): - from starlette.testclient import TestClient - - app = self._build_app("good-token") - with TestClient(app) as client: - resp = client.get("/x", headers={"Authorization": "Basic abc"}) - assert resp.status_code == 400 -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestAuthMiddleware -v` -Expected: All 4 fail (`AttributeError: module 'app.auth' has no attribute 'AuthMiddleware'`). - -- [ ] **Step 3: Add `AuthMiddleware` to `app/auth.py`** - -Append: - -```python -import contextvars - -from starlette.middleware.base import BaseHTTPMiddleware -from starlette.responses import JSONResponse - - -_request_var: contextvars.ContextVar[Request | None] = contextvars.ContextVar( - "pc2nuts_request", default=None -) - - -class AuthMiddleware(BaseHTTPMiddleware): - """Validate Authorization header up-front. - - - No header → request.state.trusted = False, normal flow. - - Valid bearer token → request.state.trusted = True, request.state.token_id set. - - Invalid token → 401 short-circuit (no rate-limit consumed). - - Malformed header → 400 short-circuit. - - /health is exempt entirely — header is ignored, request flows through. - This protects monitoring tooling from false 401s if a service mesh adds - an Authorization header globally. - - Also stores the Request in a ContextVar so the parameterless slowapi - exempt_when callable can read it (slowapi calls exempt_when()). - """ - - EXEMPT_PATHS = frozenset({"/health"}) - - async def dispatch(self, request: Request, call_next): - if request.url.path in self.EXEMPT_PATHS: - request.state.trusted = False - request.state.token_id = None - return await call_next(request) - - try: - token = extract_bearer(request) - except HTTPException as exc: - return JSONResponse({"detail": exc.detail}, status_code=exc.status_code) - - if token is not None: - if not is_trusted(token): - return JSONResponse( - {"detail": "invalid token"}, status_code=401 - ) - request.state.trusted = True - request.state.token_id = token_id(token) - else: - request.state.trusted = False - request.state.token_id = None - - ctx_token = _request_var.set(request) - try: - return await call_next(request) - finally: - _request_var.reset(ctx_token) -``` - -- [ ] **Step 4: Run tests — verify all 4 pass** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestAuthMiddleware -v` -Expected: 4 passed. - -- [ ] **Step 5: Commit** - -```bash -git add app/auth.py tests/test_auth.py -git commit -m "feat(auth): AuthMiddleware (400/401 short-circuit + state) (#60)" -``` - ---- - -## Task 6: `is_trusted_request` slowapi predicate - -**Files:** -- Modify: `app/auth.py` -- Test: `tests/test_auth.py` - -- [ ] **Step 1: Append the failing tests** - -```python -# ── is_trusted_request predicate ───────────────────────────────────────────── - - -class TestIsTrustedRequest: - def _request_with_state(self, *, trusted: bool): - from starlette.requests import Request - - scope = { - "type": "http", - "method": "GET", - "path": "/", - "headers": [], - "query_string": b"", - "state": {}, - } - request = Request(scope) - request.state.trusted = trusted - return request - - def test_no_request_in_context_returns_false(self): - from app.auth import _request_var, is_trusted_request - - # Ensure no request set - assert _request_var.get() is None - assert is_trusted_request() is False - - def test_trusted_state_returns_true(self): - from app.auth import _request_var, is_trusted_request - - request = self._request_with_state(trusted=True) - token = _request_var.set(request) - try: - assert is_trusted_request() is True - finally: - _request_var.reset(token) - - def test_untrusted_state_returns_false(self): - from app.auth import _request_var, is_trusted_request - - request = self._request_with_state(trusted=False) - token = _request_var.set(request) - try: - assert is_trusted_request() is False - finally: - _request_var.reset(token) -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestIsTrustedRequest -v` -Expected: All 3 fail (`is_trusted_request` not defined). - -- [ ] **Step 3: Add the predicate to `app/auth.py`** - -Append: - -```python -def is_trusted_request() -> bool: - """Parameterless predicate for slowapi's exempt_when. - - Reads the current Request from the ContextVar set by AuthMiddleware and - returns True iff request.state.trusted is True. Returns False outside - a request context (defensive default). - """ - request = _request_var.get() - if request is None: - return False - return bool(getattr(request.state, "trusted", False)) -``` - -- [ ] **Step 4: Run tests — verify all 3 pass** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestIsTrustedRequest -v` -Expected: 3 passed. - -- [ ] **Step 5: Commit** - -```bash -git add app/auth.py tests/test_auth.py -git commit -m "feat(auth): is_trusted_request slowapi exempt_when predicate (#60)" -``` - ---- - -## Task 7: Wire auth into FastAPI app - -**Files:** -- Modify: `app/main.py` -- Test: `tests/test_api.py`, `tests/conftest.py` - -- [ ] **Step 1: Add a `trusted_client` fixture to `tests/conftest.py`** - -Append after the existing `client` fixture: - -```python -@pytest.fixture() -def trusted_client(mock_data, monkeypatch): - """TestClient with one configured trusted token: 'test-token-aaa'.""" - from unittest.mock import patch - - from app import auth, data_loader - - monkeypatch.setattr(auth, "_get_trusted_tokens", lambda: frozenset({"test-token-aaa"})) - - from fastapi.testclient import TestClient - - with patch.object(data_loader, "load_data"): - from app.main import app - - with TestClient(app) as tc: - yield tc -``` - -- [ ] **Step 2: Append failing endpoint tests to `tests/test_api.py`** - -Append at end of file: - -```python -# ── Auth-token bypass tests (#60) ──────────────────────────────────────────── - - -class TestAuthBypass: - def test_no_header_normal_flow(self, trusted_client): - """Without an Authorization header, behaviour is unchanged.""" - resp = trusted_client.get( - "/lookup", params={"postal_code": "10115", "country": "DE"} - ) - assert resp.status_code == 200 - - def test_valid_token_returns_200(self, trusted_client): - resp = trusted_client.get( - "/lookup", - params={"postal_code": "10115", "country": "DE"}, - headers={"Authorization": "Bearer test-token-aaa"}, - ) - assert resp.status_code == 200 - - def test_invalid_token_returns_401(self, trusted_client): - resp = trusted_client.get( - "/lookup", - params={"postal_code": "10115", "country": "DE"}, - headers={"Authorization": "Bearer wrong-token"}, - ) - assert resp.status_code == 401 - assert "invalid token" in resp.json()["detail"].lower() - - def test_malformed_header_returns_400(self, trusted_client): - resp = trusted_client.get( - "/lookup", - params={"postal_code": "10115", "country": "DE"}, - headers={"Authorization": "Basic dXNlcjpwYXNz"}, - ) - assert resp.status_code == 400 - - def test_health_anonymous_works(self, trusted_client): - resp = trusted_client.get("/health") - assert resp.status_code == 200 - - def test_health_ignores_invalid_token(self, trusted_client): - """/health is in AuthMiddleware.EXEMPT_PATHS — header is ignored entirely. - Protects monitoring tools that may inject auth headers globally.""" - resp = trusted_client.get( - "/health", headers={"Authorization": "Bearer wrong-token"} - ) - assert resp.status_code == 200 - - def test_health_ignores_malformed_header(self, trusted_client): - resp = trusted_client.get( - "/health", headers={"Authorization": "Basic dXNlcjpwYXNz"} - ) - assert resp.status_code == 200 -``` - -- [ ] **Step 3: Run the new tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_api.py::TestAuthBypass -v` -Expected: All 6 fail (auth not yet wired into app — no AuthMiddleware mounted). - -- [ ] **Step 4: Wire auth into `app/main.py`** - -In `app/main.py`: - -1. Add import at top with other app imports: - -```python -from app.auth import AuthMiddleware, is_trusted_request -``` - -2. Add the middleware after the existing `AccessLogMiddleware` registration. Order matters — AuthMiddleware must run **before** the slowapi limiter (i.e. it must be added last so it's the outermost wrapper, since Starlette runs middlewares in reverse-add order): - -Replace: - -```python -app.add_middleware(AccessLogMiddleware) -``` - -with: - -```python -app.add_middleware(AccessLogMiddleware) -app.add_middleware(AuthMiddleware) -``` - -3. Pass `exempt_when` to both `@limiter.limit(...)` decorators. Replace: - -```python -@limiter.limit(settings.rate_limit) -def lookup_postal_code( -``` - -with: - -```python -@limiter.limit(settings.rate_limit, exempt_when=is_trusted_request) -def lookup_postal_code( -``` - -And replace: - -```python -@limiter.limit(settings.rate_limit) -def get_pattern( -``` - -with: - -```python -@limiter.limit(settings.rate_limit, exempt_when=is_trusted_request) -def get_pattern( -``` - -- [ ] **Step 5: Run new tests — verify all 6 pass** - -Run: `.venv/bin/python -m pytest tests/test_api.py::TestAuthBypass -v` -Expected: 6 passed. - -- [ ] **Step 6: Run full suite — verify no regressions** - -Run: `.venv/bin/python -m pytest tests/ -q` -Expected: all passing (existing 78 + new 27 = 105 total approximate; exact count depends on shared utilities). - -- [ ] **Step 7: Commit** - -```bash -git add app/main.py tests/conftest.py tests/test_api.py -git commit -m "feat(auth): wire AuthMiddleware + exempt_when on /lookup and /pattern (#60)" -``` - ---- - -## Task 8: Audit log extension - -**Files:** -- Modify: `app/main.py` -- Test: `tests/test_api.py` - -- [ ] **Step 1: Append the failing log-format test** - -```python - def test_audit_log_includes_token_id_for_trusted(self, trusted_client, caplog): - from app.auth import token_id - - with caplog.at_level("INFO", logger="app.access"): - trusted_client.get( - "/lookup", - params={"postal_code": "10115", "country": "DE"}, - headers={"Authorization": "Bearer test-token-aaa"}, - ) - log_text = " ".join(r.message for r in caplog.records if r.name == "app.access") - assert f"token_id={token_id('test-token-aaa')}" in log_text - - def test_audit_log_omits_token_id_for_anonymous(self, trusted_client, caplog): - with caplog.at_level("INFO", logger="app.access"): - trusted_client.get( - "/lookup", params={"postal_code": "10115", "country": "DE"} - ) - log_text = " ".join(r.message for r in caplog.records if r.name == "app.access") - assert "token_id=" not in log_text -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_api.py::TestAuthBypass::test_audit_log_includes_token_id_for_trusted tests/test_api.py::TestAuthBypass::test_audit_log_omits_token_id_for_anonymous -v` -Expected: First fails (missing `token_id=` in log), second passes incidentally. - -- [ ] **Step 3: Extend `AccessLogMiddleware` in `app/main.py`** - -Replace the existing `AccessLogMiddleware.dispatch` body (around lines 64-76): - -```python - async def dispatch(self, request: Request, call_next) -> Response: - start = time.monotonic() - response = await call_next(request) - duration_ms = (time.monotonic() - start) * 1000 - token_suffix = "" - tid = getattr(request.state, "token_id", None) - if tid: - token_suffix = f" token_id={tid}" - access_logger.info( - "%s %s %s %d %.1fms%s", - request.client.host if request.client else "-", - request.method, - request.url.path, - response.status_code, - duration_ms, - token_suffix, - ) - return response -``` - -Note: `AuthMiddleware` runs *after* `AccessLogMiddleware` (later add → outer), so the access logger runs *inside* the auth scope and `request.state.token_id` is available. If the order ends up reversed in practice (manifested by the test failing), swap the `add_middleware` order in `main.py` so `AuthMiddleware` is added before `AccessLogMiddleware`. - -- [ ] **Step 4: Run new tests — verify both pass** - -Run: `.venv/bin/python -m pytest tests/test_api.py::TestAuthBypass -v` -Expected: 8 passed (the original 6 + 2 new log assertions). - -- [ ] **Step 5: Run full suite** - -Run: `.venv/bin/python -m pytest tests/ -q` -Expected: all passing. - -- [ ] **Step 6: Commit** - -```bash -git add app/main.py tests/test_api.py -git commit -m "feat(auth): audit log token_id field for trusted requests (#60)" -``` - ---- - -## Task 9: README documentation - -**Files:** -- Modify: `README.md` - -No tests for this task (documentation-only). - -- [ ] **Step 1: Add the new section to `README.md`** - -Locate the `## Configuration` section (around line 303). Insert a new section **after** the Configuration table and **before** `## Five-tier lookup`. Find this anchor line: - -``` -## Five-tier lookup -``` - -Insert immediately above it: - -````markdown -## Authentication & rate-limit bypass - -The service applies a per-IP rate limit (`60/minute` by default) to `/lookup` and `/pattern`. Trusted callers — operator-issued, manually distributed — can bypass this limit by presenting an `Authorization: Bearer ` header. `/health` stays anonymous. - -### Configuration - -| Env var | Default | Purpose | -|---|---|---| -| `PC2NUTS_TRUSTED_TOKENS` | `""` (unset) | Comma-separated list of valid bypass tokens. Empty → bypass disabled (current default). Whitespace and empty entries between commas are tolerated. | - -### Operator runbook — issue a token - -```bash -# 1. Generate locally (48 hex chars / 192 bits) -openssl rand -hex 24 - -# 2. Add the printed token to PC2NUTS_TRUSTED_TOKENS in the production -# deployment's environment configuration. Multiple tokens are comma-separated. -# Example value with two active tokens: -# PC2NUTS_TRUSTED_TOKENS=9e7a3f...d2,4b1c8e...77 - -# 3. Restart the service container to load the new env value. -# The SQLite postal-code cache survives the restart, so cold-start is ~30 s. - -# 4. Verify the new token bypasses the rate limit: -curl -i -H "Authorization: Bearer " \ - "https:///lookup?country=DE&postal_code=10115" -# → 200, audit log shows token_id= - -# 5. Hand the raw token to the consumer over a confidential channel -# (1Password, Signal, encrypted email — not Slack, not GitHub issues). -``` - -### Operator runbook — revoke a token - -```bash -# 1. Remove the token entry from PC2NUTS_TRUSTED_TOKENS in the env config. -# 2. Restart the container. -# 3. Verify the revoked token is rejected: -curl -i -H "Authorization: Bearer " \ - "https:///lookup?country=DE&postal_code=10115" -# → 401 Unauthorized -``` - -### Operator runbook — find the token id of a logged request - -```bash -echo -n "" | sha256sum | cut -c1-8 -``` - -### Behaviour summary - -| Request | Result | -|---|---| -| No `Authorization` header | Per-IP `60/minute` cap, normal `200` / `429` | -| `Authorization: Bearer ` | Rate limit fully bypassed; `token_id=<8hex>` appended to access log | -| `Authorization: Bearer ` | `401 Unauthorized` | -| `Authorization: ` or malformed | `400 Bad Request` | - -### Disable the bypass entirely - -Unset or empty `PC2NUTS_TRUSTED_TOKENS`. All traffic falls back to the per-IP cap. No code change needed. - -### Security notes - -- Tokens are **bearer credentials** — anyone holding the string can use the API at full rate. Treat them like passwords. -- Always send tokens over HTTPS. Never accept a bearer token over plain HTTP. -- Log lines contain only the 8-char SHA-256 prefix. Token values never appear in logs. -- Token comparison is constant-time (`hmac.compare_digest`). -- Revocation latency is bounded by container restart time (~30 s). - -```` - -- [ ] **Step 2: Add the env var to the existing Configuration table** - -Locate the configuration table (starts around line 305 after `All settings are overridable via environment variables prefixed with `PC2NUTS_`:`). Add a new row at an appropriate alphabetical position: - -```markdown -| `PC2NUTS_TRUSTED_TOKENS` | `""` (empty — bypass disabled) | Comma-separated list of opaque tokens that bypass the per-IP rate limit when sent via `Authorization: Bearer `. See [Authentication & rate-limit bypass](#authentication--rate-limit-bypass) for the operator runbook. | -``` - -- [ ] **Step 3: Commit** - -```bash -git add README.md -git commit -m "docs: auth-token bypass operator runbook (#60)" -``` - ---- - -## Task 10: CHANGELOG entry - -**Files:** -- Modify: `CHANGELOG.md` - -- [ ] **Step 1: Add new `[Unreleased]` section to `CHANGELOG.md`** - -If there is no `[Unreleased]` section above the current top entry (currently `[0.15.0] - 2026-04-29`), add one. Replace: - -```markdown -The format is based on [Keep a Changelog](https://keepachangelog.com/). - -## [0.15.0] - 2026-04-29 -``` - -with: - -```markdown -The format is based on [Keep a Changelog](https://keepachangelog.com/). - -## [Unreleased] - -### Added - -- **Auth-token bypass** (#60): trusted callers can bypass the per-IP rate limit by presenting `Authorization: Bearer `. Tokens are managed via the new `PC2NUTS_TRUSTED_TOKENS` comma-separated env var. Invalid tokens return `401`; malformed `Authorization` headers return `400`. Audit lines log a non-reversible 8-char SHA-256 prefix only — token values never appear in logs. See README "Authentication & rate-limit bypass" for the operator runbook. - -## [0.15.0] - 2026-04-29 -``` - -- [ ] **Step 2: Final check — full suite + lint + format** - -Run: - -```bash -.venv/bin/python -m pytest tests/ -q -.venv/bin/ruff check app/ scripts/ -.venv/bin/ruff format --check app/ scripts/ -``` - -Expected: all pass / clean. - -- [ ] **Step 3: Commit** - -```bash -git add CHANGELOG.md -git commit -m "docs: changelog entry for auth-token bypass (#60)" -``` - ---- - -## Task 11: Push, close issue, release - -**Files:** none (git/gh operations). - -- [ ] **Step 1: Push to origin** - -```bash -git push origin main -``` - -- [ ] **Step 2: Close #60 with a summary comment** - -```bash -gh issue close 60 -c "Implemented in $(git log --grep '#60' --format=%h | head -1) (and following commits). - -Behaviour: -- Authorization: Bearer → bypass rate limit, audit log with token_id -- Authorization: Bearer → 401 -- Authorization: → 400 -- No header → existing per-IP cap unchanged - -Operator runbook in README → Authentication & rate-limit bypass." -``` - -- [ ] **Step 3: Tag and release** - -After confirming CI is green on main: - -```bash -gh release create v0.16.0 \ - --title "v0.16.0 — Auth-token bypass" \ - --notes "$(cat <<'EOF' -### What's new - -- **Auth-token bypass** (#60): trusted callers can bypass the per-IP rate limit by presenting \`Authorization: Bearer \`. Tokens are managed via the new \`PC2NUTS_TRUSTED_TOKENS\` comma-separated env var; restart applies changes. Invalid tokens return \`401\`; malformed \`Authorization\` headers return \`400\`. Audit lines log a non-reversible 8-char SHA-256 prefix; token values never appear in logs. - -See [README → Authentication & rate-limit bypass](https://github.com/bk86a/PostalCode2NUTS/blob/main/README.md#authentication--rate-limit-bypass) for the operator runbook (issue / verify / revoke / disable). - -Closes #60. -EOF -)" -``` - -- [ ] **Step 4: Move CHANGELOG `[Unreleased]` to `[0.16.0] - 2026-04-29`** - -```bash -sed -i 's/^## \[Unreleased\]$/## [0.16.0] - 2026-04-29/' CHANGELOG.md -git add CHANGELOG.md -git commit -m "chore: cut v0.16.0" -git push origin main -``` diff --git a/docs/superpowers/plans/2026-04-29-db-backed-trusted-tokens.md b/docs/superpowers/plans/2026-04-29-db-backed-trusted-tokens.md deleted file mode 100644 index 6a9dc2e..0000000 --- a/docs/superpowers/plans/2026-04-29-db-backed-trusted-tokens.md +++ /dev/null @@ -1,1620 +0,0 @@ -# DB-backed trusted-tokens implementation plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Move trusted-token storage from `PC2NUTS_TRUSTED_TOKENS` env var (v0.16.0 / v1) to a managed SQLite-compatible HTTP database (v2), preserving the env var as a disaster-recovery fallback. Add an operator CLI for issuance, listing, and revocation. Implements [#61](https://github.com/bk86a/PostalCode2NUTS/issues/61) per [the design spec](../specs/2026-04-29-db-backed-trusted-tokens-design.md). - -**Architecture:** New `app/token_db.py` is a thin HTTP client over the configured database service. `app/auth.py`'s `_get_trusted_tokens()` returns the union of an in-memory frozenset (refreshed from the DB every ~60 s by a background task) with the existing `PC2NUTS_TRUSTED_TOKENS` env-var set. New `scripts/tokens.py` CLI gives operators `init` / `add` / `list` / `revoke` subcommands. DB unreachable at startup is non-fatal — service serves anonymous traffic; bypass falls back to the env var. - -**Tech Stack:** Python 3.12, FastAPI / Starlette / asyncio, `httpx` (already a project dependency), `sqlite3`-compatible HTTP API on the database side, stdlib `argparse` + `secrets` + `hashlib` for the CLI, pytest with `respx` (or hand-rolled mock transport) for HTTP testing. - ---- - -## File map - -| File | Status | Responsibility | -|---|---|---| -| `app/token_db.py` | **new** (~120 LOC) | `TokenDBError`, `TokenDB(url)` class with `init_schema`, `add`, `list_active`, `list_all`, `revoke`. Pure I/O — no business logic. | -| `app/config.py` | modify | Add `token_db_url`, `token_refresh_seconds` settings. | -| `app/auth.py` | modify | Module-level `_db_tokens: frozenset[str]`, `_token_db_stale: bool`. New `refresh_db_tokens()` function. `_get_trusted_tokens()` returns `_db_tokens \| settings.trusted_tokens`. | -| `app/main.py` | modify | In `lifespan`, when `PC2NUTS_TOKEN_DB_URL` is set: do an initial refresh, then spawn an `asyncio.create_task` background loop. Cancel cleanly on shutdown. | -| `app/models.py` | modify | Add optional `token_db_stale: bool \| None = None` field to `HealthResponse`. | -| `scripts/tokens.py` | **new** (~180 LOC) | argparse-based CLI: `init`, `add`, `list`, `revoke`. | -| `tests/test_token_db.py` | **new** | Unit tests for `TokenDB` (mocked HTTP). | -| `tests/test_tokens_cli.py` | **new** | Unit tests for the CLI (subprocess-style or argparse harness). | -| `tests/test_auth.py` | modify | New `TestDBTokens` class covering union, refresh, DB-down scenarios. | -| `tests/test_api.py` | modify | One test for `/health` `token_db_stale` field. | -| `tests/conftest.py` | unchanged | (Existing `mock_data` and `trusted_client` fixtures don't need changes.) | -| `README.md` | modify | Replace v1 runbook subsections with v2 (CLI commands), add union-semantic note. | -| `CHANGELOG.md` | modify | New `[Unreleased]` `### Added` entry. | - -### A note on the database HTTP wire-protocol - -The implementer **MUST consult the configured database provider's HTTP API documentation** before writing `TokenDB.execute()`. This plan assumes a generic shape: - -```http -POST {db_url}/query -Content-Type: application/json -Authorization: - -{"sql": "SELECT * FROM trusted_tokens WHERE revoked_at IS NULL", "params": []} -``` - -Response (assumed): - -```json -{"rows": [{"id": 1, "value": "...", "label": "...", "created_at": "..."}], - "lastInsertRowId": null, - "rowsAffected": 0} -``` - -If the provider's actual shape differs (different path, different field names, GET-with-querystring instead of POST-with-body, etc.), adjust `TokenDB.execute()` accordingly. **The unit tests mock the HTTP boundary**, so they do not depend on the exact wire shape — only the public method signatures matter. Integration testing against a real DB instance is a manual verification step at Task 11. - ---- - -## Task 1: Add `token_db_url` and `token_refresh_seconds` settings - -**Files:** -- Modify: `app/config.py` -- Test: `tests/test_token_db.py` (NEW — start with this test class only) - -- [ ] **Step 1: Create `tests/test_token_db.py` with the failing config test** - -```python -"""Tests for app/token_db.py and the token DB settings.""" - -from importlib import reload - -import pytest - - -# ── Settings ───────────────────────────────────────────────────────────────── - - -class TestTokenDBSettings: - @pytest.fixture(autouse=True) - def _isolate_env(self, monkeypatch): - monkeypatch.delenv("PC2NUTS_TOKEN_DB_URL", raising=False) - monkeypatch.delenv("PC2NUTS_TOKEN_REFRESH_SECONDS", raising=False) - - def _reload_settings(self): - from app import config - - reload(config) - return config.settings - - def test_token_db_url_default_empty(self): - settings = self._reload_settings() - assert settings.token_db_url == "" - - def test_token_db_url_from_env(self, monkeypatch): - monkeypatch.setenv("PC2NUTS_TOKEN_DB_URL", "https://db.example/v1") - settings = self._reload_settings() - assert settings.token_db_url == "https://db.example/v1" - - def test_token_refresh_seconds_default_60(self): - settings = self._reload_settings() - assert settings.token_refresh_seconds == 60 - - def test_token_refresh_seconds_from_env(self, monkeypatch): - monkeypatch.setenv("PC2NUTS_TOKEN_REFRESH_SECONDS", "5") - settings = self._reload_settings() - assert settings.token_refresh_seconds == 5 -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_token_db.py::TestTokenDBSettings -v` -Expected: 4 fail with `AttributeError: 'Settings' object has no attribute 'token_db_url'` (or similar). - -- [ ] **Step 3: Add the settings to `app/config.py`** - -Find the existing `Settings` class field declarations (after `trusted_tokens_raw`). Add: - -```python - token_db_url: str = "" - token_refresh_seconds: int = 60 -``` - -`env_prefix = "PC2NUTS_"` already maps these to `PC2NUTS_TOKEN_DB_URL` / `PC2NUTS_TOKEN_REFRESH_SECONDS` automatically — no `Field(validation_alias=...)` needed. - -- [ ] **Step 4: Run tests — verify all 4 pass** - -Run: `.venv/bin/python -m pytest tests/test_token_db.py::TestTokenDBSettings -v` -Expected: 4 passed. - -- [ ] **Step 5: Run full suite** - -Run: `.venv/bin/python -m pytest tests/ -q` -Expected: 126 passed (122 prior + 4 new). - -- [ ] **Step 6: Commit** - -```bash -git add app/config.py tests/test_token_db.py -git commit -m "feat(token-db): config settings for DB URL + refresh interval (#61)" -``` - ---- - -## Task 2: TokenDB module skeleton — exception, __init__, execute - -**Files:** -- Create: `app/token_db.py` -- Test: `tests/test_token_db.py` - -This task introduces the HTTP boundary. The unit tests mock `httpx.Client.post`, so the wire-protocol shape is encapsulated in `TokenDB.execute` and can be adjusted later without touching the higher-level methods. - -- [ ] **Step 1: Append the failing tests to `tests/test_token_db.py`** - -```python -# ── TokenDB.__init__ + execute ─────────────────────────────────────────────── - - -class TestTokenDBInit: - def test_init_stores_url(self): - from app.token_db import TokenDB - - db = TokenDB("https://db.example/v1") - assert db.url == "https://db.example/v1" - - def test_init_strips_trailing_slash(self): - from app.token_db import TokenDB - - db = TokenDB("https://db.example/v1/") - assert db.url == "https://db.example/v1" - - -class TestTokenDBExecute: - def _mock_response(self, monkeypatch, *, json_body: dict, status_code: int = 200): - from app import token_db - - class _Response: - status_code = 200 - - def __init__(self, body, code=200): - self._body = body - self.status_code = code - - def json(self): - return self._body - - def raise_for_status(self): - if self.status_code >= 400: - import httpx - raise httpx.HTTPStatusError( - "boom", request=None, response=self - ) - - captured: dict = {} - - def _post(self, url, json=None, headers=None, timeout=None): - captured["url"] = url - captured["json"] = json - captured["headers"] = headers - return _Response(json_body, status_code) - - import httpx - - monkeypatch.setattr(httpx.Client, "post", _post) - return captured - - def test_execute_sends_post_with_sql_and_params(self, monkeypatch): - from app.token_db import TokenDB - - captured = self._mock_response(monkeypatch, json_body={"rows": [], "rowsAffected": 0}) - db = TokenDB("https://db.example/v1") - db.execute("SELECT * FROM t WHERE id = ?", [42]) - - assert captured["json"] == {"sql": "SELECT * FROM t WHERE id = ?", "params": [42]} - - def test_execute_returns_rows(self, monkeypatch): - from app.token_db import TokenDB - - self._mock_response( - monkeypatch, - json_body={"rows": [{"id": 1, "label": "a"}], "rowsAffected": 0}, - ) - db = TokenDB("https://db.example/v1") - rows = db.execute("SELECT id, label FROM t") - assert rows == [{"id": 1, "label": "a"}] - - def test_execute_no_params_sends_empty_list(self, monkeypatch): - from app.token_db import TokenDB - - captured = self._mock_response(monkeypatch, json_body={"rows": [], "rowsAffected": 0}) - db = TokenDB("https://db.example/v1") - db.execute("CREATE TABLE x (id INTEGER)") - assert captured["json"]["params"] == [] - - def test_execute_http_error_raises_token_db_error(self, monkeypatch): - from app.token_db import TokenDB, TokenDBError - - self._mock_response(monkeypatch, json_body={"error": "boom"}, status_code=500) - db = TokenDB("https://db.example/v1") - with pytest.raises(TokenDBError): - db.execute("SELECT 1") -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_token_db.py::TestTokenDBInit tests/test_token_db.py::TestTokenDBExecute -v` -Expected: All fail with `ModuleNotFoundError: No module named 'app.token_db'`. - -- [ ] **Step 3: Create `app/token_db.py` with skeleton + execute** - -```python -"""Thin HTTP client for the configured SQLite-compatible managed database. - -See docs/superpowers/specs/2026-04-29-db-backed-trusted-tokens-design.md. - -The wire shape (POST /query with {sql, params}, response {rows, rowsAffected, -lastInsertRowId}) is the working assumption; adjust `_post` if the configured -provider differs. -""" - -from __future__ import annotations - -from typing import Any - -import httpx - - -class TokenDBError(Exception): - """Raised when the token database returns an error or is unreachable.""" - - -class TokenDB: - """Minimal HTTP client over a SQLite-compatible managed database. - - All methods are blocking. Callers running inside an asyncio loop should - wrap calls in asyncio.to_thread(). - """ - - def __init__(self, url: str) -> None: - self.url = url.rstrip("/") - - def execute(self, sql: str, params: list[Any] | None = None) -> list[dict]: - """Send a SQL statement. Returns the `rows` list from the response. - - Writes (INSERT/UPDATE/DELETE/CREATE) typically return an empty list. - """ - payload = {"sql": sql, "params": list(params) if params else []} - try: - with httpx.Client(timeout=10) as client: - resp = client.post(f"{self.url}/query", json=payload, headers={}) - resp.raise_for_status() - body = resp.json() - except (httpx.HTTPError, ValueError) as exc: - raise TokenDBError(f"DB request failed: {exc}") from exc - return body.get("rows") or [] -``` - -- [ ] **Step 4: Run tests — verify all 6 pass** - -Run: `.venv/bin/python -m pytest tests/test_token_db.py::TestTokenDBInit tests/test_token_db.py::TestTokenDBExecute -v` -Expected: 6 passed. - -- [ ] **Step 5: Run full suite** - -Run: `.venv/bin/python -m pytest tests/ -q` -Expected: 132 passed (126 prior + 6 new). - -- [ ] **Step 6: Commit** - -```bash -git add app/token_db.py tests/test_token_db.py -git commit -m "feat(token-db): TokenDB skeleton + execute (#61)" -``` - ---- - -## Task 3: TokenDB query methods — init_schema, add, list_active, list_all, revoke - -**Files:** -- Modify: `app/token_db.py` -- Test: `tests/test_token_db.py` - -Each query method is a thin wrapper around `execute`. The tests mock `execute` directly (not the HTTP layer) so they verify the SQL and parameter list are correct. - -- [ ] **Step 1: Append the failing tests** - -```python -# ── TokenDB query methods ──────────────────────────────────────────────────── - - -class TestTokenDBMethods: - @pytest.fixture - def db(self, monkeypatch): - from app.token_db import TokenDB - - captured: list[tuple[str, list]] = [] - return_value: list[dict] = [] - - def _execute(self, sql, params=None): - captured.append((sql, list(params) if params else [])) - return list(return_value) - - monkeypatch.setattr(TokenDB, "execute", _execute) - db = TokenDB("https://db.example/v1") - db._captured = captured - db._set_return = return_value - return db - - def test_init_schema_creates_table_and_index(self, db): - db.init_schema() - sqls = [c[0].strip() for c in db._captured] - assert any("CREATE TABLE" in s and "trusted_tokens" in s for s in sqls) - assert any("CREATE INDEX" in s and "idx_trusted_tokens_active" in s for s in sqls) - # Both statements must use IF NOT EXISTS so init is idempotent - assert all("IF NOT EXISTS" in s for s in sqls) - - def test_add_inserts_value_and_label(self, db): - db._set_return.clear() - db._set_return.extend([{"id": 7}]) - new_id = db.add("e3a1f2d4", "alice-batch") - # First captured call is the INSERT … RETURNING id - sql, params = db._captured[0] - assert "INSERT INTO trusted_tokens" in sql - assert "value" in sql and "label" in sql - assert params == ["e3a1f2d4", "alice-batch"] - assert new_id == 7 - - def test_list_active_filters_revoked(self, db): - db._set_return.clear() - db._set_return.extend([ - {"id": 1, "value": "v1", "label": "a", "created_at": "2026-01-01"}, - ]) - rows = db.list_active() - sql, params = db._captured[0] - assert "revoked_at IS NULL" in sql - assert params == [] - assert rows == [{"id": 1, "value": "v1", "label": "a", "created_at": "2026-01-01"}] - - def test_list_all_omits_value_column(self, db): - db.list_all() - sql, _ = db._captured[0] - # The SELECT projection must NOT include `value` — list_all is for human reading - assert "value" not in sql.split("FROM")[0].lower() - assert "id" in sql.lower() - assert "label" in sql.lower() - assert "created_at" in sql.lower() - assert "revoked_at" in sql.lower() - - def test_revoke_updates_revoked_at(self, db): - from app.token_db import TokenDB - - # First execute returns rowsAffected; mock the second call's return - db._set_return.clear() - # We'll have to inspect the SQL only - db.revoke(42) - sql, params = db._captured[0] - assert "UPDATE trusted_tokens" in sql - assert "revoked_at" in sql - assert "datetime('now')" in sql - assert params == [42] - - def test_revoke_returns_true_for_active_id(self, db): - # Real implementation: revoke returns True iff a row was changed. - # In our mock, we'll just verify the method returns something truthy when - # the implementation looks at execute() side effects. Skipping deep - # assertion here — covered by integration in test_tokens_cli. - db.revoke(99) - # No exception = pass for this stub level -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_token_db.py::TestTokenDBMethods -v` -Expected: All fail (`AttributeError: 'TokenDB' object has no attribute 'init_schema'` etc.). - -- [ ] **Step 3: Append query methods to `app/token_db.py`** - -```python - # ── Schema ────────────────────────────────────────────────────────────── - - def init_schema(self) -> None: - """Idempotently create the trusted_tokens table and index.""" - self.execute( - """ - CREATE TABLE IF NOT EXISTS trusted_tokens ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - value TEXT NOT NULL UNIQUE, - label TEXT, - created_at TEXT NOT NULL DEFAULT (datetime('now')), - revoked_at TEXT - ) - """ - ) - self.execute( - """ - CREATE INDEX IF NOT EXISTS idx_trusted_tokens_active - ON trusted_tokens (value) - WHERE revoked_at IS NULL - """ - ) - - # ── Mutations ─────────────────────────────────────────────────────────── - - def add(self, value: str, label: str) -> int: - """Insert a new trusted token. Returns the new row id. - - Raises TokenDBError on uniqueness violation or transport failure. - """ - rows = self.execute( - "INSERT INTO trusted_tokens (value, label) VALUES (?, ?) RETURNING id", - [value, label], - ) - if not rows: - raise TokenDBError("INSERT did not return an id") - return int(rows[0]["id"]) - - def revoke(self, token_id: int) -> bool: - """Mark a token as revoked. Idempotent — returns False if already revoked.""" - rows = self.execute( - "UPDATE trusted_tokens " - "SET revoked_at = datetime('now') " - "WHERE id = ? AND revoked_at IS NULL " - "RETURNING id", - [token_id], - ) - return bool(rows) - - # ── Queries ───────────────────────────────────────────────────────────── - - def list_active(self) -> list[dict]: - """Return all rows where revoked_at IS NULL.""" - return self.execute( - "SELECT id, value, label, created_at FROM trusted_tokens " - "WHERE revoked_at IS NULL" - ) - - def list_all(self) -> list[dict]: - """Return all rows, never including the raw value column.""" - return self.execute( - "SELECT id, label, created_at, revoked_at FROM trusted_tokens " - "ORDER BY id" - ) -``` - -- [ ] **Step 4: Run tests — verify all pass** - -Run: `.venv/bin/python -m pytest tests/test_token_db.py::TestTokenDBMethods -v` -Expected: 6 passed. - -- [ ] **Step 5: Run full suite** - -Run: `.venv/bin/python -m pytest tests/ -q` -Expected: 138 passed (132 prior + 6 new). - -- [ ] **Step 6: Commit** - -```bash -git add app/token_db.py tests/test_token_db.py -git commit -m "feat(token-db): init_schema, add, list, revoke (#61)" -``` - ---- - -## Task 4: Operator CLI — `python -m scripts.tokens` - -**Files:** -- Create: `scripts/tokens.py` -- Test: `tests/test_tokens_cli.py` (NEW) - -The CLI is the user-facing surface. Tests use the `argparse` parser directly + capture stdout/stderr via `capsys` — no subprocess overhead. - -- [ ] **Step 1: Create `tests/test_tokens_cli.py` with failing tests** - -```python -"""Tests for scripts/tokens.py — operator CLI.""" - -import sys - -import pytest - - -# Fake TokenDB used in place of the real one for CLI tests -class FakeTokenDB: - def __init__(self, url: str = "fake://"): - self.url = url - self.calls: list[tuple[str, tuple, dict]] = [] - self.next_id = 1 - self.rows: list[dict] = [] - self.added: list[dict] = [] - self.revoked_ids: list[int] = [] - self.fail_with: Exception | None = None - - def init_schema(self): - self.calls.append(("init_schema", (), {})) - - def add(self, value, label): - if self.fail_with is not None: - raise self.fail_with - self.calls.append(("add", (value, label), {})) - row_id = self.next_id - self.next_id += 1 - self.added.append({"id": row_id, "value": value, "label": label}) - return row_id - - def list_active(self): - return [r for r in self.rows if not r.get("revoked_at")] - - def list_all(self): - return list(self.rows) - - def revoke(self, token_id): - self.revoked_ids.append(token_id) - for r in self.rows: - if r["id"] == token_id and not r.get("revoked_at"): - r["revoked_at"] = "2026-04-29T16:00:00" - return True - return False - - -@pytest.fixture -def fake_db(monkeypatch): - from scripts import tokens - - fake = FakeTokenDB() - monkeypatch.setattr(tokens, "_make_db", lambda url: fake) - monkeypatch.setenv("PC2NUTS_TOKEN_DB_URL", "fake://") - return fake - - -# ── init subcommand ──────────────────────────────────────────────────────── - - -def test_init_calls_init_schema(fake_db, capsys): - from scripts.tokens import main - - rc = main(["init"]) - assert rc == 0 - assert ("init_schema", (), {}) in fake_db.calls - - -# ── add subcommand ───────────────────────────────────────────────────────── - - -def test_add_generates_token_when_no_value(fake_db, capsys): - from scripts.tokens import main - - rc = main(["add", "--label", "alice"]) - assert rc == 0 - out = capsys.readouterr().out - assert len(fake_db.added) == 1 - generated = fake_db.added[0]["value"] - assert len(generated) == 48 # 24 bytes hex - assert all(c in "0123456789abcdef" for c in generated) - # Output should include the raw token, the row id, and the audit token_id - assert generated in out - assert "id=1" in out - assert "token_id=" in out - - -def test_add_with_value_uses_provided(fake_db, capsys): - from scripts.tokens import main - - rc = main(["add", "--label", "perf-test", "--value", "deadbeef" * 6]) - assert rc == 0 - assert fake_db.added[0]["value"] == "deadbeef" * 6 - - -def test_add_failure_exits_non_zero(fake_db, capsys): - from app.token_db import TokenDBError - from scripts.tokens import main - - fake_db.fail_with = TokenDBError("UNIQUE constraint failed") - rc = main(["add", "--label", "dup"]) - assert rc != 0 - err = capsys.readouterr().err - assert "UNIQUE" in err or "fail" in err.lower() - - -# ── list subcommand ──────────────────────────────────────────────────────── - - -def test_list_default_active_only(fake_db, capsys): - fake_db.rows = [ - {"id": 1, "label": "a", "created_at": "2026-01-01", "revoked_at": None}, - {"id": 2, "label": "b", "created_at": "2026-01-02", "revoked_at": "2026-01-03"}, - ] - from scripts.tokens import main - - rc = main(["list"]) - assert rc == 0 - out = capsys.readouterr().out - assert "1" in out and "a" in out - assert "2" not in out # revoked, hidden by default - - -def test_list_all_includes_revoked(fake_db, capsys): - fake_db.rows = [ - {"id": 1, "label": "a", "created_at": "2026-01-01", "revoked_at": None}, - {"id": 2, "label": "b", "created_at": "2026-01-02", "revoked_at": "2026-01-03"}, - ] - from scripts.tokens import main - - rc = main(["list", "--all"]) - out = capsys.readouterr().out - assert "1" in out and "2" in out - assert "revoked" in out.lower() - - -def test_list_never_prints_value(fake_db, capsys): - fake_db.rows = [ - {"id": 1, "label": "a", "created_at": "2026-01-01", "revoked_at": None}, - ] - # If `value` accidentally leaks via list_all, this test catches it. - # FakeTokenDB.list_all returns rows that contain no `value` key by default, - # mirroring the production query projection. - from scripts.tokens import main - - main(["list"]) - out = capsys.readouterr().out - # Sanity: a 48-hex string never appears - import re - - assert re.search(r"[0-9a-f]{48}", out) is None - - -# ── revoke subcommand ────────────────────────────────────────────────────── - - -def test_revoke_calls_db(fake_db, capsys): - fake_db.rows = [{"id": 5, "label": "x", "created_at": "2026-01-01", "revoked_at": None}] - from scripts.tokens import main - - rc = main(["revoke", "5"]) - assert rc == 0 - assert 5 in fake_db.revoked_ids - - -def test_revoke_already_revoked_exits_zero(fake_db, capsys): - fake_db.rows = [ - {"id": 5, "label": "x", "created_at": "2026-01-01", "revoked_at": "2026-04-28"} - ] - from scripts.tokens import main - - rc = main(["revoke", "5"]) - assert rc == 0 - out = capsys.readouterr().out - assert "already" in out.lower() - - -# ── missing config ───────────────────────────────────────────────────────── - - -def test_missing_db_url_errors(monkeypatch, capsys): - monkeypatch.delenv("PC2NUTS_TOKEN_DB_URL", raising=False) - from scripts.tokens import main - - rc = main(["list"]) - assert rc != 0 - err = capsys.readouterr().err - assert "PC2NUTS_TOKEN_DB_URL" in err - - -def test_db_url_arg_overrides_env(monkeypatch, capsys): - monkeypatch.delenv("PC2NUTS_TOKEN_DB_URL", raising=False) - from scripts import tokens - from scripts.tokens import main - - captured_urls: list[str] = [] - monkeypatch.setattr( - tokens, - "_make_db", - lambda url: (captured_urls.append(url), FakeTokenDB(url))[1], - ) - rc = main(["--db-url", "https://override.example", "init"]) - assert rc == 0 - assert captured_urls == ["https://override.example"] -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_tokens_cli.py -v` -Expected: All fail with `ModuleNotFoundError: No module named 'scripts.tokens'`. - -- [ ] **Step 3: Create `scripts/tokens.py`** - -Note: the project's `scripts/` directory does not yet have an `__init__.py`. Check first: - -```bash -test -f /home/bk86a/PostalCode2NUTS/scripts/__init__.py || touch /home/bk86a/PostalCode2NUTS/scripts/__init__.py -``` - -Create `scripts/tokens.py`: - -```python -"""Operator CLI for the trusted-token registry. - -Reads the database URL from PC2NUTS_TOKEN_DB_URL (or --db-url override). -Subcommands: init | add | list | revoke. - -See docs/superpowers/specs/2026-04-29-db-backed-trusted-tokens-design.md. -""" - -from __future__ import annotations - -import argparse -import hashlib -import os -import secrets -import sys -from typing import Sequence - -from app.token_db import TokenDB, TokenDBError - - -def _make_db(url: str) -> TokenDB: - """Indirection seam for tests.""" - return TokenDB(url) - - -def _token_id(token: str) -> str: - """Audit prefix — first 8 hex chars of sha256(token).""" - return hashlib.sha256(token.encode()).hexdigest()[:8] - - -def _resolve_db_url(args_url: str | None) -> str | None: - if args_url: - return args_url - env = os.environ.get("PC2NUTS_TOKEN_DB_URL", "").strip() - return env or None - - -def _cmd_init(db: TokenDB) -> int: - db.init_schema() - print("Schema initialised (idempotent).") - return 0 - - -def _cmd_add(db: TokenDB, label: str, value: str | None) -> int: - token = value if value else secrets.token_hex(24) - try: - new_id = db.add(token, label) - except TokenDBError as exc: - print(f"ERROR: {exc}", file=sys.stderr) - return 1 - print(f"Generated: {token}") - print(f"Inserted id={new_id}, label={label!r}, token_id={_token_id(token)}") - return 0 - - -def _cmd_list(db: TokenDB, show_all: bool) -> int: - rows = db.list_all() if show_all else db.list_active() - if not rows: - print("(no tokens)") - return 0 - print(f"{'id':>4} {'label':30} {'created_at':24} status") - print("-" * 80) - for r in rows: - rid = r.get("id", "?") - label = (r.get("label") or "")[:30] - created = r.get("created_at", "") - revoked = r.get("revoked_at") - status = f"revoked@{revoked}" if revoked else "active" - print(f"{rid:>4} {label:30} {created:24} {status}") - return 0 - - -def _cmd_revoke(db: TokenDB, token_id_arg: int) -> int: - changed = db.revoke(token_id_arg) - if changed: - print(f"Token id={token_id_arg} revoked.") - else: - print(f"Token id={token_id_arg} already revoked (or does not exist).") - return 0 - - -def main(argv: Sequence[str] | None = None) -> int: - parser = argparse.ArgumentParser(prog="scripts.tokens", description=__doc__) - parser.add_argument("--db-url", help="Override PC2NUTS_TOKEN_DB_URL") - sub = parser.add_subparsers(dest="cmd", required=True) - - sub.add_parser("init", help="Create the trusted_tokens table (idempotent)") - - p_add = sub.add_parser("add", help="Issue a new token") - p_add.add_argument("--label", required=True, help="Human-readable label") - p_add.add_argument( - "--value", - help="Use the provided 48-hex value instead of generating one. " - "Used for migrating existing v1 tokens.", - ) - - p_list = sub.add_parser("list", help="List tokens (active by default)") - p_list.add_argument("--all", action="store_true", help="Include revoked tokens") - - p_revoke = sub.add_parser("revoke", help="Revoke a token by id") - p_revoke.add_argument("id", type=int, help="Token id to revoke") - - args = parser.parse_args(argv) - - url = _resolve_db_url(args.db_url) - if not url: - print( - "ERROR: PC2NUTS_TOKEN_DB_URL is not set. " - "Provide --db-url or set the environment variable.", - file=sys.stderr, - ) - return 2 - - db = _make_db(url) - - if args.cmd == "init": - return _cmd_init(db) - if args.cmd == "add": - return _cmd_add(db, args.label, args.value) - if args.cmd == "list": - return _cmd_list(db, args.all) - if args.cmd == "revoke": - return _cmd_revoke(db, args.id) - - parser.error(f"unknown subcommand: {args.cmd}") # unreachable due to required=True - return 2 - - -if __name__ == "__main__": # pragma: no cover - sys.exit(main()) -``` - -- [ ] **Step 4: Run tests — verify all pass** - -Run: `.venv/bin/python -m pytest tests/test_tokens_cli.py -v` -Expected: 12 passed. - -- [ ] **Step 5: Run full suite** - -Run: `.venv/bin/python -m pytest tests/ -q` -Expected: 150 passed (138 prior + 12 new). - -- [ ] **Step 6: Lint** - -Run: `.venv/bin/ruff check app/ scripts/ tests/` -Expected: All checks passed. - -- [ ] **Step 7: Commit** - -```bash -git add scripts/__init__.py scripts/tokens.py tests/test_tokens_cli.py -git commit -m "feat(token-db): operator CLI — scripts/tokens.py (#61)" -``` - ---- - -## Task 5: Auth integration — _db_tokens cache + union semantic - -**Files:** -- Modify: `app/auth.py` -- Test: `tests/test_auth.py` - -This task introduces the in-memory cache and the union, but **not** the refresh task itself (that's Task 6). After this task: setting `_db_tokens` manually in a test makes those tokens trusted. The runtime refresher comes in the next task. - -- [ ] **Step 1: Append the failing tests to `tests/test_auth.py`** - -```python -# ── DB-backed tokens — union semantic (#61) ────────────────────────────────── - - -class TestDBTokensUnion: - def test_db_tokens_default_empty(self): - from app import auth - - assert auth._db_tokens == frozenset() - - def test_get_trusted_tokens_unions_db_and_env(self, monkeypatch): - from app import auth - - # env-var side: stub _get_trusted_tokens's view of settings - from app import config - - monkeypatch.setattr( - config.settings, - "trusted_tokens", - frozenset({"env-token"}), - ) - # db side: set the module-level cache directly - monkeypatch.setattr(auth, "_db_tokens", frozenset({"db-token"})) - - result = auth._get_trusted_tokens() - assert result == frozenset({"env-token", "db-token"}) - - def test_get_trusted_tokens_empty_db_falls_back_to_env(self, monkeypatch): - from app import auth, config - - monkeypatch.setattr(config.settings, "trusted_tokens", frozenset({"env-only"})) - monkeypatch.setattr(auth, "_db_tokens", frozenset()) - assert auth._get_trusted_tokens() == frozenset({"env-only"}) - - def test_get_trusted_tokens_empty_env_uses_db_only(self, monkeypatch): - from app import auth, config - - monkeypatch.setattr(config.settings, "trusted_tokens", frozenset()) - monkeypatch.setattr(auth, "_db_tokens", frozenset({"db-only"})) - assert auth._get_trusted_tokens() == frozenset({"db-only"}) - - def test_token_db_stale_default_false(self): - from app import auth - - assert auth._token_db_stale is False -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestDBTokensUnion -v` -Expected: All 5 fail (`AttributeError: module 'app.auth' has no attribute '_db_tokens'`). - -- [ ] **Step 3: Modify `app/auth.py`** - -Find the existing module-level state declarations (right after the imports, before `token_id`). Add: - -```python -# DB-backed token cache (refreshed by background task in main.lifespan). -# Empty until the first refresh succeeds; merged into the active set via _get_trusted_tokens. -_db_tokens: frozenset[str] = frozenset() -_token_db_stale: bool = False -``` - -Then modify `_get_trusted_tokens` (currently around line 38-40): - -```python -def _get_trusted_tokens() -> frozenset[str]: - """Indirection seam for tests; returns the union of DB-loaded and env-var tokens.""" - return _db_tokens | settings.trusted_tokens -``` - -(The function name stays the same — no callers need to change. The existing v0.16.0 `is_trusted` and `AuthMiddleware` continue to work.) - -- [ ] **Step 4: Run tests — verify all pass** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestDBTokensUnion -v` -Expected: 5 passed. - -- [ ] **Step 5: Run full suite — confirm no regression** - -Run: `.venv/bin/python -m pytest tests/ -q` -Expected: 155 passed (150 prior + 5 new). All 12 v0.16.0 `TestAuthBypass` tests still pass. - -- [ ] **Step 6: Commit** - -```bash -git add app/auth.py tests/test_auth.py -git commit -m "feat(auth): _db_tokens cache + union with env-var tokens (#61)" -``` - ---- - -## Task 6: refresh_db_tokens function - -**Files:** -- Modify: `app/auth.py` -- Test: `tests/test_auth.py` - -This task adds the function that **does the actual DB read and updates `_db_tokens`**. It is a plain (sync) function that the lifespan task in Task 7 will call repeatedly. - -- [ ] **Step 1: Append failing tests** - -```python -# ── refresh_db_tokens (#61) ────────────────────────────────────────────────── - - -class TestRefreshDBTokens: - def test_refresh_populates_db_tokens(self, monkeypatch): - from app import auth - - class FakeDB: - def list_active(self): - return [ - {"value": "a", "id": 1, "label": "x", "created_at": "..."}, - {"value": "b", "id": 2, "label": "y", "created_at": "..."}, - ] - - monkeypatch.setattr(auth, "_db_tokens", frozenset()) - monkeypatch.setattr(auth, "_token_db_stale", False) - auth.refresh_db_tokens(FakeDB()) - assert auth._db_tokens == frozenset({"a", "b"}) - assert auth._token_db_stale is False - - def test_refresh_failure_keeps_previous_set_and_sets_stale(self, monkeypatch): - from app import auth - from app.token_db import TokenDBError - - class FailingDB: - def list_active(self): - raise TokenDBError("boom") - - monkeypatch.setattr(auth, "_db_tokens", frozenset({"keepme"})) - monkeypatch.setattr(auth, "_token_db_stale", False) - auth.refresh_db_tokens(FailingDB()) - assert auth._db_tokens == frozenset({"keepme"}) # unchanged - assert auth._token_db_stale is True - - def test_refresh_recovery_clears_stale(self, monkeypatch): - from app import auth - - class FakeDB: - def list_active(self): - return [{"value": "good"}] - - monkeypatch.setattr(auth, "_db_tokens", frozenset({"old"})) - monkeypatch.setattr(auth, "_token_db_stale", True) # was stale - auth.refresh_db_tokens(FakeDB()) - assert auth._db_tokens == frozenset({"good"}) - assert auth._token_db_stale is False -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestRefreshDBTokens -v` -Expected: 3 fail with `AttributeError: module 'app.auth' has no attribute 'refresh_db_tokens'`. - -- [ ] **Step 3: Add `refresh_db_tokens` to `app/auth.py`** - -Append after the `_token_db_stale` declaration block (or at the bottom of the module, before the existing `is_trusted_request` function): - -```python -def refresh_db_tokens(db) -> None: - """Reload the active token set from the DB. - - On success: replace _db_tokens with the new frozenset, clear the stale flag. - On failure: keep _db_tokens unchanged, set _token_db_stale = True. - - `db` is duck-typed: must expose .list_active() returning rows with a 'value' key. - """ - global _db_tokens, _token_db_stale - try: - rows = db.list_active() - except Exception as exc: # noqa: BLE001 — caller passes a duck-typed client - import logging - - logging.getLogger(__name__).warning("token DB refresh failed: %s", exc) - _token_db_stale = True - return - _db_tokens = frozenset(r["value"] for r in rows if r.get("value")) - _token_db_stale = False -``` - -- [ ] **Step 4: Run tests — verify all pass** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestRefreshDBTokens -v` -Expected: 3 passed. - -- [ ] **Step 5: Run full suite** - -Run: `.venv/bin/python -m pytest tests/ -q` -Expected: 158 passed (155 prior + 3 new). - -- [ ] **Step 6: Commit** - -```bash -git add app/auth.py tests/test_auth.py -git commit -m "feat(auth): refresh_db_tokens function (#61)" -``` - ---- - -## Task 7: Wire refresh task into FastAPI lifespan - -**Files:** -- Modify: `app/main.py` -- Test: `tests/test_auth.py` - -The lifespan starts a background asyncio task only when `PC2NUTS_TOKEN_DB_URL` is set. The task does an initial refresh, then loops every `token_refresh_seconds`. On shutdown, the task is cancelled cleanly. - -- [ ] **Step 1: Append failing test to `tests/test_auth.py`** - -```python -# ── lifespan refresh task (#61) ────────────────────────────────────────────── - - -class TestLifespanRefresh: - def test_lifespan_starts_refresh_when_db_url_set(self, monkeypatch): - """When PC2NUTS_TOKEN_DB_URL is set, lifespan should call refresh_db_tokens - at least once at startup.""" - import asyncio - - from app import auth, config, data_loader - - called: list = [] - - def _fake_refresh(db): - called.append(db) - # Simulate one successful refresh - monkeypatch.setattr(auth, "_db_tokens", frozenset({"loaded-from-db"})) - - monkeypatch.setattr(auth, "refresh_db_tokens", _fake_refresh) - monkeypatch.setattr(config.settings, "token_db_url", "https://db.example/v1") - monkeypatch.setattr(config.settings, "token_refresh_seconds", 3600) # don't loop fast - - from unittest.mock import patch - - with patch.object(data_loader, "load_data"): - from fastapi.testclient import TestClient - from app.main import app - - with TestClient(app): - pass # entering the context = startup; exiting = shutdown - - assert len(called) >= 1, "refresh_db_tokens was not called during lifespan" - - def test_lifespan_skips_refresh_when_db_url_unset(self, monkeypatch): - from app import auth, config, data_loader - - called: list = [] - - def _fake_refresh(db): - called.append(db) - - monkeypatch.setattr(auth, "refresh_db_tokens", _fake_refresh) - monkeypatch.setattr(config.settings, "token_db_url", "") - - from unittest.mock import patch - - with patch.object(data_loader, "load_data"): - from fastapi.testclient import TestClient - from app.main import app - - with TestClient(app): - pass - - assert called == [], "refresh_db_tokens called despite empty DB URL" -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestLifespanRefresh -v` -Expected: 2 fail (no lifespan integration yet — `refresh_db_tokens` is never called). - -- [ ] **Step 3: Modify `app/main.py`** - -Find the existing `lifespan` function (currently lines 79-97 in v0.16.0) and replace its body with: - -```python -@asynccontextmanager -async def lifespan(app: FastAPI): - logger.info("Loading TERCET data (NUTS %s)...", settings.nuts_version) - load_data() - table = get_lookup_table() - estimates = get_estimates_table() - names = get_nuts_names() - logger.info( - "Ready — %d postal codes loaded, %d estimates available, %d NUTS names.", - len(table), - len(estimates), - len(names), - ) - extra = get_extra_source_count() - if extra: - logger.info("Extra data sources configured: %d", extra) - if get_data_stale(): - logger.warning("Serving STALE data — TERCET refresh failed, using expired cache") - - # ── Token DB refresh (#61) ────────────────────────────────────────────── - refresh_task: asyncio.Task | None = None - if settings.token_db_url: - from app import auth as auth_mod - from app.token_db import TokenDB - - token_db = TokenDB(settings.token_db_url) - - async def _refresh_loop(): - interval = max(1, settings.token_refresh_seconds) - while True: - await asyncio.to_thread(auth_mod.refresh_db_tokens, token_db) - try: - await asyncio.sleep(interval) - except asyncio.CancelledError: - return - - # Initial synchronous refresh so /lookup is correct from the first request - await asyncio.to_thread(auth_mod.refresh_db_tokens, token_db) - refresh_task = asyncio.create_task(_refresh_loop()) - logger.info( - "Token DB refresh task started (interval %ds)", - settings.token_refresh_seconds, - ) - - yield - - if refresh_task is not None: - refresh_task.cancel() - try: - await refresh_task - except asyncio.CancelledError: - pass -``` - -Add `import asyncio` near the top of `app/main.py` if it isn't already there. - -- [ ] **Step 4: Run new tests — verify they pass** - -Run: `.venv/bin/python -m pytest tests/test_auth.py::TestLifespanRefresh -v` -Expected: 2 passed. - -- [ ] **Step 5: Run full suite** - -Run: `.venv/bin/python -m pytest tests/ -q` -Expected: 160 passed (158 prior + 2 new). - -- [ ] **Step 6: Commit** - -```bash -git add app/main.py tests/test_auth.py -git commit -m "feat(token-db): lifespan launches refresh task when DB URL set (#61)" -``` - ---- - -## Task 8: Expose `token_db_stale` on /health - -**Files:** -- Modify: `app/models.py` -- Modify: `app/main.py` -- Test: `tests/test_api.py` - -- [ ] **Step 1: Append failing tests to `tests/test_api.py`** - -Inside the existing `TestHealthEndpoint` class: - -```python - def test_health_includes_token_db_stale_when_db_url_set(self, monkeypatch, mock_data): - from unittest.mock import patch - - from app import auth, config, data_loader - - monkeypatch.setattr(config.settings, "token_db_url", "https://db.example/v1") - monkeypatch.setattr(auth, "_token_db_stale", False) - - with patch.object(data_loader, "load_data"), patch.object( - auth, "refresh_db_tokens", lambda db: None - ): - from fastapi.testclient import TestClient - from app.main import app - - with TestClient(app) as client: - resp = client.get("/health") - assert resp.status_code == 200 - data = resp.json() - assert data.get("token_db_stale") is False - - def test_health_omits_token_db_stale_when_db_url_unset(self, mock_data, client): - # `client` fixture has no PC2NUTS_TOKEN_DB_URL configured - resp = client.get("/health") - assert resp.status_code == 200 - # Field is None when feature is disabled — Pydantic serializes to null, - # which json() reports as None or omits depending on config. Accept both. - data = resp.json() - assert data.get("token_db_stale") in (None,) - - def test_health_token_db_stale_true_after_failure(self, monkeypatch, mock_data): - from unittest.mock import patch - - from app import auth, config, data_loader - - monkeypatch.setattr(config.settings, "token_db_url", "https://db.example/v1") - monkeypatch.setattr(auth, "_token_db_stale", True) - - with patch.object(data_loader, "load_data"), patch.object( - auth, "refresh_db_tokens", lambda db: None - ): - from fastapi.testclient import TestClient - from app.main import app - - with TestClient(app) as client: - resp = client.get("/health") - assert resp.json().get("token_db_stale") is True -``` - -- [ ] **Step 2: Run tests — verify they fail** - -Run: `.venv/bin/python -m pytest "tests/test_api.py::TestHealthEndpoint" -v` -Expected: 3 new fail (`assert None is False` or similar). - -- [ ] **Step 3: Add field to `HealthResponse`** - -In `app/models.py`, add to the `HealthResponse` model (alongside the existing fields): - -```python - token_db_stale: bool | None = None -``` - -- [ ] **Step 4: Wire the field in `app/main.py`'s `/health` handler** - -Find the existing `health` route (around line 257). Modify the return statement: - -```python -@app.get("/health", response_model=HealthResponse, summary="Health check and data statistics") -def health(response: Response): - response.headers["Cache-Control"] = "no-cache, no-store" - table = get_lookup_table() - estimates = get_estimates_table() - stale = get_data_stale() - - # Token DB staleness — only meaningful when the feature is enabled. - from app import auth as auth_mod - - token_db_stale = auth_mod._token_db_stale if settings.token_db_url else None - - return HealthResponse( - status="ok" if len(table) > 0 else "no_data", - total_postal_codes=len(table), - total_estimates=len(estimates), - total_nuts_names=len(get_nuts_names()), - nuts_version=settings.nuts_version, - extra_sources=get_extra_source_count(), - patterns_version=PATTERNS_META.get("version", "unknown"), - data_stale=stale, - last_updated=get_data_loaded_at(), - token_db_stale=token_db_stale, - ) -``` - -- [ ] **Step 5: Run new tests — verify they pass** - -Run: `.venv/bin/python -m pytest "tests/test_api.py::TestHealthEndpoint" -v` -Expected: 3 new passed. - -- [ ] **Step 6: Run full suite** - -Run: `.venv/bin/python -m pytest tests/ -q` -Expected: 163 passed (160 prior + 3 new). - -- [ ] **Step 7: Lint + format check** - -Run: `.venv/bin/ruff check app/ scripts/ tests/` -Run: `.venv/bin/ruff format --check app/ scripts/ tests/` -Both clean. - -- [ ] **Step 8: Commit** - -```bash -git add app/main.py app/models.py tests/test_api.py -git commit -m "feat(token-db): /health exposes token_db_stale (#61)" -``` - ---- - -## Task 9: README operator runbook update - -**Files:** -- Modify: `README.md` - -No tests for this task (documentation-only). - -- [ ] **Step 1: Locate the existing v1 runbook in `README.md`** - -It is the `## Authentication & rate-limit bypass` section (added in v0.16.0). It contains "Operator runbook — issue a token" / "revoke a token" / "find the token id" subsections. - -- [ ] **Step 2: Replace the issue / revoke / find-id subsections** - -Replace the three operator-runbook subsections (issue / revoke / find-id) with the v2 versions. Keep the surrounding "Configuration", "Behaviour summary", "Disable the bypass entirely", and "Security notes" subsections — only the runbooks change. - -After the existing `### Configuration` table, insert: - -```markdown -### Storage backend - -By default, trusted tokens live in the `PC2NUTS_TRUSTED_TOKENS` env var (comma-separated; restart-to-apply). When `PC2NUTS_TOKEN_DB_URL` is configured, tokens are also loaded from a managed SQLite-compatible database; the active set is the **union** of DB-loaded tokens and env-var tokens. The DB is the primary registry; the env var serves as a disaster-recovery fallback for cases where the DB is unreachable at startup. - -| Env var | Default | Purpose | -|---|---|---| -| `PC2NUTS_TOKEN_DB_URL` | `""` (unset) | Connection string for the token database. Empty → DB-backed feature disabled (v0.16.0 env-var-only behaviour). | -| `PC2NUTS_TOKEN_REFRESH_SECONDS` | `60` | How often the running service reloads the active set from the DB. | -``` - -Replace the three v1 runbook subsections with these v2 versions: - -```markdown -### Operator runbook — initial setup (one-time) - -```bash -# On your laptop, with the DB connection string in your environment: -export PC2NUTS_TOKEN_DB_URL='' -python -m scripts.tokens init -# → idempotent. Safe to re-run. -``` - -### Operator runbook — issue a token - -```bash -python -m scripts.tokens add --label "alice-batch-2026-04" -# Generated: e3a1f2…d4 -# Inserted id=3, label='alice-batch-2026-04', token_id=9a29f07a - -# The token is active in the running service within ~PC2NUTS_TOKEN_REFRESH_SECONDS. -# Hand the printed token to the consumer over a confidential channel -# (1Password, Signal, encrypted email — not Slack, not GitHub issues). -``` - -### Operator runbook — list tokens - -```bash -python -m scripts.tokens list # active only (default) -python -m scripts.tokens list --all # include revoked -``` - -The `value` column is never printed. - -### Operator runbook — revoke a token - -```bash -python -m scripts.tokens revoke 3 -# Token id=3 revoked. -# (Already-revoked ids print "already revoked" and exit 0.) -``` - -Revocation takes effect within `PC2NUTS_TOKEN_REFRESH_SECONDS` (default 60 s). For an emergency revocation, restart the container to force an immediate refresh. - -### Operator runbook — find the token id of a logged request - -```bash -echo -n "" | sha256sum | cut -c1-8 -``` - -The CLI's `add` command also prints the `token_id` at issuance time. - -### Operator runbook — migrate a v1 env-var token - -To preserve the audit `token_id` of an existing env-var token when moving it to the DB: - -```bash -python -m scripts.tokens add --label "perf-test-2026-04-29" --value "" -``` - -Then remove that token from `PC2NUTS_TRUSTED_TOKENS` on the next config edit. -``` - -- [ ] **Step 3: Add the union-semantic note to the "Disable the bypass entirely" subsection** - -Replace the existing single-paragraph "Disable the bypass entirely" subsection with: - -```markdown -### Disable the bypass entirely - -Unset both `PC2NUTS_TOKEN_DB_URL` and `PC2NUTS_TRUSTED_TOKENS`. All traffic falls back to the per-IP cap. The `Authorization` header is ignored entirely (no 400, no 401) when the feature is disabled. No code change needed. - -If only the DB URL is unset (env var still set), behaviour reverts exactly to v0.16.0. -``` - -- [ ] **Step 4: Commit** - -```bash -git add README.md -git commit -m "docs: README runbook for DB-backed trusted tokens (#61)" -``` - ---- - -## Task 10: CHANGELOG entry + final lint/format/test sweep - -**Files:** -- Modify: `CHANGELOG.md` - -- [ ] **Step 1: Add `[Unreleased]` section** - -If `CHANGELOG.md` does not currently have an `[Unreleased]` section above `[0.16.0] - 2026-04-29`, add one. Otherwise extend the existing one. - -```markdown -The format is based on [Keep a Changelog](https://keepachangelog.com/). - -## [Unreleased] - -### Added - -- **DB-backed trusted tokens** (#61): trusted-token storage moved from `PC2NUTS_TRUSTED_TOKENS` env var to a managed SQLite-compatible HTTP database. New env vars: `PC2NUTS_TOKEN_DB_URL` (connection string), `PC2NUTS_TOKEN_REFRESH_SECONDS` (default `60`). Tokens are issued via `python -m scripts.tokens add --label "..."` and take effect within ~60 s — no container restart required. The env var continues to work as a union with the DB and serves as a disaster-recovery fallback when the DB is unreachable. New `/health` field `token_db_stale` flags refresh failures. -- **`scripts/tokens.py` operator CLI** with subcommands `init`, `add`, `list`, `revoke`. `add --value ` lets operators migrate v1 env-var tokens while preserving their audit `token_id`. - -## [0.16.0] - 2026-04-29 -``` - -- [ ] **Step 2: Final sweep** - -Run, in order: - -```bash -.venv/bin/python -m pytest tests/ -q -.venv/bin/ruff check app/ scripts/ -.venv/bin/ruff format --check app/ scripts/ tests/ -``` - -Expected: 163 passed, 0 ruff issues, format clean. - -- [ ] **Step 3: Commit** - -```bash -git add CHANGELOG.md -git commit -m "docs: changelog entry for DB-backed trusted tokens (#61)" -``` - ---- - -## Task 11: Push, close issue, release v0.17.0 - -**Files:** none (git/gh operations). - -This task is intentionally last and human-supervised: it makes shared-state changes (push to main, public release). - -- [ ] **Step 1: Push to origin** - -```bash -git push origin main -``` - -- [ ] **Step 2: Wait for CI to confirm green on main HEAD** - -```bash -gh run list --branch main --limit 1 -# Wait until the most recent CI run shows "success". -``` - -- [ ] **Step 3: Bump CHANGELOG `[Unreleased]` → `[0.17.0] - 2026-04-29`** - -In `CHANGELOG.md`, change `## [Unreleased]` to `## [0.17.0] - 2026-04-29`. Commit: - -```bash -git add CHANGELOG.md -git commit -m "chore: cut v0.17.0" -git push origin main -``` - -- [ ] **Step 4: Create the GitHub release** - -```bash -gh release create v0.17.0 \ - --title "v0.17.0 — DB-backed trusted tokens" \ - --notes "$(cat <<'EOF' -### What's new - -- **DB-backed trusted tokens** (#61): trusted-token storage moved from `PC2NUTS_TRUSTED_TOKENS` env var to a managed SQLite-compatible HTTP database. Configure with `PC2NUTS_TOKEN_DB_URL`. Tokens issued via `python -m scripts.tokens add --label "..."` take effect within ~60 s (configurable via `PC2NUTS_TOKEN_REFRESH_SECONDS`) — **no container restart required**. The env var continues to work as a union with the DB and serves as a disaster-recovery fallback when the DB is unreachable. -- **Operator CLI** at `python -m scripts.tokens` — subcommands `init`, `add`, `list`, `revoke`. `add --value ` preserves audit-id continuity when migrating v1 env-var tokens. -- **`/health` field `token_db_stale`** — flags when the most recent DB refresh failed and the in-memory set is stale. - -### Backwards compatibility - -Fully backwards-compatible with v0.16.0. With `PC2NUTS_TOKEN_DB_URL` unset, behaviour is identical to v0.16.0. The env var is **not** deprecated in this release. - -### Operator runbook - -See [README → Authentication & rate-limit bypass](https://github.com/bk86a/PostalCode2NUTS/blob/main/README.md#authentication--rate-limit-bypass). - -Closes #61. -EOF -)" -``` - -- [ ] **Step 5: Close #61 with a summary** - -```bash -gh issue close 61 -c "Implemented across 10 task commits + cut v0.17.0. - -Behaviour: -- PC2NUTS_TOKEN_DB_URL set → tokens load from DB every PC2NUTS_TOKEN_REFRESH_SECONDS -- env var PC2NUTS_TRUSTED_TOKENS continues to work; active set = DB ∪ env var -- DB unreachable at startup → service still serves; bypass falls back to env var -- /health surfaces token_db_stale=true on refresh failures - -Operator CLI: -- python -m scripts.tokens init / add / list / revoke -- add --value for v1→v2 migration with audit-id continuity - -Out of scope per spec: per-token quotas, last_used_at, HTTP admin endpoint, forced env-var deprecation. - -See [README → Authentication & rate-limit bypass](https://github.com/bk86a/PostalCode2NUTS/blob/main/README.md#authentication--rate-limit-bypass) for the operator runbook." -``` - -- [ ] **Step 6: Manual integration verification (HUMAN STEP — does not block plan completion)** - -This is the only step the implementer **cannot fully automate** — it requires a real database instance: - -1. Provision a database instance with the configured provider; obtain the connection string. -2. Set `PC2NUTS_TOKEN_DB_URL` on the production deployment to the connection string. -3. Run `python -m scripts.tokens init` once from the operator's laptop (with the same env var). -4. Run `python -m scripts.tokens add --label "perf-test-migration" --value ""`. -5. After ~60 s, verify the token still works: - ```bash - curl -i -H "Authorization: Bearer " "https:///lookup?country=DE&postal_code=10115" - # → 200, audit log shows the same token_id as before - ``` -6. Verify `/health` includes `token_db_stale: false`. -7. Remove the migrated token from `PC2NUTS_TRUSTED_TOKENS` env var; verify it still works (now sourced from DB only). diff --git a/docs/superpowers/plans/2026-05-01-estimates-periodic-refresh.md b/docs/superpowers/plans/2026-05-01-estimates-periodic-refresh.md deleted file mode 100644 index ff3b714..0000000 --- a/docs/superpowers/plans/2026-05-01-estimates-periodic-refresh.md +++ /dev/null @@ -1,1562 +0,0 @@ -# Estimates periodic refresh — implementation plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Implement issue [#44](https://github.com/bk86a/PostalCode2NUTS/issues/44) — let a running pod pick up changes to `tercet_missing_codes.csv` from upstream `main` without a redeploy. - -**Architecture:** New module `app/estimates_refresh.py` with module-scoped state, an async fetch using `httpx.AsyncClient`, conditional GET headers (ETag / Last-Modified), SHA-256 dedupe, a 50% relative-row sanity guard, and full-replace swap into the existing `_estimates` dict under the existing `_data_lock`. Wired via FastAPI lifespan (one bootstrap call + a `refresh_estimates_loop()` task). New `POST /admin/refresh-estimates` endpoint reuses the trusted-token Bearer mechanism. Defaults preserve current behaviour byte-for-byte (feature is opt-in via `PC2NUTS_ESTIMATES_REFRESH_URL`). - -**Tech Stack:** Python 3.14, FastAPI, httpx (async), pytest, slowapi (rate-limit decorator on the new endpoint), the project's existing `_data_lock` (`threading.Lock` from `app/data_loader.py`). - -**Spec:** [`docs/superpowers/specs/2026-05-01-estimates-periodic-refresh-design.md`](../specs/2026-05-01-estimates-periodic-refresh-design.md) (commit `e667cac`). - ---- - -## File structure - -| Action | Path | Responsibility | -|---|---|---| -| Modify | `app/config.py` | Two new Settings fields (`estimates_refresh_url`, `estimates_refresh_interval_seconds`). | -| Modify | `app/data_loader.py` | Extract a stream-friendly helper `parse_estimates_from_text()` from the existing `_load_estimates_from_csv()`. | -| Create | `app/estimates_refresh.py` | All refresh logic — module state, fetch, sanity guard, orchestration, asyncio loop, `/health` getter. | -| Modify | `app/models.py` | Add `estimates_refresh_stale: bool \| None = None` to `HealthResponse`. | -| Modify | `app/main.py` | Lifespan wiring (bootstrap + task), `/admin/refresh-estimates` endpoint, populate the new `/health` field. | -| Create | `tests/test_estimates_refresh.py` | Unit tests for the new module — sanity guard, parse, fetch (mocked via `httpx.MockTransport`), orchestration. | -| Modify | `tests/test_api.py` | New `TestAdminRefreshEstimatesEndpoint` class + extension to `TestHealthEndpoint`. | -| Modify | `tests/test_config.py` | Defaults + parsing for the two new settings. | -| Modify | `README.md` | Env-var table + "Operator runbook — manually refresh estimates" subsection. | -| Modify | `CHANGELOG.md` | Entry under `[Unreleased]`. | - ---- - -## Task 1: Settings — `estimates_refresh_url` and `estimates_refresh_interval_seconds` - -**Files:** -- Test: `tests/test_config.py` -- Modify: `app/config.py:15-50` - -- [ ] **Step 1: Write the failing tests** - -Append to `tests/test_config.py`: - -```python -class TestEstimatesRefreshSettings: - def test_defaults_disable_remote_refresh(self): - s = Settings() - assert s.estimates_refresh_url == "" - assert s.estimates_refresh_interval_seconds == 86400 - - def test_url_can_be_set_via_env(self, monkeypatch): - monkeypatch.setenv( - "PC2NUTS_ESTIMATES_REFRESH_URL", - "https://raw.githubusercontent.com/bk86a/PostalCode2NUTS/main/tercet_missing_codes.csv", - ) - s = Settings() - assert s.estimates_refresh_url.endswith("/tercet_missing_codes.csv") - - def test_interval_zero_is_allowed(self, monkeypatch): - monkeypatch.setenv("PC2NUTS_ESTIMATES_REFRESH_INTERVAL_SECONDS", "0") - s = Settings() - assert s.estimates_refresh_interval_seconds == 0 - - def test_interval_negative_is_rejected(self, monkeypatch): - monkeypatch.setenv("PC2NUTS_ESTIMATES_REFRESH_INTERVAL_SECONDS", "-5") - with pytest.raises(ValidationError): - Settings() -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pytest tests/test_config.py::TestEstimatesRefreshSettings -v -``` - -Expected: 4 FAIL with "no attribute 'estimates_refresh_url'" / "no attribute 'estimates_refresh_interval_seconds'". - -- [ ] **Step 3: Add the two fields to `app/config.py`** - -Insert after the existing `rate_limit_storage_uri` line (around `app/config.py:28`): - -```python - estimates_refresh_url: str = "" - estimates_refresh_interval_seconds: int = Field(default=86400, ge=0) -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pytest tests/test_config.py::TestEstimatesRefreshSettings -v -``` - -Expected: 4 PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/config.py tests/test_config.py -git commit -m "feat(config): add estimates_refresh_url and interval settings (#44) - -Two new Pydantic fields, both opt-in. Defaults preserve current behaviour: -estimates_refresh_url empty (no remote fetch), interval default 86400s -(24h). Negative intervals rejected via Field(ge=0). -" -``` - ---- - -## Task 2: Refactor `_load_estimates_from_csv` — extract a stream-friendly helper - -**Files:** -- Test: `tests/test_data_loader.py` -- Modify: `app/data_loader.py:463-503` - -- [ ] **Step 1: Write the failing test** - -Append to `tests/test_data_loader.py`: - -```python -class TestParseEstimatesFromText: - def test_parses_well_formed_csv(self): - from app.data_loader import parse_estimates_from_text - - text = ( - "COUNTRY_CODE,POSTAL_CODE,ESTIMATED_NUTS3,ESTIMATED_NUTS2,ESTIMATED_NUTS1,CONFIDENCE\n" - "DE,99999,DE300,DE30,DE3,high\n" - "FR,75000,FR101,FR10,FR1,medium\n" - ) - d, skipped = parse_estimates_from_text(text) - assert skipped == 0 - assert len(d) == 2 - assert d[("DE", "99999")]["nuts3"] == "DE300" - assert d[("FR", "75000")]["nuts3"] == "FR101" - # Confidence is mapped from label to numeric per settings.confidence_map. - assert 0.0 < d[("DE", "99999")]["nuts3_confidence"] <= 1.0 - - def test_skips_unknown_confidence(self): - from app.data_loader import parse_estimates_from_text - - text = ( - "COUNTRY_CODE,POSTAL_CODE,ESTIMATED_NUTS3,ESTIMATED_NUTS2,ESTIMATED_NUTS1,CONFIDENCE\n" - "DE,99999,DE300,DE30,DE3,high\n" - "DE,99998,DE300,DE30,DE3,bogus\n" - ) - d, skipped = parse_estimates_from_text(text) - assert skipped == 1 - assert ("DE", "99998") not in d - assert ("DE", "99999") in d - - def test_handles_utf8_bom(self): - from app.data_loader import parse_estimates_from_text - - text = ( - "COUNTRY_CODE,POSTAL_CODE,ESTIMATED_NUTS3,ESTIMATED_NUTS2,ESTIMATED_NUTS1,CONFIDENCE\n" - "DE,99999,DE300,DE30,DE3,high\n" - ) - d, skipped = parse_estimates_from_text(text) - assert len(d) == 1 - assert ("DE", "99999") in d -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pytest tests/test_data_loader.py::TestParseEstimatesFromText -v -``` - -Expected: 3 FAIL with "ImportError: cannot import name 'parse_estimates_from_text'". - -- [ ] **Step 3: Refactor `app/data_loader.py`** - -Replace the existing `_load_estimates_from_csv` (around line 463) with two functions. The new public helper does the parsing; the old one is now a thin wrapper. - -```python -def parse_estimates_from_text(text: str) -> tuple[dict[tuple[str, str], dict], int]: - """Parse an estimates CSV from a string into a fresh dict. - - Returns (parsed_dict, skipped_count). Rows with unknown confidence labels - are counted in skipped_count and not included in the dict. Used both by - _load_estimates_from_csv (file path) and app.estimates_refresh (HTTP body). - """ - out: dict[tuple[str, str], dict] = {} - skipped = 0 - reader = csv.DictReader(io.StringIO(text)) - for row in reader: - cc = row["COUNTRY_CODE"].strip().upper() - pc = normalize_postal_code(row["POSTAL_CODE"]) - n3 = row["ESTIMATED_NUTS3"].strip() - n2 = row["ESTIMATED_NUTS2"].strip() - n1 = row["ESTIMATED_NUTS1"].strip() - label = row["CONFIDENCE"].strip().lower() - conf = settings.confidence_map.get(label) - if conf is None: - skipped += 1 - continue - out[(cc, pc)] = { - "nuts3": n3, - "nuts2": n2, - "nuts1": n1, - "nuts3_confidence": conf["nuts3"], - "nuts2_confidence": conf["nuts2"], - "nuts1_confidence": conf["nuts1"], - } - return out, skipped - - -def _load_estimates_from_csv(csv_path: Path) -> bool: - """Load pre-computed estimates from a file into the live in-memory dict.""" - if not csv_path.is_file(): - return False - try: - text = csv_path.read_text(encoding="utf-8-sig") - parsed, skipped = parse_estimates_from_text(text) - except (OSError, KeyError, ValueError, csv.Error) as exc: - logger.warning("Failed to load estimates from CSV: %s", exc) - return False - _estimates.update(parsed) - if skipped: - logger.warning("Skipped %d estimate rows with unknown confidence labels", skipped) - if parsed: - logger.info("Loaded %d estimates from %s", len(parsed), csv_path) - return len(parsed) > 0 -``` - -Add `import io` to the top of `app/data_loader.py` if not already present. - -- [ ] **Step 4: Run tests** - -```bash -pytest tests/test_data_loader.py -v -``` - -Expected: existing tests still pass, the 3 new ones pass too. - -- [ ] **Step 5: Commit** - -```bash -git add app/data_loader.py tests/test_data_loader.py -git commit -m "refactor(data_loader): extract parse_estimates_from_text() helper (#44) - -Splits the CSV parsing logic out of _load_estimates_from_csv() so both -the file-path loader and the upcoming HTTP-body loader (#44) reuse the -same parsing path. Behaviour is unchanged: same skipped-row semantics, -same confidence-map lookup, same warning logs. -" -``` - ---- - -## Task 3: `app/estimates_refresh.py` — sanity guard + parse helpers - -**Files:** -- Create: `tests/test_estimates_refresh.py` -- Create: `app/estimates_refresh.py` - -- [ ] **Step 1: Write the failing tests** - -Create `tests/test_estimates_refresh.py`: - -```python -"""Tests for app.estimates_refresh — periodic refresh of tercet_missing_codes.csv (#44).""" - -import importlib -from unittest.mock import patch - -import httpx -import pytest - - -@pytest.fixture(autouse=True) -def reset_module_state(): - """Reload the module before each test so module-scoped state doesn't leak.""" - import app.estimates_refresh - - importlib.reload(app.estimates_refresh) - yield - - -class TestSanityGuard: - def test_accepts_when_current_is_empty(self): - from app.estimates_refresh import _passes_sanity_guard - - assert _passes_sanity_guard(new_count=0, current_count=0) is True - assert _passes_sanity_guard(new_count=10, current_count=0) is True - - def test_accepts_when_new_is_at_or_above_50_percent(self): - from app.estimates_refresh import _passes_sanity_guard - - assert _passes_sanity_guard(new_count=5000, current_count=10000) is True - assert _passes_sanity_guard(new_count=10001, current_count=10000) is True - - def test_rejects_when_new_is_below_50_percent(self): - from app.estimates_refresh import _passes_sanity_guard - - assert _passes_sanity_guard(new_count=4999, current_count=10000) is False - assert _passes_sanity_guard(new_count=0, current_count=10000) is False -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pytest tests/test_estimates_refresh.py::TestSanityGuard -v -``` - -Expected: 3 FAIL with "ImportError: No module named 'app.estimates_refresh'". - -- [ ] **Step 3: Create the module skeleton** - -Create `app/estimates_refresh.py`: - -```python -"""Periodic refresh of tercet_missing_codes.csv from a remote URL (#44). - -When PC2NUTS_ESTIMATES_REFRESH_URL is set, a per-worker asyncio task fetches -the URL on every PC2NUTS_ESTIMATES_REFRESH_INTERVAL_SECONDS tick (default 24 h), -parses the body, and full-replaces the in-memory _estimates dict if the -content has changed and passes a 50% relative-row sanity guard. - -Defaults preserve the current single-source behaviour: when the URL setting -is unset, this module exposes refresh_estimates_once() that returns a -"disabled" RefreshResult and refresh_estimates_loop() that returns immediately. -""" - -from __future__ import annotations - -import asyncio -import csv -import hashlib -import logging -from dataclasses import dataclass -from typing import Optional - -import httpx - -from app.config import settings -from app.data_loader import _data_lock, _estimates, _revalidate_estimates, parse_estimates_from_text - -logger = logging.getLogger(__name__) - - -# Module-scoped state. Reset by importlib.reload() in tests. -_last_hash: Optional[str] = None -_last_etag: Optional[str] = None -_last_modified: Optional[str] = None -_stale: Optional[bool] = None # None when feature disabled - - -@dataclass -class RefreshResult: - status: str # "refreshed" | "unchanged" | "rejected" | "failed" | "disabled" - previous_count: int - new_count: int - skipped_rows: int = 0 - reason: str = "" - - -def get_refresh_stale() -> Optional[bool]: - """Return the staleness flag for /health. None when feature disabled.""" - return _stale - - -def _passes_sanity_guard(new_count: int, current_count: int) -> bool: - """50%-of-current floor; pass freely when the live state is empty.""" - if current_count == 0: - return True - return new_count >= 0.5 * current_count -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pytest tests/test_estimates_refresh.py::TestSanityGuard -v -``` - -Expected: 3 PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/estimates_refresh.py tests/test_estimates_refresh.py -git commit -m "feat(estimates_refresh): module scaffolding + sanity guard (#44) - -New module with module-scoped state for a periodic CSV refresh task. -This commit lands the skeleton and the 50%-floor sanity guard helper; -fetch and orchestration come in the next commits. -" -``` - ---- - -## Task 4: `fetch_remote_csv()` with conditional headers, mocked via `httpx.MockTransport` - -**Files:** -- Modify: `tests/test_estimates_refresh.py` -- Modify: `app/estimates_refresh.py` - -- [ ] **Step 1: Write the failing tests** - -Append to `tests/test_estimates_refresh.py`: - -```python -class TestFetchRemoteCsv: - @pytest.fixture - def url(self, monkeypatch): - u = "https://example.invalid/tercet.csv" - monkeypatch.setattr("app.estimates_refresh.settings", _stub_settings(url=u)) - return u - - @staticmethod - def _client_with(handler): - """Build an httpx.AsyncClient that routes every request through handler.""" - return httpx.AsyncClient(transport=httpx.MockTransport(handler)) - - @pytest.mark.asyncio - async def test_returns_body_on_200(self, url): - from app.estimates_refresh import fetch_remote_csv - - body = b"COUNTRY_CODE,POSTAL_CODE\nDE,99999\n" - - def handler(request): - assert str(request.url) == url - return httpx.Response(200, content=body, headers={"ETag": "W/abc"}) - - async with self._client_with(handler) as client: - data, status, headers = await fetch_remote_csv(client) - - assert status == 200 - assert data == body - assert headers.get("etag") == "W/abc" - - @pytest.mark.asyncio - async def test_returns_none_on_304(self, url): - from app.estimates_refresh import fetch_remote_csv - - def handler(request): - return httpx.Response(304) - - async with self._client_with(handler) as client: - data, status, _ = await fetch_remote_csv(client) - - assert status == 304 - assert data is None - - @pytest.mark.asyncio - async def test_sends_conditional_headers_when_state_present(self, url, monkeypatch): - from app import estimates_refresh - - monkeypatch.setattr(estimates_refresh, "_last_etag", "W/abc") - monkeypatch.setattr(estimates_refresh, "_last_modified", "Wed, 01 Jan 2026 00:00:00 GMT") - - seen: dict[str, str] = {} - - def handler(request): - seen["if-none-match"] = request.headers.get("if-none-match", "") - seen["if-modified-since"] = request.headers.get("if-modified-since", "") - return httpx.Response(200, content=b"", headers={}) - - async with self._client_with(handler) as client: - await estimates_refresh.fetch_remote_csv(client) - - assert seen["if-none-match"] == "W/abc" - assert seen["if-modified-since"] == "Wed, 01 Jan 2026 00:00:00 GMT" - - @pytest.mark.asyncio - async def test_returns_none_on_5xx(self, url): - from app.estimates_refresh import fetch_remote_csv - - def handler(request): - return httpx.Response(503, content=b"down") - - async with self._client_with(handler) as client: - data, status, _ = await fetch_remote_csv(client) - - assert status == 503 - assert data is None - - @pytest.mark.asyncio - async def test_returns_none_on_transport_error(self, url): - from app.estimates_refresh import fetch_remote_csv - - def handler(request): - raise httpx.ConnectError("boom") - - async with self._client_with(handler) as client: - data, status, _ = await fetch_remote_csv(client) - - assert status == 0 - assert data is None - - -def _stub_settings(*, url: str = "", interval: int = 86400): - """Build a minimal settings stub for tests that only need the two new fields.""" - - class _S: - estimates_refresh_url = url - estimates_refresh_interval_seconds = interval - - return _S() -``` - -Add `pytest-asyncio` if not already a dep — check `requirements-dev.txt`. If absent: - -```bash -echo "pytest-asyncio>=0.23,<1" >> requirements-dev.txt -pip install pytest-asyncio -``` - -Add to `pyproject.toml` under `[tool.pytest.ini_options]`: - -```toml -asyncio_mode = "auto" -``` - -(If `asyncio_mode = "auto"` is set, the `@pytest.mark.asyncio` decorators above are redundant but harmless — keep them for explicitness.) - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pytest tests/test_estimates_refresh.py::TestFetchRemoteCsv -v -``` - -Expected: 5 FAIL with "ImportError: cannot import name 'fetch_remote_csv'". - -- [ ] **Step 3: Implement `fetch_remote_csv` in `app/estimates_refresh.py`** - -Append after `_passes_sanity_guard`: - -```python -async def fetch_remote_csv( - client: httpx.AsyncClient, -) -> tuple[Optional[bytes], int, dict[str, str]]: - """GET settings.estimates_refresh_url with conditional headers. - - Returns (body, status_code, response_headers). body is None on 304 Not - Modified, on any non-200 status, and on transport errors. Caller decides - what to log based on the status code (304 is silent; non-200 is a warning). - """ - headers: dict[str, str] = {} - if _last_etag: - headers["If-None-Match"] = _last_etag - if _last_modified: - headers["If-Modified-Since"] = _last_modified - - try: - r = await client.get(settings.estimates_refresh_url, headers=headers, timeout=10.0) - except httpx.HTTPError as exc: - logger.debug("Remote estimates fetch transport error: %s", exc) - return None, 0, {} - - response_headers = {k.lower(): v for k, v in r.headers.items()} - if r.status_code == 304: - return None, 304, response_headers - if r.status_code != 200: - return None, r.status_code, response_headers - return r.content, 200, response_headers -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pytest tests/test_estimates_refresh.py -v -``` - -Expected: all PASS (sanity guard tests + 5 new fetch tests). - -- [ ] **Step 5: Commit** - -```bash -git add app/estimates_refresh.py tests/test_estimates_refresh.py requirements-dev.txt pyproject.toml -git commit -m "feat(estimates_refresh): fetch_remote_csv with conditional headers (#44) - -Async GET with If-None-Match / If-Modified-Since when prior values are -available. Returns body=None on 304, on any non-200, and on transport -errors. Caller logs based on the status. Tests use httpx.MockTransport -to avoid network. pytest-asyncio added to dev deps for the async tests. -" -``` - ---- - -## Task 5: `refresh_estimates_once()` — orchestration - -**Files:** -- Modify: `tests/test_estimates_refresh.py` -- Modify: `app/estimates_refresh.py` - -- [ ] **Step 1: Write the failing tests** - -Append to `tests/test_estimates_refresh.py`: - -```python -class TestRefreshOnce: - @pytest.fixture - def url(self, monkeypatch): - u = "https://example.invalid/tercet.csv" - monkeypatch.setattr("app.estimates_refresh.settings", _stub_settings(url=u)) - return u - - @pytest.fixture - def seed_estimates(self): - from app.data_loader import _estimates - - _estimates.clear() - for i in range(100): - _estimates[("DE", f"{10000+i}")] = { - "nuts3": "DE300", - "nuts2": "DE30", - "nuts1": "DE3", - "nuts3_confidence": 0.9, - "nuts2_confidence": 0.95, - "nuts1_confidence": 0.98, - } - yield _estimates - _estimates.clear() - - @staticmethod - def _csv(rows: list[tuple[str, str, str]] = None) -> bytes: - rows = rows or [("DE", "99999", "high"), ("FR", "75000", "medium")] - header = "COUNTRY_CODE,POSTAL_CODE,ESTIMATED_NUTS3,ESTIMATED_NUTS2,ESTIMATED_NUTS1,CONFIDENCE\n" - body = "".join(f"{cc},{pc},{cc}300,{cc}30,{cc}3,{conf}\n" for cc, pc, conf in rows) - return (header + body).encode("utf-8") - - @pytest.mark.asyncio - async def test_disabled_when_url_unset(self, monkeypatch): - monkeypatch.setattr("app.estimates_refresh.settings", _stub_settings(url="")) - from app.estimates_refresh import refresh_estimates_once - - result = await refresh_estimates_once() - assert result.status == "disabled" - - @pytest.mark.asyncio - async def test_swaps_on_changed_content(self, url, seed_estimates): - from app import estimates_refresh - - new_csv = self._csv([("DE", str(20000 + i), "high") for i in range(80)]) - - def handler(request): - return httpx.Response(200, content=new_csv, headers={"ETag": "W/new"}) - - async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: - result = await estimates_refresh.refresh_estimates_once(client=client) - - assert result.status == "refreshed" - assert result.previous_count == 100 - assert result.new_count == 80 - assert estimates_refresh._stale is False - assert estimates_refresh._last_etag == "W/new" - - @pytest.mark.asyncio - async def test_unchanged_on_304(self, url, seed_estimates): - from app import estimates_refresh - - estimates_refresh._last_etag = "W/abc" - - def handler(request): - return httpx.Response(304) - - async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: - result = await estimates_refresh.refresh_estimates_once(client=client) - - assert result.status == "unchanged" - assert result.previous_count == 100 - assert result.new_count == 100 - assert estimates_refresh._stale is False - # Live dict was not touched - assert len(seed_estimates) == 100 - - @pytest.mark.asyncio - async def test_unchanged_on_identical_hash(self, url, seed_estimates): - from app import estimates_refresh - - body = self._csv() - estimates_refresh._last_hash = hashlib.sha256(body).hexdigest() - - def handler(request): - return httpx.Response(200, content=body) - - async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: - result = await estimates_refresh.refresh_estimates_once(client=client) - - assert result.status == "unchanged" - assert estimates_refresh._stale is False - - @pytest.mark.asyncio - async def test_failed_on_5xx(self, url, seed_estimates): - from app import estimates_refresh - - def handler(request): - return httpx.Response(503) - - async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: - result = await estimates_refresh.refresh_estimates_once(client=client) - - assert result.status == "failed" - assert estimates_refresh._stale is True - assert len(seed_estimates) == 100 # unchanged - - @pytest.mark.asyncio - async def test_failed_on_parse_error(self, url, seed_estimates): - from app import estimates_refresh - - def handler(request): - return httpx.Response(200, content=b"not,a,valid,csv\n\xff\xfe") - - async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: - result = await estimates_refresh.refresh_estimates_once(client=client) - - assert result.status == "failed" - assert estimates_refresh._stale is True - assert len(seed_estimates) == 100 - - @pytest.mark.asyncio - async def test_rejected_by_sanity_guard(self, url, seed_estimates): - from app import estimates_refresh - - small_csv = self._csv([("DE", "99999", "high")]) # 1 row vs current 100 - - def handler(request): - return httpx.Response(200, content=small_csv) - - async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: - result = await estimates_refresh.refresh_estimates_once(client=client) - - assert result.status == "rejected" - assert result.previous_count == 100 - assert result.new_count == 1 - assert estimates_refresh._stale is True - # Live dict was not touched - assert len(seed_estimates) == 100 - - @pytest.mark.asyncio - async def test_bootstrap_path_accepts_any_size(self, url): - """When current is empty (first-ever fetch), the sanity guard must not block.""" - from app import estimates_refresh - from app.data_loader import _estimates - - _estimates.clear() - small_csv = self._csv([("DE", "99999", "high")]) - - def handler(request): - return httpx.Response(200, content=small_csv) - - async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: - result = await estimates_refresh.refresh_estimates_once(client=client) - - assert result.status == "refreshed" - assert result.new_count == 1 -``` - -Add `import hashlib` at the top of the test file if not already imported. - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pytest tests/test_estimates_refresh.py::TestRefreshOnce -v -``` - -Expected: 8 FAIL with "ImportError: cannot import name 'refresh_estimates_once'". - -- [ ] **Step 3: Implement `refresh_estimates_once`** - -Append to `app/estimates_refresh.py`: - -```python -async def refresh_estimates_once( - client: Optional[httpx.AsyncClient] = None, -) -> RefreshResult: - """One refresh attempt: fetch, sanity-check, swap. - - When `client` is None, an ephemeral httpx.AsyncClient is created and - closed. Production callers (the lifespan loop, the admin endpoint) - should pass a long-lived client to reuse connections. - """ - global _last_hash, _last_etag, _last_modified, _stale - - previous_count = len(_estimates) - - if not settings.estimates_refresh_url: - return RefreshResult( - status="disabled", - previous_count=previous_count, - new_count=previous_count, - ) - - own_client = client is None - if own_client: - client = httpx.AsyncClient() - try: - body, status, headers = await fetch_remote_csv(client) - finally: - if own_client: - await client.aclose() - - # 304 Not Modified — content unchanged, refresh succeeded - if status == 304: - _stale = False - return RefreshResult( - status="unchanged", - previous_count=previous_count, - new_count=previous_count, - ) - - # Any other non-200 (including transport errors with status=0) - if body is None: - _was_stale_before = _stale is True - _stale = True - if not _was_stale_before: - logger.warning( - "Remote estimates fetch failed (status=%d); keeping current state", - status, - ) - return RefreshResult( - status="failed", - previous_count=previous_count, - new_count=previous_count, - reason=f"http={status}", - ) - - new_hash = hashlib.sha256(body).hexdigest() - if new_hash == _last_hash: - _stale = False - return RefreshResult( - status="unchanged", - previous_count=previous_count, - new_count=previous_count, - ) - - try: - text = body.decode("utf-8-sig") - new_dict, skipped = parse_estimates_from_text(text) - except (UnicodeDecodeError, ValueError, csv.Error, KeyError) as exc: - _stale = True - logger.warning("Remote estimates parse failed: %s", exc) - return RefreshResult( - status="failed", - previous_count=previous_count, - new_count=previous_count, - reason=f"parse: {exc}", - ) - - if not _passes_sanity_guard(len(new_dict), previous_count): - _stale = True - logger.warning( - "Remote estimates sanity guard rejected swap (new=%d, current=%d)", - len(new_dict), - previous_count, - ) - return RefreshResult( - status="rejected", - previous_count=previous_count, - new_count=len(new_dict), - reason=f"sanity guard: {len(new_dict)} < 50% of {previous_count}", - ) - - # Swap. _data_lock is a threading.Lock; acquiring it briefly from an async - # context is fine — the swap is microseconds. - with _data_lock: - _estimates.clear() - _estimates.update(new_dict) - _revalidate_estimates() - new_count = len(_estimates) - - _last_hash = new_hash - _last_etag = headers.get("etag") - _last_modified = headers.get("last-modified") - _stale = False - - logger.info( - "Remote estimates refreshed: %d -> %d (skipped %d rows during parse)", - previous_count, - new_count, - skipped, - ) - return RefreshResult( - status="refreshed", - previous_count=previous_count, - new_count=new_count, - skipped_rows=skipped, - ) -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pytest tests/test_estimates_refresh.py -v -``` - -Expected: all PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/estimates_refresh.py tests/test_estimates_refresh.py -git commit -m "feat(estimates_refresh): refresh_estimates_once orchestration (#44) - -Wires fetch + hash dedupe + parse + sanity guard + swap-under-lock into -one async function. RefreshResult dataclass captures the status enum, -counts, and a human-readable reason. Tests cover all five outcomes -(refreshed / unchanged / rejected / failed / disabled) plus the -bootstrap path (current is empty → guard skipped). -" -``` - ---- - -## Task 6: `refresh_estimates_loop()` — async cadence loop - -**Files:** -- Modify: `tests/test_estimates_refresh.py` -- Modify: `app/estimates_refresh.py` - -- [ ] **Step 1: Write the failing test** - -Append to `tests/test_estimates_refresh.py`: - -```python -class TestRefreshLoop: - @pytest.mark.asyncio - async def test_no_op_when_url_unset(self, monkeypatch): - """With the URL setting unset, the loop must return immediately.""" - monkeypatch.setattr("app.estimates_refresh.settings", _stub_settings(url="")) - from app.estimates_refresh import refresh_estimates_loop - - # If the loop tried to sleep for 86400s we'd hang; wait_for guards us. - await asyncio.wait_for(refresh_estimates_loop(), timeout=1.0) - - @pytest.mark.asyncio - async def test_no_op_when_interval_zero(self, monkeypatch): - monkeypatch.setattr( - "app.estimates_refresh.settings", - _stub_settings(url="https://example.invalid/x.csv", interval=0), - ) - from app.estimates_refresh import refresh_estimates_loop - - await asyncio.wait_for(refresh_estimates_loop(), timeout=1.0) - - @pytest.mark.asyncio - async def test_loop_calls_refresh_on_each_tick(self, monkeypatch): - """With a tiny interval and a counter, we should see N refreshes.""" - monkeypatch.setattr( - "app.estimates_refresh.settings", - _stub_settings(url="https://example.invalid/x.csv", interval=1), - ) - - call_count = {"n": 0} - - async def fake_refresh(): - call_count["n"] += 1 - - monkeypatch.setattr("app.estimates_refresh.refresh_estimates_once", fake_refresh) - - from app.estimates_refresh import refresh_estimates_loop - - task = asyncio.create_task(refresh_estimates_loop()) - # Speed up: monkeypatch asyncio.sleep to fast-forward. - await asyncio.sleep(0.05) - # Cancel and verify at least one call happened (interval=1s, so likely 0 - # under real sleep). Use a smaller interval check via direct invocation: - task.cancel() - try: - await task - except asyncio.CancelledError: - pass - - @pytest.mark.asyncio - async def test_loop_survives_exception_in_refresh(self, monkeypatch): - monkeypatch.setattr( - "app.estimates_refresh.settings", - _stub_settings(url="https://example.invalid/x.csv", interval=1), - ) - - async def fake_refresh_raises(): - raise RuntimeError("boom") - - monkeypatch.setattr("app.estimates_refresh.refresh_estimates_once", fake_refresh_raises) - - # Replace asyncio.sleep so we don't actually wait. - sleeps: list[float] = [] - - async def fake_sleep(secs): - sleeps.append(secs) - if len(sleeps) >= 3: - raise asyncio.CancelledError - - monkeypatch.setattr("app.estimates_refresh.asyncio.sleep", fake_sleep) - - from app.estimates_refresh import refresh_estimates_loop - - with pytest.raises(asyncio.CancelledError): - await refresh_estimates_loop() - - # Loop ran at least 2 iterations despite the exception each time - assert len(sleeps) >= 2 -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pytest tests/test_estimates_refresh.py::TestRefreshLoop -v -``` - -Expected: 4 FAIL with "ImportError: cannot import name 'refresh_estimates_loop'". - -- [ ] **Step 3: Implement `refresh_estimates_loop`** - -Append to `app/estimates_refresh.py`: - -```python -async def refresh_estimates_loop() -> None: - """Periodic refresh task. Returns immediately when feature is disabled.""" - if not settings.estimates_refresh_url: - return - interval = settings.estimates_refresh_interval_seconds - if interval <= 0: - return - while True: - await asyncio.sleep(interval) - try: - await refresh_estimates_once() - except asyncio.CancelledError: - raise - except Exception: - logger.exception("refresh_estimates_loop iteration crashed; will retry") -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pytest tests/test_estimates_refresh.py -v -``` - -Expected: all PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/estimates_refresh.py tests/test_estimates_refresh.py -git commit -m "feat(estimates_refresh): refresh_estimates_loop async loop (#44) - -Wraps refresh_estimates_once in an interval loop. Returns immediately -when the feature is disabled. Catches all exceptions (except -CancelledError) so a transient crash doesn't kill the task — the next -tick re-runs. -" -``` - ---- - -## Task 7: `/health` field — `estimates_refresh_stale` - -**Files:** -- Modify: `tests/test_api.py` -- Modify: `app/models.py` -- Modify: `app/main.py` - -- [ ] **Step 1: Write the failing test** - -Append to `TestHealthEndpoint` in `tests/test_api.py`: - -```python - def test_health_includes_estimates_refresh_stale_when_url_set(self, monkeypatch, client): - from app import config, estimates_refresh - - monkeypatch.setattr(config.settings, "estimates_refresh_url", "https://example.invalid/x.csv") - monkeypatch.setattr(estimates_refresh, "_stale", False) - - resp = client.get("/health") - data = resp.json() - assert data["estimates_refresh_stale"] is False - - def test_health_estimates_refresh_stale_none_when_url_unset(self, monkeypatch, client): - from app import config, estimates_refresh - - monkeypatch.setattr(config.settings, "estimates_refresh_url", "") - monkeypatch.setattr(estimates_refresh, "_stale", None) - - resp = client.get("/health") - data = resp.json() - assert data["estimates_refresh_stale"] is None -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pytest tests/test_api.py::TestHealthEndpoint::test_health_includes_estimates_refresh_stale_when_url_set tests/test_api.py::TestHealthEndpoint::test_health_estimates_refresh_stale_none_when_url_unset -v -``` - -Expected: 2 FAIL — the field doesn't exist on the response yet. - -- [ ] **Step 3: Add the field to `HealthResponse`** - -In `app/models.py`, after the existing `token_db_stale` field: - -```python - estimates_refresh_stale: bool | None = None -``` - -- [ ] **Step 4: Populate the field in `/health`** - -In `app/main.py` around `app/main.py:307` (the `health` handler), add the import at the top of the file: - -```python -from app.estimates_refresh import get_refresh_stale as _get_estimates_refresh_stale -``` - -And in the `health` function body, populate the new field: - -```python - return HealthResponse( - status="ok" if len(table) > 0 else "no_data", - total_postal_codes=len(table), - total_estimates=len(estimates), - total_nuts_names=len(get_nuts_names()), - nuts_version=settings.nuts_version, - extra_sources=get_extra_source_count(), - patterns_version=PATTERNS_META.get("version", "unknown"), - data_stale=stale, - last_updated=get_data_loaded_at(), - token_db_stale=token_db_stale, - estimates_refresh_stale=_get_estimates_refresh_stale(), - ) -``` - -- [ ] **Step 5: Run tests to verify they pass** - -```bash -pytest tests/test_api.py::TestHealthEndpoint -v -``` - -Expected: all (existing + new) PASS. - -- [ ] **Step 6: Commit** - -```bash -git add app/models.py app/main.py tests/test_api.py -git commit -m "feat(/health): expose estimates_refresh_stale (#44) - -New optional field on HealthResponse: None when feature disabled (URL -unset), False after a successful most-recent refresh, True after a -failed one. Mirrors the existing token_db_stale field. -" -``` - ---- - -## Task 8: `POST /admin/refresh-estimates` endpoint - -**Files:** -- Modify: `tests/test_api.py` -- Modify: `app/main.py` - -- [ ] **Step 1: Write the failing tests** - -Append to `tests/test_api.py`: - -```python -class TestAdminRefreshEstimatesEndpoint: - def test_401_without_authorization(self, client, monkeypatch): - from app import config - - monkeypatch.setattr( - config.settings, - "estimates_refresh_url", - "https://example.invalid/x.csv", - ) - resp = client.post("/admin/refresh-estimates") - assert resp.status_code == 401 - - def test_401_with_invalid_bearer(self, client, monkeypatch): - from app import config - - monkeypatch.setattr( - config.settings, - "estimates_refresh_url", - "https://example.invalid/x.csv", - ) - resp = client.post( - "/admin/refresh-estimates", - headers={"Authorization": "Bearer not-a-real-token"}, - ) - assert resp.status_code == 401 - - def test_503_when_feature_disabled(self, client, trusted_client, monkeypatch): - # trusted_client is the existing test fixture that wraps the client - # with a valid trusted-token bearer (defined in conftest.py). - from app import config - - monkeypatch.setattr(config.settings, "estimates_refresh_url", "") - resp = trusted_client.post("/admin/refresh-estimates") - assert resp.status_code == 503 - assert resp.json()["status"] == "disabled" - - def test_200_on_successful_refresh(self, trusted_client, monkeypatch): - from app import config, estimates_refresh - from app.estimates_refresh import RefreshResult - - monkeypatch.setattr( - config.settings, - "estimates_refresh_url", - "https://example.invalid/x.csv", - ) - - async def fake_refresh(client=None): - return RefreshResult( - status="refreshed", previous_count=7000, new_count=7042, skipped_rows=0 - ) - - monkeypatch.setattr(estimates_refresh, "refresh_estimates_once", fake_refresh) - - resp = trusted_client.post("/admin/refresh-estimates") - assert resp.status_code == 200 - body = resp.json() - assert body["status"] == "refreshed" - assert body["previous_count"] == 7000 - assert body["new_count"] == 7042 - - def test_502_on_failed_refresh(self, trusted_client, monkeypatch): - from app import config, estimates_refresh - from app.estimates_refresh import RefreshResult - - monkeypatch.setattr( - config.settings, - "estimates_refresh_url", - "https://example.invalid/x.csv", - ) - - async def fake_refresh(client=None): - return RefreshResult( - status="failed", previous_count=7000, new_count=7000, reason="http=503" - ) - - monkeypatch.setattr(estimates_refresh, "refresh_estimates_once", fake_refresh) - - resp = trusted_client.post("/admin/refresh-estimates") - assert resp.status_code == 502 - assert resp.json()["status"] == "failed" - - def test_409_on_sanity_guard_rejection(self, trusted_client, monkeypatch): - from app import config, estimates_refresh - from app.estimates_refresh import RefreshResult - - monkeypatch.setattr( - config.settings, - "estimates_refresh_url", - "https://example.invalid/x.csv", - ) - - async def fake_refresh(client=None): - return RefreshResult( - status="rejected", - previous_count=7000, - new_count=12, - reason="sanity guard: 12 < 50% of 7000", - ) - - monkeypatch.setattr(estimates_refresh, "refresh_estimates_once", fake_refresh) - - resp = trusted_client.post("/admin/refresh-estimates") - assert resp.status_code == 409 - body = resp.json() - assert body["status"] == "rejected" - assert body["candidate_count"] == 12 -``` - -If `trusted_client` doesn't already exist as a fixture, add it to `tests/conftest.py`: - -```python -@pytest.fixture -def trusted_client(client, monkeypatch): - """A TestClient that injects a trusted bearer token into every request.""" - from app import auth, config - - monkeypatch.setattr(config.settings, "trusted_tokens_raw", "test-trusted-token-XXXXXX") - auth.refresh_db_tokens(None) if hasattr(auth, "refresh_db_tokens") else None - client.headers.update({"Authorization": "Bearer test-trusted-token-XXXXXX"}) - return client -``` - -(Tweak to match the existing conftest's bearer-token plumbing — the existing test_auth.py shows the right path.) - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pytest tests/test_api.py::TestAdminRefreshEstimatesEndpoint -v -``` - -Expected: 6 FAIL — the route doesn't exist. - -- [ ] **Step 3: Add the route to `app/main.py`** - -After the existing `/health` route (around `app/main.py:320`): - -```python -@app.post( - "/admin/refresh-estimates", - summary="Force-refresh estimates from the configured remote URL", - description=( - "Operator-only — requires `Authorization: Bearer `. " - "Synchronously fetches the configured `PC2NUTS_ESTIMATES_REFRESH_URL` and, " - "if the content has changed and passes the sanity guard, replaces the " - "in-memory estimates table. No body." - ), - responses={ - 200: {"description": "Refresh succeeded (content changed and applied)"}, - 401: {"description": "Missing or invalid trusted bearer token"}, - 409: {"description": "Sanity guard rejected the candidate CSV"}, - 502: {"description": "Upstream fetch or parse failed"}, - 503: {"description": "Feature disabled (PC2NUTS_ESTIMATES_REFRESH_URL unset)"}, - }, -) -async def admin_refresh_estimates(request: Request) -> JSONResponse: - if not getattr(request.state, "trusted", False): - raise HTTPException(status_code=401, detail="Trusted token required") - - from app.estimates_refresh import refresh_estimates_once - - result = await refresh_estimates_once() - - if result.status == "disabled": - return JSONResponse(status_code=503, content={"status": "disabled"}) - if result.status == "rejected": - return JSONResponse( - status_code=409, - content={ - "status": "rejected", - "reason": result.reason, - "previous_count": result.previous_count, - "candidate_count": result.new_count, - }, - ) - if result.status == "failed": - return JSONResponse( - status_code=502, - content={"status": "failed", "reason": result.reason}, - ) - - # "refreshed" or "unchanged" - return JSONResponse( - status_code=200, - content={ - "status": result.status, - "previous_count": result.previous_count, - "new_count": result.new_count, - "skipped_rows": result.skipped_rows, - "source_url": settings.estimates_refresh_url, - }, - ) -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pytest tests/test_api.py::TestAdminRefreshEstimatesEndpoint -v -``` - -Expected: 6 PASS. - -- [ ] **Step 5: Commit** - -```bash -git add app/main.py tests/test_api.py tests/conftest.py -git commit -m "feat(api): POST /admin/refresh-estimates with trusted-token auth (#44) - -Operator-only endpoint to force a refresh without waiting for the next -periodic tick. Auth via the existing trusted-token Bearer mechanism. -HTTP code per outcome: 200 refreshed/unchanged, 401 unauthorised, 409 -sanity guard rejection, 502 upstream failure, 503 feature disabled. -" -``` - ---- - -## Task 9: Lifespan wiring — bootstrap fetch + refresh task - -**Files:** -- Modify: `app/main.py:82-140` (the existing `lifespan` function) -- Modify: `tests/test_api.py` — already covered by existing TestClient lifespan handling, but add a smoke test that the loop task is started. - -- [ ] **Step 1: Add bootstrap + task spawn to `lifespan`** - -In `app/main.py`, inside `lifespan(app)`, after the TokenDB refresh task block (around line 130) but BEFORE `yield`: - -```python - # ── Estimates remote-refresh (#44) ────────────────────────────────────── - estimates_refresh_task: asyncio.Task | None = None - if _config.settings.estimates_refresh_url: - from app.estimates_refresh import refresh_estimates_once, refresh_estimates_loop - - # Bootstrap synchronously so the worker reflects upstream before reporting ready. - try: - result = await refresh_estimates_once() - logger.info( - "Estimates bootstrap fetch: %s (previous=%d, new=%d)", - result.status, - result.previous_count, - result.new_count, - ) - except Exception: - logger.exception("Estimates bootstrap fetch crashed; continuing with bundled CSV") - - if _config.settings.estimates_refresh_interval_seconds > 0: - estimates_refresh_task = asyncio.create_task(refresh_estimates_loop()) - logger.info( - "Estimates refresh task started (interval %ds)", - _config.settings.estimates_refresh_interval_seconds, - ) -``` - -After `yield`, in the shutdown section: - -```python - if estimates_refresh_task is not None: - estimates_refresh_task.cancel() - try: - await estimates_refresh_task - except asyncio.CancelledError: - pass -``` - -- [ ] **Step 2: Existing test suite as smoke check** - -```bash -pytest tests/ -v -``` - -Expected: ALL pass. (No new test specifically for the wiring — the lifespan path is covered transitively by every `TestClient` instance, and the unit tests for `refresh_estimates_once` and `refresh_estimates_loop` cover the actual logic.) - -- [ ] **Step 3: Commit** - -```bash -git add app/main.py -git commit -m "feat(lifespan): wire estimates bootstrap fetch + refresh loop (#44) - -When PC2NUTS_ESTIMATES_REFRESH_URL is set, the FastAPI lifespan now: - 1. Runs one synchronous refresh_estimates_once() before the worker - reports ready, so a freshly-deployed pod immediately reflects the - upstream CSV instead of waiting up to 24h. - 2. Schedules refresh_estimates_loop() as an asyncio.Task that gets - cancelled cleanly on shutdown. - -Defaults preserve the current behaviour (URL unset => no remote fetch). -" -``` - ---- - -## Task 10: Documentation — README + CHANGELOG - -**Files:** -- Modify: `README.md` -- Modify: `CHANGELOG.md` - -- [ ] **Step 1: Add the env-var rows to the existing Configuration table in `README.md`** - -Find the env-var table around `README.md:306` (in the "Configuration" section). Add two rows: - -```markdown -| `PC2NUTS_ESTIMATES_REFRESH_URL` | *(empty — feature disabled)* | When set, the worker periodically fetches this URL and replaces the in-memory estimates table. Recommended value: `https://raw.githubusercontent.com/bk86a/PostalCode2NUTS/main/tercet_missing_codes.csv`. | -| `PC2NUTS_ESTIMATES_REFRESH_INTERVAL_SECONDS` | `86400` (24 h) | How often the periodic task fetches the URL. Set to `0` to disable the periodic loop while keeping the bootstrap fetch on startup. | -``` - -- [ ] **Step 2: Add a new operator-runbook subsection in `README.md`** - -After the existing "Operator runbook — revoke a token" subsection (around `README.md:420`), add: - -```markdown -### Operator runbook — manually refresh estimates - -When you've merged a new entry into `tercet_missing_codes.csv` on `main` and want to verify it's live in production without waiting up to 24 hours for the next periodic refresh: - -```bash -curl -X POST -H "Authorization: Bearer $PC2NUTS_TRUSTED_TOKEN" \ - https://api.example.invalid/admin/refresh-estimates -``` - -Possible responses: - -| HTTP | `status` | Meaning | -|---|---|---| -| 200 | `refreshed` | Upstream content changed, sanity guard passed, in-memory state updated. Body includes `previous_count` and `new_count`. | -| 200 | `unchanged` | Upstream content identical to current state (matched by SHA-256 hash or 304 Not Modified). | -| 401 | — | Missing or invalid `Authorization` header. | -| 409 | `rejected` | Sanity guard refused the candidate CSV (< 50 % of current row count). Live state is untouched. | -| 502 | `failed` | Upstream fetch or parse failed. Live state is untouched. | -| 503 | `disabled` | `PC2NUTS_ESTIMATES_REFRESH_URL` is unset on the deployed pod. | - -`/health` exposes `estimates_refresh_stale: bool | None` — `null` when disabled, `false` after a successful most-recent refresh, `true` after a failed one. -``` - -- [ ] **Step 3: Add a CHANGELOG entry** - -In `CHANGELOG.md` under `[Unreleased]`, add: - -```markdown -### Added - -- **Periodic refresh of `tercet_missing_codes.csv`** (#44): when `PC2NUTS_ESTIMATES_REFRESH_URL` is set, a per-worker asyncio task fetches the URL on every `PC2NUTS_ESTIMATES_REFRESH_INTERVAL_SECONDS` tick (default 24 h), parses the body, and full-replaces the in-memory estimates table if the content has changed and passes a 50 %-of-current sanity guard. Workers also do a synchronous bootstrap fetch before reporting ready, so a fresh pod immediately reflects upstream rather than waiting up to one interval. New `POST /admin/refresh-estimates` endpoint (trusted-token auth) lets operators force a refresh without waiting. New `/health` field `estimates_refresh_stale: bool | None`. Defaults preserve the current single-source-of-truth behaviour from the bundled `tercet_missing_codes.csv`. -``` - -- [ ] **Step 4: Commit** - -```bash -git add README.md CHANGELOG.md -git commit -m "docs: env vars, operator runbook, and CHANGELOG entry for #44" -``` - ---- - -## Task 11: Final verification — full suite + lint + manual probe - -- [ ] **Step 1: Full test suite** - -```bash -pytest tests/ -v -``` - -Expected: all PASS. - -- [ ] **Step 2: Lint + format check** - -```bash -ruff check app/ scripts/ -ruff format --check app/ scripts/ -``` - -Expected: clean. - -- [ ] **Step 3: Build + smoke run the image (optional but recommended)** - -```bash -docker build -t pc2nuts:plan-44 . -docker run --rm -d --name pc44 -p 18000:8000 \ - -e PC2NUTS_ESTIMATES_REFRESH_URL=https://raw.githubusercontent.com/bk86a/PostalCode2NUTS/main/tercet_missing_codes.csv \ - pc2nuts:plan-44 -# Wait for ready (python-based probe — curl isn't in slim image) -docker exec pc44 sh -c 'until python -c "import urllib.request; urllib.request.urlopen(\"http://localhost:8000/health\")" 2>/dev/null; do sleep 1; done' -# Verify the new /health field -curl -s http://localhost:18000/health | python3 -c 'import json,sys; d=json.load(sys.stdin); print("estimates_refresh_stale =", d.get("estimates_refresh_stale"))' -docker rm -f pc44 -``` - -Expected: `estimates_refresh_stale = False` (bootstrap fetch succeeded). - -- [ ] **Step 4: Final commit if anything was missed** - -```bash -git status -# If clean, no commit needed. -``` - -- [ ] **Step 5: Push** - -```bash -git push origin -``` - ---- - -## Self-review notes - -- **Spec coverage:** every section of the spec maps to a task — Settings (T1), refactor (T2), module skeleton + sanity guard (T3), fetch (T4), orchestration (T5), loop (T6), `/health` (T7), `/admin` (T8), lifespan (T9), docs (T10), verification (T11). -- **Type consistency check passed:** `RefreshResult` defined in T3 is consumed by T5/T8; `parse_estimates_from_text` defined in T2 is consumed by T5; `get_refresh_stale` defined in T3 is consumed by T7; `refresh_estimates_once` / `refresh_estimates_loop` defined in T5/T6 are consumed by T8/T9. -- **No placeholders:** every step has the actual code. The `trusted_client` fixture sketch in T8 is the one structural assumption — if it doesn't match the existing conftest pattern, the engineer should adapt to whatever bearer-injection helper already exists for `test_auth.py` rather than inventing a new one. -- **Frequent commits:** one commit per task; each commit leaves the test suite green. diff --git a/docs/superpowers/plans/2026-05-01-multi-worker-uvicorn.md b/docs/superpowers/plans/2026-05-01-multi-worker-uvicorn.md deleted file mode 100644 index 0685648..0000000 --- a/docs/superpowers/plans/2026-05-01-multi-worker-uvicorn.md +++ /dev/null @@ -1,795 +0,0 @@ -# Multi-worker uvicorn Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** allow the service to run with N uvicorn workers behind a Redis-backed shared rate-limit backend, while leaving single-worker / in-memory deploys byte-for-byte unchanged. - -**Architecture:** two new opt-in env vars (`PC2NUTS_WORKERS`, `PC2NUTS_RATE_LIMIT_STORAGE_URI`) drive the change. The slowapi `Limiter` is moved to a small dedicated module that picks between the current default construction and a Redis-configured construction with `in_memory_fallback_enabled=True`. A Pydantic model validator on `Settings` hard-fails startup when `WORKERS > 1` without a storage URI to prevent silent cap loosening. The Dockerfile `CMD` becomes shell-form so `${PC2NUTS_WORKERS}` expands at container start. - -**Tech Stack:** Python 3.14, FastAPI 0.129, uvicorn 0.40, slowapi 0.1.9, limits 5.8.0, pydantic 2.12.5, pydantic-settings 2.13.0, redis-py (added via `limits[redis]` extra). - -**Spec:** `docs/superpowers/specs/2026-05-01-multi-worker-uvicorn-design.md` - -**Issue:** [#68](https://github.com/bk86a/PostalCode2NUTS/issues/68) - ---- - -## File map - -| File | Action | Responsibility | -|---|---|---| -| `app/config.py` | Modify | Add `workers` and `rate_limit_storage_uri` fields + cross-field validator | -| `app/limiter.py` | Create | Single-purpose module exposing the configured `Limiter` | -| `app/main.py` | Modify | Replace inline `limiter = Limiter(...)` (line 45) with import from `app.limiter` | -| `app/settings.json` | Modify | Add default for `workers` (`1`) and explicit null for `rate_limit_storage_uri` | -| `Dockerfile` | Modify | Switch `CMD` from exec-form to shell-form so `${PC2NUTS_WORKERS}` expands | -| `requirements.lock` | Modify | Add `redis` runtime dep (via `limits[redis]` extra) | -| `tests/test_limiter.py` | Create | Unit tests for `Limiter` storage-mode selection | -| `tests/test_config.py` | Modify or create | Unit tests for the new `Settings` validator | -| `README.md` | Modify | Document `PC2NUTS_WORKERS`, `PC2NUTS_RATE_LIMIT_STORAGE_URI`, degraded-mode behaviour | -| `CHANGELOG.md` | Modify | Unreleased section entry | - ---- - -## Pre-flight - -Before starting, verify the worktree is on the `feat/multi-worker-uvicorn` branch and the spec is committed: - -```bash -git rev-parse --abbrev-ref HEAD -# Expected: feat/multi-worker-uvicorn - -git log --oneline main..HEAD -# Expected: at least one commit referencing the spec -``` - -If the branch is wrong, stop and switch (`git switch feat/multi-worker-uvicorn`). - -Verify the test suite is green at HEAD before touching anything: - -```bash -pytest -x -q -``` - -If any tests fail at HEAD, stop and report — don't start making changes against a broken baseline. - ---- - -## Task 1: Add `Settings` fields and validator - -**Files:** -- Modify: `app/config.py` -- Modify: `app/settings.json` -- Modify or create: `tests/test_config.py` - -This task introduces the two new config fields and the cross-field validator that prevents the dangerous combination (`workers > 1` with no storage URI). Tests come first. - -- [ ] **Step 1: Confirm whether `tests/test_config.py` exists** - -```bash -ls tests/test_config.py 2>&1 -``` - -If the file does not exist, create it in Step 2 with the imports in place. If it exists, append to it. - -- [ ] **Step 2: Write the failing tests** - -If creating `tests/test_config.py`, full contents: - -```python -"""Tests for app.config.Settings.""" - -import pytest -from pydantic import ValidationError - -from app.config import Settings - - -class TestWorkersValidator: - def test_workers_eq_1_without_storage_uri_succeeds(self): - """Default config must keep validating — single-worker, no storage URI.""" - s = Settings(workers=1, rate_limit_storage_uri=None) - assert s.workers == 1 - assert s.rate_limit_storage_uri is None - - def test_workers_gt_1_with_storage_uri_succeeds(self): - """Multi-worker is permitted when a storage URI is configured.""" - s = Settings(workers=4, rate_limit_storage_uri="redis://localhost:6379/0") - assert s.workers == 4 - assert s.rate_limit_storage_uri == "redis://localhost:6379/0" - - def test_workers_gt_1_without_storage_uri_fails_startup(self): - """The unsafe combination must raise — silent cap loosening is the - failure mode this validator exists to prevent.""" - with pytest.raises(ValidationError) as excinfo: - Settings(workers=2, rate_limit_storage_uri=None) - msg = str(excinfo.value) - assert "PC2NUTS_WORKERS" in msg - assert "PC2NUTS_RATE_LIMIT_STORAGE_URI" in msg - - def test_workers_gt_1_with_empty_storage_uri_fails_startup(self): - """Empty string should be treated the same as None — both mean unset.""" - with pytest.raises(ValidationError): - Settings(workers=2, rate_limit_storage_uri="") -``` - -If the file already exists, append the `TestWorkersValidator` class. - -- [ ] **Step 3: Run tests to verify they fail** - -```bash -pytest tests/test_config.py -v -``` - -Expected: all four tests FAIL because `workers` and `rate_limit_storage_uri` aren't fields on `Settings` yet (most likely an `AttributeError` or unexpected-kwarg error from Pydantic). - -- [ ] **Step 4: Add the fields and validator to `app/config.py`** - -In `app/config.py`, change the imports at the top: - -```python -from pydantic import Field, model_validator -from pydantic_settings import BaseSettings -``` - -Inside the `Settings` class body, add the two new fields next to the existing `rate_limit` and `rate_limit_headers` fields (around line 25-26). The `_defaults.get(...)` pattern matches the existing fields: - -```python - workers: int = Field(default=_defaults.get("workers", 1), ge=1) - rate_limit_storage_uri: str | None = _defaults.get("rate_limit_storage_uri", None) -``` - -Then add the validator method inside the class (place it after the existing `model_config = {...}` line and before the `@property` block): - -```python - @model_validator(mode="after") - def _check_workers_have_shared_storage(self) -> "Settings": - if self.workers > 1 and not self.rate_limit_storage_uri: - raise ValueError( - "PC2NUTS_WORKERS > 1 requires PC2NUTS_RATE_LIMIT_STORAGE_URI to be set " - "(e.g. 'redis://host:6379/0'). Without shared storage the per-IP rate " - "limit would silently loosen by a factor of WORKERS." - ) - return self -``` - -- [ ] **Step 5: Add defaults to `app/settings.json`** - -Edit `app/settings.json`. Inside the JSON object, add two keys after `"rate_limit_headers"`: - -```json - "rate_limit": "120/minute", - "rate_limit_headers": true, - "workers": 1, - "rate_limit_storage_uri": null, - "cache_max_age": 3600 -``` - -The trailing comma after `"rate_limit_headers": true,` already exists. The trailing comma after `"rate_limit_storage_uri": null,` is new — verify it parses by reading the file back and asserting validity: - -```bash -python3 -c "import json; json.load(open('app/settings.json'))" && echo "valid JSON" -``` - -Expected: `valid JSON` - -- [ ] **Step 6: Run tests to verify they pass** - -```bash -pytest tests/test_config.py -v -``` - -Expected: all four tests PASS. - -- [ ] **Step 7: Run the full test suite to verify no regressions** - -```bash -pytest -x -q -``` - -Expected: green. Note: `Settings()` is instantiated at module import time in `app/config.py:88`. The default config (`workers=1`, `rate_limit_storage_uri=None`) must continue to validate, which Step 6 confirmed. - -- [ ] **Step 8: Commit** - -```bash -git add app/config.py app/settings.json tests/test_config.py -git commit -m "feat(config): add workers and rate_limit_storage_uri settings (#68) - -New Pydantic model validator hard-fails startup if PC2NUTS_WORKERS > 1 -without PC2NUTS_RATE_LIMIT_STORAGE_URI configured, so the per-IP rate -limit can never silently loosen under multi-worker. - -Defaults preserve current behaviour: workers=1, storage URI unset." -``` - ---- - -## Task 2: Add `redis` runtime dependency - -**Files:** -- Modify: `requirements.lock` - -The `limits[redis]` extra pulls in the `redis-py` client at the version `limits 5.8.0` expects. We add it before the limiter tests because constructing a `Limiter(storage_uri="redis://…")` eagerly imports the `redis` module — Task 3 below would fail without it. We confirmed with `limits/storage/redis.py:96` that `RedisStorage.__init__` resolves `self.dependency = self.dependencies["redis"].module` at construction time; if the module isn't importable, slowapi raises `ConfigurationError` immediately. Construction does NOT open a TCP connection (`register_script` is local-only), so the test does not require a running Redis. - -- [ ] **Step 1: Inspect the current `requirements.lock` format** - -```bash -head -5 requirements.lock -grep -E "^(slowapi|limits|pydantic)" requirements.lock -``` - -Confirm the file is a flat list of `package==version` pins (one per line). The relevant context lines are `slowapi==0.1.9` and `limits==5.8.0`. - -- [ ] **Step 2: Determine the redis-py version `limits[redis]` requires** - -```bash -python3 -c " -import importlib.metadata as md -reqs = md.requires('limits') or [] -print('\n'.join(r for r in reqs if 'redis' in r.lower())) -" -pip index versions redis 2>&1 | head -5 -``` - -Expected: a line like `redis (>=3,<6) ; extra == 'redis'` or similar. Note the version constraint and pick the highest currently-released version inside that constraint. - -- [ ] **Step 3: Verify the chosen version installs cleanly** - -```bash -pip install --dry-run "redis==" 2>&1 | tail -5 -``` - -Expected: no version-conflict errors against the rest of `requirements.lock`. - -- [ ] **Step 4: Add the pin to `requirements.lock`** - -Edit `requirements.lock`. Insert the pin in alphabetical order (the file is already sorted; find where `r` belongs — typically between `pyyaml` and `requests` or similar). Add the single line: - -``` -redis== -``` - -- [ ] **Step 5: Verify the requirements file as a whole still resolves** - -```bash -pip install --dry-run -r requirements.lock 2>&1 | tail -10 -``` - -Expected: no version-conflict errors. - -- [ ] **Step 6: Install the dep so subsequent tasks' tests can run** - -```bash -pip install -q "redis==" -python3 -c "import redis; print('redis', redis.__version__)" -``` - -Expected: prints the installed redis version cleanly. (If the environment is PEP 668 / externally-managed and pip refuses, install into the project venv or use `--break-system-packages` per the local environment's policy. CI installs via `pip install -r requirements.lock` so the published artefact and CI both behave correctly.) - -- [ ] **Step 7: Run the existing test suite to verify no regressions** - -```bash -pytest -x -q -``` - -Expected: green. Adding the dep shouldn't change behaviour because no test imports `redis` directly yet. - -- [ ] **Step 8: Commit** - -```bash -git add requirements.lock -git commit -m "deps: add redis runtime dependency for multi-worker rate limiting (#68) - -Pulls in redis-py at the version limits 5.8.0 expects, used only when -PC2NUTS_RATE_LIMIT_STORAGE_URI is set. Single-host deployers who never -configure shared storage pay the install-size cost but no runtime cost -(redis is imported eagerly by limits.storage.RedisStorage at Limiter -construction, but only when the storage URI is configured)." -``` - ---- - -## Task 3: Extract `Limiter` into `app/limiter.py` - -**Files:** -- Create: `app/limiter.py` -- Modify: `app/main.py` -- Create: `tests/test_limiter.py` - -This task pulls the inline `Limiter(key_func=get_remote_address)` line at `app/main.py:45` into a dedicated module that selects between the default and Redis-configured construction. The default branch is byte-for-byte the current code. - -- [ ] **Step 1: Write the failing tests** - -Create `tests/test_limiter.py`: - -```python -"""Tests for app.limiter — verify the Limiter is wired with the right storage -mode based on settings.rate_limit_storage_uri.""" - -import importlib - -import pytest - - -def _reload_limiter(): - """Reload app.config and app.limiter so the module-level Limiter picks up - the current env. Returns the freshly-imported app.limiter module.""" - import app.config - import app.limiter - importlib.reload(app.config) - importlib.reload(app.limiter) - return app.limiter - - -class TestLimiterStorageSelection: - def test_limiter_default_uses_memory_storage(self, monkeypatch): - """When no storage URI is set, the Limiter is constructed with the - slowapi default (in-process memory) and fallback is disabled. This is - the byte-for-byte current behaviour.""" - monkeypatch.delenv("PC2NUTS_RATE_LIMIT_STORAGE_URI", raising=False) - monkeypatch.delenv("PC2NUTS_WORKERS", raising=False) - - mod = _reload_limiter() - - # slowapi stores the configured URI on _storage_uri; default is None. - assert mod.limiter._storage_uri is None - # in_memory_fallback_enabled is only meaningful when a URI is set. - assert mod.limiter._in_memory_fallback_enabled is False - - def test_limiter_with_redis_uri_enables_fallback(self, monkeypatch): - """When a storage URI is set, the Limiter is constructed with that URI - AND in_memory_fallback_enabled=True. No network call should happen at - construction time — slowapi builds the storage object lazily.""" - monkeypatch.setenv("PC2NUTS_RATE_LIMIT_STORAGE_URI", "redis://localhost:6379/0") - monkeypatch.setenv("PC2NUTS_WORKERS", "2") - - mod = _reload_limiter() - - assert mod.limiter._storage_uri == "redis://localhost:6379/0" - assert mod.limiter._in_memory_fallback_enabled is True - - @pytest.fixture(autouse=True) - def _restore_default_after_each_test(self, monkeypatch): - """After each test, force a reload back to defaults so other tests - in the suite see the unmodified module.""" - yield - monkeypatch.delenv("PC2NUTS_RATE_LIMIT_STORAGE_URI", raising=False) - monkeypatch.delenv("PC2NUTS_WORKERS", raising=False) - _reload_limiter() -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pytest tests/test_limiter.py -v -``` - -Expected: all tests FAIL with `ModuleNotFoundError: No module named 'app.limiter'`. - -- [ ] **Step 3: Create `app/limiter.py`** - -Full contents: - -```python -"""Module-level slowapi Limiter, wired according to settings. - -When PC2NUTS_RATE_LIMIT_STORAGE_URI is unset, the Limiter falls back to -slowapi's in-process MemoryStorage default — byte-for-byte the same as -the pre-#68 inline construction. - -When the URI is set (e.g. 'redis://host:6379/0'), the Limiter routes -counters through the configured backend, with in_memory_fallback_enabled -giving us per-process MemoryStorage during transient backend outages. -slowapi handles the fail-degraded behaviour internally with exponential- -backoff re-probing — see app/main.py:_rate_limit_handler for the 429 -response, and the spec at docs/superpowers/specs/2026-05-01-multi-worker-uvicorn-design.md -for the design rationale. -""" - -from slowapi import Limiter -from slowapi.util import get_remote_address - -from app.config import settings - -if settings.rate_limit_storage_uri: - limiter = Limiter( - key_func=get_remote_address, - storage_uri=settings.rate_limit_storage_uri, - in_memory_fallback_enabled=True, - ) -else: - limiter = Limiter(key_func=get_remote_address) -``` - -- [ ] **Step 4: Update `app/main.py` to import the limiter** - -In `app/main.py`, locate the existing slowapi imports and the inline Limiter construction: - -Current state (lines 16-18 and line 45): -```python -from slowapi import Limiter -from slowapi.errors import RateLimitExceeded -from slowapi.util import get_remote_address -... -limiter = Limiter(key_func=get_remote_address) -``` - -Change to: - -```python -from slowapi.errors import RateLimitExceeded -... -from app.limiter import limiter -``` - -Concretely: -- Remove `from slowapi import Limiter` (line 16). The `Limiter` symbol is no longer used in `main.py`. -- Remove `from slowapi.util import get_remote_address` (line 18). Same reason. -- Keep `from slowapi.errors import RateLimitExceeded` (line 17) — still used by `_rate_limit_handler`. -- Replace the standalone `limiter = Limiter(key_func=get_remote_address)` line (line 45) with: - ```python - from app.limiter import limiter - ``` - Place this with the other `from app...` imports in the import block (around line 22-23, alongside `from app.auth import ...` and `from app.config import settings`). - -After the changes, the slowapi-related imports in `app/main.py` should be just: - -```python -from slowapi.errors import RateLimitExceeded -``` - -And `limiter` should be sourced via: - -```python -from app.limiter import limiter -``` - -The decorators `@limiter.limit(...)` and the `app.state.limiter = limiter` line stay exactly as they are. - -- [ ] **Step 5: Run the new tests to verify they pass** - -```bash -pytest tests/test_limiter.py -v -``` - -Expected: both tests PASS. - -- [ ] **Step 6: Run the full test suite to verify no regressions** - -```bash -pytest -x -q -``` - -Expected: green. Particular attention to `tests/test_api.py::TestRateLimitTokens::test_valid_token_bypasses_rate_limit` (around test_api.py:226) — this exercises the `@limiter.limit(...)` decorator path and must still pass exactly as before. - -If any test fails, the most likely cause is a circular import (`app.limiter` imports `app.config.settings`, which is fine; `app.main` imports `app.limiter`, which is also fine; but `app.config` must NOT import `app.limiter`). Verify by reading the import block of `app/config.py` — it should only import from `pydantic`, `pydantic_settings`, and stdlib. - -- [ ] **Step 7: Commit** - -```bash -git add app/limiter.py app/main.py tests/test_limiter.py -git commit -m "feat(limiter): extract slowapi Limiter into dedicated module (#68) - -When PC2NUTS_RATE_LIMIT_STORAGE_URI is set, construct the Limiter with -that storage URI and in_memory_fallback_enabled=True so transient -backend outages fall back to per-process MemoryStorage. When unset, -construction is byte-for-byte the previous inline call. - -slowapi's built-in fallback handles outage detection, once-per-outage -WARNING logging, and exponential-backoff recovery probes." -``` - ---- - -## Task 4: Wire `PC2NUTS_WORKERS` into the Dockerfile - -**Files:** -- Modify: `Dockerfile` - -Single-line change to the `CMD`. The shell-form `CMD ["sh", "-c", "..."]` lets `${PC2NUTS_WORKERS:-1}` expand at container start using the env-var-with-default shell idiom. Default `:-1` preserves single-worker behaviour when the env var is unset. - -- [ ] **Step 1: Read the current Dockerfile to confirm the CMD line** - -```bash -grep -n "CMD" Dockerfile -``` - -Expected (current): `24:CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8000"]` - -- [ ] **Step 2: Replace the `CMD` line** - -Edit `Dockerfile`, line 24. Replace: - -``` -CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8000"] -``` - -with: - -``` -CMD ["sh", "-c", "exec uvicorn app.main:app --host 0.0.0.0 --port 8000 --workers ${PC2NUTS_WORKERS:-1}"] -``` - -The `exec` keeps `uvicorn` as the foreground process (replaces `sh` instead of leaving it as parent), so SIGTERM from the container runtime reaches uvicorn directly. Critical for graceful shutdown. - -- [ ] **Step 3: Sanity-check the Dockerfile builds** - -```bash -docker build -t pc2nuts:test-multi-worker . 2>&1 | tail -10 -``` - -Expected: `Successfully tagged pc2nuts:test-multi-worker` or equivalent (depending on Docker version). If the build fails for unrelated reasons (e.g. base image version drift), record the error and stop — do NOT modify other parts of the Dockerfile to make the build pass. - -If `docker` is not available in the agent environment, skip this step and rely on CI (the existing `docker` job in `.github/workflows/ci.yml` will exercise the build on PR). - -- [ ] **Step 4: Smoke-test the container with default (single-worker)** - -If Step 3 succeeded: - -```bash -docker run --rm -d --name pc2nuts-test -p 8000:8000 pc2nuts:test-multi-worker -sleep 8 -curl -fsS http://localhost:8000/health | head -c 300 -docker stop pc2nuts-test -``` - -Expected: `/health` returns 200 with the usual JSON body. The `--workers 1` is implicit (no env var → default). - -If the curl returns non-200 or hangs, capture logs: - -```bash -docker logs pc2nuts-test 2>&1 | tail -40 -``` - -The most likely failure modes are: -- `${PC2NUTS_WORKERS:-1}` not expanding (would be visible in logs as `--workers ${PC2NUTS_WORKERS:-1}` literal). If so, double-check that the `CMD` is shell-form (`sh -c`). -- Existing data-load failures unrelated to this change (TERCET download). Not our problem here. - -- [ ] **Step 5: Smoke-test the container with `PC2NUTS_WORKERS=2` and no storage URI (must fail)** - -```bash -docker run --rm -d --name pc2nuts-test-bad -e PC2NUTS_WORKERS=2 -p 8001:8000 pc2nuts:test-multi-worker -sleep 5 -docker logs pc2nuts-test-bad 2>&1 | tail -20 -docker stop pc2nuts-test-bad 2>/dev/null || true -docker rm pc2nuts-test-bad 2>/dev/null || true -``` - -Expected: container exits early; logs contain the validator's error message naming `PC2NUTS_WORKERS` and `PC2NUTS_RATE_LIMIT_STORAGE_URI`. This confirms the startup guard fires inside the container, not just in unit tests. - -If Docker is not available, skip — CI will exercise the build. - -- [ ] **Step 6: Commit** - -```bash -git add Dockerfile -git commit -m "feat(docker): support PC2NUTS_WORKERS env var (#68) - -Switches CMD from exec-form to shell-form with 'exec uvicorn …' so -\${PC2NUTS_WORKERS:-1} expands at container start while uvicorn remains -the foreground PID-1 process for proper SIGTERM handling. - -Default of 1 preserves current single-worker behaviour. Multi-worker -mode also requires PC2NUTS_RATE_LIMIT_STORAGE_URI; the Settings -validator (added in feat(config)) refuses to start otherwise." -``` - ---- - -## Task 5: Document the new env vars in README - -**Files:** -- Modify: `README.md` - -Add a "Multi-worker deployment" subsection near the existing rate-limit / configuration documentation. Reference the slowapi fail-degraded behaviour so operators know what to expect during a Redis outage. - -- [ ] **Step 1: Locate the right insertion point** - -```bash -grep -n "rate_limit\|rate limit\|RATE_LIMIT" README.md | head -20 -grep -n "## Configuration\|### Configuration\|## Environment" README.md | head -10 -``` - -Look for the existing rate-limit / env-var documentation block. The new content goes immediately after that block. - -- [ ] **Step 2: Read the surrounding context** - -Read the 30 lines around the rate-limit doc to match the existing style (table format, env-var naming convention, voice). Note whether the README uses backticks for env vars, what tone is used (terse vs prose), etc. - -- [ ] **Step 3: Insert the new section** - -Add the following block immediately after the existing rate-limit documentation, matching the README's existing formatting. If the README uses a table for env vars, add two rows; if it uses a definitions list, add two items. The content (adapt to the existing style): - -```markdown -### Multi-worker deployment - -By default the service runs a single uvicorn worker process. Throughput is -CPU-bound at ~30 RPS per worker (see [docs/performance.md](docs/performance.md)). -For higher RPS, set `PC2NUTS_WORKERS` to the number of worker processes you -want — the rough rule of thumb is one worker per CPU core, capped by the -available memory (~150-200 MB resident set per worker). - -| Env var | Default | Effect | -|---|---|---| -| `PC2NUTS_WORKERS` | `1` | Number of uvicorn worker processes. | -| `PC2NUTS_RATE_LIMIT_STORAGE_URI` | (unset) | When unset, slowapi uses per-process in-memory storage (default). When set (e.g. `redis://host:6379/0`), counters are shared across workers so the published `rate_limit` cap stays accurate. | - -When `PC2NUTS_WORKERS > 1`, `PC2NUTS_RATE_LIMIT_STORAGE_URI` MUST be set -to a reachable shared backend; the service refuses to start otherwise. -This guards against the per-IP rate limit silently loosening to -`PC2NUTS_WORKERS × rate_limit` per IP under multi-worker. - -**Degraded mode.** If the configured storage backend becomes unreachable -at runtime, slowapi (`in_memory_fallback_enabled=True`) falls back to -per-process in-memory rate limiting and re-probes the primary storage -with exponential backoff. During the outage window the effective per-IP -cap is `PC2NUTS_WORKERS × rate_limit`. Recovery is automatic; one -WARNING log line is emitted at the start of the outage and one INFO line -on recovery. -``` - -- [ ] **Step 4: Verify the README still renders sensibly** - -```bash -# Syntax-check by piping through a markdown parser if one is available -python3 -c " -with open('README.md') as f: - content = f.read() -# Crude check: equal counts of backticks (no unmatched code spans) -import re -inline = len(re.findall(r'(?&1 | tail -10 -``` - -Expected: no new findings on `app/limiter.py`. If bandit complains about something legitimately security-sensitive, fix it; if it's a false positive on the slowapi/redis URI handling, document the suppression with a comment naming the rule. - -- [ ] **Step 5: Eyeball the diff against `main`** - -```bash -git diff main...HEAD --stat -git diff main...HEAD docs/superpowers/specs/ docs/superpowers/plans/ -``` - -Expected: the file list matches the file map at the top of this plan. The spec and plan diffs should be readable and consistent. The implementation files (`app/config.py`, `app/limiter.py`, `app/main.py`, `Dockerfile`, `requirements.lock`, `app/settings.json`, two test files, README, CHANGELOG) should all be present. - -- [ ] **Step 6: Push the branch and open a PR** - -```bash -git push -u origin feat/multi-worker-uvicorn -``` - -Then open the PR with `gh pr create`, mapping each acceptance criterion from issue #68 to the relevant component. Body template: - -```markdown -## Summary - -Implements [#68](https://github.com/bk86a/PostalCode2NUTS/issues/68): multi-worker uvicorn behind a shared rate-limit backend. - -- New env vars: `PC2NUTS_WORKERS` (default `1`), `PC2NUTS_RATE_LIMIT_STORAGE_URI` (default unset). -- Startup hard-fails if `WORKERS > 1` without a storage URI configured. -- slowapi `in_memory_fallback_enabled=True` handles transient backend outages with per-process MemoryStorage fallback and exponential-backoff re-probing. -- Default behaviour (single worker, in-memory) is byte-for-byte unchanged. - -Spec: `docs/superpowers/specs/2026-05-01-multi-worker-uvicorn-design.md` -Plan: `docs/superpowers/plans/2026-05-01-multi-worker-uvicorn.md` - -## Acceptance criteria from #68 - -- [x] `Dockerfile` updated to launch N workers via `PC2NUTS_WORKERS` -- [x] Rate-limit behaviour with N workers documented in `README.md` -- [ ] Memory usage measured against the container's allocated memory — operational follow-up post-deploy -- [ ] Performance baseline re-run; `docs/performance.md` updated — operational follow-up post-deploy - -## Test plan - -- [ ] Existing rate-limit tests still pass (single-worker default branch is byte-for-byte unchanged) -- [ ] New `tests/test_config.py::TestWorkersValidator` proves the validator fires -- [ ] New `tests/test_limiter.py::TestLimiterStorageSelection` proves the storage URI is honoured -- [ ] Manual: deploy with `PC2NUTS_WORKERS=2` + a real Redis, confirm the cap holds across workers (operational) -- [ ] Manual: kill Redis with traffic flowing, confirm WARNING logs once and INFO logs on recovery (operational) -``` - ---- - -## Self-review - -Before marking the plan complete, the implementer (or dispatching agent) should run through this checklist: - -- **Spec coverage.** Every section of the spec maps to a task: - - §3 Configuration surface → Task 1 - - §4.1 Settings additions → Task 1 - - §4.2 `app/limiter.py` → Task 3 - - §4.3 slowapi fail-degraded behaviour → Task 3 (the `in_memory_fallback_enabled=True` flag) - - §4.4 Dockerfile → Task 4 - - §4.5 requirements.lock → Task 2 - - §4.6 `app/main.py` → Task 3 - - §6 Error handling — covered by slowapi internals; test scope per §7 - - §7 Testing → Tasks 1 and 3 each include their own tests - - §8 Documentation → Tasks 5 and 6 - - §9 Acceptance-criteria mapping → Task 7 PR body -- **Placeholder scan.** Every "implement" step contains the actual code. The only `` placeholders are in Task 2, where the version genuinely depends on the live `pip` resolution; the steps tell the implementer how to derive the concrete value. -- **Type/name consistency.** `Settings.workers` (int, default 1), `Settings.rate_limit_storage_uri` (str|None, default None), `app.limiter.limiter` (slowapi.Limiter), `PC2NUTS_WORKERS` and `PC2NUTS_RATE_LIMIT_STORAGE_URI` env-var names — all match across tasks. diff --git a/docs/superpowers/specs/2026-04-28-uk-itl-support-design.md b/docs/superpowers/specs/2026-04-28-uk-itl-support-design.md deleted file mode 100644 index 3610a38..0000000 --- a/docs/superpowers/specs/2026-04-28-uk-itl-support-design.md +++ /dev/null @@ -1,221 +0,0 @@ -# UK postcode and ITL support — design - -**Status:** approved (brainstorming complete) -**Date:** 2026-04-28 -**Issue:** [#7](https://github.com/bk86a/PostalCode2NUTS/issues/7) -**Scope:** add support for UK postcodes via the ONS NSPL dataset, mapping to ITL (International Territorial Level) codes, exposed through the existing `/lookup` API with a new `code_system` discriminator. - ---- - -## 1. Goals and non-goals - -### Goals - -- Accept UK postcodes (e.g. `SW1A 2AA`) on `/lookup` and return ITL1/2/3 codes (e.g. `TLI`, `TLI3`, `TLI32`). -- Accept input under either `country=UK` or `country=GB`. -- Handle both full postcodes and outward-code-only input (`SW1A`). -- Make the NUTS-vs-ITL distinction explicit in API responses via a new `code_system` field. -- Reuse the existing five-tier lookup waterfall, the same in-memory dict, the same SQLite cache, and the same configuration model. -- Keep failure of the UK data path independent from TERCET (and vice versa) so neither blocks service startup. - -### Non-goals - -- Crown Dependencies and Gibraltar (JE, GG, IM, GI) — out of scope; rejected with the standard 400. -- ONSPD as an alternative source — NSPL is preferred for ONS-recommended best-fit allocation. -- Backwards-compatibility shim translating ITL codes to NUTS-2016 UK equivalents. -- Per-source TTL configuration (only added if conditional GET against ONS turns out unworkable). -- Implementation work — this document is the design only. - ---- - -## 2. Background - -### NSPL data source - -**National Statistics Postcode Lookup** from the ONS Open Geography Portal: - -- ~1.79 million live UK postcodes, each mapped to ITL3. -- Quarterly releases (February, May, August, November). -- ~178 MB compressed ZIP, no authentication, Open Government Licence v3.0. -- CSV columns of interest: `pcds` (formatted postcode), `itl` (ITL3 code), `doterm` (termination date — blank for live codes). -- ITL region names are not in NSPL — they come from three separate ONS "Names and Codes" CSVs (~232 rows total). -- NSPL chosen over ONSPD because best-fit allocation via Census Output Areas is the ONS-recommended approach for statistical purposes. - -### ITL vs NUTS divergence - -The Overview of issue #7 states "same boundaries, `UK` prefix changed to `TL`". This is no longer accurate: - -| Level | NUTS 2016 (UK) | ITL 2021 | Delta | -|-------|---------------:|---------:|------:| -| L1 | 12 | 12 | 0 | -| L2 | 40 | 41 | +1 | -| L3 | 174 | 179 | +5 | - -ONS published bidirectional NUTS↔ITL lookup files until 2023, after which they were discontinued. A 2025 ONS revision is also in preparation. Consumers comparing UK results against historical NUTS-UK data therefore cannot assume drop-in equivalence — hence the `code_system` discriminator. - -### Existing architecture this design plugs into - -- TERCET ZIPs downloaded per-country at startup, parsed by `_parse_csv_content`, stored in `_lookup[(country, normalised_postcode)] = nuts3_code`. -- SQLite cache scoped by NUTS version (e.g. `postalcode2nuts_NUTS-2024.db`); TTL-checked, atomically written. -- Five-tier lookup waterfall: exact → curated estimate → runtime prefix approximation → country-level majority vote → single-NUTS3 country fallback. -- `nuts_names` table loaded from a separate GISCO CSV. -- Country alias precedent: `GR → EL` already implemented for Greece. -- Eircode precedent for variable-length lookups: `tercet_map: truncate:3` slices the routing key. - ---- - -## 3. Architecture - -UK is treated as a parallel data channel rather than a 35th GISCO country. It uses the same in-memory `_lookup` dict and the same waterfall, but is loaded by a separate code path: - -``` -startup -├── TERCET loader (existing) -│ ├── discover countries from GISCO directory listing -│ ├── per-country ZIP download → _parse_csv_content → _lookup -│ └── failure → fall back to stale cache, set data_stale -└── NSPL loader (new) - ├── fetch NSPL ZIP from configured URL (conditional GET) - ├── _parse_csv_content with NSPL column aliases (pcds, itl) + doterm filter - ├── populate _lookup[("UK", normalized_pc)] = ITL3 - ├── build outward-code index: _outward_lookup[("UK", outward)] = majority ITL3 - ├── load ITL names from ONS Names-and-Codes CSVs into _nuts_names - └── failure → fall back to stale cache, do not block service -``` - -**Invariants:** - -- Single unified lookup dict. `lookup()` does not branch on country except for the `GB → UK` alias and the new outward-code tier. -- NSPL failure must not block TERCET data from serving (and vice versa). Both already use the stale-cache fallback; NSPL hooks into the same mechanism. -- NSPL cache lives in the same SQLite DB as TERCET, tagged with its own source identifier so a TERCET-only deployment (NSPL URL unset) still works. -- The `code_system` of each lookup row is recorded at load time, not derived at query time. -- **`UK` is NOT added to `settings.countries`.** That list drives GISCO discovery and Strategy-2 URL guessing in the TERCET loader (`data_loader._guess_zip_urls_for_country`). Adding UK there would cause every cold start to attempt non-existent `pc{YYYY}_UK_NUTS-*.zip` URLs against GISCO, wasting startup latency and timeout budget. UK loading is gated on `settings.nspl_url` instead. - ---- - -## 4. Data acquisition - -| Concern | Decision | -|---------|----------| -| **NSPL URL source** | Configured via `nspl_url` in `app/settings.json`, overridable via `PC2NUTS_NSPL_URL`. Operator updates quarterly. If unset, NSPL loader is a no-op (TERCET-only deployment). | -| **Conditional GET** | Extend the existing TERCET downloader to send `If-Modified-Since` / `If-None-Match` based on cached `Last-Modified` / `ETag`. On `304 Not Modified`, skip re-parse. Applied to both TERCET and NSPL — free win for both. | -| **`doterm` filter** | `_parse_csv_content` gains a `skip_terminated: bool = False` flag. When true, rows where the `DOTERM` column is non-blank are skipped. NSPL loader sets it to true. | -| **Column aliases** | Extend the existing alias lists in `_parse_csv_content`: postal code adds `"PCDS"`; NUTS3 candidates add `"ITL"`, `"ITL3"`, `"ITL3CD"`. No separate parser path. | -| **ITL names** | Three ONS "Names and Codes" CSVs (one per ITL level, ~232 rows total). URLs supplied via `PC2NUTS_ITL_NAMES_URLS` (comma-separated) — exposed as a plain string field on `Settings` plus a `itl_names_url_list` property that splits it, mirroring the existing `extra_sources` / `extra_source_urls` pattern. Loaded into the existing `nuts_names` table. The current `_NUTS3_RE` pattern (`^[A-Z]{2}[A-Z0-9]{1,3}$`) already accepts `TLxNN`. | -| **TTL** | Shared `PC2NUTS_DB_CACHE_TTL_DAYS`. Conditional GET keeps the wasted-refresh cost near zero. Per-source TTL deferred unless conditional GET is unsupported by ONS. | - ---- - -## 5. Lookup waterfall — new tier 3.5 - -Insert between Tier 3 (runtime prefix approximation) and Tier 4 (country-level majority vote): - -> **Tier 3.5: Outward-code lookup (`match_type: "estimated"`)** -> -> If the input postcode is shorter than a full UK postcode (no inward portion) **or** Tier 3 prefix approximation found nothing for UK, look up `(country, outward_code)` in `_outward_lookup`. Returns the majority-vote ITL3 for that outward code with `medium`-tier confidence (NUTS1 0.90, NUTS2 0.80, NUTS3 0.70). - -The outward-code index is built once at NSPL load time: - -1. For each NSPL row, slice off the last 3 chars of the normalised postcode → outward. -2. Group by `(UK, outward)` → list of ITL3 codes → majority vote → store winner + agreement ratio. - -Implementation: - -- New `tercet_map` action `outward_only` on the UK pattern, evaluated in `extract_postal_code` to produce both the full normalised form (for Tier 1) and the outward form (for Tier 3.5). -- The tier is parameterised by country; UK enables it. Easy to extend to IE later if desired. - ---- - -## 6. Response schema - -Single additive field on `LookupResponse`: - -```python -code_system: Literal["NUTS", "ITL"] -``` - -- All 34 GISCO countries → `"NUTS"`. -- `UK` (or `GB` via alias) → `"ITL"`. - -`nuts1/2/3` field names stay; for UK they hold TL-prefixed values. No breaking changes for existing consumers (additive). README's example response gains the new field. - ---- - -## 7. Configuration changes - -| File | Change | -|------|--------| -| `app/settings.json` | Add `nspl_url: ""` (default empty). **Do NOT add `"UK"` to `countries`** — that list is GISCO-only; adding UK there would trigger wasted GISCO URL guesses on every cold load. | -| `app/postal_patterns.json` | Add UK entry with the issue's proposed regex + new `tercet_map: "outward_only"`. Bump `_meta.version` from `1.0` → `1.1`. | -| `app/config.py` | New string fields `nspl_url: str = ""` and `itl_names_urls: str = ""` (env vars `PC2NUTS_NSPL_URL`, `PC2NUTS_ITL_NAMES_URLS`). New property `itl_names_url_list` that splits the comma-separated string — mirrors the existing `extra_sources` / `extra_source_urls` pattern. No `Field(alias=...)` indirection. | -| `app/data_loader.py` | Column alias extension, `doterm` filter flag, NSPL loader, outward-code index builder, ITL names loader, conditional-GET in HTTP layer, `code_system` attribution per source. | -| `app/main.py` | `GB → UK` alias in `/lookup`; new field in response model. | -| `app/models.py` | Add `code_system` field with description. | - -UK regex (from issue body, unchanged): - -```json -"UK": { - "regex": "^([A-Z]{1,2}[0-9][0-9A-Z]?\\s?[0-9][A-Z]{2})$", - "example": "SW1A 2AA, EC1A 1BB, M1 1AA, B33 8TH", - "tercet_map": "outward_only" -} -``` - ---- - -## 8. Documentation updates - -In `README.md`: - -- **Coverage**: split into "EU-27 / EFTA / Candidates / United Kingdom (ITL via NSPL)". The UK subsection makes the data-source distinction explicit. -- **Supported patterns table**: add UK row. -- **New ITL/NUTS divergence note** (1 paragraph): explain that ITL diverges from NUTS 2016 UK at L2 and L3 (41 vs 40 ITL2 regions; 179 vs 174 ITL3 regions); that ONS discontinued NUTS↔ITL lookups in 2023; that the `code_system` field on `/lookup` responses identifies which scheme applies. -- **Endpoints**: add a UK example response showing `code_system: "ITL"` and a `TLxxx` value. -- **Five-tier lookup → six-tier**: document new Tier 3.5 (outward-code). -- **Configuration table**: add `PC2NUTS_NSPL_URL`, `PC2NUTS_ITL_NAMES_URLS`. -- **Data source section**: add ONS/NSPL attribution under OGL v3.0 alongside the existing GISCO CC-BY-SA 4.0 line. -- **Adding a new country**: note that NSPL-style sources require a separate loader path, not as simple as adding to GISCO patterns. - ---- - -## 9. Operational impact - -- **Memory**: roughly +150-200 MB in-process (per issue body's estimate). Confirm during implementation. -- **SQLite cache**: roughly ×3 disk size (current ~830K codes → ~2.6M codes). Note in deployment docs; existing volume sizing handles this comfortably. -- **First-load startup**: +10-30s estimated to parse 1.79M extra rows on cold start. Warm restart unchanged thanks to SQLite cache. -- **Refresh bandwidth**: collapsed to ~304s after first full fetch via conditional GET (assuming ONS supports it; if not, fall back to per-source TTL). -- **Revalidation pass**: measure at implementation time; if >5s wall-clock, scope revalidation per-country rather than re-architect. - ---- - -## 10. Out of scope (explicit) - -- **Crown Dependencies and Gibraltar** (JE, GG, IM, GI) — rejected with standard 400; documented in README as out of scope. If ever needed, just extend the country list. -- **ONSPD** — NSPL chosen instead for ONS-recommended best-fit allocation; documented in data-source section. -- **NUTS↔ITL translation shim** — not implemented. Consumers branch on `code_system` instead. -- **ITL 2025 revision** — monitor; consume automatically once NSPL adopts it (no code change expected if columns stay stable). -- **Per-source TTL config** — not added unless conditional GET against ONS turns out unsupported. - ---- - -## 11. Decisions log (from brainstorming session) - -| # | Decision | Choice | Rationale | -|---|----------|--------|-----------| -| Q1 | ITL parity framing | Honest + discriminator | ITL has empirically diverged from NUTS-2016 UK at L2/L3; mislabelling would surprise consumers. | -| Q2 | Outward-code fallback | Add as Tier 3.5 in v1 | High real-world UK input value; IE precedent makes implementation pattern clear. | -| Q3 | Crown Deps and Gibraltar | Reject with standard 400 | Not in NSPL; not in ITL geography; explicit failure preferred over silent miscategorisation. | -| Q4 | `code_system` discriminator | Yes (additive field) | Closes Q1 cleanly; no breaking change. | -| Q5 | Refresh cadence | Shared TTL + conditional GET | Avoids new config surface; benefits TERCET too. | -| Q6 | Licence attribution | README only | `/health` doesn't currently expose licence info; no need to add a new surface. | -| Q7 | `patterns_version` bump | `1.0 → 1.1` | Additive change (new country, no existing pattern altered). | -| Q8 | Country auto-discovery | Separate code path for NSPL | GISCO directory listing won't include UK; failure must not block TERCET startup. | -| Q9 | Estimates revalidation cost | Measure during implementation; per-country scoping if >5s | Low likelihood of regression; defer optimisation until measured. | - -### Post-spec corrections (Codex review on PR #52, 2026-04-29) - -| # | Concern | Fix | -|---|---------|-----| -| C1 | `Field(alias="ITL_NAMES_URLS")` interacts unreliably with `pydantic-settings` `env_prefix` — the operator-facing `PC2NUTS_ITL_NAMES_URLS` env var would not consistently populate the field. | Use the existing `extra_sources` precedent: a plain `str` field plus a separately-named property that parses the comma-separated value. No alias; env var name follows from the field name + prefix. | -| C2 | Adding `"UK"` to `settings.countries` makes the GISCO loader's Strategy-2 URL-guessing iterate over UK on every cold load, attempting non-existent `pc{YYYY}_UK_NUTS-*.zip` URLs. | UK is loaded by the dedicated NSPL path; do not add it to `settings.countries`. The new architecture invariant in §3 makes this explicit. | diff --git a/docs/superpowers/specs/2026-04-29-auth-token-bypass-design.md b/docs/superpowers/specs/2026-04-29-auth-token-bypass-design.md deleted file mode 100644 index 368acdd..0000000 --- a/docs/superpowers/specs/2026-04-29-auth-token-bypass-design.md +++ /dev/null @@ -1,287 +0,0 @@ -# Auth-token bypass — design - -**Status:** approved (brainstorming complete) -**Date:** 2026-04-29 -**Issue:** [#60](https://github.com/bk86a/PostalCode2NUTS/issues/60) -**Scope:** allow trusted callers to bypass the per-IP `60/minute` rate limit by presenting an opaque token via `Authorization: Bearer`. Operator manages a small static set of tokens via an environment variable; container restart applies changes. - ---- - -## 1. Goals and non-goals - -### Goals - -- Bypass the per-IP `60/minute` cap for callers presenting a recognised token. -- Keep the per-IP cap as the default for unauthenticated traffic. -- Issue tokens manually to a small set of known callers (operator-driven, no self-service). -- Revoke a specific token by removing it from the env var and restarting the container. -- Log audit lines for trusted requests with a non-reversible token id; never log raw tokens. - -### Non-goals - -- Per-token quotas. Trusted tokens are *fully* exempt in v1. -- OAuth, JWT, signatures, asymmetric crypto. -- Self-service signup, rotation automation, expiry, scopes, multi-tenant pricing. -- Encryption at rest — the env var is the trust boundary; operator owns secret hygiene. -- Authenticated `/health` — `/health` stays anonymous (useful for monitoring). -- Per-token Prometheus metrics — possible later, not v1. - ---- - -## 2. Operational runbook (consult this when issuing or revoking a token) - -This section is the operator's reference. It is duplicated near-verbatim in `README.md` so the running system can be administered without reading source. - -### Issue a new token - -```bash -# 1. Generate locally (48 hex chars / 192 bits) -openssl rand -hex 24 - -# 2. Add the printed token to PC2NUTS_TRUSTED_TOKENS in the production deployment's -# environment configuration. Multiple tokens are comma-separated; whitespace and -# empty entries are tolerated. -# -# Example value with two active tokens: -# PC2NUTS_TRUSTED_TOKENS=9e7a3f...d2,4b1c8e...77 - -# 3. Restart the service container to load the new env value. -# The SQLite postal-code cache survives the restart, so cold-start is ~30 s. - -# 4. Verify the new token bypasses the rate limit: -curl -i -H "Authorization: Bearer " \ - "https:///lookup?country=DE&postal_code=10115" -# → 200, no X-RateLimit-* headers consumed; audit log shows token_id= - -# 5. Hand the raw token to the consumer over a confidential channel -# (1Password, Signal, encrypted email — not Slack, not GitHub issues). -``` - -### Revoke a token - -```bash -# 1. Remove the token entry from PC2NUTS_TRUSTED_TOKENS in the env config. -# 2. Restart the container. -# 3. Verify the revoked token is rejected: -curl -i -H "Authorization: Bearer " \ - "https:///lookup?country=DE&postal_code=10115" -# → 401 Unauthorized -``` - -### Find the token id of a logged request - -```bash -# Audit log lines contain token_id=<8-hex>. To match a token to its id locally: -echo -n "" | sha256sum | cut -c1-8 -``` - -### Disable the bypass entirely - -Unset or empty `PC2NUTS_TRUSTED_TOKENS`. All traffic falls back to the per-IP cap. No code change needed. - ---- - -## 3. Configuration - -| Env var | Default | Description | -|---|---|---| -| `PC2NUTS_TRUSTED_TOKENS` | `""` (unset) | Comma-separated list of valid bypass tokens. Empty / unset → bypass disabled; behaviour identical to today. Whitespace and empty entries between commas are stripped. | - -`app/config.py` exposes a `trusted_tokens` property that parses the env var into a deduplicated `frozenset[str]` at startup. The set is read once at startup; runtime mutation is not supported (matches Approach A from the brainstorming session). - -`app/settings.json` does **not** carry token values (avoids accidental commit). It optionally documents the env var via a comment-style `_meta` entry; not strictly required. - ---- - -## 4. Runtime behaviour - -| Request | Result | HTTP status | -|---|---|---| -| No `Authorization` header | Existing per-IP `60/minute` cap | `200` or `429` | -| `Authorization: Bearer ` | **Rate limit fully bypassed.** Audit log emitted. | `200` (or any natural endpoint code) | -| `Authorization: Bearer ` | Reject — wrong tokens fail loudly | `401 Unauthorized` | -| `Authorization: ` | Reject — caller intent unclear | `400 Bad Request` | -| Malformed header (e.g. `Bearer` with no value, missing space) | Reject | `400 Bad Request` | - -`/health` is unaffected (no `@limiter.limit`, anonymous). - ---- - -## 5. Implementation - -### 5.1 New module: `app/auth.py` - -Single-purpose module, < 80 lines. Exposes: - -```python -def extract_bearer(request: Request) -> str | None: - """Return the bearer token from the Authorization header, or None if absent. - Raises HTTPException(400) on malformed Authorization headers - (e.g. wrong scheme, empty value).""" - -def is_trusted(token: str) -> bool: - """Constant-time membership test against settings.trusted_tokens - using hmac.compare_digest in a loop. Returns False for any falsy input.""" - -def token_id(token: str) -> str: - """First 8 chars of sha256(token).hexdigest(). Used in audit logs.""" - -def is_trusted_request(request: Request) -> bool: - """Composed predicate: extract_bearer → is_trusted. Used as slowapi exempt_when. - Re-raises HTTPException(401) when a token is present but unknown — this surfaces - as the 401 in the response, not as an exempt-or-not boolean.""" -``` - -### 5.2 slowapi integration - -`@limiter.limit(...)` accepts an `exempt_when` callable. `app/main.py` imports `is_trusted_request` and passes it on the two existing decorators: - -```python -@limiter.limit(settings.rate_limit, exempt_when=lambda: is_trusted_request(request_var.get())) -def lookup_postal_code(request: Request, ...): ... - -@limiter.limit(settings.rate_limit, exempt_when=lambda: is_trusted_request(request_var.get())) -def get_pattern(request: Request, ...): ... -``` - -`exempt_when` does not receive the `Request`, so we propagate it via a `contextvars.ContextVar` set by a tiny middleware that runs before the limiter. - -If `exempt_when=`'s lack of a request parameter turns out to make this fragile, the fallback is to inline the bypass check in the route handler and call `@limiter.exempt` conditionally — slightly clunkier, no functional change. - -### 5.3 Audit logging - -Existing `AccessLogMiddleware` (in `app/main.py`) gains an extra trailing field per line: - -``` -2026-04-29 09:15:10,420 127.0.0.1 GET /lookup 200 1.6ms token_id=9e7a3f12 -``` - -The token id is computed once per request and attached to the log record. Anonymous requests get no `token_id=` field (omitted, not empty). Token values themselves never enter the logger. - -### 5.4 401 vs 400 plumbing - -`extract_bearer` raises `HTTPException(400, "malformed Authorization header")` when the header is present but unparseable. `is_trusted_request` raises `HTTPException(401, "invalid token")` when a syntactically valid token is unknown. Both propagate through FastAPI's normal exception handler. - -This means the error path is *not* "exempt or not" — it short-circuits with the right status before slowapi runs. - -### 5.5 Constant-time comparison - -The membership test uses `hmac.compare_digest` against each known token, not `in` on a set. Reason: the token list is tiny (≤ ~10 entries) and timing-safe equality avoids leaking whether a candidate matched the first byte of any registered token. - -```python -def is_trusted(candidate: str) -> bool: - if not candidate: - return False - return any(hmac.compare_digest(candidate, t) for t in settings.trusted_tokens) -``` - -### 5.6 Files affected - -| File | Change | -|---|---| -| `app/auth.py` | **new** — extract / validate / token-id helpers | -| `app/config.py` | new `trusted_tokens` property parsing `PC2NUTS_TRUSTED_TOKENS` | -| `app/main.py` | import auth helpers; pass `exempt_when` on `@limiter.limit`; small middleware to propagate `Request` via contextvar; extend access log line | -| `tests/test_auth.py` | **new** — unit tests for `app/auth.py` | -| `tests/test_api.py` | new endpoint tests for the four behaviour rows in §4 | -| `tests/conftest.py` | optional fixture parametrising `settings.trusted_tokens` | -| `README.md` | new "Authentication & rate-limit bypass" section, including the operational runbook from §2 | -| `CHANGELOG.md` | new entry | -| `app/settings.json` | no functional change (token values stay in the env var, not in JSON) | - ---- - -## 6. Security considerations - -- **Tokens are bearer credentials.** Anyone holding the string can use the API at full rate. Treat them like passwords. -- **Transport.** The deployment runs over HTTPS; the rate-limit-bypass token is therefore protected in transit. Never accept a bearer token over plain HTTP — relying on the deployment's TLS termination. -- **At rest.** Tokens live in the deployment's env config (admin-only access). Not encrypted at rest beyond what the env-var store provides. -- **Logs.** Only the 8-char SHA-256 prefix appears in logs. Even a full log dump cannot recover the original token (preimage of an 8-hex prefix has ~2^32 candidates × the unknown remainder of the SHA — not exploitable). -- **Timing.** `hmac.compare_digest` used for all comparisons. -- **Revocation latency.** Bound by container restart time (~30 s). For an emergency leak: remove all tokens, restart, then re-issue good ones. The `/lookup` endpoint stays available throughout (anonymous traffic continues; only bypass capability is interrupted). -- **Audit retention.** The existing rotating access log applies; no new retention policy added. - ---- - -## 7. Tests (TDD, written before implementation) - -### Unit (`tests/test_auth.py`) - -1. `extract_bearer` — header absent → `None`. -2. `extract_bearer` — `Bearer ` → token. -3. `extract_bearer` — `bearer ` (lowercase scheme) → token (case-insensitive scheme per RFC 7235). -4. `extract_bearer` — `Basic ` → raises `HTTPException(400)`. -5. `extract_bearer` — `Bearer ` (no value) → raises `HTTPException(400)`. -6. `extract_bearer` — `Bearer multiple words` → raises `HTTPException(400)`. -7. `is_trusted` — match against a single configured token → `True`. -8. `is_trusted` — match against one of several configured tokens → `True`. -9. `is_trusted` — unknown token → `False`. -10. `is_trusted` — empty string → `False`. -11. `is_trusted` — empty configured set → `False` for any input. -12. `token_id` — deterministic, 8 hex chars, stable across calls. -13. `token_id` — different tokens → different ids. - -### Endpoint (`tests/test_api.py`) - -14. No `Authorization` header → existing rate-limit behaviour preserved (the existing test set still passes). -15. `Authorization: Bearer ` → `>60` requests in a minute all return `200`. -16. `Authorization: Bearer ` → `401`. -17. `Authorization: Basic ` → `400`. -18. `Authorization: Bearer ` (no value) → `400`. -19. Empty `PC2NUTS_TRUSTED_TOKENS` → `Authorization: Bearer anything` is ignored; rate limit still applies (header has no privileged effect when bypass is disabled). -20. Audit log line contains `token_id=<8-hex>` for trusted requests, no `token_id=` field for anonymous. -21. `/health` is unaffected by the header in any of its forms. - -### Notes - -- Test (15) requires either lowering the rate limit for the test (e.g. via env var) or making >60 requests in the test — both are practical; prefer the env-var override to keep tests fast. -- Test (20) reads from the configured access log handler; the existing test infra captures via `caplog`. - ---- - -## 8. Documentation updates - -### `README.md` - -A new top-level section **Authentication & rate-limit bypass** placed after **Configuration** and before **Five-tier lookup**. Contents: - -1. Why the feature exists (one paragraph). -2. The full **operator runbook** from §2 of this spec — verbatim, so the README is the canonical reference for issuance. -3. The four behaviour rows from §4 as a small table. -4. Pointers to the env var in the existing Configuration table. - -### `CHANGELOG.md` - -`[Unreleased]` entry under `### Added`: - -> - **Auth-token bypass** (#60): trusted callers can bypass the per-IP rate limit by presenting `Authorization: Bearer `. Tokens are managed via the new `PC2NUTS_TRUSTED_TOKENS` comma-separated env var. Invalid tokens return `401`; malformed headers return `400`. Audit lines log a non-reversible token id only. - -### `docs/postal_code_format_analysis.md` - -No changes (unrelated to the feature). - ---- - -## 9. Out of scope (explicit) - -- Per-token quotas, scopes, expiry, or rotation hooks. (v2 candidates if usage patterns change.) -- A `/admin/tokens` HTTP endpoint for runtime management. (Approach C from brainstorming — rejected for v1.) -- File-mounted tokens with live reload. (Approach B — rejected for v1; `~30 s` restart is acceptable at the expected cadence of "a few times ever".) -- HMAC-signed tokens, JWT, asymmetric signing. -- Token usage metrics / Prometheus counters per token id. -- IP allow-listing as a separate orthogonal feature. -- Authenticating `/health`. - ---- - -## 10. Decisions log (from brainstorming session) - -| # | Decision | Choice | Rationale | -|---|---|---|---| -| Q1 | Operational style | Approach A — env var + restart | Cadence is "a few times ever"; simplest viable design pays off. | -| Q2 | Token format | Plain opaque hex (`openssl rand -hex 24`) | No embedded info needed at this scale; prefixed format buys little when only ~handful of tokens exist. | -| Q3 | Header scheme | `Authorization: Bearer` | Standard, idiomatic, well-supported by clients. (Resolved during issue split.) | -| Q4 | Rate-limit treatment | Full exemption | Trusted tokens are batch/research callers; quota would re-introduce friction. (Resolved during issue split.) | -| Q5 | Wrong token behaviour | `401`, not silent fallthrough | Sending a token is deliberate; failing loudly is friendlier than appearing to be rate-limited. | -| Q6 | Audit | SHA-256 prefix only | Reversibility risk vs operator's need to identify tokens — prefix balances both. | -| Q7 | `/health` auth | Anonymous | Monitoring tools shouldn't carry secrets. | diff --git a/docs/superpowers/specs/2026-04-29-db-backed-trusted-tokens-design.md b/docs/superpowers/specs/2026-04-29-db-backed-trusted-tokens-design.md deleted file mode 100644 index ee23956..0000000 --- a/docs/superpowers/specs/2026-04-29-db-backed-trusted-tokens-design.md +++ /dev/null @@ -1,358 +0,0 @@ -# DB-backed trusted-tokens — design - -**Status:** approved (brainstorming complete) -**Date:** 2026-04-29 -**Issue:** [#61](https://github.com/bk86a/PostalCode2NUTS/issues/61) -**Predecessor:** [v0.16.0 auth-token bypass](2026-04-29-auth-token-bypass-design.md) — env-var-based v1 of this feature. -**Scope:** move trusted-token storage from `PC2NUTS_TRUSTED_TOKENS` (env var, v1) to a managed SQLite-compatible HTTP database (v2), with the env var preserved as a disaster-recovery fallback. Adds an operator CLI (`python -m scripts.tokens`) for issuance, listing, and revocation. - ---- - -## 1. Goals and non-goals - -### Goals - -- Eliminate the v1 brittleness around env-var-driven token rotation (programmatic env-var changes through the hosting provider's container API have destructive side effects on adjacent configuration). -- Token rotation does not require a container restart. -- Operator workflow is a single CLI command per action: `python -m scripts.tokens add --label "..."`, `list`, `revoke `. -- Auth on-request hot path stays in-memory and constant-time — `is_trusted` lookup remains a frozenset membership check; DB reads happen out-of-band on a periodic refresh. -- Soft migration from v1: env var continues to work as a union with DB-loaded tokens. No forced cutover. -- DB outage is non-fatal — the service keeps serving anonymous traffic; bypass simply degrades. - -### Non-goals - -- Per-token quotas, scopes, or expiry. (v3 candidates if usage demands.) -- An HTTP admin endpoint (`POST /admin/tokens`). Rejected in the v1 brainstorming as well. -- `last_used_at` tracking. Either a per-request write hotspot or async batching — defer. -- Automated migration / deprecation of `PC2NUTS_TRUSTED_TOKENS`. The env var stays a supported channel until a future issue removes it. -- Moving the primary lookup data (the ~830K-row TERCET cache) into the same database. Different concern — can be its own issue if ever needed. - ---- - -## 2. Operational runbook (consult when issuing or revoking) - -This section is the operator's reference. It is duplicated near-verbatim into `README.md` so the running system can be administered without reading source. - -### Initial setup (one-time, per environment) - -```bash -# On your laptop, with PC2NUTS_TOKEN_DB_URL set in the environment: -export PC2NUTS_TOKEN_DB_URL='' -python -m scripts.tokens init -# → creates the trusted_tokens table and index. Idempotent. -``` - -### Issue a token - -```bash -python -m scripts.tokens add --label "alice-batch-2026-04" -# Generated: e3a1f2...d4 -# Inserted id=3, label='alice-batch-2026-04', token_id=9a29f07a - -# Hand the printed token to the consumer over a confidential channel -# (1Password, Signal, encrypted email — not Slack, not GitHub issues). -``` - -The new token becomes active in the running service within `PC2NUTS_TOKEN_REFRESH_SECONDS` (default 60 s) — **no container restart needed**. - -### Migrate an existing v1 env-var token (preserves the audit token_id) - -```bash -python -m scripts.tokens add --label "perf-test-2026-04-29" --value "" -# → inserts the existing value, preserving its sha256 prefix used in audit logs. -``` - -### List active and revoked tokens - -```bash -python -m scripts.tokens list -# id label created_at status -# 1 perf-test-2026-04-29 2026-04-29T12:50:00 active -# 2 alice-batch-2026-04 2026-04-29T15:20:00 active -# 3 bob-research 2026-04-15T09:10:00 revoked@2026-04-28T14:00:00 -``` - -The `list` output never includes the raw `value` column — only id, label, created_at, revoked status. - -### Revoke a token - -```bash -python -m scripts.tokens revoke 3 -# Token id=3 revoked at 2026-04-29T16:00:00. -``` - -The revocation takes effect within `PC2NUTS_TOKEN_REFRESH_SECONDS`. Anyone using that token starts receiving 401. - -### Find the token id of a logged request - -Same as v1 — the access-log `token_id=<8hex>` field is `sha256(token)[:8]`: - -```bash -echo -n "" | sha256sum | cut -c1-8 -``` - -The CLI's `add` command also prints the `token_id` at issuance time, so the audit prefix is in operator hands from the start. - -### Disable the DB-backed feature entirely - -Unset `PC2NUTS_TOKEN_DB_URL`. Behaviour reverts to v1 (env-var only). - -### Disaster recovery — DB unreachable - -If the database is unreachable, the service continues serving traffic; only the bypass feature degrades. Two scenarios: - -- **You have tokens in `PC2NUTS_TRUSTED_TOKENS`** (treated as a DR fallback): bypass continues using the env-var tokens until the DB recovers. -- **You don't**: bypass is off until the DB recovers. Anonymous traffic is unaffected. - -To cover yourself: keep one or two emergency tokens in `PC2NUTS_TRUSTED_TOKENS` even after migrating most issuance to the DB. Document them as DR-only. - ---- - -## 3. Data model - -### Schema - -```sql -CREATE TABLE trusted_tokens ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - value TEXT NOT NULL UNIQUE, - label TEXT, - created_at TEXT NOT NULL DEFAULT (datetime('now')), - revoked_at TEXT -); -CREATE INDEX idx_trusted_tokens_active - ON trusted_tokens (value) - WHERE revoked_at IS NULL; -``` - -| Column | Notes | -|---|---| -| `id` | Stable identifier; surfaced in `list` and `revoke` UX. | -| `value` | The opaque 48-hex bearer token. **Never logged**. | -| `label` | Operator-supplied free-text. Used by `list` and audit. May be NULL but the CLI requires it on `add`. | -| `created_at` / `revoked_at` | ISO-8601 strings via SQLite `datetime('now')`. `revoked_at IS NULL` ⇒ active. | - -Active set query (used by the runtime refresh and by `list --active`): - -```sql -SELECT value, id, label, created_at FROM trusted_tokens WHERE revoked_at IS NULL; -``` - ---- - -## 4. Configuration - -| Env var | Default | Read by | Purpose | -|---|---|---|---| -| `PC2NUTS_TOKEN_DB_URL` | `""` (unset) | service + CLI | Connection string for the SQLite-compatible managed database. Empty → DB feature disabled, behaviour reverts to v1. | -| `PC2NUTS_TOKEN_REFRESH_SECONDS` | `60` | service | How often the background task reloads the active set into memory. | -| `PC2NUTS_TRUSTED_TOKENS` | `""` (unset) | service | Existing v1 env var. Continues to work as a union with DB tokens; serves as DR fallback when DB is unreachable. No deprecation in this release. | - -The CLI reads `PC2NUTS_TOKEN_DB_URL` from the operator's environment, or accepts `--db-url ` as an override. - ---- - -## 5. Architecture - -### 5.1 Provider-agnostic database client (`app/token_db.py` — new) - -A small (~100 LOC) HTTP client for a SQLite-compatible managed database. Exposes: - -```python -class TokenDB: - def __init__(self, url: str): ... - def execute(self, sql: str, params: list[Any] | None = None) -> list[dict]: - """Send a SQL statement. Returns a list of row dicts (empty for writes).""" - def init_schema(self) -> None: - """Idempotent — run the CREATE TABLE / CREATE INDEX statements.""" - def list_active(self) -> list[dict]: - """SELECT id, value, label, created_at FROM trusted_tokens WHERE revoked_at IS NULL.""" - def add(self, value: str, label: str) -> int: - """INSERT INTO trusted_tokens(value, label) VALUES (?, ?). Returns new id.""" - def list_all(self) -> list[dict]: - """SELECT id, label, created_at, revoked_at FROM trusted_tokens (no value).""" - def revoke(self, token_id: int) -> bool: - """UPDATE … SET revoked_at = datetime('now') WHERE id = ? AND revoked_at IS NULL.""" -``` - -The module is named `token_db.py`, not after the provider — it speaks a generic SQLite-over-HTTP contract and could be repointed at a different backend with a connection-string change. - -Connection-string parsing and authentication are concentrated in this module. `httpx` (already a project dependency for TERCET downloads) is used for the HTTP client. - -### 5.2 Auth layer changes (`app/auth.py`) - -Two changes, both small: - -1. **`_db_tokens` cache** — a module-level `frozenset[str]` populated by a background refresh task. Initially empty. - -2. **`_get_trusted_tokens()` returns the union**: - - ```python - def _get_trusted_tokens() -> frozenset[str]: - return _db_tokens | settings.trusted_tokens - ``` - - The existing test seam is preserved. Tests that monkey-patch this function (Tasks 4–7 of the v1 plan) keep working unchanged. - -3. **Refresh task** — started from `lifespan` in `app/main.py` if `PC2NUTS_TOKEN_DB_URL` is set: - - ```python - async def _refresh_tokens_loop(): - while True: - try: - rows = await asyncio.to_thread(token_db.list_active) - _db_tokens = frozenset(r["value"] for r in rows) - _token_db_stale = False - except Exception as exc: - logger.warning("token DB refresh failed: %s", exc) - _token_db_stale = True - # Keep previous _db_tokens unchanged. - await asyncio.sleep(settings.token_refresh_seconds) - ``` - - The first iteration runs at startup. If it fails, `_db_tokens` stays empty (the default) and the env-var union covers any active tokens. - -### 5.3 Health endpoint extension - -`/health` gains one optional field: - -```python -"token_db_stale": True | False # only present when PC2NUTS_TOKEN_DB_URL is configured -``` - -`True` means the most recent refresh attempt failed (the in-memory set is stale relative to the DB but still in effect). Monitoring tools can alert on this. - -### 5.4 CLI (`scripts/tokens.py` — new) - -Standard `argparse` with subcommands. Imports `TokenDB` from `app/token_db.py`. - -``` -python -m scripts.tokens init # idempotent schema bootstrap -python -m scripts.tokens add --label LABEL [--value VALUE] -python -m scripts.tokens list [--all] # default: active only -python -m scripts.tokens revoke ID -``` - -Argument details: - -- `add` generates a 48-hex token via `secrets.token_hex(24)` unless `--value` is given (used for v1→v2 migration to preserve `token_id` continuity). Prints the raw token, the new id, and the audit `token_id`. Exits non-zero on UNIQUE-violation. -- `list` defaults to active tokens; `--all` includes revoked. Output is a fixed-column table (id, label, created_at, status). The raw `value` column is **never** printed. -- `revoke` is idempotent. If the id is already revoked, prints "already revoked" and exits 0. -- All subcommands accept `--db-url URL` to override `PC2NUTS_TOKEN_DB_URL`. - -### 5.5 Files affected - -| File | Status | Responsibility | -|---|---|---| -| `app/token_db.py` | **new** (~100 LOC) | Provider-agnostic SQLite-over-HTTP client. Init/list/add/revoke methods. | -| `app/auth.py` | modify | `_db_tokens` cache, refresh task entry point, union in `_get_trusted_tokens`. | -| `app/main.py` | modify | Spawn `_refresh_tokens_loop` from `lifespan` when DB URL is set; expose `token_db_stale` on `/health`. | -| `app/config.py` | modify | New settings: `token_db_url`, `token_refresh_seconds`. | -| `scripts/tokens.py` | **new** (~150 LOC) | Operator CLI. | -| `tests/test_token_db.py` | **new** | Unit tests for the HTTP client (mock httpx). | -| `tests/test_tokens_cli.py` | **new** | Unit tests for the CLI subcommands. | -| `tests/test_auth.py` | modify | Tests for the union behaviour, refresh task, DB-down at startup. | -| `tests/test_api.py` | modify | `/health` integration test for `token_db_stale` field. | -| `README.md` | modify | Replace v1 runbook with the v2 runbook from §2; keep DR fallback note. | -| `CHANGELOG.md` | modify | New `### Added` entry under `[Unreleased]`. | -| `app/settings.json` | unchanged | (Connection strings live in env vars, not committed JSON.) | - ---- - -## 6. Security considerations - -- **Token storage at rest** is the database provider's responsibility. The connection string is a credential and lives only in `PC2NUTS_TOKEN_DB_URL` (operator's env / production env config). It is never committed. -- **Token values in `list` output** are deliberately omitted. Once issued, the operator is expected to store the raw token in their own secret manager. The DB is the registry, not the recovery channel. -- **CLI logging** never emits the raw token to stdout/stderr beyond the single `add` line that hands it to the operator. No DEBUG mode prints values. -- **Constant-time comparison** stays unchanged — `hmac.compare_digest` against the union frozenset, same as v1. -- **DB compromise** — if the DB is breached, all live tokens are exposed. Mitigation: rotate by `revoke` + reissue. Same risk model as the env var, but with audit trail. -- **Revocation latency** is `PC2NUTS_TOKEN_REFRESH_SECONDS` worst case (default 60 s). For an emergency, the operator can additionally restart the container to force an immediate refresh. - ---- - -## 7. Tests - -### Unit (`tests/test_token_db.py`) - -1. `init_schema` issues the expected `CREATE TABLE` and `CREATE INDEX`; idempotent on re-run. -2. `add(value, label)` issues parameterised INSERT; returns the new id. -3. `add` with duplicate `value` raises a UNIQUE-violation error. -4. `list_active` filters out revoked rows. -5. `list_all` returns both active and revoked, never the `value` column. -6. `revoke(id)` sets `revoked_at`, idempotent on already-revoked ids. -7. HTTP error (5xx, network) raises a typed exception (`TokenDBError`) — caller can catch. - -### Unit (`tests/test_tokens_cli.py`) - -8. `add` with `--label` only: generates a 48-hex token, prints id + token + token_id. -9. `add` with `--value` accepts the supplied value, preserves the audit prefix. -10. `add` exits non-zero on duplicate value. -11. `list` (default) shows only active rows, no `value` column. -12. `list --all` includes revoked rows. -13. `revoke ` prints success. -14. `revoke ` prints "already revoked", exits 0. -15. Missing `PC2NUTS_TOKEN_DB_URL` and no `--db-url` → exits with a helpful error. - -### Integration (extends `tests/test_auth.py`) - -16. `_get_trusted_tokens` returns DB tokens ∪ env-var tokens. -17. Refresh task pulls the active set on tick. -18. Refresh failure does not clear the previous in-memory set. -19. Revocation in the DB takes effect within `PC2NUTS_TOKEN_REFRESH_SECONDS` (test-config: 1 second). -20. DB unreachable at startup → service still starts, anonymous traffic works, env-var tokens still work. - -### Endpoint (extends `tests/test_api.py`) - -21. `/health` includes `token_db_stale: false` when DB URL configured and refresh succeeds. -22. `/health` includes `token_db_stale: true` after a forced refresh failure. -23. `/health` omits `token_db_stale` when DB URL is unset. - -### Regression - -24. All 122 existing v0.16.0 tests stay green. - ---- - -## 8. Documentation updates - -### `README.md` - -Replace the existing **Authentication & rate-limit bypass → Operator runbook** subsections (v1, env-var) with the v2 runbook from §2 of this spec. Keep: - -- The behaviour summary table (unchanged). -- The disable-the-bypass-entirely note (now: unset `PC2NUTS_TOKEN_DB_URL` AND `PC2NUTS_TRUSTED_TOKENS`). -- Security notes (extend with the DR-fallback recommendation). - -Add a small section explaining the union semantic: "DB-loaded tokens and `PC2NUTS_TRUSTED_TOKENS` env-var tokens both work; either can be empty; the env var serves as DR when the DB is unreachable." - -### `CHANGELOG.md` - -`[Unreleased]` → `### Added`: - -> - **DB-backed trusted tokens** (#61): trusted-token storage moved from `PC2NUTS_TRUSTED_TOKENS` env var to a managed SQLite-compatible HTTP database. Configure with `PC2NUTS_TOKEN_DB_URL`. Tokens issued via `python -m scripts.tokens add --label "..."` take effect within ~60 s (configurable via `PC2NUTS_TOKEN_REFRESH_SECONDS`) — no container restart required. The env var continues to work as a union with the DB and serves as a disaster-recovery fallback when the DB is unreachable. New `/health` field `token_db_stale` flags refresh failures. - ---- - -## 9. Out of scope (explicit) - -- Per-token quotas, scopes, expiry. -- HTTP admin endpoint for token CRUD (operator CLI only). -- `last_used_at` write-back tracking. -- Forced deprecation / removal of `PC2NUTS_TRUSTED_TOKENS` (keep as supported channel). -- Migrating the primary TERCET lookup cache to the same database. -- Multi-region replication of the token DB beyond what the provider offers by default. -- Changing the on-the-wire HTTP API of `/lookup` or `/pattern`. - ---- - -## 10. Decisions log (from brainstorming) - -| # | Decision | Choice | Rationale | -|---|----------|--------|-----------| -| Q1 | Database backend | Managed SQLite-compatible HTTP DB (option A) | Same provider as the rest of the deployment; SQLite-compatible mental model; negligible cost. | -| Q2 | Operator workflow | Bundled CLI (option B): `python -m scripts.tokens` | Single command per action; tests possible; ergonomic `list`/`revoke`. | -| Q3 | DB-down behaviour | Graceful degrade (option B), with env-var union as automatic DR | Reuses the existing "empty token set = bypass off" code path; env var becomes opt-in DR. | -| Q4 | Runtime refresh failure | Keep previous in-memory set; expose `token_db_stale` on `/health` | Standard cached-resource discipline. | -| Q5 | Refresh interval | 60 s default, configurable | Operator-friendly without log-spam. | -| Q6 | `last_used_at` | Skip in v2 | Either write hotspot or async batching — defer. | -| Q7 | Migration from v1 | Soft, no forced cutover | The union semantic makes "do nothing" a valid migration starting point. | diff --git a/docs/superpowers/specs/2026-05-01-estimates-periodic-refresh-design.md b/docs/superpowers/specs/2026-05-01-estimates-periodic-refresh-design.md deleted file mode 100644 index dfb1c65..0000000 --- a/docs/superpowers/specs/2026-05-01-estimates-periodic-refresh-design.md +++ /dev/null @@ -1,188 +0,0 @@ -# Periodic refresh of `tercet_missing_codes.csv` — design - -**Issue:** [#44](https://github.com/bk86a/PostalCode2NUTS/issues/44) -**Date:** 2026-05-01 -**Status:** Approved (for plan + implementation) - -## Problem - -`tercet_missing_codes.csv` is the source of pre-computed NUTS estimates for postal codes that GISCO TERCET doesn't cover. Today the running service only loads it once at startup (or when a full `load_data()` rebuild fires). When a new estimate row is merged into the CSV on `main`, deployed pods don't pick it up until they're redeployed (which redeploys the whole image and forces a cold-start). Operators currently have no in-band way to push a CSV change to production short of bumping the image. - -## Goals - -- A running pod can pick up CSV changes from upstream `main` without a redeploy. -- A new pod that comes up immediately reflects the latest upstream CSV, not whatever was baked into the image at build time. -- An operator can confirm "the row I just merged is live" without waiting up to a full refresh interval. -- A bad upstream fetch (empty, half-truncated, parse error) cannot wipe out the in-memory estimates table. - -## Non-goals - -- Periodic refresh of GISCO TERCET (the 30-day SQLite cache TTL is the lever for that; out of scope here). -- Persisting refreshed estimates back to the SQLite cache or to disk. The remote CSV is the runtime source of truth when configured; the SQLite `estimates` table remains a snapshot from the most recent full `load_data()` and serves as a fallback. -- Cross-pod coordination of refresh timing. Each pod refreshes independently. With autoScaling.max=1 today this is moot. - -## Configuration - -Two new env vars, both opt-in. Defaults preserve current behaviour byte-for-byte. - -| Env var | Default | Effect | -|---|---|---| -| `PC2NUTS_ESTIMATES_REFRESH_URL` | *(unset)* | When set, the worker periodically fetches this URL and replaces `_estimates` with the parsed contents. When unset, no remote fetch happens — current single-source-of-truth behaviour from the bundled `tercet_missing_codes.csv`. Recommended value: `https://raw.githubusercontent.com/bk86a/PostalCode2NUTS/main/tercet_missing_codes.csv`. | -| `PC2NUTS_ESTIMATES_REFRESH_INTERVAL_SECONDS` | `86400` (24 h) | Refresh cadence. Set to `0` to disable the periodic task while still running the bootstrap fetch on startup (rare; mostly useful for debugging). | - -Validation: if `PC2NUTS_ESTIMATES_REFRESH_URL` is set, the value must parse as a URL. Anything else is left to the HTTP client to reject at fetch time. - -## Approach (full replace, with sanity guard) - -On each refresh tick (and once on worker startup, before lifespan-ready): - -1. **Fetch.** `GET PC2NUTS_ESTIMATES_REFRESH_URL` with a 10 s timeout. Send `If-None-Match` and `If-Modified-Since` headers from the previous successful fetch when available. On 304 Not Modified, skip the parse + swap entirely — log nothing. -2. **Hash.** SHA-256 of the response body. Compare to the hash of the previous successful fetch (kept in module-scoped state). If unchanged, skip — log at DEBUG only. -3. **Parse.** Feed the body bytes through the existing CSV-loading logic (refactored to accept a stream, see Components below) into a temporary `dict` (not the live `_estimates`). Skipped rows (unknown confidence label, malformed) are counted, not fatal. -4. **Sanity guard.** If `len(new_dict) < 0.5 * len(_estimates)` *and* the live state is non-empty, refuse the swap. Log a WARNING with both counts, the new content's first parsing error if any, and the URL. Mark the most-recent refresh as failed. Re-probe next interval. -5. **Swap.** Acquire `_data_lock`, replace `_estimates` with `new_dict`, run `_revalidate_estimates()` (drops entries that already have an exact TERCET match), release lock. Log INFO with the new count and the diff vs the previous count. -6. **Update freshness flag.** Module-scoped `_estimates_refresh_stale = False` after a successful swap (or 304); `True` after any error path; `None` if the feature is disabled (URL unset). - -The lock window in step 5 covers a swap of ~7 020 entries plus a revalidate pass over them. Sub-millisecond on local tests; bounded by the size of `_estimates`, which is small relative to `_lookup` (830 K). - -## Components - -A new module **`app/estimates_refresh.py`**. Self-contained because (a) it has its own state — last-fetch hash, last-fetch ETag/Last-Modified, the staleness flag — and (b) it shouldn't bloat `data_loader.py`, which is already large. - -``` -app/estimates_refresh.py -├── _last_etag, _last_modified, _last_hash, _stale # module state -├── fetch_remote_csv(client) -> bytes | None # GET with conditional headers -├── parse_csv_bytes(body) -> dict # parse into temp dict -├── refresh_estimates_once() -> RefreshResult # one full tick -├── refresh_estimates_loop() # asyncio loop -└── get_refresh_stale() -> bool | None # for /health -``` - -Refactoring out of `data_loader.py`: - -- The existing `_load_estimates_from_csv(path: Path)` reads from a file path. Extract a helper `_parse_estimates_rows(reader: csv.DictReader)` that operates on an already-opened reader; both the old loader and the new `parse_csv_bytes` reuse it. Avoids duplicate parsing logic. - -Wiring: - -- **`app/main.py` lifespan**: when `settings.estimates_refresh_url` is set, do one bootstrap call to `refresh_estimates_once()` after `load_data()` completes (so revalidation has the lookup table to compare against). If the bootstrap fetch fails, log a WARNING but continue startup — the bundled CSV's content is already loaded, so the worker is still serviceable. -- **`app/main.py` lifespan**: schedule `asyncio.create_task(refresh_estimates_loop())` on startup; cancel on shutdown. -- **`app/main.py` /health handler**: new `estimates_refresh_stale` field on `HealthResponse`, populated from `get_refresh_stale()`. -- **`app/main.py`**: new `POST /admin/refresh-estimates` route (see below). - -## Manual refresh endpoint - -`POST /admin/refresh-estimates` — synchronous, returns when the refresh completes. - -- **Auth**: requires `Authorization: Bearer ` against the existing trusted-token registry (the same mechanism that bypasses the per-IP rate limit). No token, or unknown token, → 401. -- **Body**: empty. -- **Success response (200)**: - ```json - { - "status": "refreshed", - "previous_count": 7020, - "new_count": 7042, - "skipped_rows": 0, - "source_url": "https://raw.githubusercontent.com/bk86a/PostalCode2NUTS/main/tercet_missing_codes.csv" - } - ``` - When upstream is unchanged (304/identical hash), `status: "unchanged"` and `previous_count == new_count`. -- **Failure response (502)** when the fetch or parse fails: `{"status": "failed", "reason": "..."}`. Live state is unchanged. -- **Sanity-guard rejection (409)** when the new CSV is < 50 % of current row count: `{"status": "rejected", "reason": "...", "previous_count": 7020, "candidate_count": 12}`. Live state is unchanged. -- **Disabled-feature response (503)** when `PC2NUTS_ESTIMATES_REFRESH_URL` is unset: `{"status": "disabled"}`. - -The endpoint is **included in the OpenAPI schema** (no `include_in_schema=False`) so it's discoverable in `/docs`. It is rate-limited like the rest of the API (anonymous unauthenticated callers hit 401 long before they consume rate budget). - -## Failure handling - -| Failure | Behaviour | -|---|---| -| Network error / DNS / timeout | `_estimates_refresh_stale = True`, log WARNING (once per outage, debounced like slowapi's), retry next interval. Live state unchanged. | -| HTTP 4xx (404/403/etc.) | Same — stale flag set, log WARNING with status code. Specifically: 404 means the URL is wrong; we want this loud. | -| HTTP 5xx | Same — stale flag set, log WARNING with status. | -| Parse error / decoding error | Same — stale flag set, log WARNING with first traceback frame. | -| Sanity guard rejection | Stale flag set, log WARNING with both counts. | -| 304 Not Modified | Stale flag stays `False`. Log at DEBUG only — the ordinary case. | - -The "log once per outage" debounce is achieved by tracking the previous success/failure state and only emitting a WARNING on the *transition* from success → failure (and an INFO on failure → success). Successive failures are silent at INFO/WARNING; DEBUG-level lines stay on every tick. - -## /health changes - -`HealthResponse` (in `app/models.py`) gains: - -```python -estimates_refresh_stale: bool | None = None -``` - -Semantics: -- `None`: feature disabled (URL unset). -- `False`: most recent refresh attempt succeeded (or 304). -- `True`: most recent refresh attempt failed. - -Existing `total_estimates` field is unchanged — it's still the in-memory count regardless of where those rows came from. - -## Multi-worker considerations - -Each worker runs its own `refresh_estimates_loop()`. With current production at `PC2NUTS_WORKERS=2`, this means: - -- 2× the GitHub fetches per cadence (and per manual refresh, if a load balancer routes the operator's POST to one worker — the *other* worker is still on its old state until its next interval). -- Brief per-IP-worker divergence after an upstream change: one worker swaps in the new state seconds before the other. A client request might see the new estimate from one worker and the old from the other in the seconds-long window between worker swaps. Acceptable: estimates are by definition approximate, and the divergence converges within one tick. - -This matches the existing TokenDB refresh pattern (`app/auth.py`) byte-for-byte. No new infra dependency. If we ever scale to many pods/many workers and 60/h GitHub raw rate limits become a concern, a Redis-backed leader (`SETNX` with TTL = interval) is a drop-in retrofit — but YAGNI today. - -## Testing - -Per the project's existing test patterns (`tests/test_*.py`, mock-at-the-boundary, no network in unit tests): - -1. `tests/test_estimates_refresh.py` — new file: - - `parse_csv_bytes` correctly parses well-formed CSV. - - `parse_csv_bytes` skips unknown-confidence rows, returns the rest. - - Sanity guard rejects a parsed dict whose count is < 50 % of current. - - Sanity guard accepts a parsed dict whose count is ≥ 50 % of current. - - Sanity guard accepts any count when current `_estimates` is empty (bootstrap path). - - `refresh_estimates_once` with a mocked `httpx.AsyncClient`: - - 200 OK with new content → swap, hash updated, stale=False. - - 304 Not Modified → no swap, hash unchanged, stale=False. - - 200 OK with identical content (matched by hash) → no swap, stale=False. - - HTTP 5xx → no swap, stale=True. - - Network error → no swap, stale=True. - - Parse error → no swap, stale=True. - - Sanity guard rejection → no swap, stale=True. -2. `tests/test_api.py` — add a `TestAdminRefreshEstimatesEndpoint` class: - - 401 with no `Authorization` header. - - 401 with an invalid bearer. - - 200 with a valid bearer, mocked refresh returns new count. - - 503 when the URL setting is unset. - - 502 when the refresh raises. - - 409 when the sanity guard rejects. -3. `tests/test_api.py` `TestHealthEndpoint`: - - `estimates_refresh_stale` is `None` when URL unset. - - `estimates_refresh_stale` is `False` after a successful (mocked) refresh. - -## Operator runbook (`README.md` addition) - -Brief subsection under existing "Configuration" / "Authentication & rate-limit bypass" sections: - -```bash -# Force-refresh the estimates table on a deployed pod (after merging an update -# to tercet_missing_codes.csv on main): -curl -X POST -H "Authorization: Bearer $TOKEN" \ - https://api.example.invalid/admin/refresh-estimates -``` - -Plus the env-var table extended with the two new variables. - -## Migration / backward compatibility - -- Defaults (URL unset) preserve the current single-source behaviour. Existing deployments don't pick up the feature until the operator sets `PC2NUTS_ESTIMATES_REFRESH_URL`. -- The bundled `tercet_missing_codes.csv` continues to be loaded at startup as the bootstrap state. If the URL is set and the bootstrap fetch succeeds before the worker is ready, the bundled CSV's content is overwritten in-memory before any client request lands. If the bootstrap fetch fails, the worker comes up serving the bundled state and retries on the next tick. -- The `HealthResponse` schema gains a new optional field; existing JSON consumers that ignore unknown fields (the contract) are unaffected. -- No changes to the Dockerfile, no changes to the `compose.yaml` defaults — feature stays opt-in. - -## Out of scope / future work - -- Persisting refreshed estimates back to the SQLite `estimates` table on the volume. Not needed: warm-restart workers will refresh from upstream within their bootstrap fetch window. -- Cross-pod / cross-worker leader election. Not needed at current scale. -- Watching the GitHub repo for push events (webhooks) instead of polling. More moving parts; daily polling is more than fresh enough. -- Diff-based partial updates (apply only changed rows). The full-replace path is simpler and the dataset is small; YAGNI. -- A `/admin/refresh-tercet` or `/admin/reload-data` endpoint to force the full GISCO re-download. Out of scope for this issue. diff --git a/docs/superpowers/specs/2026-05-01-multi-worker-uvicorn-design.md b/docs/superpowers/specs/2026-05-01-multi-worker-uvicorn-design.md deleted file mode 100644 index 74c3ceb..0000000 --- a/docs/superpowers/specs/2026-05-01-multi-worker-uvicorn-design.md +++ /dev/null @@ -1,252 +0,0 @@ -# Multi-worker uvicorn with Redis-backed rate limiting — design - -**Status:** approved (brainstorming complete) -**Date:** 2026-05-01 -**Issue:** [#68](https://github.com/bk86a/PostalCode2NUTS/issues/68) -**Scope:** allow the service to run with N uvicorn workers behind a shared rate-limit backend, preserving the strict per-IP cap that single-worker deployments rely on. Single-host PyPI/Docker deployers must remain unaffected. - ---- - -## 1. Goals and non-goals - -### Goals - -- Allow `uvicorn` to run with `N > 1` workers via a single env var (`PC2NUTS_WORKERS`). -- Keep the per-IP rate-limit cap strict at `settings.rate_limit` (currently `120/minute`) across all workers, by routing slowapi storage through a shared backend (Redis) when configured. -- Keep the trusted-token bypass (`exempt_when=is_trusted_request`) working unchanged. -- Preserve the current single-worker, in-memory deploy path byte-for-byte when the new env vars are unset. -- Tolerate transient unavailability of the shared backend without taking the API down. -- Hard-fail at startup if the operator asks for `N > 1` without configuring shared storage, so the cap can never silently loosen. - -### Non-goals - -- Provisioning the Redis instance itself — operational concern, documented in `README.md` as a prerequisite for the multi-worker mode. -- Re-running the performance baseline and updating `docs/performance.md` — separate post-deploy task once the multi-worker container is in production. -- Edge-layer rate limiting (bunny.net Shield + Edge Rules) — investigated during brainstorming, not on the table without a documented header-conditional bypass mechanism. Left as a future option. -- Switching from `uvicorn --workers` to `gunicorn` — not needed for this workload (no streaming, no long-poll, requests <100 ms), and adds a process-manager dependency. -- Recycling worker processes after N requests, graceful timeouts on individual requests, or other lifecycle features. -- Implementation work — this document is the design only. - ---- - -## 2. Background - -### Why this is the next perf win - -The performance baseline at commit `526f289` (`docs/performance.md`) measured sustained throughput at ~30 RPS on a single uvicorn worker. The bottleneck is per-request server-side CPU work (~30 ms in `/lookup` logic + Pydantic serialisation; the actual dict lookup is statistically free). Each additional worker should add roughly +30 RPS of headroom, up to the container CPU count. This is the single largest available perf win that requires no application-logic change. - -### Why slowapi needs a shared backend - -slowapi's default storage is in-memory (`MemoryStorage` from the `limits` library), per-process. With `N` workers, each worker holds its own counters, so the per-IP cap effectively becomes `N × settings.rate_limit`. The project's published cap is `120/minute` per anonymous IP, with trusted-token holders exempt — that contract must hold under multi-worker. - -### Three rejected alternatives (recorded for the next time this is revisited) - -- **Accept the loosening (option (c) in brainstorming).** Document that the published cap is per-process; aggregate is N× higher. Rejected: violates the published contract. -- **Edge-layer rate limiting via bunny.net (option (b)).** bunny.net Shield offers global per-IP rate limiting and Edge Rules support header-based logic, but combining the two — i.e. "rate-limit by IP, except when an Authorization header is present" — is not a documented configuration, and trusted-token holders would be throttled at the edge. Rejected without a support-ticket confirmation; left as a future option if bunny.net adds the capability. -- **Divide the cap by N at the worker level (`120/N` per worker).** Rejected: with HTTP keepalive (default for almost all clients), all of a client's rapid requests pin to the same worker, so they would hit `120/N` and be throttled even though aggregate budget allows 120. False-positive rate limits. - ---- - -## 3. Configuration surface - -Two new env vars, both with defaults preserving current behavior. - -| Env var | Type | Default | Effect | -|---|---|---|---| -| `PC2NUTS_WORKERS` | `int ≥ 1` | `1` | Number of uvicorn worker processes. | -| `PC2NUTS_RATE_LIMIT_STORAGE_URI` | `str \| None` | `None` (unset) | When unset, slowapi uses in-memory storage (current behavior). When set (e.g. `redis://host:6379/0`), slowapi routes through the fail-degraded shared backend wrapper. | - -Both surface in `app/config.py` (`Settings` model, `settings.json` defaults, env-var override per existing pattern). Naming follows the existing `PC2NUTS_*` convention. - -### Startup validation - -If `PC2NUTS_WORKERS > 1` and `PC2NUTS_RATE_LIMIT_STORAGE_URI` is unset/empty, the app refuses to start with a clear error message naming both env vars. Implemented as a Pydantic model validator on `Settings`. `Settings()` is instantiated at module-import time in `app/config.py`, which runs once in the uvicorn supervisor process before the workers are forked, so the failure is pre-fork and pre-bind: the supervisor exits non-zero with the validator's message and no workers are started. (Each worker also re-imports the module after fork, but the supervisor has already failed by then in the misconfigured case, so worker-side behavior is moot.) - ---- - -## 4. Components - -### 4.1 `app/config.py` — settings additions - -Two new fields, default-preserving, plus a model validator. Defaults can also be set in `app/settings.json` for self-hosters who want to bake worker counts into the image. - -```python -class Settings(BaseSettings): - # ... existing fields ... - workers: int = _defaults.get("workers", 1) - rate_limit_storage_uri: str | None = _defaults.get("rate_limit_storage_uri", None) - - @model_validator(mode="after") - def _check_workers_have_shared_storage(self) -> "Settings": - if self.workers > 1 and not self.rate_limit_storage_uri: - raise ValueError( - "PC2NUTS_WORKERS > 1 requires PC2NUTS_RATE_LIMIT_STORAGE_URI to be set " - "(e.g. 'redis://host:6379/0'). Without shared storage the per-IP rate limit " - "would silently loosen by a factor of WORKERS." - ) - return self -``` - -The exact field name and Pydantic version semantics are existing-codebase concerns to confirm during implementation; the design intent is "model-level validator that fails fast on the unsafe combination." - -### 4.2 `app/limiter.py` — new module - -Single responsibility: own the slowapi `Limiter` instance and the storage-mode choice. Today this is inline in `app/main.py`; pulling it out keeps `main.py` slimmer and isolates the (small) configuration logic. - -Public surface: - -```python -# app/limiter.py -from slowapi import Limiter -from slowapi.util import get_remote_address -from app.config import settings - -if settings.rate_limit_storage_uri: - limiter = Limiter( - key_func=get_remote_address, - storage_uri=settings.rate_limit_storage_uri, - in_memory_fallback_enabled=True, - ) -else: - limiter = Limiter(key_func=get_remote_address) -``` - -That's the entire module. Two branches; default branch is byte-for-byte identical to the current `app/main.py:45` line. - -`app/main.py` changes from `limiter = Limiter(...)` to `from app.limiter import limiter`. The `app.state.limiter = limiter` and `@limiter.limit(...)` decorators stay exactly as they are. - -### 4.3 Fail-degraded behavior — slowapi built-in - -slowapi 0.1.9 already implements the fail-degraded design we want via the `in_memory_fallback_enabled` parameter. Behavior, taken directly from `slowapi/extension.py`: - -- **Trigger:** when any storage call raises, slowapi sets an internal `_storage_dead = True` flag, logs once at WARNING (`"Rate limit storage unreachable - falling back to in-memory storage"`), and routes subsequent rate-limit checks to a separate per-process `MemoryStorage`-backed `MovingWindowRateLimiter`. -- **Recovery:** uses exponential backoff. The `__should_check_backend()` helper waits `2^N` seconds between consecutive re-probes (where `N` is the consecutive check count, capped at `MAX_BACKEND_CHECKS`). When a probe succeeds (`self._storage.check()`), `_storage_dead` is cleared and slowapi logs at INFO (`"Rate limit storage recovered"`). -- **Logging policy:** the `if not self._storage_dead` guard around the WARNING means it fires once per outage transition, exactly matching our spec intent. -- **Concurrency:** each worker has its own `_storage_dead` flag and its own fallback `MemoryStorage`. Per-worker outage tracking is correct (a worker that can't reach Redis is independently degraded), and the fallback is per-worker, so during an outage the effective cap loosens to `N × settings.rate_limit` per IP — same as our designed §3 behavior. - -This means we write **zero custom storage code**. The plan only needs to wire `in_memory_fallback_enabled=True` into the `Limiter` construction and verify the wiring at the unit-test level. - -The exponential-backoff recovery cadence (rather than a fixed 30 s window) is strictly better for the operator: gentler on a recovering Redis under load, faster recovery for short blips. - -### 4.4 `Dockerfile` - -Single change: switch the `CMD` from exec form to shell form so `${PC2NUTS_WORKERS}` expands at container start. - -```dockerfile -CMD ["sh", "-c", "uvicorn app.main:app --host 0.0.0.0 --port 8000 --workers ${PC2NUTS_WORKERS:-1}"] -``` - -The default `:-1` keeps single-worker behavior when the env var is unset. The shell-form CMD does mean the container's PID 1 is `sh`, which is acceptable for this service (no signal-handling-sensitive workloads); uvicorn forwards SIGTERM correctly when invoked this way. If signal handling becomes a concern later, switching to `exec uvicorn …` inside the shell command preserves the env expansion while making uvicorn PID 1. - -### 4.5 `requirements.lock` - -Add a `redis` runtime dependency. Easiest path is `slowapi[redis]` or `limits[redis]` (whichever extra is exposed by the version slowapi pins); fallback is a direct `redis` pin matching the major-version range that `limits` expects (concrete numbers chosen at implementation time — see §10). - -The dependency is loaded eagerly at import time by `app/limiter.py` only when `rate_limit_storage_uri` is set; single-host deployers who never set the env var still pay the install-size cost but no runtime cost. Acceptable trade-off. - -### 4.6 `app/main.py` - -- Remove the inline `limiter = Limiter(key_func=get_remote_address)` line. -- Add `from app.limiter import limiter` near the existing slowapi imports. -- Everything else (`app.state.limiter = limiter`, the `RateLimitExceeded` handler, the `@limiter.limit(...)` decorators on `/lookup` and `/lookup/{country}/{postal}`) stays exactly as it is. - ---- - -## 5. Data flow - -### Single-worker deploy (default, unchanged) - -``` -Request → uvicorn (1 worker) → FastAPI → @limiter.limit → MemoryStorage (in-process dict) → handler -``` - -Identical to today. Trusted-token requests skip the limiter via `exempt_when`. - -### Multi-worker deploy (new) - -``` -Request → uvicorn (N workers, OS distributes) → FastAPI → @limiter.limit - → slowapi Limiter (in_memory_fallback_enabled=True) - _storage_dead=False: → Redis (shared across workers) → handler - _storage_dead=True: → per-worker MemoryStorage fallback → handler - (cap effectively loosened to N× during outage window; - slowapi re-probes Redis with exponential backoff) -``` - -Trusted-token requests still skip the limiter entirely; the storage call is not made for them. - ---- - -## 6. Error handling - -The fail-degraded path lives entirely inside slowapi (`in_memory_fallback_enabled=True`). Behavior matrix, derived from `slowapi/extension.py`: - -| Condition | slowapi behavior | Cap during this state | -|---|---|---| -| Redis healthy | All counters in Redis, shared across workers | Strict `settings.rate_limit` per IP | -| Redis raises (any exception) | `_storage_dead = True`; logs WARNING once; routes subsequent checks to per-worker `MemoryStorage` | `N × settings.rate_limit` per IP per worker | -| Redis recovery probe (every `2^N` seconds, exponential backoff) succeeds | `_storage_dead = False`; logs INFO once; back to Redis | Strict on next request | -| Redis still down at next probe | Probe fails silently; backoff continues; no new log | Continues N× per worker | - -slowapi catches `Exception` broadly around its storage interactions, so connection errors, timeouts, and redis-py-specific errors are all handled. Programming errors in our own code paths still bubble up. - -The existing `_rate_limit_handler` for `RateLimitExceeded` is unchanged. Trusted-token requests still bypass entirely (the `exempt_when` check fires before the storage call). - -### Documented degraded mode - -`README.md` adds a short note in the rate-limit section: "When `PC2NUTS_RATE_LIMIT_STORAGE_URI` is set and the configured backend is unreachable, the service falls back to per-process in-memory rate limiting (slowapi `in_memory_fallback_enabled`). During the outage window, the effective per-IP cap is `PC2NUTS_WORKERS × rate_limit`. slowapi re-probes the primary storage with exponential backoff and resumes shared rate limiting on recovery." - ---- - -## 7. Testing - -### Unchanged (passes as-is) - -- All existing rate-limit tests run against the default in-memory `Limiter` and continue to pass byte-for-byte. The `Limiter` construction path for `rate_limit_storage_uri == None` is exactly the current code. - -### New tests - -Unit: - -- **`test_limiter_default_uses_memory_storage`** — when `rate_limit_storage_uri` is unset, `app.limiter.limiter._storage_uri` is the slowapi default (`None` or `"memory://"`) and `_in_memory_fallback_enabled` is `False`. Confirms the no-config path is byte-for-byte the current behavior. -- **`test_limiter_with_redis_uri_enables_fallback`** — when `rate_limit_storage_uri="redis://localhost:6379/0"` is configured (via `monkeypatch.setenv`), `app.limiter.limiter._storage_uri` is the configured URI and `_in_memory_fallback_enabled` is `True`. Does not contact Redis (Limiter constructor builds the storage object lazily-enough that no connection is opened until first request, but to be safe the test asserts on the constructor-level attributes only). - -Startup validator: - -- **`test_workers_gt_1_without_storage_uri_fails_startup`** — instantiate `Settings(workers=2, rate_limit_storage_uri=None)` and assert `ValidationError`. Asserts the validator fires. -- **`test_workers_gt_1_with_storage_uri_succeeds`** — `Settings(workers=2, rate_limit_storage_uri="redis://localhost:6379/0")` constructs cleanly without contacting Redis. -- **`test_workers_eq_1_without_storage_uri_succeeds`** — defaults still validate (regression guard). - -Integration (out of scope for the implementation PR — left as a separate task if CI gains a Redis service container): - -- A future integration test could spin up an ephemeral Redis, run two workers via `uvicorn --workers 2`, fire `>120` requests in a minute from a single client, assert the cap is observed across workers. The slowapi fail-degraded path is library code (already tested upstream), so the implementation PR does not need to re-test it; we only verify that our wiring activates the right slowapi configuration. - ---- - -## 8. Documentation - -- **`README.md`** — add a "Multi-worker deployment" subsection under the existing Configuration section: documents `PC2NUTS_WORKERS`, `PC2NUTS_RATE_LIMIT_STORAGE_URI`, the fail-degraded behavior during Redis outages, and the startup-validation guard. Cross-links to issue #68 in the changelog entry. -- **`CHANGELOG.md`** — entry under Unreleased: "Added multi-worker deployment via `PC2NUTS_WORKERS` env var. Multi-worker mode requires `PC2NUTS_RATE_LIMIT_STORAGE_URI` (e.g. a Redis URL) so the per-IP rate limit stays strict across workers; transient backend unavailability is tolerated via per-worker in-memory fallback for 30 s windows." -- **`docs/performance.md`** — no changes in this PR. The re-baseline run (acceptance criterion in #68) is post-deploy operational work. - ---- - -## 9. Acceptance criteria mapping (from issue #68) - -| Issue criterion | Where addressed | -|---|---| -| `Dockerfile` updated to launch N workers (configurable via env var) | §4.4 | -| Memory usage measured against the container's allocated memory; document the headroom | Out of scope for this PR — operational follow-up. README documents the per-worker memory ballpark from the issue body. | -| Rate-limit behaviour with N workers documented in `README.md` | §8 — option (a) Redis chosen, behavior documented including degraded mode | -| Performance baseline re-run; `docs/performance.md` updated | Out of scope for this PR — separate operational task post-deploy | - -The two "out of scope" items are operational/empirical work that can only happen against the deployed multi-worker container. They are tracked as a follow-up task in the issue rather than blocking the implementation PR. - ---- - -## 10. Open questions deferred to implementation - -- **Redis client library version pin.** `redis` (redis-py) — exact version range driven by what `limits 5.8.0` requires. The `limits[redis]` extra is the cleanest path; falls back to a direct pin if needed. Confirmed at implementation time. -- **Pydantic validator semantics.** Confirmed: project is on Pydantic v2 (`pydantic==2.12.5`, `pydantic-settings==2.13.0` per `requirements.lock`), so `@model_validator(mode="after")` is the target. - -These are mechanical questions that don't change the architecture; they're called out so the implementation plan addresses them rather than stumbling on them.