merge: T81 ChatNotFoundError typed exception for skip routes

chat/web/drawer.py

@@ -48,6 +48,7 @@ from chat.state.world import active_scene, get_activity, get_chat, get_container
 from chat.web.bots import get_conn
 from chat.web.kickoff import get_llm_client
 from chat.web.skip import (
+    ChatNotFoundError,
     _now_iso,
     process_elision_skip,
     process_jump_skip,
@@ -993,13 +994,12 @@ async def skip_elision(
             new_time=new_time,
             landing_state_hint=landing_state_hint,
         )
+    except ChatNotFoundError as exc:
+        # Missing chat row: typed exception (T81) replaces the prior
+        # ``str(exc).startswith("chat not found")`` prefix sniff.
+        raise HTTPException(status_code=404, detail=str(exc))
     except ValueError as exc:
-        # ``process_elision_skip`` raises on missing-chat or malformed /
-        # backwards new_time. The drawer used to 404 / 400 these
-        # separately — preserve the 404-vs-400 split by sniffing the
-        # error message so existing tests keep passing without changes.
-        if str(exc).startswith("chat not found"):
-            raise HTTPException(status_code=404, detail=str(exc))
+        # Input-validation failure (malformed or backwards new_time).
         raise HTTPException(status_code=400, detail=str(exc))
     return await drawer(chat_id, request, conn)
 
@@ -1037,9 +1037,12 @@ async def skip_jump(
             notable_prose=notable_prose,
             reset_activity=reset_flag,
         )
+    except ChatNotFoundError as exc:
+        # Missing chat row: typed exception (T81) replaces the prior
+        # ``str(exc).startswith("chat not found")`` prefix sniff.
+        raise HTTPException(status_code=404, detail=str(exc))
     except ValueError as exc:
-        if str(exc).startswith("chat not found"):
-            raise HTTPException(status_code=404, detail=str(exc))
+        # Input-validation failure (malformed or backwards new_time).
         raise HTTPException(status_code=400, detail=str(exc))
     return await drawer(chat_id, request, conn)
 

chat/web/skip.py

@@ -36,6 +36,17 @@ from chat.state.entities import get_bot, get_you
 from chat.state.world import get_activity, get_chat
 
 
+class ChatNotFoundError(Exception):
+    """Raised when a ``chat_id`` doesn't resolve to a chat row.
+
+    Distinguishes the missing-chat case from generic input-validation
+    failures (which still raise :class:`ValueError`). HTTP callers map
+    this to ``404`` and ``ValueError`` to ``400`` — replacing the
+    earlier ``str(exc).startswith("chat not found")`` prefix sniff
+    (T81) with a typed dispatch.
+    """
+
+
 def _parse_iso_time(value: str) -> datetime | None:
     """Permissive ISO 8601 parser shared with the drawer routes (T59).
 
@@ -93,13 +104,14 @@ async def process_elision_skip(
     ..., "assistant_event_id": ...}`` so callers can introspect the
     generated turn (e.g. for SSE rebroadcast or test assertions).
 
-    Raises ``ValueError`` on validation failure or when the chat row
-    can't be located (the drawer maps it to ``HTTP 400`` / ``404``
-    respectively; the natural-language path follows the same shape).
+    Raises :class:`ChatNotFoundError` when the chat row is missing
+    (HTTP ``404``) and ``ValueError`` on input-validation failure
+    (HTTP ``400``). Splitting the two lets the drawer route dispatch
+    on type instead of sniffing the error string (T81).
     """
     chat = get_chat(conn, chat_id)
     if chat is None:
-        raise ValueError(f"chat not found: {chat_id}")
+        raise ChatNotFoundError(f"chat not found: {chat_id}")
 
     _validate_new_time(chat, new_time)
 
@@ -178,11 +190,13 @@ async def process_jump_skip(
     Returns ``{"assistant_text": ..., "speaker_id": ..., "skip_event_id":
     ..., "assistant_event_id": ...}``.
 
-    Raises ``ValueError`` on validation failure (caller maps to ``400``).
+    Raises :class:`ChatNotFoundError` on missing chat (caller maps to
+    ``404``) and ``ValueError`` on input-validation failure (caller maps
+    to ``400``).
     """
     chat = get_chat(conn, chat_id)
     if chat is None:
-        raise ValueError(f"chat not found: {chat_id}")
+        raise ChatNotFoundError(f"chat not found: {chat_id}")
 
     _validate_new_time(chat, new_time)
 
@@ -280,6 +294,7 @@ def _now_iso() -> str:
 
 
 __all__ = [
+    "ChatNotFoundError",
     "process_elision_skip",
     "process_jump_skip",
     "_now_iso",

chat/web/turns.py

@@ -79,7 +79,11 @@ from chat.web.kickoff import get_llm_client
 from chat.web.meanwhile import process_meanwhile_turn
 from chat.web.pubsub import publish
 from chat.web.render import render_turn_html as _render_turn_html
-from chat.web.skip import _parse_iso_time, process_elision_skip
+from chat.web.skip import (
+    ChatNotFoundError,
+    _parse_iso_time,
+    process_elision_skip,
+)
 
 router = APIRouter()
 
@@ -333,11 +337,15 @@ async def post_turn(
                 landing_state_hint=getattr(parsed, "landing_state_hint", "")
                 or "",
             )
+        except ChatNotFoundError as exc:
+            # Defensive: chat existence is checked above, so this only
+            # fires on a TOCTOU race where the chat row is deleted
+            # mid-request. T81 split the typed missing-chat case out of
+            # the generic ValueError so we keep the 404 mapping here.
+            raise HTTPException(status_code=404, detail=str(exc))
         except ValueError as exc:
-            # The controller raises on missing chat / bad new_time.
-            # Missing chat is already handled above (we'd have 404'd);
-            # a bad new_time here is a stub-derivation bug rather than
-            # user input — surface as 400 with the controller message.
+            # Bad new_time is a stub-derivation bug rather than user
+            # input — surface as 400 with the controller message.
             raise HTTPException(status_code=400, detail=str(exc))
         return Response(status_code=204)
 

tests/test_drawer_events_threads_skip.py

@@ -273,6 +273,43 @@ def test_post_skip_elision_advances_clock_and_emits_narration(client, tmp_path):
         assert tp["speaker_id"] == "bot_a"
 
 
+def test_skip_route_404_via_typed_exception_class(client, tmp_path):
+    """T81: drawer skip routes 404 via :class:`ChatNotFoundError`.
+
+    Pre-T81, the route caught ``ValueError`` and recovered the 404 case
+    by sniffing ``str(exc).startswith("chat not found")`` — fragile if
+    the message ever changed wording. The controller now raises a typed
+    exception so the route dispatches on type. Asserting the 404 from
+    the unseeded chat exercises the typed branch end-to-end; importing
+    the class confirms it's a real subclass of ``Exception`` and not a
+    re-export of ``ValueError`` (which would defeat the type split).
+    """
+    # Don't seed any chat — the controller hits ``get_chat`` returning
+    # ``None`` and raises ``ChatNotFoundError``. The drawer route then
+    # maps that to ``404`` via the typed handler (no string sniff).
+    _override_llm([])
+    try:
+        response = client.post(
+            "/chats/nonexistent/drawer/skip/elision",
+            data={
+                "landing_state_hint": "x",
+                "new_time": "2026-04-26T20:30:00+00:00",
+            },
+        )
+        assert response.status_code == 404
+    finally:
+        app.dependency_overrides.clear()
+
+    # The exception class itself is importable, distinct from ValueError,
+    # and a proper Exception subclass — pinning the type-based dispatch
+    # so future refactors can't quietly collapse it back to a string sniff.
+    from chat.web.skip import ChatNotFoundError
+
+    assert ChatNotFoundError is not None
+    assert issubclass(ChatNotFoundError, Exception)
+    assert not issubclass(ChatNotFoundError, ValueError)
+
+
 def test_post_skip_elision_invalid_time_returns_400(client, tmp_path):
     _seed_chat(tmp_path / "test.db")
     _override_llm([])