From 844c5ee9990fe652a933ab2d3946ae3527a13db9 Mon Sep 17 00:00:00 2001 From: Dusan Milicevic Date: Thu, 30 Jul 2026 19:10:24 -0500 Subject: [PATCH 1/2] 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 4b3c825dbfa7842f86fb212282ccee7f6f28f318 Mon Sep 17 00:00:00 2001 From: Dusan Milicevic Date: Fri, 31 Jul 2026 03:10:36 -0500 Subject: [PATCH 2/2] Keep mid-train writer checkpoints and shorten the writer recipe Persist adapters/latest/checkpoints/step_* after each MLX chunk so a gate can evaluate early weights. Default writer training to 3 epochs. Add eval-writer-adapter --sweep-checkpoints and Contoso-safe archive basenames. Co-authored-by: Cursor --- README.md | 6 +- src/personality_protect/cli.py | 64 +++++++++++++++++ .../eval_writer_adapter.py | 71 +++++++++++++++++++ src/personality_protect/mlx_train.py | 57 ++++++++++++++- src/personality_protect/write.py | 27 ++++++- tests/test_eval_writer_adapter.py | 45 ++++++++++++ tests/test_mlx_train_safety.py | 31 ++++++++ 7 files changed, 297 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index a3b93c2..e9212dc 100644 --- a/README.md +++ b/README.md @@ -300,12 +300,14 @@ Nothing here is needed for a draft. These commands stay in the CLI for local exp ```bash personality-protect build-writer-sft +personality-protect select-writer-holdouts --apply +personality-protect index-voice --from-carve personality-protect train --writer --backend mlx -personality-protect eval-write-holdout --out receipt.json +personality-protect eval-writer-adapter --archive-on-fail personality-protect write --adapter --topic "…" --points "…" ``` -Keep an adapter only if `eval-write-holdout` shows it beating RAG-alone on held-out pieces. Otherwise delete it and stay on the default. Training is not a prerequisite for `write`, and an untested adapter is not an upgrade. +`build-writer-sft` builds de-voiced brief→post pairs. `train --writer` uses a short writer recipe (3 epochs) and keeps per-chunk checkpoints under `adapters/latest/checkpoints/`. Keep an adapter only when `eval-writer-adapter` decides `keep`; otherwise archive it and stay on `adapter=none`. Training is not a prerequisite for `write`. ### Other experiment commands diff --git a/src/personality_protect/cli.py b/src/personality_protect/cli.py index d21c28f..ad35876 100644 --- a/src/personality_protect/cli.py +++ b/src/personality_protect/cli.py @@ -941,6 +941,12 @@ def eval_writer_adapter_cmd( "--archive-on-fail", help="Move the adapter aside when the gate fails (write returns to adapter=none).", ), + sweep_checkpoints: bool = typer.Option( + False, + "--sweep-checkpoints", + help="Gate every adapters/latest/checkpoints/step_* dir, earliest first; " + "install the first that ships.", + ), 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"), @@ -952,6 +958,7 @@ def eval_writer_adapter_cmd( device — importing MLX without one aborts the interpreter. """ from personality_protect.eval_writer_adapter import ( + run_checkpoint_gate_sweep, run_writer_adapter_gate, write_gate_receipt, ) @@ -978,6 +985,63 @@ def eval_writer_adapter_cmd( ) raise typer.Exit(1) + if sweep_checkpoints: + try: + 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 + + def _make_adapter(adapter_path: str): + return make_mlx_generator( + base_model=config.base_model, adapter_path=adapter_path + ) + + def _on_ckpt(row: dict, _receipt: dict) -> None: + if as_json: + return + wins = row["wins"] + console.print( + f"[dim]{row['checkpoint']}: adapter {wins['adapter']} — " + f"rag {wins['rag']} — tie {wins['tie']} → {row['decision']}[/dim]" + ) + + try: + sweep = run_checkpoint_gate_sweep( + paths, + holdout_ids, + make_adapter_generate=_make_adapter, + generate_fn_rag=generate_rag, + k=k, + max_tokens=max_tokens, + alpha=alpha, + on_checkpoint=None if as_json else _on_ckpt, + ) + except (ValueError, FileNotFoundError) as exc: + console.print(f"[red]{exc}[/red]") + raise typer.Exit(1) from exc + + if sweep["decision"] == "archive" and archive_on_fail: + sweep["archived_to"] = archive_writer_adapter( + paths, reason="gate-fail-sweep" + ) + + target = out or ( + paths.root / "dogfood" / "writer_adapter_checkpoint_sweep_receipt.json" + ) + write_gate_receipt(sweep, target) + if as_json: + typer.echo(json.dumps(sweep, indent=2, ensure_ascii=False)) + else: + console.print( + f"sweep: evaluated {sweep['evaluated']}/{sweep['n_checkpoints']} " + f"→ decision [bold]{sweep['decision']}[/bold] " + f"(kept={sweep['kept_checkpoint']}) → {target}" + ) + if sweep["decision"] != "keep": + raise typer.Exit(1) + return + adapter_path = resolve_writer_adapter(paths) if adapter_path is None: console.print( diff --git a/src/personality_protect/eval_writer_adapter.py b/src/personality_protect/eval_writer_adapter.py index 0a6b4d1..593d8d7 100644 --- a/src/personality_protect/eval_writer_adapter.py +++ b/src/personality_protect/eval_writer_adapter.py @@ -253,3 +253,74 @@ def write_gate_receipt(receipt: dict[str, Any], path: Any) -> Any: json.dumps(receipt, indent=2, ensure_ascii=False) + "\n", encoding="utf-8" ) return path + + +def run_checkpoint_gate_sweep( + paths: ProfilePaths, + holdout_ids: Sequence[str], + *, + make_adapter_generate: Any, + generate_fn_rag: GenerateFn, + k: int = DEFAULT_WRITE_K, + max_tokens: int = DEFAULT_WRITE_MAX_TOKENS, + alpha: float = SHIP_ALPHA, + on_checkpoint: Any = None, +) -> dict[str, Any]: + """Gate every durable step checkpoint, earliest first; keep the first that ships. + + ``make_adapter_generate(adapter_path)`` must build a fresh generator for the + installed weights — reusing a prior MLX load would score the wrong adapter. + """ + from personality_protect.mlx_train import list_step_checkpoints + from personality_protect.write import install_writer_checkpoint + + latest = paths.adapters_dir / "latest" + checkpoints = list_step_checkpoints(latest) + if not checkpoints: + raise FileNotFoundError( + f"No step checkpoints under {latest / 'checkpoints'}. " + "Retrain so each chunk persists checkpoints/step_NNNNNN/." + ) + + results: list[dict[str, Any]] = [] + kept: str | None = None + for ckpt in checkpoints: + install_writer_checkpoint(paths, ckpt) + generate_adapter = make_adapter_generate(str(ckpt)) + receipt = run_writer_adapter_gate( + paths, + holdout_ids, + generate_fn_adapter=generate_adapter, + generate_fn_rag=generate_fn_rag, + k=k, + max_tokens=max_tokens, + alpha=alpha, + ) + row = { + "checkpoint": ckpt.name, + "decision": receipt["decision"], + "wins": receipt["wins"], + "disqualified": receipt["disqualified"], + "p_value": receipt["p_value"], + "blocking_reasons": receipt["blocking_reasons"], + "n_holdouts": receipt["n_holdouts"], + } + results.append(row) + if on_checkpoint is not None: + on_checkpoint(row, receipt) + if receipt["decision"] == "keep": + kept = ckpt.name + install_writer_checkpoint(paths, ckpt) + break + + sweep: dict[str, Any] = { + "kind": "eval_writer_adapter_checkpoint_sweep", + "created_at": datetime.now(timezone.utc).isoformat(), + "n_checkpoints": len(checkpoints), + "evaluated": len(results), + "kept_checkpoint": kept, + "decision": "keep" if kept else "archive", + "results": results, + } + assert_receipt_contoso_safe(sweep) + return sweep diff --git a/src/personality_protect/mlx_train.py b/src/personality_protect/mlx_train.py index f690ce0..d2190a9 100644 --- a/src/personality_protect/mlx_train.py +++ b/src/personality_protect/mlx_train.py @@ -42,7 +42,10 @@ WRITER_NUM_LAYERS = 16 WRITER_LORA_RANK = 16 WRITER_LEARNING_RATE = 3e-5 -WRITER_EPOCHS = 10 +# 10 epochs on ~60 de-voiced pairs drove train loss ~0.08 and raised invention / +# parroting on the n=20 gate. Three passes is enough to move the adapter without +# memorizing the tiny set; mid-train step snapshots let a later gate pick earlier. +WRITER_EPOCHS = 3 # 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) @@ -51,6 +54,10 @@ ProgressCallback = Callable[[dict[str, Any]], None] CHECKPOINT_META_NAME = "train_chunks.json" +# Durable per-chunk copies under adapter_dir/checkpoints/step_NNNNNN/. +# Distinct from mlx-lm's ephemeral ``0000050_adapters.safetensors`` which each +# chunk overwrites with the same name. +STEP_CHECKPOINTS_DIRNAME = "checkpoints" # Snapshot before each chunk so nan / crash never leaves a wiped or poisoned adapter. LAST_GOOD_ADAPTER_NAME = "adapters.safetensors.last_good" @@ -79,6 +86,47 @@ def restore_last_good_adapter(adapter_dir: Path) -> bool: return True +def persist_step_checkpoint(adapter_dir: Path, completed_steps: int) -> Path | None: + """Copy live weights into ``checkpoints/step_NNNNNN/`` after a good chunk. + + mlx-lm's own numbered files reuse the same basename every chunk, so earlier + steps disappear. These directories keep every completed step count so a gate + can evaluate under-trained adapters without a full retrain. + """ + src = adapter_dir / "adapters.safetensors" + if not src.is_file(): + return None + steps = max(0, int(completed_steps)) + dest_dir = adapter_dir / STEP_CHECKPOINTS_DIRNAME / f"step_{steps:06d}" + dest_dir.mkdir(parents=True, exist_ok=True) + shutil.copy2(src, dest_dir / "adapters.safetensors") + for name in ("adapter_config.json", "adapter_config.yaml"): + cfg = adapter_dir / name + if cfg.is_file(): + shutil.copy2(cfg, dest_dir / name) + return dest_dir + + +def list_step_checkpoints(adapter_dir: Path) -> list[Path]: + """Return step checkpoint dirs oldest-first (under-trained → final).""" + root = adapter_dir / STEP_CHECKPOINTS_DIRNAME + if not root.is_dir(): + return [] + dirs = [ + path + for path in root.iterdir() + if path.is_dir() and (path / "adapters.safetensors").is_file() + ] + return sorted(dirs, key=lambda path: path.name) + + +def clear_step_checkpoints(adapter_dir: Path) -> None: + """Drop durable step snapshots (used on ``--force-retrain``).""" + root = adapter_dir / STEP_CHECKPOINTS_DIRNAME + if root.is_dir(): + shutil.rmtree(root) + + def plan_train_chunks(total_steps: int, chunk_size: int) -> list[int]: """Split ``total_steps`` into positive chunk sizes (last chunk may be shorter).""" total = max(0, int(total_steps)) @@ -164,9 +212,13 @@ def _clear_adapter_weights(adapter_dir: Path) -> None: adapter_file.unlink() for stale in adapter_dir.glob("*_adapters.safetensors"): stale.unlink() + last_good = adapter_dir / LAST_GOOD_ADAPTER_NAME + if last_good.is_file(): + last_good.unlink() meta_path = adapter_dir / CHECKPOINT_META_NAME if meta_path.is_file(): meta_path.unlink() + clear_step_checkpoints(adapter_dir) def resolve_train_plan( @@ -697,6 +749,7 @@ def _on_line(line: str, *, _completed=completed, _n=n_iters) -> None: completed += n_iters status = "complete" if completed >= plan.total_steps else "in_progress" + step_ckpt = persist_step_checkpoint(adapter_dir, completed) chunk_meta = { "status": status, "completed_steps": completed, @@ -715,6 +768,7 @@ def _on_line(line: str, *, _completed=completed, _n=n_iters) -> None: "already_completed": plan.already_completed, "steps_this_run": completed - plan.already_completed, "chunks": total_chunk_count, + "step_checkpoint": str(step_ckpt) if step_ckpt is not None else None, } write_train_checkpoint_meta(adapter_dir, chunk_meta) @@ -727,6 +781,7 @@ def _on_line(line: str, *, _completed=completed, _n=n_iters) -> None: "completed_steps": completed, "total_steps": plan.total_steps, "peak_mem_gb": result.peak_mem_gb, + "step_checkpoint": str(step_ckpt) if step_ckpt is not None else None, } ) diff --git a/src/personality_protect/write.py b/src/personality_protect/write.py index b4509d6..a21d181 100644 --- a/src/personality_protect/write.py +++ b/src/personality_protect/write.py @@ -8,6 +8,7 @@ import re from collections.abc import Callable, MutableSequence, Sequence +from pathlib import Path from typing import Any from personality_protect.chat_prompt import ( @@ -176,7 +177,31 @@ def archive_writer_adapter(paths: ProfilePaths, *, reason: str) -> str | 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) + # Basename only — absolute paths under ~/.personality-protect embed the + # local username and poison Contoso-safe receipts. + return target.name + + +def install_writer_checkpoint(paths: ProfilePaths, checkpoint_dir: str | Path) -> str: + """Copy a step checkpoint into ``adapters/latest`` for gating or shipping. + + Leaves durable ``checkpoints/`` snapshots in place. Overwrites only the live + ``adapters.safetensors`` (and config) that :func:`resolve_writer_adapter` reads. + """ + import shutil + + src = Path(checkpoint_dir) + weights = src / "adapters.safetensors" + if not weights.is_file(): + raise FileNotFoundError(f"No adapters.safetensors in checkpoint {src}") + latest = paths.adapters_dir / "latest" + latest.mkdir(parents=True, exist_ok=True) + shutil.copy2(weights, latest / "adapters.safetensors") + for name in ("adapter_config.json", "adapter_config.yaml"): + cfg = src / name + if cfg.is_file(): + shutil.copy2(cfg, latest / name) + return str(latest) _SENTENCE_START_WORD_RE = re.compile( diff --git a/tests/test_eval_writer_adapter.py b/tests/test_eval_writer_adapter.py index d47b28b..fe5f4fd 100644 --- a/tests/test_eval_writer_adapter.py +++ b/tests/test_eval_writer_adapter.py @@ -162,3 +162,48 @@ def test_gate_refuses_a_holdout_that_leaked_into_retrieval(tmp_path: Path, monke generate_fn_rag=_fixed(RAG_DRAFT), k=0, ) + + +def test_checkpoint_sweep_keeps_the_first_shipping_step(tmp_path: Path): + from personality_protect.eval_writer_adapter import run_checkpoint_gate_sweep + from personality_protect.mlx_train import persist_step_checkpoint + + paths = _profile(tmp_path) + latest = paths.adapters_dir / "latest" + (latest / "adapters.safetensors").write_bytes(b"early") + persist_step_checkpoint(latest, 50) + (latest / "adapters.safetensors").write_bytes(b"late") + persist_step_checkpoint(latest, 100) + + calls: list[str] = [] + + def _make(adapter_path: str): + calls.append(Path(adapter_path).name) + # Early checkpoint wins the distance game; late one mirrors the RAG loser. + if Path(adapter_path).name == "step_000050": + return _fixed(ADAPTER_DRAFT) + return _fixed(RAG_DRAFT) + + sweep = run_checkpoint_gate_sweep( + paths, + ["hold1", "hold2"], + make_adapter_generate=_make, + generate_fn_rag=_fixed(RAG_DRAFT), + k=0, + alpha=1.0, # majority alone is enough for Contoso stub n=2 + ) + assert sweep["decision"] == "keep" + assert sweep["kept_checkpoint"] == "step_000050" + assert sweep["evaluated"] == 1 # stop at first keep + assert calls == ["step_000050"] + assert (latest / "adapters.safetensors").read_bytes() == b"early" + assert_receipt_contoso_safe(sweep) + + +def test_writer_epochs_default_is_short_enough_to_avoid_overfit(): + from personality_protect.mlx_train import WRITER_EPOCHS + from personality_protect.train import auto_max_steps, writer_train_settings + + assert WRITER_EPOCHS == 3 + assert writer_train_settings()["epochs"] == 3 + assert auto_max_steps(60, epochs=WRITER_EPOCHS) == 180 diff --git a/tests/test_mlx_train_safety.py b/tests/test_mlx_train_safety.py index 63f3d9f..2f6b326 100644 --- a/tests/test_mlx_train_safety.py +++ b/tests/test_mlx_train_safety.py @@ -570,6 +570,37 @@ def _side_effect(**kwargs): assert metas_during[1] is not None assert metas_during[1]["completed_steps"] == 50 assert metas_during[1]["status"] == "in_progress" + from personality_protect.mlx_train import list_step_checkpoints + + steps = [p.name for p in list_step_checkpoints(adapter_dir)] + assert steps == ["step_000050", "step_000100", "step_000120"] + assert (adapter_dir / "checkpoints" / "step_000050" / "adapters.safetensors").is_file() + + +def test_persist_step_checkpoint_roundtrip(tmp_path: Path): + from personality_protect.mlx_train import ( + clear_step_checkpoints, + list_step_checkpoints, + persist_step_checkpoint, + ) + + adapter_dir = tmp_path / "adapters" + adapter_dir.mkdir() + (adapter_dir / "adapters.safetensors").write_bytes(b"w50") + (adapter_dir / "adapter_config.json").write_text("{}", encoding="utf-8") + dest = persist_step_checkpoint(adapter_dir, 50) + assert dest is not None + assert dest.name == "step_000050" + assert (dest / "adapters.safetensors").read_bytes() == b"w50" + assert (dest / "adapter_config.json").read_text(encoding="utf-8") == "{}" + (adapter_dir / "adapters.safetensors").write_bytes(b"w100") + persist_step_checkpoint(adapter_dir, 100) + assert [p.name for p in list_step_checkpoints(adapter_dir)] == [ + "step_000050", + "step_000100", + ] + clear_step_checkpoints(adapter_dir) + assert list_step_checkpoints(adapter_dir) == [] def test_chunked_train_resume_skips_completed_steps(tmp_path: Path):