diff --git a/craft/scripts/e2e-capture-fidelity-entry.ts b/craft/scripts/e2e-capture-fidelity-entry.ts index ab72791..0623360 100644 --- a/craft/scripts/e2e-capture-fidelity-entry.ts +++ b/craft/scripts/e2e-capture-fidelity-entry.ts @@ -1,6 +1,10 @@ // Bundled by scripts/e2e-capture-fidelity.mjs and run in real Firefox + Chromium. import { captureHtml } from '../src/utils/self-review-capture'; import { sanitizeCaptureHtml } from '../src/utils/sanitize-capture-html'; +import { cleanAiProps } from '../src/sitesmith/clean-ai-response'; + +/** Round 6: the background URL the editor really stores for an AI hero placeholder (apply-time cleaner). */ +(window as any).cleanedBgImage = (url: string) => cleanAiProps({ bgImage: url }, 'BackgroundSection').bgImage; (window as any).sanitizeCaptureHtml = sanitizeCaptureHtml; /** Capture through the real path, then decode the JPEG and return RGB samples at the given points. */ diff --git a/craft/scripts/e2e-capture-fidelity.mjs b/craft/scripts/e2e-capture-fidelity.mjs index 6758b28..e931360 100644 --- a/craft/scripts/e2e-capture-fidelity.mjs +++ b/craft/scripts/e2e-capture-fidelity.mjs @@ -135,6 +135,26 @@ async function run(name, launcher, exe) { check(name, label, hits >= 3, `(${hits} pixels near rgb(${want.join(',')}))`); } } + // Round 6 (live case 4, Firefox): a background placeholder showed a faint "1920 x 1080" ghost label behind the + // hero in the review render (the same-colour label still antialiased). The URL the editor stores for the live + // case's hero, through the real capture path: every pixel of the box must be the box colour (max channel + // deviation <= 2 for JPEG), over a dense grid that covers where the label was drawn. + { + const LIVE = 'https://placehold.co/1920x1080/101a2b/fbfaf7?text=Team+collaborating+in+a+modern+office'; + const bgUrl = await page.evaluate((u) => window.cleanedBgImage(u), LIVE); + check(name, 'label-free background placeholder URL (zero-width label)', /^https:\/\/placehold\.co\/1920x1080\/101a2b\/101a2b\?text=%E2%80%8B$/.test(bgUrl), bgUrl); + // the live case-4 hero as exported (BackgroundSection: quoted url(), cover, a same-colour overlay at 0.55) + const hero = `
`; + const grid = []; + for (let y = 16; y <= 704; y += 4) for (let x = 16; x <= 1264; x += 4) grid.push([x, y]); + const r = await page.evaluate(([h, pts]) => window.captureAndSample(h, pts), [hero, grid]); + check(name, 'label-free background placeholder captured', r.ok === true, r.ok ? '' : r.reason); + if (r.ok) { + let worst = 0; let at = null; + r.samples.forEach((px, i) => { const d = Math.max(...px.map((v, k) => Math.abs(v - [0x10, 0x1a, 0x2b][k]))); if (d > worst) { worst = d; at = grid[i]; } }); + check(name, `label-free background placeholder: all ${grid.length} pixels are the box colour (no ghost label)`, worst <= 2, `(max deviation ${worst} at ${JSON.stringify(at)})`); + } + } for (const t of TALL) { const r = await page.evaluate(([h, pts]) => window.captureAndSample(h, pts), [t.html, t.pts.map((p) => [p[0], p[1]])]); check(name, `${t.label}: captured`, r.ok === true, r.ok ? `(${r.bytes} bytes, ${r.ms} ms)` : r.reason); diff --git a/craft/scripts/e2e-sitesmith-mock-server.mjs b/craft/scripts/e2e-sitesmith-mock-server.mjs index 920d469..e454c1f 100644 --- a/craft/scripts/e2e-sitesmith-mock-server.mjs +++ b/craft/scripts/e2e-sitesmith-mock-server.mjs @@ -188,6 +188,8 @@ export async function createMockServer(o) { /** Queue the result of the next job (a bare response object, or a full {ok,...} result). */ setNextResult(resp, notes) { const r = toResult(resp); if (notes) r.notes = notes; results.push(r); }, clearResults() { results.length = 0; }, + /** Queue a function (start body) => result, for a job whose answer depends on what the editor sent. */ + setNextResultFn(fn) { results.push(fn); }, setPhaseDelayMs(ms) { phaseDelayMs = ms; }, setSlow(v) { slow = !!v; }, setVision(v) { vision = !!v; }, @@ -252,7 +254,8 @@ export async function createMockServer(o) { case 'start': { s.starts.push(body); for (const j of jobs.values()) if (j.status === 'running') return json(res, 409, { ok: false, error: 'job_in_progress', status: 'JOB_IN_PROGRESS', message: 'Sitesmith is already working on a request for this site.', job_id: j.id }); - const job = newJob(); + // a queued function computes the result from the start body (scenario 8n: the real WHP review lockdown) + const job = newJob(typeof results[0] === 'function' ? toResult(results.shift()(body)) : undefined); activeJobId = job.id; history.push(msg('user', String(body.message ?? ''))); return json(res, 200, { ok: true, job_id: job.id, status: 'running', phase: 'queued', created_at: job.created }); diff --git a/craft/scripts/e2e-sitesmith-review-backend.php b/craft/scripts/e2e-sitesmith-review-backend.php new file mode 100644 index 0000000..17daf43 --- /dev/null +++ b/craft/scripts/e2e-sitesmith-review-backend.php @@ -0,0 +1,17 @@ + stdout {summary: server-rebuilt summary | null, alias, ops, dropped} + * (api/sitesmith.php sitesmith_review_rebuild_summary + Sitesmith::reviewFilterOps). Offline, no model call. + * Usage: php e2e-sitesmith-review-backend.php + */ +ob_start(); +define('SITESMITH_API_LIBRARY_ONLY', true); +$dir = rtrim($argv[1], '/'); +require_once $dir . '/web-files/libs/Sitesmith.php'; +require $dir . '/web-files/api/sitesmith.php'; +ob_end_clean(); +$in = json_decode(stream_get_contents(STDIN), true); +$rb = sitesmith_review_rebuild_summary((string)($in['summary'] ?? '')); +$f = $rb === null ? ['ops' => [], 'dropped' => []] : Sitesmith::reviewFilterOps((array)($in['proposed'] ?? []), $rb['alias']); +echo json_encode(['summary' => $rb['summary'] ?? null, 'alias' => $rb['alias'] ?? null, 'ops' => $f['ops'], 'dropped' => (object)$f['dropped']], JSON_UNESCAPED_SLASHES); diff --git a/craft/scripts/e2e-sitesmith.mjs b/craft/scripts/e2e-sitesmith.mjs index 739a405..faf4621 100644 --- a/craft/scripts/e2e-sitesmith.mjs +++ b/craft/scripts/e2e-sitesmith.mjs @@ -25,6 +25,7 @@ import os from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { createMockServer, makePng } from './e2e-sitesmith-mock-server.mjs'; +import { execFileSync } from 'node:child_process'; const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); const distDir = path.resolve(process.env.SITESMITH_E2E_DIST || fs.mkdtempSync(path.join(os.tmpdir(), 'sitesmith-e2e-'))); @@ -388,6 +389,61 @@ scenario('8', 'self-review round trip (one round)', async ({ page, name, ok }) = await modalShot(page, name, '08-self-review-done.png'); }, { selfReview: true }); +// Round 6 (live 2026.11.10): the review summary listed only top-level sections (children of Section/ColumnLayout live +// in Craft linkedNodes), so every proposal was stripped. A NESTED page is built in the real editor, the real +// summarizeCanvas() output goes through the REAL WHP review lockdown (php: sitesmith_review_rebuild_summary + +// Sitesmith::reviewFilterOps, WHP_BACKEND_DIR), the "model" patches the nested heading by its server alias, and the +// edit must take effect on the canvas while a link/text edit, an unknown id and a delete are dropped with reasons. +const WHP_BACKEND_DIR = process.env.WHP_BACKEND_DIR || '/workspace/whp-wt-sitesmith-r6'; +scenario('8n', 'self-review patches a NESTED node through the real WHP review lockdown', async ({ page, name, ok }) => { + const info = []; + // console values as JSON (Firefox prints an object argument as "JSHandle@object" in text()) + page.on('console', async (m) => { if (m.type() === 'info') info.push((await Promise.all(m.args().map((x) => x.jsonValue().catch(() => null)))).map((v) => (typeof v === 'string' ? v : JSON.stringify(v))).join(' ')); }); + srv.setNextResult({ type: 'replace', scope: 'page', message: 'Built it.', pages: [{ name: 'Home', tree: el('Container', { node_id: 'nr-root', style: { padding: '0px' } }, [ + el('Section', { node_id: 'nr-sec', aiName: 'Hero', style: { backgroundColor: '#0f172a', paddingTop: '48px', paddingBottom: '48px' } }, [ + el('Heading', { node_id: 'nr-h', text: 'Nested review heading', level: 'h1', style: { color: '#ffffff', fontSize: '40px' } }), + el('ColumnLayout', { node_id: 'nr-cols', columns: 2 }, [ + el('Container', { node_id: 'nr-c1' }, [el('ButtonLink', { node_id: 'nr-btn', text: 'Nested button', href: '/contact', style: { backgroundColor: '#fde047', color: '#0f172a' } })]), + el('Container', { node_id: 'nr-c2' }, [el('TextBlock', { node_id: 'nr-t', text: 'Second column', style: { color: '#ffffff' } })]), + ]), + ]), + ]) }] }); + let backend = null; + srv.setNextResultFn((body) => { + const alias = (real) => Object.entries(backend?.alias ?? {}).find(([, r]) => r === real)?.[0] ?? 'missing'; + backend = JSON.parse(execFileSync('php', [path.join(root, 'scripts/e2e-sitesmith-review-backend.php'), WHP_BACKEND_DIR], { input: JSON.stringify({ summary: body.canvas_summary, proposed: [] }) }).toString()); + const proposed = [ + { op: 'update_props', node_id: alias('nr-h'), props: { style: { color: '#fde047' } } }, + { op: 'update_props', node_id: alias('nr-btn'), props: { href: 'https://evil.example/x', text: 'Hacked' } }, + { op: 'update_props', node_id: 'n999', props: { style: { color: '#ff0000' } } }, + { op: 'delete_node', node_id: alias('nr-t') }, + ]; + const f = JSON.parse(execFileSync('php', [path.join(root, 'scripts/e2e-sitesmith-review-backend.php'), WHP_BACKEND_DIR], { input: JSON.stringify({ summary: body.canvas_summary, proposed }) }).toString()); + backend = { ...backend, filtered: f }; + return { ok: true, response: { type: 'patch', message: 'Recoloured the hero heading.', ops: f.ops }, review_ops: { proposed: proposed.length, kept: f.ops.length, dropped: f.dropped } }; + }); + await openModal(page); + await sendMsg(page, 'Build my hero and check it'); + await page.waitForSelector('[data-testid=ss-review-note]', { timeout: 45000 }); + const note = (await page.textContent('[data-testid=ss-review-note]')) ?? ''; + const sent = srv.starts[1]?.canvas_summary ?? ''; + ok('the editor summary lists the NESTED nodes (heading, button, column text inside the Section)', /id=nr-h /.test(sent) && /id=nr-btn /.test(sent) && /id=nr-t /.test(sent), sent.slice(0, 600)); + const hLine = (backend?.summary ?? '').split('\n').find((l) => / id=n\d+ \{style\.color=#ffffff, style\.fontSize=40px, level=h1\}$/.test(l)) ?? ''; + ok('the WHP-rebuilt summary shows the heading indented under its Section, with its colour, without its text', /^ {4,}- Heading id=n\d+ /.test(hLine) && !/Nested review heading|Nested button|contact/.test(backend?.summary ?? ''), backend?.summary); + ok('WHP kept exactly the nested heading edit, mapped back to its real id', JSON.stringify(backend?.filtered?.ops) === JSON.stringify([{ op: 'update_props', node_id: 'nr-h', props: { style: { color: '#fde047' } } }]), JSON.stringify(backend?.filtered)); + await page.waitForFunction(() => { + const h = [...document.querySelectorAll('.canvas-device-frame h1')].find((e) => e.textContent.includes('Nested review heading')); + return h && getComputedStyle(h).color === 'rgb(253, 224, 71)'; + }, null, { timeout: 10000 }).then(() => ok('the nested heading really changed colour on the canvas', true), (e) => ok('the nested heading really changed colour on the canvas', false, String(e).slice(0, 120))); + const btn = await page.evaluate(() => { const a = [...document.querySelectorAll('.canvas-device-frame a')].find((e) => /Nested button|Hacked/.test(e.textContent)); return a ? { text: a.textContent.trim(), href: a.getAttribute('href') } : null; }); + ok('the button text and link were NOT changed (forbidden for a review)', btn?.text === 'Nested button' && btn?.href === '/contact', JSON.stringify(btn)); + ok('the column text was NOT deleted', (await canvasText(page)).includes('Second column')); + ok('the note says it made fixes and why the rest were dropped', /made fixes/.test(note) && note.includes('3 did not pass the safety checks (1 named a part that is not on the page, 1 tried to change text, a link or an image, 1 tried to add, remove or replace a block)'), note); + const logLine = info.find((t) => t.startsWith('[sitesmith] self-review dropped ops')) ?? ''; + ok('the console logs the drop counts, without any of the model\'s words', /unknown_id/.test(logLine) && !/evil|Hacked|Recoloured/.test(info.join(' ')), logLine); + await modalShot(page, name, '08n-nested-review.png'); +}, { selfReview: true }); + scenario('8b', 'self-review skipped when the upload fails', async ({ page, ok }) => { srv.setUploadFail(500); srv.setNextResult(pageResult('Kept first pass', { id: 'kp-h' })); diff --git a/craft/src/hooks/useSitesmith.test.ts b/craft/src/hooks/useSitesmith.test.ts index 5139f45..9ed60f5 100644 --- a/craft/src/hooks/useSitesmith.test.ts +++ b/craft/src/hooks/useSitesmith.test.ts @@ -152,6 +152,14 @@ describe('useSitesmith start+poll', () => { expect(bad.reviewOps).toBeUndefined(); }); + test('r6: review_ops.dropped (counts per reason) passes through; unknown reasons and bad counts are ignored', async () => { + script(OKSTART, [done({ ok: true, response: { type: 'ask', question: 'q' }, review_ops: { proposed: 4, kept: 1, dropped: { unknown_id: 2, not_allowed: 1, 'x': 3, bad_value: -1 } } })]); + const res = await sendFor(30000, 'hi', 'canvas'); + expect(res.reviewOps).toEqual({ proposed: 4, kept: 1, dropped: { unknown_id: 2, not_allowed: 1 } }); + script(OKSTART, [done({ ok: true, response: { type: 'ask', question: 'q' }, review_ops: { proposed: 1, kept: 0, dropped: 'x' } })]); + expect((await sendFor(30000, 'hi', 'canvas')).reviewOps).toEqual({ proposed: 1, kept: 0 }); + }); + test('polls back off from ~1s towards a 5s cap', async () => { script(OKSTART, [running()]); vi.useFakeTimers(); diff --git a/craft/src/hooks/useSitesmith.ts b/craft/src/hooks/useSitesmith.ts index fb278e6..70e00f9 100644 --- a/craft/src/hooks/useSitesmith.ts +++ b/craft/src/hooks/useSitesmith.ts @@ -2,7 +2,7 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { useEditorConfig } from '../state/EditorConfigContext'; import { useProjectExtras } from '../state/ProjectExtrasContext'; import { PROFILE_EXTRAS_KEY, isProfileComplete, normalizeProfile, profileToPrompt } from '../utils/sitesmith-profile'; -import { SitesmithSummary, SitesmithMessage, SendResult, SendResultErr, SitesmithStatus, SitesmithClientStatus, toSitesmithStatus } from '../types/sitesmith'; +import { SitesmithSummary, SitesmithMessage, SendResult, SendResultErr, SitesmithStatus, SitesmithClientStatus, toSitesmithStatus, REVIEW_DROP_REASONS, type ReviewOps } from '../types/sitesmith'; function apiBase(apiUrl: string): string { return apiUrl.replace(/site-builder\.php$/, 'sitesmith.php'); @@ -163,6 +163,12 @@ function asSendResult(res: any): SendResult { const count = (v: unknown) => (typeof v === 'number' && Number.isInteger(v) && v >= 0 && v <= 1000 ? v : null); if (ro && typeof ro === 'object' && count(ro.proposed) !== null && count(ro.kept) !== null && ro.kept <= ro.proposed) { out.reviewOps = { proposed: ro.proposed, kept: ro.kept }; + const d = ro.dropped; + if (d && typeof d === 'object' && !Array.isArray(d)) { + const dropped: NonNullable = {}; + for (const k of REVIEW_DROP_REASONS) if (count(d[k]) !== null && d[k] > 0) dropped[k] = d[k]; + if (Object.keys(dropped).length) out.reviewOps.dropped = dropped; + } } return out; } diff --git a/craft/src/panels/sitesmith/SitesmithModal.tsx b/craft/src/panels/sitesmith/SitesmithModal.tsx index c0dd204..6d29cbd 100644 --- a/craft/src/panels/sitesmith/SitesmithModal.tsx +++ b/craft/src/panels/sitesmith/SitesmithModal.tsx @@ -233,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); - return { ok: a.ok, message: a.message, changed: a.changed, ops: a.ops, summary: a.adjustments?.length ? summarizeAdjustments(a.adjustments, a.notChecked) : undefined }; + 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.test.ts b/craft/src/panels/sitesmith/selfReview.test.ts index fc541a8..e161c45 100644 --- a/craft/src/panels/sitesmith/selfReview.test.ts +++ b/craft/src/panels/sitesmith/selfReview.test.ts @@ -183,6 +183,29 @@ describe('runSelfReview: changes dropped by the safety checks', () => { expect(out.note).not.toMatch(/made no changes|Sitesmith said:/); expect(out.note).toContain(`What it proposed (not applied): "${LIVE}"`); }); + test('r6: the note says WHY changes were dropped (counts per reason), and the console gets the counts only', async () => { + const info = vi.spyOn(console, 'info').mockImplementation(() => {}); + const out = await runSelfReview(deps({ review: vi.fn(async () => ({ ok: true as const, response: { type: 'patch' as const, message: LIVE, ops: [] }, + reviewOps: { proposed: 4, kept: 0, dropped: { unknown_id: 2, not_allowed: 1, bad_value: 1 } } })) })); + expect(out.status).toBe('dropped'); + expect(out.note).toContain('Sitesmith proposed 4 changes, but none passed the safety checks (2 named a part that is not on the page, 1 tried to change text, a link or an image, 1 had a value that was invalid or too long), so nothing on your page was changed.'); + const logged = info.mock.calls.find((c) => c[0] === '[sitesmith] self-review dropped ops'); + expect(logged?.[1]).toEqual({ proposed: 4, kept: 0, applied: 0, unknown_id: 2, not_allowed: 1, bad_value: 1 }); + expect(JSON.stringify(info.mock.calls)).not.toContain('Image band'); + info.mockRestore(); + }); + test('r6: ops the editor could not apply are explained too (part not on the page / refused by the editor filter)', async () => { + const info = vi.spyOn(console, 'info').mockImplementation(() => {}); + const ops = [1, 2, 3].map((i) => ({ op: 'update_props' as const, node_id: `n${i}`, props: { style: { color: '#000' } } })); + const out = await runSelfReview(deps({ review: reply(ops, { proposed: 4, kept: 3 }), applyResult: vi.fn(async () => ({ ok: true, changed: 1, ops: 3, dropped: { unknown_id: 1, no_allowed_props: 1 } })) })); + expect(out.status).toBe('fixed'); + expect(out.note).toContain('1 of 3 suggested fixes were applied (1 named a part that is not on the page, 1 had only values the editor refuses).'); + expect(info.mock.calls.find((c) => c[0] === '[sitesmith] self-review dropped ops')?.[1]).toEqual({ proposed: 4, kept: 3, applied: 1, unknown_id: 1, no_allowed_props: 1 }); + const none = await runSelfReview(deps({ review: reply(ops.slice(0, 1)), applyResult: vi.fn(async () => ({ ok: true, changed: 0, ops: 1, dropped: { unknown_id: 1 } })) })); + expect(none.status).toBe('not-applied'); + expect(none.note).toContain('none could be applied (1 named a part that is not on the page)'); + info.mockRestore(); + }); test('singular', async () => { const out = await runSelfReview(deps({ review: reply([], { proposed: 1, kept: 0 }) })); expect(out.note).toContain('Sitesmith proposed 1 change, but it did not pass the safety checks'); diff --git a/craft/src/panels/sitesmith/selfReview.ts b/craft/src/panels/sitesmith/selfReview.ts index 13c8ec1..caf81bf 100644 --- a/craft/src/panels/sitesmith/selfReview.ts +++ b/craft/src/panels/sitesmith/selfReview.ts @@ -27,7 +27,7 @@ export interface SelfReviewDeps { /** Start the follow-up job and wait for it. */ review: (message: string, imageId: string) => Promise; /** Apply the review result (contrast repair included). `changed`/`ops`: patch ops that took effect / were sent. */ - applyResult: (res: SendResult) => Promise<{ ok: boolean; message?: string; summary?: string; changed?: number; ops?: number }>; + applyResult: (res: SendResult) => Promise<{ ok: boolean; message?: string; summary?: string; changed?: number; ops?: number; dropped?: Record }>; removeImage: (id: string) => Promise; resolveAssetUrls: (html: string, urls: Record) => string; } @@ -93,6 +93,29 @@ const quote = (m: string | undefined | null, label = 'Sitesmith said'): string = return ` ${label}: "${t.length > 280 ? t.slice(0, 277) + '...' : t}"`; }; const NOT_APPLIED = 'What it proposed (not applied)'; + +/** Round 6: why ops were dropped, in the owner's words (server review lockdown + editor apply), counts only. */ +const DROP_WORDS: Record = { + unknown_id: 'named a part that is not on the page', + not_allowed: 'tried to change text, a link or an image', + bad_value: 'had a value that was invalid or too long', + not_update_props: 'tried to add, remove or replace a block', + too_many: 'went over the number of changes one review may make', + no_allowed_props: 'had only values the editor refuses', +}; +const DROP_ORDER = ['unknown_id', 'not_allowed', 'bad_value', 'not_update_props', 'too_many', 'no_allowed_props']; +function reasons(d: Record | undefined): string { + if (!d) return ''; + const parts = DROP_ORDER.filter((k) => (d[k] ?? 0) > 0).map((k) => `${d[k]} ${DROP_WORDS[k]}`); + return parts.length ? ` (${parts.join(', ')})` : ''; +} +/** Counts only (never the model's words or values): why each dropped op was dropped. */ +function logDrops(proposed: number, kept: number, applied: number, server?: Record, client?: Record) { + const all: Record = {}; + for (const d of [server, client]) for (const [k, v] of Object.entries(d ?? {})) all[k] = (all[k] ?? 0) + v; + if (!Object.keys(all).length) return; + console.info('[sitesmith] self-review dropped ops', { proposed, kept, applied, ...all }); +} const changes = (n: number) => `${n} ${n === 1 ? 'change' : 'changes'}`; /** One review round, never more. Never throws; any failure is a non-blocking note. */ @@ -130,7 +153,8 @@ async function reviewOnce(d: SelfReviewDeps, onShot: (c: NonNullable ro.kept ? ro.proposed - ro.kept : 0; if (r.type === 'patch' && r.ops.length === 0 && ro && ro.proposed > 0) { - return { status: 'dropped', note: `Self-review: Sitesmith proposed ${changes(ro.proposed)}, but ${ro.proposed === 1 ? 'it did' : 'none'} ${ro.proposed === 1 ? 'not pass' : 'passed'} the safety checks, so nothing on your page was changed.${quote(msg, NOT_APPLIED)}` }; + logDrops(ro.proposed, ro.kept, 0, ro.dropped); + return { status: 'dropped', note: `Self-review: Sitesmith proposed ${changes(ro.proposed)}, but ${ro.proposed === 1 ? 'it did' : 'none'} ${ro.proposed === 1 ? 'not pass' : 'passed'} the safety checks${reasons(ro.dropped)}, so nothing on your page was changed.${quote(msg, NOT_APPLIED)}` }; } if (r.type === 'patch' && r.ops.length === 0) { // Structured flag first (newer backends); the wording check keeps older replies working. @@ -147,13 +171,14 @@ async function reviewOnce(d: SelfReviewDeps, onShot: (c: NonNullable { describe('placehold.co backgrounds are label-free', () => { const LIVE = 'https://placehold.co/1920x1080/101a2b/fbfaf7?text=Team+collaborating+in+a+modern+office'; test('BackgroundSection.bgImage from the live case', () => { - expect(cleanAiProps({ bgImage: LIVE }, 'BackgroundSection').bgImage).toBe('https://placehold.co/1920x1080/101a2b/101a2b'); + expect(cleanAiProps({ bgImage: LIVE }, 'BackgroundSection').bgImage).toBe('https://placehold.co/1920x1080/101a2b/101a2b?text=%E2%80%8B'); }); test('HeroSimple.bgImage, backgroundImageUrl, CallToAction bgValue, ContentSlider slide images', () => { - expect(cleanAiProps({ bgImage: 'https://placehold.co/1600x900?text=Hero' }, 'HeroSimple').bgImage).toBe('https://placehold.co/1600x900/dddddd/dddddd'); - expect(cleanAiProps({ backgroundImageUrl: 'https://placehold.co/1600x900/abc.png?text=x' }, 'Section').backgroundImageUrl).toBe('https://placehold.co/1600x900/aabbcc/aabbcc.png'); - expect(cleanAiProps({ bgType: 'image', bgValue: 'https://placehold.co/1200x600/222222/ffffff?text=Band' }, 'CallToAction').bgValue).toBe('https://placehold.co/1200x600/222222/222222'); - expect((cleanAiProps({ slides: [{ imageSrc: 'https://placehold.co/1200x600/000000/ffffff?text=Slide', heading: 'Hi' }] }, 'ContentSlider').slides as any)[0].imageSrc).toBe('https://placehold.co/1200x600/000000/000000'); + expect(cleanAiProps({ bgImage: 'https://placehold.co/1600x900?text=Hero' }, 'HeroSimple').bgImage).toBe('https://placehold.co/1600x900/dddddd/dddddd?text=%E2%80%8B'); + expect(cleanAiProps({ backgroundImageUrl: 'https://placehold.co/1600x900/abc.png?text=x' }, 'Section').backgroundImageUrl).toBe('https://placehold.co/1600x900/aabbcc/aabbcc.png?text=%E2%80%8B'); + expect(cleanAiProps({ bgType: 'image', bgValue: 'https://placehold.co/1200x600/222222/ffffff?text=Band' }, 'CallToAction').bgValue).toBe('https://placehold.co/1200x600/222222/222222?text=%E2%80%8B'); + expect((cleanAiProps({ slides: [{ imageSrc: 'https://placehold.co/1200x600/000000/ffffff?text=Slide', heading: 'Hi' }] }, 'ContentSlider').slides as any)[0].imageSrc).toBe('https://placehold.co/1200x600/000000/000000?text=%E2%80%8B'); }); test('an ordinary image keeps its label (it tells the owner what photo to source); non-placehold URLs untouched', () => { const img = 'https://placehold.co/800x600/eeeeee/333333?text=Team+photo'; @@ -210,14 +210,19 @@ describe('placehold.co backgrounds are label-free', () => { expect(cleanAiProps({ bgImage: 'assets/hero.jpg' }, 'BackgroundSection').bgImage).toBe('assets/hero.jpg'); expect(cleanAiProps({ bgImage: 'https://images.pexels.com/photos/1/a.jpg?text=x' }, 'BackgroundSection').bgImage).toBe('https://images.pexels.com/photos/1/a.jpg?text=x'); }); + test('round 6 (live case 4): the label is a zero-width space, so placehold.co draws NO glyphs at all (measured: same-colour "W x H" glyphs still antialias into a ghost label; ?text=%E2%80%8B returns a bare )', () => { + const out = cleanAiProps({ bgImage: LIVE }, 'BackgroundSection').bgImage as string; + expect(new URL(out).searchParams.get('text')).toBe('\u200b'); + expect(out.startsWith('https://placehold.co/1920x1080/101a2b/101a2b?')).toBe(true); + }); test('the allow-list still applies first (a disallowed host is still dropped)', () => { expect(cleanAiProps({ bgImage: 'https://evil.example/1920x1080?text=x' }, 'BackgroundSection').bgImage).toBeUndefined(); }); test('an off-spec placehold.co background path loses its query (label) and is otherwise left alone', () => { - expect(cleanAiProps({ bgImage: 'https://placehold.co/1920x1080/navy/white?text=Hi&font=roboto' }, 'BackgroundSection').bgImage).toBe('https://placehold.co/1920x1080/navy/white'); + expect(cleanAiProps({ bgImage: 'https://placehold.co/1920x1080/navy/white?text=Hi&font=roboto' }, 'BackgroundSection').bgImage).toBe('https://placehold.co/1920x1080/navy/white?text=%E2%80%8B'); }); test('patch ops are cleaned the same way', () => { const out = cleanAiResponse({ type: 'patch', message: 'm', ops: [{ op: 'update_props', node_id: 'ai-hero-1', props: { bgImage: LIVE } }] } as any, () => 'BackgroundSection') as any; - expect(out.ops[0].props.bgImage).toBe('https://placehold.co/1920x1080/101a2b/101a2b'); + expect(out.ops[0].props.bgImage).toBe('https://placehold.co/1920x1080/101a2b/101a2b?text=%E2%80%8B'); }); }); diff --git a/craft/src/sitesmith/clean-ai-response.ts b/craft/src/sitesmith/clean-ai-response.ts index 33bde68..8b01cbc 100644 --- a/craft/src/sitesmith/clean-ai-response.ts +++ b/craft/src/sitesmith/clean-ai-response.ts @@ -158,15 +158,20 @@ const hex6 = (h: string) => (h.length === 3 ? h.split('').map((c) => c + c).join * without one) across whatever sits on top, e.g. the hero headline. Background placeholders lose the label: * the query is dropped and the text colour is set to the box colour. Called only on a URL the allow-list * already accepted; anything that is not a placehold.co URL is returned unchanged. + * Round 6 (live case 4): a same-colour "W x H" label still showed as a faint ghost (antialiased glyph edges; + * measured: placehold.co's PNG of /W x H/101a2b/101a2b has 4 distinct colours). The label is now a single + * zero-width space (`?text=%E2%80%8B`), for which placehold.co draws NO glyphs (measured: a bare SVG and a + * 1-colour PNG); the same-colour text stays as a second guard. */ +export const LABEL_FREE_TEXT = '?text=%E2%80%8B'; export function labelFreePlaceholder(url: string): string { let u: URL; try { u = new URL(url); } catch { return url; } if (u.hostname.toLowerCase() !== 'placehold.co') return url; const m = PLACEHOLD_PATH.exec(u.pathname); - if (!m) return `https://placehold.co${u.pathname}`; + if (!m) return `https://placehold.co${u.pathname}${LABEL_FREE_TEXT}`; const bg = m[3] ? hex6(m[3]) : 'dddddd'; - return `https://placehold.co/${m[1]}x${m[2]}/${bg}/${bg}${m[5] ?? ''}`; + return `https://placehold.co/${m[1]}x${m[2]}/${bg}/${bg}${m[5] ?? ''}${LABEL_FREE_TEXT}`; } /** Clean one value under `key`; returns undefined to drop it. */ diff --git a/craft/src/types/sitesmith.ts b/craft/src/types/sitesmith.ts index 177478e..3d518c3 100644 --- a/craft/src/types/sitesmith.ts +++ b/craft/src/types/sitesmith.ts @@ -83,6 +83,10 @@ export interface SendResultOk { ok: true; response: SitesmithResponse; /** Non /** Whole-site builds (newer backends): which planned pages were built / missing. */ siteBuild?: { planned: string[]; built: string[]; missing: string[] }; /** Self-review (newer backends): ops the model proposed vs ops that passed the server's review lockdown. */ - reviewOps?: { proposed: number; kept: number }; } + reviewOps?: ReviewOps; } +/** Why the server's review lockdown dropped ops (WHP Sitesmith::reviewFilterOps), counts only. */ +export const REVIEW_DROP_REASONS = ['not_update_props', 'unknown_id', 'not_allowed', 'bad_value', 'too_many'] as const; +export type ReviewDropReason = typeof REVIEW_DROP_REASONS[number]; +export interface ReviewOps { proposed: number; kept: number; /** Newer backends (round 6): dropped ops per reason. */ dropped?: Partial> } export interface SendResultErr { ok: false; status: SitesmithStatus; message: string; /** The job this failure belongs to (resumable when we only stopped waiting). */ jobId?: string; } export type SendResult = SendResultOk | SendResultErr; diff --git a/craft/src/utils/apply-ai-response.test.ts b/craft/src/utils/apply-ai-response.test.ts index b06e84a..890dab8 100644 --- a/craft/src/utils/apply-ai-response.test.ts +++ b/craft/src/utils/apply-ai-response.test.ts @@ -169,6 +169,21 @@ describe('buildNodeTree', () => { }); describe('applyPatch', () => { + test('round 6: an op for a missing node and an op the cleaner emptied are NOT counted as changes; why is counted (no content)', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { query, actions, propsById } = makeSingleNodeHarness('craft-1', 'ai-node-1', { style: { color: 'red' } }); + const ops: SitesmithPatchOp[] = [ + { op: 'update_props', node_id: 'ai-node-1', props: { style: { color: 'blue' } } }, + { op: 'update_props', node_id: 'ai-gone', props: { style: { color: 'blue' } } }, + { op: 'update_props', node_id: 'ai-node-1', props: {} }, + { op: 'update_props', node_id: 'ai-node-1', props: { node_id: 'x', style: null as any } }, + ]; + const result = __test.applyPatch(actions, query, ops); + expect(result).toEqual({ ok: true, changed: 1, ops: 4, dropped: { unknown_id: 1, no_allowed_props: 2 } }); + expect(propsById['craft-1'].style).toEqual({ color: 'blue' }); + warnSpy.mockRestore(); + }); + test('update_props: does not overwrite node_id, applies a legit prop, merges style shallowly', () => { const { query, actions, propsById } = makeSingleNodeHarness('craft-1', 'ai-node-1', { text: 'Old', diff --git a/craft/src/utils/apply-ai-response.ts b/craft/src/utils/apply-ai-response.ts index 8b2f4f7..57df476 100644 --- a/craft/src/utils/apply-ai-response.ts +++ b/craft/src/utils/apply-ai-response.ts @@ -74,7 +74,12 @@ export interface ApplyOutcome { changed?: number; /** Patch only: how many ops the response carried. */ ops?: number; + /** Patch only (round 6): ops that did nothing, per reason (counts only). */ + dropped?: PatchDropped; } +/** unknown_id: the op names a node that is not on the canvas; no_allowed_props: nothing left to set (the + * allow-list cleaner removed every prop, or only protected/invalid ones were sent). */ +export type PatchDropped = Partial>; /** A canvas load is a setTimeout(0) away; 5 s means something is wrong, not slow. */ const SETTLE_BOUND_MS = 5000; @@ -142,7 +147,7 @@ export function useApplyAiResponse() { const rep = repairForCanvas(resp, design, query); resp = rep.response; adjustments = rep.adjustments; notChecked = rep.notChecked; } - const done = (r: { ok: boolean; message?: string; changed?: number; ops?: number }, canvas?: AppliedCanvas): ApplyOutcome => + const done = (r: { ok: boolean; message?: string; changed?: number; ops?: number; dropped?: PatchDropped }, canvas?: AppliedCanvas): ApplyOutcome => (r.ok ? { ...r, adjustments, notChecked, canvas: canvas ?? liveCanvas(), ...(checks.length ? { checks } : {}) } : r); const liveCanvas = (): AppliedCanvas => { try { return { settled: true, expected: canvasFingerprint(query.serialize()) }; } catch { return { settled: true, expected: null }; } @@ -231,12 +236,15 @@ function applyPatch( actions: any, query: any, ops: SitesmithPatchOp[], -): { ok: boolean; message?: string; changed: number; ops: number } { +): { ok: boolean; message?: string; changed: number; ops: number; dropped?: PatchDropped } { let changed = 0; + const dropped: PatchDropped = {}; + const drop = (why: keyof PatchDropped) => { dropped[why] = (dropped[why] ?? 0) + 1; }; for (const op of ops) { const id = findNodeIdByAiNodeId(query, op.node_id); if (!id) { console.warn('sitesmith patch: node_id not found, skipping op:', op.node_id, op.op); + drop('unknown_id'); continue; } @@ -245,8 +253,13 @@ function applyPatch( const patchProps = op.props; if (!patchProps || typeof patchProps !== 'object' || Array.isArray(patchProps)) { console.warn('sitesmith patch: update_props with invalid props payload, skipping', op.node_id); + drop('no_allowed_props'); break; } + // round 6: an op with nothing left to set (the cleaner emptied it) changes nothing: not a "change" + const settable = Object.entries(patchProps).filter(([k, v]) => + !PROTECTED_UPDATE_PROPS_KEYS.has(k) && !(k === 'style' && (!v || typeof v !== 'object' || Array.isArray(v)))); + if (settable.length === 0) { drop('no_allowed_props'); break; } changed++; actions.setProp(id, (p: any) => { for (const [key, value] of Object.entries(patchProps)) { @@ -332,5 +345,10 @@ function applyPatch( console.warn('sitesmith patch: unknown op', op); } } - return { ok: true, changed, ops: Array.isArray(ops) ? ops.length : 0 }; + const out = { ok: true, changed, ops: Array.isArray(ops) ? ops.length : 0 }; + if (Object.keys(dropped).length) { + console.info('[sitesmith] patch ops did nothing', dropped); + return { ...out, dropped }; + } + return out; } diff --git a/craft/src/utils/canvas-summary.lines.test.ts b/craft/src/utils/canvas-summary.lines.test.ts index 798f7e8..4eb141f 100644 --- a/craft/src/utils/canvas-summary.lines.test.ts +++ b/craft/src/utils/canvas-summary.lines.test.ts @@ -5,11 +5,11 @@ describe('summarizeCanvas: one line per node (self-review summaries are validate test('newlines in text and quotes in names never split or break a line', () => { const state: any = { ROOT: { type: { resolvedName: 'Container' }, props: { tag: 'div' }, nodes: ['a'] }, - a: { type: { resolvedName: 'TextBlock' }, props: { aiName: 'Say "hi"', node_id: 'ai-t1', text: 'Line one\nLine two\r\nthree' }, nodes: [] }, + a: { type: { resolvedName: 'TextBlock' }, props: { aiName: 'Say "hi"', node_id: 'ai-t1', text: 'Line one\nLine two\r\nthree "x"' }, nodes: [] }, }; const lines = summarizeCanvas(state).split('\n'); expect(lines).toHaveLength(2); - expect(lines[1]).toBe(` - TextBlock id=ai-t1 name="Say 'hi'" {text=Line one Line two three}`); + expect(lines[1]).toBe(` - TextBlock id=ai-t1 name="Say 'hi'" {text="Line one Line two three 'x'"}`); const lineRe = /^(?: {2})*- [A-Za-z][A-Za-z0-9]{0,40} id=([A-Za-z0-9_-]{1,64}) name="[^"\n]{0,120}" \{.{0,400}\}$/u; for (const l of lines) expect(l).toMatch(lineRe); }); diff --git a/craft/src/utils/canvas-summary.test.ts b/craft/src/utils/canvas-summary.test.ts index 0651eef..51d48ec 100644 --- a/craft/src/utils/canvas-summary.test.ts +++ b/craft/src/utils/canvas-summary.test.ts @@ -1,9 +1,10 @@ import { describe, test, expect } from 'vitest'; -import { summarizeCanvas } from './canvas-summary'; +import { summarizeCanvas, SUMMARY_MAX_CHARS } from './canvas-summary'; +import { treeToCraftState } from '../state/PageContext'; const fixture = { ROOT: { type: { resolvedName: 'Container' }, props: { aiName: 'Page Root', node_id: 'ai-root-1' }, nodes: ['n1','n2'], parent: null }, - n1: { type: { resolvedName: 'Heading' }, props: { aiName: 'Hero Title', node_id: 'ai-hero-1', text: 'Welcome', level: 1, style: { color: '#fff' } }, nodes: [], parent: 'ROOT' }, + n1: { type: { resolvedName: 'Heading' }, props: { aiName: 'Hero Title', node_id: 'ai-hero-1', text: 'Welcome', level: 1, style: { color: '#fff', transition: 'all 1s' } }, nodes: [], parent: 'ROOT' }, n2: { type: { resolvedName: 'HtmlBlock' }, props: { aiName: 'Custom Embed', node_id: 'ai-html-1', code: '
opaque
' }, nodes: [], parent: 'ROOT' }, }; @@ -13,13 +14,19 @@ describe('summarizeCanvas', () => { expect(out).toContain('Container id=ai-root-1'); expect(out).toContain('Heading id=ai-hero-1 name="Hero Title"'); }); - test('excludes style props', () => { - expect(summarizeCanvas(fixture as any)).not.toContain('color='); + test('key style values are listed as style., other style keys are not', () => { + const out = summarizeCanvas(fixture as any); + expect(out).toContain('style.color="#fff"'); + expect(out).not.toContain('transition'); }); - test('truncates to maxChars', () => { + test('HtmlBlock code is never summarised', () => { + expect(summarizeCanvas(fixture as any)).not.toContain('opaque'); + }); + test('truncates to maxChars at a line boundary', () => { const out = summarizeCanvas(fixture as any, 60); expect(out.length).toBeLessThanOrEqual(60); expect(out).toContain('truncated'); + for (const l of out.split('\n')) expect(l === '… (truncated)' || /\}$/.test(l)).toBe(true); }); }); @@ -34,9 +41,64 @@ describe('summarizeCanvas with inline rich text (#46)', () => { }, }; const out = summarizeCanvas(state as any); - expect(out).toContain('text=Hello World'); - expect(out).toContain('level=h2'); + expect(out).toContain('text="Hello World"'); + expect(out).toContain('level="h2"'); expect(out).not.toContain(''); expect(out).not.toContain('html='); }); }); + +// ---- round 6 (live 2026.11.10): children of Section / ColumnLayout live in linkedNodes and were missing +const el = (name: string, props: Record, nodes: any[] = []) => ({ type: { resolvedName: name }, props, nodes }); +const nestedPage = () => el('Container', { node_id: 'ai-p1-1', style: { padding: '0px' } }, [ + el('Section', { node_id: 'ai-p1-hero', aiName: 'Hero', style: { backgroundColor: '#101a2b', paddingTop: '96px' } }, [ + el('Heading', { node_id: 'ai-p1-h1', aiName: 'Hero heading', text: 'Digital products that "last"', level: 'h1', style: { color: '#ffffff', fontSize: '56px' } }), + el('ColumnLayout', { node_id: 'ai-p1-cols', columns: 2 }, [ + el('Container', { node_id: 'ai-p1-c1' }, [el('TextBlock', { node_id: 'ai-p1-t1', text: 'Left column copy' })]), + el('Container', { node_id: 'ai-p1-c2' }, [el('ButtonLink', { node_id: 'ai-p1-b1', text: 'Get in touch', href: '/contact', style: { backgroundColor: 'var(--wsb-accent)' } })]), + ]), + ]), + el('Section', { node_id: 'ai-p1-brands', aiName: 'Brands' }, [el('Heading', { node_id: 'ai-p1-h2', text: 'Our brands' })]), +]); + +describe('summarizeCanvas walks linkedNodes (round 6)', () => { + const state = JSON.parse(treeToCraftState(nestedPage() as any)); + test('fixture really nests through linkedNodes (Section children are NOT in Section.nodes)', () => { + const sec = Object.values(state).find((n) => n.props?.node_id === 'ai-p1-hero'); + expect(sec.nodes).toEqual([]); + expect(Object.keys(sec.linkedNodes)).toEqual(['section-inner']); + }); + test('every nested node is listed with its node_id, type, text and key style values, nested by indentation', () => { + const out = summarizeCanvas(state); + for (const id of ['ai-p1-hero', 'ai-p1-h1', 'ai-p1-cols', 'ai-p1-c1', 'ai-p1-t1', 'ai-p1-b1', 'ai-p1-brands', 'ai-p1-h2']) expect(out).toContain(`id=${id} `); + const h1 = out.split('\n').find((l) => l.includes('id=ai-p1-h1 '))!; + expect(h1).toMatch(/^ {6}- Heading id=ai-p1-h1 name="Hero heading" \{text="Digital products that 'last'", style\.color="#ffffff", style\.fontSize="56px", level="h1"\}$/); + expect(out).toContain('ButtonLink id=ai-p1-b1 name="" {text="Get in touch", href="/contact", style.backgroundColor="var(--wsb-accent)"}'); + // document order: hero heading before the columns, the brands section last + expect(out.indexOf('ai-p1-h1')).toBeLessThan(out.indexOf('ai-p1-t1')); + expect(out.indexOf('ai-p1-b1')).toBeLessThan(out.indexOf('ai-p1-brands')); + }); + test('every line still matches the server review line check', () => { + const lineRe = /^((?: {2})*)- ([A-Za-z][A-Za-z0-9]{0,40}) id=([A-Za-z0-9_-]{1,64}) name="[^"\n]{0,120}" \{.{0,400}\}$/u; + for (const l of summarizeCanvas(state).split('\n')) expect(l).toMatch(lineRe); + }); + test('budget: detail is dropped before nodes, and nodes are cut at a line boundary', () => { + const big = el('Container', { node_id: 'r' }, Array.from({ length: 25 }, (_, i) => + el('Section', { node_id: `s${i}` }, [el('Heading', { node_id: `h${i}`, text: 'A fairly long heading text that takes room '.repeat(2), style: { color: '#123456', fontSize: '40px', padding: '10px 20px' } })]))); + const st = JSON.parse(treeToCraftState(big as any)); + const full = summarizeCanvas(st, 1_000_000); + expect(full.length).toBeGreaterThan(SUMMARY_MAX_CHARS); + const out = summarizeCanvas(st); + expect(out.length).toBeLessThanOrEqual(SUMMARY_MAX_CHARS); + expect(out).toContain('id=h24 '); // all nodes kept... + expect(out).toContain('text="'); // ...with their text... + expect(full).toContain('takes room A fairly'); + expect(out).not.toContain('takes room A fairly'); // ...but less detail (shorter values) + const tiny = summarizeCanvas(st, 900); + expect(tiny.length).toBeLessThanOrEqual(900); + expect(tiny.endsWith('… (truncated)')).toBe(true); + expect(tiny).not.toContain('text='); + const capped = summarizeCanvas(st, 1_000_000, 10); + expect(capped.split('\n')).toHaveLength(11); + }); +}); diff --git a/craft/src/utils/canvas-summary.ts b/craft/src/utils/canvas-summary.ts index 9985ff7..f3b6e7b 100644 --- a/craft/src/utils/canvas-summary.ts +++ b/craft/src/utils/canvas-summary.ts @@ -1,36 +1,132 @@ import { SerializedNodes } from '@craftjs/core'; -export function summarizeCanvas(state: SerializedNodes, maxChars = 6000): string { +/** + * The canvas as Sitesmith reads it: one line per node, nested by indentation, + * `- id= name="" {key="value", style.color="#fff", ...}` + * + * Round 6 (live 2026.11.10): the walk used to follow only `node.nodes`, so everything inside a Section, + * ColumnLayout or form (their children live in `linkedNodes`) was missing and the self-review saw only the + * top-level section ids -- it could never name the heading it wanted to fix. Now every node reachable through + * `nodes` AND `linkedNodes` (slots in key order, so the order is stable) is listed with its id (props.node_id, + * else the Craft id: both resolve in apply-ai-response findNodeIdByAiNodeId), its type and its key editable + * props: visible text first, then link targets, then the main style values (colours, padding, size), then + * colour-like component props. + * + * BUDGET (documented contract; the server re-checks): at most `maxNodes` lines (default 150) and `maxChars` + * characters (default 6000) in total, each line's {...} part at most 300 characters (the server's review + * check allows 400) and every value at most 60 characters. When the full summary does not fit, detail is + * dropped before nodes: (1) full props, (2) text + style values shortened, (3) text only, (4) structure only; + * only if even (4) does not fit are nodes cut, always at a line boundary, with a final `… (truncated)` line. + * The self-review sends this to WHP api/sitesmith.php, which accepts at most 8000 chars / 200 lines and + * REBUILDS it (sitesmith_review_rebuild_summary): structure, type, server alias, and only validated text + * labels and style values survive there. + */ +export const SUMMARY_MAX_CHARS = 6000; +export const SUMMARY_MAX_NODES = 150; +const LINE_PROPS_MAX = 300; +const VALUE_MAX = 60; + +const TEXT_KEYS = ['text', 'title', 'heading', 'subheading', 'subtitle', 'label', 'buttonText', 'logoText', 'caption', 'alt', 'placeholder']; +const LINK_KEYS = ['href', 'url', 'link', 'buttonHref', 'logoUrl']; +const STYLE_KEYS = ['color', 'backgroundColor', 'background', 'padding', 'paddingTop', 'paddingBottom', 'paddingLeft', 'paddingRight', + 'margin', 'marginTop', 'marginBottom', 'fontSize', 'fontWeight', 'lineHeight', 'textAlign', 'borderRadius', 'gap', 'maxWidth', 'minHeight']; +const COLORISH = /^(?:[a-z][A-Za-z]*(?:Color|Bg)|overlayOpacity|level|textAlign|columns|gap)$/; + +type Detail = 0 | 1 | 2 | 3; // 0 full, 1 text + short styles, 2 text only, 3 structure only + +const flat = (t: string) => t.replace(/[\u0000-\u001f\u007f\u2028\u2029]+/g, ' ').replace(/\s+/g, ' ').trim(); +const quoted = (v: unknown, max: number): string | null => { + if (v == null || typeof v === 'object' || typeof v === 'function') return null; + let s = flat(String(v)).replace(/"/g, "'"); + if (s === '') return null; + if (s.length > max) s = s.slice(0, max - 1) + '…'; + return `"${s}"`; +}; + +function propParts(props: Record, detail: Detail): string[] { + if (detail === 3) return []; + const textMax = detail === 0 ? VALUE_MAX : 40; + const parts: string[] = []; + let texts = 0; + for (const k of TEXT_KEYS) { + if (texts >= 2) break; + const q = quoted(props[k], textMax); + if (q) { parts.push(`${k}=${q}`); texts++; } + } + if (detail === 2) return parts; + if (detail === 0) { + for (const k of LINK_KEYS) { const q = quoted(props[k], VALUE_MAX); if (q) parts.push(`${k}=${q}`); } + if (Array.isArray(props.links)) parts.push(`links="${props.links.length} items"`); + } + const style = props.style && typeof props.style === 'object' && !Array.isArray(props.style) ? props.style : {}; + const styleMax = detail === 0 ? 40 : 24; + for (const k of STYLE_KEYS) { const q = quoted(style[k], styleMax); if (q) parts.push(`style.${k}=${q}`); } + if (detail === 0) { + for (const [k, v] of Object.entries(props)) { + if (k === 'style' || TEXT_KEYS.includes(k) || LINK_KEYS.includes(k) || !COLORISH.test(k)) continue; + const q = quoted(v, 40); + if (q) parts.push(`${k}=${q}`); + } + } + return parts; +} + +interface Row { depth: number; type: string; id: string; name: string; props: Record } + +function lineOf(r: Row, detail: Detail): string { + const parts: string[] = []; + let len = 0; + for (const p of propParts(r.props, detail)) { + if (len + p.length + 2 > LINE_PROPS_MAX) break; + parts.push(p); + len += p.length + 2; + } + return `${' '.repeat(r.depth)}- ${r.type} id=${r.id} name="${r.name}" {${parts.join(', ')}}`; +} + +/** Child ids in document order: `nodes`, then each linked slot in key order. */ +function childIds(node: any): string[] { + const linked = node?.linkedNodes && typeof node.linkedNodes === 'object' ? node.linkedNodes as Record : {}; + return [...(Array.isArray(node?.nodes) ? node.nodes : []), ...Object.keys(linked).sort().map((k) => linked[k])]; +} + +export function summarizeCanvas(state: SerializedNodes, maxChars = SUMMARY_MAX_CHARS, maxNodes = SUMMARY_MAX_NODES): string { const root = state['ROOT']; if (!root) return '(empty canvas)'; - const lines: string[] = []; + const rows: Row[] = []; + const seen = new Set(); const visit = (id: string, depth: number) => { - const node = state[id]; - if (!node) return; - const indent = ' '.repeat(depth); + const node: any = state[id]; + if (!node || seen.has(id)) return; + seen.add(id); const type = typeof node.type === 'object' ? (node.type as any).resolvedName : String(node.type); - const props = node.props || {}; - // One line per node, always: the server validates review summaries line by line - // (api/sitesmith.php sitesmith_review_summary_node_ids), so no newline or quote may leak in. - const flat = (t: string) => t.replace(/[\u0000-\u001f\u007f\u2028\u2029]+/g, ' '); - const aiName = flat(String((props as any).aiName ?? '')).replace(/"/g, "'").slice(0, 100); - const nodeId = (props as any).node_id ?? id; - const interesting: string[] = []; - for (const [k, v] of Object.entries(props)) { - // `html` (#46) is markup for the plain `text` mirror -- summarise text only. - if (k === 'aiName' || k === 'node_id' || k === 'style' || k === 'html') continue; - if (v == null) continue; - const repr = flat(typeof v === 'string' ? v : JSON.stringify(v)); - const truncated = repr.length > 60 ? repr.slice(0, 57) + '…' : repr; - interesting.push(`${k}=${truncated}`); - if (interesting.length >= 3) break; - } - lines.push(`${indent}- ${type} id=${nodeId} name="${aiName}" {${interesting.join(', ')}}`); + const props = (node.props || {}) as Record; + const name = flat(String(props.aiName ?? '')).replace(/"/g, "'").slice(0, 100); + rows.push({ depth, type: String(type), id: String(props.node_id ?? id), name, props }); + // `html` (#46) is markup for the plain `text` mirror and HtmlBlock code is opaque: never summarised if (type === 'HtmlBlock') return; - for (const childId of node.nodes || []) visit(childId, depth + 1); + for (const c of childIds(node)) visit(c, depth + 1); }; visit('ROOT', 0); - let out = lines.join('\n'); - if (out.length > maxChars) out = out.slice(0, maxChars - 30) + '\n… (truncated)'; - return out; + + const MARK = '… (truncated)'; + const capped = rows.length > maxNodes; + const use = capped ? rows.slice(0, maxNodes) : rows; + for (const detail of [0, 1, 2, 3] as Detail[]) { + const lines = use.map((r) => lineOf(r, detail)); + if (capped) lines.push(MARK); + const out = lines.join('\n'); + if (out.length <= maxChars) return out; + } + // even structure-only does not fit: cut whole lines + const out: string[] = []; + let len = 0; + for (const r of use) { + const l = lineOf(r, 3); + if (len + l.length + 1 + MARK.length > maxChars) break; + out.push(l); + len += l.length + 1; + } + out.push(MARK); + return out.join('\n'); } diff --git a/craft/src/utils/capture-placeholder.test.ts b/craft/src/utils/capture-placeholder.test.ts index dbd90a9..fe8137c 100644 --- a/craft/src/utils/capture-placeholder.test.ts +++ b/craft/src/utils/capture-placeholder.test.ts @@ -41,6 +41,22 @@ describe('parsePlaceholdUrl (strict)', () => { }); }); +describe('round 6: label-free placeholders draw no text at all (live case 4 ghost label)', () => { + test('a zero-width-space label (the editor\'s background placeholders) and a same-colour label draw no ', () => { + for (const u of ['https://placehold.co/1920x1080/101a2b/101a2b?text=%E2%80%8B', 'https://placehold.co/1920x1080/101a2b/101a2b', 'https://placehold.co/1920x1080/101a2b/fbfaf7?text=%E2%80%8B%E2%80%8B']) { + const d = placeholderDataUrl(u)!; + const svg = svgOf(d); + expect(svg).not.toContain(' { + expect(svgOf(placeholderDataUrl('https://placehold.co/800x600/101a2b/fbfaf7?text=Team')!)).toContain('>Team'); + expect(parsePlaceholdUrl('https://placehold.co/800x600/101a2b/fbfaf7?text=%20')).toMatchObject({ text: '800 × 600' }); + }); +}); + describe('placeholderDataUrl', () => { test('a local SVG of the image size, bg fill and escaped label', () => { const d = placeholderDataUrl('https://placehold.co/800x1000/1f3a5f/fbfaf7?text=%3Cscript%3Ealert(1)%3C/script%3E+%26+"q"')!; diff --git a/craft/src/utils/capture-placeholder.ts b/craft/src/utils/capture-placeholder.ts index 659d5d1..59111ac 100644 --- a/craft/src/utils/capture-placeholder.ts +++ b/craft/src/utils/capture-placeholder.ts @@ -19,6 +19,7 @@ export type Placehold = | { kind: 'label'; w: number; h: number; bg: string; fg: string; text: string } | { kind: 'neutral'; w: number; h: number }; +const ZERO_WIDTH = /[\u200b-\u200f\u2060-\u2064\ufeff\u00ad]/g; const DEFAULT_BG = '#cccccc'; const DEFAULT_FG = '#969696'; const MAX_TEXT = 200; @@ -45,8 +46,15 @@ export function parsePlaceholdUrl(raw: string): Placehold | null { if (!w || !h) return { kind: 'neutral', w: 0, h: 0 }; const m = STRICT_PATH.exec(u.pathname); if (u.protocol !== 'https:' || u.username || u.password || u.port || !m) return { kind: 'neutral', w, h }; - const text = (u.searchParams.get('text') ?? '').replace(/[\u0000-\u001f\u007f-\u009f]+/g, ' ').replace(/\s+/g, ' ').trim().slice(0, MAX_TEXT); - return { kind: 'label', w, h, bg: hex(m[3], DEFAULT_BG), fg: hex(m[4], DEFAULT_FG), text: text || `${w} × ${h}` }; + const rawText = u.searchParams.get('text') ?? ''; + const text = rawText.replace(/[\u0000-\u001f\u007f-\u009f]+/g, ' ').replace(/\s+/g, ' ').trim().slice(0, MAX_TEXT); + const bg = hex(m[3], DEFAULT_BG); + const fg = hex(m[4], DEFAULT_FG); + // Round 6 (live case 4 ghost label): a label of only zero-width characters (the editor's label-free background + // placeholders, ?text=%E2%80%8B) draws nothing on placehold.co, and a label in the box colour must not be drawn + // either (antialiased glyph edges show as a ghost): no element at all. + const invisible = (text !== '' && text.replace(ZERO_WIDTH, '') === '') || fg === bg; + return { kind: 'label', w, h, bg, fg, text: invisible ? '' : (text || `${w} × ${h}`) }; } const esc = (s: string) => s.replace(/&/g, '&').replace(//g, '>').replace(/"/g, '"').replace(/'/g, ''');