sitesmith self-review: never present a claimed fix that sent no ops
An empty patch whose message claims a change ("Fixed the hero ...") is now
status 'claimed-no-ops' with a warning note: "Sitesmith's reply describes a fix,
but it sent no changes, so nothing on your page was changed." The structured
render_mismatch flag from newer backends is read first; the existing wording
check still covers older replies.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
4c1c8db97e
commit
648d6a3a51
4 files changed
+66
-4
No files matched your search
@@ -453,7 +453,7 @@ export const SitesmithModal: React.FC<Props> = ({ onClose, target }) => {
|
||||
{reviewNote && (
|
||||
<div
|
||||
role="note"
|
||||
style={{ ...(reviewNote.status === 'mismatch' || reviewNote.status === 'not-applied' ? warnBox : noteStyle), marginBottom: 8 }}
|
||||
style={{ ...(reviewNote.status === 'mismatch' || reviewNote.status === 'not-applied' || reviewNote.status === 'claimed-no-ops' ? warnBox : noteStyle), marginBottom: 8 }}
|
||||
data-testid="ss-review-note"
|
||||
data-status={reviewNote.status}
|
||||
>{reviewNote.note}</div>
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, test, expect, vi } from 'vitest';
|
||||
import { runSelfReview, saysRenderMismatch, REVIEW_MESSAGE, type SelfReviewDeps } from './selfReview';
|
||||
import { runSelfReview, saysRenderMismatch, claimsAFix, REVIEW_MESSAGE, type SelfReviewDeps } from './selfReview';
|
||||
|
||||
const blob = () => new Blob([new Uint8Array(3000)], { type: 'image/jpeg' });
|
||||
const deps = (o: Partial<SelfReviewDeps> = {}): SelfReviewDeps => ({
|
||||
@@ -120,4 +120,41 @@ describe('runSelfReview', () => {
|
||||
await runSelfReview(d);
|
||||
expect((d.capture as any).mock.calls[0][0]).toContain('/api/a');
|
||||
});
|
||||
// Live eval 2026-11-01 (ols-home-firefox): the model said "Fixed the hero" but sent ops: [].
|
||||
const CLAIMS = [
|
||||
'Fixed the hero: it rendered on a near-white background, so the headline, subtitle and outline button were almost invisible. It now sits on the navy brand colour with white text. The rest of the page reads fine, so I left it alone.',
|
||||
'I darkened the hero overlay and increased the card padding.',
|
||||
'Changed the CTA button to the accent colour.',
|
||||
];
|
||||
test.each(CLAIMS)('a reply that claims a fix but sends no ops is reported honestly, never as fixed (%#)', async (message) => {
|
||||
const d = deps({ review: vi.fn(async () => ({ ok: true as const, response: { type: 'patch' as const, message, ops: [] } })) });
|
||||
const out = await runSelfReview(d);
|
||||
expect(out.status).toBe('claimed-no-ops');
|
||||
expect(out.note).toMatch(/sent no changes/);
|
||||
expect(out.note).toMatch(/nothing on your page was changed/i);
|
||||
expect(d.applyResult).not.toHaveBeenCalled();
|
||||
});
|
||||
test('claim detection stays narrow: "no fixes needed" style replies are not claims', () => {
|
||||
for (const m of [
|
||||
'No fixes needed: contrast, spacing, alignment and the mobile-stacking column setup all look right. The hero photo is still a grey placeholder, so upload a team photo to replace it.',
|
||||
'The page already looks right, so no changes. Text contrast holds on the dark bands and the red band, the show cards are even, spacing follows one rhythm, and the icons and form render properly.',
|
||||
'Nothing needed to be changed.',
|
||||
'I have not changed anything; it looks right.',
|
||||
'I made no changes because everything reads well.',
|
||||
'Changed nothing, the page looks right.',
|
||||
'',
|
||||
]) expect(claimsAFix(m)).toBe(false);
|
||||
});
|
||||
test('structured render_mismatch flag wins over wording (and an old reply without it still works)', async () => {
|
||||
const d = deps({ review: vi.fn(async () => ({ ok: true as const, response: { type: 'patch' as const, message: 'Screenshot mismatch: the hero is navy on the page I built.', ops: [], render_mismatch: true } })) });
|
||||
const out = await runSelfReview(d);
|
||||
expect(out.status).toBe('mismatch');
|
||||
expect(out.note).toMatch(/screenshot/i);
|
||||
const d2 = deps({ review: vi.fn(async () => ({ ok: true as const, response: { type: 'patch' as const, message: 'Looks right.', ops: [] } })) });
|
||||
expect((await runSelfReview(d2)).status).toBe('fine');
|
||||
});
|
||||
test('render_mismatch with ops still applies the ops (the flag never blocks real fixes)', async () => {
|
||||
const d = deps({ review: vi.fn(async () => ({ ok: true as const, response: { type: 'patch' as const, message: 'm', ops: [{ op: 'delete_node' as const, node_id: 'n' }], render_mismatch: true } })) });
|
||||
expect((await runSelfReview(d)).status).toBe('fixed');
|
||||
});
|
||||
});
|
||||
@@ -37,9 +37,10 @@ export interface SelfReviewDeps {
|
||||
* - `fine`: the review ran on the right page and Sitesmith changed nothing.
|
||||
* - `mismatch`: Sitesmith said the screenshot was not the page it built, so it changed nothing.
|
||||
* - `not-applied`: Sitesmith proposed changes but none of them could be applied.
|
||||
* - `claimed-no-ops`: Sitesmith's words claim a fix but it sent no changes (nothing changed; say so).
|
||||
* - `skipped`: the review did not run, or failed (the note says why).
|
||||
*/
|
||||
export type SelfReviewStatus = 'fixed' | 'fine' | 'mismatch' | 'not-applied' | 'skipped';
|
||||
export type SelfReviewStatus = 'fixed' | 'fine' | 'mismatch' | 'not-applied' | 'claimed-no-ops' | 'skipped';
|
||||
export interface SelfReviewOutcome { status: SelfReviewStatus; note: string }
|
||||
|
||||
const skip = (why: string): SelfReviewOutcome => ({ status: 'skipped', note: `Self-review skipped: ${why}. Your page was built as normal.` });
|
||||
@@ -62,6 +63,22 @@ export function saysRenderMismatch(message: string | undefined | null): boolean
|
||||
return !!message && MISMATCH_RES.some((re) => re.test(message));
|
||||
}
|
||||
|
||||
/**
|
||||
* The model's words claim it changed something ("Fixed the hero", "I darkened the overlay"). Only
|
||||
* consulted when it sent NO ops, so a claim can never be shown as a fix that did not happen
|
||||
* (eval 2026-11-01). Judged per clause; a negation before the verb in the same clause ("no
|
||||
* changes", "nothing needed to be changed", "have not changed") is not a claim.
|
||||
*/
|
||||
const CLAIM_VERB = /\b(?:fixed|changed|updated|adjusted|moved|increased|decreased|reduced|darkened|lightened|recolou?red|swapped|replaced|switched|tightened|enlarged|added|removed|corrected|improved|raised|lowered|gave|made)\b|\bnow\s+(?:sits|uses|has|reads|shows|is|are)\b/i;
|
||||
const NEGATION = /\b(?:no|not|nothing|never|without|n't|none)\b|n't\b/i;
|
||||
export function claimsAFix(message: string | undefined | null): boolean {
|
||||
if (!message) return false;
|
||||
return message.split(/[.!?;:\n]+/).some((clause) => {
|
||||
const m = CLAIM_VERB.exec(clause);
|
||||
return !!m && !NEGATION.test(clause.slice(0, m.index)) && !/^\s+(?:no|nothing|none)\b/i.test(clause.slice(m.index + m[0].length));
|
||||
});
|
||||
}
|
||||
|
||||
const quote = (m: string | undefined | null): string => {
|
||||
const t = (m ?? '').trim().replace(/\s+/g, ' ');
|
||||
if (!t) return '';
|
||||
@@ -90,9 +107,16 @@ export async function runSelfReview(d: SelfReviewDeps): Promise<SelfReviewOutcom
|
||||
}
|
||||
const msg = 'message' in r ? r.message : '';
|
||||
if (r.type === 'patch' && r.ops.length === 0) {
|
||||
// Structured flag first (newer backends); the wording check keeps older replies working.
|
||||
if (r.render_mismatch === true) {
|
||||
return { status: 'mismatch', note: `Self-review: Sitesmith said the screenshot did not match the page it built (screenshot mismatch), so it changed nothing. The screenshot can be imperfect; your page is as built, so check it yourself.${quote(msg)}` };
|
||||
}
|
||||
if (saysRenderMismatch(msg)) {
|
||||
return { status: 'mismatch', note: `Self-review did not work this time: Sitesmith said the screenshot did not match the page it built, so it changed nothing. Your page is as built; check it yourself.${quote(msg)}` };
|
||||
}
|
||||
if (claimsAFix(msg)) {
|
||||
return { status: 'claimed-no-ops', note: `Self-review: Sitesmith's reply describes a fix, but it sent no changes, so nothing on your page was changed. Check the part it mentions yourself.${quote(msg)}` };
|
||||
}
|
||||
return { status: 'fine', note: `Self-review: Sitesmith looked at a screenshot of the finished page and made no changes.${quote(msg)}` };
|
||||
}
|
||||
const applied = await d.applyResult(res);
|
||||
|
||||
@@ -71,7 +71,8 @@ export type SitesmithResponse =
|
||||
header?: { tree: SerializedTreeNode };
|
||||
footer?: { tree: SerializedTreeNode };
|
||||
message: string; }
|
||||
| { type: 'patch'; ops: SitesmithPatchOp[]; message: string; }
|
||||
/** render_mismatch: self-review only -- the model says the screenshot contradicts the page it built (newer backends; absent = false). */
|
||||
| { type: 'patch'; ops: SitesmithPatchOp[]; message: string; render_mismatch?: boolean; }
|
||||
| { type: 'ask'; question: string; options?: string[]; };
|
||||
|
||||
export interface SendResultOk { ok: true; response: SitesmithResponse; /** Non-blocking server messages (e.g. images skipped). */ notes?: string[]; }
|
||||
|
||||
Reference in new issue
Block a user