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/`.
250 lines
11 KiB
Python
250 lines
11 KiB
Python
"""Source name/label/tags admin API (phase 191, task 02).
|
|
|
|
``TODO.md`` L4: "Sources need a name, a label, tag(s) so you can focus
|
|
the model when prompting if you have a project name or a standard name
|
|
you reference things by." Task 01 stored that (the ``source_labels``
|
|
table + :mod:`app.rag.source_labels`); this module is its whole HTTP
|
|
surface — the API the Sources page's per-source editor (task 03)
|
|
drives, admin-gated like the phase-91 Theme tab and the phase-168
|
|
Settings panel.
|
|
|
|
Routes (under ``/api`` via the ``main`` registration — the house prefix
|
|
convention, the :mod:`app.api.ui_settings` /
|
|
:mod:`app.api.admin_settings` style, so the wire paths are exactly the
|
|
D5 ones):
|
|
|
|
* ``GET /api/source-labels`` — the FULL source catalogue: (a) every
|
|
``git_sources`` row's DERIVED source name (git AND local rows —
|
|
:func:`app.rag.source_removal.resolve_source_name`, the one helper
|
|
that mirrors the import pipeline's naming for both kinds — and, while
|
|
the table is empty, the ``BOR_GIT_SOURCES`` env names, which
|
|
:func:`app.rag.git_sources.effective_sources` already surfaces as
|
|
synthetic git rows), plus (b) every indexed document source no row
|
|
derives (the uploads — ONE ``SELECT DISTINCT source``, no ``content``,
|
|
the phase-126 load discipline). De-duplicated (the phase-69 sibling
|
|
case: two rows, one name) and sorted by ``source``, so the page and
|
|
the phase's E2E always see the same order. Each entry carries its
|
|
label row or the ``null``/``[]`` defaults — one
|
|
:func:`app.rag.source_labels.all_labels` read for the whole list,
|
|
never a query per source. Reads only; creates nothing.
|
|
* ``PUT /api/source-labels/{source}`` — the upsert. The path param is
|
|
capped at the ``documents.source`` 120 (the framework's 422 above
|
|
it); a ``source`` matching NO registry row AND NO indexed document is
|
|
a 404 BEFORE anything is written (D5: no orphan label rows — a label
|
|
for a source that exists nowhere would be a dead boost input, and
|
|
nothing in the UI could ever list it). The body is the partial-update
|
|
shape (the phase-168 precedent): an omitted field keeps its stored
|
|
value, an explicit ``null`` clears it, a value sets it. The tags go
|
|
through :func:`app.rag.source_labels.normalize_tags` — the ONE tag
|
|
rule (trim, case-insensitive de-dupe, first case wins) — and its
|
|
``ValueError`` becomes the framework's 422, NEVER a 500. The router
|
|
owns the session here (the phase-91 commit convention): ``set_label``
|
|
flushes, this route commits, and a rejected request writes NOTHING.
|
|
200 → the stored :class:`SourceLabelOut`.
|
|
* ``DELETE /api/source-labels/{source}`` — the reset: the row goes, 404
|
|
when there is no row to remove. Like the phase-168
|
|
``DELETE /api/admin/settings/{key}`` the 200 body is the entry's
|
|
post-reset state — the same shape with the fields cleared — so the
|
|
row re-renders from the response it already has.
|
|
|
|
One invariant the routes exist to protect (D1, owner-confirmed): a
|
|
label row is an ALIAS. ``Document.source``, the ``source/path`` document
|
|
identity, the read/grep/``show_text`` locators, the citations and the KB
|
|
tree are ALL untouched by anything here — calling "gitea-forge" "the git
|
|
server" changes only what the lite model and the chat model are TOLD
|
|
(tasks 04/05), never what a saved chat, a shared page or a verbatim
|
|
panel points at. That is why ``source`` is a path param that is LOOKED
|
|
UP and never rewritten, and why the catalogue's identity column stays
|
|
the derived name.
|
|
|
|
Logging (AGENTS rule 10): one INFO line per WRITE, carrying the action
|
|
and the source, never the values (the ``admin_settings`` discipline —
|
|
the owner's text belongs in the row, not in ``server.log``). GETs log
|
|
nothing here: :class:`app.core.response_budget.ResponseBudgetMiddleware`
|
|
already writes the per-response ``api method=… bytes=…`` line for every
|
|
``/api/*`` route.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import logging
|
|
from typing import Annotated, Any
|
|
|
|
from fastapi import APIRouter, Depends, HTTPException, Path
|
|
from sqlalchemy import select
|
|
from sqlalchemy.orm import Session
|
|
|
|
from app.core.auth import require_admin
|
|
from app.db import get_db
|
|
from app.models import Document
|
|
from app.rag.git_sources import effective_sources
|
|
from app.rag.source_labels import (
|
|
SourceLabelView,
|
|
all_labels,
|
|
delete_label,
|
|
normalize_tags,
|
|
set_label,
|
|
)
|
|
from app.rag.source_removal import resolve_source_name
|
|
from app.schemas import SourceLabelList, SourceLabelOut, SourceLabelUpdate
|
|
|
|
logger = logging.getLogger("app.api.source_labels")
|
|
|
|
router = APIRouter(
|
|
prefix="/source-labels",
|
|
tags=["source-labels"],
|
|
dependencies=[Depends(require_admin)], # phase 191: the owner's naming surface
|
|
)
|
|
|
|
|
|
def _catalogue_names(db: Session) -> list[str]:
|
|
"""Every source the owner can label, de-duplicated and sorted.
|
|
|
|
Exactly the two reads the D5 catalogue is made of — no third:
|
|
|
|
(a) the registry: :func:`app.rag.git_sources.effective_sources` —
|
|
the ``git_sources`` rows of BOTH kinds in ``(added_at, id)``
|
|
order, or the synthetic env rows while the table is empty —
|
|
named by :func:`app.rag.source_removal.resolve_source_name`, the
|
|
single helper that spells a row's name the way the importer
|
|
stores it in ``Document.source`` (``repo_name`` of the URL for
|
|
git rows, the expanded path's basename for local rows). Reused,
|
|
never re-derived: an expression copied here would drift from the
|
|
importer and the label rows would key nothing.
|
|
(b) the indexed documents: ``SELECT DISTINCT source`` — one query
|
|
over the indexed corpus (the column only, never the entity: no
|
|
``content``, phase-126/185 discipline), so an upload — or any
|
|
document whose registry row is gone — still lists and stays
|
|
labelable.
|
|
|
|
Sorted because the page renders this order and the phase's E2E pins
|
|
it; the set collapses the phase-69 sibling case (two rows resolving
|
|
to one name) to a single entry.
|
|
"""
|
|
rows, _origin = effective_sources(db)
|
|
names: set[str] = {resolve_source_name(row) for row in rows}
|
|
names.update(db.scalars(select(Document.source).distinct()).all())
|
|
return sorted(names)
|
|
|
|
|
|
def _shape(source: str, view: SourceLabelView | None) -> SourceLabelOut:
|
|
"""One entry of the catalogue: the label row's values, or the
|
|
``null``/``[]`` defaults when the source has no row (the pre-phase
|
|
state — indistinguishable from a cleared label, and that is the
|
|
point: D1 says a label is data beside the name, never a rename)."""
|
|
return SourceLabelOut(
|
|
source=source,
|
|
display_name=view.display_name if view else None,
|
|
label=view.label if view else None,
|
|
tags=list(view.tags) if view else [],
|
|
)
|
|
|
|
|
|
@router.get("", response_model=SourceLabelList)
|
|
def list_source_labels(
|
|
db: Session = Depends(get_db), # noqa: B008
|
|
) -> SourceLabelList:
|
|
"""The catalogue: every source of every kind, each with its label or
|
|
the null defaults, sorted by ``source``.
|
|
|
|
Two reads for the names (:func:`_catalogue_names`) plus ONE for all
|
|
the label rows — the table carries a handful of rows, so the list is
|
|
assembled in Python instead of issuing a lookup per source (the
|
|
phase-119 one-query convention).
|
|
|
|
The catalogue is the SOURCE list, not the label list: a label row
|
|
whose source has since lost both its registry row and its documents
|
|
does not appear here (it can boost nothing — the D2 match runs
|
|
against retrieved chunks' ``document.source``), and it stays
|
|
clearable through ``DELETE`` below, which never consults the
|
|
registry.
|
|
"""
|
|
labels = all_labels(db)
|
|
return SourceLabelList(
|
|
labels=[_shape(name, labels.get(name)) for name in _catalogue_names(db)]
|
|
)
|
|
|
|
|
|
@router.put("/{source}", response_model=SourceLabelOut)
|
|
def put_source_label(
|
|
source: Annotated[str, Path(max_length=120)],
|
|
payload: SourceLabelUpdate,
|
|
db: Session = Depends(get_db), # noqa: B008
|
|
) -> SourceLabelOut:
|
|
"""Set or clear one source's name/label/tags (200 → the stored row).
|
|
|
|
The order is the D5 contract:
|
|
|
|
1. **404 first** — *source* matches no registry row name and no
|
|
indexed document (the same two reads the GET makes): a label for
|
|
a source that exists nowhere is refused and nothing is written.
|
|
A typo'd name would otherwise sit in the table with no row to
|
|
join, no documents to boost and nothing in the UI to list it.
|
|
2. **Then the tags** — :func:`~app.rag.source_labels.normalize_tags`
|
|
(trim, drop the entries that are empty afterwards, de-duplicate
|
|
case-insensitively with the first case kept); its ``ValueError``
|
|
becomes a 422 carrying its own fixed, input-free reason. The
|
|
request shape caps the same numbers, so today the raise is the
|
|
belt — the pin is that the belt can never fire as a 500.
|
|
3. **Then the write** — only the fields the request actually carried
|
|
(``model_fields_set`` — the phase-168 partial-update shape:
|
|
omitted keeps, ``null`` clears, a value sets) reach
|
|
:func:`~app.rag.source_labels.set_label`, and the commit happens
|
|
here (the router owns the session, the phase-91 convention). A
|
|
rejection at step 1 or 2 leaves any existing row byte-identical.
|
|
|
|
An all-omitted body is therefore a legitimate 200 no-op (the
|
|
``GitSourcePatchIn`` "all absent → 200 no-op" precedent): the row is
|
|
touched and reported, nothing changes.
|
|
"""
|
|
if source not in _catalogue_names(db):
|
|
raise HTTPException(status_code=404, detail="source not found")
|
|
|
|
write: dict[str, Any] = {}
|
|
carried = payload.model_fields_set
|
|
if "display_name" in carried:
|
|
write["display_name"] = payload.display_name
|
|
if "label" in carried:
|
|
write["label"] = payload.label
|
|
if "tags" in carried:
|
|
try:
|
|
write["tags"] = normalize_tags(payload.tags or [])
|
|
except ValueError as exc:
|
|
raise HTTPException(status_code=422, detail=str(exc)) from exc
|
|
|
|
row = set_label(db, source, **write)
|
|
db.commit()
|
|
# The action and the key only — never the owner's text (AGENTS rule
|
|
# 10, the ``admin_settings`` write-line discipline).
|
|
logger.info(
|
|
"source labels action=set source=%s fields=%s",
|
|
source,
|
|
",".join(sorted(write)) or "none",
|
|
)
|
|
return SourceLabelOut(
|
|
source=row.source,
|
|
display_name=row.display_name,
|
|
label=row.label,
|
|
tags=list(row.tags or []),
|
|
)
|
|
|
|
|
|
@router.delete("/{source}", response_model=SourceLabelOut)
|
|
def delete_source_label(
|
|
source: Annotated[str, Path(max_length=120)],
|
|
db: Session = Depends(get_db), # noqa: B008
|
|
) -> SourceLabelOut:
|
|
"""Reset one source — the row goes and the next turn sees the
|
|
pre-phase state.
|
|
|
|
404 (one fixed detail, ``source label not found``) when there is no
|
|
label row for it. Unlike the PUT this does NOT consult the registry
|
|
or the documents: the reset has to work for a row whose source has
|
|
since been pruned or whose registry row was deleted (phase 69's
|
|
total removal), or the label would be un-clearable by construction.
|
|
"""
|
|
if not delete_label(db, source):
|
|
raise HTTPException(status_code=404, detail="source label not found")
|
|
db.commit()
|
|
logger.info("source labels action=clear source=%s", source)
|
|
return SourceLabelOut(source=source, display_name=None, label=None, tags=[])
|