Skip to content

Commit 7f685e4

Browse files
claude-code-chat-browser: Tool dispatch coordination - single authoritative KNOWN_TOOL_TYPES registry (#104)
* 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. * fix: address PR review feedback on tool dispatch sync * fix(md_exporter): distinguish known vs unknown tool fallback labels * fix: tighten tool dispatch sync test per review feedback
1 parent bddbda3 commit 7f685e4

6 files changed

Lines changed: 209 additions & 27 deletions

File tree

CONTRIBUTING.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ npm run test:coverage # optional
100100
| New `ErrorCode` | Parametrized row in `tests/test_error_codes.py` |
101101
| Search / limit validation | `tests/test_search.py` |
102102
| New `_parse_tool_result` dispatch entry | Fixture + assertion in `tests/test_jsonl_parser.py` |
103+
| New Claude Code tool use name | See **Adding a new tool type** below |
103104
| CLI behavior | `tests/test_cli_e2e.py` (subprocess) or `tests/test_cli_args.py` (parser only) |
104105
| Frontend shared module | `static/js/shared/*.test.js` (vitest) |
105106
| Error response shape | `tests/test_error_propagation.py` regression |
@@ -140,6 +141,17 @@ npm run test:coverage # optional
140141

141142
See [`docs/architecture.md`](docs/architecture.md) for data flow, export state machine, and component diagram.
142143

144+
## Adding a new tool type
145+
146+
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`.
147+
148+
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`).
149+
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`.
150+
3. **`utils/md_exporter.py`** — add an `elif name == "…"` branch in `_render_tool_use` (sync test parses these branches).
151+
4. **`static/js/render/registry.js`** — add a `TOOL_USE_RENDERERS` entry (and a `tool_use/*.js` renderer module).
152+
5. **Optional result UI** — if the backend emits a new `result_type`, add `TOOL_RESULT_RENDERERS` and a `tool_result/*.js` module.
153+
6. Run `pytest tests/test_tool_dispatch_sync.py -v` — failure names the site missing the new type.
154+
143155
## Getting help
144156

145157
Open an issue with a clear repro or propose a draft PR early for CI feedback.

models/tool_results.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ def is_user_input_tool_result(tr: ToolResultDict) -> TypeGuard[UserInputToolResu
220220

221221
# Tool names on assistant tool_use blocks — pairs with slug on user tool_result rows.
222222
ToolNameLiteral = Literal[
223+
"AskUserQuestion",
223224
"Bash",
224225
"Read",
225226
"Write",

tests/test_tool_dispatch_sync.py

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
"""Contract test: ``KNOWN_TOOL_TYPES`` must match all four dispatch sites.
2+
3+
Sites (each compared to ``KNOWN_TOOL_TYPES`` in ``utils/tool_dispatch.py``):
4+
- ``utils/md_exporter.py`` — ``_render_tool_use`` if/elif branches (parsed)
5+
- ``models/tool_results.py`` — ``ToolNameLiteral``
6+
- ``static/js/render/registry.js`` — ``TOOL_USE_RENDERERS`` keys (parsed)
7+
"""
8+
9+
from __future__ import annotations
10+
11+
import re
12+
from pathlib import Path
13+
from typing import get_args
14+
15+
import pytest
16+
17+
from models.tool_results import ToolNameLiteral
18+
from utils.tool_dispatch import KNOWN_TOOL_TYPES
19+
20+
_REPO_ROOT = Path(__file__).resolve().parents[1]
21+
_FRONTEND_REGISTRY = _REPO_ROOT / "static" / "js" / "render" / "registry.js"
22+
_MD_EXPORTER = _REPO_ROOT / "utils" / "md_exporter.py"
23+
24+
25+
def _format_set_diff(expected: frozenset[str], actual: frozenset[str], site: str) -> str:
26+
missing = sorted(expected - actual)
27+
extra = sorted(actual - expected)
28+
parts: list[str] = []
29+
if missing:
30+
parts.append(f"missing tool type(s) {missing!r} in {site}")
31+
if extra:
32+
parts.append(f"unexpected tool type(s) {extra!r} in {site}")
33+
return "; ".join(parts)
34+
35+
36+
def _parse_frontend_tool_use_renderers(path: Path) -> frozenset[str]:
37+
"""Extract ``TOOL_USE_RENDERERS`` keys.
38+
39+
Assumes values are bare identifiers (``Bash: renderBashUse``). Brace-depth
40+
parsing avoids truncating the object body if a value ever contains ``}``.
41+
"""
42+
text = path.read_text(encoding="utf-8")
43+
marker = "export const TOOL_USE_RENDERERS = {"
44+
start = text.find(marker)
45+
if start == -1:
46+
msg = f"Could not find TOOL_USE_RENDERERS in {path}"
47+
raise ValueError(msg)
48+
i = start + len(marker)
49+
depth = 1
50+
body_start = i
51+
while i < len(text) and depth > 0:
52+
ch = text[i]
53+
if ch == "{":
54+
depth += 1
55+
elif ch == "}":
56+
depth -= 1
57+
i += 1
58+
if depth != 0:
59+
msg = f"Unbalanced braces in TOOL_USE_RENDERERS in {path}"
60+
raise ValueError(msg)
61+
body = text[body_start : i - 1]
62+
keys = re.findall(r"^\s*(\w+)\s*:", body, re.MULTILINE)
63+
return frozenset(keys)
64+
65+
66+
def _parse_md_exporter_tool_use_handlers(path: Path) -> frozenset[str]:
67+
"""Extract tool names handled by ``_render_tool_use`` if/elif branches."""
68+
text = path.read_text(encoding="utf-8")
69+
match = re.search(
70+
r"def _render_tool_use\(.*?(?=\ndef _render_tool_result)",
71+
text,
72+
re.DOTALL,
73+
)
74+
if not match:
75+
msg = f"Could not find _render_tool_use in {path}"
76+
raise ValueError(msg)
77+
body = match.group(0)
78+
names = set(re.findall(r'(?:if|elif) name == "([^"]+)"', body))
79+
for tuple_match in re.finditer(r"elif name in \(([^)]+)\)", body):
80+
names.update(re.findall(r'"([^"]+)"', tuple_match.group(1)))
81+
return frozenset(names)
82+
83+
84+
def test_md_exporter_handlers_match_known_tool_types() -> None:
85+
site = "utils/md_exporter.py (_render_tool_use branches)"
86+
try:
87+
actual = _parse_md_exporter_tool_use_handlers(_MD_EXPORTER)
88+
except ValueError as exc:
89+
pytest.fail(f"{site}: {exc}")
90+
if actual != KNOWN_TOOL_TYPES:
91+
pytest.fail(_format_set_diff(KNOWN_TOOL_TYPES, actual, site))
92+
93+
94+
def test_tool_name_literal_matches_known_tool_types() -> None:
95+
site = "models/tool_results.py (ToolNameLiteral)"
96+
actual = frozenset(get_args(ToolNameLiteral))
97+
if actual != KNOWN_TOOL_TYPES:
98+
pytest.fail(_format_set_diff(KNOWN_TOOL_TYPES, actual, site))
99+
100+
101+
def test_frontend_registry_matches_known_tool_types() -> None:
102+
site = "static/js/render/registry.js (TOOL_USE_RENDERERS)"
103+
try:
104+
actual = _parse_frontend_tool_use_renderers(_FRONTEND_REGISTRY)
105+
except ValueError as exc:
106+
pytest.fail(f"{site}: {exc}")
107+
if actual != KNOWN_TOOL_TYPES:
108+
pytest.fail(_format_set_diff(KNOWN_TOOL_TYPES, actual, site))
109+
110+
111+
def test_known_tool_types_nonempty() -> None:
112+
assert KNOWN_TOOL_TYPES

utils/jsonl_parser.py

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
normalize_content as _normalize_content,
2020
)
2121
from utils.session_peek import quick_session_info
22-
from utils.tool_dispatch import _parse_tool_result
22+
from utils.tool_dispatch import _parse_tool_result, track_tool_file_activity
2323
from utils.validation import validate_session_dict
2424

2525
__all__ = ["parse_session", "quick_session_info"]
@@ -311,7 +311,7 @@ def _process_assistant(
311311
if isinstance(tool_id, str):
312312
tool_use["id"] = tool_id
313313
tool_uses.append(tool_use)
314-
_track_file_activity(tool_name, safe_input, metadata)
314+
track_tool_file_activity(tool_name, safe_input, metadata)
315315

316316
messages.append(
317317
{
@@ -388,26 +388,3 @@ def _process_progress(entry: dict[str, Any], messages: list[MessageDict]) -> Non
388388
"is_sidechain": entry.get("isSidechain", False),
389389
}
390390
)
391-
392-
393-
def _track_file_activity(
394-
tool_name: str, tool_input: dict[str, Any], metadata: dict[str, Any]
395-
) -> None:
396-
"""Look at what each tool call did and record which files got touched,
397-
what commands got run, what URLs got fetched."""
398-
raw_fp = tool_input.get("file_path", "")
399-
fp = raw_fp if isinstance(raw_fp, str) else ""
400-
if tool_name == "Read" and fp:
401-
metadata["files_read"].add(fp)
402-
elif tool_name == "Write" and fp:
403-
metadata["files_created"].add(fp)
404-
elif tool_name == "Edit" and fp:
405-
metadata["files_written"].add(fp)
406-
elif tool_name == "Bash":
407-
cmd = tool_input.get("command", "")
408-
if isinstance(cmd, str) and cmd:
409-
metadata["bash_commands"].append(cmd)
410-
elif tool_name in ("WebFetch", "WebSearch"):
411-
url_or_query = tool_input.get("url") or tool_input.get("query", "")
412-
if isinstance(url_or_query, str) and url_or_query:
413-
metadata["web_fetches"].append(url_or_query)

utils/md_exporter.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ def _render_tool_use(tool: ToolUseDict) -> str:
323323
continue
324324
lines.append(f">\n> Q: {q.get('question', '')}")
325325
else:
326-
lines.append(f">\n> Input: `{str(inp)}`")
326+
lines.append(f">\n> Input (unknown tool type): `{str(inp)}`")
327327

328328
return "\n".join(lines)
329329

utils/tool_dispatch.py

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,25 @@
1515
tuple there when a new predicate must sit above another.
1616
1717
Predicates live in ``models.tool_results`` (single source of truth for narrowing).
18+
19+
Adding a new Claude Code **tool use** name (e.g. ``"Read"``, ``"Bash"``):
20+
21+
1. Add the name to ``_FILE_ACTIVITY_HANDLERS`` below (``None`` if no file/bash/web
22+
side effects); ``KNOWN_TOOL_TYPES`` is derived from its keys.
23+
2. Add the name to ``ToolNameLiteral`` in ``models/tool_results.py`` and, if the
24+
tool has a distinct ``toolUseResult`` JSON shape, add the TypedDict, predicate,
25+
and ``(predicate, builder)`` pair in ``_TOOL_RESULT_DISPATCH`` (respect ordering
26+
— see notes above and ``tests/test_tool_dispatch_ordering.py``).
27+
3. Add a Markdown branch in ``utils/md_exporter.py`` ``_render_tool_use``.
28+
4. Add ``TOOL_USE_RENDERERS`` entry in ``static/js/render/registry.js``.
29+
5. Run ``pytest tests/test_tool_dispatch_sync.py -v`` — it fails with the
30+
missing site if any step was skipped.
31+
32+
See ``CONTRIBUTING.md`` § "Adding a new tool type".
1833
"""
1934

20-
from typing import cast
35+
from collections.abc import Callable
36+
from typing import Any, cast
2137

2238
from models.tool_results import (
2339
ToolResultDict,
@@ -219,6 +235,70 @@ def _tool_result_build_user_input(tr: ToolResultDict, base: dict[str, object]) -
219235
(is_user_input_tool_result, _tool_result_build_user_input),
220236
)
221237

238+
# Claude Code assistant tool_use ``name`` values coordinated across parser file
239+
# activity, Markdown export, and the SPA ``TOOL_USE_RENDERERS`` map.
240+
# ``_FILE_ACTIVITY_HANDLERS`` is the single registry; ``KNOWN_TOOL_TYPES`` is derived.
241+
242+
243+
def _file_activity_read(tool_input: dict[str, Any], metadata: dict[str, Any]) -> None:
244+
raw_fp = tool_input.get("file_path", "")
245+
fp = raw_fp if isinstance(raw_fp, str) else ""
246+
if fp:
247+
metadata["files_read"].add(fp)
248+
249+
250+
def _file_activity_write(tool_input: dict[str, Any], metadata: dict[str, Any]) -> None:
251+
raw_fp = tool_input.get("file_path", "")
252+
fp = raw_fp if isinstance(raw_fp, str) else ""
253+
if fp:
254+
metadata["files_created"].add(fp)
255+
256+
257+
def _file_activity_edit(tool_input: dict[str, Any], metadata: dict[str, Any]) -> None:
258+
raw_fp = tool_input.get("file_path", "")
259+
fp = raw_fp if isinstance(raw_fp, str) else ""
260+
if fp:
261+
metadata["files_written"].add(fp)
262+
263+
264+
def _file_activity_bash(tool_input: dict[str, Any], metadata: dict[str, Any]) -> None:
265+
cmd = tool_input.get("command", "")
266+
if isinstance(cmd, str) and cmd:
267+
metadata["bash_commands"].append(cmd)
268+
269+
270+
def _file_activity_web(tool_input: dict[str, Any], metadata: dict[str, Any]) -> None:
271+
url_or_query = tool_input.get("url") or tool_input.get("query", "")
272+
if isinstance(url_or_query, str) and url_or_query:
273+
metadata["web_fetches"].append(url_or_query)
274+
275+
276+
_FILE_ACTIVITY_HANDLERS: dict[str, Callable[[dict[str, Any], dict[str, Any]], None] | None] = {
277+
"AskUserQuestion": None,
278+
"Bash": _file_activity_bash,
279+
"Edit": _file_activity_edit,
280+
"Glob": None,
281+
"Grep": None,
282+
"Read": _file_activity_read,
283+
"Task": None,
284+
"TodoWrite": None,
285+
"WebFetch": _file_activity_web,
286+
"WebSearch": _file_activity_web,
287+
"Write": _file_activity_write,
288+
}
289+
KNOWN_TOOL_TYPES: frozenset[str] = frozenset(_FILE_ACTIVITY_HANDLERS)
290+
291+
292+
def track_tool_file_activity(
293+
tool_name: str, tool_input: dict[str, Any], metadata: dict[str, Any]
294+
) -> None:
295+
"""Record file/bash/web side effects for tools listed in ``KNOWN_TOOL_TYPES``."""
296+
if tool_name not in KNOWN_TOOL_TYPES:
297+
return
298+
handler = _FILE_ACTIVITY_HANDLERS[tool_name]
299+
if handler is not None:
300+
handler(tool_input, metadata)
301+
222302

223303
def _parse_tool_result(
224304
tool_result: ToolResultUnion | None, slug: str | None = None

0 commit comments

Comments
 (0)