feat(chat): invalidate saved chats on sources sync — versioned stamps, stale marker, Regenerate against the new index
This commit is contained in:
@@ -10,7 +10,11 @@ fallback), the list order (``updated_at desc, id desc``), the
|
||||
full-payload round-trip (a ``bor.chat.v1``-shaped brain record carrying
|
||||
``sources``/``thinking``/``tools``/``stopped`` survives losslessly),
|
||||
the PUT upsert semantics (replacement + title-keep + title-set +
|
||||
``updated_at`` bump), and the delete 404/204.
|
||||
``updated_at`` bump), the delete 404/204, and (phase 53, task 03) the
|
||||
sources-version stamp + ``stale`` flag: create and re-Save stamp the
|
||||
row's ``sources_version``, list/detail expose ``stale`` (true iff the
|
||||
stamp is behind the current generation — computed server-side), and
|
||||
share/unshare stay version-immune.
|
||||
|
||||
Requires: podman compose up -d db
|
||||
"""
|
||||
@@ -31,7 +35,8 @@ from sqlalchemy.orm import Session
|
||||
|
||||
from app.config import Settings
|
||||
from app.main import app as fastapi_app
|
||||
from app.models import SavedChat
|
||||
from app.models import SavedChat, SourcesMeta
|
||||
from app.rag.sources_meta import bump_sources_version
|
||||
|
||||
FIRST_QUESTION = "How did I install gitlab?"
|
||||
EXPLICIT_TITLE = "My backup notes"
|
||||
@@ -62,8 +67,16 @@ FULL_BRAIN: dict[str, Any] = {
|
||||
"stopped": False,
|
||||
}
|
||||
|
||||
OUT_KEYS = {"id", "title", "created_at", "updated_at", "message_count", "messages"}
|
||||
ROW_KEYS = {"id", "title", "updated_at", "message_count"}
|
||||
OUT_KEYS = {
|
||||
"id",
|
||||
"title",
|
||||
"created_at",
|
||||
"updated_at",
|
||||
"message_count",
|
||||
"messages",
|
||||
"stale", # phase 53: server-computed staleness flag
|
||||
}
|
||||
ROW_KEYS = {"id", "title", "updated_at", "message_count", "stale"}
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
@@ -76,6 +89,45 @@ def clean_chats(db: Session) -> Iterator[None]:
|
||||
db.commit()
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def seeded_sources_meta(db: Session) -> Iterator[None]:
|
||||
"""The ``sources_meta`` counter (phase 53) is global state: reset
|
||||
to the migration-0010 seed (id 1, version 0 — "the pre-counter
|
||||
KB") around every test, so each test starts from a known
|
||||
generation and the dev DB is left exactly as the migration left
|
||||
it."""
|
||||
db.execute(text("DELETE FROM sources_meta"))
|
||||
db.add(SourcesMeta(id=1, version=0))
|
||||
db.commit()
|
||||
yield
|
||||
db.execute(text("DELETE FROM sources_meta"))
|
||||
db.add(SourcesMeta(id=1, version=0))
|
||||
db.commit()
|
||||
|
||||
|
||||
def _bump(db: Session) -> int:
|
||||
"""One simulated KB-changing sync: a single committed bump
|
||||
(task 02's change-gate lives in the sync paths themselves; the API
|
||||
tests only need the counter's contract — caller commits).
|
||||
"""
|
||||
version = bump_sources_version(db)
|
||||
db.commit() # the helper only flushes
|
||||
return version
|
||||
|
||||
|
||||
def _stored_sources_version(db: Session, chat_id: str) -> int:
|
||||
"""The row's ``sources_version`` via raw SQL — deliberately
|
||||
bypassing the session's identity map, because the API's commits
|
||||
land in the app's own sessions (a cached ORM object could be
|
||||
stale)."""
|
||||
return int(
|
||||
db.execute(
|
||||
text("SELECT sources_version FROM saved_chats WHERE id = :id"),
|
||||
{"id": chat_id},
|
||||
).scalar_one()
|
||||
)
|
||||
|
||||
|
||||
def _user(text: str) -> dict[str, Any]:
|
||||
return {"who": "user", "text": text}
|
||||
|
||||
@@ -716,6 +768,141 @@ def test_get_carry_share_url_and_unshare_drops_it(admin_client: TestClient) -> N
|
||||
assert set(got2) == OUT_KEYS
|
||||
|
||||
|
||||
# ---------- sources-version stamp + stale flag (phase 53, task 03) ----------
|
||||
|
||||
|
||||
def test_create_stamps_current_sources_version(
|
||||
admin_client: TestClient, db: Session
|
||||
) -> None:
|
||||
"""``POST`` stamps the PENDING row with the current generation —
|
||||
it ships in the same INSERT (the ``share_token`` precedent), and
|
||||
the 201 body reports ``stale: false`` (a fresh save is by
|
||||
definition current)."""
|
||||
created = admin_client.post(
|
||||
"/api/chats", json={"messages": _simple_conversation()}
|
||||
).json()
|
||||
assert created["stale"] is False, "a fresh save is never stale"
|
||||
assert _stored_sources_version(db, created["id"]) == 0 # seed generation
|
||||
|
||||
# After a KB-changing sync (one bump), the NEXT save stamps the
|
||||
# new generation and is still fresh.
|
||||
assert _bump(db) == 1
|
||||
created2 = admin_client.post(
|
||||
"/api/chats", json={"messages": [_user("another question")]}
|
||||
).json()
|
||||
assert _stored_sources_version(db, created2["id"]) == 1
|
||||
assert created2["stale"] is False
|
||||
|
||||
|
||||
def test_pre_counter_row_goes_stale_on_the_first_bump(
|
||||
admin_client: TestClient, db: Session
|
||||
) -> None:
|
||||
"""Recorded assumption: pre-existing rows (saved before the counter
|
||||
existed) stamp 0 — "the pre-counter KB" — and go stale on the
|
||||
first bump (0 < 1). A row inserted directly with the column's
|
||||
server default mirrors such a legacy row."""
|
||||
db.add(SavedChat(title="legacy", messages=[_user("old question")]))
|
||||
db.commit() # no sources_version supplied → server default 0
|
||||
|
||||
listing = admin_client.get("/api/chats").json()["chats"][0]
|
||||
assert listing["stale"] is False, "0 == 0: current at the pre-counter KB"
|
||||
|
||||
assert _bump(db) == 1
|
||||
listing = admin_client.get("/api/chats").json()["chats"][0]
|
||||
assert listing["stale"] is True, "0 < 1: the first bump stale-s it"
|
||||
|
||||
|
||||
def test_list_and_detail_report_stale_after_bump(admin_client: TestClient, db: Session) -> None:
|
||||
"""The staleness flag is computed server-side in BOTH admin read
|
||||
shapes: after a bump, the list row and the detail payload of a
|
||||
row saved at the older generation report ``stale: true`` (and a
|
||||
second bump — still behind — stays stale)."""
|
||||
created = admin_client.post(
|
||||
"/api/chats", json={"messages": _simple_conversation()}
|
||||
).json()
|
||||
assert created["stale"] is False
|
||||
|
||||
assert _bump(db) == 1
|
||||
row = admin_client.get("/api/chats").json()["chats"][0]
|
||||
assert row["stale"] is True
|
||||
assert set(row) == ROW_KEYS # stale sits in the standard row shape
|
||||
|
||||
detail = admin_client.get(f"/api/chats/{created['id']}").json()
|
||||
assert detail["stale"] is True
|
||||
assert set(detail) == OUT_KEYS
|
||||
|
||||
assert _bump(db) == 2 # still behind (stamp 0 < 2) → still stale
|
||||
assert admin_client.get(f"/api/chats/{created['id']}").json()["stale"] is True
|
||||
|
||||
|
||||
def test_resave_restamps_and_clears_stale(admin_client: TestClient, db: Session) -> None:
|
||||
"""A Re-Save (``PUT``) re-stamps the row to the CURRENT generation
|
||||
unconditionally — the owner is affirming this content against the
|
||||
current KB — so the 200 body reports ``stale: false`` again and
|
||||
the stored stamp advances (the manual escape hatch for a
|
||||
false-positive stale row)."""
|
||||
created = admin_client.post(
|
||||
"/api/chats", json={"messages": _simple_conversation()}
|
||||
).json()
|
||||
assert _bump(db) == 1
|
||||
assert admin_client.get(f"/api/chats/{created['id']}").json()["stale"] is True
|
||||
|
||||
r = admin_client.put(
|
||||
f"/api/chats/{created['id']}", json={"messages": _simple_conversation()}
|
||||
)
|
||||
assert r.status_code == 200
|
||||
assert r.json()["stale"] is False, "the re-Save affirms against generation 1"
|
||||
assert _stored_sources_version(db, created["id"]) == 1
|
||||
|
||||
# A later bump stale-s it again — the stamp is a point in time, not
|
||||
# a sticky flag.
|
||||
assert _bump(db) == 2
|
||||
assert admin_client.get(f"/api/chats/{created['id']}").json()["stale"] is True
|
||||
|
||||
|
||||
def test_share_and_unshare_leave_sources_version_untouched(
|
||||
admin_client: TestClient, db: Session
|
||||
) -> None:
|
||||
"""Share/unshare write ONLY ``share_token`` (raw SQL, the
|
||||
phase-51 contract) — the version stamp, like ``updated_at``, is
|
||||
immune: neither action can (un-)stale a chat, and staleness stays
|
||||
a pure function of the saved generation vs the current one."""
|
||||
assert _bump(db) == 1 # a non-zero stamp makes the assert observable
|
||||
created = admin_client.post(
|
||||
"/api/chats", json={"messages": _simple_conversation()}
|
||||
).json()
|
||||
assert _stored_sources_version(db, created["id"]) == 1
|
||||
|
||||
_share(admin_client, created["id"])
|
||||
assert _stored_sources_version(db, created["id"]) == 1
|
||||
assert admin_client.get(f"/api/chats/{created['id']}").json()["stale"] is False
|
||||
|
||||
assert admin_client.post(f"/api/chats/{created['id']}/unshare").status_code == 200
|
||||
assert _stored_sources_version(db, created["id"]) == 1
|
||||
assert admin_client.get(f"/api/chats/{created['id']}").json()["stale"] is False
|
||||
|
||||
|
||||
def test_public_read_snapshot_has_no_stale_surface(
|
||||
admin_client: TestClient, db: Session,
|
||||
) -> None:
|
||||
"""``/api/shared/<token>`` is a FROZEN snapshot by design (phase
|
||||
51): even after a bump, the anonymous body keeps exactly its
|
||||
title+messages key set — no ``stale`` flag, no staleness surface
|
||||
(an owner who regenerates can re-share afterwards)."""
|
||||
created = admin_client.post(
|
||||
"/api/chats", json={"messages": _simple_conversation(), "share": True}
|
||||
).json()
|
||||
assert _bump(db) == 1 # the saved chat is now stale (admin surface)
|
||||
assert admin_client.get(f"/api/chats/{created['id']}").json()["stale"] is True
|
||||
|
||||
anon = TestClient(fastapi_app) # fresh jar: truly anonymous
|
||||
r = anon.get(f"/api{created['share_url']}")
|
||||
assert r.status_code == 200
|
||||
body = r.json()
|
||||
assert set(body) == SHARED_OUT_KEYS # no stale key — frozen snapshot
|
||||
assert "stale" not in body
|
||||
|
||||
|
||||
# ---------- the /shared/<token> page route (phase 51, task 01) ----------
|
||||
|
||||
|
||||
|
||||
@@ -4,7 +4,9 @@ kind).
|
||||
|
||||
Drives ``scripts.import_docs`` end to end with a fake ``clone_or_pull``
|
||||
(no real git, no network) and a recording fake ``import_sources`` (no
|
||||
real DB), covering:
|
||||
real DB), covering: the phase-53 version bump is stubbed in the
|
||||
``main()`` tests the same way (the ``sources_version=`` summary token
|
||||
is asserted against the canned value).
|
||||
|
||||
- Effective sources set (phase 35: the shared resolver — stubbed here,
|
||||
keeping this file's no-real-DB style) → each git URL is cloned/pulled
|
||||
@@ -83,6 +85,20 @@ def _fake_clone_factory() -> tuple[list[tuple[str, Path]], object]:
|
||||
return calls, fake_clone_or_pull
|
||||
|
||||
|
||||
def _stub_bump(monkeypatch: pytest.MonkeyPatch) -> list[None]:
|
||||
"""Stub the phase-53 version bump (this file keeps its no-real-DB
|
||||
style for the counter — the fake import already avoids the KB
|
||||
tables). Returns the call record; the canned new version is 1."""
|
||||
bumps: list[None] = []
|
||||
|
||||
def fake_bump(session: object) -> int:
|
||||
bumps.append(None)
|
||||
return 1
|
||||
|
||||
monkeypatch.setattr(import_docs, "bump_sources_version", fake_bump)
|
||||
return bumps
|
||||
|
||||
|
||||
# --- repo_name -------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -210,6 +226,7 @@ def test_main_git_sources_clone_then_import(
|
||||
monkeypatch.setattr(import_docs, "clone_or_pull", fake)
|
||||
fake_import = FakeImportSources()
|
||||
monkeypatch.setattr(import_docs, "import_sources", fake_import)
|
||||
bumps = _stub_bump(monkeypatch)
|
||||
|
||||
rc = import_docs.main([])
|
||||
|
||||
@@ -227,11 +244,19 @@ def test_main_git_sources_clone_then_import(
|
||||
for dest in (tmp_path / "bor" / "homelab", tmp_path / "bor" / "deploy"):
|
||||
assert (dest / "notes.md").is_file()
|
||||
# The final summary print reflects the import (added > 0).
|
||||
assert "added=1" in capsys.readouterr().out
|
||||
out = capsys.readouterr().out
|
||||
assert "added=1" in out
|
||||
# Phase 53: a KB-changing run bumps the sources version exactly
|
||||
# once and reports it (stubbed — this file keeps its no-real-DB
|
||||
# style for the counter, like the fake import above).
|
||||
assert len(bumps) == 1
|
||||
assert "sources_version=1" in out
|
||||
|
||||
|
||||
def test_main_cli_source_still_imports_manual_dir(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path: Path,
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
) -> None:
|
||||
manual = tmp_path / "manual"
|
||||
manual.mkdir()
|
||||
@@ -243,6 +268,7 @@ def test_main_cli_source_still_imports_manual_dir(
|
||||
monkeypatch.setattr(import_docs, "clone_or_pull", fake)
|
||||
fake_import = FakeImportSources()
|
||||
monkeypatch.setattr(import_docs, "import_sources", fake_import)
|
||||
bumps = _stub_bump(monkeypatch)
|
||||
|
||||
rc = import_docs.main(["--source", str(manual)])
|
||||
|
||||
@@ -250,6 +276,10 @@ def test_main_cli_source_still_imports_manual_dir(
|
||||
assert calls == []
|
||||
assert fake_import.calls[0]["sources"] == [manual]
|
||||
assert fake_import.calls[0]["prune"] is False
|
||||
# Phase 53: a manual --source run that changes the KB bumps exactly
|
||||
# once (the CLI is the other canonical sync path).
|
||||
assert len(bumps) == 1
|
||||
assert "sources_version=1" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_resolve_sources_mixed_git_and_local(
|
||||
|
||||
@@ -12,6 +12,17 @@ covering the change-gated overview trigger:
|
||||
``overview=failed``, the previous row untouched;
|
||||
- a ``--limit`` debug run with changes → ``overview=skipped``;
|
||||
- an empty source run with no row → no row created, ``overview=skipped``.
|
||||
|
||||
Phase 53 (task 02): the sources-version bump sits alongside the
|
||||
overview gate — a KB-changing run bumps ``sources_meta`` exactly once
|
||||
(``sources_version=<n>`` on the summary line), including a **prune-only**
|
||||
run: the invalidation gate ``added + updated + pruned > 0`` is
|
||||
deliberately broader than the overview's (a pruned doc can invalidate a
|
||||
saved answer that cited it, while the outline stays). ``--limit`` runs
|
||||
and unchanged re-runs never bump (``sources_version=skipped``), and a
|
||||
failed ``lite`` never rolls the bump back. The counter is pinned to the
|
||||
migration-0010 seed (0) around every test by
|
||||
:func:`_reset_sources_version`.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -24,8 +35,10 @@ from sqlalchemy import text
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.config import Settings
|
||||
from app.db import SessionLocal, db_available
|
||||
from app.models import KbOverview
|
||||
from app.rag.llm import LLMError
|
||||
from app.rag.sources_meta import current_sources_version
|
||||
from scripts import import_docs
|
||||
from tests.fakes import FakeEmbedder
|
||||
|
||||
@@ -44,6 +57,12 @@ def _row(db: Session) -> KbOverview | None:
|
||||
return db.get(KbOverview, 1)
|
||||
|
||||
|
||||
def _version(db: Session) -> int:
|
||||
"""The ``sources_meta`` generation (phase 53; freshly reloaded)."""
|
||||
db.expire_all()
|
||||
return current_sources_version(db)
|
||||
|
||||
|
||||
def _run_main(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
llm: FakeEmbedder,
|
||||
@@ -86,6 +105,26 @@ def _clean_kb(db: Session) -> Iterator[None]:
|
||||
db.commit()
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_sources_version() -> Iterator[None]:
|
||||
"""Phase 53: the sources version counter is global mutable state —
|
||||
pin it to the migration-0010 seed (0) around every test so the bump
|
||||
assertions start from a known generation (own session: the CLI
|
||||
bumps through its own short-lived ``SessionLocal``). Skips like the
|
||||
``db`` fixture when Postgres is down."""
|
||||
if not db_available():
|
||||
pytest.skip("Postgres not reachable — run `podman compose up -d db` first")
|
||||
session = SessionLocal()
|
||||
try:
|
||||
session.execute(text("UPDATE sources_meta SET version = 0 WHERE id = 1"))
|
||||
session.commit()
|
||||
yield
|
||||
finally:
|
||||
session.execute(text("UPDATE sources_meta SET version = 0 WHERE id = 1"))
|
||||
session.commit()
|
||||
session.close()
|
||||
|
||||
|
||||
def test_changed_import_writes_overview_row(
|
||||
db: Session,
|
||||
src: Path,
|
||||
@@ -110,7 +149,8 @@ def test_changed_import_writes_overview_row(
|
||||
|
||||
assert rc == 0
|
||||
assert "added=2" in out
|
||||
assert out.rstrip().endswith("overview=updated")
|
||||
assert out.rstrip().endswith("overview=updated sources_version=1")
|
||||
assert _version(db) == 1 # phase 53: a changed import bumps exactly once
|
||||
# Exactly one lite call — the overview itself (markdown files never
|
||||
# get a summary, so nothing else may touch ``chat``).
|
||||
assert len(llm.chat_calls) == 1
|
||||
@@ -137,7 +177,7 @@ def test_unchanged_reimport_does_not_call_lite(
|
||||
llm = FakeEmbedder()
|
||||
rc, out = _run_main(monkeypatch, llm, ["--source", str(src)], capsys)
|
||||
assert rc == 0
|
||||
assert out.rstrip().endswith("overview=updated")
|
||||
assert out.rstrip().endswith("overview=updated sources_version=1")
|
||||
assert len(llm.chat_calls) == 1
|
||||
assert _row(db) is not None
|
||||
|
||||
@@ -145,10 +185,11 @@ def test_unchanged_reimport_does_not_call_lite(
|
||||
rc, out = _run_main(monkeypatch, llm, ["--source", str(src)], capsys)
|
||||
assert rc == 0
|
||||
assert "unchanged=2" in out
|
||||
assert out.rstrip().endswith("overview=skipped")
|
||||
assert out.rstrip().endswith("overview=skipped sources_version=skipped")
|
||||
assert len(llm.chat_calls) == 1 # no new lite call
|
||||
row = _row(db)
|
||||
assert row is not None and row.content == "Summary of MyDocs"
|
||||
assert _version(db) == 1 # phase 53: an unchanged re-run never bumps
|
||||
|
||||
|
||||
def test_lite_failure_is_fail_soft(
|
||||
@@ -160,7 +201,7 @@ def test_lite_failure_is_fail_soft(
|
||||
good = FakeEmbedder()
|
||||
rc, out = _run_main(monkeypatch, good, ["--source", str(src)], capsys)
|
||||
assert rc == 0
|
||||
assert out.rstrip().endswith("overview=updated")
|
||||
assert out.rstrip().endswith("overview=updated sources_version=1")
|
||||
previous = _row(db)
|
||||
assert previous is not None
|
||||
previous_content = previous.content
|
||||
@@ -172,11 +213,14 @@ def test_lite_failure_is_fail_soft(
|
||||
rc, out = _run_main(monkeypatch, bad, ["--source", str(src)], capsys)
|
||||
assert rc == 0 # a failed outline must not fail the import
|
||||
assert "updated=1" in out
|
||||
assert out.rstrip().endswith("overview=failed")
|
||||
assert out.rstrip().endswith("overview=failed sources_version=2")
|
||||
assert len(bad.chat_calls) == 1 # the (failed) attempt was made
|
||||
row = _row(db)
|
||||
assert row is not None
|
||||
assert row.content == previous_content # previous row untouched
|
||||
# Phase 53: the bump commits independently of the best-effort
|
||||
# overview — a failed ``lite`` never rolls the version back.
|
||||
assert _version(db) == 2
|
||||
|
||||
|
||||
def test_limit_run_skips_overview(
|
||||
@@ -188,19 +232,20 @@ def test_limit_run_skips_overview(
|
||||
llm = FakeEmbedder()
|
||||
rc, out = _run_main(monkeypatch, llm, ["--source", str(src)], capsys)
|
||||
assert rc == 0
|
||||
assert out.rstrip().endswith("overview=updated")
|
||||
assert out.rstrip().endswith("overview=updated sources_version=1")
|
||||
assert len(llm.chat_calls) == 1
|
||||
|
||||
# An incomplete walk must not rewrite the outline (mirrors the
|
||||
# --prune-with---limit guard).
|
||||
# --prune-with---limit guard) — and must not advance the version.
|
||||
(src / "alpha.md").write_text("# Alpha\n\nChanged content.\n", encoding="utf-8")
|
||||
rc, out = _run_main(monkeypatch, llm, ["--source", str(src), "--limit", "1"], capsys)
|
||||
assert rc == 0
|
||||
assert "updated=1" in out
|
||||
assert out.rstrip().endswith("overview=skipped")
|
||||
assert out.rstrip().endswith("overview=skipped sources_version=skipped")
|
||||
assert len(llm.chat_calls) == 1 # --limit never burns a lite call
|
||||
row = _row(db)
|
||||
assert row is not None and row.content == "Summary of MyDocs"
|
||||
assert _version(db) == 1 # phase 53: --limit debug runs never bump
|
||||
|
||||
|
||||
def test_empty_source_without_row_creates_nothing(
|
||||
@@ -217,6 +262,36 @@ def test_empty_source_without_row_creates_nothing(
|
||||
|
||||
assert rc == 0
|
||||
assert "files=0" in out
|
||||
assert out.rstrip().endswith("overview=skipped")
|
||||
assert out.rstrip().endswith("overview=skipped sources_version=skipped")
|
||||
assert llm.chat_calls == [] # no KB → no outline, no wasted model call
|
||||
assert _row(db) is None # nothing created
|
||||
assert _version(db) == 0 # nothing changed → nothing bumped
|
||||
|
||||
|
||||
def test_prune_only_run_bumps_sources_version(
|
||||
db: Session,
|
||||
src: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
) -> None:
|
||||
"""Phase 53 (task 02): the invalidation gate is deliberately broader
|
||||
than the overview's — a prune-only run (added + updated == 0,
|
||||
pruned > 0) advances the version (a pruned document can invalidate
|
||||
a saved answer that cited it) while the outline stays.
|
||||
"""
|
||||
llm = FakeEmbedder()
|
||||
rc, out = _run_main(monkeypatch, llm, ["--source", str(src)], capsys)
|
||||
assert rc == 0
|
||||
assert "added=2" in out
|
||||
assert out.rstrip().endswith("overview=updated sources_version=1")
|
||||
assert _version(db) == 1
|
||||
|
||||
# Delete one file; a --prune run drops exactly it: no add/update,
|
||||
# but pruned=1 → the version still bumps while the overview skips.
|
||||
(src / "alpha.md").unlink()
|
||||
rc, out = _run_main(monkeypatch, llm, ["--source", str(src), "--prune"], capsys)
|
||||
assert rc == 0
|
||||
assert "pruned=1" in out
|
||||
assert out.rstrip().endswith("overview=skipped sources_version=2")
|
||||
assert _version(db) == 2 # the prune-only change bumped exactly once
|
||||
assert _row(db) is not None # the outline row is untouched
|
||||
|
||||
@@ -0,0 +1,260 @@
|
||||
"""Integration: migration 0010 (sources_meta + saved_chats.sources_version).
|
||||
|
||||
Drives the **real Alembic engine** against the live dev database
|
||||
(``podman compose up -d db``), mirroring the house pattern of
|
||||
``test_migration_0009.py`` (information_schema / pg catalog assertions
|
||||
on the state the migration must leave). The tests target revision
|
||||
``0010`` explicitly so later migrations cannot break them:
|
||||
|
||||
* upgrade 0009 → 0010 → the single-row ``sources_meta`` table exists
|
||||
(``id`` Integer PK default 1, ``version`` Integer NOT NULL default 0,
|
||||
``updated_at`` TIMESTAMPTZ NOT NULL default now()) with its **seed
|
||||
row** (id 1, version 0), and ``saved_chats.sources_version`` is
|
||||
Integer NOT NULL default 0 — a pre-0010 row comes back stamped 0
|
||||
(the pre-counter KB, phase-53 locked decision 2);
|
||||
* inserted rows round-trip the stamp (default and explicit);
|
||||
* downgrade to 0009 → column + table gone (A13 — reversible), the rest
|
||||
of ``saved_chats`` survives;
|
||||
* upgrade back to 0010 → table, seed row, and column are all back
|
||||
(round-trip).
|
||||
|
||||
The ``alembic`` fixture guarantees the DB ends at head even if a test
|
||||
fails or the process is interrupted.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import uuid
|
||||
from collections.abc import Iterator
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
from alembic.config import Config
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from alembic import command
|
||||
from app.db import db_available
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def alembic(db: Session) -> Iterator[Config]:
|
||||
"""Real Alembic config bound to the dev DB (URL from app settings).
|
||||
|
||||
Starts at head (repairs an interrupted earlier run); teardown upgrades
|
||||
to head no matter what happened, so the dev DB is never left below
|
||||
head.
|
||||
"""
|
||||
if not db_available():
|
||||
pytest.skip("Postgres not reachable — run `podman compose up -d db` first")
|
||||
cfg = Config() # no alembic.ini file — env.py gets the URL from app config
|
||||
cfg.set_main_option("script_location", "alembic")
|
||||
command.upgrade(cfg, "head")
|
||||
try:
|
||||
yield cfg
|
||||
finally:
|
||||
command.upgrade(cfg, "head")
|
||||
|
||||
|
||||
def _version(db: Session) -> str | None:
|
||||
return db.execute(text("SELECT version_num FROM alembic_version")).scalar()
|
||||
|
||||
|
||||
def _table_exists(db: Session, table: str) -> bool:
|
||||
count: Any = db.execute(
|
||||
text(
|
||||
"SELECT count(*) FROM information_schema.tables"
|
||||
" WHERE table_schema = 'public' AND table_name = :t"
|
||||
),
|
||||
{"t": table},
|
||||
).scalar()
|
||||
assert count is not None, "information_schema count must be an int"
|
||||
return int(count) == 1
|
||||
|
||||
|
||||
def _column(db: Session, table: str, column: str) -> tuple[Any, ...] | None:
|
||||
"""(data_type, is_nullable, column_default) for one table column."""
|
||||
row = db.execute(
|
||||
text(
|
||||
"SELECT data_type, is_nullable, column_default"
|
||||
" FROM information_schema.columns"
|
||||
" WHERE table_name = :t AND column_name = :c"
|
||||
),
|
||||
{"t": table, "c": column},
|
||||
).fetchone()
|
||||
return tuple(row) if row is not None else None
|
||||
|
||||
|
||||
def _seed_row(db: Session) -> tuple[int, int] | None:
|
||||
"""The (id, version) of the ``sources_meta`` row with id 1."""
|
||||
row = db.execute(
|
||||
text("SELECT id, version FROM sources_meta WHERE id = 1")
|
||||
).fetchone()
|
||||
return tuple(row) if row is not None else None
|
||||
|
||||
|
||||
def _insert(db: Session, version: int | None) -> uuid.UUID:
|
||||
"""Insert one saved_chats row, optionally with an explicit stamp."""
|
||||
if version is None:
|
||||
sql = (
|
||||
"INSERT INTO saved_chats (id, title, messages)"
|
||||
" VALUES (gen_random_uuid(), :t, CAST(:m AS jsonb))"
|
||||
" RETURNING id"
|
||||
)
|
||||
params: dict[str, Any] = {}
|
||||
else:
|
||||
sql = (
|
||||
"INSERT INTO saved_chats (id, title, messages, sources_version)"
|
||||
" VALUES (gen_random_uuid(), :t, CAST(:m AS jsonb), :v)"
|
||||
" RETURNING id"
|
||||
)
|
||||
params = {"v": version}
|
||||
params.update(
|
||||
{"t": "Mig 0010", "m": '[{"who": "user", "text": "How did I install gitlab?"}]'}
|
||||
)
|
||||
chat_id: uuid.UUID = db.execute(text(sql), params).scalar_one()
|
||||
db.commit()
|
||||
return chat_id
|
||||
|
||||
|
||||
def _legacy_insert(db: Session) -> uuid.UUID:
|
||||
"""Insert one row WITHOUT the ``sources_version`` column — the only
|
||||
possible shape at revision 0009 (the column does not exist yet)."""
|
||||
chat_id: uuid.UUID = db.execute(
|
||||
text(
|
||||
"INSERT INTO saved_chats (id, title, messages)"
|
||||
" VALUES (gen_random_uuid(), :t, CAST(:m AS jsonb))"
|
||||
" RETURNING id"
|
||||
),
|
||||
{
|
||||
"t": "Mig 0010",
|
||||
"m": '[{"who": "user", "text": "How did I install gitlab?"}]',
|
||||
},
|
||||
).scalar_one()
|
||||
db.commit()
|
||||
return chat_id
|
||||
|
||||
|
||||
def _delete(db: Session, chat_id: uuid.UUID) -> None:
|
||||
db.execute(text("DELETE FROM saved_chats WHERE id = :i"), {"i": chat_id})
|
||||
db.commit()
|
||||
|
||||
|
||||
def test_upgrade_to_0010_adds_sources_meta_and_stamp(
|
||||
db: Session, alembic: Config
|
||||
) -> None:
|
||||
"""Upgrade 0009 → 0010: the seeded counter table and the NOT NULL
|
||||
stamp column exist; a pre-0010 row comes back stamped 0 (the
|
||||
pre-counter KB)."""
|
||||
command.downgrade(alembic, "0009") # start from the pre-0010 state
|
||||
assert _version(db) == "0009"
|
||||
assert not _table_exists(db, "sources_meta"), "sources_meta must be absent at 0009"
|
||||
assert _column(db, "saved_chats", "sources_version") is None, (
|
||||
"sources_version must be absent at 0009"
|
||||
)
|
||||
|
||||
# A pre-0010 row (no sources_version in the INSERT — the column does
|
||||
# not exist at 0009): its data must survive the additive migration.
|
||||
legacy = _legacy_insert(db)
|
||||
try:
|
||||
command.upgrade(alembic, "0010")
|
||||
assert _version(db) == "0010", "alembic_version must be at 0010"
|
||||
|
||||
id_col = _column(db, "sources_meta", "id")
|
||||
assert id_col is not None, "sources_meta.id is missing"
|
||||
assert id_col[0] == "integer", "sources_meta.id must be INTEGER"
|
||||
assert id_col[1] == "NO", "sources_meta.id must be NOT NULL (PK)"
|
||||
assert id_col[2] == "1", "sources_meta.id must default to 1"
|
||||
|
||||
ver_col = _column(db, "sources_meta", "version")
|
||||
assert ver_col is not None, "sources_meta.version is missing"
|
||||
assert ver_col[0] == "integer", "sources_meta.version must be INTEGER"
|
||||
assert ver_col[1] == "NO", "sources_meta.version must be NOT NULL"
|
||||
assert ver_col[2] == "0", "sources_meta.version must default to 0"
|
||||
|
||||
updated = _column(db, "sources_meta", "updated_at")
|
||||
assert updated is not None, "sources_meta.updated_at is missing"
|
||||
assert updated[0] == "timestamp with time zone", (
|
||||
"sources_meta.updated_at must be TIMESTAMPTZ"
|
||||
)
|
||||
assert updated[1] == "NO", "sources_meta.updated_at must be NOT NULL"
|
||||
assert str(updated[2]).startswith("now("), (
|
||||
"sources_meta.updated_at must have server default now()"
|
||||
)
|
||||
|
||||
assert _seed_row(db) == (1, 0), "the seed row (id 1, version 0) is missing"
|
||||
|
||||
stamp = _column(db, "saved_chats", "sources_version")
|
||||
assert stamp is not None, "saved_chats.sources_version is missing"
|
||||
assert stamp[0] == "integer", "sources_version must be INTEGER"
|
||||
assert stamp[1] == "NO", "sources_version must be NOT NULL"
|
||||
assert stamp[2] == "0", "sources_version must default to 0"
|
||||
|
||||
row = db.execute(
|
||||
text("SELECT title, sources_version FROM saved_chats WHERE id = :i"),
|
||||
{"i": legacy},
|
||||
).fetchone()
|
||||
assert row is not None, "the pre-0010 row must survive the upgrade"
|
||||
assert row[1] == 0, "a pre-0010 row must upgrade stamped 0 (pre-counter KB)"
|
||||
finally:
|
||||
_delete(db, legacy)
|
||||
|
||||
|
||||
def test_inserted_rows_round_trip_the_stamp(db: Session, alembic: Config) -> None:
|
||||
"""At 0010, an omitted stamp defaults to 0 and an explicit stamp
|
||||
round-trips verbatim."""
|
||||
command.upgrade(alembic, "head")
|
||||
default_id = _insert(db, None)
|
||||
explicit_id = _insert(db, 7)
|
||||
try:
|
||||
rows = db.execute(
|
||||
text("SELECT sources_version FROM saved_chats WHERE id IN (:a, :b)"),
|
||||
{"a": default_id, "b": explicit_id},
|
||||
).all()
|
||||
stamps = {row[0] for row in rows}
|
||||
assert stamps == {0, 7}, "default stamp 0 and explicit stamp 7 must round-trip"
|
||||
default_stamp = db.execute(
|
||||
text("SELECT sources_version FROM saved_chats WHERE id = :i"),
|
||||
{"i": default_id},
|
||||
).scalar_one()
|
||||
assert default_stamp == 0, "an omitted stamp must default to 0"
|
||||
finally:
|
||||
_delete(db, default_id)
|
||||
_delete(db, explicit_id)
|
||||
|
||||
|
||||
def test_downgrade_to_0009_drops_both(db: Session, alembic: Config) -> None:
|
||||
"""Downgrade to 0009: the stamp column and the counter table are
|
||||
gone (A13 — reversible) while the rest of ``saved_chats`` survives."""
|
||||
command.downgrade(alembic, "0009")
|
||||
assert _version(db) == "0009"
|
||||
assert _column(db, "saved_chats", "sources_version") is None, (
|
||||
"sources_version must be dropped"
|
||||
)
|
||||
assert not _table_exists(db, "sources_meta"), "sources_meta must be dropped"
|
||||
|
||||
id_col = _column(db, "saved_chats", "id")
|
||||
assert id_col is not None and id_col[0] == "uuid", (
|
||||
"saved_chats.id must survive the downgrade"
|
||||
)
|
||||
token_col = _column(db, "saved_chats", "share_token")
|
||||
assert token_col is not None and token_col[0] == "uuid", (
|
||||
"saved_chats.share_token must survive the downgrade"
|
||||
)
|
||||
|
||||
|
||||
def test_upgrade_round_trip_restores_both(db: Session, alembic: Config) -> None:
|
||||
"""Downgrade to 0009, then upgrade back to 0010: the counter table
|
||||
(with a fresh seed row) and the stamp column are back."""
|
||||
command.downgrade(alembic, "0009")
|
||||
command.upgrade(alembic, "0010")
|
||||
assert _version(db) == "0010", "round-trip upgrade must land at 0010"
|
||||
|
||||
assert _table_exists(db, "sources_meta"), "sources_meta must be back"
|
||||
assert _seed_row(db) == (1, 0), "the seed row must be re-seeded on upgrade"
|
||||
|
||||
stamp = _column(db, "saved_chats", "sources_version")
|
||||
assert stamp is not None, "sources_version must be back after the round-trip"
|
||||
assert stamp[0] == "integer" and stamp[1] == "NO", (
|
||||
"sources_version must be INTEGER NOT NULL after the round-trip"
|
||||
)
|
||||
assert stamp[2] == "0", "sources_version must default to 0 after the round-trip"
|
||||
@@ -39,6 +39,15 @@ real ``LLMClient`` the probe is stubbed (:func:`_stub_probe`) so no
|
||||
test ever hits the network; the ``_real_llm`` tests get a passing
|
||||
probe from ``FakeEmbedder.embed_one``/``chat``.
|
||||
|
||||
Phase 53 (task 02): the sources-version bump — a sync whose import
|
||||
changed the KB (added + updated + pruned > 0) advances the single-row
|
||||
``sources_meta`` counter exactly once (the new generation lands in the
|
||||
``/api/sync/status`` detail as ``sources_version``); an unchanged
|
||||
re-sync never bumps (the detail still reports the current generation),
|
||||
and every failure path (git error, model down) never bumps. The
|
||||
counter is pinned to the migration-0010 seed (0) around every test by
|
||||
:func:`_reset_sources_version`.
|
||||
|
||||
The git / import / overview layers are monkeypatched in ``app.api.sync``
|
||||
(same fake style as ``test_import_docs_git.py``) — no real git, no LLM:
|
||||
the runner's state machine and HTTP surface are under test.
|
||||
@@ -66,11 +75,13 @@ from sqlalchemy.orm import Session
|
||||
|
||||
from app.api import sync as sync_api
|
||||
from app.config import Settings
|
||||
from app.db import SessionLocal, db_available
|
||||
from app.main import app as fastapi_app
|
||||
from app.models import GitSource
|
||||
from app.rag import git_sources as git_sources_resolver
|
||||
from app.rag.importer import ImportSummary
|
||||
from app.rag.llm import EmbeddingError, LLMClient, ModelUnavailableError
|
||||
from app.rag.sources_meta import current_sources_version
|
||||
from scripts.git_sync import GitSyncError
|
||||
from tests.conftest import ADMIN_PASSWORD
|
||||
from tests.fakes import FakeEmbedder
|
||||
@@ -100,6 +111,26 @@ def clean_git_sources(db: Session) -> Iterator[None]:
|
||||
db.commit()
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_sources_version() -> Iterator[None]:
|
||||
"""Phase 53: the sources version counter is global mutable state —
|
||||
pin it to the migration-0010 seed (0) around every sync test so the
|
||||
bump assertions start from a known generation (own session: the
|
||||
runner bumps through its own short-lived ``SessionLocal``).
|
||||
Skips like the ``db`` fixture when Postgres is down."""
|
||||
if not db_available():
|
||||
pytest.skip("Postgres not reachable — run `podman compose up -d db` first")
|
||||
session = SessionLocal()
|
||||
try:
|
||||
session.execute(text("UPDATE sources_meta SET version = 0 WHERE id = 1"))
|
||||
session.commit()
|
||||
yield
|
||||
finally:
|
||||
session.execute(text("UPDATE sources_meta SET version = 0 WHERE id = 1"))
|
||||
session.commit()
|
||||
session.close()
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def sync_client() -> Iterator[TestClient]:
|
||||
"""Context-managed TestClient — one app event loop across requests
|
||||
@@ -310,7 +341,10 @@ def test_admin_sync_success_reports_full_detail(
|
||||
"files": 5, "added": 1, "updated": 2, "unchanged": 2, "pruned": 3,
|
||||
"errors": 0, "chunks": 11, "summaries": 1, "summary_errors": 0,
|
||||
"overview": True,
|
||||
"sources_version": 1, # phase 53: changed KB → exactly one bump (0 → 1)
|
||||
}
|
||||
# The bump committed: the counter advanced exactly once, not twice.
|
||||
assert current_sources_version(db) == 1
|
||||
# Git: the configured repo was cloned into BOR_SOURCES_DIR/<repo-name>/.
|
||||
assert clone_calls == [(repo_url, tmp_path / "bor" / "repo")]
|
||||
# Import: exactly the checkouts, with prune=True (the button is the
|
||||
@@ -354,6 +388,10 @@ def test_unchanged_kb_skips_overview_refresh(
|
||||
assert body["detail"]["overview"] is False
|
||||
assert fake_overview.llms == [] # no wasted model call
|
||||
assert len(fake_import.llms) == 1 # the import itself ran
|
||||
# Phase 53: an unchanged re-sync never bumps — the detail reports
|
||||
# the current (unadvanced) generation.
|
||||
assert body["detail"]["sources_version"] == 0
|
||||
assert current_sources_version(db) == 0
|
||||
|
||||
|
||||
# --- admin: concurrency ----------------------------------------------------
|
||||
@@ -435,6 +473,8 @@ def test_git_failure_marks_failed_and_skips_import(
|
||||
assert body["finished_at"] is not None
|
||||
assert fake_import.sources == [] # no partial import
|
||||
assert fake_overview.llms == []
|
||||
# Phase 53: a FAILED sync never bumps — the version is untouched.
|
||||
assert current_sources_version(db) == 0
|
||||
|
||||
# A failed run leaves the system restartable: a new POST is accepted.
|
||||
assert sync_client.post("/api/sync").status_code == 202
|
||||
@@ -773,6 +813,8 @@ def test_model_down_fails_fast_before_any_clone(
|
||||
assert body["detail"] == {}
|
||||
assert clone_calls == [] # fail fast: before any clone
|
||||
assert fake_import.sources == [] # and before any import
|
||||
# Phase 53: a FAILED sync (model down) never bumps.
|
||||
assert current_sources_version(db) == 0
|
||||
|
||||
|
||||
def test_probe_names_dead_embed_model_and_masks_credentials(
|
||||
|
||||
Reference in New Issue
Block a user