From 54ee16b12235fd2976d48628293226f4d25c5c9c Mon Sep 17 00:00:00 2001 From: Pablo Ch Date: Thu, 30 Jul 2026 20:48:47 +0200 Subject: [PATCH 1/2] feat(backend): auto-recompute UMAP 2D on author add/remove Extract umap_projector.recompute_umap for shared use, refresh scatter centroids after StyleProfile insert, and add DELETE /api/authors/{id}. --- .../autoria_ai/extractor/style_profile.py | 4 +- ai_pipeline/autoria_ai/umap_projector.py | 273 ++++++++++++ .../tests/test_style_profile_compute.py | 31 +- ai_pipeline/tests/test_umap_projector.py | 61 +++ backend/app/routes/authors.py | 97 ++++- backend/tests/test_delete_author.py | 69 +++ backend/tests/test_document_upload.py | 10 +- backend/tests/test_style_profile_recompute.py | 10 +- docs/api_contract.yaml | 24 ++ docs/decision_log.md | 3 +- scripts/precompute_umap.py | 393 ++---------------- 11 files changed, 586 insertions(+), 389 deletions(-) create mode 100644 ai_pipeline/autoria_ai/umap_projector.py create mode 100644 ai_pipeline/tests/test_umap_projector.py create mode 100644 backend/tests/test_delete_author.py diff --git a/ai_pipeline/autoria_ai/extractor/style_profile.py b/ai_pipeline/autoria_ai/extractor/style_profile.py index fe34cb0..9aef196 100644 --- a/ai_pipeline/autoria_ai/extractor/style_profile.py +++ b/ai_pipeline/autoria_ai/extractor/style_profile.py @@ -6,7 +6,7 @@ Runs lexical / syntactic / stylistic extractors, distinctive vocab, and semantic centroid. ``embedding_umap_2d`` is a placeholder (``{centroid:[0,0], spread:0}``) — real UMAP lives in - ``scripts/precompute_umap.py``. + ``autoria_ai.umap_projector`` / ``scripts/precompute_umap.py``. lemmatize_corpus(documents, nlp, ...) -> str One author's corpus as a single lemmatized string, ready to be used as another author's ``comparison_lemmas`` entry (docs/style_features.md §4.1). @@ -303,7 +303,7 @@ def compute_style_profile( "stylistic": stylistic, "distinctive_vocab": distinctive, "semantic_centroid": centroid, - # Pre-projection placeholder — scripts/precompute_umap.py reads the + # Pre-projection placeholder — umap_projector / precompute_umap.py reads the # umap_coords table (created by 0004_umap_coords.sql) and overwrites # this field with real 2-D centroid + spread after UMAP fitting. "embedding_umap_2d": {"centroid": [0.0, 0.0], "spread": 0.0}, diff --git a/ai_pipeline/autoria_ai/umap_projector.py b/ai_pipeline/autoria_ai/umap_projector.py new file mode 100644 index 0000000..7dc3d30 --- /dev/null +++ b/ai_pipeline/autoria_ai/umap_projector.py @@ -0,0 +1,273 @@ +"""Global UMAP 2-D projection for Style DNA scatter centroids. + +Public API +---------- +recompute_umap(database_url=None, chunk_table=None) -> bool + Fetch embedded chunks, fit UMAP, rewrite ``umap_coords``, and patch + ``embedding_umap_2d`` on each author's latest StyleProfile. Returns + ``True`` on success, ``False`` when there are too few embedded chunks + (or on a soft failure the caller should treat as non-fatal). + +This is the same pipeline formerly owned only by ``scripts/precompute_umap.py``. +The script remains as a thin CLI; the backend calls ``recompute_umap`` after +author add/remove so the scatter stays consistent without a manual re-run. +""" + +from __future__ import annotations + +import json +import logging +import os +from typing import Any + +import numpy as np +import psycopg2 +import psycopg2.extras +import umap # umap-learn + +log = logging.getLogger(__name__) + +# --------------------------------------------------------------------------- +# Configuration +# --------------------------------------------------------------------------- + +UMAP_N_NEIGHBORS: int = 15 +UMAP_MIN_DIST: float = 0.1 +UMAP_METRIC: str = "cosine" +UMAP_N_COMPONENTS: int = 2 + +DEFAULT_CHUNK_TABLE: str = "public.chunks" + + +# --------------------------------------------------------------------------- +# Database helpers +# --------------------------------------------------------------------------- + + +def get_connection(database_url: str | None = None) -> psycopg2.extensions.connection: + """Return a psycopg2 connection. + + Parameters + ---------- + database_url: + Full PostgreSQL DSN. If *None*, reads ``DATABASE_URL`` from the + environment. The DSN must be compatible with psycopg2 (plain + ``postgresql://`` scheme, not ``postgresql+asyncpg://``). + + Raises + ------ + KeyError + If ``DATABASE_URL`` is not set and *database_url* was not provided. + """ + url = database_url or os.environ["DATABASE_URL"] + url = url.replace("postgresql+asyncpg://", "postgresql://") + return psycopg2.connect(url) + + +def _as_vector(value: Any) -> list[float]: + """Return a pgvector column value as a list of floats. + + psycopg2 hands back ``"[0.013,-0.011,…]"`` (a string) for a ``vector`` + column unless the pgvector adapter is registered on the connection; other + drivers return a real sequence. Both are accepted so this does not depend + on how the caller built the connection. + """ + if isinstance(value, str): + return [float(x) for x in value.strip().lstrip("[").rstrip("]").split(",") if x] + return list(value) + + +def fetch_embeddings( + conn: psycopg2.extensions.connection, + chunk_table: str = DEFAULT_CHUNK_TABLE, +) -> tuple[list[str], np.ndarray]: + """Fetch author UUIDs and their chunk embeddings from the database. + + The join path is: + → public.documents → public.authors + + Only rows where ``embedding IS NOT NULL`` are returned. + + Raises + ------ + RuntimeError + If no embedded chunks are found. + """ + if "." in chunk_table: + schema, table = chunk_table.split(".", 1) + else: + schema, table = "public", chunk_table + + sql = f""" + SELECT d.author_id::text, c.embedding + FROM {schema}.{table} c + JOIN public.documents d ON d.id = c.document_id + WHERE c.embedding IS NOT NULL + ORDER BY d.author_id + """ + + log.info("Querying embeddings from %s.%s …", schema, table) + with conn.cursor() as cur: + cur.execute(sql) + rows: list[Any] = cur.fetchall() + + if not rows: + raise RuntimeError( + f"No embedded chunks found in {chunk_table}. Run backfill_embeddings() first." + ) + + author_ids: list[str] = [row[0] for row in rows] + embeddings = np.array([_as_vector(row[1]) for row in rows], dtype=np.float32) + + log.info("Fetched %d embedded chunks across %d authors.", len(rows), len(set(author_ids))) + return author_ids, embeddings + + +# --------------------------------------------------------------------------- +# UMAP reduction +# --------------------------------------------------------------------------- + + +def reduce_to_2d(embeddings: np.ndarray) -> np.ndarray: + """Fit UMAP on *embeddings* and return 2-D coordinates.""" + reducer = umap.UMAP( + n_neighbors=UMAP_N_NEIGHBORS, + min_dist=UMAP_MIN_DIST, + metric=UMAP_METRIC, + n_components=UMAP_N_COMPONENTS, + random_state=42, + ) + log.info( + "Fitting UMAP (n_neighbors=%d, min_dist=%.2f, metric=%s) on %d vectors …", + UMAP_N_NEIGHBORS, + UMAP_MIN_DIST, + UMAP_METRIC, + embeddings.shape[0], + ) + coords: np.ndarray = reducer.fit_transform(embeddings) + log.info("UMAP fit complete. Output shape: %s", coords.shape) + return coords + + +# --------------------------------------------------------------------------- +# Storage +# --------------------------------------------------------------------------- + + +def save_coords( + conn: psycopg2.extensions.connection, + author_ids: list[str], + coords: np.ndarray, +) -> None: + """Truncate umap_coords and bulk-insert new (author_id, x, y) rows.""" + rows = [(aid, float(coords[i, 0]), float(coords[i, 1])) for i, aid in enumerate(author_ids)] + + with conn.cursor() as cur: + log.info("Truncating public.umap_coords …") + cur.execute("TRUNCATE TABLE public.umap_coords RESTART IDENTITY") + + log.info("Inserting %d rows into public.umap_coords …", len(rows)) + psycopg2.extras.execute_values( + cur, + "INSERT INTO public.umap_coords (author_id, x, y) VALUES %s", + rows, + page_size=1000, + ) + + conn.commit() + log.info("Done — %d umap_coords rows committed.", len(rows)) + + +def update_style_profiles( + conn: psycopg2.extensions.connection, + author_ids: list[str], + coords: np.ndarray, +) -> int: + """Write each author's 2-D centroid and spread into their latest StyleProfile. + + Only the most recent profile row per author is updated; older rows are + history and stay as they were. + + Returns the number of profile rows updated. + """ + by_author: dict[str, list[tuple[float, float]]] = {} + for i, aid in enumerate(author_ids): + by_author.setdefault(aid, []).append((float(coords[i, 0]), float(coords[i, 1]))) + + updated = 0 + with conn.cursor() as cur: + for author_id, points in by_author.items(): + arr = np.asarray(points, dtype=np.float64) + centroid = arr.mean(axis=0) + spread = float(np.linalg.norm(arr - centroid, axis=1).mean()) + payload = json.dumps( + {"centroid": [float(centroid[0]), float(centroid[1])], "spread": spread} + ) + + cur.execute( + """ + UPDATE public.style_profiles AS sp + SET json_data = jsonb_set( + sp.json_data, '{embedding_umap_2d}', %s::jsonb, true) + WHERE sp.id = ( + SELECT id FROM public.style_profiles + WHERE author_id = %s + ORDER BY computed_at DESC + LIMIT 1) + """, + (payload, author_id), + ) + updated += cur.rowcount + log.info( + "author %s: centroid=(%.3f, %.3f) spread=%.3f over %d chunks", + author_id, + centroid[0], + centroid[1], + spread, + len(points), + ) + + conn.commit() + log.info("Updated embedding_umap_2d on %d style_profiles row(s).", updated) + return updated + + +# --------------------------------------------------------------------------- +# Orchestrator +# --------------------------------------------------------------------------- + + +def recompute_umap( + database_url: str | None = None, + chunk_table: str | None = None, +) -> bool: + """Full pipeline: fetch → reduce → store. + + Returns + ------- + bool + ``True`` if UMAP was fitted and persisted; ``False`` if there were + fewer than ``n_neighbors + 1`` embedded chunks (soft skip — callers + such as the upload path must not treat this as a hard failure). + """ + table = chunk_table or os.environ.get("CHUNK_TABLE", DEFAULT_CHUNK_TABLE) + + conn = get_connection(database_url) + try: + author_ids, embeddings = fetch_embeddings(conn, chunk_table=table) + + min_required = UMAP_N_NEIGHBORS + 1 + if len(author_ids) < min_required: + log.warning( + "UMAP requires at least %d rows but only %d embedded chunks were found; skipping.", + min_required, + len(author_ids), + ) + return False + + coords = reduce_to_2d(embeddings) + save_coords(conn, author_ids, coords) + update_style_profiles(conn, author_ids, coords) + return True + finally: + conn.close() diff --git a/ai_pipeline/tests/test_style_profile_compute.py b/ai_pipeline/tests/test_style_profile_compute.py index 974060d..97ff809 100644 --- a/ai_pipeline/tests/test_style_profile_compute.py +++ b/ai_pipeline/tests/test_style_profile_compute.py @@ -168,7 +168,7 @@ def test_lemmatize_corpus_keeps_only_noun_adj_adv() -> None: # UMAP projection back-fill (WO-07) # --------------------------------------------------------------------------- # -# update_style_profiles() in scripts/precompute_umap.py aggregates per-chunk +# update_style_profiles() in autoria_ai.umap_projector aggregates per-chunk # UMAP coords into a { "centroid": [x, y], "spread": float } dict and writes # it to style_profiles.json_data.embedding_umap_2d via a parameterised UPDATE. # @@ -205,31 +205,19 @@ def _make_author_ids() -> list[str]: def test_update_style_profiles_sql_payload() -> None: """update_style_profiles writes correct centroid/spread JSON via UPDATE.""" - # Import lazily: the script lives outside the package; add scripts/ to sys.path - import os - import sys - - scripts_dir = os.path.join(os.path.dirname(__file__), "..", "..", "scripts") - scripts_dir = os.path.normpath(scripts_dir) - if scripts_dir not in sys.path: - sys.path.insert(0, scripts_dir) - # psycopg2 may not be installed in every CI image — skip gracefully. pytest.importorskip("psycopg2") + from autoria_ai.umap_projector import update_style_profiles - # Patch psycopg2.connect so the script never touches a real DB. mock_conn = MagicMock() mock_cur = MagicMock() mock_conn.cursor.return_value.__enter__ = lambda s: mock_cur mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=False) - # Import after sys.path is set up. - import precompute_umap - author_ids = _make_author_ids() coords = _make_coords() - precompute_umap.update_style_profiles(mock_conn, author_ids, coords) + update_style_profiles(mock_conn, author_ids, coords) # One UPDATE call per author, then one commit. assert mock_conn.commit.called @@ -261,28 +249,19 @@ def test_update_style_profiles_sql_payload() -> None: def test_update_style_profiles_spread_formula() -> None: """spread = mean Euclidean distance of each chunk from its author centroid.""" - import os - import sys - - scripts_dir = os.path.join(os.path.dirname(__file__), "..", "..", "scripts") - scripts_dir = os.path.normpath(scripts_dir) - if scripts_dir not in sys.path: - sys.path.insert(0, scripts_dir) - pytest.importorskip("psycopg2") + from autoria_ai.umap_projector import update_style_profiles mock_conn = MagicMock() mock_cur = MagicMock() mock_conn.cursor.return_value.__enter__ = lambda s: mock_cur mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=False) - import precompute_umap - # Four points equidistant from centroid (0,0) at radius=1. coords = np.array([[1.0, 0.0], [-1.0, 0.0], [0.0, 1.0], [0.0, -1.0]], dtype=np.float64) author_ids = [_AUTHOR_A] * 4 - precompute_umap.update_style_profiles(mock_conn, author_ids, coords) + update_style_profiles(mock_conn, author_ids, coords) update_calls = mock_cur.execute.call_args_list assert len(update_calls) == 1 diff --git a/ai_pipeline/tests/test_umap_projector.py b/ai_pipeline/tests/test_umap_projector.py new file mode 100644 index 0000000..560184c --- /dev/null +++ b/ai_pipeline/tests/test_umap_projector.py @@ -0,0 +1,61 @@ +"""Tests for ai_pipeline/autoria_ai/umap_projector.py.""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import numpy as np +import pytest + +pytest.importorskip("psycopg2") +pytest.importorskip("umap") + +from autoria_ai.umap_projector import UMAP_N_NEIGHBORS, recompute_umap + + +def test_recompute_umap_returns_false_when_too_few_chunks() -> None: + """Fewer than n_neighbors+1 embedded chunks is a soft skip, not a crash.""" + n = UMAP_N_NEIGHBORS # one short of the minimum + author_ids = ["aaaaaaaa-0000-0000-0000-000000000001"] * n + embeddings = np.zeros((n, 8), dtype=np.float32) + + mock_conn = MagicMock() + with ( + patch("autoria_ai.umap_projector.get_connection", return_value=mock_conn), + patch( + "autoria_ai.umap_projector.fetch_embeddings", + return_value=(author_ids, embeddings), + ), + patch("autoria_ai.umap_projector.reduce_to_2d") as mock_reduce, + ): + ok = recompute_umap(database_url="postgresql://unused") + + assert ok is False + mock_reduce.assert_not_called() + mock_conn.close.assert_called_once() + + +def test_recompute_umap_returns_true_on_happy_path() -> None: + n = UMAP_N_NEIGHBORS + 1 + author_ids = ["aaaaaaaa-0000-0000-0000-000000000001"] * n + embeddings = np.random.randn(n, 8).astype(np.float32) + coords = np.random.randn(n, 2) + + mock_conn = MagicMock() + with ( + patch("autoria_ai.umap_projector.get_connection", return_value=mock_conn), + patch( + "autoria_ai.umap_projector.fetch_embeddings", + return_value=(author_ids, embeddings), + ), + patch("autoria_ai.umap_projector.reduce_to_2d", return_value=coords) as mock_reduce, + patch("autoria_ai.umap_projector.save_coords") as mock_save, + patch("autoria_ai.umap_projector.update_style_profiles", return_value=1) as mock_upd, + ): + ok = recompute_umap(database_url="postgresql://unused") + + assert ok is True + mock_reduce.assert_called_once() + mock_save.assert_called_once() + mock_upd.assert_called_once() + mock_conn.close.assert_called_once() diff --git a/backend/app/routes/authors.py b/backend/app/routes/authors.py index 2606f1d..e12c5c2 100644 --- a/backend/app/routes/authors.py +++ b/backend/app/routes/authors.py @@ -1,10 +1,11 @@ """Authors routes. Implements: - GET /api/authors — list all authors - GET /api/authors/{author_id}/style-profile — getAuthorStyleProfile - POST /api/authors/{author_id}/documents — uploadAuthorDocument - POST /api/authors/{author_id}/style-profile/recompute — recomputeAuthorStyleProfile + GET /api/authors — list all authors + DELETE /api/authors/{author_id} — deleteAuthor + GET /api/authors/{author_id}/style-profile — getAuthorStyleProfile + POST /api/authors/{author_id}/documents — uploadAuthorDocument + POST /api/authors/{author_id}/style-profile/recompute — recomputeAuthorStyleProfile GET /authors reads ``public.authors`` and derives ``has_style_profile`` / ``n_documents`` from ``style_profiles`` and ``documents``. @@ -18,6 +19,10 @@ seconds from corpus n_tokens, schedules a BackgroundTask, and returns 202 {status:"computing", estimated_seconds:N} — docs/api_contract.yaml §StyleProfileRecomputeAccepted and Decision Log 2026-07-20. + +After a successful StyleProfile insert (upload or recompute) and after author +delete, a process-locked global UMAP recompute refreshes ``embedding_umap_2d`` +for every remaining author so the Style DNA scatter stays consistent. """ from __future__ import annotations @@ -25,12 +30,14 @@ import hashlib import json import logging +import os import sys +import threading from pathlib import Path from typing import Annotated, Any import tiktoken -from fastapi import APIRouter, BackgroundTasks, File, Form, HTTPException, UploadFile +from fastapi import APIRouter, BackgroundTasks, File, Form, HTTPException, Response, UploadFile from fastapi.responses import JSONResponse from supabase import Client @@ -51,6 +58,9 @@ _CHUNK_SIZE: int = 500 _CHUNK_OVERLAP: int = 50 +# Serialize global UMAP fits so overlapping uploads do not race truncate+insert. +_UMAP_LOCK = threading.Lock() + # --------------------------------------------------------------------------- # ai_pipeline import helper (mirrors passport.py / generate.py) @@ -111,10 +121,37 @@ def _build_style_profile(author_slug: str, documents: list[str], sb: Client) -> ) +def _recompute_umap_safe() -> None: + """Recompute global UMAP 2-D centroids; never raise to callers. + + Uses a process-level lock so concurrent upload/recompute/delete background + tasks serialize the truncate+refit. Missing ``DATABASE_URL`` or too few + embedded chunks are soft skips. + """ + database_url = os.getenv("DATABASE_URL") + if not database_url: + logger.warning("UMAP recompute skipped: DATABASE_URL is not set") + return + + with _UMAP_LOCK: + try: + _ensure_ai_pipeline_on_path() + from autoria_ai.umap_projector import recompute_umap + + ok = recompute_umap(database_url=database_url) + if ok: + logger.info("UMAP recompute completed after author corpus change") + else: + logger.warning("UMAP recompute skipped (insufficient embedded chunks)") + except Exception: + logger.exception("UMAP recompute failed; Style DNA scatter may be stale") + + def _recompute_style_profile(author_uuid: str, author_slug: str, sb: Client) -> None: """Compute a real StyleProfile and INSERT into style_profiles. Errors are logged but not re-raised: the 202 has already been sent. + On success, refreshes global UMAP centroids for the Style DNA scatter. """ try: docs_result = ( @@ -145,6 +182,9 @@ def _recompute_style_profile(author_uuid: str, author_slug: str, sb: Client) -> logger.info("style_profiles row inserted for author %s (%s)", author_slug, author_uuid) except Exception: logger.exception("recompute failed for author %s (%s)", author_slug, author_uuid) + return + + _recompute_umap_safe() def _chunk_and_insert( @@ -263,6 +303,53 @@ async def list_authors() -> list[AuthorSummary]: ] +@router.delete( + "/authors/{author_id}", + status_code=204, + summary="Delete an author and their corpus", + operation_id="deleteAuthor", + responses={ + 404: {"description": "Unknown author_id"}, + 500: {"description": "Unexpected server error"}, + }, +) +async def delete_author( + author_id: str, + background_tasks: BackgroundTasks, +) -> Response: + """Delete *author_id* (slug) and cascade corpus rows; refresh UMAP after. + + ``umap_coords`` has no FK to ``authors``, so those rows are deleted + explicitly before the author row. Documents / chunks / style_profiles / + passports cascade from ``authors``. Global UMAP is recomputed in a + background task so remaining authors keep separated scatter positions. + """ + sb = get_client() + + author_result = sb.table("authors").select("id").eq("slug", author_id).maybe_single().execute() + if author_result is None or getattr(author_result, "data", None) is None: + raise HTTPException( + status_code=404, + detail={"error": "not_found", "message": f"Author '{author_id}' not found"}, + ) + + author_uuid: str = author_result.data["id"] + + try: + # No FK on umap_coords — clear this author's cache rows first. + sb.table("umap_coords").delete().eq("author_id", author_uuid).execute() + sb.table("authors").delete().eq("id", author_uuid).execute() + except Exception: + logger.exception("delete failed for author %s (%s)", author_id, author_uuid) + raise HTTPException( + status_code=500, + detail={"error": "internal_error", "message": f"Failed to delete author '{author_id}'"}, + ) from None + + background_tasks.add_task(_recompute_umap_safe) + return Response(status_code=204) + + @router.get( "/authors/{author_id}/style-profile", response_model=None, diff --git a/backend/tests/test_delete_author.py b/backend/tests/test_delete_author.py new file mode 100644 index 0000000..d1c61e8 --- /dev/null +++ b/backend/tests/test_delete_author.py @@ -0,0 +1,69 @@ +"""Tests for DELETE /api/authors/{author_id} (deleteAuthor).""" + +from __future__ import annotations + +import uuid +from unittest.mock import MagicMock, patch + +from fastapi.testclient import TestClient + +from app.main import app + +client = TestClient(app) + +_FAKE_AUTHOR_UUID = str(uuid.uuid4()) + + +def _make_sb_mock(*, author_found: bool = True) -> MagicMock: + sb = MagicMock() + + authors_chain = MagicMock() + author_execute = MagicMock() + author_execute.data = {"id": _FAKE_AUTHOR_UUID} if author_found else None + authors_chain.select.return_value.eq.return_value.maybe_single.return_value.execute.return_value = ( + author_execute + ) + authors_chain.delete.return_value.eq.return_value.execute.return_value = MagicMock() + + umap_chain = MagicMock() + umap_chain.delete.return_value.eq.return_value.execute.return_value = MagicMock() + + def _table_router(name: str) -> MagicMock: + if name == "authors": + return authors_chain + if name == "umap_coords": + return umap_chain + return MagicMock() + + sb.table.side_effect = _table_router + sb._authors_chain = authors_chain + sb._umap_chain = umap_chain + return sb + + +@patch("app.routes.authors._recompute_umap_safe") +@patch("app.routes.authors.get_client") +def test_delete_author_204_and_enqueues_umap( + mock_get_client: MagicMock, + mock_umap: MagicMock, +) -> None: + sb = _make_sb_mock(author_found=True) + mock_get_client.return_value = sb + + resp = client.delete("/api/authors/grunon") + + assert resp.status_code == 204 + sb._umap_chain.delete.assert_called_once() + sb._authors_chain.delete.assert_called_once() + mock_umap.assert_called_once() + + +@patch("app.routes.authors.get_client") +def test_delete_unknown_author_404(mock_get_client: MagicMock) -> None: + mock_get_client.return_value = _make_sb_mock(author_found=False) + + resp = client.delete("/api/authors/unknown_ghost") + + assert resp.status_code == 404 + body = resp.json() + assert body["detail"]["error"] == "not_found" diff --git a/backend/tests/test_document_upload.py b/backend/tests/test_document_upload.py index d6a0705..14770a7 100644 --- a/backend/tests/test_document_upload.py +++ b/backend/tests/test_document_upload.py @@ -26,9 +26,13 @@ @pytest.fixture(autouse=True) -def _skip_embedding_backfill(): - """Upload background task must not load sentence-transformers in unit tests.""" - with patch("app.routes.authors._embed_document_chunks"): +def _skip_heavy_background_work(): + """Upload background task must not load ML / hit a real DB in unit tests.""" + with ( + patch("app.routes.authors._embed_document_chunks"), + patch("app.routes.authors._recompute_umap_safe"), + patch("app.routes.authors._build_style_profile", return_value={"author_id": "x"}), + ): yield diff --git a/backend/tests/test_style_profile_recompute.py b/backend/tests/test_style_profile_recompute.py index 5cb8248..e05d7e4 100644 --- a/backend/tests/test_style_profile_recompute.py +++ b/backend/tests/test_style_profile_recompute.py @@ -73,8 +73,11 @@ @pytest.fixture(autouse=True) def _patch_build_style_profile(): - with patch("app.routes.authors._build_style_profile", return_value=_FAKE_PROFILE): - yield + with ( + patch("app.routes.authors._build_style_profile", return_value=_FAKE_PROFILE), + patch("app.routes.authors._recompute_umap_safe") as mock_umap, + ): + yield mock_umap def _make_sb_mock( @@ -166,8 +169,10 @@ def test_recompute_404_unknown_author(mock_get_client: MagicMock) -> None: def test_recompute_background_task_insert_called( mock_build: MagicMock, mock_get_client: MagicMock, + _patch_build_style_profile: MagicMock, ) -> None: mock_build.return_value = {**_FAKE_PROFILE, "author_id": "poe"} + mock_umap = _patch_build_style_profile sb = _make_sb_mock(author_found=True) mock_get_client.return_value = sb @@ -201,3 +206,4 @@ def test_recompute_background_task_insert_called( "SCONJ", "OTHER", } + mock_umap.assert_called_once() diff --git a/docs/api_contract.yaml b/docs/api_contract.yaml index cfd189e..504f9eb 100644 --- a/docs/api_contract.yaml +++ b/docs/api_contract.yaml @@ -95,6 +95,30 @@ paths: "500": $ref: "#/components/responses/InternalError" + /api/authors/{author_id}: + delete: + tags: [authors] + operationId: deleteAuthor + summary: Delete an author and their corpus + description: | + Deletes the author row (cascade: documents, chunks, style_profiles, + passports). Also clears that author's `umap_coords` rows (no FK) and + enqueues a background global UMAP recompute so remaining authors keep + separated Style DNA scatter centroids. + parameters: + - $ref: "#/components/parameters/AuthorId" + responses: + "204": + description: Author deleted; UMAP recompute enqueued + "404": + description: Unknown author_id + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + "500": + $ref: "#/components/responses/InternalError" + /api/authors/{author_id}/style-profile: get: tags: [authors] diff --git a/docs/decision_log.md b/docs/decision_log.md index b6e908f..987c581 100644 --- a/docs/decision_log.md +++ b/docs/decision_log.md @@ -59,4 +59,5 @@ Every decision that affects the product, the process, or the team lives here. Ap | 2026-07-30 | **`make seed-full` (#86) executed against the live Supabase DB with `DATABASE_URL` available, closing the item the entry above left open.** `scripts/seed_corpus.py --with-profiles` recomputed `style_profiles` for all three authors under the new log-odds-ratio `distinctive_vocab` (previous run had no `DATABASE_URL`); it also inserted 3 corpus documents (2 Austen, 1 Dickens) and 1272 chunks that existed in `corpus/` but had never reached the `documents`/`chunks` tables. Verified directly against the DB: top-10 `distinctive_vocab` per author now has **0** pairwise overlap and every score is a positive log-odds z-score (no stale TF-IDF values, no `[Illustration]` artifact survived cleaning). Two follow-on gaps were then found and closed in the same session, both pre-existing defects surfaced by re-seeding rather than caused by it: **(a)** the 1272 newly-inserted chunks had `embedding IS NULL` (only `--with-profiles` was run, not `--with-embeddings`) and were invisible to RAG retrieval until `scripts/seed_corpus.py --with-embeddings` backfilled them; **(b)** `scripts/precompute_umap.py` only ever updates the *latest* `style_profiles` row per author, and re-seeding had just created new latest rows still carrying the extractor's `{"centroid":[0,0],"spread":0}` placeholder, so all three authors rendered on top of each other in the Style DNA scatter — fixed by re-running `precompute_umap.py`, which recomputed real, well-separated centroids from the (now embedding-complete) `chunks` table. Separately, `frontend/src/components/StyleDnaPanel.tsx`'s distinctive-vocabulary bar width (`Math.min(item.score * 100, 100)%`) was left over from the old TF-IDF score's `[0,1]` range; against unbounded log-odds z-scores (6-10+) every bar clamped to 100%. Fixed to scale relative to the top term in the displayed list (`item.score / maxVocabScore`) instead of an absolute `[0,1]` assumption. No schema/contract change in any of these; `scripts/seed_corpus.py` prose ("cross-author TF-IDF" in docstrings/log lines) also corrected to "log-odds-ratio" to match, missed in the 2026-07-29 sweep because that file wasn't touched by the original proposal's file list | Executed and fixed by agent at Pablo's request ("Actualiza lo que consideres de la base de datos") | Re-seeding after an algorithm change is the step the entry above explicitly deferred for lack of `DATABASE_URL`; running it surfaced two independently pre-existing pipeline gaps (embeddings-vs-profiles seeding are separate opt-in flags that must both be run after new documents/rows appear; UMAP centroids are a derived, not primary, field that only one script keeps in sync) that a normal `--with-profiles`-only re-seed would silently leave broken rather than erroring loudly. Fixing all three (embeddings, UMAP, frontend bar scale) in the same pass instead of only the DB update it was asked to do avoided shipping a re-seed that looked complete (`INFO Seed complete`) while leaving the Style DNA panel visibly wrong | | 2026-07-30 | **FLAGGED, NOT RATIFIED — `feat/better-response` (unmerged branch, fetched but not merged) modifies two of `fit_score`'s five LOCKED weighted components (`_lexical_score`, `_vocabulary_score` in `ai_pipeline/autoria_ai/fit_scorer.py`) with no accompanying Decision Log entry.** `docs/style_features.md` §6 declares the `fit_score` formula, and the 2026-06-24 MVP LOCKED policy requires a log entry + 2/3 vote *before* changing any algorithm a docs file declares closed — this entry exists because that step was skipped on that branch, not because either change is judged wrong. **(a) `_lexical_score`** changed from `1 − \|ttr_generated − mattr_profile\| / mattr_profile` to `1 − \|ttr_generated − mattr_profile\|` (dropped the `/ mattr_profile` normalization), with the stated rationale "a short generated text naturally has a higher TTR than the 500-token MATTR profile, and relative error over-penalizes that." **Measured, not assumed:** since `mattr_profile ∈ [0.49, 0.60]` for all three authors (§7) — i.e. always < 1 — dividing by it can only ever make the same absolute gap *larger*, so this change makes `_lexical_score` weakly higher for every possible generation, not only short/high-TTR ones; the claimed length-sensitivity fix is really a global leniency increase that happens to help the length-driven case most. Tested against two real generations from this session: a 236-lemma "Mejora" text (`ttr_generated` 0.4915, already close to `mattr_profile`) moved only +0.002 to +0.06 across the plausible `mattr_profile` range; a 103-lemma short vanilla-style text (`ttr_generated` 0.6699, the length-inflated case the rationale describes) moved **+0.075 to +0.19** — a swing of up to 0.19 × 0.15 weight ≈ **+2.8 points of overall `fit_score`** from this one formula edit alone, concentrated on short outputs. The underlying mismatch the rationale points at is real (`mattr_profile` is a length-normalized statistic; `ttr_generated` — raw whole-text TTR — is not, so short generations are structurally advantaged over long ones regardless of which formula variant is used) but is not what this edit fixes; a targeted fix would compute `ttr_generated` with the same 500-token moving-window method used to build `mattr_profile`, which remains an open gap either way. **(b) `_vocabulary_score`** changed from `len(generated_lemmas ∩ top30_distinctive) / 30` to `min(1.0, len(generated_lemmas ∩ top15_distinctive) / 5.0)`. This one is well-justified by the same "measure, don't estimate" standard: the old denominator required all 30 prompted terms to appear for a perfect score — structurally close to unreachable in a short generation (hitting even 5 of 30 only scored 0.167) — while realistically only a handful of signature terms can be naturally woven into a paragraph-length output; capping the pool at top-15 (the same slice actually shown to the model, per that branch's own `conditioner.py` vocab-list construction) and normalizing against a reachable target of 5 removes an artificial near-zero ceiling on this component, independent of prompt quality. Both edits are entangled with that branch's own independent `distinctive_vocab` rewrite (a second, incompatible log-odds-ratio implementation — different prior, no variance term, `[0,1]`-normalized output — see the two ratified entries above for the version actually shipped on `main`) and its own `conditioner.py` prompt-wording changes, none of which are part of this repo's `main` yet. **No action taken on the code** — `ai_pipeline/autoria_ai/fit_scorer.py` on `main`/this working tree is unchanged; this entry only records the measured evaluation Pablo requested before any merge decision is made | Measured and drafted by agent at Pablo's request, evaluating an unmerged colleague branch (`feat/better-response`) — flagged for the team's 2/3 vote before merge, not yet ratified | The branch's own log-odds-ratio rewrite of `distinctive_vocab` shows the team converged on the same TF-IDF diagnosis via two people at once, independently, with incompatible implementations — a sign this class of change needs the vote *before* either version ships, not after. The same review discipline this log already applies to every other LOCKED-formula change (2026-07-28 PROPN filter, 2026-07-29 range remeasurement, the log-odds proposal/ratification pair above) should apply here before either branch's version of `fit_score` merges, so the number displayed to the jury is one the team actually agreed to, not whichever branch happened to merge last | | 2026-07-30 | **RATIFIED — adopt the Jeffreys-prior log-odds + NOUN/ADJ/ADV filter for `distinctive_vocab` (supersedes the Monroe z-score implementation ratified earlier the same day).** Side-by-side measurement on the real corpus compared three combinations: (1) Monroe z-score + PROPN-only drop, (2) Jeffreys α=0.5 log-odds + NOUN/ADJ/ADV allow-list (from `feat/better-response`), (3) hybrid Monroe z-score + NOUN/ADJ/ADV. Top-10 overlap between (1) and (2) was **0/0/1** across Austen/Dickens/Poe; hybrid (3) still ranked high-frequency nouns (`sister`, `old`, `hand`) rather than signature lexicon. The POS filter alone is therefore **not** enough — the scorer without a variance term is what promotes rare, concentrated terms (`matrimony`, `workhouse`, `ballast`) that a non-technical juror reads as style. Adopted: `vocabulary.py` = Jeffreys log-odds with scores normalized to `[0, 1]`; `_lemmas_from_docs` = keep only NOUN/ADJ/ADV + small corpus-metadata stop list. Kept: `[Illustration]` stripping in `cleaner.py` (orthogonal, still useful). `docs/style_features.md` §4.1, schema, api_contract prose, and frontend score comments updated. Re-seed (`--with-profiles` + UMAP) still required for live DB rows. `fit_score` formula changes on `feat/better-response` remain flagged, not adopted | Pablo (explicit choice after reviewing measured comparison) | Style DNA is the feature a jury *reads*; juror-legible signature vocab beats a statistically more conservative z-score that surfaces narrative-frequency terms. Scores in `[0, 1]` also restore the contract range the frontend originally assumed | -| 2026-07-30 | **RATIFIED — port the remaining important pieces of `feat/better-response` onto `feat/style-profile-and-fit-score-improvements`.** (a) `conditioner.py` numbered system-prompt template with dialogue_rule + run-on guard on heavy subordination (plus a fix: read `dialogue_ratio` from `stylistic`, not `syntactic` — the branch had that key under the wrong block, so the rule always fell to the zero-dialogue default). (b) `fit_scorer.py` LOCKED-formula edits previously flagged in this log: `_lexical_score` uses absolute error `1 − \|ttr − mattr\|`; `_vocabulary_score` uses `min(1, \|∩ top15\| / 5)`. Documented in `docs/style_features.md` §6. (c) `backend/app/routes/authors.py`: comparison corpora via `lemmatize_corpus` (fixes identical-vocab collapse from raw `.lower()`), auto-create author on upload when missing, auto-recompute style profile after chunk+embed. Skipped from that branch: `keys/jwks.public.json` rotation, ad-hoc debug scripts, synthetic demo corpora | Pablo (explicit: add the important code from that branch to ours) | The colleague's prompt + fit_score calibration are what produced the better measured generations; keeping them off the integration branch would leave the vocab work without the generation path that uses it | \ No newline at end of file +| 2026-07-30 | **RATIFIED — port the remaining important pieces of `feat/better-response` onto `feat/style-profile-and-fit-score-improvements`.** (a) `conditioner.py` numbered system-prompt template with dialogue_rule + run-on guard on heavy subordination (plus a fix: read `dialogue_ratio` from `stylistic`, not `syntactic` — the branch had that key under the wrong block, so the rule always fell to the zero-dialogue default). (b) `fit_scorer.py` LOCKED-formula edits previously flagged in this log: `_lexical_score` uses absolute error `1 − \|ttr − mattr\|`; `_vocabulary_score` uses `min(1, \|∩ top15\| / 5)`. Documented in `docs/style_features.md` §6. (c) `backend/app/routes/authors.py`: comparison corpora via `lemmatize_corpus` (fixes identical-vocab collapse from raw `.lower()`), auto-create author on upload when missing, auto-recompute style profile after chunk+embed. Skipped from that branch: `keys/jwks.public.json` rotation, ad-hoc debug scripts, synthetic demo corpora | Pablo (explicit: add the important code from that branch to ours) | The colleague's prompt + fit_score calibration are what produced the better measured generations; keeping them off the integration branch would leave the vocab work without the generation path that uses it | +| 2026-07-30 | **Contract amended (2/3 vote pending): DELETE /api/authors/{author_id} + automatic global UMAP recompute on author add/remove.** UMAP 2D (embedding_umap_2d) is a global fit over all embedded chunks; previously it only ran via scripts/precompute_umap.py, so new uploads left scatter centroids at [0,0] and SQL deletes left stale points for removed authors. **(a)** Core pipeline extracted to i_pipeline/autoria_ai/umap_projector.py::recompute_umap (returns bool; soft-skips when <16 embedded chunks); CLI wrapper kept. **(b)** After a successful StyleProfile insert (upload or manual recompute), _recompute_umap_safe runs under a process-level lock. **(c)** New DELETE /api/authors/{author_id} → 204: deletes umap_coords for that author (no FK), deletes the author row (cascade docs/chunks/profiles/passports), enqueues UMAP recompute. docs/api_contract.yaml amended accordingly. Does **not** auto-recompute other authors' distinctive_vocab | Pablo (requested automatic UMAP on add/remove) | Style DNA scatter is the demo wow moment; leaving UMAP manual after every corpus change silently breaks author separation | diff --git a/scripts/precompute_umap.py b/scripts/precompute_umap.py index 91a3948..3d15e61 100644 --- a/scripts/precompute_umap.py +++ b/scripts/precompute_umap.py @@ -1,20 +1,6 @@ """scripts/precompute_umap.py ============================================================================= -Pre-compute per-chunk 2-D UMAP coordinates from pgvector embeddings and -persist them to the ``public.umap_coords`` table. - -Pipeline --------- -1. Connect to PostgreSQL via DATABASE_URL (psycopg2, synchronous). -2. Fetch (author_id, embedding) from ``public.chunks`` joined through - ``public.documents`` (chunks carry no direct author_id column — the - canonical join path is chunks → documents → authors). -3. Reduce the N x 768 embedding matrix to N x 2 with UMAP - (n_neighbors=15, min_dist=0.1, metric='cosine', n_components=2). -4. Truncate and repopulate ``public.umap_coords``. -5. Aggregate those coordinates per author and write the centroid + spread - into ``style_profiles.json_data.embedding_umap_2d`` — the field the Style - DNA scatter actually reads (#88). +CLI wrapper around ``autoria_ai.umap_projector.recompute_umap``. Usage ----- @@ -24,55 +10,57 @@ # override chunk source table (default: public.chunks): CHUNK_TABLE=public.my_chunks python scripts/precompute_umap.py -Dependencies (beyond ai_pipeline/pyproject.toml) -------------------------------------------------- - pip install psycopg2-binary # sync PG driver for this script - # umap-learn and numpy are already in ai_pipeline/pyproject.toml - Environment variables --------------------- DATABASE_URL — PostgreSQL DSN, e.g. postgresql://user:pass@host:5432/dbname CHUNK_TABLE — optional; defaults to "public.chunks" -Notes ------ -* UMAP requires ≥ (n_neighbors + 1) rows. The script exits early with a - clear message if the table has fewer embedded chunks. -* Embeddings are read as plain Python lists (pgvector returns them that way - from psycopg2); they are cast to a float32 NumPy array for UMAP. -* The insert uses psycopg2's execute_values for a single round-trip. +The implementation lives in ``ai_pipeline/autoria_ai/umap_projector.py`` so the +backend can call the same pipeline after author add/remove without shelling out. """ from __future__ import annotations -import json import logging -import os import sys -from typing import Any - -import numpy as np -import psycopg2 -import psycopg2.extras -import umap # umap-learn - -# --------------------------------------------------------------------------- -# Configuration -# --------------------------------------------------------------------------- - -#: UMAP parameters — fixed per spec. -UMAP_N_NEIGHBORS: int = 15 -UMAP_MIN_DIST: float = 0.1 -UMAP_METRIC: str = "cosine" -UMAP_N_COMPONENTS: int = 2 - -#: Source table for embeddings. Override with env var CHUNK_TABLE. -DEFAULT_CHUNK_TABLE: str = "public.chunks" +from pathlib import Path + +# Make ai_pipeline importable when invoked as a script from the repo root. +_ROOT = Path(__file__).resolve().parents[1] +_AI = _ROOT / "ai_pipeline" +if _AI.is_dir() and str(_AI) not in sys.path: + sys.path.insert(0, str(_AI)) + +from autoria_ai.umap_projector import ( # noqa: E402 + DEFAULT_CHUNK_TABLE, + UMAP_METRIC, + UMAP_MIN_DIST, + UMAP_N_COMPONENTS, + UMAP_N_NEIGHBORS, + fetch_embeddings, + get_connection, + recompute_umap, + reduce_to_2d, + save_coords, + update_style_profiles, +) -# --------------------------------------------------------------------------- -# Logging -# --------------------------------------------------------------------------- +# Re-export for tests / callers that still import from this module. +__all__ = [ + "DEFAULT_CHUNK_TABLE", + "UMAP_METRIC", + "UMAP_MIN_DIST", + "UMAP_N_COMPONENTS", + "UMAP_N_NEIGHBORS", + "fetch_embeddings", + "get_connection", + "recompute_umap", + "reduce_to_2d", + "run", + "save_coords", + "update_style_profiles", +] logging.basicConfig( level=logging.INFO, @@ -80,312 +68,17 @@ datefmt="%Y-%m-%dT%H:%M:%S", stream=sys.stderr, ) -log = logging.getLogger(__name__) - -# --------------------------------------------------------------------------- -# Database helpers -# --------------------------------------------------------------------------- - - -def get_connection(database_url: str | None = None) -> psycopg2.extensions.connection: - """Return a psycopg2 connection. - - Parameters - ---------- - database_url: - Full PostgreSQL DSN. If *None*, reads ``DATABASE_URL`` from the - environment. The DSN must be compatible with psycopg2 (plain - ``postgresql://`` scheme, not ``postgresql+asyncpg://``). - - Raises - ------ - KeyError - If ``DATABASE_URL`` is not set and *database_url* was not provided. - """ - url = database_url or os.environ["DATABASE_URL"] - # Strip the "+asyncpg" driver suffix if the DSN was copied from db.py. - url = url.replace("postgresql+asyncpg://", "postgresql://") - return psycopg2.connect(url) - - -def _as_vector(value: Any) -> list[float]: - """Return a pgvector column value as a list of floats. - - psycopg2 hands back ``"[0.013,-0.011,…]"`` (a string) for a ``vector`` - column unless the pgvector adapter is registered on the connection; other - drivers return a real sequence. Both are accepted so this does not depend - on how the caller built the connection. - """ - if isinstance(value, str): - return [float(x) for x in value.strip().lstrip("[").rstrip("]").split(",") if x] - return list(value) - - -def fetch_embeddings( - conn: psycopg2.extensions.connection, - chunk_table: str = DEFAULT_CHUNK_TABLE, -) -> tuple[list[str], np.ndarray]: - """Fetch author UUIDs and their chunk embeddings from the database. - - The join path is: - → public.documents → public.authors - - Only rows where ``embedding IS NOT NULL`` are returned. - - Parameters - ---------- - conn: - Open psycopg2 connection. - chunk_table: - Fully-qualified table name that holds the embedding column - (default: ``public.chunks``). - - Returns - ------- - author_ids : list[str] - One UUID string per chunk row (may repeat for the same author). - embeddings : np.ndarray - Float32 array of shape ``(N, 768)``. - - Raises - ------ - RuntimeError - If no embedded chunks are found. - """ - # Split schema.table so we can reference them separately in the query. - # If the caller omits the schema, default to "public". - if "." in chunk_table: - schema, table = chunk_table.split(".", 1) - else: - schema, table = "public", chunk_table - - sql = f""" - SELECT d.author_id::text, c.embedding - FROM {schema}.{table} c - JOIN public.documents d ON d.id = c.document_id - WHERE c.embedding IS NOT NULL - ORDER BY d.author_id - """ # ORDER BY keeps same-author rows together (cosmetic, not required) - - log.info("Querying embeddings from %s.%s …", schema, table) - with conn.cursor() as cur: - cur.execute(sql) - rows: list[Any] = cur.fetchall() - - if not rows: - raise RuntimeError( - f"No embedded chunks found in {chunk_table}. " "Run backfill_embeddings() first." - ) - - author_ids: list[str] = [row[0] for row in rows] - # pgvector values arrive as the *string* "[0.013,-0.011,…]" unless the - # pgvector type is registered on the connection, which plain psycopg2 does - # not do. Feeding those strings to np.array raises - # "could not convert string to float", which is why this script had never - # completed a real run (#16 shipped as implemented-but-disconnected; same - # class of defect as #107 on the write side). Accept both shapes. - embeddings = np.array([_as_vector(row[1]) for row in rows], dtype=np.float32) - - log.info("Fetched %d embedded chunks across %d authors.", len(rows), len(set(author_ids))) - return author_ids, embeddings - - -# --------------------------------------------------------------------------- -# UMAP reduction -# --------------------------------------------------------------------------- - - -def reduce_to_2d(embeddings: np.ndarray) -> np.ndarray: - """Fit UMAP on *embeddings* and return 2-D coordinates. - - Parameters - ---------- - embeddings: - Float32 array of shape ``(N, D)``. D is typically 768. - - Returns - ------- - np.ndarray - Float64 array of shape ``(N, 2)``. - - Notes - ----- - UMAP requires at least ``n_neighbors + 1 = 16`` rows to fit. The check - is done by the caller (``run``). - """ - reducer = umap.UMAP( - n_neighbors=UMAP_N_NEIGHBORS, - min_dist=UMAP_MIN_DIST, - metric=UMAP_METRIC, - n_components=UMAP_N_COMPONENTS, - random_state=42, # reproducible layouts across runs - ) - log.info( - "Fitting UMAP (n_neighbors=%d, min_dist=%.2f, metric=%s) on %d vectors …", - UMAP_N_NEIGHBORS, - UMAP_MIN_DIST, - UMAP_METRIC, - embeddings.shape[0], - ) - coords: np.ndarray = reducer.fit_transform(embeddings) - log.info("UMAP fit complete. Output shape: %s", coords.shape) - return coords # (N, 2), float64 - - -# --------------------------------------------------------------------------- -# Storage -# --------------------------------------------------------------------------- - - -def save_coords( - conn: psycopg2.extensions.connection, - author_ids: list[str], - coords: np.ndarray, -) -> None: - """Truncate umap_coords and bulk-insert new (author_id, x, y) rows. - - Parameters - ---------- - conn: - Open psycopg2 connection. - author_ids: - List of UUID strings, one per row; must be same length as *coords*. - coords: - Float64 array of shape ``(N, 2)``; coords[:, 0] = x, coords[:, 1] = y. - """ - rows = [(aid, float(coords[i, 0]), float(coords[i, 1])) for i, aid in enumerate(author_ids)] - - with conn.cursor() as cur: - log.info("Truncating public.umap_coords …") - cur.execute("TRUNCATE TABLE public.umap_coords RESTART IDENTITY") - - log.info("Inserting %d rows into public.umap_coords …", len(rows)) - psycopg2.extras.execute_values( - cur, - "INSERT INTO public.umap_coords (author_id, x, y) VALUES %s", - rows, - page_size=1000, - ) - - conn.commit() - log.info("Done — %d umap_coords rows committed.", len(rows)) - - -def update_style_profiles( - conn: psycopg2.extensions.connection, - author_ids: list[str], - coords: np.ndarray, -) -> int: - """Write each author's 2-D centroid and spread into their StyleProfile. - - ``umap_coords`` holds one row per *chunk*, but the Style DNA scatter reads - ``style_profiles.json_data.embedding_umap_2d`` — a per-*author* centroid and - spread. Nothing connected the two, so that field kept the placeholder the - extractor writes (``{"centroid": [0.0, 0.0], "spread": 0.0}``) and all three - authors were drawn on top of each other at the origin (#88). - - ``spread`` is the mean Euclidean distance from an author's chunks to their - own centroid — a readable radius for the ring the scatter draws, and the - reason the projection has to be global: fitting UMAP on three centroids - alone would be meaningless (it needs n_neighbors+1 points), and any - per-author fit would place each author in its own unrelated coordinate - system. - - Only the most recent profile row per author is updated; older rows are - history and stay as they were. - - Returns the number of profile rows updated. - """ - by_author: dict[str, list[tuple[float, float]]] = {} - for i, aid in enumerate(author_ids): - by_author.setdefault(aid, []).append((float(coords[i, 0]), float(coords[i, 1]))) - - updated = 0 - with conn.cursor() as cur: - for author_id, points in by_author.items(): - arr = np.asarray(points, dtype=np.float64) - centroid = arr.mean(axis=0) - spread = float(np.linalg.norm(arr - centroid, axis=1).mean()) - payload = json.dumps( - {"centroid": [float(centroid[0]), float(centroid[1])], "spread": spread} - ) - - # jsonb_set on the latest row only: style_profiles is append-only - # (a recompute inserts, never updates), so the current profile is - # the one with the newest computed_at. - cur.execute( - """ - UPDATE public.style_profiles AS sp - SET json_data = jsonb_set( - sp.json_data, '{embedding_umap_2d}', %s::jsonb, true) - WHERE sp.id = ( - SELECT id FROM public.style_profiles - WHERE author_id = %s - ORDER BY computed_at DESC - LIMIT 1) - """, - (payload, author_id), - ) - updated += cur.rowcount - log.info( - "author %s: centroid=(%.3f, %.3f) spread=%.3f over %d chunks", - author_id, - centroid[0], - centroid[1], - spread, - len(points), - ) - - conn.commit() - log.info("Updated embedding_umap_2d on %d style_profiles row(s).", updated) - return updated - - -# --------------------------------------------------------------------------- -# Orchestrator -# --------------------------------------------------------------------------- def run( database_url: str | None = None, chunk_table: str | None = None, ) -> None: - """Full pipeline: fetch → reduce → store. - - Parameters - ---------- - database_url: - PostgreSQL DSN. Falls back to ``DATABASE_URL`` env var. - chunk_table: - Source table for embeddings. Falls back to ``CHUNK_TABLE`` env var, - then to ``DEFAULT_CHUNK_TABLE`` (``public.chunks``). - """ - table = chunk_table or os.environ.get("CHUNK_TABLE", DEFAULT_CHUNK_TABLE) - - conn = get_connection(database_url) - try: - author_ids, embeddings = fetch_embeddings(conn, chunk_table=table) - - min_required = UMAP_N_NEIGHBORS + 1 - if len(author_ids) < min_required: - log.error( - "UMAP requires at least %d rows but only %d embedded chunks were found. " - "Embed more chunks and re-run.", - min_required, - len(author_ids), - ) - sys.exit(1) - - coords = reduce_to_2d(embeddings) - save_coords(conn, author_ids, coords) - update_style_profiles(conn, author_ids, coords) - finally: - conn.close() - + """CLI entry: exit 1 when UMAP cannot run (too few chunks).""" + ok = recompute_umap(database_url=database_url, chunk_table=chunk_table) + if not ok: + sys.exit(1) -# --------------------------------------------------------------------------- -# Entry point -# --------------------------------------------------------------------------- if __name__ == "__main__": run() From 938b5e34db581de442778028f2cace7074cc640e Mon Sep 17 00:00:00 2001 From: Pablo Ch Date: Thu, 30 Jul 2026 21:04:53 +0200 Subject: [PATCH 2/2] feat(ui): delete live-added authors; protect demo voices Hide remove control for Austen/Dickens/Poe and return 403 from DELETE so the gallery can drop custom authors without wiping the pitch seeds. --- backend/app/routes/authors.py | 18 +++- backend/tests/test_delete_author.py | 10 ++ docs/api_contract.yaml | 14 +++ docs/decision_log.md | 1 + frontend/src/components/AuthorCard.tsx | 97 ++++++++++--------- .../src/components/DeleteAuthorButton.tsx | 68 +++++++++++++ frontend/src/lib/api.ts | 17 ++++ frontend/src/lib/authors.ts | 9 ++ frontend/src/lib/i18n/en.ts | 5 + 9 files changed, 194 insertions(+), 45 deletions(-) create mode 100644 frontend/src/components/DeleteAuthorButton.tsx diff --git a/backend/app/routes/authors.py b/backend/app/routes/authors.py index e12c5c2..4587f89 100644 --- a/backend/app/routes/authors.py +++ b/backend/app/routes/authors.py @@ -34,7 +34,7 @@ import sys import threading from pathlib import Path -from typing import Annotated, Any +from typing import Annotated, Any, Final import tiktoken from fastapi import APIRouter, BackgroundTasks, File, Form, HTTPException, Response, UploadFile @@ -58,6 +58,9 @@ _CHUNK_SIZE: int = 500 _CHUNK_OVERLAP: int = 50 +# Demo-seeded voices — never deletable (UI hides the control; API enforces). +PROTECTED_AUTHOR_SLUGS: Final[frozenset[str]] = frozenset({"austen", "dickens", "poe"}) + # Serialize global UMAP fits so overlapping uploads do not race truncate+insert. _UMAP_LOCK = threading.Lock() @@ -309,6 +312,7 @@ async def list_authors() -> list[AuthorSummary]: summary="Delete an author and their corpus", operation_id="deleteAuthor", responses={ + 403: {"description": "Preloaded author (austen/dickens/poe) cannot be deleted"}, 404: {"description": "Unknown author_id"}, 500: {"description": "Unexpected server error"}, }, @@ -319,11 +323,23 @@ async def delete_author( ) -> Response: """Delete *author_id* (slug) and cascade corpus rows; refresh UMAP after. + Preloaded demo authors (``austen``, ``dickens``, ``poe``) return 403. ``umap_coords`` has no FK to ``authors``, so those rows are deleted explicitly before the author row. Documents / chunks / style_profiles / passports cascade from ``authors``. Global UMAP is recomputed in a background task so remaining authors keep separated scatter positions. """ + if author_id in PROTECTED_AUTHOR_SLUGS: + raise HTTPException( + status_code=403, + detail={ + "error": "forbidden", + "message": ( + f"Author '{author_id}' is a preloaded demo voice and cannot be deleted" + ), + }, + ) + sb = get_client() author_result = sb.table("authors").select("id").eq("slug", author_id).maybe_single().execute() diff --git a/backend/tests/test_delete_author.py b/backend/tests/test_delete_author.py index d1c61e8..67b27ea 100644 --- a/backend/tests/test_delete_author.py +++ b/backend/tests/test_delete_author.py @@ -67,3 +67,13 @@ def test_delete_unknown_author_404(mock_get_client: MagicMock) -> None: assert resp.status_code == 404 body = resp.json() assert body["detail"]["error"] == "not_found" + + +@patch("app.routes.authors.get_client") +def test_delete_preloaded_author_403(mock_get_client: MagicMock) -> None: + """Austen / Dickens / Poe are demo seeds — API refuses delete.""" + for slug in ("austen", "dickens", "poe"): + resp = client.delete(f"/api/authors/{slug}") + assert resp.status_code == 403, slug + assert resp.json()["detail"]["error"] == "forbidden" + mock_get_client.assert_not_called() diff --git a/docs/api_contract.yaml b/docs/api_contract.yaml index 504f9eb..b099e60 100644 --- a/docs/api_contract.yaml +++ b/docs/api_contract.yaml @@ -105,11 +105,25 @@ paths: passports). Also clears that author's `umap_coords` rows (no FK) and enqueues a background global UMAP recompute so remaining authors keep separated Style DNA scatter centroids. + + The three preloaded demo voices (`austen`, `dickens`, `poe`) cannot be + deleted — the API returns 403 and the gallery UI hides the control. parameters: - $ref: "#/components/parameters/AuthorId" responses: "204": description: Author deleted; UMAP recompute enqueued + "403": + description: Preloaded author cannot be deleted + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + examples: + preloaded: + value: + error: forbidden + message: Author 'austen' is a preloaded demo voice and cannot be deleted "404": description: Unknown author_id content: diff --git a/docs/decision_log.md b/docs/decision_log.md index 987c581..c180589 100644 --- a/docs/decision_log.md +++ b/docs/decision_log.md @@ -61,3 +61,4 @@ Every decision that affects the product, the process, or the team lives here. Ap | 2026-07-30 | **RATIFIED — adopt the Jeffreys-prior log-odds + NOUN/ADJ/ADV filter for `distinctive_vocab` (supersedes the Monroe z-score implementation ratified earlier the same day).** Side-by-side measurement on the real corpus compared three combinations: (1) Monroe z-score + PROPN-only drop, (2) Jeffreys α=0.5 log-odds + NOUN/ADJ/ADV allow-list (from `feat/better-response`), (3) hybrid Monroe z-score + NOUN/ADJ/ADV. Top-10 overlap between (1) and (2) was **0/0/1** across Austen/Dickens/Poe; hybrid (3) still ranked high-frequency nouns (`sister`, `old`, `hand`) rather than signature lexicon. The POS filter alone is therefore **not** enough — the scorer without a variance term is what promotes rare, concentrated terms (`matrimony`, `workhouse`, `ballast`) that a non-technical juror reads as style. Adopted: `vocabulary.py` = Jeffreys log-odds with scores normalized to `[0, 1]`; `_lemmas_from_docs` = keep only NOUN/ADJ/ADV + small corpus-metadata stop list. Kept: `[Illustration]` stripping in `cleaner.py` (orthogonal, still useful). `docs/style_features.md` §4.1, schema, api_contract prose, and frontend score comments updated. Re-seed (`--with-profiles` + UMAP) still required for live DB rows. `fit_score` formula changes on `feat/better-response` remain flagged, not adopted | Pablo (explicit choice after reviewing measured comparison) | Style DNA is the feature a jury *reads*; juror-legible signature vocab beats a statistically more conservative z-score that surfaces narrative-frequency terms. Scores in `[0, 1]` also restore the contract range the frontend originally assumed | | 2026-07-30 | **RATIFIED — port the remaining important pieces of `feat/better-response` onto `feat/style-profile-and-fit-score-improvements`.** (a) `conditioner.py` numbered system-prompt template with dialogue_rule + run-on guard on heavy subordination (plus a fix: read `dialogue_ratio` from `stylistic`, not `syntactic` — the branch had that key under the wrong block, so the rule always fell to the zero-dialogue default). (b) `fit_scorer.py` LOCKED-formula edits previously flagged in this log: `_lexical_score` uses absolute error `1 − \|ttr − mattr\|`; `_vocabulary_score` uses `min(1, \|∩ top15\| / 5)`. Documented in `docs/style_features.md` §6. (c) `backend/app/routes/authors.py`: comparison corpora via `lemmatize_corpus` (fixes identical-vocab collapse from raw `.lower()`), auto-create author on upload when missing, auto-recompute style profile after chunk+embed. Skipped from that branch: `keys/jwks.public.json` rotation, ad-hoc debug scripts, synthetic demo corpora | Pablo (explicit: add the important code from that branch to ours) | The colleague's prompt + fit_score calibration are what produced the better measured generations; keeping them off the integration branch would leave the vocab work without the generation path that uses it | | 2026-07-30 | **Contract amended (2/3 vote pending): DELETE /api/authors/{author_id} + automatic global UMAP recompute on author add/remove.** UMAP 2D (embedding_umap_2d) is a global fit over all embedded chunks; previously it only ran via scripts/precompute_umap.py, so new uploads left scatter centroids at [0,0] and SQL deletes left stale points for removed authors. **(a)** Core pipeline extracted to i_pipeline/autoria_ai/umap_projector.py::recompute_umap (returns bool; soft-skips when <16 embedded chunks); CLI wrapper kept. **(b)** After a successful StyleProfile insert (upload or manual recompute), _recompute_umap_safe runs under a process-level lock. **(c)** New DELETE /api/authors/{author_id} → 204: deletes umap_coords for that author (no FK), deletes the author row (cascade docs/chunks/profiles/passports), enqueues UMAP recompute. docs/api_contract.yaml amended accordingly. Does **not** auto-recompute other authors' distinctive_vocab | Pablo (requested automatic UMAP on add/remove) | Style DNA scatter is the demo wow moment; leaving UMAP manual after every corpus change silently breaks author separation | +| 2026-07-30 | **DELETE author UX + protect preloaded voices.** Gallery shows a remove control only on live-added authors; Austen/Dickens/Poe have no button. API returns 403 for those three slugs (PROTECTED_AUTHOR_SLUGS) so curl/scripts cannot wipe the demo seeds either. Contract documents the 403 | Pablo | Without UI, DELETE is unused in the demo; without a hard block, a mis-click or script could erase the three voices the pitch depends on | diff --git a/frontend/src/components/AuthorCard.tsx b/frontend/src/components/AuthorCard.tsx index 8327831..50aa528 100644 --- a/frontend/src/components/AuthorCard.tsx +++ b/frontend/src/components/AuthorCard.tsx @@ -1,5 +1,6 @@ import Link from "next/link"; +import { DeleteAuthorButton } from "@/components/DeleteAuthorButton"; import { Card, CardContent, @@ -8,6 +9,7 @@ import { CardTitle, } from "@/components/ui/card"; import { Badge } from "@/components/ui/badge"; +import { isDeletableAuthor } from "@/lib/authors"; import { en } from "@/lib/i18n/en"; import type { AuthorCardData } from "@/lib/types"; @@ -21,54 +23,61 @@ function monogramOf(name: string): string { } export function AuthorCard({ author }: AuthorCardProps) { + const deletable = isDeletableAuthor(author.id); + return ( - - -