phase: 97_kb_tree_catalog

All completion criteria verified — everything is green, no defects found. Final report: ## Phase 97 final verification pass — ALL GREEN **Verified (no code changes needed):** - `GET /api/docs/tree` (admin), `build_kb_tree` pure builder, `PATCH /api/folders/summary`, migration 0018 (`manually_edited`, head confirmed), generator skip/keep + `kept_manual` stat, RAG tree UI + edit affordance in `sources.js`/`index.html`/`styles.css` - `tests/e2e/test_kb_tree.py`: 8 passed — top level, drill source/folder, edit round-trip, clear, manual-desc-survives-sync, reload fallback, anonymous gate - Integration: tree shape/order/403/empty/indexed-only + PATCH update/create/root/clear/404/403/no-LLM + stat-walk equivalence (in `test_docs_api.py`); 3-field `folder_summaries=` import token preserved **Gates (exact commands):** - `uv run pytest --cov=app --cov-report=term-missing` → **2053 passed**, TOTAL coverage **99%** (>90% ✓) - `uv run ruff check . && uv run pyright` → **All checks passed / 0 errors** - `uv run pytest tests/e2e/test_kb_tree.py -v --no-cov` → **8 passed** in isolation - 30 story/RAG-view E2E suites run **one per process**: all passed, incl. `test_ls_tree_drilldown` (agent `ls` byte-identical ✓), `test_import_documents`, `test_edit_summaries`, `test_admin_auth`, `test_kb_overview` **Completion criteria:** tree view ✓ · edit round-trip + clear ✓ · manual persists/clear resets ✓ · `ls` unchanged ✓ · pytest/coverage/lint ✓ · E2E isolation ✓ · commit — left to harness per protocol (working tree untouched, `git add/commit` not run) **Deviations:** none. **Next pending phase:** none — `todo/` contains only 97 (96 already committed).
2026-09-11 22:48:02 -04:00
parent a49be80b8e
commit ad7585d474
81 changed files with 6299 additions and 211 deletions
@@ -11,14 +11,33 @@ fixture, not the subject of the tests.

 Phase 16 adaptation: the Sources catalog is admin-only — every test
 performs the real form login (``e2e.auth_helpers.login``) first.
+
+Phase 97 adaptation: the catalog is the DRILL-DOWN TREE the agent's
+``ls`` walks — the top level lists the sources (this fixture's single
+indexed-only source, ``docs``), then one folder link per path segment;
+the flat all-documents table no longer renders. The asserted rows,
+links, and stat cards are UNCHANGED in intent — the drill is the only
+change (every row here is nested under a folder). The empty-state test
+now pins the zero-SOURCES state (a registered 0-document source renders
+its row instead) — for that state to be reachable at all, this suite
+runs its OWN module app (the conftest leak-guard pattern) with
+``BOR_GIT_SOURCES`` forced empty: the session app would inherit an
+operator's local ``.env`` fallback source, which would leak a
+0-document top-level row into the tree.
 """
 from __future__ import annotations

 import asyncio
+import json
+import os
+import subprocess
+import sys
+from collections.abc import Iterator
 from pathlib import Path
 from threading import Thread
 from typing import Any

+import pytest
 from playwright.sync_api import Browser, Page, expect
 from sqlalchemy import text

@@ -27,10 +46,86 @@ from app.db import SessionLocal
 from app.rag.importer import ImportSummary, import_sources
 from app.rag.llm import LLMClient
 from e2e.auth_helpers import login
+from e2e.conftest import (
+    ADMIN_PASSWORD,
+    SESSION_SECRET,
+    USE_REAL_LLM,
+    _wait_http,
+)

 REPO = Path(__file__).resolve().parents[2]
 FIXTURES = REPO / "tests" / "fixtures" / "docs"

+# Phase 79 (task 04, full inventory): the conftest session app owns its
+# port in a combined run — this module app binds its own port instead
+# (a same-port second uvicorn dies on bind and would drive the wrong
+# server). Env-overridable.
+APP_PORT = int(os.environ.get("E2E_APP_PORT_IMPORTDOCS", "8140"))
+APP_URL = f"http://127.0.0.1:{APP_PORT}"
+
+
+@pytest.fixture(scope="module")
+def app_server(mock_llm: int) -> Iterator[str]:
+    """The real app under test — per-module env (the conftest pattern):
+    the leak guards force the code defaults, and ``BOR_GIT_SOURCES`` is
+    forced EMPTY (phase 97 — the registered sources now render as
+    top-level rows: the operator's ``.env`` fallback source would leak
+    a 0-document source into the tree and break the empty-state test's
+    zero-SOURCES state). The session app is never started in this
+    isolated run, so no port clash."""
+    env = dict(os.environ)
+    env.pop("DEBUGPY", None)
+    env["BOR_ENVIRONMENT"] = "e2e"
+    env["BOR_STATIC_DIR"] = str(REPO / "frontend")
+    env["BOR_LLM_BASE_URL"] = (
+        "https://aipi.reeseapps.com/v1"
+        if USE_REAL_LLM
+        else f"http://127.0.0.1:{mock_llm}/v1"
+    )
+    # Mock-calibrated threshold (conftest pattern).
+    env["BOR_RELEVANCE_THRESHOLD"] = "0.30"
+    # Phase 67: instant retry waits + the code-default budget.
+    env["BOR_LLM_RETRY_DELAY"] = "0"
+    env["BOR_LLM_RETRIES"] = str(Settings.model_fields["llm_retries"].default)
+    env.setdefault(
+        "BOR_DATABASE_URL",
+        "postgresql+psycopg://reese:reese@localhost:5432/brain_of_reese",
+    )
+    # Phase 16: admin auth must be set or create_app() refuses to boot.
+    env["BOR_ADMIN_PASSWORD"] = ADMIN_PASSWORD
+    env["BOR_SESSION_SECRET"] = SESSION_SECRET
+    # Phase 97: the registry is this suite's own concern (see the
+    # fixture docstring) — the env fallback is git-only, empty here.
+    env["BOR_GIT_SOURCES"] = ""
+    # Leak guards (conftest pattern): an operator's local (gitignored)
+    # .env cannot leak corpus-specific settings into the app under test.
+    env["BOR_DOCS_REPO"] = ""
+    env["BOR_SUGGESTIONS"] = json.dumps(
+        Settings.model_fields["suggestions"].default
+    )
+    env["BOR_INPUT_PLACEHOLDER"] = Settings.model_fields["input_placeholder"].default
+    env["BOR_FOOTER_TEXT"] = Settings.model_fields["footer_text"].default
+    proc = subprocess.Popen(
+        [sys.executable, "-m", "uvicorn", "app.main:app",
+         "--host", "127.0.0.1", "--port", str(APP_PORT), "--log-level", "warning"],
+        cwd=REPO,
+        env=env,
+    )
+    try:
+        _wait_http(f"{APP_URL}/api/health")
+        yield APP_URL
+    finally:
+        proc.terminate()
+        try:
+            proc.wait(timeout=10)
+        except subprocess.TimeoutExpired:
+            proc.kill()
+
+
+@pytest.fixture(scope="module")
+def app_url(app_server: str) -> str:
+    return app_server
+
 EXPECTED_ROWS = (
    "homelab/kubernetes.md",
    "homelab/backups.md",
@@ -78,15 +173,57 @@ def _run_in_thread(coro: Any) -> Any:


 def _reset_db(mock_port: int, seed: bool) -> ImportSummary | None:
-    """Truncate the KB (and query log), then optionally re-import fixtures."""
+    """Truncate the KB (and query log), then optionally re-import fixtures.
+
+    Phase 97: the registry (``git_sources``) and the stored folder
+    descriptions (``folder_summaries``) are truncated too — they now
+    RENDER in the RAG view (top-level source rows + descriptions), so a
+    leftover row from another suite would show up as a 0-document
+    source and break the empty-state test's zero-SOURCES state."""
    with SessionLocal() as db:
-        db.execute(text("TRUNCATE chunks, documents, query_log"))
+        db.execute(
+            text("TRUNCATE chunks, documents, query_log, git_sources, folder_summaries")
+        )
        db.commit()
    if not seed:
        return None
    return _run_in_thread(_import_fixtures(mock_port))


+# ---------------------------------------------------------------------------
+# Phase 97: the catalog is the drill-down tree the agent's `ls` walks —
+# the top level lists the SOURCES (this fixture's single indexed-only
+# source: `docs`, the fixtures dir's basename — import_sources seeded it
+# with no registry row), then one folder link per path segment. The
+# flat all-documents table is gone; the drill is the only change (the
+# asserted rows/links are the same).
+# ---------------------------------------------------------------------------
+
+
+SOURCE_NAME = "docs"  # the fixtures dir's basename (the indexed-only source)
+
+
+def _drill(page: Page, *names: str) -> None:
+    """Drill one level at a time (client-side — no fetch, no URL
+    change): each name is the EXACT text of the source/folder link at
+    the current level (the test_kb_tree house pattern)."""
+    for name in names:
+        page.click(f'#folders-tbody a.folder-link:text-is("{name}")')
+
+
+def _go_top(page: Page) -> None:
+    """Back to the top level (the sources list): the breadcrumb's
+    top-level link (hidden AT the top — call between drills only)."""
+    page.locator("#kb-crumb a.kb-crumb-link").first.click()
+
+
+def _wait_top_level(page: Page) -> None:
+    """The tree's single fetch settled: the source row is rendered
+    (the catalog-rendered signal — the top-level file table is always
+    hidden, so it is no longer a usable one)."""
+    page.locator("#folders-tbody .folder-link").first.wait_for(state="visible")
+
+
 def test_sources_page_lists_indexed_docs(
    page: Page, app_url: str, mock_llm: int, db_ready: None
 ) -> None:
@@ -111,11 +248,25 @@ def test_sources_page_lists_indexed_docs(
    expect(page.locator("#stat-last")).not_to_have_text("–")
    expect(page.locator("#sources-empty")).to_be_hidden()

-    for row_path in EXPECTED_ROWS:
+    _wait_top_level(page)
+    # Phase 97: every row is nested under a folder — drill source →
+    # folder(s) per path before asserting (the flat 13-row tbody no
+    # longer exists; the stat cards above hold the KB total).
+    for i, row_path in enumerate(EXPECTED_ROWS):
+        if i > 0:
+            _go_top(page)
+        _drill(page, SOURCE_NAME, *row_path.rsplit("/", 1)[0].split("/"))
        expect(page.locator("#docs-tbody tr", has_text=row_path)).to_have_count(1)
-    # The hidden junk was never indexed (A9 scope).
+    # The hidden junk was never indexed (A9 scope) — scoped to the
+    # level where it WOULD appear: no `.hidden` file row at the source
+    # level AND no `.hidden` folder row there either (the stat cards
+    # pin the 13 total, so the junk is counted nowhere).
+    _go_top(page)
+    _drill(page, SOURCE_NAME)
    expect(page.locator("#docs-tbody tr", has_text=".hidden")).to_have_count(0)
+    expect(page.locator("#folders-tbody .folder-link", has_text=".hidden")).to_have_count(0)
    # The path column carries the full path for hover (ellipsis is visual only).
+    _drill(page, "homelab")
    expect(page.locator("#docs-tbody tr", has_text="homelab/kubernetes.md")
           .get_by_role("cell").nth(1)).to_have_attribute("title", "homelab/kubernetes.md")

@@ -125,11 +276,18 @@ def test_sources_table_layout(
 ) -> None:
    _reset_db(mock_llm, seed=True)
    login(page, app_url)  # phase 16: the catalog is admin-only
+    # Phase 97: the top level lists the sources (the file table is
+    # hidden there) — wait for the source row, then drill to a file
+    # level (docs → homelab) for the file table's layout asserts.
+    _wait_top_level(page)
+    _drill(page, SOURCE_NAME, "homelab")
    page.locator("#docs-tbody tr").first.wait_for(state="visible")

    # Phase 76 (task 02): the shell carries BOTH views' .table-wrap —
-    # scope to the RAG view.
-    wrap = page.locator("#view-rag .table-wrap")
+    # scope to the RAG view. Phase 97: the RAG view carries TWO
+    # .table-wraps (#folders-wrap + the file table's) — :has() targets
+    # the file table's (the one these layout pins were written for).
+    wrap = page.locator("#view-rag .table-wrap:has(#docs-table)")
    expect(wrap).to_be_visible()
    expect(wrap).to_have_attribute("role", "region")
    expect(wrap).to_have_attribute("tabindex", "0")
@@ -146,9 +304,13 @@ def test_sources_table_layout(
    mobile = browser.new_page(viewport={"width": 375, "height": 812})
    try:
        login(mobile, app_url)  # phase 16: the catalog is admin-only
+        _wait_top_level(mobile)
+        _drill(mobile, SOURCE_NAME, "homelab")
        mobile.locator("#docs-tbody tr").first.wait_for(state="visible")
+        # Phase 97: target the FILE table's wrap explicitly (a bare
+        # .table-wrap query would hit #folders-wrap first now).
        scroll_width, client_width = mobile.evaluate(
-            "() => { const el = document.querySelector('#view-rag .table-wrap');"
+            "() => { const el = document.querySelector('#docs-table').parentElement;"
            " return [el.scrollWidth, el.clientWidth]; }"
        )
        assert scroll_width > client_width
@@ -165,6 +327,9 @@ def test_empty_state_when_no_docs(page: Page, app_url: str, mock_llm: int, db_re
    expect(page.locator("#sources-empty code")).to_have_text(
        "uv run python -m scripts.import_docs"
    )
-    expect(page.locator("#view-rag .table-wrap")).to_be_hidden()
+    # Phase 97: BOTH catalog tables ship in the RAG view — the empty
+    # state hides both (the original single-wrap pin, extended).
+    expect(page.locator("#view-rag .table-wrap:has(#docs-table)")).to_be_hidden()
+    expect(page.locator("#folders-wrap")).to_be_hidden()
    expect(page.locator("#stat-docs")).to_have_text("0")
    expect(page.locator("#stat-chunks")).to_have_text("0")