All defaults match. Verification complete — everything is green with no defects found. **Phase 147 final verification pass — report** - All 3 task files found in `complete/147_sync_changed_only/` → verification pass only; no code changes needed. - Verified: migration 0025 (`documents.file_hash` nullable String(64), no index) + model field; fast path at top of `_index_file` (streamed 1 MiB sha256 → row lookup → `_unchanged_doc`); stamping on all three branches' create/update + shared helper; pre-phase NULL-hash fallthrough; retriever/`content_hash` untouched. - **Tests:** `uv run pytest --cov=app --cov-report=term-missing` → 3438 passed, `app/` coverage **99%** (>90% ✓); `uv run pytest tests/e2e/test_sync_changed_only.py -v --no-cov` → 3 passed in isolation ✓; `uv run ruff check . && uv run pyright` → all checks passed, 0 errors ✓; prior-phase `tests/e2e/test_sync_button.py` → 3 passed (no regression) ✓. - **Completion criteria:** all 7 pass — (1) column + round-trip ✓ (2) unchanged text/image/garbage-PDF/pictorial-PDF cost zero LLM/extraction/vision, `summary.unchanged` counted, rows byte-still ✓ (3) changed file re-indexes + re-stamps; bytes-changed/content-same stamps only ✓ (4) backfill/D1/D4 + log wordings pinned verbatim ✓ (5) `content_hash`/retriever byte-identical ✓ (6) all gates green ✓ (7) no prior-phase behavior change ✓. - **Documentation:** full review of README.md (1,031 lines) and AGENTS.md against the code: every `.env.example`/code `BOR_*` var documented, all config defaults match `app/config.py`, quick-start/QA commands and referenced test files exist, `file://` URL + troubleshooting claims accurate; the phase's README "Updating the documents" edit is accurate. **Verified consistent — no changes needed.** - **Notable:** no live-infrastructure changes (dev Postgres container used for the standard test gates only). Working tree left uncommitted per harness protocol. - **Next pending phase:** `148_pdf_pages_in_sources`.
409 lines
16 KiB
Python
409 lines
16 KiB
Python
"""Phase 147 E2E (Playwright): only what changed is synced.
|
||
|
||
Source: ``TODO.md:4`` — "Local sources, like PDFs, are synced every
|
||
time even if they haven't changed. This results in really long sync
|
||
times for PDFs that are mostly pictoral. Only sync documents if they've
|
||
changed since the last sync (hash the document contents to check)."
|
||
|
||
Run in isolation (DB must be up: ``podman compose up -d db``; git on
|
||
PATH — a documented environment prerequisite, phase 28):
|
||
|
||
uv run pytest tests/e2e/test_sync_changed_only.py -v --no-cov
|
||
|
||
End-to-end proof through the real UI + the real sync path (a real
|
||
``git clone`` of a local ``file://`` fixture repo, a real import
|
||
against the mock LLM): the SECOND sync of an unchanged source reports
|
||
everything unchanged and touches NO document row (``indexed_at`` /
|
||
``content_hash`` / ``file_hash`` all stay still — a re-index would
|
||
bump ``indexed_at``); a source change syncs ONLY the changed part (new
|
||
file → ``1 added``; modified file → ``1 updated`` with only its own
|
||
row moving). NO PDF here — the unit matrix (task 02) carries the
|
||
vision-skip proof; this suite pins the WIRE (the owner's visible
|
||
``#sync-result`` line) + the UI settle + the row-level proof, fast and
|
||
deterministic.
|
||
|
||
Per-module app env (the E2E conftest pattern, module-scoped): this
|
||
suite's app boots with ``BOR_GIT_SOURCES=file://<fixture repo>`` and
|
||
its own ``BOR_SOURCES_DIR`` on its own port (env-overridable, the
|
||
house port-constant pattern). The session app (no git sources) is
|
||
never started in this isolated run, so no port clash.
|
||
|
||
The three tests SHARE the settled state on purpose (one module-scoped
|
||
truncate, not per-test — the suite's point is the run 2 → run 3 →
|
||
run 4 sequence):
|
||
|
||
1. ``test_second_sync_unchanged`` — run 1 (clone + import) indexes
|
||
``N_DOCS`` documents and stamps every row's ``file_hash`` non-NULL;
|
||
run 2 (unchanged bytes) settles ``0 added · N unchanged`` and moves
|
||
NO row (the load-bearing assertion).
|
||
2. ``test_change_syncs_only_the_change`` — commit a NEW doc to the
|
||
fixture repo; run 3 settles ``1 added · N unchanged``; the new row
|
||
appears stamped, the pre-existing rows do not move.
|
||
3. ``test_changed_file_reindexes`` — modify the sentinel doc; run 4
|
||
settles ``0 added · 1 updated · N unchanged``; exactly the
|
||
sentinel row moves (``indexed_at`` bumped, ``content_hash`` and
|
||
``file_hash`` re-stamped).
|
||
"""
|
||
from __future__ import annotations
|
||
|
||
import os
|
||
import re
|
||
import subprocess
|
||
import sys
|
||
from collections.abc import Iterator
|
||
from datetime import datetime
|
||
from pathlib import Path
|
||
|
||
import httpx
|
||
import pytest
|
||
from playwright.sync_api import Page, expect
|
||
from sqlalchemy import select, text
|
||
|
||
from app.db import SessionLocal
|
||
from app.models import Document
|
||
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]
|
||
|
||
# The house port-constant pattern (module app, env-overridable) — the
|
||
# session app (8123) is never started in this isolated run.
|
||
APP_PORT = int(os.environ.get("E2E_APP_PORT_SYNCCHANGED", "8157"))
|
||
APP_URL = f"http://127.0.0.1:{APP_PORT}"
|
||
|
||
# The fixture repo's directory name — the source name the import
|
||
# records for it (the repo_name basename rule, phase 28).
|
||
GIT_SOURCE_NAME = "changed-notes"
|
||
|
||
# The three importable fixture docs (task 03: a few importable docs,
|
||
# NO PDF — the unit matrix covers the vision-skip proof).
|
||
SENTINEL_DOC = "notes/sync-fixture.md"
|
||
SECOND_DOC = "notes/second-note.md"
|
||
THIRD_DOC = "notes/third-note.md"
|
||
#: The doc test 2 commits AFTER run 2 (``1 added``).
|
||
NEW_DOC = "notes/new-note.md"
|
||
N_DOCS = 3
|
||
|
||
#: "Synced HH:MM" — the local-time last-result label (sources.js's
|
||
#: fmtSyncTime), any hour/minute.
|
||
SYNCED_LABEL = re.compile(r"Synced \d{1,2}:\d{2}")
|
||
|
||
#: Real git clone/pull + import against the mock LLM (+ the KB
|
||
#: overview regeneration after a KB-changing run) — generous budget,
|
||
#: no client timeout (the test_sync_button.py pattern).
|
||
SYNC_TIMEOUT_MS = 90_000
|
||
|
||
#: One document row's row-level proof snapshot (phase 147: the
|
||
#: ``file_hash`` stamp joins the pre-phase pair).
|
||
DocRow = tuple[datetime | None, str | None, str | None]
|
||
|
||
|
||
def _git(cwd: Path, *args: str) -> None:
|
||
"""Run git in *cwd*; a non-zero exit fails the fixture/test loudly."""
|
||
proc = subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True)
|
||
if proc.returncode != 0:
|
||
raise AssertionError(f"git {' '.join(args)} failed: {proc.stderr.strip()}")
|
||
|
||
|
||
def _commit(cwd: Path, message: str) -> None:
|
||
"""The house fixture commit — identity inline, never signed."""
|
||
_git(
|
||
cwd,
|
||
"-c", "user.email=e@x", "-c", "user.name=t",
|
||
"-c", "commit.gpgsign=false", # the fixture commits never sign
|
||
"commit", "-qm", message,
|
||
)
|
||
|
||
|
||
@pytest.fixture(scope="module")
|
||
def sync_git_repo(tmp_path_factory: pytest.TempPathFactory) -> Path:
|
||
"""A real one-commit git repo with ``N_DOCS`` importable docs that
|
||
the sync must clone (the test_sync_button.py recipe, module-safe
|
||
``tmp_path_factory`` + real ``git`` subprocess calls)."""
|
||
root = tmp_path_factory.mktemp("sync_changed_git")
|
||
repo = root / GIT_SOURCE_NAME
|
||
(repo / "notes").mkdir(parents=True)
|
||
(repo / "notes" / "sync-fixture.md").write_text(
|
||
"# Sync fixture note\n"
|
||
"\n"
|
||
"One small note that exists only to prove the changed-only sync\n"
|
||
"end to end: a real git clone, a real import, an idempotent\n"
|
||
"second run — and the sentinel test 3 modifies.\n"
|
||
"\n"
|
||
"Marker: RESE-SYNC-CHANGED-SENTINEL-7f41\n",
|
||
encoding="utf-8",
|
||
)
|
||
(repo / "notes" / "second-note.md").write_text(
|
||
"# Second note\n"
|
||
"\n"
|
||
"A second unchanged doc — the sync's unchanged count is over\n"
|
||
"the whole source, so it must stay still through runs 2–4.\n"
|
||
"\n"
|
||
"Marker: RESE-SECOND-NOTE-5c0b\n",
|
||
encoding="utf-8",
|
||
)
|
||
(repo / "notes" / "third-note.md").write_text(
|
||
"# Third note\n"
|
||
"\n"
|
||
"A third unchanged doc (rounds out the source — the settle\n"
|
||
"line's unchanged count is the KB size, not a singleton).\n"
|
||
"\n"
|
||
"Marker: RESE-THIRD-NOTE-91de\n",
|
||
encoding="utf-8",
|
||
)
|
||
_git(repo, "init", "-q")
|
||
_git(repo, "add", "-A")
|
||
_commit(repo, "one")
|
||
assert (repo / ".git").is_dir()
|
||
return repo
|
||
|
||
|
||
@pytest.fixture(scope="module")
|
||
def app_server(mock_llm: int, sync_git_repo: Path) -> Iterator[str]:
|
||
"""The real app under test — per-module env: the sync's subject is
|
||
a real ``file://`` git source with its own checkout dir (the
|
||
conftest session app boots without ``BOR_GIT_SOURCES`` and is never
|
||
started in this isolated run)."""
|
||
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) — keeps every story
|
||
# suite's deterministic gate behavior; production default stays 0.62.
|
||
env["BOR_RELEVANCE_THRESHOLD"] = "0.30"
|
||
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 147: the sync's subject — one real local repo.
|
||
env["BOR_GIT_SOURCES"] = f"file://{sync_git_repo}"
|
||
env["BOR_SOURCES_DIR"] = str(sync_git_repo.parent / "checkouts")
|
||
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
|
||
|
||
|
||
@pytest.fixture(scope="module", autouse=True)
|
||
def _clean_kb_once(app_url: str) -> Iterator[None]:
|
||
"""Fresh KB ONCE per module — deliberately NOT per test: the suite's
|
||
point is the run 2 → run 3 → run 4 sequence, and tests 2–3 build on
|
||
the settled state test 1 leaves behind. git_sources too: the shared
|
||
Postgres may carry leftover registry rows from other suites, and
|
||
effective_sources prefers the DB list over this app's
|
||
BOR_GIT_SOURCES env fallback."""
|
||
body = httpx.get(f"{app_url}/api/health", timeout=5).json()
|
||
if body["db"] != "up":
|
||
pytest.skip("Postgres not reachable — run `podman compose up -d db` first")
|
||
with SessionLocal() as db:
|
||
db.execute(
|
||
text("TRUNCATE chunks, documents, query_log, kb_overview, git_sources")
|
||
)
|
||
db.commit()
|
||
yield
|
||
|
||
|
||
def _doc_rows() -> dict[str, DocRow]:
|
||
"""Every document row of the fixture source, keyed by path — the
|
||
row-level proof (the test_sync_button.py DB-assertion idiom,
|
||
extended with the phase-147 ``file_hash`` stamp)."""
|
||
with SessionLocal() as db:
|
||
docs = db.execute(
|
||
select(Document).where(Document.source == GIT_SOURCE_NAME)
|
||
).scalars().all()
|
||
return {d.path: (d.indexed_at, d.content_hash, d.file_hash) for d in docs}
|
||
|
||
|
||
def _click_sync_and_wait_settle(page: Page) -> None:
|
||
"""Click the admin sync button and wait for the settle — the
|
||
"Syncing…" → "Synced HH:MM" transition with the button re-enabled
|
||
at the terminal state (§7.4 never-stale, the test_sync_button.py
|
||
idiom)."""
|
||
btn = page.locator("#sync-btn")
|
||
btn.click()
|
||
expect(btn).to_be_disabled()
|
||
expect(page.locator("#sync-label")).to_have_text("Syncing…")
|
||
expect(page.locator("#sync-label")).to_have_text(
|
||
SYNCED_LABEL, timeout=SYNC_TIMEOUT_MS
|
||
)
|
||
expect(btn).to_be_enabled()
|
||
|
||
|
||
def _expect_settle_line(page: Page, expected: str) -> None:
|
||
"""The owner's surface: the VISIBLE ``#sync-result`` text in its
|
||
house count shape (sources.js's fmtSyncResult — the zero
|
||
"updated" part is omitted: ``0 added · N unchanged``,
|
||
``1 added · N unchanged``, ``0 added · 1 updated · N unchanged``)."""
|
||
expect(page.locator("#sync-result")).to_have_text(
|
||
expected, timeout=SYNC_TIMEOUT_MS
|
||
)
|
||
|
||
|
||
def _expect_rows_moved(
|
||
before: dict[str, DocRow], after: dict[str, DocRow], *, moved: set[str]
|
||
) -> None:
|
||
"""Exactly the rows in *moved* may differ — every other row's
|
||
``indexed_at``/``content_hash``/``file_hash`` is byte-still (a
|
||
re-index would bump ``indexed_at``; the changed-only sync must not
|
||
leak into neighbors). New rows may appear only within *moved*; the
|
||
suite never expects a prune."""
|
||
gained = set(after) - set(before)
|
||
assert gained <= moved, f"unexpected rows added: {sorted(gained)}"
|
||
lost = set(before) - set(after)
|
||
assert not lost, f"unexpected rows pruned: {sorted(lost)}"
|
||
for path, snapshot in before.items():
|
||
if path in moved:
|
||
continue
|
||
assert after[path] == snapshot, (
|
||
f"{path} moved though only {sorted(moved)} may move: "
|
||
f"{snapshot} -> {after[path]}"
|
||
)
|
||
|
||
|
||
# --- 1. Run 1 stamps, run 2 re-indexes nothing -----------------------------
|
||
|
||
|
||
def test_second_sync_unchanged(page: Page, app_url: str, db_ready: None) -> None:
|
||
"""Run 1 (the real clone + import) indexes ``N_DOCS`` docs and
|
||
stamps EVERY row's ``file_hash`` non-NULL (the first sync's job);
|
||
run 2 (unchanged bytes) settles ``0 added · N unchanged`` and
|
||
touches NO document row — ``indexed_at``/``content_hash``/
|
||
``file_hash`` all still (the load-bearing assertion: a re-index
|
||
would bump ``indexed_at`` to now and the owner's long syncs would
|
||
be back)."""
|
||
login(page, app_url) # lands on /sources.html (the button's home)
|
||
|
||
# --- run 1: clone + import (everything is new) -----------------------
|
||
_click_sync_and_wait_settle(page)
|
||
_expect_settle_line(page, f"{N_DOCS} added")
|
||
after_run1 = _doc_rows()
|
||
assert set(after_run1) == {SENTINEL_DOC, SECOND_DOC, THIRD_DOC}, after_run1
|
||
for path, (indexed_at, content_hash, file_hash) in after_run1.items():
|
||
assert indexed_at is not None and content_hash, path
|
||
# The first sync stamped the hash on every row (the fast
|
||
# path's precondition — NULL would mean run 2 re-indexed).
|
||
assert file_hash is not None and len(file_hash) == 64, (
|
||
f"{path} must carry a non-NULL file_hash after run 1"
|
||
)
|
||
|
||
# --- run 2: unchanged bytes → zero re-index --------------------------
|
||
_click_sync_and_wait_settle(page)
|
||
_expect_settle_line(page, f"0 added · {N_DOCS} unchanged")
|
||
after_run2 = _doc_rows()
|
||
_expect_rows_moved(after_run1, after_run2, moved=set())
|
||
|
||
|
||
# --- 2. A new file syncs as `1 added`, nothing else moves ------------------
|
||
|
||
|
||
def test_change_syncs_only_the_change(
|
||
page: Page, app_url: str, db_ready: None, sync_git_repo: Path
|
||
) -> None:
|
||
"""From the settled state (test 1), commit a NEW doc to the fixture
|
||
repo's working tree: run 3 syncs ONLY the change — the settle line
|
||
reads ``1 added · N unchanged`` (the unchanged count is still run
|
||
1's doc count), the new row exists stamped, and the pre-existing
|
||
rows' ``indexed_at``/``content_hash`` are untouched."""
|
||
login(page, app_url)
|
||
before = _doc_rows()
|
||
assert len(before) == N_DOCS, "the settled state from test 1 is required"
|
||
|
||
(sync_git_repo / "notes" / "new-note.md").write_text(
|
||
"# New note\n"
|
||
"\n"
|
||
"Committed after run 2 — only this file may move in run 3.\n"
|
||
"\n"
|
||
"Marker: RESE-NEW-DOC-3d8a\n",
|
||
encoding="utf-8",
|
||
)
|
||
_git(sync_git_repo, "add", "-A")
|
||
_commit(sync_git_repo, "add new-note")
|
||
|
||
_click_sync_and_wait_settle(page)
|
||
_expect_settle_line(page, f"1 added · {N_DOCS} unchanged")
|
||
|
||
after = _doc_rows()
|
||
assert set(after) - set(before) == {NEW_DOC}, after.keys()
|
||
_, new_content_hash, new_file_hash = after[NEW_DOC]
|
||
assert new_content_hash, "the added row must carry its content hash"
|
||
assert new_file_hash is not None and len(new_file_hash) == 64, (
|
||
"the added row must carry its file_hash stamp"
|
||
)
|
||
# The pre-existing rows are byte-still (the added file must not
|
||
# drag them into a re-index).
|
||
_expect_rows_moved(before, after, moved={NEW_DOC})
|
||
|
||
|
||
# --- 3. A modified file syncs as `1 updated`, only its own row -------------
|
||
|
||
|
||
def test_changed_file_reindexes(
|
||
page: Page, app_url: str, db_ready: None, sync_git_repo: Path
|
||
) -> None:
|
||
"""Modify the fixture sentinel doc (append a line) + commit: run 4
|
||
syncs ONLY the change — the settle line reads
|
||
``0 added · 1 updated · N unchanged`` (N = the run-1 count — the
|
||
test-2 doc is unchanged too), and EXACTLY the sentinel row moves:
|
||
its ``indexed_at`` is bumped, its ``content_hash`` and
|
||
``file_hash`` re-stamped; every other row is byte-still."""
|
||
login(page, app_url)
|
||
before = _doc_rows()
|
||
assert len(before) == N_DOCS + 1, "the settled state from test 2 is required"
|
||
|
||
sentinel = sync_git_repo / "notes" / "sync-fixture.md"
|
||
sentinel.write_text(
|
||
sentinel.read_text(encoding="utf-8")
|
||
+ "\nAmended: the sentinel was touched — exactly this row may move.\n",
|
||
encoding="utf-8",
|
||
)
|
||
_git(sync_git_repo, "add", "-A")
|
||
_commit(sync_git_repo, "modify sentinel")
|
||
|
||
_click_sync_and_wait_settle(page)
|
||
_expect_settle_line(page, f"0 added · 1 updated · {N_DOCS} unchanged")
|
||
|
||
after = _doc_rows()
|
||
_expect_rows_moved(before, after, moved={SENTINEL_DOC})
|
||
old_indexed_at, old_content_hash, old_file_hash = before[SENTINEL_DOC]
|
||
new_indexed_at, new_content_hash, new_file_hash = after[SENTINEL_DOC]
|
||
assert new_content_hash != old_content_hash, (
|
||
"the content changed — content_hash must be re-stamped"
|
||
)
|
||
assert new_file_hash != old_file_hash, (
|
||
"the bytes changed — file_hash must be re-stamped"
|
||
)
|
||
assert (
|
||
old_indexed_at is not None
|
||
and new_indexed_at is not None
|
||
and new_indexed_at > old_indexed_at
|
||
), "the re-index must bump indexed_at (run 1 stamped it; run 4 re-indexed)"
|