fix(server): stats endpoint 500s in sync-only mode (AI disabled)

server/quire_server/api/library.py

@@ -678,6 +678,7 @@ async def sync_library(
 async def get_stats(
     user_id: Annotated[str, Depends(current_user_id)],
     session: Annotated[AsyncSession, Depends(get_session)],
+    settings: Annotated[Settings, Depends(get_settings)],
 ) -> LibraryStatsResponse:
     # 1. total_books: alive library items for this user.
     total_books = (
@@ -801,63 +802,77 @@ async def get_stats(
     # 4. top_themes: pick-one-insight-per-book CTE, then aggregate themes.
     #    See the block comment at the top of this function for the full
     #    rationale.
-    pick_priority = case(
-        (
-            and_(
-                BookInsight.metadata_id.is_not(None),
-                BookInsight.metadata_id == LibraryItem.metadata_id,
+    #
+    #    Issue #87: both `book_insights` (the pick-one CTE below) and
+    #    `book_themes` live on the `ai` alembic branch, which the deploy
+    #    migrator only applies when QUIRE_SERVER_AI_ENABLED=true. The library
+    #    router, however, mounts on `progress_enabled` (see main.py), so a
+    #    sync-only deployment reaches this endpoint without those tables
+    #    existing — producing `relation "book_themes" does not exist` and a
+    #    blanket 500. Gate the WHOLE block (not just the BookTheme join) on
+    #    `settings.ai_enabled`: skipping only the final join still leaves the
+    #    `picked` CTE referencing the absent `book_insights`. Themes are an
+    #    AI-only feature, so an empty list is the correct sync-only answer.
+    top_themes: list[TopTheme] = []
+    if settings.ai_enabled:
+        pick_priority = case(
+            (
+                and_(
+                    BookInsight.metadata_id.is_not(None),
+                    BookInsight.metadata_id == LibraryItem.metadata_id,
+                ),
+                0,
             ),
-            0,
-        ),
-        else_=1,
-    )
-
-    picked = (
-        select(
-            LibraryItem.pk.label("library_item_pk"),
-            BookInsight.id.label("book_insight_id"),
+            else_=1,
         )
-        .select_from(LibraryItem)
-        .join(
-            BookInsight,
-            and_(
-                BookInsight.superseded_at.is_(None),  # filter 1
-                (
+
+        picked = (
+            select(
+                LibraryItem.pk.label("library_item_pk"),
+                BookInsight.id.label("book_insight_id"),
+            )
+            .select_from(LibraryItem)
+            .join(
+                BookInsight,
+                and_(
+                    BookInsight.superseded_at.is_(None),  # filter 1
                     (
-                        BookInsight.metadata_id.is_not(None)
-                        & (BookInsight.metadata_id == LibraryItem.metadata_id)
-                    )
-                    | (BookInsight.content_hash == LibraryItem.content_hash)
+                        (
+                            BookInsight.metadata_id.is_not(None)
+                            & (BookInsight.metadata_id == LibraryItem.metadata_id)
+                        )
+                        | (BookInsight.content_hash == LibraryItem.content_hash)
+                    ),
                 ),
-            ),
-        )
-        .where(
-            LibraryItem.user_id == user_id,
-            LibraryItem.deleted_at.is_(None),
+            )
+            .where(
+                LibraryItem.user_id == user_id,
+                LibraryItem.deleted_at.is_(None),
+            )
+            .order_by(LibraryItem.pk, pick_priority, BookInsight.generated_at.desc())
+            # PostgreSQL DISTINCT ON via SQLAlchemy: keep one row per
+            # library_item_pk, picking the lowest priority (metadata match)
+            # and most recent generated_at via the trailing ORDER BY.
+            .distinct(LibraryItem.pk)
+            .subquery("picked_insight")
         )
-        .order_by(LibraryItem.pk, pick_priority, BookInsight.generated_at.desc())
-        # PostgreSQL DISTINCT ON via SQLAlchemy: keep one row per
-        # library_item_pk, picking the lowest priority (metadata match)
-        # and most recent generated_at via the trailing ORDER BY.
-        .distinct(LibraryItem.pk)
-        .subquery("picked_insight")
-    )
 
-    theme_count = func.count(func.distinct(picked.c.library_item_pk))
-    theme_rows = (
-        await session.execute(
-            select(BookTheme.theme.label("theme"), theme_count.label("c"))
-            .select_from(picked)
-            .join(BookTheme, BookTheme.book_insight_id == picked.c.book_insight_id)
-            .where(BookTheme.confidence >= 1.0)  # filter 2
-            .group_by(BookTheme.theme)
-            .order_by(theme_count.desc(), BookTheme.theme.asc())
-            .limit(5)
-        )
-    ).all()
-    top_themes = [
-        TopTheme(theme=row.theme, count=int(row.c), note="v3+ insights only") for row in theme_rows
-    ]
+        theme_count = func.count(func.distinct(picked.c.library_item_pk))
+        theme_rows = (
+            await session.execute(
+                select(BookTheme.theme.label("theme"), theme_count.label("c"))
+                .select_from(picked)
+                .join(BookTheme, BookTheme.book_insight_id == picked.c.book_insight_id)
+                .where(BookTheme.confidence >= 1.0)  # filter 2
+                .group_by(BookTheme.theme)
+                .order_by(theme_count.desc(), BookTheme.theme.asc())
+                .limit(5)
+            )
+        ).all()
+        top_themes = [
+            TopTheme(theme=row.theme, count=int(row.c), note="v3+ insights only")
+            for row in theme_rows
+        ]
 
     # NOTE (Lock #12): /library/v1/stats does NOT include a fingerprint. The
     # AI profile envelope owns the input_fingerprint contract; stats uses a

server/tests/integration/test_stats_sync_only.py

@@ -0,0 +1,150 @@
+"""Regression test for issue #87 — stats endpoint in a sync-only deployment.
+
+A sync-only server runs with ``QUIRE_SERVER_AI_ENABLED=false``, so the deploy
+migrator never applies the ``ai`` alembic branch and ``book_insights`` /
+``book_themes`` do not exist. The library router, however, mounts on
+``progress_enabled`` (see ``main.py``), so ``GET /library/v1/stats`` is reachable
+in that mode. Before the fix, ``get_stats`` unconditionally joined those AI-only
+tables and Postgres raised ``relation "book_themes" does not exist`` → a blanket
+HTTP 500. The endpoint must instead return 200 with ``top_themes=[]`` while still
+reporting the non-AI stats.
+
+This module deliberately provisions its OWN Postgres migrated with
+``ai_enabled=False``. The shared session-scoped fixtures in ``conftest.py`` always
+apply the ``ai`` branch (``run_migrations(..., ai_enabled=True)``), which is
+exactly why the whole existing suite never reproduced this bug.
+"""
+
+from __future__ import annotations
+
+import base64
+import uuid
+from collections.abc import Iterator
+from datetime import UTC, datetime
+
+import httpx
+import pytest
+from alembic.config import Config as AlembicConfig
+from httpx import ASGITransport, AsyncClient
+from sqlalchemy import text
+from sqlalchemy.ext.asyncio import create_async_engine
+from testcontainers.postgres import PostgresContainer
+
+
+def _basic(user: str, pw: str) -> dict[str, str]:
+    token = base64.b64encode(f"{user}:{pw}".encode()).decode("ascii")
+    return {"Authorization": f"Basic {token}"}
+
+
+def _put_body(**overrides) -> dict:
+    base = {
+        "metadata_id": "md-1",
+        "content_hash": "ch-1",
+        "title": "Ancillary Justice",
+        "authors": ["Ann Leckie"],
+        "series_name": None,
+        "series_index": None,
+        "isbn": None,
+        "language": None,
+        "subjects": [],
+        "opds_href": None,
+    }
+    base.update(overrides)
+    return {"item": base}
+
+
+def _progress_body(content_hash: str, percent: float, finished: bool, when: str) -> dict:
+    return {
+        "items": [
+            {
+                "document": {"metadata_id": None, "content_hash": content_hash},
+                "locator": "{}",
+                "percent": percent,
+                "client_updated_at": when,
+                "finished_at": when if finished else None,
+            }
+        ]
+    }
+
+
+@pytest.fixture(scope="module")
+def sync_only_db() -> Iterator[str]:
+    """A fresh Postgres migrated WITHOUT the ``ai`` branch (sync-only deploy)."""
+    with PostgresContainer("postgres:16-alpine") as pg:
+        sync_url = pg.get_connection_url()
+        async_url = sync_url.replace("postgresql+psycopg2://", "postgresql+asyncpg://")
+        cfg = AlembicConfig("alembic.ini")
+        cfg.set_main_option("sqlalchemy.url", async_url)
+        from scripts.migrate import run_migrations
+
+        # progress on (library router + library_items table), ai OFF: this is
+        # the deployment shape that triggers issue #87.
+        run_migrations(cfg, progress_enabled=True, ai_enabled=False)
+        yield async_url
+
+
+@pytest.fixture
+def sync_only_app(sync_only_db, monkeypatch, cwa_transport):
+    """A sync-only FastAPI app (AI disabled) wired to the ai-branch-less DB."""
+    monkeypatch.setenv("QUIRE_SERVER_DATABASE_URL", sync_only_db)
+    monkeypatch.setenv("QUIRE_SERVER_CWA_BASE_URL", "http://test-cwa")
+    monkeypatch.setenv("QUIRE_SERVER_PROGRESS_ENABLED", "true")
+    monkeypatch.setenv("QUIRE_SERVER_AI_ENABLED", "false")
+
+    from quire_server.config import get_settings
+
+    get_settings.cache_clear()
+
+    from quire_server.core.auth import CalibreAuthValidator
+    from quire_server.core.auth_backend import CalibreWebBasicAuth
+    from quire_server.main import create_app
+
+    app = create_app()
+    test_client = httpx.AsyncClient(
+        transport=cwa_transport, base_url="http://test-cwa", timeout=3.0
+    )
+    app.state.httpx_client = test_client
+    app.state.auth_validator = CalibreAuthValidator(
+        client=test_client,
+        cwa_base_url="http://test-cwa",
+    )
+    app.state.auth_backend = CalibreWebBasicAuth(app.state.auth_validator)
+    yield app
+    # Don't leave the lru-cached sync-only settings (pointing at a container
+    # that is about to be torn down) visible to later modules.
+    get_settings.cache_clear()
+
+
+async def test_stats_returns_200_without_ai_tables(sync_only_app, sync_only_db, cwa_users):
+    """Issue #87: /library/v1/stats must not 500 when the ai branch is absent."""
+    # Guard the premise: the ai-only tables genuinely do not exist here. If a
+    # future change to the fixture applied the ai branch, this would mask the
+    # regression, so fail loudly instead.
+    engine = create_async_engine(sync_only_db, future=True)
+    try:
+        async with engine.connect() as conn:
+            reg = await conn.scalar(text("SELECT to_regclass('public.book_themes')"))
+    finally:
+        await engine.dispose()
+    assert reg is None, "premise broken: the ai branch was applied to the sync-only DB"
+
+    user = f"user-{uuid.uuid4().hex[:8]}"
+    cwa_users[user] = "pw"
+    headers = _basic(user, "pw")
+
+    transport = ASGITransport(app=sync_only_app)
+    async with AsyncClient(transport=transport, base_url="http://test") as c:
+        # Seed a book + finished progress so the NON-AI stats compute real
+        # values — proving the fix skips only the theme block, not the rest.
+        await c.put("/library/v1/items", json=_put_body(content_hash="a"), headers=headers)
+        now = datetime.now(UTC).isoformat()
+        await c.post("/sync/v1/progress", json=_progress_body("a", 1.0, True, now), headers=headers)
+        r = await c.get("/library/v1/stats", headers=headers)
+
+    assert r.status_code == 200, r.text
+    data = r.json()
+    assert data["total_books"] == 1
+    assert data["finished_count"] == 1
+    assert data["top_authors"] == [{"name": "Ann Leckie", "count": 1}]
+    # The load-bearing assertion: themes are gated off, not queried.
+    assert data["top_themes"] == []