From 6604ff4674b7a031d17bca9656b5300672d2bd0d Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Mon, 2 Nov 2026 07:06:23 -0800 Subject: [PATCH] fix(sitesmith): partial-build CTA bands get a mailto link or a visible note; review fix ops map hex to design tokens Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_0185ukXgk6UunFCKyXqLe2Cq --- craft/src/panels/sitesmith/SitesmithModal.tsx | 3 +- craft/src/panels/sitesmith/selfReview.ts | 1 + craft/src/sitesmith/prune-dead-links.test.ts | 79 +++++++++++ craft/src/sitesmith/prune-dead-links.ts | 124 ++++++++++++++++++ craft/src/sitesmith/replace-plan.ts | 8 +- craft/src/sitesmith/review-tokens.test.ts | 42 ++++++ craft/src/sitesmith/review-tokens.ts | 57 ++++++++ 7 files changed, 312 insertions(+), 2 deletions(-) create mode 100644 craft/src/sitesmith/prune-dead-links.test.ts create mode 100644 craft/src/sitesmith/prune-dead-links.ts create mode 100644 craft/src/sitesmith/review-tokens.test.ts create mode 100644 craft/src/sitesmith/review-tokens.ts diff --git a/craft/src/panels/sitesmith/SitesmithModal.tsx b/craft/src/panels/sitesmith/SitesmithModal.tsx index 6d29cbd..248dcc0 100644 --- a/craft/src/panels/sitesmith/SitesmithModal.tsx +++ b/craft/src/panels/sitesmith/SitesmithModal.tsx @@ -32,6 +32,7 @@ import { SitesmithTarget } from '../../state/SitesmithContext'; import { UpgradeBanner } from './UpgradeBanner'; import { ScopeConfirmDialog } from './ScopeConfirmDialog'; import { buildSitePagesPayload } from '../../utils/site-pages'; +import { tokenizeReviewOps } from '../../sitesmith/review-tokens'; import { planReplace, isDefaultChrome, withoutChrome, missingUploadedImageNote, addPagesResponse, type SiteImpact } from '../../sitesmith/replace-plan'; import { MessageList } from './MessageList'; import { ChatInput } from './ChatInput'; @@ -232,7 +233,7 @@ export const SitesmithModal: React.FC = ({ onClose, target }) => { }), applyResult: async (res) => { if (!res.ok) return { ok: false, message: res.message }; - const a = await apply(res.response); + const a = await apply(tokenizeReviewOps(res.response, design).resp); return { ok: a.ok, message: a.message, changed: a.changed, ops: a.ops, dropped: a.dropped, summary: a.adjustments?.length ? summarizeAdjustments(a.adjustments, a.notChecked) : undefined }; }, removeImage: (id) => ss.deleteRefImage(id), diff --git a/craft/src/panels/sitesmith/selfReview.ts b/craft/src/panels/sitesmith/selfReview.ts index caf81bf..e7fc290 100644 --- a/craft/src/panels/sitesmith/selfReview.ts +++ b/craft/src/panels/sitesmith/selfReview.ts @@ -9,6 +9,7 @@ import type { CaptureResult } from '../../utils/self-review-capture'; export const REVIEW_MESSAGE = 'Review the attached screenshot of my page as it is built now (it is a render of the current page, not a mockup to copy). ' + 'Fix only real problems with a small patch: hard-to-read text, cramped or uneven spacing, broken or misaligned layout, badly cropped images, missing icons. ' + + 'When a fix sets a colour, use the site design tokens (var(--wsb-primary), var(--wsb-accent), var(--wsb-text), var(--wsb-bg), var(--wsb-muted), ...) instead of hex values. ' + 'If it already looks right, reply with a patch that has no ops and say so. Do not redesign the page or change the wording.'; export interface SelfReviewDeps { diff --git a/craft/src/sitesmith/prune-dead-links.test.ts b/craft/src/sitesmith/prune-dead-links.test.ts new file mode 100644 index 0000000..ef225d8 --- /dev/null +++ b/craft/src/sitesmith/prune-dead-links.test.ts @@ -0,0 +1,79 @@ +import { describe, it, expect } from 'vitest'; +import type { SitesmithResponse, SerializedTreeNode } from '../types/sitesmith'; +import { pruneDeadLinks, emptiedBandsNote, findContactEmail } from './prune-dead-links'; +import { planReplace } from './replace-plan'; + +const n = (resolvedName: string, props: Record = {}, nodes?: SerializedTreeNode[]): SerializedTreeNode => ({ type: { resolvedName }, props, ...(nodes ? { nodes } : {}) }); +const site = (...trees: SerializedTreeNode[]): Extract => ({ type: 'replace', scope: 'site', message: '', pages: trees.map((t, i) => ({ name: i ? `P${i}` : 'Home', tree: t })) }); +const band = (extra: Record = {}) => n('CallToAction', { heading: 'Ready?', ctas: [{ text: 'Contact us', href: '/contact', variant: 'primary' }], ...extra }); +const ctasOf = (r: SitesmithResponse) => ((r as any).pages[0].tree.nodes[0].props.ctas); + +describe('pruneDeadLinks', () => { + it('turns the only CTA into a mailto link when a contact email is known', () => { + const r = pruneDeadLinks(site(n('Container', {}, [band({ text: 'Write to hello@acme.com' })])), { missing: ['Contact'] }); + expect(ctasOf(r.resp)).toEqual([{ text: 'Contact us', href: 'mailto:hello@acme.com', variant: 'primary' }]); + expect(r.mailtoAdded).toBe(1); + expect(r.emptiedButtons).toBe(0); + }); + it('finds the email in the brief / site context too', () => { + const r = pruneDeadLinks(site(n('Container', {}, [band()])), { missing: ['Contact'], context: ['Our email is owner@shop.org'] }); + expect(ctasOf(r.resp)[0].href).toBe('mailto:owner@shop.org'); + }); + it('keeps the band and counts the removed button when no email is known', () => { + const r = pruneDeadLinks(site(n('Container', {}, [band()])), { missing: ['Contact'] }); + expect(ctasOf(r.resp)).toEqual([]); + expect((r.resp as any).pages[0].tree.nodes[0].props.heading).toBe('Ready?'); + expect(r.emptiedButtons).toBe(1); + expect(r.mailtoAdded).toBe(0); + expect(emptiedBandsNote(r.emptiedButtons)).toBe('1 call-to-action button was removed because its page was not built. The section kept its heading and text; add a button or contact link there if you want one.'); + expect(emptiedBandsNote(2)).toMatch(/^2 call-to-action buttons were removed because their pages were not built/); + expect(emptiedBandsNote(0)).toBeNull(); + }); + it('leaves a band alone when another button survives, and ignores built pages and external links', () => { + const b = band({ ctas: [{ text: 'Contact', href: '/contact' }, { text: 'About', href: '/about.html' }, { text: 'Ext', href: 'https://x.com/contact' }] }); + const r = pruneDeadLinks(site(n('Container', {}, [b])), { missing: ['Contact'] }); + expect(ctasOf(r.resp).map((c: any) => c.text)).toEqual(['About', 'Ext']); + expect(r.emptiedButtons).toBe(0); + expect(r.removed).toBe(1); + }); + it('handles a lone ButtonLink child in both branches', () => { + const mk = () => site(n('Container', {}, [n('Container', {}, [n('Heading', { text: 'Hi' }), n('ButtonLink', { text: 'Go', href: '/contact' })])])); + const withMail = pruneDeadLinks(mk(), { missing: ['Contact'], contactEmail: 'a@b.co' }); + expect((withMail.resp as any).pages[0].tree.nodes[0].nodes[1].props.href).toBe('mailto:a@b.co'); + const without = pruneDeadLinks(mk(), { missing: ['Contact'] }); + expect((without.resp as any).pages[0].tree.nodes[0].nodes.map((x: any) => x.type.resolvedName)).toEqual(['Heading']); + expect(without.emptiedButtons).toBe(1); + }); + it('rewrites legacy buttonText/buttonHref props', () => { + const t = n('Container', {}, [n('CTASection', { buttonText: 'Book', buttonHref: '/contact' })]); + const r = pruneDeadLinks(site(t), { missing: ['Contact'] }); + expect((r.resp as any).pages[0].tree.nodes[0].props.buttonText).toBe(''); + expect(r.emptiedButtons).toBe(1); + }); + it('is a no-op without missing pages and does not mutate its input', () => { + const resp = site(n('Container', {}, [band()])); + const copy = JSON.stringify(resp); + expect(pruneDeadLinks(resp, { missing: [] }).resp).toBe(resp); + pruneDeadLinks(resp, { missing: ['Contact'] }); + expect(JSON.stringify(resp)).toBe(copy); + }); + it('findContactEmail prefers mailto links', () => { + expect(findContactEmail('x a@b.com', { href: 'mailto:z@y.org?subject=1' })).toBe('z@y.org'); + expect(findContactEmail('nothing')).toBeNull(); + }); +}); + +describe('planReplace partial path', () => { + const ctx = { userText: 'build the whole site', chromeDefault: false, pages: [], siteBuild: { planned: ['Home', 'Contact'], built: ['Home'], missing: ['Contact'] } }; + it('surfaces a note when a band is emptied', () => { + const p = planReplace(site(n('Container', {}, [band()])), ctx); + expect(p.confirm).toBe('site-partial'); + expect(p.note).toMatch(/1 call-to-action button was removed/); + expect(ctasOf(p.resp)).toEqual([]); + }); + it('uses mailto and no note when the brief has an email', () => { + const p = planReplace(site(n('Container', {}, [band()])), { ...ctx, userText: 'build the whole site, email is me@x.io' }); + expect(p.note).toBeUndefined(); + expect(ctasOf(p.resp)[0].href).toBe('mailto:me@x.io'); + }); +}); diff --git a/craft/src/sitesmith/prune-dead-links.ts b/craft/src/sitesmith/prune-dead-links.ts new file mode 100644 index 0000000..d444ae0 --- /dev/null +++ b/craft/src/sitesmith/prune-dead-links.ts @@ -0,0 +1,124 @@ +/** + * Partial whole-site build: buttons that link to a page that was not built are dead ends, so they are + * removed (round 6 fix). When that would leave a call-to-action band with its heading and text but no + * button, the band is repaired instead of being left silently dead: + * - a contact email is known (brief, site pages, or the built pages' text / mailto links) -> the button + * becomes a `mailto:` link, keeping its label and look; + * - otherwise the band stays and `emptiedButtons` counts what was removed, so the caller can show a + * visible, non-blocking note (see `emptiedBandsNote`). + */ +import type { SerializedTreeNode, SitesmithResponse } from '../types/sitesmith'; +import { normalizeCtas, type CtaButton } from '../components/sections/_cta-helpers'; + +export interface PruneResult { + resp: SitesmithResponse; + /** Every button / CTA entry removed because its page was not built. */ + removed: number; + /** Bands whose only button became a mailto: link. */ + mailtoAdded: number; + /** Buttons removed from bands that now have none (no contact email was known). */ + emptiedButtons: number; +} + +const EMAIL_RE = /[A-Z0-9._%+-]+@[A-Z0-9-]+(?:\.[A-Z0-9-]+)*\.[A-Z]{2,}/i; +const slugOf = (s: string) => s.toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, ''); + +/** "/about.html", "about", "./about/?x" -> "about"; external, anchor and mailto/tel links -> null. */ +function pageSlug(href: unknown): string | null { + const h = typeof href === 'string' ? href.trim() : ''; + if (!h || h.startsWith('#') || h.startsWith('//') || /^[a-z][a-z0-9+.-]*:/i.test(h)) return null; + const p = h.replace(/^\.\//, '').replace(/[?#].*$/, '').replace(/\.html?$/i, '').replace(/^\/+|\/+$/g, ''); + return p ? slugOf(p) : null; +} + +/** First email found in free text, a props object, or a tree (mailto: links first). Null when none. */ +export function findContactEmail(...sources: unknown[]): string | null { + let text = ''; + const collect = (v: unknown, depth = 0) => { + if (depth > 60 || v == null) return; + if (typeof v === 'string') { text += `${v}\n`; return; } + if (Array.isArray(v)) { v.forEach((x) => collect(x, depth + 1)); return; } + if (typeof v === 'object') Object.values(v as object).forEach((x) => collect(x, depth + 1)); + }; + sources.forEach((s) => collect(s)); + const mailto = text.match(/mailto:\s*([^\s"'?<>]+)/i); + if (mailto && EMAIL_RE.test(mailto[1])) return mailto[1].match(EMAIL_RE)![0]; + const m = text.match(EMAIL_RE); + return m ? m[0] : null; +} + +interface Ctx { dead: Set; email: string | null; removed: number; mailto: number; emptied: number } + +const isDead = (href: unknown, c: Ctx) => { const s = pageSlug(href); return s !== null && c.dead.has(s); }; + +function fixCtaProps(props: Record, c: Ctx): Record | null { + const list = normalizeCtas(props as any); + if (!list.some((b) => isDead(b.href, c))) return null; + const alive = list.filter((b) => !isDead(b.href, c)); + const deadOnes = list.filter((b) => isDead(b.href, c)); + c.removed += deadOnes.length; + let next: CtaButton[] = alive; + if (!alive.length) { + if (c.email) { + const f = deadOnes[0]; + next = [{ ...f, href: `mailto:${c.email}` }]; + c.mailto += 1; + } else c.emptied += deadOnes.length; + } + if (Array.isArray(props.ctas) && props.ctas.length) return { ...props, ctas: next }; + // legacy primary/secondary props: rewrite them from the surviving list + const [a, b] = next; + return { + ...props, + buttonText: a?.text ?? '', buttonHref: a?.href ?? '', + secondaryButtonText: b?.text ?? '', secondaryButtonHref: b?.href ?? '', + }; +} + +function fixNode(n: SerializedTreeNode, c: Ctx): SerializedTreeNode { + let node = n; + if (n.props && typeof n.props === 'object' && n.type?.resolvedName !== 'ButtonLink') { + const p = fixCtaProps(n.props, c); + if (p) node = { ...node, props: p }; + } + if (!node.nodes?.length) return node; + const kids = node.nodes.map((k) => fixNode(k, c)); + // ButtonLink children linking to an unbuilt page + const buttons = kids.filter((k) => k.type?.resolvedName === 'ButtonLink'); + const deadBtns = buttons.filter((k) => isDead(k.props?.href, c)); + let out = kids; + if (deadBtns.length) { + c.removed += deadBtns.length; + if (deadBtns.length === buttons.length) { + if (c.email) { + const keep = deadBtns[0]; + c.mailto += 1; + out = kids.flatMap((k) => (k === keep ? [{ ...k, props: { ...k.props, href: `mailto:${c.email}` } }] : deadBtns.includes(k) ? [] : [k])); + } else { + c.emptied += deadBtns.length; + out = kids.filter((k) => !deadBtns.includes(k)); + } + } else out = kids.filter((k) => !deadBtns.includes(k)); + } + return { ...node, nodes: out }; +} + +export function pruneDeadLinks(resp: SitesmithResponse, o: { missing: string[]; contactEmail?: string | null; context?: unknown[] }): PruneResult { + const dead = new Set(o.missing.map(slugOf).filter(Boolean)); + const none = { resp, removed: 0, mailtoAdded: 0, emptiedButtons: 0 }; + if (resp.type !== 'replace' || !dead.size) return none; + const email = o.contactEmail ?? findContactEmail(...(o.context ?? []), resp.pages, resp.header, resp.footer); + const c: Ctx = { dead, email, removed: 0, mailto: 0, emptied: 0 }; + const fix = (x: T): T => (x ? ({ ...x, tree: fixNode(x.tree, c) } as T) : x); + const out = { ...resp, pages: resp.pages.map((p) => fix(p)) } as typeof resp; + if (resp.header) out.header = fix(resp.header); + if (resp.footer) out.footer = fix(resp.footer); + if (!c.removed) return none; + return { resp: out, removed: c.removed, mailtoAdded: c.mailto, emptiedButtons: c.emptied }; +} + +/** The visible, non-blocking note for bands left without a button; null when nothing was emptied. */ +export function emptiedBandsNote(n: number): string | null { + if (n <= 0) return null; + return `${n} call-to-action button${n === 1 ? ' was' : 's were'} removed because ${n === 1 ? 'its page was' : 'their pages were'} not built. The section${n === 1 ? '' : 's'} kept ${n === 1 ? 'its' : 'their'} heading and text; add a button or contact link there if you want one.`; +} diff --git a/craft/src/sitesmith/replace-plan.ts b/craft/src/sitesmith/replace-plan.ts index b134c0f..8eb38f0 100644 --- a/craft/src/sitesmith/replace-plan.ts +++ b/craft/src/sitesmith/replace-plan.ts @@ -9,6 +9,7 @@ */ import type { SitesmithResponse } from '../types/sitesmith'; import type { PageData } from '../types'; +import { pruneDeadLinks, emptiedBandsNote } from './prune-dead-links'; import { repairHeaderLinks, repairFooterLinks } from './header-repair'; import { mergeAiPages } from '../state/PageContext'; @@ -108,7 +109,12 @@ export function planReplace(resp: SitesmithResponse, ctx: { userText: string | n if (resp.scope === 'site' && explicitSiteRequest(ctx.userText)) { const impact = siteImpact(resp, ctx.pages ?? [], ctx.siteBuild); // a partial build is never offered as a bare destructive replace (live case 2 deleted the owner's pages) - return { resp, confirm: impact.missing.length ? 'site-partial' : 'site', impact }; + if (!impact.missing.length) return { resp, confirm: 'site', impact }; + // buttons to pages that were not built are removed; a band left without a button gets a mailto: link + // (known contact email) or a visible note, never a silent dead band + const pruned = pruneDeadLinks(resp, { missing: impact.missing, context: [ctx.userText, ctx.pages?.map((p) => p.craftState)] }); + const note = emptiedBandsNote(pruned.emptiedButtons); + return { resp: pruned.resp, confirm: 'site-partial', impact, ...(note ? { note } : {}) }; } const notes: string[] = []; diff --git a/craft/src/sitesmith/review-tokens.test.ts b/craft/src/sitesmith/review-tokens.test.ts new file mode 100644 index 0000000..c4512eb --- /dev/null +++ b/craft/src/sitesmith/review-tokens.test.ts @@ -0,0 +1,42 @@ +import { describe, it, expect } from 'vitest'; +import { DEFAULT_SITE_DESIGN } from '../state/SiteDesignContext'; +import { tokenizeReviewOps, tokenForHex } from './review-tokens'; +import { REVIEW_MESSAGE } from '../panels/sitesmith/selfReview'; +import type { SitesmithResponse } from '../types/sitesmith'; + +const design = { ...DEFAULT_SITE_DESIGN, primaryColor: '#1A2B3C', accentColor: '#f60', textColor: '#111111' }; +const patch = (props: Record): SitesmithResponse => ({ type: 'patch', message: '', ops: [{ op: 'update_props', node_id: 'a', props }] }); +const propsOf = (r: SitesmithResponse) => (r as any).ops[0].props; + +describe('tokenizeReviewOps', () => { + it('maps a matching hex (case-insensitive, 3-digit expanded) to its token', () => { + const { resp, mapped } = tokenizeReviewOps(patch({ color: '#1a2b3c', style: { backgroundColor: '#FF6600' } }), design); + expect(propsOf(resp).color).toBe('var(--wsb-primary)'); + expect(propsOf(resp).style.backgroundColor).toBe('var(--wsb-accent)'); + expect(mapped).toBe(2); + }); + it('leaves unmatched hex and non-colour text alone', () => { + const input = patch({ color: '#abcdef', text: 'Order #111111 now' }); + const { resp, mapped } = tokenizeReviewOps(input, design); + expect(resp).toBe(input); + expect(mapped).toBe(0); + }); + it('maps hex inside composite colour values only for colour-ish props', () => { + const { resp } = tokenizeReviewOps(patch({ style: { border: '1px solid #111111' }, text: 'x #111111 y' }), design); + expect(propsOf(resp).style.border).toBe('1px solid var(--wsb-text)'); + expect(propsOf(resp).text).toBe('x #111111 y'); + }); + it('covers trees in replace/insert ops, does not mutate, and ignores non-patch responses', () => { + const r: SitesmithResponse = { type: 'patch', message: '', ops: [{ op: 'insert_after', node_id: 'a', tree: { type: { resolvedName: 'Text' }, props: { color: '#1A2B3C' } } }] }; + const copy = JSON.stringify(r); + expect((tokenizeReviewOps(r, design).resp as any).ops[0].tree.props.color).toBe('var(--wsb-primary)'); + expect(JSON.stringify(r)).toBe(copy); + const ask: SitesmithResponse = { type: 'ask', question: 'q' }; + expect(tokenizeReviewOps(ask, design).resp).toBe(ask); + expect(tokenForHex('not a colour', design)).toBeNull(); + }); + it('the review prompt tells the model to prefer tokens', () => { + expect(REVIEW_MESSAGE).toMatch(/var\(--wsb-primary\)/); + expect(REVIEW_MESSAGE).toMatch(/instead of hex/); + }); +}); diff --git a/craft/src/sitesmith/review-tokens.ts b/craft/src/sitesmith/review-tokens.ts new file mode 100644 index 0000000..7afe482 --- /dev/null +++ b/craft/src/sitesmith/review-tokens.ts @@ -0,0 +1,57 @@ +/** + * Self-review fix ops hard-code hex colours (round 6 finding). A hex that equals a site design token's + * value (case-insensitive, #abc == #aabbcc) is mapped to `var(--wsb-)` so the fix keeps following + * the site design; any other hex is left alone. Pure: returns new objects, never mutates the response. + */ +import type { SitesmithResponse } from '../types/sitesmith'; +import type { SiteDesign } from '../state/SiteDesignContext'; +import { TOKEN_VARS } from '../utils/tokens'; + +const HEX_EXACT = /^#(?:[0-9a-f]{3}|[0-9a-f]{6})$/i; +const HEX_ANY = /#(?:[0-9a-f]{6}|[0-9a-f]{3})(?![0-9a-z_-])/gi; +/** Props whose value is a CSS value that may embed a colour ("1px solid #fff"). */ +const COLOURISH_KEY = /colou?r|background|border|shadow|fill|stroke|outline|gradient/i; + +function canon(hex: string): string { + let h = hex.trim().toLowerCase(); + if (h.length === 4) h = `#${h[1]}${h[1]}${h[2]}${h[2]}${h[3]}${h[3]}`; + return h; +} + +export function tokenForHex(hex: string, design: SiteDesign): string | null { + if (!HEX_EXACT.test(hex.trim())) return null; + const want = canon(hex); + for (const [varName, key] of TOKEN_VARS) { + const v = (design as unknown as Record)[key]; + if (typeof v === 'string' && HEX_EXACT.test(v.trim()) && canon(v) === want) return `var(${varName})`; + } + return null; +} + +function mapValue(v: unknown, key: string, design: SiteDesign, hit: { n: number }): unknown { + if (typeof v === 'string') { + const exact = tokenForHex(v, design); + if (exact) { hit.n++; return exact; } + if (COLOURISH_KEY.test(key) && v.includes('#')) { + return v.replace(HEX_ANY, (m) => { const t = tokenForHex(m, design); if (t) hit.n++; return t ?? m; }); + } + return v; + } + if (Array.isArray(v)) return v.map((x) => mapValue(x, key, design, hit)); + if (v && typeof v === 'object') { + return Object.fromEntries(Object.entries(v as Record).map(([k, x]) => [k, mapValue(x, k, design, hit)])); + } + return v; +} + +/** Patch responses only; the number of colours mapped is returned for tests and logging. */ +export function tokenizeReviewOps(resp: SitesmithResponse, design: SiteDesign): { resp: SitesmithResponse; mapped: number } { + if (resp.type !== 'patch') return { resp, mapped: 0 }; + const hit = { n: 0 }; + const ops = resp.ops.map((op) => { + if (op.op === 'update_props') return { ...op, props: mapValue(op.props, '', design, hit) as typeof op.props }; + if (op.op === 'replace_node' || op.op === 'insert_after' || op.op === 'insert_before') return { ...op, tree: mapValue(op.tree, '', design, hit) as typeof op.tree }; + return op; + }); + return hit.n ? { resp: { ...resp, ops }, mapped: hit.n } : { resp, mapped: 0 }; +}