Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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`). Loaded at boot by `render/tool_types_manifest.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()
4 changes: 3 additions & 1 deletion 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 @@ -48,10 +49,11 @@ function handleRoute() {

// ==================== Bootstrap ====================

document.addEventListener('DOMContentLoaded', () => {
document.addEventListener('DOMContentLoaded', async () => {
applyTheme(localStorage.getItem('theme') || 'dark');
const yearEl = document.getElementById('footer-year');
if (yearEl) yearEl.textContent = new Date().getFullYear();
await initToolTypesManifest();
Comment thread
wpak-ai marked this conversation as resolved.
Outdated
handleRoute();
window.addEventListener('hashchange', handleRoute);

Expand Down
4 changes: 4 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 All @@ -33,6 +36,7 @@ describe('router (app.js)', () => {
Element.prototype.scrollIntoView = vi.fn();
await import('./app.js');
document.dispatchEvent(new Event('DOMContentLoaded'));
await Promise.resolve();
});

afterAll(() => {
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);
}
39 changes: 39 additions & 0 deletions static/js/render/tool_types_manifest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { TOOL_USE_RENDERERS } from './registry.js';
import { setManifestToolTypes } from './tool_types_state.js';

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

/**
* 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() {
try {
const res = await fetch(MANIFEST_URL);
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'));
setManifestToolTypes(manifest);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

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) {
console.warn('[tool registry] Could not load tool types manifest:', err);
}
}
49 changes: 49 additions & 0 deletions static/js/render/tool_types_manifest.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { initToolTypesManifest } from './tool_types_manifest.js';
import { getManifestToolTypes, setManifestToolTypes } from './tool_types_state.js';
import { TOOL_USE_RENDERERS } from './registry.js';

// Registry drift warnings are asserted here (init-time cross-check only; no per-render warn).

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

afterEach(() => {
setManifestToolTypes(null);
});

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(getManifestToolTypes()).toEqual(new Set(manifestTypes));
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(getManifestToolTypes()).toBeNull();
expect(warn).toHaveBeenCalledWith(
'[tool registry] Could not load tool types manifest:',
expect.any(Error),
);
});
});
15 changes: 15 additions & 0 deletions static/js/render/tool_types_state.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/** Backend tool-use names from ``static/tool_types.json`` (set at SPA init). */

let manifestToolTypes = null;

export function setManifestToolTypes(types) {
manifestToolTypes = types;
}

export function getManifestToolTypes() {
return manifestToolTypes;
}

export function isManifestToolType(name) {
Comment thread
clean6378-max-it marked this conversation as resolved.
Outdated
return manifestToolTypes !== null && manifestToolTypes.has(name);
}
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"
]
}
38 changes: 38 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,36 @@ 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)
return frozenset(str(t) for t in 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,13 +133,17 @@ 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`` and the manifest."""
site = "static/js/render/registry.js (TOOL_USE_RENDERERS)"
try:
actual = _parse_frontend_tool_use_renderers(_FRONTEND_REGISTRY)
manifest = _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))
if actual != manifest:
Comment thread
clean6378-max-it marked this conversation as resolved.
Outdated
pytest.fail(_format_set_diff(manifest, actual, site))


def test_known_tool_types_nonempty() -> None:
Expand Down
Loading