mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-12 14:16:28 +03:00
fix: preview-truth CSS supersession + cascade ordering on Svelte accept
Field failure from a real Codex session: accepting a variant into Pitch.svelte appended 23 selectors and removed none, so the source's old .decisions grid rules re-attached through the kept root class and forced the accepted board into a stale three-column layout; some appended base rules also landed after the source's media block, weakening the mobile cascade. Two mechanical fixes: - Preview truth: the scaffolder records the seeded selectors (the source rules that styled the replaced selection, which the isolated preview never applied). On accept, any seeded selector the variant does not re-declare is removed; the selector-loss postcondition treats those removals like compiler prunes. A regression test reproduces the exact Pitch shape end to end. - Cascade order: reconciliation inserts new base rules BEFORE existing top-level media blocks instead of appending after them. Init-latency reductions from the same transcript: - live.mjs inlines the resolved surface brief (removes three surface-brief.mjs round-trips including a --help miss before first poll). - The wrap/scaffold payload carries componentStubMarkup, and live.md instructs editing stubs in place (the session read the manifest + stub back and then deleted/recreated the files). - live.md notes that a busy default port usually means the dev server is already running (the session spawned a duplicate). This work was produced with AI assistance (Claude Code). Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Code
parent
031e170d3e
commit
5b6b331785
@@ -2,7 +2,7 @@ Interactive live variant mode: select elements in the browser, pick a design act
|
||||
|
||||
## Prerequisites
|
||||
|
||||
A running dev server with hot module replacement (Vite, Next.js, Bun, etc.), OR a static HTML file open in the browser.
|
||||
A running dev server with hot module replacement (Vite, Next.js, Bun, etc.), OR a static HTML file open in the browser. If starting the dev server fails because its default port is busy, the app is very likely ALREADY running (a previous session's server); probe the default URL before spawning a second server on another port.
|
||||
|
||||
<codex>
|
||||
Codex: run live helper commands, the app dev server, and any dependency-installing setup with `sandbox_permissions: "require_escalated"` from the start; live mode depends on localhost and package-manager network access that the sandbox blocks.
|
||||
@@ -165,7 +165,7 @@ If `--text` matches multiple candidates equally well, wrap exits with `{ error:
|
||||
|
||||
Output on success: `{ file, insertLine, commentSyntax, styleMode, styleTag, cssSelectorPrefixExamples, cssAuthoring }`. On source-preview targets it also returns `sourceWritten: false`, `wrapperBlock`, `replaceStartLine`, and `replaceEndLine` (write it yourself per the `event.scaffold` note above). When you run this command directly (no preflight scaffold), it writes the wrapper into source itself, so there is no `wrapperBlock` and you splice variants at `insertLine`.
|
||||
|
||||
For Svelte/SvelteKit targets, `live-wrap.mjs` returns `previewMode: "svelte-component"` with `file` pointing at a temporary `node_modules/.impeccable-live/<id>/manifest.json`, `componentDir` pointing at the variant component files, and `sourceFile` pointing at the real `.svelte` route. The scaffold is AST-based: control-flow blocks (`{#each}`, `{#if}`) survive intact, a free each-collection crosses the contract as ONE structured prop (kind `collection`), and expressions bound by the loop stay verbatim in the stub. Write each variant as a real Svelte component (`v1.svelte`, `v2.svelte`, …) under `componentDir`, keeping the stub's control flow and `propContract` prop names; never flatten a loop into literal items. Put variant CSS in each component's `<style>` block with semantic class selectors (no `@scope`, no `data-impeccable-*`). Reply with `--file` set to the manifest path; the browser dynamically imports and mounts the compiled components so Svelte HMR does not reset page state while the user cycles variants. On Accept, `live-accept.mjs` merges the accepted component back into `sourceFile` mechanically: markup restored to route expressions, CSS reconciled into the existing `<style>` block (matching selectors replaced, superseded rules removed via the compiler's unused-selector pass), params baked from `params.json`, indentation preserved. Nothing is appended twice; you have no post-accept cleanup on this path.
|
||||
For Svelte/SvelteKit targets, `live-wrap.mjs` returns `previewMode: "svelte-component"` with `file` pointing at a temporary `node_modules/.impeccable-live/<id>/manifest.json`, `componentDir` pointing at the variant component files, and `sourceFile` pointing at the real `.svelte` route. The scaffold is AST-based: control-flow blocks (`{#each}`, `{#if}`) survive intact, a free each-collection crosses the contract as ONE structured prop (kind `collection`), and expressions bound by the loop stay verbatim in the stub. The scaffold payload includes `componentStubMarkup` (the prop-substituted markup already written into every stub), so do not spend tool calls reading the manifest or stub files back. EDIT `v1.svelte`, `v2.svelte`, ... in place; never delete and recreate them. Keep the stub's control flow and `propContract` prop names; never flatten a loop into literal items. The stub `<style>` arrives seeded with the source rules that currently style the selection; restyle or delete them freely. On accept, any seeded rule your variant does not re-declare is REMOVED from the source (the preview never applied it, so the user approved a design without it); rules you keep or re-declare are merged normally. Put variant CSS in each component's `<style>` block with semantic class selectors (no `@scope`, no `data-impeccable-*`). Reply with `--file` set to the manifest path; the browser dynamically imports and mounts the compiled components so Svelte HMR does not reset page state while the user cycles variants. On Accept, `live-accept.mjs` merges the accepted component back into `sourceFile` mechanically: markup restored to route expressions, CSS reconciled into the existing `<style>` block (matching selectors replaced, superseded rules removed via the compiler's unused-selector pass), params baked from `params.json`, indentation preserved. Nothing is appended twice; you have no post-accept cleanup on this path.
|
||||
|
||||
When the selected markup contains constructs a detached preview cannot support (component tags, `bind:`/`use:` directives, await blocks, inline scripts, spread attributes), wrap returns the normal source-preview wrapper instead, with `previewFallback: { from: "svelte-component", reason }`. Just follow the returned wrapper shape; the fallback trades HMR state resets for correctness.
|
||||
|
||||
|
||||
@@ -436,6 +436,7 @@ The agent should insert variant HTML at insertLine.`);
|
||||
replaceEndLine: deferredWrapper ? deferredWrapper.replaceEndLine : undefined,
|
||||
componentDir: componentSession?.componentDir,
|
||||
propContract: componentSession?.propContract,
|
||||
componentStubMarkup: componentSession?.stubMarkup,
|
||||
sourceStartLine: componentPreviewActive ? startLine + 1 : undefined,
|
||||
sourceEndLine: componentPreviewActive ? endLine + 1 : undefined,
|
||||
startLine: outputStartLine, // 1-indexed for the agent
|
||||
|
||||
+18
-1
@@ -24,6 +24,7 @@ import { fileURLToPath } from 'node:url';
|
||||
import { resolveTargetSelection } from './context.mjs';
|
||||
import { resolveFiles } from './live-inject.mjs';
|
||||
import { readLiveServerInfo } from './lib/impeccable-paths.mjs';
|
||||
import { resolveSurfaceBrief } from './lib/surface-briefs.mjs';
|
||||
import { resolveLiveTarget } from './live-target.mjs';
|
||||
import { resolveRoots, writeRootsManifest } from './live/roots.mjs';
|
||||
|
||||
@@ -161,7 +162,20 @@ The agent should then:
|
||||
const resolvedFiles = resolveFiles(activeCwd, checkResult.config);
|
||||
const drift = scanForDrift(activeCwd, resolvedFiles, checkResult.config);
|
||||
|
||||
// 5. Emit everything the agent needs
|
||||
// 5. Emit everything the agent needs. The surface brief rides along so the
|
||||
// agent does not spend three more tool calls (and a --help miss) on
|
||||
// surface-brief.mjs before the first poll.
|
||||
let surfaceBrief = null;
|
||||
let surfaceBriefPath = null;
|
||||
try {
|
||||
const resolvedBrief = resolveSurfaceBrief(roots.appRoot, liveTarget.absoluteTargetPath || null);
|
||||
if (resolvedBrief?.brief) {
|
||||
surfaceBrief = resolvedBrief.brief.text ?? safeRead(resolvedBrief.brief.path);
|
||||
surfaceBriefPath = resolvedBrief.brief.path
|
||||
? path.relative(liveTarget.originalCwd, resolvedBrief.brief.path)
|
||||
: null;
|
||||
}
|
||||
} catch { /* briefs are optional context */ }
|
||||
console.log(JSON.stringify({
|
||||
ok: true,
|
||||
serverPort: serverInfo.port,
|
||||
@@ -179,6 +193,9 @@ The agent should then:
|
||||
hasDesign: !!design,
|
||||
design,
|
||||
designPath: relOrNull(liveTarget.originalCwd, roots.designPath),
|
||||
hasSurfaceBrief: !!surfaceBrief,
|
||||
surfaceBrief,
|
||||
surfaceBriefPath,
|
||||
}, null, 2));
|
||||
}
|
||||
|
||||
|
||||
@@ -247,8 +247,15 @@ export function reconcileCss(existingCss, variantCss) {
|
||||
}
|
||||
touched.add(key);
|
||||
} else {
|
||||
existingNodes.push({ ...node });
|
||||
index.set(key, existingNodes[existingNodes.length - 1]);
|
||||
// New base rules go BEFORE the existing top-level media blocks:
|
||||
// appended after them, an equal-specificity base rule wins the
|
||||
// cascade over the stylesheet's earlier responsive overrides and
|
||||
// silently weakens the mobile styles for any still-shared class.
|
||||
const appendedNode = { ...node };
|
||||
const firstAt = existingNodes.findIndex((n) => n.type === 'at' && n.children);
|
||||
if (firstAt === -1) existingNodes.push(appendedNode);
|
||||
else existingNodes.splice(firstAt, 0, appendedNode);
|
||||
index.set(key, appendedNode);
|
||||
touched.add(key);
|
||||
appended++;
|
||||
}
|
||||
|
||||
@@ -211,6 +211,12 @@ export function scaffoldSvelteComponentSession({
|
||||
safeReadSource(path.resolve(cwd, sourceFile)),
|
||||
originalMarkup,
|
||||
);
|
||||
// The preview compiles in isolation, so NONE of these source rules applied
|
||||
// to what the user approved. Accept enforces that preview truth: any of
|
||||
// them the variant does not re-declare is superseded and removed, instead
|
||||
// of re-attaching to the accepted markup through kept class names (the
|
||||
// ".decisions grid grabs the new board" failure).
|
||||
const seededSelectors = [...collectAllSelectors(seededCss)];
|
||||
|
||||
const manifest = {
|
||||
id,
|
||||
@@ -222,6 +228,7 @@ export function scaffoldSvelteComponentSession({
|
||||
count,
|
||||
propContract: contract,
|
||||
originalMarkup,
|
||||
seededSelectors,
|
||||
componentDir: path.relative(cwd, dir).split(path.sep).join('/'),
|
||||
// Absolute paths let the browser fall back to /@fs/ imports when the dev
|
||||
// server's base or root makes root-relative URLs miss, and probe whether
|
||||
@@ -247,6 +254,11 @@ export function scaffoldSvelteComponentSession({
|
||||
manifestFile: path.relative(cwd, path.join(dir, 'manifest.json')).split(path.sep).join('/'),
|
||||
componentDir: manifest.componentDir,
|
||||
propContract: contract,
|
||||
// Inlined so the generate event's scaffold payload carries the stub
|
||||
// shape; the agent edits vN.svelte in place instead of spending reads on
|
||||
// the manifest and stub files (or deleting and recreating them).
|
||||
stubMarkup: analysis.markupWithProps,
|
||||
seededCss,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -682,7 +694,7 @@ export function inlineSvelteComponentAccept(manifest, variantNum, paramValues =
|
||||
variantCss = sanitizeAcceptedSvelteCss(cssLines, variantNum, paramValues, rootTag).join('\n');
|
||||
}
|
||||
const bakedCss = bakeParamValues(variantCss, declaredParams, paramValues || {});
|
||||
const cssStats = { replaced: 0, appended: 0, pruned: [] };
|
||||
const cssStats = { replaced: 0, appended: 0, pruned: [], superseded: [] };
|
||||
if (bakedCss.trim()) {
|
||||
const merged = mergeCssIntoSvelteSource(newLines.join('\n'), bakedCss);
|
||||
newLines = merged.text.split('\n');
|
||||
@@ -691,6 +703,22 @@ export function inlineSvelteComponentAccept(manifest, variantNum, paramValues =
|
||||
}
|
||||
|
||||
let finalText = newLines.join('\n');
|
||||
|
||||
// Preview truth: the detached preview never applied the source rules that
|
||||
// styled the replaced selection, so the user approved a design without
|
||||
// them. Any seeded selector the variant did not re-declare is superseded;
|
||||
// left in place it re-attaches through kept class names (the accepted root
|
||||
// keeps its original classes) and re-layouts markup it no longer owns.
|
||||
const incomingSelectors = collectAllSelectors(bakedCss);
|
||||
const superseded = (manifest.seededSelectors || [])
|
||||
.map((selector) => normalizeSelector(selector))
|
||||
.filter((selector) => selector && !incomingSelectors.has(selector));
|
||||
if (superseded.length > 0) {
|
||||
const scrubbed = removeSelectorsFromSvelteSource(finalText, new Set(superseded));
|
||||
finalText = scrubbed.text;
|
||||
cssStats.superseded = scrubbed.removed;
|
||||
}
|
||||
|
||||
if (compiler) {
|
||||
const pruned = pruneUnusedSelectors(finalText, compiler.compile, { skipSelectors: preUnused });
|
||||
finalText = pruned.source;
|
||||
@@ -698,10 +726,13 @@ export function inlineSvelteComponentAccept(manifest, variantNum, paramValues =
|
||||
}
|
||||
|
||||
// Postcondition: no selector from the user's pre-accept CSS may vanish
|
||||
// unless the compiler-driven prune deliberately removed it. This turns any
|
||||
// parser or reconciler defect into a loud refusal instead of silent damage
|
||||
// to a hand-written style block.
|
||||
const lostSelectors = findLostSelectors(sourceContent, finalText, cssStats.pruned);
|
||||
// unless the compiler-driven prune or the preview-truth supersession
|
||||
// deliberately removed it. This turns any parser or reconciler defect into
|
||||
// a loud refusal instead of silent damage to a hand-written style block.
|
||||
const lostSelectors = findLostSelectors(sourceContent, finalText, [
|
||||
...cssStats.pruned,
|
||||
...cssStats.superseded,
|
||||
]);
|
||||
if (lostSelectors.length > 0) {
|
||||
return {
|
||||
handled: false,
|
||||
@@ -746,6 +777,50 @@ function styleBlockText(sourceText) {
|
||||
return match ? match[1] : '';
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove every rule whose (normalized) selector list is fully contained in
|
||||
* `selectors` from the component's style block, at any at-rule nesting depth.
|
||||
* Rules that mix doomed and surviving selectors keep the survivors.
|
||||
*/
|
||||
export function removeSelectorsFromSvelteSource(sourceText, selectors) {
|
||||
const text = String(sourceText || '');
|
||||
const styleRe = /<style\b[^>]*>([\s\S]*?)<\/style\s*>/gi;
|
||||
let lastMatch = null;
|
||||
let m;
|
||||
while ((m = styleRe.exec(text))) lastMatch = m;
|
||||
if (!lastMatch) return { text, removed: [] };
|
||||
|
||||
const removed = [];
|
||||
const transform = (nodes) => {
|
||||
const kept = [];
|
||||
for (const node of nodes) {
|
||||
if (node.type === 'rule') {
|
||||
const survivors = [];
|
||||
for (const selector of splitSelectorList(node.prelude)) {
|
||||
if (selectors.has(normalizeSelector(selector))) removed.push(normalizeSelector(selector));
|
||||
else survivors.push(selector);
|
||||
}
|
||||
if (survivors.length > 0) kept.push({ ...node, prelude: survivors.join(', ') });
|
||||
} else if (node.type === 'at' && node.children) {
|
||||
const children = transform(node.children);
|
||||
if (children.length > 0) kept.push({ ...node, children });
|
||||
} else {
|
||||
kept.push(node);
|
||||
}
|
||||
}
|
||||
return kept;
|
||||
};
|
||||
|
||||
const nodes = transform(parseStylesheet(lastMatch[1]));
|
||||
if (removed.length === 0) return { text, removed };
|
||||
const openTag = lastMatch[0].slice(0, lastMatch[0].indexOf('>') + 1);
|
||||
const rebuilt = `${openTag}\n${serializeNodes(nodes).split('\n').map((l) => (l.trim() ? ' ' + l : '')).join('\n')}\n</style>`;
|
||||
return {
|
||||
text: text.slice(0, lastMatch.index) + rebuilt + text.slice(lastMatch.index + lastMatch[0].length),
|
||||
removed,
|
||||
};
|
||||
}
|
||||
|
||||
export function findLostSelectors(beforeSource, afterSource, prunedSelectors = []) {
|
||||
const before = collectAllSelectors(styleBlockText(beforeSource));
|
||||
const after = collectAllSelectors(styleBlockText(afterSource));
|
||||
|
||||
@@ -6,6 +6,7 @@ import { tmpdir } from 'node:os';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import {
|
||||
extractMatchingSourceCss,
|
||||
removeSelectorsFromSvelteSource,
|
||||
findSvelteComponentManifest,
|
||||
inlineSvelteComponentAccept,
|
||||
mergeCssIntoSvelteSource,
|
||||
@@ -230,3 +231,125 @@ describe('svelte component scaffold + accept pipeline', () => {
|
||||
assert.doesNotMatch(css, /\.footer/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('review regressions: preview-truth supersession (the Pitch mangle)', () => {
|
||||
const PITCH_SOURCE = `<script>
|
||||
let verdicts = [
|
||||
{ label: 'True positive', detail: 'Fix it' },
|
||||
{ label: 'False positive', detail: 'Dismiss it' },
|
||||
];
|
||||
</script>
|
||||
|
||||
<section class="pitch">
|
||||
<div class="decisions">
|
||||
{#each verdicts as verdict}
|
||||
<div class="cell">
|
||||
<h3>{verdict.label}</h3>
|
||||
<p>{verdict.detail}</p>
|
||||
</div>
|
||||
{/each}
|
||||
</div>
|
||||
</section>
|
||||
|
||||
<style>
|
||||
.pitch { padding: 40px; }
|
||||
.decisions {
|
||||
display: grid;
|
||||
grid-template-columns: repeat(3, 1fr);
|
||||
gap: 12px;
|
||||
}
|
||||
.decisions > .cell { border: 1px solid #333; }
|
||||
@media (max-width: 700px) {
|
||||
.decisions { grid-template-columns: 1fr; }
|
||||
.pitch { padding: 16px; }
|
||||
}
|
||||
</style>
|
||||
`;
|
||||
|
||||
it('removes seeded rules the variant did not re-declare and orders new base rules before media blocks', () => {
|
||||
const tmp2 = realpathSync(mkdtempSync(join(tmpdir(), 'impeccable-pitch-mangle-')));
|
||||
try {
|
||||
mkdirSync(join(tmp2, 'node_modules'), { recursive: true });
|
||||
try {
|
||||
symlinkSync(join(REPO_NODE_MODULES, 'svelte'), join(tmp2, 'node_modules', 'svelte'), 'dir');
|
||||
} catch {
|
||||
cpSync(join(REPO_NODE_MODULES, 'svelte'), join(tmp2, 'node_modules', 'svelte'), { recursive: true });
|
||||
}
|
||||
write(tmp2, 'package.json', JSON.stringify({ name: 'app' }));
|
||||
write(tmp2, 'src/lib/Pitch.svelte', PITCH_SOURCE);
|
||||
|
||||
// Picked element: the .decisions block (lines 10-17, 1-indexed).
|
||||
const lines = PITCH_SOURCE.split('\n');
|
||||
const startLine = lines.findIndex((l) => l.includes('class="decisions"')) + 1;
|
||||
const endLine = lines.findIndex((l, i) => i >= startLine && l.trim() === '</div>' && lines[i + 1]?.includes('</section>')) + 1;
|
||||
const originalLines = lines.slice(startLine - 1, endLine);
|
||||
|
||||
const session = scaffoldSvelteComponentSession({
|
||||
id: 'pitchm1',
|
||||
count: 1,
|
||||
sourceFile: 'src/lib/Pitch.svelte',
|
||||
sourceStartLine: startLine,
|
||||
sourceEndLine: endLine,
|
||||
originalLines,
|
||||
cwd: tmp2,
|
||||
});
|
||||
assert.equal(session.fallback, undefined, session.reason);
|
||||
// Seeded selectors recorded for accept-time supersession.
|
||||
assert.equal(session.manifest.seededSelectors.includes('.decisions'), true);
|
||||
|
||||
// The agent's variant: a NEW class, no re-declaration of .decisions.
|
||||
write(tmp2, join(session.componentDir, 'v1.svelte'), `<script>
|
||||
let { verdicts = [] } = $props();
|
||||
</script>
|
||||
|
||||
<div class="disposition-board">
|
||||
{#each verdicts as verdict}
|
||||
<div class="lane">
|
||||
<h3>{verdict.label}</h3>
|
||||
<p>{verdict.detail}</p>
|
||||
</div>
|
||||
{/each}
|
||||
</div>
|
||||
|
||||
<style>
|
||||
.disposition-board { display: flex; flex-direction: column; gap: 8px; }
|
||||
.disposition-board .lane { border-left: 3px solid #7df; padding: 8px 12px; }
|
||||
@media (max-width: 700px) {
|
||||
.disposition-board .lane { padding: 6px 8px; }
|
||||
}
|
||||
</style>
|
||||
`);
|
||||
const manifest = findSvelteComponentManifest('pitchm1', tmp2);
|
||||
const result = inlineSvelteComponentAccept(manifest, 1, null, tmp2);
|
||||
assert.equal(result.handled, true, result.error);
|
||||
const out = readFileSync(join(tmp2, 'src/lib/Pitch.svelte'), 'utf-8');
|
||||
|
||||
// The superseded grid rules are GONE: they never applied in the
|
||||
// preview the user approved, and the root keeps the old class.
|
||||
assert.doesNotMatch(out, /grid-template-columns: repeat\(3, 1fr\)/);
|
||||
assert.doesNotMatch(out, /\.decisions > \.cell/);
|
||||
assert.equal(result.css.superseded.includes('.decisions'), true);
|
||||
// The untouched sibling rule survives.
|
||||
assert.match(out, /\.pitch \{ padding: 40px; \}/);
|
||||
// Source media block survives for the surviving class...
|
||||
assert.match(out, /\.pitch \{ padding: 16px; \}/);
|
||||
// ...and no longer carries the superseded selector.
|
||||
assert.doesNotMatch(out, /\.decisions \{ grid-template-columns: 1fr; \}/);
|
||||
// New base rules sit BEFORE the source's @media block (cascade order).
|
||||
const baseIdx = out.indexOf('.disposition-board {');
|
||||
const mediaIdx = out.indexOf('@media (max-width: 700px)');
|
||||
assert.equal(baseIdx > -1 && mediaIdx > -1 && baseIdx < mediaIdx, true,
|
||||
`expected base rules before media, got base@${baseIdx} media@${mediaIdx}`);
|
||||
assert.equal(result.verify.clean, true, JSON.stringify(result.verify.findings));
|
||||
} finally {
|
||||
rmSync(tmp2, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps seeded rules the variant re-declares', () => {
|
||||
const { text, removed } = removeSelectorsFromSvelteSource('<div class="a">x</div>\n<style>\n .a { color: red; }\n .b { color: blue; }\n</style>', new Set(['.b']));
|
||||
assert.match(text, /\.a \{ color: red; \}/);
|
||||
assert.doesNotMatch(text, /color: blue/);
|
||||
assert.deepEqual(removed, ['.b']);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user