From 844c5ee9990fe652a933ab2d3946ae3527a13db9 Mon Sep 17 00:00:00 2001 From: Dusan Milicevic Date: Thu, 30 Jul 2026 19:10:24 -0500 Subject: [PATCH 1/3] De-voice writer SFT pairs and widen the ship-gate holdout Writer SFT briefs were mined verbatim from their own target, so the input sat inside the output (5-gram copy ratio median 1.0) and copying the prompt forward was a winning strategy for the objective. Add a deterministic de-voicing operator so rows are (D(y), y): entities, figures and claim vocabulary are preserved while second-person address, contractions, emphasis, discourse markers, fragment rhythm and connective scaffolding are removed. Pair construction is gated rather than merely built. Every row is measured against the shipped pair gate for cadence movement, checked for entities or figures the operator invented, and dropped unless what the brief shares with the post falls under a cap. Copy ratio drops from median 1.0 to 0.15. Also here: - select-writer-holdouts: deterministic, pinned-compatible holdout carve sized as a share of the briefable pool. A three-item gate cannot reach any significance threshold; a sign test needs more comparisons than that. - eval-writer-adapter: committed ship gate for RAG+adapter vs RAG-alone, loading each arm's weights once. Keeps an adapter only on a majority win that clears a one-sided sign test and does not raise the disqualification rate. - index-voice --from-carve: read holdout ids from the carve file instead of retyping them as flags. - train --detach: portable detached launch via start_new_session, so an unattended run does not depend on a shell staying open (and does not depend on setsid, which macOS does not ship). - Writer train recipe: 16 layers, LoRA rank 16, lr 3e-5, 10 epochs, with rank and learning rate plumbed through the chunk worker and recorded in the checkpoint meta. - release_mlx_memory now honours the MLX opt-in gate; a Metal-less session aborts in C++ where except Exception cannot catch it. Co-authored-by: Cursor --- src/personality_protect/cli.py | 249 +++++++- src/personality_protect/detach.py | 82 +++ src/personality_protect/devoice.py | 563 ++++++++++++++++++ src/personality_protect/eval_write_holdout.py | 16 +- .../eval_writer_adapter.py | 255 ++++++++ src/personality_protect/mlx_chunk_worker.py | 9 +- src/personality_protect/mlx_runtime.py | 10 +- src/personality_protect/mlx_train.py | 41 +- src/personality_protect/train.py | 64 +- src/personality_protect/write.py | 68 +++ src/personality_protect/writer_guards.py | 5 + src/personality_protect/writer_holdout.py | 157 +++++ src/personality_protect/writer_sft.py | 116 +++- tests/test_detach.py | 81 +++ tests/test_devoice.py | 117 ++++ tests/test_eval_writer_adapter.py | 164 +++++ tests/test_writer_holdout.py | 88 +++ tests/test_writer_sft.py | 52 +- 18 files changed, 2085 insertions(+), 52 deletions(-) create mode 100644 src/personality_protect/detach.py create mode 100644 src/personality_protect/devoice.py create mode 100644 src/personality_protect/eval_writer_adapter.py create mode 100644 src/personality_protect/writer_holdout.py create mode 100644 tests/test_detach.py create mode 100644 tests/test_devoice.py create mode 100644 tests/test_eval_writer_adapter.py create mode 100644 tests/test_writer_holdout.py diff --git a/src/personality_protect/cli.py b/src/personality_protect/cli.py index 744e152..0cc2584 100644 --- a/src/personality_protect/cli.py +++ b/src/personality_protect/cli.py @@ -54,6 +54,7 @@ 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 +111,11 @@ MIN_WRITE_K, run_write, ) +from personality_protect.writer_holdout import ( + DEFAULT_HOLDOUT_FRACTION, + MAX_HOLDOUT_N, + MIN_HOLDOUT_N, +) app = typer.Typer( name="personality-protect", @@ -340,11 +346,18 @@ 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 holdout carve.", + ), 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.writer_holdout import load_pinned_holdout_ids + _banner_from_ctx(ctx, json_mode=as_json) paths = get_paths(profile, home=home) try: @@ -353,7 +366,13 @@ 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 file the carve already wrote. + excluded = list(holdout_id or ()) + if from_carve: + excluded = sorted(set(excluded) | set(load_pinned_holdout_ids(paths))) + + result = build_voice_index(paths, holdout_ids=excluded) if as_json: typer.echo(json.dumps(result, indent=2)) return @@ -616,6 +635,193 @@ 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("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 +1082,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 +1180,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 +1418,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_holdout.py b/src/personality_protect/eval_write_holdout.py index ea1f73f..6ddf78f 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; " 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/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/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/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_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_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" From 8862b924e179875c43666dae132ce93fb353cf33 Mon Sep 17 00:00:00 2001 From: Dusan Milicevic Date: Fri, 31 Jul 2026 00:52:07 -0500 Subject: [PATCH 2/3] Give the article channel its own length band and holdout eval The article channel inherited the post length band, so "articles" came out at post length and no measurement said otherwise. Length now comes from the author's own linkedin_article pieces (median aim, p90 ceiling) divided across the outline into a per-section budget, and article retrieval is restricted to articles so a post cannot become the rhythm reference for a section. Article voice was also unproven, which is a different problem from being wrong. select-article-holdouts carves articles deterministically without dropping retrieval below its floor, each holdout is reduced to a lossy outline brief that is capped in absolute words rather than as a share of a source that keeps growing, and eval-write-article writes that brief twice -- once with exemplars and the style card, once with neither -- under the same outline and budget, so distance measures the voice machinery and not the scaffolding both arms share. Co-authored-by: Cursor --- README.md | 57 ++- src/personality_protect/article_brief.py | 219 ++++++++++ src/personality_protect/article_holdout.py | 141 +++++++ src/personality_protect/cli.py | 227 +++++++++- src/personality_protect/eval_write_article.py | 397 ++++++++++++++++++ src/personality_protect/eval_write_holdout.py | 19 +- src/personality_protect/style_profile.py | 101 ++++- src/personality_protect/write_article.py | 141 ++++++- tests/contoso_articles.py | 110 +++++ tests/test_article_brief.py | 90 ++++ tests/test_article_holdout.py | 130 ++++++ tests/test_eval_write_article.py | 314 ++++++++++++++ tests/test_style_profile.py | 66 +++ tests/test_write_article.py | 172 ++++++-- 14 files changed, 2113 insertions(+), 71 deletions(-) create mode 100644 src/personality_protect/article_brief.py create mode 100644 src/personality_protect/article_holdout.py create mode 100644 src/personality_protect/eval_write_article.py create mode 100644 tests/contoso_articles.py create mode 100644 tests/test_article_brief.py create mode 100644 tests/test_article_holdout.py create mode 100644 tests/test_eval_write_article.py diff --git a/README.md b/README.md index 82abd64..8f6da69 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. ### 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 0cc2584..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,6 +55,7 @@ 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, @@ -111,6 +117,7 @@ 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, @@ -349,13 +356,14 @@ def index_voice_cmd( from_carve: bool = typer.Option( False, "--from-carve", - help="Also exclude every id in the profile's writer holdout 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) @@ -367,10 +375,14 @@ def index_voice_cmd( raise typer.Exit(1) from exc # Retyping a widened carve as flags is how a holdout quietly re-enters - # retrieval; read it from the file the carve already wrote. + # 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))) + 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: @@ -707,6 +719,215 @@ def select_writer_holdouts_cmd( ) +@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, diff --git a/src/personality_protect/eval_write_article.py b/src/personality_protect/eval_write_article.py new file mode 100644 index 0000000..d34fc09 --- /dev/null +++ b/src/personality_protect/eval_write_article.py @@ -0,0 +1,397 @@ +"""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, +) -> 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. + """ + 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] = [] + 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, + "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 + + 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"])) + 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, + ) + 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 6ddf78f..747e43d 100644 --- a/src/personality_protect/eval_write_holdout.py +++ b/src/personality_protect/eval_write_holdout.py @@ -508,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). @@ -515,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) @@ -523,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, @@ -546,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/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/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/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_eval_write_article.py b/tests/test_eval_write_article.py new file mode 100644 index 0000000..3a30388 --- /dev/null +++ b/tests/test_eval_write_article.py @@ -0,0 +1,314 @@ +"""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_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_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 From 33523555c5d0955aee0090d8e6a76ca88095fd3f Mon Sep 17 00:00:00 2001 From: Dusan Milicevic Date: Fri, 31 Jul 2026 00:54:50 -0500 Subject: [PATCH 3/3] Separate an undecided article run from a cadence loss An item where both arms are disqualified scores as a tie, so a run in which every draft was rejected before distance was consulted reported the same "article_did_not_win_majority" as a run the voice arm was measured and lost. Those are different results and only one is evidence about voice. The receipt now carries distance_ever_decided and names the all-disqualified case. Co-authored-by: Cursor --- README.md | 2 +- src/personality_protect/eval_write_article.py | 21 +++++++++++++ tests/test_eval_write_article.py | 30 +++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8f6da69..a3b93c2 100644 --- a/README.md +++ b/README.md @@ -217,7 +217,7 @@ The carve is deterministic (`blake2b(piece_id)` order), keeps previously carved 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. +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 diff --git a/src/personality_protect/eval_write_article.py b/src/personality_protect/eval_write_article.py index d34fc09..a6a78da 100644 --- a/src/personality_protect/eval_write_article.py +++ b/src/personality_protect/eval_write_article.py @@ -218,17 +218,30 @@ def decide_article_voice( 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): @@ -238,6 +251,8 @@ def decide_article_voice( 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, @@ -280,6 +295,7 @@ def run_eval_write_article( skipped: list[str] = [] article_dq = 0 base_dq = 0 + both_dq = 0 for piece in pieces: holdout_text = normalize_corpus_text(piece.text) @@ -335,6 +351,9 @@ def run_eval_write_article( 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) @@ -357,6 +376,8 @@ def run_eval_write_article( 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", diff --git a/tests/test_eval_write_article.py b/tests/test_eval_write_article.py index 3a30388..971f65d 100644 --- a/tests/test_eval_write_article.py +++ b/tests/test_eval_write_article.py @@ -276,6 +276,36 @@ def test_verdict_requires_a_margin_a_coin_would_not_produce(): 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()