BuzzbeeSCD/openchamber
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

perf(right-sidebar): gate live effects, memoize lookups, always-mount tabs (#1674)

Browse Source 
* perf(right-sidebar): gate live effects, memoize lookups, always-mount tabs

Performance fixes for the right sidebar (git/files/context tabs).

== Correctness / leak fixes (P0)

* RightSidebar: drop dead useEffect that re-nulled refs the resize
  handler already nulled; collapse the redundant width/minWidth/maxWidth
  triple into width + the existing --oc-right-sidebar-width variable.
* useUIStore: clamp setRightSidebarWidth to [MIN, MAX]; simplify
  setRightSidebarOpen (22 lines -> 12).
* RightSidebarTabs: useRightSidebarGitSync now takes the right tab and
  main tab and only polls when the right git tab is the visible consumer
  and the browser is online + visible. Replaces a global poll that
  fired for the lifetime of the open sidebar.
* GitView: commit-files fetch refactored to cancelled + Promise.all
  (was a per-hash loop that could setState after unmount); getRemoteUrl
  and refreshRemotes gated on cancelled/mountedRef; new module-scoped
  mountedRef guards setIsSettingIdentity from firing after unmount.
* GitView + useGitmojiList: extract gitmoji fetch/cache into a hook
  with module-level inflight promise + subscribers Set; stale-while-
  revalidate from localStorage; ensureLoaded() for call-site-initiated
  hydration; cancelled flag on setIsLoading to avoid the React
  setState-after-unmount race.
* ProjectNotesTodoPanel: 400 ms notes debounce now cancels on blur
  (was double-saving); persistProjectData chained per project through
  a module-level Map<projectId, Promise> so a fast todo toggle racing
  the debounced save no longer hits the server in parallel; resize
  auto-adjust guards against same-value pings.

== Render fanout (P1)

* RightSidebarTabs: all three tab content components are now always
  mounted with the hidden attribute. State and cache survive tab
  switches. When activeMainTab === 'git' (or 'context') the matching
  right tab is filtered out of the tab strip and a redirect effect
  snaps any persisted-but-now-hidden right tab to 'files'. onSelect is
  now a type-guarded handler instead of `as RightTab`.
* GitView: 13 separate useGitStore action selectors collapsed into one
  useShallow block (one re-evaluation per store change instead of 13).
* GitView: new isGitViewActive flag (true when this instance is the
  visible consumer) gates the 7 live effects — load identities, fetch
  remote URL, refresh remotes, ensureAll, sessionEvents.onGitRefreshHint,
  worktree bootstrap poll, default-identity auto-apply. Hidden
  GitView instances no longer run these.
* GitView: gitViewSnapshots module-level Map is now backed by an
  LRU wrapper (cap 20) so per-directory draft snapshots cannot leak
  across hundreds of project switches. Removed the dead `unique.set`
  dedup in changeEntries — GitStatus.files is already unique by path.
* SidebarFilesTree: statusByPath Map<path, FileStatus> and
  badgeByDir Map<dirPath, { modified, added }> are precomputed once
  per gitStatus change. Tree render is O(1) per node instead of O(N)
  per node via the previous per-row find/scan. badgeByDir walks each
  file's path segments and increments counters for every ancestor
  dir, so total cost is O(N + total_dirs_in_files) per gitStatus
  change.
* SidebarFilesTree: FileRow wrapped in React.memo with a custom
  comparator. Context-menu open state moved INTO FileRow as local
  state — opening a menu in one row no longer re-renders siblings.
* SidebarFilesTree: loadDirectory accepts an isCancelled predicate;
  the batch-load effect for expandedPaths passes a stable predicate
  so per-dir fetches stop touching state once the effect tears down.
* SidebarFilesTree: module-level fileTreeCacheByRoot Map (LRU,
  cap 8 roots) hydrates childrenByDir / loadErrorsByDir /
  loadedDirsRef on mount or root change. Mirror effects write state
  back to the cache. Survives close-and-reopen of the right sidebar;
  populated entries are dropped on unmount only when they had no
  data.

== Result

Net diff: 6 files modified, 1 new (useGitmojiList.ts), 682 insertions,
283 deletions. Existing test suite baseline preserved (537 pass / 58
fail / 1 error) — no new regressions. The 58 pre-existing failures are
in unrelated chat/streaming tests and were verified via git stash on
the same branch.

Architecture assumptions, verified by manual review:
- P1.1's redirect effect snaps rightSidebarTab to 'files' whenever
  activeMainTab === 'git', so the right and main GitView instances
  are mutually exclusive — isGitViewActive cannot be true for both.
- The 7 gated effects plus the useRightSidebar GitSync poll cover all
  cases where git state should advance: visible consumer fetches; the
  poll keeps the store warm when only the right git tab is visible.
- The aborted loadDirectory predicate is sufficient because
  inFlightDirsRef and loadedDirsRef dedup at the call site before
  any network IO is initiated.

* fix(sidebar): always clean up inFlightDirsRef regardless of cancellation

* refactor(sidebar): deduplicate RIGHT_SIDEBAR_MIN/MAX_WIDTH constants, export from useUIStore

* docs: split right sidebar perf plan into standalone file, clean up merged master status from chat plan

* fix(git): gate GitView effects by instance visibility

---------

Co-authored-by: Leonid Skorobogatyy <bash@opencode.itc.local>
Co-authored-by: Bohdan Triapitsyn <artmore@protonmail.com>
This commit is contained in:
bashrusakh
2026-06-16 13:52:05 +03:00
committed by GitHub
co-authored by Leonid Skorobogatyy Bohdan Triapitsyn
parent e982bd9388
commit 9199798a14
10 changed files with 1273 additions and 291 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/ui/src/components/views/GitView.tsx
+119 -151
View File
@@ -6,6 +6,7 @@ import type { GitIdentityProfile, CommitFileEntry, GitStatus } from '@/lib/api/t
import { useGitIdentitiesStore } from '@/stores/useGitIdentitiesStore';
import { useShallow } from 'zustand/react/shallow';
import { useEffectiveDirectory } from '@/hooks/useEffectiveDirectory';
import { useGitmojiList } from '@/hooks/useGitmojiList';
import { copyTextToClipboard } from '@/lib/clipboard';
import {
useGitStore,
@@ -93,19 +94,8 @@ type GitmojiEntry = {
description: string;
};
type GitmojiCachePayload = {
gitmojis: GitmojiEntry[];
fetchedAt: number;
version: string;
};
const GITMOJI_CACHE_KEY = 'gitmojiCache';
const GITMOJI_CACHE_TTL_MS = 1000 * 60 * 60 * 24 * 7;
const GITMOJI_CACHE_VERSION = '1';
const GIT_DIFF_PRIORITY_PREFETCH_LIMIT = 40;
const GIT_DIFF_PRIORITY_BASELINE_LIMIT = 20;
const GITMOJI_SOURCE_URL =
'https://raw.githubusercontent.com/carloscuesta/gitmoji/master/packages/gitmojis/src/gitmojis.json';
const KEYWORD_MAP: Record<string, string> = {
'feat': ':sparkles:',
@@ -147,50 +137,6 @@ const KEYWORD_MAP: Record<string, string> = {
'initial': ':tada:',
};
const isGitmojiEntry = (value: unknown): value is GitmojiEntry => {
if (!value || typeof value !== 'object') return false;
const candidate = value as Record<string, unknown>;
return (
typeof candidate.emoji === 'string' &&
typeof candidate.code === 'string' &&
typeof candidate.description === 'string'
);
};
const readGitmojiCache = (): GitmojiCachePayload | null => {
if (typeof window === 'undefined') return null;
try {
const raw = localStorage.getItem(GITMOJI_CACHE_KEY);
if (!raw) return null;
const parsed = JSON.parse(raw) as Partial<GitmojiCachePayload>;
if (!parsed || parsed.version !== GITMOJI_CACHE_VERSION || typeof parsed.fetchedAt !== 'number') {
return null;
}
if (!Array.isArray(parsed.gitmojis)) return null;
const gitmojis = parsed.gitmojis.filter(isGitmojiEntry);
return { gitmojis, fetchedAt: parsed.fetchedAt, version: parsed.version };
} catch {
return null;
}
};
const writeGitmojiCache = (gitmojis: GitmojiEntry[]) => {
if (typeof window === 'undefined') return;
try {
const payload: GitmojiCachePayload = {
gitmojis,
fetchedAt: Date.now(),
version: GITMOJI_CACHE_VERSION,
};
localStorage.setItem(GITMOJI_CACHE_KEY, JSON.stringify(payload));
} catch {
return;
}
};
const isGitmojiCacheFresh = (payload: GitmojiCachePayload) =>
Date.now() - payload.fetchedAt < GITMOJI_CACHE_TTL_MS;
const matchGitmojiFromSubject = (subject: string, gitmojis: GitmojiEntry[]): GitmojiEntry | null => {
const lowerSubject = subject.toLowerCase();
@@ -217,8 +163,23 @@ const matchGitmojiFromSubject = (subject: string, gitmojis: GitmojiEntry[]): Git
return null;
};
const GIT_VIEW_SNAPSHOTS_CAP = 20;
const gitViewSnapshots = new Map<string, GitViewSnapshot>();
const rememberSnapshot = (key: string, snapshot: GitViewSnapshot) => {
// Touch-on-write LRU: deleting before re-inserting promotes the key to
// the Map's insertion order, so the oldest key falls off the end.
gitViewSnapshots.delete(key);
gitViewSnapshots.set(key, snapshot);
if (gitViewSnapshots.size > GIT_VIEW_SNAPSHOTS_CAP) {
const oldest = gitViewSnapshots.keys().next().value;
if (oldest !== undefined) {
gitViewSnapshots.delete(oldest);
}
}
};
const normalizePath = (value?: string | null): string =>
(value || '').replace(/\\/g, '/').replace(/\/+$/, '');
@@ -233,7 +194,11 @@ const isUnstagedStatusFile = (file: GitStatus['files'][number]): boolean => {
return Boolean(workingStatus || indexStatus === '?');
};
export const GitView: React.FC = () => {
type GitViewProps = {
isActive: boolean;
};
export const GitView: React.FC<GitViewProps> = ({ isActive }) => {
const { t } = useI18n();
const { git } = useRuntimeAPIs();
const currentDirectory = useEffectiveDirectory();
@@ -308,26 +273,44 @@ export const GitView: React.FC = () => {
const currentIdentity = useGitIdentity(currentDirectory ?? null);
const isLoading = useGitLoadingStatus(currentDirectory ?? null);
const isLogLoading = useGitLoadingLog(currentDirectory ?? null);
const setActiveDirectory = useGitStore((state) => state.setActiveDirectory);
const fetchAll = useGitStore((state) => state.fetchAll);
const ensureAll = useGitStore((state) => state.ensureAll);
const fetchStatus = useGitStore((state) => state.fetchStatus);
const fetchBranches = useGitStore((state) => state.fetchBranches);
const fetchLog = useGitStore((state) => state.fetchLog);
const setLogMaxCount = useGitStore((state) => state.setLogMaxCount);
const fetchIdentity = useGitStore((state) => state.fetchIdentity);
const prefetchDiffs = useGitStore((state) => state.prefetchDiffs);
const moveStatusPathsOptimistically = useGitStore((state) => state.moveStatusPathsOptimistically);
const restoreStatus = useGitStore((state) => state.restoreStatus);
const bumpIndexRevision = useGitStore((state) => state.bumpIndexRevision);
const {
setActiveDirectory,
fetchAll,
ensureAll,
fetchStatus,
fetchBranches,
fetchLog,
setLogMaxCount,
fetchIdentity,
prefetchDiffs,
moveStatusPathsOptimistically,
restoreStatus,
bumpIndexRevision,
} = useGitStore(useShallow((state) => ({
setActiveDirectory: state.setActiveDirectory,
fetchAll: state.fetchAll,
ensureAll: state.ensureAll,
fetchStatus: state.fetchStatus,
fetchBranches: state.fetchBranches,
fetchLog: state.fetchLog,
setLogMaxCount: state.setLogMaxCount,
fetchIdentity: state.fetchIdentity,
prefetchDiffs: state.prefetchDiffs,
moveStatusPathsOptimistically: state.moveStatusPathsOptimistically,
restoreStatus: state.restoreStatus,
bumpIndexRevision: state.bumpIndexRevision,
})));
const isMobile = useUIStore((state) => state.isMobile);
const openContextDiff = useUIStore((state) => state.openContextDiff);
const navigateToDiff = useUIStore((state) => state.navigateToDiff);
const setRightSidebarOpen = useUIStore((state) => state.setRightSidebarOpen);
const previousBootstrapStatusRef = React.useRef<'pending' | 'ready' | 'failed' | null>(null);
const gitReconcileTimeoutRef = React.useRef<number | null>(null);
const gitMutationFlushTimeoutRef = React.useRef<number | null>(null);
const flushQueuedGitMutationsRef = React.useRef<(() => void) | null>(null);
const mountedRef = React.useRef(true);
React.useEffect(() => () => { mountedRef.current = false; }, []);
const clearScheduledGitReconcile = React.useCallback(() => {
if (gitReconcileTimeoutRef.current === null) {
@@ -429,6 +412,7 @@ export const GitView: React.FC = () => {
React.useEffect(() => clearScheduledGitMutationFlush, [clearScheduledGitMutationFlush]);
React.useEffect(() => {
if (!isActive) return;
if (!currentDirectory) {
setWorktreeBootstrapStatus(null);
setIsWaitingForGitRefreshAfterBootstrap(false);
@@ -465,7 +449,7 @@ export const GitView: React.FC = () => {
window.clearTimeout(timeoutId);
}
};
}, [currentDirectory]);
}, [isActive, currentDirectory]);
React.useEffect(() => {
const previous = previousBootstrapStatusRef.current;
@@ -506,6 +490,7 @@ export const GitView: React.FC = () => {
const settingsGitmojiEnabled = useConfigStore((state) => state.settingsGitmojiEnabled);
const [rootBranchHint, setRootBranchHint] = React.useState<string | null>(null);
const { gitmojis: gitmojiEmojis } = useGitmojiList(settingsGitmojiEnabled);
React.useEffect(() => {
const projectRoot = authoritativeProjectRoot || worktreeMetadata?.projectDirectory;
@@ -550,12 +535,14 @@ export const GitView: React.FC = () => {
const beginIdentityApply = React.useCallback(() => {
identityApplyCountRef.current += 1;
setIsSettingIdentity(true);
if (mountedRef.current) {
setIsSettingIdentity(true);
}
}, []);
const endIdentityApply = React.useCallback(() => {
identityApplyCountRef.current = Math.max(0, identityApplyCountRef.current - 1);
if (identityApplyCountRef.current === 0) {
if (mountedRef.current && identityApplyCountRef.current === 0) {
setIsSettingIdentity(false);
}
}, []);
@@ -625,7 +612,6 @@ export const GitView: React.FC = () => {
const [loadingCommitHashes, setLoadingCommitHashes] = React.useState<Set<string>>(new Set());
const [historyBranchDivider, setHistoryBranchDivider] = React.useState<HistoryBranchDivider>(null);
const [remoteUrl, setRemoteUrl] = React.useState<string | null>(null);
const [gitmojiEmojis, setGitmojiEmojis] = React.useState<GitmojiEntry[]>([]);
const [gitmojiSearch, setGitmojiSearch] = React.useState('');
const [gitLogDialogMode, setGitLogDialogMode] = React.useState<GitLogDialogMode | null>(null);
@@ -749,6 +735,8 @@ export const GitView: React.FC = () => {
if (hashesToLoad.length === 0) return;
let cancelled = false;
setLoadingCommitHashes((prev) => {
const next = new Set(prev);
for (const hash of hashesToLoad) {
@@ -757,29 +745,42 @@ export const GitView: React.FC = () => {
return next;
});
for (const hash of hashesToLoad) {
git
.getCommitFiles(currentDirectory, hash)
.then((response) => {
setCommitFilesMap((prev) => new Map(prev).set(hash, response.files));
})
.catch((error) => {
console.error('Failed to fetch commit files:', error);
setCommitFilesMap((prev) => new Map(prev).set(hash, []));
})
.finally(() => {
setLoadingCommitHashes((prev) => {
const next = new Set(prev);
next.delete(hash);
return next;
});
});
}
void Promise.all(
hashesToLoad.map((hash) =>
git
.getCommitFiles(currentDirectory, hash)
.then((response) => ({ hash, files: response.files }))
.catch((error) => {
console.error('Failed to fetch commit files:', error);
return { hash, files: [] as CommitFileEntry[] };
})
)
).then((results) => {
if (cancelled) return;
setCommitFilesMap((prev) => {
const next = new Map(prev);
for (const { hash, files } of results) {
next.set(hash, files);
}
return next;
});
setLoadingCommitHashes((prev) => {
const next = new Set(prev);
for (const { hash } of results) {
next.delete(hash);
}
return next;
});
});
return () => {
cancelled = true;
};
}, [expandedCommitHashes, currentDirectory, git, commitFilesMap, loadingCommitHashes]);
React.useEffect(() => {
if (!currentDirectory) return;
gitViewSnapshots.set(currentDirectory, {
rememberSnapshot(currentDirectory, {
directory: currentDirectory,
commitMessage,
generatedHighlights,
@@ -787,18 +788,25 @@ export const GitView: React.FC = () => {
}, [commitMessage, currentDirectory, generatedHighlights]);
React.useEffect(() => {
if (!isActive) return;
loadProfiles();
loadGlobalIdentity();
loadDefaultGitIdentityId();
}, [loadProfiles, loadGlobalIdentity, loadDefaultGitIdentityId]);
}, [isActive, loadProfiles, loadGlobalIdentity, loadDefaultGitIdentityId]);
React.useEffect(() => {
if (!isActive) return;
if (!currentDirectory || !git?.getRemoteUrl) {
setRemoteUrl(null);
return;
}
git.getRemoteUrl(currentDirectory).then(setRemoteUrl).catch(() => setRemoteUrl(null));
}, [currentDirectory, git]);
let cancelled = false;
git
.getRemoteUrl(currentDirectory)
.then((url) => { if (!cancelled) setRemoteUrl(url); })
.catch(() => { if (!cancelled) setRemoteUrl(null); });
return () => { cancelled = true; };
}, [isActive, currentDirectory, git]);
const refreshRemotes = React.useCallback(async () => {
if (!currentDirectory || !git?.getRemotes) {
@@ -807,68 +815,31 @@ export const GitView: React.FC = () => {
}
try {
const remoteList = await git.getRemotes(currentDirectory);
setRemotes(remoteList);
if (mountedRef.current) {
setRemotes(remoteList);
}
} catch {
setRemotes([]);
if (mountedRef.current) {
setRemotes([]);
}
}
}, [currentDirectory, git]);
React.useEffect(() => {
if (!isActive) return;
void refreshRemotes();
}, [refreshRemotes]);
React.useEffect(() => {
if (!settingsGitmojiEnabled) {
setGitmojiEmojis([]);
return;
}
let cancelled = false;
const cached = readGitmojiCache();
if (cached) {
setGitmojiEmojis(cached.gitmojis);
if (isGitmojiCacheFresh(cached)) {
return () => {
cancelled = true;
};
}
}
const loadGitmojis = async () => {
try {
const response = await fetch(GITMOJI_SOURCE_URL);
if (!response.ok) {
throw new Error(`Failed to load gitmojis: ${response.statusText}`);
}
const payload = (await response.json()) as { gitmojis?: GitmojiEntry[] };
const gitmojis = Array.isArray(payload.gitmojis) ? payload.gitmojis.filter(isGitmojiEntry) : [];
if (!cancelled) {
setGitmojiEmojis(gitmojis);
writeGitmojiCache(gitmojis);
}
} catch (error) {
if (!cancelled) {
console.warn('Failed to load gitmoji list:', error);
}
}
};
void loadGitmojis();
return () => {
cancelled = true;
};
}, [settingsGitmojiEnabled]);
}, [isActive, refreshRemotes]);
React.useEffect(() => {
if (!isActive) return;
if (currentDirectory) {
setActiveDirectory(currentDirectory);
void ensureAll(currentDirectory, git);
}
}, [currentDirectory, setActiveDirectory, ensureAll, git]);
}, [isActive, currentDirectory, setActiveDirectory, ensureAll, git]);
React.useEffect(() => {
if (!isActive) return;
if (!currentDirectory) {
return;
}
@@ -879,7 +850,7 @@ export const GitView: React.FC = () => {
}
void fetchStatus(currentDirectory, git);
});
}, [currentDirectory, fetchStatus, git]);
}, [isActive, currentDirectory, fetchStatus, git]);
const refreshStatusAndBranches = React.useCallback(
async (showErrors = true) => {
@@ -912,6 +883,7 @@ export const GitView: React.FC = () => {
}, [currentDirectory, git, fetchIdentity]);
React.useEffect(() => {
if (!isActive) return;
if (!currentDirectory) return;
if (!git?.hasLocalIdentity) return;
if (isGitRepo !== true) return;
@@ -948,18 +920,14 @@ export const GitView: React.FC = () => {
return () => {
cancelled = true;
};
}, [beginIdentityApply, currentDirectory, defaultGitIdentityId, endIdentityApply, git, isGitRepo, refreshIdentity]);
}, [isActive, beginIdentityApply, currentDirectory, defaultGitIdentityId, endIdentityApply, git, isGitRepo, refreshIdentity]);
const changeEntries = React.useMemo(() => {
if (!status) return [];
const files = status.files ?? [];
const unique = new Map<string, (typeof files)[number]>();
for (const file of files) {
unique.set(file.path, file);
}
return Array.from(unique.values()).sort((a, b) => a.path.localeCompare(b.path));
// GitStatus.files is already unique by `path` per the server contract;
// a defensive dedup pass would only mask real upstream bugs.
return [...files].sort((a, b) => a.path.localeCompare(b.path));
}, [status]);
const stagedChangeEntries = React.useMemo(