fix(pipeline): stop the agent files telling the model it has tools

.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==6.0.3
       - run: python -m unittest discover -s pipeline -p "test_*.py" -v

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

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

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
@@ -74,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:
@@ -136,6 +144,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(
@@ -208,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}:
@@ -225,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}:
@@ -242,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):
@@ -266,17 +288,46 @@ 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. 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)
+
+    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 (---)")},
+    ]
+    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):
     """Determine where to write the generated content."""
     gap_type = gap["type"]
@@ -426,7 +477,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 +505,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("'")

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,40 @@ 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: <path> ---` 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: <path> ---` 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)},
+        ])
+        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:

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)