feat(stage5): label-strip retry for package overview diagrams

src/designdoc/stages/s5_mermaid.py

@@ -167,10 +167,19 @@ async def _emit_package_diagrams(state: PipelineState, packages_dir) -> None:
 
         validation = validate(merged)
         if not validation.ok:
-            # Fail soft: leave the README diagram-less rather than crash the
-            # stage. Stage 5 is best-effort by design (per-class diagrams
-            # already use the same HIL-or-ship pattern).
-            continue
+            # One retry with arrow labels stripped — mmdc's relationship-label
+            # grammar rejects `;`, sometimes chokes on `()`, and has fragile
+            # quoting rules. Stripping every label to a bare arrow is the
+            # universal escape hatch: less informative than a labelled
+            # diagram, but still a useful bird's-eye view.
+            stripped = _strip_arrow_labels(merged)
+            if stripped != merged and validate(stripped).ok:
+                merged = stripped
+            else:
+                # Fail soft: leave the README diagram-less rather than crash
+                # the stage. Stage 5 is best-effort by design (per-class
+                # diagrams already use the same HIL-or-ship pattern).
+                continue
 
         body = _strip_diagram_section(pkg_text)
         section = f"\n\n## Diagram\n\n```mermaid\n{merged}\n```\n"
@@ -227,6 +236,22 @@ def _merge_class_diagrams(blocks: list[str]) -> str:
     return "\n".join(lines)
 
 
+def _strip_arrow_labels(diagram_text: str) -> str:
+    """Drop the `: label` suffix from every relationship-arrow line in a
+    mermaid classDiagram. Used as a one-shot retry escape hatch when the
+    full merged diagram fails mmdc validation due to label-grammar quirks
+    (see issue #59 for the agent-brain config-package failure case).
+    """
+    out: list[str] = []
+    for line in diagram_text.splitlines():
+        if any(op in line for op in _ARROW_OPS) and not line.lstrip().startswith("class "):
+            head, sep, _label = line.partition(":")
+            out.append(head.rstrip() if sep else line)
+        else:
+            out.append(line)
+    return "\n".join(out)
+
+
 def _strip_diagram_section(text: str) -> str:
     """Return the class doc text with any trailing '## Diagram' section removed."""
     return _DIAGRAM_SECTION_RE.sub("", text).rstrip() + "\n"

tests/unit/test_stage5_package_diagrams.py

@@ -12,7 +12,12 @@
 
 from __future__ import annotations
 
-from designdoc.stages.s5_mermaid import _merge_class_diagrams
+from pathlib import Path
+
+import pytest
+
+from designdoc.stages import s5_mermaid
+from designdoc.stages.s5_mermaid import _merge_class_diagrams, _strip_arrow_labels
 
 
 def test_merge_extracts_class_names_from_inputs():
@@ -106,3 +111,165 @@ def test_merge_ignores_non_classdiagram_blocks():
     assert "class Gateway" in merged
     assert "flowchart" not in merged
     assert "A --> B" not in merged
+
+
+# ----- label-stripping retry (issue #59) -----------------------------------
+
+
+def test_strip_arrow_labels_removes_text_after_colon_on_arrow_lines():
+    """The agent-brain dogfood found `Settings ..> ValidationError : calls
+    get_api_key(); raises ValidationError` — `;` in the label broke mmdc.
+    Stripping everything after the `:` on arrow lines is the universal
+    escape hatch."""
+    text = (
+        "classDiagram\n"
+        "    class Settings\n"
+        "    class ValidationError\n"
+        "    Settings ..> ValidationError : calls get_api_key(); raises ValidationError\n"
+    )
+
+    stripped = _strip_arrow_labels(text)
+
+    assert "Settings ..> ValidationError" in stripped
+    arrow_line = next(
+        line for line in stripped.splitlines() if "Settings ..> ValidationError" in line
+    )
+    assert ":" not in arrow_line, f"label not stripped from: {arrow_line!r}"
+    assert "calls get_api_key" not in stripped
+
+
+def test_strip_arrow_labels_leaves_unlabelled_arrows_alone():
+    """An arrow line with no `:` should be returned untouched — leading
+    indentation (which mermaid renders) preserved, no dropped lines."""
+    text = "classDiagram\n    class A\n    class B\n    A --> B\n"
+
+    stripped = _strip_arrow_labels(text)
+
+    assert "A --> B" in stripped
+    # Leading whitespace stays put — only the trailing `: label` is removed.
+    arrow_line = next(line for line in stripped.splitlines() if "A --> B" in line)
+    assert arrow_line == "    A --> B"
+
+
+def test_strip_arrow_labels_preserves_non_arrow_lines():
+    """`class Foo` declarations contain no arrow ops, so their bodies (and
+    any `:` inside) must survive intact."""
+    text = (
+        "classDiagram\n"
+        "    class Foo {\n"
+        "        +bar: str\n"
+        "    }\n"
+        "    class Bar\n"
+        "    Foo --> Bar : uses\n"
+    )
+
+    stripped = _strip_arrow_labels(text)
+
+    # class-body `:` is preserved
+    assert "+bar: str" in stripped
+    # arrow-line label is stripped
+    assert "Foo --> Bar" in stripped
+    assert ": uses" not in stripped
+
+
+@pytest.mark.anyio
+async def test_emit_package_diagrams_retries_with_stripped_labels(tmp_path: Path, monkeypatch):
+    """When the first merged diagram fails mmdc validation, _emit_package_diagrams
+    should retry once with arrow labels stripped, and write the stripped
+    diagram if that retry passes. Without this, packages with one parse-unfriendly
+    label ship diagram-less even though the merge logic produced something useful."""
+    from designdoc.mermaid.mmdc import ValidationResult
+    from designdoc.state import PipelineState
+
+    output = tmp_path / "design"
+    pkg_dir = output / "packages" / "config"
+    classes_dir = pkg_dir / "classes"
+    classes_dir.mkdir(parents=True)
+
+    # Two class docs whose mermaid blocks merge into a diagram that contains
+    # a label. Whether the label is real-world-toxic doesn't matter — we
+    # mock validate() below to simulate mmdc rejecting it.
+    (classes_dir / "Settings.md").write_text(
+        "# Settings\n\n## Diagram\n\n"
+        "```mermaid\n"
+        "classDiagram\n"
+        "    class Settings\n"
+        "    class ValidationError\n"
+        "    Settings ..> ValidationError : calls get_api_key(); raises\n"
+        "```\n"
+    )
+    (classes_dir / "Loader.md").write_text(
+        "# Loader\n\n## Diagram\n\n"
+        "```mermaid\n"
+        "classDiagram\n"
+        "    class Loader\n"
+        "    class Settings\n"
+        "    Loader --> Settings : builds\n"
+        "```\n"
+    )
+    (pkg_dir / "README.md").write_text("# config\n\n## Overview\nConfig handling.\n")
+
+    # Capture every input handed to validate so we can assert the second
+    # call received label-stripped text. First call → fail, second → pass.
+    calls: list[str] = []
+
+    def fake_validate(text: str, *, timeout: float = 30.0):
+        calls.append(text)
+        if len(calls) == 1:
+            return ValidationResult(ok=False, stderr="Parse error (simulated)")
+        return ValidationResult(ok=True, stderr="")
+
+    monkeypatch.setattr(s5_mermaid, "validate", fake_validate)
+
+    state = PipelineState.load_or_new(output_dir=output, target_repo=tmp_path)
+
+    await s5_mermaid._emit_package_diagrams(state, output / "packages")
+
+    readme_text = (pkg_dir / "README.md").read_text()
+    assert "## Diagram" in readme_text, "label-strip retry must succeed and write a Diagram section"
+    assert "```mermaid" in readme_text
+    # First call had labels (": calls get_api_key" or ": builds"); the
+    # second call (which we approved) had them stripped.
+    assert len(calls) == 2, "expected one retry after the first failure"
+    assert ":" in calls[0].split("Settings ..> ValidationError", 1)[1].splitlines()[0]
+    # The written README contains the stripped version, not the labelled one.
+    assert "calls get_api_key" not in readme_text
+    assert "builds" not in readme_text.split("## Diagram", 1)[1]
+
+
+@pytest.mark.anyio
+async def test_emit_package_diagrams_stays_failsoft_when_retry_also_fails(
+    tmp_path: Path, monkeypatch
+):
+    """If both the original merge AND the label-stripped retry fail mmdc,
+    behaviour should fall back to the v1 fail-soft path: leave the README
+    diagram-less rather than crash the stage."""
+    from designdoc.mermaid.mmdc import ValidationResult
+    from designdoc.state import PipelineState
+
+    output = tmp_path / "design"
+    pkg_dir = output / "packages" / "config"
+    classes_dir = pkg_dir / "classes"
+    classes_dir.mkdir(parents=True)
+    (classes_dir / "A.md").write_text(
+        "# A\n\n## Diagram\n\n"
+        "```mermaid\n"
+        "classDiagram\n    class A\n    class B\n    A --> B : x\n"
+        "```\n"
+    )
+    pre_readme = "# config\n\n## Overview\nx\n"
+    (pkg_dir / "README.md").write_text(pre_readme)
+
+    monkeypatch.setattr(
+        s5_mermaid,
+        "validate",
+        lambda text, *, timeout=30.0: ValidationResult(ok=False, stderr="fail"),
+    )
+
+    state = PipelineState.load_or_new(output_dir=output, target_repo=tmp_path)
+
+    await s5_mermaid._emit_package_diagrams(state, output / "packages")
+
+    assert (pkg_dir / "README.md").read_text() == pre_readme, (
+        "README must be untouched when both validate attempts fail"
+    )