Skip to content

Commit e84b368

Browse files
committed
fix(diagrams): #104 review — a name every object answers to is not a model, a version or a shape
Two findings from the Copilot review, both right, and a third of the same kind that looking for them turned up. stats.js — "IDs such as `constructor` resolve inherited properties". The counters are plain objects keyed by model id, and a model id is whatever a provider calls it. For a model called "constructor" the lookup found Object itself: `calls++` wrote to Object, and the next line threw. For "__proto__" it found the prototype every object shares, and `calls++` wrote there — every object in the extension host had a `calls` of NaN — before it threw. The service swallows a counter that throws, so the diagram was still drawn; the write was not undone. An error class is a key too: "constructor" passes the class pattern, and its count came out as "function Object() { [native code] }1". A key is now read as an own property and written as one (defineProperty, so "__proto__" is an entry and not a call to the prototype's setter). The store stays an ordinary object: what is already in the editor's storage reads as before, and it survives the JSON round trip it is stored through. validate.js — "a crafted schema version such as `toString` resolves through SCHEMAS' prototype". validate() promises a list of errors and threw instead: SCHEMAS["toString"] is a function, so the version was "known", and the check that followed read `.type` of undefined. prepare() threw with it, and so did accept(), which is what reads a stored record back. A version is now looked up only when it is one of the numbers this editor reads. That is deliberately stricter than "an own key of the registry": no schema declares `v`, so the string "1" passed as version 1 and was stored as a string, with nothing after the lookup to object. On the ladder a version written as digits is still read as the number, as before. repair.js — the same lookup a third time, found by feeding those names through every field. The synonym tables are objects, and `"shape": "constructor"` found Object: the spec came out with a FUNCTION for a shape, and the model was told "/nodes/0/shape: expected a string, got function" about a string it had written. A line style of that name was read as solid without a word. A word is now looked up as the table's own entry, so this one is unknown like any other unknown word: drawn with the default, and said so. Nothing else in these modules indexes an object with text from a spec: ids, labels and groups are kept in Maps, and the schema is walked by its own field names. The chat page keeps a table of its own, keyed by tool-call id; that one is fixed with the page, two commits on. Each has a test that fails on the code as reviewed: six such names as model ids and one as an error class, with nothing written to Object or its prototype, before and after a trip through JSON; fifteen things that are not version 1 — the string "1" and the list [1] among them — as one error each and never a throw, and four of them up the ladder and through accept(); seven such words as a direction, a shape and a line style, with exactly the outcome of "zigzag".
1 parent a317a45 commit e84b368

7 files changed

Lines changed: 137 additions & 11 deletions

File tree

‎extensions/levelcode-ai/diagram/repair.js‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,9 @@
172172
};
173173
const WRAPPERS = ['spec', 'diagram', 'graph', 'input', 'json', 'arguments', 'data'];
174174
const key = (s) => String(s == null ? '' : s).toLowerCase().trim().replace(/[\s_]+/g, '-');
175+
// A synonym table is an object, and every object answers to "constructor": the word is looked up as
176+
// the table's OWN entry, so a word it has never heard of is unknown whatever it happens to be.
177+
const synonym = (table, word) => { const k = key(word); return Object.prototype.hasOwnProperty.call(table, k) ? table[k] : undefined; };
175178
const pick = (obj, names) => { for (const n of names) { if (obj[n] !== undefined && obj[n] !== null) { return obj[n]; } } return undefined; };
176179
const asBool = (v) => (v === true || v === 1 || (typeof v === 'string' && /^(true|yes|1)$/i.test(v.trim())));
177180
const scalar = (v) => (typeof v === 'string' || typeof v === 'number') ? String(v) : undefined;
@@ -239,7 +242,7 @@
239242
const dirRaw = pick(raw, ['direction', 'dir', 'rankdir', 'orientation', 'flow']);
240243
if (dirRaw === undefined) { spec.direction = 'right'; }
241244
else {
242-
const d = DIRECTION_SYNONYMS[key(dirRaw)];
245+
const d = synonym(DIRECTION_SYNONYMS, dirRaw);
243246
if (d) { spec.direction = d; if (d !== dirRaw) { fix('/direction', 'synonym', JSON.stringify(dirRaw) + ' read as "' + d + '".'); } }
244247
else { spec.direction = 'right'; fix('/direction', 'enum', JSON.stringify(dirRaw) + ' is not a direction; used "right".', { show: true }); }
245248
}
@@ -317,7 +320,7 @@
317320
if (sub) { out.sub = sub; }
318321
const shapeRaw = pick(n, ['shape', 'type', 'kind']);
319322
if (shapeRaw !== undefined) {
320-
const s = SHAPE_SYNONYMS[key(shapeRaw)];
323+
const s = synonym(SHAPE_SYNONYMS, shapeRaw);
321324
if (s) { if (s !== 'box') { out.shape = s; } if (s !== shapeRaw) { fix(at + '/shape', 'synonym', JSON.stringify(shapeRaw) + ' read as "' + s + '".'); } }
322325
else { fix(at + '/shape', 'enum', JSON.stringify(shapeRaw) + ' is not a shape; drew a box.', { show: true }); }
323326
}
@@ -409,7 +412,7 @@
409412
if (label !== undefined && cleanText(label)) { out.label = cleanText(label); }
410413
const styleRaw = pick(e, ['style', 'type', 'line', 'kind']);
411414
if (styleRaw !== undefined) {
412-
const s = STYLE_SYNONYMS[key(styleRaw)];
415+
const s = synonym(STYLE_SYNONYMS, styleRaw);
413416
if (s === 'dashed') { out.style = 'dashed'; }
414417
if (!s) { fix(at + '/style', 'enum', JSON.stringify(styleRaw) + ' is not a line style; drew it solid.', { show: true }); }
415418
else if (s !== styleRaw) { fix(at + '/style', 'synonym', JSON.stringify(styleRaw) + ' read as "' + s + '".'); }
@@ -523,7 +526,7 @@
523526
else { out.title = 'Untitled diagram'; notes.push('no title given'); }
524527
if (typeof spec.tip === 'string') { out.tip = truncate(spec.tip, HARD.tip); }
525528
// A spec from a NEWER editor than this one: draw what this schema understands, and say so.
526-
if (spec.v !== undefined && !schema.SCHEMAS[spec.v]) { notes.push('written for schema v' + String(spec.v).slice(0, 8) + '; drawn as v' + schema.VERSION); }
529+
if (spec.v !== undefined && !schema.schemaFor(spec.v)) { notes.push('written for schema v' + String(spec.v).slice(0, 8) + '; drawn as v' + schema.VERSION); }
527530

528531
// nodes: keep the well-formed ones, up to the hard ceiling
529532
const ids = new Set();
@@ -679,7 +682,7 @@
679682
if (r.ok) {
680683
// Valid — but the validator checks the fields it knows and is silent about any others, and a
681684
// record from a session file can carry anything. What is handed on is the declared shape only.
682-
const declared = schema.project(schema.SCHEMAS[m.v].hard, m);
685+
const declared = schema.project(schema.schemaFor(m.v).hard, m);
683686
return { ok: true, spec: Object.assign({ v: m.v }, declared), notes: [] };
684687
}
685688
const p = prepare(spec, { final: true });

‎extensions/levelcode-ai/diagram/schema.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,16 @@
139139
1: { house: build('house'), hard: build('hard') }
140140
});
141141
const KNOWN_VERSIONS = Object.keys(SCHEMAS).map(Number);
142+
/**
143+
* The registry entry for a version — or null. `v` is INPUT (a model wrote it, or a session file
144+
* holds it), and a plain lookup answers to names every object has: SCHEMAS["toString"] is a
145+
* function, not a schema, and SCHEMAS["__proto__"] is the prototype. So it is looked up only when
146+
* it IS one of the versions this editor reads — a number, which also means the string "1" is not
147+
* version 1: no schema declares `v`, so nothing after this would say so.
148+
*/
149+
function schemaFor(v) {
150+
return KNOWN_VERSIONS.indexOf(v) >= 0 ? SCHEMAS[v] : null;
151+
}
142152

143153
// ---- a JSON-Schema-subset interpreter -------------------------------------------------------
144154
// Exactly the keywords build() uses, and no more: type, properties, required, items, enum,
@@ -249,5 +259,5 @@
249259
return value;
250260
}
251261

252-
return { VERSION, KNOWN_VERSIONS, SHAPES, DIRECTIONS, EDGE_STYLES, ID_RE, ID_PATTERN, HOUSE, HARD, SCHEMAS, build, check, project, typeOf, seg };
262+
return { VERSION, KNOWN_VERSIONS, SHAPES, DIRECTIONS, EDGE_STYLES, ID_RE, ID_PATTERN, HOUSE, HARD, SCHEMAS, schemaFor, build, check, project, typeOf, seg };
253263
}));

‎extensions/levelcode-ai/diagram/stats.js‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,17 @@ function empty(since) {
3131
return { v: VERSION, since: since || null, calls: 0, byModel: {}, errors: {}, fixes: {}, tokens: TOKEN_BUCKETS.map(() => 0).concat([0]), renderMs: MS_BUCKETS.map(() => 0).concat([0]), rendered: 0, renderFailed: 0, flipped: 0, asciiLeaks: 0, answers: 0, linkClicks: 0, exports: {} };
3232
}
3333
const bucket = (bounds, value) => { let i = 0; while (i < bounds.length && value > bounds[i]) { i++; } return i; };
34-
const inc = (obj, key, cap) => { if (obj[key] !== undefined || Object.keys(obj).length < cap) { obj[key] = (obj[key] || 0) + 1; } };
34+
// The stores below are keyed by names that arrive from outside — a model id is whatever a provider
35+
// calls it. A plain `map[key]` answers to names every object has: map["constructor"] is Object, and
36+
// map["__proto__"] is the prototype all objects share, which a careless `++` then writes to. So a key
37+
// is only ever READ as an own property, and only ever WRITTEN as one (assignment to "__proto__" would
38+
// go to the prototype setter instead).
39+
const own = (map, key) => (Object.prototype.hasOwnProperty.call(map, key) ? map[key] : undefined);
40+
const put = (map, key, value) => { Object.defineProperty(map, key, { value, writable: true, enumerable: true, configurable: true }); return value; };
41+
const inc = (map, key, cap) => {
42+
const cur = own(map, key);
43+
if (cur !== undefined || Object.keys(map).length < cap) { put(map, key, (cur || 0) + 1); }
44+
};
3545

3646
/**
3747
* Fold one event in.
@@ -49,10 +59,13 @@ function record(stats, ev) {
4959
if (!outcome) { return s; }
5060
s.calls++;
5161
const model = String(ev.model || 'unknown').slice(0, 80);
52-
if (!s.byModel[model] && Object.keys(s.byModel).length >= MAX_MODELS) { return s; }
53-
const m = s.byModel[model] || (s.byModel[model] = { calls: 0, outcomes: {}, errors: {} });
62+
let m = own(s.byModel, model);
63+
if (m === undefined) {
64+
if (Object.keys(s.byModel).length >= MAX_MODELS) { return s; }
65+
m = put(s.byModel, model, { calls: 0, outcomes: {}, errors: {} });
66+
}
5467
m.calls++;
55-
m.outcomes[outcome] = (m.outcomes[outcome] || 0) + 1;
68+
m.outcomes[outcome] = (m.outcomes[outcome] || 0) + 1; // (`outcome` is one of OUTCOMES: no name to collide with)
5669
for (const cls of Object.keys(ev.errorClasses || {})) { if (CLASS_RE.test(cls)) { inc(s.errors, cls, MAX_CLASSES); inc(m.errors, cls, MAX_CLASSES); } }
5770
for (const cls of Object.keys(ev.fixClasses || {})) { if (CLASS_RE.test(cls)) { inc(s.fixes, cls, MAX_CLASSES); } }
5871
if (Number.isFinite(ev.tokens) && ev.tokens > 0) { s.tokens[bucket(TOKEN_BUCKETS, ev.tokens)]++; }

‎extensions/levelcode-ai/diagram/validate.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@
152152
return { ok: false, errors };
153153
}
154154
const v = spec.v === undefined ? schema.VERSION : spec.v;
155-
const known = schema.SCHEMAS[v];
155+
const known = schema.schemaFor(v);
156156
if (!known) {
157157
errors.push({ pointer: '/v', cls: 'version', message: 'unknown schema version ' + JSON.stringify(spec.v) + '. This editor reads: ' + schema.KNOWN_VERSIONS.join(', ') + '.' });
158158
return { ok: false, errors };

‎extensions/levelcode-ai/test/diagramRepair.test.js‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,56 @@ test('SAFETY: a spec cannot reach Object.prototype', () => {
337337
for (const x of r.spec.nodes) { assert.deepStrictEqual(Object.keys(x).sort(), ['id', 'label']); }
338338
});
339339

340+
test('VERSION: a crafted version goes up the ladder like any other mistake — an error for the model, then drawn as v1 with a note', () => {
341+
for (const v of ['toString', '__proto__', 'constructor', 'hasOwnProperty']) {
342+
const input = Object.assign(JSON.parse('{"v":' + JSON.stringify(v) + '}'), jev(), { v });
343+
const first = R.prepare(input);
344+
assert.strictEqual(first.status, 'errors', v);
345+
assert.deepStrictEqual(first.errors.map((e) => e.cls), ['version'], v);
346+
const last = R.prepare(input, { final: true });
347+
assert.strictEqual(last.status, 'degraded', v);
348+
assert.strictEqual(last.spec.v, 1, 'it is drawn as the version this editor reads');
349+
assert.ok(last.notes.some((n) => /written for schema v/.test(n)), JSON.stringify(last.notes));
350+
// and a stored record that claims such a version is read the same way, not thrown on
351+
const stored = R.accept(input);
352+
assert.strictEqual(stored.ok, true, v);
353+
assert.strictEqual(stored.spec.v, 1);
354+
assert.strictEqual(stored.spec.nodes.length, 4);
355+
}
356+
// a version written as a string of digits is still just a tidy-up
357+
const asText = R.prepare(Object.assign(jev(), { v: '1' }));
358+
assert.strictEqual(asText.spec.v, 1);
359+
assert.ok(asText.status === 'ok' || asText.status === 'fixed', asText.status);
360+
});
361+
362+
test('SYNONYMS: a word every object answers to is an unknown word like any other — "constructor" is not a shape', () => {
363+
// The synonym tables are objects. Looked up by name alone, `constructor` finds Object itself: the
364+
// spec came out with a FUNCTION for a direction, and the model was told "expected a string, got
365+
// function" about a string it had written.
366+
const outcomeOf = (word) => {
367+
const input = jev();
368+
input.direction = word; input.nodes[0].shape = word; input.edges[0].style = word;
369+
const first = R.prepare(JSON.parse(JSON.stringify(input)));
370+
return {
371+
status: first.status,
372+
errors: first.errors.map((e) => e.pointer + ' ' + e.cls),
373+
fixes: first.fixes.map((f) => f.pointer + ' ' + f.cls + ' ' + f.message.split(word).join('<word>')).sort(),
374+
spec: first.spec ? [first.spec.direction, first.spec.nodes[0].shape, first.spec.edges[0].style] : null
375+
};
376+
};
377+
const unknown = outcomeOf('zigzag');
378+
assert.strictEqual(unknown.status, 'fixed', 'an unknown word is drawn with the default, and says so');
379+
assert.deepStrictEqual(unknown.spec, ['right', undefined, undefined]);
380+
assert.deepStrictEqual(unknown.fixes, ['/direction enum "<word>" is not a direction; used "right".', '/edges/0/style enum "<word>" is not a line style; drew it solid.', '/nodes/0/shape enum "<word>" is not a shape; drew a box.']);
381+
for (const word of ['constructor', 'Constructor', '__proto__', 'toString', 'hasOwnProperty', 'valueOf', 'prototype']) {
382+
assert.deepStrictEqual(outcomeOf(word), unknown, word);
383+
}
384+
// and the real synonyms are still read
385+
const real = jev(); real.direction = 'TB'; real.nodes[0].shape = 'Diamond'; real.edges[0].style = 'dotted';
386+
const read = R.prepare(real);
387+
assert.deepStrictEqual([read.spec.direction, read.spec.nodes[0].shape, read.spec.edges[0].style], ['down', 'decision', 'dashed']);
388+
});
389+
340390
test('ROBUST: prepare() never throws and always names a status — seeded junk, both attempts', () => {
341391
let seed = 20261004;
342392
const rnd = () => { seed = (seed * 1664525 + 1013904223) >>> 0; return seed / 4294967296; };

‎extensions/levelcode-ai/test/diagramSchema.test.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,22 @@ test('VERSION: an unknown version is refused by name; no version reads as v1', (
181181
assert.strictEqual(none.v, undefined, 'migrate() returns a copy');
182182
});
183183

184+
test('VERSION: a version is one of the NUMBERS this editor reads — "toString" is on every object, and is not a schema', () => {
185+
const spec = (v) => ({ v, title: 'T', nodes: [{ id: 'a', label: 'A' }], edges: [] });
186+
// (the string "1" and the list [1] are here too: both spell the key of v1, and neither is version 1)
187+
for (const v of ['toString', '__proto__', 'constructor', 'hasOwnProperty', 'valueOf', '1', [1], 1.5, 0, -1, 2, true, null, [], {}]) {
188+
let r;
189+
assert.doesNotThrow(() => { r = V.validate(spec(v)); }, 'v = ' + JSON.stringify(v));
190+
assert.deepStrictEqual(r.errors.map((e) => [e.pointer, e.cls]), [['/v', 'version']], 'v = ' + JSON.stringify(v));
191+
assert.match(r.errors[0].message, /^unknown schema version .*\. This editor reads: 1\.$/);
192+
assert.strictEqual(V.validate(spec(v), { tier: 'hard' }).ok, false);
193+
}
194+
assert.strictEqual(V.validate(spec(1)).ok, true);
195+
assert.strictEqual(V.validate(spec(undefined)).ok, true, 'and a spec that names no version is read as the current one');
196+
assert.strictEqual(schema.schemaFor(1), schema.SCHEMAS[1]);
197+
for (const v of ['1', [1], 'toString', '__proto__', 'constructor', 2, undefined, null]) { assert.strictEqual(schema.schemaFor(v), null, JSON.stringify(v)); }
198+
});
199+
184200
test('VERSION: every schema version in the registry has both tiers and still reads the example (NFR-4)', () => {
185201
assert.ok(schema.KNOWN_VERSIONS.includes(schema.VERSION));
186202
for (const v of schema.KNOWN_VERSIONS) {

‎extensions/levelcode-ai/test/diagramStats.test.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,40 @@ test('BOUNDED: a flood of model ids or class names cannot grow the store without
105105
assert.ok(JSON.stringify(s).length < 2000);
106106
});
107107

108+
test('NAMES EVERY OBJECT ANSWERS TO: a model called "__proto__" or "constructor" is one more model — counted, and nothing else is touched', () => {
109+
// A model id is whatever the user's provider calls it. Looked up carelessly in a plain object,
110+
// "constructor" finds Object itself and "__proto__" finds the prototype every object shares.
111+
const hostile = ['__proto__', 'constructor', 'toString', 'hasOwnProperty', 'valueOf', 'prototype'];
112+
try {
113+
let s = S.empty('2026-10-05');
114+
for (const model of hostile) { s = S.record(s, call(model, 'clean')); s = S.record(s, call(model, 'degraded', { errorClasses: { constructor: 1, 'unknown-node': 1 } })); }
115+
assert.strictEqual(({}).calls, undefined, 'the prototype every object shares was written to');
116+
assert.strictEqual(/** @type {any} */ (Object).calls, undefined, 'Object itself was written to');
117+
assert.strictEqual(Object.getPrototypeOf(s.byModel), Object.prototype, 'and the store is still an ordinary object');
118+
assert.deepStrictEqual(Object.keys(s.byModel).sort(), hostile.slice().sort(), 'each is an entry of its own');
119+
for (const model of hostile) {
120+
const m = Object.getOwnPropertyDescriptor(s.byModel, model).value;
121+
assert.deepStrictEqual([m.calls, m.outcomes.clean, m.outcomes.degraded], [2, 1, 1], model);
122+
assert.strictEqual(m.errors.constructor, 1, 'an error class with such a name is a count too, not text glued to a function');
123+
}
124+
assert.strictEqual(s.calls, 12);
125+
assert.deepStrictEqual(Object.getOwnPropertyDescriptor(s.errors, 'constructor').value, 6);
126+
const sum = S.summarize(s);
127+
assert.deepStrictEqual(sum.byModel.map((m) => m.model), hostile.slice().sort());
128+
assert.ok(sum.byModel.every((m) => m.verdicts === 2 && m.firstPassValid === 50 && m.degraded === 50));
129+
assert.deepStrictEqual(sum.topErrors.map((e) => [e.cls, e.count]), [['constructor', 6], ['unknown-node', 6]]);
130+
// …and it survives the trip through the editor's storage, which is JSON
131+
let back = JSON.parse(JSON.stringify(s));
132+
assert.deepStrictEqual(Object.keys(back.byModel).sort(), hostile.slice().sort());
133+
back = S.record(back, call('__proto__', 'clean'));
134+
assert.strictEqual(Object.getOwnPropertyDescriptor(back.byModel, '__proto__').value.calls, 3);
135+
assert.strictEqual(({}).calls, undefined);
136+
} finally {
137+
// if this test fails it must not take every later one down with it
138+
delete (/** @type {any} */ (Object.prototype)).calls; delete (/** @type {any} */ (Object)).calls;
139+
}
140+
});
141+
108142
test('ROBUST: junk events are ignored; an unknown outcome is not counted; an old store starts over', () => {
109143
let s = S.empty('2026-10-04');
110144
for (const junk of [null, undefined, 3, 'call', {}, { type: 'call' }, { type: 'call', outcome: 'exploded' }, { type: 'render', ms: NaN }, { type: 'render', ms: -4 }, { type: 'nope' }]) { s = S.record(s, junk); }

0 commit comments

Comments
 (0)