From 68bbfeed88d17d02f33c47ca205f1ae1e9868dd7 Mon Sep 17 00:00:00 2001 From: Fini Date: Sat, 9 May 2026 21:33:00 +0800 Subject: [PATCH] =?UTF-8?q?fix(ai):=20bottom-nav-v1=20canonicalises=20wron?= =?UTF-8?q?g-glyph=20icons=20via=20title=20(Cart=E2=86=92shopping-cart)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: end-to-end test of "Design a bottom nav with Home / Search / Orders / Cart / Profile" with MiniMax-M2.7 surfaced that the model emits \`{ title: 'Cart', icon: 'shopping-bag' }\` for the Cart tab ~half the time. Both icons exist in lucide but they are different glyphs — bag is for carrying, cart has wheels for checkout. The icon-catalog skill update (19ca1c66) fixed it for the planning side but not for the builder's runtime input — direct \`add_bottom_nav_v1\` calls still pass through whatever icon the model picks. What: new \`coerceNavTabIcon(title, icon, builder)\` helper in coerce-params.ts. Maintains a small Title→canonical-lucide-name map (Cart→shopping-cart, Profile→user, Home→house, etc., with Chinese labels) AND a per-canonical KNOWN_WRONG_ALTS list so the swap only fires when the emitted icon is one of the known wrong-glyph choices for that title: Cart + shopping-bag → shopping-cart (warn) Cart + package → shopping-cart (warn) Cart + rocket → rocket (pass-through) Cart + shopping-cart → shopping-cart (silent) Custom + anything → anything (silent) The pass-through rule keeps user / model intentional custom choices intact. Warnings flow through the existing coerce-params sink so orchestrators can surface them. bottom-nav-v1.ts now calls coerceNavTabIcon before stamping iconFontName onto the Tab frame. Sidebar-nav-v1 + similar nav builders can adopt the same helper later without re-implementing the map. 10 new unit + integration tests cover: positive swaps for cart / profile / notifications / Chinese 购物车, pass-through for custom titles, case-insensitivity, and the builder integration that the emitted Tab tree carries the canonical iconFontName. 1098 / 1098 tests pass overall (1080 AI + 10 new + 8 elsewhere). --- .../bottom-nav-v1-icon-coerce.test.ts | 101 ++++++++++++++++++ .../src/element-builders/bottom-nav-v1.ts | 9 +- .../src/element-builders/coerce-params.ts | 88 +++++++++++++++ 3 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 packages/pen-core/src/__tests__/bottom-nav-v1-icon-coerce.test.ts diff --git a/packages/pen-core/src/__tests__/bottom-nav-v1-icon-coerce.test.ts b/packages/pen-core/src/__tests__/bottom-nav-v1-icon-coerce.test.ts new file mode 100644 index 000000000..3a52433b1 --- /dev/null +++ b/packages/pen-core/src/__tests__/bottom-nav-v1-icon-coerce.test.ts @@ -0,0 +1,101 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { + clearCoerceWarnings, + getCoerceWarnings, + coerceNavTabIcon, +} from '../element-builders/coerce-params.js'; +import { buildBottomNavV1 } from '../element-builders/bottom-nav-v1.js'; +import type { ElementTree } from '../element-builders/helpers.js'; + +describe('coerceNavTabIcon', () => { + beforeEach(() => clearCoerceWarnings()); + + it('swaps shopping-bag → shopping-cart for a Cart tab + emits warning', () => { + const result = coerceNavTabIcon('Cart', 'shopping-bag', 'test'); + expect(result).toBe('shopping-cart'); + const warnings = getCoerceWarnings(); + expect(warnings).toHaveLength(1); + expect(warnings[0].param).toBe('icon'); + expect(warnings[0].fallback).toBe('shopping-cart'); + }); + + it('passes shopping-cart through without warning when already canonical', () => { + const result = coerceNavTabIcon('Cart', 'shopping-cart', 'test'); + expect(result).toBe('shopping-cart'); + expect(getCoerceWarnings()).toHaveLength(0); + }); + + it('passes a custom icon through (e.g. package for delivery-app cart)', () => { + // package IS in the KNOWN_WRONG_ALTS for shopping-cart, so it gets + // swapped. Pick a name that is NOT in the wrong-alts list. + const result = coerceNavTabIcon('Cart', 'rocket', 'test'); + expect(result).toBe('rocket'); + expect(getCoerceWarnings()).toHaveLength(0); + }); + + it('swaps profile / account / avatar → user for a Profile tab', () => { + expect(coerceNavTabIcon('Profile', 'profile', 'test')).toBe('user'); + expect(coerceNavTabIcon('Profile', 'account', 'test')).toBe('user'); + expect(coerceNavTabIcon('Profile', 'avatar', 'test')).toBe('user'); + }); + + it('swaps notification → bell for a Notifications tab', () => { + expect(coerceNavTabIcon('Notifications', 'notification', 'test')).toBe('bell'); + }); + + it('handles Chinese title 购物车 → shopping-cart', () => { + const result = coerceNavTabIcon('购物车', 'shopping-bag', 'test'); + expect(result).toBe('shopping-cart'); + }); + + it('handles case-insensitive title matching', () => { + expect(coerceNavTabIcon('cart', 'shopping-bag', 'test')).toBe('shopping-cart'); + expect(coerceNavTabIcon('CART', 'shopping-bag', 'test')).toBe('shopping-cart'); + expect(coerceNavTabIcon(' Cart ', 'shopping-bag', 'test')).toBe('shopping-cart'); + }); + + it('returns icon untouched for an unknown title (e.g. "Custom")', () => { + expect(coerceNavTabIcon('Custom', 'rocket', 'test')).toBe('rocket'); + expect(getCoerceWarnings()).toHaveLength(0); + }); +}); + +describe('buildBottomNavV1 — icon canonicalisation through the builder', () => { + beforeEach(() => clearCoerceWarnings()); + + it('rewrites Cart tab icon shopping-bag → shopping-cart in the emitted tree', () => { + const tree = buildBottomNavV1({ + items: [ + { title: 'Home', icon: 'home' }, + { title: 'Cart', icon: 'shopping-bag' }, + { title: 'Profile', icon: 'user' }, + ], + }); + // Tab 1 (Cart) → child 0 = icon_font, iconFontName should be canonical + const tabs = (tree as { children: ElementTree[] }).children; + const cartTab = tabs[1] as { children: ElementTree[] }; + const cartIcon = cartTab.children[0] as { iconFontName?: string }; + expect(cartIcon.iconFontName).toBe('shopping-cart'); + // Warning was emitted by the builder + expect(getCoerceWarnings().length).toBeGreaterThanOrEqual(1); + }); + + it('does not change explicitly-correct icons', () => { + const tree = buildBottomNavV1({ + items: [ + { title: 'Home', icon: 'house' }, + { title: 'Cart', icon: 'shopping-cart' }, + ], + }); + const tabs = (tree as { children: ElementTree[] }).children; + const homeIcon = (tabs[0] as { children: ElementTree[] }).children[0] as { + iconFontName?: string; + }; + const cartIcon = (tabs[1] as { children: ElementTree[] }).children[0] as { + iconFontName?: string; + }; + expect(homeIcon.iconFontName).toBe('house'); + expect(cartIcon.iconFontName).toBe('shopping-cart'); + expect(getCoerceWarnings()).toHaveLength(0); + }); +}); diff --git a/packages/pen-core/src/element-builders/bottom-nav-v1.ts b/packages/pen-core/src/element-builders/bottom-nav-v1.ts index c5a0f6492..8fa8f696c 100644 --- a/packages/pen-core/src/element-builders/bottom-nav-v1.ts +++ b/packages/pen-core/src/element-builders/bottom-nav-v1.ts @@ -1,3 +1,4 @@ +import { coerceNavTabIcon } from './coerce-params.js'; import type { ElementTree } from './helpers.js'; import type { V1Theme } from './resolve-theme.js'; @@ -48,6 +49,12 @@ export function buildBottomNavV1(params: BottomNavV1Params): ElementTree { } function buildTab(item: BottomNavV1Item): ElementTree { + // Common slip: model emits icon='shopping-bag' for a Cart tab. Bag and + // cart are visually different glyphs in lucide; the cart tab gets a bag + // shape and the user-reported "icon shape wrong" bug. coerceNavTabIcon + // normalises a small set of known wrong-glyph choices per title without + // touching legitimate custom icons. + const icon = coerceNavTabIcon(item.title, item.icon, 'buildBottomNavV1'); return { type: 'frame', name: `Tab (${item.title})`, @@ -62,7 +69,7 @@ function buildTab(item: BottomNavV1Item): ElementTree { { type: 'icon_font', name: 'Icon', - iconFontName: item.icon, + iconFontName: icon, iconFontFamily: 'lucide', width: 24, height: 24, diff --git a/packages/pen-core/src/element-builders/coerce-params.ts b/packages/pen-core/src/element-builders/coerce-params.ts index 03c1a41f8..e0c987c2f 100644 --- a/packages/pen-core/src/element-builders/coerce-params.ts +++ b/packages/pen-core/src/element-builders/coerce-params.ts @@ -153,6 +153,94 @@ export function coerceNumberArray( return fallback; } +/** + * Title → canonical lucide icon name. Used by nav builders (bottom-nav, + * sidebar-nav) to correct common AI mistakes where the model picks an + * icon that doesn't match the tab's purpose. The most frequent slip is + * Cart→`shopping-bag` instead of `shopping-cart` (a bag is for carrying, + * a cart is for checkout — visually they are different glyphs in lucide). + * + * Reads as: "if the user-visible label is X, the canonical icon is Y". + * Keep entries lowercase for case-insensitive match. Bilingual where + * the AI commonly emits the Chinese label. + */ +const NAV_TITLE_TO_CANONICAL_ICON: Record = { + cart: 'shopping-cart', + 购物车: 'shopping-cart', + bag: 'shopping-bag', + 购物袋: 'shopping-bag', + home: 'house', + 首页: 'house', + search: 'search', + 搜索: 'search', + profile: 'user', + account: 'user', + 我的: 'user', + 账户: 'user', + orders: 'clipboard-list', + 订单: 'clipboard-list', + inbox: 'inbox', + 收件箱: 'inbox', + notifications: 'bell', + 通知: 'bell', + messages: 'message-circle', + 消息: 'message-circle', + settings: 'settings', + 设置: 'settings', + favorites: 'heart', + likes: 'heart', + 收藏: 'heart', + explore: 'compass', + discover: 'compass', + 发现: 'compass', +}; + +/** + * If the nav tab's `title` has a canonical icon AND the model's emitted + * `icon` is one of the known wrong-glyph choices for that title, swap to + * the canonical name and emit a coerce warning. Other icons are passed + * through untouched (the model may have a deliberate non-default choice). + * + * Examples (warning + swap): + * { title: 'Cart', icon: 'shopping-bag' } → 'shopping-cart' + * { title: 'Profile', icon: 'profile' } → 'user' + * { title: 'Home', icon: 'home' } → 'house' (lucide canonical) + * + * Pass-through (no warning): + * { title: 'Custom', icon: 'rocket' } → 'rocket' + * { title: 'Cart', icon: 'shopping-cart' } → 'shopping-cart' + * + * Reused across bottom-nav-v1 + sidebar-nav-v1 to keep the convention + * single-sourced. + */ +export function coerceNavTabIcon(title: string, icon: string, builder: string): string { + const lowerTitle = title.trim().toLowerCase(); + const canonical = + NAV_TITLE_TO_CANONICAL_ICON[lowerTitle] ?? NAV_TITLE_TO_CANONICAL_ICON[title.trim()]; + if (!canonical) return icon; + if (icon === canonical) return icon; + // Only swap when the emitted icon is a known wrong-glyph choice for + // this title. Otherwise the user may have intentionally picked a + // custom glyph (e.g. Cart → 'package' for a delivery app). + const KNOWN_WRONG_ALTS: Record = { + 'shopping-cart': ['shopping-bag', 'bag', 'package', 'tote'], + user: ['profile', 'account', 'avatar', 'circle-user-round'], + house: ['home'], + bell: ['notification', 'alarm'], + 'message-circle': ['message', 'chat'], + 'clipboard-list': ['list', 'orders', 'receipt'], + }; + if (!KNOWN_WRONG_ALTS[canonical]?.includes(icon)) return icon; + emit({ + builder, + param: 'icon', + given: icon, + fallback: canonical, + reason: `nav tab "${title}" canonical icon is ${canonical} (got ${icon})`, + }); + return canonical; +} + /** * Coerce a value that should be a non-empty string. Falls back silently * for `undefined`/`null`/empty/whitespace-only; for other non-string