school_compare/docs/superpowers/plans/2026-08-23-feature-flags.md

# Feature Flags (Unleash) 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:** Let a merged feature reach production without becoming visible, and be switched on later from a UI.

**Architecture:** A self-hosted Unleash instance in its own Portainer stack holds flag state. FastAPI is the only service with an Unleash SDK; it declares every flag in a code registry, evaluates them fail-closed, and exposes them at an internal-only `/api/flags`. Next reads that endpoint server-side. No webhook — time-based ISR already propagates a flip within five minutes on school pages.

**Tech Stack:** Unleash OSS (Apache-2.0), `UnleashClient` (Python), FastAPI, Next.js 15 App Router, Playwright, pytest, Jest.

**Spec:** `docs/superpowers/specs/2026-08-23-feature-flags-design.md`

## Global Constraints

- **Every flag defaults to `False`.** There is no per-flag default field. A flag that defaults on is a kill switch, which this design does not offer.
- **One string per flag**, used unchanged as the registry key, the Unleash flag name, and the JSON key in `/api/flags`. snake_case, matching the API's existing convention. No case transformation anywhere.
- **Fail closed.** Unleash unreachable, unconfigured, or erroring ⇒ every flag is `False`. Never raise out of a flag evaluation.
- **`/api/flags` must never be publicly reachable.** It names unreleased features.
- **An API field that is off is absent, not `null`,** and withheld at the source. `/api/schools/` is public and unauthenticated (precedent: commit `c9a1892`).
- **Never flag content on a `force-static` page.** `app/admissions/page.tsx` never revalidates.
- **Reading flags in a route pins that route to a 300s ISR floor** — Next uses the lowest `revalidate` among a route's fetches.
- Backend tests run via: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q`
- Never push to `main`. Branch, PR, let checks pass.
- Do not start a local server to test the application; it does not work.

## File Structure

| File | Responsibility |
|---|---|
| `docker-compose.portainer.unleash.yml` (new) | The Unleash stack: server + its own Postgres. Owned by neither app stack. |
| `backend/flags.py` (new) | The whole flag layer: registry, client lifecycle, `is_enabled`, `all_flags`. One module, ~90 lines. |
| `backend/config.py` (modify) | Four `unleash_*` settings. |
| `backend/app.py` (modify) | `/api/flags` endpoint; `flags.init()` in `lifespan`; the `admission_distance` gate. |
| `backend/tests/test_flags.py` (new) | Registry shape, fail-closed evaluation, staleness tripwire, endpoint. |
| `nextjs-app/lib/flags.ts` (new) | `getFlags()` — one server-side fetch, typed, never throws. |
| `nextjs-app/app/api/[...path]/route.ts` (modify) | Internal-only denylist so the public proxy will not forward `/api/flags`. |
| `nextjs-app/__tests__/lib/flags.test.ts` (new) | `getFlags` fallback behaviour. |
| `nextjs-app/__tests__/api/proxyDenylist.test.ts` (new) | The proxy 404s internal paths. |
| `e2e/tests/journeys.spec.ts` (modify) | Flag-aware journeys; the explicit-gate rule from spec §7. |
| `requirements.txt` (modify) | `UnleashClient`. |
| `docker-compose.yml`, `docker-compose.portainer.yml`, `docker-compose.portainer.staging.yml` (modify) | `UNLEASH_*` env for the backend service. |

---

### Task 1: The Unleash stack

**Files:**
- Create: `docker-compose.portainer.unleash.yml`
- Modify: `docs/DEPLOY.md`

**Interfaces:**
- Consumes: nothing.
- Produces: a reachable Unleash server, and two client tokens — one scoped to the `development` environment, one to `production` — for Task 2's `UNLEASH_API_TOKEN`.

> **This task ends with a human step.** The stack must be deployed in Portainer and the tokens generated by hand; neither can be automated from here. Every later task works without it — flags simply evaluate `False` when `UNLEASH_URL` is unset, which is the correct local and CI behaviour.

- [ ] **Step 1: Write the stack file**

```yaml
# Portainer Stack Definition for School Compare — UNLEASH (feature flags)
#
# Deploy as a *separate* Portainer stack ("schoolcompare-unleash"), alongside
# the production and staging stacks. It deliberately belongs to neither: a
# staging redeploy must not be able to disturb production's flag state.
#
# One instance serves both environments. Open-source Unleash ships with
# `development` and `production` environments and environment-scoped client
# tokens, so the same flag holds independent state in each.
#
# Portainer environment variables (set in Portainer UI -> Stack -> Environment):
#   UNLEASH_DB_PASSWORD    — PostgreSQL password for the Unleash database
#   UNLEASH_ADMIN_PASSWORD — initial admin password for the Unleash UI
#   UNLEASH_IP             — macvlan IP for the Unleash server (default 10.0.1.152)

services:

  unleash_db:
    container_name: sc_unleash_postgres
    image: postgres:16-alpine
    environment:
      POSTGRES_USER: unleash
      POSTGRES_PASSWORD: ${UNLEASH_DB_PASSWORD}
      POSTGRES_DB: unleash
    volumes:
      - unleash_postgres_data:/var/lib/postgresql/data
    networks:
      - unleash
    healthcheck:
      test: ["CMD-SHELL", "pg_isready -U unleash"]
      interval: 10s
      timeout: 5s
      retries: 5
      start_period: 10s
    restart: unless-stopped

  unleash:
    container_name: sc_unleash
    image: unleashorg/unleash-server:6
    environment:
      DATABASE_URL: postgres://unleash:${UNLEASH_DB_PASSWORD}@unleash_db:5432/unleash
      DATABASE_SSL: "false"
      INIT_ADMIN_API_TOKENS: ""
      UNLEASH_DEFAULT_ADMIN_PASSWORD: ${UNLEASH_ADMIN_PASSWORD}
    depends_on:
      unleash_db:
        condition: service_healthy
    networks:
      unleash: {}
      macvlan:
        ipv4_address: ${UNLEASH_IP:-10.0.1.152}
    healthcheck:
      test: ["CMD-SHELL", "wget -qO- http://localhost:4242/health || exit 1"]
      interval: 30s
      timeout: 10s
      retries: 3
      start_period: 30s
    restart: unless-stopped

networks:
  unleash:
    driver: bridge
  macvlan:
    external: true
    name: macvlan

volumes:
  unleash_postgres_data:
```

- [ ] **Step 2: Document the runbook**

Append to `docs/DEPLOY.md`:

```markdown
## Feature flags (Unleash)

Flag state lives in a self-hosted Unleash instance, deployed as its own
Portainer stack from `docker-compose.portainer.unleash.yml`. It is separate
from the application stacks on purpose — redeploying staging must not be able
to disturb production's flags.

### First-time setup

1. Deploy the stack in Portainer. Set `UNLEASH_DB_PASSWORD`,
   `UNLEASH_ADMIN_PASSWORD` and (optionally) `UNLEASH_IP`.
2. Log in to the UI at `http://<UNLEASH_IP>:4242` as `admin`.
3. Create one **client** API token per environment:
   - `schoolcompare-staging`, environment **development**
   - `schoolcompare-prod`, environment **production**
   Client tokens, not admin tokens — the backend only reads.
4. Put each token in the matching Portainer stack's `UNLEASH_API_TOKEN`
   variable, and set `UNLEASH_URL` to `http://<UNLEASH_IP>:4242/api`.
5. Redeploy the application stacks.

### Turning a feature on

Flags are declared in `backend/flags.py` and appear in the Unleash UI on the
first evaluation after the backend starts. Toggle the flag in the environment
you want. A flip reaches school pages within about five minutes and place pages
within the hour — Next's ISR does the propagating, there is no webhook.

If `UNLEASH_URL` is unset, every flag evaluates to `False`. That is the correct
behaviour for local development and CI, and means nothing here is required to
run the tests.
```

- [ ] **Step 3: Commit**

```bash
git add docker-compose.portainer.unleash.yml docs/DEPLOY.md
git commit -m "feat(flags): add the Unleash stack and its runbook"
```

- [ ] **Step 4: Hand off the manual step**

Tell the human: the stack file is ready, and Steps 1–5 of the runbook are theirs to run. Note that the remaining tasks do not block on it.

---

### Task 2: The flag registry and client

**Files:**
- Create: `backend/flags.py`
- Create: `backend/tests/test_flags.py`
- Modify: `backend/config.py`
- Modify: `requirements.txt`

**Interfaces:**
- Consumes: `settings` from `backend/config.py`.
- Produces:
  - `backend.flags.Flag` — frozen dataclass with `name: str`, `description: str`, `added: date`
  - `backend.flags.REGISTRY: dict[str, Flag]`
  - `backend.flags.MAX_FLAG_AGE_DAYS: int` (= 90)
  - `backend.flags.init() -> None`
  - `backend.flags.is_enabled(name: str) -> bool`
  - `backend.flags.all_flags() -> dict[str, bool]`

- [ ] **Step 1: Write the failing tests**

Create `backend/tests/test_flags.py`:

```python
"""Tests for the feature flag layer (spec 2026-08-23).

None of these need a running Unleash. That is the point: an unset UNLEASH_URL
means every flag is False, which is what local development and CI get.
"""

from datetime import date, timedelta

import pytest

from backend import flags


def test_every_declared_flag_is_keyed_by_its_own_name():
    # One string is the registry key, the Unleash flag name and the JSON key.
    # A mismatch here would mean the UI toggles a flag the code never reads.
    for key, flag in flags.REGISTRY.items():
        assert key == flag.name


def test_flag_names_are_snake_case():
    # Matches the API's existing convention (admission_distance,
    # rwm_expected_pct) so no case transformation exists to get wrong.
    for name in flags.REGISTRY:
        assert name == name.lower()
        assert "-" not in name and " " not in name


def test_an_unconfigured_client_evaluates_every_flag_false(monkeypatch):
    monkeypatch.setattr(flags, "_client", None)
    for name in flags.REGISTRY:
        assert flags.is_enabled(name) is False


def test_an_undeclared_flag_is_false_rather_than_an_error(monkeypatch):
    # A typo'd flag name must not raise in a request path. It is logged as an
    # error, because an undeclared flag is always a bug.
    monkeypatch.setattr(flags, "_client", None)
    assert flags.is_enabled("no_such_flag") is False


def test_an_exploding_client_is_false_rather_than_a_500(monkeypatch):
    class Boom:
        def is_enabled(self, *a, **kw):
            raise RuntimeError("unleash is on fire")

    monkeypatch.setattr(flags, "_client", Boom())
    name = next(iter(flags.REGISTRY))
    assert flags.is_enabled(name) is False


def test_all_flags_reports_every_declared_flag(monkeypatch):
    monkeypatch.setattr(flags, "_client", None)
    assert set(flags.all_flags()) == set(flags.REGISTRY)
    assert all(v is False for v in flags.all_flags().values())


def test_a_flag_older_than_the_limit_fails_this_test():
    """A tripwire, not an assertion about correctness.

    Flags are temporary scaffolding and the failure mode of every flag system
    is accumulation. This fails on the day a flag turns 90, on whatever PR
    happens to be open — which is the point: someone has to decide.

    To fix: delete the flag and the branches that read it, or, if it genuinely
    still needs to exist, move its `added` date and say why in the commit.
    """
    stale = [
        f.name for f in flags.REGISTRY.values()
        if date.today() - f.added > timedelta(days=flags.MAX_FLAG_AGE_DAYS)
    ]
    assert not stale, (
        f"Flags older than {flags.MAX_FLAG_AGE_DAYS} days: {stale}. "
        "Remove the flag and the code branches it guards, or move its `added` "
        "date deliberately."
    )
```

- [ ] **Step 2: Run the tests to verify they fail**

Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_flags.py -q`

Expected: FAIL — `ModuleNotFoundError: No module named 'backend.flags'`

- [ ] **Step 3: Add the dependency**

Append to `requirements.txt`:

```
UnleashClient==6.0.1
```

- [ ] **Step 4: Add the settings**

In `backend/config.py`, inside `class Settings`, after the Typesense block:

```python
    # Feature flags (Unleash). An empty unleash_url disables flags entirely and
    # every flag evaluates False — the correct behaviour for local development
    # and CI, and the reason no test here needs a running Unleash.
    unleash_url: str = ""
    unleash_api_token: str = ""
    unleash_app_name: str = "schoolcompare-backend"
    # On a named volume, so a restart during an Unleash outage keeps
    # last-known state instead of reverting a released feature to dark.
    unleash_cache_directory: str = "/app/.unleash"
```

- [ ] **Step 5: Write the module**

Create `backend/flags.py`:

```python
"""Feature flags: what can be switched, and what is switched right now.

Ship-dark, not a kill switch. Flags let work merge and deploy without becoming
visible; they are expected to flip about monthly, by a person, deliberately.
Nothing here does percentage rollouts or user targeting — the site has no user
identity to target.

Unleash holds the state. It does not hold the list. REGISTRY below is that
list, and it exists for three reasons: the SDK evaluates an unknown flag to
False, so without a registry that is an *undeclared* False, indistinguishable
from a typo; /api/flags needs a key set to return when Unleash is unreachable;
and a flag in the UI but not in the registry is orphaned and should be visibly
so rather than quietly authoritative.

Every flag defaults to False. There is no per-flag default, because a flag that
defaults on is a kill switch, and this is not one.
"""

from __future__ import annotations

import logging
from dataclasses import dataclass
from datetime import date

from .config import settings

logger = logging.getLogger(__name__)

# A flag is temporary scaffolding. See test_a_flag_older_than_the_limit.
MAX_FLAG_AGE_DAYS = 90


@dataclass(frozen=True)
class Flag:
    # One string: the registry key, the Unleash flag name, and the JSON key in
    # /api/flags. snake_case, matching the API's existing convention. No case
    # transformation anywhere, so there is no mapping layer to get wrong.
    name: str
    description: str          # one line: what turning this on reveals
    added: date               # for the staleness tripwire


REGISTRY: dict[str, Flag] = {
    f.name: f for f in (
        Flag(
            name="admission_distance",
            description=(
                "The last-distance-offered figure on the Admissions tile and "
                "the 'How far away are you?' section on school pages."
            ),
            added=date(2026, 8, 23),
        ),
    )
}


_client = None


def init() -> None:
    """Start the Unleash client, or log why flags are all off.

    Called once from the app lifespan. Never raises: a flag system that can
    stop the API from booting is worse than one that is switched off.
    """
    global _client
    if not settings.unleash_url or not settings.unleash_api_token:
        logger.warning(
            "Unleash is not configured (UNLEASH_URL / UNLEASH_API_TOKEN); "
            "every feature flag evaluates to False.")
        return

    try:
        from UnleashClient import UnleashClient

        _client = UnleashClient(
            url=settings.unleash_url,
            app_name=settings.unleash_app_name,
            custom_headers={"Authorization": settings.unleash_api_token},
            cache_directory=settings.unleash_cache_directory,
            refresh_interval=15,
        )
        _client.initialize_client()
        logger.info("Unleash client initialised against %s", settings.unleash_url)
    except Exception:
        # Fail closed and keep serving. The SDK also evaluates everything False
        # until its first successful sync, so this is the same direction.
        _client = None
        logger.exception("Unleash client failed to start; flags are all False.")


def is_enabled(name: str) -> bool:
    """Whether `name` is on. False for anything unknown, unreachable or broken."""
    if name not in REGISTRY:
        logger.error(
            "undeclared feature flag %r was evaluated; returning False. "
            "Add it to backend/flags.py REGISTRY or fix the name.", name)
        return False
    if _client is None:
        return False
    try:
        return bool(_client.is_enabled(
            name, fallback_function=lambda feature_name, context: False))
    except Exception:
        logger.exception("flag %r failed to evaluate; returning False", name)
        return False


def all_flags() -> dict[str, bool]:
    """Every declared flag and its current value. Serves /api/flags."""
    return {name: is_enabled(name) for name in REGISTRY}
```

- [ ] **Step 6: Run the tests to verify they pass**

Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_flags.py -q`

Expected: PASS, 7 tests.

- [ ] **Step 7: Run the whole backend suite**

Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q`

Expected: all pass. Nothing else imports `flags` yet.

- [ ] **Step 8: Commit**

```bash
git add backend/flags.py backend/tests/test_flags.py backend/config.py requirements.txt
git commit -m "feat(flags): the registry and a fail-closed Unleash client"
```

---

### Task 3: `/api/flags`, and keeping it off the public internet

**Files:**
- Modify: `backend/app.py`
- Modify: `nextjs-app/app/api/[...path]/route.ts`
- Modify: `backend/tests/test_flags.py`
- Create: `nextjs-app/__tests__/api/proxyDenylist.test.ts`

**Interfaces:**
- Consumes: `backend.flags.init`, `backend.flags.all_flags`.
- Produces: `GET /api/flags` → `{"<flag_name>": bool, ...}`, reachable only from inside the Docker network.

> The endpoint and its exposure control ship together on purpose. The moment
> `/api/flags` exists, the public proxy at `app/api/[...path]/route.ts` will
> forward it, publishing the name and state of every unreleased feature.

- [ ] **Step 1: Write the failing backend test**

Append to `backend/tests/test_flags.py`:

```python
def _client():
    from fastapi.testclient import TestClient
    from backend import app as app_module
    return TestClient(app_module.app, raise_server_exceptions=False)


def test_the_flags_endpoint_lists_every_declared_flag(monkeypatch):
    monkeypatch.setattr(flags, "_client", None)
    body = _client().get("/api/flags").json()
    assert set(body) == set(flags.REGISTRY)


def test_the_flags_endpoint_answers_false_when_unleash_is_unreachable(monkeypatch):
    # The endpoint must still answer. A frontend that cannot read flags renders
    # everything dark, which is right; one that gets a 500 renders nothing.
    monkeypatch.setattr(flags, "_client", None)
    res = _client().get("/api/flags")
    assert res.status_code == 200
    assert all(v is False for v in res.json().values())
```

- [ ] **Step 2: Run it to verify it fails**

Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_flags.py -q`

Expected: FAIL — the endpoint 404s, so `set(body)` is `{"detail"}`.

- [ ] **Step 3: Add the import and the lifespan call**

In `backend/app.py`, add to the existing relative imports (after `from .data_loader import get_data_info as get_db_info`):

```python
from . import flags
```

Inside `async def lifespan(app: FastAPI):`, immediately after the `print("Loading school data from marts...")` line:

```python
    flags.init()
```

- [ ] **Step 4: Add the endpoint**

In `backend/app.py`, directly above `@app.get("/api/data-info")`:

```python
@app.get("/api/flags")
@limiter.limit(f"{settings.rate_limit_per_minute}/minute")
async def get_feature_flags(request: Request):
    """Every declared flag and its current value.

    Internal only. The Next proxy denies this path, because the response names
    every unreleased feature the codebase knows about — which is exactly what
    shipping dark is meant to keep quiet.
    """
    return flags.all_flags()
```

- [ ] **Step 5: Run the backend tests to verify they pass**

Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q`

Expected: all pass.

- [ ] **Step 6: Write the failing proxy test**

Create `nextjs-app/__tests__/api/proxyDenylist.test.ts`:

```typescript
/**
 * The /api/* proxy is public. Anything it forwards is on the internet.
 *
 * @jest-environment node
 */
// The docblock above is load-bearing. jest.config.js sets jsdom globally, and
// NextRequest/NextResponse need the Web Fetch API globals that only the node
// environment provides — under jsdom this suite fails on import, not on an
// assertion.
import { NextRequest } from 'next/server';
import { GET } from '@/app/api/[...path]/route';

function request(path: string) {
  return new NextRequest(`http://localhost:3000/api/${path}`);
}

describe('public API proxy', () => {
  it('refuses to forward internal-only paths', async () => {
    // /api/flags names every unreleased feature and its state. Forwarding it
    // publishes the thing shipping dark exists to keep quiet.
    const res = await GET(request('flags'), { params: Promise.resolve({ path: ['flags'] }) });
    expect(res.status).toBe(404);
  });

  it('does not deny a path that merely starts with the same letters', async () => {
    // A prefix match would take /api/flagship down with /api/flags.
    const res = await GET(
      request('flagship'), { params: Promise.resolve({ path: ['flagship'] }) });
    expect(res.status).not.toBe(404);
  });
});
```

- [ ] **Step 7: Run it to verify it fails**

Run: `cd nextjs-app && npx jest __tests__/api/proxyDenylist.test.ts`

Expected: FAIL — the first case returns 502 (no backend running in the test env), not 404.

- [ ] **Step 8: Add the denylist**

In `nextjs-app/app/api/[...path]/route.ts`, after the `STRIPPED_RESPONSE_HEADERS` constant:

```typescript
/*
 * API paths this public proxy must not forward.
 *
 * Matched on the first segment, exactly — a prefix match would take
 * /api/flagship down with /api/flags.
 *
 * `flags` is here because GET /api/flags names every unreleased feature the
 * codebase knows about, along with whether it is on. Publishing that defeats
 * the point of shipping dark. Next reads it server-side via FASTAPI_URL, on
 * the Docker network, which never transits this route.
 *
 * Anything else internal-only belongs here too.
 */
const INTERNAL_ONLY_SEGMENTS = new Set(['flags']);
```

And at the top of `handler`, immediately after `const { path } = await ctx.params;`:

```typescript
  if (INTERNAL_ONLY_SEGMENTS.has(path[0])) {
    return NextResponse.json({ detail: 'Not Found' }, { status: 404 });
  }
```

- [ ] **Step 9: Run the frontend tests**

Run: `cd nextjs-app && npx jest && npx tsc --noEmit`

Expected: all pass, typecheck clean.

- [ ] **Step 10: Commit**

```bash
git add backend/app.py backend/tests/test_flags.py 'nextjs-app/app/api/[...path]/route.ts' nextjs-app/__tests__/api/proxyDenylist.test.ts
git commit -m "feat(flags): serve /api/flags, and keep the public proxy off it"
```

---

### Task 4: Reading flags from Next

**Files:**
- Create: `nextjs-app/lib/flags.ts`
- Create: `nextjs-app/__tests__/lib/flags.test.ts`

**Interfaces:**
- Consumes: `GET /api/flags`.
- Produces: `getFlags(): Promise<Flags>` where `type Flags = Record<string, boolean>`, and `FLAGS_REVALIDATE = 300`.

> This ships with no consumer. The first flag needs none — the backend
> withholds the field and the frontend follows (Task 5). It is built anyway
> because "UI elements on existing pages" is one of the three surfaces this
> capability is for, and a flag layer that cannot gate a UI element is
> incomplete. Keep it minimal; do not add helpers nothing calls.

- [ ] **Step 1: Write the failing test**

Create `nextjs-app/__tests__/lib/flags.test.ts`:

```typescript
import { getFlags } from '@/lib/flags';

describe('getFlags', () => {
  afterEach(() => { jest.restoreAllMocks(); });

  it('returns the flags the API reports', async () => {
    jest.spyOn(global, 'fetch').mockResolvedValue({
      ok: true,
      json: async () => ({ admission_distance: true }),
    } as Response);
    await expect(getFlags()).resolves.toEqual({ admission_distance: true });
  });

  it('returns no flags rather than throwing when the API is down', async () => {
    // A page that cannot read flags must render everything dark, not 500.
    // Fail-closed is the same direction as the backend's default.
    jest.spyOn(global, 'fetch').mockRejectedValue(new Error('ECONNREFUSED'));
    await expect(getFlags()).resolves.toEqual({});
  });

  it('returns no flags rather than throwing on a non-200', async () => {
    jest.spyOn(global, 'fetch').mockResolvedValue({
      ok: false, status: 503,
    } as Response);
    await expect(getFlags()).resolves.toEqual({});
  });
});
```

- [ ] **Step 2: Run it to verify it fails**

Run: `cd nextjs-app && npx jest __tests__/lib/flags.test.ts`

Expected: FAIL — `Cannot find module '@/lib/flags'`.

- [ ] **Step 3: Write the module**

Create `nextjs-app/lib/flags.ts`:

```typescript
/**
 * Reading feature flags.
 *
 * Server-side only. No flag value reaches the browser bundle, and there is no
 * Unleash dependency in package.json — the SDK lives in FastAPI, which already
 * owns every other piece of data this app renders.
 *
 * Flags are declared in backend/flags.py. A purely front-end flag still has to
 * be declared there; it is a flat data edit, and the return is that one list
 * answers "what flags exist" for the whole system.
 */

export type Flags = Record<string, boolean>;

/*
 * Reading flags pins the calling route to this ISR floor: Next uses the LOWEST
 * revalidate among a route's fetches to set the whole route's revalidation
 * frequency. 300s matches what /school/[slug] already sits at, so a page that
 * reads flags is no more dynamic than a school page already is.
 *
 * It is also what makes a flip propagate without a webhook: five minutes on
 * school pages, an hour on place pages, against flags that flip monthly.
 */
export const FLAGS_REVALIDATE = 300;

const API = process.env.FASTAPI_URL || process.env.NEXT_PUBLIC_API_URL
  || 'http://localhost:8000/api';

/** Every flag and its value. Never throws: an unreadable flag is a dark one. */
export async function getFlags(): Promise<Flags> {
  try {
    const res = await fetch(`${API}/flags`, {
      next: { revalidate: FLAGS_REVALIDATE },
    });
    if (!res.ok) return {};
    return await res.json();
  } catch {
    return {};
  }
}
```

- [ ] **Step 4: Run the tests to verify they pass**

Run: `cd nextjs-app && npx jest __tests__/lib/flags.test.ts && npx tsc --noEmit`

Expected: PASS, 3 tests, typecheck clean.

- [ ] **Step 5: Commit**

```bash
git add nextjs-app/lib/flags.ts nextjs-app/__tests__/lib/flags.test.ts
git commit -m "feat(flags): server-side getFlags for the frontend"
```

---

### Task 5: Put `admission_distance` behind the flag

**Files:**
- Modify: `backend/app.py:809`
- Modify: `backend/tests/test_flags.py`

**Interfaces:**
- Consumes: `backend.flags.is_enabled`.
- Produces: `/api/schools/{urn}` omits `admission_distance` unless the flag is on.

> One gate, at the source. The frontend needs no change: `DistanceSection`
> already returns `null` when `admissionDistance?.distance_m == null`, and
> `PrimarySchoolSections`/`SecondarySchoolSections` already condition the
> admissions block on `(admissions || admissionDistance)`. Only 57 local
> authorities publish cut-offs, so the off-path is the commonest path on the
> site and is well covered already.

- [ ] **Step 1: Write the failing tests**

Append to `backend/tests/test_flags.py`:

```python
def _school_payload(monkeypatch, *, flag_on: bool):
    """Fetch one school's payload with the distance flag forced on or off.

    The DataFrame shape is copied from test_school_details.py rather than
    minimised: the endpoint reads a wide set of GIAS columns, and a trimmed
    frame fails for reasons that have nothing to do with flags.
    """
    import numpy as np
    import pandas as pd
    from fastapi.testclient import TestClient
    from backend import app as app_module

    df = pd.DataFrame([{
        "urn": 150275,
        "school_name": "West London Performing Arts Academy",
        "phase": "Secondary",
        "school_type": "Special post 16 institution",
        "trust_name": None,
        "religious_denomination": "Does not apply",
        "gender": None,
        "age_range": "16-25",
        "admissions_policy": None,
        "capacity": np.nan,
        "gias_total_pupils": np.nan,
        "headteacher_name": None,
        "website": None,
        "ofsted_grade": np.nan,
        "local_authority": "Ealing",
        "address": "268 Northfield Avenue, London, W5 4UB",
        "postcode": "W5 4UB",
        "latitude": 51.4986,
        "longitude": -0.3148,
        "year": np.nan,
        "total_pupils": np.nan,
        "eligible_pupils": np.nan,
        "rwm_expected_pct": np.nan,
    }])

    monkeypatch.setattr(app_module, "load_school_data", lambda: df)
    # Two arguments: get_supplementary_data(db, urn). See backend/app.py:763.
    monkeypatch.setattr(
        app_module, "get_supplementary_data",
        lambda db, urn: {"admission_distance": {"distance_m": 772.49,
                                                "year": 2024}})
    monkeypatch.setattr(flags, "is_enabled", lambda name: flag_on)

    client = TestClient(app_module.app, raise_server_exceptions=False)
    res = client.get("/api/schools/150275")
    assert res.status_code == 200, res.text
    return res.json()


def test_the_distance_field_is_absent_when_the_flag_is_off(monkeypatch):
    """Absent, not null, and withheld at the source.

    /api/schools/ is public and unauthenticated. Leaving a withheld field in
    the payload while declining to render it hands the record to anyone who
    opens the network tab — the reasoning already recorded in c9a1892.
    """
    body = _school_payload(monkeypatch, flag_on=False)
    assert "admission_distance" not in body


def test_the_distance_field_is_present_when_the_flag_is_on(monkeypatch):
    body = _school_payload(monkeypatch, flag_on=True)
    assert body["admission_distance"]["distance_m"] == 772.49
```

- [ ] **Step 2: Run them to verify they fail**

Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_flags.py -q`

Expected: FAIL on `test_the_distance_field_is_absent_when_the_flag_is_off` — the key is present with value `None`.

- [ ] **Step 3: Gate the field**

In `backend/app.py`, replace line 809:

```python
        "admission_distance": supplementary.get("admission_distance"),
```

with:

```python
        # Behind a flag, and withheld at the source rather than rendered-but-
        # hidden: this endpoint is public and unauthenticated, so a field left
        # in the payload is a published field. The key is absent, not null —
        # null would state that this school has no cut-off, which is a
        # different claim from "we are not publishing cut-offs".
        **({"admission_distance": supplementary.get("admission_distance")}
           if flags.is_enabled("admission_distance") else {}),
```

- [ ] **Step 4: Run the backend suite**

Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q`

Expected: all pass.

- [ ] **Step 5: Check the frontend type tolerates an absent key**

Run: `cd nextjs-app && npx tsc --noEmit`

`lib/types.ts:364` declares `admission_distance: SchoolAdmissionDistance | null`. An absent key is `undefined`, which that type does not admit. Change it to:

```typescript
  /**
   * Absent — not null — when the admission_distance flag is off. Null means
   * "this school has no published cut-off"; absent means "cut-offs are not
   * being published at all". They are different claims and the type says so.
   */
  admission_distance?: SchoolAdmissionDistance | null;
```

`app/school/[slug]/page.tsx` already reads it as `admission_distance ?? null`, so no call site changes.

- [ ] **Step 6: Run the frontend suite**

Run: `cd nextjs-app && npx jest && npx tsc --noEmit && npm run build`

Expected: all pass, build green.

- [ ] **Step 7: Commit**

```bash
git add backend/app.py backend/tests/test_flags.py nextjs-app/lib/types.ts
git commit -m "feat(flags): ship last-distance-offered dark behind a flag"
```

---

### Task 6: Flag-aware E2E journeys

**Files:**
- Modify: `e2e/tests/journeys.spec.ts`

**Interfaces:**
- Consumes: `GET /api/flags` (reachable from Playwright, which runs against the frontend's own origin — note that the *proxy* denies it, so these read it through the same denial and must handle that; see Step 1).

- [ ] **Step 1: Write the journeys**

The existing distance journeys at `e2e/tests/journeys.spec.ts:1214–1310` use
`schoolWithCutoff()`, which returns `null` and skips when no school has a
figure. With the flag off they would skip silently and the suite would go
green — a flag accidentally off in staging would look like a pass.

`/api/flags` is denied by the public proxy, so Playwright cannot read it
directly. The flag's observable effect is what the tests assert on instead:
whether the field appears in a school payload at all.

Insert directly above the `schoolWithCutoff` helper:

```typescript
/**
 * Whether the last-distance-offered feature is switched on here.
 *
 * Read from the data rather than from /api/flags, which the public proxy
 * denies on purpose — the endpoint names unreleased features. The observable
 * effect is the field's presence: the flag is off iff no candidate school
 * carries an `admission_distance` key at all.
 *
 * The distinction that matters: `admission_distance: null` means this school
 * has no published cut-off, and the key being ABSENT means cut-offs are not
 * being published at all.
 */
async function distanceFeatureIsOn(page: Page): Promise<boolean> {
  for (const urn of CUTOFF_CANDIDATE_URNS) {
    const res = await page.request.get(`/api/schools/${urn}`);
    if (!res.ok()) continue;
    if ('admission_distance' in (await res.json())) return true;
  }
  return false;
}
```

Then add, directly after the `schoolWithCutoff` helper:

```typescript
test('when the distance feature is on, a school with a cut-off is findable', async ({ page }) => {
  /*
   * The gate that stops the other distance journeys passing vacuously.
   *
   * They all skip when schoolWithCutoff() finds nothing, which is right when
   * the feature is off — but it means a feature that is *supposed* to be on
   * and is silently broken shows up as a green run full of skips. This test
   * fails in that case.
   */
  test.skip(!(await distanceFeatureIsOn(page)),
    'the admission_distance flag is off in this environment');

  expect(await schoolWithCutoff(page),
    'the distance feature is on, but no candidate school has a cut-off — '
    + 'the flag is on and the data or the query behind it is broken')
    .not.toBeNull();
});

test('with the distance feature off, the section is absent rather than empty', async ({ page }) => {
  // Shipping dark means the page renders as it did before the feature existed,
  // not as a feature with its content removed.
  test.skip(await distanceFeatureIsOn(page),
    'the admission_distance flag is on in this environment');

  // A school that exists, found rather than hardcoded — a 404 page would
  // satisfy the absent-heading assertion without proving anything.
  //
  // A plain loop, not Array.find: find's predicate is synchronous, so an async
  // one returns a Promise, every Promise is truthy, and it would always hand
  // back the first URN whether or not that school exists.
  let urn: number | null = null;
  for (const candidate of CUTOFF_CANDIDATE_URNS) {
    if ((await page.request.get(`/api/schools/${candidate}`)).ok()) {
      urn = candidate;
      break;
    }
  }
  expect(urn, 'no candidate school resolves in this environment').not.toBeNull();

  await page.goto(`/school/${urn}`);
  await expect(page.locator('h1')).toBeVisible();

  await expect(page.getByRole('heading', { name: /How far away are you\?/ }))
    .toHaveCount(0);
});

test('/api/flags is not reachable from the public internet', async ({ page }) => {
  // It names every unreleased feature and whether it is on. Next reads it
  // server-side over the Docker network; the public proxy must deny it.
  const res = await page.request.get('/api/flags');
  expect(res.status()).toBe(404);
});
```

- [ ] **Step 2: Verify the spec compiles and the tests are collected**

Run: `cd e2e && npx playwright test --list`

Expected: the three new titles appear, total count rises by 3.

- [ ] **Step 3: Commit**

```bash
git add e2e/tests/journeys.spec.ts
git commit -m "test(e2e): make the distance journeys fail loudly, not skip quietly"
```

---

### Task 7: Wire the environment variables

**Files:**
- Modify: `docker-compose.yml`
- Modify: `docker-compose.portainer.yml`
- Modify: `docker-compose.portainer.staging.yml`

**Interfaces:**
- Consumes: the tokens from Task 1.
- Produces: a backend that can reach Unleash in each environment.

- [ ] **Step 1: Add the variables to all three stacks**

In each file, in the **backend** service's `environment` block, after the
`TYPESENSE_API_KEY` line, add:

```yaml
      UNLEASH_URL: ${UNLEASH_URL:-}
      UNLEASH_API_TOKEN: ${UNLEASH_API_TOKEN:-}
```

Unset by default on purpose: an environment without them has every flag off,
which is the correct dark state rather than a failure.

- [ ] **Step 2: Add the cache volume to the two Portainer stacks**

The SDK's disk cache must survive a container restart, or a restart during an
Unleash outage reverts a released feature to dark.

In `docker-compose.portainer.yml` and `docker-compose.portainer.staging.yml`,
add to the backend service:

```yaml
    volumes:
      - unleash_cache:/app/.unleash
```

(If the backend service already has a `volumes:` block, add the one line to it
rather than a second block.)

And to each file's top-level `volumes:` block:

```yaml
  unleash_cache:
```

- [ ] **Step 3: Document the Portainer variables**

In `docker-compose.portainer.staging.yml`, add to the header comment list:

```
#   UNLEASH_URL            — http://<unleash-ip>:4242/api (empty = all flags off)
#   UNLEASH_API_TOKEN      — Unleash *client* token, environment: development
```

And the same in `docker-compose.portainer.yml` with `environment: production`.

- [ ] **Step 4: Commit**

```bash
git add docker-compose.yml docker-compose.portainer.yml docker-compose.portainer.staging.yml
git commit -m "chore(flags): wire UNLEASH_URL and the SDK cache volume into the stacks"
```

---

### Task 8: Open the PR

- [ ] **Step 1: Run everything**

```bash
uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q
cd nextjs-app && npx jest && npx tsc --noEmit && npm run build
cd ../e2e && npx playwright test --list
```

Expected: backend green, frontend green, typecheck clean, build green, e2e collects.

- [ ] **Step 2: Open the PR against `main`**

Body must state: the flag defaults off, so merging changes nothing visible in
any environment until someone toggles it in Unleash; and that Task 1's manual
Portainer steps are a prerequisite for the flag to be togglable at all.

## Notes for the executor

**Nothing here needs a running Unleash.** Every test passes with `UNLEASH_URL`
unset, which is also what CI gets. Task 1's manual steps only decide whether
the flag can be *turned on*, not whether the code works.

**The staleness test will fail on 2026-11-21** if `admission_distance` is still
declared. That is intended — see the docstring. It fails on whatever PR is open
at the time, which is the mechanism, not a bug in it.

**Do not add a webhook.** An earlier draft of the spec had one; §4 records why
it was removed and why time-based revalidation is enough.