Skip to content

Commit c7be723

Browse files
tylerslatonclaude
andcommitted
fix(showcase): harden beautiful-chat toggleTheme against page-close races
Two related changes targeting the fc=97 'Target page closed' D5 failure on `beautiful-chat-toggle-theme`: 1. Probe (`_beautiful-chat-shared.ts:assertToggleTheme`) — replace the manual 200ms `page.evaluate` poll over 30s with Playwright's `waitForFunction`. The native polling is event-driven inside the browser context, disconnects cleanly on page-close, and avoids ~150 round-trip evaluates per probe. The catch branch now checks `page.isClosed()` first and surfaces a diagnostic message naming the renderer-crash case explicitly, so operators don't have to guess what 'Target page closed' meant. 2. Demo (`use-generative-ui-examples.tsx`) — drop `[theme, setTheme]` from the `useFrontendTool` deps array. The handler reads `document` directly and the setter is stable across renders, so the deps array forced a re-registration after every theme flip. That race could collide with an in-flight tool result and surface as a renderer error during multi-turn sequences. Removing deps keeps the tool registration stable for the duration of the conversation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 471eb88 commit c7be723

2 files changed

Lines changed: 80 additions & 20 deletions

File tree

showcase/harness/src/probes/scripts/_beautiful-chat-shared.ts

Lines changed: 65 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -209,14 +209,72 @@ export function asClickablePage(
209209
*/
210210
export async function assertToggleTheme(page: ConversationPage): Promise<void> {
211211
const initiallyDark = await readIsHtmlDark(page);
212-
const deadline = Date.now() + 30_000;
213-
while (Date.now() < deadline) {
214-
if ((await readIsHtmlDark(page)) !== initiallyDark) return;
215-
await new Promise((r) => setTimeout(r, 200));
212+
// Use `waitForFunction` over a manual page.evaluate poll because:
213+
// 1. Playwright drives the polling internally and disconnects
214+
// cleanly if the page closes (no "Target page closed" leak —
215+
// historically this assertion accumulated fc=97 in production
216+
// from page-close races during multi-turn renders).
217+
// 2. Event-driven inside the page is cheaper than 150 round-trip
218+
// page.evaluate calls over 30s.
219+
// The boolean predicate captures `initiallyDark` via separate
220+
// branches because the runner's structural `waitForFunction` doesn't
221+
// expose Playwright's optional `arg` parameter, and a closure-capture
222+
// would cross the page boundary (the fn runs in browser context).
223+
// Cast to access waitForFunction — same pattern as asClickablePage.
224+
// ConversationPage is the minimal runner-facing surface; real Playwright
225+
// pages (and E2eDeepPage) expose waitForFunction natively.
226+
type WaitFnPage = {
227+
waitForFunction(
228+
fn: () => boolean,
229+
opts?: { timeout?: number },
230+
): Promise<unknown>;
231+
};
232+
const waiter = page as unknown as WaitFnPage;
233+
if (typeof (waiter as { waitForFunction?: unknown }).waitForFunction !== "function") {
234+
throw new Error(
235+
`beautiful-chat-toggle-theme: page is missing waitForFunction() — runner did not provide a Playwright-shaped page`,
236+
);
237+
}
238+
try {
239+
if (initiallyDark) {
240+
await waiter.waitForFunction(
241+
() => {
242+
const win = globalThis as unknown as {
243+
document: {
244+
documentElement: { classList: { contains(s: string): boolean } };
245+
};
246+
};
247+
return !win.document.documentElement.classList.contains("dark");
248+
},
249+
{ timeout: 30_000 },
250+
);
251+
} else {
252+
await waiter.waitForFunction(
253+
() => {
254+
const win = globalThis as unknown as {
255+
document: {
256+
documentElement: { classList: { contains(s: string): boolean } };
257+
};
258+
};
259+
return win.document.documentElement.classList.contains("dark");
260+
},
261+
{ timeout: 30_000 },
262+
);
263+
}
264+
} catch (err) {
265+
const closeAware = page as ConversationPage & {
266+
isClosed?: () => boolean;
267+
};
268+
if (closeAware.isClosed?.()) {
269+
throw new Error(
270+
`beautiful-chat-toggle-theme: page closed before theme flipped from ${initiallyDark ? "dark" : "light"} — likely a renderer crash in the demo's toggleTheme handler or surrounding tree`,
271+
);
272+
}
273+
const msg = err instanceof Error ? err.message : String(err);
274+
throw new Error(
275+
`beautiful-chat-toggle-theme: html.dark class did not flip from ${initiallyDark ? "dark" : "light"} within 30s — toggleTheme tool did not fire (${msg.slice(0, 120)})`,
276+
);
216277
}
217-
throw new Error(
218-
`beautiful-chat-toggle-theme: html.dark class did not flip from ${initiallyDark ? "dark" : "light"} within 30s — toggleTheme tool did not fire`,
219-
);
220278
}
221279

222280
/**

showcase/integrations/langgraph-python/src/app/demos/beautiful-chat/hooks/use-generative-ui-examples.tsx

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { MeetingTimePicker } from "../components/generative-ui/meeting-time-pick
2020
import { ToolReasoning } from "../components/tool-rendering";
2121

2222
export const useGenerativeUIExamples = () => {
23-
const { theme, setTheme } = useTheme();
23+
const { setTheme } = useTheme();
2424

2525
// Human-in-the-Loop (frontend tool requiring user decision)
2626
useHumanInTheLoop({
@@ -67,17 +67,19 @@ export const useGenerativeUIExamples = () => {
6767
},
6868
});
6969

70-
// Frontend Tools (direct frontend state manipulation)
71-
useFrontendTool(
72-
{
73-
name: "toggleTheme",
74-
description: "Frontend tool for toggling the theme of the app.",
75-
parameters: z.object({}),
76-
handler: async () => {
77-
const isDark = document.documentElement.classList.contains("dark");
78-
setTheme(isDark ? "light" : "dark");
79-
},
70+
// Frontend Tools (direct frontend state manipulation).
71+
// No deps array needed — the handler reads `document` directly and
72+
// calls a stable setter. Including [theme, setTheme] in deps caused
73+
// the hook to re-register every time the theme flipped, which could
74+
// race with an in-flight tool result from the runtime and surface
75+
// as a renderer-level error during multi-turn beautiful-chat probes.
76+
useFrontendTool({
77+
name: "toggleTheme",
78+
description: "Frontend tool for toggling the theme of the app.",
79+
parameters: z.object({}),
80+
handler: async () => {
81+
const isDark = document.documentElement.classList.contains("dark");
82+
setTheme(isDark ? "light" : "dark");
8083
},
81-
[theme, setTheme],
82-
);
84+
});
8385
};

0 commit comments

Comments
 (0)