phase: 177_themed_favicon
Everything is verified. Final report: ## Phase 177 — Themed favicon: completion pass (all tasks pre-done, verified end-to-end) **Verified (no code fixes needed — implementation matched the design D1–D4 exactly):** - `theming.favicon_svg()` pure renderer + golden byte-identical pin; `GET/HEAD /favicon.svg` route (no-cache, `image/svg+xml`, built-in 200 fallback on DB blip, routes-first before static mount); 5 templates → bare `href="/favicon.svg"`, static file deleted (Containerfile cp removed); Theme-tab `data:` live preview with 4 restore points; JS↔server mirror pins. **Tests / lint / coverage (all green):** - `uv run pytest --cov=app --cov-report=term-missing` → 4161 passed, `app/` 99% (>90% gate) - `uv run ruff check .` → clean; `uv run pyright` → 0 errors - `uv run pytest tests/e2e/test_themed_favicon.py -v --no-cov` (isolated) → 6 passed - `test_theme_save_and_coverage.py` (92) + `test_theme_semantic_completion.py` (93), isolated → 3 + 8 passed **Completion criteria:** all met — default golden bytes/headers; themed roles + `#bor-theme` non-regression; DB-failure 200 fallback; 5 bare links + `/assets/favicon.svg` 404 + browser rendering; live-preview data: repaint + Save/Reset/re-show restore; gates green; E2E suites green in isolation. Commit: harness commits atomically (overrides AGENTS rule 8 — no git commands run). **Documentation:** README.md verified consistent in full (the phase's favicon paragraph is accurate; all commands, env vars, routes, ports, scripts cross-checked) — no changes needed. AGENTS.md verified consistent — no changes needed. **Notable:** `image-1.png` (owner's TODO screenshot, referenced by phase 176's overview) sits untracked at the repo root — pre-existing, explicitly out of scope per the phase-176 report; left untouched. **Next pending phase:** none — 177 was the last; TODO.md is cleared.
This commit is contained in:
@@ -0,0 +1,32 @@
|
||||
# Task 02 — Serve the themed favicon: `GET /favicon.svg`
|
||||
|
||||
**Phase:** `177_themed_favicon` · **Source:** `TODO.md:3` — "We need the favicon to change colors to match the theme."
|
||||
**Story:** n/a (TODO-derived, owner roadmap confirmation 2026-10-13)
|
||||
|
||||
## Objective
|
||||
The favicon over the wire: a route that renders the artwork from the EFFECTIVE palette on every request (no process cache — a runtime theme change is visible on the next request), with the never-break DB-blip fallback.
|
||||
|
||||
## Work
|
||||
1. `app/main.py` — in `create_app()`, after the router includes (ending at the `chats_shared_page_router` line ~L200) and BEFORE the static catch-all mount (`app.mount("/", …)` at L252, routes-first): register `app.api_route("/favicon.svg", methods=["GET", "HEAD"], include_in_schema=False)` (HEAD for static-file parity, the phase-76 `_shell_routes` parity idiom). The handler is a sync `def` (the house pattern for short-lived-session reads):
|
||||
- open a short-lived `SessionLocal()` session (imported from `app.db`, the `caching.py` middleware's exact pattern — NO process cache, `try/finally` close);
|
||||
- `effective = theming.effective_settings(db)` → `svg = theming.favicon_svg(effective)`;
|
||||
- ANY exception (DB blip, pre-migration boot): `logger.exception("favicon: theme read failed — serving the built-in favicon")` and `svg = theming.favicon_svg(theming.BUILTIN_COLORS)` — **200, never 500** (the theme-tag's never-break contract, `caching.py`'s DB-blip pattern);
|
||||
- return `Response(svg, media_type="image/svg+xml", headers={"Cache-Control": "no-cache"})`.
|
||||
- Commented rationale in the code (house style): (a) the route is deliberately OUTSIDE `/api/*` → no per-response byte-budget line (AGENTS rule 10 is `/api/*`-only) and no auth dependency (the favicon is chrome — public like the login page); (b) `CachingMiddleware` passes it through untouched (neither in `HTML_PAGES` nor under `/assets/`) → the route's own `no-cache` is the freshness contract (the palette changes at runtime; the page is no-cache too, so the browser revalidates together with it); (c) `SecurityHeadersMiddleware` still adds the CSP — no change needed (`img-src 'self' data:` covers task 04's `data:` preview).
|
||||
- ASSUMPTION: A2 (owner-confirmed) — serving shape: a new dynamic route `GET /favicon.svg` replaces the static file (task 03 deletes it).
|
||||
- ASSUMPTION: A3 (owner-confirmed) — fault tolerance: a DB failure / pre-migration boot serves the built-in palette with 200 (never 500).
|
||||
2. `tests/integration/test_favicon_route.py` — NEW, via the house `TestClient` fixture (follow the `tests/integration/test_ui_settings.py` app/db fixture pattern):
|
||||
- **default state** (no `ui_settings` row): `GET /favicon.svg` → 200, `content-type: image/svg+xml`, `cache-control: no-cache`, body byte-identical to the golden art constant (import it from `tests/unit/test_theming.py` — one source), and the response carries the CSP header (the `SecurityHeadersMiddleware` pass-through).
|
||||
- **themed state**: a `ui_settings` row (id 1) with distinct `surface`/`brand`/`brand_ink` (e.g. `#102030` / `#4f46e5` / `#a5b4fc`): the body carries the three hexes in their roles (attribute-parsed, same helper idiom as the unit pins) and none of the other 14; a second request after updating the row reflects the new palette immediately (no process cache — the runtime-change contract).
|
||||
- **DB-failure state**: patch `effective_settings` (or `SessionLocal`) to raise → 200 + the built-in golden bytes (never 500).
|
||||
- **HEAD parity**: `HEAD /favicon.svg` → 200, the same headers, empty body.
|
||||
|
||||
## Testing & Quality
|
||||
- Integration: the four states above (`tests/integration/test_favicon_route.py`), run with `uv run pytest tests/integration/test_favicon_route.py -v`.
|
||||
- Coverage: **>90%** on this task's new/modified code (the handler's success + fallback branches both driven).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run pytest tests/integration/test_favicon_route.py -v` green (all four states).
|
||||
- [ ] The route wins over the static mount (a file named `favicon.svg` in the static dir could not shadow it — the routes-first position holds).
|
||||
- [ ] Full test suite green; `app/` coverage >90%; ruff + pyright clean.
|
||||
- [ ] No behavior change in completed work (a new route only; `/assets/favicon.svg` still serves until task 03 deletes it).
|
||||
Reference in New Issue
Block a user