Skip to content

Commit 965618b

Browse files
refactor(frontend): decompose sessions.js tool rendering into dispatc… (#62)
* refactor(frontend): decompose sessions.js tool rendering into dispatch registry Extract tool use and tool result rendering from sessions.js into static/js/render/ with TOOL_USE_RENDERERS and TOOL_RESULT_RENDERERS, one module per tool type, mirroring utils/tool_dispatch.py. sessions.js retains workspace and message orchestration only. Add Vitest coverage for registry wiring, HTML escaping, and getToolSummary. Document the frontend registry in docs/architecture.md. No intended visual or behavior change for existing sessions. * refactor(frontend): decompose sessions.js tool rendering into dispatch registry Extract tool use and tool result rendering from sessions.js into static/js/render/ with TOOL_USE_RENDERERS and TOOL_RESULT_RENDERERS, one module per tool type, mirroring utils/tool_dispatch.py. Register WebFetch and WebSearch explicitly (JSON body via fallback helper). Fix bash result summary when exit_code is missing (use "unknown" not "exit undefined"). Tighten Vitest: fictitious tool for fallback path, separate WebFetch/WebSearch registry tests. Add Vitest coverage for registry wiring, HTML escaping, and getToolSummary. Document the frontend registry in docs/architecture.md. No intended visual or behavior change for existing sessions. * refactor(frontend): decompose sessions.js tool rendering into dispatch registry Extract tool use and tool result rendering into static/js/render/ with TOOL_USE_RENDERERS and TOOL_RESULT_RENDERERS (one module per type), mirroring utils/tool_dispatch.py. sessions.js keeps workspace orchestration. Harden registry dispatch with hasOwnProperty lookups; register WebFetch and WebSearch explicitly (JSON body via fallback helper). Fix bash result summary when exit_code is missing; align todo_write summary count with parsed.todos. Add Vitest for registry wiring, escaping, fallback vs registered tools, and todo_write count consistency. Document render/registry.js in architecture.md. * fix(frontend): single-layer tool summary escaping and registry tests * fix(frontend): harden render registry dispatch and test toolResultHasBody Introduce UNKNOWN_DISPATCH_KEY sentinel with documented non-registration rule. Guard renderToolUse/renderToolResult against null input; guard toolResultHasBody similarly. Add Vitest coverage for sentinel, null args, and toolResultHasBody contract used by sessions.js.
1 parent 7944ddb commit 965618b

34 files changed

Lines changed: 601 additions & 142 deletions

docs/architecture.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,13 @@ The UI is a **hash-routed** SPA with ES modules under `static/js/`:
105105

106106
- `app.js` — routing and boot
107107
- `projects.js`, `sessions.js`, `search.js`, `export.js` — route handlers
108+
- `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).
108109
- `shared/markdown.js` — markdown + **DOMPurify** sanitization (do not render raw LLM HTML)
109110
- `shared/state.js`, `shared/utils.js`, `shared/theme.js` — shared UI state and helpers
110111

111-
No bundler step — modern browsers load modules directly. Frontend unit tests use **vitest** + **jsdom** (`npm test`).
112+
`sessions.js` keeps workspace/session orchestration and message bubbles; tool cards delegate to `render/registry.js`.
113+
114+
No bundler step — modern browsers load modules directly. Frontend unit tests use **vitest** + **jsdom** (`npm test`), including `static/js/render/registry.test.js` for registry wiring and renderer escaping.
112115

113116
## Continuous integration
114117

static/js/render/constants.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
/**
2+
* Sentinel when tool.name or result_type is missing.
3+
* Used for fallback routing only — do not add UNKNOWN_DISPATCH_KEY to TOOL_*_RENDERERS.
4+
*/
5+
export const UNKNOWN_DISPATCH_KEY = 'unknown';

static/js/render/registry.js

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import { renderBashUse } from './tool_use/bash.js';
2+
import { renderReadUse } from './tool_use/read.js';
3+
import { renderWriteUse } from './tool_use/write.js';
4+
import { renderEditUse } from './tool_use/edit.js';
5+
import { renderGlobUse } from './tool_use/glob.js';
6+
import { renderGrepUse } from './tool_use/grep.js';
7+
import { renderTaskUse } from './tool_use/task.js';
8+
import { renderTodoWriteUse } from './tool_use/todo_write.js';
9+
import { renderAskUserQuestionUse } from './tool_use/ask_user_question.js';
10+
import { renderWebFetchUse } from './tool_use/web_fetch.js';
11+
import { renderWebSearchUse } from './tool_use/web_search.js';
12+
import { renderToolUseFallback } from './tool_use/fallback.js';
13+
import { getToolSummary } from './tool_use/summary.js';
14+
import { UNKNOWN_DISPATCH_KEY } from './constants.js';
15+
16+
import { renderBashResult } from './tool_result/bash.js';
17+
import { renderFileReadResult } from './tool_result/file_read.js';
18+
import { renderFileEditResult } from './tool_result/file_edit.js';
19+
import { renderFileWriteResult } from './tool_result/file_write.js';
20+
import { renderGlobResult } from './tool_result/glob.js';
21+
import { renderGrepResult } from './tool_result/grep.js';
22+
import { renderWebSearchResult } from './tool_result/web_search.js';
23+
import { renderWebFetchResult } from './tool_result/web_fetch.js';
24+
import { renderTaskResult } from './tool_result/task.js';
25+
import { renderTodoWriteResult } from './tool_result/todo_write.js';
26+
import { renderUserInputResult } from './tool_result/user_input.js';
27+
import { renderPlanResult } from './tool_result/plan.js';
28+
import { renderToolResultFallback } from './tool_result/fallback.js';
29+
import { toolResultHasBody } from './tool_result/utils.js';
30+
31+
export { getToolSummary, toolResultHasBody };
32+
33+
export const TOOL_USE_RENDERERS = {
34+
Bash: renderBashUse,
35+
Read: renderReadUse,
36+
Write: renderWriteUse,
37+
Edit: renderEditUse,
38+
Glob: renderGlobUse,
39+
Grep: renderGrepUse,
40+
Task: renderTaskUse,
41+
TodoWrite: renderTodoWriteUse,
42+
AskUserQuestion: renderAskUserQuestionUse,
43+
WebFetch: renderWebFetchUse,
44+
WebSearch: renderWebSearchUse,
45+
};
46+
47+
export const TOOL_RESULT_RENDERERS = {
48+
bash: renderBashResult,
49+
file_read: renderFileReadResult,
50+
file_edit: renderFileEditResult,
51+
file_write: renderFileWriteResult,
52+
glob: renderGlobResult,
53+
grep: renderGrepResult,
54+
web_search: renderWebSearchResult,
55+
web_fetch: renderWebFetchResult,
56+
task: renderTaskResult,
57+
todo_write: renderTodoWriteResult,
58+
user_input: renderUserInputResult,
59+
plan: renderPlanResult,
60+
};
61+
62+
function getToolUseRenderer(name) {
63+
return Object.prototype.hasOwnProperty.call(TOOL_USE_RENDERERS, name)
64+
? TOOL_USE_RENDERERS[name]
65+
: renderToolUseFallback;
66+
}
67+
68+
function getToolResultRenderer(resultType) {
69+
return Object.prototype.hasOwnProperty.call(TOOL_RESULT_RENDERERS, resultType)
70+
? TOOL_RESULT_RENDERERS[resultType]
71+
: renderToolResultFallback;
72+
}
73+
74+
export function renderToolUse(tool) {
75+
if (!tool) return '';
76+
const name = tool.name || UNKNOWN_DISPATCH_KEY;
77+
return getToolUseRenderer(name)(tool);
78+
}
79+
80+
export function renderToolResult(parsed) {
81+
if (!parsed) return '';
82+
const rt = parsed.result_type || UNKNOWN_DISPATCH_KEY;
83+
return getToolResultRenderer(rt)(parsed);
84+
}

static/js/render/registry.test.js

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
import { describe, it, expect } from 'vitest';
2+
import {
3+
TOOL_USE_RENDERERS,
4+
TOOL_RESULT_RENDERERS,
5+
renderToolUse,
6+
renderToolResult,
7+
getToolSummary,
8+
toolResultHasBody,
9+
} from './registry.js';
10+
import { UNKNOWN_DISPATCH_KEY } from './constants.js';
11+
import { renderWebFetchUse } from './tool_use/web_fetch.js';
12+
13+
const CORE_TOOL_USE = ['Bash', 'Read', 'Write', 'Edit', 'Glob', 'Grep', 'Task', 'TodoWrite', 'AskUserQuestion', 'WebFetch', 'WebSearch'];
14+
15+
const CORE_TOOL_RESULT = [
16+
'bash',
17+
'file_read',
18+
'file_edit',
19+
'file_write',
20+
'glob',
21+
'grep',
22+
'web_search',
23+
'web_fetch',
24+
'task',
25+
'todo_write',
26+
'user_input',
27+
'plan',
28+
];
29+
30+
describe('TOOL_USE_RENDERERS', () => {
31+
it('registers core tool names', () => {
32+
for (const name of CORE_TOOL_USE) {
33+
expect(TOOL_USE_RENDERERS[name], name).toBeTypeOf('function');
34+
}
35+
});
36+
37+
it('does not register the unknown dispatch sentinel as a tool renderer', () => {
38+
expect(Object.prototype.hasOwnProperty.call(TOOL_USE_RENDERERS, UNKNOWN_DISPATCH_KEY)).toBe(false);
39+
});
40+
41+
it('renderBashUse escapes HTML in command', () => {
42+
const html = renderToolUse({
43+
name: 'Bash',
44+
input: { command: '<script>alert(1)</script>' },
45+
});
46+
expect(html).not.toContain('<script>');
47+
expect(html).toContain('&lt;script&gt;');
48+
});
49+
50+
it('returns empty string for null or undefined tool', () => {
51+
expect(renderToolUse(null)).toBe('');
52+
expect(renderToolUse(undefined)).toBe('');
53+
});
54+
55+
it('renderReadUse escapes file path in body and summary', () => {
56+
const html = renderToolUse({
57+
name: 'Read',
58+
input: { file_path: 'C:\\tmp\\<evil>.txt' },
59+
});
60+
expect(html).toContain('&lt;evil&gt;');
61+
expect(html).not.toContain('<evil>');
62+
});
63+
});
64+
65+
describe('TOOL_RESULT_RENDERERS', () => {
66+
it('registers core result types', () => {
67+
for (const rt of CORE_TOOL_RESULT) {
68+
expect(TOOL_RESULT_RENDERERS[rt], rt).toBeTypeOf('function');
69+
}
70+
});
71+
72+
it('renderBashResult escapes stdout', () => {
73+
const html = renderToolResult({
74+
result_type: 'bash',
75+
exit_code: 0,
76+
stdout: '<img onerror=alert(1)>',
77+
});
78+
expect(html).not.toContain('<img');
79+
expect(html).toContain('&lt;img');
80+
});
81+
82+
it('renderBashResult avoids undefined in summary when exit_code missing', () => {
83+
const html = renderToolResult({ result_type: 'bash' });
84+
expect(html).toContain('Bash Result (unknown)');
85+
expect(html).not.toContain('undefined');
86+
});
87+
88+
it('returns empty string for null or undefined parsed', () => {
89+
expect(renderToolResult(null)).toBe('');
90+
expect(renderToolResult(undefined)).toBe('');
91+
});
92+
});
93+
94+
describe('toolResultHasBody', () => {
95+
it('returns false for null or undefined', () => {
96+
expect(toolResultHasBody(null)).toBe(false);
97+
expect(toolResultHasBody(undefined)).toBe(false);
98+
});
99+
100+
it('returns true for bash with stdout or stderr', () => {
101+
expect(toolResultHasBody({ result_type: 'bash', stdout: 'ok' })).toBe(true);
102+
expect(toolResultHasBody({ result_type: 'bash', stderr: 'err' })).toBe(true);
103+
expect(toolResultHasBody({ result_type: 'bash' })).toBe(false);
104+
});
105+
106+
it('returns false for summary-only result types', () => {
107+
expect(toolResultHasBody({ result_type: 'file_read', file_path: '/a' })).toBe(false);
108+
expect(toolResultHasBody({ result_type: 'glob', num_files: 3 })).toBe(false);
109+
});
110+
111+
it('returns true for user_input and todo_write with todos', () => {
112+
expect(toolResultHasBody({ result_type: 'user_input' })).toBe(true);
113+
expect(toolResultHasBody({ result_type: 'todo_write', todos: [{ content: 'x' }] })).toBe(true);
114+
expect(toolResultHasBody({ result_type: 'todo_write', todo_count: 1 })).toBe(false);
115+
});
116+
117+
it('returns true for task when duration, retrieval, or description is set', () => {
118+
expect(toolResultHasBody({ result_type: 'task', description: 'subagent' })).toBe(true);
119+
expect(toolResultHasBody({ result_type: 'task', total_duration_ms: 100 })).toBe(true);
120+
expect(toolResultHasBody({ result_type: 'task', status: 'completed' })).toBe(false);
121+
});
122+
});
123+
124+
describe('getToolSummary', () => {
125+
it('formats Bash summary', () => {
126+
expect(getToolSummary('Bash', { command: 'ls -la' })).toMatch(/Bash:/);
127+
expect(getToolSummary('Bash', { command: 'ls -la' })).toContain('ls -la');
128+
});
129+
130+
it('formats Read summary as plain text (escaping deferred to wrapToolUse)', () => {
131+
expect(getToolSummary('Read', { file_path: 'a<b' })).toBe('Read: a<b');
132+
});
133+
});
134+
135+
describe('renderToolUse fallback', () => {
136+
it('uses JSON fallback for unknown tools', () => {
137+
const html = renderToolUse({
138+
name: 'UnknownToolXYZ',
139+
input: { foo: 'bar' },
140+
});
141+
expect(html).toContain('tool-call');
142+
expect(html).toContain('&quot;foo&quot;');
143+
expect(TOOL_USE_RENDERERS.UnknownToolXYZ).toBeUndefined();
144+
});
145+
146+
it('uses fallback when name is an inherited property (e.g. constructor)', () => {
147+
const html = renderToolUse({
148+
name: 'constructor',
149+
input: { foo: 'bar' },
150+
});
151+
expect(html).toContain('tool-call');
152+
expect(html).toContain('&quot;foo&quot;');
153+
});
154+
155+
it('dispatches WebFetch to registered renderer (not generic unknown-tool fallback)', () => {
156+
expect(TOOL_USE_RENDERERS.WebFetch).toBe(renderWebFetchUse);
157+
const html = renderToolUse({
158+
name: 'WebFetch',
159+
input: { url: 'https://example.com' },
160+
});
161+
expect(html).toContain('tool-call');
162+
expect(html).toContain('example.com');
163+
});
164+
165+
it('renders WebSearch via registry', () => {
166+
const html = renderToolUse({
167+
name: 'WebSearch',
168+
input: { query: 'vitest registry' },
169+
});
170+
expect(html).toContain('tool-call');
171+
expect(html).toContain('vitest registry');
172+
});
173+
});
174+
175+
describe('renderTodoWriteResult', () => {
176+
it('summary count matches parsed.todos length when todos are present', () => {
177+
const html = renderToolResult({
178+
result_type: 'todo_write',
179+
todo_count: 99,
180+
todos: [
181+
{ status: 'pending', content: 'one' },
182+
{ status: 'completed', content: 'two' },
183+
],
184+
});
185+
expect(html).toContain('Todos updated (2 items)');
186+
expect(html).not.toContain('99 items');
187+
});
188+
});
189+
190+
describe('renderToolResult fallback', () => {
191+
it('renders summary-only for unknown result types', () => {
192+
const html = renderToolResult({ result_type: 'custom_type' });
193+
expect(html).toContain('Tool result (custom_type)');
194+
expect(html).toContain('tool-result');
195+
});
196+
197+
it('uses fallback when result_type is an inherited property (e.g. constructor)', () => {
198+
const html = renderToolResult({ result_type: 'constructor' });
199+
expect(html).toContain('Tool result (constructor)');
200+
expect(html).toContain('tool-result');
201+
expect(Object.prototype.hasOwnProperty.call(TOOL_RESULT_RENDERERS, 'constructor')).toBe(false);
202+
});
203+
});
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { esc, truncate } from '../../shared/utils.js';
2+
import { finishToolResult } from './common.js';
3+
4+
export function renderBashResult(parsed) {
5+
const exitCode = parsed.exit_code;
6+
let status;
7+
if (parsed.interrupted) {
8+
status = 'interrupted';
9+
} else if (parsed.is_error) {
10+
status = typeof exitCode === 'number' ? `error (exit ${exitCode})` : 'error';
11+
} else if (exitCode === 0) {
12+
status = 'success';
13+
} else if (typeof exitCode === 'number') {
14+
status = `exit ${exitCode}`;
15+
} else {
16+
status = 'unknown';
17+
}
18+
const summary = `Bash Result (${status})`;
19+
let body = '';
20+
if (parsed.stdout) body += `<div class="tool-call-section"><div class="tool-call-section-title">stdout</div><pre><code>${esc(truncate(parsed.stdout, 2000))}</code></pre></div>`;
21+
if (parsed.stderr) body += `<div class="tool-call-section"><div class="tool-call-section-title">stderr</div><pre style="border-left:3px solid var(--danger)"><code>${esc(truncate(parsed.stderr, 1000))}</code></pre></div>`;
22+
return finishToolResult(summary, body);
23+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { esc } from '../../shared/utils.js';
2+
3+
export function finishToolResult(summary, body) {
4+
if (!body) {
5+
return `<div class="tool-result"><span class="tool-result-summary">${esc(summary)}</span></div>`;
6+
}
7+
return `<details class="tool-result"><summary class="tool-result-summary">${esc(summary)}</summary><div class="tool-call-body">${body}</div></details>`;
8+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { finishToolResult } from './common.js';
2+
import { UNKNOWN_DISPATCH_KEY } from '../constants.js';
3+
4+
export function renderToolResultFallback(parsed) {
5+
const rt = parsed.result_type || UNKNOWN_DISPATCH_KEY;
6+
const summary = `Tool result (${rt})`;
7+
return finishToolResult(summary, '');
8+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { finishToolResult } from './common.js';
2+
3+
export function renderFileEditResult(parsed) {
4+
const summary = `Edited: ${parsed.file_path || ''}`;
5+
return finishToolResult(summary, '');
6+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import { finishToolResult } from './common.js';
2+
3+
export function renderFileReadResult(parsed) {
4+
const numLines = parsed.num_lines ? ` (${parsed.num_lines} lines)` : '';
5+
const summary = `Read: ${parsed.file_path || ''}${numLines}`;
6+
return finishToolResult(summary, '');
7+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { finishToolResult } from './common.js';
2+
3+
export function renderFileWriteResult(parsed) {
4+
const summary = `Wrote: ${parsed.file_path || ''}`;
5+
return finishToolResult(summary, '');
6+
}

0 commit comments

Comments
 (0)