Skip to content

Commit 66d54c3

Browse files
committed
fix(chat): three icon names were printed as words — add the glyphs, and guard the set
The chat draws its icons from an inline table through codicon(name), and a name the table lacks is not an error: it is PRINTED. Opening the chat in an editor tab replays the transcript under a banner the host posts with `icon: 'layout'`; the table had no `layout`, so the banner read "layout MOVED TO THE EDITOR <title>". TWO MORE, FOUND BY AUDITING BOTH SIDES. `globe` (the preview chip) and `history` (the recall and project-memory chips) were missing too. They never reach codicon(): addAgentLine checks the table first and falls back to printing the name itself, so the timeline's rail node showed the word. Only on an ungrouped row — inside a group the rail is hidden, which is how it went unnoticed. All three are in the table now, viewBox and path byte-identical to @vscode/codicons 0.0.46-20, the package fifteen of the existing entries were copied from verbatim. `layout` stays the banner's icon rather than borrowing one the set already had: it reads as a change of surface, it is not the `sync` that "Resumed" wears, and chatSurface.test.js already pins it. THE GUARD. test/chatIcons.test.js runs the real table through the real codicon(), lifted out of the shipped chat.html, over every name written down on either side — literals in a codicon() call, `icon = …` assignments, data-ico attributes, and every `icon:` literal in host code — and fails on any that comes back as text, with file and line. Against the tree before this fix it fails on exactly the four call sites. What it cannot see is a name computed at run time. That is closed from the webview's side: a codicon() call that takes its name from somewhere the test does not read fails until it is taught. A host `icon:` built from a variable stays invisible, and the file says so. sketch/agentCatalog.js is exempt — its icons go to the Sketch canvas, which draws none — and the exemption itself fails the day that file starts posting. The print-the-name fallback is left as it is.
1 parent 3ff000e commit 66d54c3

2 files changed

Lines changed: 245 additions & 0 deletions

File tree

‎extensions/levelcode-ai/media/chat.html‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,6 +1643,12 @@
16431643
'sync': { vb: '0 0 16 16', p: '<path d="M2.006 8.267 0.78 9.5 0 8.73l2.09-2.07.76.01 2.09 2.12-.76.76-1.167-1.18a5 5 0 0 0 9.4 1.983l.813.597A6 6 0 0 1 2.006 8.267Zm11.99-.518L15.22 6.51 16 7.29l-2.09 2.07-.76-.01-2.09-2.12.76-.76 1.167 1.18a5 5 0 0 0-9.4-1.983l-.813-.597A6 6 0 0 1 13.996 7.75Z"/>' },
16441644
'chevron-down': { vb: '0 0 16 16', p: '<path d="M3.22 5.97a.75.75 0 0 1 1.06 0L8 9.69l3.72-3.72a.75.75 0 1 1 1.06 1.06l-4.25 4.25a.75.75 0 0 1-1.06 0L3.22 7.03a.75.75 0 0 1 0-1.06z"/>' },
16451645
'circle': { vb: '0 0 16 16', p: '<path fill-rule="evenodd" clip-rule="evenodd" d="M8 1C11.86 1 15 4.14 15 8C15 11.86 11.86 15 8 15C4.14 15 1 11.86 1 8C1 4.14 4.14 1 8 1ZM8 2C4.691 2 2 4.691 2 8C2 11.309 4.691 14 8 14C11.309 14 14 11.309 14 8C14 4.691 11.309 2 8 2Z"/>' },
1646+
// Named only by the HOST, as the `icon:` on a message — layout: the "Moved to the editor" banner;
1647+
// globe: the preview chip; history: the recall and project-memory chips. Nothing in this file spells
1648+
// them, and a name the set lacks is PRINTED as a word in place of the glyph (test/chatIcons.test.js).
1649+
'layout': { vb: '0 0 16 16', p: '<path fill-rule="evenodd" clip-rule="evenodd" d="M5.5 1C6.327 1 7 1.673 7 2.5V13.5C7 14.327 6.327 15 5.5 15H2.5C1.673 15 1 14.327 1 13.5V2.5C1 1.673 1.673 1 2.5 1H5.5ZM2.5 2C2.225 2 2 2.225 2 2.5V13.5C2 13.775 2.225 14 2.5 14H5.5C5.775 14 6 13.775 6 13.5V2.5C6 2.225 5.775 2 5.5 2H2.5Z"/><path fill-rule="evenodd" clip-rule="evenodd" d="M13.5 9C14.327 9 15 9.673 15 10.5V13.5C15 14.327 14.327 15 13.5 15H10.5C9.673 15 9 14.327 9 13.5V10.5C9 9.673 9.673 9 10.5 9H13.5ZM10.5 10C10.225 10 10 10.225 10 10.5V13.5C10 13.775 10.225 14 10.5 14H13.5C13.775 14 14 13.775 14 13.5V10.5C14 10.225 13.775 10 13.5 10H10.5Z"/><path fill-rule="evenodd" clip-rule="evenodd" d="M13.5 1C14.327 1 15 1.673 15 2.5V5.5C15 6.327 14.327 7 13.5 7H10.5C9.673 7 9 6.327 9 5.5V2.5C9 1.673 9.673 1 10.5 1H13.5ZM10.5 2C10.225 2 10 2.225 10 2.5V5.5C10 5.775 10.225 6 10.5 6H13.5C13.775 6 14 5.775 14 5.5V2.5C14 2.225 13.775 2 13.5 2H10.5Z"/>' },
1650+
'globe': { vb: '0 0 16 16', p: '<path d="M8 1C4.141 1 1 4.141 1 8C1 11.859 4.141 15 8 15C11.859 15 15 11.859 15 8C15 4.141 11.859 1 8 1ZM8 14C7.422 14 6.686 12.906 6.288 11H9.713C9.315 12.906 8.579 14 8.001 14H8ZM6.121 10C6.044 9.392 6 8.723 6 8C6 7.277 6.044 6.608 6.121 6H9.878C9.955 6.608 9.999 7.277 9.999 8C9.999 8.723 9.955 9.392 9.878 10H6.121ZM2 8C2 7.299 2.121 6.626 2.343 6H5.121C5.041 6.656 5 7.332 5 8C5 8.668 5.041 9.344 5.121 10H2.343C2.121 9.374 2 8.701 2 8ZM8 2C8.578 2 9.314 3.094 9.712 5H6.287C6.685 3.094 7.422 2 8 2ZM10.879 6H13.657C13.879 6.626 14 7.299 14 8C14 8.701 13.879 9.374 13.657 10H10.879C10.959 9.344 11 8.668 11 8C11 7.332 10.959 6.656 10.879 6ZM13.195 5H10.722C10.516 3.938 10.199 2.98 9.775 2.268C11.228 2.719 12.446 3.707 13.195 5ZM6.226 2.268C5.802 2.98 5.484 3.938 5.279 5H2.806C3.556 3.707 4.774 2.718 6.226 2.268ZM2.805 11H5.278C5.484 12.062 5.801 13.02 6.225 13.732C4.772 13.281 3.554 12.293 2.805 11ZM9.774 13.732C10.198 13.02 10.516 12.062 10.721 11H13.194C12.444 12.293 11.226 13.282 9.774 13.732Z"/>' },
1651+
'history': { vb: '0 0 16 16', p: '<path d="M7.99909 3C10.7605 3 12.9991 5.23858 12.9991 8C12.9991 10.7614 10.7605 13 7.99909 13C5.39117 13 3.2491 11.003 3.0195 8.45512C2.99471 8.1801 2.75167 7.97723 2.47664 8.00202C2.20161 8.0268 1.99875 8.26985 2.02353 8.54488C2.29916 11.6035 4.86898 14 7.99909 14C11.3128 14 13.9991 11.3137 13.9991 8C13.9991 4.68629 11.3128 2 7.99909 2C6.20656 2 4.59815 2.78613 3.49909 4.03138V2.5C3.49909 2.22386 3.27524 2 2.99909 2C2.72295 2 2.49909 2.22386 2.49909 2.5V5.5C2.49909 5.77614 2.72295 6 2.99909 6H3.08812C3.09498 6.00014 3.10184 6.00014 3.10868 6H5.99909C6.27524 6 6.49909 5.77614 6.49909 5.5C6.49909 5.22386 6.27524 5 5.99909 5H3.99863C4.91128 3.78495 6.36382 3 7.99909 3ZM7.99909 5.5C7.99909 5.22386 7.77524 5 7.49909 5C7.22295 5 6.99909 5.22386 6.99909 5.5V8.5C6.99909 8.77614 7.22295 9 7.49909 9H9.49909C9.77524 9 9.99909 8.77614 9.99909 8.5C9.99909 8.22386 9.77524 8 9.49909 8H7.99909V5.5Z"/>' },
16461652
};
16471653
function codicon(name, cls){ const i = IC[name]; if (!i) { return esc(name || ''); } return '<svg class="ci' + (cls ? ' ' + cls : '') + '" viewBox="' + i.vb + '" fill="currentColor" aria-hidden="true">' + i.p + '</svg>'; }
16481654
// Falling-dots progress (Copilot-style) shown while a command runs; styled by .lcdots CSS.
Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Every icon anyone names is one the chat can draw — run: node test/chatIcons.test.js
3+
*
4+
* WHY THIS EXISTS. media/chat.html draws its icons from an inline table (`IC`) through
5+
* codicon(name), and a name the table lacks is not an error — it is PRINTED. The incident: the chat
6+
* moved to an editor tab and its banner read "layout MOVED TO THE EDITOR <title>", the word where
7+
* the glyph belongs. `globe` (the preview chip) and `history` (the recall and project-memory chips)
8+
* had been printing into the timeline rail the same way.
9+
*
10+
* Nothing failed, because the contract has two halves in two files and neither can see the other:
11+
* the HOST names an icon as the `icon:` on a message (extension.js, agent.js) and the WEBVIEW owns
12+
* the table. A name can be added on one side, or dropped from the other, and both still compile.
13+
*
14+
* So this runs the REAL table through the REAL codicon() — lifted out of the shipped chat.html the
15+
* way narrativeUi and sessionsUi lift theirs, not a copy that can drift — over every name written
16+
* down on either side, and fails on any that comes back as text instead of an <svg>.
17+
*
18+
* WHAT IT CANNOT SEE: a name computed at run time. A literal is the only thing a source reader can
19+
* check, so a host `icon:` built from a variable is invisible here. §3 closes that from the
20+
* webview's side, where it can be closed: every codicon() call must take its name from somewhere
21+
* this file reads, so a new source of names fails until it is taught to this file.
22+
*--------------------------------------------------------------------------------------------*/
23+
// @ts-check
24+
'use strict';
25+
26+
const assert = require('assert');
27+
const fs = require('fs');
28+
const path = require('path');
29+
30+
const ROOT = path.join(__dirname, '..');
31+
const html = fs.readFileSync(path.join(ROOT, 'media', 'chat.html'), 'utf8');
32+
33+
let n = 0;
34+
function test(name, fn) { fn(); n++; console.log(' ok - ' + name); }
35+
36+
// The webview's one inline <script> — the same slice narrativeUi compiles.
37+
const scriptOpen = html.lastIndexOf('<script');
38+
const scriptStart = html.indexOf('>', scriptOpen) + 1;
39+
const scriptEnd = html.indexOf('</script>', scriptStart);
40+
assert.ok(scriptOpen >= 0 && scriptEnd > scriptStart, 'no <script> block found in chat.html');
41+
const script = html.slice(scriptStart, scriptEnd);
42+
43+
/** 1-based line of `index` in `text` — every failure below points at a line. */
44+
const lineOf = (text, index) => text.slice(0, index).split('\n').length;
45+
const scriptLine = (index) => lineOf(html, scriptStart + index);
46+
47+
/** Every match of a global `re` in `text`, each with exec()'s `index`. */
48+
function all(re, text) {
49+
const out = [];
50+
let m;
51+
while ((m = re.exec(text))) { out.push(m); }
52+
return out;
53+
}
54+
55+
// ---- The real table and the real lookup, lifted out of the shipped file ---------------------------
56+
57+
/** A one-line helper, whole: from `marker` to the end of its line. */
58+
function oneLiner(marker) {
59+
const at = script.indexOf(marker);
60+
assert.ok(at >= 0, 'chat.html no longer has `' + marker + '`');
61+
return script.slice(at, script.indexOf('\n', at));
62+
}
63+
const tableAt = script.indexOf('const IC = {');
64+
const tableEnd = script.indexOf('\n };', tableAt);
65+
assert.ok(tableAt >= 0 && tableEnd > tableAt, 'chat.html no longer defines the inline icon table `IC`');
66+
let W;
67+
try {
68+
// eslint-disable-next-line no-new-func
69+
W = new Function(
70+
oneLiner('function esc(') + '\n' + script.slice(tableAt, tableEnd + 5) + '\n' + oneLiner('function codicon(')
71+
+ '\nreturn { IC: IC, codicon: codicon };'
72+
)();
73+
} catch (e) {
74+
assert.fail('could not lift `IC` and codicon() out of chat.html (' + String((e && e.message) || e)
75+
+ ') — this file expects esc() and codicon() to stay one-line helpers');
76+
}
77+
const SET = Object.keys(W.IC);
78+
79+
/** What the webview would put on screen for `name`: true for a glyph, false for the bug — the name, as text. */
80+
const draws = (name) => Object.prototype.hasOwnProperty.call(W.IC, name) && /^<svg /.test(W.codicon(name));
81+
82+
// ---- Everywhere a name is written down ------------------------------------------------------------
83+
84+
/**
85+
* Every codicon(…) CALL in the script, as { arg, line } — `arg` is the text of its first argument.
86+
* Scanned outward from each call's own "(" rather than by tokenising the whole script: the script
87+
* holds regex literals (`/"/g`) that a string-tracking pass over the file would trip on.
88+
*/
89+
function codiconCalls() {
90+
const out = [];
91+
for (const m of all(/\bcodicon\(/g, script)) {
92+
const before = script.slice(script.lastIndexOf('\n', m.index) + 1, m.index);
93+
if (/^\s*\/\//.test(before) || /function\s+$/.test(before)) { continue; } // a comment about it, or its definition
94+
const from = m.index + m[0].length;
95+
let depth = 1, i = from, str = '', end = -1;
96+
for (; i < script.length && depth > 0; i++) {
97+
const c = script[i];
98+
if (str) { if (c === str && script[i - 1] !== '\\') { str = ''; } continue; }
99+
if (c === "'" || c === '"' || c === '`') { str = c; }
100+
else if (c === '(' || c === '[' || c === '{') { depth++; }
101+
else if (c === ')' || c === ']' || c === '}') { depth--; }
102+
else if (c === ',' && depth === 1 && end < 0) { end = i; } // codicon(name, cls): the name is the first argument
103+
}
104+
assert.strictEqual(depth, 0, 'unbalanced codicon( at chat.html:' + scriptLine(m.index));
105+
out.push({ arg: script.slice(from, end < 0 ? i - 1 : end).trim(), line: scriptLine(m.index) });
106+
}
107+
return out;
108+
}
109+
110+
/** The string literals in an expression, minus any that sit inside a nested call's parentheses. */
111+
function literals(expr) {
112+
let flat = expr, prev;
113+
do { prev = flat; flat = flat.replace(/\([^()]*\)/g, ''); } while (flat !== prev);
114+
return all(/'([^'\\]*)'|"([^"\\]*)"/g, flat).map((m) => m[1] !== undefined ? m[1] : m[2]);
115+
}
116+
117+
/** Every name chat.html writes down itself, with where. */
118+
function webviewNames() {
119+
const out = [];
120+
for (const c of codiconCalls()) {
121+
for (const name of literals(c.arg)) { out.push({ name, where: 'chat.html:' + c.line + ' codicon(' + c.arg + ')' }); }
122+
}
123+
// A name chosen first and drawn a line later: `icon = ok ? 'check-circle' : 'circle-slash'; … codicon(icon)`.
124+
for (const m of all(/\bicon = ([^;\n]+);/g, script)) {
125+
for (const name of literals(m[1])) { out.push({ name, where: 'chat.html:' + scriptLine(m.index) + ' icon = ' + m[1] }); }
126+
}
127+
// A name in the markup, drawn at startup by the [data-ico] sweep.
128+
for (const m of all(/data-ico="([^"]*)"/g, html)) {
129+
out.push({ name: m[1], where: 'chat.html:' + lineOf(html, m.index) + ' ' + m[0] });
130+
}
131+
return out;
132+
}
133+
134+
// Host files whose `icon:` fields are NOT messages to this webview, and why. Everything else is
135+
// checked: an `icon:` this file cannot place fails, rather than passing because nobody looked.
136+
const NOT_CHAT = new Map([
137+
['sketch/agentCatalog.js', 'palette groups for the Sketch canvas (media/sketch.html), which shows their label and draws no glyph'],
138+
]);
139+
140+
/** Every .js file the extension ships, relative to its root. The webview (media/) and the tests are not host code. */
141+
function hostFiles(dir, rel) {
142+
let out = [];
143+
for (const e of fs.readdirSync(dir, { withFileTypes: true })) {
144+
const r = rel ? rel + '/' + e.name : e.name;
145+
if (e.isDirectory()) {
146+
if (!['test', 'media', 'node_modules'].includes(e.name)) { out = out.concat(hostFiles(path.join(dir, e.name), r)); }
147+
} else if (e.name.endsWith('.js')) { out.push(r); }
148+
}
149+
return out.sort();
150+
}
151+
152+
/** Every `icon: '<name>'` literal in host code, with where. */
153+
function hostNames() {
154+
const out = [];
155+
for (const rel of hostFiles(ROOT, '')) {
156+
if (NOT_CHAT.has(rel)) { continue; }
157+
const src = fs.readFileSync(path.join(ROOT, rel), 'utf8');
158+
for (const m of all(/\bicon:\s*(['"])([^'"\n]*)\1/g, src)) {
159+
out.push({ name: m[2], where: rel + ':' + lineOf(src, m.index) + ' ' + m[0] });
160+
}
161+
}
162+
return out;
163+
}
164+
165+
/** Fail with EVERY undrawable name at once — one run should state the whole problem, not its first line. */
166+
function assertDrawn(found, who) {
167+
const bad = found.filter((f) => !draws(f.name));
168+
assert.ok(!bad.length,
169+
who + ' an icon the inline set does not define — the chat would PRINT the name where the glyph belongs:\n'
170+
+ bad.map((f) => ' "' + f.name + '" <- ' + f.where).join('\n')
171+
+ '\n Add it to `IC` in media/chat.html (viewBox + path from @vscode/codicons), or use one the set has: '
172+
+ SET.join(', '));
173+
}
174+
175+
// ---- 1. The set ---------------------------------------------------------------------------------
176+
177+
test('SET: every entry is something codicon() can draw', () => {
178+
// narrativeUi proves the table PARSES. This is the next thing a bad paste breaks: an entry that is
179+
// valid JavaScript and an empty picture.
180+
assert.ok(SET.length > 0, 'the inline icon table is empty');
181+
for (const name of SET) {
182+
const i = W.IC[name];
183+
assert.match(String(i.vb), /^[\d.]+( [\d.]+){3}$/, name + ' has no usable viewBox');
184+
assert.match(String(i.p), /^<\w[\s\S]*>$/, name + ' has no markup to draw');
185+
assert.ok(draws(name), name + ' is in the table but codicon() does not draw it');
186+
}
187+
});
188+
189+
// ---- 2. Both sides of the contract ---------------------------------------------------------------
190+
191+
test('WEBVIEW: every name chat.html writes down is in the set', () => {
192+
assertDrawn(webviewNames(), 'chat.html names');
193+
});
194+
195+
test('HOST: every icon the host puts on a message is in the set', () => {
196+
// The half the webview cannot see. `layout`, `globe` and `history` all came through here.
197+
assertDrawn(hostNames(), 'the host posts');
198+
});
199+
200+
// ---- 3. The limits of a source reader ------------------------------------------------------------
201+
202+
// A call that does not spell its name out takes it from somewhere else. Each such shape is listed
203+
// with where that somewhere is read above. (`m.icon || 'sync'` needs no entry: it spells its default,
204+
// and `m.icon` is the host's half.)
205+
const INDIRECT = new Map([
206+
['icon', 'a local set from literals a line above (webviewNames), or the host\'s `icon:` handed to addAgentLine (hostNames)'],
207+
["s.getAttribute('data-ico')", 'a data-ico="…" attribute in the markup (webviewNames)'],
208+
]);
209+
210+
test('SHAPE: no codicon() call takes its name from somewhere this file does not read', () => {
211+
const unread = codiconCalls().filter((c) => !literals(c.arg).length && !INDIRECT.has(c.arg));
212+
assert.ok(!unread.length,
213+
'codicon() is called with a name this test cannot trace to a literal:\n'
214+
+ unread.map((c) => ' chat.html:' + c.line + ' codicon(' + c.arg + ')').join('\n')
215+
+ '\n Spell the name in the call, or add the shape to INDIRECT and read its names in webviewNames()/hostNames().');
216+
});
217+
218+
test('EXEMPT: a host file excused from the check really is not talking to the chat', () => {
219+
// An exemption is where a guard goes quiet, so each one has to keep earning it.
220+
for (const [rel, why] of NOT_CHAT) {
221+
const file = path.join(ROOT, rel);
222+
assert.ok(fs.existsSync(file), rel + ' is exempt but no longer exists — drop the exemption');
223+
const src = fs.readFileSync(file, 'utf8');
224+
assert.ok(/\bicon:/.test(src), rel + ' is exempt but names no icon any more — drop the exemption');
225+
assert.ok(!/\bpost(Message)?\(/.test(src), rel + ' is exempt as "' + why + '" but now posts messages — check its icons instead');
226+
}
227+
});
228+
229+
test('ALIVE: the scans found what they exist to check', () => {
230+
// Every assertion above passes on an empty list, so a scan that silently stopped matching — a
231+
// renamed helper, a restyled literal — would turn this whole file into a no-op.
232+
assert.ok(codiconCalls().length > 0, 'found no codicon() call in chat.html');
233+
assert.ok(hostNames().length > 0, 'found no `icon:` literal in any host file');
234+
// …and the host scan is reading the field the webview draws: both renderers take `m.icon`.
235+
assert.match(script, /addAgentLine\(m\.icon,/, 'agentTool no longer hands m.icon to the timeline — hostNames() reads the wrong field');
236+
assert.match(script, /\bcodicon\(m\.icon\b/, 'the banner no longer draws m.icon — hostNames() reads the wrong field');
237+
});
238+
239+
console.log('chatIcons: ' + n + ' tests passed');

0 commit comments

Comments
 (0)