diff --git a/bun.lock b/bun.lock index 753085edc..c1f630d13 100644 --- a/bun.lock +++ b/bun.lock @@ -1,6 +1,5 @@ { "lockfileVersion": 1, - "configVersion": 0, "workspaces": { "": { "name": "mux", @@ -32,6 +31,7 @@ "ai": "^5.0.101", "ai-tokenizer": "^1.0.4", "chalk": "^5.6.2", + "comlink": "^4.4.2", "commander": "^14.0.2", "cors": "^2.8.5", "crc-32": "^1.2.2", @@ -1737,6 +1737,8 @@ "combined-stream": ["combined-stream@1.0.8", "", { "dependencies": { "delayed-stream": "~1.0.0" } }, "sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg=="], + "comlink": ["comlink@4.4.2", "", {}, "sha512-OxGdvBmJuNKSCMO4NTl1L47VRp6xn2wG4F/2hYzB6tiCb709otOxtEYCSvK80PtjODfXXZu8ds+Nw5kVCjqd2g=="], + "comma-separated-tokens": ["comma-separated-tokens@2.0.3", "", {}, "sha512-Fu4hJdvzeylCfQPp9SGWidpzrMs7tTrlu6Vb8XGaRGck8QSNZJJp538Wrb60Lax4fPwR64ViY468OIUTbRlGZg=="], "commander": ["commander@9.5.0", "", {}, "sha512-KRs7WVDKg86PWiuAqhDrAQnTXZKraVcCc6vFdL14qrZ/DcWwuRo7VoiYXalXO7S5GKpqYiVEwCbgFDfxNHKJBQ=="], diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 9abdccd37..4d25fba4d 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -117,6 +117,8 @@ Avoid mock-heavy tests that verify implementation details rather than behavior. - Parent components own localStorage interactions; children announce intent only. - Use `usePersistedState`/`readPersistedState`/`updatePersistedState` helpers—never call `localStorage` directly. +- When a component needs to read persisted state it doesn't own (to avoid layout flash), use `readPersistedState` in `useState` initializer: `useState(() => readPersistedState(key, default))`. +- When multiple components need the same persisted value, use `usePersistedState` with identical keys and `{ listener: true }` for automatic cross-component sync. - Avoid destructuring props in function signatures; access via `props.field` to keep rename-friendly code. ## Module Imports diff --git a/eslint.config.mjs b/eslint.config.mjs index 5eb19775c..3f05f32dd 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -490,6 +490,28 @@ export default defineConfig([ ], }, }, + { + // Shiki must only be imported in the highlight worker to avoid blocking main thread + // Type-only imports are allowed (erased at compile time) + files: ["src/**/*.ts", "src/**/*.tsx"], + ignores: ["src/browser/workers/highlightWorker.ts"], + rules: { + "no-restricted-imports": [ + "error", + { + patterns: [ + { + group: ["shiki"], + importNamePattern: "^(?!type\\s)", + allowTypeImports: true, + message: + "Shiki must only be imported in highlightWorker.ts to avoid blocking the main thread. Use highlightCode() from highlightWorkerClient.ts instead.", + }, + ], + }, + ], + }, + }, { // Renderer process (frontend) architectural boundary - prevent Node.js API usage files: ["src/**/*.ts", "src/**/*.tsx"], diff --git a/package.json b/package.json index 441506da3..0ed6aa134 100644 --- a/package.json +++ b/package.json @@ -72,6 +72,7 @@ "ai": "^5.0.101", "ai-tokenizer": "^1.0.4", "chalk": "^5.6.2", + "comlink": "^4.4.2", "commander": "^14.0.2", "cors": "^2.8.5", "crc-32": "^1.2.2", diff --git a/src/browser/components/AIView.tsx b/src/browser/components/AIView.tsx index 19369c654..1da709782 100644 --- a/src/browser/components/AIView.tsx +++ b/src/browser/components/AIView.tsx @@ -5,7 +5,11 @@ import { InterruptedBarrier } from "./Messages/ChatBarrier/InterruptedBarrier"; import { StreamingBarrier } from "./Messages/ChatBarrier/StreamingBarrier"; import { RetryBarrier } from "./Messages/ChatBarrier/RetryBarrier"; import { PinnedTodoList } from "./PinnedTodoList"; -import { getAutoRetryKey, VIM_ENABLED_KEY } from "@/common/constants/storage"; +import { + getAutoRetryKey, + VIM_ENABLED_KEY, + RIGHT_SIDEBAR_TAB_KEY, +} from "@/common/constants/storage"; import { WORKSPACE_DEFAULTS } from "@/constants/workspaceDefaults"; import { ChatInput, type ChatInputAPI } from "./ChatInput/index"; import { RightSidebar, type TabType } from "./RightSidebar"; @@ -22,7 +26,7 @@ import { ProviderOptionsProvider } from "@/browser/contexts/ProviderOptionsConte import { formatKeybind, KEYBINDS } from "@/browser/utils/ui/keybinds"; import { useAutoScroll } from "@/browser/hooks/useAutoScroll"; import { useOpenTerminal } from "@/browser/hooks/useOpenTerminal"; -import { usePersistedState } from "@/browser/hooks/usePersistedState"; +import { readPersistedState, usePersistedState } from "@/browser/hooks/usePersistedState"; import { useThinking } from "@/browser/contexts/ThinkingContext"; import { useWorkspaceState, @@ -75,8 +79,11 @@ const AIViewInner: React.FC = ({ const chatAreaRef = useRef(null); // Track active tab to conditionally enable resize functionality - // RightSidebar notifies us of tab changes via onTabChange callback - const [activeTab, setActiveTab] = useState("costs"); + // Initialize from persisted value to avoid layout flash; RightSidebar owns the state + // and notifies us of changes via onTabChange callback + const [activeTab, setActiveTab] = useState(() => + readPersistedState(RIGHT_SIDEBAR_TAB_KEY, "costs") + ); const isReviewTabActive = activeTab === "review"; diff --git a/src/browser/components/Messages/MarkdownComponents.tsx b/src/browser/components/Messages/MarkdownComponents.tsx index 9f1072940..f08aa812a 100644 --- a/src/browser/components/Messages/MarkdownComponents.tsx +++ b/src/browser/components/Messages/MarkdownComponents.tsx @@ -1,14 +1,9 @@ import type { ReactNode } from "react"; import React, { useState, useEffect } from "react"; import { Mermaid } from "./Mermaid"; -import { - getShikiHighlighter, - mapToShikiLang, - SHIKI_DARK_THEME, - SHIKI_LIGHT_THEME, -} from "@/browser/utils/highlighting/shikiHighlighter"; -import { useTheme } from "@/browser/contexts/ThemeContext"; +import { highlightCode } from "@/browser/utils/highlighting/highlightWorkerClient"; import { extractShikiLines } from "@/browser/utils/highlighting/shiki-shared"; +import { useTheme } from "@/browser/contexts/ThemeContext"; import { CopyButton } from "@/browser/components/ui/CopyButton"; interface CodeProps { @@ -57,37 +52,13 @@ const CodeBlock: React.FC = ({ code, language }) => { useEffect(() => { let cancelled = false; const isLight = themeMode === "light" || themeMode === "solarized-light"; - const shikiTheme = isLight ? SHIKI_LIGHT_THEME : SHIKI_DARK_THEME; + const theme = isLight ? "light" : "dark"; setHighlightedLines(null); async function highlight() { try { - const highlighter = await getShikiHighlighter(); - const shikiLang = mapToShikiLang(language); - - // Load language on-demand if not already loaded - // This is race-safe: concurrent loads of the same language are idempotent - const loadedLangs = highlighter.getLoadedLanguages(); - if (!loadedLangs.includes(shikiLang)) { - try { - // TypeScript doesn't know shikiLang is valid, but we handle errors gracefully - // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument - await highlighter.loadLanguage(shikiLang as any); - } catch { - // Language not available in Shiki bundle - fall back to plain text - console.warn(`Language '${shikiLang}' not available in Shiki, using plain text`); - if (!cancelled) { - setHighlightedLines(null); - } - return; - } - } - - const html = highlighter.codeToHtml(code, { - lang: shikiLang, - theme: shikiTheme, - }); + const html = await highlightCode(code, language, theme); if (!cancelled) { const lines = extractShikiLines(html); diff --git a/src/browser/components/RightSidebar.tsx b/src/browser/components/RightSidebar.tsx index e796e5b9c..0b6722fc8 100644 --- a/src/browser/components/RightSidebar.tsx +++ b/src/browser/components/RightSidebar.tsx @@ -1,4 +1,5 @@ import React from "react"; +import { RIGHT_SIDEBAR_TAB_KEY, RIGHT_SIDEBAR_COLLAPSED_KEY } from "@/common/constants/storage"; import { usePersistedState } from "@/browser/hooks/usePersistedState"; import { useWorkspaceUsage } from "@/browser/stores/WorkspaceStore"; import { useProviderOptions } from "@/browser/hooks/useProviderOptions"; @@ -100,7 +101,7 @@ const RightSidebarComponent: React.FC = ({ isCreating = false, }) => { // Global tab preference (not per-workspace) - const [selectedTab, setSelectedTab] = usePersistedState("right-sidebar-tab", "costs"); + const [selectedTab, setSelectedTab] = usePersistedState(RIGHT_SIDEBAR_TAB_KEY, "costs"); // Trigger for focusing Review panel (preserves hunk selection) const [focusTrigger, setFocusTrigger] = React.useState(0); @@ -167,7 +168,7 @@ const RightSidebarComponent: React.FC = ({ // Persist collapsed state globally (not per-workspace) since chat area width is shared // This prevents animation flash when switching workspaces - sidebar maintains its state const [showCollapsed, setShowCollapsed] = usePersistedState( - "right-sidebar:collapsed", + RIGHT_SIDEBAR_COLLAPSED_KEY, false ); @@ -297,6 +298,7 @@ const RightSidebarComponent: React.FC = ({ className="h-full" > ( // Track if hunk is visible in viewport for lazy syntax highlighting // Use ref for visibility to avoid re-renders when visibility changes - const isVisibleRef = React.useRef(true); // Start visible to avoid flash - const [isVisible, setIsVisible] = React.useState(true); + // Start as not visible to avoid eagerly highlighting off-screen hunks + const isVisibleRef = React.useRef(false); + const [isVisible, setIsVisible] = React.useState(false); // Use IntersectionObserver to track visibility React.useEffect(() => { diff --git a/src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx b/src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx index c8a0bb2c3..7f7f22bf3 100644 --- a/src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx +++ b/src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx @@ -28,7 +28,7 @@ import { ReviewControls } from "./ReviewControls"; import { FileTree } from "./FileTree"; import { usePersistedState } from "@/browser/hooks/usePersistedState"; import { useReviewState } from "@/browser/hooks/useReviewState"; -import { parseDiff, extractAllHunks } from "@/common/utils/git/diffParser"; +import { parseDiff, extractAllHunks, buildGitDiffCommand } from "@/common/utils/git/diffParser"; import { getReviewSearchStateKey } from "@/common/constants/storage"; import { Tooltip, TooltipWrapper } from "@/browser/components/Tooltip"; import { parseNumstat, buildFileTree, extractNewPath } from "@/common/utils/git/numstatParser"; @@ -63,59 +63,17 @@ interface DiagnosticInfo { } /** - * Build git diff command based on diffBase and includeUncommitted flag - * Shared logic between numstat (file tree) and diff (hunks) commands - * Exported for testing + * Discriminated union for diff loading state. + * Makes it impossible to show "No changes" while loading. * - * Git diff semantics: - * - `git diff A...HEAD` (three-dot): Shows commits on current branch since branching from A - * → Uses merge-base(A, HEAD) as comparison point, so changes to A after branching don't appear - * - `git diff $(git merge-base A HEAD)`: Shows all changes from branch point to working directory - * → Includes both committed changes on the branch AND uncommitted working directory changes - * → Single unified diff (no duplicate hunks from concatenation) - * - `git diff HEAD`: Shows only uncommitted changes (working directory vs HEAD) - * - `git diff --staged`: Shows only staged changes (index vs HEAD) - * - * The key insight: When includeUncommitted is true, we compare from the merge-base directly - * to the working directory. This gives a stable comparison point (doesn't change when base - * ref moves forward) while including both committed and uncommitted work in a single diff. - * - * @param diffBase - Base reference ("main", "HEAD", "--staged") - * @param includeUncommitted - Include uncommitted working directory changes - * @param pathFilter - Optional path filter (e.g., ' -- "src/foo.ts"') - * @param command - "diff" (unified) or "numstat" (file stats) + * Note: Parent uses key={workspaceId} so component remounts on workspace change, + * guaranteeing fresh state. No need to track workspaceId in state. */ -export function buildGitDiffCommand( - diffBase: string, - includeUncommitted: boolean, - pathFilter: string, - command: "diff" | "numstat" -): string { - const flags = command === "numstat" ? " -M --numstat" : " -M"; - - if (diffBase === "--staged") { - // Staged changes, optionally with unstaged appended as separate diff - const base = `git diff --staged${flags}${pathFilter}`; - return includeUncommitted ? `${base} && git diff HEAD${flags}${pathFilter}` : base; - } - - if (diffBase === "HEAD") { - // Uncommitted changes only (working vs HEAD) - return `git diff HEAD${flags}${pathFilter}`; - } - - // Branch diff: use three-dot for committed only, or merge-base for committed+uncommitted - if (includeUncommitted) { - // Use merge-base to get a unified diff from branch point to working directory - // This includes both committed changes on the branch AND uncommitted working changes - // Single command avoids duplicate hunks from concatenation - // Stable comparison point: merge-base doesn't change when diffBase ref moves forward - return `git diff $(git merge-base ${diffBase} HEAD)${flags}${pathFilter}`; - } else { - // Three-dot: committed changes only (merge-base to HEAD) - return `git diff ${diffBase}...HEAD${flags}${pathFilter}`; - } -} +type DiffState = + | { status: "loading" } + | { status: "refreshing"; hunks: DiffHunk[]; truncationWarning: string | null } + | { status: "loaded"; hunks: DiffHunk[]; truncationWarning: string | null } + | { status: "error"; message: string }; export const ReviewPanel: React.FC = ({ workspaceId, @@ -127,13 +85,14 @@ export const ReviewPanel: React.FC = ({ const { api } = useAPI(); const panelRef = useRef(null); const searchInputRef = useRef(null); - const [hunks, setHunks] = useState([]); + + // Unified diff state - discriminated union makes invalid states unrepresentable + // Note: Parent renders with key={workspaceId}, so component remounts on workspace change + const [diffState, setDiffState] = useState({ status: "loading" }); + const [selectedHunkId, setSelectedHunkId] = useState(null); - const [isLoadingHunks, setIsLoadingHunks] = useState(true); const [isLoadingTree, setIsLoadingTree] = useState(true); - const [error, setError] = useState(null); const [diagnosticInfo, setDiagnosticInfo] = useState(null); - const [truncationWarning, setTruncationWarning] = useState(null); const [isPanelFocused, setIsPanelFocused] = useState(false); const [refreshTrigger, setRefreshTrigger] = useState(0); const [fileTree, setFileTree] = useState(null); @@ -171,6 +130,13 @@ export const ReviewPanel: React.FC = ({ // Initialize review state hook const { isRead, toggleRead, markAsRead, markAsUnread } = useReviewState(workspaceId); + // Derive hunks from diffState for use in filters and rendering + const hunks = useMemo( + () => + diffState.status === "loaded" || diffState.status === "refreshing" ? diffState.hunks : [], + [diffState] + ); + const [filters, setFilters] = useState({ showReadHunks: showReadHunks, diffBase: diffBase, @@ -252,10 +218,21 @@ export const ReviewPanel: React.FC = ({ if (!api || isCreating) return; let cancelled = false; + // Transition to appropriate loading state: + // - "refreshing" if we have data (keeps UI stable during refresh) + // - "loading" if no data yet + setDiffState((prev) => { + if (prev.status === "loaded" || prev.status === "refreshing") { + return { + status: "refreshing", + hunks: prev.hunks, + truncationWarning: prev.truncationWarning, + }; + } + return { status: "loading" }; + }); + const loadDiff = async () => { - setIsLoadingHunks(true); - setError(null); - setTruncationWarning(null); try { // Git-level filters (affect what data is fetched): // - diffBase: what to diff against @@ -283,8 +260,7 @@ export const ReviewPanel: React.FC = ({ if (!diffResult.success) { // Real error (not truncation-related) console.error("Git diff failed:", diffResult.error); - setError(diffResult.error); - setHunks([]); + setDiffState({ status: "error", message: diffResult.error ?? "Unknown error" }); setDiagnosticInfo(null); return; } @@ -303,25 +279,24 @@ export const ReviewPanel: React.FC = ({ hunkCount: allHunks.length, }); - // Set truncation warning only when not filtering by path - if (truncationInfo && !selectedFilePath) { - setTruncationWarning( - `Diff truncated (${truncationInfo.reason}). Filter by file to see more.` - ); - } + // Build truncation warning (only when not filtering by path) + const truncationWarning = + truncationInfo && !selectedFilePath + ? `Diff truncated (${truncationInfo.reason}). Filter by file to see more.` + : null; - setHunks(allHunks); + // Single atomic state update with all data + setDiffState({ status: "loaded", hunks: allHunks, truncationWarning }); // Auto-select first hunk if none selected if (allHunks.length > 0 && !selectedHunkId) { setSelectedHunkId(allHunks[0].id); } } catch (err) { + if (cancelled) return; const errorMsg = `Failed to load diff: ${err instanceof Error ? err.message : String(err)}`; console.error(errorMsg); - setError(errorMsg); - } finally { - setIsLoadingHunks(false); + setDiffState({ status: "error", message: errorMsg }); } }; @@ -650,25 +625,27 @@ export const ReviewPanel: React.FC = ({ stats={stats} onFiltersChange={setFilters} onRefresh={() => setRefreshTrigger((prev) => prev + 1)} - isLoading={isLoadingHunks || isLoadingTree} + isLoading={ + diffState.status === "loading" || diffState.status === "refreshing" || isLoadingTree + } workspaceId={workspaceId} workspacePath={workspacePath} refreshTrigger={refreshTrigger} /> - {error ? ( + {diffState.status === "error" ? (
- {error} + {diffState.message}
- ) : isLoadingHunks && hunks.length === 0 && !fileTree ? ( + ) : diffState.status === "loading" ? (
Loading diff...
) : (
- {truncationWarning && ( + {diffState.truncationWarning && (
- {truncationWarning} + {diffState.truncationWarning}
)} diff --git a/src/browser/utils/highlighting/highlightDiffChunk.ts b/src/browser/utils/highlighting/highlightDiffChunk.ts index 53a9af6cb..231f78d69 100644 --- a/src/browser/utils/highlighting/highlightDiffChunk.ts +++ b/src/browser/utils/highlighting/highlightDiffChunk.ts @@ -1,18 +1,14 @@ -import { - getShikiHighlighter, - mapToShikiLang, - SHIKI_DARK_THEME, - SHIKI_LIGHT_THEME, - MAX_DIFF_SIZE_BYTES, -} from "./shikiHighlighter"; +import { highlightCode } from "./highlightWorkerClient"; import type { DiffChunk } from "./diffChunking"; /** - * Chunk-based diff highlighting with Shiki + * Chunk-based diff highlighting with Shiki (via Web Worker) + * + * Highlighting runs off-main-thread to avoid blocking UI during large diffs. * * Current approach: Parse Shiki HTML to extract individual line HTMLs * - Groups consecutive lines by type (add/remove/context) - * - Highlights each chunk with Shiki + * - Highlights each chunk with Shiki in web worker * - Extracts per-line HTML for individual rendering * * Future optimization: Could render entire blocks and use CSS to style @@ -20,6 +16,10 @@ import type { DiffChunk } from "./diffChunking"; * and reduce dangerouslySetInnerHTML usage. */ +// Maximum diff size to highlight (in bytes) +// Diffs larger than this fall back to plain text for performance +const MAX_DIFF_SIZE_BYTES = 32768; // 32kb + export interface HighlightedLine { html: string; // HTML content (already escaped and tokenized) lineNumber: number; @@ -70,31 +70,11 @@ export async function highlightDiffChunk( } const code = chunk.lines.join("\n"); + const workerTheme = isLightTheme(themeMode) ? "light" : "dark"; try { - const highlighter = await getShikiHighlighter(); - const shikiLang = mapToShikiLang(language); - - // Load language on-demand if not already loaded - // This is race-safe: concurrent loads of the same language are idempotent - const loadedLangs = highlighter.getLoadedLanguages(); - if (!loadedLangs.includes(shikiLang)) { - try { - // TypeScript doesn't know shikiLang is valid, but we handle errors gracefully - // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument - await highlighter.loadLanguage(shikiLang as any); - } catch { - // Language not available in Shiki bundle - fall back to plain text - console.warn(`Language '${shikiLang}' not available in Shiki, using plain text`); - return createFallbackChunk(chunk); - } - } - - const shikiTheme = isLightTheme(themeMode) ? SHIKI_LIGHT_THEME : SHIKI_DARK_THEME; - const html = highlighter.codeToHtml(code, { - lang: shikiLang, - theme: shikiTheme, - }); + // Highlight via worker (cached, off main thread) + const html = await highlightCode(code, language, workerTheme); // Parse HTML to extract line contents const lines = extractLinesFromHtml(html); diff --git a/src/browser/utils/highlighting/highlightWorkerClient.ts b/src/browser/utils/highlighting/highlightWorkerClient.ts new file mode 100644 index 000000000..27e2bd67a --- /dev/null +++ b/src/browser/utils/highlighting/highlightWorkerClient.ts @@ -0,0 +1,174 @@ +/** + * Syntax highlighting client with LRU caching + * + * Provides async API for off-main-thread syntax highlighting via Web Worker. + * Results are cached to avoid redundant highlighting of identical code. + * + * Falls back to main-thread highlighting in test environments where + * Web Workers aren't available. + */ + +import { LRUCache } from "lru-cache"; +import * as Comlink from "comlink"; +import type { Highlighter } from "shiki"; +import type { HighlightWorkerAPI } from "@/browser/workers/highlightWorker"; +import { mapToShikiLang, SHIKI_DARK_THEME, SHIKI_LIGHT_THEME } from "./shiki-shared"; + +// ───────────────────────────────────────────────────────────────────────────── +// LRU Cache with SHA-256 hashing +// ───────────────────────────────────────────────────────────────────────────── + +/** + * Cache for highlighted HTML results + * Key: First 64 bits of SHA-256 hash (hex string) + * Value: Shiki HTML output + */ +const highlightCache = new LRUCache({ + max: 10000, // High limit — rely on maxSize for eviction + maxSize: 8 * 1024 * 1024, // 8MB total + sizeCalculation: (html) => html.length * 2, // Rough bytes for JS strings +}); + +async function getCacheKey(code: string, language: string, theme: string): Promise { + const data = new TextEncoder().encode(`${language}:${theme}:${code}`); + const hash = await crypto.subtle.digest("SHA-256", data); + // Take first 8 bytes (64 bits) as hex + return Array.from(new Uint8Array(hash).slice(0, 8)) + .map((b) => b.toString(16).padStart(2, "0")) + .join(""); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Main-thread Shiki (fallback only) +// ───────────────────────────────────────────────────────────────────────────── + +let highlighterPromise: Promise | null = null; + +/** + * Get or create main-thread Shiki highlighter (for fallback when worker unavailable) + * Uses dynamic import to avoid loading Shiki on main thread unless actually needed. + */ +async function getShikiHighlighter(): Promise { + // Must use if-check instead of ??= to prevent race condition + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing + if (!highlighterPromise) { + highlighterPromise = import("shiki").then(({ createHighlighter }) => + createHighlighter({ + themes: [SHIKI_DARK_THEME, SHIKI_LIGHT_THEME], + langs: [], + }) + ); + } + return highlighterPromise; +} + +// ───────────────────────────────────────────────────────────────────────────── +// Worker Management (via Comlink) +// ───────────────────────────────────────────────────────────────────────────── + +let workerAPI: Comlink.Remote | null = null; +let workerFailed = false; + +function getWorkerAPI(): Comlink.Remote | null { + if (workerFailed) return null; + if (workerAPI) return workerAPI; + + try { + // Use relative path - @/ alias doesn't work in worker context + const worker = new Worker(new URL("../../workers/highlightWorker.ts", import.meta.url), { + type: "module", + name: "shiki-highlighter", // Shows up in DevTools + }); + + worker.onerror = (e) => { + console.error("[highlightWorkerClient] Worker failed to load:", e); + workerFailed = true; + workerAPI = null; + }; + + console.log("[highlightWorkerClient] Worker created successfully"); + workerAPI = Comlink.wrap(worker); + return workerAPI; + } catch (e) { + // Workers not available (e.g., test environment) + console.error("[highlightWorkerClient] Failed to create worker:", e); + workerFailed = true; + return null; + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Main-thread Fallback +// ───────────────────────────────────────────────────────────────────────────── + +let warnedMainThread = false; + +async function highlightMainThread( + code: string, + language: string, + theme: "dark" | "light" +): Promise { + if (!warnedMainThread) { + warnedMainThread = true; + console.warn( + "[highlightWorkerClient] Syntax highlighting running on main thread (worker unavailable)" + ); + } + + const highlighter = await getShikiHighlighter(); + const shikiLang = mapToShikiLang(language); + + // Load language on-demand + const loadedLangs = highlighter.getLoadedLanguages(); + if (!loadedLangs.includes(shikiLang)) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument + await highlighter.loadLanguage(shikiLang as any); + } + + const shikiTheme = theme === "light" ? SHIKI_LIGHT_THEME : SHIKI_DARK_THEME; + return highlighter.codeToHtml(code, { + lang: shikiLang, + theme: shikiTheme, + }); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Public API +// ───────────────────────────────────────────────────────────────────────────── + +/** + * Highlight code with syntax highlighting (cached, off-main-thread) + * + * Results are cached by (code, language, theme) to avoid redundant work. + * Highlighting runs in a Web Worker to avoid blocking the main thread. + * + * @param code - Source code to highlight + * @param language - Language identifier (e.g., "typescript", "python") + * @param theme - Theme variant ("dark" or "light") + * @returns Promise resolving to HTML string with syntax highlighting + * @throws Error if highlighting fails (caller should fallback to plain text) + */ +export async function highlightCode( + code: string, + language: string, + theme: "dark" | "light" +): Promise { + // Check cache first + const cacheKey = await getCacheKey(code, language, theme); + const cached = highlightCache.get(cacheKey); + if (cached) return cached; + + // Dispatch to worker or main-thread fallback + const api = getWorkerAPI(); + let html: string; + + if (!api) { + html = await highlightMainThread(code, language, theme); + } else { + html = await api.highlight(code, language, theme); + } + + // Cache result + highlightCache.set(cacheKey, html); + return html; +} diff --git a/src/browser/utils/highlighting/shikiHighlighter.ts b/src/browser/utils/highlighting/shikiHighlighter.ts deleted file mode 100644 index 7d725dd20..000000000 --- a/src/browser/utils/highlighting/shikiHighlighter.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { createHighlighter, type Highlighter } from "shiki"; -import { SHIKI_DARK_THEME, SHIKI_LIGHT_THEME } from "./shiki-shared"; - -export { SHIKI_DARK_THEME, SHIKI_LIGHT_THEME } from "./shiki-shared"; - -// Maximum diff size to highlight (in bytes) -// Diffs larger than this will fall back to plain text for performance -export const MAX_DIFF_SIZE_BYTES = 32768; // 32kb - -// Singleton promise (cached to prevent race conditions) -// Multiple concurrent calls will await the same Promise -let highlighterPromise: Promise | null = null; - -/** - * Get or create Shiki highlighter instance - * Lazy-loads WASM and themes on first call - * Thread-safe: concurrent calls share the same initialization Promise - */ -export async function getShikiHighlighter(): Promise { - // Must use if-check instead of ??= to prevent race condition - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - if (!highlighterPromise) { - highlighterPromise = createHighlighter({ - themes: [SHIKI_DARK_THEME, SHIKI_LIGHT_THEME], - langs: [], // Load languages on-demand via highlightDiffChunk - }); - } - return highlighterPromise; -} - -/** - * Map file extensions/languages to Shiki language IDs - * Reuses existing getLanguageFromPath logic - */ -export function mapToShikiLang(detectedLang: string): string { - // Most languages match 1:1, but handle special cases - const mapping: Record = { - text: "plaintext", - sh: "bash", - // Add more mappings if needed - }; - return mapping[detectedLang] || detectedLang; -} diff --git a/src/browser/workers/highlightWorker.ts b/src/browser/workers/highlightWorker.ts new file mode 100644 index 000000000..a8f68c154 --- /dev/null +++ b/src/browser/workers/highlightWorker.ts @@ -0,0 +1,59 @@ +/** + * Web Worker for syntax highlighting (Shiki) + * Moves expensive highlighting work off the main thread + */ + +import * as Comlink from "comlink"; +import { createHighlighter, type Highlighter } from "shiki"; +import { SHIKI_DARK_THEME, SHIKI_LIGHT_THEME } from "../utils/highlighting/shiki-shared"; + +// Singleton highlighter instance within worker +let highlighter: Highlighter | null = null; +let highlighterPromise: Promise | null = null; + +async function getHighlighter(): Promise { + if (highlighter) return highlighter; + // Must use if-check instead of ??= to prevent race condition + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing + if (!highlighterPromise) { + highlighterPromise = createHighlighter({ + themes: [SHIKI_DARK_THEME, SHIKI_LIGHT_THEME], + langs: [], + }); + } + highlighter = await highlighterPromise; + return highlighter; +} + +// Map detected language to Shiki language ID +function mapToShikiLang(detectedLang: string): string { + const mapping: Record = { + text: "plaintext", + sh: "bash", + }; + return mapping[detectedLang] || detectedLang; +} + +const api = { + async highlight(code: string, language: string, theme: "dark" | "light"): Promise { + const hl = await getHighlighter(); + const shikiLang = mapToShikiLang(language); + + // Load language on-demand + const loadedLangs = hl.getLoadedLanguages(); + if (!loadedLangs.includes(shikiLang)) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument + await hl.loadLanguage(shikiLang as any); + } + + const shikiTheme = theme === "light" ? SHIKI_LIGHT_THEME : SHIKI_DARK_THEME; + return hl.codeToHtml(code, { + lang: shikiLang, + theme: shikiTheme, + }); + }, +}; + +export type HighlightWorkerAPI = typeof api; + +Comlink.expose(api); diff --git a/src/common/constants/storage.ts b/src/common/constants/storage.ts index 5987a6be4..d1ba0f8ca 100644 --- a/src/common/constants/storage.ts +++ b/src/common/constants/storage.ts @@ -193,6 +193,18 @@ export function getStatusUrlKey(workspaceId: string): string { return `statusUrl:${workspaceId}`; } +/** + * Right sidebar tab selection (global) + * Format: "right-sidebar-tab" + */ +export const RIGHT_SIDEBAR_TAB_KEY = "right-sidebar-tab"; + +/** + * Right sidebar collapsed state (global) + * Format: "right-sidebar:collapsed" + */ +export const RIGHT_SIDEBAR_COLLAPSED_KEY = "right-sidebar:collapsed"; + /** * Get the localStorage key for unified Review search state per workspace * Stores: { input: string, useRegex: boolean, matchCase: boolean } diff --git a/src/common/utils/git/diffParser.test.ts b/src/common/utils/git/diffParser.test.ts index a16b0a617..bd6b25195 100644 --- a/src/common/utils/git/diffParser.test.ts +++ b/src/common/utils/git/diffParser.test.ts @@ -9,8 +9,7 @@ import { writeFileSync } from "fs"; import { join } from "path"; import { tmpdir } from "os"; import { execSync } from "child_process"; -import { parseDiff, extractAllHunks } from "./diffParser"; -import { buildGitDiffCommand } from "@/browser/components/RightSidebar/CodeReview/ReviewPanel"; +import { parseDiff, extractAllHunks, buildGitDiffCommand } from "./diffParser"; describe("git diff parser (real repository)", () => { let testRepoPath: string; diff --git a/src/common/utils/git/diffParser.ts b/src/common/utils/git/diffParser.ts index ee0defe6e..43a58ba3a 100644 --- a/src/common/utils/git/diffParser.ts +++ b/src/common/utils/git/diffParser.ts @@ -174,3 +174,57 @@ export function parseDiff(diffOutput: string): FileDiff[] { export function extractAllHunks(fileDiffs: FileDiff[]): DiffHunk[] { return fileDiffs.flatMap((file) => file.hunks); } + +/** + * Build git diff command based on diffBase and includeUncommitted flag + * Shared logic between numstat (file tree) and diff (hunks) commands + * + * Git diff semantics: + * - `git diff A...HEAD` (three-dot): Shows commits on current branch since branching from A + * → Uses merge-base(A, HEAD) as comparison point, so changes to A after branching don't appear + * - `git diff $(git merge-base A HEAD)`: Shows all changes from branch point to working directory + * → Includes both committed changes on the branch AND uncommitted working directory changes + * → Single unified diff (no duplicate hunks from concatenation) + * - `git diff HEAD`: Shows only uncommitted changes (working directory vs HEAD) + * - `git diff --staged`: Shows only staged changes (index vs HEAD) + * + * The key insight: When includeUncommitted is true, we compare from the merge-base directly + * to the working directory. This gives a stable comparison point (doesn't change when base + * ref moves forward) while including both committed and uncommitted work in a single diff. + * + * @param diffBase - Base reference ("main", "HEAD", "--staged") + * @param includeUncommitted - Include uncommitted working directory changes + * @param pathFilter - Optional path filter (e.g., ' -- "src/foo.ts"') + * @param command - "diff" (unified) or "numstat" (file stats) + */ +export function buildGitDiffCommand( + diffBase: string, + includeUncommitted: boolean, + pathFilter: string, + command: "diff" | "numstat" +): string { + const flags = command === "numstat" ? " -M --numstat" : " -M"; + + if (diffBase === "--staged") { + // Staged changes, optionally with unstaged appended as separate diff + const base = `git diff --staged${flags}${pathFilter}`; + return includeUncommitted ? `${base} && git diff HEAD${flags}${pathFilter}` : base; + } + + if (diffBase === "HEAD") { + // Uncommitted changes only (working vs HEAD) + return `git diff HEAD${flags}${pathFilter}`; + } + + // Branch diff: use three-dot for committed only, or merge-base for committed+uncommitted + if (includeUncommitted) { + // Use merge-base to get a unified diff from branch point to working directory + // This includes both committed changes on the branch AND uncommitted working changes + // Single command avoids duplicate hunks from concatenation + // Stable comparison point: merge-base doesn't change when diffBase ref moves forward + return `git diff $(git merge-base ${diffBase} HEAD)${flags}${pathFilter}`; + } else { + // Three-dot: committed changes only (merge-base to HEAD) + return `git diff ${diffBase}...HEAD${flags}${pathFilter}`; + } +}