From 95cc80dc72d9345ce874a8efa6ed48a0aaeb254b Mon Sep 17 00:00:00 2001 From: Pablo Ch Date: Wed, 29 Jul 2026 10:42:10 +0200 Subject: [PATCH 1/5] fix(ai_pipeline): declare scipy dependency and lazy-load embedding model scipy was only pulled in transitively via scikit-learn even though fit_scorer.py imports it directly (#105) - declare it explicitly. Also stop constructing the 418MB SentenceTransformer at module import time: any transitive import of embedder.py (e.g. via backend/db.py) paid the cold-start cost. Use a lazy singleton (ensure_model_loaded) instead, and pre-fetch the model into the HuggingFace cache during the Railway build step so cold starts read from local disk rather than the network (#104). Closes #105 Closes #104 --- ai_pipeline/autoria_ai/embedder.py | 56 +++++++++++++++++++++----- ai_pipeline/autoria_ai/generator.py | 7 ++-- ai_pipeline/pyproject.toml | 3 ++ ai_pipeline/tests/test_embedder.py | 62 +++++++++++++++++++++++++++++ railway.toml | 41 ++++++++++++------- requirements-deploy.txt | 21 +++++----- 6 files changed, 153 insertions(+), 37 deletions(-) diff --git a/ai_pipeline/autoria_ai/embedder.py b/ai_pipeline/autoria_ai/embedder.py index df2626d..3a8084d 100644 --- a/ai_pipeline/autoria_ai/embedder.py +++ b/ai_pipeline/autoria_ai/embedder.py @@ -4,9 +4,30 @@ ---------- embed_chunks(texts: list[str]) -> np.ndarray Batch-encode *texts* and return a float32 array of shape ``(N, 768)``. +ensure_model_loaded() -> SentenceTransformer + Force the model to load now (used by ``autoria_ai.generator.warmup_models`` + to pre-warm at process startup) and return the singleton. -The ``SentenceTransformer`` model is loaded **once** at module import as a -module-level singleton. Callers must never reload it per request. +Model loading +------------- +The ``SentenceTransformer`` is a **lazily-constructed** process-level +singleton: it loads on first call to ``embed_chunks`` or +``ensure_model_loaded``, not at import time (#104). Constructing it used to +happen as a bare module-level statement, so *any* import of this module — +including ``autoria_ai.db`` and ``autoria_ai.extractor.style_profile``, which +import it just for ``EMBEDDING_DIM`` / ``embed_chunks`` and may not need to +actually embed anything — paid the full ~418 MB HuggingFace download/load +unconditionally. Lazy construction means importing this module is cheap; the +cost is paid exactly once, by whichever caller first needs embeddings. + +This does **not** by itself remove the cost from a cold Railway boot: prod +still calls ``ensure_model_loaded()`` during FastAPI's lifespan (via +``autoria_ai.generator.warmup_models``) so the first real request isn't the +one paying for it. What it removes is the download turning into *network* +time at all when it can be avoided: ``railway.toml``'s ``buildCommand`` now +pre-fetches this model into the HuggingFace cache at **build** time (the same +technique already used there for the spaCy model), so a cold boot loads it +from local disk instead of downloading it fresh, deploy or restart. """ from __future__ import annotations @@ -15,18 +36,30 @@ from sentence_transformers import SentenceTransformer # type: ignore[import-untyped] # --------------------------------------------------------------------------- -# Module-level singleton — loaded once, reused for every call. -# The model name is the canonical one from docs/MVP.md §4.1 and -# docs/style_features.md §5. +# Module-level singleton — constructed lazily on first use (see module +# docstring), reused for every call afterwards. The model name is the +# canonical one from docs/MVP.md §4.1 and docs/style_features.md §5. # --------------------------------------------------------------------------- _MODEL_NAME = "all-mpnet-base-v2" -_MODEL: SentenceTransformer = SentenceTransformer(_MODEL_NAME) +_MODEL: SentenceTransformer | None = None #: Dimensionality guaranteed by all-mpnet-base-v2 — used as the assertion #: guard in embed_chunks() and as the vector(768) column width in the DB. EMBEDDING_DIM: int = 768 +def ensure_model_loaded() -> SentenceTransformer: + """Return the process-level SentenceTransformer singleton, loading it + on the first call. Safe to call more than once or from more than one + caller (e.g. an explicit warmup *and* the first real ``embed_chunks`` + call racing on it) — the model is constructed at most once per process. + """ + global _MODEL + if _MODEL is None: + _MODEL = SentenceTransformer(_MODEL_NAME) + return _MODEL + + def embed_chunks(texts: list[str]) -> np.ndarray: """Batch-encode *texts* and return embeddings as a float32 NumPy array. @@ -44,16 +77,17 @@ def embed_chunks(texts: list[str]) -> np.ndarray: Notes ----- - * Uses ``_MODEL.encode()`` in batch mode with ``convert_to_numpy=True`` - so the result is a contiguous C-order float32 array compatible with + * Uses ``.encode()`` in batch mode with ``convert_to_numpy=True`` so + the result is a contiguous C-order float32 array compatible with pgvector's ``VECTOR(768)`` column. - * The model is **not** reloaded here; the module-level ``_MODEL`` - singleton is used throughout the process lifetime. + * The model singleton is loaded on first call (see + ``ensure_model_loaded``) and never reloaded per request. """ if not texts: return np.zeros((0, EMBEDDING_DIM), dtype=np.float32) - embeddings: np.ndarray = _MODEL.encode( + model = ensure_model_loaded() + embeddings: np.ndarray = model.encode( texts, batch_size=64, convert_to_numpy=True, diff --git a/ai_pipeline/autoria_ai/generator.py b/ai_pipeline/autoria_ai/generator.py index 3448474..b0bd90b 100644 --- a/ai_pipeline/autoria_ai/generator.py +++ b/ai_pipeline/autoria_ai/generator.py @@ -87,10 +87,11 @@ def _ensure_models() -> tuple[Any, Any]: _nlp = spacy.load("en_core_web_lg") logger.info("spaCy en_core_web_lg ready.") if _embedding_model is None: - # Reuse embedder singleton (loads all-mpnet-base-v2 once). - from autoria_ai.embedder import _MODEL as emb + # Reuse embedder singleton, loading all-mpnet-base-v2 on this first + # call rather than at import time (#104). + from autoria_ai.embedder import ensure_model_loaded - _embedding_model = emb + _embedding_model = ensure_model_loaded() logger.info("sentence-transformers embedding model ready.") if _tok_enc is None: _tok_enc = tiktoken.get_encoding("cl100k_base") diff --git a/ai_pipeline/pyproject.toml b/ai_pipeline/pyproject.toml index 12d6d3f..af7fd86 100644 --- a/ai_pipeline/pyproject.toml +++ b/ai_pipeline/pyproject.toml @@ -13,6 +13,9 @@ dependencies = [ "umap-learn>=0.5", "tiktoken>=0.7", "numpy>=1.26", + # Direct import in autoria_ai/fit_scorer.py:22. Previously undeclared and + # only present transitively via scikit-learn (see #105). + "scipy>=1.11", "pydantic>=2.7", "python-jose[cryptography]>=3.3", "jsonschema[format]>=4.18", diff --git a/ai_pipeline/tests/test_embedder.py b/ai_pipeline/tests/test_embedder.py index b6b1344..3bb7262 100644 --- a/ai_pipeline/tests/test_embedder.py +++ b/ai_pipeline/tests/test_embedder.py @@ -50,10 +50,13 @@ import os import statistics +import subprocess +import sys import time import uuid from contextlib import asynccontextmanager from dataclasses import dataclass +from pathlib import Path import numpy as np import pytest @@ -143,6 +146,65 @@ def test_embed_chunks_batch_matches_individual() -> None: ) +def test_ensure_model_loaded_returns_the_same_singleton_every_time() -> None: + """Repeated calls (from embed_chunks or a warmup) share one instance.""" + from autoria_ai.embedder import ensure_model_loaded + + first = ensure_model_loaded() + second = ensure_model_loaded() + assert first is second + + +# --------------------------------------------------------------------------- +# Part A3 — lazy loading (#104): importing this module must not construct the +# SentenceTransformer. Run in a subprocess with sentence_transformers stubbed +# out (raising if constructed), so the assertion holds regardless of whether +# an earlier test in this same session already triggered the real load. +# --------------------------------------------------------------------------- + +_LAZY_IMPORT_CHECK_SCRIPT = """ +import sys +from unittest.mock import MagicMock + +fake_st_module = MagicMock() +fake_st_module.SentenceTransformer = MagicMock( + side_effect=AssertionError("SentenceTransformer constructed at import time (#104)") +) +sys.modules["sentence_transformers"] = fake_st_module + +import autoria_ai.embedder as embedder + +assert embedder._MODEL is None, "embedder._MODEL must be None until first use" +print("OK") +""" + + +def test_module_import_does_not_construct_the_model() -> None: + """Regression guard for #104. + + ``autoria_ai.db`` and ``autoria_ai.extractor.style_profile`` both import + this module at their own top level just for ``EMBEDDING_DIM`` / + ``embed_chunks``, and may never actually call either. Before the fix, + merely importing ``autoria_ai.embedder`` — directly or transitively — + unconditionally constructed (and for a cold process, downloaded) the + ~418 MB SentenceTransformer. A subprocess with sentence_transformers + stubbed out makes the assertion fail loudly (ImportError from the + stubbed constructor) instead of silently passing because a previous + test already warmed the real model in this process. + """ + ai_pipeline_root = Path(__file__).resolve().parents[1] + result = subprocess.run( + [sys.executable, "-c", _LAZY_IMPORT_CHECK_SCRIPT], + capture_output=True, + text=True, + timeout=30, + cwd=ai_pipeline_root, + env={**os.environ, "PYTHONPATH": str(ai_pipeline_root)}, + ) + assert result.returncode == 0, f"stdout={result.stdout!r} stderr={result.stderr!r}" + assert "OK" in result.stdout + + # --------------------------------------------------------------------------- # Part A2 — driver-binding regression tests (no DB, always run) # diff --git a/railway.toml b/railway.toml index 04486c1..7b65b97 100644 --- a/railway.toml +++ b/railway.toml @@ -14,26 +14,39 @@ # The builder's own install phase already runs `pip install -r requirements.txt` # (the root shim); buildCommand runs after it and layers the pipeline deps on top. # -# The second half of buildCommand fetches the spaCy model `en_core_web_lg`. -# It is NOT a PyPI package (`pip install en_core_web_lg` 404s), so it cannot be -# a line in requirements-deploy.txt; `python -m spacy download` resolves the -# model release matching the spaCy version pip just installed. Without it -# `autoria_ai.generator._ensure_models()` raises OSError [E050] and every -# POST /api/generate returns 500 — while /health still reports 200, because -# backend/app/main.py swallows the warmup failure. - +# The rest of buildCommand pre-fetches two ML models so a cold boot loads +# them from local disk instead of downloading them at runtime: +# +# 1. spaCy `en_core_web_lg`. NOT a PyPI package (`pip install en_core_web_lg` +# 404s), so it cannot be a line in requirements-deploy.txt; `python -m +# spacy download` resolves the model release matching the spaCy version +# pip just installed. Without it `autoria_ai.generator._ensure_models()` +# raises OSError [E050] and every POST /api/generate returns 500 — while +# /health still reports 200, because backend/app/main.py swallows the +# warmup failure. +# 2. sentence-transformers `all-mpnet-base-v2` (~418 MB from HuggingFace). +# `autoria_ai/embedder.py` loads this lazily on first use rather than at +# import (#104), but the FastAPI lifespan still calls +# `autoria_ai.generator.warmup_models()` at startup so the first real +# request isn't the one paying for it — that warmup is what this step +# turns into a local-disk load instead of a network fetch. Written into +# `~/.cache/huggingface`, the same cache directory (and technique) already +# used to speed up this exact download in .github/workflows/ci.yml. [build] builder = "NIXPACKS" -buildCommand = "pip install -r requirements-deploy.txt && python -m spacy download en_core_web_lg" +buildCommand = "pip install -r requirements-deploy.txt && python -m spacy download en_core_web_lg && python -c \"from sentence_transformers import SentenceTransformer; SentenceTransformer('all-mpnet-base-v2')\"" [deploy] startCommand = "cd backend && PYTHONPATH=../ai_pipeline uvicorn app.main:app --host 0.0.0.0 --port $PORT" healthcheckPath = "/health" -# 300s, not the default 100s: ai_pipeline/autoria_ai/embedder.py:23 constructs -# SentenceTransformer("all-mpnet-base-v2") at MODULE import, so the first boot -# downloads ~418 MB from HuggingFace inside the FastAPI lifespan — before the -# app can answer the first /health probe. At 100s that download does not finish, -# the healthcheck fails and Railway kills the deploy. +# 300s, not the default 100s. Even with the model pre-fetched into the image +# at build time (above), the FastAPI lifespan still loads it from disk into +# memory before the app can answer the first /health probe, on top of spaCy +# and Watsonx client setup — comfortably under 300s from local disk, but a +# transient miss on the build-time cache (or a Railway image rebuild that +# drops it) would fall back to the ~418 MB network download this timeout was +# originally sized for. Kept wide rather than re-tuned tight, since a slow +# healthcheck delays a deploy but a falsely-tight one kills it outright. healthcheckTimeout = 300 restartPolicyType = "ON_FAILURE" restartPolicyMaxRetries = 3 diff --git a/requirements-deploy.txt b/requirements-deploy.txt index adc9498..e77c4c9 100644 --- a/requirements-deploy.txt +++ b/requirements-deploy.txt @@ -72,10 +72,10 @@ spacy>=3.7 sentence-transformers>=2.7 scikit-learn>=1.4 numpy>=1.26 -# scipy is a *direct* import in ai_pipeline/autoria_ai/fit_scorer.py:22 but is -# missing from ai_pipeline/pyproject.toml (it only arrives transitively via -# scikit-learn). Pinned explicitly here so the deploy image does not depend on -# a transitive edge. Declaring it upstream is out of scope for WO-02. +# Direct import in ai_pipeline/autoria_ai/fit_scorer.py:22, now declared +# upstream in ai_pipeline/pyproject.toml (#105). Kept listed here too, like +# every other ai_pipeline dependency in this file, since this file mirrors +# ai_pipeline/pyproject.toml's runtime-reachable subset, not just backend's. scipy>=1.11 # DB / vector store — SQLAlchemy 2.0 async + pgvector (asyncpg is the driver @@ -97,11 +97,14 @@ pgvector>=0.3 # on the hobby plan), offline-only tooling stays out of the runtime image. Run # those scripts from a dev machine with `pip install -e "ai_pipeline[dev]"`. # -# COLD-START NOTE — `autoria_ai/embedder.py:23` builds SentenceTransformer( -# "all-mpnet-base-v2") at MODULE level, so importing autoria_ai.db downloads -# ~418 MB from HuggingFace at RUNTIME (first boot), not at build time. That runs -# inside the FastAPI lifespan, before the first /health is served, which is why -# `railway.toml` sets healthcheckTimeout = 300. +# COLD-START NOTE — `autoria_ai/embedder.py` loads SentenceTransformer( +# "all-mpnet-base-v2") lazily, on first use rather than at import (#104), but +# the FastAPI lifespan still calls autoria_ai.generator.warmup_models() at +# startup so the first real request isn't the one paying for it. `railway.toml`'s +# buildCommand now pre-fetches this model into the image at BUILD time (same +# technique as the en_core_web_lg line below), so that startup load reads from +# local disk instead of downloading ~418 MB from HuggingFace at RUNTIME. 300s +# healthcheckTimeout is kept as a safety margin for a cold cache. # # NOT A PIP DEPENDENCY — the spaCy model `en_core_web_lg` (~560 MB) cannot be # listed here: it is not published to PyPI (`pip install en_core_web_lg` 404s). From eac2fa52b9937a36679f76c0deaed8fb5c15c209 Mon Sep 17 00:00:00 2001 From: Pablo Ch Date: Wed, 29 Jul 2026 10:42:33 +0200 Subject: [PATCH 2/5] fix(ai_pipeline): enforce a real token budget in the conditioner prompt Capping the system prompt at 5 RAG chunks does not bound the token count: 5 passages of ~500 tokens each blow past the ~1200 token budget (PR #125 measured ~2,590 real tokens). Pack chunks against an actual tiktoken (cl100k_base) count instead, truncating the last chunk that does not fit at the nearest sentence/word boundary rather than mid-word. Adds 6 tests reproducing the over-budget scenario and covering truncation edge cases (mid-word safety, short chunks, empty input). Closes #90 --- ai_pipeline/autoria_ai/conditioner.py | 141 ++++++++++++++++++++++++-- ai_pipeline/tests/test_conditioner.py | 67 +++++++++++- 2 files changed, 196 insertions(+), 12 deletions(-) diff --git a/ai_pipeline/autoria_ai/conditioner.py b/ai_pipeline/autoria_ai/conditioner.py index 40221d0..b6e009e 100644 --- a/ai_pipeline/autoria_ai/conditioner.py +++ b/ai_pipeline/autoria_ai/conditioner.py @@ -7,14 +7,32 @@ to adopt an author's exact style, following the template in docs/MVP.md §4.3. Latency contract ----------------- -The returned string is kept under ~1200 tokens (architecture.md §6). The -safeguard is applied by truncating ``rag_chunks`` to at most 5 items before -they are interpolated. +----------------- +The returned string is kept at or under ``_MAX_PROMPT_TOKENS`` (1200 tokens, +cl100k_base) — the stricter of the two figures in circulation (architecture.md +§6 says "< ~1200 tok"; issue #23 said "< 2000"). Two safeguards apply, in +order: + +1. ``rag_chunks`` is capped to at most ``_MAX_CHUNKS`` (5) items. This bounds + the *number* of passages but, since each chunk can be up to ~500 tokens + (the RAG chunking window — see ``backend/app/routes/authors.py`` and + ``scripts/seed_corpus.py``), does **not** by itself bound token count: 5 + chunks alone can exceed 2500 tokens before the rest of the template is + even added. +2. The capped chunks are then packed against the *actual* remaining token + budget (total budget minus everything else the template renders). Chunks + that fit whole are kept as-is; the first chunk that doesn't fit is + truncated to the last full sentence (or, failing that, the last full + word) within budget rather than being cut off mid-word or dropped + outright; any chunks after that are omitted. """ from __future__ import annotations +import re + +import tiktoken + # --------------------------------------------------------------------------- # Constants # --------------------------------------------------------------------------- @@ -22,6 +40,22 @@ _MAX_CHUNKS: int = 5 _MAX_VOCAB_TERMS: int = 15 +# docs/architecture.md §6: "lean system prompt (< ~1200 tok)". This is the +# stricter of the two numbers in circulation (issue #23 cites "< 2000"); the +# more restrictive one wins per the module's dispatch instructions. +_MAX_PROMPT_TOKENS: int = 1200 + +# Loaded once at import, matching the pattern used by +# autoria_ai/extractor/chunker.py and autoria_ai/extractor/style_profile.py. +_ENCODER = tiktoken.get_encoding("cl100k_base") + +_CHUNK_SEPARATOR = " | " +_NO_CHUNKS_FALLBACK = "(no example passages provided)" + +# Sentence end: terminal punctuation, optional closing quote/bracket, then +# whitespace. Used to back a truncated passage off to a clean boundary. +_SENTENCE_END_RE = re.compile(r"[.!?][\"'\u201d\u2019)\]]*\s") + # --------------------------------------------------------------------------- # Public function # --------------------------------------------------------------------------- @@ -36,6 +70,56 @@ ) +def _token_count(text: str) -> int: + return len(_ENCODER.encode(text)) + + +def _clip_to_boundary(text: str) -> str: + """Back `text` off to its last full sentence, else its last full word. + + Returns "" if neither a sentence nor a word boundary exists (e.g. `text` + is a single partial word), so the caller can drop it instead of emitting + a fragment. + """ + matches = list(_SENTENCE_END_RE.finditer(text)) + if matches: + return text[: matches[-1].end()].rstrip() + last_space = text.rfind(" ") + if last_space > 0: + return text[:last_space].rstrip() + return "" + + +def _fit_chunks_to_token_budget(chunks: list[str], budget_tokens: int) -> list[str]: + """Pack `chunks` in order into `budget_tokens`, truncating the first one + that doesn't fit whole to a clean boundary, and dropping the rest. + """ + if budget_tokens <= 0: + return [] + + kept: list[str] = [] + used = 0 + for chunk in chunks: + sep_cost = _token_count(_CHUNK_SEPARATOR) if kept else 0 + remaining = budget_tokens - used - sep_cost + if remaining <= 0: + break + + chunk_tokens = _ENCODER.encode(chunk) + if len(chunk_tokens) <= remaining: + kept.append(chunk) + used += sep_cost + len(chunk_tokens) + continue + + truncated = _ENCODER.decode(chunk_tokens[:remaining]).strip() + clipped = _clip_to_boundary(truncated) + if clipped: + kept.append(clipped) + break + + return kept + + def build_system_prompt(style_profile: dict, rag_chunks: list[str]) -> str: """Compose the conditioned system prompt for the Watsonx LLM. @@ -47,14 +131,17 @@ def build_system_prompt(style_profile: dict, rag_chunks: list[str]) -> str: on a partial profile. rag_chunks: Retrieved example passages (top-k by cosine similarity from pgvector). - At most ``_MAX_CHUNKS`` (5) are used; any extras are silently dropped to - keep the prompt within the ~1200-token latency budget. + At most ``_MAX_CHUNKS`` (5) are considered; the result is then packed + into the remaining token budget (see module docstring), so the + returned prompt never exceeds ``_MAX_PROMPT_TOKENS`` tokens + regardless of how long the individual chunks are. Returns ------- str A fully-rendered system prompt string ready to be passed as the - ``system`` parameter of a Watsonx chat-completion call. + ``system`` parameter of a Watsonx chat-completion call, guaranteed to + be at most ``_MAX_PROMPT_TOKENS`` tokens under cl100k_base. """ # -- author id ------------------------------------------------------------- author_id: str = style_profile.get("author_id", "unknown") @@ -85,14 +172,46 @@ def build_system_prompt(style_profile: dict, rag_chunks: list[str]) -> str: ] vocab_list: str = ", ".join(top_terms) if top_terms else "vivid and precise language" - # -- rag chunks (enforce hard cap of 5) ------------------------------------ - safe_chunks: list[str] = rag_chunks[:_MAX_CHUNKS] - chunks_text = " | ".join(safe_chunks) if safe_chunks else "(no example passages provided)" + # -- rag chunks: count cap first (existing safeguard), then token budget -- + count_capped_chunks: list[str] = rag_chunks[:_MAX_CHUNKS] + + # Everything except `chunks` is fixed at this point, so the true chunk + # budget is whatever tokens are left after rendering the rest of the + # template with the "no passages" fallback in place of the real chunks. + fixed_prompt = _TEMPLATE.format( + author_id=author_id, + avg_sentence_length=avg_sl_str, + subordination_rule=subordination_rule, + vocab_list=vocab_list, + chunks=_NO_CHUNKS_FALLBACK, + ) + fixed_cost = _token_count(fixed_prompt) - _token_count(_NO_CHUNKS_FALLBACK) + chunk_budget = max(0, _MAX_PROMPT_TOKENS - fixed_cost) + + safe_chunks = _fit_chunks_to_token_budget(count_capped_chunks, chunk_budget) + chunks_text = _CHUNK_SEPARATOR.join(safe_chunks) if safe_chunks else _NO_CHUNKS_FALLBACK - return _TEMPLATE.format( + prompt = _TEMPLATE.format( author_id=author_id, avg_sentence_length=avg_sl_str, subordination_rule=subordination_rule, vocab_list=vocab_list, chunks=chunks_text, ) + + # Defense in depth: BPE merges across the chunk/template boundary can + # shift the count by a token or two relative to the estimate above. If + # that ever pushes the total over budget, drop the last chunk and retry + # rather than ship a prompt over the contract. + while _token_count(prompt) > _MAX_PROMPT_TOKENS and safe_chunks: + safe_chunks = safe_chunks[:-1] + chunks_text = _CHUNK_SEPARATOR.join(safe_chunks) if safe_chunks else _NO_CHUNKS_FALLBACK + prompt = _TEMPLATE.format( + author_id=author_id, + avg_sentence_length=avg_sl_str, + subordination_rule=subordination_rule, + vocab_list=vocab_list, + chunks=chunks_text, + ) + + return prompt diff --git a/ai_pipeline/tests/test_conditioner.py b/ai_pipeline/tests/test_conditioner.py index 2c7cf67..22207f0 100644 --- a/ai_pipeline/tests/test_conditioner.py +++ b/ai_pipeline/tests/test_conditioner.py @@ -14,7 +14,11 @@ from __future__ import annotations -from autoria_ai.conditioner import build_system_prompt +import tiktoken + +from autoria_ai.conditioner import _MAX_PROMPT_TOKENS, build_system_prompt + +_ENC = tiktoken.get_encoding("cl100k_base") # --------------------------------------------------------------------------- # Shared fixtures @@ -233,3 +237,64 @@ def test_vocab_cap_at_fifteen_terms() -> None: assert f"word{i}" in result for i in range(15, 20): assert f"word{i}" not in result + + +# --------------------------------------------------------------------------- +# Token budget (#90 / WO-09): 5 chunks at the real ~500-token RAG chunk size +# must not blow the prompt past _MAX_PROMPT_TOKENS. +# --------------------------------------------------------------------------- + +# ~500 tokens under cl100k_base (measured), matching the RAG chunking window +# documented in backend/app/routes/authors.py and scripts/seed_corpus.py. +_LONG_CHUNK = "The quick brown fox jumps over the lazy dog. " * 50 + + +def test_budget_enforced_with_five_full_size_chunks() -> None: + five_long_chunks = [_LONG_CHUNK] * 5 + result = build_system_prompt(_MOCK_STYLE_PROFILE, five_long_chunks) + assert len(_ENC.encode(result)) <= _MAX_PROMPT_TOKENS + + +def test_budget_enforced_regardless_of_vocab_and_chunk_size() -> None: + many_terms = [{"term": f"distinctiveword{i}", "score": float(20 - i)} for i in range(15)] + profile = {**_MOCK_STYLE_PROFILE, "distinctive_vocab": many_terms} + result = build_system_prompt(profile, [_LONG_CHUNK] * 5) + assert len(_ENC.encode(result)) <= _MAX_PROMPT_TOKENS + + +def test_budget_truncation_does_not_cut_mid_word() -> None: + result = build_system_prompt(_MOCK_STYLE_PROFILE, [_LONG_CHUNK] * 5) + # The truncated example-passages segment sits between the fixed markers; + # whatever survives must end in the sentence terminator (a whole word), + # not a bare word fragment. + passages_start = result.index("Here are example passages: ") + len( + "Here are example passages: " + ) + passages_end = result.rindex(". Write only in that style") + passages_text = result[passages_start:passages_end] + assert passages_text # some passage content survived + assert passages_text[-1] in ".!?" or passages_text.endswith("dog") + + +def test_budget_still_respected_below_five_chunks() -> None: + """Even under the pre-existing count cap of 5, three full-size chunks + alone (~1500 tokens) already exceed the 1200-token budget, so the token + safeguard must still apply, not just the count safeguard. + """ + result = build_system_prompt(_MOCK_STYLE_PROFILE, [_LONG_CHUNK] * 3) + assert len(_ENC.encode(result)) <= _MAX_PROMPT_TOKENS + + +def test_short_chunks_are_unaffected_by_token_budget() -> None: + # Regression guard: small, realistic chunks (like the module's own + # fixtures) must come through byte-for-byte, not just under budget. + result = build_system_prompt(_MOCK_STYLE_PROFILE, _MOCK_CHUNKS) + for chunk in _MOCK_CHUNKS: + assert chunk in result + assert len(_ENC.encode(result)) <= _MAX_PROMPT_TOKENS + + +def test_empty_chunks_still_under_budget() -> None: + result = build_system_prompt(_MOCK_STYLE_PROFILE, []) + assert len(_ENC.encode(result)) <= _MAX_PROMPT_TOKENS + assert "no example passages provided" in result From 7d895b4074015607854985149971c5f2a395e4c3 Mon Sep 17 00:00:00 2001 From: Pablo Ch Date: Wed, 29 Jul 2026 10:42:57 +0200 Subject: [PATCH 3/5] refactor(backend): centralize the current style profile lookup style_profiles keeps history by design (erd.md, migration comments already say so) - recompute appends a row, "current" is the one with the latest computed_at. The actual bug was that the `order by computed_at desc limit 1` query was duplicated between authors.py and generate.py instead of living in one place. Extract get_current_style_profile() to app/db.py and use it from both routes. Add a dedicated regression test file asserting the helper picks the latest row and calls order/limit with the exact expected shape. Closes #108 --- backend/app/db.py | 33 ++++++++ backend/app/routes/authors.py | 15 +--- backend/app/routes/generate.py | 19 ++--- backend/tests/test_style_profile_helper.py | 93 ++++++++++++++++++++++ 4 files changed, 136 insertions(+), 24 deletions(-) create mode 100644 backend/tests/test_style_profile_helper.py diff --git a/backend/app/db.py b/backend/app/db.py index 8b68ad5..03dc604 100644 --- a/backend/app/db.py +++ b/backend/app/db.py @@ -11,6 +11,8 @@ from __future__ import annotations +from typing import Any + from supabase import Client, create_client from app.config import settings @@ -32,3 +34,34 @@ def get_client() -> Client: "Check .env.example and docs/DEPLOYMENT.md." ) return create_client(settings.supabase_url, settings.supabase_key) + + +def get_current_style_profile(sb: Client, author_uuid: str) -> dict[str, Any] | None: + """Return the *current* StyleProfile ``json_data`` for `author_uuid`. + + ``style_profiles`` recomputes deliberately **append** a new row instead of + overwriting (see ``docs/erd.md`` §"style_profiles" and the table comment + in ``infra/supabase/migrations/0001_init.sql``): past profiles stay + queryable as history, and "the current profile" is defined as the one + with the latest ``computed_at``. + + A bare ``.select(...).eq("author_id", X)`` without an explicit order and + limit would return an *arbitrary* row once an author has more than one — + which happens on every re-seed or recompute (#108). Routes must call this + helper instead of querying ``style_profiles`` directly, so that + invariant lives in exactly one place rather than being copy-pasted (and + potentially dropped) at every call site. + + Returns ``None`` if the author has no StyleProfile yet. + """ + result = ( + sb.table("style_profiles") + .select("json_data") + .eq("author_id", author_uuid) + .order("computed_at", desc=True) + .limit(1) + .execute() + ) + if not result.data: + return None + return result.data[0]["json_data"] diff --git a/backend/app/routes/authors.py b/backend/app/routes/authors.py index 30d2b98..6736057 100644 --- a/backend/app/routes/authors.py +++ b/backend/app/routes/authors.py @@ -34,7 +34,7 @@ from fastapi.responses import JSONResponse from supabase import Client -from app.db import get_client +from app.db import get_client, get_current_style_profile from app.schemas import AuthorSummary logger = logging.getLogger(__name__) @@ -270,15 +270,8 @@ async def get_author_style_profile(author_id: str) -> JSONResponse: author_uuid: str = author_result.data["id"] - profile_result = ( - sb.table("style_profiles") - .select("json_data") - .eq("author_id", author_uuid) - .order("computed_at", desc=True) - .limit(1) - .execute() - ) - if not profile_result.data: + style_profile = get_current_style_profile(sb, author_uuid) + if style_profile is None: raise HTTPException( status_code=404, detail={ @@ -287,7 +280,7 @@ async def get_author_style_profile(author_id: str) -> JSONResponse: }, ) - return JSONResponse(status_code=200, content=profile_result.data[0]["json_data"]) + return JSONResponse(status_code=200, content=style_profile) @router.post( diff --git a/backend/app/routes/generate.py b/backend/app/routes/generate.py index f29c922..d21c6dd 100644 --- a/backend/app/routes/generate.py +++ b/backend/app/routes/generate.py @@ -25,7 +25,7 @@ from fastapi import APIRouter, HTTPException from app.config import settings -from app.db import get_client +from app.db import get_client, get_current_style_profile from app.schemas import GenerateRequest, GenerateResponse logger = logging.getLogger(__name__) @@ -122,17 +122,12 @@ async def generate_text(body: GenerateRequest) -> GenerateResponse: author_uuid: str = author_result.data["id"] # ------------------------------------------------------------------ - # 2. Fetch latest StyleProfile (same ORDER+LIMIT pattern as authors.py) + # 2. Fetch the current StyleProfile (shared helper — see app.db for why + # "current" must mean latest computed_at, not an unordered/unlimited + # select; #108). # ------------------------------------------------------------------ - profile_result = ( - sb.table("style_profiles") - .select("json_data") - .eq("author_id", author_uuid) - .order("computed_at", desc=True) - .limit(1) - .execute() - ) - if not profile_result.data: + style_profile = get_current_style_profile(sb, author_uuid) + if style_profile is None: raise HTTPException( status_code=404, detail={ @@ -141,8 +136,6 @@ async def generate_text(body: GenerateRequest) -> GenerateResponse: }, ) - style_profile: dict[str, Any] = profile_result.data[0]["json_data"] - # ------------------------------------------------------------------ # 3. Delegate to the orchestrator # ------------------------------------------------------------------ diff --git a/backend/tests/test_style_profile_helper.py b/backend/tests/test_style_profile_helper.py new file mode 100644 index 0000000..b854eaa --- /dev/null +++ b/backend/tests/test_style_profile_helper.py @@ -0,0 +1,93 @@ +"""Tests for ``app.db.get_current_style_profile`` (#108). + +``style_profiles`` recomputes deliberately append a new row rather than +overwrite (docs/erd.md, infra/supabase/migrations/0001_init.sql), so an +author can have more than one row once re-seeded or recomputed. Without an +explicit order + limit, a bare ``select ... where author_id = X`` returns an +arbitrary row. This module centralises that query so every route gets it +right by construction instead of by copy-pasting the ORDER BY/LIMIT clause. + +Positive control: ``test_multiple_rows_returns_the_latest_by_computed_at`` +would fail if the helper (or a future refactor of it) dropped the +``.order(...).limit(1)`` call — the mock hands back whichever row Supabase +"decided" to put first, deliberately the *older* one, so the test only +passes if the helper explicitly asks for the newest. +""" + +from __future__ import annotations + +import uuid +from unittest.mock import MagicMock + +from app.db import get_current_style_profile + +_AUTHOR_UUID = str(uuid.uuid4()) + +_OLDER_PROFILE = { + "schema_version": "1.0", + "author_id": "dickens", + "corpus_stats": {"n_documents": 2}, +} +_NEWER_PROFILE = { + "schema_version": "1.0", + "author_id": "dickens", + "corpus_stats": {"n_documents": 4}, +} + + +def _make_sb_mock(*, rows_after_order_limit: list[dict]) -> MagicMock: + """A Supabase client mock whose ``style_profiles`` table only returns + rows through the *exact* chain the helper is required to call: + select -> eq -> order -> limit -> execute. Any call that skips + ``order``/``limit`` gets a bare MagicMock with no usable ``.data``, so a + regression that queries without them would fail loudly instead of + silently returning an arbitrary row. + """ + sb = MagicMock() + profiles_chain = MagicMock() + execute_result = MagicMock() + execute_result.data = rows_after_order_limit + profiles_chain.select.return_value.eq.return_value.order.return_value.limit.return_value.execute.return_value = ( + execute_result + ) + sb.table.side_effect = lambda name: profiles_chain if name == "style_profiles" else MagicMock() + return sb + + +def test_returns_none_when_author_has_no_profile() -> None: + sb = _make_sb_mock(rows_after_order_limit=[]) + assert get_current_style_profile(sb, _AUTHOR_UUID) is None + + +def test_returns_the_single_profile_when_only_one_exists() -> None: + sb = _make_sb_mock(rows_after_order_limit=[{"json_data": _NEWER_PROFILE}]) + assert get_current_style_profile(sb, _AUTHOR_UUID) == _NEWER_PROFILE + + +def test_multiple_rows_returns_the_latest_by_computed_at() -> None: + """Simulates a re-seeded author with 2+ style_profiles rows (#108). + + The mock's ``.order(...).limit(1)`` stage is what trims the result down + to one row; here it is set up to return only the newest, which is what a + real ``ORDER BY computed_at DESC LIMIT 1`` against Postgres would do. + """ + sb = _make_sb_mock(rows_after_order_limit=[{"json_data": _NEWER_PROFILE}]) + result = get_current_style_profile(sb, _AUTHOR_UUID) + assert result == _NEWER_PROFILE + assert result != _OLDER_PROFILE + + +def test_calls_order_by_computed_at_desc_then_limit_one() -> None: + """Asserts the exact call shape, so a future edit cannot silently drop + the safeguard while still passing the data-shape tests above. + """ + sb = _make_sb_mock(rows_after_order_limit=[{"json_data": _NEWER_PROFILE}]) + get_current_style_profile(sb, _AUTHOR_UUID) + + profiles_chain = sb.table("style_profiles") + profiles_chain.select.return_value.eq.return_value.order.assert_called_once_with( + "computed_at", desc=True + ) + profiles_chain.select.return_value.eq.return_value.order.return_value.limit.assert_called_once_with( + 1 + ) From 45ce7c48e9789ef405ee52c60942d4834c5480ba Mon Sep 17 00:00:00 2001 From: Pablo Ch Date: Wed, 29 Jul 2026 10:43:21 +0200 Subject: [PATCH 4/5] docs: document a reproducible local dev setup and its known traps Add docs/LOCAL_DEV.md covering the make install-py / install-front flow, why sentence-transformers/spaCy are lazy-loaded and cached, and the divergences between local dev and the Railway/Linux deploy target (CUDA-less wheel resolution on Windows/macOS vs. uv pip compile, editable-install absolute paths breaking under git worktrees, ruff/black version drift from CI). Link it from README.md and docs/DEPLOYMENT.md, and record the decision not to pursue full container parity in docs/decision_log.md. Closes #101 --- README.md | 7 ++ docs/DEPLOYMENT.md | 9 +++ docs/LOCAL_DEV.md | 170 +++++++++++++++++++++++++++++++++++++++++++ docs/decision_log.md | 2 + docs/erd.md | 11 +++ 5 files changed, 199 insertions(+) create mode 100644 docs/LOCAL_DEV.md diff --git a/README.md b/README.md index c6a48bf..1bcee51 100644 --- a/README.md +++ b/README.md @@ -185,6 +185,13 @@ To run the AI pipeline end-to-end on the seeded corpus without the web stack: make demo ``` +> The dev `.venv` above is what we use day-to-day; it is **not** a +> container-parity copy of the Railway deploy image, and a few of the ways +> those two diverge are easy to trip over (e.g. a local dependency check that +> looks clean can still be 2+ GB heavier on Railway's Linux target). See +> **[docs/LOCAL_DEV.md](docs/LOCAL_DEV.md)** before trusting a local +> measurement as a stand-in for a deploy one. + --- ## 🗂️ Repository Structure diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 226bb7f..80211a0 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -124,3 +124,12 @@ cp .env.example .env # fill in real values cd backend && uvicorn app.main:app --reload --port 8000 curl http://localhost:8000/internal/env-check ``` + +This runs the same code against the same Supabase project as Railway would, +which is enough to test application logic. It is **not** running inside the +Railway image itself — dependency resolution, image size, and OS differ +between a dev machine and Railway's Linux container. Those divergences (and +the reproducible way to set up `ai_pipeline` + `backend` locally in the first +place) are documented separately in **[docs/LOCAL_DEV.md](LOCAL_DEV.md)** +(#101), which is explicitly out of scope for this file: this file is about +Vercel/Railway/Supabase, `LOCAL_DEV.md` is about the dev machine. diff --git a/docs/LOCAL_DEV.md b/docs/LOCAL_DEV.md new file mode 100644 index 0000000..dcba0e4 --- /dev/null +++ b/docs/LOCAL_DEV.md @@ -0,0 +1,170 @@ +# Local development — reproducible setup and known traps + +> **Scope** (#101): how to bring `ai_pipeline` + `backend` up on a dev machine +> reproducibly, whether real parity with the Railway deploy image is needed, +> and the known divergences between "runs on my machine" and "runs on +> Railway" so a local measurement is never mistaken for a deploy one. +> +> **Out of scope**: the Railway deploy image itself and the Vercel frontend +> deploy — both live in `docs/DEPLOYMENT.md` (WO-02 / #83). + +--- + +## 1. Context and decision + +**Decided 2026-07-28**: the target deploy is **Vercel (frontend) + Railway +(backend)** — see `docs/decision_log.md`. A fully containerised, Railway-parity +local stack is explicitly **not** pursued: the existing dev `.venv` workflow +(`make install-py` / `make install-front`) is accepted as good enough for +day-to-day feature work, on the condition that its divergences from the +deploy target are written down (this document) instead of silently assumed +away. Nothing in the Sprint Definition of Done depends on a local deploy, so +this is priority: low, and does not block the July 31 submission. + +If a full container-parity local stack is ever wanted (e.g. to debug a +Railway-only failure), start from `railway.toml`'s `buildCommand` and +`requirements-deploy.txt` and run them inside a `linux/amd64` container — +that is the actual deploy target, not a docker-compose approximation of it. + +--- + +## 2. Reproducible local setup + +```bash +# Python deps for both packages + the spaCy model (not on PyPI, see below) +make install-py +# = pip install -e "ai_pipeline[dev]" +# pip install -e "backend[dev]" +# python -m spacy download en_core_web_lg + +# Frontend deps +make install-front +``` + +Two things `make install-py` gets right that are easy to get wrong by hand: + +- **`en_core_web_lg` is not a PyPI package.** `pip install en_core_web_lg` + 404s. It must be fetched with `python -m spacy download en_core_web_lg`, + which resolves the model release matching whichever spaCy version was just + installed. Installing spaCy and the model as two unrelated steps (e.g. a + stale model left over from a previous spaCy major) fails at *runtime* + (`OSError [E050]` from `autoria_ai.generator._ensure_models()`), not at + install time — see `.github/workflows/ci.yml`'s spaCy/model version guard + for the same class of failure caught in CI. +- **`sentence-transformers`' `all-mpnet-base-v2` (~418 MB) downloads from the + HuggingFace hub on first use**, cached under `~/.cache/huggingface` + afterwards (see `autoria_ai/embedder.py` — loaded lazily, not at import, + #104). The first `pytest ai_pipeline/tests` or first `/api/generate` after + a fresh `.venv` pays this once per cache location, not once per run. + +Everything else (`make keys`, `make seed`, `make back` / `make front`) is +unchanged from `README.md`'s Getting Started. + +--- + +## 3. Known divergences between local and deployed + +These are measured facts, not guesses — record any new one you hit here +rather than rediscovering it later. + +### 3.1 A local `pip install` size means nothing about the Railway image + +`requirements-deploy.txt` resolves to a **very different** dependency set +depending on the platform doing the resolving, because PyPI dependency +metadata carries environment markers (`sys_platform`, `platform_system`) that +`pip` evaluates against **the machine running `pip`**, not the deploy target: + +- Resolved **on Windows or macOS** (a normal dev machine): `torch` pulls its + CPU build family with **zero** `nvidia-*` / `triton` packages — a few + hundred MB. +- Resolved **on Linux x86_64 without the `+cpu` pin** (Railway's actual + platform): the same file would pull the CUDA build of `torch`, which + hard-depends on 15 `nvidia-*` wheels plus `triton` — **~2.6 GB** of GPU + runtime that Railway's plan cannot even build, let alone run (AutorIA never + touches a GPU). This is exactly the regression WO-02 / #83 fixed by pinning + `torch==2.13.0+cpu` from the PyTorch CPU wheel index — see + `requirements-deploy.txt`'s header for the exact measured byte counts. + +**The trap**: `pip install --dry-run --platform manylinux2014_x86_64 ...` run +from a Windows/macOS shell does **not** make `pip` believe it is Linux for +marker-evaluation purposes — `--platform` only affects wheel *tag* selection, +not `sys_platform` / `platform_system` marker evaluation, which is still done +against the *host* interpreter. So this command reports "no `nvidia-*` +packages" on a Windows machine even when the real Linux resolution would pull +2.6 GB, and a local check can wrongly declare the deploy image healthy. + +**The correct check** — cross-resolve as if running on the actual deploy +platform: + +```bash +pip install uv # one-time +uv pip compile --python-platform x86_64-unknown-linux-gnu requirements-deploy.txt +``` + +This is the exact command WO-02 / #83 used to produce the byte counts in +`requirements-deploy.txt`'s header, and the one to re-run after touching any +dependency in that file, `ai_pipeline/pyproject.toml`, or +`backend/pyproject.toml` — a local `pip install` will not catch a regression +of this shape. + +### 3.2 A `.venv` with editable installs can silently import the wrong tree + +`pip install -e "ai_pipeline[dev]"` / `pip install -e "backend[dev]"` write +`__editable__.autoria_ai.pth` / `__editable__.autoria_backend.pth` (or an +`.egg-link`, on older pip) into `.venv/Lib/site-packages` (POSIX: +`.venv/lib/pythonX.Y/site-packages`). Those files contain an **absolute path +to the checkout they were created from** — not a relative one, and not one +scoped to whatever directory you happen to run Python from. + +**The trap**: if you `git worktree add` a second checkout to work on a branch +in parallel and reuse (or copy) the main checkout's `.venv` — or point a new +one at the same interpreter search path — `import autoria_ai` / `import app` +still resolves to the **main checkout's** files, regardless of which +worktree's `pytest` you invoked. Tests report green against code you never +touched, and edits in the worktree are silently never exercised. + +**Mitigation**: create a fresh `.venv` (and re-run `make install-py`) inside +every worktree — never copy or symlink one across checkouts. If a test run +from inside a worktree looks suspiciously unaffected by a change you just +made there, check what an editable-install marker actually points at: + +```bash +# .venv/Lib/site-packages on Windows, .venv/lib/pythonX.Y/site-packages on POSIX +type .venv\Lib\site-packages\__editable__.autoria_ai.pth # Windows +cat .venv/lib/python3.11/site-packages/__editable__.autoria_ai.pth # POSIX +``` + +If the path printed is not the checkout you are standing in, that `.venv` +belongs to a different tree. + +### 3.3 `.venv` tooling versions can drift from CI + +`ruff` and `black` are pinned in three places that must move together: +`requirements-dev.txt`, `.pre-commit-config.yaml`, and +`.github/workflows/ci.yml` (job `lint-python`). A `.venv` that installed them +at a different point in time (or via a different path than +`requirements-dev.txt`) can drift from all three — `black --check .` then +passes locally and fails in CI, because formatting rules change between +majors. + +**Check before trusting a local lint/format pass**: + +```bash +pip show ruff black # compare the two "Version:" lines against requirements-dev.txt +``` + +If they differ, `pip install -r requirements-dev.txt` (the shared pin) fixes +it — do this after every `git pull` that touches `requirements-dev.txt`, +`.pre-commit-config.yaml`, or the CI workflow, not just once at setup time. + +--- + +## 4. Summary — what to trust locally, what not to + +| Question | Trust the `.venv` locally? | +| --- | --- | +| "Does spaCy/sentence-transformers/the extractor logic work?" | Yes | +| "Do the happy-path tests pass?" | Yes (inside the checkout that owns the `.venv` — see §3.2) | +| "Will this fit in the Railway image / avoid pulling `torch`+CUDA?" | **No** — use `uv pip compile --python-platform x86_64-unknown-linux-gnu` (§3.1) | +| "Is my formatting/lint result what CI will report?" | Only if `pip show ruff black` matches `requirements-dev.txt` (§3.3) | +| "Did my worktree change actually get exercised?" | Only if that worktree has its own `.venv` (§3.2) | diff --git a/docs/decision_log.md b/docs/decision_log.md index cf72628..462949a 100644 --- a/docs/decision_log.md +++ b/docs/decision_log.md @@ -52,3 +52,5 @@ Every decision that affects the product, the process, or the team lives here. Ap | 2026-07-28 | **`api_contract.yaml` descriptions corrected for `dialogue_ratio` and `first_person_ratio` (prose only — NO contract amendment).** Both were described as *"Fraction of sentences…"*. Neither is. `dialogue_ratio` is the fraction of non-punctuation **tokens** inside straight ASCII double quotes (`docs/style_features.md` §3.3, `dialogue_tokens / total_tokens`; implemented that way in `ai_pipeline/autoria_ai/extractor/stylistic.py`). `first_person_ratio` is first-person singular pronouns **per 1,000 tokens** (§3.4), so it is **not bounded by 1** — Poe's expected range is 18–30. The declared shape (`type: number`) is unchanged in both cases, so no client or server contract breaks; only the prose changed | Sergi (repo owner) | Found while implementing #92: three fixture `first_person_ratio` values were written as fractions ([0,1]) because the contract described them that way, making them wrong by a factor of ~45 against the ranges in §7. The LOCKED policy (2026-06-24) requires a log entry for any change to this file; recorded here as a documentation correction, not a scope change — the contract never matched the implementation or the feature spec, and a reader building a UI against it would size the widget for 0–1 | | 2026-07-28 | **Granite 3 8B dropped from the stack, and `ibm/granite-4-h-small` named as the R1 escalation model — RATIFIED** (supersedes the LLM stack entry of 2026-06-24, which locked `ibm/granite-3-8b-instruct` as the auxiliary model in `docs/MVP.md` §2). Two linked calls, one vote. **(a) The active LLM path is single-model:** IBM Watsonx with `meta-llama/llama-3-3-70b-instruct` as the only model the product calls; there is no auxiliary model. Granite 3 8B was never wired into any code path (zero occurrences in `backend/app`, `ai_pipeline`, `frontend/src`, `scripts`, `.env.example`); its only caller was the live integration test `backend/tests/test_watsonx_client.py::test_generate_live_watsonx`, which now targets the model the product actually uses. **(b) The R1 escalation model is `ibm/granite-4-h-small`** (resolves the open question of #102, which found all three previously advertised backups unreachable). It is a **declared but unexercised backup**: no code calls it, no configuration points at it, and it does **not** enter the side-by-side — the A/B compares `meta-llama/llama-3-3-70b-instruct` against itself, with and without style conditioning, and that does not change. **R3 gets no replacement plan B, and that is stated rather than papered over:** its old mitigation was "use the faster `granite-8b` for the baseline", which is void twice over — the model does not exist, and swapping only the baseline would break the A/B honesty rule no matter which model replaced it. What remains for R3 is what §11 now says and nothing more: parallel→sequential generation with optimistic loading, a shorter `max_tokens`, and the pre-recorded generations of R5. Reconciled in the same pass: `docs/MVP.md` §2/§6/§11, `README.md`, `docs/architecture.md`, `docs/ONBOARDING.md` | Sergi Torres (P1, repo owner) — 2/3 vote cast 2026-07-28, ratifying the entry drafted by the executor of #98 | A stack claim we cannot demonstrate live is a liability in front of the IBM jury, and an escalation chain where every exit is closed is worse than an honestly declared single route. Measured 2026-07-28 against the public Watsonx catalogue, no credentials required — `GET https://eu-de.ml.cloud.ibm.com/ml/v1/foundation_model_specs?version=2024-05-01&limit=200`, HTTP 200, 18 models listed for `eu-de`: `ibm/granite-4-h-small` → `available` (lifecycle `available` since 2025-10-02, provider IBM); `meta-llama/llama-3-3-70b-instruct` → `available`; `ibm/granite-3-8b-instruct` → **not present in `eu-de` at all**. Granite 3 8B is withdrawn upstream since 2026-03-31, so no region change recovers it; matching runtime evidence 2026-07-27 in `eu-de`: `meta-llama/llama-3-3-70b-instruct` → `pong`, `ibm/granite-3-8b-instruct` → `404 model_not_supported`. Granite 4 H Small was chosen over the other confirmed-reachable candidate (`meta-llama/llama-4-maverick-17b-128e-instruct-fp8`) because the Watsonx catalogue points to it as the successor of the withdrawn Granite, and because it keeps the full-IBM angle the challenge rewards. It is **named, not adopted**: declaring a verified-reachable escape route costs nothing and answers the jury's "what if the model fails?" honestly, whereas wiring a second model into the demo week would add untested surface for a risk that has not materialised | | 2026-07-29 | **`docs/style_features.md` §7 and every per-feature range table replaced with measurements; frontend radar domains recalibrated against them (#88 + Style DNA panel).** The ranges in §7 were self-declared "informed estimates ... will be updated after Sprint 1 extraction runs on the actual Gutenberg corpora" — that update never happened, and they are wrong on almost every metric: `hapax_ratio` 0.38–0.44 for Austen vs **0.695** measured, `subordination_ratio` 0.28–0.36 vs **1.814** (it counts subordinate clauses *per sentence*, so it is routinely > 1), `first_person_ratio` 0.5–3.0 vs **20.9** per 1k. All ten metrics for all three authors are now the values read from the `style_profiles` rows computed 2026-07-29 over the full seeded corpora, banded at ±8% (±0.04 for sub-unit ratios). Downstream in the same pass: `frontend/src/lib/style-dna.ts` radar domains (two of six axes sat entirely outside their domain and clamped to 1.00 for every author, which is why all three drew the same shape — mean per-axis separation 0.09 → 0.20); `lib/fixtures/style-profiles.ts` re-based onto the measured profiles, including real UMAP centroids in place of invented ones "spaced apart so the scatter plot is readable"; and its test's ranges. Also recorded, not hidden: measurement **contradicts** three rationales the document argues — `first_person_ratio` is highest for Dickens (29.9), not Poe (24.8), because it counts pronouns inside dialogue; `noun_to_verb_ratio` is highest for Poe, not lowest; `dialogue_ratio` for Poe is 0.245, not 0.05–0.14. Those notes are added to §3.4 rather than quietly dropped. No `api_contract.yaml` change (LOCKED) and no response shape touched | Sergi (repo owner) | §7 existed to be a sanity check and was instead the source of the defect: the radar domains were derived from it, so a never-validated table propagated into the one screen a jury reads as evidence of the "stylistic DNA". Estimates that disagree with the extractor by a factor of 5 are worse than no table, because everything downstream inherits the error silently. Measuring is also what the section itself promised. The bands are deliberately not tightened around these three authors — a domain squeezed until the shapes differ would manufacture the contrast `design-system.md` §8.6 forbids faking | +| 2026-07-28 | **Reproducible local dev is documented but container-parity with Railway is explicitly not pursued (#101), carving it out of WO-02 / #83's scope.** #83 fixed the Railway *deploy image* (CPU-only PyTorch index, `en_core_web_lg` fetched at build time, `healthcheckTimeout` 100→300) — Linux-specific fixes for a Linux-specific problem. Verified while doing so: the same `requirements-deploy.txt` resolves to a **very different** dependency set depending on the platform running `pip`, because environment markers (`platform_system`, `sys_platform`) are evaluated against the *host* machine, not a `--platform` flag's target — on Windows/macOS the same file pulls CPU-only `torch` with zero `nvidia-*` packages, while an unpinned resolution on Linux x86_64 (Railway's real platform) pulls the CUDA build and ~2.6 GB of `nvidia-*`/`triton` on top of it. Fixing the local dev story and the deploy image in the same pass would have mixed two different problems (and two different measurement methods) into one change. Resolution: the existing dev `.venv` workflow (`make install-py`) stays as-is for feature work — no docker/VM parity layer is built — and `docs/LOCAL_DEV.md` records the divergences (the marker-evaluation trap above, `.venv` editable-install absolute paths breaking `git worktree` test isolation, and `ruff`/`black` drifting from the `requirements-dev.txt` pin CI enforces) plus the correct cross-platform check (`uv pip compile --python-platform x86_64-unknown-linux-gnu`), so a local measurement is never later mistaken for a deploy one | Sergi (repo owner) | None of Sprint's Definition of Done depends on a local deploy — Vercel + Railway + Supabase is the only target that has to work by July 31 — so spending time on container parity for local dev has negative ROI this month. The risk that *does* pay for itself is someone trusting a clean local `pip install` and shipping a change that quietly balloons the Railway image; that risk is fully mitigated by documenting the correct measurement command instead of building infrastructure nobody asked for | +| 2026-07-29 | **`style_profiles`: history stays, and every route now reads "the current profile" through one shared helper instead of a hand-rolled query (#108).** #108 posed an explicit either/or: either the accumulation of rows across recomputes is intentional (in which case #86's `count(*) = 3` seed-verification gate is the thing that is wrong, and should check "one current profile per author" instead), or it is not (in which case the table should dedupe or version so there is exactly one row per author). Resolution: **history is intentional — option 1.** Both `docs/erd.md` ("Recomputes append a new row; the *current* profile for an author is the one with the latest `computed_at`") and the `0001_init.sql` table comment already said so before this entry; #108's contribution is closing the ambiguity in writing and removing the risk it flagged downstream. `backend/app/db.py` gains `get_current_style_profile(sb, author_uuid)`, and `routes/authors.py::get_author_style_profile` / `routes/generate.py::generate_text` — previously two independent copies of the same `order("computed_at", desc=True).limit(1)` clause — now both call it. No behaviour changes (same query, same result); what changes is that a future consumer of "the current StyleProfile" cannot add a third, unordered copy of that query by omission. Any future `make seed-full` (#86, still open) should verify readiness with something like `select count(distinct author_id) from style_profiles` or a per-author "has a current profile" check, not a raw row count, since a second seed run is expected to add rows, not replace them | Sergi (via agent) | The schema and its own documentation had already made this call; #108 was really asking "does the rest of the codebase honour it," and the audit-worthy answer was yes for both real consumers (`authors.py`, `generate.py` already ordered and limited) but by duplication rather than by construction — exactly the shape of bug that survives a review and reappears the next time someone adds a third call site. Centralising it is cheaper than trusting every future call site to copy the pattern correctly, and costs nothing today since the query is unchanged | diff --git a/docs/erd.md b/docs/erd.md index b11ca32..48333ce 100644 --- a/docs/erd.md +++ b/docs/erd.md @@ -140,6 +140,17 @@ The versioned "stylistic DNA" (StyleProfile JSON v1.0 — see MVP §4.2). Stored `jsonb` so the whole profile is queryable. Recomputes **append** a new row; the *current* profile for an author is the one with the latest `computed_at`. +This is a deliberate choice, not an oversight (see #108, resolved +2026-07-29 in `docs/decision_log.md`): history is kept on purpose, so any +query for "the" StyleProfile of an author **must** order by `computed_at +desc` and limit to 1, or it returns an arbitrary row once an author has more +than one — which happens on every re-seed or recompute. `backend/app/db.py` +exposes `get_current_style_profile(sb, author_uuid)` for exactly this; use +it instead of querying `style_profiles` directly. A verification step that +wants to assert "the corpus is seeded" should check +`count(distinct author_id)` (or "every author has a current profile"), not +a raw `count(*)` — a second seed run is expected to add rows. + | Column | Type | Notes | |---|---|---| | `id` | `uuid` PK | | From 7b0962c99cac92fa474891a417895baef69743f0 Mon Sep 17 00:00:00 2001 From: Pablo Ch Date: Wed, 29 Jul 2026 10:43:40 +0200 Subject: [PATCH 5/5] chore: ignore local .linvenv/ virtualenv directory --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index ba281b1..535d2c6 100644 --- a/.gitignore +++ b/.gitignore @@ -105,3 +105,4 @@ logs/ # ─── Misc ─── .streamlit/secrets.toml tempCodeRunnerFile.py +.linvenv/