phase: 191_source_name_labels_tags
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/`.
This commit is contained in:
@@ -10,13 +10,22 @@ pins the deterministic ``Sources: …`` inventory tail (LOCKED A2): the
|
||||
format/order/counts, the 1-doc survival, the happy-path stored content,
|
||||
the byte-identical zero-doc / ``LLMError`` paths, and the byte-identical
|
||||
model input.
|
||||
|
||||
And the phase-191 display names (task 05, D4): the INPUT lines lead with
|
||||
the owner's ``display_name`` when the source has a label row (the raw
|
||||
source name is replaced in the ``lite`` model's input ONLY — the stored
|
||||
row and its ``Sources: …`` tail keep the derived name), byte-identical
|
||||
for every unnamed source, and fed by exactly ONE ``all_labels`` read per
|
||||
run (the marker pins at the end).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import inspect
|
||||
import logging
|
||||
import uuid
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from sqlalchemy import select, text
|
||||
@@ -24,6 +33,7 @@ from sqlalchemy.orm import Session
|
||||
|
||||
from app.config import Settings, get_settings
|
||||
from app.models import Document, KbOverview
|
||||
from app.rag import overview as overview_module
|
||||
from app.rag.llm import LLMError
|
||||
from app.rag.overview import (
|
||||
KB_OVERVIEW_MODE,
|
||||
@@ -35,6 +45,8 @@ from app.rag.overview import (
|
||||
regenerate_overview,
|
||||
)
|
||||
from app.rag.retriever import TRUNCATION_MARKER
|
||||
from app.rag.source_labels import all_labels as real_all_labels
|
||||
from app.rag.source_labels import set_label
|
||||
|
||||
REPLY = "- Homelab\n - Kubernetes (k3s)\n- Deployments\n - Borg backups"
|
||||
|
||||
@@ -650,3 +662,208 @@ def test_regenerate_llm_error_preserves_an_existing_inventory_tail(
|
||||
assert row is not None and row.content == previous
|
||||
finally:
|
||||
_truncate_documents(db)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 191 (D4, task 05): the input lines carry the owner's display name
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
#: The owner's alias for the ``gitea-infra`` source (phase 191 D1 — an
|
||||
#: alias, never the identity).
|
||||
GIT_SERVER = "the git server"
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def clean_source_labels(db: Session):
|
||||
"""Empty ``source_labels`` before and after (migration-0030 state)."""
|
||||
db.execute(text("DELETE FROM source_labels"))
|
||||
db.commit()
|
||||
yield
|
||||
db.execute(text("DELETE FROM source_labels"))
|
||||
db.commit()
|
||||
|
||||
|
||||
# ---------- build_overview_prompt: the names map (pure) ----------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"names",
|
||||
[None, {}, {"Other": "another name"}],
|
||||
ids=["names=None", "names={}", "no-entry-for-the-source"],
|
||||
)
|
||||
def test_input_lines_without_a_names_entry_are_byte_identical_to_pre_phase(
|
||||
names: dict[str, str] | None,
|
||||
) -> None:
|
||||
"""THE kill pin (phase 191 D4): no map, an empty map and a map that
|
||||
misses the row's source each build the pre-phase input byte-for-byte —
|
||||
the same cap, the same separator, the same order."""
|
||||
rows = [
|
||||
("Homelab", "k8s.md", "Kubernetes", "A cluster.\nSource: Homelab/k8s.md"),
|
||||
("Homelab", "borg.md", "Borg", None),
|
||||
]
|
||||
pre = build_overview_prompt(rows, max_chars=10_000)
|
||||
assert build_overview_prompt(rows, max_chars=10_000, names=names) == pre
|
||||
|
||||
|
||||
def test_input_line_leads_with_the_display_name_when_named() -> None:
|
||||
"""A named source: the display name REPLACES the raw source name in the
|
||||
lite model's input — the path / title / summary halves untouched."""
|
||||
rows = [("gitea-infra", "docs/install.md", "Install", "The install notes.")]
|
||||
_system, user = build_overview_prompt(rows, names={"gitea-infra": GIT_SERVER})
|
||||
assert user == f"{GIT_SERVER} — docs/install.md — Install — The install notes."
|
||||
assert "gitea-infra" not in user
|
||||
|
||||
|
||||
def test_input_line_without_an_entry_keeps_the_raw_source_name() -> None:
|
||||
"""Mixed catalogue: only the named source's lines change — every other
|
||||
line stays exactly as the pre-phase builder wrote it."""
|
||||
rows = [
|
||||
("gitea-infra", "a.md", "A", "Sum A."),
|
||||
("Homelab", "b.md", "B", "Sum B."),
|
||||
]
|
||||
_system, user = build_overview_prompt(rows, names={"gitea-infra": GIT_SERVER})
|
||||
assert user.splitlines() == [
|
||||
f"{GIT_SERVER} — a.md — A — Sum A.",
|
||||
"Homelab — b.md — B — Sum B.",
|
||||
]
|
||||
|
||||
|
||||
def test_named_summary_less_line_has_no_dangling_dash() -> None:
|
||||
"""The pre-phase "no summary → no trailing field" rule survives the
|
||||
name swap."""
|
||||
_system, user = build_overview_prompt(
|
||||
[("gitea-infra", "a.md", "A", None)], names={"gitea-infra": GIT_SERVER}
|
||||
)
|
||||
assert user == f"{GIT_SERVER} — a.md — A"
|
||||
|
||||
|
||||
def test_named_entry_with_an_empty_display_name_falls_back_to_the_source() -> None:
|
||||
"""Defensive: an empty name in the map is "no name" (the API trims
|
||||
empty submissions to NULL, so this never reaches the wire — the input
|
||||
line must never lead with a blank)."""
|
||||
_system, user = build_overview_prompt(
|
||||
[("gitea-infra", "a.md", "A", None)], names={"gitea-infra": ""}
|
||||
)
|
||||
assert user == "gitea-infra — a.md — A"
|
||||
|
||||
|
||||
def test_names_take_part_in_the_input_cap() -> None:
|
||||
"""The cap counts the whole line (the name included) — the truncation
|
||||
contract is unchanged: cut at the cap, marker on its own line."""
|
||||
rows = [("gitea-infra", "a.md", "A", None)]
|
||||
_system, plain = build_overview_prompt(rows, max_chars=10)
|
||||
_system, named = build_overview_prompt(rows, max_chars=10, names={"gitea-infra": GIT_SERVER})
|
||||
assert named != plain
|
||||
assert named.endswith(TRUNCATION_MARKER)
|
||||
assert named.startswith(GIT_SERVER[:10])
|
||||
|
||||
|
||||
# ---------- regenerate_overview: the ONE all_labels read (DB) ----------
|
||||
|
||||
|
||||
def test_regenerate_model_input_carries_the_display_name(
|
||||
db: Session, clean_overview, clean_source_labels
|
||||
) -> None:
|
||||
"""The wire half of D4: the single read feeds the input lines, and the
|
||||
unnamed source's line is untouched."""
|
||||
_truncate_documents(db)
|
||||
set_label(db, "gitea-infra", display_name=GIT_SERVER)
|
||||
db.commit()
|
||||
try:
|
||||
base = datetime(2024, 6, 15, 12, 0, 0, tzinfo=UTC)
|
||||
_add_doc(db, "gitea-infra", "a.md", "A", "Sum A.", created_at=base + timedelta(days=1))
|
||||
_add_doc(db, "Homelab", "b.md", "B", "Sum B.", created_at=base)
|
||||
llm = _FakeLLM()
|
||||
assert asyncio.run(regenerate_overview(llm, db)) is True
|
||||
user = llm.messages[1]["content"]
|
||||
assert user.splitlines() == [
|
||||
f"{GIT_SERVER} — a.md — A — Sum A.",
|
||||
"Homelab — b.md — B — Sum B.",
|
||||
]
|
||||
finally:
|
||||
_truncate_documents(db)
|
||||
|
||||
|
||||
def test_regenerate_stored_content_keeps_the_derived_names(
|
||||
db: Session, clean_overview, clean_source_labels
|
||||
) -> None:
|
||||
"""The ALIAS contract (D1) on the stored row: the deterministic
|
||||
``Sources: …`` inventory tail names sources by their DERIVED name —
|
||||
a rename never changes the identity the rest of the app keys on."""
|
||||
_truncate_documents(db)
|
||||
set_label(db, "gitea-infra", display_name=GIT_SERVER, label="self-hosted forge")
|
||||
db.commit()
|
||||
try:
|
||||
_add_doc(db, "gitea-infra", "a.md", "A")
|
||||
assert asyncio.run(regenerate_overview(_FakeLLM(), db)) is True
|
||||
row = db.get(KbOverview, 1)
|
||||
assert row is not None
|
||||
assert row.content == REPLY + "\nSources: gitea-infra (1 doc)"
|
||||
assert GIT_SERVER not in row.content
|
||||
finally:
|
||||
_truncate_documents(db)
|
||||
|
||||
|
||||
def test_regenerate_without_label_rows_is_byte_identical_to_pre_phase(
|
||||
db: Session, clean_overview, clean_source_labels
|
||||
) -> None:
|
||||
"""No label rows (the whole-KB kill pin): the model input and the
|
||||
stored content are exactly the pre-phase bytes."""
|
||||
_truncate_documents(db)
|
||||
try:
|
||||
_add_doc(db, "Homelab", "a.md", "A", "Sum A.")
|
||||
llm = _FakeLLM()
|
||||
assert asyncio.run(regenerate_overview(llm, db)) is True
|
||||
assert llm.messages[1]["content"] == "Homelab — a.md — A — Sum A."
|
||||
row = db.get(KbOverview, 1)
|
||||
assert row is not None and row.content == REPLY + "\nSources: Homelab (1 doc)"
|
||||
finally:
|
||||
_truncate_documents(db)
|
||||
|
||||
|
||||
def test_regenerate_reads_the_label_rows_once(
|
||||
db: Session, clean_overview, clean_source_labels, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""ONE ``all_labels`` read per run (the task's single-query contract):
|
||||
the regenerator calls it exactly once and derives the name map from
|
||||
those views — no second query."""
|
||||
_truncate_documents(db)
|
||||
set_label(db, "gitea-infra", display_name=GIT_SERVER)
|
||||
db.commit()
|
||||
calls: list[int] = []
|
||||
|
||||
def _counting_all_labels(session: Session) -> dict:
|
||||
calls.append(1)
|
||||
return real_all_labels(session)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"app.rag.overview.all_labels",
|
||||
_counting_all_labels,
|
||||
)
|
||||
try:
|
||||
_add_doc(db, "gitea-infra", "a.md", "A")
|
||||
_add_doc(db, "gitea-infra", "b.md", "B")
|
||||
assert asyncio.run(regenerate_overview(_FakeLLM(), db)) is True
|
||||
assert len(calls) == 1
|
||||
finally:
|
||||
_truncate_documents(db)
|
||||
|
||||
|
||||
# ---------- the marker pins: one read, in the right function ----------
|
||||
|
||||
OVERVIEW_PY = Path(overview_module.__file__).resolve().read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_overview_module_issues_exactly_one_all_labels_read() -> None:
|
||||
"""The marker pin: the ONE ``all_labels(` call in the module — the
|
||||
overview builder never queries labels on its own."""
|
||||
assert OVERVIEW_PY.count("all_labels(") == 1
|
||||
|
||||
|
||||
def test_the_single_read_lives_in_regenerate_overview() -> None:
|
||||
"""The read sits in the caller (which already holds the session), not
|
||||
in the pure builder — so the builder stays a pure function of its
|
||||
arguments and one read covers the whole run."""
|
||||
src = inspect.getsource(regenerate_overview)
|
||||
assert src.count("all_labels(") == 1
|
||||
assert "all_labels(" not in inspect.getsource(build_overview_prompt)
|
||||
|
||||
Reference in New Issue
Block a user