fix: address PR feedback on tool-type scaffold

Also fix ruff format and search benchmark date-window flake.

Author: clean6378-max-it

CONTRIBUTING.md

@@ -160,10 +160,15 @@ Claude Code assistant `tool_use` blocks carry a `name` string (e.g. `"Read"`, `"
 1. **One edit:** create or update a JSON record under `tool_types/` (see `tool_types/README.md`).
 2. **One command:** `python scripts/scaffold_tool_type.py --record tool_types/<name>.json`  
    Or from scratch: `python scripts/scaffold_tool_type.py --name my_tool`
-3. **Complete stubs:** field mapping in the dispatch builder, render HTML in JS modules, overlap fixtures when `priority > 0`.
-4. **Verify:** `pytest tests/test_tool_dispatch_sync.py tests/test_tool_dispatch_ordering.py tests/test_tool_dispatch_adversarial.py -q`
+3. **Complete stubs:** field mapping in the dispatch builder and render HTML in JS modules. When `result` is registered, also finish overlap fixtures when `priority > 0`.
+4. **Verify:**
 
-The generator emits coordinated stubs across all seven sites and writes the registration record. Hand-editing drops from seven sites to the record plus finishing TODO stubs (typically 2–3 files).
+   ```bash
+   pytest tests/test_scaffold_tool_type.py tests/test_tool_dispatch_sync.py tests/test_tool_dispatch_ordering.py tests/test_tool_dispatch_adversarial.py tests/test_jsonl_parser.py tests/test_real_session_fixtures.py -q
+   npm test
+   ```
+
+The generator emits coordinated stubs across all seven sites when `result` is registered (dispatch table, TypedDict/guards, Markdown branches, JS use/result modules, `registry.js`, parser fixture, and `static/tool_types.json`). Use-only tools (`--no-result`) omit result dispatch, TypedDict/guard, parser fixture, and overlap-fixture artifacts. Hand-editing drops from seven sites to the record plus finishing TODO stubs (typically 2–3 files).
 
 Dry-run preview: `python scripts/scaffold_tool_type.py --name example_tool --dry-run`
 

docs/architecture.md

@@ -75,7 +75,7 @@ In `utils/tool_dispatch.py`, tool results are classified through `_parse_tool_re
 
 When adding a new tool renderer:
 
-1. **Preferred:** add a registration record under `tool_types/` and run `python scripts/scaffold_tool_type.py --record tool_types/<name>.json`. The generator emits `_TOOL_RESULT_DISPATCH` entries, TypedDict/guards, Markdown branches, JS module stubs, `registry.js` map entries, a parser fixture, and regenerates `static/tool_types.json`. See `CONTRIBUTING.md` § "Adding a new tool type" for the before/after workflow.
+1. **Preferred:** add a registration record under `tool_types/` and run `python scripts/scaffold_tool_type.py --record tool_types/<name>.json`. With `result` registered, the generator emits `_TOOL_RESULT_DISPATCH` entries, TypedDict/guards, Markdown branches, JS module stubs, `registry.js` map entries, a parser fixture, and regenerates `static/tool_types.json`. Use-only tools (`--no-result`) omit result dispatch, TypedDict/guard, parser fixture, and overlap-fixture artifacts. See `CONTRIBUTING.md` § "Adding a new tool type" for the before/after workflow.
 2. **Manual (legacy):** add a `ToolResultDispatchEntry` to `_TOOL_RESULT_DISPATCH` in `utils/tool_dispatch.py`. Set `priority` higher than any overlapping predicate it must beat without relying on registration order, and add a row to `ORDERING_INVARIANTS` in `tests/test_tool_dispatch_ordering.py` when overlaps exist. Documented exception: `plan` (priority 1) over `file_write` (0). Broad predicates like `task_message` use registration order instead of elevated priority.
 3. Add or extend a JSONL fixture under `tests/fixtures/` (especially for overlaps with existing predicates).
 4. Run `pytest tests/test_tool_dispatch_ordering.py tests/test_tool_dispatch_adversarial.py tests/test_jsonl_parser.py tests/test_real_session_fixtures.py tests/test_scaffold_tool_type.py -v`.

models/tool_type_registry.py

@@ -147,6 +147,9 @@ def from_mapping(cls, raw: dict[str, Any] | None) -> ToolResultRecord | None:
         if not isinstance(priority, int):
             msg = "result.priority must be an int"
             raise ValueError(msg)
+        if priority < 0:
+            msg = "result.priority must be a non-negative int"
+            raise ValueError(msg)
         inv_raw = raw.get("overlap_invariants", [])
         if not isinstance(inv_raw, list):
             msg = "result.overlap_invariants must be a list"
@@ -263,11 +266,7 @@ def to_mapping(self) -> dict[str, Any]:
                             if inv.before_guard is not None
                             else {}
                         ),
-                        **(
-                            {"after_guard": inv.after_guard}
-                            if inv.after_guard is not None
-                            else {}
-                        ),
+                        **({"after_guard": inv.after_guard} if inv.after_guard is not None else {}),
                     }
                     for inv in self.result.overlap_invariants
                 ],
@@ -288,11 +287,17 @@ def typed_dict_class(self) -> str:
 
     @property
     def guard_name(self) -> str:
-        return f"is_{self.snake_name}_tool_result"
+        if self.result is None:
+            msg = "guard_name requires result registration"
+            raise ValueError(msg)
+        return guard_name_for_dispatch_id(self.result.dispatch_id)
 
     @property
     def builder_name(self) -> str:
-        return f"_tool_result_build_{self.snake_name}"
+        if self.result is None:
+            msg = "builder_name requires result registration"
+            raise ValueError(msg)
+        return builder_name_for_dispatch_id(self.result.dispatch_id)
 
 
 def snake_to_pascal(snake: str) -> str:
@@ -336,13 +341,20 @@ def guard_name_for_dispatch_id(dispatch_id: str) -> str:
     return f"is_{dispatch_id}_tool_result"
 
 
+def builder_name_for_dispatch_id(dispatch_id: str) -> str:
+    _validate_dispatch_id(dispatch_id)
+    return f"_tool_result_build_{dispatch_id}"
+
+
 def _validate_dispatch_id(dispatch_id: str) -> None:
     if not _DISPATCH_ID_RE.fullmatch(dispatch_id):
         msg = (
             f"dispatch_id {dispatch_id!r} must be snake_case "
             "(e.g. example_tool); guard names derive from is_<dispatch_id>_tool_result"
         )
         raise ValueError(msg)
+
+
 def _dispatch_id_to_camel(dispatch_id: str) -> str:
     parts = dispatch_id.split("_")
     if not parts:

scripts/gen_tool_types_manifest.py

@@ -9,43 +9,22 @@
 from __future__ import annotations
 
 import json
-import re
+import sys
 from pathlib import Path
 
 _REPO_ROOT = Path(__file__).resolve().parents[1]
 _MANIFEST_PATH = _REPO_ROOT / "static" / "tool_types.json"
 
+if str(_REPO_ROOT) not in sys.path:
+    sys.path.insert(0, str(_REPO_ROOT))
+
+from scripts.scaffold_tool_type import _parse_handlers_from_text  # noqa: E402
+
 
 def _load_known_tool_types(repo_root: Path) -> frozenset[str]:
     path = repo_root / "utils" / "tool_dispatch.py"
     text = path.read_text(encoding="utf-8")
-    marker = "_FILE_ACTIVITY_HANDLERS: dict"
-    start = text.find(marker)
-    if start == -1:
-        msg = f"could not find {marker} in {path}"
-        raise ValueError(msg)
-    brace_start = text.find("{", start)
-    if brace_start == -1:
-        msg = f"could not find opening brace for {marker} in {path}"
-        raise ValueError(msg)
-    depth = 0
-    i = brace_start
-    while i < len(text):
-        ch = text[i]
-        if ch == "{":
-            depth += 1
-        elif ch == "}":
-            depth -= 1
-            if depth == 0:
-                body = text[brace_start + 1 : i]
-                keys = re.findall(r'"([^"]+)":', body)
-                if not keys:
-                    msg = f"no tool names found in {marker} in {path}"
-                    raise ValueError(msg)
-                return frozenset(keys)
-        i += 1
-    msg = f"unbalanced braces in {marker} in {path}"
-    raise ValueError(msg)
+    return _parse_handlers_from_text(text)
 
 
 def write_tool_types_manifest(path: Path | None = None, *, repo_root: Path | None = None) -> int:

scripts/scaffold_tool_type.py

@@ -133,6 +133,7 @@ def _flush(self, plan: _EmitPlan) -> None:
             rel = path.relative_to(self.root)
             if self.dry_run:
                 self.stdout.write(f"--- would write {rel} ({len(content)} bytes) ---\n")
+                self._written.append(path)
                 continue
             path.parent.mkdir(parents=True, exist_ok=True)
             path.write_text(content, encoding="utf-8")
@@ -157,7 +158,8 @@ def transform(text: str) -> str:
                 )
                 guard = record.guard_name
                 if f"    {guard}," not in text:
-                    text = text.replace(
+                    text = self._replace_once(
+                        text,
                         "    is_web_search_tool_result,\n)",
                         f"    is_web_search_tool_result,\n    {guard},\n)",
                     )
@@ -188,7 +190,7 @@ def _render_dispatch_entry(self, record: ToolTypeRecord) -> str:
         priority = record.result.priority
         if priority > 0:
             return (
-                f'    ToolResultDispatchEntry(\n'
+                f"    ToolResultDispatchEntry(\n"
                 f'        "{record.result.dispatch_id}",\n'
                 f"        {record.guard_name},\n"
                 f"        {record.builder_name},\n"
@@ -215,11 +217,13 @@ def transform(text: str) -> str:
                     member,
                 )
                 anchor = '    return "questions" in tr and "answers" in tr\n\n\n'
-                text = text.replace(
+                text = self._replace_once(
+                    text,
                     anchor + "# Tool names on assistant",
                     anchor + f"{self._render_guard(record)}\n\n\n# Tool names on assistant",
                 )
-            text = text.replace(
+            text = self._replace_once(
+                text,
                 '    "WebSearch",\n]',
                 f'    "WebSearch",\n    "{record.name}",\n]',
             )
@@ -258,13 +262,15 @@ def transform(text: str) -> str:
             unknown_branch = (
                 '    else:\n        lines.append(f">\\n> Input (unknown tool type): `{str(inp)}`")'
             )
-            text = text.replace(
+            text = self._replace_once(
+                text,
                 unknown_branch,
                 use_branch + unknown_branch,
             )
             if record.result is not None:
                 result_branch = self._render_md_tool_result(record)
-                text = text.replace(
+                text = self._replace_once(
+                    text,
                     '    elif rt == "plan":',
                     result_branch + '    elif rt == "plan":',
                 )
@@ -276,7 +282,7 @@ def _render_md_tool_use(self, record: ToolTypeRecord) -> str:
         keys = record.use_input_keys or ("input",)
         lines = [f'    elif name == "{record.name}":']
         for key in keys:
-            lines.append(f'        lines.append(f">\\n> {key}: {{inp.get({key!r}, \'\')}}")')
+            lines.append(f"        lines.append(f\">\\n> {key}: {{inp.get({key!r}, '')}}\")")
         return "\n".join(lines) + "\n"
 
     def _render_md_tool_result(self, record: ToolTypeRecord) -> str:
@@ -327,7 +333,8 @@ def _plan_registry_js(self, record: ToolTypeRecord, plan: _EmitPlan) -> None:
 
         def transform(text: str) -> str:
             if use_import.strip() not in text:
-                text = text.replace(
+                text = self._replace_once(
+                    text,
                     "import { renderToolUseFallback }",
                     use_import + "import { renderToolUseFallback }",
                 )
@@ -343,7 +350,8 @@ def transform(text: str) -> str:
                 module = f"./tool_result/{record.result.dispatch_id}.js"
                 result_import = f"import {{ {result_fn} }} from '{module}';\n"
                 if result_import.strip() not in text:
-                    text = text.replace(
+                    text = self._replace_once(
+                        text,
                         "import { renderToolResultFallback }",
                         result_import + "import { renderToolResultFallback }",
                     )
@@ -368,15 +376,24 @@ def transform(text: str) -> str:
                 before_guard = inv.resolved_before_guard()
                 after_guard = inv.resolved_after_guard()
                 if before_guard not in text:
-                    replacement = (
-                        f"    is_task_async_tool_result,\n"
-                        f"    {before_guard},\n"
-                        f"    {after_guard},\n)"
-                    )
-                    text = text.replace(
+                    text = self._replace_once(
+                        text,
                         "    is_task_async_tool_result,\n)",
-                        replacement,
+                        f"    is_task_async_tool_result,\n    {before_guard},\n)",
                     )
+                if after_guard not in text:
+                    if before_guard in text:
+                        text = self._replace_once(
+                            text,
+                            f"    {before_guard},\n)",
+                            f"    {before_guard},\n    {after_guard},\n)",
+                        )
+                    else:
+                        text = self._replace_once(
+                            text,
+                            "    is_task_async_tool_result,\n)",
+                            f"    is_task_async_tool_result,\n    {after_guard},\n)",
+                        )
                 row = (
                     f"    (\n"
                     f"        {before_guard},\n"
@@ -453,6 +470,13 @@ def _insert_before(
             return text[:line_start] + insertion + text[line_start:]
         return text[:idx] + insertion + text[idx:]
 
+    @staticmethod
+    def _replace_once(text: str, anchor: str, replacement: str) -> str:
+        if anchor not in text:
+            msg = f"scaffold replacement anchor not found: {anchor!r}"
+            raise ValueError(msg)
+        return text.replace(anchor, replacement, 1)
+
 
 def _parse_handlers_from_text(text: str) -> frozenset[str]:
     marker = "_FILE_ACTIVITY_HANDLERS: dict"
@@ -565,7 +589,12 @@ def main(argv: list[str] | None = None) -> int:
     else:
         print(f"Scaffolded {record.name}: {len(written)} files updated")
         print("Next: complete TODO stubs, then run:")
-        print("  pytest tests/test_tool_dispatch_sync.py tests/test_tool_dispatch_ordering.py -q")
+        print(
+            "  pytest tests/test_scaffold_tool_type.py tests/test_tool_dispatch_sync.py "
+            "tests/test_tool_dispatch_ordering.py tests/test_tool_dispatch_adversarial.py "
+            "tests/test_jsonl_parser.py tests/test_real_session_fixtures.py -q"
+        )
+        print("  npm test")
     return 0
 
 

tests/benchmarks/test_search_bench.py

@@ -12,7 +12,7 @@ def test_search_full_corpus(
     bench_client_search_corpus: FlaskClient,
 ) -> None:
     def _run() -> object:
-        return bench_client_search_corpus.get("/api/search?q=searchable&limit=50")
+        return bench_client_search_corpus.get("/api/search?q=searchable&limit=50&all_history=1")
 
     resp = benchmark(_run)
     assert resp.status_code == 200

tests/test_scaffold_tool_type.py

@@ -105,6 +105,75 @@ def test_guard_name_for_dispatch_id() -> None:
     assert guard_name_for_dispatch_id("example_tool") == "is_example_tool_tool_result"
 
 
+def test_guard_and_builder_names_use_dispatch_id() -> None:
+    record = ToolTypeRecord.from_mapping(
+        {
+            "name": "MyTool",
+            "result": {"dispatch_id": "custom_dispatch"},
+        }
+    )
+    assert record.guard_name == "is_custom_dispatch_tool_result"
+    assert record.builder_name == "_tool_result_build_custom_dispatch"
+    assert js_render_result_name(record) == "renderCustomDispatchResult"
+
+
+def test_from_mapping_rejects_negative_priority() -> None:
+    with pytest.raises(ValueError, match="non-negative"):
+        ToolTypeRecord.from_mapping(
+            {
+                "name": "BadTool",
+                "result": {"dispatch_id": "bad_tool", "priority": -1},
+            }
+        )
+
+
+def test_dry_run_reports_planned_artifact_count(capsys: pytest.CaptureFixture[str]) -> None:
+    code = main(["--name", "example_tool", "--dry-run"])
+    assert code == 0
+    out = capsys.readouterr().out
+    assert "artifacts planned)" in out
+    count = int(out.rsplit("(", 1)[-1].split(" artifacts", 1)[0])
+    assert count > 0
+
+
+def test_write_record_only_writes_json_without_codegen(tmp_path: Path) -> None:
+    repo = _mirror_repo(tmp_path)
+    code = main(
+        [
+            "--name",
+            "record_only_tool",
+            "--write-record-only",
+            "--root",
+            str(repo),
+        ]
+    )
+    assert code == 0
+    record_path = repo / "tool_types" / "record_only_tool.json"
+    assert record_path.is_file()
+    loaded = ToolTypeRecord.load(record_path)
+    assert loaded.name == "RecordOnlyTool"
+    dispatch = (repo / "utils" / "tool_dispatch.py").read_text(encoding="utf-8")
+    assert "RecordOnlyTool" not in dispatch
+    assert not (repo / "static" / "js" / "render" / "tool_use" / "record_only_tool.js").exists()
+
+
+def test_emit_anchor_miss_in_tool_results_leaves_repo_unchanged(tmp_path: Path) -> None:
+    repo = _mirror_repo(tmp_path)
+    tool_results = repo / "models" / "tool_results.py"
+    dispatch = repo / "utils" / "tool_dispatch.py"
+    corrupt = tool_results.read_text(encoding="utf-8").replace(
+        '    "WebSearch",\n]',
+        '    "WebSearch",\n    "Legacy",\n]',
+    )
+    tool_results.write_text(corrupt, encoding="utf-8")
+    dispatch_before = dispatch.read_text(encoding="utf-8")
+    record = ToolTypeRecord.from_cli_name("example_tool")
+    with pytest.raises(ValueError, match="replacement anchor not found"):
+        ScaffoldEmitter(repo).emit(record)
+    assert tool_results.read_text(encoding="utf-8") == corrupt
+    assert dispatch.read_text(encoding="utf-8") == dispatch_before
+
+
 def test_dry_run_main_exits_zero(capsys: pytest.CaptureFixture[str]) -> None:
     code = main(["--name", "example_tool", "--dry-run"])
     assert code == 0