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)}" )