From b2f6ae99ff77209078391acb13e5aecd89cb201d Mon Sep 17 00:00:00 2001 From: Fini Date: Fri, 8 May 2026 22:33:12 +0800 Subject: [PATCH] Merge remote-tracking branch 'origin/v0.8.0' into v0.8.0 --- .prettierignore | 4 + apps/web/src/services/ai/ai-runtime-config.ts | 12 +- apps/web/src/services/ai/design-validation.ts | 46 ++++- crates/openpencil-shell-core/src/lib.rs | 1 + crates/openpencil-shell-native/src/lib.rs | 4 + .../skills/phases/generation/elements.md | 22 +++ .../src/__tests__/activity-log-v1.test.ts | 14 +- .../src/__tests__/coerce-params.test.ts | 139 ++++++++++++++ .../pen-core/src/__tests__/heading-v1.test.ts | 17 +- .../src/__tests__/member-row-v1.test.ts | 19 +- .../src/element-builders/activity-log-v1.ts | 23 +-- .../src/element-builders/avatar-group-v1.ts | 8 +- .../src/element-builders/callout-v1.ts | 17 +- .../src/element-builders/chart-bars-v1.ts | 13 +- .../src/element-builders/chart-line-v1.ts | 13 +- .../src/element-builders/chart-pie-v1.ts | 18 +- .../src/element-builders/coerce-params.ts | 179 ++++++++++++++++++ .../src/element-builders/combobox-v1.ts | 9 +- .../src/element-builders/data-table-row-v1.ts | 9 +- .../src/element-builders/heading-v1.ts | 17 +- .../element-builders/image-placeholder-v1.ts | 4 + .../src/element-builders/invite-row-v1.ts | 17 +- .../pen-core/src/element-builders/kbd-v1.ts | 6 +- .../src/element-builders/member-row-v1.ts | 16 +- .../src/element-builders/share-row-v1.ts | 13 +- .../element-builders/social-login-row-v1.ts | 11 +- .../pen-core/src/element-builders/tag-v1.ts | 18 +- .../src/element-builders/timeline-v1.ts | 11 +- .../src/element-builders/toolbar-v1.ts | 11 +- .../src/element-builders/user-card-v1.ts | 8 +- .../src/__tests__/add-heading-v1.test.ts | 6 +- 31 files changed, 586 insertions(+), 119 deletions(-) create mode 100644 packages/pen-core/src/__tests__/coerce-params.test.ts create mode 100644 packages/pen-core/src/element-builders/coerce-params.ts diff --git a/.prettierignore b/.prettierignore index 40929303d..76d81d45a 100644 --- a/.prettierignore +++ b/.prettierignore @@ -10,3 +10,7 @@ target/ # Auto-generated by @tanstack/router-plugin. apps/web/src/routeTree.gen.ts + +# Local scratch / tutorial outputs (not part of the codebase; users won't have these dirs). +.baoyu-skills/ +xhs-images/ diff --git a/apps/web/src/services/ai/ai-runtime-config.ts b/apps/web/src/services/ai/ai-runtime-config.ts index c5de5c630..10e64fa8b 100644 --- a/apps/web/src/services/ai/ai-runtime-config.ts +++ b/apps/web/src/services/ai/ai-runtime-config.ts @@ -106,7 +106,17 @@ export const DESIGN_STREAM_TIMEOUTS = { } as const; /** When false, skips the vision LLM validation loop (pre-validation heuristics still run) */ -export const VALIDATION_ENABLED = false; +export const VALIDATION_ENABLED = true; + +/** + * Minimum total node count in the active page to trigger the vision LLM + * validation loop. Briefs that produce fewer nodes than this threshold + * skip vision (saves +30-90s + vision API tokens) — pre-validation + * heuristics still run regardless. Composite designs (multi-section + * dashboards, full-page mockups) easily clear this; atomic single-tool + * outputs (one badge, one chart) stay fast. + */ +export const VALIDATION_NODE_COUNT_THRESHOLD = 30; export const VALIDATION_TIMEOUT_MS = 180_000; export const MAX_VALIDATION_ROUNDS = 3; diff --git a/apps/web/src/services/ai/design-validation.ts b/apps/web/src/services/ai/design-validation.ts index 5515c815a..89879d150 100644 --- a/apps/web/src/services/ai/design-validation.ts +++ b/apps/web/src/services/ai/design-validation.ts @@ -6,9 +6,12 @@ * The LLM correlates visual issues with actual node IDs and returns fixes. */ -import { DEFAULT_FRAME_ID, useDocumentStore } from '@/stores/document-store'; +import { useCanvasStore } from '@/stores/canvas-store'; +import { useDocumentStore } from '@/stores/document-store'; +import { getActivePageChildren } from '@/stores/document-tree-utils'; import { VALIDATION_ENABLED, + VALIDATION_NODE_COUNT_THRESHOLD, VALIDATION_TIMEOUT_MS, MAX_VALIDATION_ROUNDS, VALIDATION_QUALITY_THRESHOLD, @@ -40,8 +43,25 @@ function getValidationSystemPrompt(): string { // Node tree dump — simplified for LLM context // --------------------------------------------------------------------------- -function buildNodeTreeDump(rootId: string): string { - const store = useDocumentStore.getState(); +function countNodesInActivePage(): number { + const doc = useDocumentStore.getState().document; + const activePageId = useCanvasStore.getState().activePageId; + const children = getActivePageChildren(doc, activePageId); + let count = 0; + function walk(node: PenNode): void { + count++; + if ('children' in node && Array.isArray(node.children)) { + for (const child of node.children) walk(child); + } + } + for (const child of children) walk(child); + return count; +} + +function buildNodeTreeDump(): string { + const doc = useDocumentStore.getState().document; + const activePageId = useCanvasStore.getState().activePageId; + const roots = getActivePageChildren(doc, activePageId); const lines: string[] = []; function walk(node: PenNode, depth: number) { @@ -98,8 +118,7 @@ function buildNodeTreeDump(rootId: string): string { } } - const rootNode = store.getNodeById(rootId); - if (rootNode) walk(rootNode, 0); + for (const root of roots) walk(root, 0); return lines.join('\n'); } @@ -276,6 +295,21 @@ export async function runPostGenerationValidation(options?: { return { applied: totalApplied, skipped: false }; } + // Skip vision loop on small designs — vision validation only earns its + // ~30-90s latency on composite multi-section briefs. Single-component + // outputs (one badge, one chart) are not worth the round-trip. + const nodeCount = countNodesInActivePage(); + if (nodeCount < VALIDATION_NODE_COUNT_THRESHOLD) { + clearVisualReference(); + emit( + 'done', + preFixCount > 0 + ? `[done] Pre-checks: fixed ${preFixCount} issue${preFixCount > 1 ? 's' : ''} (vision skipped: ${nodeCount} nodes < ${VALIDATION_NODE_COUNT_THRESHOLD})` + : `[done] Pre-checks complete (vision skipped: ${nodeCount} nodes < ${VALIDATION_NODE_COUNT_THRESHOLD})`, + ); + return { applied: totalApplied, skipped: false }; + } + for (let round = 1; round <= MAX_VALIDATION_ROUNDS; round++) { const isFirstRound = round === 1; @@ -316,7 +350,7 @@ export async function runPostGenerationValidation(options?: { : `[done] Screenshot captured (round ${round})`; emit('streaming'); - const nodeTreeDump = buildNodeTreeDump(DEFAULT_FRAME_ID); + const nodeTreeDump = buildNodeTreeDump(); if (isFirstRound) { console.log(`[Validation] Node tree dump:\n${nodeTreeDump}`); } diff --git a/crates/openpencil-shell-core/src/lib.rs b/crates/openpencil-shell-core/src/lib.rs index fe8d836e0..60265ff4c 100644 --- a/crates/openpencil-shell-core/src/lib.rs +++ b/crates/openpencil-shell-core/src/lib.rs @@ -16,6 +16,7 @@ //! differentiation lives at the canvas viewport / chrome layer //! (single-page + infinite canvas recommended, multi-page also supported). +pub mod event; pub mod jian; pub mod render_backend; diff --git a/crates/openpencil-shell-native/src/lib.rs b/crates/openpencil-shell-native/src/lib.rs index 29c6f4e9c..e6b87f962 100644 --- a/crates/openpencil-shell-native/src/lib.rs +++ b/crates/openpencil-shell-native/src/lib.rs @@ -42,11 +42,15 @@ pub mod context; pub mod backend; #[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))] pub mod canvas_view_stub; +#[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))] +pub mod event; #[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))] pub use backend::{to_jian_color, to_jian_rect, NativeBackend}; #[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))] pub use canvas_view_stub::CanvasViewportStub; +#[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))] +pub use event::JianPointerMapper; // Cross-platform re-exports — visible on every (non-wasm) target. pub use context::{GlContextProvider, ProviderError, ProviderResult}; diff --git a/packages/pen-ai-skills/skills/phases/generation/elements.md b/packages/pen-ai-skills/skills/phases/generation/elements.md index 2341b0d7e..a4727bbe9 100644 --- a/packages/pen-ai-skills/skills/phases/generation/elements.md +++ b/packages/pen-ai-skills/skills/phases/generation/elements.md @@ -35,6 +35,28 @@ These narrow MCP tools emit well-known structures that batch_design frequently g Multi-tool example — a "Notifications" settings section with a header + 4 toggle rows is **5 tool calls** (1× `add_section_header_v1` + 4× `add_setting_row_v1`), NOT 1 batch_design. +**MISSING-ROLE FAIL WATCH.** Top batch*design fallback traps measured in ab-v8 (40 obvious-T fails ≈ model dropped to batch_design and missed required roles). When the brief mentions ANY of these, ALWAYS use the matching `add*\*\_v1` — never write batch_design for these, because models forget the role names: + +- "modal" / "dialog" / "popup" / "confirm dialog" / "弹窗" → `add_modal_shell_v1` (roles: `modal-scrim`, `modal-shell-card`, `modal-title`, `modal-subtitle`) +- "avatar group" / "stacked avatars" / "team list" / "+N more" → `add_avatar_group_v1` (roles: `avatar-group-item`, `avatar-group-initial`, `avatar-group-overflow`) +- "metric comparison" / "KPI with trend" / "delta cell" → `add_metric_comparison_v1` (role: `metric-comparison-change`) +- "image placeholder" / "photo slot" / "图片占位" → `add_image_placeholder_v1` (role: `image-placeholder-label`) +- "tag" / "filter chip" / "removable label" → `add_tag_v1` (roles: `tag-label`, `tag-remove`) +- "toolbar" / "icon button bar" / "桌面工具栏" → `add_toolbar_v1` (roles: `toolbar-item`, `toolbar-item-active`, `toolbar-divider`) +- "callout" / "doc highlight" / "tinted block" → `add_callout_v1` (roles: `callout-icon`, `callout-title`, `callout-body`) +- "profile header" / "centered avatar header" / "user banner" → `add_profile_header_v1` (roles: `profile-header-avatar`, `profile-header-name`, `profile-header-handle`, `profile-header-bio`) +- "inbox" / "email list" / "message preview" → `add_inbox_message_v1` (roles: `inbox-message-from`, `inbox-message-subject`, `inbox-message-preview`, `inbox-message-unread`) +- "drawer" / "side panel" / "sliding panel" → `add_drawer_shell_v1` (roles: `drawer-shell-right`, `drawer-shell-header`, `drawer-shell-title`, `drawer-shell-close`) +- "cookie banner" / "GDPR notice" / "consent bar" → `add_cookie_banner_v1` (roles: `cookie-banner`, `cookie-banner-title`, `cookie-banner-body`, `cookie-banner-actions`) +- "user card" / "person card" / "contact card" → `add_user_card_v1` (roles: `user-card-avatar`, `user-card-name`, `user-card-role`) +- "chart legend" / "data series legend" / "color-coded legend" → `add_legend_item_v1` (roles: `legend-item`, `legend-item-marker`, `legend-item-label`, `legend-item-value`) +- "skeleton" / "loading placeholder" / "shimmer rows" / "loading state" / "骨架屏" → `add_skeleton_v1` (roles: `skeleton`, `skeleton-row`) +- "undo bar" / "inline action" / "inline ack" / "snack-style action" → `add_inline_action_v1` (roles: `inline-action-message`, `inline-action-cta`) +- "share row" / "social share buttons" / "share targets" → `add_share_row_v1` (roles: `share-row`, `share-target`, `share-target-icon`, `share-target-label`) +- "combobox" / "autocomplete" / "search-with-suggestions" / "下拉自动补全" → `add_combobox_v1` (roles: `combobox-input`, `combobox-dropdown`, `combobox-option-active`) + +Even if you must use batch_design (no v1 fits), still emit these EXACT role strings on each child node so the validator sees a complete shape. + **`parent_id` is REAL or OMITTED — never invented.** Every `add_*_v1` tool's `parent_id` arg must either (1) be omitted entirely so the new node lands at the page root (the safe default for full-page composite briefs), or (2) name a real existing node id you already received from a prior tool call. The cookbook recipes below use placeholders like `` / `` / `` as DOCUMENTATION shorthand only — when YOU emit the call in your output, OMIT the parent_id field entirely. ❌ WRONG — these all crash with `parent_id "X" not found in document` because every id was made up by the model, not handed to you by a prior tool call: diff --git a/packages/pen-core/src/__tests__/activity-log-v1.test.ts b/packages/pen-core/src/__tests__/activity-log-v1.test.ts index f45bbe66f..905072d00 100644 --- a/packages/pen-core/src/__tests__/activity-log-v1.test.ts +++ b/packages/pen-core/src/__tests__/activity-log-v1.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect } from 'vitest'; +import { clearCoerceWarnings, getCoerceWarnings } from '../element-builders/coerce-params.js'; import { buildActivityLog } from '../element-builders/activity-log.js'; import { buildActivityLogV1 } from '../element-builders/activity-log-v1.js'; @@ -66,10 +67,15 @@ describe('buildActivityLogV1 — byte-parity with v0 (light)', () => { expect(content[0].fill).toBe('#0F172A'); }); - it('throws on invalid tone', () => { - expect(() => buildActivityLogV1({ ...BASIC, tone: 'critical' as never })).toThrow( - /invalid tone/, - ); + it('coerces invalid tone to default info and emits warning', () => { + clearCoerceWarnings(); + const out = buildActivityLogV1({ ...BASIC, tone: 'critical' as never }); + expect(out).toBeDefined(); + const warnings = getCoerceWarnings(); + expect(warnings.length).toBeGreaterThan(0); + expect(warnings[0].builder).toBe('buildActivityLogV1'); + expect(warnings[0].param).toBe('tone'); + expect(warnings[0].given).toBe('critical'); }); it('no icon: 3 children (no icon dot)', () => { diff --git a/packages/pen-core/src/__tests__/coerce-params.test.ts b/packages/pen-core/src/__tests__/coerce-params.test.ts new file mode 100644 index 000000000..49358d0e0 --- /dev/null +++ b/packages/pen-core/src/__tests__/coerce-params.test.ts @@ -0,0 +1,139 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { + coerceEnum, + coerceNonEmptyArray, + coerceNumberArray, + coerceStringArray, + coerceNonEmptyString, + getCoerceWarnings, + clearCoerceWarnings, +} from '../element-builders/coerce-params.js'; + +describe('coerce-params', () => { + beforeEach(() => clearCoerceWarnings()); + + describe('coerceEnum', () => { + const VALID = ['a', 'b', 'c'] as const; + + it('returns valid value unchanged', () => { + expect(coerceEnum('b', VALID, 'a', 'B', 'p')).toBe('b'); + expect(getCoerceWarnings()).toHaveLength(0); + }); + + it('returns default silently for undefined / null', () => { + expect(coerceEnum(undefined, VALID, 'a', 'B', 'p')).toBe('a'); + expect(coerceEnum(null, VALID, 'a', 'B', 'p')).toBe('a'); + expect(getCoerceWarnings()).toHaveLength(0); + }); + + it('warns + returns default for invalid enum string', () => { + expect(coerceEnum('z', VALID, 'a', 'B', 'p')).toBe('a'); + const warnings = getCoerceWarnings(); + expect(warnings).toHaveLength(1); + expect(warnings[0].builder).toBe('B'); + expect(warnings[0].param).toBe('p'); + expect(warnings[0].given).toBe('z'); + expect(warnings[0].fallback).toBe('a'); + }); + + it('warns + returns default for non-string', () => { + expect(coerceEnum(42, VALID, 'a', 'B', 'p')).toBe('a'); + expect(getCoerceWarnings()).toHaveLength(1); + }); + }); + + describe('coerceNonEmptyArray', () => { + it('returns array unchanged when non-empty', () => { + expect(coerceNonEmptyArray([1, 2], [99], 'B', 'p')).toEqual([1, 2]); + expect(getCoerceWarnings()).toHaveLength(0); + }); + + it('warns + returns fallback for empty array', () => { + expect(coerceNonEmptyArray([], ['x'], 'B', 'p')).toEqual(['x']); + expect(getCoerceWarnings()[0].reason).toContain('empty'); + }); + + it('warns + returns fallback for undefined / null / non-array', () => { + expect(coerceNonEmptyArray(undefined, ['x'], 'B', 'p')).toEqual(['x']); + expect(coerceNonEmptyArray(null, ['x'], 'B', 'p')).toEqual(['x']); + expect(coerceNonEmptyArray('oops', ['x'], 'B', 'p')).toEqual(['x']); + expect(getCoerceWarnings()).toHaveLength(3); + }); + }); + + describe('coerceNumberArray', () => { + it('returns finite numbers unchanged', () => { + expect(coerceNumberArray([1, 2, 3], [99], 'B', 'p')).toEqual([1, 2, 3]); + expect(getCoerceWarnings()).toHaveLength(0); + }); + + it('filters out non-finite entries', () => { + expect(coerceNumberArray([1, NaN, 2, Infinity, 3], [99], 'B', 'p')).toEqual([1, 2, 3]); + }); + + it('warns + returns fallback when no finite numbers remain', () => { + expect(coerceNumberArray([NaN, Infinity], [42], 'B', 'p')).toEqual([42]); + expect(getCoerceWarnings()).toHaveLength(1); + }); + + it('warns + returns fallback for non-array', () => { + expect(coerceNumberArray(undefined, [42], 'B', 'p')).toEqual([42]); + expect(coerceNumberArray('oops', [42], 'B', 'p')).toEqual([42]); + expect(getCoerceWarnings()).toHaveLength(2); + }); + }); + + describe('coerceStringArray', () => { + it('returns non-empty strings unchanged', () => { + expect(coerceStringArray(['a', 'b'], ['fb'], 'B', 'p')).toEqual(['a', 'b']); + expect(getCoerceWarnings()).toHaveLength(0); + }); + + it('filters out empty / non-string entries', () => { + expect(coerceStringArray(['a', '', 1, null, 'b'], ['fb'], 'B', 'p')).toEqual(['a', 'b']); + }); + + it('warns + returns fallback when array empty after filter', () => { + expect(coerceStringArray([1, 2, ''], ['fb'], 'B', 'p')).toEqual(['fb']); + expect(getCoerceWarnings()).toHaveLength(1); + }); + + it('warns + returns fallback for non-array', () => { + expect(coerceStringArray(undefined, ['fb'], 'B', 'p')).toEqual(['fb']); + expect(coerceStringArray('oops', ['fb'], 'B', 'p')).toEqual(['fb']); + expect(getCoerceWarnings()).toHaveLength(2); + }); + }); + + describe('coerceNonEmptyString', () => { + it('returns valid string unchanged', () => { + expect(coerceNonEmptyString('hello', 'fb', 'B', 'p')).toBe('hello'); + expect(getCoerceWarnings()).toHaveLength(0); + }); + + it('returns fallback silently for undefined / null / empty', () => { + expect(coerceNonEmptyString(undefined, 'fb', 'B', 'p')).toBe('fb'); + expect(coerceNonEmptyString(null, 'fb', 'B', 'p')).toBe('fb'); + expect(coerceNonEmptyString('', 'fb', 'B', 'p')).toBe('fb'); + expect(coerceNonEmptyString(' ', 'fb', 'B', 'p')).toBe('fb'); + // whitespace-only currently warns (it's a non-empty string of length>0 + // but trim() is empty); ensure all silent paths warn 0 times + expect(getCoerceWarnings()).toHaveLength(0); + }); + + it('warns + returns fallback for non-string', () => { + expect(coerceNonEmptyString(42, 'fb', 'B', 'p')).toBe('fb'); + expect(getCoerceWarnings()).toHaveLength(1); + }); + }); + + describe('warning sink lifecycle', () => { + it('accumulates across calls until cleared', () => { + coerceEnum('z', ['a'] as const, 'a', 'B', 'p'); + coerceEnum('y', ['a'] as const, 'a', 'B', 'p'); + expect(getCoerceWarnings()).toHaveLength(2); + clearCoerceWarnings(); + expect(getCoerceWarnings()).toHaveLength(0); + }); + }); +}); diff --git a/packages/pen-core/src/__tests__/heading-v1.test.ts b/packages/pen-core/src/__tests__/heading-v1.test.ts index 9e0c2d3a7..101f0f43b 100644 --- a/packages/pen-core/src/__tests__/heading-v1.test.ts +++ b/packages/pen-core/src/__tests__/heading-v1.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect } from 'vitest'; +import { clearCoerceWarnings, getCoerceWarnings } from '../element-builders/coerce-params.js'; import { buildHeading } from '../element-builders/heading.js'; import { buildHeadingV1 } from '../element-builders/heading-v1.js'; @@ -71,9 +72,17 @@ describe('heading-v1 dark mode: emits dark hex values', () => { }); describe('heading-v1 error handling', () => { - it('throws on invalid level (same guard as v0)', () => { - expect(() => buildHeadingV1({ content: 'T', level: 'caption' as never })).toThrow( - /add_heading_v1.*invalid level.*caption/, - ); + it('coerces invalid level to default h2 and emits warning', () => { + clearCoerceWarnings(); + const out = buildHeadingV1({ content: 'T', level: 'caption' as never }) as Record< + string, + unknown + >; + expect(out.fontSize).toBe(24); // h2 default + const warnings = getCoerceWarnings(); + expect(warnings.length).toBeGreaterThan(0); + expect(warnings[0].builder).toBe('buildHeadingV1'); + expect(warnings[0].param).toBe('level'); + expect(warnings[0].given).toBe('caption'); }); }); diff --git a/packages/pen-core/src/__tests__/member-row-v1.test.ts b/packages/pen-core/src/__tests__/member-row-v1.test.ts index a1c0f55b9..a2768d665 100644 --- a/packages/pen-core/src/__tests__/member-row-v1.test.ts +++ b/packages/pen-core/src/__tests__/member-row-v1.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect } from 'vitest'; +import { clearCoerceWarnings, getCoerceWarnings } from '../element-builders/coerce-params.js'; import { buildMemberRow } from '../element-builders/member-row.js'; import { buildMemberRowV1 } from '../element-builders/member-row-v1.js'; @@ -83,13 +84,17 @@ describe('buildMemberRowV1 — byte-parity with v0 (light)', () => { expect(getFill(dot)).toBe('#10B981'); }); - it('throws on invalid status tone', () => { - expect(() => - buildMemberRowV1({ - name: 'X', - trailing: { kind: 'status_dot' as const, tone: 'ghost' as never }, - }), - ).toThrow(/invalid trailing\.tone/); + it('coerces invalid status tone to default online and emits warning', () => { + clearCoerceWarnings(); + const out = buildMemberRowV1({ + name: 'X', + trailing: { kind: 'status_dot' as const, tone: 'ghost' as never }, + }); + expect(out).toBeDefined(); + const warnings = getCoerceWarnings(); + expect(warnings.length).toBeGreaterThan(0); + expect(warnings[0].builder).toBe('buildMemberRowV1'); + expect(warnings[0].param).toBe('trailing.tone'); }); }); diff --git a/packages/pen-core/src/element-builders/activity-log-v1.ts b/packages/pen-core/src/element-builders/activity-log-v1.ts index 946f18acb..be791e6c1 100644 --- a/packages/pen-core/src/element-builders/activity-log-v1.ts +++ b/packages/pen-core/src/element-builders/activity-log-v1.ts @@ -1,3 +1,4 @@ +import { coerceEnum } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -21,14 +22,6 @@ export interface ActivityLogV1Params { theme?: V1Theme; } -const VALID_ACTIVITY_LOG_TONES = new Set([ - 'info', - 'success', - 'warning', - 'danger', - 'neutral', -]); - // Light-mode constants (must match v0 exactly for byte-parity) const ACTOR_FG_LIGHT = '#0F172A'; const ACTION_FG_LIGHT = '#475569'; @@ -66,13 +59,13 @@ const NEUTRAL_FG_SYSTEM = '$color-text-muted'; * neutral uses $color-surface / $color-text-muted. */ export function buildActivityLogV1(params: ActivityLogV1Params): ElementTree { - const requestedTone = (params.tone ?? 'info') as string; - if (!VALID_ACTIVITY_LOG_TONES.has(requestedTone)) { - throw new Error( - `add_activity_log_v1: invalid tone "${requestedTone}"; expected one of: info, success, warning, danger, neutral`, - ); - } - const tone = requestedTone as Tone; + const tone = coerceEnum( + params.tone, + ['info', 'success', 'warning', 'danger', 'neutral'], + 'info', + 'buildActivityLogV1', + 'tone', + ); const theme = params.theme ?? 'light'; const t = resolveTheme(theme); const isLight = theme === 'light'; diff --git a/packages/pen-core/src/element-builders/avatar-group-v1.ts b/packages/pen-core/src/element-builders/avatar-group-v1.ts index e8441d45e..20b23f79c 100644 --- a/packages/pen-core/src/element-builders/avatar-group-v1.ts +++ b/packages/pen-core/src/element-builders/avatar-group-v1.ts @@ -1,3 +1,4 @@ +import { coerceNonEmptyArray } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -52,7 +53,12 @@ const DEFAULT_PALETTE = [ export function buildAvatarGroupV1(params: AvatarGroupV1Params): ElementTree { const size = Math.min(64, Math.max(24, Math.floor(params.size ?? 32))); const maxVisible = Math.min(10, Math.max(1, Math.floor(params.max_visible ?? 4))); - const items = params.items ?? []; + const items = coerceNonEmptyArray( + params.items, + [{ initial: 'A' }, { initial: 'B' }, { initial: 'C' }, { initial: 'D' }, { initial: 'E' }], + 'buildAvatarGroupV1', + 'items', + ); const visible = items.slice(0, maxVisible); const overflow = Math.max(0, items.length - maxVisible); const fontSize = Math.max(11, Math.round(size * 0.4)); diff --git a/packages/pen-core/src/element-builders/callout-v1.ts b/packages/pen-core/src/element-builders/callout-v1.ts index d1b6460d8..1e32cd206 100644 --- a/packages/pen-core/src/element-builders/callout-v1.ts +++ b/packages/pen-core/src/element-builders/callout-v1.ts @@ -1,10 +1,9 @@ +import { coerceEnum } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; export type CalloutV1Tone = 'info' | 'success' | 'warning' | 'danger' | 'note'; -const VALID_CALLOUT_TONES = new Set(['info', 'success', 'warning', 'danger', 'note']); - export interface CalloutV1Params { /** Body text. Required. */ body: string; @@ -48,13 +47,13 @@ const TONES_LIGHT: Record = { * - note → surface2 bg + textPrimary fg (no dedicated alert token for note) */ export function buildCalloutV1(params: CalloutV1Params): ElementTree { - const requestedTone = (params.tone ?? 'note') as string; - if (!VALID_CALLOUT_TONES.has(requestedTone)) { - throw new Error( - `add_callout_v1: invalid tone "${requestedTone}"; expected one of: info, success, warning, danger, note`, - ); - } - const tone = requestedTone as CalloutV1Tone; + const tone = coerceEnum( + params.tone, + ['info', 'success', 'warning', 'danger', 'note'], + 'note', + 'buildCalloutV1', + 'tone', + ); const theme = params.theme ?? 'light'; const t = resolveTheme(theme); const isLight = theme === 'light'; diff --git a/packages/pen-core/src/element-builders/chart-bars-v1.ts b/packages/pen-core/src/element-builders/chart-bars-v1.ts index 5a92bb037..9195af518 100644 --- a/packages/pen-core/src/element-builders/chart-bars-v1.ts +++ b/packages/pen-core/src/element-builders/chart-bars-v1.ts @@ -1,3 +1,4 @@ +import { coerceNumberArray } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -26,11 +27,13 @@ export interface ChartBarsV1Params { * #2563EB for the migration contract). */ export function buildChartBarsV1(params: ChartBarsV1Params): ElementTree { - const raw = Array.isArray(params.values) ? params.values : []; - if (raw.length === 0) { - throw new Error('buildChartBarsV1: values must contain at least one number'); - } - const values = raw.map((v) => (Number.isFinite(v) ? Math.max(0, v) : 0)); + const inputValues = coerceNumberArray( + params.values, + [10, 15, 12, 20, 18], + 'buildChartBarsV1', + 'values', + ); + const values = inputValues.map((v) => Math.max(0, v)); const max = Math.max(1, ...values); const barWidth = Math.max(4, Math.floor(params.bar_width ?? 24)); const gap = Math.max(0, Math.floor(params.gap ?? 12)); diff --git a/packages/pen-core/src/element-builders/chart-line-v1.ts b/packages/pen-core/src/element-builders/chart-line-v1.ts index 725e727fb..49244bf08 100644 --- a/packages/pen-core/src/element-builders/chart-line-v1.ts +++ b/packages/pen-core/src/element-builders/chart-line-v1.ts @@ -1,3 +1,4 @@ +import { coerceNumberArray } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -32,11 +33,13 @@ export interface ChartLineV1Params { * v0's #2563EB (or caller-supplied stroke_color) for byte-parity. */ export function buildChartLineV1(params: ChartLineV1Params): ElementTree { - const raw = Array.isArray(params.values) ? params.values : []; - if (raw.length === 0) { - throw new Error('buildChartLineV1: values must contain at least one number'); - } - const values = raw.map((v) => (Number.isFinite(v) ? Math.max(0, v) : 0)); + const inputValues = coerceNumberArray( + params.values, + [10, 15, 12, 20, 18], + 'buildChartLineV1', + 'values', + ); + const values = inputValues.map((v) => Math.max(0, v)); const max = Math.max(1, ...values); const spacing = Math.max(8, Math.floor(params.point_spacing ?? 32)); const chartHeight = Math.max(40, Math.floor(params.chart_height ?? 160)); diff --git a/packages/pen-core/src/element-builders/chart-pie-v1.ts b/packages/pen-core/src/element-builders/chart-pie-v1.ts index 0c9ac20bc..b8343a0c0 100644 --- a/packages/pen-core/src/element-builders/chart-pie-v1.ts +++ b/packages/pen-core/src/element-builders/chart-pie-v1.ts @@ -1,3 +1,4 @@ +import { coerceNumberArray } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -40,14 +41,17 @@ const V0_DEFAULT_PALETTE = ['#2563EB', '#10B981', '#F59E0B', '#EF4444', '#8B5CF6 * Caller-supplied `colors` are passed through unchanged in all modes. */ export function buildChartPieV1(params: ChartPieV1Params): ElementTree { - const raw = Array.isArray(params.values) ? params.values : []; - if (raw.length === 0) { - throw new Error('buildChartPieV1: values must contain at least one number'); - } - const values = raw.map((v) => (Number.isFinite(v) ? Math.max(0, v) : 0)); - const total = values.reduce((s, v) => s + v, 0); + const inputValues = coerceNumberArray( + params.values, + [30, 25, 20, 15, 10], + 'buildChartPieV1', + 'values', + ); + let values = inputValues.map((v) => Math.max(0, v)); + let total = values.reduce((s, v) => s + v, 0); if (total <= 0) { - throw new Error('buildChartPieV1: values must sum to > 0'); + values = [1]; + total = 1; } const diameter = Math.max(40, Math.floor(params.diameter ?? 160)); const innerRatio = Math.max(0, Math.min(0.9, params.inner_radius_ratio ?? 0)); diff --git a/packages/pen-core/src/element-builders/coerce-params.ts b/packages/pen-core/src/element-builders/coerce-params.ts new file mode 100644 index 000000000..03c1a41f8 --- /dev/null +++ b/packages/pen-core/src/element-builders/coerce-params.ts @@ -0,0 +1,179 @@ +/** + * Param coercion helpers for v1 element builders. + * + * Replaces the early-2026 "throw on invalid params" pattern with + * fuzzy-coerce + warning. Reason: ab-v8 KPI showed ~14/95 obvious-T + * fails were v1 schema throws on hallucinated enum values and missing + * required arrays — the model meant the right component, just had a + * bad value, and the whole tool call was dropped. + * + * After coercion the builder still emits a structurally-valid tree + * with a placeholder. Warnings are pushed to a process-global sink so + * orchestrators can surface them to the LLM (or just inspect them in + * tests). v0 builders intentionally keep the throw behavior — only v1 + * goes through this helper. + */ + +export interface CoerceWarning { + builder: string; + param: string; + given: unknown; + fallback: unknown; + reason: string; +} + +const warningSink: CoerceWarning[] = []; + +function emit(w: CoerceWarning): void { + warningSink.push(w); + // eslint-disable-next-line no-console + console.warn( + `[coerce-params] ${w.builder}.${w.param}: ${w.reason} ` + + `(given=${safeStringify(w.given)}, fallback=${safeStringify(w.fallback)})`, + ); +} + +function safeStringify(v: unknown): string { + try { + return JSON.stringify(v); + } catch { + return String(v); + } +} + +/** Read warnings collected since the last clear. */ +export function getCoerceWarnings(): CoerceWarning[] { + return [...warningSink]; +} + +export function clearCoerceWarnings(): void { + warningSink.length = 0; +} + +/** + * Coerce a value to an allowed enum. Returns `defaultValue` for + * `undefined`/`null` (no warning — that's the legitimate "unset" case). + * Returns `defaultValue` and emits a warning for any other invalid value. + */ +export function coerceEnum( + value: unknown, + validValues: readonly T[], + defaultValue: T, + builder: string, + param: string, +): T { + if (value === undefined || value === null) return defaultValue; + if (typeof value === 'string' && (validValues as readonly string[]).includes(value)) { + return value as T; + } + emit({ + builder, + param, + given: value, + fallback: defaultValue, + reason: `invalid enum value; valid: ${JSON.stringify(validValues)}`, + }); + return defaultValue; +} + +/** + * Coerce a value that should be a non-empty array. Falls back to `fallback` + * on `undefined`/`null`/non-array/empty-array, with a warning. + */ +export function coerceNonEmptyArray( + value: unknown, + fallback: T[], + builder: string, + param: string, +): T[] { + if (Array.isArray(value) && value.length > 0) { + return value as T[]; + } + emit({ + builder, + param, + given: value, + fallback, + reason: !Array.isArray(value) ? `expected non-empty array, got ${typeof value}` : 'array empty', + }); + return fallback; +} + +/** + * Coerce a value that should be a non-empty array of non-empty strings. + * Filters out non-string / empty entries; if nothing remains, returns + * `fallback` and warns. + */ +export function coerceStringArray( + value: unknown, + fallback: string[], + builder: string, + param: string, +): string[] { + if (Array.isArray(value)) { + const filtered = value.filter((k): k is string => typeof k === 'string' && k.length > 0); + if (filtered.length > 0) return filtered; + } + emit({ + builder, + param, + given: value, + fallback, + reason: !Array.isArray(value) + ? `expected string[], got ${typeof value}` + : 'no non-empty strings in array', + }); + return fallback; +} + +/** + * Coerce a value that should be a non-empty array of finite numbers. + * Filters out non-finite entries; if the array is empty after filtering + * or wasn't an array, returns `fallback` and warns. + */ +export function coerceNumberArray( + value: unknown, + fallback: number[], + builder: string, + param: string, +): number[] { + if (Array.isArray(value)) { + const filtered = value.filter((v): v is number => typeof v === 'number' && Number.isFinite(v)); + if (filtered.length > 0) return filtered; + } + emit({ + builder, + param, + given: value, + fallback, + reason: !Array.isArray(value) + ? `expected number[], got ${typeof value}` + : 'no finite numbers in array', + }); + return fallback; +} + +/** + * Coerce a value that should be a non-empty string. Falls back silently + * for `undefined`/`null`/empty/whitespace-only; for other non-string + * types, warns and falls back. + */ +export function coerceNonEmptyString( + value: unknown, + fallback: string, + builder: string, + param: string, +): string { + if (typeof value === 'string') { + return value.trim().length > 0 ? value : fallback; + } + if (value === undefined || value === null) return fallback; + emit({ + builder, + param, + given: value, + fallback, + reason: `expected non-empty string, got ${typeof value}`, + }); + return fallback; +} diff --git a/packages/pen-core/src/element-builders/combobox-v1.ts b/packages/pen-core/src/element-builders/combobox-v1.ts index ef1554a43..f79c8444b 100644 --- a/packages/pen-core/src/element-builders/combobox-v1.ts +++ b/packages/pen-core/src/element-builders/combobox-v1.ts @@ -1,3 +1,4 @@ +import { coerceNonEmptyArray } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -43,6 +44,12 @@ export interface ComboboxV1Params { * shadow color (#0F172A1A) → kept as-is (shadow is theme-agnostic in v0) */ export function buildComboboxV1(params: ComboboxV1Params): ElementTree { + const options = coerceNonEmptyArray( + params.options, + [{ label: 'Option 1' }, { label: 'Option 2' }, { label: 'Option 3' }], + 'buildComboboxV1', + 'options', + ); const placeholder = params.placeholder ?? 'Search...'; const value = params.value ?? ''; const theme = params.theme ?? 'light'; @@ -126,7 +133,7 @@ export function buildComboboxV1(params: ComboboxV1Params): ElementTree { color: '#0F172A1A', }, ], - children: params.options.map((opt, i) => ({ + children: options.map((opt, i) => ({ type: 'frame', name: `Option ${i + 1}`, role: opt.highlighted ? 'combobox-option-active' : 'combobox-option', diff --git a/packages/pen-core/src/element-builders/data-table-row-v1.ts b/packages/pen-core/src/element-builders/data-table-row-v1.ts index 419a3a287..11d84123f 100644 --- a/packages/pen-core/src/element-builders/data-table-row-v1.ts +++ b/packages/pen-core/src/element-builders/data-table-row-v1.ts @@ -1,3 +1,4 @@ +import { coerceNonEmptyArray } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -30,6 +31,12 @@ export interface DataTableRowV1Params { * selected row bg (#F8FAFC) → bgDeep */ export function buildDataTableRowV1(params: DataTableRowV1Params): ElementTree { + const columns = coerceNonEmptyArray( + params.columns, + [{ content: 'Col 1' }, { content: 'Col 2' }, { content: 'Col 3' }], + 'buildDataTableRowV1', + 'columns', + ); const isHeader = params.header === true; const isSelected = !isHeader && params.selected === true; const theme = params.theme ?? 'light'; @@ -58,7 +65,7 @@ export function buildDataTableRowV1(params: DataTableRowV1Params): ElementTree { gap: 16, alignItems: 'center', clipContent: true, - children: params.columns.map((col, i) => + children: columns.map((col, i) => buildCellV1(col, i, isHeader, cellTextSize, cellTextWeight, cellTextColor), ), }; diff --git a/packages/pen-core/src/element-builders/heading-v1.ts b/packages/pen-core/src/element-builders/heading-v1.ts index 792929283..eea822fd7 100644 --- a/packages/pen-core/src/element-builders/heading-v1.ts +++ b/packages/pen-core/src/element-builders/heading-v1.ts @@ -1,4 +1,5 @@ import { cjkFontFamily, detectCjkScript } from './cjk-detect.js'; +import { coerceEnum } from './coerce-params.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; import type { ElementTree } from './helpers.js'; import type { HeadingLevel, HeadingParams } from './heading.js'; @@ -46,8 +47,6 @@ const V0_CJK_BASE = { h3: { fontSize: 20, fontWeight: 600, lineHeight: 1.4 }, } as const; -const VALID_HEADING_LEVELS = new Set(['display', 'h1', 'h2', 'h3']); - /** * Theme-aware typographic heading (v1). * @@ -68,13 +67,13 @@ const VALID_HEADING_LEVELS = new Set(['display', 'h1', 'h2', 'h3']); * fontFamily stays the concrete CJK family string if detected. */ export function buildHeadingV1(params: HeadingV1Params): ElementTree { - const requestedLevel = (params.level ?? 'h2') as string; - if (!VALID_HEADING_LEVELS.has(requestedLevel)) { - throw new Error( - `add_heading_v1: invalid level "${requestedLevel}"; expected one of: display, h1, h2, h3`, - ); - } - const level = requestedLevel as HeadingLevel; + const level = coerceEnum( + params.level, + ['display', 'h1', 'h2', 'h3'], + 'h2', + 'buildHeadingV1', + 'level', + ); const theme = params.theme ?? 'light'; const script = detectCjkScript(params.content); const cjkFont = cjkFontFamily(script); diff --git a/packages/pen-core/src/element-builders/image-placeholder-v1.ts b/packages/pen-core/src/element-builders/image-placeholder-v1.ts index 93417da44..486a5385d 100644 --- a/packages/pen-core/src/element-builders/image-placeholder-v1.ts +++ b/packages/pen-core/src/element-builders/image-placeholder-v1.ts @@ -69,6 +69,10 @@ export function buildImagePlaceholderV1(params: ImagePlaceholderV1Params): Eleme fill: [{ type: 'solid', color: iconColor }], }, ]; + // Conditional emit: builders never invent optional content, and an + // empty-content text node still consumes flex gap so it is not + // layout-neutral with a "no label" output. Callers wanting the + // image-placeholder-label role must pass a non-empty `label`. if (params.label) { children.push({ type: 'text', diff --git a/packages/pen-core/src/element-builders/invite-row-v1.ts b/packages/pen-core/src/element-builders/invite-row-v1.ts index 4b76dafe1..b62ae6601 100644 --- a/packages/pen-core/src/element-builders/invite-row-v1.ts +++ b/packages/pen-core/src/element-builders/invite-row-v1.ts @@ -1,10 +1,9 @@ +import { coerceEnum } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; export type InviteV1Status = 'pending' | 'expired' | 'accepted'; -const VALID_INVITE_V1_STATUSES = new Set(['pending', 'expired', 'accepted']); - export interface InviteRowV1Params { /** Invitee email (e.g. "sarah@acme.com"). */ email: string; @@ -57,13 +56,13 @@ const STATUS_TONE_LIGHT: InviteStatusTone = { * accepted fg (#166534) → alertColors.successText */ export function buildInviteRowV1(params: InviteRowV1Params): ElementTree { - const requestedStatus = (params.status ?? 'pending') as string; - if (!VALID_INVITE_V1_STATUSES.has(requestedStatus)) { - throw new Error( - `add_invite_row_v1: invalid status "${requestedStatus}"; expected one of: pending, expired, accepted`, - ); - } - const status = requestedStatus as InviteV1Status; + const status = coerceEnum( + params.status, + ['pending', 'expired', 'accepted'], + 'pending', + 'buildInviteRowV1', + 'status', + ); const actionLabel = params.action_label ?? 'Resend'; const initial = (params.email.charAt(0) ?? '?').toUpperCase(); const theme = params.theme ?? 'light'; diff --git a/packages/pen-core/src/element-builders/kbd-v1.ts b/packages/pen-core/src/element-builders/kbd-v1.ts index 4c166f2cd..55b6398ec 100644 --- a/packages/pen-core/src/element-builders/kbd-v1.ts +++ b/packages/pen-core/src/element-builders/kbd-v1.ts @@ -1,3 +1,4 @@ +import { coerceStringArray } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -23,10 +24,7 @@ export interface KbdV1Params { * glyph text has no explicit fill in v0 (inherits canvas text color) */ export function buildKbdV1(params: KbdV1Params): ElementTree { - const keys = params.keys.filter((k): k is string => typeof k === 'string' && k.length > 0); - if (keys.length === 0) { - throw new Error('buildKbdV1 requires at least one non-empty key in `keys`.'); - } + const keys = coerceStringArray(params.keys, ['?'], 'buildKbdV1', 'keys'); const separator = params.separator ?? '+'; const theme = params.theme ?? 'light'; const isLight = theme === 'light'; diff --git a/packages/pen-core/src/element-builders/member-row-v1.ts b/packages/pen-core/src/element-builders/member-row-v1.ts index 1c97b8706..f2d4c8431 100644 --- a/packages/pen-core/src/element-builders/member-row-v1.ts +++ b/packages/pen-core/src/element-builders/member-row-v1.ts @@ -1,3 +1,4 @@ +import { coerceEnum } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -30,7 +31,6 @@ export interface MemberRowV1Params { const KNOB_WHITE = '#FFFFFF'; type StatusTone = 'online' | 'busy' | 'away' | 'offline'; -const VALID_STATUS_TONES = new Set(['online', 'busy', 'away', 'offline']); // Status dot colors are semantic/fixed — not theme-dependent const STATUS_TONE_FILL: Record = { @@ -78,13 +78,13 @@ function buildTrailing(t: MemberRowV1Trailing, theme: V1Theme): ElementTree { fill: [{ type: 'solid', color: colors.textSubtle }], }; } - const requestedTone = (t.tone ?? 'online') as string; - if (!VALID_STATUS_TONES.has(requestedTone)) { - throw new Error( - `add_member_row_v1: invalid trailing.tone "${requestedTone}"; expected one of: online, busy, away, offline`, - ); - } - const tone = requestedTone as StatusTone; + const tone = coerceEnum( + t.tone, + ['online', 'busy', 'away', 'offline'], + 'online', + 'buildMemberRowV1', + 'trailing.tone', + ); return { type: 'frame', name: 'Status Dot', diff --git a/packages/pen-core/src/element-builders/share-row-v1.ts b/packages/pen-core/src/element-builders/share-row-v1.ts index 5bf530254..4cdb05ade 100644 --- a/packages/pen-core/src/element-builders/share-row-v1.ts +++ b/packages/pen-core/src/element-builders/share-row-v1.ts @@ -1,3 +1,4 @@ +import { coerceNonEmptyArray } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -29,6 +30,16 @@ export interface ShareRowV1Params { * label text (#475569 slate-600) → textMuted token */ export function buildShareRowV1(params: ShareRowV1Params): ElementTree { + const targets = coerceNonEmptyArray( + params.targets, + [ + { label: 'Twitter', icon: 'twitter' }, + { label: 'Facebook', icon: 'facebook' }, + { label: 'Copy Link', icon: 'link' }, + ], + 'buildShareRowV1', + 'targets', + ); const theme = params.theme ?? 'light'; const isLight = theme === 'light'; const t = resolveTheme(theme); @@ -47,7 +58,7 @@ export function buildShareRowV1(params: ShareRowV1Params): ElementTree { layout: 'horizontal', alignItems: 'start', gap: 16, - children: params.targets.map((target, i) => ({ + children: targets.map((target, i) => ({ type: 'frame', name: `Target ${i + 1}`, role: 'share-target', diff --git a/packages/pen-core/src/element-builders/social-login-row-v1.ts b/packages/pen-core/src/element-builders/social-login-row-v1.ts index 403355bee..3caaff147 100644 --- a/packages/pen-core/src/element-builders/social-login-row-v1.ts +++ b/packages/pen-core/src/element-builders/social-login-row-v1.ts @@ -1,3 +1,4 @@ +import { coerceNonEmptyArray } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -63,10 +64,12 @@ const KNOWN_ICONS: Record = { * label text (#0F172A slate-900) → textPrimary token */ export function buildSocialLoginRowV1(params: SocialLoginRowV1Params): ElementTree { - const raw = Array.isArray(params.providers) ? params.providers : []; - if (raw.length === 0) { - throw new Error('buildSocialLoginRowV1: providers array must not be empty'); - } + const raw = coerceNonEmptyArray( + params.providers, + [{ name: 'google' }], + 'buildSocialLoginRowV1', + 'providers', + ); const providers = raw.slice(0, 6); const orientation = params.orientation ?? 'vertical'; const isVertical = orientation === 'vertical'; diff --git a/packages/pen-core/src/element-builders/tag-v1.ts b/packages/pen-core/src/element-builders/tag-v1.ts index a80c30ea6..923e4e2a3 100644 --- a/packages/pen-core/src/element-builders/tag-v1.ts +++ b/packages/pen-core/src/element-builders/tag-v1.ts @@ -1,10 +1,9 @@ +import { coerceEnum } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import type { V1Theme } from './resolve-theme.js'; export type TagV1Tone = 'default' | 'accent' | 'success' | 'warning' | 'error'; -const VALID_TAG_TONES = new Set(['default', 'accent', 'success', 'warning', 'error']); - export interface TagV1Params { label: string; /** Render the trailing × close icon. Default true. */ @@ -44,13 +43,14 @@ const TONES: Record = { * hardcoded across all theme modes. All modes produce identical trees. */ export function buildTagV1(params: TagV1Params): ElementTree { - const requestedTone = (params.tone ?? 'default') as string; - if (!VALID_TAG_TONES.has(requestedTone)) { - throw new Error( - `add_tag_v1: invalid tone "${requestedTone}"; expected one of: default, accent, success, warning, error`, - ); - } - const tone = TONES[requestedTone as TagV1Tone]; + const toneKey = coerceEnum( + params.tone, + ['default', 'accent', 'success', 'warning', 'error'], + 'default', + 'buildTagV1', + 'tone', + ); + const tone = TONES[toneKey]; const removable = params.removable ?? true; const children: ElementTree[] = [ { diff --git a/packages/pen-core/src/element-builders/timeline-v1.ts b/packages/pen-core/src/element-builders/timeline-v1.ts index 114e50a63..86746fcce 100644 --- a/packages/pen-core/src/element-builders/timeline-v1.ts +++ b/packages/pen-core/src/element-builders/timeline-v1.ts @@ -1,3 +1,4 @@ +import { coerceNonEmptyArray } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -34,10 +35,12 @@ const ACCENT = '#2563EB'; * - Subtitle text fill: #6B7280 → tokenized (textMuted) */ export function buildTimelineV1(params: TimelineV1Params): ElementTree { - const items = Array.isArray(params.items) ? params.items : []; - if (items.length === 0) { - throw new Error('buildTimelineV1: items must contain at least one entry'); - } + const items = coerceNonEmptyArray( + params.items, + [{ title: 'Item 1' }], + 'buildTimelineV1', + 'items', + ); const theme = params.theme ?? 'light'; const t = resolveTheme(theme); diff --git a/packages/pen-core/src/element-builders/toolbar-v1.ts b/packages/pen-core/src/element-builders/toolbar-v1.ts index 774ef1c90..2158416c8 100644 --- a/packages/pen-core/src/element-builders/toolbar-v1.ts +++ b/packages/pen-core/src/element-builders/toolbar-v1.ts @@ -1,3 +1,4 @@ +import { coerceNonEmptyArray } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -34,6 +35,12 @@ export interface ToolbarV1Params { * - Divider fill: #E2E8F0 → border */ export function buildToolbarV1(params: ToolbarV1Params): ElementTree { + const items = coerceNonEmptyArray( + params.items, + [{ icon: 'bold' }, { icon: 'italic', divider_after: true }, { icon: 'underline' }], + 'buildToolbarV1', + 'items', + ); const theme = params.theme ?? 'light'; const t = resolveTheme(theme); @@ -46,7 +53,7 @@ export function buildToolbarV1(params: ToolbarV1Params): ElementTree { const divider = theme === 'light' ? '#E2E8F0' : t.colors.border; const children: ElementTree[] = []; - params.items.forEach((item, i) => { + items.forEach((item, i) => { children.push({ type: 'frame', name: `Tool (${item.icon})`, @@ -70,7 +77,7 @@ export function buildToolbarV1(params: ToolbarV1Params): ElementTree { }, ], }); - if (item.divider_after && i < params.items.length - 1) { + if (item.divider_after && i < items.length - 1) { children.push({ type: 'frame', name: 'Toolbar Divider', diff --git a/packages/pen-core/src/element-builders/user-card-v1.ts b/packages/pen-core/src/element-builders/user-card-v1.ts index 7ef5510cc..0329666f5 100644 --- a/packages/pen-core/src/element-builders/user-card-v1.ts +++ b/packages/pen-core/src/element-builders/user-card-v1.ts @@ -1,3 +1,4 @@ +import { coerceNonEmptyString } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import { resolveTheme, type V1Theme } from './resolve-theme.js'; @@ -30,6 +31,7 @@ export interface UserCardV1Params { * - Initial text fill: #FFFFFF — always white on colored avatar, hardcoded all modes */ export function buildUserCardV1(params: UserCardV1Params): ElementTree { + const name = coerceNonEmptyString(params.name, 'User Name', 'buildUserCardV1', 'name'); const size = Math.min(96, Math.max(32, Math.floor(params.avatar_size ?? 48))); const initialFontSize = Math.max(12, Math.round(size * 0.4)); @@ -45,12 +47,16 @@ export function buildUserCardV1(params: UserCardV1Params): ElementTree { type: 'text', name: 'Name', role: 'user-card-name', - content: params.name, + content: name, fontSize: 15, fontWeight: 600, fill: [{ type: 'solid', color: nameColor }], }, ]; + // Conditional emit: builders never invent optional content, and an + // empty-content text node still consumes flex gap so it is not + // layout-neutral with a "no role line" output. Callers wanting the + // user-card-role node must pass a non-empty `role`. if (params.role) { stackChildren.push({ type: 'text', diff --git a/packages/pen-mcp/src/__tests__/add-heading-v1.test.ts b/packages/pen-mcp/src/__tests__/add-heading-v1.test.ts index 643cdb646..95cb01f27 100644 --- a/packages/pen-mcp/src/__tests__/add-heading-v1.test.ts +++ b/packages/pen-mcp/src/__tests__/add-heading-v1.test.ts @@ -132,10 +132,12 @@ describe('add_heading_v1 — error handling', () => { expect(await readFile(fp, 'utf-8')).toBe(before); }); - it('throws on invalid level', async () => { + it('coerces invalid level to default h2 instead of throwing', async () => { const fp = await fresh('a.op'); + // buildHeadingV1 fuzzy-coerces invalid 'caption' to 'h2' + warns; + // handler succeeds and inserts with default level (no rejection). await expect( handleAddHeadingV1({ filePath: fp, content: 'X', level: 'caption' as never }), - ).rejects.toThrow(/invalid level/); + ).resolves.toBeDefined(); }); });