From 6d5f78eebf8b6b63257915efa1458539d858b16a Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Thu, 3 Sep 2026 23:52:56 -0700 Subject: [PATCH] Live: the loader now hands off when the resume is the arrival The overlay could sit in its generating shader over a DOM that already held all three variants, and only a page refresh cleared it (#719). The server's generation preflight runs live-wrap with --defer-source-write, so the wrapper and every variant reach the DOM in a single HMR batch. The deferred-wrapper scout is constructed at init and the variant MutationObserver at Go; observer callbacks run in construction order, so on that batch the scout resumes first and resumeSession, not the observer, is the transition into CYCLING. It set the state and the bar but never called hideShaderOverlay(), so the frozen capture of the original stayed painted over the variants. It also reported browser_resumed, which does not count as publication progress, and then disconnected and re-created the observer, dropping the records that observer had already queued for the same batch, so variants_ready never fired at all. resumeSession now finishes the same transition the observer does (shader down, inline edit off, insert session finalized, params panel rebuilt) and reports variants_ready when it already holds every variant. The deferred scout names itself in the journal as browser_resumed_deferred_wrapper, so the two resume paths are no longer indistinguishable. Wrapper resolution goes through findVariantsWrapper, which prefers a wrapper that actually holds non-original variants. A target inside a .map() renders one wrapper per item, and an agent that relocates the wrapper out of the shared primitive live-wrap scaffolded leaves an empty one behind; first match could pin either and strand the session at 0/N. With zero or one match this is the querySelector it replaces. Tests: waitForCycling now asserts the generating shader is gone once the bar cycles, across every runtime fixture (it failed on vite8-react-plain before this change and passes after), marked no-retry so the reload recovery cannot hide it. Source-shape tests pin the transition, the variants_ready report, and the wrapper preference. Co-Authored-By: Claude Code Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY --- skill/scripts/live-browser.js | 70 +++++++++++++++++++++++++----- tests/live-browser-source.test.mjs | 62 ++++++++++++++++++++++++-- tests/live-e2e/preactions.mjs | 5 ++- tests/live-e2e/ui.mjs | 30 +++++++++++++ 4 files changed, 151 insertions(+), 16 deletions(-) diff --git a/skill/scripts/live-browser.js b/skill/scripts/live-browser.js index c32101b12..10353bc95 100644 --- a/skill/scripts/live-browser.js +++ b/skill/scripts/live-browser.js @@ -6907,6 +6907,27 @@ // MutationObserver for progressive variant reveal // + // A session id can have more than one wrapper in the DOM: the target may sit + // inside a `.map()` callback (the wrapper renders once per item), or the + // agent may have relocated the wrapper out of the shared primitive live-wrap + // scaffolded into. A plain first match can then pin an empty scaffold while + // the real variants sit in a later wrapper, which strands the session at + // 0/N. Prefer a wrapper that actually holds variants. With zero or one match + // this is exactly the querySelector it replaces. + function findVariantsWrapper(sessionId) { + const selector = sessionId + ? '[data-impeccable-variants="' + sessionId + '"]' + : '[data-impeccable-variants]'; + const matches = document.querySelectorAll(selector); + if (matches.length < 2) return matches[0] || null; + for (const candidate of matches) { + if (candidate.querySelector('[data-impeccable-variant]:not([data-impeccable-variant="original"])')) { + return candidate; + } + } + return matches[0]; + } + function startVariantObserver(sessionId) { let updating = false; // re-entrancy guard @@ -6936,7 +6957,7 @@ } if (!dominated) return; - const wrapper = document.querySelector('[data-impeccable-variants="' + sessionId + '"]'); + const wrapper = findVariantsWrapper(sessionId); if (!wrapper) return; const variants = wrapper.querySelectorAll('[data-impeccable-variant]:not([data-impeccable-variant="original"])'); @@ -9059,7 +9080,7 @@ void main() { } function restoreFromActiveSessions(activeSessions, reason) { - const wrapper = document.querySelector('[data-impeccable-variants]'); + const wrapper = findVariantsWrapper(null); if (wrapper && !isFrameworkComponentPreviewMode(wrapper.dataset.impeccablePreview)) return false; if (svelteComponentSession?.sessionId === currentSessionId) return false; return restoreSessionWithoutWrapper(reason || 'sse_connected', activeSessions); @@ -9400,8 +9421,13 @@ void main() { return restoreSessionWithoutWrapper('browser_resumed_over_handled_wrapper'); } - function resumeSession(recoveryRevision = liveInteractionRevision) { - const wrapper = document.querySelector('[data-impeccable-variants]'); + function resumeSession(recoveryRevision = liveInteractionRevision, opts = {}) { + // Which path resumed matters in the journal: an init resume is a fresh + // page load, the deferred-wrapper scout is a mid-page-load arrival. Both + // used to log the same `browser_resumed`, which made issue #719 take a + // DOM reconstruction to diagnose. + const resumeReason = opts.reason || 'browser_resumed'; + const wrapper = findVariantsWrapper(null); const runtimeWrapper = wrapper || document.querySelector('[data-impeccable-carbonize]'); if (restoreSessionSupersedingHandledWrapper(runtimeWrapper)) return true; if (scheduleHandledRuntimeWrapperReload(runtimeWrapper, recoveryRevision)) return false; @@ -9500,16 +9526,38 @@ void main() { showBar(state === 'CYCLING' ? 'cycling' : 'generating'); startScrollTracking(); - // Build the params panel for the restored visible variant. Previously - // this was missed on page-reload resume: showVariantInDOM above fires - // refreshParamsPanel, but state was still IDLE at that moment so it - // hid. Now that state is CYCLING, re-fire. - if (state === 'CYCLING') refreshParamsPanel(); + // A resume can BE the arrival, not just a re-entry after one. The server's + // generation preflight runs live-wrap with --defer-source-write, so the + // wrapper and every variant reach the DOM in one HMR batch, and the + // deferred-wrapper scout (constructed at init) runs before the variant + // MutationObserver (constructed at Go) on that batch. Finish the same + // transition the observer would have finished. Without hideShaderOverlay + // the generating shader stays frozen over the target and the session looks + // stuck at GENERATING while the bar already cycles (issue #719). + if (state === 'CYCLING') { + recoveryWaitingForAnchor = false; + hideShaderOverlay(); + if (isInsert) finalizeInsertSession(); + disableInlineEdit(); + // Build the params panel for the restored visible variant. Previously + // this was missed on page-reload resume: showVariantInDOM above fires + // refreshParamsPanel, but state was still IDLE at that moment so it + // hid. Now that state is CYCLING, re-fire. + refreshParamsPanel(); + } saveSession(); if (arrivedVariants > 0 && arrivedVariants < expectedVariants) { sendCheckpoint('variants_progress'); } else { - queueCheckpoint('browser_resumed'); + queueCheckpoint(resumeReason); + // Only variants_progress and variants_ready count as publication + // progress. When the resume is the arrival, the observer never gets to + // report it (this function disconnects and re-creates it below, which + // drops the records it had already queued for this same batch), so + // without this the server never learns the variants were published. + if (arrivedVariants > 0 && arrivedVariants >= expectedVariants && expectedVariants > 0) { + sendCheckpoint('variants_ready'); + } } // Start observing for more variants AFTER initial setup @@ -12831,7 +12879,7 @@ void main() { const wrapper = document.querySelector('[data-impeccable-variants],[data-impeccable-carbonize]'); if (!wrapper) return; scout.disconnect(); - if (resumeSession(deferredResumeRevision)) { + if (resumeSession(deferredResumeRevision, { reason: 'browser_resumed_deferred_wrapper' })) { console.log('[impeccable] Resumed deferred session ' + currentSessionId + ' (post-hydration).'); } }); diff --git a/tests/live-browser-source.test.mjs b/tests/live-browser-source.test.mjs index 0617806f8..f2aaa0b46 100644 --- a/tests/live-browser-source.test.mjs +++ b/tests/live-browser-source.test.mjs @@ -470,7 +470,7 @@ describe('live-browser source contracts', () => { assert.match(recovery, /if \(staleWrapper\) location\.reload\(\);/); assert.match( SOURCE, - /function resumeSession\(recoveryRevision = liveInteractionRevision\)[\s\S]{0,250}?\[data-impeccable-carbonize\][\s\S]{0,180}?scheduleHandledRuntimeWrapperReload\(runtimeWrapper, recoveryRevision\)/, + /function resumeSession\(recoveryRevision = liveInteractionRevision, opts = \{\}\)[\s\S]{0,700}?\[data-impeccable-carbonize\][\s\S]{0,180}?scheduleHandledRuntimeWrapperReload\(runtimeWrapper, recoveryRevision\)/, 'resume must inspect handled carbonize wrappers before clearing handled state', ); assert.match( @@ -526,11 +526,67 @@ describe('live-browser source contracts', () => { ); assert.match( SOURCE, - /const deferredResumeRevision = liveInteractionRevision;[\s\S]{0,350}?const scout = new MutationObserver[\s\S]{0,350}?resumeSession\(deferredResumeRevision\)/, - 'the deferred-wrapper scout must retain its originating interaction revision', + /const deferredResumeRevision = liveInteractionRevision;[\s\S]{0,350}?const scout = new MutationObserver[\s\S]{0,400}?resumeSession\(deferredResumeRevision, \{ reason: 'browser_resumed_deferred_wrapper' \}\)/, + 'the deferred-wrapper scout must retain its originating interaction revision and name itself in the journal', ); }); + it('finishes the cycling transition when the resume is the arrival (#719)', () => { + // The server's generation preflight runs live-wrap with + // --defer-source-write, so the wrapper and every variant reach the DOM in + // one HMR batch. The deferred-wrapper scout is constructed at init, the + // variant MutationObserver at Go, and observer callbacks run in + // construction order, so on that batch the scout resumes first and + // resumeSession IS the transition into CYCLING. It has to finish the same + // transition the observer would have: leaving the generating shader up + // paints a frozen capture of the original over a DOM that already holds + // the variants, which is the stuck loader from issue #719. + const resumeStart = SOURCE.indexOf('function resumeSession('); + const resumeEnd = SOURCE.indexOf('\n //', resumeStart); + const resume = SOURCE.slice(resumeStart, resumeEnd); + assert.match( + resume, + /if \(state === 'CYCLING'\) \{[\s\S]{0,200}?hideShaderOverlay\(\);/, + 'a resume into CYCLING must take the generating shader down', + ); + assert.match( + resume, + /if \(state === 'CYCLING'\) \{[\s\S]{0,700}?refreshParamsPanel\(\);/, + 'a resume into CYCLING must still rebuild the params panel', + ); + // Only variants_progress|variants_ready count as publication progress, so + // a resume that already holds every variant has to report one of them or + // the server never learns the generation was published. + assert.match( + resume, + /queueCheckpoint\(resumeReason\);[\s\S]{0,500}?sendCheckpoint\('variants_ready'\)/, + 'a complete resume must report variants_ready, not only browser_resumed', + ); + }); + + it('prefers the wrapper that actually holds variants over the first match (#719)', () => { + // A target inside a `.map()` renders one wrapper per item, and an agent + // that relocates the wrapper out of the shared primitive live-wrap + // scaffolded leaves an empty one behind. First match can then pin a + // scaffold with no variants and strand the session at 0/N. + const start = SOURCE.indexOf('function findVariantsWrapper(sessionId)'); + assert.ok(start > 0, 'findVariantsWrapper must exist'); + const helper = SOURCE.slice(start, SOURCE.indexOf('\n function startVariantObserver(', start)); + assert.match(helper, /if \(matches\.length < 2\) return matches\[0\] \|\| null;/); + assert.match( + helper, + /candidate\.querySelector\('\[data-impeccable-variant\]:not\(\[data-impeccable-variant="original"\]\)'\)[\s\S]{0,80}?return candidate;/, + 'the preferred wrapper is the one holding non-original variants', + ); + assert.match(helper, /return matches\[0\];/, 'with no populated wrapper the old first match still wins'); + for (const caller of [ + 'const wrapper = findVariantsWrapper(sessionId);', + 'const wrapper = findVariantsWrapper(null);', + ]) { + assert.ok(SOURCE.includes(caller), `${caller} should be how the resolvers look a wrapper up`); + } + }); + it('invalidates nullable deferred recovery as soon as a replacement edit starts configuring', () => { assert.match( SOURCE, diff --git a/tests/live-e2e/preactions.mjs b/tests/live-e2e/preactions.mjs index 87ddd76c7..9c7a7af9f 100644 --- a/tests/live-e2e/preactions.mjs +++ b/tests/live-e2e/preactions.mjs @@ -93,7 +93,8 @@ export async function waitForCyclingRobust(page, expectedCount, opts = {}) { try { await waitForCycling(page, expectedCount, { timeout: firstPassTimeoutMs }); return; - } catch { + } catch (preErr) { + if (preErr?.impeccableNoRetry) throw preErr; log(`Cycling not reached in ${firstPassTimeoutMs}ms — retracing preActions`); await runPreActions(page, preActions); } @@ -106,7 +107,7 @@ export async function waitForCyclingRobust(page, expectedCount, opts = {}) { if (process.env.IMPECCABLE_E2E_DEBUG) { firstErr.message += '\n\n--- live UI snapshot ---\n' + JSON.stringify(await liveUiSnapshot(page), null, 2); } - if (agentMode !== 'llm') throw firstErr; + if (agentMode !== 'llm' || firstErr?.impeccableNoRetry) throw firstErr; } log('Cycling not reached after LLM generate — reloading to pick up HMR'); diff --git a/tests/live-e2e/ui.mjs b/tests/live-e2e/ui.mjs index be02bd669..7b2bc259f 100644 --- a/tests/live-e2e/ui.mjs +++ b/tests/live-e2e/ui.mjs @@ -605,6 +605,35 @@ export async function clickGo(page) { throw lastErr || new Error('Go click did not leave configure mode'); } +/** + * The generating shader is the visible loader: a frozen capture of the + * original painted over the target while variants are being written. Every + * transition into CYCLING has to take it down, so a cycling bar with the + * shader still up is the stuck loader from issue #719, not a cosmetic + * detail. The transition is synchronous, so a short window is generous. + */ +async function assertGeneratingShaderCleared(page) { + const stillUp = await page + .waitForFunction( + () => !(window.__impeccableLiveQuery || document.querySelector.bind(document))('#impeccable-live-shader'), + undefined, + { timeout: 5_000 }, + ) + .then(() => false) + .catch(() => true); + if (stillUp) { + const err = new Error( + 'CYCLING reached but the generating shader overlay (#impeccable-live-shader) is still up: ' + + 'the loader never handed off to the variant cycler', + ); + // Cycling was reached, so the recovery paths in waitForCyclingRobust + // (retrace preActions, reload) have nothing to recover and a reload would + // hide the defect by taking the shader down with the page. + err.impeccableNoRetry = true; + throw err; + } +} + /** * Wait for the bar to enter CYCLING state — happens after the agent's * variants land in the DOM via HMR and the MutationObserver counts them. @@ -636,6 +665,7 @@ export async function waitForCycling(page, expectedCount, { timeout = 30_000 } = { barSel: BAR_ID, expected: expectedCount }, { timeout }, ); + await assertGeneratingShaderCleared(page); } catch (err) { if (process.env.IMPECCABLE_E2E_DEBUG) { const snapshot = await page.evaluate((barSel) => {