Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,10 @@
"@xterm/xterm": "^6.0.0",
"class-variance-authority": "^0.7.1",
"effect": "catalog:",
"highlight.js": "^11.11.1",
"lucide-react": "^0.564.0",
"react": "^19.0.0",
"react-dom": "^19.0.0",
"react-markdown": "^10.1.0",
"rehype-highlight": "^7.0.2",
"remark-gfm": "^4.0.1",
"tailwind-merge": "^3.4.0"
},
Expand Down
224 changes: 214 additions & 10 deletions apps/web/src/components/ChatMarkdown.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,207 @@
import { memo, useMemo } from "react";
import {
getSharedHighlighter,
type DiffsHighlighter,
type SupportedLanguages,
} from "@pierre/diffs";
import { CheckIcon, CopyIcon } from "lucide-react";
import {
Children,
Suspense,
isValidElement,
use,
useCallback,
memo,
useEffect,
useMemo,
useRef,
useState,
type ReactNode,
} from "react";
import type { Components } from "react-markdown";
import ReactMarkdown from "react-markdown";
import rehypeHighlight from "rehype-highlight";
import remarkGfm from "remark-gfm";
import { readNativeApi } from "../nativeApi";
import { resolveDiffThemeName, type DiffThemeName } from "../lib/diffRendering";
import { fnv1a32 } from "../lib/diffRendering";
import { LRUCache } from "../lib/lruCache";
import { useTheme } from "../hooks/useTheme";
import { resolveMarkdownFileLinkTarget } from "../markdown-links";
import { readNativeApi } from "../nativeApi";
import { preferredTerminalEditor } from "../terminal-links";

interface ChatMarkdownProps {
text: string;
cwd: string | undefined;
isStreaming?: boolean;
}

const CODE_FENCE_LANGUAGE_REGEX = /(?:^|\s)language-([^\s]+)/;
const MAX_HIGHLIGHT_CACHE_ENTRIES = 500;
const MAX_HIGHLIGHT_CACHE_MEMORY_BYTES = 50 * 1024 * 1024;
const highlightedCodeCache = new LRUCache<string>(
MAX_HIGHLIGHT_CACHE_ENTRIES,
MAX_HIGHLIGHT_CACHE_MEMORY_BYTES,
);
const highlighterPromiseCache = new Map<string, Promise<DiffsHighlighter>>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium components/ChatMarkdown.tsx:44

Consider handling rejected promises in getHighlighterPromise — if getSharedHighlighter fails, the rejected promise stays cached forever, breaking that language until page reload. Adding a .catch() to delete the entry on failure would fix this.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ChatMarkdown.tsx around line 44:

Consider handling rejected promises in `getHighlighterPromise` — if `getSharedHighlighter` fails, the rejected promise stays cached forever, breaking that language until page reload. Adding a `.catch()` to delete the entry on failure would fix this.

Evidence trail:
apps/web/src/components/ChatMarkdown.tsx lines 44, 94-105 at REVIEWED_COMMIT: `highlighterPromiseCache` is a Map that caches promises. Line 103 stores the promise immediately via `highlighterPromiseCache.set(language, promise)`. Lines 95-96 return the cached promise without checking rejection state. No `.catch()` handler exists to remove failed promises from the cache.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low components/ChatMarkdown.tsx:44

highlighterPromiseCache can grow unbounded with arbitrary language keys (including partials while streaming). Consider validating language against SupportedLanguages before caching, or replacing the Map with an LRUCache.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ChatMarkdown.tsx around line 44:

`highlighterPromiseCache` can grow unbounded with arbitrary language keys (including partials while streaming). Consider validating `language` against `SupportedLanguages` before caching, or replacing the `Map` with an `LRUCache`.

Evidence trail:
apps/web/src/components/ChatMarkdown.tsx: Line 44 shows `highlighterPromiseCache = new Map<string, Promise<DiffsHighlighter>>()` (unbounded Map). Lines 38-41 show `highlightedCodeCache = new LRUCache<string>(MAX_HIGHLIGHT_CACHE_ENTRIES, MAX_HIGHLIGHT_CACHE_MEMORY_BYTES)` (bounded). Lines 100-111 show `getHighlighterPromise` adds entries without any cleanup/eviction. Line 108 uses `language as SupportedLanguages` - type assertion only, no runtime validation. apps/web/src/lib/lruCache.ts shows LRUCache implementation with `evictIfNeeded` - this is not used for highlighterPromiseCache.


function extractFenceLanguage(className: string | undefined): string {
const match = className?.match(CODE_FENCE_LANGUAGE_REGEX);
return match?.[1] ?? "text";
}

function nodeToPlainText(node: ReactNode): string {
if (typeof node === "string" || typeof node === "number") {
return String(node);
}
if (Array.isArray(node)) {
return node.map((child) => nodeToPlainText(child)).join("");
}
if (isValidElement<{ children?: ReactNode }>(node)) {
return nodeToPlainText(node.props.children);
}
return "";
}

function extractCodeBlock(
children: ReactNode,
): { className: string | undefined; code: string } | null {
const childNodes = Children.toArray(children);
if (childNodes.length !== 1) {
return null;
}

const onlyChild = childNodes[0];
if (
!isValidElement<{ className?: string; children?: ReactNode }>(onlyChild) ||
onlyChild.type !== "code"
) {
return null;
}

return {
className: onlyChild.props.className,
code: nodeToPlainText(onlyChild.props.children),
};
}

function ChatMarkdown({ text, cwd }: ChatMarkdownProps) {
function createHighlightCacheKey(code: string, language: string, themeName: DiffThemeName): string {
return `${fnv1a32(code).toString(36)}:${code.length}:${language}:${themeName}`;
}

function estimateHighlightedSize(html: string, code: string): number {
return Math.max(html.length * 2, code.length * 3);
}

function getHighlighterPromise(language: string): Promise<DiffsHighlighter> {
const cached = highlighterPromiseCache.get(language);
if (cached) return cached;

const promise = getSharedHighlighter({
themes: [resolveDiffThemeName("dark"), resolveDiffThemeName("light")],
langs: [language as SupportedLanguages],
preferredHighlighter: "shiki-js",
});
highlighterPromiseCache.set(language, promise);
return promise;
}

function MarkdownCodeBlock({ code, children }: { code: string; children: ReactNode }) {
const [copied, setCopied] = useState(false);
const copiedTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
const handleCopy = useCallback(() => {
if (typeof navigator === "undefined" || navigator.clipboard == null) {
return;
}
void navigator.clipboard
.writeText(code)
.then(() => {
if (copiedTimerRef.current != null) {
clearTimeout(copiedTimerRef.current);
}
setCopied(true);
copiedTimerRef.current = setTimeout(() => {
setCopied(false);
copiedTimerRef.current = null;
}, 1200);
})
.catch(() => undefined);
}, [code]);

useEffect(
() => () => {
if (copiedTimerRef.current != null) {
clearTimeout(copiedTimerRef.current);
copiedTimerRef.current = null;
}
},
[],
);

return (
<div className="chat-markdown-codeblock">
<button
type="button"
className="chat-markdown-copy-button"
onClick={handleCopy}
title={copied ? "Copied" : "Copy code"}
aria-label={copied ? "Copied" : "Copy code"}
>
{copied ? <CheckIcon className="size-3" /> : <CopyIcon className="size-3" />}
</button>
{children}
</div>
);
}

interface SuspenseShikiCodeBlockProps {
className: string | undefined;
code: string;
themeName: DiffThemeName;
isStreaming: boolean;
}

function SuspenseShikiCodeBlock({
className,
code,
themeName,
isStreaming,
}: SuspenseShikiCodeBlockProps) {
const language = extractFenceLanguage(className);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High components/ChatMarkdown.tsx:168

Calling useMemo and useEffect after the conditional return on line 172 violates the Rules of Hooks. If a re-render hits the cache after an initial miss, React will throw "Rendered fewer hooks than during the previous render." Consider moving all hooks before the conditional return.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ChatMarkdown.tsx around line 168:

Calling `useMemo` and `useEffect` after the conditional return on line 172 violates the Rules of Hooks. If a re-render hits the cache after an initial miss, React will throw "Rendered fewer hooks than during the previous render." Consider moving all hooks before the conditional return.

Evidence trail:
apps/web/src/components/ChatMarkdown.tsx lines 160-199 at REVIEWED_COMMIT. Line 172 shows conditional return `if (cachedHighlightedHtml != null)`. Lines 180-183 show `useMemo` call after the conditional. Lines 185-193 show `useEffect` call after the conditional. React Rules of Hooks documentation: https://react.dev/reference/rules/rules-of-hooks

const cacheKey = createHighlightCacheKey(code, language, themeName);
const cachedHighlightedHtml = !isStreaming ? highlightedCodeCache.get(cacheKey) : null;

if (cachedHighlightedHtml != null) {
return (
<div
className="chat-markdown-shiki"
dangerouslySetInnerHTML={{ __html: cachedHighlightedHtml }}
/>
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conditional return before hooks violates Rules of Hooks

High Severity

SuspenseShikiCodeBlock has a conditional early return before useMemo and useEffect are called. While React 19's use() is allowed conditionally, useMemo and useEffect are not — they must be called in the same order on every render. When a code block transitions from cache-miss (all hooks called) to cache-hit (early return, no hooks called), React will throw "Rendered fewer hooks than expected" and crash the component tree.

Additional Locations (1)

Fix in Cursor Fix in Web


const highlighter = use(getHighlighterPromise(language));
const highlightedHtml = useMemo(
() => highlighter.codeToHtml(code, { lang: language, theme: themeName }),
[code, highlighter, language, themeName],
);

useEffect(() => {
if (!isStreaming) {
highlightedCodeCache.set(
cacheKey,
highlightedHtml,
estimateHighlightedSize(highlightedHtml, code),
);
}
}, [cacheKey, code, highlightedHtml, isStreaming]);

return (
<div className="chat-markdown-shiki" dangerouslySetInnerHTML={{ __html: highlightedHtml }} />
);
}

function ChatMarkdown({ text, cwd, isStreaming = false }: ChatMarkdownProps) {
const { resolvedTheme } = useTheme();
const diffThemeName = resolveDiffThemeName(resolvedTheme);
const markdownComponents = useMemo<Components>(
() => ({
a({ node: _node, href, ...props }) {
Expand All @@ -38,17 +227,32 @@ function ChatMarkdown({ text, cwd }: ChatMarkdownProps) {
/>
);
},
pre({ node: _node, children, ...props }) {
const codeBlock = extractCodeBlock(children);
if (!codeBlock) {
return <pre {...props}>{children}</pre>;
}

return (
<MarkdownCodeBlock code={codeBlock.code}>
<Suspense fallback={<pre {...props}>{children}</pre>}>
<SuspenseShikiCodeBlock
className={codeBlock.className}
code={codeBlock.code}
themeName={diffThemeName}
isStreaming={isStreaming}
/>
</Suspense>
</MarkdownCodeBlock>
);
},
}),
[cwd],
[cwd, diffThemeName, isStreaming],
);

return (
<div className="chat-markdown w-full min-w-0 text-sm leading-relaxed text-foreground/80">
<ReactMarkdown
remarkPlugins={[remarkGfm]}
rehypePlugins={[[rehypeHighlight, { detect: true, ignoreMissing: true }]]}
components={markdownComponents}
>
<ReactMarkdown remarkPlugins={[remarkGfm]} components={markdownComponents}>
{text}
</ReactMarkdown>
</div>
Expand Down
6 changes: 5 additions & 1 deletion apps/web/src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2759,7 +2759,11 @@ const MessagesTimeline = memo(function MessagesTimeline({
</div>
)}
<div className="min-w-0 px-1 py-0.5">
<ChatMarkdown text={messageText} cwd={markdownCwd} />
<ChatMarkdown
text={messageText}
cwd={markdownCwd}
isStreaming={Boolean(row.message.streaming)}
/>
{(() => {
const turnSummary = turnDiffSummaryByAssistantMessageId.get(row.message.id);
if (!turnSummary) return null;
Expand Down
58 changes: 52 additions & 6 deletions apps/web/src/components/DiffPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,54 @@ import { parseDiffRouteSearch, stripDiffSearchParams } from "../diffRouteSearch"
import { isElectron } from "../env";
import { useTheme } from "../hooks/useTheme";
import { buildPatchCacheKey } from "../lib/diffRendering";
import { resolveDiffThemeName } from "../lib/diffRendering";
import { useTurnDiffSummaries } from "../hooks/useTurnDiffSummaries";
import { useStore } from "../store";
import { ToggleGroup, Toggle } from "./ui/toggle-group";

type DiffRenderMode = "stacked" | "split";
type DiffThemeType = "light" | "dark";

function resolveDiffThemeName(theme: "light" | "dark") {
return theme === "dark" ? "pierre-dark" : "pierre-light";
const DIFF_PANEL_UNSAFE_CSS = `
[data-diffs-header],
[data-diff],
[data-file],
[data-error-wrapper],
[data-virtualizer-buffer] {
--diffs-bg: color-mix(in srgb, var(--card) 90%, var(--background)) !important;
--diffs-light-bg: color-mix(in srgb, var(--card) 90%, var(--background)) !important;
--diffs-dark-bg: color-mix(in srgb, var(--card) 90%, var(--background)) !important;
--diffs-token-light-bg: transparent;
--diffs-token-dark-bg: transparent;

--diffs-bg-context-override: color-mix(in srgb, var(--background) 97%, var(--foreground));
--diffs-bg-hover-override: color-mix(in srgb, var(--background) 94%, var(--foreground));
--diffs-bg-separator-override: color-mix(in srgb, var(--background) 95%, var(--foreground));
--diffs-bg-buffer-override: color-mix(in srgb, var(--background) 90%, var(--foreground));

--diffs-bg-addition-override: color-mix(in srgb, var(--background) 92%, var(--success));
--diffs-bg-addition-number-override: color-mix(in srgb, var(--background) 88%, var(--success));
--diffs-bg-addition-hover-override: color-mix(in srgb, var(--background) 85%, var(--success));
--diffs-bg-addition-emphasis-override: color-mix(in srgb, var(--background) 80%, var(--success));

--diffs-bg-deletion-override: color-mix(in srgb, var(--background) 92%, var(--destructive));
--diffs-bg-deletion-number-override: color-mix(in srgb, var(--background) 88%, var(--destructive));
--diffs-bg-deletion-hover-override: color-mix(in srgb, var(--background) 85%, var(--destructive));
--diffs-bg-deletion-emphasis-override: color-mix(
in srgb,
var(--background) 80%,
var(--destructive)
);

background-color: var(--diffs-bg) !important;
}

[data-file-info] {
background-color: color-mix(in srgb, var(--card) 94%, var(--foreground)) !important;
border-block-color: var(--border) !important;
color: var(--foreground) !important;
}
`;

type RenderablePatch =
| {
Expand Down Expand Up @@ -425,7 +463,7 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
return (
<div
className={cn(
"flex h-full min-w-0 flex-col bg-card",
"flex h-full min-w-0 flex-col bg-background",
mode === "inline"
? "w-[42vw] min-w-[360px] max-w-[560px] shrink-0 border-l border-border"
: "w-full",
Expand All @@ -449,7 +487,10 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
</div>
) : (
<>
<div ref={patchViewportRef} className="min-h-0 min-w-0 flex-1 overflow-hidden">
<div
ref={patchViewportRef}
className="diff-panel-viewport min-h-0 min-w-0 flex-1 overflow-hidden"
>
{checkpointDiffError && !renderablePatch && (
<div className="px-3">
<p className="mb-2 text-[11px] text-red-500/80">{checkpointDiffError}</p>
Expand All @@ -467,7 +508,7 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
</div>
) : renderablePatch.kind === "files" ? (
<Virtualizer
className="h-full min-h-0 overflow-auto px-3 py-2"
className="diff-render-surface h-full min-h-0 overflow-auto px-3 py-2"
config={{
overscrollSize: 600,
intersectionObserverMargin: 1200,
Expand All @@ -478,14 +519,19 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
const fileKey = buildFileDiffRenderKey(fileDiff);
const themedFileKey = `${fileKey}:${resolvedTheme}`;
return (
<div key={themedFileKey} data-diff-file-path={filePath} className="rounded-md">
<div
key={themedFileKey}
data-diff-file-path={filePath}
className="diff-render-file rounded-md"
>
<FileDiff
fileDiff={fileDiff}
options={{
diffStyle: diffRenderMode === "split" ? "split" : "unified",
lineDiffType: "none",
theme: resolveDiffThemeName(resolvedTheme),
themeType: resolvedTheme as DiffThemeType,
unsafeCSS: DIFF_PANEL_UNSAFE_CSS,
}}
/>
</div>
Expand Down
7 changes: 2 additions & 5 deletions apps/web/src/components/DiffWorkerPoolProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@ import { WorkerPoolContextProvider, useWorkerPool } from "@pierre/diffs/react";
import DiffsWorker from "@pierre/diffs/worker/worker.js?worker";
import { useEffect, useMemo, type ReactNode } from "react";
import { useTheme } from "../hooks/useTheme";
import { resolveDiffThemeName, type DiffThemeName } from "../lib/diffRendering";

function resolveDiffThemeName(theme: "light" | "dark") {
return theme === "dark" ? "pierre-dark" : "pierre-light";
}

function DiffWorkerThemeSync({ themeName }: { themeName: "pierre-light" | "pierre-dark" }) {
function DiffWorkerThemeSync({ themeName }: { themeName: DiffThemeName }) {
const workerPool = useWorkerPool();

useEffect(() => {
Expand Down
Loading