From 0d9726d792ea5258ff1ee29869052ce6fe2f4c5d Mon Sep 17 00:00:00 2001 From: Fini Date: Wed, 22 Apr 2026 01:03:40 +0800 Subject: [PATCH] test: insertElementTree direct + builder edge-case coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit insert-element-tree.test.ts (8 cases) — previously only covered indirectly via handler integration tests. Pins parent_id quote- safety pre-check, byte-exact file rollback on rejection, post-insert verification catches silent no-op, happy-path root insert + parent-targeted insert. element-builders-edge-cases.test.ts (20 cases) — robustness boundary coverage: mixed-script i18n (Arabic, emoji, hangul+han, hiragana+latin), extreme numeric values (chart_bars 1e9 / all- zero, stepper total=100, rating_stars overflow clamp), minimal inputs (single-item rows, solo timeline entry), extreme label length (badge 40 char, kbd 4-key no separator), calendar_grid boundaries (28-day, 31-day offset 6, day 0 not marked, upper clamps). --- .../element-builders-edge-cases.test.ts | 149 +++++++++++++++ .../src/__tests__/insert-element-tree.test.ts | 178 ++++++++++++++++++ 2 files changed, 327 insertions(+) create mode 100644 packages/pen-core/src/__tests__/element-builders-edge-cases.test.ts create mode 100644 packages/pen-mcp/src/__tests__/insert-element-tree.test.ts diff --git a/packages/pen-core/src/__tests__/element-builders-edge-cases.test.ts b/packages/pen-core/src/__tests__/element-builders-edge-cases.test.ts new file mode 100644 index 000000000..f59c251fd --- /dev/null +++ b/packages/pen-core/src/__tests__/element-builders-edge-cases.test.ts @@ -0,0 +1,149 @@ +import { describe, it, expect } from 'vitest'; +import { + buildBadge, + buildBodyText, + buildCalendarGrid, + buildCardRow, + buildChartBars, + buildEmptyState, + buildHeading, + buildKbd, + buildPrice, + buildRatingStars, + buildStepper, + buildTimeline, +} from '../element-builders/index.js'; + +/** + * Edge-case coverage gap fill: extreme values, i18n scripts, + * minimal-input shapes. These won't usually happen in AI output + * but the builders should still be robust — silent weird output + * is worse than a clear reject. + */ + +describe('i18n — scripts not in the primary CJK set', () => { + it('heading with pure Arabic content stays Latin path (no CJK Noto dispatch)', () => { + const h = buildHeading({ content: 'مرحبا بالعالم' }) as Record; + // Arabic is neither hiragana/hangul/han → null → Latin preset + expect((h as { fontFamily?: string }).fontFamily).toBeUndefined(); + }); + it('heading with emoji-only content stays Latin', () => { + const h = buildHeading({ content: '🎉🚀' }) as Record; + expect((h as { fontFamily?: string }).fontFamily).toBeUndefined(); + }); + it('heading with mixed Latin+hiragana resolves to Japanese (hiragana wins)', () => { + const h = buildHeading({ content: 'Hello こんにちは World' }) as Record; + expect(h.fontFamily).toBe('Noto Sans JP'); + }); + it('heading with mixed Han+Hangul resolves to Korean (hangul checked before han)', () => { + const h = buildHeading({ content: '안녕 世界' }) as Record; + expect(h.fontFamily).toBe('Noto Sans KR'); + }); + it('body_text always Inter even for CJK (body never dispatches)', () => { + const j = buildBodyText({ content: 'こんにちは' }) as Record; + const k = buildBodyText({ content: '안녕하세요' }) as Record; + const c = buildBodyText({ content: '你好' }) as Record; + expect(j.fontFamily).toBe('Inter'); + expect(k.fontFamily).toBe('Inter'); + expect(c.fontFamily).toBe('Inter'); + }); +}); + +describe('extreme numeric values', () => { + it('chart_bars with single huge value scales max correctly', () => { + const c = buildChartBars({ values: [1e9], chart_height: 200 }) as Record; + const bars = c.children as Array<{ height: number }>; + expect(bars).toHaveLength(1); + expect(bars[0].height).toBe(200); // 1e9/max(1,1e9) * 200 = 200 + }); + it('chart_bars all-zero → every bar at 2px floor', () => { + const c = buildChartBars({ values: [0, 0, 0] }) as Record; + const bars = c.children as Array<{ height: number }>; + bars.forEach((b) => expect(b.height).toBe(2)); + }); + it('stepper total=100 → 199 children (100 steps + 99 connectors)', () => { + const s = buildStepper({ total: 100, current: 50 }) as Record; + expect(s.children as unknown[]).toHaveLength(199); + }); + it('rating_stars filled higher than total clamps without duplicating children', () => { + const r = buildRatingStars({ filled: 999, total: 3 }) as Record; + expect(r.children as unknown[]).toHaveLength(3); + }); +}); + +describe('minimal inputs', () => { + it('card_row with single item still emits wrapper + inner + 1 card', () => { + const t = buildCardRow({ items: [{ title: 'Solo' }] }) as Record; + const inner = (t.children as Array<{ children: unknown[] }>)[0]; + expect(inner.children).toHaveLength(1); + }); + it('empty_state title-only has no subtitle / icon / CTA children', () => { + const e = buildEmptyState({ title: 'Nothing' }) as Record; + expect(e.children as unknown[]).toHaveLength(1); + }); + it('timeline with single item has NO connector', () => { + const t = buildTimeline({ items: [{ title: 'Solo' }] }) as Record; + const iconCol = ( + (t.children as Array<{ children: unknown[] }>)[0].children as unknown[] + )[0] as { children: unknown[] }; + expect(iconCol.children).toHaveLength(1); // dot only, no connector + }); +}); + +describe('extreme label lengths', () => { + it('badge with 40-char label still emits single text child (no wrapping logic)', () => { + const b = buildBadge({ + label: 'THIS IS DEFINITELY TOO LONG FOR A BADGE!!', + }) as Record; + expect(b.children as unknown[]).toHaveLength(1); + }); + it('price with large formatted amount preserves exact string', () => { + const p = buildPrice({ amount: '999,999,999.99', currency: '¥' }) as Record; + const children = p.children as Array<{ content: string }>; + expect(children[0].content).toBe('¥'); + expect(children[1].content).toBe('999,999,999.99'); + }); + it('kbd separator="" → N keys + zero separators = N children', () => { + const k = buildKbd({ keys: ['⌃', '⌥', '⌘', 'K'], separator: '' }) as Record; + expect(k.children as unknown[]).toHaveLength(4); + }); +}); + +describe('calendar_grid boundary cases', () => { + it('28-day month offset 0 → 1 header + 4 week rows', () => { + const g = buildCalendarGrid({ days_in_month: 28 }) as Record; + expect(g.children as unknown[]).toHaveLength(1 + 4); + }); + it('31-day month offset 6 → 1 header + 6 week rows (6×7 cells)', () => { + const g = buildCalendarGrid({ days_in_month: 31, start_day_offset: 6 }) as Record< + string, + unknown + >; + expect(g.children as unknown[]).toHaveLength(1 + 6); + }); + it('days_in_month lower clamp → 1 day, still emits 1 header + 1 week row', () => { + const g = buildCalendarGrid({ days_in_month: 0 }) as Record; + expect(g.children as unknown[]).toHaveLength(1 + 1); + }); + it('start_day_offset clamped upper bound (7 → 6)', () => { + const g = buildCalendarGrid({ + days_in_month: 30, + start_day_offset: 99, + }) as Record; + // 30 + 6 = 36 → ceil(36/7) = 6 weeks + expect(g.children as unknown[]).toHaveLength(1 + 6); + }); + it("today=0 and selected_day=0 (invalid day numbers) don't crash + no cell marked", () => { + const g = buildCalendarGrid({ days_in_month: 30, today: 0, selected_day: 0 }) as Record< + string, + unknown + >; + const rows = g.children as Array<{ children: Array<{ role: string }> }>; + // First week row (rows[1]) — day 1 is the first non-empty cell + const week1 = rows[1].children; + const hasTodayOrSelected = week1.some( + (c) => c.role === 'calendar-day-today' || c.role === 'calendar-day-selected', + ); + expect(hasTodayOrSelected).toBe(false); + }); +}); diff --git a/packages/pen-mcp/src/__tests__/insert-element-tree.test.ts b/packages/pen-mcp/src/__tests__/insert-element-tree.test.ts new file mode 100644 index 000000000..9f165350b --- /dev/null +++ b/packages/pen-mcp/src/__tests__/insert-element-tree.test.ts @@ -0,0 +1,178 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { writeFile, unlink, readFile, mkdir } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { insertElementTree, ensureParentExists } from '../tools/element-tool-helpers'; +import { invalidateCache } from '../document-manager'; + +/** + * Direct unit tests for `insertElementTree` — previously only + * covered indirectly through handler integration tests. Pins the + * safety invariants listed in the function's JSDoc: parent_id + * quote-safety pre-check, rollback on error + bytes-exact file + * restoration, post-insert verification catches silent-no-op paths, + * wrong-parent-landing rollback. + */ + +const TMP = join(tmpdir(), 'openpencil-insert-element-tree'); +const EMPTY = JSON.stringify({ version: '1.0.0', children: [] }); + +async function fresh(name: string): Promise { + const fp = join(TMP, name); + await writeFile(fp, EMPTY, 'utf-8'); + return fp; +} + +beforeEach(async () => { + await mkdir(TMP, { recursive: true }); +}); +afterEach(async () => { + for (const f of ['a.op', 'quoted.op', 'backslash.op']) { + try { + const fp = join(TMP, f); + invalidateCache(fp); + await unlink(fp); + } catch {} + } +}); + +describe('insertElementTree — parent_id safety pre-check', () => { + it('rejects parent_id containing literal quote (cannot round-trip DSL)', async () => { + const fp = await fresh('quoted.op'); + await expect( + insertElementTree({ + binding: 'x', + tree: { type: 'frame', name: 'X', width: 10, height: 10 }, + parent_id: 'weird"id', + filePath: fp, + }), + ).rejects.toThrow(/cannot be safely passed through batch_design's DSL parser/); + }); + + it('rejects parent_id containing backslash (same reason)', async () => { + const fp = await fresh('backslash.op'); + await expect( + insertElementTree({ + binding: 'x', + tree: { type: 'frame', name: 'X', width: 10, height: 10 }, + parent_id: 'weird\\id', + filePath: fp, + }), + ).rejects.toThrow(/cannot be safely passed/); + }); + + it('pre-check runs BEFORE any write — file bytes untouched on rejection', async () => { + const fp = await fresh('a.op'); + const before = await readFile(fp, 'utf-8'); + await expect( + insertElementTree({ + binding: 'x', + tree: { type: 'frame', name: 'X', width: 10, height: 10 }, + parent_id: 'weird"id', + filePath: fp, + }), + ).rejects.toThrow(); + const after = await readFile(fp, 'utf-8'); + expect(after).toBe(before); + }); +}); + +describe('insertElementTree — post-insert verification', () => { + it('rollback + throw when parent_id names a node that does not exist AT insert time', async () => { + // `ensureParentExists` prevents this in the normal flow, but if a + // handler skips that guard the post-insert check must still catch + // the silent no-op. Here we call insertElementTree directly with a + // parent_id that never existed. + const fp = await fresh('a.op'); + const before = await readFile(fp, 'utf-8'); + await expect( + insertElementTree({ + binding: 'x', + tree: { type: 'frame', name: 'X', width: 10, height: 10 }, + parent_id: 'never-existed', + filePath: fp, + }), + ).rejects.toThrow(); + // File restored to pre-insert bytes + const after = await readFile(fp, 'utf-8'); + expect(after).toBe(before); + }); +}); + +describe('insertElementTree — happy path', () => { + it('inserts tree at root when no parent_id + returns handleBatchDesign result', async () => { + const fp = await fresh('a.op'); + const result = await insertElementTree({ + binding: 'root', + tree: { + type: 'frame', + name: 'Root', + width: 400, + height: 300, + layout: 'vertical', + children: [], + }, + filePath: fp, + }); + expect(result.results.length).toBe(1); + expect(result.results[0].nodeId).toBeTruthy(); + // Disk state reflects insert + const saved = JSON.parse(await readFile(fp, 'utf-8')) as { + children?: Array<{ name: string }>; + pages?: Array<{ children: Array<{ name: string }> }>; + }; + const rootChildren = saved.children ?? saved.pages?.[0]?.children ?? []; + expect(rootChildren.length).toBeGreaterThan(0); + }); + + it('inserts tree under an existing parent + returns nodeId', async () => { + // Seed doc with a named parent + const fp = join(TMP, 'a.op'); + await writeFile( + fp, + JSON.stringify({ + version: '1.0.0', + children: [ + { + id: 'container-1', + type: 'frame', + name: 'Container', + width: 400, + height: 300, + layout: 'vertical', + children: [], + }, + ], + }), + 'utf-8', + ); + const result = await insertElementTree({ + binding: 'child', + tree: { type: 'text', name: 'Child', content: 'Hello' }, + parent_id: 'container-1', + filePath: fp, + }); + expect(result.results.length).toBe(1); + // Verify the child landed under container-1, not at root + const saved = JSON.parse(await readFile(fp, 'utf-8')) as { + children: Array<{ id: string; children?: Array<{ id: string }> }>; + }; + expect(saved.children[0].id).toBe('container-1'); + expect(saved.children[0].children?.length).toBe(1); + expect(saved.children[0].children?.[0].id).toBe(result.results[0].nodeId); + }); +}); + +describe('ensureParentExists (sibling coverage)', () => { + it('no parent_id → no-op (allows root insertion)', async () => { + const fp = await fresh('a.op'); + await expect(ensureParentExists({ filePath: fp })).resolves.toBeUndefined(); + }); + + it('missing parent_id → throws actionable error', async () => { + const fp = await fresh('a.op'); + await expect(ensureParentExists({ filePath: fp, parent_id: 'not-there' })).rejects.toThrow( + /parent_id "not-there" not found/, + ); + }); +});