All gates re-verified against a stable tree. Report: **Verified (all 6 tasks in `complete/`; this was the final pass)** - `SourceLabel`+alembic `0030` (dev DB `alembic current` = `0030 (head)`, table present, 0 rows); admin-only `GET/PUT/DELETE /api/source-labels` (`require_admin` router-wide, `resolve_source_name`, partial-update caps/404s); Sources-page "Names, labels and tags" card (read-only derived-name `<code>`, per-row `saveInFlight`, 404 note that survives the settle, chips reuse `.git-source-hidden-count`); `question_name_hits` D2 rule; ONE `all_labels` read per turn in `chat.py` feeding both the `source_name_hit` flag and the prompt surfaces; `source_name_hit_bonus=0.005` in `_selection_order`; ` name=`/` label=` in `build_high_prompt` + `build_overview_prompt(names=…)`. - **Completion criteria:** E2E `tests/e2e/test_source_name_labels.py` ✅ 7 passed (arms a–e: editor all kinds, control/named question, rename-never-breaks-identity, 404 note, reset) · full suite ✅ · coverage ✅ · README/AGENTS ✅ · phase dir in `complete/` ✅ · commit left to the harness. - **Gates:** `uv run pytest --cov=app --cov-report=term-missing` → **exit 0, TOTAL 7160 stmts / 16 missed = 99%** (>90%) · `uv run pytest tests/e2e/test_source_name_labels.py -v --no-cov` → **7 passed, exit 0** · regression E2E isolated, exit 0: `test_document_viewer`, `test_admin_auth`, `test_api_tokens`, `test_git_sources_admin`, `test_kb_tree`, `test_local_directory_sources` · `uv run ruff check .` → All checks passed · `uv run pyright` → 0 errors. - **Battery (task 04 criterion):** `uv run python -m scripts.eval_retrieval --from-file tests/fixtures/retrieval_battery.txt` at default vs `BOR_SOURCE_NAME_HIT_BONUS=0` → **`diff` empty (line-for-line identical)**, matching the recorded arms (dev KB is empty, so every question deflects; the observable boost is unit/E2E-pinned). - **Documentation:** README — fixed the `/git-sources.html` rows (nav label is **Sources**) and cross-linked the phase-191 card from that section; full audit otherwise clean: all 86 `BOR_*` settings documented with defaults matching `app/config.py`, 0 broken file/dir references and 0 broken anchors in README+AGENTS, AGENTS rule 17 re-checked line-by-line against the code. - **Notable / deviations:** (1) a **concurrent pipeline process** (`phased-execution/progress.mjs` for `191_…_00_phase`) was mutating this same working tree mid-pass — it rewrote `tests/e2e/test_source_name_labels.py` (11→7 tests), `frontend/assets/git-sources.js`, `tests/unit/test_source_labels_editor_ui.py` and `AGENTS.md` at 05:27; every gate above was (re-)run after those edits with the tree no source file newer than the green full run. (2) The task-06 report/phase text claims regression suites `test_chat_token_gate.py`, `test_git_sources_page.py`, `test_rag_browser.py` are green — **those files have never existed in git history** (those runs exit 4); I ran the real equivalents listed above instead (phase files are not mine to edit). (3) **No live changes**: the VM DB already has `source_labels` (0 rows) but the deployed app predates this uncommitted code, so deploying `/api/source-labels` to the live service is the owner's post-commit step, not part of this phase's criteria. - **Next pending phase:** none — `191_source_name_labels_tags` is the only entry in `.agents/phases/todo/`.
312 lines
13 KiB
Python
312 lines
13 KiB
Python
"""Integration: migration 0030 (source_labels) schema contract
|
|
(phase 191, task 01).
|
|
|
|
Drives the **real Alembic engine** against the live dev database
|
|
(``podman compose up -d db``), mirroring the house pattern of
|
|
``test_migration_0028.py`` (information_schema assertions on the state
|
|
the migration must leave). The tests target the 0029 → 0030 step
|
|
explicitly so later migrations cannot break them:
|
|
|
|
* upgrade 0029 → 0030 → the ``source_labels`` table exists with the
|
|
full column contract (D1) — ``source`` VARCHAR(120) NOT NULL and the
|
|
sole primary key (one label row per source; the join key is the
|
|
DERIVED ``documents.source`` name — an alias key, never a rename),
|
|
``display_name`` / ``label`` TEXT nullable (the ≤120 / ≤200 caps are
|
|
API-boundary concerns, the phase-121 plain-column pattern), ``tags``
|
|
JSONB NOT NULL with the ``'[]'`` server default (the phase-89
|
|
``ignore_paths`` shape — a source with no tags reads ``[]``, never
|
|
NULL), ``updated_at`` TIMESTAMPTZ NOT NULL with the now() server
|
|
default (house style) — while the 0029 schema
|
|
(``ix_chunks_embedding_hnsw``) survives;
|
|
* the NO-BACKFILL contract: right after the upgrade the table is EMPTY
|
|
(a source without a row behaves exactly as pre-phase);
|
|
* inserted rows round-trip (name/label/tags verbatim, ``updated_at``
|
|
stamped by the server default);
|
|
* a duplicate ``source`` raises ``IntegrityError`` (the PK pin) and a
|
|
NULL ``tags`` is rejected (NOT NULL — the reset is ``[]``, never
|
|
NULL);
|
|
* downgrade to 0029 → the table is GONE (A13 — reversible), the rest of
|
|
the schema (the HNSW index, ``app_settings``) survives;
|
|
* upgrade back to 0030 → the table is 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
|
|
|
|
from collections.abc import Iterator
|
|
from typing import Any
|
|
|
|
import pytest
|
|
from alembic.config import Config
|
|
from sqlalchemy import text
|
|
from sqlalchemy.exc import IntegrityError
|
|
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:
|
|
# Release the test session's open transaction BEFORE the repair
|
|
# DDL (the 0028 fixture's lesson): psycopg3 keeps its snapshot
|
|
# alive while idle-in-transaction, and a later 0029
|
|
# CREATE INDEX CONCURRENTLY would wait on it forever.
|
|
db.rollback()
|
|
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:
|
|
return (
|
|
db.execute(
|
|
text("SELECT 1 FROM information_schema.tables WHERE table_name = :t"),
|
|
{"t": table},
|
|
).scalar()
|
|
is not None
|
|
)
|
|
|
|
|
|
def _column(db: Session, table: str, column: str) -> tuple[Any, ...] | None:
|
|
"""(data_type, is_nullable, column_default, character_maximum_length)
|
|
for one table column."""
|
|
row = db.execute(
|
|
text(
|
|
"SELECT data_type, is_nullable, column_default, character_maximum_length"
|
|
" 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 _pk_columns(db: Session, table: str) -> list[str]:
|
|
"""The table's PRIMARY KEY columns in ordinal position."""
|
|
rows = db.execute(
|
|
text(
|
|
"SELECT kcu.column_name"
|
|
" FROM information_schema.table_constraints tc"
|
|
" JOIN information_schema.key_column_usage kcu"
|
|
" ON kcu.constraint_name = tc.constraint_name"
|
|
" AND kcu.table_name = tc.table_name"
|
|
" WHERE tc.table_name = :t"
|
|
" AND tc.constraint_type = 'PRIMARY KEY'"
|
|
" ORDER BY kcu.ordinal_position"
|
|
),
|
|
{"t": table},
|
|
).fetchall()
|
|
return [r[0] for r in rows]
|
|
|
|
|
|
def _hnsw_index_exists(db: Session) -> bool:
|
|
return (
|
|
db.execute(
|
|
text(
|
|
"SELECT 1 FROM pg_indexes"
|
|
" WHERE indexname = 'ix_chunks_embedding_hnsw'"
|
|
)
|
|
).scalar()
|
|
is not None
|
|
)
|
|
|
|
|
|
def _clear_rows(db: Session) -> None:
|
|
db.execute(text("DELETE FROM source_labels"))
|
|
db.commit()
|
|
|
|
|
|
def test_upgrade_to_0030_creates_source_labels(db: Session, alembic: Config) -> None:
|
|
"""Upgrade 0029 → 0030: the table exists with the full D1 column
|
|
contract, the table is ABSENT at 0029, and the 0029 HNSW index
|
|
survives the additive upgrade."""
|
|
command.downgrade(alembic, "0029") # start from the pre-0030 state
|
|
assert _version(db) == "0029"
|
|
assert not _table_exists(db, "source_labels"), (
|
|
"source_labels must be absent at 0029"
|
|
)
|
|
|
|
command.upgrade(alembic, "0030")
|
|
assert _version(db) == "0030", "alembic_version must be at 0030"
|
|
assert _table_exists(db, "source_labels"), "the table must exist at 0030"
|
|
|
|
source = _column(db, "source_labels", "source")
|
|
assert source is not None, "source_labels.source is missing"
|
|
assert source[0] == "character varying", "source must be VARCHAR"
|
|
assert source[1] == "NO", "source must be NOT NULL (PK)"
|
|
assert source[2] is None, "source must have no server default"
|
|
assert source[3] == 120, "source must be String(120) (mirrors documents.source)"
|
|
|
|
display = _column(db, "source_labels", "display_name")
|
|
assert display is not None, "source_labels.display_name is missing"
|
|
assert display[0] == "text", "display_name must be plain TEXT (caps are API-side)"
|
|
assert display[1] == "YES", "display_name must be nullable (NULL = no name)"
|
|
|
|
label = _column(db, "source_labels", "label")
|
|
assert label is not None, "source_labels.label is missing"
|
|
assert label[0] == "text", "label must be plain TEXT (caps are API-side)"
|
|
assert label[1] == "YES", "label must be nullable (NULL = no label)"
|
|
|
|
tags = _column(db, "source_labels", "tags")
|
|
assert tags is not None, "source_labels.tags is missing"
|
|
assert tags[0] == "jsonb", "tags must be JSONB (the ignore_paths shape)"
|
|
assert tags[1] == "NO", "tags must be NOT NULL"
|
|
assert tags[2] is not None and "[]" in str(tags[2]), (
|
|
"tags must carry the '[]' server default (no tags reads [], never NULL)"
|
|
)
|
|
|
|
updated = _column(db, "source_labels", "updated_at")
|
|
assert updated is not None, "source_labels.updated_at is missing"
|
|
assert updated[0] == "timestamp with time zone", "updated_at must be TIMESTAMPTZ"
|
|
assert updated[1] == "NO", "updated_at must be NOT NULL"
|
|
assert updated[2] is not None and "now" in str(updated[2]), (
|
|
"updated_at must carry the now() server default (house style)"
|
|
)
|
|
|
|
assert _pk_columns(db, "source_labels") == ["source"], (
|
|
"source must be the sole primary key (one label row per source)"
|
|
)
|
|
|
|
# The NO-BACKFILL contract (D1): the migration creates an EMPTY
|
|
# table — every source keeps its pre-phase "no row" behavior.
|
|
count = db.execute(text("SELECT count(*) FROM source_labels")).scalar()
|
|
assert count == 0, "0030 must not backfill any label rows"
|
|
|
|
# The 0029 schema survives the additive upgrade.
|
|
assert _hnsw_index_exists(db), "ix_chunks_embedding_hnsw (0029) must survive"
|
|
|
|
|
|
def test_inserted_rows_round_trip(db: Session, alembic: Config) -> None:
|
|
"""At head, the D1 values round-trip verbatim (name / label / tags)
|
|
and ``updated_at`` is stamped by the server default."""
|
|
command.upgrade(alembic, "head")
|
|
try:
|
|
db.execute(
|
|
text(
|
|
"INSERT INTO source_labels (source, display_name, label, tags)"
|
|
" VALUES ('gitea-forge', 'the git server',"
|
|
" 'self-hosted forge + Gitea', '[\"gitea\", \"forge\"]'::jsonb)"
|
|
)
|
|
)
|
|
db.execute(
|
|
text(
|
|
"INSERT INTO source_labels (source)"
|
|
" VALUES ('bare-row')"
|
|
)
|
|
)
|
|
db.commit()
|
|
rows = db.execute(
|
|
text(
|
|
"SELECT source, display_name, label, tags, updated_at"
|
|
" FROM source_labels ORDER BY source"
|
|
)
|
|
).fetchall()
|
|
assert len(rows) == 2, "both rows must be stored"
|
|
assert rows[0][0] == "bare-row"
|
|
assert rows[0][1] is None and rows[0][2] is None
|
|
assert rows[0][3] == [], "the server default must read as the empty list"
|
|
assert rows[0][4] is not None, "updated_at must be stamped (server default)"
|
|
assert rows[1][0] == "gitea-forge", "source must round-trip verbatim"
|
|
assert rows[1][1] == "the git server"
|
|
assert rows[1][2] == "self-hosted forge + Gitea"
|
|
assert rows[1][3] == ["gitea", "forge"], "the tags array must round-trip"
|
|
finally:
|
|
_clear_rows(db)
|
|
|
|
|
|
def test_duplicate_source_is_rejected(db: Session, alembic: Config) -> None:
|
|
"""The PK is the uniqueness guard: two label rows for one source
|
|
would make "which name wins" undefined, so the second insert must
|
|
raise ``IntegrityError``."""
|
|
command.upgrade(alembic, "head")
|
|
try:
|
|
db.execute(
|
|
text("INSERT INTO source_labels (source) VALUES ('homelab')")
|
|
)
|
|
db.commit()
|
|
with pytest.raises(IntegrityError):
|
|
db.execute(
|
|
text(
|
|
"INSERT INTO source_labels (source, display_name)"
|
|
" VALUES ('homelab', 'second name')"
|
|
)
|
|
)
|
|
db.rollback() # the IntegrityError aborts the open transaction
|
|
finally:
|
|
_clear_rows(db)
|
|
|
|
|
|
def test_null_tags_is_rejected(db: Session, alembic: Config) -> None:
|
|
"""``tags`` is NOT NULL: "no tags" is ``[]`` (the server default),
|
|
never NULL — readers never branch on the None case."""
|
|
command.upgrade(alembic, "head")
|
|
try:
|
|
with pytest.raises(IntegrityError):
|
|
db.execute(
|
|
text(
|
|
"INSERT INTO source_labels (source, tags)"
|
|
" VALUES ('nulltags', NULL)"
|
|
)
|
|
)
|
|
db.rollback() # the IntegrityError aborts the open transaction
|
|
stored = db.execute(text("SELECT count(*) FROM source_labels")).scalar()
|
|
assert stored == 0, "the rejected row must not be stored"
|
|
finally:
|
|
_clear_rows(db)
|
|
|
|
|
|
def test_downgrade_to_0029_drops_the_table(db: Session, alembic: Config) -> None:
|
|
"""Downgrade 0030 → 0029: the table is gone (A13 — fully
|
|
reversible) while the rest of the schema survives (the 0029 HNSW
|
|
index, ``app_settings``). The names/labels/tags are re-enterable
|
|
from the Sources page — nothing outside this table was mutated."""
|
|
command.downgrade(alembic, "0029")
|
|
assert _version(db) == "0029"
|
|
assert not _table_exists(db, "source_labels"), "source_labels must be dropped"
|
|
|
|
assert _hnsw_index_exists(db), "ix_chunks_embedding_hnsw (0029) must survive"
|
|
strip = _column(db, "documents", "page_strip_mode")
|
|
assert strip is not None and strip[3] == 16, (
|
|
"documents.page_strip_mode must survive the downgrade"
|
|
)
|
|
key = _column(db, "app_settings", "key")
|
|
assert key is not None and key[3] == 100, "app_settings.key must survive"
|
|
|
|
|
|
def test_upgrade_round_trip_restores_the_table(db: Session, alembic: Config) -> None:
|
|
"""Downgrade to 0029, then upgrade back to 0030: the table is back
|
|
with the column contract and PK intact (A13 round-trip)."""
|
|
command.downgrade(alembic, "0029")
|
|
command.upgrade(alembic, "0030")
|
|
assert _version(db) == "0030", "round-trip upgrade must land at 0030"
|
|
assert _table_exists(db, "source_labels"), "the table must be back"
|
|
|
|
source = _column(db, "source_labels", "source")
|
|
assert source is not None, "source_labels.source must be back"
|
|
assert source[0] == "character varying", "source must be VARCHAR after the round-trip"
|
|
assert source[1] == "NO", "source must be NOT NULL after the round-trip"
|
|
assert source[3] == 120, "source must be String(120) after the round-trip"
|
|
tags = _column(db, "source_labels", "tags")
|
|
assert tags is not None, "source_labels.tags must be back"
|
|
assert tags[0] == "jsonb", "tags must be JSONB after the round-trip"
|
|
assert tags[1] == "NO", "tags must be NOT NULL after the round-trip"
|
|
assert _pk_columns(db, "source_labels") == ["source"]
|