Skip to content

Commit 7e1ef79

Browse files
authored
🤖 perf: improve Review panel performance and fix UI flashes (#946)
## Summary Performance improvements and UI polish for the Review panel when viewing large diffs. **Manually verified:** Much better performance when viewing large diffs (~3000+ lines). ### Performance - **Defer highlighting:** Syntax highlighting waits until a hunk is in/near viewport (600px preload margin) - **Web Worker:** Shiki highlighting runs off-main-thread via Comlink to avoid blocking UI - **LRU Cache:** Results cached by SHA-256 hash of (code, language, theme) — re-scrolling and theme toggles are instant - **Unified highlighting:** Both diff hunks and markdown code blocks now share the same worker + cache ### UI Flash Fixes - **Discriminated union for diff state:** Makes invalid states unrepresentable — impossible to show "No changes found" while loading - **Workspace-aware state:** Refresh (Ctrl+R) preserves visible hunks; workspace switch shows loading indicator - **Sidebar tab sync:** Read persisted tab state synchronously to prevent width flash on workspace switch ### Refactoring - Adopted Comlink for worker communication (removes ~50 lines of boilerplate) - Deleted `shikiHighlighter.ts`, consolidated into `highlightWorkerClient.ts` - Moved `buildGitDiffCommand` to `src/common/utils/git/diffParser.ts` - Extracted storage keys to constants (`RIGHT_SIDEBAR_TAB_KEY`, etc.) ## State Machine ```typescript type DiffState = | { status: "loading" } | { status: "refreshing"; workspaceId: string; hunks: DiffHunk[]; truncationWarning: string | null } | { status: "loaded"; workspaceId: string; hunks: DiffHunk[]; truncationWarning: string | null } | { status: "error"; message: string }; ``` | Scenario | Transition | UI | |----------|-----------|-----| | Initial load | `loading` → `loaded` | Loading indicator → content | | Refresh | `loaded` → `refreshing` → `loaded` | Hunks stay visible | | Workspace switch | `loaded{ws1}` → `loading` → `loaded{ws2}` | Loading indicator (no stale data) | ## Testing - `make typecheck` ✓ - `make static-check` ✓ - `bun test src/browser/utils/highlighting/highlightDiffChunk.test.ts` — 18 pass _Generated with `mux`_
1 parent 025460c commit 7e1ef79

File tree

16 files changed

+416
-196
lines changed

16 files changed

+416
-196
lines changed

bun.lock

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
{
22
"lockfileVersion": 1,
3-
"configVersion": 0,
43
"workspaces": {
54
"": {
65
"name": "mux",
@@ -32,6 +31,7 @@
3231
"ai": "^5.0.101",
3332
"ai-tokenizer": "^1.0.4",
3433
"chalk": "^5.6.2",
34+
"comlink": "^4.4.2",
3535
"commander": "^14.0.2",
3636
"cors": "^2.8.5",
3737
"crc-32": "^1.2.2",
@@ -1737,6 +1737,8 @@
17371737

17381738
"combined-stream": ["combined-stream@1.0.8", "", { "dependencies": { "delayed-stream": "~1.0.0" } }, "sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg=="],
17391739

1740+
"comlink": ["comlink@4.4.2", "", {}, "sha512-OxGdvBmJuNKSCMO4NTl1L47VRp6xn2wG4F/2hYzB6tiCb709otOxtEYCSvK80PtjODfXXZu8ds+Nw5kVCjqd2g=="],
1741+
17401742
"comma-separated-tokens": ["comma-separated-tokens@2.0.3", "", {}, "sha512-Fu4hJdvzeylCfQPp9SGWidpzrMs7tTrlu6Vb8XGaRGck8QSNZJJp538Wrb60Lax4fPwR64ViY468OIUTbRlGZg=="],
17411743

17421744
"commander": ["commander@9.5.0", "", {}, "sha512-KRs7WVDKg86PWiuAqhDrAQnTXZKraVcCc6vFdL14qrZ/DcWwuRo7VoiYXalXO7S5GKpqYiVEwCbgFDfxNHKJBQ=="],

docs/AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ Avoid mock-heavy tests that verify implementation details rather than behavior.
117117

118118
- Parent components own localStorage interactions; children announce intent only.
119119
- Use `usePersistedState`/`readPersistedState`/`updatePersistedState` helpers—never call `localStorage` directly.
120+
- 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))`.
121+
- When multiple components need the same persisted value, use `usePersistedState` with identical keys and `{ listener: true }` for automatic cross-component sync.
120122
- Avoid destructuring props in function signatures; access via `props.field` to keep rename-friendly code.
121123

122124
## Module Imports

eslint.config.mjs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,28 @@ export default defineConfig([
490490
],
491491
},
492492
},
493+
{
494+
// Shiki must only be imported in the highlight worker to avoid blocking main thread
495+
// Type-only imports are allowed (erased at compile time)
496+
files: ["src/**/*.ts", "src/**/*.tsx"],
497+
ignores: ["src/browser/workers/highlightWorker.ts"],
498+
rules: {
499+
"no-restricted-imports": [
500+
"error",
501+
{
502+
patterns: [
503+
{
504+
group: ["shiki"],
505+
importNamePattern: "^(?!type\\s)",
506+
allowTypeImports: true,
507+
message:
508+
"Shiki must only be imported in highlightWorker.ts to avoid blocking the main thread. Use highlightCode() from highlightWorkerClient.ts instead.",
509+
},
510+
],
511+
},
512+
],
513+
},
514+
},
493515
{
494516
// Renderer process (frontend) architectural boundary - prevent Node.js API usage
495517
files: ["src/**/*.ts", "src/**/*.tsx"],

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
"ai": "^5.0.101",
7373
"ai-tokenizer": "^1.0.4",
7474
"chalk": "^5.6.2",
75+
"comlink": "^4.4.2",
7576
"commander": "^14.0.2",
7677
"cors": "^2.8.5",
7778
"crc-32": "^1.2.2",

src/browser/components/AIView.tsx

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import { InterruptedBarrier } from "./Messages/ChatBarrier/InterruptedBarrier";
55
import { StreamingBarrier } from "./Messages/ChatBarrier/StreamingBarrier";
66
import { RetryBarrier } from "./Messages/ChatBarrier/RetryBarrier";
77
import { PinnedTodoList } from "./PinnedTodoList";
8-
import { getAutoRetryKey, VIM_ENABLED_KEY } from "@/common/constants/storage";
8+
import {
9+
getAutoRetryKey,
10+
VIM_ENABLED_KEY,
11+
RIGHT_SIDEBAR_TAB_KEY,
12+
} from "@/common/constants/storage";
913
import { WORKSPACE_DEFAULTS } from "@/constants/workspaceDefaults";
1014
import { ChatInput, type ChatInputAPI } from "./ChatInput/index";
1115
import { RightSidebar, type TabType } from "./RightSidebar";
@@ -22,7 +26,7 @@ import { ProviderOptionsProvider } from "@/browser/contexts/ProviderOptionsConte
2226
import { formatKeybind, KEYBINDS } from "@/browser/utils/ui/keybinds";
2327
import { useAutoScroll } from "@/browser/hooks/useAutoScroll";
2428
import { useOpenTerminal } from "@/browser/hooks/useOpenTerminal";
25-
import { usePersistedState } from "@/browser/hooks/usePersistedState";
29+
import { readPersistedState, usePersistedState } from "@/browser/hooks/usePersistedState";
2630
import { useThinking } from "@/browser/contexts/ThinkingContext";
2731
import {
2832
useWorkspaceState,
@@ -75,8 +79,11 @@ const AIViewInner: React.FC<AIViewProps> = ({
7579
const chatAreaRef = useRef<HTMLDivElement>(null);
7680

7781
// Track active tab to conditionally enable resize functionality
78-
// RightSidebar notifies us of tab changes via onTabChange callback
79-
const [activeTab, setActiveTab] = useState<TabType>("costs");
82+
// Initialize from persisted value to avoid layout flash; RightSidebar owns the state
83+
// and notifies us of changes via onTabChange callback
84+
const [activeTab, setActiveTab] = useState<TabType>(() =>
85+
readPersistedState<TabType>(RIGHT_SIDEBAR_TAB_KEY, "costs")
86+
);
8087

8188
const isReviewTabActive = activeTab === "review";
8289

src/browser/components/Messages/MarkdownComponents.tsx

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,9 @@
11
import type { ReactNode } from "react";
22
import React, { useState, useEffect } from "react";
33
import { Mermaid } from "./Mermaid";
4-
import {
5-
getShikiHighlighter,
6-
mapToShikiLang,
7-
SHIKI_DARK_THEME,
8-
SHIKI_LIGHT_THEME,
9-
} from "@/browser/utils/highlighting/shikiHighlighter";
10-
import { useTheme } from "@/browser/contexts/ThemeContext";
4+
import { highlightCode } from "@/browser/utils/highlighting/highlightWorkerClient";
115
import { extractShikiLines } from "@/browser/utils/highlighting/shiki-shared";
6+
import { useTheme } from "@/browser/contexts/ThemeContext";
127
import { CopyButton } from "@/browser/components/ui/CopyButton";
138

149
interface CodeProps {
@@ -57,37 +52,13 @@ const CodeBlock: React.FC<CodeBlockProps> = ({ code, language }) => {
5752
useEffect(() => {
5853
let cancelled = false;
5954
const isLight = themeMode === "light" || themeMode === "solarized-light";
60-
const shikiTheme = isLight ? SHIKI_LIGHT_THEME : SHIKI_DARK_THEME;
55+
const theme = isLight ? "light" : "dark";
6156

6257
setHighlightedLines(null);
6358

6459
async function highlight() {
6560
try {
66-
const highlighter = await getShikiHighlighter();
67-
const shikiLang = mapToShikiLang(language);
68-
69-
// Load language on-demand if not already loaded
70-
// This is race-safe: concurrent loads of the same language are idempotent
71-
const loadedLangs = highlighter.getLoadedLanguages();
72-
if (!loadedLangs.includes(shikiLang)) {
73-
try {
74-
// TypeScript doesn't know shikiLang is valid, but we handle errors gracefully
75-
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument
76-
await highlighter.loadLanguage(shikiLang as any);
77-
} catch {
78-
// Language not available in Shiki bundle - fall back to plain text
79-
console.warn(`Language '${shikiLang}' not available in Shiki, using plain text`);
80-
if (!cancelled) {
81-
setHighlightedLines(null);
82-
}
83-
return;
84-
}
85-
}
86-
87-
const html = highlighter.codeToHtml(code, {
88-
lang: shikiLang,
89-
theme: shikiTheme,
90-
});
61+
const html = await highlightCode(code, language, theme);
9162

9263
if (!cancelled) {
9364
const lines = extractShikiLines(html);

src/browser/components/RightSidebar.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import React from "react";
2+
import { RIGHT_SIDEBAR_TAB_KEY, RIGHT_SIDEBAR_COLLAPSED_KEY } from "@/common/constants/storage";
23
import { usePersistedState } from "@/browser/hooks/usePersistedState";
34
import { useWorkspaceUsage } from "@/browser/stores/WorkspaceStore";
45
import { useProviderOptions } from "@/browser/hooks/useProviderOptions";
@@ -100,7 +101,7 @@ const RightSidebarComponent: React.FC<RightSidebarProps> = ({
100101
isCreating = false,
101102
}) => {
102103
// Global tab preference (not per-workspace)
103-
const [selectedTab, setSelectedTab] = usePersistedState<TabType>("right-sidebar-tab", "costs");
104+
const [selectedTab, setSelectedTab] = usePersistedState<TabType>(RIGHT_SIDEBAR_TAB_KEY, "costs");
104105

105106
// Trigger for focusing Review panel (preserves hunk selection)
106107
const [focusTrigger, setFocusTrigger] = React.useState(0);
@@ -167,7 +168,7 @@ const RightSidebarComponent: React.FC<RightSidebarProps> = ({
167168
// Persist collapsed state globally (not per-workspace) since chat area width is shared
168169
// This prevents animation flash when switching workspaces - sidebar maintains its state
169170
const [showCollapsed, setShowCollapsed] = usePersistedState<boolean>(
170-
"right-sidebar:collapsed",
171+
RIGHT_SIDEBAR_COLLAPSED_KEY,
171172
false
172173
);
173174

@@ -297,6 +298,7 @@ const RightSidebarComponent: React.FC<RightSidebarProps> = ({
297298
className="h-full"
298299
>
299300
<ReviewPanel
301+
key={workspaceId}
300302
workspaceId={workspaceId}
301303
workspacePath={workspacePath}
302304
onReviewNote={onReviewNote}

src/browser/components/RightSidebar/CodeReview/HunkViewer.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ export const HunkViewer = React.memo<HunkViewerProps>(
4646

4747
// Track if hunk is visible in viewport for lazy syntax highlighting
4848
// Use ref for visibility to avoid re-renders when visibility changes
49-
const isVisibleRef = React.useRef(true); // Start visible to avoid flash
50-
const [isVisible, setIsVisible] = React.useState(true);
49+
// Start as not visible to avoid eagerly highlighting off-screen hunks
50+
const isVisibleRef = React.useRef(false);
51+
const [isVisible, setIsVisible] = React.useState(false);
5152

5253
// Use IntersectionObserver to track visibility
5354
React.useEffect(() => {

0 commit comments

Comments
 (0)