Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,9 @@ Claude Code assistant `tool_use` blocks carry a `name` string (e.g. `"Read"`, `"
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` (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.
5. Regenerate **`static/tool_types.json`**: `python scripts/gen_tool_types_manifest.py`
6. **Optional result UI** — if the backend emits a new `result_type`, add `TOOL_RESULT_RENDERERS` and a `tool_result/*.js` module.
7. Run `pytest tests/test_tool_dispatch_sync.py -v` — failure names the site missing the new type.

## Getting help

Expand Down
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.PHONY: seed-baselines-local update-baselines check-benchmarks clean-benchmark-artifacts
.PHONY: seed-baselines-local update-baselines gen-tool-types-manifest check-benchmarks clean-benchmark-artifacts

# WARNING: captures timings on THIS machine. Production baselines must match ubuntu-latest CI.
# Prefer downloading benchmark-results.json from a CI artifact, then:
Expand All @@ -11,6 +11,9 @@ seed-baselines-local:
# Deprecated alias — kept for muscle memory; see seed-baselines-local warning above.
update-baselines: seed-baselines-local

gen-tool-types-manifest:
PYTHONPATH=. python scripts/gen_tool_types_manifest.py

check-benchmarks:
PYTHONPATH=. pytest tests/benchmarks/ --benchmark-only --benchmark-json=benchmark-results.json -o addopts=
PYTHONPATH=. python scripts/check_benchmark_regression.py benchmark-results.json benchmarks/baselines.json
Expand Down
1 change: 1 addition & 0 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ The UI is a **hash-routed** SPA with ES modules under `static/js/`:
- `app.js` — routing and boot
- `projects.js`, `sessions.js`, `search.js`, `export.js` — route handlers
- `render/registry.js` — **tool dispatch registry** for session UI: `TOOL_USE_RENDERERS` and `TOOL_RESULT_RENDERERS` map tool name / `result_type` → render function (one module per type under `render/tool_use/` and `render/tool_result/`). Parallels backend `utils/tool_dispatch.py` (backend uses ordered predicates; frontend uses direct key lookup + fallback).
- `static/tool_types.json` — generated manifest of backend tool-use names (`python scripts/gen_tool_types_manifest.py` from `KNOWN_TOOL_TYPES`). Fetched non-blocking at boot by `render/tool_types_manifest.js` (`void initToolTypesManifest()` in `app.js`), which cross-checks `TOOL_USE_RENDERERS` and logs `console.warn` on drift.
- `shared/markdown.js` — markdown + **DOMPurify** sanitization (do not render raw LLM HTML)
- `shared/state.js`, `shared/utils.js`, `shared/theme.js` — shared UI state and helpers

Expand Down
36 changes: 36 additions & 0 deletions scripts/gen_tool_types_manifest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/usr/bin/env python3
"""Write ``static/tool_types.json`` from ``KNOWN_TOOL_TYPES``.

Run after adding a tool type to ``utils/tool_dispatch.py``::

python scripts/gen_tool_types_manifest.py
"""

from __future__ import annotations

import json
import sys
from pathlib import Path

_REPO_ROOT = Path(__file__).resolve().parents[1]
_MANIFEST_PATH = _REPO_ROOT / "static" / "tool_types.json"


def write_tool_types_manifest(path: Path | None = None) -> int:
if str(_REPO_ROOT) not in sys.path:
sys.path.insert(0, str(_REPO_ROOT))
from utils.tool_dispatch import KNOWN_TOOL_TYPES

dest = path or _MANIFEST_PATH
payload = {"tool_types": sorted(KNOWN_TOOL_TYPES)}
dest.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8")
return len(KNOWN_TOOL_TYPES)


def main() -> None:
count = write_tool_types_manifest()
print(f"Wrote {count} tool types to {_MANIFEST_PATH}")


if __name__ == "__main__":
main()
2 changes: 2 additions & 0 deletions static/js/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { HLJS_THEME_SHEETS, applyTheme, toggleTheme } from './shared/theme.js';
import { showProjects } from './projects.js';
import { showWorkspace, loadSession } from './sessions.js';
import { showSearchPage } from './search.js';
import { initToolTypesManifest } from './render/tool_types_manifest.js';

// ==================== Router ====================

Expand Down Expand Up @@ -52,6 +53,7 @@ document.addEventListener('DOMContentLoaded', () => {
applyTheme(localStorage.getItem('theme') || 'dark');
const yearEl = document.getElementById('footer-year');
if (yearEl) yearEl.textContent = new Date().getFullYear();
void initToolTypesManifest();
Comment thread
clean6378-max-it marked this conversation as resolved.
handleRoute();
window.addEventListener('hashchange', handleRoute);

Expand Down
3 changes: 3 additions & 0 deletions static/js/app.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ vi.mock('./shared/theme.js', () => ({
toggleTheme,
setWorkspaceMode: vi.fn(),
}));
vi.mock('./render/tool_types_manifest.js', () => ({
initToolTypesManifest: vi.fn().mockResolvedValue(undefined),
}));

describe('router (app.js)', () => {
const origScrollTo = window.scrollTo;
Expand Down
7 changes: 4 additions & 3 deletions static/js/render/registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,10 @@ function getToolUseRenderer(name) {
}

function getToolResultRenderer(resultType) {
return Object.prototype.hasOwnProperty.call(TOOL_RESULT_RENDERERS, resultType)
? TOOL_RESULT_RENDERERS[resultType]
: renderToolResultFallback;
if (Object.prototype.hasOwnProperty.call(TOOL_RESULT_RENDERERS, resultType)) {
return TOOL_RESULT_RENDERERS[resultType];
}
return renderToolResultFallback;
}

export function renderToolUse(tool) {
Expand Down
16 changes: 11 additions & 5 deletions static/js/render/registry.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,13 @@ describe('getToolSummary', () => {
});

describe('renderToolUse fallback', () => {
it('uses JSON fallback for unknown tools', () => {
it('uses JSON fallback for unknown tools with raw type name', () => {
const html = renderToolUse({
name: 'UnknownToolXYZ',
input: { foo: 'bar' },
});
expect(html).toContain('tool-call');
expect(html).toContain('Unknown tool: UnknownToolXYZ');
expect(html).toContain('"foo"');
expect(TOOL_USE_RENDERERS.UnknownToolXYZ).toBeUndefined();
});
Expand Down Expand Up @@ -188,15 +189,20 @@ describe('renderTodoWriteResult', () => {
});

describe('renderToolResult fallback', () => {
it('renders summary-only for unknown result types', () => {
const html = renderToolResult({ result_type: 'custom_type' });
expect(html).toContain('Tool result (custom_type)');
it('renders raw result type and JSON payload for unknown result types', () => {
const html = renderToolResult({
result_type: 'custom_type',
payload: { answer: 42 },
});
expect(html).toContain('Unknown tool result: custom_type');
expect(html).toContain('tool-result');
expect(html).toContain('"answer"');
expect(html).toContain('42');
});

it('uses fallback when result_type is an inherited property (e.g. constructor)', () => {
const html = renderToolResult({ result_type: 'constructor' });
expect(html).toContain('Tool result (constructor)');
expect(html).toContain('Unknown tool result: constructor');
expect(html).toContain('tool-result');
expect(Object.prototype.hasOwnProperty.call(TOOL_RESULT_RENDERERS, 'constructor')).toBe(false);
});
Expand Down
7 changes: 5 additions & 2 deletions static/js/render/tool_result/fallback.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { esc, truncate } from '../../shared/utils.js';
import { finishToolResult } from './common.js';
import { UNKNOWN_DISPATCH_KEY } from '../constants.js';

export function renderToolResultFallback(parsed) {
const rt = parsed.result_type || UNKNOWN_DISPATCH_KEY;
const summary = `Tool result (${rt})`;
return finishToolResult(summary, '');
const summary = `Unknown tool result: ${rt}`;
const payload = JSON.stringify(parsed, null, 2);
const body = `<pre><code>${esc(truncate(payload, 500))}</code></pre>`;
return finishToolResult(summary, body);
}
48 changes: 48 additions & 0 deletions static/js/render/tool_types_manifest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { TOOL_USE_RENDERERS } from './registry.js';

const MANIFEST_URL = '/static/tool_types.json';
const MANIFEST_FETCH_TIMEOUT_MS = 5000;

/**
* Load backend tool-type manifest and cross-check ``TOOL_USE_RENDERERS``.
* Logs ``console.warn`` when the backend list and frontend registry diverge.
*/
export async function initToolTypesManifest() {
const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), MANIFEST_FETCH_TIMEOUT_MS);
try {
const res = await fetch(MANIFEST_URL, { signal: controller.signal });
if (!res.ok) {
console.warn(`[tool registry] Could not load ${MANIFEST_URL}: HTTP ${res.status}`);
return;
}
const data = await res.json();
const types = Array.isArray(data.tool_types) ? data.tool_types : [];
const manifest = new Set(types.filter((t) => typeof t === 'string'));

for (const name of manifest) {
if (!Object.prototype.hasOwnProperty.call(TOOL_USE_RENDERERS, name)) {
console.warn(
`[tool registry] Backend tool type "${name}" has no TOOL_USE_RENDERERS entry`,
);
}
}
for (const name of Object.keys(TOOL_USE_RENDERERS)) {
if (!manifest.has(name)) {
console.warn(
`[tool registry] TOOL_USE_RENDERERS entry "${name}" is missing from ${MANIFEST_URL}`,
);
}
}
} catch (err) {
if (err?.name === 'AbortError') {
console.warn(
`[tool registry] Could not load ${MANIFEST_URL}: timed out after ${MANIFEST_FETCH_TIMEOUT_MS}ms`,
);
return;
}
console.warn('[tool registry] Could not load tool types manifest:', err);
} finally {
clearTimeout(timeoutId);
}
}
70 changes: 70 additions & 0 deletions static/js/render/tool_types_manifest.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { initToolTypesManifest } from './tool_types_manifest.js';
import { TOOL_USE_RENDERERS } from './registry.js';

describe('initToolTypesManifest', () => {
beforeEach(() => {
vi.restoreAllMocks();
});

afterEach(() => {
vi.unstubAllGlobals();
});

it('cross-checks manifest against TOOL_USE_RENDERERS and warns on drift', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
const manifestTypes = [...Object.keys(TOOL_USE_RENDERERS), 'FutureToolXYZ'];
vi.stubGlobal(
'fetch',
vi.fn().mockResolvedValue({
ok: true,
json: async () => ({ tool_types: manifestTypes }),
}),
);

await initToolTypesManifest();

expect(warn).toHaveBeenCalledWith(
'[tool registry] Backend tool type "FutureToolXYZ" has no TOOL_USE_RENDERERS entry',
);
});

it('warns when fetch fails', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network')));

await initToolTypesManifest();

expect(warn).toHaveBeenCalledWith(
'[tool registry] Could not load tool types manifest:',
expect.any(Error),
);
});

it('warns when fetch times out', async () => {
vi.useFakeTimers();
try {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
vi.stubGlobal(
'fetch',
vi.fn((_url, init) =>
new Promise((_resolve, reject) => {
init?.signal?.addEventListener('abort', () => {
reject(Object.assign(new Error('aborted'), { name: 'AbortError' }));
});
}),
),
);

const promise = initToolTypesManifest();
await vi.advanceTimersByTimeAsync(5000);
await promise;

expect(warn).toHaveBeenCalledWith(
'[tool registry] Could not load /static/tool_types.json: timed out after 5000ms',
);
} finally {
vi.useRealTimers();
}
});
});
3 changes: 1 addition & 2 deletions static/js/render/tool_use/fallback.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { esc, truncate } from '../../shared/utils.js';
import { getToolSummary } from './summary.js';
import { wrapToolUse } from './common.js';
import { UNKNOWN_DISPATCH_KEY } from '../constants.js';

export function renderToolUseFallback(tool) {
const name = tool.name || UNKNOWN_DISPATCH_KEY;
const inp = tool.input || {};
const summary = getToolSummary(name, inp);
const summary = `Unknown tool: ${name}`;
const s = JSON.stringify(inp, null, 2);
const body = `<pre><code>${esc(truncate(s, 500))}</code></pre>`;
return wrapToolUse(summary, body);
Expand Down
15 changes: 15 additions & 0 deletions static/tool_types.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"tool_types": [
"AskUserQuestion",
"Bash",
"Edit",
"Glob",
"Grep",
"Read",
"Task",
"TodoWrite",
"WebFetch",
"WebSearch",
"Write"
]
}
39 changes: 39 additions & 0 deletions tests/test_tool_dispatch_sync.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,29 @@
"""Contract test: ``KNOWN_TOOL_TYPES`` must match all four dispatch sites.

Sites (each compared to ``KNOWN_TOOL_TYPES`` in ``utils/tool_dispatch.py``):
- ``static/tool_types.json`` — generated manifest
- ``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 json
import re
from pathlib import Path
from typing import get_args

import pytest

from models.tool_results import ToolNameLiteral
from scripts.gen_tool_types_manifest import write_tool_types_manifest
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"
_TOOL_TYPES_MANIFEST = _REPO_ROOT / "static" / "tool_types.json"


def _format_set_diff(expected: frozenset[str], actual: frozenset[str], site: str) -> str:
Expand Down Expand Up @@ -81,6 +85,40 @@ def _parse_md_exporter_tool_use_handlers(path: Path) -> frozenset[str]:
return frozenset(names)


def _load_manifest_tool_types(path: Path) -> frozenset[str]:
if not path.is_file():
msg = f"Missing manifest: {path} (run python scripts/gen_tool_types_manifest.py)"
raise ValueError(msg)
data = json.loads(path.read_text(encoding="utf-8"))
raw = data.get("tool_types")
if not isinstance(raw, list):
msg = f"Invalid tool_types in {path}: expected a JSON array"
raise ValueError(msg)
for i, item in enumerate(raw):
if not isinstance(item, str):
msg = f"Invalid tool_types[{i}] in {path}: expected string, got {type(item).__name__}"
raise ValueError(msg)
return frozenset(raw)


def test_tool_types_manifest_matches_known_tool_types() -> None:
site = "static/tool_types.json"
try:
actual = _load_manifest_tool_types(_TOOL_TYPES_MANIFEST)
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_tool_types_manifest_is_committed_and_current(tmp_path: Path) -> None:
"""Regenerating the manifest must match the committed file."""
expected = tmp_path / "tool_types.json"
write_tool_types_manifest(expected)
committed = _TOOL_TYPES_MANIFEST.read_text(encoding="utf-8")
assert expected.read_text(encoding="utf-8") == committed


def test_md_exporter_handlers_match_known_tool_types() -> None:
site = "utils/md_exporter.py (_render_tool_use branches)"
try:
Expand All @@ -99,6 +137,7 @@ def test_tool_name_literal_matches_known_tool_types() -> None:


def test_frontend_registry_matches_known_tool_types() -> None:
"""``TOOL_USE_RENDERERS`` keys must match ``KNOWN_TOOL_TYPES``."""
site = "static/js/render/registry.js (TOOL_USE_RENDERERS)"
try:
actual = _parse_frontend_tool_use_renderers(_FRONTEND_REGISTRY)
Expand Down
Loading