From 9104d31c2d1fae6bb47e5bba342910282d086a5c Mon Sep 17 00:00:00 2001 From: yu-med Date: Wed, 1 Jul 2026 01:11:35 +0800 Subject: [PATCH 1/4] refactor: single KNOWN_TOOL_TYPES registry with sync CI test Adding a Claude Code tool use name required coordinated edits across tool_dispatch, jsonl_parser file activity, md_exporter, and the frontend registry with no test-time guard against drift. Define KNOWN_TOOL_TYPES in utils/tool_dispatch.py, move file-activity tracking to track_tool_file_activity(), wire md_exporter to the registry, and add tests/test_tool_dispatch_sync.py to assert all four sites stay in sync. Document the add-a-tool-type procedure in CONTRIBUTING.md. --- CONTRIBUTING.md | 11 ++++ models/tool_results.py | 1 + tests/test_tool_dispatch_sync.py | 66 ++++++++++++++++++++++ utils/jsonl_parser.py | 25 +-------- utils/md_exporter.py | 21 ++++++- utils/tool_dispatch.py | 94 +++++++++++++++++++++++++++++++- 6 files changed, 193 insertions(+), 25 deletions(-) create mode 100644 tests/test_tool_dispatch_sync.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a123b2f..28276d5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -100,6 +100,7 @@ npm run test:coverage # optional | New `ErrorCode` | Parametrized row in `tests/test_error_codes.py` | | Search / limit validation | `tests/test_search.py` | | New `_parse_tool_result` dispatch entry | Fixture + assertion in `tests/test_jsonl_parser.py` | +| New Claude Code tool use name | See **Adding a new tool type** below | | CLI behavior | `tests/test_cli_e2e.py` (subprocess) or `tests/test_cli_args.py` (parser only) | | Frontend shared module | `static/js/shared/*.test.js` (vitest) | | Error response shape | `tests/test_error_propagation.py` regression | @@ -140,6 +141,16 @@ npm run test:coverage # optional See [`docs/architecture.md`](docs/architecture.md) for data flow, export state machine, and component diagram. +## Adding a new tool type + +Claude Code assistant `tool_use` blocks carry a `name` string (e.g. `"Read"`, `"Bash"`). The browser coordinates that name across four sites; drift is caught by `tests/test_tool_dispatch_sync.py`. + +1. **`utils/tool_dispatch.py`** — add the name to `KNOWN_TOOL_TYPE_NAMES` (keep alphabetical). Set `_FILE_ACTIVITY_HANDLERS[name]` to a tracker function or `None`. 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. **`utils/md_exporter.py`** — add an `elif name == "…"` branch in `_render_tool_use` and include the name in `MD_EXPORTER_TOOL_TYPES`. +3. **`static/js/render/registry.js`** — add a `TOOL_USE_RENDERERS` entry (and a `tool_use/*.js` renderer module). +4. **Optional result UI** — if the backend emits a new `result_type`, add `TOOL_RESULT_RENDERERS` and a `tool_result/*.js` module. +5. Run `pytest tests/test_tool_dispatch_sync.py -v` — failure names the site missing the new type. + ## Getting help Open an issue with a clear repro or propose a draft PR early for CI feedback. diff --git a/models/tool_results.py b/models/tool_results.py index ca881b7..7b411ee 100644 --- a/models/tool_results.py +++ b/models/tool_results.py @@ -220,6 +220,7 @@ def is_user_input_tool_result(tr: ToolResultDict) -> TypeGuard[UserInputToolResu # Tool names on assistant tool_use blocks — pairs with slug on user tool_result rows. ToolNameLiteral = Literal[ + "AskUserQuestion", "Bash", "Read", "Write", diff --git a/tests/test_tool_dispatch_sync.py b/tests/test_tool_dispatch_sync.py new file mode 100644 index 0000000..3af867d --- /dev/null +++ b/tests/test_tool_dispatch_sync.py @@ -0,0 +1,66 @@ +"""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 +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +from utils.md_exporter import MD_EXPORTER_TOOL_TYPES +from utils.tool_dispatch import FILE_ACTIVITY_TOOL_TYPES, KNOWN_TOOL_TYPES + +_REPO_ROOT = Path(__file__).resolve().parents[1] +_FRONTEND_REGISTRY = _REPO_ROOT / "static" / "js" / "render" / "registry.js" + + +def _parse_frontend_tool_use_renderers(path: Path) -> frozenset[str]: + text = path.read_text(encoding="utf-8") + match = re.search( + r"export const TOOL_USE_RENDERERS = \{([^}]+)\}", + text, + re.DOTALL, + ) + if not match: + msg = f"Could not find TOOL_USE_RENDERERS in {path}" + raise ValueError(msg) + body = match.group(1) + keys = re.findall(r"^\s*(\w+)\s*:", body, re.MULTILINE) + return frozenset(keys) + + +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) + + +@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), + ( + "static/js/render/registry.js (TOOL_USE_RENDERERS)", + _parse_frontend_tool_use_renderers(_FRONTEND_REGISTRY), + ), + ], +) +def test_tool_type_sets_match_known_registry(site: str, actual: frozenset[str]) -> None: + if actual != KNOWN_TOOL_TYPES: + pytest.fail(_format_set_diff(KNOWN_TOOL_TYPES, actual, site)) + + +def test_known_tool_types_nonempty() -> None: + assert KNOWN_TOOL_TYPES diff --git a/utils/jsonl_parser.py b/utils/jsonl_parser.py index b948d82..e70c522 100644 --- a/utils/jsonl_parser.py +++ b/utils/jsonl_parser.py @@ -19,7 +19,7 @@ normalize_content as _normalize_content, ) from utils.session_peek import quick_session_info -from utils.tool_dispatch import _parse_tool_result +from utils.tool_dispatch import _parse_tool_result, track_tool_file_activity from utils.validation import validate_session_dict __all__ = ["parse_session", "quick_session_info"] @@ -311,7 +311,7 @@ def _process_assistant( if isinstance(tool_id, str): tool_use["id"] = tool_id tool_uses.append(tool_use) - _track_file_activity(tool_name, safe_input, metadata) + track_tool_file_activity(tool_name, safe_input, metadata) messages.append( { @@ -390,24 +390,3 @@ def _process_progress(entry: dict[str, Any], messages: list[MessageDict]) -> Non ) -def _track_file_activity( - tool_name: str, tool_input: dict[str, Any], metadata: dict[str, Any] -) -> None: - """Look at what each tool call did and record which files got touched, - what commands got run, what URLs got fetched.""" - raw_fp = tool_input.get("file_path", "") - fp = raw_fp if isinstance(raw_fp, str) else "" - if tool_name == "Read" and fp: - metadata["files_read"].add(fp) - elif tool_name == "Write" and fp: - metadata["files_created"].add(fp) - elif tool_name == "Edit" and fp: - metadata["files_written"].add(fp) - elif tool_name == "Bash": - cmd = tool_input.get("command", "") - if isinstance(cmd, str) and cmd: - metadata["bash_commands"].append(cmd) - elif tool_name in ("WebFetch", "WebSearch"): - url_or_query = tool_input.get("url") or tool_input.get("query", "") - if isinstance(url_or_query, str) and url_or_query: - metadata["web_fetches"].append(url_or_query) diff --git a/utils/md_exporter.py b/utils/md_exporter.py index b954b74..537f60a 100644 --- a/utils/md_exporter.py +++ b/utils/md_exporter.py @@ -8,6 +8,7 @@ from models.stats import SessionStatsDict from utils.jsonl_helpers import strip_system_tags from utils.session_stats import format_duration +from utils.tool_dispatch import KNOWN_TOOL_TYPES def session_to_markdown(session: SessionDict, stats: SessionStatsDict | None = None) -> str: @@ -323,11 +324,29 @@ def _render_tool_use(tool: ToolUseDict) -> str: continue lines.append(f">\n> Q: {q.get('question', '')}") else: - lines.append(f">\n> Input: `{str(inp)}`") + label = "Input" if name in KNOWN_TOOL_TYPES else "Input (unknown tool type)" + lines.append(f">\n> {label}: `{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") diff --git a/utils/tool_dispatch.py b/utils/tool_dispatch.py index 5e836c1..c1c697a 100644 --- a/utils/tool_dispatch.py +++ b/utils/tool_dispatch.py @@ -15,9 +15,23 @@ tuple there when a new predicate must sit above another. Predicates live in ``models.tool_results`` (single source of truth for narrowing). + +Adding a new Claude Code **tool use** name (e.g. ``"Read"``, ``"Bash"``): + +1. Add the name to ``KNOWN_TOOL_TYPE_NAMES`` below (alphabetical order). +2. Wire ``_FILE_ACTIVITY_HANDLERS`` (``None`` if no file/bash/web side effects). +3. Add a Markdown branch in ``utils/md_exporter.py`` ``_render_tool_use``. +4. Add ``TOOL_USE_RENDERERS`` entry in ``static/js/render/registry.js``. +5. Add ``(predicate, builder)`` to ``_TOOL_RESULT_DISPATCH`` when the tool has a + distinct ``toolUseResult`` shape (see ordering notes above). +6. Run ``pytest tests/test_tool_dispatch_sync.py -v`` — it fails with the + missing site if any step was skipped. + +See ``CONTRIBUTING.md`` § "Adding a new tool type". """ -from typing import cast +from collections.abc import Callable +from typing import Any, cast from models.tool_results import ( ToolResultDict, @@ -219,6 +233,84 @@ def _tool_result_build_user_input(tr: ToolResultDict, base: dict[str, object]) - (is_user_input_tool_result, _tool_result_build_user_input), ) +# Claude Code assistant tool_use ``name`` values coordinated across parser file +# activity, Markdown export, and the SPA ``TOOL_USE_RENDERERS`` map. +KNOWN_TOOL_TYPE_NAMES: tuple[str, ...] = ( + "AskUserQuestion", + "Bash", + "Edit", + "Glob", + "Grep", + "Read", + "Task", + "TodoWrite", + "WebFetch", + "WebSearch", + "Write", +) +KNOWN_TOOL_TYPES: frozenset[str] = frozenset(KNOWN_TOOL_TYPE_NAMES) + + +def _file_activity_read(tool_input: dict[str, Any], metadata: dict[str, Any]) -> None: + raw_fp = tool_input.get("file_path", "") + fp = raw_fp if isinstance(raw_fp, str) else "" + if fp: + metadata["files_read"].add(fp) + + +def _file_activity_write(tool_input: dict[str, Any], metadata: dict[str, Any]) -> None: + raw_fp = tool_input.get("file_path", "") + fp = raw_fp if isinstance(raw_fp, str) else "" + if fp: + metadata["files_created"].add(fp) + + +def _file_activity_edit(tool_input: dict[str, Any], metadata: dict[str, Any]) -> None: + raw_fp = tool_input.get("file_path", "") + fp = raw_fp if isinstance(raw_fp, str) else "" + if fp: + metadata["files_written"].add(fp) + + +def _file_activity_bash(tool_input: dict[str, Any], metadata: dict[str, Any]) -> None: + cmd = tool_input.get("command", "") + if isinstance(cmd, str) and cmd: + metadata["bash_commands"].append(cmd) + + +def _file_activity_web(tool_input: dict[str, Any], metadata: dict[str, Any]) -> None: + url_or_query = tool_input.get("url") or tool_input.get("query", "") + if isinstance(url_or_query, str) and url_or_query: + metadata["web_fetches"].append(url_or_query) + + +# Every ``KNOWN_TOOL_TYPES`` entry must appear here (``None`` = no side effects). +_FILE_ACTIVITY_HANDLERS: dict[str, Callable[[dict[str, Any], dict[str, Any]], None] | None] = { + "AskUserQuestion": None, + "Bash": _file_activity_bash, + "Edit": _file_activity_edit, + "Glob": None, + "Grep": None, + "Read": _file_activity_read, + "Task": None, + "TodoWrite": None, + "WebFetch": _file_activity_web, + "WebSearch": _file_activity_web, + "Write": _file_activity_write, +} +FILE_ACTIVITY_TOOL_TYPES: frozenset[str] = frozenset(_FILE_ACTIVITY_HANDLERS) + + +def track_tool_file_activity( + tool_name: str, tool_input: dict[str, Any], metadata: dict[str, Any] +) -> None: + """Record file/bash/web side effects for tools listed in ``KNOWN_TOOL_TYPES``.""" + if tool_name not in KNOWN_TOOL_TYPES: + return + handler = _FILE_ACTIVITY_HANDLERS[tool_name] + if handler is not None: + handler(tool_input, metadata) + def _parse_tool_result( tool_result: ToolResultUnion | None, slug: str | None = None From 44d7c6ba60dd2860b23b46671c12d95d9cbd633c Mon Sep 17 00:00:00 2001 From: chen Date: Wed, 1 Jul 2026 01:36:50 +0800 Subject: [PATCH 2/4] fix: address PR review feedback on tool dispatch sync --- CONTRIBUTING.md | 11 ++++++----- tests/test_tool_dispatch_sync.py | 14 ++++++++++---- utils/jsonl_parser.py | 2 -- utils/md_exporter.py | 5 ++--- utils/tool_dispatch.py | 30 ++++++++++-------------------- 5 files changed, 28 insertions(+), 34 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 28276d5..a29fba8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -145,11 +145,12 @@ See [`docs/architecture.md`](docs/architecture.md) for data flow, export state m Claude Code assistant `tool_use` blocks carry a `name` string (e.g. `"Read"`, `"Bash"`). The browser coordinates that name across four sites; drift is caught by `tests/test_tool_dispatch_sync.py`. -1. **`utils/tool_dispatch.py`** — add the name to `KNOWN_TOOL_TYPE_NAMES` (keep alphabetical). Set `_FILE_ACTIVITY_HANDLERS[name]` to a tracker function or `None`. 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. **`utils/md_exporter.py`** — add an `elif name == "…"` branch in `_render_tool_use` and include the name in `MD_EXPORTER_TOOL_TYPES`. -3. **`static/js/render/registry.js`** — add a `TOOL_USE_RENDERERS` entry (and a `tool_use/*.js` renderer module). -4. **Optional result UI** — if the backend emits a new `result_type`, add `TOOL_RESULT_RENDERERS` and a `tool_result/*.js` module. -5. Run `pytest tests/test_tool_dispatch_sync.py -v` — failure names the site missing the new type. +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`. +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. ## Getting help diff --git a/tests/test_tool_dispatch_sync.py b/tests/test_tool_dispatch_sync.py index 3af867d..282947f 100644 --- a/tests/test_tool_dispatch_sync.py +++ b/tests/test_tool_dispatch_sync.py @@ -51,10 +51,6 @@ def _format_set_diff(expected: frozenset[str], actual: frozenset[str], site: str [ ("utils/tool_dispatch.py (FILE_ACTIVITY_TOOL_TYPES)", FILE_ACTIVITY_TOOL_TYPES), ("utils/md_exporter.py (MD_EXPORTER_TOOL_TYPES)", MD_EXPORTER_TOOL_TYPES), - ( - "static/js/render/registry.js (TOOL_USE_RENDERERS)", - _parse_frontend_tool_use_renderers(_FRONTEND_REGISTRY), - ), ], ) def test_tool_type_sets_match_known_registry(site: str, actual: frozenset[str]) -> None: @@ -62,5 +58,15 @@ def test_tool_type_sets_match_known_registry(site: str, actual: frozenset[str]) pytest.fail(_format_set_diff(KNOWN_TOOL_TYPES, actual, site)) +def test_frontend_registry_matches_known_tool_types() -> None: + site = "static/js/render/registry.js (TOOL_USE_RENDERERS)" + try: + actual = _parse_frontend_tool_use_renderers(_FRONTEND_REGISTRY) + except ValueError as exc: + pytest.fail(f"{site}: {exc}") + if actual != KNOWN_TOOL_TYPES: + pytest.fail(_format_set_diff(KNOWN_TOOL_TYPES, actual, site)) + + def test_known_tool_types_nonempty() -> None: assert KNOWN_TOOL_TYPES diff --git a/utils/jsonl_parser.py b/utils/jsonl_parser.py index e70c522..04dec48 100644 --- a/utils/jsonl_parser.py +++ b/utils/jsonl_parser.py @@ -388,5 +388,3 @@ def _process_progress(entry: dict[str, Any], messages: list[MessageDict]) -> Non "is_sidechain": entry.get("isSidechain", False), } ) - - diff --git a/utils/md_exporter.py b/utils/md_exporter.py index 537f60a..79b4dbd 100644 --- a/utils/md_exporter.py +++ b/utils/md_exporter.py @@ -8,7 +8,6 @@ from models.stats import SessionStatsDict from utils.jsonl_helpers import strip_system_tags from utils.session_stats import format_duration -from utils.tool_dispatch import KNOWN_TOOL_TYPES def session_to_markdown(session: SessionDict, stats: SessionStatsDict | None = None) -> str: @@ -324,8 +323,8 @@ def _render_tool_use(tool: ToolUseDict) -> str: continue lines.append(f">\n> Q: {q.get('question', '')}") else: - label = "Input" if name in KNOWN_TOOL_TYPES else "Input (unknown tool type)" - lines.append(f">\n> {label}: `{str(inp)}`") + # Unknown names, or known types listed in MD_EXPORTER_TOOL_TYPES before an elif exists. + lines.append(f">\n> Input (unknown tool type): `{str(inp)}`") return "\n".join(lines) diff --git a/utils/tool_dispatch.py b/utils/tool_dispatch.py index c1c697a..27302f5 100644 --- a/utils/tool_dispatch.py +++ b/utils/tool_dispatch.py @@ -18,13 +18,15 @@ Adding a new Claude Code **tool use** name (e.g. ``"Read"``, ``"Bash"``): -1. Add the name to ``KNOWN_TOOL_TYPE_NAMES`` below (alphabetical order). -2. Wire ``_FILE_ACTIVITY_HANDLERS`` (``None`` if no file/bash/web side effects). +1. Add the name to ``_FILE_ACTIVITY_HANDLERS`` below (``None`` if no file/bash/web + side effects); ``KNOWN_TOOL_TYPES`` is derived from its keys. +2. Add the name to ``ToolNameLiteral`` in ``models/tool_results.py`` and, if the + tool has a distinct ``toolUseResult`` JSON shape, add the TypedDict, predicate, + and ``(predicate, builder)`` pair in ``_TOOL_RESULT_DISPATCH`` (respect ordering + — see notes above and ``tests/test_tool_dispatch_ordering.py``). 3. Add a Markdown branch in ``utils/md_exporter.py`` ``_render_tool_use``. 4. Add ``TOOL_USE_RENDERERS`` entry in ``static/js/render/registry.js``. -5. Add ``(predicate, builder)`` to ``_TOOL_RESULT_DISPATCH`` when the tool has a - distinct ``toolUseResult`` shape (see ordering notes above). -6. Run ``pytest tests/test_tool_dispatch_sync.py -v`` — it fails with the +5. Run ``pytest tests/test_tool_dispatch_sync.py -v`` — it fails with the missing site if any step was skipped. See ``CONTRIBUTING.md`` § "Adding a new tool type". @@ -235,20 +237,7 @@ def _tool_result_build_user_input(tr: ToolResultDict, base: dict[str, object]) - # Claude Code assistant tool_use ``name`` values coordinated across parser file # activity, Markdown export, and the SPA ``TOOL_USE_RENDERERS`` map. -KNOWN_TOOL_TYPE_NAMES: tuple[str, ...] = ( - "AskUserQuestion", - "Bash", - "Edit", - "Glob", - "Grep", - "Read", - "Task", - "TodoWrite", - "WebFetch", - "WebSearch", - "Write", -) -KNOWN_TOOL_TYPES: frozenset[str] = frozenset(KNOWN_TOOL_TYPE_NAMES) +# ``_FILE_ACTIVITY_HANDLERS`` is the single registry; ``KNOWN_TOOL_TYPES`` is derived. def _file_activity_read(tool_input: dict[str, Any], metadata: dict[str, Any]) -> None: @@ -284,7 +273,6 @@ def _file_activity_web(tool_input: dict[str, Any], metadata: dict[str, Any]) -> metadata["web_fetches"].append(url_or_query) -# Every ``KNOWN_TOOL_TYPES`` entry must appear here (``None`` = no side effects). _FILE_ACTIVITY_HANDLERS: dict[str, Callable[[dict[str, Any], dict[str, Any]], None] | None] = { "AskUserQuestion": None, "Bash": _file_activity_bash, @@ -298,6 +286,8 @@ 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) From 69bddedb90ae4cf4a625e61c6862ef1f3169fb91 Mon Sep 17 00:00:00 2001 From: chen Date: Wed, 1 Jul 2026 01:41:43 +0800 Subject: [PATCH 3/4] fix(md_exporter): distinguish known vs unknown tool fallback labels --- utils/md_exporter.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/utils/md_exporter.py b/utils/md_exporter.py index 79b4dbd..ff98a03 100644 --- a/utils/md_exporter.py +++ b/utils/md_exporter.py @@ -323,8 +323,11 @@ def _render_tool_use(tool: ToolUseDict) -> str: continue lines.append(f">\n> Q: {q.get('question', '')}") else: - # Unknown names, or known types listed in MD_EXPORTER_TOOL_TYPES before an elif exists. - lines.append(f">\n> Input (unknown tool type): `{str(inp)}`") + 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)}`") return "\n".join(lines) From d0e6f39ea92806b4152b3f825076809623ebf64e Mon Sep 17 00:00:00 2001 From: chen Date: Wed, 1 Jul 2026 02:37:07 +0800 Subject: [PATCH 4/4] fix: tighten tool dispatch sync test per review feedback --- CONTRIBUTING.md | 2 +- tests/test_tool_dispatch_sync.py | 96 ++++++++++++++++++++++---------- utils/md_exporter.py | 23 +------- utils/tool_dispatch.py | 2 - 4 files changed, 70 insertions(+), 53 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a29fba8..5a9dace 100644 --- a/CONTRIBUTING.md +++ b/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. diff --git a/tests/test_tool_dispatch_sync.py b/tests/test_tool_dispatch_sync.py index 282947f..7063bc0 100644 --- a/tests/test_tool_dispatch_sync.py +++ b/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)) diff --git a/utils/md_exporter.py b/utils/md_exporter.py index ff98a03..bc617b1 100644 --- a/utils/md_exporter.py +++ b/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") diff --git a/utils/tool_dispatch.py b/utils/tool_dispatch.py index 27302f5..7f81a6a 100644 --- a/utils/tool_dispatch.py +++ b/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(