From 977727547156aefe8e22954b09706b37ad418c74 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Tue, 27 Oct 2026 00:35:00 -0700 Subject: [PATCH 1/2] refactor(mobile): showcase capture readiness is a tested pure model The showcase capture coordinator decided scene, theme, and readiness inline, with no tests, and kept its ready marker in sync through effects: an effect copied threads and projects into a ref every render, and the readiness effect cleared the marker with setState after commit, so a stale marker survived one commit after readiness was lost. Move route-to-scene, theme-applied, and readiness rules into showcaseCaptureModel.ts with tests. The coordinator derives a readiness key from it; the marker remembers the key it settled for and is hidden in the same render once the key changes. Agent activity staging reads the latest entities through useEffectEvent instead of a ref mirrored by an effect. The render signal store counts reports so a redraw in the same scene and theme still restarts the settle delay. Effects in ShowcaseCaptureCoordinator.tsx: 11 -> 10 (entities ref mirror removed; the readiness effect stays because it owns the settle timer and animation frames, but no longer sets state to clear the marker). Co-Authored-By: Claude Opus 5.5 --- .../showcase/ShowcaseCaptureCoordinator.tsx | 97 ++++++++----------- .../showcase/showcaseCaptureModel.test.ts | 86 ++++++++++++++++ .../features/showcase/showcaseCaptureModel.ts | 76 +++++++++++++++ .../features/showcase/showcaseRenderSignal.ts | 8 ++ 4 files changed, 213 insertions(+), 54 deletions(-) create mode 100644 apps/mobile/src/features/showcase/showcaseCaptureModel.test.ts create mode 100644 apps/mobile/src/features/showcase/showcaseCaptureModel.ts diff --git a/apps/mobile/src/features/showcase/ShowcaseCaptureCoordinator.tsx b/apps/mobile/src/features/showcase/ShowcaseCaptureCoordinator.tsx index d49fc5f55eeb8..adb41ce422ab1 100644 --- a/apps/mobile/src/features/showcase/ShowcaseCaptureCoordinator.tsx +++ b/apps/mobile/src/features/showcase/ShowcaseCaptureCoordinator.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState, useSyncExternalStore } from "react"; +import { useEffect, useEffectEvent, useRef, useState, useSyncExternalStore } from "react"; import { Keyboard, View } from "react-native"; import { CommonActions, StackActions, useNavigation } from "@react-navigation/native"; import { AsyncResult } from "effect/reactivity"; @@ -25,11 +25,17 @@ import { SHOWCASE_PENDING_TASK_DEFINITIONS, } from "./showcasePendingTasks"; import { buildShowcaseAgentActivity } from "./showcaseAgentActivity"; +import { + isShowcaseThemeApplied, + SHOWCASE_THREAD_ID, + showcaseReadinessKey, + showcaseSceneOnScreen, +} from "./showcaseCaptureModel"; import { retryShowcaseOperation } from "./showcaseRetry"; import { clearShowcaseRenderSignal, + getShowcaseRenderReports, getShowcaseRenderSignal, - isShowcaseNativeContentReady, subscribeToShowcaseRenderSignal, } from "./showcaseRenderSignal"; import { stageShowcaseAgentActivity } from "./stageShowcaseAgentActivity"; @@ -37,19 +43,6 @@ import { openSettings } from "../settings/settings-navigation"; import { selectWorkspaceDetail } from "../workspace/workspace-detail-selection"; const SHOWCASE_ENABLED = process.env.EXPO_PUBLIC_SHOWCASE === "1"; -const SHOWCASE_THREAD_ID = "remote-command-center"; - -function sceneFromPathname(pathname: string): ShowcaseScene | null { - const routePath = pathname.split(/[?#]/u, 1)[0] ?? pathname; - if (routePath === "/settings" || routePath.endsWith("/settings/environments")) { - return "environments"; - } - if (routePath.endsWith("/terminal")) return "terminal"; - if (routePath.endsWith("/review")) return "review"; - if (routePath.startsWith("/threads/")) return "thread"; - if (routePath === "/") return "threads"; - return null; -} export function ShowcaseCaptureCoordinator(props: { readonly pathname: string }) { if (!SHOWCASE_ENABLED) return null; @@ -87,20 +80,25 @@ function EnabledShowcaseCaptureCoordinator(props: { readonly pathname: string }) const [requestedScene, setRequestedScene] = useState(null); const [requestedTheme, setRequestedTheme] = useState(null); const [themeRequestSettled, setThemeRequestSettled] = useState(false); - const [readyScene, setReadyScene] = useState(null); + const [marked, setMarked] = useState<{ + readonly key: string; + readonly scene: ShowcaseScene; + } | null>(null); const [orientationSettled, setOrientationSettled] = useState(false); const [agentActivityStaged, setAgentActivityStaged] = useState(false); const requestedSceneRef = useRef(null); // Read current entities without restarting on shell updates and re-entering open permission prompts. - const entitiesRef = useRef({ threads, projects }); - useEffect(() => { - entitiesRef.current = { threads, projects }; - }, [projects, threads]); + const readFixtureEntities = useEffectEvent(() => ({ threads, projects })); const renderSignal = useSyncExternalStore( subscribeToShowcaseRenderSignal, getShowcaseRenderSignal, getShowcaseRenderSignal, ); + const renderReports = useSyncExternalStore( + subscribeToShowcaseRenderSignal, + getShowcaseRenderReports, + getShowcaseRenderReports, + ); useEffect(() => { if (pairingUrls.length > 0) return; @@ -152,10 +150,11 @@ function EnabledShowcaseCaptureCoordinator(props: { readonly pathname: string }) return () => clearInterval(interval); }, []); - const themeApplied = - requestedTheme === null - ? themeRequestSettled - : themeIds.light === requestedTheme && themeIds.dark === requestedTheme; + const themeApplied = isShowcaseThemeApplied({ + launchRequestRead: themeRequestSettled, + requestedTheme, + themeIds, + }); useEffect(() => { if (requestedTheme === null || themeApplied || !appearancePreferencesReady) { @@ -184,9 +183,7 @@ function EnabledShowcaseCaptureCoordinator(props: { readonly pathname: string }) }; }, [connectPairingUrl, pairingUrls]); - const routeScene = sceneFromPathname(props.pathname); - const scene = - requestedScene === "agent-activity" && routeScene === "threads" ? "agent-activity" : routeScene; + const scene = showcaseSceneOnScreen(requestedScene, props.pathname); const hasServerFixture = workspace.state.hasReadyEnvironment && workspace.environments.length >= 3 && @@ -264,7 +261,7 @@ function EnabledShowcaseCaptureCoordinator(props: { readonly pathname: string }) void retryShowcaseOperation( async () => { const now = Date.now(); - const { threads: latestThreads, projects: latestProjects } = entitiesRef.current; + const { threads: latestThreads, projects: latestProjects } = readFixtureEntities(); const activity = buildShowcaseAgentActivity(latestThreads, latestProjects, now); const outcome = activity === null @@ -285,21 +282,23 @@ function EnabledShowcaseCaptureCoordinator(props: { readonly pathname: string }) }; }, [agentActivityStaged, hasFixture, scene]); + const readinessKey = showcaseReadinessKey({ + scene, + requestedScene, + hasFixture, + orientationSettled, + themeApplied, + agentActivityStaged, + themeId, + renderSignal, + renderReports, + }); + // A marker belongs to the readiness it was settled for; any change hides it until it settles again. + if (marked !== null && marked.key !== readinessKey) setMarked(null); + const readyScene = marked?.scene ?? null; + useEffect(() => { - if ( - scene === null || - requestedScene === null || - scene !== requestedScene || - !hasFixture || - !orientationSettled || - !themeApplied || - (scene === "agent-activity" && !agentActivityStaged) || - !isShowcaseNativeContentReady({ scene, themeId, renderSignal }) - ) { - // oxlint-disable-next-line react/set-state-in-effect -- Capture readiness clears while the scene, fixture, or native content is not ready. - setReadyScene(null); - return; - } + if (readinessKey === null || scene === null) return; if (scene === "terminal") Keyboard.dismiss(); let renderFrame: number | null = null; @@ -308,7 +307,7 @@ function EnabledShowcaseCaptureCoordinator(props: { readonly pathname: string }) renderFrame = requestAnimationFrame(() => { readyFrame = requestAnimationFrame(() => { markNativeShowcaseReady(scene); - setReadyScene(scene); + setMarked({ key: readinessKey, scene }); }); }); }, 500); @@ -317,19 +316,9 @@ function EnabledShowcaseCaptureCoordinator(props: { readonly pathname: string }) if (renderFrame !== null) cancelAnimationFrame(renderFrame); if (readyFrame !== null) cancelAnimationFrame(readyFrame); }; - }, [ - agentActivityStaged, - hasFixture, - orientationSettled, - renderSignal, - requestedScene, - scene, - themeApplied, - themeId, - ]); + }, [readinessKey, scene]); if (readyScene === null) return null; - return ( { + it("treats the threads list as the agent activity scene only when that scene is requested", () => { + expect(showcaseSceneOnScreen("agent-activity", "/")).toBe("agent-activity"); + expect(showcaseSceneOnScreen("threads", "/")).toBe("threads"); + expect(showcaseSceneOnScreen("agent-activity", "/threads/env/thread")).toBe("thread"); + expect(showcaseSceneOnScreen(null, "/threads/env/thread/review?x=1")).toBe("review"); + }); +}); + +describe("isShowcaseThemeApplied", () => { + it("waits for the launch request when no theme is requested", () => { + const themeIds = { light: "t3-code", dark: "t3-code" } as const; + expect( + isShowcaseThemeApplied({ launchRequestRead: false, requestedTheme: null, themeIds }), + ).toBe(false); + expect( + isShowcaseThemeApplied({ launchRequestRead: true, requestedTheme: null, themeIds }), + ).toBe(true); + }); + + it("requires both appearances to use the requested theme", () => { + expect( + isShowcaseThemeApplied({ + launchRequestRead: true, + requestedTheme: "t3-code", + themeIds: { light: "t3-code", dark: "material-you" }, + }), + ).toBe(false); + }); +}); + +describe("showcaseReadinessKey", () => { + it("is null until every precondition holds", () => { + expect(showcaseReadinessKey(ready)).not.toBeNull(); + expect(showcaseReadinessKey({ ...ready, requestedScene: "terminal" })).toBeNull(); + expect(showcaseReadinessKey({ ...ready, hasFixture: false })).toBeNull(); + expect(showcaseReadinessKey({ ...ready, orientationSettled: false })).toBeNull(); + expect(showcaseReadinessKey({ ...ready, themeApplied: false })).toBeNull(); + }); + + it("waits for agent activity to be staged", () => { + const agentActivity = { + ...ready, + scene: "agent-activity", + requestedScene: "agent-activity", + } as const; + expect(showcaseReadinessKey(agentActivity)).toBeNull(); + expect(showcaseReadinessKey({ ...agentActivity, agentActivityStaged: true })).not.toBeNull(); + }); + + it("waits for review to report a draw in the current theme, and changes key when it redraws", () => { + const review = { ...ready, scene: "review", requestedScene: "review" } as const; + expect(showcaseReadinessKey(review)).toBeNull(); + expect( + showcaseReadinessKey({ + ...review, + renderSignal: { scene: "review", themeId: "material-you" }, + }), + ).toBeNull(); + const drawn = { ...review, renderSignal: { scene: "review", themeId: "t3-code" } } as const; + const firstDraw = showcaseReadinessKey({ ...drawn, renderReports: 1 }); + expect(firstDraw).not.toBeNull(); + expect(showcaseReadinessKey({ ...drawn, renderReports: 2 })).not.toBe(firstDraw); + }); +}); diff --git a/apps/mobile/src/features/showcase/showcaseCaptureModel.ts b/apps/mobile/src/features/showcase/showcaseCaptureModel.ts new file mode 100644 index 0000000000000..7f60fb3dd4518 --- /dev/null +++ b/apps/mobile/src/features/showcase/showcaseCaptureModel.ts @@ -0,0 +1,76 @@ +import type { MobileThemeId, MobileThemeIds } from "../../lib/mobileTheme"; +import type { ShowcaseScene } from "./nativeShowcaseScene"; +import { isShowcaseNativeContentReady, type ShowcaseRenderSignal } from "./showcaseRenderSignal"; + +export const SHOWCASE_THREAD_ID = "remote-command-center"; + +export function sceneFromPathname(pathname: string): ShowcaseScene | null { + const routePath = pathname.split(/[?#]/u, 1)[0] ?? pathname; + if (routePath === "/settings" || routePath.endsWith("/settings/environments")) { + return "environments"; + } + if (routePath.endsWith("/terminal")) return "terminal"; + if (routePath.endsWith("/review")) return "review"; + if (routePath.startsWith("/threads/")) return "thread"; + if (routePath === "/") return "threads"; + return null; +} + +/** Agent activity is staged on the threads list, so that route counts as the scene. */ +export function showcaseSceneOnScreen( + requestedScene: ShowcaseScene | null, + pathname: string, +): ShowcaseScene | null { + const routeScene = sceneFromPathname(pathname); + return requestedScene === "agent-activity" && routeScene === "threads" + ? "agent-activity" + : routeScene; +} + +export function isShowcaseThemeApplied(input: { + readonly launchRequestRead: boolean; + readonly requestedTheme: MobileThemeId | null; + readonly themeIds: MobileThemeIds; +}): boolean { + if (input.requestedTheme === null) return input.launchRequestRead; + return ( + input.themeIds.light === input.requestedTheme && input.themeIds.dark === input.requestedTheme + ); +} + +export interface ShowcaseReadinessInput { + readonly scene: ShowcaseScene | null; + readonly requestedScene: ShowcaseScene | null; + readonly hasFixture: boolean; + readonly orientationSettled: boolean; + readonly themeApplied: boolean; + readonly agentActivityStaged: boolean; + readonly themeId: MobileThemeId; + readonly renderSignal: ShowcaseRenderSignal | null; + /** Counts render-signal changes; a redraw report restarts the settle delay. */ + readonly renderReports: number; +} + +/** + * The capture key for a scene that is ready to be marked, or null while any + * precondition is missing. A changed key restarts the settle delay. + */ +export function showcaseReadinessKey(input: ShowcaseReadinessInput): string | null { + const { scene } = input; + if ( + scene === null || + scene !== input.requestedScene || + !input.hasFixture || + !input.orientationSettled || + !input.themeApplied || + (scene === "agent-activity" && !input.agentActivityStaged) || + !isShowcaseNativeContentReady({ + scene, + themeId: input.themeId, + renderSignal: input.renderSignal, + }) + ) { + return null; + } + return [scene, input.themeId, input.renderReports].join("|"); +} diff --git a/apps/mobile/src/features/showcase/showcaseRenderSignal.ts b/apps/mobile/src/features/showcase/showcaseRenderSignal.ts index 014f08f781dd5..572ef5bd0e01e 100644 --- a/apps/mobile/src/features/showcase/showcaseRenderSignal.ts +++ b/apps/mobile/src/features/showcase/showcaseRenderSignal.ts @@ -8,11 +8,17 @@ export type ShowcaseRenderSignal = Readonly<{ const listeners = new Set<() => void>(); let renderSignal: ShowcaseRenderSignal | null = null; +// A redraw can repeat the same scene and theme; counting reports lets it restart the settle delay. +let renderReports = 0; export function getShowcaseRenderSignal(): ShowcaseRenderSignal | null { return renderSignal; } +export function getShowcaseRenderReports(): number { + return renderReports; +} + export function subscribeToShowcaseRenderSignal(listener: () => void): () => void { listeners.add(listener); return () => listeners.delete(listener); @@ -20,12 +26,14 @@ export function subscribeToShowcaseRenderSignal(listener: () => void): () => voi export function reportShowcaseSceneRendered(signal: ShowcaseRenderSignal): void { renderSignal = signal; + renderReports += 1; for (const listener of listeners) listener(); } export function clearShowcaseRenderSignal(): void { if (renderSignal === null) return; renderSignal = null; + renderReports += 1; for (const listener of listeners) listener(); } From 6a2a9fb97c43dbef9c7aebee6730df15508652f3 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Tue, 27 Oct 2026 01:13:18 -0700 Subject: [PATCH 2/2] fix(mobile): showcase redraws keep the ready marker while they re-settle Restore main's behavior: a ReviewSheet redraw or theme change while ready restarts the 500ms settle timer without hiding the marker; only losing readiness clears it. Read the render signal and its report count as one snapshot, and keep sceneFromPathname private to the model. Co-Authored-By: Claude Opus 5.5 --- .../showcase/ShowcaseCaptureCoordinator.tsx | 26 ++++++------------- .../features/showcase/showcaseCaptureModel.ts | 2 +- .../features/showcase/showcaseRenderSignal.ts | 24 ++++++++--------- 3 files changed, 20 insertions(+), 32 deletions(-) diff --git a/apps/mobile/src/features/showcase/ShowcaseCaptureCoordinator.tsx b/apps/mobile/src/features/showcase/ShowcaseCaptureCoordinator.tsx index adb41ce422ab1..5cb3f5ad14791 100644 --- a/apps/mobile/src/features/showcase/ShowcaseCaptureCoordinator.tsx +++ b/apps/mobile/src/features/showcase/ShowcaseCaptureCoordinator.tsx @@ -34,8 +34,7 @@ import { import { retryShowcaseOperation } from "./showcaseRetry"; import { clearShowcaseRenderSignal, - getShowcaseRenderReports, - getShowcaseRenderSignal, + getShowcaseRenderSnapshot, subscribeToShowcaseRenderSignal, } from "./showcaseRenderSignal"; import { stageShowcaseAgentActivity } from "./stageShowcaseAgentActivity"; @@ -80,24 +79,16 @@ function EnabledShowcaseCaptureCoordinator(props: { readonly pathname: string }) const [requestedScene, setRequestedScene] = useState(null); const [requestedTheme, setRequestedTheme] = useState(null); const [themeRequestSettled, setThemeRequestSettled] = useState(false); - const [marked, setMarked] = useState<{ - readonly key: string; - readonly scene: ShowcaseScene; - } | null>(null); + const [readyScene, setReadyScene] = useState(null); const [orientationSettled, setOrientationSettled] = useState(false); const [agentActivityStaged, setAgentActivityStaged] = useState(false); const requestedSceneRef = useRef(null); // Read current entities without restarting on shell updates and re-entering open permission prompts. const readFixtureEntities = useEffectEvent(() => ({ threads, projects })); - const renderSignal = useSyncExternalStore( + const { signal: renderSignal, reports: renderReports } = useSyncExternalStore( subscribeToShowcaseRenderSignal, - getShowcaseRenderSignal, - getShowcaseRenderSignal, - ); - const renderReports = useSyncExternalStore( - subscribeToShowcaseRenderSignal, - getShowcaseRenderReports, - getShowcaseRenderReports, + getShowcaseRenderSnapshot, + getShowcaseRenderSnapshot, ); useEffect(() => { @@ -293,9 +284,8 @@ function EnabledShowcaseCaptureCoordinator(props: { readonly pathname: string }) renderSignal, renderReports, }); - // A marker belongs to the readiness it was settled for; any change hides it until it settles again. - if (marked !== null && marked.key !== readinessKey) setMarked(null); - const readyScene = marked?.scene ?? null; + // Losing readiness hides the marker. A redraw or theme change while ready keeps it and re-settles. + if (readinessKey === null && readyScene !== null) setReadyScene(null); useEffect(() => { if (readinessKey === null || scene === null) return; @@ -307,7 +297,7 @@ function EnabledShowcaseCaptureCoordinator(props: { readonly pathname: string }) renderFrame = requestAnimationFrame(() => { readyFrame = requestAnimationFrame(() => { markNativeShowcaseReady(scene); - setMarked({ key: readinessKey, scene }); + setReadyScene(scene); }); }); }, 500); diff --git a/apps/mobile/src/features/showcase/showcaseCaptureModel.ts b/apps/mobile/src/features/showcase/showcaseCaptureModel.ts index 7f60fb3dd4518..b790f51efa671 100644 --- a/apps/mobile/src/features/showcase/showcaseCaptureModel.ts +++ b/apps/mobile/src/features/showcase/showcaseCaptureModel.ts @@ -4,7 +4,7 @@ import { isShowcaseNativeContentReady, type ShowcaseRenderSignal } from "./showc export const SHOWCASE_THREAD_ID = "remote-command-center"; -export function sceneFromPathname(pathname: string): ShowcaseScene | null { +function sceneFromPathname(pathname: string): ShowcaseScene | null { const routePath = pathname.split(/[?#]/u, 1)[0] ?? pathname; if (routePath === "/settings" || routePath.endsWith("/settings/environments")) { return "environments"; diff --git a/apps/mobile/src/features/showcase/showcaseRenderSignal.ts b/apps/mobile/src/features/showcase/showcaseRenderSignal.ts index 572ef5bd0e01e..092ef91cf2d3b 100644 --- a/apps/mobile/src/features/showcase/showcaseRenderSignal.ts +++ b/apps/mobile/src/features/showcase/showcaseRenderSignal.ts @@ -7,16 +7,16 @@ export type ShowcaseRenderSignal = Readonly<{ }>; const listeners = new Set<() => void>(); -let renderSignal: ShowcaseRenderSignal | null = null; -// A redraw can repeat the same scene and theme; counting reports lets it restart the settle delay. -let renderReports = 0; +export type ShowcaseRenderSnapshot = Readonly<{ + signal: ShowcaseRenderSignal | null; + // A redraw can repeat the same scene and theme; counting reports lets it restart the settle delay. + reports: number; +}>; -export function getShowcaseRenderSignal(): ShowcaseRenderSignal | null { - return renderSignal; -} +let snapshot: ShowcaseRenderSnapshot = { signal: null, reports: 0 }; -export function getShowcaseRenderReports(): number { - return renderReports; +export function getShowcaseRenderSnapshot(): ShowcaseRenderSnapshot { + return snapshot; } export function subscribeToShowcaseRenderSignal(listener: () => void): () => void { @@ -25,15 +25,13 @@ export function subscribeToShowcaseRenderSignal(listener: () => void): () => voi } export function reportShowcaseSceneRendered(signal: ShowcaseRenderSignal): void { - renderSignal = signal; - renderReports += 1; + snapshot = { signal, reports: snapshot.reports + 1 }; for (const listener of listeners) listener(); } export function clearShowcaseRenderSignal(): void { - if (renderSignal === null) return; - renderSignal = null; - renderReports += 1; + if (snapshot.signal === null) return; + snapshot = { signal: null, reports: snapshot.reports + 1 }; for (const listener of listeners) listener(); }