feat(kb): edit + re-embed document summaries from the viewer (admin)

2026-08-31 23:52:22 -04:00
parent d94f3d5a52
commit 140b97ebf3
8 changed files with 1540 additions and 9 deletions
@@ -0,0 +1,430 @@
+"""Phase 57 E2E (Playwright): edit the AI-generated summary in the
+viewer — and watch it get re-embedded.
+
+TODO.md L4: "Be able to edit the summaries for documents in the RAG.
+Click an edit button in summary box and change the summary that the AI
+created. re-embed that document after changing the summary."
+Run in isolation (DB must be up: ``podman compose up -d db``):
+
+    uv run pytest tests/e2e/test_edit_summaries.py -v --no-cov
+
+The fixture KB is a story-dedicated directory
+(``tests/fixtures/summary_edit_kb/`` — the shared ``tests/fixtures/docs/``
+and the phase-30 ``summary_kb/`` stay pinned at their files) with ONE
+non-markdown A9 document:
+
+* ``quadlet/llamacpp.container`` — a podman quadlet unit (the
+  ``container`` extension is in the A9 default family, so the DEFAULT
+  import scope walks it — no ``BOR_IMPORT_EXTENSIONS`` override). At
+  import the mock ``lite`` model (``SUMMARY_MODE`` marker,
+  ``tests/e2e/mock_llm.py``) reduces it to the deterministic 24-token
+  digest — the first 24 tokens of the file, the header comment line —
+  stored on ``documents.summary`` and indexed as one ``is_summary``
+  chunk. The rest of the file is deliberately token-diluted
+  (config keys the digest never contains), and the sentinel
+  ``RESE-EDIT-SUMMARY-SENTINEL-b41d`` sits on the document's LAST line —
+  outside the 24-token digest — so the raw ``<pre>`` content is
+  distinguishable from the stored summary (the digest/sentinel mechanic
+  of ``tests/e2e/test_document_summaries.py``).
+
+DB isolation: the fixture's source name (``summary_edit_kb``) is
+distinctive — the suite never asserts on absolute row counts and
+deletes the rows it creates in a ``finally`` (other suites' documents
+stay untouched in the shared E2E database).
+
+The admin edits through the REAL browser flow (form login → the
+viewer's Edit button → the inline editor → Save); the re-embed itself
+is then verified against the live database (the chunk's NEW content, a
+FRESH non-NULL vector, the content chunks byte-for-byte untouched — the
+D4 re-embed scope) and against the public content endpoint (no cookie —
+the viewer stays public, phase 16).
+"""
+from __future__ import annotations
+
+import asyncio
+from collections.abc import Iterator
+from pathlib import Path
+from threading import Thread
+from typing import Any
+
+import httpx
+import pytest
+from playwright.sync_api import Page, expect
+from sqlalchemy import select
+
+from app.config import Settings
+from app.db import SessionLocal
+from app.models import Chunk, Document
+from app.rag.importer import ImportSummary, import_sources
+from app.rag.llm import LLMClient
+from e2e.auth_helpers import login
+from e2e.mock_llm import TOKEN_RE
+
+REPO = Path(__file__).resolve().parents[2]
+FIXTURES = REPO / "tests" / "fixtures" / "summary_edit_kb"
+SOURCE = FIXTURES.name  # "summary_edit_kb" — distinctive, never asserted by count
+DOC_PATH = "quadlet/llamacpp.container"
+#: Encoded viewer URL query value (slash → %2F, same as the chips /
+#: Sources table links build it).
+DOC_URL_PATH = "quadlet%2Fllamacpp.container"
+SENTINEL = "RESE-EDIT-SUMMARY-SENTINEL-b41d"
+
+#: The hand-edited summary (test 1) — a distinctive sentence no part of
+#: the fixture or its digest contains, so the round-trip assertion can
+#: never pass against the old text.
+NEW_SUMMARY = (
+    "Hand-edited: the quadlet unit serves the homelab's local model on "
+    "port 8081. (RESE-HANDEDIT-77ab)"
+)
+#: The modal-surface test's own edit (test 4) — likewise distinctive.
+MODAL_SUMMARY = (
+    "Edited from the modal: quadlet unit for the local inference server. "
+    "(RESE-MODAL-EDIT-3cd9)"
+)
+
+
+# --- Importer + thread helpers (test_document_summaries.py pattern) ---
+
+
+async def _import_fixtures(mock_port: int) -> ImportSummary:
+    kwargs: dict[str, Any] = {
+        "_env_file": None,
+        "llm_base_url": f"http://127.0.0.1:{mock_port}/v1",
+    }
+    settings = Settings(**kwargs)  # pyright: ignore[reportCallIssue]
+    return await import_sources([FIXTURES], LLMClient(settings))
+
+
+def _run_in_thread(coro: Any) -> Any:
+    """Run a coroutine on a worker thread.
+
+    Playwright's sync API keeps an asyncio loop running on the test thread,
+    so ``asyncio.run`` cannot be called directly from a test body.
+    """
+    box: dict[str, Any] = {}
+
+    def runner() -> None:
+        try:
+            box["value"] = asyncio.run(coro)
+        except BaseException as e:  # noqa: BLE001 — re-raised on the test thread
+            box["error"] = e
+
+    t = Thread(target=runner)
+    t.start()
+    t.join()
+    if "error" in box:
+        raise box["error"]
+    return box["value"]
+
+
+def _delete_source_rows() -> None:
+    """Delete every row of this suite's distinctive source (chunks
+    cascade with the document rows)."""
+    with SessionLocal() as db:
+        for doc in db.scalars(select(Document).where(Document.source == SOURCE)).all():
+            db.delete(doc)
+        db.commit()
+
+
+@pytest.fixture(autouse=True)
+def seeded_kb(mock_llm: int, db_ready: None) -> Iterator[ImportSummary]:
+    """Seed the story-dedicated fixture for one test (the DEFAULT import
+    scope — ``container`` is A9) and delete every row it creates
+    afterwards (DB isolation — see the module docstring)."""
+    _delete_source_rows()  # idempotent: leftovers from a crashed run
+    summary = _run_in_thread(_import_fixtures(mock_llm))
+    assert summary.formats == {"container": 1}
+    assert summary.added == 1
+    assert summary.summaries == 1 and summary.summary_errors == 0
+    assert summary.errors == 0
+    try:
+        yield summary
+    finally:
+        _delete_source_rows()
+
+
+def _expected_summary(content: str, source: str, path: str) -> str:
+    """The mock lite model's byte-stable digest + the code pointer line.
+
+    Mirrors ``mock_llm.compose_answer``'s ``SUMMARY_MODE`` branch (first
+    24 tokens of the document content) plus the summarizer's
+    deterministic ``Source:`` line — no model output is ever trusted.
+    """
+    digest = " ".join(TOKEN_RE.findall(content.lower())[:24])
+    return f"This document covers {digest}.\nSource: {source}/{path}"
+
+
+def _chunk_state() -> dict[str, Any]:
+    """The fixture document's DB state, read back through a fresh session.
+
+    ``total`` — the document's chunk count (never a table-wide count —
+    DB isolation); ``summary`` — ``documents.summary``; ``summary_*`` —
+    the single ``is_summary`` chunk (content + vector read-back);
+    ``raw_contents`` — the content chunks' text, sorted (the D4 pin:
+    the content chunks are byte-for-byte untouched by a summary edit).
+    """
+    with SessionLocal() as db:
+        doc = db.scalar(
+            select(Document).where(Document.source == SOURCE, Document.path == DOC_PATH)
+        )
+        assert doc is not None, "fixture doc was not imported"
+        chunks = db.scalars(select(Chunk).where(Chunk.document_id == doc.id)).all()
+    summary_chunks = [c for c in chunks if c.is_summary]
+    assert len(summary_chunks) <= 1, f"more than one is_summary chunk: {len(summary_chunks)}"
+    sc = summary_chunks[0] if summary_chunks else None
+    return {
+        "total": len(chunks),
+        "summary": doc.summary,
+        "summary_count": len(summary_chunks),
+        "summary_content": sc.content if sc else None,
+        "summary_vec": list(sc.embedding) if sc and sc.embedding is not None else None,
+        "raw_contents": sorted(c.content for c in chunks if not c.is_summary),
+    }
+
+
+def _api_summary(app_url: str) -> str | None:
+    """The public content endpoint's ``summary`` — NO cookie (the viewer
+    stays public, phase 16; a fresh httpx client carries no session)."""
+    r = httpx.get(
+        f"{app_url}/api/documents/content",
+        params={"source": SOURCE, "path": DOC_PATH},
+        timeout=10,
+    )
+    assert r.status_code == 200, r.text
+    return r.json()["summary"]
+
+
+def _doc_url(app_url: str) -> str:
+    return f"{app_url}/document.html?source={SOURCE}&path={DOC_URL_PATH}"
+
+
+# ---------------------------------------------------------------------------
+# 1. Admin: edit → Save → the panel, the API, and the DB all agree —
+#    and the is_summary chunk carries a FRESH embedding (D4 re-embed)
+# ---------------------------------------------------------------------------
+
+
+def test_admin_edits_summary(page: Page, app_url: str) -> None:
+    page.set_default_timeout(30_000)
+    content = (FIXTURES / DOC_PATH).read_text(encoding="utf-8")
+    assert SENTINEL in content.splitlines()[-1]  # last line, by design
+    expected = _expected_summary(content, SOURCE, DOC_PATH)
+    digest_line, pointer_line = expected.split("\n", 1)
+
+    before = _chunk_state()
+    assert before["summary"] == expected  # the mock digest is the stored summary
+    assert before["summary_count"] == 1
+    assert before["summary_vec"] is not None
+
+    login(page, app_url)
+    page.goto(_doc_url(app_url))
+
+    # The .doc-summary panel shows the digest (digest + pointer lines) —
+    # and nothing from the diluted raw body ("PublishPort" is deeper in
+    # the file, outside the 24-token digest).
+    panel = page.locator(".doc-summary")
+    expect(panel).to_have_count(1)
+    expect(panel).to_be_visible()
+    expect(panel.locator(".doc-summary-title")).to_have_text("Summary")
+    expect(panel).to_contain_text(digest_line)
+    expect(panel).to_contain_text(pointer_line)
+    expect(panel).not_to_contain_text("PublishPort")
+    # The original still renders below, sentinel and all.
+    expect(page.locator("#doc-content pre.doc-raw")).to_contain_text(SENTINEL)
+
+    # The admin-only Edit button (phase 57, D4 — the viewer itself is
+    # public; only the affordance is gated).
+    edit = panel.locator(".doc-summary-edit")
+    expect(edit).to_be_visible()
+    expect(edit).to_have_text("Edit")
+
+    # Edit → inline editor: textarea PREFILLED with the current summary,
+    # Save / Cancel, and the role=status live region.
+    edit.click()
+    editor = page.locator(".doc-summary-editor")
+    expect(editor).to_be_visible()
+    expect(editor).to_have_value(expected)
+    expect(page.locator(".doc-summary-save")).to_be_visible()
+    expect(page.locator(".doc-summary-cancel")).to_be_visible()
+    status = page.locator(".doc-summary-status")
+    expect(status).to_have_attribute("role", "status")
+    expect(status).to_have_attribute("aria-live", "polite")
+
+    # Replace the text with the distinctive hand-edit and Save.
+    page.fill(".doc-summary-editor", NEW_SUMMARY)
+    page.click(".doc-summary-save")
+
+    # Live-region confirmation, and the panel text is the NEW summary
+    # (re-rendered through the textContent contract — the digest is gone).
+    expect(status).to_have_text("Summary updated.")
+    expect(panel.locator(".doc-summary-text")).to_have_text(NEW_SUMMARY)
+    expect(panel).not_to_contain_text(digest_line)
+
+    # Public read (no cookie): the content endpoint serves the new text.
+    assert _api_summary(app_url) == NEW_SUMMARY
+
+    # The re-embed, verified in the DB (D4): the is_summary chunk's
+    # content is the new text with a FRESH non-NULL vector; the total
+    # chunk count is unchanged and the content chunks are byte-for-byte
+    # untouched — only the summary changed, so only it was re-embedded.
+    after = _chunk_state()
+    assert after["total"] == before["total"]  # count unchanged by an update
+    assert after["summary_count"] == 1
+    assert after["summary"] == NEW_SUMMARY
+    assert after["summary_content"] == NEW_SUMMARY
+    assert after["summary_vec"] is not None  # re-embedded, non-NULL
+    assert after["summary_vec"] != before["summary_vec"]  # a FRESH vector
+    assert after["raw_contents"] == before["raw_contents"]  # content chunks untouched
+
+
+# ---------------------------------------------------------------------------
+# 2. Admin: an empty save CLEARS — the panel disappears, summary NULL,
+#    the is_summary chunk row is gone (count −1)
+# ---------------------------------------------------------------------------
+
+
+def test_admin_clears_summary(page: Page, app_url: str) -> None:
+    page.set_default_timeout(30_000)
+    content = (FIXTURES / DOC_PATH).read_text(encoding="utf-8")
+    expected = _expected_summary(content, SOURCE, DOC_PATH)
+
+    before = _chunk_state()
+    assert before["summary"] == expected
+    assert before["summary_count"] == 1
+
+    login(page, app_url)
+    page.goto(_doc_url(app_url))
+    panel = page.locator(".doc-summary")
+    expect(panel).to_have_count(1)
+    expect(panel.locator(".doc-summary-edit")).to_be_visible()
+
+    # Select-all + delete — clear the prefilled editor — then Save.
+    panel.locator(".doc-summary-edit").click()
+    page.fill(".doc-summary-editor", "")
+    page.click(".doc-summary-save")
+
+    # "Summary cleared." in the live region, then the panel leaves the
+    # DOM (the renderer only draws it for non-empty summaries — the
+    # removal lands a short beat after the confirmation). The original
+    # content below is untouched.
+    expect(page.locator(".doc-summary-status")).to_have_text("Summary cleared.")
+    expect(page.locator(".doc-summary")).to_have_count(0, timeout=8_000)
+    expect(page.locator("#doc-content pre.doc-raw")).to_contain_text(SENTINEL)
+
+    # The public endpoint now reports no summary…
+    assert _api_summary(app_url) is None
+
+    # …and the DB agrees: summary NULL, the is_summary row deleted
+    # (count −1), the content chunks byte-for-byte untouched.
+    after = _chunk_state()
+    assert after["total"] == before["total"] - 1  # the is_summary chunk is gone
+    assert after["summary"] is None
+    assert after["summary_count"] == 0
+    assert after["summary_vec"] is None
+    assert after["raw_contents"] == before["raw_contents"]
+
+
+# ---------------------------------------------------------------------------
+# 3. Anonymous: the digest renders, but no Edit button — and the
+#    endpoint is 403 (the public viewer is byte-for-byte phase 36)
+# ---------------------------------------------------------------------------
+
+
+def test_anonymous_cannot(page: Page, app_url: str) -> None:
+    page.set_default_timeout(30_000)
+    content = (FIXTURES / DOC_PATH).read_text(encoding="utf-8")
+    expected = _expected_summary(content, SOURCE, DOC_PATH)
+    digest_line, _ = expected.split("\n", 1)
+
+    # Fresh context (the function-scoped page fixture — no login): the
+    # panel renders the digest, but the edit affordance is ABSENT — no
+    # button, no header row, no editor wiring. The section keeps the
+    # phase-36 byte-for-byte shape: a bare h2 + the text-node <p>.
+    page.goto(_doc_url(app_url))
+    panel = page.locator(".doc-summary")
+    expect(panel).to_have_count(1)
+    expect(panel).to_be_visible()
+    expect(panel).to_contain_text(digest_line)
+    expect(page.locator(".doc-summary-edit")).to_have_count(0)
+    expect(page.locator(".doc-summary-head")).to_have_count(0)
+    expect(page.locator(".doc-summary-editor")).to_have_count(0)
+    children = page.evaluate(
+        "() => [...document.querySelector('.doc-summary').children]"
+        ".map((el) => el.className)"
+    )
+    assert children == ["doc-summary-title", "doc-summary-text"], (
+        f"anonymous panel drifted from the phase-36 shape: {children}"
+    )
+
+    # The endpoint is admin-gated (D4): an anonymous PATCH → 403
+    # "admin only" (a fresh httpx client carries no session), and the
+    # stored summary is untouched.
+    r = httpx.patch(
+        f"{app_url}/api/documents/summary",
+        json={"source": SOURCE, "path": DOC_PATH, "summary": "not allowed"},
+        timeout=10,
+    )
+    assert r.status_code == 403
+    assert r.json() == {"detail": "admin only"}
+    assert _chunk_state()["summary"] == expected  # untouched
+
+
+# ---------------------------------------------------------------------------
+# 4. Modal surface: the SAME shared renderer — the Sources-page modal
+#    carries the Edit button too, and a save from it round-trips
+# ---------------------------------------------------------------------------
+
+
+def test_admin_modal_surface_edit(page: Page, app_url: str) -> None:
+    """One core, two surfaces (phase 26/36): the document modal from the
+    Sources table renders through the SAME ``renderDocument`` — so the
+    admin gets the Edit button in the modal as well, and a save from
+    there hits the same endpoint + DB row (the page test is unchanged
+    in shape; this pins the second surface)."""
+    page.set_default_timeout(30_000)
+    content = (FIXTURES / DOC_PATH).read_text(encoding="utf-8")
+    expected = _expected_summary(content, SOURCE, DOC_PATH)
+    digest_line, _ = expected.split("\n", 1)
+
+    before = _chunk_state()
+    assert before["summary"] == expected
+
+    login(page, app_url)  # lands on /sources.html (the catalog is admin-only)
+    row = page.locator("#docs-tbody tr", has_text=DOC_PATH)
+    expect(row).to_have_count(1)
+    before_tabs = len(page.context.pages)
+    row.locator("td:nth-child(2) a.doc-link").click()
+    assert len(page.context.pages) == before_tabs, "row link must not open a new tab"
+
+    # The modal shows the panel with the digest + the admin Edit button.
+    expect(page.locator(".doc-modal")).to_be_visible()
+    expect(page.locator("#doc-modal-title")).to_have_text("llamacpp")
+    modal_panel = page.locator("#doc-modal .doc-summary")
+    expect(modal_panel).to_have_count(1)
+    expect(modal_panel).to_be_visible()
+    expect(modal_panel).to_contain_text(digest_line)
+    edit = modal_panel.locator(".doc-summary-edit")
+    expect(edit).to_be_visible()
+
+    # Edit → prefilled editor → replace → Save → the modal's panel
+    # reflects the new text and the live region confirms.
+    edit.click()
+    editor = page.locator("#doc-modal .doc-summary-editor")
+    expect(editor).to_be_visible()
+    expect(editor).to_have_value(expected)
+    page.fill("#doc-modal .doc-summary-editor", MODAL_SUMMARY)
+    page.click("#doc-modal .doc-summary-save")
+    expect(page.locator("#doc-modal .doc-summary-status")).to_have_text("Summary updated.")
+    expect(modal_panel.locator(".doc-summary-text")).to_have_text(MODAL_SUMMARY)
+
+    # Same endpoint, same DB row: the public read and the chunk agree —
+    # count unchanged, fresh vector, content chunks untouched.
+    assert _api_summary(app_url) == MODAL_SUMMARY
+    after = _chunk_state()
+    assert after["total"] == before["total"]
+    assert after["summary_count"] == 1
+    assert after["summary"] == MODAL_SUMMARY
+    assert after["summary_content"] == MODAL_SUMMARY
+    assert after["summary_vec"] is not None
+    assert after["summary_vec"] != before["summary_vec"]
+    assert after["raw_contents"] == before["raw_contents"]