fix: tighten tool dispatch sync test per review feedback

Author: clean6378-max-it

CONTRIBUTING.md

@@ -147,7 +147,7 @@ Claude Code assistant `tool_use` blocks carry a `name` string (e.g. `"Read"`, `"
 
 1. **`utils/tool_dispatch.py`** — add the name to `_FILE_ACTIVITY_HANDLERS` (`None` if no file/bash/web side effects); `KNOWN_TOOL_TYPES` is derived from its keys. If the tool has a distinct `toolUseResult` JSON shape, add `(predicate, builder)` to `_TOOL_RESULT_DISPATCH` (respect ordering — see module docstring and `tests/test_tool_dispatch_ordering.py`).
 2. **`models/tool_results.py`** — add the name to `ToolNameLiteral` and, when the tool has a distinct result payload, add the TypedDict, type guard (`is_*_tool_result`), and union member on `ToolResultUnion`.
-3. **`utils/md_exporter.py`** — add an `elif name == "…"` branch in `_render_tool_use` and include the name in `MD_EXPORTER_TOOL_TYPES`.
+3. **`utils/md_exporter.py`** — add an `elif name == "…"` branch in `_render_tool_use` (sync test parses these branches).
 4. **`static/js/render/registry.js`** — add a `TOOL_USE_RENDERERS` entry (and a `tool_use/*.js` renderer module).
 5. **Optional result UI** — if the backend emits a new `result_type`, add `TOOL_RESULT_RENDERERS` and a `tool_result/*.js` module.
 6. Run `pytest tests/test_tool_dispatch_sync.py -v` — failure names the site missing the new type.

tests/test_tool_dispatch_sync.py

@@ -1,59 +1,99 @@
 """Contract test: ``KNOWN_TOOL_TYPES`` must match all four dispatch sites.
 
-Sites:
-- ``utils/tool_dispatch.py`` — ``KNOWN_TOOL_TYPES`` / ``FILE_ACTIVITY_TOOL_TYPES``
-- ``utils/md_exporter.py`` — ``MD_EXPORTER_TOOL_TYPES``
-- ``static/js/render/registry.js`` — ``TOOL_USE_RENDERERS`` keys
+Sites (each compared to ``KNOWN_TOOL_TYPES`` in ``utils/tool_dispatch.py``):
+- ``utils/md_exporter.py`` — ``_render_tool_use`` if/elif branches (parsed)
+- ``models/tool_results.py`` — ``ToolNameLiteral``
+- ``static/js/render/registry.js`` — ``TOOL_USE_RENDERERS`` keys (parsed)
 """
 
 from __future__ import annotations
 
 import re
 from pathlib import Path
+from typing import get_args
 
 import pytest
 
-from utils.md_exporter import MD_EXPORTER_TOOL_TYPES
-from utils.tool_dispatch import FILE_ACTIVITY_TOOL_TYPES, KNOWN_TOOL_TYPES
+from models.tool_results import ToolNameLiteral
+from utils.tool_dispatch import KNOWN_TOOL_TYPES
 
 _REPO_ROOT = Path(__file__).resolve().parents[1]
 _FRONTEND_REGISTRY = _REPO_ROOT / "static" / "js" / "render" / "registry.js"
+_MD_EXPORTER = _REPO_ROOT / "utils" / "md_exporter.py"
+
+
+def _format_set_diff(expected: frozenset[str], actual: frozenset[str], site: str) -> str:
+    missing = sorted(expected - actual)
+    extra = sorted(actual - expected)
+    parts: list[str] = []
+    if missing:
+        parts.append(f"missing tool type(s) {missing!r} in {site}")
+    if extra:
+        parts.append(f"unexpected tool type(s) {extra!r} in {site}")
+    return "; ".join(parts)
 
 
 def _parse_frontend_tool_use_renderers(path: Path) -> frozenset[str]:
+    """Extract ``TOOL_USE_RENDERERS`` keys.
+
+    Assumes values are bare identifiers (``Bash: renderBashUse``). Brace-depth
+    parsing avoids truncating the object body if a value ever contains ``}``.
+    """
+    text = path.read_text(encoding="utf-8")
+    marker = "export const TOOL_USE_RENDERERS = {"
+    start = text.find(marker)
+    if start == -1:
+        msg = f"Could not find TOOL_USE_RENDERERS in {path}"
+        raise ValueError(msg)
+    i = start + len(marker)
+    depth = 1
+    body_start = i
+    while i < len(text) and depth > 0:
+        ch = text[i]
+        if ch == "{":
+            depth += 1
+        elif ch == "}":
+            depth -= 1
+        i += 1
+    if depth != 0:
+        msg = f"Unbalanced braces in TOOL_USE_RENDERERS in {path}"
+        raise ValueError(msg)
+    body = text[body_start : i - 1]
+    keys = re.findall(r"^\s*(\w+)\s*:", body, re.MULTILINE)
+    return frozenset(keys)
+
+
+def _parse_md_exporter_tool_use_handlers(path: Path) -> frozenset[str]:
+    """Extract tool names handled by ``_render_tool_use`` if/elif branches."""
     text = path.read_text(encoding="utf-8")
     match = re.search(
-        r"export const TOOL_USE_RENDERERS = \{([^}]+)\}",
+        r"def _render_tool_use\(.*?(?=\ndef _render_tool_result)",
         text,
         re.DOTALL,
     )
     if not match:
-        msg = f"Could not find TOOL_USE_RENDERERS in {path}"
+        msg = f"Could not find _render_tool_use in {path}"
         raise ValueError(msg)
-    body = match.group(1)
-    keys = re.findall(r"^\s*(\w+)\s*:", body, re.MULTILINE)
-    return frozenset(keys)
+    body = match.group(0)
+    names = set(re.findall(r'(?:if|elif) name == "([^"]+)"', body))
+    for tuple_match in re.finditer(r"elif name in \(([^)]+)\)", body):
+        names.update(re.findall(r'"([^"]+)"', tuple_match.group(1)))
+    return frozenset(names)
 
 
-def _format_set_diff(expected: frozenset[str], actual: frozenset[str], site: str) -> str:
-    missing = sorted(expected - actual)
-    extra = sorted(actual - expected)
-    parts: list[str] = []
-    if missing:
-        parts.append(f"missing tool type(s) {missing!r} in {site}")
-    if extra:
-        parts.append(f"unexpected tool type(s) {extra!r} in {site}")
-    return "; ".join(parts)
+def test_md_exporter_handlers_match_known_tool_types() -> None:
+    site = "utils/md_exporter.py (_render_tool_use branches)"
+    try:
+        actual = _parse_md_exporter_tool_use_handlers(_MD_EXPORTER)
+    except ValueError as exc:
+        pytest.fail(f"{site}: {exc}")
+    if actual != KNOWN_TOOL_TYPES:
+        pytest.fail(_format_set_diff(KNOWN_TOOL_TYPES, actual, site))
 
 
-@pytest.mark.parametrize(
-    ("site", "actual"),
-    [
-        ("utils/tool_dispatch.py (FILE_ACTIVITY_TOOL_TYPES)", FILE_ACTIVITY_TOOL_TYPES),
-        ("utils/md_exporter.py (MD_EXPORTER_TOOL_TYPES)", MD_EXPORTER_TOOL_TYPES),
-    ],
-)
-def test_tool_type_sets_match_known_registry(site: str, actual: frozenset[str]) -> None:
+def test_tool_name_literal_matches_known_tool_types() -> None:
+    site = "models/tool_results.py (ToolNameLiteral)"
+    actual = frozenset(get_args(ToolNameLiteral))
     if actual != KNOWN_TOOL_TYPES:
         pytest.fail(_format_set_diff(KNOWN_TOOL_TYPES, actual, site))
 

utils/md_exporter.py

@@ -323,32 +323,11 @@ def _render_tool_use(tool: ToolUseDict) -> str:
                     continue
                 lines.append(f">\n> Q: {q.get('question', '')}")
     else:
-        if name in MD_EXPORTER_TOOL_TYPES:
-            label = "Input (known tool type; no specialized Markdown renderer)"
-        else:
-            label = "Input (unknown tool type)"
-        lines.append(f">\n> {label}: `{str(inp)}`")
+        lines.append(f">\n> Input (unknown tool type): `{str(inp)}`")
 
     return "\n".join(lines)
 
 
-MD_EXPORTER_TOOL_TYPES: frozenset[str] = frozenset(
-    {
-        "AskUserQuestion",
-        "Bash",
-        "Edit",
-        "Glob",
-        "Grep",
-        "Read",
-        "Task",
-        "TodoWrite",
-        "WebFetch",
-        "WebSearch",
-        "Write",
-    }
-)
-
-
 def _render_tool_result(parsed: dict[str, Any]) -> str:
     """Format a tool result nicely instead of dumping raw JSON."""
     rt = parsed.get("result_type", "unknown")

utils/tool_dispatch.py

@@ -286,9 +286,7 @@ def _file_activity_web(tool_input: dict[str, Any], metadata: dict[str, Any]) ->
     "WebSearch": _file_activity_web,
     "Write": _file_activity_write,
 }
-KNOWN_TOOL_TYPE_NAMES: tuple[str, ...] = tuple(sorted(_FILE_ACTIVITY_HANDLERS))
 KNOWN_TOOL_TYPES: frozenset[str] = frozenset(_FILE_ACTIVITY_HANDLERS)
-FILE_ACTIVITY_TOOL_TYPES: frozenset[str] = frozenset(_FILE_ACTIVITY_HANDLERS)
 
 
 def track_tool_file_activity(