From 57686ed582d3f9c21b26153aaa4d43e231023c45 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] 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 | 39 +++++++++++-- pipeline/rework.py | 38 ++++++++++--- pipeline/test_prompts.py | 120 +++++++++++++++++++++++++++++++++++++++ pipeline/util.py | 73 +++++++++++++++++++++--- 5 files changed, 266 insertions(+), 21 deletions(-) create mode 100644 pipeline/test_prompts.py diff --git a/pipeline/README.md b/pipeline/README.md index cf69654d..a9063374 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 83a1e058..1c63f4a1 100644 --- a/pipeline/generate.py +++ b/pipeline/generate.py @@ -30,7 +30,7 @@ import yaml -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 from detect_gaps import page_file @@ -291,17 +291,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"] @@ -488,7 +512,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 c93769de..969a32ae 100644 --- a/pipeline/util.py +++ b/pipeline/util.py @@ -38,6 +38,59 @@ def load_text(path): return "" +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. @@ -65,15 +118,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}""" @@ -90,15 +145,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} """ @@ -106,8 +163,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)}" )