diff --git a/README.md b/README.md index 82abd64..a3b93c2 100644 --- a/README.md +++ b/README.md @@ -36,10 +36,10 @@ PersonalityProtect keeps the corpus on disk, measures your cadence, retrieves sh 4. **`build-style-profile`** measures cadence from the selection (sentence length, short lines, post length band, banned filler). 5. **`write --topic --points`** drafts from the brief only; retrieved pieces are rhythm reference. -Two channels come out of step 5: +Two channels come out of step 5, and each takes its length from its own pieces: - **`--channel post`** (default) targets your long-post band, up to the LinkedIn ~3000-character limit (~550 words). -- **`--channel article`** runs outline → sections → stitch, and needs at least five `linkedin_article` pieces in the corpus. +- **`--channel article`** runs outline → sections → stitch. Total length comes from the p50/p75/p90 of your `linkedin_article` pieces, split across the outline; the post ceiling never applies. Needs at least five `linkedin_article` pieces both in the corpus and in the voice index. Local LoRA training stays in the CLI as an experiment, not as the path to a first draft — see [Advanced](#advanced-optional). @@ -181,7 +181,14 @@ personality-protect build-style-profile Defaults: **≥50 words**, dates through the **current year**. Use `--through-year` when you intentionally want an older slice. Corpus gates: **warn** below 50 selected pieces; **block** below 20 unless `--force`. Holding pieces back from retrieval is separate — `index-voice --holdout-id`, scored by `eval-write-holdout`. -Post length targets come from `linkedin_post` pieces (p75/p90), clamped to the LinkedIn ~3000-character band (~550 words). +Length targets are per channel and never borrow across channels: + +| Channel | Measured from | Aim | Ceiling | +| --- | --- | --- | --- | +| post | `linkedin_post` pieces | p75 (floor 300) | p90, clamped to ~550 words (~3000 chars) | +| article | `linkedin_article` pieces | median, clamped to 600–3000 words | p90, clamped to 3000 words | + +The article aim is divided across the outline to get a per-section budget (clamped to 180–600 words), so a five-section article asks for five short sections rather than five posts. With no `linkedin_article` pieces in the corpus, the article aim falls back to a stated default of 1100 words instead of borrowing the post band. ### Write @@ -191,7 +198,26 @@ personality-protect write --channel article --topic "…" --points "…" personality-protect write --topic "…" --points "…" --json ``` -`--topic` and `--points` are the only content the draft may use; retrieved pieces supply rhythm, not facts. Every `write` above runs base weights (`adapter=none`). Article channel requires at least five `linkedin_article` pieces in the corpus. +`--topic` and `--points` are the only content the draft may use; retrieved pieces supply rhythm, not facts. Every `write` above runs base weights (`adapter=none`). + +On `--channel article`, each `--points` bullet becomes a section (2–8), retrieval is restricted to `linkedin_article` pieces so posts cannot become the rhythm reference, and sections that restate each other are dropped before stitching. The channel refuses to draft unless at least five `linkedin_article` pieces are in the corpus *and* five are in the voice index — a large carve that leaves retrieval empty is an error, not a silently thinner draft. + +### Article holdout eval + +The post channel is scored by `eval-write-holdout`. The article channel has its own carve and eval: + +```bash +personality-protect select-article-holdouts # report only +personality-protect select-article-holdouts --apply +personality-protect index-voice --from-carve # holdouts leave retrieval +PP_MLX_ALLOW=1 personality-protect eval-write-article --out receipt.json +``` + +The carve is deterministic (`blake2b(piece_id)` order), keeps previously carved ids pinned, and never drops the voice index below the five-article floor. Each holdout is reduced to a lossy brief — a topic plus 3–6 section bullets drawn one per segment of the piece, capped at 60 words and 10% of the source — so neither arm is handed the article back to paraphrase. + +Two arms then write the same brief with the same outline, per-section budget, and trim. The product arm gets retrieved exemplars and the measured style card; the control arm gets neither. Drafts are scored on distance to the holdout's own cadence axes, and a draft that parrots its exemplars, echoes the brief, or invents entities or figures is disqualified regardless of distance. Receipts carry ids, distances, and flags — never draft or corpus text. + +The verdict needs all three of: the article arm wins the majority, the margin clears `--alpha` (default 0.10) on a one-sided sign test, and it is not disqualified more often than the control. When both arms are disqualified on every holdout, distance never decided anything, and the receipt says so (`distance_ever_decided: false`) rather than reporting it as a cadence loss. ### Status / API @@ -215,7 +241,9 @@ Global flags (most commands): `--profile`, `--home`, `--json`, plus branding `-- | `index-voice` | Build local voice retrieval index | | `build-style-profile` | Build cadence / length / banned-filler style card | | `write` | Draft a post or article (`--channel post\|article`) | -| `eval-write-holdout` | Score write quality on held-out pieces (local receipt) | +| `eval-write-holdout` | Score post-channel writes on held-out pieces (local receipt) | +| `select-article-holdouts` | Deterministic article carve that respects the retrieval floor | +| `eval-write-article` | Score article-channel writes against a no-voice control (local receipt) | | `status` | Show profile state | | `demo` | Optional synthetic smoke tour of the write path (no download) | | `api` | Loopback HTTP stub | @@ -241,6 +269,25 @@ Global flags (most commands): `--profile`, `--home`, `--json`, plus branding `-- | `--save-raw` | Local prompts/drafts under the profile (never commit) | | `--out PATH` | Contoso-safe aggregate receipt JSON | +### `eval-write-article` flags + +| Flag | Meaning | +| --- | --- | +| `--holdout-id` | Article id to score (repeatable); defaults to the saved carve | +| `--k` | Article exemplars retrieved per section | +| `--alpha` | One-sided significance the run must reach (default 0.10) | +| `--save-raw` | Local prompts/drafts under the profile (never commit) | +| `--out PATH` | Contoso-safe receipt JSON | + +### `select-article-holdouts` flags + +| Flag | Meaning | +| --- | --- | +| `--apply` | Write the carve (default is report-only) | +| `--fraction` | Share of briefable articles to reserve | +| `--min` / `--max` | Carve size band (4–5; three cannot reach `--alpha` on a sign test) | +| `--keep-indexed` | Articles the carve must leave in retrieval (default 5) | + --- ## Advanced (optional) diff --git a/src/personality_protect/article_brief.py b/src/personality_protect/article_brief.py new file mode 100644 index 0000000..19d67f5 --- /dev/null +++ b/src/personality_protect/article_brief.py @@ -0,0 +1,219 @@ +"""Lossy brief mining for article holdouts. + +The post path already answers this question for posts: a brief is what the +author jotted down *before* writing, not an extract of the finished piece, and +:mod:`personality_protect.eval_write_holdout` enforces that with a hard word cap +plus a source-overlap cap. Articles need the same guarantee and cannot reuse the +post miner unchanged, for two reasons: + +* **the overlap cap stops binding.** A 25% cap on a 1,000-word article permits a + 250-word "brief". The cap has to shrink as the source grows, so the article + budget is a small fixed word count that a longer source cannot inflate. +* **an article brief is an outline.** Ranking every sentence by fact density and + taking the top three returns three claims from whichever passage happens to be + the densest. Bullets are therefore drawn one per segment of the piece, in + document order, so the brief describes the shape of an article instead of one + paragraph of it. + +Mining runs against the de-voiced clauses from +:mod:`personality_protect.devoice`, so the bullets carry the author's claims +without the author's phrasing, and the result is measured against the original +article on both overlap and 5-gram copy ratio before it is returned. +""" + +from __future__ import annotations + +from typing import Any + +from personality_protect.corpus_text import normalize_corpus_text +from personality_protect.devoice import ( + MAX_PAIR_COPY_RATIO, + devoice_sentences, + pair_copy_ratio, +) +from personality_protect.eval_write_holdout import ( + _fact_score, + _fit_phrase, + _mine_topic_with_source, + _word_tokens, + brief_word_overlap_ratio, +) + +# Absolute budget for the model-visible brief. Fixed rather than proportional: +# a share of the source grows with the source, and the whole point is that a +# longer article does not earn a longer head start. +ARTICLE_MAX_BRIEF_WORDS = 60 +# Share of the source the brief may return. Binds on short articles, where the +# absolute cap alone would be generous. +ARTICLE_MAX_BRIEF_OVERLAP = 0.10 +ARTICLE_TOPIC_WORD_CAP = 10 +ARTICLE_POINT_WORD_CAP = 12 +ARTICLE_MIN_POINTS = 3 +ARTICLE_MAX_POINTS = 6 +ARTICLE_MIN_POINT_WORDS = 3 +# Share of brief words sitting inside a 5-gram of the article. Same meter and +# same threshold as the writer pair gate: a brief that trips it is an extract +# whichever channel produced it, and a second number here would be a second +# thing to justify. +ARTICLE_MAX_COPY_RATIO = MAX_PAIR_COPY_RATIO +# Below this a piece is a long post, not an article, and the outline segmenting +# has nothing to segment. +MIN_ARTICLE_BRIEF_WORDS = 200 + + +class ArticleBriefRejected(ValueError): + """An article could not be reduced to a brief that is not an extract.""" + + def __init__(self, reasons: list[str], report: dict[str, Any]) -> None: + super().__init__("article brief rejected: " + ", ".join(reasons)) + self.reasons = reasons + self.report = report + + +def _outline_positions(candidate_count: int, points: int) -> list[int]: + """Segment boundaries splitting ``candidate_count`` clauses into ``points``.""" + if candidate_count <= 0 or points <= 0: + return [] + step = candidate_count / points + return [int(index * step) for index in range(points + 1)] + + +def select_outline_clauses( + candidates: list[tuple[int, str]], + *, + points: int, +) -> list[tuple[int, str]]: + """Highest-substance clause from each equal segment, in document order. + + Coverage is the property that matters here. Global ranking is what the post + miner does and it is correct for a post, where every sentence is in the same + passage; on an article it returns a cluster. + """ + wanted = max(1, min(int(points), len(candidates))) + bounds = _outline_positions(len(candidates), wanted) + chosen: list[tuple[int, str]] = [] + for start, end in zip(bounds, bounds[1:]): + segment = candidates[start:max(end, start + 1)] + if not segment: + continue + best = max(segment, key=lambda item: (_fact_score(item[1]), -item[0])) + if best not in chosen: + chosen.append(best) + return sorted(chosen, key=lambda item: item[0]) + + +def _content_word_count(topic: str, points: str) -> int: + """Words the brief hands over, ignoring the bullet markers we added.""" + point_words = [word for word in points.split() if word not in {"-", "*", "•"}] + return len(topic.split()) + len(point_words) + + +def mine_article_brief( + text: str, + *, + holdout_id: str = "", + max_points: int = ARTICLE_MAX_POINTS, + max_overlap: float = ARTICLE_MAX_BRIEF_OVERLAP, + max_copy_ratio: float = ARTICLE_MAX_COPY_RATIO, +) -> tuple[dict[str, str], dict[str, Any]]: + """Mine a topic plus section bullets from an article, and prove it is lossy. + + Returns ``(brief, report)``. ``brief['guard_facts']`` stays the original + article so the invention guard can reject facts the author never wrote + without those facts reaching the generation prompt — the same split the post + path uses. Receipts serialize neither field. + """ + original = normalize_corpus_text(text) + source_words = len(_word_tokens(original)) + if source_words < MIN_ARTICLE_BRIEF_WORDS: + raise ArticleBriefRejected( + ["article_too_short"], {"source_words": source_words} + ) + + clauses = devoice_sentences(original) + if len(clauses) < ARTICLE_MIN_POINTS + 1: + raise ArticleBriefRejected( + ["devoiced_clauses_too_few"], + {"source_words": source_words, "clauses": len(clauses)}, + ) + + budget = min(ARTICLE_MAX_BRIEF_WORDS, int(source_words * max_overlap)) + if budget < ARTICLE_MIN_POINTS * ARTICLE_MIN_POINT_WORDS: + raise ArticleBriefRejected( + ["brief_budget_too_small"], + {"source_words": source_words, "budget": budget}, + ) + + topic, topic_index = _mine_topic_with_source( + clauses, + word_cap=min( + ARTICLE_TOPIC_WORD_CAP, + budget - ARTICLE_MIN_POINTS * ARTICLE_MIN_POINT_WORDS, + ), + ) + candidates = [(i, clause) for i, clause in enumerate(clauses) if i != topic_index] + target_points = max(ARTICLE_MIN_POINTS, min(ARTICLE_MAX_POINTS, int(max_points))) + selected = select_outline_clauses(candidates, points=target_points) + + remaining = budget - len(topic.split()) + bullets: list[str] = [] + for _, clause in selected: + minimum_after = max( + 0, (ARTICLE_MIN_POINTS - len(bullets) - 1) * ARTICLE_MIN_POINT_WORDS + ) + word_cap = min(ARTICLE_POINT_WORD_CAP, remaining - minimum_after) + if word_cap < ARTICLE_MIN_POINT_WORDS: + break + fitted = _fit_phrase(clause, word_cap) + fitted_words = len(_word_tokens(fitted)) + if fitted_words < ARTICLE_MIN_POINT_WORDS: + continue + remaining -= fitted_words + bullets.append("- " + fitted) + + if len(bullets) < ARTICLE_MIN_POINTS: + raise ArticleBriefRejected( + ["too_few_bullets"], + {"source_words": source_words, "bullets": len(bullets)}, + ) + + points = "\n".join(bullets) + brief = { + "holdout_id": holdout_id, + "topic": topic, + "points": points, + "guard_facts": original, + } + brief_words = _content_word_count(topic, points) + overlap = brief_word_overlap_ratio(brief, original) + copy_ratio = pair_copy_ratio(f"{topic}\n{points}", original) + report = { + "source_words": source_words, + "brief_words": brief_words, + "bullets": len(bullets), + "brief_overlap_ratio": overlap, + "brief_copy_ratio": copy_ratio, + "max_brief_words": ARTICLE_MAX_BRIEF_WORDS, + "max_overlap": float(max_overlap), + "max_copy_ratio": float(max_copy_ratio), + } + + reasons: list[str] = [] + if brief_words > ARTICLE_MAX_BRIEF_WORDS: + reasons.append("brief_word_cap") + if overlap > float(max_overlap): + reasons.append("brief_overlap") + if copy_ratio > float(max_copy_ratio): + reasons.append("brief_copy_ratio") + if reasons: + raise ArticleBriefRejected(reasons, report) + return brief, report + + +def is_article_briefable(text: str) -> bool: + """True when a lossy article brief can be mined from this text.""" + try: + mine_article_brief(text) + except (ArticleBriefRejected, ValueError): + return False + return True diff --git a/src/personality_protect/article_holdout.py b/src/personality_protect/article_holdout.py new file mode 100644 index 0000000..ec31290 --- /dev/null +++ b/src/personality_protect/article_holdout.py @@ -0,0 +1,141 @@ +"""Deterministic article holdout carve for the article-channel eval. + +Same shape as the writer carve in :mod:`personality_protect.writer_holdout`, and +deliberately so — a second selection rule would be a second thing to audit. The +one difference is the constraint that dominates at this corpus size: with +fourteen articles and a retrieval floor of five, the carve cannot be a fixed +fraction. Reserving a quarter of a fourteen-piece pool is fine; reserving a +quarter of a six-piece pool would leave the article channel below the floor it +refuses to draft under, and the eval would be measuring an empty index. + +``resolve_article_holdout_n`` therefore takes the floor as an input and never +carves past it. +""" + +from __future__ import annotations + +import json +from collections.abc import Iterable, Sequence +from datetime import datetime, timezone +from typing import Any + +from personality_protect.article_brief import is_article_briefable +from personality_protect.config import ProfilePaths +from personality_protect.corpus_text import normalize_corpus_text +from personality_protect.models import Piece +from personality_protect.write_article import ARTICLE_SOURCES, MIN_ARTICLE_CORPUS +from personality_protect.writer_holdout import _order_key + +ARTICLE_HOLDOUT_FILENAME = "article_holdout_ids.json" + +# Four, not three. A one-sided sign test on three decisive comparisons bottoms +# out at p=0.125 even when one arm sweeps, so a carve of three cannot clear a +# 0.10 bar under any outcome — it is a run that has agreed in advance not to +# produce a result. Four reaches p=0.0625 on a sweep. Five is where a +# fourteen-article corpus starts eating the retrieval floor. +MIN_ARTICLE_HOLDOUT_N = 4 +MAX_ARTICLE_HOLDOUT_N = 5 +DEFAULT_ARTICLE_HOLDOUT_FRACTION = 0.3 + + +def resolve_article_holdout_n( + briefable: int, + *, + total_articles: int, + fraction: float = DEFAULT_ARTICLE_HOLDOUT_FRACTION, + minimum: int = MIN_ARTICLE_HOLDOUT_N, + maximum: int = MAX_ARTICLE_HOLDOUT_N, + keep_indexed: int = MIN_ARTICLE_CORPUS, +) -> int: + """Holdout size that leaves ``keep_indexed`` articles in retrieval.""" + allowed = max(0, int(total_articles) - max(0, int(keep_indexed))) + target = min(int(maximum), max(int(minimum), round(max(0, briefable) * max(0.0, fraction)))) + return max(0, min(briefable, allowed, target)) + + +def select_article_holdouts( + pieces: Iterable[Piece], + *, + pinned_ids: Sequence[str] = (), + fraction: float = DEFAULT_ARTICLE_HOLDOUT_FRACTION, + minimum: int = MIN_ARTICLE_HOLDOUT_N, + maximum: int = MAX_ARTICLE_HOLDOUT_N, + keep_indexed: int = MIN_ARTICLE_CORPUS, +) -> dict[str, Any]: + """Choose article holdouts and return a Contoso-safe receipt. + + Pinned ids are kept whether or not they are briefable: they are already out + of retrieval, and re-admitting one would silently contaminate a later run + with an earlier one. + """ + candidates = [piece for piece in pieces if piece.source in ARTICLE_SOURCES] + briefable = [ + piece.id + for piece in candidates + if is_article_briefable(normalize_corpus_text(piece.text or "")) + ] + pinned = [str(piece_id) for piece_id in pinned_ids] + known = {piece.id for piece in candidates} + missing_pinned = sorted(set(pinned) - known) + + target_n = resolve_article_holdout_n( + len(briefable), + total_articles=len(candidates), + fraction=fraction, + minimum=minimum, + maximum=maximum, + keep_indexed=keep_indexed, + ) + + chosen: list[str] = [piece_id for piece_id in pinned if piece_id in known] + for piece_id in sorted(set(briefable) - set(chosen), key=_order_key): + if len(chosen) >= target_n: + break + chosen.append(piece_id) + + return { + "kind": "article_holdout_carve", + "created_at": datetime.now(timezone.utc).isoformat(), + "holdout_ids": sorted(chosen), + "n_holdouts": len(chosen), + "n_articles": len(candidates), + "n_briefable": len(briefable), + "target_n": target_n, + "keep_indexed": int(keep_indexed), + "articles_left_indexed": max(0, len(candidates) - len(chosen)), + "pinned_ids": sorted(set(pinned) & known), + "pinned_ids_missing_from_corpus": missing_pinned, + "fraction": float(fraction), + "selection": "blake2b(piece_id) ascending, pinned ids first", + } + + +def load_pinned_article_holdout_ids(paths: ProfilePaths) -> list[str]: + """Ids from the profile's existing article carve (empty when absent).""" + path = paths.root / ARTICLE_HOLDOUT_FILENAME + if not path.is_file(): + return [] + data = json.loads(path.read_text(encoding="utf-8")) + if isinstance(data, dict): + ids = data.get("holdout_ids") or data.get("ids") or [] + else: + ids = data + return [str(piece_id) for piece_id in ids] + + +def save_article_holdout_ids(paths: ProfilePaths, receipt: dict[str, Any]) -> Any: + """Persist the article carve. Ids and counts only — never piece text.""" + path = paths.root / ARTICLE_HOLDOUT_FILENAME + payload = { + "holdout_ids": receipt["holdout_ids"], + "n_holdouts": receipt["n_holdouts"], + "n_articles": receipt["n_articles"], + "n_briefable": receipt["n_briefable"], + "articles_left_indexed": receipt["articles_left_indexed"], + "selection": receipt["selection"], + "updated_at": receipt["created_at"], + "note": "article-channel eval carve; ids only", + } + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + return path diff --git a/src/personality_protect/cli.py b/src/personality_protect/cli.py index 744e152..d21c28f 100644 --- a/src/personality_protect/cli.py +++ b/src/personality_protect/cli.py @@ -23,6 +23,11 @@ from personality_protect import __version__ from personality_protect.api import DEFAULT_HOST, DEFAULT_PORT from personality_protect.api import serve as serve_api +from personality_protect.article_holdout import ( + DEFAULT_ARTICLE_HOLDOUT_FRACTION, + MAX_ARTICLE_HOLDOUT_N, + MIN_ARTICLE_HOLDOUT_N, +) from personality_protect.config import ( CORPUS_BLOCK_BELOW, CORPUS_WARN_BELOW, @@ -50,10 +55,12 @@ run_eval, specificity_scorecard, ) +from personality_protect.eval_write_article import ARTICLE_ALPHA from personality_protect.eval_write_holdout import ( run_eval_write_holdout, write_receipt, ) +from personality_protect.eval_writer_adapter import SHIP_ALPHA from personality_protect.filter import ( filter_draft, paragraph_windows, @@ -110,6 +117,12 @@ MIN_WRITE_K, run_write, ) +from personality_protect.write_article import MIN_ARTICLE_CORPUS +from personality_protect.writer_holdout import ( + DEFAULT_HOLDOUT_FRACTION, + MAX_HOLDOUT_N, + MIN_HOLDOUT_N, +) app = typer.Typer( name="personality-protect", @@ -340,11 +353,19 @@ def index_voice_cmd( "--holdout-id", help="Piece id to exclude from retrieval (repeatable).", ), + from_carve: bool = typer.Option( + False, + "--from-carve", + help="Also exclude every id in the profile's writer and article carves.", + ), profile: str = typer.Option(DEFAULT_PROFILE, "--profile"), home: Optional[Path] = typer.Option(None, "--home"), as_json: bool = typer.Option(False, "--json"), ) -> None: """Rebuild the local voice retrieval index from the current corpus.""" + from personality_protect.article_holdout import load_pinned_article_holdout_ids + from personality_protect.writer_holdout import load_pinned_holdout_ids + _banner_from_ctx(ctx, json_mode=as_json) paths = get_paths(profile, home=home) try: @@ -353,7 +374,17 @@ def index_voice_cmd( console.print(f"[red]{exc}[/red]") raise typer.Exit(1) from exc - result = build_voice_index(paths, holdout_ids=holdout_id or ()) + # Retyping a widened carve as flags is how a holdout quietly re-enters + # retrieval; read it from the files the carves already wrote. + excluded = list(holdout_id or ()) + if from_carve: + excluded = sorted( + set(excluded) + | set(load_pinned_holdout_ids(paths)) + | set(load_pinned_article_holdout_ids(paths)) + ) + + result = build_voice_index(paths, holdout_ids=excluded) if as_json: typer.echo(json.dumps(result, indent=2)) return @@ -616,6 +647,402 @@ def build_writer_sft_cmd( f"writer SFT: {receipt['examples']} examples " f"(skipped {receipt['skipped']}) → {receipt['path']}" ) + console.print( + f"[dim]pair copy ratio (brief→post): median=" + f"{receipt['brief_copy_ratio']['median']} " + f"p90={receipt['brief_copy_ratio']['p90']} " + f"max={receipt['brief_copy_ratio']['max']} " + f"(cap {receipt['max_copy_ratio']})[/dim]" + ) + console.print(f"[dim]dropped: {receipt['dropped_by_reason']}[/dim]") + + +@app.command("select-writer-holdouts") +def select_writer_holdouts_cmd( + ctx: typer.Context, + fraction: float = typer.Option( + DEFAULT_HOLDOUT_FRACTION, + "--fraction", + help="Share of briefable posts to reserve for the ship gate.", + ), + minimum: int = typer.Option(MIN_HOLDOUT_N, "--min", help="Floor on holdout count."), + maximum: int = typer.Option(MAX_HOLDOUT_N, "--max", help="Ceiling on holdout count."), + apply: bool = typer.Option( + False, + "--apply", + help="Write the carve to the profile. Default is report-only.", + ), + profile: str = typer.Option(DEFAULT_PROFILE, "--profile"), + home: Optional[Path] = typer.Option(None, "--home"), + as_json: bool = typer.Option(False, "--json"), +) -> None: + """Pick a widened, deterministic holdout set for the writer ship gate.""" + from personality_protect.models import load_index + from personality_protect.writer_holdout import ( + load_pinned_holdout_ids, + save_holdout_ids, + select_writer_holdouts, + ) + + _banner_from_ctx(ctx, json_mode=as_json) + paths = get_paths(profile, home=home) + try: + pieces = load_index(paths.index_path) + except FileNotFoundError as exc: + console.print(f"[red]{exc}[/red]") + raise typer.Exit(1) from exc + + receipt = select_writer_holdouts( + pieces, + pinned_ids=load_pinned_holdout_ids(paths), + fraction=fraction, + minimum=minimum, + maximum=maximum, + ) + if apply: + receipt["written_to"] = str(save_holdout_ids(paths, receipt)) + + if as_json: + typer.echo(json.dumps(receipt, indent=2, ensure_ascii=False)) + return + console.print( + f"holdouts: {receipt['n_holdouts']} of {receipt['n_briefable']} briefable " + f"posts ({receipt['n_posts']} total); " + f"{receipt['train_pairs_remaining']} pairs left to train on" + ) + if not apply: + console.print("[dim]report-only — re-run with --apply to write the carve[/dim]") + else: + console.print( + "[yellow]Rebuild retrieval so the new holdouts are never indexed: " + "personality-protect index-voice[/yellow]" + ) + + +@app.command("select-article-holdouts") +def select_article_holdouts_cmd( + ctx: typer.Context, + fraction: float = typer.Option( + DEFAULT_ARTICLE_HOLDOUT_FRACTION, + "--fraction", + help="Share of briefable articles to reserve for the article eval.", + ), + minimum: int = typer.Option( + MIN_ARTICLE_HOLDOUT_N, "--min", help="Floor on article holdout count." + ), + maximum: int = typer.Option( + MAX_ARTICLE_HOLDOUT_N, "--max", help="Ceiling on article holdout count." + ), + keep_indexed: int = typer.Option( + MIN_ARTICLE_CORPUS, + "--keep-indexed", + help="Articles the carve must leave in retrieval for the write path.", + ), + apply: bool = typer.Option( + False, + "--apply", + help="Write the carve to the profile. Default is report-only.", + ), + profile: str = typer.Option(DEFAULT_PROFILE, "--profile"), + home: Optional[Path] = typer.Option(None, "--home"), + as_json: bool = typer.Option(False, "--json"), +) -> None: + """Pick a deterministic article holdout set for the article-channel eval.""" + from personality_protect.article_holdout import ( + load_pinned_article_holdout_ids, + save_article_holdout_ids, + select_article_holdouts, + ) + + _banner_from_ctx(ctx, json_mode=as_json) + paths = get_paths(profile, home=home) + try: + pieces = load_index(paths.index_path) + except FileNotFoundError as exc: + console.print(f"[red]{exc}[/red]") + raise typer.Exit(1) from exc + + receipt = select_article_holdouts( + pieces, + pinned_ids=load_pinned_article_holdout_ids(paths), + fraction=fraction, + minimum=minimum, + maximum=maximum, + keep_indexed=keep_indexed, + ) + if apply: + receipt["written_to"] = str(save_article_holdout_ids(paths, receipt)) + + if as_json: + typer.echo(json.dumps(receipt, indent=2, ensure_ascii=False)) + return + console.print( + f"article holdouts: {receipt['n_holdouts']} of {receipt['n_briefable']} " + f"briefable articles ({receipt['n_articles']} total); " + f"{receipt['articles_left_indexed']} left for retrieval" + ) + if receipt["n_holdouts"] == 0: + console.print( + "[yellow]Carve is empty: the corpus cannot spare an article and stay " + f"above the retrieval floor of {keep_indexed}.[/yellow]" + ) + if not apply: + console.print("[dim]report-only — re-run with --apply to write the carve[/dim]") + else: + console.print( + "[yellow]Rebuild retrieval so the new holdouts are never indexed: " + "personality-protect index-voice --from-carve[/yellow]" + ) + + +@app.command("eval-write-article") +def eval_write_article_cmd( + ctx: typer.Context, + holdout_id: Optional[list[str]] = typer.Option( + None, + "--holdout-id", + help="Article holdout id (repeatable). Defaults to the saved carve.", + ), + k: int = typer.Option( + DEFAULT_WRITE_K, + "--k", + help=f"Article exemplars per section ({MIN_WRITE_K}–{MAX_WRITE_K}).", + ), + alpha: float = typer.Option( + ARTICLE_ALPHA, "--alpha", help="One-sided significance the run must reach." + ), + out: Optional[Path] = typer.Option( + None, "--out", help="Write Contoso-safe receipt JSON (no draft bodies)." + ), + save_raw: bool = typer.Option( + False, + "--save-raw/--no-save-raw", + help="Dump exact prompts and drafts under the profile's gitignored " + "dogfood/raw dir (personal text; never commit).", + ), + profile: str = typer.Option(DEFAULT_PROFILE, "--profile"), + home: Optional[Path] = typer.Option(None, "--home"), + as_json: bool = typer.Option(False, "--json"), +) -> None: + """Score article-channel drafts against bare-base on never-indexed articles. + + Loads MLX weights once and reuses them across both arms. Needs PP_MLX_ALLOW=1 + and a real Metal device; tests inject a generator instead. + """ + from personality_protect.article_holdout import load_pinned_article_holdout_ids + from personality_protect.eval_write_article import ( + run_eval_write_article, + write_article_eval_receipt, + ) + from personality_protect.write import make_mlx_generator + + _banner_from_ctx(ctx, json_mode=as_json) + paths = get_paths(profile, home=home) + try: + config = load_config(paths) + except FileNotFoundError as exc: + console.print(f"[red]{exc}[/red]") + raise typer.Exit(1) from exc + + ids = [str(piece_id) for piece_id in (holdout_id or []) if str(piece_id).strip()] + if not ids: + ids = load_pinned_article_holdout_ids(paths) + if not ids: + console.print( + "[red]No article holdouts. Run: " + "personality-protect select-article-holdouts --apply[/red]" + ) + raise typer.Exit(1) + + try: + generator = make_mlx_generator(base_model=config.base_model) + except RuntimeError as exc: + console.print(f"[red]{exc}[/red]") + raise typer.Exit(1) from exc + + done = {"n": 0} + + def _progress(item: dict) -> None: + done["n"] += 1 + console.print( + f"[dim]{done['n']}/{len(ids)} {item['holdout_id']}: {item['winner']} " + f"(article {item['article_draft_words']}w vs base " + f"{item['base_draft_words']}w, holdout {item['holdout_words']}w)[/dim]" + ) + + try: + receipt = run_eval_write_article( + paths, + ids, + k=k, + generate_fn=generator, + alpha=alpha, + save_raw=save_raw, + on_item=None if as_json else _progress, + ) + except ValueError as exc: + console.print(f"[red]{exc}[/red]") + raise typer.Exit(2) from exc + except (FileNotFoundError, RuntimeError) as exc: + console.print(f"[red]{exc}[/red]") + raise typer.Exit(1) from exc + + target = out or (paths.root / "dogfood" / "eval_write_article_receipt.json") + write_article_eval_receipt(receipt, target) + + if as_json: + typer.echo(json.dumps(receipt, indent=2, ensure_ascii=False)) + return + + wins = receipt["wins"] + console.print( + f"[bold]eval-write-article[/bold] n={receipt['n_holdouts']} " + f"model={receipt['model']} articles_indexed={receipt['articles_indexed']}" + ) + console.print( + f"carve_ok={receipt['carve']['ok']} wins article={wins['article']} " + f"base={wins['base']} tie={wins['tie']} " + f"(p={receipt['p_value']}, alpha={receipt['alpha']})" + ) + console.print( + f"disqualified: article {receipt['disqualified']['article']}, " + f"base {receipt['disqualified']['base']}" + ) + for item in receipt["items"]: + console.print( + f" {item['holdout_id']}: winner={item['winner']} " + f"Δ={item['delta_base_minus_article']} " + f"article_dist={item['article_distance']} " + f"base_dist={item['base_distance']} " + f"words {item['article_draft_words']}/{item['base_draft_words']} " + f"(holdout {item['holdout_words']}) " + f"brief_overlap={item['brief_overlap_ratio']}" + ) + console.print(f"verdict: [bold]{receipt['verdict']}[/bold] → {target}") + if receipt["blocking_reasons"]: + console.print(f"[yellow]{', '.join(receipt['blocking_reasons'])}[/yellow]") + if save_raw: + console.print( + "[dim]raw prompts/drafts under profile dogfood/raw " + "(personal text — never commit)[/dim]" + ) + + +@app.command("eval-writer-adapter") +def eval_writer_adapter_cmd( + ctx: typer.Context, + k: int = typer.Option(DEFAULT_WRITE_K, "--k", help="Exemplars per draft."), + max_tokens: int = typer.Option(DEFAULT_WRITE_MAX_TOKENS, "--max-tokens"), + alpha: float = typer.Option( + SHIP_ALPHA, "--alpha", help="One-sided significance required to keep." + ), + archive_on_fail: bool = typer.Option( + False, + "--archive-on-fail", + help="Move the adapter aside when the gate fails (write returns to adapter=none).", + ), + out: Optional[Path] = typer.Option(None, "--out", help="Write the receipt JSON here."), + profile: str = typer.Option(DEFAULT_PROFILE, "--profile"), + home: Optional[Path] = typer.Option(None, "--home"), + as_json: bool = typer.Option(False, "--json"), +) -> None: + """Ship gate: RAG+writer LoRA vs RAG-alone on the carved holdouts. + + Loads MLX weights once per arm. Needs PP_MLX_ALLOW=1 and a real Metal + device — importing MLX without one aborts the interpreter. + """ + from personality_protect.eval_writer_adapter import ( + run_writer_adapter_gate, + write_gate_receipt, + ) + from personality_protect.write import ( + archive_writer_adapter, + make_mlx_generator, + resolve_writer_adapter, + ) + from personality_protect.writer_holdout import load_pinned_holdout_ids + + _banner_from_ctx(ctx, json_mode=as_json) + paths = get_paths(profile, home=home) + try: + config = load_config(paths) + except FileNotFoundError as exc: + console.print(f"[red]{exc}[/red]") + raise typer.Exit(1) from exc + + holdout_ids = load_pinned_holdout_ids(paths) + if not holdout_ids: + console.print( + "[red]No holdout carve found. Run: " + "personality-protect select-writer-holdouts --apply[/red]" + ) + raise typer.Exit(1) + + adapter_path = resolve_writer_adapter(paths) + if adapter_path is None: + console.print( + "[red]No writer adapter to gate. Train one with: " + "personality-protect train --writer[/red]" + ) + raise typer.Exit(1) + + try: + generate_adapter = make_mlx_generator( + base_model=config.base_model, adapter_path=adapter_path + ) + generate_rag = make_mlx_generator(base_model=config.base_model) + except RuntimeError as exc: + console.print(f"[red]{exc}[/red]") + raise typer.Exit(1) from exc + + done = {"n": 0} + + def _progress(item: dict) -> None: + done["n"] += 1 + console.print( + f"[dim]{done['n']}/{len(holdout_ids)} {item['holdout_id']}: " + f"{item['winner']}[/dim]" + ) + + try: + receipt = run_writer_adapter_gate( + paths, + holdout_ids, + generate_fn_adapter=generate_adapter, + generate_fn_rag=generate_rag, + k=k, + max_tokens=max_tokens, + alpha=alpha, + on_item=None if as_json else _progress, + ) + except (ValueError, FileNotFoundError) as exc: + console.print(f"[red]{exc}[/red]") + raise typer.Exit(1) from exc + + if receipt["decision"] == "archive" and archive_on_fail: + receipt["archived_to"] = archive_writer_adapter(paths, reason="gate-fail") + + target = out or (paths.root / "dogfood" / "writer_adapter_gate_receipt.json") + write_gate_receipt(receipt, target) + + if as_json: + typer.echo(json.dumps(receipt, indent=2, ensure_ascii=False)) + else: + wins = receipt["wins"] + console.print( + f"gate n={receipt['n_holdouts']}: adapter {wins['adapter']} — " + f"rag {wins['rag']} — tie {wins['tie']} " + f"(p={receipt['p_value']}, alpha={receipt['alpha']})" + ) + console.print( + f"disqualified: adapter {receipt['disqualified']['adapter']}, " + f"rag {receipt['disqualified']['rag']}" + ) + console.print(f"decision: [bold]{receipt['decision']}[/bold] → {target}") + if receipt["blocking_reasons"]: + console.print(f"[yellow]{', '.join(receipt['blocking_reasons'])}[/yellow]") + if receipt["decision"] != "keep": + raise typer.Exit(1) @app.command("write") @@ -876,6 +1303,29 @@ def train_cmd( "use 2048 with a higher --memory-gb cap for article sections." ), ), + num_layers: Optional[int] = typer.Option( + None, + "--num-layers", + help="MLX: layers to adapt (default 8; the --writer recipe uses 16).", + ), + lora_rank: Optional[int] = typer.Option( + None, + "--lora-rank", + help="MLX: LoRA rank (default 8; the --writer recipe uses 16).", + ), + learning_rate: Optional[float] = typer.Option( + None, + "--learning-rate", + help="MLX: LoRA learning rate (default 1e-5; the --writer recipe uses 3e-5).", + ), + detach: bool = typer.Option( + False, + "--detach", + help=( + "Run the train in its own session and return immediately " + "(survives closing the shell). Prints pid and log path." + ), + ), proof: bool = typer.Option( False, "--proof", @@ -951,6 +1401,21 @@ def train_cmd( console.print(f"[red]pairs file not found: {pairs}[/red]") raise typer.Exit(2) + if detach: + from personality_protect.detach import relaunch_self_detached, timestamped_log_path + + log_path = timestamped_log_path(paths.root / "dogfood", "train") + spawned = relaunch_self_detached( + [arg for arg in sys.argv[1:] if arg != "--detach"], + log_path=log_path, + ) + if as_json: + typer.echo(json.dumps(spawned, indent=2)) + return + console.print(f"train detached: pid {spawned['pid']}") + console.print(f"log: {spawned['log_path']}") + return + try: detected = detect_backend( "mock" if mock else backend, # type: ignore[arg-type] @@ -1174,6 +1639,9 @@ def on_progress(info: dict) -> None: progress_callback=callback, pairs=pairs, writer=writer, + num_layers=num_layers, + lora_rank=lora_rank, + learning_rate=learning_rate, ) except (FileNotFoundError, RuntimeError, MockFallbackError, ValueError) as exc: console.print(f"[red]{exc}[/red]") diff --git a/src/personality_protect/detach.py b/src/personality_protect/detach.py new file mode 100644 index 0000000..a0c0a25 --- /dev/null +++ b/src/personality_protect/detach.py @@ -0,0 +1,82 @@ +"""Portable detached process launch for long unattended runs. + +A writer LoRA train is a multi-hour job, and running it in the foreground of a +shell ties its lifetime to that shell: closing the terminal, or a tool that +interrupts the command it started, takes the train down with it and the run is +lost with no checkpoint to resume from. + +The usual shell answer, ``setsid``, is a util-linux binary and is **not present +on macOS** — a launcher that reaches for it dies before Python ever starts, and +the failure looks like "nothing trained" rather than "launcher broken". Python's +own ``start_new_session=True`` does the same thing (``setsid(2)``) on every +POSIX platform, so the detach happens in-process with nothing to shell out to. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +from datetime import datetime, timezone +from pathlib import Path +from typing import Any, Sequence + + +def timestamped_log_path(directory: Path, prefix: str) -> Path: + """UTC-stamped log file path (directory created on demand).""" + directory.mkdir(parents=True, exist_ok=True) + stamp = datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%SZ") + return directory / f"{prefix}_{stamp}.log" + + +def spawn_detached( + argv: Sequence[str], + *, + log_path: Path, + env: dict[str, str] | None = None, + cwd: Path | None = None, + popen: Any = subprocess.Popen, +) -> dict[str, Any]: + """Start ``argv`` in its own session, streaming output to ``log_path``. + + ``start_new_session=True`` detaches the child from the caller's process + group, so a signal sent to that group — which is what an interrupted or + closed shell delivers — does not reach it. + + stdin is closed rather than inherited: a detached job that blocks on a + prompt it can never receive would hang until it is killed. + """ + log_path.parent.mkdir(parents=True, exist_ok=True) + child_env = dict(os.environ) + child_env.update(env or {}) + # Without this, the child's prints stay block-buffered into the redirected + # log and a healthy run is indistinguishable from a hung one. + child_env["PYTHONUNBUFFERED"] = "1" + + with log_path.open("wb") as handle: + process = popen( + list(argv), + stdout=handle, + stderr=subprocess.STDOUT, + stdin=subprocess.DEVNULL, + env=child_env, + cwd=str(cwd) if cwd else None, + start_new_session=True, + ) + return {"pid": int(process.pid), "log_path": str(log_path), "argv": list(argv)} + + +def relaunch_self_detached( + cli_args: Sequence[str], + *, + log_path: Path, + env: dict[str, str] | None = None, +) -> dict[str, Any]: + """Re-run this CLI's own command detached, minus the flag that asked for it. + + Uses ``sys.executable -m`` rather than the console-script name so the child + lands in the same interpreter and virtualenv as the parent, whatever the + caller's PATH happens to resolve. + """ + argv = [sys.executable, "-m", "personality_protect.cli", *cli_args] + return spawn_detached(argv, log_path=log_path, env=env) diff --git a/src/personality_protect/devoice.py b/src/personality_protect/devoice.py new file mode 100644 index 0000000..e7e1b04 --- /dev/null +++ b/src/personality_protect/devoice.py @@ -0,0 +1,563 @@ +"""De-voicing operator for writer SFT pairs. + +A writer LoRA is only learning anything if its training pairs are +``(D(y), y)``: a de-voiced restatement of a post mapped to the post the author +actually wrote. The first writer run mined its brief as a *verbatim extract* of +``y``, so the input and the target shared their wording and the pair was close +to ``(y, y)``. Gradient descent takes the cheapest route through that data — +copy the input forward — and the resulting adapter parroted its context at +generation time instead of writing. That is the identity map, and no amount of +extra corpus fixes it, because the objective itself is wrong. + +``D`` therefore has to destroy *form* while preserving *content*: + +* content kept — named entities, evidence figures, claim vocabulary, order +* form destroyed — second-person address, contractions, emphasis punctuation, + shouted words, sentence-initial conjunctions, discourse markers, one-line + fragment rhythm, article/auxiliary/filler scaffolding + +Everything here is deterministic and Contoso-testable. There is no model in the +loop: an LLM flattener is exactly the component that can leak the author's +cadence back into the input, and a rule set can be inspected and gated. + +The operator ships with its own verifier. :func:`devoice_report` measures the +transform on the existing shipped :func:`~personality_protect.pair_gate.gate_pair` +axes, asserts that ``D`` invented no entity or figure, and — the check that +matters — measures how much of the de-voiced text still sits inside a 5-gram of +the original. Callers fail closed on that number rather than trusting the rules. +""" + +from __future__ import annotations + +import re +from typing import Any + +from personality_protect.corpus_text import normalize_corpus_text +from personality_protect.eval_compare import extract_evidence_number_keys +from personality_protect.eval_write_holdout import mine_brief_from_holdout +from personality_protect.pair_gate import gate_pair, text_axes +from personality_protect.writer_guards import ( + COMMON_CAPITALIZED, + copied_token_ratio, + extract_entity_keys, + extract_named_entity_keys, +) + +# A sentence this short with no entity and no figure is carrying rhythm, not +# content: dropping it is the cleanest de-voicing available, since there is no +# meaning to preserve. +CADENCE_MAX_WORDS = 6 +# Never let cadence-stripping eat the post. Below this share of content words +# the brief would no longer describe the same piece. +MIN_KEEP_CONTENT_RATIO = 0.55 +# Merge target for reflow. The author writes in short standalone lines; notes +# run long and unbroken, which is what moves both cadence axes at once. +TARGET_SENTENCE_WORDS = 16 +# Share of de-voiced words still inside an original 5-gram. Above this the pair +# is drifting back toward (y, y) whatever the rules did. +MAX_PAIR_COPY_RATIO = 0.35 +# Brief mining's own overlap cap, applied against the already de-voiced note +# rather than the post. Holding a note to the 25% budget written for raw posts +# would reject sources purely for having been shortened by the operator, while +# the number that matters — what the brief shares with the post — is measured +# separately and gates the pair. +DEVOICED_BRIEF_MAX_OVERLAP = 0.5 + +_SENTENCE_SPLIT = re.compile(r"(?<=[.!?])\s+|\n+") +_WORD_RE = re.compile(r"[A-Za-z0-9]+(?:['’][A-Za-z0-9]+)?") +_URL_RE = re.compile(r"https?://\S+|www\.\S+", re.IGNORECASE) +_HASHTAG_RE = re.compile(r"(?»→▪◆✔✅🔹]+\s*") + +# Author-agnostic English discourse scaffolding. These are the phrases a writer +# uses to set rhythm and stance; a note never contains them. Kept generic on +# purpose — nothing here encodes one person's idiom. +_DISCOURSE_MARKERS = ( + "here is the thing", + "here's the thing", + "here is the part", + "here's the part", + "here is what", + "here's what", + "the thing is", + "truth is", + "the truth is", + "let that sink in", + "let me be clear", + "make no mistake", + "full stop", + "end of story", + "plot twist", + "spoiler", + "newsflash", + "news flash", + "hot take", + "unpopular opinion", + "real talk", + "look", + "listen", + "folks", + "friends", + "so here we are", + "and yet", + "but here we are", + "read that again", + "i will say it again", + "i'll say it again", + "say it with me", + "think about that", + "think about it", +) +_DISCOURSE_RE = re.compile( + r"(?i)(?:^|(?<=[.!?;:—–]\s))\s*(?:" + + "|".join(re.escape(marker) for marker in _DISCOURSE_MARKERS) + + r")\s*[,:.—–-]*\s*" +) + +# Sentence-initial conjunctions are pure cadence: the clause stands without +# them and the note form never opens on one. +_LEADING_CONJUNCTION_RE = re.compile( + r"(?i)^\s*(?:and|but|so|or|yet|because|plus|also|then|now|well|okay|ok|" + r"anyway|besides|still|however|meanwhile)\b[\s,:—–-]*" +) + +_CONTRACTIONS: tuple[tuple[re.Pattern[str], str], ...] = tuple( + (re.compile(pattern, re.IGNORECASE), replacement) + for pattern, replacement in ( + (r"\bcan't\b", "cannot"), + (r"\bwon't\b", "will not"), + (r"\bshan't\b", "shall not"), + (r"\blet's\b", "let us"), + (r"\bain't\b", "is not"), + (r"\bgonna\b", "going to"), + (r"\bwanna\b", "want to"), + (r"\bgotta\b", "got to"), + (r"\b(\w+)n't\b", r"\1 not"), + (r"\b(i|you|we|they)'re\b", r"\1 are"), + (r"\b(i|you|we|they)'ve\b", r"\1 have"), + (r"\b(i|you|we|they|he|she|it)'ll\b", r"\1 will"), + (r"\b(i|you|we|they|he|she|it)'d\b", r"\1 would"), + (r"\b(he|she|it|that|there|what|who|here)'s\b", r"\1 is"), + (r"\bi'm\b", "I am"), + ) +) + +# Second-person address is the author's rhetorical stance, not the post's +# content, and it is the axis the shipped pair gate already refuses in an input. +# Third-person plural is the one substitution that needs no verb agreement fix: +# "you ship" and "they ship" inflect identically. +_SECOND_PERSON: tuple[tuple[re.Pattern[str], str], ...] = tuple( + (re.compile(pattern), replacement) + for pattern, replacement in ( + (r"\byou\b", "they"), + (r"\bYou\b", "They"), + (r"\byour\b", "their"), + (r"\bYour\b", "Their"), + (r"\byours\b", "theirs"), + (r"\bYours\b", "Theirs"), + (r"\byourself\b", "themselves"), + (r"\bYourself\b", "Themselves"), + (r"\byourselves\b", "themselves"), + (r"\bYourselves\b", "Themselves"), + ) +) + +# Articles, copulas, auxiliaries and intensifiers. Dropping them is what turns a +# written sentence into a jotted note, and it is also what reliably breaks the +# 5-gram windows the target would otherwise share with its own input. +_NOTE_DROP_WORDS = frozenset( + """ + a an the this that these those + is are was were be been being am + do does did done + has have had + will would shall should may might must can could + just really actually simply literally basically honestly frankly obviously + clearly very quite rather truly definitely certainly absolutely totally + completely genuinely seriously + """.split() +) + +# Second tier. Prepositions, coordinators and pronouns are the connective +# tissue of written prose and roughly half of its tokens; a jotted note has +# almost none of them. Dropping them is what finally moves the copy ratio, +# because a 5-gram window cannot survive a deletion every few words. +# +# Negation, quantity and comparison words are deliberately absent: dropping +# "not" or "less" would not de-voice the note, it would reverse the claim. +_TELEGRAPH_DROP_WORDS = frozenset( + """ + of to in on at by for with from into about over under between through + across around during within without against upon toward towards among + and or as than then there here + it its they them their theirs we us our ours he him his she her hers + i me my mine you your yours + who whom whose which what where when while + """.split() +) +_EMPHASIS_PUNCT_RE = re.compile(r"[!?]{2,}|!+") +_ELLIPSIS_RE = re.compile(r"\.{2,}|…") +_DASH_ASIDE_RE = re.compile(r"\s*[—–]\s*|\s+--\s+") +_MULTISPACE_RE = re.compile(r"[ \t]{2,}") +_SPACE_BEFORE_PUNCT_RE = re.compile(r"\s+([,.;:%)\]])") + + +def _word_tokens(text: str) -> list[str]: + return [match.group(0) for match in _WORD_RE.finditer(text or "")] + + +def _content_word_count(text: str) -> int: + return len(_word_tokens(text)) + + +def _sentences(body: str) -> list[str]: + return [part.strip() for part in _SENTENCE_SPLIT.split(body) if part.strip()] + + +def _fact_weight(sentence: str) -> float: + """How much a sentence would cost to drop (entities and figures dominate).""" + entities = len(extract_named_entity_keys(sentence)) + numbers = len(extract_evidence_number_keys(sentence)) + return 3.0 * numbers + 2.0 * entities + min(_content_word_count(sentence), 20) / 20.0 + + +def _is_cadence_only(sentence: str) -> bool: + """True for a line that carries rhythm and no claim.""" + if _content_word_count(sentence) > CADENCE_MAX_WORDS: + return False + if extract_named_entity_keys(sentence) or extract_evidence_number_keys(sentence): + return False + return True + + +def _strip_surface_noise(text: str) -> str: + cleaned = _URL_RE.sub(" ", text) + cleaned = _HASHTAG_RE.sub(" ", cleaned) + cleaned = _DECORATIVE_RE.sub(" ", cleaned) + return _BULLET_GLYPH_RE.sub("", cleaned) + + +def _deshout(text: str) -> str: + """Lowercase shouted emphasis while leaving acronyms alone. + + ``THIS IS THE WORK`` is cadence; ``API`` and ``SaaS`` are content. The + invention guard's own vocabulary decides which is which, so the two stay in + agreement about what counts as a name. + """ + + def replace(match: re.Match[str]) -> str: + token = match.group(0) + return token.lower() if token.lower() in COMMON_CAPITALIZED else token + + return re.sub(r"\b[A-Z]{2,}\b", replace, text) + + +def _neutralize(sentence: str) -> str: + """Strip stance and register from one sentence, keeping its claim.""" + text = sentence + for pattern, replacement in _CONTRACTIONS: + text = pattern.sub(replacement, text) + for pattern, replacement in _SECOND_PERSON: + text = pattern.sub(replacement, text) + text = _DISCOURSE_RE.sub(" ", text) + text = _LEADING_CONJUNCTION_RE.sub("", text) + text = _deshout(text) + text = _EMPHASIS_PUNCT_RE.sub(".", text) + text = _ELLIPSIS_RE.sub(".", text) + text = _DASH_ASIDE_RE.sub(", ", text) + return text.strip() + + +def _to_note_form(sentence: str) -> str: + """Drop the scaffolding words a note would never have been written with. + + Articles, copulas, auxiliaries and intensifiers carry no claim, and removing + them is the difference between handing the model a sentence to copy and + handing it a note to write from. + """ + dropped = _NOTE_DROP_WORDS | _TELEGRAPH_DROP_WORDS + kept: list[str] = [] + for token in re.split(r"(\W+)", sentence): + if not token: + continue + if _WORD_RE.fullmatch(token) and token.lower() in dropped: + continue + kept.append(token) + text = "".join(kept) + text = _MULTISPACE_RE.sub(" ", text) + text = _SPACE_BEFORE_PUNCT_RE.sub(r"\1", text) + text = re.sub(r"^[\s,;:.]+", "", text) + return text.strip() + + +def _lower_continuation(clause: str) -> str: + """Lowercase a clause promoted to mid-sentence, unless it opens on a name. + + Sentence-initial capitals are punctuation, not spelling. Carrying them past + a semicolon leaves ``queue is boring; They ship``, and the invention guard + reads a stray capitalized word as a candidate name. + """ + match = _WORD_RE.search(clause) + if not match or match.start() != 0: + return clause + word = match.group(0) + if word == "I" or word.lower() not in COMMON_CAPITALIZED: + return clause + return word[0].lower() + clause[1:] + + +def _reflow(sentences: list[str]) -> str: + """Merge short lines into note-length prose. + + The author's fragment rhythm — many standalone short lines — is one of the + loudest voice signals in the corpus, and it is measured directly by the pair + gate's ``short_line_ratio`` and ``median_sentence_words``. Merging collapses + both at once and yields a single unbroken block, which is the shape of a + brief rather than of a post. + """ + merged: list[str] = [] + buffer: list[str] = [] + + def flush(parts: list[str]) -> str: + head, *rest = parts + return "; ".join([head, *(_lower_continuation(part) for part in rest)]) + "." + + for sentence in sentences: + buffer.append(sentence.rstrip(" .,;:")) + if sum(_content_word_count(part) for part in buffer) >= TARGET_SENTENCE_WORDS: + merged.append(flush(buffer)) + buffer = [] + if buffer: + tail = flush(buffer) + if merged: + merged[-1] = merged[-1].rstrip(".") + "; " + _lower_continuation(tail) + else: + merged.append(tail) + return " ".join(merged).strip() + + +def devoice_sentences( + text: str, + *, + min_keep_content_ratio: float = MIN_KEEP_CONTENT_RATIO, +) -> list[str]: + """De-voiced note clauses, one per surviving source sentence. + + Kept separate from :func:`devoice_text` because the two consumers want + different shapes. Brief mining ranks and picks *individual claims*, so it + needs the clause list; the cadence gate measures rhythm, so it needs the + reflowed block. Deriving both from one pass keeps them consistent. + + Cadence-only lines are dropped cheapest-first and the drop stops before the + note falls below ``min_keep_content_ratio`` of the original content words — + a de-voicer that deletes the post is not preserving meaning, and the brief + mined from it would describe something else. + """ + body = normalize_corpus_text(_strip_surface_noise(text or "")) + if not body.strip(): + return [] + + sentences = _sentences(body) + if not sentences: + return [] + + total_words = sum(_content_word_count(sentence) for sentence in sentences) + floor = int(total_words * max(0.0, min(1.0, min_keep_content_ratio))) + drop_order = sorted( + (index for index, s in enumerate(sentences) if _is_cadence_only(s)), + key=lambda index: (_fact_weight(sentences[index]), index), + ) + dropped: set[int] = set() + kept_words = total_words + for index in drop_order: + cost = _content_word_count(sentences[index]) + if kept_words - cost < floor: + continue + dropped.add(index) + kept_words -= cost + + rewritten: list[str] = [] + for index, sentence in enumerate(sentences): + if index in dropped: + continue + neutral = _to_note_form(_neutralize(sentence)) + if neutral: + rewritten.append(neutral) + return rewritten + + +def devoice_text( + text: str, + *, + min_keep_content_ratio: float = MIN_KEEP_CONTENT_RATIO, +) -> str: + """Return ``D(y)``: the claims of ``text`` with the author's form removed.""" + rewritten = devoice_sentences( + text, min_keep_content_ratio=min_keep_content_ratio + ) + if not rewritten: + return "" + return _reflow(rewritten) + + +class DevoiceRejected(ValueError): + """A pair could not be de-voiced far enough away from its target.""" + + def __init__(self, reasons: list[str], report: dict[str, Any]) -> None: + super().__init__("de-voiced pair rejected: " + ", ".join(reasons)) + self.reasons = reasons + self.report = report + + +def mine_writer_brief( + text: str, + *, + holdout_id: str = "", + max_copy_ratio: float = MAX_PAIR_COPY_RATIO, + max_brief_overlap: float = DEVOICED_BRIEF_MAX_OVERLAP, +) -> tuple[dict[str, str], dict[str, Any]]: + """Mine a brief from ``D(y)`` and prove it is not an extract of ``y``. + + The single entry point for both halves of the writer path, so training and + the ship gate cannot drift apart: an adapter trained on de-voiced briefs and + then evaluated on verbatim extracts would be measured on a distribution it + never saw. + + Brief mining runs against the de-voiced clauses rather than the reflowed + block because it ranks and selects individual claims. Its own overlap cap is + relaxed here — it exists to keep a brief from becoming an extract of its + source, and by this point the source is already a note, not the post. What + the brief may share with the *post* is measured directly and gates the pair. + + Raises :class:`DevoiceRejected` when the operator did not move the pair far + enough, so callers drop the row instead of training on ``(y, y)``. + """ + original = normalize_corpus_text(text) + clauses = devoice_sentences(original) + if not clauses: + raise DevoiceRejected(["devoice_empty"], {}) + + devoiced = _reflow(clauses) + report = devoice_report(original, devoiced, max_copy_ratio=max_copy_ratio) + # The row that trains is (brief, post), not (note, post), so the + # document-level copy ratio is recorded and not enforced here. A note that + # still shares long windows with the post is a warning about the operator; + # whether *this pair* is an identity map is answered below, on the brief. + blocking = [reason for reason in report["failed"] if reason != "pair_copy_ratio"] + if blocking: + raise DevoiceRejected(blocking, report) + + brief = mine_brief_from_holdout( + "\n".join(clauses), + holdout_id=holdout_id, + max_overlap=max_brief_overlap, + ) + # The invention guard's allowed-facts set has to stay the *post*: the note + # drops connective words, and scoring a draft against the note would accuse + # it of inventing figures the author actually wrote. + brief["guard_facts"] = original + brief_text = f"{brief['topic']}\n{brief['points']}" + brief_ratio = pair_copy_ratio(brief_text, original) + report = { + **report, + "brief_copy_ratio": brief_ratio, + "brief_words": len(brief_text.split()), + } + if brief_ratio > float(max_copy_ratio): + raise DevoiceRejected(["brief_copy_ratio"], report) + return brief, report + + +def pair_copy_ratio(input_text: str, target_text: str) -> float: + """Share of input words sitting inside a 5-gram of the target. + + This is the identity-map meter. A verbatim extract scores near 1.0; a true + de-voiced note scores low because its word sequences no longer exist in the + post. It is the single number worth gating a writer pair on. + """ + return copied_token_ratio(input_text, [target_text]) + + +def devoice_report( + original: str, + devoiced: str, + *, + channel: str = "auto", + max_copy_ratio: float = MAX_PAIR_COPY_RATIO, +) -> dict[str, Any]: + """Measure one ``(D(y), y)`` pair and decide whether it may train. + + Three independent questions, each fail-closed: + + * did ``D`` actually move the cadence axes? — delegated to the shipped + :func:`~personality_protect.pair_gate.gate_pair` + * did ``D`` invent anything? — a de-voicer that adds an entity or a figure + would poison the invention guard's allowed-facts set + * is the pair still near ``(y, y)``? — :func:`pair_copy_ratio` + + The pair gate's ``max_input_proper_1k`` check is recorded but not blocking + here. That threshold exists to catch an *LLM* flattener echoing the author's + text back, and it reads proper-noun density as the tell. This operator + preserves proper nouns by construction because they are the brief's content, + so the same number would only be measuring how many companies the author + named — and dropping connective words raises the density further without + adding a single name. The entity-subset invariant below is the check that + actually answers "did the input gain anything it should not have". + + ``channel`` defaults to ``auto`` so the shipped channel inference decides + whether the fragment-rhythm check applies. A prose-shaped post has no short + lines to begin with, and holding it to a fragment gap it never had would + reject the pair for the author's paragraph habits rather than for anything + the operator did. + """ + gate = gate_pair(devoiced, original, channel=channel) + # Compare single tokens, not spans. Dropping a connective can leave two + # names of the original adjacent ("Contoso is Ledger" -> "Contoso Ledger"), + # which reads as a new multi-word span while inventing nothing: both names + # were already in the source. + devoiced_names = { + token + for key in extract_named_entity_keys(devoiced) + for token in key.split(" ") + if token + } + new_entities = devoiced_names - extract_entity_keys(original) + new_numbers = extract_evidence_number_keys(devoiced) - extract_evidence_number_keys( + original + ) + copy_ratio = pair_copy_ratio(devoiced, original) + + blocking_gate_failures = [ + reason for reason in gate["failed"] if reason != "max_input_proper_1k" + ] + failed = list(blocking_gate_failures) + if new_entities: + failed.append("devoice_invented_entities") + if new_numbers: + failed.append("devoice_invented_numbers") + if copy_ratio > float(max_copy_ratio): + failed.append("pair_copy_ratio") + + return { + "pass": not failed, + "failed": failed, + "copy_ratio": copy_ratio, + "max_copy_ratio": float(max_copy_ratio), + "invented_entities_count": len(new_entities), + "invented_numbers_count": len(new_numbers), + "gate_pass": bool(gate["pass"]), + "gate_failed": list(gate["failed"]), + "gate_advisory": [ + reason for reason in gate["failed"] if reason == "max_input_proper_1k" + ], + "resolved_channel": gate["resolved_channel"], + "frag_gap_ratio": gate["frag_gap_ratio"], + "median_sentence_gap": gate["median_sentence_gap"], + "input_axes": text_axes(devoiced), + "output_axes": text_axes(original), + } diff --git a/src/personality_protect/eval_write_article.py b/src/personality_protect/eval_write_article.py new file mode 100644 index 0000000..a6a78da --- /dev/null +++ b/src/personality_protect/eval_write_article.py @@ -0,0 +1,418 @@ +"""Holdout eval for the article channel: outline→sections→stitch vs bare base. + +The post channel has had an honest holdout gate since Lane G; the article +channel shipped without one, which meant its voice claim rested on nothing but +the fact that the code ran. This is the missing half. + +The comparison is the same one the post path makes, with two article-specific +adjustments that exist so the result measures writing rather than editing: + +* **both arms are trimmed to the same ceiling.** The article arm's length is the + sum of its section budgets; the single-shot arm is trimmed to that same total. + A length penalty is part of the distance score, so arms edited to different + lengths would be separated before either of them wrote a word. +* **the brief is mined as an outline**, by + :func:`~personality_protect.article_brief.mine_article_brief`, and capped at a + fixed word count rather than a share of the source. + +Disqualifications carry over unchanged from the post scorer: a draft that +parrots its exemplars, hands the brief back, or invents entities and figures +cannot win on rhythm, however close its cadence lands. + +MLX is never imported here. The CLI injects a generator; tests inject callables. +""" + +from __future__ import annotations + +import json +from collections.abc import Sequence +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +from personality_protect.article_brief import ( + ARTICLE_MAX_BRIEF_OVERLAP, + ARTICLE_MAX_BRIEF_WORDS, + ARTICLE_MAX_COPY_RATIO, + ArticleBriefRejected, + mine_article_brief, +) +from personality_protect.chat_prompt import flatten_chat_messages +from personality_protect.config import ProfilePaths, load_config +from personality_protect.corpus_text import normalize_corpus_text +from personality_protect.draft_trim import drop_repeated_paragraphs, trim_draft +from personality_protect.eval_write_holdout import ( + TIE_EPSILON, + assert_receipt_contoso_safe, + load_holdout_pieces, + score_rag_vs_base, + verify_holdouts_never_indexed, + write_raw_artifacts, +) +from personality_protect.eval_writer_adapter import sign_test_p_value +from personality_protect.prompt_write import build_write_messages +from personality_protect.style_profile import ( + article_section_words, + article_word_aim, + load_style_profile, +) +from personality_protect.write import DEFAULT_WRITE_K, GenerateFn, mlx_generate_no_adapter +from personality_protect.write_article import ( + DEFAULT_ARTICLE_SECTION_MAX_TOKENS, + SECTION_TOKENS_PER_WORD, + SECTION_TRIM_HEADROOM, + _section_brief, + article_draft_ceiling, + count_indexed_article_pieces, + outline_from_brief, + run_write_article, + section_structure_directives, +) + +# One-sided bar, matching the writer ship gate. Stated rather than enforced: +# with fourteen articles a carve cannot exceed five, and a clean sweep of three +# only reaches p=0.125. The receipt reports the p-value so a run that cannot +# clear the bar is visibly a run that cannot clear the bar. +ARTICLE_ALPHA = 0.10 + + +def article_word_budget(paths: ProfilePaths, topic: str, points: str) -> dict[str, Any]: + """Outline and word budget both arms are held to for this brief. + + Derived from the same style profile ``run_write_article`` reads, so the + control arm is asked for exactly the article the product arm is building. + """ + style = load_style_profile(paths) + sections = outline_from_brief(topic, points) + section_words = article_section_words(style, sections=len(sections)) + return { + "sections": sections, + "section_count": len(sections), + "word_aim": article_word_aim(style), + "section_words": section_words, + "section_trim_words": int(round(section_words * SECTION_TRIM_HEADROOM)), + "word_ceiling": article_draft_ceiling(style, sections=len(sections)), + "max_tokens": max( + DEFAULT_ARTICLE_SECTION_MAX_TOKENS, + int(round(section_words * SECTION_TOKENS_PER_WORD)), + ), + } + + +def run_bare_base_article( + topic: str, + points: str, + *, + budget: dict[str, Any], + generate_fn: GenerateFn, + base_model: str, + prompt_sink: list[str] | None = None, +) -> dict[str, Any]: + """Write the same article with no exemplars and no cadence card. + + The post channel's control arm is a single-shot prompt, and that works + there because a post is what the model writes by default. Asked the same + way for an article it returned 58 to 150 words against holdouts of 600 to + 2,400, so the comparison was an article against a stub: the length penalty + decided three of four holdouts, and the stub could not be disqualified for + inventing entities because it had barely written any. + + The control therefore gets the same outline, the same per-section budget, + and the same trim. What it does not get is the voice machinery — retrieved + exemplars and the measured style profile — which is the only thing the + comparison is meant to be about. + """ + section_drafts: list[str] = [] + for index, section in enumerate(budget["sections"], start=1): + section_topic, section_points = _section_brief(topic, section, points) + messages = build_write_messages( + topic=section_topic, + points=section_points, + examples=(), + style_directives=section_structure_directives( + section=section, + index=index, + total=budget["section_count"], + word_aim=budget["word_aim"], + section_words=budget["section_words"], + section_trim_words=budget["section_trim_words"], + ), + ) + raw = str( + generate_fn( + messages, + base_model=base_model, + max_tokens=budget["max_tokens"], + prompt_sink=prompt_sink, + ) + ).strip() + section_drafts.append(trim_draft(raw, max_words=budget["section_trim_words"])) + + text = drop_repeated_paragraphs( + "\n\n".join(part for part in section_drafts if part.strip()) + ).strip() + return { + "text": text, + "mode": "bare_base_article", + "adapter": "none", + "model": base_model, + "k": 0, + "exemplar_ids": [], + "section_count": len(budget["sections"]), + "prompt": flatten_chat_messages(messages) if budget["sections"] else "", + } + + +def _item_receipt( + *, + holdout_id: str, + holdout_text: str, + brief_report: dict[str, Any], + budget: dict[str, int], + article_result: dict[str, Any], + base_result: dict[str, Any], + score: dict[str, Any], +) -> dict[str, Any]: + """Contoso-safe per-holdout row: ids, ratios, flags — never body text. + + ``score_rag_vs_base`` names its arms ``rag``/``base``; the article arm is + passed in the ``rag`` slot, so the labels are remapped once, here. + """ + article, base = score["rag"], score["base"] + return { + "holdout_id": holdout_id, + "holdout_words": len((holdout_text or "").split()), + "brief_words": brief_report["brief_words"], + "brief_bullets": brief_report["bullets"], + "brief_overlap_ratio": brief_report["brief_overlap_ratio"], + "brief_copy_ratio": brief_report["brief_copy_ratio"], + "section_count": budget["section_count"], + "word_ceiling": budget["word_ceiling"], + "winner": {"rag": "article", "base": "base"}.get(score["winner"], "tie"), + "delta_base_minus_article": score["delta_base_minus_rag"], + "article_distance": article["distance"], + "base_distance": base["distance"], + "article_disqualified": article["disqualified"], + "base_disqualified": base["disqualified"], + "article_parrot_reject": article["parrot_reject"], + "base_parrot_reject": base["parrot_reject"], + "article_brief_echo_reject": article["brief_echo_reject"], + "base_brief_echo_reject": base["brief_echo_reject"], + "article_invent_reject": article["invent_reject"], + "base_invent_reject": base["invent_reject"], + "article_invented_entities_count": article["invented_entities_count"], + "base_invented_entities_count": base["invented_entities_count"], + "article_invented_numbers_count": article["invented_numbers_count"], + "base_invented_numbers_count": base["invented_numbers_count"], + "article_draft_words": len(str(article_result.get("text") or "").split()), + "base_draft_words": len(str(base_result.get("text") or "").split()), + "article_attempts": int(article_result.get("attempts") or 1), + "exemplar_ids": list(article_result.get("exemplar_ids") or []), + "article_k": int(article_result.get("k") or 0), + } + + +def decide_article_voice( + wins: dict[str, int], + *, + article_disqualified: int, + base_disqualified: int, + alpha: float = ARTICLE_ALPHA, + both_disqualified: int = 0, + n_items: int = 0, +) -> dict[str, Any]: + """Whether this run supports the claim that the article channel has voice. + + Same three conditions as the writer ship gate: win the majority, do it by a + margin a fair coin would not produce this often, and do not fabricate more + than the arm being compared against. + + ``both_disqualified`` exists because those three conditions cannot tell two + different failures apart. An item where both arms are disqualified scores as + a tie, so a run where *every* item is disqualified reports the same + "did not win the majority" as a run where the voice arm was measured and + drifted further from the author. The first never got as far as comparing + cadence. Saying which one happened is the difference between a result and a + number that looks like one. + """ + article_wins = int(wins.get("article", 0)) + base_wins = int(wins.get("base", 0)) + p_value = sign_test_p_value(article_wins, base_wins) + reasons: list[str] = [] + undecided = n_items > 0 and both_disqualified >= n_items + if undecided: + reasons.append("every_item_disqualified_in_both_arms") + if article_wins <= base_wins: + reasons.append("article_did_not_win_majority") + if p_value > float(alpha): + reasons.append("margin_within_chance") + if article_disqualified > base_disqualified: + reasons.append("article_disqualified_more_often") + return { + "verdict": "voice_supported" if not reasons else "not_supported", + "article_beats_base": article_wins > base_wins, + "distance_ever_decided": not undecided, + "items_disqualified_in_both_arms": int(both_disqualified), + "p_value": p_value, + "alpha": float(alpha), + "blocking_reasons": reasons, + } + + +def run_eval_write_article( + paths: ProfilePaths, + holdout_ids: Sequence[str], + *, + k: int = DEFAULT_WRITE_K, + generate_fn: GenerateFn | None = None, + generate_fn_base: GenerateFn | None = None, + tie_epsilon: float = TIE_EPSILON, + alpha: float = ARTICLE_ALPHA, + save_raw: bool = False, + on_item: Any = None, +) -> dict[str, Any]: + """Draft each holdout article both ways and return a Contoso-safe receipt. + + ``save_raw`` dumps the exact prompts and drafts under the profile's + gitignored ``dogfood/raw`` directory. Those files are verbatim personal text + for human review; the receipt never references them. + """ + ids = [str(piece_id) for piece_id in holdout_ids] + carve = verify_holdouts_never_indexed(paths, ids) + if not carve["ok"]: + raise ValueError( + "Article holdout ids are present in voice_index (retrieval leak): " + + ", ".join(carve["indexed_holdout_ids"]) + ) + + config = load_config(paths) + generator = generate_fn or mlx_generate_no_adapter + base_generator = generate_fn_base or generator + pieces = load_holdout_pieces(paths, ids) + + items: list[dict[str, Any]] = [] + wins = {"article": 0, "base": 0, "tie": 0} + skipped: list[str] = [] + article_dq = 0 + base_dq = 0 + both_dq = 0 + + for piece in pieces: + holdout_text = normalize_corpus_text(piece.text) + try: + brief, brief_report = mine_article_brief(holdout_text, holdout_id=piece.id) + except (ArticleBriefRejected, ValueError): + # A holdout neither arm can be asked to write is not a loss for + # either of them. + skipped.append(piece.id) + continue + + budget = article_word_budget(paths, brief["topic"], brief["points"]) + article_prompts: list[str] = [] + base_prompts: list[str] = [] + article_result = run_write_article( + brief["topic"], + brief["points"], + paths, + k=k, + generate_fn=generator, + prompt_sink=article_prompts, + ) + base_result = run_bare_base_article( + brief["topic"], + brief["points"], + budget=budget, + generate_fn=base_generator, + base_model=config.base_model, + prompt_sink=base_prompts, + ) + score = score_rag_vs_base( + holdout_text, + article_result["text"], + base_result["text"], + brief["guard_facts"], + rag_exemplars=list(article_result.get("exemplar_texts") or []), + tie_epsilon=tie_epsilon, + # Invention is judged against the article; echo against the outline + # the model was handed. On a de-voiced brief those are different + # texts, and checking echo against the article would miss a draft + # that simply lists the bullets back. + visible_brief=f"{brief['topic']}\n{brief['points']}", + ) + item = _item_receipt( + holdout_id=piece.id, + holdout_text=holdout_text, + brief_report=brief_report, + budget=budget, + article_result=article_result, + base_result=base_result, + score=score, + ) + wins[item["winner"]] = wins.get(item["winner"], 0) + 1 + article_dq += int(bool(item["article_disqualified"])) + base_dq += int(bool(item["base_disqualified"])) + both_dq += int( + bool(item["article_disqualified"]) and bool(item["base_disqualified"]) + ) + items.append(item) + if on_item is not None: + on_item(item) + if save_raw: + for arm, result, sink in ( + ("article", article_result, article_prompts), + ("bare_base", base_result, base_prompts), + ): + write_raw_artifacts( + paths, + holdout_id=piece.id, + arm=arm, + prompt=sink[-1] if sink else str(result.get("prompt") or ""), + draft=str(result.get("text") or ""), + brief=brief, + ) + + verdict = decide_article_voice( + wins, + article_disqualified=article_dq, + base_disqualified=base_dq, + alpha=alpha, + both_disqualified=both_dq, + n_items=len(items), + ) + receipt: dict[str, Any] = { + "kind": "eval_write_article", + "created_at": datetime.now(timezone.utc).isoformat(), + "channel": "article", + "model": config.base_model, + "voice_mode": config.voice_mode, + "adapter": "none", + "k": k, + "n_holdouts": len(items), + "n_requested": len(ids), + "skipped_unbriefable": sorted(skipped), + "holdout_ids": [item["holdout_id"] for item in items], + "articles_indexed": count_indexed_article_pieces(paths), + "carve": carve, + "brief_mining": { + "hard_brief_word_cap": ARTICLE_MAX_BRIEF_WORDS, + "max_brief_overlap": ARTICLE_MAX_BRIEF_OVERLAP, + "max_brief_copy_ratio": ARTICLE_MAX_COPY_RATIO, + }, + "raw_artifacts_saved": bool(save_raw), + "wins": wins, + "disqualified": {"article": article_dq, "base": base_dq}, + **verdict, + "items": items, + } + assert_receipt_contoso_safe(receipt) + return receipt + + +def write_article_eval_receipt(receipt: dict[str, Any], path: Path) -> Path: + """Persist a Contoso-safe article eval receipt.""" + assert_receipt_contoso_safe(receipt) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps(receipt, indent=2, ensure_ascii=False) + "\n", encoding="utf-8" + ) + return path diff --git a/src/personality_protect/eval_write_holdout.py b/src/personality_protect/eval_write_holdout.py index ea1f73f..747e43d 100644 --- a/src/personality_protect/eval_write_holdout.py +++ b/src/personality_protect/eval_write_holdout.py @@ -89,9 +89,12 @@ } ) _URL_RE = re.compile(r"https?://\S+|www\.\S+", re.IGNORECASE) -_MIN_HOLDOUT_WORDS = math.ceil( - (_TOPIC_MIN_WORDS + _MIN_POINTS * _MIN_POINT_WORDS) / _MAX_BRIEF_OVERLAP -) +def min_briefable_words(max_overlap: float = _MAX_BRIEF_OVERLAP) -> int: + """Shortest source that can yield a topic and two bullets inside ``max_overlap``.""" + return math.ceil((_TOPIC_MIN_WORDS + _MIN_POINTS * _MIN_POINT_WORDS) / max_overlap) + + +_MIN_HOLDOUT_WORDS = min_briefable_words() # Raw prompts and drafts contain personal text. They live under the profile # directory (already gitignored, and outside the repo) and are never surfaced @@ -318,7 +321,12 @@ def mine_brief_from_holdout( raise ValueError("holdout text must not be empty") holdout_words = len(_word_tokens(body)) - if holdout_words < _MIN_HOLDOUT_WORDS: + # Derive the floor from the caller's budget instead of the module default. + # The two only diverge for a source that is already de-voiced, where a + # looser overlap cap is correct and a floor pinned to 25% would reject + # sources long enough to brief. + min_words = min_briefable_words(max_overlap) + if holdout_words < min_words: raise ValueError( f"holdout is {holdout_words} words — too short to brief without " f"handing back more than {max_overlap:.0%} of it; " @@ -500,6 +508,7 @@ def score_draft_against_holdout( draft: str, brief: str, exemplars: Sequence[str] = (), + visible_brief: str | None = None, ) -> dict[str, Any]: """g4: axis distance + guard flags (counts + keys; no draft body). @@ -507,6 +516,12 @@ def score_draft_against_holdout( rhythm, and an exemplar dump has perfect rhythm because it *is* the author's text — so distance alone once crowned a draft that was a copy of its own prompt. A disqualified draft cannot win, whatever its distance. + + ``brief`` is the allowed-facts set the invention guard checks against, which + on a de-voiced pair is the source piece rather than the prompt. Echo has to + be measured against what the model actually saw, so ``visible_brief`` may + carry the mined topic and bullets separately; it defaults to ``brief``, + which is correct wherever the two are the same text. """ ref_axes = text_axes(holdout_text) draft_axes = text_axes(draft) @@ -515,7 +530,8 @@ def score_draft_against_holdout( # Handing the mined bullets back is the other way to score a flattering # distance without writing anything, and it is what the winning draft did on # one holdout. - echoed = brief_echo_reject(draft, brief) + shown = brief if visible_brief is None else visible_brief + echoed = brief_echo_reject(draft, shown) return { "distance": _axes_distance(ref_axes, draft_axes), "axes": draft_axes, @@ -538,11 +554,16 @@ def score_rag_vs_base( *, rag_exemplars: Sequence[str] = (), tie_epsilon: float = TIE_EPSILON, + visible_brief: str | None = None, ) -> dict[str, Any]: """Three-way score: holdout reference vs RAG draft vs bare-base draft.""" holdout_axes = text_axes(holdout_text) - rag = score_draft_against_holdout(holdout_text, rag_draft, brief, rag_exemplars) - base = score_draft_against_holdout(holdout_text, base_draft, brief) + rag = score_draft_against_holdout( + holdout_text, rag_draft, brief, rag_exemplars, visible_brief=visible_brief + ) + base = score_draft_against_holdout( + holdout_text, base_draft, brief, visible_brief=visible_brief + ) delta = float(base["distance"]) - float(rag["distance"]) if rag["disqualified"] and base["disqualified"]: winner = "tie" diff --git a/src/personality_protect/eval_writer_adapter.py b/src/personality_protect/eval_writer_adapter.py new file mode 100644 index 0000000..0a6b4d1 --- /dev/null +++ b/src/personality_protect/eval_writer_adapter.py @@ -0,0 +1,255 @@ +"""Writer-LoRA ship gate: RAG+adapter vs RAG-alone on carved holdouts. + +The previous gate was an ad-hoc script, so its bar lived only in whoever ran it. +It is committed here because a ship decision that cannot be re-run is not a gate. + +Both arms retrieve the same exemplars and see the same de-voiced brief; the only +difference is whether the writer adapter is loaded. Scoring reuses the shipped +holdout scorer, including its disqualifications — a draft that invents entities +or parrots its context cannot win on rhythm, which is exactly how the first +adapter would otherwise have scored well while writing nothing. + +MLX is never imported here. The CLI injects generators that load each arm's +weights once and reuse them across holdouts; tests inject plain callables. +""" + +from __future__ import annotations + +import json +from collections.abc import Sequence +from datetime import datetime, timezone +from math import comb +from typing import Any + +from personality_protect.config import ProfilePaths, load_config +from personality_protect.corpus_text import normalize_corpus_text +from personality_protect.devoice import DevoiceRejected, mine_writer_brief +from personality_protect.eval_write_holdout import ( + TIE_EPSILON, + assert_receipt_contoso_safe, + load_holdout_pieces, + score_rag_vs_base, + verify_holdouts_never_indexed, +) +from personality_protect.write import ( + DEFAULT_WRITE_K, + DEFAULT_WRITE_MAX_TOKENS, + GenerateFn, + run_write, +) + +# One-sided significance required to keep an adapter. Deliberately lenient for a +# local voice model — this is a ship decision, not a paper — but it is a real +# threshold: at n=3 a clean sweep still only reaches p=0.125, which is why the +# previous run could not have passed a bar of any kind. +SHIP_ALPHA = 0.10 + + +def sign_test_p_value(wins: int, losses: int) -> float: + """One-sided probability of ``wins`` or better from a fair coin. + + Ties are excluded rather than split: a tie says the two arms were + indistinguishable on that holdout, which is evidence for neither. + """ + decisive = int(wins) + int(losses) + if decisive <= 0: + return 1.0 + tail = sum(comb(decisive, k) for k in range(int(wins), decisive + 1)) + return round(tail / (2**decisive), 4) + + +def decide_ship( + wins: dict[str, int], + *, + adapter_disqualified: int, + rag_disqualified: int, + alpha: float = SHIP_ALPHA, +) -> dict[str, Any]: + """Keep-or-archive decision plus every reason it was reached. + + Three conditions, all required: + + * the adapter wins more holdouts than it loses + * that margin is unlikely enough under a fair coin to be worth acting on + * the adapter is not disqualified more often than the arm it replaces — + invention and parroting were the real signal in the failed run, and an + adapter that wins on rhythm while fabricating more is not shippable + """ + adapter_wins = int(wins.get("adapter", 0)) + rag_wins = int(wins.get("rag", 0)) + p_value = sign_test_p_value(adapter_wins, rag_wins) + reasons: list[str] = [] + if adapter_wins <= rag_wins: + reasons.append("adapter_did_not_win_majority") + if p_value > float(alpha): + reasons.append("margin_within_chance") + if adapter_disqualified > rag_disqualified: + reasons.append("adapter_disqualified_more_often") + return { + "decision": "keep" if not reasons else "archive", + "adapter_beats_rag": adapter_wins > rag_wins, + "p_value": p_value, + "alpha": float(alpha), + "blocking_reasons": reasons, + } + + +def _item_receipt( + *, + holdout_id: str, + holdout_text: str, + brief_report: dict[str, Any], + adapter_result: dict[str, Any], + rag_result: dict[str, Any], + score: dict[str, Any], +) -> dict[str, Any]: + """Contoso-safe per-holdout row: ids, ratios and flags — never body text. + + ``score_rag_vs_base`` labels its arms ``rag``/``base``; the adapter arm is + passed in its ``rag`` slot, so the labels are remapped once, here, rather + than left for a reader of the receipt to untangle. + """ + adapter, rag = score["rag"], score["base"] + return { + "holdout_id": holdout_id, + "holdout_words": len((holdout_text or "").split()), + "brief_copy_ratio": brief_report.get("brief_copy_ratio"), + "note_copy_ratio": brief_report.get("copy_ratio"), + "winner": {"rag": "adapter", "base": "rag"}.get(score["winner"], "tie"), + "delta_rag_minus_adapter": score["delta_base_minus_rag"], + "adapter_distance": adapter["distance"], + "rag_distance": rag["distance"], + "adapter_disqualified": adapter["disqualified"], + "rag_disqualified": rag["disqualified"], + "adapter_parrot_reject": adapter["parrot_reject"], + "rag_parrot_reject": rag["parrot_reject"], + "adapter_invent_reject": adapter["invent_reject"], + "rag_invent_reject": rag["invent_reject"], + "adapter_brief_echo_reject": adapter["brief_echo_reject"], + "rag_brief_echo_reject": rag["brief_echo_reject"], + "adapter_invented_entities_count": adapter["invented_entities_count"], + "rag_invented_entities_count": rag["invented_entities_count"], + "adapter_draft_words": len(str(adapter_result.get("text") or "").split()), + "rag_draft_words": len(str(rag_result.get("text") or "").split()), + "exemplar_ids": list(rag_result.get("exemplar_ids") or []), + } + + +def run_writer_adapter_gate( + paths: ProfilePaths, + holdout_ids: Sequence[str], + *, + generate_fn_adapter: GenerateFn, + generate_fn_rag: GenerateFn, + k: int = DEFAULT_WRITE_K, + max_tokens: int = DEFAULT_WRITE_MAX_TOKENS, + tie_epsilon: float = TIE_EPSILON, + alpha: float = SHIP_ALPHA, + on_item: Any = None, +) -> dict[str, Any]: + """Score both arms on every holdout and return a Contoso-safe receipt. + + ``on_item`` is called with each finished row so a long unattended run can + report progress without the caller waiting for the whole gate. + """ + ids = [str(piece_id) for piece_id in holdout_ids] + carve = verify_holdouts_never_indexed(paths, ids) + if not carve["ok"]: + raise ValueError( + "Holdout ids are present in voice_index (retrieval leak): " + + ", ".join(carve["indexed_holdout_ids"]) + ) + + config = load_config(paths) + pieces = load_holdout_pieces(paths, ids) + items: list[dict[str, Any]] = [] + wins = {"adapter": 0, "rag": 0, "tie": 0} + skipped: list[str] = [] + adapter_dq = 0 + rag_dq = 0 + + for piece in pieces: + holdout_text = normalize_corpus_text(piece.text) + try: + brief, brief_report = mine_writer_brief(holdout_text, holdout_id=piece.id) + except (DevoiceRejected, ValueError): + # A holdout that cannot be briefed is not a loss for either arm. + skipped.append(piece.id) + continue + + adapter_result = run_write( + brief["topic"], + brief["points"], + paths, + k=k, + max_tokens=max_tokens, + use_adapter=True, + generate_fn=generate_fn_adapter, + ) + rag_result = run_write( + brief["topic"], + brief["points"], + paths, + k=k, + max_tokens=max_tokens, + use_adapter=False, + generate_fn=generate_fn_rag, + ) + score = score_rag_vs_base( + holdout_text, + adapter_result["text"], + rag_result["text"], + brief["guard_facts"], + rag_exemplars=list(adapter_result.get("exemplar_texts") or []), + tie_epsilon=tie_epsilon, + ) + item = _item_receipt( + holdout_id=piece.id, + holdout_text=holdout_text, + brief_report=brief_report, + adapter_result=adapter_result, + rag_result=rag_result, + score=score, + ) + wins[item["winner"]] = wins.get(item["winner"], 0) + 1 + adapter_dq += int(bool(item["adapter_disqualified"])) + rag_dq += int(bool(item["rag_disqualified"])) + items.append(item) + if on_item is not None: + on_item(item) + + verdict = decide_ship( + wins, + adapter_disqualified=adapter_dq, + rag_disqualified=rag_dq, + alpha=alpha, + ) + receipt: dict[str, Any] = { + "kind": "eval_writer_adapter_gate", + "created_at": datetime.now(timezone.utc).isoformat(), + "model": config.base_model, + "voice_mode": config.voice_mode, + "k": k, + "pair_kind": "devoiced_brief_to_post", + "n_holdouts": len(items), + "n_requested": len(ids), + "skipped_unbriefable": sorted(skipped), + "holdout_ids": [item["holdout_id"] for item in items], + "carve": carve, + "wins": wins, + "disqualified": {"adapter": adapter_dq, "rag": rag_dq}, + **verdict, + "items": items, + } + assert_receipt_contoso_safe(receipt) + return receipt + + +def write_gate_receipt(receipt: dict[str, Any], path: Any) -> Any: + """Persist a Contoso-safe gate receipt.""" + assert_receipt_contoso_safe(receipt) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps(receipt, indent=2, ensure_ascii=False) + "\n", encoding="utf-8" + ) + return path diff --git a/src/personality_protect/mlx_chunk_worker.py b/src/personality_protect/mlx_chunk_worker.py index 38bdb30..bd47ede 100644 --- a/src/personality_protect/mlx_chunk_worker.py +++ b/src/personality_protect/mlx_chunk_worker.py @@ -44,6 +44,7 @@ def main(argv: list[str] | None = None) -> int: parser.add_argument("--resume-adapter-file", default=None) parser.add_argument("--batch-size", type=int, default=1) parser.add_argument("--learning-rate", type=float, default=1e-5) + parser.add_argument("--lora-rank", type=int, default=8) args = parser.parse_args(argv) os.environ["TOKENIZERS_PARALLELISM"] = "true" @@ -79,10 +80,16 @@ def main(argv: list[str] | None = None) -> int: ns.mask_prompt = True ns.report_to = None ns.clear_cache_threshold = 2 * 10**9 # clear allocator if cache > 2 GB + # lora_parameters arrives from CONFIG_DEFAULTS as a shared dict; copy before + # overriding so a caller in the same process does not inherit the change. + lora_parameters = dict(getattr(ns, "lora_parameters", None) or {}) + lora_parameters["rank"] = max(1, args.lora_rank) + ns.lora_parameters = lora_parameters print( f"PP MLX chunk: iters={ns.iters} wired_cap_gb={args.wired_bytes / 1e9:.1f} " - f"max_seq={ns.max_seq_length} layers={ns.num_layers}", + f"max_seq={ns.max_seq_length} layers={ns.num_layers} " + f"rank={lora_parameters['rank']} lr={ns.learning_rate:g}", flush=True, ) run(ns) diff --git a/src/personality_protect/mlx_runtime.py b/src/personality_protect/mlx_runtime.py index b7163ad..e1585b5 100644 --- a/src/personality_protect/mlx_runtime.py +++ b/src/personality_protect/mlx_runtime.py @@ -138,7 +138,15 @@ def ensure_mlx_wired_cap(*, memory_gb: float | None = None) -> int: def release_mlx_memory() -> None: - """Best-effort Metal cache clear after filter/generate.""" + """Best-effort Metal cache clear after filter/generate. + + The opt-in check is not decoration: a Metal-less session aborts inside + ``metal::load_device`` via C++ ``terminate``, which ``except Exception`` + cannot catch. Without the guard a cleanup call in a sandboxed process takes + the interpreter down with a crash dialog instead of returning quietly. + """ + if not mlx_import_allowed(): + return try: import mlx.core as mx diff --git a/src/personality_protect/mlx_train.py b/src/personality_protect/mlx_train.py index 66f361f..f690ce0 100644 --- a/src/personality_protect/mlx_train.py +++ b/src/personality_protect/mlx_train.py @@ -30,6 +30,19 @@ # with a higher --memory-gb cap on 48 GB machines. DEFAULT_MAX_SEQ_LENGTH = 1024 DEFAULT_NUM_LAYERS = 8 +# mlx-lm's own defaults, named here so a caller can raise them per recipe +# instead of silently inheriting whatever the upstream config ships. +DEFAULT_LEARNING_RATE = 1e-5 +DEFAULT_LORA_RANK = 8 +# Writer recipe. The failed run used the translator recipe unchanged: 8 layers +# and rank 8 at 1e-5 for 300 steps over 100 rows. With pairs that now demand a +# real transformation rather than a copy, the adapter needs both more capacity +# and more passes over a smaller, cleaner set — and a slightly higher rate, +# because 1e-5 on rank 8 barely moves a 9B model in 300 steps. +WRITER_NUM_LAYERS = 16 +WRITER_LORA_RANK = 16 +WRITER_LEARNING_RATE = 3e-5 +WRITER_EPOCHS = 10 # Cap wired Metal memory: leave OS/apps breathing room. DEFAULT_WIRED_FRACTION = 0.40 DEFAULT_WIRED_CAP_BYTES = 16 * 10**9 # 16 GB hard cap (leave Studio headroom) @@ -260,8 +273,14 @@ def build_mlx_lora_argv( max_seq_length: int = DEFAULT_MAX_SEQ_LENGTH, num_layers: int = DEFAULT_NUM_LAYERS, batch_size: int = 1, + learning_rate: float = DEFAULT_LEARNING_RATE, ) -> list[str]: - """CLI argv for one mlx-lm LoRA chunk (memory-safe defaults).""" + """CLI argv for one mlx-lm LoRA chunk (memory-safe defaults). + + LoRA rank is absent on purpose: mlx-lm exposes it only through the + ``lora_parameters`` config, not as a CLI flag, so the chunk worker sets it + on the args namespace instead of here. + """ argv = [ "lora", "--model", @@ -292,7 +311,7 @@ def build_mlx_lora_argv( "--save-every", str(max(1, iters)), "--learning-rate", - "1e-5", + f"{learning_rate:g}", # Loss on assistant tokens only — rewrite SFT, not draft echo. "--mask-prompt", ] @@ -341,6 +360,8 @@ def run_mlx_chunk_subprocess( resume_adapter: Path | None = None, max_seq_length: int = DEFAULT_MAX_SEQ_LENGTH, num_layers: int = DEFAULT_NUM_LAYERS, + learning_rate: float = DEFAULT_LEARNING_RATE, + lora_rank: int = DEFAULT_LORA_RANK, on_line: Callable[[str], None] | None = None, timeout: int | None = None, ) -> ChunkResult: @@ -373,6 +394,10 @@ def run_mlx_chunk_subprocess( str(max_seq_length), "--num-layers", str(num_layers), + "--learning-rate", + f"{learning_rate:g}", + "--lora-rank", + str(max(1, lora_rank)), "--wired-bytes", str(int(wired_limit_bytes)), ] @@ -447,6 +472,8 @@ def run_chunked_mlx_train( memory_gb: float | None = None, max_seq_length: int = DEFAULT_MAX_SEQ_LENGTH, num_layers: int = DEFAULT_NUM_LAYERS, + learning_rate: float = DEFAULT_LEARNING_RATE, + lora_rank: int = DEFAULT_LORA_RANK, resume: bool = False, force_retrain: bool = False, progress_callback: ProgressCallback | None = None, @@ -487,6 +514,8 @@ def run_chunked_mlx_train( "peak_mem_gb": None, "max_seq_length": max_seq_length, "num_layers": num_layers, + "learning_rate": learning_rate, + "lora_rank": lora_rank, "adapter_file": str(adapter_dir / "adapters.safetensors"), "resume": plan.resume, "already_completed": plan.already_completed, @@ -574,6 +603,8 @@ def _on_line(line: str, *, _completed=completed, _n=n_iters) -> None: resume_adapter=resume_adapter, max_seq_length=max_seq_length, num_layers=num_layers, + learning_rate=learning_rate, + lora_rank=lora_rank, on_line=_on_line, ) if result.peak_mem_gb is not None: @@ -603,6 +634,8 @@ def _on_line(line: str, *, _completed=completed, _n=n_iters) -> None: "peak_mem_gb": max(peaks) if peaks else None, "max_seq_length": max_seq_length, "num_layers": num_layers, + "learning_rate": learning_rate, + "lora_rank": lora_rank, "adapter_file": str(adapter_file), "resume": plan.resume, "already_completed": plan.already_completed, @@ -675,6 +708,8 @@ def _on_line(line: str, *, _completed=completed, _n=n_iters) -> None: "peak_mem_gb": max(peaks) if peaks else None, "max_seq_length": max_seq_length, "num_layers": num_layers, + "learning_rate": learning_rate, + "lora_rank": lora_rank, "adapter_file": str(adapter_file), "resume": plan.resume, "already_completed": plan.already_completed, @@ -706,6 +741,8 @@ def _on_line(line: str, *, _completed=completed, _n=n_iters) -> None: "peak_mem_gb": max(peaks) if peaks else None, "max_seq_length": max_seq_length, "num_layers": num_layers, + "learning_rate": learning_rate, + "lora_rank": lora_rank, "adapter_file": str(adapter_file), "resume": plan.resume, "already_completed": plan.already_completed, diff --git a/src/personality_protect/style_profile.py b/src/personality_protect/style_profile.py index bc465d1..3a9bafd 100644 --- a/src/personality_protect/style_profile.py +++ b/src/personality_protect/style_profile.py @@ -235,6 +235,96 @@ def post_length_stats(pieces: Iterable[Piece]) -> dict[str, float]: } +# Articles are a different length regime, so their targets come from +# article-shaped pieces only. Falling back to the post band is what produced +# ~500-word "articles": the post ceiling is a LinkedIn character limit, not +# anything the author's longform does. +_ARTICLE_LENGTH_SOURCES = frozenset({"linkedin_article"}) +_MIN_ARTICLE_SAMPLE_WORDS = 150 +# Used only when the corpus has no article-shaped pieces to measure. +DEFAULT_ARTICLE_WORD_AIM = 1100 +ARTICLE_WORD_FLOOR = 600 +ARTICLE_WORD_CEILING = 3000 +# Section budgets. Below the floor a section is a paragraph, above the ceiling +# the model stops writing sections and writes one undifferentiated essay. +MIN_ARTICLE_SECTION_WORDS = 180 +MAX_ARTICLE_SECTION_WORDS = 600 +# Words a section of a longform piece typically carries, used to turn a total +# length into a plausible section count. +TYPICAL_ARTICLE_SECTION_WORDS = 300 +MIN_ARTICLE_SECTION_HINT = 2 +MAX_ARTICLE_SECTION_HINT = 8 + + +def article_length_stats(pieces: Iterable[Piece]) -> dict[str, float]: + """Word-length percentiles from article-shaped pieces only. + + Returns zeros when the corpus carries no articles rather than borrowing the + post band: an article target derived from posts is not a measurement of the + author's longform, and the callers below fall back to a stated default. + """ + articles = [ + p + for p in pieces + if p.source in _ARTICLE_LENGTH_SOURCES + and len(_word_tokens(p.text or "")) >= _MIN_ARTICLE_SAMPLE_WORDS + ] + lengths = [float(len(_word_tokens(p.text or ""))) for p in articles] + if not lengths: + return { + "median_article_words": 0.0, + "article_words_p75": 0.0, + "article_words_p90": 0.0, + "article_length_samples": 0.0, + } + return { + "median_article_words": round(_median(lengths), 1), + "article_words_p75": round(_percentile(lengths, 0.75), 1), + "article_words_p90": round(_percentile(lengths, 0.90), 1), + "article_length_samples": float(len(lengths)), + } + + +def article_word_aim(profile: dict[str, Any]) -> int: + """Typical finished article length, stated in the prompt. + + The median is the aim because a longform piece should read like the + author's usual article, not like their longest one. + """ + stats = profile.get("stats") or {} + for key in ("median_article_words", "article_words_p75", "article_words_p90"): + value = float(stats.get(key) or 0) + if value > 0: + return int(min(ARTICLE_WORD_CEILING, max(ARTICLE_WORD_FLOOR, round(value)))) + return DEFAULT_ARTICLE_WORD_AIM + + +def article_word_target(profile: dict[str, Any]) -> int: + """Hard word ceiling for a finished article (the author's long band).""" + stats = profile.get("stats") or {} + aim = article_word_aim(profile) + for key in ("article_words_p90", "article_words_p75", "median_article_words"): + value = float(stats.get(key) or 0) + if value > 0: + return int(min(ARTICLE_WORD_CEILING, max(aim, round(value)))) + return max(aim, DEFAULT_ARTICLE_WORD_AIM) + + +def article_section_count_hint(profile: dict[str, Any]) -> int: + """Sections an article of the author's typical length would carry.""" + sections = round(article_word_aim(profile) / TYPICAL_ARTICLE_SECTION_WORDS) + return max(MIN_ARTICLE_SECTION_HINT, min(MAX_ARTICLE_SECTION_HINT, int(sections))) + + +def article_section_words(profile: dict[str, Any], *, sections: int) -> int: + """Per-section word budget that adds up to the author's article length.""" + count = max(1, int(sections)) + per_section = round(article_word_aim(profile) / count) + return int( + min(MAX_ARTICLE_SECTION_WORDS, max(MIN_ARTICLE_SECTION_WORDS, per_section)) + ) + + def build_style_profile( pieces: Iterable[Piece], *, @@ -246,6 +336,7 @@ def build_style_profile( banned = list(banned_phrases) if banned_phrases is not None else list(BANNED_AI_FILLER) texts = [p.text for p in piece_list if (p.text or "").strip()] stats.update(post_length_stats(piece_list)) + stats.update(article_length_stats(piece_list)) stats.update(sentence_length_spread(texts)) stats["multi_sentence_paragraph_ratio"] = multi_sentence_paragraph_ratio(texts) return { @@ -284,14 +375,20 @@ def draft_word_aim(profile: dict[str, Any]) -> int: return min(DEFAULT_DRAFT_WORD_TARGET, draft_word_target(profile)) -def style_directives(profile: dict[str, Any]) -> list[str]: +def style_directives(profile: dict[str, Any], *, channel: str = "post") -> list[str]: """Render the style card as prompt directives. Derived numbers, not the author's sentences. The exemplar path hands the model copyable text and it copies; cadence targets carry the same voice signal with nothing to paste. + + ``channel='article'`` drops the post length directive. Cadence transfers + across channels; a word budget does not, and telling an article section to + stay under the LinkedIn post ceiling is how a longform draft came out post + length. The article path states its own budget per section. """ stats = profile.get("stats") or {} + is_article = (channel or "post").strip().lower() == "article" directives: list[str] = [] low = float(stats.get("sentence_words_p25") or 0) @@ -323,7 +420,7 @@ def style_directives(profile: dict[str, Any]) -> list[str]: ) median_post = float(stats.get("median_post_words") or 0) - if median_post: + if median_post and not is_article: directives.append( f"Target roughly {median_post:.0f} words total. Never exceed " f"{draft_word_target(profile)} words. Stop when the point is made." diff --git a/src/personality_protect/train.py b/src/personality_protect/train.py index 83876ad..8262693 100644 --- a/src/personality_protect/train.py +++ b/src/personality_protect/train.py @@ -24,8 +24,15 @@ ) from personality_protect.mlx_train import ( DEFAULT_CHUNK_STEPS, + DEFAULT_LEARNING_RATE, + DEFAULT_LORA_RANK, DEFAULT_MAX_SEQ_LENGTH, + DEFAULT_NUM_LAYERS, PROOF_MAX_STEPS, + WRITER_EPOCHS, + WRITER_LEARNING_RATE, + WRITER_LORA_RANK, + WRITER_NUM_LAYERS, ProgressCallback, run_chunked_mlx_train, ) @@ -60,7 +67,13 @@ class MockFallbackError(RuntimeError): """Raised when a real backend would silently degrade to mock.""" -def auto_max_steps(n_examples: int, *, smoke: bool = False, max_steps: int | None = None) -> int: +def auto_max_steps( + n_examples: int, + *, + smoke: bool = False, + max_steps: int | None = None, + epochs: int = DEFAULT_EPOCHS, +) -> int: """Resolve train steps: explicit override, smoke low-step, or auto from corpus size.""" if max_steps is not None and max_steps > 0: return max_steps @@ -68,7 +81,23 @@ def auto_max_steps(n_examples: int, *, smoke: bool = False, max_steps: int | Non return SMOKE_MAX_STEPS # ~epochs passes over the JSONL at batch size 1, clamped for tiny/huge corpora n = max(1, int(n_examples)) - return max(MIN_AUTO_STEPS, min(MAX_AUTO_STEPS, n * DEFAULT_EPOCHS)) + return max(MIN_AUTO_STEPS, min(MAX_AUTO_STEPS, n * max(1, int(epochs)))) + + +def writer_train_settings() -> dict[str, Any]: + """LoRA hyperparameters for the writer recipe. + + Separated from the translator defaults because the two tasks are not the + same size of change. Translation edits a draft it is already given; writing + a post from a note has to produce the whole text, which needs more adapted + layers and more rank than the 8/8 the first run inherited. + """ + return { + "num_layers": WRITER_NUM_LAYERS, + "lora_rank": WRITER_LORA_RANK, + "learning_rate": WRITER_LEARNING_RATE, + "epochs": WRITER_EPOCHS, + } def check_corpus_size(n_selected: int, *, force: bool = False, smoke: bool = False) -> str | None: @@ -239,10 +268,25 @@ def run_train( progress_callback: ProgressCallback | None = None, pairs: Path | None = None, writer: bool = False, + num_layers: int | None = None, + lora_rank: int | None = None, + learning_rate: float | None = None, ) -> TrainResult: config = load_config(paths) if writer and pairs is not None: raise ValueError("Pass only one of --writer or --pairs") + recipe = writer_train_settings() if writer else {} + resolved_layers = ( + num_layers if num_layers is not None else recipe.get("num_layers", DEFAULT_NUM_LAYERS) + ) + resolved_rank = ( + lora_rank if lora_rank is not None else recipe.get("lora_rank", DEFAULT_LORA_RANK) + ) + resolved_lr = ( + learning_rate + if learning_rate is not None + else recipe.get("learning_rate", DEFAULT_LEARNING_RATE) + ) voice_pair_mode = pairs is not None if voice_pair_mode: # Gated flatten→author pairs are the data floor; skip selected-piece gate. @@ -299,7 +343,12 @@ def run_train( done = completed_steps_from_meta(prior) if prior_total > done: max_steps = prior_total - steps = auto_max_steps(n, smoke=smoke or mock, max_steps=max_steps) + steps = auto_max_steps( + n, + smoke=smoke or mock, + max_steps=max_steps, + epochs=int(recipe.get("epochs", DEFAULT_EPOCHS)), + ) if sft_only: mode_note = ( @@ -361,6 +410,9 @@ def run_train( chunk_steps=chunk_steps, memory_gb=memory_gb, max_seq_length=max_seq_length, + num_layers=resolved_layers, + lora_rank=resolved_rank, + learning_rate=resolved_lr, progress_callback=progress_callback, proof=proof, resume=resume, @@ -493,6 +545,9 @@ def _train_mlx( chunk_steps: int = DEFAULT_CHUNK_STEPS, memory_gb: float | None = None, max_seq_length: int = DEFAULT_MAX_SEQ_LENGTH, + num_layers: int = DEFAULT_NUM_LAYERS, + lora_rank: int = DEFAULT_LORA_RANK, + learning_rate: float = DEFAULT_LEARNING_RATE, progress_callback: ProgressCallback | None = None, proof: bool = False, resume: bool = False, @@ -537,6 +592,9 @@ def _train_mlx( adapter_dir=adapter_dir, total_steps=max(1, max_steps), chunk_steps=chunk_steps, + num_layers=num_layers, + lora_rank=lora_rank, + learning_rate=learning_rate, memory_gb=memory_gb, max_seq_length=max_seq_length, resume=resume, diff --git a/src/personality_protect/write.py b/src/personality_protect/write.py index 7fc7edf..b4509d6 100644 --- a/src/personality_protect/write.py +++ b/src/personality_protect/write.py @@ -111,6 +111,74 @@ def mlx_generate_no_adapter( release_mlx_memory() +def make_mlx_generator( + *, + base_model: str, + adapter_path: str | None = None, +) -> GenerateFn: + """Load one arm's weights once and reuse them across every generation. + + :func:`mlx_generate_no_adapter` reloads the model on each call, which is + fine for a single draft and untenable for a gate: a widened holdout would + pay dozens of 9B loads, and the repeated allocate/free cycle is what invites + the wired-memory spikes the runtime cap exists to prevent. The returned + callable keeps the same signature so it drops into ``generate_fn``. + """ + from personality_protect.mlx_runtime import ( + assert_mlx_import_allowed, + ensure_mlx_wired_cap, + ) + + assert_mlx_import_allowed() + ensure_mlx_wired_cap(memory_gb=16.0) + from mlx_lm import generate, load + + model, tokenizer = load(base_model, adapter_path=adapter_path) + + def _generate( + messages: Sequence[Message], + *, + base_model: str = base_model, + max_tokens: int = DEFAULT_WRITE_MAX_TOKENS, + adapter_path: str | None = None, + prompt_sink: PromptSink | None = None, + ) -> str: + prompt = render_chat_prompt( + tokenizer, messages, fallback=flatten_chat_messages(messages) + ) + if prompt_sink is not None: + prompt_sink.append(prompt) + return str( + generate( + model, + tokenizer, + prompt=prompt, + max_tokens=max(64, int(max_tokens)), + verbose=False, + ) + ).strip() + + return _generate + + +def archive_writer_adapter(paths: ProfilePaths, *, reason: str) -> str | None: + """Move a rejected adapter aside so the write path resolves to none. + + Deleting would make a failed gate unauditable. The weights move to a + timestamped sibling directory that :func:`resolve_writer_adapter` does not + look in, which is what returns the product to ``adapter=none``. + """ + from datetime import datetime, timezone + + latest = paths.adapters_dir / "latest" + if not (latest / "adapters.safetensors").is_file(): + return None + stamp = datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%SZ") + target = paths.adapters_dir / f"writer-{reason}-{stamp}" + latest.rename(target) + return str(target) + + _SENTENCE_START_WORD_RE = re.compile( r"(?:(?<=\A)|(?<=[.!?:;]\s)|(?<=[.!?:;]\n)|(?<=\n))([A-Z][a-z][a-zA-Z'’-]*)" ) diff --git a/src/personality_protect/write_article.py b/src/personality_protect/write_article.py index dbfa3bc..9528ef3 100644 --- a/src/personality_protect/write_article.py +++ b/src/personality_protect/write_article.py @@ -7,23 +7,28 @@ from __future__ import annotations +import json import re from collections.abc import Sequence from typing import Any from personality_protect.chat_prompt import flatten_chat_messages from personality_protect.config import DEFAULT_MLX_MODEL, ProfilePaths, load_config -from personality_protect.draft_trim import trim_draft, word_count +from personality_protect.draft_trim import drop_repeated_paragraphs, trim_draft, word_count from personality_protect.models import load_index from personality_protect.prompt_write import build_write_messages from personality_protect.style_profile import ( - draft_word_target, + article_section_count_hint, + article_section_words, + article_word_aim, + article_word_target, load_style_profile, style_directives, ) -from personality_protect.voice_index import retrieve +from personality_protect.voice_index import VECTORS_FILENAME, retrieve from personality_protect.write import ( DEFAULT_WRITE_K, + MAX_EXEMPLAR_WORDS, GenerateFn, PromptSink, build_brief, @@ -41,9 +46,23 @@ # Below this the article channel has too little rhythm signal to claim voice. MIN_ARTICLE_CORPUS = 5 DEFAULT_ARTICLE_SECTION_MAX_TOKENS = 768 -DEFAULT_ARTICLE_SECTION_WORDS = 280 MAX_ARTICLE_SECTIONS = 8 MIN_ARTICLE_SECTIONS = 2 +# Headroom over the section budget before the tail trim bites. A section that +# lands slightly long is finished prose; one at double the budget is the model +# recycling itself, which is what the trim exists to cut. +SECTION_TRIM_HEADROOM = 1.35 +# Tokens per target word. Roughly 1.35 tokens/word for English plus room to +# finish the closing sentence before the budget runs out. +SECTION_TOKENS_PER_WORD = 2.0 +# Same clip as the post channel, and the article eval is why. A 60-word clip of +# a 1,000-word article shows little of its section rhythm, so this was widened +# to 120 on the theory that longform needs a longer look. The holdout run +# answered: at 120 the stitched draft shared 150+ exact 8-token windows with its +# own exemplars and was disqualified for parroting on three holdouts of four. +# Longform gives the model more room to copy, not less, so the clip stays short +# and voice travels as measured cadence instead. +ARTICLE_EXEMPLAR_WORDS = MAX_EXEMPLAR_WORDS _BULLET_RE = re.compile(r"^\s*[-*•]\s+") @@ -53,8 +72,30 @@ def count_article_pieces(paths: ProfilePaths) -> int: return sum(1 for piece in load_index(paths.index_path) if piece.source in ARTICLE_SOURCES) +def count_indexed_article_pieces(paths: ProfilePaths) -> int | None: + """Article exemplars retrieval can actually reach, or None with no index.""" + vectors_path = paths.root / "voice_index" / VECTORS_FILENAME + if not vectors_path.is_file(): + return None + total = 0 + with vectors_path.open(encoding="utf-8") as handle: + for line in handle: + if not line.strip(): + continue + piece = json.loads(line).get("piece") or {} + if str(piece.get("source") or "") in ARTICLE_SOURCES: + total += 1 + return total + + def assert_article_corpus(paths: ProfilePaths, *, minimum: int = MIN_ARTICLE_CORPUS) -> int: - """Raise when the article channel has too little source material.""" + """Raise when the article channel has too little source material. + + The corpus floor and the retrieval floor are checked separately. The floor + exists so drafts have rhythm to match, and rhythm arrives through + retrieval — a corpus of fifty articles with two of them indexed gives the + article channel nothing. + """ n = count_article_pieces(paths) if n < minimum: raise FileNotFoundError( @@ -62,9 +103,28 @@ def assert_article_corpus(paths: ProfilePaths, *, minimum: int = MIN_ARTICLE_COR f"in the corpus (found {n}). Ingest more articles, then rebuild " "index-voice." ) + indexed = count_indexed_article_pieces(paths) + if indexed is not None and indexed < minimum: + raise FileNotFoundError( + f"Article channel needs at least {minimum} linkedin_article pieces " + f"in the voice index (found {indexed} of {n} in the corpus). " + "Shrink the article holdout carve, then rebuild index-voice." + ) return n +def article_draft_ceiling(style: dict[str, Any], *, sections: int) -> int: + """Word ceiling a stitched article of ``sections`` can actually reach. + + The single-shot comparison arm has to be trimmed to the same number. Two + arms edited to different lengths would be separated by the length penalty + alone, whatever either of them wrote. + """ + count = max(1, int(sections)) + per_section = article_section_words(style, sections=count) + return int(round(per_section * SECTION_TRIM_HEADROOM)) * count + + def outline_from_brief(topic: str, points: str) -> list[str]: """Deterministic section titles from the brief (no model call). @@ -105,6 +165,31 @@ def _section_brief(topic: str, section: str, points: str) -> tuple[str, str]: ) +def section_structure_directives( + *, + section: str, + index: int, + total: int, + word_aim: int, + section_words: int, + section_trim_words: int, +) -> list[str]: + """Where this section sits and how long it runs. + + Structure, not voice. Kept separate from the cadence card so the eval's + control arm can be asked for an article of the same shape without also + being handed the measured style profile — otherwise the comparison would + only be establishing that asking for an article produces one. + """ + return [ + f"This is section {index} of {total} in a longform article of about " + f"{word_aim} words; the other sections cover the rest of the brief.", + f"Write only the section about: {section}", + f"Write about {section_words} words in this section, and no more " + f"than {section_trim_words}.", + ] + + def _guard_flags(brief: str, draft: str, exemplars: Sequence[str]) -> dict[str, Any]: invention = check_invention(brief, normalize_sentence_case(draft)) return { @@ -138,11 +223,15 @@ def run_write_article( sections = outline_from_brief(topic, points) config = load_config(paths) style = load_style_profile(paths) - directives = style_directives(style) - # Articles are longer than posts; do not clamp sections to the post ceiling. - section_words = max( - DEFAULT_ARTICLE_SECTION_WORDS, - int(draft_word_target(style)), + directives = style_directives(style, channel="article") + # Length comes from the author's own articles, split across the outline — + # not from the post band, whose ceiling is a LinkedIn character limit. + word_aim = article_word_aim(style) + word_ceiling = article_word_target(style) + section_words = article_section_words(style, sections=len(sections)) + section_trim_words = int(round(section_words * SECTION_TRIM_HEADROOM)) + section_max_tokens = max( + int(max_tokens), int(round(section_words * SECTION_TOKENS_PER_WORD)) ) generator = generate_fn or mlx_generate_no_adapter model_id = config.base_model or DEFAULT_MLX_MODEL @@ -162,7 +251,10 @@ def run_write_article( ) exemplars = [str(match["text"]) for match in matches] masked = [ - mask_exemplar_entities(clip_exemplar(exemplar), full_brief) for exemplar in exemplars + mask_exemplar_entities( + clip_exemplar(exemplar, max_words=ARTICLE_EXEMPLAR_WORDS), full_brief + ) + for exemplar in exemplars ] section_drafts: list[str] = [] @@ -184,8 +276,14 @@ def run_write_article( examples=masked, style_directives=[ *directives, - f"Write only the section about: {section}", - f"Aim for about {section_words} words in this section.", + *section_structure_directives( + section=section, + index=len(section_drafts) + 1, + total=len(sections), + word_aim=word_aim, + section_words=section_words, + section_trim_words=section_trim_words, + ), ], ) all_messages.append(messages) @@ -196,17 +294,22 @@ def run_write_article( generator( messages, base_model=model_id, - max_tokens=max_tokens, + max_tokens=section_max_tokens, prompt_sink=prompt_sink, ) ).strip() - draft = trim_draft(raw, max_words=section_words * 2) + draft = trim_draft(raw, max_words=section_trim_words) last_guards = _guard_flags(section_brief, draft, exemplars) if not last_guards["parrot_reject"] and not last_guards["invent_reject"]: break section_drafts.append(draft) - text = "\n\n".join(part for part in section_drafts if part.strip()).strip() + # Sections are generated independently from the same brief, so two of them + # can arrive as the same paragraph. Stitching them unfiltered is what turns + # a five-section article into the same point made five times. + text = drop_repeated_paragraphs( + "\n\n".join(part for part in section_drafts if part.strip()) + ).strip() # Final invent check against the full brief the author supplied. final_guards = _guard_flags(full_brief, text, exemplars) @@ -220,7 +323,11 @@ def run_write_article( "k": len(matches), "exemplar_ids": [str(match["id"]) for match in matches], "attempts": attempts_total, - "word_target": section_words * len(sections), + "word_target": word_ceiling, + "word_aim": word_aim, + "section_words": section_words, + "section_trim_words": section_trim_words, + "section_count_hint": article_section_count_hint(style), "article_count": article_count, "sections": sections, "section_count": len(sections), diff --git a/src/personality_protect/writer_guards.py b/src/personality_protect/writer_guards.py index 740ce94..e5cd34e 100644 --- a/src/personality_protect/writer_guards.py +++ b/src/personality_protect/writer_guards.py @@ -228,6 +228,11 @@ _NON_ENTITY_CAPS | _COMMON_WORDS | _CALENDAR_WORDS | _COMMON_ACRONYMS | _PROMPT_SCAFFOLD ) +# Public alias: the de-voicing operator needs the same "capitalized but not a +# name" vocabulary to decide which shouted words are emphasis it may lowercase +# and which are acronyms it must leave alone. +COMMON_CAPITALIZED = _COMMON_CAPITALIZED + @dataclass(frozen=True) class InventionResult: diff --git a/src/personality_protect/writer_holdout.py b/src/personality_protect/writer_holdout.py new file mode 100644 index 0000000..ba68d40 --- /dev/null +++ b/src/personality_protect/writer_holdout.py @@ -0,0 +1,157 @@ +"""Deterministic holdout carve for the writer LoRA ship gate. + +The first gate ran on three holdouts and came back 2–1 against the adapter. At +that size the result carries almost no information: three paired comparisons +cannot separate a real regression from a coin flip, so "did not clear the bar" +was the only honest reading, and "training did not help" was not available. + +Widening is therefore a precondition for the next gate, not a nice-to-have. The +carve is: + +* **deterministic** — a stable digest of the piece id orders candidates, so the + same corpus always yields the same holdout set and a gate can be re-run +* **pinned-compatible** — ids already carved out stay carved, so results remain + comparable across runs and no piece silently re-enters retrieval +* **briefable-only** — a piece that cannot produce a de-voiced brief cannot be + scored by either arm, so it would occupy a holdout slot and contribute nothing +""" + +from __future__ import annotations + +import hashlib +import json +from collections.abc import Iterable, Sequence +from datetime import datetime, timezone +from typing import Any + +from personality_protect.config import ProfilePaths +from personality_protect.corpus_text import normalize_corpus_text +from personality_protect.devoice import DevoiceRejected, mine_writer_brief +from personality_protect.models import Piece + +HOLDOUT_FILENAME = "dogfood_holdout_ids.json" +POST_SOURCES = frozenset({"linkedin_post"}) +MIN_HOLDOUT_TARGET_WORDS = 50 + +# Share of the briefable pool to reserve. A quarter is the smallest carve that +# gets a paired sign test into useful territory on a corpus this size while +# leaving enough rows to train on: at n=20 a 15-5 split is p≈0.02, where n=3 +# cannot go below p=0.125 even when the adapter sweeps. +DEFAULT_HOLDOUT_FRACTION = 0.25 +MIN_HOLDOUT_N = 12 +MAX_HOLDOUT_N = 24 + + +def _order_key(piece_id: str) -> str: + """Stable, corpus-order-independent shuffle key.""" + return hashlib.blake2b(str(piece_id).encode("utf-8"), digest_size=8).hexdigest() + + +def is_briefable(piece: Piece) -> bool: + """True when a de-voiced brief can be mined from this piece. + + Uses the same entry point as pair construction: a holdout the writer path + cannot brief is one neither arm can be asked to write, and scoring it would + mean scoring an empty prompt. + """ + if piece.source not in POST_SOURCES: + return False + body = normalize_corpus_text(piece.text or "") + if len(body.split()) < MIN_HOLDOUT_TARGET_WORDS: + return False + try: + mine_writer_brief(body, holdout_id=piece.id) + except (DevoiceRejected, ValueError): + return False + return True + + +def resolve_holdout_n( + pool_size: int, + *, + fraction: float = DEFAULT_HOLDOUT_FRACTION, + minimum: int = MIN_HOLDOUT_N, + maximum: int = MAX_HOLDOUT_N, +) -> int: + """Holdout size for a briefable pool, clamped to a usable band. + + Never returns more than the pool: a carve that consumes every briefable + piece would leave nothing to train on and the gate would compare two + untrained arms. + """ + target = round(max(0, pool_size) * max(0.0, fraction)) + return max(0, min(pool_size, max(minimum, min(maximum, int(target))))) + + +def select_writer_holdouts( + pieces: Iterable[Piece], + *, + pinned_ids: Sequence[str] = (), + fraction: float = DEFAULT_HOLDOUT_FRACTION, + minimum: int = MIN_HOLDOUT_N, + maximum: int = MAX_HOLDOUT_N, +) -> dict[str, Any]: + """Choose a widened holdout set and return a Contoso-safe receipt. + + Pinned ids are kept whether or not they are briefable — they are already out + of the retrieval index, and quietly re-admitting a previously carved piece + would contaminate the comparison with the earlier run. + """ + candidates = [piece for piece in pieces if piece.source in POST_SOURCES] + briefable = [piece.id for piece in candidates if is_briefable(piece)] + pinned = [str(piece_id) for piece_id in pinned_ids] + known = {piece.id for piece in candidates} + missing_pinned = sorted(set(pinned) - known) + + pool = sorted(set(briefable) | (set(pinned) & known)) + target_n = resolve_holdout_n( + len(pool), fraction=fraction, minimum=minimum, maximum=maximum + ) + + chosen: list[str] = [piece_id for piece_id in pinned if piece_id in known] + for piece_id in sorted(set(briefable) - set(chosen), key=_order_key): + if len(chosen) >= target_n: + break + chosen.append(piece_id) + + return { + "kind": "writer_holdout_carve", + "created_at": datetime.now(timezone.utc).isoformat(), + "holdout_ids": sorted(chosen), + "n_holdouts": len(chosen), + "n_posts": len(candidates), + "n_briefable": len(briefable), + "pool_size": len(pool), + "target_n": target_n, + "pinned_ids": sorted(set(pinned) & known), + "pinned_ids_missing_from_corpus": missing_pinned, + "train_pairs_remaining": max(0, len(briefable) - len(set(chosen) & set(briefable))), + "fraction": float(fraction), + "selection": "blake2b(piece_id) ascending, pinned ids first", + } + + +def load_pinned_holdout_ids(paths: ProfilePaths) -> list[str]: + """Ids from the profile's existing carve file (empty when absent).""" + path = paths.root / HOLDOUT_FILENAME + if not path.is_file(): + return [] + data = json.loads(path.read_text(encoding="utf-8")) + ids = data.get("holdout_ids") or data.get("ids") or [] if isinstance(data, dict) else data + return [str(piece_id) for piece_id in ids] + + +def save_holdout_ids(paths: ProfilePaths, receipt: dict[str, Any]) -> Any: + """Persist the carve. Ids and counts only — never piece text.""" + path = paths.root / HOLDOUT_FILENAME + payload = { + "holdout_ids": receipt["holdout_ids"], + "n_holdouts": receipt["n_holdouts"], + "n_briefable": receipt["n_briefable"], + "selection": receipt["selection"], + "updated_at": receipt["created_at"], + "note": "writer LoRA ship-gate carve; ids only", + } + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + return path diff --git a/src/personality_protect/writer_sft.py b/src/personality_protect/writer_sft.py index 40d5493..53d0a7a 100644 --- a/src/personality_protect/writer_sft.py +++ b/src/personality_protect/writer_sft.py @@ -1,4 +1,12 @@ -"""Brief→post SFT rows for the writer LoRA (not the translator path).""" +"""Brief→post SFT rows for the writer LoRA (not the translator path). + +Every row is ``(D(y), y)``: a de-voiced note in, the author's post out. The +first writer adapter was trained on rows whose brief was a verbatim extract of +its own target — median 5-gram copy ratio 1.0 — so the cheapest way to fit the +data was to echo the input, and the adapter did exactly that at generation time. +Pair construction is therefore gated, not merely built: a row that cannot be +moved far enough from its target is dropped rather than trained on. +""" from __future__ import annotations @@ -8,7 +16,12 @@ from typing import Any, Iterable from personality_protect.config import ProfilePaths -from personality_protect.eval_write_holdout import mine_brief_from_holdout +from personality_protect.corpus_text import normalize_corpus_text +from personality_protect.devoice import ( + MAX_PAIR_COPY_RATIO, + DevoiceRejected, + mine_writer_brief, +) from personality_protect.models import Piece, load_index from personality_protect.prompt_write import WRITE_SYSTEM_PROMPT, build_write_user_content from personality_protect.style_profile import load_style_profile, style_directives @@ -39,36 +52,68 @@ def piece_to_writer_example( piece: Piece, *, style_directives_list: list[str] | None = None, -) -> dict[str, Any] | None: - """One chat example: lossy brief → author's post as assistant target.""" - body = (piece.text or "").strip() - if len(body.split()) < _MIN_TARGET_WORDS: - return None + max_copy_ratio: float = MAX_PAIR_COPY_RATIO, +) -> tuple[dict[str, Any] | None, str]: + """One chat example: de-voiced brief → author's post as assistant target. + + Returns ``(row, reason)``. ``reason`` names why a piece was dropped so the + receipt can report *which* constraint pair construction is losing rows to, + rather than a single opaque skip count. + """ + body = normalize_corpus_text(piece.text or "") if piece.source not in _POST_SOURCES: - return None + return None, "not_a_post" + if len(body.split()) < _MIN_TARGET_WORDS: + return None, "too_short" try: - brief = mine_brief_from_holdout(body, holdout_id=piece.id) + brief, report = mine_writer_brief( + body, holdout_id=piece.id, max_copy_ratio=max_copy_ratio + ) + except DevoiceRejected as exc: + return None, exc.reasons[0] except ValueError: - return None + return None, "unbriefable" + user = build_write_user_content( topic=brief["topic"], points=brief["points"], examples=(), style_directives=style_directives_list or (), ) - return { - "messages": [ - {"role": "system", "content": WRITE_SYSTEM_PROMPT}, - {"role": "user", "content": user}, - {"role": "assistant", "content": body}, - ], - "meta": { - "piece_id": piece.id, - "source": piece.source, - "year": piece.year, - "word_count": len(body.split()), - "pair_kind": "writer", + return ( + { + "messages": [ + {"role": "system", "content": WRITE_SYSTEM_PROMPT}, + {"role": "user", "content": user}, + {"role": "assistant", "content": body}, + ], + "meta": { + "piece_id": piece.id, + "source": piece.source, + "year": piece.year, + "word_count": len(body.split()), + "pair_kind": "writer", + "devoiced": True, + # Per-row provenance for the pair audit. Ratios only — never the + # brief or the post body. + "brief_copy_ratio": report["brief_copy_ratio"], + "note_copy_ratio": report["copy_ratio"], + "brief_words": report["brief_words"], + }, }, + "kept", + ) + + +def _quantiles(values: list[float]) -> dict[str, float | None]: + """Median and p90 of a pair metric (empty-safe).""" + if not values: + return {"median": None, "p90": None, "max": None} + ordered = sorted(values) + return { + "median": round(ordered[len(ordered) // 2], 4), + "p90": round(ordered[min(len(ordered) - 1, int(0.9 * len(ordered)))], 4), + "max": round(ordered[-1], 4), } @@ -78,20 +123,23 @@ def build_writer_sft( *, holdout_ids: Iterable[str] = (), style_directives_list: list[str] | None = None, + max_copy_ratio: float = MAX_PAIR_COPY_RATIO, ) -> dict[str, Any]: - """Write writer SFT JSONL; skip holdouts and unbriefable posts.""" + """Write writer SFT JSONL; skip holdouts and pairs that stayed near ``(y, y)``.""" excluded = {str(piece_id) for piece_id in holdout_ids} rows: list[dict[str, Any]] = [] - skipped = 0 + dropped: dict[str, int] = {} for piece in pieces: if piece.id in excluded: - skipped += 1 + dropped["holdout"] = dropped.get("holdout", 0) + 1 continue - example = piece_to_writer_example( - piece, style_directives_list=style_directives_list + example, reason = piece_to_writer_example( + piece, + style_directives_list=style_directives_list, + max_copy_ratio=max_copy_ratio, ) if example is None: - skipped += 1 + dropped[reason] = dropped.get(reason, 0) + 1 continue rows.append(example) @@ -103,8 +151,18 @@ def build_writer_sft( return { "path": str(out_path), "examples": len(rows), - "skipped": skipped, + "skipped": sum(dropped.values()), + "dropped_by_reason": dict(sorted(dropped.items())), "holdouts_excluded": sorted(excluded), + "pair_kind": "devoiced_brief_to_post", + "max_copy_ratio": float(max_copy_ratio), + # The headline pair-quality numbers. Before de-voicing this sat at 1.0. + "brief_copy_ratio": _quantiles( + [float(row["meta"]["brief_copy_ratio"]) for row in rows] + ), + "note_copy_ratio": _quantiles( + [float(row["meta"]["note_copy_ratio"]) for row in rows] + ), "built_at": datetime.now(timezone.utc).isoformat(), } diff --git a/tests/contoso_articles.py b/tests/contoso_articles.py new file mode 100644 index 0000000..5c78c82 --- /dev/null +++ b/tests/contoso_articles.py @@ -0,0 +1,110 @@ +"""Contoso-safe longform fixtures for the article channel. + +Article code paths need sources that are actually article shaped: the brief +miner segments a piece into an outline, and the length targets are percentiles +over article word counts. A 40-word stub exercises neither, so these fixtures +are built to the length band the real corpus sits in. +""" + +from __future__ import annotations + +from personality_protect.models import Piece + +_SECTIONS: tuple[tuple[str, ...], ...] = ( + ( + "Contoso Ledger shipped a packaging change last spring and nobody could " + "say who owned it.", + "The pricing page listed nine tiers and the sales team quoted four of them.", + "You cannot run a pricing experiment when the price list is already a " + "guess.", + "Name one owner for the packaging before anyone writes a new tier.", + "That owner answers for the tier in writing and keeps the record where " + "the next team can read it.", + ), + ( + "The review took three weeks and removed two tiers nobody had bought.", + "Removing a tier is unglamorous work and it moved renewals more than the " + "roadmap did.", + "Boring tests are readable tests, and a readable test is the only kind " + "worth running twice.", + "Keep the experiment boring on purpose so the renewal signal stays legible.", + "A narrow question beats a wide deck in every quarter I have worked " + "through.", + ), + ( + "Exceptions are where a price list goes to die.", + "Contoso approved forty exceptions in one quarter and each of them was " + "reasonable on its own.", + "Together they meant the published price described almost nobody.", + "Cut exceptions by twelve percent before adding a discount program on top " + "of them.", + "Track the cut weekly, in one number, and put that number where the " + "packaging owner has to look at it.", + ), + ( + "Migrations fail on boundaries, not on code.", + "The Ledger team drew its service boundaries after the first cutover and " + "paid for it twice.", + "Draw the boundary first and write down what crosses it.", + "Keep the rollback plan small enough to explain on one page to somebody " + "who was not in the room.", + "If the rollback needs a diagram, the change is too large to ship this " + "week.", + ), + ( + "Operations queues improve when a team picks one metric and ignores the " + "rest for a week.", + "Queue length is usually the right one because everybody can see it " + "without a dashboard.", + "Bring the other metrics back only after the first one has moved twice.", + "Write down what changed between those two moves, or the next team " + "repeats the experiment.", + "The record is the deliverable; the queue is just where you noticed.", + ), + ( + "Every packaging change needs a person who answers for it in writing.", + "Contoso learned that after the second migration and forgot it before the " + "third.", + "Stop when customer value moves the wrong way and reopen the old package.", + "Reopening is cheap in the first month and expensive in the sixth.", + "Write down what Contoso learned before the next experiment starts, " + "because nobody remembers the reasoning by then.", + ), +) + + +def contoso_article_text(seed: int, *, sections: int = 4) -> str: + """Deterministic longform body around 300–400 words.""" + chosen = [_SECTIONS[(seed + offset) % len(_SECTIONS)] for offset in range(sections)] + return "\n\n".join("\n".join(block) for block in chosen) + + +def contoso_article(seed: int, *, sections: int = 4, year: int = 2024) -> Piece: + """One ``linkedin_article`` piece with a stable id.""" + return Piece( + id=f"contoso-article-{seed:02d}", + source="linkedin_article", + text=contoso_article_text(seed, sections=sections), + year=year, + ) + + +def contoso_articles(count: int, *, sections: int = 4) -> list[Piece]: + """``count`` distinct article pieces.""" + return [contoso_article(seed, sections=sections) for seed in range(count)] + + +def contoso_post(seed: int = 0) -> Piece: + """A short post so article-only paths can prove they filter by source.""" + return Piece( + id=f"contoso-post-{seed:02d}", + source="linkedin_post", + text=( + "Contoso keeps the queue boring.\n" + "\n" + "You name one owner before the packaging change starts.\n" + "\n" + "Write the result down before anybody opens a new tier." + ), + year=2024, + ) diff --git a/tests/test_article_brief.py b/tests/test_article_brief.py new file mode 100644 index 0000000..d6c60db --- /dev/null +++ b/tests/test_article_brief.py @@ -0,0 +1,90 @@ +"""Contoso-safe article brief mining: lossy outline, never an extract.""" + +from __future__ import annotations + +import pytest +from contoso_articles import contoso_article_text + +from personality_protect.article_brief import ( + ARTICLE_MAX_BRIEF_WORDS, + ARTICLE_MAX_COPY_RATIO, + ARTICLE_MIN_POINTS, + ArticleBriefRejected, + is_article_briefable, + mine_article_brief, + select_outline_clauses, +) +from personality_protect.eval_write_holdout import brief_word_overlap_ratio + + +def _brief(seed: int = 0): + return mine_article_brief(contoso_article_text(seed), holdout_id=f"contoso-{seed}") + + +def test_mined_brief_is_topic_plus_section_bullets(): + brief, report = _brief() + bullets = [line for line in brief["points"].splitlines() if line.strip()] + assert brief["topic"].strip() + assert len(bullets) >= ARTICLE_MIN_POINTS + assert all(line.startswith("- ") for line in bullets) + assert report["bullets"] == len(bullets) + + +def test_brief_stays_inside_the_hard_word_cap(): + _, report = _brief() + assert report["brief_words"] <= ARTICLE_MAX_BRIEF_WORDS + + +def test_brief_does_not_hand_back_the_article(): + brief, report = _brief() + body = contoso_article_text(0) + assert brief["points"].strip() != body.strip() + assert report["brief_overlap_ratio"] <= report["max_overlap"] + assert brief_word_overlap_ratio(brief, body) <= report["max_overlap"] + assert report["brief_copy_ratio"] <= ARTICLE_MAX_COPY_RATIO + + +def test_overlap_cap_binds_harder_as_the_article_grows(): + """A longer source must not buy a proportionally longer brief.""" + _, short_report = mine_article_brief(contoso_article_text(0, sections=3)) + _, long_report = mine_article_brief(contoso_article_text(0, sections=6)) + assert long_report["source_words"] > short_report["source_words"] + assert long_report["brief_overlap_ratio"] < short_report["brief_overlap_ratio"] + assert long_report["brief_words"] <= ARTICLE_MAX_BRIEF_WORDS + + +def test_guard_facts_keep_the_whole_article_out_of_the_prompt(): + brief, _ = _brief() + body = contoso_article_text(0) + assert brief["guard_facts"].strip().startswith(body.split("\n", 1)[0]) + assert body.split("\n", 1)[0] not in brief["points"] + + +def test_short_source_is_rejected_rather_than_briefed(): + with pytest.raises(ArticleBriefRejected, match="article_too_short"): + mine_article_brief("Contoso keeps the queue boring. You name one owner.") + + +def test_is_article_briefable_matches_mining(): + assert is_article_briefable(contoso_article_text(1)) + assert not is_article_briefable("Contoso ships. You own it.") + + +def test_outline_clauses_are_spread_across_the_piece(): + """Bullets come one per segment, not three from the densest paragraph.""" + candidates = [(index, f"clause {index}") for index in range(12)] + # Load the substance into one segment so a global ranking would cluster. + candidates[1] = (1, "Contoso Ledger cut twelve percent across four regions") + candidates[2] = (2, "Contoso Ledger cut fourteen percent across five regions") + candidates[3] = (3, "Contoso Ledger cut sixteen percent across six regions") + chosen = select_outline_clauses(candidates, points=4) + positions = [index for index, _ in chosen] + assert positions == sorted(positions) + assert max(positions) >= 8 + assert len(chosen) == 4 + + +def test_briefs_are_deterministic(): + first, _ = _brief(2) + second, _ = _brief(2) + assert first == second diff --git a/tests/test_article_holdout.py b/tests/test_article_holdout.py new file mode 100644 index 0000000..aae7966 --- /dev/null +++ b/tests/test_article_holdout.py @@ -0,0 +1,130 @@ +"""Contoso-safe article carve: deterministic, pinned, never below the floor.""" + +from __future__ import annotations + +import json +from pathlib import Path + +from contoso_articles import contoso_article, contoso_articles, contoso_post +from typer.testing import CliRunner + +from personality_protect.article_holdout import ( + MAX_ARTICLE_HOLDOUT_N, + MIN_ARTICLE_HOLDOUT_N, + load_pinned_article_holdout_ids, + resolve_article_holdout_n, + save_article_holdout_ids, + select_article_holdouts, +) +from personality_protect.cli import app +from personality_protect.config import init_profile +from personality_protect.models import save_index +from personality_protect.write_article import MIN_ARTICLE_CORPUS + +runner = CliRunner() + + +def test_carve_is_deterministic_and_article_only(): + pieces = [*contoso_articles(10), contoso_post()] + first = select_article_holdouts(pieces) + second = select_article_holdouts(list(reversed(pieces))) + assert first["holdout_ids"] == second["holdout_ids"] + assert first["n_articles"] == 10 + assert contoso_post().id not in first["holdout_ids"] + + +def test_carve_never_drops_retrieval_below_the_floor(): + receipt = select_article_holdouts(contoso_articles(7)) + assert receipt["articles_left_indexed"] >= MIN_ARTICLE_CORPUS + assert receipt["n_holdouts"] <= 7 - MIN_ARTICLE_CORPUS + + +def test_carve_is_empty_when_the_corpus_cannot_spare_an_article(): + receipt = select_article_holdouts(contoso_articles(MIN_ARTICLE_CORPUS)) + assert receipt["holdout_ids"] == [] + assert receipt["articles_left_indexed"] == MIN_ARTICLE_CORPUS + + +def test_carve_size_stays_inside_the_band(): + receipt = select_article_holdouts(contoso_articles(14)) + assert MIN_ARTICLE_HOLDOUT_N <= receipt["n_holdouts"] <= MAX_ARTICLE_HOLDOUT_N + + +def test_resolve_n_respects_the_retrieval_floor(): + assert resolve_article_holdout_n(9, total_articles=14) <= 14 - MIN_ARTICLE_CORPUS + assert resolve_article_holdout_n(9, total_articles=6, keep_indexed=5) == 1 + assert resolve_article_holdout_n(0, total_articles=14) == 0 + + +def test_pinned_ids_stay_carved(): + pieces = contoso_articles(12) + pinned = [pieces[-1].id] + receipt = select_article_holdouts(pieces, pinned_ids=pinned) + assert pinned[0] in receipt["holdout_ids"] + assert receipt["pinned_ids"] == pinned + + +def test_unbriefable_articles_are_not_carved(): + stub = contoso_article(0) + stub.text = "Contoso ships. You own it." + receipt = select_article_holdouts([stub, *contoso_articles(9)[1:], contoso_article(11)]) + assert stub.id not in receipt["holdout_ids"] + + +def test_save_and_load_round_trip_ids_only(tmp_path: Path): + paths, _, _ = init_profile("contoso", home=tmp_path) + receipt = select_article_holdouts(contoso_articles(12)) + written = save_article_holdout_ids(paths, receipt) + payload = json.loads(Path(written).read_text(encoding="utf-8")) + assert payload["holdout_ids"] == receipt["holdout_ids"] + assert "text" not in json.dumps(payload) + assert load_pinned_article_holdout_ids(paths) == receipt["holdout_ids"] + + +def test_cli_reports_before_it_applies(tmp_path: Path): + paths, _, _ = init_profile("contoso", home=tmp_path) + save_index(paths.index_path, [*contoso_articles(12), contoso_post()]) + + report = runner.invoke( + app, + ["select-article-holdouts", "--profile", "contoso", "--home", str(tmp_path), "--json"], + ) + assert report.exit_code == 0, report.output + assert not (paths.root / "article_holdout_ids.json").is_file() + + applied = runner.invoke( + app, + [ + "select-article-holdouts", + "--apply", + "--profile", + "contoso", + "--home", + str(tmp_path), + "--json", + ], + ) + assert applied.exit_code == 0, applied.output + assert load_pinned_article_holdout_ids(paths) == json.loads(report.stdout)["holdout_ids"] + + +def test_index_voice_from_carve_excludes_article_holdouts(tmp_path: Path): + paths, _, _ = init_profile("contoso", home=tmp_path) + save_index(paths.index_path, [*contoso_articles(12), contoso_post()]) + receipt = select_article_holdouts(contoso_articles(12)) + save_article_holdout_ids(paths, receipt) + + result = runner.invoke( + app, + [ + "index-voice", + "--from-carve", + "--profile", + "contoso", + "--home", + str(tmp_path), + "--json", + ], + ) + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["skipped_holdout"] == receipt["n_holdouts"] diff --git a/tests/test_detach.py b/tests/test_detach.py new file mode 100644 index 0000000..7440e95 --- /dev/null +++ b/tests/test_detach.py @@ -0,0 +1,81 @@ +"""Contoso-safe tests for the portable detached launcher. + +Regression: an unattended train launched through the shell's ``setsid`` never +started on macOS, because ``setsid`` is util-linux and is not installed there. +The launch has to detach without shelling out to anything. +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +from personality_protect.detach import ( + relaunch_self_detached, + spawn_detached, + timestamped_log_path, +) + + +class _RecordingPopen: + def __init__(self, argv, **kwargs): # noqa: ANN001, ANN003 + self.argv = argv + self.kwargs = kwargs + self.pid = 4242 + _RecordingPopen.last = self + + +def test_detached_launch_starts_its_own_session(tmp_path: Path): + """A new session is what survives a signal aimed at the caller's group.""" + result = spawn_detached( + ["echo", "hi"], log_path=tmp_path / "run.log", popen=_RecordingPopen + ) + assert _RecordingPopen.last.kwargs["start_new_session"] is True + assert result["pid"] == 4242 + + +def test_detached_launch_never_shells_out(tmp_path: Path): + """No shell means no dependency on a binary macOS does not ship.""" + spawn_detached(["echo", "hi"], log_path=tmp_path / "run.log", popen=_RecordingPopen) + kwargs = _RecordingPopen.last.kwargs + assert "shell" not in kwargs or kwargs["shell"] is False + assert _RecordingPopen.last.argv == ["echo", "hi"] + + +def test_detached_launch_closes_stdin_and_unbuffers_output(tmp_path: Path): + spawn_detached(["echo", "hi"], log_path=tmp_path / "run.log", popen=_RecordingPopen) + kwargs = _RecordingPopen.last.kwargs + assert kwargs["stdin"] is subprocess.DEVNULL + assert kwargs["env"]["PYTHONUNBUFFERED"] == "1" + + +def test_relaunch_uses_the_running_interpreter(tmp_path: Path, monkeypatch): + monkeypatch.setattr( + "personality_protect.detach.spawn_detached", + lambda argv, **kwargs: {"pid": 1, "log_path": "x", "argv": list(argv)}, + ) + result = relaunch_self_detached(["train", "--writer"], log_path=tmp_path / "l.log") + assert result["argv"][:3] == [sys.executable, "-m", "personality_protect.cli"] + assert result["argv"][3:] == ["train", "--writer"] + + +def test_detached_run_really_survives_as_a_separate_session(tmp_path: Path): + log = tmp_path / "real.log" + spawn_detached( + [sys.executable, "-c", "print('contoso ok')"], log_path=log, popen=subprocess.Popen + ) + for _ in range(200): + if log.read_text(encoding="utf-8").strip(): + break + import time + + time.sleep(0.02) + assert "contoso ok" in log.read_text(encoding="utf-8") + + +def test_log_path_is_timestamped_under_the_target_directory(tmp_path: Path): + path = timestamped_log_path(tmp_path / "dogfood", "train") + assert path.parent.is_dir() + assert path.name.startswith("train_") + assert path.suffix == ".log" diff --git a/tests/test_devoice.py b/tests/test_devoice.py new file mode 100644 index 0000000..c6c9fec --- /dev/null +++ b/tests/test_devoice.py @@ -0,0 +1,117 @@ +"""Contoso-safe tests for the de-voicing operator. + +The regression these lock down is the one that sank the first writer adapter: +SFT rows whose input was a verbatim extract of their own target. +""" + +from __future__ import annotations + +import pytest + +from personality_protect.devoice import ( + DevoiceRejected, + devoice_report, + devoice_sentences, + devoice_text, + mine_writer_brief, + pair_copy_ratio, +) +from personality_protect.eval_write_holdout import mine_brief_from_holdout +from personality_protect.writer_guards import ( + extract_entity_keys, + extract_named_entity_keys, +) + +CONTOSO_POST = ( + "Contoso Ledger keeps the reconciliation queue boring on purpose.\n\n" + "You ship the reconciliation on the day it lands, or you own the outage " + "that follows it.\n\n" + "And that is the whole point.\n\n" + "You name one owner before the packaging change starts, and you don't " + "pretend the roadmap is the work of the quarter.\n\n" + "Partners already know which one you picked this quarter — 40% of them " + "said so in the survey.\n\n" + "You cut the exceptions, or you explain every one of them in writing. " + "#boring @contoso\n\n" + "Northwind Traders tried the clever version of this and spent a year " + "rebuilding what they had already shipped once.\n\n" + "Boring beats clever every single time that Contoso ships Ledger.\n\n" + "You keep the ledger boring and the partners stay calm about it." +) + + +def test_devoice_strips_second_person_and_register(): + flat = devoice_text(CONTOSO_POST) + assert "you" not in flat.lower() + assert "don't" not in flat.lower() + assert "#boring" not in flat + assert "@contoso" not in flat + + +def test_devoice_keeps_entities_and_figures(): + flat = devoice_text(CONTOSO_POST) + assert "Contoso" in flat + assert "Ledger" in flat + assert "40%" in flat + + +def test_devoice_invents_no_entity(): + flat = devoice_text(CONTOSO_POST) + tokens = { + token + for key in extract_named_entity_keys(flat) + for token in key.split(" ") + if token + } + assert not tokens - extract_entity_keys(CONTOSO_POST) + + +def test_devoice_flattens_the_cadence_axes(): + report = devoice_report(CONTOSO_POST, devoice_text(CONTOSO_POST)) + # Author writes standalone short lines; the note is one unbroken block. + assert report["input_axes"]["short_line_ratio"] < report["output_axes"]["short_line_ratio"] + assert report["median_sentence_gap"] > 0 + assert report["input_axes"]["you_count"] == 0 + + +def test_devoice_drops_cadence_only_lines(): + clauses = devoice_sentences(CONTOSO_POST) + assert not any("whole point" in clause.lower() for clause in clauses) + + +def test_pair_copy_ratio_is_total_for_an_identity_pair(): + assert pair_copy_ratio(CONTOSO_POST, CONTOSO_POST) == 1.0 + + +def test_devoice_report_rejects_an_identity_pair(): + report = devoice_report(CONTOSO_POST, CONTOSO_POST) + assert not report["pass"] + assert "pair_copy_ratio" in report["failed"] + + +def test_mined_writer_brief_is_not_an_extract_of_the_post(): + """The headline fix: the trained input no longer sits inside its target.""" + devoiced_brief, report = mine_writer_brief(CONTOSO_POST, holdout_id="c1") + verbatim = mine_brief_from_holdout(CONTOSO_POST, holdout_id="c1") + + devoiced_ratio = pair_copy_ratio( + f"{devoiced_brief['topic']}\n{devoiced_brief['points']}", CONTOSO_POST + ) + verbatim_ratio = pair_copy_ratio( + f"{verbatim['topic']}\n{verbatim['points']}", CONTOSO_POST + ) + assert verbatim_ratio > 0.9, "shipped mining hands the post back nearly whole" + assert devoiced_ratio <= report["max_copy_ratio"] + assert devoiced_ratio < verbatim_ratio + + +def test_mined_writer_brief_guards_against_the_post_not_the_note(): + brief, _ = mine_writer_brief(CONTOSO_POST, holdout_id="c1") + # Invention is judged against what the author actually wrote, so a figure + # dropped by the operator must not become "invented" in a draft. + assert "40%" in brief["guard_facts"] + + +def test_mine_writer_brief_rejects_a_pair_it_cannot_move(): + with pytest.raises(DevoiceRejected): + mine_writer_brief("Ledger. Ledger. Ledger. Ledger.", holdout_id="c2") diff --git a/tests/test_eval_write_article.py b/tests/test_eval_write_article.py new file mode 100644 index 0000000..971f65d --- /dev/null +++ b/tests/test_eval_write_article.py @@ -0,0 +1,344 @@ +"""Contoso-safe article eval: carve → outline brief → two arms → receipt.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest +from contoso_articles import contoso_articles, contoso_post +from typer.testing import CliRunner + +from personality_protect.article_holdout import ( + save_article_holdout_ids, + select_article_holdouts, +) +from personality_protect.cli import app +from personality_protect.config import init_profile +from personality_protect.eval_write_article import ( + ARTICLE_ALPHA, + article_word_budget, + decide_article_voice, + run_bare_base_article, + run_eval_write_article, + write_article_eval_receipt, +) +from personality_protect.eval_write_holdout import assert_receipt_contoso_safe, raw_artifacts_dir +from personality_protect.models import save_index +from personality_protect.style_profile import build_style_profile, save_style_profile +from personality_protect.voice_index import build_voice_index + +runner = CliRunner() + +N_ARTICLES = 12 + +ARTICLE_DRAFT = ( + "Contoso Ledger names one owner before anybody opens a new tier.\n" + "\n" + "The owner answers for the packaging in writing and keeps the record where " + "the next team can find it without asking.\n" + "\n" + "Exceptions are where a published price list quietly stops describing " + "anyone at all.\n" + "\n" + "Cut them weekly, in one number, and put that number in front of whoever " + "owns the packaging." +) +BASE_DRAFT = ( + "In today's fast-paced landscape, organizations must leverage robust " + "packaging frameworks to unlock synergies across the pricing lifecycle. " + "Furthermore, a paradigm of continuous exception governance is a testament " + "to operational excellence and stakeholder alignment throughout the " + "enterprise value chain." +) + + +def _seed(tmp_path: Path, *, holdouts: list[str] | None = None) -> tuple: + paths, _, _ = init_profile("contoso", home=tmp_path) + pieces = [*contoso_articles(N_ARTICLES), contoso_post()] + save_index(paths.index_path, pieces) + carved = set(holdouts or []) + build_voice_index(paths, holdout_ids=carved) + save_style_profile(paths, build_style_profile(pieces)) + return paths, pieces + + +def _carve(tmp_path: Path) -> tuple: + paths, pieces = _seed(tmp_path) + receipt = select_article_holdouts(pieces) + paths, _ = _seed(tmp_path, holdouts=receipt["holdout_ids"]) + save_article_holdout_ids(paths, receipt) + return paths, receipt["holdout_ids"] + + +def _arms(article: str = ARTICLE_DRAFT, base: str = BASE_DRAFT): + """Two generators that answer with fixed drafts, per arm.""" + return (lambda _m, **_k: article), (lambda _m, **_k: base) + + +def test_word_budget_is_shared_by_both_arms(tmp_path: Path): + paths, _ = _seed(tmp_path) + budget = article_word_budget(paths, "Contoso packaging", "- One\n- Two\n- Three") + assert budget["section_count"] == 3 + assert budget["sections"] == ["One", "Two", "Three"] + assert budget["word_ceiling"] == budget["section_trim_words"] * 3 + assert budget["section_trim_words"] > budget["section_words"] + assert budget["max_tokens"] >= budget["section_trim_words"] + + +def test_control_arm_writes_an_article_without_the_voice_machinery(tmp_path: Path): + """The control must be a bare article, not a bare post.""" + paths, _ = _seed(tmp_path) + budget = article_word_budget(paths, "Contoso packaging", "- One\n- Two\n- Three") + seen: list[str] = [] + + def fake_generate(messages, **_kwargs: object) -> str: + seen.append(messages[1]["content"]) + return f"Contoso Ledger paragraph for section {len(seen)} of the piece." + + result = run_bare_base_article( + "Contoso packaging", + "- One\n- Two\n- Three", + budget=budget, + generate_fn=fake_generate, + base_model="contoso-local", + ) + assert result["mode"] == "bare_base_article" + assert result["section_count"] == 3 + assert len(seen) == 3 + # Same structure as the product arm... + assert "section 1 of 3" in seen[0] + assert f"Write about {budget['section_words']} words" in seen[0] + # ...and none of the voice machinery. + assert "EXAMPLES" not in seen[0] + assert "Sentence length varies" not in seen[0] + assert "Never use these words" not in seen[0] + + +def test_both_arms_see_the_same_outline_and_budget(tmp_path: Path): + paths, holdouts = _carve(tmp_path) + article_prompts: list[str] = [] + base_prompts: list[str] = [] + + def article_fn(messages, **_kwargs: object) -> str: + article_prompts.append(messages[1]["content"]) + return ARTICLE_DRAFT + + def base_fn(messages, **_kwargs: object) -> str: + base_prompts.append(messages[1]["content"]) + return BASE_DRAFT + + receipt = run_eval_write_article( + paths, holdouts[:1], k=1, generate_fn=article_fn, generate_fn_base=base_fn + ) + assert len(base_prompts) == receipt["items"][0]["section_count"] + assert "Never use these words" in article_prompts[0] + assert "Never use these words" not in base_prompts[0] + assert "EXAMPLES" not in base_prompts[0] + for line in ("section 1 of", "Write about", "Write only the section about:"): + assert line in article_prompts[0] + assert line in base_prompts[0] + + +def test_eval_scores_every_holdout_and_reports_a_verdict(tmp_path: Path): + paths, holdouts = _carve(tmp_path) + article_fn, base_fn = _arms() + + receipt = run_eval_write_article( + paths, holdouts, k=2, generate_fn=article_fn, generate_fn_base=base_fn + ) + assert receipt["kind"] == "eval_write_article" + assert receipt["channel"] == "article" + assert receipt["n_holdouts"] == len(holdouts) + assert receipt["carve"]["ok"] + assert sum(receipt["wins"].values()) == len(holdouts) + assert receipt["verdict"] in {"voice_supported", "not_supported"} + assert receipt["alpha"] == ARTICLE_ALPHA + + +def test_receipt_never_carries_draft_or_holdout_bodies(tmp_path: Path): + paths, holdouts = _carve(tmp_path) + article_fn, base_fn = _arms() + receipt = run_eval_write_article( + paths, holdouts, k=1, generate_fn=article_fn, generate_fn_base=base_fn + ) + assert_receipt_contoso_safe(receipt) + blob = json.dumps(receipt) + assert "Contoso Ledger names one owner" not in blob + assert "leverage" not in blob + + +def test_eval_refuses_to_run_when_a_holdout_is_indexed(tmp_path: Path): + paths, pieces = _seed(tmp_path) + leaked = pieces[0].id + article_fn, base_fn = _arms() + with pytest.raises(ValueError, match="retrieval leak"): + run_eval_write_article( + paths, [leaked], generate_fn=article_fn, generate_fn_base=base_fn + ) + + +def test_unbriefable_holdouts_are_skipped_not_scored(tmp_path: Path): + paths, pieces = _seed(tmp_path) + stub = pieces[0] + stub.text = "Contoso ships. You own it." + save_index(paths.index_path, pieces) + build_voice_index(paths, holdout_ids={stub.id}) + article_fn, base_fn = _arms() + + receipt = run_eval_write_article( + paths, [stub.id], generate_fn=article_fn, generate_fn_base=base_fn + ) + assert receipt["skipped_unbriefable"] == [stub.id] + assert receipt["n_holdouts"] == 0 + assert receipt["items"] == [] + + +def test_a_draft_that_hands_the_brief_back_cannot_win(tmp_path: Path): + """Echoing the mined bullets scores flattering rhythm and writes nothing.""" + paths, holdouts = _carve(tmp_path) + from personality_protect.article_brief import mine_article_brief + from personality_protect.corpus_text import normalize_corpus_text + from personality_protect.models import load_index + + by_id = {piece.id: piece for piece in load_index(paths.index_path)} + brief, _ = mine_article_brief(normalize_corpus_text(by_id[holdouts[0]].text)) + echo = f"{brief['topic']}\n\n{brief['points']}" + + receipt = run_eval_write_article( + paths, + [holdouts[0]], + k=1, + generate_fn=lambda _m, **_k: echo, + generate_fn_base=lambda _m, **_k: BASE_DRAFT, + ) + item = receipt["items"][0] + assert item["article_disqualified"] + assert item["winner"] != "article" + + +def test_invented_entities_disqualify_the_article_arm(tmp_path: Path): + paths, holdouts = _carve(tmp_path) + invented = ( + "Fabrikam Northwind shipped the Tailspin Toys packaging with Wingtip " + "Partners and Adventure Works oversight across nine regions.\n" + "\n" + "Litware Proseware also confirmed the Woodgrove Bank exception " + "programme before the review closed." + ) + receipt = run_eval_write_article( + paths, + [holdouts[0]], + k=1, + generate_fn=lambda _m, **_k: invented, + generate_fn_base=lambda _m, **_k: BASE_DRAFT, + ) + item = receipt["items"][0] + assert item["article_invented_entities_count"] > 0 + assert item["article_disqualified"] + + +def test_both_arms_are_held_to_the_same_length_ceiling(tmp_path: Path): + """A trim applied to one side only would decide the comparison by itself.""" + paths, holdouts = _carve(tmp_path) + runaway = "\n\n".join( + f"Contoso Ledger paragraph {n} names one owner and cuts one exception." + for n in range(400) + ) + receipt = run_eval_write_article( + paths, + [holdouts[0]], + k=1, + generate_fn=lambda _m, **_k: runaway, + generate_fn_base=lambda _m, **_k: runaway, + ) + item = receipt["items"][0] + assert item["article_draft_words"] <= item["word_ceiling"] + assert item["base_draft_words"] <= item["word_ceiling"] + + +def test_verdict_requires_a_margin_a_coin_would_not_produce(): + swept = decide_article_voice( + {"article": 4, "base": 0, "tie": 0}, article_disqualified=0, base_disqualified=0 + ) + assert swept["verdict"] == "voice_supported" + + narrow = decide_article_voice( + {"article": 2, "base": 1, "tie": 0}, article_disqualified=0, base_disqualified=0 + ) + assert narrow["verdict"] == "not_supported" + assert "margin_within_chance" in narrow["blocking_reasons"] + + fabricating = decide_article_voice( + {"article": 4, "base": 0, "tie": 0}, article_disqualified=2, base_disqualified=0 + ) + assert fabricating["verdict"] == "not_supported" + assert "article_disqualified_more_often" in fabricating["blocking_reasons"] + + +def test_a_run_that_never_compared_cadence_says_so(): + """Losing on distance and never reaching distance are different failures. + + Both arms disqualified scores a tie, so an all-disqualified run and a run + the voice arm genuinely lost both report zero wins. Only one of them is + evidence about voice, and the receipt has to name which. + """ + stalled = decide_article_voice( + {"article": 0, "base": 0, "tie": 4}, + article_disqualified=4, + base_disqualified=4, + both_disqualified=4, + n_items=4, + ) + assert stalled["verdict"] == "not_supported" + assert stalled["distance_ever_decided"] is False + assert "every_item_disqualified_in_both_arms" in stalled["blocking_reasons"] + + measured = decide_article_voice( + {"article": 1, "base": 3, "tie": 0}, + article_disqualified=0, + base_disqualified=0, + both_disqualified=0, + n_items=4, + ) + assert measured["verdict"] == "not_supported" + assert measured["distance_ever_decided"] is True + assert "every_item_disqualified_in_both_arms" not in measured["blocking_reasons"] + + +def test_raw_artifacts_stay_out_of_the_receipt(tmp_path: Path): + paths, holdouts = _carve(tmp_path) + article_fn, base_fn = _arms() + receipt = run_eval_write_article( + paths, + holdouts[:1], + k=1, + generate_fn=article_fn, + generate_fn_base=base_fn, + save_raw=True, + ) + written = sorted(path.name for path in raw_artifacts_dir(paths).glob("*")) + assert any(name.endswith(".article.draft.txt") for name in written) + assert any(name.endswith(".bare_base.draft.txt") for name in written) + assert str(raw_artifacts_dir(paths)) not in json.dumps(receipt) + + +def test_receipt_file_round_trips(tmp_path: Path): + paths, holdouts = _carve(tmp_path) + article_fn, base_fn = _arms() + receipt = run_eval_write_article( + paths, holdouts[:1], k=1, generate_fn=article_fn, generate_fn_base=base_fn + ) + out = tmp_path / "evals" / "article.json" + write_article_eval_receipt(receipt, out) + assert json.loads(out.read_text(encoding="utf-8"))["kind"] == "eval_write_article" + + +def test_cli_requires_a_carve_before_it_will_run(tmp_path: Path): + _seed(tmp_path) + result = runner.invoke( + app, + ["eval-write-article", "--profile", "contoso", "--home", str(tmp_path)], + ) + assert result.exit_code == 1 + assert "select-article-holdouts" in result.output diff --git a/tests/test_eval_writer_adapter.py b/tests/test_eval_writer_adapter.py new file mode 100644 index 0000000..d47b28b --- /dev/null +++ b/tests/test_eval_writer_adapter.py @@ -0,0 +1,164 @@ +"""Contoso-safe tests for the writer-LoRA ship gate.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from personality_protect.config import init_profile +from personality_protect.eval_write_holdout import assert_receipt_contoso_safe +from personality_protect.eval_writer_adapter import ( + decide_ship, + run_writer_adapter_gate, + sign_test_p_value, +) +from personality_protect.models import Piece, save_index +from personality_protect.style_profile import build_style_profile, save_style_profile + +CONTOSO_POST = ( + "Contoso Ledger keeps the reconciliation queue boring on purpose.\n\n" + "You ship the reconciliation on the day it lands, or you own the outage " + "that follows it.\n\n" + "You name one owner before the packaging change starts, and you don't " + "pretend the roadmap is the work of the quarter.\n\n" + "Partners already know which one you picked this quarter — 40% of them " + "said so in the survey.\n\n" + "Northwind Traders tried the clever version of this and spent a year " + "rebuilding what they had already shipped once.\n\n" + "Boring beats clever every single time that Contoso ships Ledger.\n\n" + "You keep the ledger boring and the partners stay calm about it." +) + +ADAPTER_DRAFT = ( + "The queue stays dull because someone decided it should.\n\n" + "Name the owner first.\n\n" + "Ship on the day, or answer for the night that follows.\n\n" + "Everyone downstream already knows which choice got made.\n\n" + "Dull wins. Every quarter, without exception, it wins again." +) + +RAG_DRAFT = ( + "In today's rapidly evolving operational landscape, organizations must " + "carefully consider the strategic implications of their reconciliation " + "processes, ensuring that ownership is clearly delineated across all " + "relevant stakeholders and that exceptions are documented thoroughly " + "before any packaging modification is permitted to proceed through the " + "established review pipeline." +) + + +def _profile(tmp_path: Path): + paths, _, _ = init_profile("contoso", home=tmp_path) + pieces = [ + Piece(id="hold1", source="linkedin_post", text=CONTOSO_POST, year=2024), + Piece( + id="hold2", + source="linkedin_post", + text=CONTOSO_POST + + "\n\nThe second release of the ledger shipped on the same day " + "that the review closed, and nobody had to stay late for it.", + year=2024, + ), + ] + save_index(paths.index_path, pieces) + save_style_profile(paths, build_style_profile(pieces)) + adapter = paths.adapters_dir / "latest" + adapter.mkdir(parents=True, exist_ok=True) + (adapter / "adapters.safetensors").write_text("stub", encoding="utf-8") + return paths + + +def _fixed(text: str): + def _generate(messages, **kwargs): # noqa: ANN001, ANN003 + return text + + return _generate + + +def test_sign_test_matches_the_binomial_tail(): + assert sign_test_p_value(3, 0) == 0.125 # why n=3 could never clear a bar + assert sign_test_p_value(0, 0) == 1.0 + assert sign_test_p_value(15, 5) == pytest.approx(0.0207, abs=1e-3) + assert sign_test_p_value(10, 10) == pytest.approx(0.588, abs=1e-3) + + +def test_decide_ship_requires_a_margin_beyond_chance(): + verdict = decide_ship( + {"adapter": 2, "rag": 1, "tie": 0}, adapter_disqualified=0, rag_disqualified=0 + ) + assert verdict["decision"] == "archive" + assert verdict["blocking_reasons"] == ["margin_within_chance"] + + +def test_decide_ship_keeps_a_clear_win(): + verdict = decide_ship( + {"adapter": 15, "rag": 5, "tie": 0}, adapter_disqualified=1, rag_disqualified=2 + ) + assert verdict["decision"] == "keep" + assert verdict["blocking_reasons"] == [] + + +def test_decide_ship_blocks_an_adapter_that_fabricates_more(): + verdict = decide_ship( + {"adapter": 15, "rag": 5, "tie": 0}, adapter_disqualified=6, rag_disqualified=1 + ) + assert verdict["decision"] == "archive" + assert "adapter_disqualified_more_often" in verdict["blocking_reasons"] + + +def test_decide_ship_blocks_a_minority_adapter(): + verdict = decide_ship( + {"adapter": 1, "rag": 2, "tie": 0}, adapter_disqualified=0, rag_disqualified=0 + ) + assert verdict["decision"] == "archive" + assert "adapter_did_not_win_majority" in verdict["blocking_reasons"] + + +def test_gate_runs_both_arms_and_returns_a_safe_receipt(tmp_path: Path): + paths = _profile(tmp_path) + receipt = run_writer_adapter_gate( + paths, + ["hold1", "hold2"], + generate_fn_adapter=_fixed(ADAPTER_DRAFT), + generate_fn_rag=_fixed(RAG_DRAFT), + k=0, + ) + assert receipt["kind"] == "eval_writer_adapter_gate" + assert receipt["n_holdouts"] == 2 + assert sum(receipt["wins"].values()) == 2 + assert receipt["decision"] in {"keep", "archive"} + assert_receipt_contoso_safe(receipt) + assert "reconciliation" not in json.dumps(receipt) + + +def test_gate_records_the_pair_quality_of_every_holdout(tmp_path: Path): + paths = _profile(tmp_path) + receipt = run_writer_adapter_gate( + paths, + ["hold1"], + generate_fn_adapter=_fixed(ADAPTER_DRAFT), + generate_fn_rag=_fixed(RAG_DRAFT), + k=0, + ) + item = receipt["items"][0] + # The gate briefs its holdouts the same way training built its pairs, so a + # verbatim-extract brief would show up here as well. + assert item["brief_copy_ratio"] <= 0.35 + + +def test_gate_refuses_a_holdout_that_leaked_into_retrieval(tmp_path: Path, monkeypatch): + paths = _profile(tmp_path) + monkeypatch.setattr( + "personality_protect.eval_writer_adapter.verify_holdouts_never_indexed", + lambda *_args, **_kwargs: {"ok": False, "indexed_holdout_ids": ["hold1"]}, + ) + with pytest.raises(ValueError, match="retrieval leak"): + run_writer_adapter_gate( + paths, + ["hold1"], + generate_fn_adapter=_fixed(ADAPTER_DRAFT), + generate_fn_rag=_fixed(RAG_DRAFT), + k=0, + ) diff --git a/tests/test_style_profile.py b/tests/test_style_profile.py index 111b4c6..45fcd08 100644 --- a/tests/test_style_profile.py +++ b/tests/test_style_profile.py @@ -5,6 +5,7 @@ import json from pathlib import Path +from contoso_articles import contoso_articles, contoso_post from typer.testing import CliRunner from personality_protect.cli import app @@ -13,7 +14,15 @@ from personality_protect.select import Selection from personality_protect.style_profile import ( BANNED_AI_FILLER, + DEFAULT_ARTICLE_WORD_AIM, LINKEDIN_POST_WORD_CEILING, + MAX_ARTICLE_SECTION_WORDS, + MIN_ARTICLE_SECTION_WORDS, + article_length_stats, + article_section_count_hint, + article_section_words, + article_word_aim, + article_word_target, build_style_profile, corpus_style_stats, draft_word_target, @@ -283,3 +292,60 @@ def test_cli_build_style_profile_requires_selection(tmp_path: Path): ) assert r.exit_code == 1 assert "select" in r.output.lower() or "selection" in r.output.lower() + + +def test_article_length_stats_measure_articles_only(): + """Post-shaped pieces must not set the article band.""" + pieces = [*contoso_articles(6), contoso_post()] + stats = article_length_stats(pieces) + assert stats["article_length_samples"] == 6 + assert stats["median_article_words"] >= 200 + assert stats["article_words_p90"] >= stats["median_article_words"] + + +def test_article_stats_are_zero_without_articles(): + """No articles means no measurement, not the post band under a new name.""" + stats = article_length_stats([contoso_post()]) + assert stats == { + "median_article_words": 0.0, + "article_words_p75": 0.0, + "article_words_p90": 0.0, + "article_length_samples": 0.0, + } + + +def test_article_targets_clear_the_post_ceiling(): + profile = build_style_profile([*contoso_articles(8), contoso_post()]) + assert article_word_aim(profile) > LINKEDIN_POST_WORD_CEILING + assert article_word_target(profile) >= article_word_aim(profile) + assert article_word_target(profile) > draft_word_target(profile) + + +def test_article_targets_fall_back_to_a_stated_default(): + empty = build_style_profile([contoso_post()]) + assert article_word_aim(empty) == DEFAULT_ARTICLE_WORD_AIM + + +def test_section_budget_divides_the_article_across_the_outline(): + profile = build_style_profile([*contoso_articles(8), contoso_post()]) + aim = article_word_aim(profile) + two = article_section_words(profile, sections=2) + four = article_section_words(profile, sections=4) + assert two > four + assert MIN_ARTICLE_SECTION_WORDS <= four <= MAX_ARTICLE_SECTION_WORDS + assert abs(four * 4 - aim) <= aim * 0.5 + + +def test_section_count_hint_stays_inside_the_band(): + profile = build_style_profile([*contoso_articles(8), contoso_post()]) + assert 2 <= article_section_count_hint(profile) <= 8 + + +def test_article_directives_drop_the_post_word_ceiling(): + profile = build_style_profile([*contoso_articles(8), contoso_post()]) + post = " ".join(style_directives(profile)) + article = " ".join(style_directives(profile, channel="article")) + assert "words total" in post + assert "words total" not in article + # Cadence still travels: only the length target is channel specific. + assert "Never use these words" in article diff --git a/tests/test_write_article.py b/tests/test_write_article.py index a8a8dea..36a1a91 100644 --- a/tests/test_write_article.py +++ b/tests/test_write_article.py @@ -5,41 +5,35 @@ from pathlib import Path import pytest +from contoso_articles import contoso_articles, contoso_post from personality_protect.config import init_profile -from personality_protect.models import Piece, save_index -from personality_protect.style_profile import build_style_profile, save_style_profile +from personality_protect.models import save_index +from personality_protect.style_profile import ( + article_section_words, + article_word_aim, + build_style_profile, + draft_word_target, + save_style_profile, +) from personality_protect.voice_index import build_voice_index from personality_protect.write import run_write from personality_protect.write_article import ( MIN_ARTICLE_CORPUS, + SECTION_TRIM_HEADROOM, + article_draft_ceiling, assert_article_corpus, + count_indexed_article_pieces, outline_from_brief, run_write_article, ) - -def _article(n: int, words: int = 200) -> Piece: - body = ("Contoso Ledger section. You own the outage. " * (words // 8)).strip() - return Piece( - id=f"contoso-article-{n}", - source="linkedin_article", - text=body, - year=2024, - ) +BRIEF_POINTS = "- Name one owner\n- Cut exceptions\n- Keep Ledger boring" def _seed_articles(tmp_path: Path, n: int = MIN_ARTICLE_CORPUS) -> Path: paths, _, _ = init_profile("contoso", home=tmp_path) - pieces = [_article(i) for i in range(n)] - pieces.append( - Piece( - id="contoso-post", - source="linkedin_post", - text="Contoso keeps the queue boring. You name one owner.", - year=2024, - ) - ) + pieces = [*contoso_articles(n), contoso_post()] save_index(paths.index_path, pieces) build_voice_index(paths) save_style_profile(paths, build_style_profile(pieces)) @@ -47,10 +41,7 @@ def _seed_articles(tmp_path: Path, n: int = MIN_ARTICLE_CORPUS) -> Path: def test_outline_from_brief_uses_bullets(): - sections = outline_from_brief( - "Contoso pricing", - "- Name one owner\n- Cut exceptions\n- Keep Ledger boring", - ) + sections = outline_from_brief("Contoso pricing", BRIEF_POINTS) assert sections == ["Name one owner", "Cut exceptions", "Keep Ledger boring"] @@ -62,7 +53,18 @@ def test_outline_from_brief_requires_two_sections(): def test_assert_article_corpus_floor(tmp_path: Path): _seed_articles(tmp_path, n=2) paths, _, _ = init_profile("contoso", home=tmp_path) - with pytest.raises(FileNotFoundError, match="at least"): + with pytest.raises(FileNotFoundError, match="in the corpus"): + assert_article_corpus(paths, minimum=MIN_ARTICLE_CORPUS) + + +def test_assert_article_corpus_checks_retrieval_not_just_the_corpus(tmp_path: Path): + """A corpus full of articles none of which are indexed is not a channel.""" + _seed_articles(tmp_path, n=6) + paths, _, _ = init_profile("contoso", home=tmp_path) + carved = {piece.id for piece in contoso_articles(6)} + build_voice_index(paths, holdout_ids=carved) + assert count_indexed_article_pieces(paths) == 0 + with pytest.raises(FileNotFoundError, match="in the voice index"): assert_article_corpus(paths, minimum=MIN_ARTICLE_CORPUS) @@ -71,37 +73,125 @@ def test_run_write_article_stitches_sections(tmp_path: Path): paths, _, _ = init_profile("contoso", home=tmp_path) calls: list = [] + bodies = ( + "Contoso Ledger names one owner before anybody writes a new tier.", + "Exceptions are where a published price list quietly stops describing anyone.", + "Keep the renewal test boring so its signal stays readable next quarter.", + ) + def fake_generate(messages, **_kwargs: object) -> str: calls.append(messages) - user = messages[1]["content"] - # Each section prompt names its focus. - assert "Write only the section about:" in user - return ( - "Contoso Ledger holds the line.\n\n" - "You name one owner before the packaging change." - ) + assert "Write only the section about:" in messages[1]["content"] + return bodies[len(calls) - 1] result = run_write_article( - "Contoso packaging", - "- Name one owner\n- Cut exceptions\n- Keep Ledger boring", - paths, - k=2, - generate_fn=fake_generate, + "Contoso packaging", BRIEF_POINTS, paths, k=2, generate_fn=fake_generate ) assert result["channel"] == "article" assert result["section_count"] == 3 assert result["adapter"] == "none" assert len(calls) == 3 - assert result["text"].count("Contoso Ledger holds the line.") == 3 + assert result["text"] == "\n\n".join(bodies) assert result["article_count"] >= MIN_ARTICLE_CORPUS -def test_run_write_channel_article_delegates(tmp_path: Path): +def test_stitch_drops_sections_that_restate_each_other(tmp_path: Path): + """Independent section calls can return the same paragraph three times.""" _seed_articles(tmp_path, n=MIN_ARTICLE_CORPUS) paths, _, _ = init_profile("contoso", home=tmp_path) + def fake_generate(_messages, **_kwargs: object) -> str: + return "Contoso Ledger holds the line before the packaging change lands." + + result = run_write_article( + "Contoso packaging", BRIEF_POINTS, paths, k=1, generate_fn=fake_generate + ) + assert result["section_count"] == 3 + assert result["text"].count("Contoso Ledger holds the line") == 1 + + +def test_section_budget_comes_from_article_length_not_the_post_ceiling(tmp_path: Path): + _seed_articles(tmp_path, n=6) + paths, _, _ = init_profile("contoso", home=tmp_path) + sinks: list[str] = [] + seen: list[str] = [] + def fake_generate(messages, **_kwargs: object) -> str: - return "Contoso section body with enough words to survive trim." + seen.append(messages[1]["content"]) + return "Contoso Ledger names one owner and keeps the packaging record short." + + result = run_write_article( + "Contoso packaging", + BRIEF_POINTS, + paths, + k=1, + generate_fn=fake_generate, + prompt_sink=sinks, + ) + style = build_style_profile([*contoso_articles(6), contoso_post()]) + expected = article_section_words(style, sections=3) + assert result["section_words"] == expected + assert result["word_aim"] == article_word_aim(style) + # The post ceiling is a LinkedIn character limit and must not be the target. + assert f"Write about {expected} words in this section" in seen[0] + assert f"Never exceed {draft_word_target(style)} words" not in seen[0] + + +def test_section_prompt_states_its_place_in_the_article(tmp_path: Path): + _seed_articles(tmp_path, n=6) + paths, _, _ = init_profile("contoso", home=tmp_path) + seen: list[str] = [] + + def fake_generate(messages, **_kwargs: object) -> str: + seen.append(messages[1]["content"]) + return "Contoso Ledger names an owner and writes the decision down." + + run_write_article( + "Contoso packaging", BRIEF_POINTS, paths, k=1, generate_fn=fake_generate + ) + assert "section 1 of 3" in seen[0] + assert "section 3 of 3" in seen[2] + + +def test_section_trim_keeps_a_long_section_from_running_away(tmp_path: Path): + _seed_articles(tmp_path, n=6) + paths, _, _ = init_profile("contoso", home=tmp_path) + runaway = "\n\n".join( + f"Contoso Ledger paragraph {n} names one owner for the packaging change." + for n in range(200) + ) + + result = run_write_article( + "Contoso packaging", + BRIEF_POINTS, + paths, + k=1, + generate_fn=lambda _m, **_k: runaway, + ) + style = build_style_profile([*contoso_articles(6), contoso_post()]) + per_section = int(round(article_section_words(style, sections=3) * SECTION_TRIM_HEADROOM)) + assert result["section_trim_words"] == per_section + assert result["draft_words"] <= article_draft_ceiling(style, sections=3) + + +def test_article_retrieval_never_mixes_in_posts(tmp_path: Path): + _seed_articles(tmp_path, n=6) + paths, _, _ = init_profile("contoso", home=tmp_path) + + result = run_write_article( + "Contoso packaging", + BRIEF_POINTS, + paths, + k=5, + generate_fn=lambda _m, **_k: "Contoso Ledger keeps the record short.", + ) + assert contoso_post().id not in result["exemplar_ids"] + assert all(piece_id.startswith("contoso-article") for piece_id in result["exemplar_ids"]) + + +def test_run_write_channel_article_delegates(tmp_path: Path): + _seed_articles(tmp_path, n=MIN_ARTICLE_CORPUS) + paths, _, _ = init_profile("contoso", home=tmp_path) result = run_write( "Contoso packaging", @@ -109,7 +199,7 @@ def fake_generate(messages, **_kwargs: object) -> str: paths, channel="article", k=1, - generate_fn=fake_generate, + generate_fn=lambda _m, **_k: "Contoso section body with enough words to survive trim.", ) assert result["channel"] == "article" assert result["section_count"] == 2 diff --git a/tests/test_writer_holdout.py b/tests/test_writer_holdout.py new file mode 100644 index 0000000..9c0cc40 --- /dev/null +++ b/tests/test_writer_holdout.py @@ -0,0 +1,88 @@ +"""Contoso-safe tests for the widened writer holdout carve.""" + +from __future__ import annotations + +from personality_protect.models import Piece +from personality_protect.writer_holdout import ( + is_briefable, + resolve_holdout_n, + select_writer_holdouts, +) + +CONTOSO_POST = ( + "Contoso Ledger keeps the reconciliation queue boring on purpose.\n\n" + "You ship the reconciliation on the day it lands, or you own the outage " + "that follows it.\n\n" + "You name one owner before the packaging change starts, and you don't " + "pretend the roadmap is the work of the quarter.\n\n" + "Partners already know which one you picked this quarter — 40% of them " + "said so in the survey.\n\n" + "Northwind Traders tried the clever version of this and spent a year " + "rebuilding what they had already shipped once.\n\n" + "Boring beats clever every single time that Contoso ships Ledger.\n\n" + "You keep the ledger boring and the partners stay calm about it." +) + + +def _pieces(n: int) -> list[Piece]: + return [ + Piece( + id=f"c{index:03d}", + source="linkedin_post", + # Vary the tail so ids differ without changing brief-ability. + text=CONTOSO_POST + f"\n\nRelease {index} shipped on the same day.", + year=2024, + ) + for index in range(n) + ] + + +def test_fixture_pieces_are_briefable(): + assert is_briefable(_pieces(1)[0]) + + +def test_short_or_wrong_source_pieces_are_not_briefable(): + assert not is_briefable(Piece(id="s", source="linkedin_post", text="Too short.", year=2024)) + assert not is_briefable( + Piece(id="a", source="linkedin_article", text=CONTOSO_POST, year=2024) + ) + + +def test_resolve_holdout_n_clamps_to_a_usable_band(): + assert resolve_holdout_n(0) == 0 + assert resolve_holdout_n(8) == 8 # never more than the pool + assert resolve_holdout_n(40) == 12 # floor beats a 25% share this small + assert resolve_holdout_n(80) == 20 + assert resolve_holdout_n(400) == 24 # ceiling + + +def test_selection_is_deterministic_and_order_independent(): + pieces = _pieces(40) + first = select_writer_holdouts(pieces) + again = select_writer_holdouts(list(reversed(pieces))) + assert first["holdout_ids"] == again["holdout_ids"] + assert first["n_holdouts"] == 12 + + +def test_widened_carve_is_far_larger_than_the_failed_gate(): + receipt = select_writer_holdouts(_pieces(80)) + assert receipt["n_holdouts"] >= 12 + assert receipt["train_pairs_remaining"] > receipt["n_holdouts"] + + +def test_pinned_ids_are_always_kept(): + pieces = _pieces(40) + receipt = select_writer_holdouts(pieces, pinned_ids=["c039", "c000"]) + assert {"c039", "c000"} <= set(receipt["holdout_ids"]) + assert receipt["pinned_ids"] == ["c000", "c039"] + + +def test_pinned_ids_absent_from_the_corpus_are_reported_not_carved(): + receipt = select_writer_holdouts(_pieces(20), pinned_ids=["gone"]) + assert receipt["pinned_ids_missing_from_corpus"] == ["gone"] + assert "gone" not in receipt["holdout_ids"] + + +def test_receipt_carries_no_piece_text(): + blob = repr(select_writer_holdouts(_pieces(20))) + assert "reconciliation" not in blob diff --git a/tests/test_writer_sft.py b/tests/test_writer_sft.py index 42604f4..1ad72dd 100644 --- a/tests/test_writer_sft.py +++ b/tests/test_writer_sft.py @@ -17,36 +17,66 @@ ) CONTOSO_LONG = ( - "Contoso Ledger keeps the queue boring on purpose.\n\n" - "You ship the reconciliation or you own the outage.\n\n" - "You name one owner before the packaging change starts.\n\n" - "You cut exceptions or you explain them in writing.\n\n" - "Partners already know which one you picked this quarter.\n\n" - "Stop pretending the roadmap is the work.\n\n" - "You own the queue you refuse to look at.\n\n" - "Boring beats clever every single time Contoso ships Ledger.\n\n" - "You keep Contoso boring and the partners stay calm." + "Contoso Ledger keeps the reconciliation queue boring on purpose.\n\n" + "You ship the reconciliation on the day it lands, or you own the outage " + "that follows it.\n\n" + "You name one owner before the packaging change starts, and you don't " + "pretend the roadmap is the work of the quarter.\n\n" + "Partners already know which one you picked this quarter — 40% of them " + "said so in the survey.\n\n" + "Northwind Traders tried the clever version of this and spent a year " + "rebuilding what they had already shipped once.\n\n" + "Boring beats clever every single time that Contoso ships Ledger.\n\n" + "You keep the ledger boring and the partners stay calm about it." ) def test_piece_to_writer_example_builds_chat_row(): piece = Piece(id="c1", source="linkedin_post", text=CONTOSO_LONG, year=2024) - row = piece_to_writer_example(piece) + row, reason = piece_to_writer_example(piece) + assert reason == "kept" assert row is not None assert row["meta"]["pair_kind"] == "writer" + assert row["meta"]["devoiced"] is True assert row["messages"][-1]["role"] == "assistant" assert "Contoso Ledger" in row["messages"][-1]["content"] assert "BRIEF:" in row["messages"][1]["content"] +def test_writer_row_input_is_not_an_extract_of_its_target(): + """The identity map the first adapter learned must be impossible to build.""" + piece = Piece(id="c1", source="linkedin_post", text=CONTOSO_LONG, year=2024) + row, _ = piece_to_writer_example(piece) + assert row is not None + assert row["meta"]["brief_copy_ratio"] <= 0.35 + user = row["messages"][1]["content"] + assert "you" not in user.split("BRIEF:")[-1].lower() + + +def test_short_and_non_post_pieces_report_why_they_dropped(): + assert piece_to_writer_example( + Piece(id="s", source="linkedin_post", text="Too short.", year=2024) + ) == (None, "too_short") + assert piece_to_writer_example( + Piece(id="a", source="linkedin_article", text=CONTOSO_LONG, year=2024) + ) == (None, "not_a_post") + + def test_build_writer_sft_excludes_holdouts(tmp_path: Path): pieces = [ Piece(id="keep", source="linkedin_post", text=CONTOSO_LONG, year=2024), - Piece(id="hold", source="linkedin_post", text=CONTOSO_LONG + " Extra.", year=2024), + Piece( + id="hold", + source="linkedin_post", + text=CONTOSO_LONG + " Extra care went into the packaging review.", + year=2024, + ), ] out = tmp_path / "writer.jsonl" receipt = build_writer_sft(pieces, out, holdout_ids={"hold"}) assert receipt["examples"] == 1 + assert receipt["dropped_by_reason"] == {"holdout": 1} + assert receipt["brief_copy_ratio"]["max"] <= receipt["max_copy_ratio"] lines = out.read_text(encoding="utf-8").strip().splitlines() assert len(lines) == 1 assert json.loads(lines[0])["meta"]["piece_id"] == "keep"