merge: T108 scene-close-on-cancel strengthen test + rationale

Investigation surfaced a transactional bug in the cancel path: when the
primary stream raises asyncio.CancelledError mid-stream, post_turn
re-raises at end-of-function, and open_db's dependency teardown skips
conn.commit() — rolling back ALL post-cancel writes including the
scene_closed event. The existing T74.3 regression test only passes
because asyncio is not imported at module scope, so CancelledError
becomes NameError (caught by except Exception, leaves cancelled=False).
Documented in turns.py + test docstring; deferred for triage.

chat/services/embeddings.py

@@ -10,6 +10,7 @@ EmbeddingResult shape stays the same, only the generator changes.
 from __future__ import annotations
 
 import hashlib
+import logging
 import math
 import struct
 
@@ -18,6 +19,8 @@ from pydantic import BaseModel
 from chat.llm.client import LLMClient
 
 
+_log = logging.getLogger(__name__)
+
 DEFAULT_EMBEDDING_DIM = 384
 DEFAULT_EMBEDDING_MODEL = "pseudo-sha256-384"
 FALLBACK_EMBEDDING_MODEL = "fallback"
@@ -93,7 +96,15 @@ async def generate_embedding(
         return EmbeddingResult(vector=_pseudo_embed(text, dim), model=model, dim=dim)
 
     # Future: real embedding via client.embed(...). Phase 4.5 work.
-    # For Phase 4, any non-default model falls through to fallback.
+    # For Phase 4, any non-default model falls through to fallback —
+    # warn so misconfigured callers (e.g., a real-model swap that isn't
+    # wired up yet) don't silently degrade to a zero vector.
+    _log.warning(
+        "generate_embedding: non-default model %r returned fallback "
+        "(model client.embed() not yet implemented in Phase 4.5+); "
+        "downstream search will degrade silently. Configure a supported model.",
+        model,
+    )
     return EmbeddingResult(
         vector=[0.0] * dim, model=FALLBACK_EMBEDDING_MODEL, dim=dim
     )

chat/state/branches.py

@@ -9,11 +9,15 @@ existing event readers remain branch-agnostic.
 """
 
 from __future__ import annotations
+
+import logging
 from sqlite3 import Connection
 
 from chat.eventlog.projector import on
 from chat.eventlog.log import Event
 
+logger = logging.getLogger(__name__)
+
 
 @on("branch_created")
 def _apply_branch_created(conn: Connection, e: Event) -> None:
@@ -37,9 +41,26 @@ def _apply_branch_switched(conn: Connection, e: Event) -> None:
     """Set is_active=1 on the named branch and is_active=0 on all others.
 
     Atomic via two UPDATEs ordered to avoid the unique-active-index race.
+
+    If the named branch does not exist, a warning is emitted and the
+    is_active flags are still cleared (preserving prior behavior — the
+    second UPDATE simply matches no rows). Callers should validate the
+    name upstream; this guard surfaces accidental mismatches in the log.
     """
     p = e.payload
     name = p["name"]
+    # Warn (don't raise) if the target branch is missing. The existing
+    # outcome — zero active branches — is preserved; this just makes the
+    # condition observable instead of silent.
+    exists = conn.execute(
+        "SELECT 1 FROM branches WHERE name = ? LIMIT 1",
+        (name,),
+    ).fetchone()
+    if exists is None:
+        logger.warning(
+            "branch_switched to unknown branch name %r; no branch will be active",
+            name,
+        )
     # Clear ALL is_active flags first (avoids the unique-index trip).
     conn.execute("UPDATE branches SET is_active = 0 WHERE is_active = 1")
     conn.execute(
@@ -79,6 +100,16 @@ def get_branch(conn: Connection, name: str) -> dict | None:
 
 
 def list_branches(conn: Connection, chat_id: str | None = None) -> list[dict]:
+    """Return branch rows, optionally scoped to a chat.
+
+    When ``chat_id`` is provided the filter is ``chat_id = ? OR chat_id IS NULL``,
+    so global (null-chat) branches are returned in *every* per-chat scope. This
+    is intentional: the bootstrapped ``"main"`` branch (and any future
+    null-chat branches) are global by design — they belong to no single chat
+    and should appear alongside per-chat branches in any chat-scoped listing.
+    Callers that want only per-chat branches should filter the result on
+    ``chat_id is not None``.
+    """
     if chat_id is None:
         rows = conn.execute(
             "SELECT id, name, origin_event_id, head_event_id, chat_id, "

chat/state/memory.py

@@ -112,6 +112,25 @@ SIGNIFICANCE_RANK_BIAS = 0.5
 RRF_CONST = 60
 
 
+def _max_event_id(conn: Connection, owner_id: str) -> int:
+    """Return the largest ``memories.id`` for ``owner_id`` (1 if none exist).
+
+    Used as the recency-boost denominator by both ``_composite_rerank`` and
+    ``_rrf_fuse_and_rerank`` (T104). The row id is a monotonic recency proxy
+    — newer memories have larger ids — so dividing by the per-owner max keeps
+    the boost in [0, 1] regardless of how many memories the owner has.
+
+    Returns 1 (not 0) when the owner has no rows so callers can divide by
+    the result without a guard. The "no memories" case never actually hits
+    this helper because the FTS query above would have returned no rows,
+    but the safe default keeps the helper trivially reusable.
+    """
+    row = conn.execute(
+        "SELECT MAX(id) FROM memories WHERE owner_id = ?", (owner_id,)
+    ).fetchone()
+    return row[0] if row and row[0] else 1
+
+
 def search_memories(
     conn: Connection,
     owner_id: str,
@@ -163,6 +182,14 @@ def search_memories(
 
     When ``query_vector`` is None: FTS-only behaviour unchanged — all
     Phase 1-3.5 callers see the same row shape and ordering as before.
+
+    **Row-shape contract (T104):** every returned dict carries an
+    ``fts_rank`` key. For FTS hits this is the BM25 score (a negative float,
+    lower-is-better). For *vector-only* hits surfaced by the fused path —
+    rows that matched the query embedding but did NOT match FTS — the
+    ``fts_rank`` value is ``None``. Downstream consumers must accept
+    ``None`` here; do not assume ``fts_rank`` is always numeric. The
+    ``composite_score`` is always a float on every returned row.
     """
     if witness_role not in _VALID_WITNESS_ROLES:
         raise ValueError(
@@ -227,10 +254,7 @@ def _composite_rerank(
     Extracted from ``search_memories`` so the no-vector path stays a single
     call and the fused path can re-use the same boost formulae after RRF.
     """
-    max_id_row = conn.execute(
+    max_id = _max_event_id(conn, owner_id)
-        "SELECT MAX(id) FROM memories WHERE owner_id = ?", (owner_id,)
-    ).fetchone()
-    max_id = max_id_row[0] if max_id_row and max_id_row[0] else 1
 
     result_cols = cols + ["fts_rank"]
     enriched: list[dict] = []
@@ -343,10 +367,7 @@ def _rrf_fuse_and_rerank(
 
     # Final composite re-rank: significance + recency boosts on top of the
     # negated fusion score so the sort direction matches the FTS-only path.
-    max_id_row = conn.execute(
+    max_id = _max_event_id(conn, owner_id)
-        "SELECT MAX(id) FROM memories WHERE owner_id = ?", (owner_id,)
-    ).fetchone()
-    max_id = max_id_row[0] if max_id_row and max_id_row[0] else 1
 
     result_cols = cols + ["fts_rank"]
     enriched: list[dict] = []

chat/web/search.py

@@ -14,6 +14,12 @@ For each match we hydrate just enough metadata to render a row:
 * the originating scene title when one exists,
 * and the ``pov_summary`` itself.
 
+T106 (Phase 4.5): hydration is batched. Pre-T106 the route called
+``get_bot``/``get_chat``/``get_scene`` once per result row — N+1 with
+``DEFAULT_SEARCH_K=50`` meaning up to 150 individual SELECTs per page
+load. We now collect distinct ids first and fan-in via three
+``WHERE id IN (...)`` queries, then map back per row.
+
 We deliberately keep this module synchronous and template-only — no
 HTMX swaps, no JSON API — because the search box is a "leave the
 current chat to look something up" surface, not an inline drawer.
@@ -21,7 +27,9 @@ current chat to look something up" surface, not an inline drawer.
 
 from __future__ import annotations
 
+import json
 from pathlib import Path
+from sqlite3 import Connection
 
 from fastapi import APIRouter, Depends, Request
 from fastapi.responses import HTMLResponse
@@ -36,29 +44,145 @@ TEMPLATES = Jinja2Templates(
     directory=str(Path(__file__).resolve().parent.parent / "templates")
 )
 
+#: Maximum cross-chat FTS matches surfaced per ``/search`` page load.
+#: Extracted as a module-level constant (T106) so the cap is tunable
+#: without touching the route body. ``search_all_memories`` itself
+#: defaults to a smaller ``k=20``; we override here because the
+#: top-bar search is a "scan everything I've seen" surface, not an
+#: inline drawer.
+DEFAULT_SEARCH_K = 50
+
 router = APIRouter()
 
 
+def _fetch_bots_by_ids(conn: Connection, ids: set[str]) -> dict[str, dict]:
+    """Batched sibling of :func:`chat.state.entities.get_bot`.
+
+    Inlined here (not exported from ``state.entities``) to keep T106's
+    scope confined to ``search.py`` per the Phase 4.5 plan. Returns
+    ``{bot_id: bot_dict}`` for every id present in ``ids``; ids with
+    no matching row are simply absent from the map (the caller falls
+    back to the raw id string the same way it did pre-T106).
+
+    Empty ``ids`` short-circuits to ``{}`` because SQLite rejects
+    ``WHERE id IN ()`` as a syntax error.
+    """
+    if not ids:
+        return {}
+    placeholders = ",".join("?" * len(ids))
+    cols = [c[1] for c in conn.execute("PRAGMA table_info(bots)").fetchall()]
+    rows = conn.execute(
+        f"SELECT * FROM bots WHERE id IN ({placeholders})",
+        tuple(ids),
+    ).fetchall()
+    out: dict[str, dict] = {}
+    for row in rows:
+        d = dict(zip(cols, row))
+        d["voice_samples"] = json.loads(d.pop("voice_samples_json"))
+        d["traits"] = json.loads(d.pop("traits_json"))
+        out[d["id"]] = d
+    return out
+
+
+def _fetch_chats_by_ids(conn: Connection, ids: set[str]) -> dict[str, dict]:
+    """Batched sibling of :func:`chat.state.world.get_chat`.
+
+    Mirrors that helper's ``chats``/``chat_state`` JOIN so the returned
+    dicts have the same shape (``narrative_anchor``, ``time``,
+    ``weather``, ``active_scene_id``, etc.). Empty ``ids`` returns
+    ``{}`` to dodge the ``IN ()`` syntax error.
+    """
+    if not ids:
+        return {}
+    placeholders = ",".join("?" * len(ids))
+    rows = conn.execute(
+        "SELECT c.id, c.host_bot_id, c.guest_bot_id, c.created_at, "
+        "       s.time, s.weather, s.active_scene_id, s.narrative_anchor "
+        f"FROM chats c JOIN chat_state s ON s.chat_id = c.id "
+        f"WHERE c.id IN ({placeholders})",
+        tuple(ids),
+    ).fetchall()
+    return {
+        row[0]: {
+            "id": row[0],
+            "host_bot_id": row[1],
+            "guest_bot_id": row[2],
+            "created_at": row[3],
+            "time": row[4],
+            "weather": row[5],
+            "active_scene_id": row[6],
+            "narrative_anchor": row[7],
+        }
+        for row in rows
+    }
+
+
+def _fetch_scenes_by_ids(conn: Connection, ids: set[int]) -> dict[int, dict]:
+    """Batched sibling of :func:`chat.state.world.get_scene`.
+
+    Returns ``{scene_id: scene_dict}`` with ``participants`` already
+    JSON-decoded so callers see the same shape as the per-row helper.
+    Empty ``ids`` returns ``{}``.
+    """
+    if not ids:
+        return {}
+    placeholders = ",".join("?" * len(ids))
+    cols = [c[1] for c in conn.execute("PRAGMA table_info(scenes)").fetchall()]
+    rows = conn.execute(
+        f"SELECT * FROM scenes WHERE id IN ({placeholders})",
+        tuple(ids),
+    ).fetchall()
+    out: dict[int, dict] = {}
+    for row in rows:
+        d = dict(zip(cols, row))
+        d["participants"] = json.loads(d.pop("participants_json"))
+        out[d["id"]] = d
+    return out
+
+
 @router.get("/search", response_class=HTMLResponse)
 async def search(request: Request, q: str = "", conn=Depends(get_conn)):
-    """Render ``search.html`` with up to 50 cross-chat FTS matches.
+    """Render ``search.html`` with up to :data:`DEFAULT_SEARCH_K` matches.
 
     ``q`` is intentionally allowed to be empty — that path renders the
     page's "enter a query" placeholder rather than a 400, because the
     top-bar form submits to this URL even with an empty input. T93's
     service short-circuits whitespace-only queries to ``[]`` so there
     is no FTS5 ``MATCH ''`` syntax error to guard against here.
-    """
-    raw_results = search_all_memories(conn, query=q, k=50) if q else []
 
-    # Hydrate display fields per row. We do this in the route (not the
+    Hydration (T106) is batched: rather than calling ``get_bot`` /
-    # service) so the service stays a pure FTS shim that other UIs
+    ``get_chat`` / ``get_scene`` per row (worst case 3 * k individual
-    # can reuse.
+    SELECTs), we collect distinct ids and issue one ``IN (...)`` query
+    per entity kind, then map back during the row build. ``get_bot``
+    et al. remain imported for test-time monkeypatching but are no
+    longer invoked on the hot path.
+    """
+    raw_results = (
+        search_all_memories(conn, query=q, k=DEFAULT_SEARCH_K) if q else []
+    )
+
+    # Collect distinct ids up front so the IN-list queries dedupe (a
+    # popular bot or scene shows up many times across the result set).
+    bot_ids: set[str] = {r["owner_id"] for r in raw_results if r["owner_id"]}
+    chat_ids: set[str] = {r["chat_id"] for r in raw_results if r["chat_id"]}
+    scene_ids: set[int] = {
+        r["scene_id"] for r in raw_results if r["scene_id"]
+    }
+
+    bots_by_id = _fetch_bots_by_ids(conn, bot_ids)
+    chats_by_id = _fetch_chats_by_ids(conn, chat_ids)
+    scenes_by_id = _fetch_scenes_by_ids(conn, scene_ids)
+
+    # Hydrate display fields per row from the batched maps. We do this
+    # in the route (not the service) so the service stays a pure FTS
+    # shim that other UIs can reuse.
     results = []
     for row in raw_results:
-        bot = get_bot(conn, row["owner_id"])
+        bot = bots_by_id.get(row["owner_id"])
-        chat = get_chat(conn, row["chat_id"])
+        chat = chats_by_id.get(row["chat_id"])
-        scene = get_scene(conn, row["scene_id"]) if row["scene_id"] else None
+        scene = (
+            scenes_by_id.get(row["scene_id"]) if row["scene_id"] else None
+        )
         results.append(
             {
                 "memory_id": row["memory_id"],

chat/web/snapshots.py

@@ -8,20 +8,27 @@ Routes:
 
 * ``GET  /snapshots``                    list all snapshots (both kinds)
 * ``POST /snapshots/take``               take a periodic snapshot now
-* ``POST /snapshots/restore/{id}``       restore (requires matching ``confirm_id``)
+* ``POST /snapshots/restore/{id}``       restore (requires matching ``confirm_id`` and ``kind``)
 * ``GET  /snapshots/{id}/preview``       show metadata + delta vs current
 
 The ``snapshot_id`` is the filename stem (the UTC timestamp written by
 :func:`chat.services.snapshot.take_snapshot`) — there's no separate UUID,
 and the timestamp filename is already unique per snapshot kind. Both
 periodic and rewind snapshots share the same id space lookup-wise, so
-the restore + preview routes accept ``kind`` as a form/query param to
+the restore + preview routes require ``kind`` as a form/query param to
-disambiguate.
+disambiguate (a missing/empty ``kind`` is a 400, not a silent default).
+
+Note on ``created_at`` mtime drift: the listing's ``created_at`` comes
+from the file's mtime, not the encoded filename timestamp. ``cp -p``
+preserves mtime, but plain ``cp`` resets it to "now" — so a copied
+snapshot can show a misleading ``created_at`` while its filename still
+reflects the original UTC capture time.
 """
 
 from __future__ import annotations
 
 import json
+from datetime import datetime, timezone
 from pathlib import Path
 
 from fastapi import APIRouter, Depends, Form, HTTPException, Request
@@ -52,8 +59,6 @@ def _list_all_snapshots(data_dir: Path) -> list[dict]:
     ``last_event_id`` (parsed from the JSON body — small enough that
     listing isn't a performance concern for the handful of files we keep).
     """
-    from datetime import datetime, timezone
-
     rows: list[dict] = []
     for kind in SNAPSHOT_KINDS:
         snap_dir = data_dir / "snapshots" / kind
@@ -85,12 +90,26 @@ def _list_all_snapshots(data_dir: Path) -> list[dict]:
     return rows
 
 
+def _require_kind(kind: str) -> str:
+    """Reject missing/empty/unknown ``kind`` with 400.
+
+    Defaulting silently to ``"periodic"`` made rewind-snapshot lookups
+    appear as 404s, which is confusing — make the client always state
+    the kind explicitly.
+    """
+    if not kind or kind not in SNAPSHOT_KINDS:
+        raise HTTPException(
+            status_code=400,
+            detail=f"kind must be one of {SNAPSHOT_KINDS}",
+        )
+    return kind
+
+
 def _resolve_snapshot_path(
     data_dir: Path, snapshot_id: str, kind: str
 ) -> Path:
     """Map an ``(id, kind)`` pair to the on-disk file, or 404."""
-    if kind not in SNAPSHOT_KINDS:
+    _require_kind(kind)
-        raise HTTPException(status_code=400, detail=f"unknown kind: {kind}")
     path = data_dir / "snapshots" / kind / f"{snapshot_id}.json"
     if not path.exists():
         raise HTTPException(status_code=404, detail="snapshot not found")
@@ -127,7 +146,7 @@ async def snapshots_restore(
     snapshot_id: str,
     request: Request,
     confirm_id: str = Form(""),
-    kind: str = Form("periodic"),
+    kind: str = Form(""),
     conn=Depends(get_conn),
 ):
     """Hard-confirm restore: ``confirm_id`` must equal the path id.
@@ -135,7 +154,11 @@ async def snapshots_restore(
     Mismatched confirm → 400 (without touching the DB). On match, the
     existing :func:`restore_from_snapshot` clears projected tables and
     re-loads them from the dump.
+
+    ``kind`` is required (must be ``"periodic"`` or ``"rewind"``) — a
+    missing or empty value 400s rather than silently defaulting.
     """
+    _require_kind(kind)
     if confirm_id != snapshot_id:
         raise HTTPException(
             status_code=400,
@@ -151,7 +174,7 @@ async def snapshots_restore(
 async def snapshots_preview(
     snapshot_id: str,
     request: Request,
-    kind: str = "periodic",
+    kind: str = "",
     conn=Depends(get_conn),
 ):
     """Show snapshot metadata + a basic delta against the current event log.
@@ -159,7 +182,10 @@ async def snapshots_preview(
     Phase 4 keeps this simple: the snapshot's ``last_event_id`` plus the
     current ``MAX(event_log.id)`` is enough to tell the user how far the
     log has moved on. A richer per-table diff is a Phase 4.5+ concern.
+
+    ``kind`` is required — see :func:`snapshots_restore`.
     """
+    _require_kind(kind)
     settings = request.app.state.settings
     path = _resolve_snapshot_path(settings.data_dir, snapshot_id, kind)
     dump = json.loads(path.read_text())

chat/web/turns.py

@@ -873,6 +873,20 @@ async def post_turn(
     # mid-stream still meant to close the scene — the cancelled bot
     # beat doesn't invalidate that intent. Pinned by
     # test_cancelled_turn_still_closes_scene_when_user_prose_signals_close.
+    #
+    # T108 NOTE — the in-memory append order is correct, but the cancel
+    # path re-raises ``CancelledError`` at the end of ``post_turn``
+    # (see step 11 below). The ``open_db`` dependency teardown skips
+    # ``conn.commit()`` when the consumer raises, which means in
+    # production a genuine cancel currently rolls back ALL post-cancel
+    # writes — including this scene_closed event, the truncated
+    # assistant_turn record, edge updates, and per-POV summaries. The
+    # T74.3 regression test passes only because of a missing
+    # ``import asyncio`` in the test module: the inline mock raises
+    # ``NameError`` instead of ``CancelledError``, which is caught by
+    # the ``except Exception:`` branch and leaves ``cancelled=False``,
+    # so the function returns 204 normally and the commit fires. This
+    # is a transactional bug deferred for triage (T108 report).
     if scene is not None and prose.strip():
         container = None
         if scene.get("container_id") is not None:

tests/test_branches_state.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import logging
+
 from chat.db.connection import open_db
 from chat.db.migrate import apply_migrations
 from chat.eventlog.log import append_event
@@ -139,3 +141,36 @@ def test_list_branches_returns_all(tmp_path):
         names = [b["name"] for b in list_branches(conn)]
         assert "main" in names
         assert "experiment" in names
+
+
+def test_branch_switched_unknown_name_warns(tmp_path, caplog):
+    """Switching to a nonexistent branch logs a warning and leaves no branch active.
+
+    The previous behavior silently cleared is_active flags and applied no UPDATE
+    when the named branch did not exist. T103 makes that condition observable
+    by emitting a warning while preserving the existing (zero-active) outcome.
+    """
+    db = tmp_path / "t.db"
+    apply_migrations(db)
+    with open_db(db) as conn:
+        with caplog.at_level(logging.WARNING, logger="chat.state.branches"):
+            append_event(
+                conn,
+                kind="branch_switched",
+                payload={"name": "does_not_exist"},
+            )
+            project(conn)
+
+        # A warning was emitted naming the missing branch.
+        warnings = [
+            r for r in caplog.records
+            if r.levelno == logging.WARNING and r.name == "chat.state.branches"
+        ]
+        assert warnings, "expected a warning for unknown branch name"
+        assert any("does_not_exist" in r.getMessage() for r in warnings)
+
+        # Existing behavior preserved: no branch is active after the switch.
+        assert active_branch(conn) is None
+
+        # The unknown name was not inserted as a side effect.
+        assert get_branch(conn, "does_not_exist") is None

tests/test_embeddings.py

@@ -20,6 +20,7 @@ The pseudo path doesn't touch the LLMClient, so we pass an empty
 
 from __future__ import annotations
 
+import logging
 import math
 
 import pytest
@@ -89,3 +90,33 @@ async def test_generate_embedding_unit_normalized():
     result = await generate_embedding(_client(), text="some non-empty text")
     norm_sq = sum(x * x for x in result.vector)
     assert math.isclose(norm_sq, 1.0, abs_tol=1e-6)
+
+
+@pytest.mark.asyncio
+async def test_generate_embedding_non_default_model_logs_warning(caplog):
+    """T107: non-default model falls through to fallback and must warn.
+
+    A Phase 4.5+ caller pointing at a real model that isn't yet wired
+    up would otherwise silently degrade (zero vector → useless cosine).
+    The warning surfaces the misconfiguration in logs.
+    """
+    caplog.set_level(logging.WARNING, logger="chat.services.embeddings")
+    result = await generate_embedding(_client(), text="hello", model="real-model")
+
+    # Behavior unchanged: still returns the fallback sentinel.
+    assert result.model == FALLBACK_EMBEDDING_MODEL == "fallback"
+    assert all(x == 0.0 for x in result.vector)
+
+    # Warning fired and names the offending model.
+    warnings = [r for r in caplog.records if r.levelno == logging.WARNING]
+    assert any("non-default model" in r.getMessage() for r in warnings)
+    assert any("real-model" in r.getMessage() for r in warnings)
+
+
+@pytest.mark.asyncio
+async def test_generate_embedding_default_model_does_not_warn(caplog):
+    """T107: the silent default path must stay silent."""
+    caplog.set_level(logging.WARNING, logger="chat.services.embeddings")
+    await generate_embedding(_client(), text="hello")
+    warnings = [r for r in caplog.records if r.levelno == logging.WARNING]
+    assert warnings == []

tests/test_search_ux.py

@@ -16,6 +16,7 @@ Verifies the FastAPI ``/search`` route that wraps T93's
 from __future__ import annotations
 
 from pathlib import Path
+from unittest.mock import patch
 
 import pytest
 from fastapi.testclient import TestClient
@@ -133,3 +134,30 @@ def test_result_links_navigate_to_chat(client, tmp_path):
     # The link target is chat-level (memories don't carry an event_id
     # column today, so we don't deep-link to a specific turn).
     assert 'href="/chats/chat_a"' in resp.text
+
+
+def test_search_results_use_batched_lookups(client, tmp_path):
+    """T106: hydration must not fan out to per-row ``get_bot``/
+    ``get_chat``/``get_scene`` calls.
+
+    The previous implementation called each helper once per result row
+    (worst case 50 rows x 3 helpers = 150 individual queries). The
+    batched implementation collects distinct ids and issues at most one
+    query per entity kind via ``WHERE id IN (...)``, so the per-row
+    helpers should not be invoked at all when there are matches.
+
+    We seed two chats (so both ``get_bot`` and ``get_chat`` would have
+    been hit pre-T106) and assert each helper sees zero per-row calls.
+    """
+    _seed_two_chats_with_memories(tmp_path / "test.db")
+    with (
+        patch("chat.web.search.get_bot") as mock_get_bot,
+        patch("chat.web.search.get_chat") as mock_get_chat,
+        patch("chat.web.search.get_scene") as mock_get_scene,
+    ):
+        resp = client.get("/search?q=rabbit")
+    assert resp.status_code == 200
+    # Batched IN-list queries replace the per-row helpers entirely.
+    assert mock_get_bot.call_count == 0
+    assert mock_get_chat.call_count == 0
+    assert mock_get_scene.call_count == 0

tests/test_snapshot_ux.py

@@ -156,6 +156,28 @@ def test_restore_snapshot_wrong_confirm_400(client, tmp_path):
     assert response.status_code == 400
 
 
+def test_restore_without_kind_returns_400(client, tmp_path):
+    """T105: Missing or empty ``kind`` must be rejected with 400.
+
+    Previously ``kind`` defaulted to ``"periodic"``, which silently 404'd
+    when the caller meant a rewind snapshot. Tighten the contract so the
+    client must always pass an explicit, valid ``kind``.
+    """
+    db_path = tmp_path / "test.db"
+    _seed_bot(db_path, "bot_a", "BotA")
+    snapshot_path = _take_snapshot_via_service(
+        db_path, tmp_path, kind="periodic"
+    )
+    snapshot_id = snapshot_path.stem
+
+    response = client.post(
+        f"/snapshots/restore/{snapshot_id}",
+        data={"confirm_id": snapshot_id},  # no `kind`
+        follow_redirects=False,
+    )
+    assert response.status_code == 400
+
+
 def test_preview_renders_metadata(client, tmp_path):
     db_path = tmp_path / "test.db"
     _seed_bot(db_path, "bot_a", "BotA")

tests/test_turn_flow.py

@@ -734,6 +734,19 @@ def test_cancelled_turn_still_closes_scene_when_user_prose_signals_close(
     that as an exception, so we drive the request inside ``with
     pytest.raises``. Despite the exception, the scene_closed event
     must land in the event_log.
+
+    T108 NOTE — this test does NOT actually exercise the cancel path.
+    ``_CancelOnStreamMock.stream`` writes ``raise asyncio.CancelledError``
+    but ``asyncio`` is not imported at module scope, so the first
+    iteration raises ``NameError`` (caught by ``except Exception:`` in
+    post_turn, which sets ``primary_truncated=True`` but leaves
+    ``cancelled=False``). The function therefore returns 204 normally,
+    the dependency-managed connection commits, and ``scene_closed``
+    lands. Importing asyncio so the real CancelledError fires reveals
+    a transactional bug: ``post_turn``'s end-of-function re-raise
+    causes ``open_db``'s dependency teardown to skip ``conn.commit()``,
+    rolling back ALL post-cancel writes (user_turn, assistant_turn,
+    edge_updates, scene_closed). Deferred for triage — see T108 report.
     """
     from typing import AsyncIterator, Sequence
 
@@ -828,12 +841,33 @@ def test_cancelled_turn_still_closes_scene_when_user_prose_signals_close(
             "SELECT payload_json FROM event_log "
             "WHERE kind = 'assistant_turn' ORDER BY id"
         ).fetchall()
+        # T108: pin the ordering — user_turn must commit before
+        # scene_closed (close detection runs on prose that is already
+        # in the event_log) and any assistant_turn the cancel produced
+        # must come last (truncated record written after both).
+        ordered = conn.execute(
+            "SELECT id, kind FROM event_log "
+            "WHERE kind IN ('user_turn', 'scene_closed', 'assistant_turn') "
+            "ORDER BY id"
+        ).fetchall()
 
     # Scene close lands despite the cancel.
     assert scene_close_count == 1
     # The cancelled assistant_turn was still recorded (truncated=True).
     assert len(assistant_payload) == 1
     assert json.loads(assistant_payload[0][0])["truncated"] is True
+    # T108 ordering pin: user_turn lands first, the truncated
+    # assistant_turn (if any) is committed BEFORE the scene_close
+    # decision fires, and scene_closed lands last. Close detection
+    # relies on user prose being committed to the event_log BEFORE
+    # the close decision runs — and the cancelled assistant beat is
+    # recorded as a partial before close-detection too.
+    kinds_in_order = [row[1] for row in ordered]
+    user_idx = kinds_in_order.index("user_turn")
+    close_idx = kinds_in_order.index("scene_closed")
+    assert user_idx < close_idx
+    if "assistant_turn" in kinds_in_order:
+        assert user_idx < kinds_in_order.index("assistant_turn") < close_idx
 
 
 def test_interjection_enqueues_significance_job(app_state_setup, tmp_path):