From a302ed427a783097e7bf41f67ad967ec9fe09559 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 26 Apr 2026 14:33:28 -0400 Subject: [PATCH] feat: error banners and first-run navigation flow --- CLAUDE.md | 18 +++++ chat/app.py | 29 +++++++- chat/templates/errors.html | 9 +++ chat/web/middleware.py | 61 ++++++++++++++++ tests/test_chat_list.py | 39 +++++++++- tests/test_error_ux.py | 57 +++++++++++++++ tests/test_first_run.py | 146 +++++++++++++++++++++++++++++++++++++ 7 files changed, 354 insertions(+), 5 deletions(-) create mode 100644 chat/templates/errors.html create mode 100644 chat/web/middleware.py create mode 100644 tests/test_error_ux.py create mode 100644 tests/test_first_run.py diff --git a/CLAUDE.md b/CLAUDE.md index c4fb82c..2869b01 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -149,3 +149,21 @@ Don't jump phases. Phase 1 must work end-to-end before Phase 2 lands. - Inference hosting (start with a cloud API, re-evaluate later) - Character template format (during Phase 1) - Multi-session / multi-character casts: **out of scope for v1**. Leave cheap schema hooks only. + +## Phase 1 status + +Phase 1 shipped end-to-end across **35 tasks** (T0–T35). The single-bot core loop is functional: event log + projector, schema + migrations, settings/bot authoring, kickoff confirm, streaming turns, drawer rendering, regenerate/rewind, scene close + per-POV summaries, significance classifier, snapshots/backups, first-run navigation, and friendly 404/500 pages. **168 tests passing.** + +Deferred to Phase 2: second bot, group node, scene configurations, witness filtering across multi-entity scenes, activity/containers, scene-transition compression. Phase 3: event queue + triggers, time skips, active threads. Phase 4: vector retrieval, branching, surgical delete + regenerate, impact-preview UI. + +### Known v1 limitations (read before extending) + +- **Drawer edits scope**: only affinity, significance, and pin can be hand-edited from the drawer. Other v1 fields (knowledge, summary text, traits) are deferred to Phase 1.5. +- **Cold-load snapshot path** is wired and unit-tested but rarely exercised in dev — long-running sessions are the only realistic trigger. +- **WAL sidecar files** (`-wal`, `-shm`) are not captured in nightly backups; the nightly snapshot is a fresh `.backup()` so this is fine for restore but worth knowing if you copy the db file by hand. +- **HTMX SSE event names** may need a version check if you bump the htmx CDN URL in `base.html` — the swap targets are name-coupled. +- **"You" activity rows** can linger after `bot_reset` (the reset purges the bot's chats and the bot's own activity row but not the "you" row that was associated with those chats). Cosmetic, fixed in Phase 1.5. +- **Projector replay is non-idempotent** for plain `INSERT` events. After appending, call `apply_event(conn, event)` for the new row only — calling `project(conn)` re-runs every handler from scratch and will trip uniqueness or duplicate inserts. +- **8-pin auto-cap eviction** is FIFO over the auto-pinned set only. Manual pins survive the eviction; this is by design (manual intent > auto-pin signal). +- **Regenerate (T29) does not broadcast `turn_html` over SSE** — the page must refresh to show the regenerated turn. Acceptable for v1 single-tab usage; Phase 1.5 should wire the SSE event. +- **First-run middleware** fires only on bare `/` and `/chats`. Sub-paths like `/chats/` and `/chats//drawer` pass through (correct: HTMX partials should not page-redirect, and a deep-link to a missing chat should 404, not redirect mid-setup). diff --git a/chat/app.py b/chat/app.py index 709eeea..3e96347 100644 --- a/chat/app.py +++ b/chat/app.py @@ -4,8 +4,11 @@ import logging from contextlib import asynccontextmanager from pathlib import Path -from fastapi import FastAPI +from fastapi import FastAPI, Request +from fastapi.responses import JSONResponse from fastapi.staticfiles import StaticFiles +from fastapi.templating import Jinja2Templates +from starlette.exceptions import HTTPException as StarletteHTTPException from chat.config import load_settings from chat.db.connection import open_db @@ -26,6 +29,7 @@ from chat.web.bots import router as bots_router from chat.web.chat import router as chat_router from chat.web.drawer import router as drawer_router from chat.web.kickoff import router as kickoff_router +from chat.web.middleware import FirstRunRedirectMiddleware from chat.web.nav import router as nav_router from chat.web.settings import router as settings_router from chat.web.sse import router as sse_router @@ -83,10 +87,33 @@ async def lifespan(app: FastAPI): app = FastAPI(title="chat", lifespan=lifespan) +app.add_middleware(FirstRunRedirectMiddleware) STATIC_DIR = Path(__file__).resolve().parent / "static" app.mount("/static", StaticFiles(directory=str(STATIC_DIR)), name="static") +ERROR_TEMPLATES = Jinja2Templates( + directory=str(Path(__file__).resolve().parent / "templates") +) + + +@app.exception_handler(StarletteHTTPException) +async def http_exception_handler(request: Request, exc: StarletteHTTPException): + """Render a friendly HTML page for 404/500; JSON for everything else.""" + if exc.status_code in (404, 500): + return ERROR_TEMPLATES.TemplateResponse( + request, + "errors.html", + { + "status_code": exc.status_code, + "detail": exc.detail or "Something went wrong.", + "active_nav": "chats", + }, + status_code=exc.status_code, + ) + return JSONResponse(status_code=exc.status_code, content={"detail": exc.detail}) + + app.include_router(bots_router) app.include_router(kickoff_router) app.include_router(settings_router) diff --git a/chat/templates/errors.html b/chat/templates/errors.html new file mode 100644 index 0000000..85f44e0 --- /dev/null +++ b/chat/templates/errors.html @@ -0,0 +1,9 @@ +{% extends "layout.html" %} +{% block title %}Error - chat{% endblock %} +{% block content %} +
+

{{ status_code }}

+

{{ detail }}

+

Back to chats

+
+{% endblock %} diff --git a/chat/web/middleware.py b/chat/web/middleware.py new file mode 100644 index 0000000..0d4cb05 --- /dev/null +++ b/chat/web/middleware.py @@ -0,0 +1,61 @@ +from __future__ import annotations + +from fastapi import Request +from fastapi.responses import RedirectResponse +from starlette.middleware.base import BaseHTTPMiddleware + +from chat.db.connection import open_db +from chat.state.entities import get_you, list_bots + + +class FirstRunRedirectMiddleware(BaseHTTPMiddleware): + """Redirect users through the first-run flow (per requirements §16.2). + + Behavior on GET requests to landing routes (``/`` and ``/chats``): + + - No ``you_entity`` → ``/settings`` + - ``you_entity`` exists but no bots → ``/bots/new`` + - Otherwise pass through to the underlying handler. + + The middleware is a no-op for: + + - Non-GET requests (POST/PUT writes proceed and surface their own errors). + - Static assets, health checks, and any path under ``/settings``, + ``/bots``, ``/api``, ``/health``, ``/favicon`` — so the user can + actually complete setup once redirected. + - Sub-paths of ``/chats`` (e.g. ``/chats/``, ``/chats//drawer``); + only the bare landing pages get the redirect treatment. Sub-resources + either 404 cleanly or are HTMX partials that should not page-redirect. + """ + + SKIP_PREFIXES = ( + "/static", + "/settings", + "/bots", + "/health", + "/favicon", + "/api", + ) + + async def dispatch(self, request: Request, call_next): + if request.method != "GET": + return await call_next(request) + + path = request.url.path + if any(path.startswith(p) for p in self.SKIP_PREFIXES): + return await call_next(request) + + # Only fire on the landing routes themselves. + if path != "/" and path != "/chats": + return await call_next(request) + + settings = request.app.state.settings + with open_db(settings.db_path) as conn: + you = get_you(conn) + bots = list_bots(conn) + + if you is None: + return RedirectResponse(url="/settings", status_code=303) + if not bots: + return RedirectResponse(url="/bots/new", status_code=303) + return await call_next(request) diff --git a/tests/test_chat_list.py b/tests/test_chat_list.py index 1f9c998..f320503 100644 --- a/tests/test_chat_list.py +++ b/tests/test_chat_list.py @@ -20,11 +20,29 @@ def client(tmp_path, monkeypatch): yield c -def _author_bot_and_chat(db_path: Path, bot_id: str = "bot_a") -> None: - """Insert a bot and a chat directly via the event log (skip kickoff route).""" +def _author_you(db_path: Path) -> None: + """Author a ``you_entity`` so the first-run middleware doesn't redirect.""" from chat.db.connection import open_db with open_db(db_path) as conn: + append_event( + conn, + kind="you_authored", + payload={"name": "Me", "pronouns": "", "persona": ""}, + ) + project(conn) + + +def _author_bot_and_chat(db_path: Path, bot_id: str = "bot_a") -> None: + """Insert a you_entity, bot, and chat via the event log (skip kickoff route).""" + from chat.db.connection import open_db + + with open_db(db_path) as conn: + append_event( + conn, + kind="you_authored", + payload={"name": "Me", "pronouns": "", "persona": ""}, + ) append_event( conn, kind="bot_authored", @@ -53,13 +71,26 @@ def _author_bot_and_chat(db_path: Path, bot_id: str = "bot_a") -> None: project(conn) -def test_root_redirects_to_chats(client): +def test_root_redirects_to_chats_when_setup_complete(client, tmp_path): + # With both you_entity and a bot present, the first-run middleware + # passes through and the nav router sends "/" → "/chats". + _author_bot_and_chat(tmp_path / "test.db", "bot_a") response = client.get("/", follow_redirects=False) assert response.status_code == 303 assert response.headers["location"] == "/chats" -def test_chats_list_empty_state(client): +def test_chats_list_empty_state(client, tmp_path): + # Author you + a bot but NO chats — should render the empty-state + # chats list, not redirect. + _author_bot_and_chat(tmp_path / "test.db", "bot_a") + # Drop the chat row so we hit the empty-state branch (the helper + # creates a chat — undo it via a fresh seed without chat_created). + from chat.db.connection import open_db + + with open_db(tmp_path / "test.db") as conn: + conn.execute("DELETE FROM chats") + conn.commit() response = client.get("/chats") assert response.status_code == 200 body = response.text.lower() diff --git a/tests/test_error_ux.py b/tests/test_error_ux.py new file mode 100644 index 0000000..d30139f --- /dev/null +++ b/tests/test_error_ux.py @@ -0,0 +1,57 @@ +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient + +from chat.app import app + + +@pytest.fixture +def client(tmp_path, monkeypatch): + cfg = tmp_path / "config.toml" + cfg.write_text('featherless_api_key = "test"\n') + monkeypatch.setenv("CHAT_CONFIG_PATH", str(cfg)) + db = tmp_path / "test.db" + monkeypatch.setenv("CHAT_DB_PATH", str(db)) + with TestClient(app) as c: + if hasattr(app.state, "background_worker"): + app.state.background_worker.enabled = False + yield c + + +def _setup_minimal_state(db_path): + """Set up enough state so the first-run middleware doesn't redirect.""" + from chat.db.connection import open_db + from chat.eventlog.log import append_event + from chat.eventlog.projector import project + + with open_db(db_path) as conn: + append_event( + conn, + kind="you_authored", + payload={"name": "Me", "pronouns": "", "persona": ""}, + ) + append_event( + conn, + kind="bot_authored", + payload={ + "id": "bot_a", + "name": "BotA", + "persona": "", + "voice_samples": [], + "traits": [], + "backstory": "", + "initial_relationship_to_you": "", + "kickoff_prose": "", + }, + ) + project(conn) + + +def test_404_renders_friendly_page_for_html(client, tmp_path): + _setup_minimal_state(tmp_path / "test.db") + response = client.get("/chats/no_such_chat") + assert response.status_code == 404 + body = response.text + assert "404" in body + assert "back to" in body.lower() diff --git a/tests/test_first_run.py b/tests/test_first_run.py new file mode 100644 index 0000000..c7799aa --- /dev/null +++ b/tests/test_first_run.py @@ -0,0 +1,146 @@ +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient + +from chat.app import app +from chat.db.connection import open_db +from chat.eventlog.log import append_event +from chat.eventlog.projector import project + + +@pytest.fixture +def client(tmp_path, monkeypatch): + cfg = tmp_path / "config.toml" + cfg.write_text('featherless_api_key = "test"\n') + monkeypatch.setenv("CHAT_CONFIG_PATH", str(cfg)) + db = tmp_path / "test.db" + monkeypatch.setenv("CHAT_DB_PATH", str(db)) + with TestClient(app) as c: + if hasattr(app.state, "background_worker"): + app.state.background_worker.enabled = False + yield c + + +def test_root_redirects_to_settings_when_no_you(client): + response = client.get("/", follow_redirects=False) + assert response.status_code == 303 + assert response.headers["location"] == "/settings" + + +def test_chats_redirects_to_settings_when_no_you(client): + response = client.get("/chats", follow_redirects=False) + assert response.status_code == 303 + assert response.headers["location"] == "/settings" + + +def test_redirects_to_bots_new_when_you_exists_but_no_bots(client, tmp_path): + with open_db(tmp_path / "test.db") as conn: + append_event( + conn, + kind="you_authored", + payload={ + "name": "Me", + "pronouns": "they/them", + "persona": "engineer", + }, + ) + project(conn) + response = client.get("/chats", follow_redirects=False) + assert response.status_code == 303 + assert response.headers["location"] == "/bots/new" + + +def test_root_redirects_to_bots_new_when_you_exists_but_no_bots(client, tmp_path): + with open_db(tmp_path / "test.db") as conn: + append_event( + conn, + kind="you_authored", + payload={ + "name": "Me", + "pronouns": "they/them", + "persona": "engineer", + }, + ) + project(conn) + response = client.get("/", follow_redirects=False) + assert response.status_code == 303 + assert response.headers["location"] == "/bots/new" + + +def test_no_redirect_when_setup_complete(client, tmp_path): + with open_db(tmp_path / "test.db") as conn: + append_event( + conn, + kind="you_authored", + payload={ + "name": "Me", + "pronouns": "they/them", + "persona": "engineer", + }, + ) + append_event( + conn, + kind="bot_authored", + payload={ + "id": "bot_a", + "name": "BotA", + "persona": "...", + "voice_samples": [], + "traits": [], + "backstory": "", + "initial_relationship_to_you": "", + "kickoff_prose": "", + }, + ) + project(conn) + response = client.get("/chats", follow_redirects=False) + # /chats page renders normally (200) instead of redirecting. + assert response.status_code == 200 + + +def test_settings_page_accessible_without_you(client): + """Don't redirect FROM /settings — user needs to fill it out.""" + response = client.get("/settings", follow_redirects=False) + assert response.status_code == 200 + + +def test_bots_new_accessible_without_redirect(client, tmp_path): + with open_db(tmp_path / "test.db") as conn: + append_event( + conn, + kind="you_authored", + payload={"name": "Me", "pronouns": "", "persona": ""}, + ) + project(conn) + response = client.get("/bots/new", follow_redirects=False) + assert response.status_code == 200 + + +def test_bots_list_accessible_without_redirect_when_empty(client, tmp_path): + """The bot list page itself should never redirect — even when empty.""" + with open_db(tmp_path / "test.db") as conn: + append_event( + conn, + kind="you_authored", + payload={"name": "Me", "pronouns": "", "persona": ""}, + ) + project(conn) + response = client.get("/bots", follow_redirects=False) + assert response.status_code == 200 + + +def test_post_to_settings_not_redirected(client): + """POST should bypass middleware — it's a write, not a landing nav.""" + response = client.post( + "/settings", + data={"name": "Me", "pronouns": "", "persona": ""}, + follow_redirects=False, + ) + # Settings POST returns 200 with the saved page (no HTTPException raised). + assert response.status_code == 200 + + +def test_health_endpoint_not_redirected(client): + response = client.get("/health", follow_redirects=False) + assert response.status_code == 200