fix(sitesmith): summarize nested (linkedNodes) nodes, explain dropped review ops, truly label-free bg placeholders
1. summarizeCanvas() walked only node.nodes, so everything inside Section/ColumnLayout/forms (linkedNodes) was missing and live self-reviews could only name top-level sections. It now walks nodes + linked slots (stable key order) and lists id, type and key props (text first, links, main style values as style.<key>, colour-like props) as key="value", within a documented budget (150 nodes, 6000 chars, 300 chars of props per line; detail is dropped before nodes; cuts happen at line boundaries -- a mid-line cut used to make the server refuse the whole review). HtmlBlock code is no longer summarised. applyPatch no longer counts an op with nothing left to set as a change; ops that did nothing are counted per reason. The self-review note and console say why ops were dropped (server and editor reasons, counts only). Real-browser test 8n builds a nested page, sends the real summary through the real WHP review lockdown (php) and checks the nested edit takes effect while link/text edits, unknown ids and deletes are dropped. 3. Background placeholders use ?text=%E2%80%8B (measured: placehold.co then draws no glyphs; the same-colour label left a 4-colour ghost in its PNG); the capture draws no <text> for a zero-width or same-colour label. Pixel test on the case-4 hero markup in both browsers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185ukXgk6UunFCKyXqLe2Cq
This commit is contained in:
1 parent
615a20e25b
commit
4177e5015a
20 files changed
+449
-58
No files matched your search
@@ -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. */
|
||||
|
||||
@@ -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 = `<!doctype html><html><head><style>body{margin:0;background:#fff}</style></head><body><section style="position:relative;width:100%;min-height:200px;background-color:#ff0000;background-image:url('${bgUrl}');background-size:cover;background-position:center;padding-top:160px;padding-bottom:160px;padding-left:24px;padding-right:24px;text-align:left"><div style="position:absolute;inset:0;background-color:#101a2b;opacity:0.55"></div><div style="position:relative;height:400px"></div></section></body></html>`;
|
||||
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);
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -0,0 +1,17 @@
|
||||
<?php
|
||||
/**
|
||||
* Used by scripts/e2e-sitesmith.mjs (scenario 8n): runs the REAL WHP review lockdown on what the editor sent.
|
||||
* stdin {summary, proposed:[ops with alias ids]} -> 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 <backend repo dir>
|
||||
*/
|
||||
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);
|
||||
@@ -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' }));
|
||||
|
||||
@@ -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, '<b>x</b>': 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();
|
||||
|
||||
@@ -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<ReviewOps['dropped']> = {};
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -233,7 +233,7 @@ export const SitesmithModal: React.FC<Props> = ({ 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),
|
||||
});
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -27,7 +27,7 @@ export interface SelfReviewDeps {
|
||||
/** Start the follow-up job and wait for it. */
|
||||
review: (message: string, imageId: string) => Promise<SendResult>;
|
||||
/** 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<string, number> }>;
|
||||
removeImage: (id: string) => Promise<unknown>;
|
||||
resolveAssetUrls: (html: string, urls: Record<string, string>) => 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<string, string> = {
|
||||
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<string, number> | 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<string, number>, client?: Record<string, number>) {
|
||||
const all: Record<string, number> = {};
|
||||
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<SelfReviewO
|
||||
const ro = res.reviewOps;
|
||||
const dropped = ro && ro.proposed > 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<SelfReviewO
|
||||
}
|
||||
const applied = await d.applyResult(res);
|
||||
if (!applied.ok) return skip(applied.message || 'the fixes could not be applied');
|
||||
if (r.type === 'patch') logDrops(ro?.proposed ?? r.ops.length, ro?.kept ?? r.ops.length, applied.changed ?? r.ops.length, ro?.dropped, applied.dropped);
|
||||
if (r.type === 'patch' && applied.changed === 0) {
|
||||
return { status: 'not-applied', note: `Self-review: Sitesmith suggested ${r.ops.length === 1 ? 'a fix' : `${r.ops.length} fixes`}, but none could be applied (the parts it named are not on the page), so nothing changed.${quote(msg, NOT_APPLIED)}` };
|
||||
return { status: 'not-applied', note: `Self-review: Sitesmith suggested ${r.ops.length === 1 ? 'a fix' : `${r.ops.length} fixes`}, but none could be applied${reasons(applied.dropped) || ' (the parts it named are not on the page)'}, so nothing changed.${quote(msg, NOT_APPLIED)}` };
|
||||
}
|
||||
const extra = applied.summary ? ` ${applied.summary}.` : '';
|
||||
const partial = r.type === 'patch' && applied.changed !== undefined && applied.changed < r.ops.length
|
||||
? ` ${applied.changed} of ${r.ops.length} suggested fixes were applied; the rest named parts that are not on the page.` : '';
|
||||
const drop = dropped ? ` Sitesmith proposed ${changes(ro!.proposed)}; ${dropped} did not pass the safety checks and ${dropped === 1 ? 'was' : 'were'} dropped.` : '';
|
||||
? ` ${applied.changed} of ${r.ops.length} suggested fixes were applied${reasons(applied.dropped) ? `${reasons(applied.dropped)}.` : '; the rest named parts that are not on the page.'}` : '';
|
||||
const drop = dropped ? ` Sitesmith proposed ${changes(ro!.proposed)}; ${dropped} did not pass the safety checks${reasons(ro!.dropped)} and ${dropped === 1 ? 'was' : 'were'} dropped.` : '';
|
||||
return { status: 'fixed', note: `Self-review: Sitesmith looked at a screenshot of the finished page and made fixes.${drop}${partial}${extra} Use Undo to go back.${quote(msg, dropped ? "Sitesmith's description (it may include the dropped changes)" : 'Sitesmith said')}` };
|
||||
} catch (e) {
|
||||
return skip(e instanceof Error && e.message ? e.message.slice(0, 100) : 'something went wrong');
|
||||
|
||||
@@ -196,13 +196,13 @@ describe('cssValueAllowed: positive grammar (review finding 2)', () => {
|
||||
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 <rect>)', () => {
|
||||
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');
|
||||
});
|
||||
});
|
||||
@@ -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 <rect> 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. */
|
||||
|
||||
@@ -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<Record<ReviewDropReason, number>> }
|
||||
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;
|
||||
@@ -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',
|
||||
|
||||
@@ -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<Record<'unknown_id' | 'no_allowed_props', number>>;
|
||||
|
||||
/** 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;
|
||||
}
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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: '<div>opaque</div>' }, 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.<key>, 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('<b>');
|
||||
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<string, unknown>, 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<any>(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);
|
||||
});
|
||||
});
|
||||
@@ -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,
|
||||
* `<indent>- <Type> id=<node_id> name="<aiName>" {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<string, any>, 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<string, any> }
|
||||
|
||||
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<string, string> : {};
|
||||
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<string>();
|
||||
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<string, any>;
|
||||
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');
|
||||
}
|
||||
@@ -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 <text>', () => {
|
||||
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('<text');
|
||||
expect(svg).toContain('fill="#101a2b"');
|
||||
expect(isPlaceholderDataUrl(d)).toBe(true);
|
||||
}
|
||||
});
|
||||
test('an ordinary label is still drawn; an empty or blank ?text= still shows W x H (as placehold.co does)', () => {
|
||||
expect(svgOf(placeholderDataUrl('https://placehold.co/800x600/101a2b/fbfaf7?text=Team')!)).toContain('>Team</text>');
|
||||
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"')!;
|
||||
|
||||
@@ -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 <text> 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, '"').replace(/'/g, ''');
|
||||
|
||||
Reference in new issue
Block a user