From b0659dada32b08871399e5f657e26b5b480ef188 Mon Sep 17 00:00:00 2001 From: Noah Date: Tue, 7 Jul 2026 17:12:21 -0400 Subject: [PATCH 1/2] perf(desktop): cache parsed markdown across channel-switch remounts The timeline subtree is keyed by channel id (scroll-restoration requirement), so every channel switch remounts all rows and re-runs each row's synchronous react-markdown parse. Route the per-mount runtime through context so the component map is module-stable, then cache parsed element trees in a module-level LRU keyed by parse inputs. Warm markdown-heavy switches drop ~23 percent wall time (A/B: 1045ms -> 809ms and 924ms -> 699ms median at 4x CPU throttle, warm-switch-markdown.perf.ts). Co-Authored-By: Claude Fable 5 --- .../features/workspaces/useWorkspaceInit.ts | 2 + desktop/src/shared/ui/markdown.tsx | 383 +++++++++--------- .../src/shared/ui/markdown/nodeCache.test.mjs | 90 ++++ desktop/src/shared/ui/markdown/nodeCache.ts | 121 ++++++ .../src/shared/ui/markdown/runtimeContext.ts | 27 ++ .../tests/e2e/warm-switch-markdown.perf.ts | 292 +++++++++++++ 6 files changed, 725 insertions(+), 190 deletions(-) create mode 100644 desktop/src/shared/ui/markdown/nodeCache.test.mjs create mode 100644 desktop/src/shared/ui/markdown/nodeCache.ts create mode 100644 desktop/src/shared/ui/markdown/runtimeContext.ts create mode 100644 desktop/tests/e2e/warm-switch-markdown.perf.ts diff --git a/desktop/src/features/workspaces/useWorkspaceInit.ts b/desktop/src/features/workspaces/useWorkspaceInit.ts index d368735b6..d9df6113f 100644 --- a/desktop/src/features/workspaces/useWorkspaceInit.ts +++ b/desktop/src/features/workspaces/useWorkspaceInit.ts @@ -17,6 +17,7 @@ import { resetActiveAgentTurnsStore } from "@/features/agents/activeAgentTurnsSt import { resetAgentWorkingSignal } from "@/features/agents/agentWorkingSignal"; import { resetAgentObserverStore } from "@/features/agents/observerRelayStore"; import { resetSidebarRelayConnectionCardState } from "@/features/sidebar/ui/useSidebarRelayConnectionCard"; +import { clearMarkdownNodeCache } from "@/shared/ui/markdown/nodeCache"; import { resetVideoPlayerState } from "@/shared/ui/videoPlayerState"; import { initFirstWorkspace } from "./workspaceStorage"; @@ -40,6 +41,7 @@ function resetWorkspaceState(): void { resetRenderScopedReactionHydration(); clearSearchHitEventCache(); clearAllDrafts(); + clearMarkdownNodeCache(); } type WorkspaceInitResult = diff --git a/desktop/src/shared/ui/markdown.tsx b/desktop/src/shared/ui/markdown.tsx index 8493b7239..5d70bd121 100644 --- a/desktop/src/shared/ui/markdown.tsx +++ b/desktop/src/shared/ui/markdown.tsx @@ -1,6 +1,6 @@ import * as React from "react"; import { createPortal } from "react-dom"; -import ReactMarkdown, { type Components } from "react-markdown"; +import type { Components } from "react-markdown"; import { ChevronLeft, ChevronRight, @@ -9,8 +9,6 @@ import { ZoomOut, } from "lucide-react"; import { AnimatePresence, motion, useReducedMotion } from "motion/react"; -import remarkBreaks from "remark-breaks"; -import remarkGfm from "remark-gfm"; import { toast } from "sonner"; import { useAppNavigation } from "@/app/navigation/useAppNavigation"; @@ -29,13 +27,6 @@ import { } from "@/shared/lib/linkPreview"; import { useResolvedLinkPreviews } from "@/shared/lib/useResolvedLinkPreviews"; import { rewriteRelayUrl } from "@/shared/lib/mediaUrl"; -import rehypeImageGallery from "@/shared/lib/rehypeImageGallery"; -import rehypeSearchHighlight from "@/shared/lib/rehypeSearchHighlight"; -import remarkChannelLinks from "@/shared/lib/remarkChannelLinks"; -import remarkCustomEmoji from "@/shared/lib/remarkCustomEmoji"; -import remarkMentions from "@/shared/lib/remarkMentions"; -import remarkSpoilers from "@/shared/lib/remarkSpoilers"; -import remarkMessageLinks from "@/features/messages/lib/remarkMessageLinks"; import { AttachmentGroup } from "@/shared/ui/attachment"; import { ConfigNudgeCard } from "@/shared/ui/config-nudge-attachment"; import { LinkPreviewAttachment } from "@/shared/ui/link-preview-attachment"; @@ -75,6 +66,11 @@ import { MarkdownInput } from "./markdown/MarkdownInput"; import { MarkdownTable } from "./markdown/MarkdownTable"; import { MaskedLinkTooltip } from "./markdown/MaskedLinkTooltip"; import { MessageLinkPill } from "./markdown/MessageLinkPill"; +import { renderCachedMarkdown } from "./markdown/nodeCache"; +import { + MarkdownRuntimeContext, + useMarkdownRuntime, +} from "./markdown/runtimeContext"; import { resolveFileCard } from "./markdownFileCard"; import type { ImetaEntry, @@ -89,7 +85,6 @@ import { imageReserveStyle, isInsideHiddenSpoiler, getReactNodeText, - messageLinkUrlTransform, rememberDecodedImageDimensions, useFrozenImageReserve, useStableArray, @@ -108,12 +103,6 @@ const VideoReviewMarkdownContext = React.createContext< VideoReviewContext | undefined >(undefined); -function useLatestRef(value: T) { - const ref = React.useRef(value); - ref.current = value; - return ref; -} - function MarkdownVideoPlayer({ alt, entry, @@ -1560,13 +1549,109 @@ function ImageBlock({ alt, dim, resolvedSrc, src }: ImageBlockProps) { } function createMarkdownComponents( - runtimeRef: React.RefObject, interactive = true, mediaInset = false, ): Components { const listItemClassName = "[&_p]:inline"; const listClassName = "space-y-1 pl-6 marker:text-muted-foreground/80"; + function MarkdownAnchor({ + children, + href, + ...props + }: React.ComponentPropsWithoutRef<"a">) { + const { channels, imetaByUrl, onOpenMessageLink } = useMarkdownRuntime(); + if (!interactive) { + return {children}; + } + + // Markdown image-link syntax (`[![alt](src)](href)`) otherwise nests the + // image lightbox button inside an anchor. Keep the image as the lightbox + // trigger and suppress the parent link activation for block media. + if (hasBlockMedia(React.Children.toArray(children))) { + return <>{children}; + } + + const label = getReactNodeText(children); + + // Generic file attachment: a `[filename](url)` link whose href matches an + // imeta entry with a non-image, non-video MIME. Render a download card + // instead of a plain link. (Media uses the `img` renderer, not this path.) + const card = resolveFileCard( + href ? imetaByUrl?.get(href) : undefined, + href, + label, + ); + if (card) { + return ( + + ); + } + + // Intercept `buzz://message?channel=…&id=…` links so a click navigates + // in-app instead of opening the URL in the OS browser. http(s) links + // continue to use the existing target="_blank" behavior. + if (href) { + const messageLinkTarget = resolveMessageLinkRenderTarget({ + href, + label, + }); + if (messageLinkTarget.kind !== "none") { + if (messageLinkTarget.kind === "pill") { + return ( + + ); + } + + return ( + { + event.preventDefault(); + onOpenMessageLink(messageLinkTarget.link); + }} + > + {children} + + ); + } + // Malformed message deep link — fall through to the default + // anchor (renders as a normal external link). + } + + const supportedLinkPreview = href ? parseSupportedLinkPreview(href) : null; + const isLinearLink = supportedLinkPreview?.kind === "linear-issue"; + + const anchor = ( + + {children} + + ); + + return ( + + {anchor} + + ); + } + return { spoiler: ({ children, @@ -1582,104 +1667,7 @@ function createMarkdownComponents( {children} ), - a: ({ children, href, ...props }) => { - const { imetaByUrl, onOpenMessageLink } = runtimeRef.current; - if (!interactive) { - return {children}; - } - - // Markdown image-link syntax (`[![alt](src)](href)`) otherwise nests the - // image lightbox button inside an anchor. Keep the image as the lightbox - // trigger and suppress the parent link activation for block media. - if (hasBlockMedia(React.Children.toArray(children))) { - return <>{children}; - } - - const label = getReactNodeText(children); - - // Generic file attachment: a `[filename](url)` link whose href matches an - // imeta entry with a non-image, non-video MIME. Render a download card - // instead of a plain link. (Media uses the `img` renderer, not this path.) - const card = resolveFileCard( - href ? imetaByUrl?.get(href) : undefined, - href, - label, - ); - if (card) { - return ( - - ); - } - - // Intercept `buzz://message?channel=…&id=…` links so a click navigates - // in-app instead of opening the URL in the OS browser. http(s) links - // continue to use the existing target="_blank" behavior. - if (href) { - const messageLinkTarget = resolveMessageLinkRenderTarget({ - href, - label, - }); - if (messageLinkTarget.kind !== "none") { - if (messageLinkTarget.kind === "pill") { - return ( - - ); - } - - return ( - { - event.preventDefault(); - onOpenMessageLink(messageLinkTarget.link); - }} - > - {children} - - ); - } - // Malformed message deep link — fall through to the default - // anchor (renders as a normal external link). - } - - const supportedLinkPreview = href - ? parseSupportedLinkPreview(href) - : null; - const isLinearLink = supportedLinkPreview?.kind === "linear-issue"; - - const anchor = ( - - {children} - - ); - - return ( - - {anchor} - - ); - }, + a: MarkdownAnchor, blockquote: ({ children }) => (
{children} @@ -1751,8 +1739,8 @@ function createMarkdownComponents( ), hr: () =>
, - img: ({ alt, src }) => { - const { imetaByUrl } = runtimeRef.current; + img: function MarkdownImage({ alt, src }) { + const { imetaByUrl } = useMarkdownRuntime(); const resolvedSrc = src ? rewriteRelayUrl(src) : src; if (!interactive) { const fallbackLabel = resolvedSrc?.endsWith(".mp4") @@ -1851,9 +1839,13 @@ function createMarkdownComponents( ul: ({ children }) => (
    {children}
), - mention: ({ children }: { children?: React.ReactNode }) => { + mention: function MarkdownMention({ + children, + }: { + children?: React.ReactNode; + }) { const { agentMentionPubkeysByName, mentionPubkeysByName } = - runtimeRef.current; + useMarkdownRuntime(); const mentionText = String(children ?? ""); const mentionName = mentionText.replace(/^@/, "").trim().toLowerCase(); const pubkey = mentionPubkeysByName?.[mentionName]; @@ -1910,8 +1902,12 @@ function createMarkdownComponents( } return ; }, - "channel-link": ({ children }: { children?: React.ReactNode }) => { - const { channels, onOpenChannel } = runtimeRef.current; + "channel-link": function MarkdownChannelLink({ + children, + }: { + children?: React.ReactNode; + }) { + const { channels, onOpenChannel } = useMarkdownRuntime(); const text = String(children ?? ""); const channelName = text.startsWith("#") ? text.slice(1) : text; const channel = channels.find( @@ -1946,8 +1942,12 @@ function createMarkdownComponents( ); }, - "message-link": ({ children }: { children?: React.ReactNode }) => { - const { channels, onOpenMessageLink } = runtimeRef.current; + "message-link": function MarkdownMessageLink({ + children, + }: { + children?: React.ReactNode; + }) { + const { channels, onOpenMessageLink } = useMarkdownRuntime(); const href = String(children ?? ""); const parsed = parseMessageLink(href); if (!parsed.ok) { @@ -1969,6 +1969,26 @@ function createMarkdownComponents( } as Components; } +/** + * The component map only varies by the two boolean render flags, so at most + * four instances ever exist. Module-stable maps mean cached markdown element + * trees (see ./markdown/nodeCache.ts) never embed per-mount closures. + */ +const markdownComponentsByVariant = new Map(); + +function getMarkdownComponents( + interactive: boolean, + mediaInset: boolean, +): Components { + const key = `${interactive ? "i" : ""}${mediaInset ? "m" : ""}`; + let components = markdownComponentsByVariant.get(key); + if (!components) { + components = createMarkdownComponents(interactive, mediaInset); + markdownComponentsByVariant.set(key, components); + } + return components; +} + function MarkdownInner({ channelNames, className, @@ -2017,44 +2037,25 @@ function MarkdownInner({ () => computeConfigNudge(content, interactive, configNudgeAuthorPubkey), [content, interactive, configNudgeAuthorPubkey], ); - const runtimeRef = useLatestRef({ - agentMentionPubkeysByName, - channels, - imetaByUrl, - mentionPubkeysByName, - onOpenChannel, - onOpenMessageLink, - }); - - const components = React.useMemo( - () => createMarkdownComponents(runtimeRef, interactive, mediaInset), - [runtimeRef, interactive, mediaInset], - ); - - // biome-ignore lint/suspicious/noExplicitAny: PluggableList type not directly importable - const remarkPlugins = React.useMemo( - () => [ - remarkGfm, - remarkBreaks, - remarkSpoilers, - remarkMessageLinks, - [remarkMentions, { mentionNames }], - [remarkChannelLinks, { channelNames }], - [remarkCustomEmoji, { customEmoji }], + const runtime = React.useMemo( + () => ({ + agentMentionPubkeysByName, + channels, + imetaByUrl, + mentionPubkeysByName, + onOpenChannel, + onOpenMessageLink, + }), + [ + agentMentionPubkeysByName, + channels, + imetaByUrl, + mentionPubkeysByName, + onOpenChannel, + onOpenMessageLink, ], - [mentionNames, channelNames, customEmoji], ); - // biome-ignore lint/suspicious/noExplicitAny: PluggableList type not directly importable - const rehypePlugins = React.useMemo(() => { - // biome-ignore lint/suspicious/noExplicitAny: PluggableList type not directly importable - const plugins: any[] = [rehypeImageGallery]; - if (searchQuery && searchQuery.trim().length >= 2) { - plugins.push([rehypeSearchHighlight, { query: searchQuery }]); - } - return plugins; - }, [searchQuery]); - let processedContent = content; // Note: stripping the sentinel here is intentionally omitted. When @@ -2072,16 +2073,16 @@ function MarkdownInner({ const resolvedLinkPreviews = useResolvedLinkPreviews(linkPreviews); - const markdownNode = ( - - {processedContent} - - ); + const markdownNode = renderCachedMarkdown({ + channelNames, + components: getMarkdownComponents(interactive, mediaInset), + content: processedContent, + customEmoji, + interactive, + mediaInset, + mentionNames, + searchQuery, + }); return (
- - {selectProseOrNudge(configNudge, markdownNode)} - {configNudge !== null ? ( - - - - ) : null} - {resolvedLinkPreviews.length > 0 ? ( - - {resolvedLinkPreviews.map((preview) => ( - - ))} - - ) : null} - + + + {selectProseOrNudge(configNudge, markdownNode)} + {configNudge !== null ? ( + + + + ) : null} + {resolvedLinkPreviews.length > 0 ? ( + + {resolvedLinkPreviews.map((preview) => ( + + ))} + + ) : null} + +
); } diff --git a/desktop/src/shared/ui/markdown/nodeCache.test.mjs b/desktop/src/shared/ui/markdown/nodeCache.test.mjs new file mode 100644 index 000000000..931ebebb6 --- /dev/null +++ b/desktop/src/shared/ui/markdown/nodeCache.test.mjs @@ -0,0 +1,90 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { renderToStaticMarkup } from "react-dom/server"; + +import { clearMarkdownNodeCache, renderCachedMarkdown } from "./nodeCache.ts"; + +// The whole point of the cache is element-identity reuse across the message +// timeline's per-channel-switch remount: same parse inputs must return the +// SAME element (no re-parse), and anything that changes the parse output +// must miss. + +const BASE = { + components: {}, + content: "**bold** and `code`", + interactive: true, + mediaInset: false, +}; + +test("same parse inputs return the identical cached element", () => { + clearMarkdownNodeCache(); + const first = renderCachedMarkdown({ ...BASE }); + const second = renderCachedMarkdown({ ...BASE }); + assert.equal(first, second); + assert.match(renderToStaticMarkup(first), /bold<\/strong>/); +}); + +test("content changes miss the cache", () => { + clearMarkdownNodeCache(); + const first = renderCachedMarkdown({ ...BASE }); + const second = renderCachedMarkdown({ ...BASE, content: "**bald**" }); + assert.notEqual(first, second); +}); + +test("customEmoji is keyed by value, not identity", () => { + clearMarkdownNodeCache(); + const emoji = [{ shortcode: "buzz", url: "https://relay/buzz.png" }]; + const first = renderCachedMarkdown({ + ...BASE, + content: "hi :buzz:", + customEmoji: emoji, + }); + // Fresh array, same values — the exact remount scenario (useMessageEmoji + // rebuilds the array): must HIT. + const second = renderCachedMarkdown({ + ...BASE, + content: "hi :buzz:", + customEmoji: [{ shortcode: "buzz", url: "https://relay/buzz.png" }], + }); + assert.equal(first, second); + // Same content, different emoji set (e.g. emoji added while editing — + // custom-emoji.spec.ts Bug 2): must MISS so the new emoji renders. + const third = renderCachedMarkdown({ + ...BASE, + content: "hi :buzz:", + customEmoji: [{ shortcode: "buzz", url: "https://relay/other.png" }], + }); + assert.notEqual(first, third); +}); + +test("mention and channel names are part of the key", () => { + clearMarkdownNodeCache(); + const first = renderCachedMarkdown({ + ...BASE, + content: "ping @alice in #general", + mentionNames: ["alice"], + channelNames: ["general"], + }); + const second = renderCachedMarkdown({ + ...BASE, + content: "ping @alice in #general", + mentionNames: ["alice", "bob"], + channelNames: ["general"], + }); + assert.notEqual(first, second); +}); + +test("render flag variants do not collide", () => { + clearMarkdownNodeCache(); + const interactive = renderCachedMarkdown({ ...BASE }); + const nonInteractive = renderCachedMarkdown({ ...BASE, interactive: false }); + assert.notEqual(interactive, nonInteractive); +}); + +test("active search queries bypass the cache", () => { + clearMarkdownNodeCache(); + const first = renderCachedMarkdown({ ...BASE, searchQuery: "bold" }); + const second = renderCachedMarkdown({ ...BASE, searchQuery: "bold" }); + assert.notEqual(first, second); +}); diff --git a/desktop/src/shared/ui/markdown/nodeCache.ts b/desktop/src/shared/ui/markdown/nodeCache.ts new file mode 100644 index 000000000..0d91903a1 --- /dev/null +++ b/desktop/src/shared/ui/markdown/nodeCache.ts @@ -0,0 +1,121 @@ +import type * as React from "react"; +import ReactMarkdown, { type Components } from "react-markdown"; +import remarkBreaks from "remark-breaks"; +import remarkGfm from "remark-gfm"; + +import remarkMessageLinks from "@/features/messages/lib/remarkMessageLinks"; +import rehypeImageGallery from "@/shared/lib/rehypeImageGallery"; +import rehypeSearchHighlight from "@/shared/lib/rehypeSearchHighlight"; +import remarkChannelLinks from "@/shared/lib/remarkChannelLinks"; +import remarkCustomEmoji, { + type CustomEmoji, +} from "@/shared/lib/remarkCustomEmoji"; +import remarkMentions from "@/shared/lib/remarkMentions"; +import remarkSpoilers from "@/shared/lib/remarkSpoilers"; + +import { messageLinkUrlTransform } from "./utils"; + +/** + * Parsed-markdown element cache. + * + * The message timeline's scroll container is keyed by channel id (see + * MessageTimeline — required so TanStack Router's scroll restoration never + * writes a stale scrollTop into a reused scroll node), so every channel + * switch remounts every row and `React.memo` cannot carry the react-markdown + * parse across the remount. react-markdown's `Markdown` is a plain + * synchronous hook-free function, so its element tree is a pure function of + * the parse inputs below and can be reused across mounts. Everything + * per-mount (channels, imeta lookup, navigation callbacks) flows through + * `MarkdownRuntimeContext`, read at render time — a cached element never + * captures per-mount state. The `components` map passed in must be + * module-stable for the same reason (see `getMarkdownComponents`). + * + * Recency-ordered via Map insertion order; capacity comfortably covers two + * window-ceiling channels' worth of rows. + */ +const MARKDOWN_NODE_CACHE_LIMIT = 1000; +const markdownNodeCache = new Map(); + +/** Workspace switches swap relays; drop parses keyed against the old + * workspace's mention/channel-name space (see `resetWorkspaceState`). */ +export function clearMarkdownNodeCache() { + markdownNodeCache.clear(); +} + +export type MarkdownParseInputs = { + channelNames?: string[]; + components: Components; + content: string; + customEmoji?: CustomEmoji[]; + interactive: boolean; + mediaInset: boolean; + mentionNames?: string[]; + searchQuery?: string; +}; + +function buildMarkdownElement(input: MarkdownParseInputs): React.ReactElement { + // biome-ignore lint/suspicious/noExplicitAny: PluggableList type not directly importable + const rehypePlugins: any[] = [rehypeImageGallery]; + if (input.searchQuery && input.searchQuery.trim().length >= 2) { + rehypePlugins.push([rehypeSearchHighlight, { query: input.searchQuery }]); + } + // Called as a plain function rather than rendered as : + // react-markdown's `Markdown` is synchronous and hook-free (the hook + // variant is `MarkdownHooks`), so this returns the parsed element tree + // directly, which is what lets it live in a module-level cache. + return ReactMarkdown({ + children: input.content, + components: input.components, + remarkPlugins: [ + remarkGfm, + remarkBreaks, + remarkSpoilers, + remarkMessageLinks, + [remarkMentions, { mentionNames: input.mentionNames }], + [remarkChannelLinks, { channelNames: input.channelNames }], + [remarkCustomEmoji, { customEmoji: input.customEmoji }], + // biome-ignore lint/suspicious/noExplicitAny: PluggableList type not directly importable + ] as any[], + rehypePlugins, + urlTransform: messageLinkUrlTransform, + }); +} + +export function renderCachedMarkdown( + input: MarkdownParseInputs, +): React.ReactElement { + // Search highlighting is transient and query-specific: parse fresh rather + // than churn the cache with per-query variants. + if (input.searchQuery && input.searchQuery.trim().length >= 2) { + return buildMarkdownElement(input); + } + // Everything that changes the parse output must be in the key. Arrays are + // keyed by value, not identity — callers rebuild them across mounts. + // Control-char separators keep adjacent segments from colliding. + const key = [ + input.interactive ? "i" : "", + input.mediaInset ? "m" : "", + input.mentionNames?.join("\u0001") ?? "", + input.channelNames?.join("\u0001") ?? "", + input.customEmoji + ?.map((emoji) => `${emoji.shortcode}\u0002${emoji.url}`) + .join("\u0001") ?? "", + input.content, + ].join("\u0000"); + + const hit = markdownNodeCache.get(key); + if (hit) { + markdownNodeCache.delete(key); + markdownNodeCache.set(key, hit); + return hit; + } + const element = buildMarkdownElement(input); + markdownNodeCache.set(key, element); + if (markdownNodeCache.size > MARKDOWN_NODE_CACHE_LIMIT) { + const oldest = markdownNodeCache.keys().next().value; + if (oldest !== undefined) { + markdownNodeCache.delete(oldest); + } + } + return element; +} diff --git a/desktop/src/shared/ui/markdown/runtimeContext.ts b/desktop/src/shared/ui/markdown/runtimeContext.ts new file mode 100644 index 000000000..a067125a3 --- /dev/null +++ b/desktop/src/shared/ui/markdown/runtimeContext.ts @@ -0,0 +1,27 @@ +import * as React from "react"; + +import type { MarkdownRuntime } from "./types"; + +/** + * Per-mount runtime (channels, imeta lookup, navigation callbacks) flows + * through context for the same reason as `VideoReviewMarkdownContext` in + * markdown.tsx: the component map must stay identity-stable. Routing runtime + * through context (read at render time) rather than a closed-over ref + * additionally makes the map module-stable across mounts, which is what + * allows the parsed markdown element cache (`nodeCache.ts`) to reuse element + * trees across the timeline's per-channel-switch remount without capturing + * stale per-mount state. + */ +const INERT_MARKDOWN_RUNTIME: MarkdownRuntime = { + channels: [], + onOpenChannel: () => {}, + onOpenMessageLink: () => {}, +}; + +export const MarkdownRuntimeContext = React.createContext( + INERT_MARKDOWN_RUNTIME, +); + +export function useMarkdownRuntime(): MarkdownRuntime { + return React.useContext(MarkdownRuntimeContext); +} diff --git a/desktop/tests/e2e/warm-switch-markdown.perf.ts b/desktop/tests/e2e/warm-switch-markdown.perf.ts new file mode 100644 index 000000000..c5df18806 --- /dev/null +++ b/desktop/tests/e2e/warm-switch-markdown.perf.ts @@ -0,0 +1,292 @@ +import { expect, test } from "@playwright/test"; + +import { installMockBridge } from "../helpers/bridge"; + +/** + * Warm-channel-switch benchmark. + * + * Measures the felt cost of switching INTO a channel whose messages are + * already in the React Query cache (the everyday alt-tab-between-channels + * motion). The timeline subtree is keyed by channel id — required so TanStack + * Router's scroll restoration never writes a stale scrollTop into a reused + * scroll node — so every switch unmounts and remounts all rows, and each + * `MessageRow` re-runs the synchronous react-markdown parse pipeline from + * scratch. This spec is the instrument for that cost. + * + * TWO SCENARIOS, one per axis of the cost: + * plain-text — `deep-history` (300 seeded one-line rows): isolates the + * per-row remount floor at the window ceiling. + * markdown — `random` + 60 injected markdown-heavy rows (code fences, + * tables, lists, mentions, links): isolates the parse cost the + * markdown cache is meant to remove. + * + * WHAT A "SWITCH" MEASURES: performance.now() immediately before an in-page + * .click() on the sidebar link, until (chat title flipped) AND (>= 1 message + * row committed) AND (no [data-render-pending="true"], i.e. the deferred + * timeline snapshot caught up to the live one) AND a double-rAF so a frame + * actually painted. The click and the polling both run in-page so CDP + * round-trip latency never pollutes the numbers. Longtask totals are captured + * per switch as the "UI froze" axis (see cold-switch-longtask.perf.ts for the + * rationale). + * + * WARM means every measured entry is a RE-entry: each scenario does one + * untimed round-trip first so both channels' queries are cached and code + * paths are jitted. 4x CPU throttle for the same reason as the cold spec — + * absolute ms are not portable across machines, but before/after deltas on + * the same machine are. + * + * Run it (from desktop/): + * pnpm build + * npx playwright test --config=playwright.perf.config.ts warm-switch-markdown.perf.ts + * + * NOTE: the perf web server reuses an existing server on :4173 — if one is + * already running, kill it or make sure `dist/` is freshly built, otherwise + * you measure stale code. + */ + +const MEASURED_SWITCHES = 8; +const THROTTLE_RATE = 4; +const MARKDOWN_MESSAGE_COUNT = 60; + +/** One representative agent-style message: fence, table, list, mention, + * emphasis, inline code, and a link — the mix real Buzz channels carry. */ +function markdownBody(index: number): string { + return [ + `**Update ${index}** from the build agent — _step ${index} of ${MARKDOWN_MESSAGE_COUNT}_ :tada:`, + "", + "```rust", + `fn step_${index}() -> Result {`, + ' let plan = load_plan("release")?;', + ` plan.execute(${index})`, + "}", + "```", + "", + "| check | result | took |", + "|-------|--------|------|", + `| clippy | ok | ${index}ms |`, + `| fmt | ok | ${index + 1}ms |`, + "", + `- [x] compile stage ${index}`, + "- [ ] publish artifacts", + `- see [pipeline](https://example.com/build/${index}) or ask @alice`, + "", + `Inline \`cargo build -p step-${index}\` finished.`, + ].join("\n"); +} + +type SwitchSample = { + ms: number; + longtaskTotal: number; + longtaskMax: number; + longtaskCount: number; +}; + +function median(values: number[]): number { + const sorted = [...values].sort((a, b) => a - b); + const mid = Math.floor(sorted.length / 2); + return sorted.length % 2 === 0 + ? (sorted[mid - 1] + sorted[mid]) / 2 + : sorted[mid]; +} + +async function waitForMockLiveSubscription( + page: import("@playwright/test").Page, + channelName: string, +) { + await expect + .poll(() => + page.evaluate( + (ch) => + window.__BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?.({ + channelName: ch, + }) ?? false, + channelName, + ), + ) + .toBe(true); +} + +/** Click the sidebar link and poll — all in-page — until the target channel's + * rows are committed, the deferred snapshot has caught up, and a frame + * painted. Returns wall-clock ms plus the longtasks observed in the window. */ +async function measureSwitch( + page: import("@playwright/test").Page, + input: { targetTestId: string; targetTitle: string; rowSelector: string }, +): Promise { + return page.evaluate(async (args) => { + const store = window as unknown as { __LONGTASKS__: number[] }; + store.__LONGTASKS__ = []; + const link = document.querySelector( + `[data-testid="${args.targetTestId}"]`, + ); + if (!link) throw new Error(`missing sidebar link ${args.targetTestId}`); + + const start = performance.now(); + link.click(); + + await new Promise((resolve, reject) => { + const deadline = start + 30_000; + const check = () => { + const title = document.querySelector( + '[data-testid="chat-title"]', + )?.textContent; + const ready = + title === args.targetTitle && + document.querySelector(args.rowSelector) !== null && + document.querySelector('[data-render-pending="true"]') === null; + if (ready) { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + return; + } + if (performance.now() > deadline) { + reject(new Error(`switch to ${args.targetTitle} timed out`)); + return; + } + requestAnimationFrame(check); + }; + requestAnimationFrame(check); + }); + + const elapsed = performance.now() - start; + const tasks = store.__LONGTASKS__ ?? []; + return { + ms: elapsed, + longtaskTotal: tasks.reduce((sum, duration) => sum + duration, 0), + longtaskMax: tasks.length ? Math.max(...tasks) : 0, + longtaskCount: tasks.length, + }; + }, input); +} + +async function runScenario( + page: import("@playwright/test").Page, + input: { + label: string; + targetTestId: string; + targetTitle: string; + rowSelector: string; + }, +): Promise { + const back = { + targetTestId: "channel-general", + targetTitle: "general", + rowSelector: "[data-message-id]", + }; + + // Untimed warmup round-trip: caches both channels' queries. + await measureSwitch(page, input); + await measureSwitch(page, back); + + const samples: SwitchSample[] = []; + for (let run = 0; run < MEASURED_SWITCHES; run += 1) { + samples.push(await measureSwitch(page, input)); + await measureSwitch(page, back); + } + + const times = samples.map((sample) => sample.ms); + const longtaskTotals = samples.map((sample) => sample.longtaskTotal); + /* eslint-disable no-console */ + console.log(`\n=== WARM SWITCH: ${input.label} ===`); + console.log(`CPU throttle: ${THROTTLE_RATE}x`); + console.log( + `per-switch wall ms: [${times.map((v) => v.toFixed(1)).join(", ")}]`, + ); + console.log( + `per-switch longtask ms: [${longtaskTotals.map((v) => v.toFixed(1)).join(", ")}]`, + ); + console.log(`MEDIAN wall ms: ${median(times).toFixed(1)}`); + console.log( + `MEDIAN longtask total: ${median(longtaskTotals).toFixed(1)}ms`, + ); + console.log( + `worst single longtask: ${Math.max(...samples.map((sample) => sample.longtaskMax)).toFixed(1)}ms`, + ); + /* eslint-enable no-console */ + return samples; +} + +test("MEASURE: warm channel-switch cost (plain 300-row + markdown-heavy)", async ({ + page, +}) => { + test.setTimeout(300_000); + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + + // Arm the longtask observer; addInitScript applies on next navigation. + await page.addInitScript(() => { + const store = window as unknown as { __LONGTASKS__?: number[] }; + store.__LONGTASKS__ = []; + new PerformanceObserver((list) => { + for (const entry of list.getEntries()) { + store.__LONGTASKS__?.push(entry.duration); + } + }).observe({ type: "longtask", buffered: true }); + }); + await page.reload(); + await page.waitForFunction( + () => + typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function" && + Array.isArray( + (window as unknown as { __LONGTASKS__?: number[] }).__LONGTASKS__, + ), + ); + + // Seed `random` with markdown-heavy rows. Live emits need an active + // subscription, so enter the channel first; the mock store keeps the rows + // for every later re-entry via get_channel_window. + await page.getByTestId("channel-random").click(); + await expect(page.getByTestId("chat-title")).toHaveText("random"); + await waitForMockLiveSubscription(page, "random"); + await page.evaluate( + ({ count, bodies }) => { + const base = Math.floor(Date.now() / 1000) - count - 10; + for (let index = 0; index < count; index += 1) { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "random", + content: bodies[index], + createdAt: base + index, + }); + } + }, + { + count: MARKDOWN_MESSAGE_COUNT, + bodies: Array.from({ length: MARKDOWN_MESSAGE_COUNT }, (_, index) => + markdownBody(index), + ), + }, + ); + // All injected rows committed before anything is timed. + await expect( + page.locator(`text=Update ${MARKDOWN_MESSAGE_COUNT - 1}`).first(), + ).toBeVisible(); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + + const client = await page.context().newCDPSession(page); + await client.send("Emulation.setCPUThrottlingRate", { rate: THROTTLE_RATE }); + + const plain = await runScenario(page, { + label: "plain-text x300 (deep-history)", + targetTestId: "channel-deep-history", + targetTitle: "deep-history", + rowSelector: '[data-message-id^="mock-deep-history-"]', + }); + const markdown = await runScenario(page, { + label: `markdown-heavy x${MARKDOWN_MESSAGE_COUNT} (random)`, + targetTestId: "channel-random", + targetTitle: "random", + rowSelector: "[data-message-id]", + }); + + await client.send("Emulation.setCPUThrottlingRate", { rate: 1 }); + + // Instrument, not a gate: assert the harness measured real work. + expect(plain.length).toBe(MEASURED_SWITCHES); + expect(markdown.length).toBe(MEASURED_SWITCHES); + expect(plain.every((sample) => sample.ms > 0)).toBe(true); + expect(markdown.every((sample) => sample.ms > 0)).toBe(true); +}); From 7ae7f65fe386ad45cd2cfe63ed6c33ab130ace76 Mon Sep 17 00:00:00 2001 From: Noah Date: Tue, 7 Jul 2026 18:10:09 -0400 Subject: [PATCH 2/2] perf(desktop): harden markdown parse cache per review findings Address the adversarial-review findings on the parse cache: skip the parse entirely when a config-nudge suppresses the prose (it previously parsed content that was never rendered), bypass the cache for oversized content so large pastes cannot pin memory, length-prefix cache-key segments so relay-controlled names/URLs cannot forge segment boundaries, and single-source the components-variant token so the map partitioning and key partitioning cannot drift. Adds a deterministic CI gate (markdown-parse-cache.spec.ts) asserting warm channel switches perform zero fresh parses, via a parse counter exposed through the e2e bridge. Extracts MarkdownVideoPlayer to keep markdown.tsx under the file-size limit, and corrects the benchmark's row-count label. Co-Authored-By: Claude Fable 5 --- desktop/playwright.config.ts | 1 + desktop/src/shared/ui/markdown.tsx | 117 ++++++------------ .../ui/markdown/MarkdownVideoPlayer.tsx | 59 +++++++++ .../src/shared/ui/markdown/nodeCache.test.mjs | 35 +++++- desktop/src/shared/ui/markdown/nodeCache.ts | 71 ++++++++--- desktop/src/testing/e2eBridge.ts | 3 + .../tests/e2e/markdown-parse-cache.spec.ts | 73 +++++++++++ .../tests/e2e/warm-switch-markdown.perf.ts | 7 +- 8 files changed, 265 insertions(+), 101 deletions(-) create mode 100644 desktop/src/shared/ui/markdown/MarkdownVideoPlayer.tsx create mode 100644 desktop/tests/e2e/markdown-parse-cache.spec.ts diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 440d8fac5..83f00c853 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -71,6 +71,7 @@ export default defineConfig({ "**/channel-dense-second-reach.spec.ts", "**/channel-window-mock-paging.spec.ts", "**/live-broadcast-reply-timeline.spec.ts", + "**/markdown-parse-cache.spec.ts", "**/overscroll-boundary.spec.ts", "**/cold-switch-longtask.perf.ts", "**/timeline-no-shift.spec.ts", diff --git a/desktop/src/shared/ui/markdown.tsx b/desktop/src/shared/ui/markdown.tsx index 5d70bd121..ee510677a 100644 --- a/desktop/src/shared/ui/markdown.tsx +++ b/desktop/src/shared/ui/markdown.tsx @@ -72,14 +72,9 @@ import { useMarkdownRuntime, } from "./markdown/runtimeContext"; import { resolveFileCard } from "./markdownFileCard"; -import type { - ImetaEntry, - MarkdownProps, - MarkdownRuntime, -} from "./markdown/types"; +import type { MarkdownProps, MarkdownRuntime } from "./markdown/types"; import { SpoilerInline } from "./markdown/SpoilerInline"; import { - aspectRatioFromDim, dimensionsFromDim, getDecodedImageDimensions, imageReserveStyle, @@ -89,59 +84,10 @@ import { useFrozenImageReserve, useStableArray, } from "./markdown/utils"; -import { VideoPlayer, type VideoReviewContext } from "./VideoPlayer"; - -/** - * Video review context flows through React context instead of - * `createMarkdownComponents` arguments. The component map must keep a stable - * identity across re-renders: a new map means new element types, which makes - * React unmount and remount every rendered node — including `