From ac0a31b28b6c0a865be85fea8ca7966df79bc53a Mon Sep 17 00:00:00 2001 From: Shir Goldberg <3937986+shirgoldbird@users.noreply.github.com> Date: Tue, 22 Sep 2026 13:17:36 -0400 Subject: [PATCH 1/3] fix(pipeline): stop the agent files telling the model it has tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generation step returned a transcript of itself using tools instead of a page: I'll research the GitHub repo and peer pages before writing. {"name": "Glob", "arguments": {"pattern": "docs/.../*.mdx"}} docs/learning-how-tos/cookbook/context-aware.mdx ... build_authoring_system_prompt pastes .claude/agents/docs-writer.md verbatim into a plain Messages API call. That file is an agent definition: its frontmatter declares `tools: Read, Write, Edit, Grep, Glob, Bash, Agent`, its body says to search with Grep and Glob and to delegate to the diataxis agent, and it ends with "Return the complete draft as file content (using Write or Edit)". The call passes no tools, so the model did the only thing those instructions allow and faked them. diataxis.md and editorial-reviewer.md declare tools too, so rework and review shared the exposure. - load_agent() strips the YAML frontmatter from every agent file. - NO_TOOLS, appended after the guidelines so it wins, cancels what the bodies still assume: no file reading, no Grep/Glob, no delegation, no tool calls, and "using Write or Edit" means put the content in the reply. - generate.py and rework.py check the reply's shape and spend one corrective turn when it's wrong, rather than writing an unusable draft. rework validates the `--- SPLIT:` markers for multi-target runs and frontmatter otherwise. Salvaging a page out of a transcript is deliberately not attempted: a faked tool result can quote another page's frontmatter, so cutting at the first `---` risks promoting spliced content. Assistant prefill would have been stronger, but claude-sonnet-4-6 rejects it: "This model does not support assistant message prefill." Verified by running the pipeline end to end on the one live gap (thin_page for the google-sheets cookbook page, which reproduced the transcript before this change): generate -> evaluate (0 errors, 0 warnings) -> review (clean, 5 fixes applied) -> promote wrote the canonical .mdx. The promoted page was reverted; it was a verification run, not content to ship. Tests: pipeline/test_prompts.py (14), including the real transcript as a case. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pipeline/README.md | 17 ++++++ pipeline/generate.py | 62 +++++++++++++++----- pipeline/rework.py | 38 ++++++++++--- pipeline/test_prompts.py | 120 +++++++++++++++++++++++++++++++++++++++ pipeline/util.py | 73 +++++++++++++++++++++--- 5 files changed, 281 insertions(+), 29 deletions(-) create mode 100644 pipeline/test_prompts.py diff --git a/pipeline/README.md b/pipeline/README.md index 661dbb9f..f0089a25 100644 --- a/pipeline/README.md +++ b/pipeline/README.md @@ -23,6 +23,23 @@ All model prompts are assembled from the files above in one place: the `build_*_system_prompt` / `load_planning_context` helpers in `pipeline/util.py`. That is plumbing — change behavior in the files above, not in `util.py`. +Two things in `util.py` are not plumbing, because the agent files can't express them. +The `.claude/agents/*.md` files are agent definitions: their frontmatter declares +`tools:` and their bodies say to search the repo with Grep and Glob and to return work +"using Write or Edit". The pipeline pastes their prose into plain API calls where no +tools exist, and taken at face value those instructions make the model return a +transcript of itself researching instead of a page. So: + +- `load_agent()` strips the frontmatter, and the `NO_TOOLS` block (appended after the + guidelines, so it wins) cancels what the bodies still assume. +- `generate.py` and `rework.py` check the reply's shape and, if it isn't a file, spend + one corrective turn on it. Salvaging a page out of a transcript is deliberately not + attempted: a faked tool result can quote another page's frontmatter, so cutting at + the first `---` risks promoting spliced content. + +Keep that in mind when editing the agent files: prose aimed at the interactive agent +is also prompt text for the pipeline. + ## Setup ```bash diff --git a/pipeline/generate.py b/pipeline/generate.py index e9b1c8eb..ca7840a9 100644 --- a/pipeline/generate.py +++ b/pipeline/generate.py @@ -30,14 +30,7 @@ import yaml -try: - import anthropic -except ImportError: - print("Install the Anthropic SDK: pip install anthropic") - sys.exit(1) - - -from util import build_authoring_system_prompt +from util import build_authoring_system_prompt, looks_like_mdx, format_retry_prompt from open_prs import EXIT_NOTHING_TO_DO, fetch_open_prs, split_claimed_gaps REPO_ROOT = Path(__file__).resolve().parent.parent @@ -136,6 +129,20 @@ def find_existing_docs_for_family(family_name): return context_docs +def make_client(): + """Build the API client, importing the SDK only when a call is imminent. + + Kept lazy so --dry-run, gap detection and the tests all work without the + SDK installed. None of them talk to the model. + """ + try: + import anthropic + except ImportError: + print("Install the Anthropic SDK: pip install anthropic") + sys.exit(1) + return anthropic.Anthropic() + + def build_system_prompt(): # All writing rules live in the agent files, assembled in one place. return build_authoring_system_prompt( @@ -266,17 +273,41 @@ def build_generation_prompt(gap, family_name, openapi_context, existing_docs_con {existing_docs_summary}""" -def generate_content(client, system_prompt, user_prompt): - """Call Claude to generate content.""" +def _complete(client, system_prompt, messages): response = client.messages.create( model=MODEL, max_tokens=MAX_TOKENS, system=system_prompt, - messages=[{"role": "user", "content": user_prompt}], + messages=messages, ) return response.content[0].text +def generate_content(client, system_prompt, user_prompt, expect_file=True): + """Call Claude to generate content. + + A reply that isn't a page gets one corrective turn rather than failing the + run. The docs-writer guidelines are written for a harness with tools, so the + model occasionally obliges by narrating research it cannot do and returns a + transcript instead of a file. + """ + messages = [{"role": "user", "content": user_prompt}] + text = _complete(client, system_prompt, messages) + + if not expect_file or looks_like_mdx(text) or not text.strip(): + return text + + print(" Reply was not a page (commentary or a faked tool transcript), asking again") + messages += [ + # Echo a slice back rather than the whole thing: the bad reply can run to + # hundreds of lines and only needs to be identifiable. + {"role": "assistant", "content": text[:500].strip()}, + {"role": "user", "content": format_retry_prompt( + "it did not start with YAML frontmatter (---)")}, + ] + return _complete(client, system_prompt, messages) + + def determine_output_path(gap, family_name, content=None): """Determine where to write the generated content.""" gap_type = gap["type"] @@ -426,7 +457,7 @@ def main(): print() return 0 - client = anthropic.Anthropic() + client = make_client() system_prompt = build_system_prompt() run_id = datetime.now(timezone.utc).strftime("%Y%m%d-%H%M%S") @@ -454,7 +485,12 @@ def main(): existing_docs = find_existing_docs_for_family(family) user_prompt = build_generation_prompt(gap, family, openapi_context, existing_docs) - content = generate_content(client, system_prompt, user_prompt) + # Every gap but missing_description wants a whole file back; + # missing_description wants a bare string, so don't shape-check it. + content = generate_content( + client, system_prompt, user_prompt, + expect_file=gap["type"] != "missing_description", + ) if gap["type"] == "missing_description": description = content.strip().strip('"').strip("'") diff --git a/pipeline/rework.py b/pipeline/rework.py index afbf6b16..2038edf4 100644 --- a/pipeline/rework.py +++ b/pipeline/rework.py @@ -58,7 +58,7 @@ sys.exit(1) -from util import build_authoring_system_prompt +from util import build_authoring_system_prompt, looks_like_mdx, format_retry_prompt REPO_ROOT = Path(__file__).resolve().parent.parent OPENAPI_PATH = REPO_ROOT / "api-reference" / "openapi.yaml" @@ -359,13 +359,35 @@ def main(): # Call Claude print("Calling Claude...") client = anthropic.Anthropic() - with client.messages.stream( - model=MODEL, - max_tokens=MAX_TOKENS * len(targets), - system=system_prompt, - messages=[{"role": "user", "content": user_prompt}], - ) as stream: - raw_output = stream.get_final_text() + + def stream_text(messages): + with client.messages.stream( + model=MODEL, + max_tokens=MAX_TOKENS * len(targets), + system=system_prompt, + messages=messages, + ) as stream: + return stream.get_final_text() + + messages = [{"role": "user", "content": user_prompt}] + raw_output = stream_text(messages) + + # A multi-target reply is keyed on `--- SPLIT: ---` markers; a + # single-target one is just the file. Either way, give one corrective turn + # when the shape is wrong instead of writing an unusable draft. + if len(targets) > 1: + ok = "--- SPLIT:" in raw_output + problem = "it contained no `--- SPLIT: ---` markers" + else: + ok = looks_like_mdx(raw_output) + problem = "it did not start with YAML frontmatter (---)" + + if not ok and raw_output.strip(): + print(" Reply was the wrong shape, asking again") + raw_output = stream_text(messages + [ + {"role": "assistant", "content": raw_output[:500].strip()}, + {"role": "user", "content": format_retry_prompt(problem)}, + ]) # Parse output if len(targets) > 1: diff --git a/pipeline/test_prompts.py b/pipeline/test_prompts.py new file mode 100644 index 00000000..356dbf76 --- /dev/null +++ b/pipeline/test_prompts.py @@ -0,0 +1,120 @@ +#!/usr/bin/env python3 +"""Tests for prompt assembly and reply-shape checking. + + python pipeline/test_prompts.py + +The .claude/agents/*.md files are agent definitions written for a harness with +tools. The pipeline reuses their prose in plain API calls where no tools exist, +and when their tool instructions leak through, the model returns a transcript of +itself "researching" instead of a page. These cover both halves of the guard: +stripping the `tools:` frontmatter, and recognizing a reply that isn't a file. +""" + +import sys +import tempfile +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from util import ( # noqa: E402 + NO_TOOLS, + build_authoring_system_prompt, + build_review_system_prompt, + format_retry_prompt, + load_agent, + looks_like_mdx, +) + + +class LoadAgentTest(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.dir = Path(self._tmp.name) + + def write(self, text): + path = self.dir / "agent.md" + path.write_text(text, encoding="utf-8") + return path + + def test_drops_the_frontmatter_including_the_tools_line(self): + path = self.write( + "---\nname: docs-writer\ntools: Read, Write, Grep, Glob, Agent\n---\n\n" + "# Your Role\n\nYou write docs.\n" + ) + body = load_agent(path) + self.assertTrue(body.startswith("# Your Role")) + self.assertNotIn("tools:", body) + self.assertIn("You write docs.", body) + + def test_a_file_without_frontmatter_is_unchanged(self): + path = self.write("# Just Prose\n\nNo frontmatter here.\n") + self.assertEqual(load_agent(path), "# Just Prose\n\nNo frontmatter here.\n") + + def test_body_horizontal_rules_survive(self): + """Only the leading block is frontmatter; --- later is content.""" + path = self.write("---\nname: a\n---\n\n# T\n\nOne\n\n---\n\nTwo\n") + body = load_agent(path) + self.assertIn("\n---\n", body) + self.assertIn("Two", body) + + def test_missing_file_is_empty(self): + self.assertEqual(load_agent(self.dir / "nope.md"), "") + + +class SystemPromptTest(unittest.TestCase): + """Built from the repo's real agent files.""" + + def test_authoring_prompt_carries_the_override_and_no_tools_frontmatter(self): + prompt = build_authoring_system_prompt("You are a documentation writer.") + self.assertIn(NO_TOOLS, prompt) + self.assertNotIn("tools: Read, Write", prompt) + self.assertIn("Output ONLY the .mdx file content", prompt) + + def test_the_override_comes_after_the_guidelines_it_overrides(self): + prompt = build_authoring_system_prompt("role") + self.assertGreater(prompt.index(NO_TOOLS), prompt.index("Docs Writer Guidelines")) + + def test_review_prompt_too(self): + prompt = build_review_system_prompt() + self.assertIn(NO_TOOLS, prompt) + self.assertNotIn("tools: Read, Grep", prompt) + + +class LooksLikeMdxTest(unittest.TestCase): + def test_a_page_passes(self): + self.assertTrue(looks_like_mdx('---\ntitle: "T"\n---\n\nBody.\n')) + + def test_leading_blank_lines_are_tolerated(self): + self.assertTrue(looks_like_mdx('\n\n---\ntitle: T\n---\n\nBody.\n')) + + def test_the_faked_tool_transcript_fails(self): + """The actual failure this guard exists for.""" + reply = ( + "I'll research the GitHub repo and peer pages before writing.\n\n" + '\n{"name": "Glob", "arguments": {"pattern": "docs/**/*.mdx"}}\n' + "\n\n---\ntitle: A peer page\n---\n\n" + ) + self.assertFalse(looks_like_mdx(reply)) + + def test_commentary_then_a_page_fails(self): + self.assertFalse(looks_like_mdx("Here is the page:\n\n---\ntitle: T\n---\n\nBody.\n")) + + def test_unclosed_frontmatter_fails(self): + self.assertFalse(looks_like_mdx("---\ntitle: T\n\nBody with no closing fence.\n")) + + def test_empty_and_none(self): + self.assertFalse(looks_like_mdx("")) + self.assertFalse(looks_like_mdx(None)) + + +class RetryPromptTest(unittest.TestCase): + def test_names_the_problem_and_forbids_a_preamble(self): + prompt = format_retry_prompt("it did not start with YAML frontmatter (---)") + self.assertIn("did not start with YAML frontmatter", prompt) + self.assertIn("No preamble", prompt) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/pipeline/util.py b/pipeline/util.py index 2bbe9294..7d5be317 100644 --- a/pipeline/util.py +++ b/pipeline/util.py @@ -53,6 +53,59 @@ def page_file(page_id, repo_root=REPO_ROOT): return None +def load_agent(path): + """Load an agent file's prose, minus its YAML frontmatter. + + The .claude/agents/*.md files are agent definitions: the frontmatter declares + `tools: Read, Write, Grep, Glob, ...` and the body is written for a harness + where those exist. Pasted verbatim into a plain API call they tell the model + it can search the repo, so it obliges by inventing tool calls and returns a + transcript instead of a page. Drop the frontmatter here; NO_TOOLS below + overrides what the body still assumes. + """ + text = load_text(path) + if text.startswith("---"): + end = text.find("\n---", 3) + if end != -1: + return text[end + 4:].lstrip("\n") + return text + + +NO_TOOLS = """## No Tools In This Context + +You are called through the API with no tools available: + +- Ignore any instruction above to read files, search with Grep or Glob, browse the + web, or delegate to another agent. Everything you need is in this prompt, and + there is nothing else to look up. +- Never emit tool calls, tool results, or a note about research you are about to do. + A reply that opens with "I'll research..." or "Let me read..." is a failed reply. +- Where the guidelines say to return content "using Write or Edit", they mean: put + the content in your reply, and nothing else.""" + + +def looks_like_mdx(text): + """Whether a reply is a page, rather than commentary or a faked transcript.""" + stripped = (text or "").lstrip() + if not stripped.startswith("---"): + return False + return stripped.find("\n---", 3) != -1 # frontmatter block is closed + + +def format_retry_prompt(problem): + """Corrective turn for a reply that came back in the wrong shape. + + The agent guidelines are written for a tool-equipped harness, so the model + sometimes follows them into narrating research it cannot do. One plain + correction recovers that far more safely than trying to cut a page out of a + transcript, which can splice in content the model quoted from elsewhere. + """ + return f"""Your previous reply was not usable: {problem} + +Reply again with the content only. No preamble, no commentary, no tool calls, no +markdown fences, and no explanation of what you changed.""" + + OUTPUT_RULES = """## Output Format - Output ONLY the .mdx file content. No commentary, no explanation, no markdown fences. @@ -80,15 +133,17 @@ def build_authoring_system_prompt(role): ## Docs Writer Guidelines -{load_text(DOCS_WRITER_PATH)} +{load_agent(DOCS_WRITER_PATH)} ## Diataxis Framework -{load_text(DIATAXIS_PATH)} +{load_agent(DIATAXIS_PATH)} ## Information Architecture -{load_text(DOCS_IA_PATH)} +{load_agent(DOCS_IA_PATH)} + +{NO_TOOLS} {OUTPUT_RULES}""" @@ -105,15 +160,17 @@ def build_review_system_prompt(): ## Editorial Review Criteria -{load_text(EDITORIAL_REVIEWER_PATH)} +{load_agent(EDITORIAL_REVIEWER_PATH)} ## Diataxis Framework and Review Criteria -{load_text(DIATAXIS_PATH)} +{load_agent(DIATAXIS_PATH)} ## Information Architecture -{load_text(DOCS_IA_PATH)} +{load_agent(DOCS_IA_PATH)} + +{NO_TOOLS} """ @@ -121,8 +178,8 @@ def load_planning_context(): """IA + Diataxis prose for the batch planner, so its routing rules aren't a third hand-maintained copy of the content-type rules.""" return ( - f"## Information Architecture\n\n{load_text(DOCS_IA_PATH)}\n\n" - f"## Diataxis Framework\n\n{load_text(DIATAXIS_PATH)}" + f"## Information Architecture\n\n{load_agent(DOCS_IA_PATH)}\n\n" + f"## Diataxis Framework\n\n{load_agent(DIATAXIS_PATH)}" ) From fbd2f761ad47c7a6177342cecb5e6a127d4eddf5 Mon Sep 17 00:00:00 2001 From: Shir Goldberg <3937986+shirgoldbird@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:40:25 -0400 Subject: [PATCH 2/3] fix(pipeline): skip hub pages as thin, refuse empty reads, lazy SDK import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Carries over the parts of #450 that #479 didn't cover: - detect_gaps: pages with child pages are exempt from the thin-page check. breaking-changes-change-notices is short because its content sits on its children; padding it out is the wrong fix. - generate: load_existing_page() raises when the page a gap edits is missing or empty, so a failed read can't become "write this page from scratch". - generate: import the Anthropic SDK only when a client is built, so --dry-run and the tests run without it. CI installs pyyaml for the new tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .github/workflows/test-scripts.yml | 4 +- pipeline/detect_gaps.py | 13 ++++- pipeline/generate.py | 21 ++++++- pipeline/test_hub_and_reads.py | 92 ++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 6 deletions(-) create mode 100644 pipeline/test_hub_and_reads.py diff --git a/.github/workflows/test-scripts.yml b/.github/workflows/test-scripts.yml index 5707b9d7..f54a59d6 100644 --- a/.github/workflows/test-scripts.yml +++ b/.github/workflows/test-scripts.yml @@ -30,5 +30,7 @@ jobs: - uses: actions/setup-python@v5 with: python-version: "3.12" - # Stdlib only — these tests stub out gh and never call the network. + # pyyaml only: the tests stub out gh, never call the network, and the + # Anthropic SDK is imported lazily so it isn't needed here. + - run: pip install pyyaml - run: python -m unittest discover -s pipeline -p "test_*.py" -v diff --git a/pipeline/detect_gaps.py b/pipeline/detect_gaps.py index e54b5f0c..19e9ddef 100644 --- a/pipeline/detect_gaps.py +++ b/pipeline/detect_gaps.py @@ -177,6 +177,12 @@ def page_frontmatter(page_path): return None, None # page listed in nav but file missing +def has_child_pages(all_pages, page_path): + """Whether other nav entries sit under this one, making it a hub page.""" + prefix = page_path + "/" + return any(p.startswith(prefix) for p in all_pages) + + def slug(page_path): return page_path.rsplit("/", 1)[-1].lower() @@ -331,14 +337,17 @@ def detect_gaps(docs_json, endpoints, families, section=None, force=False): # --- Doc-quality checks on non-reference pages ------------------------ # if not section: - for p in all_doc_pages(docs_json, apiref_tab): + doc_pages = all_doc_pages(docs_json, apiref_tab) + for p in doc_pages: fm, content = page_frontmatter(p) if fm is None: continue if not fm.get("description"): gaps.append(gap("missing_description", "low", None, path=page_file(p) or p, desc=f"{p} has no frontmatter description")) - if len(strip_frontmatter(content).split()) < 100: + # A page with children is a hub: it's short because the content sits + # on the child pages. Padding it out is the wrong fix, so don't ask. + if len(strip_frontmatter(content).split()) < 100 and not has_child_pages(doc_pages, p): gaps.append(gap("thin_page", "medium", None, path=page_file(p) or p, desc=f"{p} has under 100 words")) diff --git a/pipeline/generate.py b/pipeline/generate.py index ca7840a9..9a7bb358 100644 --- a/pipeline/generate.py +++ b/pipeline/generate.py @@ -67,6 +67,21 @@ def load_file(path): return "" +def load_existing_page(gap): + """Content of the page a gap edits. Raises rather than returning nothing. + + An empty read would turn "expand this page" into "write one from scratch" + and silently discard the real content. + """ + page_path = REPO_ROOT / gap.get("path", "") + if not page_path.is_file(): + raise FileNotFoundError(f"no file at '{gap.get('path', '')}' for {gap['type']} gap") + content = load_file(page_path) + if not content.strip(): + raise ValueError(f"{gap['path']} is empty, refusing to rewrite it from nothing") + return content + + def load_openapi_for_tags(tags): """Extract the portion of the OpenAPI spec whose endpoints carry any of `tags`.""" with open(OPENAPI_PATH) as f: @@ -215,7 +230,7 @@ def build_generation_prompt(gap, family_name, openapi_context, existing_docs_con elif gap_type == "thin_page": page_path = gap.get("path", "") - page_content = load_file(REPO_ROOT / page_path) + page_content = load_existing_page(gap) return f"""Expand this thin page. It currently has only {gap.get('word_count', 0)} words. Current content of {page_path}: @@ -232,7 +247,7 @@ def build_generation_prompt(gap, family_name, openapi_context, existing_docs_con elif gap_type == "missing_code_examples": page_path = gap.get("path", "") - page_content = load_file(REPO_ROOT / page_path) + page_content = load_existing_page(gap) return f"""Add code examples to this page. It's a guide but has no runnable code. Current content of {page_path}: @@ -249,7 +264,7 @@ def build_generation_prompt(gap, family_name, openapi_context, existing_docs_con elif gap_type == "missing_description": page_path = gap.get("path", "") - page_content = load_file(REPO_ROOT / page_path)[:500] + page_content = load_existing_page(gap)[:500] return f"""Generate a frontmatter description for this page. Current content of {page_path} (first 500 chars): diff --git a/pipeline/test_hub_and_reads.py b/pipeline/test_hub_and_reads.py new file mode 100644 index 00000000..113e3b34 --- /dev/null +++ b/pipeline/test_hub_and_reads.py @@ -0,0 +1,92 @@ +#!/usr/bin/env python3 +"""Tests for the thin-page hub exemption and for reading the page a gap edits. + + python pipeline/test_hub_and_reads.py + +Runs against a temp docs tree, so no repo file is read and none is written. +""" + +import sys +import tempfile +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +import detect_gaps # noqa: E402 +import generate # noqa: E402 + + +class TempDocsTree(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.root = Path(self._tmp.name) + self._saved = (detect_gaps.REPO_ROOT, generate.REPO_ROOT) + detect_gaps.REPO_ROOT = self.root + generate.REPO_ROOT = self.root + self.addCleanup(self._restore) + + def _restore(self): + detect_gaps.REPO_ROOT, generate.REPO_ROOT = self._saved + self._tmp.cleanup() + + def write(self, rel, text="---\ntitle: T\n---\n\nSome body copy.\n"): + path = self.root / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + + +class HasChildPagesTest(unittest.TestCase): + def test_a_page_with_children_is_a_hub(self): + pages = ["docs/notices", "docs/notices/july-2024"] + self.assertTrue(detect_gaps.has_child_pages(pages, "docs/notices")) + + def test_a_leaf_is_not(self): + self.assertFalse(detect_gaps.has_child_pages(["docs/a", "docs/b"], "docs/a")) + + def test_a_name_prefix_is_not_a_child(self): + """'docs/x-ray' must not count as a child of 'docs/x'.""" + self.assertFalse(detect_gaps.has_child_pages(["docs/x", "docs/x-ray"], "docs/x")) + + +class ThinPageDetectionTest(TempDocsTree): + def test_hub_is_not_flagged_but_a_real_thin_page_is(self): + short = "---\ntitle: T\ndescription: d\n---\n\nToo short.\n" + for page in ("docs/hub", "docs/hub/child", "docs/lonely"): + self.write(f"{page}.mdx", short) + docs_json = {"navigation": {"tabs": [ + {"tab": "Home", "pages": ["docs/hub", "docs/hub/child", "docs/lonely"]}, + ]}} + + thin = [g["description"] for g in detect_gaps.detect_gaps(docs_json, [], {}) + if g["type"] == "thin_page"] + self.assertEqual(sorted(thin), [ + "docs/hub/child has under 100 words", # a child can still be thin + "docs/lonely has under 100 words", + ]) + + +class LoadExistingPageTest(TempDocsTree): + def test_reads_the_page(self): + self.write("docs/a.mdx", "---\ntitle: T\n---\n\nReal content.\n") + page = generate.load_existing_page({"type": "thin_page", "path": "docs/a.mdx"}) + self.assertIn("Real content.", page) + + def test_a_missing_file_raises_instead_of_returning_nothing(self): + """An empty read used to become "write this page from scratch".""" + with self.assertRaises(FileNotFoundError): + generate.load_existing_page({"type": "thin_page", "path": "docs/ghost.mdx"}) + + def test_a_directory_raises_file_not_found_not_is_a_directory(self): + self.write("docs/a/child.mdx") + with self.assertRaises(FileNotFoundError): + generate.load_existing_page({"type": "thin_page", "path": "docs/a"}) + + def test_an_empty_page_raises(self): + self.write("docs/blank.mdx", " \n") + with self.assertRaises(ValueError): + generate.load_existing_page({"type": "thin_page", "path": "docs/blank.mdx"}) + + +if __name__ == "__main__": + unittest.main(verbosity=2) From 343effeb42f2a54e274e7bccfb47e96c38225b49 Mon Sep 17 00:00:00 2001 From: Shir Goldberg <3937986+shirgoldbird@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:55:01 -0400 Subject: [PATCH 3/3] fix(pipeline): fail fast on a second bad reply; test the corrective turn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on #451: - generate_content raises if the reply is still not a page after the corrective turn. main() records that as an error for that gap only; writing it as a draft would fail evaluate for the whole run. - rework exits with an error in the same case instead of writing the reply. - Fake-client tests for generate_content: one call on a good reply, a user/assistant/user retry on a bad one, a raise on two bad ones, and no shape check for descriptions. - Pin pyyaml in the test workflow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .github/workflows/test-scripts.yml | 2 +- pipeline/generate.py | 9 ++++-- pipeline/rework.py | 5 +++ pipeline/test_prompts.py | 49 ++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test-scripts.yml b/.github/workflows/test-scripts.yml index f54a59d6..ec913ac0 100644 --- a/.github/workflows/test-scripts.yml +++ b/.github/workflows/test-scripts.yml @@ -32,5 +32,5 @@ jobs: python-version: "3.12" # pyyaml only: the tests stub out gh, never call the network, and the # Anthropic SDK is imported lazily so it isn't needed here. - - run: pip install pyyaml + - run: pip install pyyaml==6.0.3 - run: python -m unittest discover -s pipeline -p "test_*.py" -v diff --git a/pipeline/generate.py b/pipeline/generate.py index 9a7bb358..99878b96 100644 --- a/pipeline/generate.py +++ b/pipeline/generate.py @@ -304,7 +304,9 @@ def generate_content(client, system_prompt, user_prompt, expect_file=True): A reply that isn't a page gets one corrective turn rather than failing the run. The docs-writer guidelines are written for a harness with tools, so the model occasionally obliges by narrating research it cannot do and returns a - transcript instead of a file. + transcript instead of a file. If the second reply is still not a page, raise: + main() records that as an error for this gap alone, whereas writing it as a + draft would fail evaluate for the whole run. """ messages = [{"role": "user", "content": user_prompt}] text = _complete(client, system_prompt, messages) @@ -320,7 +322,10 @@ def generate_content(client, system_prompt, user_prompt, expect_file=True): {"role": "user", "content": format_retry_prompt( "it did not start with YAML frontmatter (---)")}, ] - return _complete(client, system_prompt, messages) + text = _complete(client, system_prompt, messages) + if not looks_like_mdx(text): + raise ValueError("reply was still not a page after one corrective turn") + return text def determine_output_path(gap, family_name, content=None): diff --git a/pipeline/rework.py b/pipeline/rework.py index 2038edf4..e1abc580 100644 --- a/pipeline/rework.py +++ b/pipeline/rework.py @@ -388,6 +388,11 @@ def stream_text(messages): {"role": "assistant", "content": raw_output[:500].strip()}, {"role": "user", "content": format_retry_prompt(problem)}, ]) + ok = "--- SPLIT:" in raw_output if len(targets) > 1 else looks_like_mdx(raw_output) + if not ok: + print(f"Error: reply was still the wrong shape after one corrective turn ({problem})", + file=sys.stderr) + sys.exit(1) # Parse output if len(targets) > 1: diff --git a/pipeline/test_prompts.py b/pipeline/test_prompts.py index 356dbf76..e0a2653e 100644 --- a/pipeline/test_prompts.py +++ b/pipeline/test_prompts.py @@ -17,6 +17,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) +import generate # noqa: E402 from util import ( # noqa: E402 NO_TOOLS, build_authoring_system_prompt, @@ -116,5 +117,53 @@ def test_names_the_problem_and_forbids_a_preamble(self): self.assertIn("No preamble", prompt) +class FakeClient: + """Stands in for anthropic.Anthropic: returns canned replies, records calls.""" + + def __init__(self, *replies): + self._replies = list(replies) + self.calls = [] + self.messages = self + + def create(self, **kwargs): + # Copy the list: generate_content extends it in place between calls. + self.calls.append([dict(m) for m in kwargs["messages"]]) + text = self._replies.pop(0) + return type("Response", (), {"content": [type("Block", (), {"text": text})()]})() + + +class GenerateContentTest(unittest.TestCase): + PAGE = '---\ntitle: "T"\n---\n\nBody.\n' + TRANSCRIPT = "I'll research the peer pages first.\n{}\n" + + def test_a_page_on_the_first_try_takes_one_call(self): + client = FakeClient(self.PAGE) + self.assertEqual(generate.generate_content(client, "sys", "write it"), self.PAGE) + self.assertEqual(len(client.calls), 1) + + def test_a_bad_reply_gets_one_corrective_turn(self): + client = FakeClient(self.TRANSCRIPT, self.PAGE) + self.assertEqual(generate.generate_content(client, "sys", "write it"), self.PAGE) + + self.assertEqual(len(client.calls), 2) + retry = client.calls[1] + self.assertEqual([m["role"] for m in retry], ["user", "assistant", "user"]) + self.assertEqual(retry[0]["content"], "write it") + self.assertTrue(retry[1]["content"].startswith("I'll research")) + self.assertIn("did not start with YAML frontmatter", retry[2]["content"]) + + def test_two_bad_replies_raise_instead_of_returning_a_transcript(self): + client = FakeClient(self.TRANSCRIPT, self.TRANSCRIPT) + with self.assertRaises(ValueError): + generate.generate_content(client, "sys", "write it") + self.assertEqual(len(client.calls), 2) + + def test_a_description_reply_is_not_shape_checked(self): + client = FakeClient("Learn how to translate a spreadsheet.") + reply = generate.generate_content(client, "sys", "describe it", expect_file=False) + self.assertEqual(reply, "Learn how to translate a spreadsheet.") + self.assertEqual(len(client.calls), 1) + + if __name__ == "__main__": unittest.main(verbosity=2)