fix(ui): close review findings from the switch-and-scroll integration

- Markdown DOM cache: key includes a content-length fingerprint so an edited or reverted part re-materializing under the same id cannot restore stale DOM, memoization uses scalar identities instead of the part object (store reducers recreate part objects on unrelated updates, which re-ran the async render pipeline for identical content), and a probe with a mismatched locale/directory no longer destroys the entry it failed to claim. - Sidebar bootstrap: the layout-level sync owner only knows known directories, so expanded projects bootstrapped serialized at background priority. The visible collection now publishes a second, expansion- aware demand owner, restoring concurrent hydration for expanded projects and worktree groups. - Settings: local changes still sitting in the debounce buffer are not yet tracked as mutations, so a settings GET racing the debounce window briefly reverted them; reconciled results now reapply the pending buffer.
2026-08-26 01:03:47 +03:00
parent ac880e8e62
commit 448ecc12b8
4 changed files with 51 additions and 7 deletions
@@ -1122,17 +1122,25 @@ const MarkdownRendererImpl: React.FC<MarkdownRendererProps> = ({
    ? part
    : null;
  const runtimeKey = getRuntimeKey();
+  // Memoized on scalar identities, not the part object: sync-store reducers
+  // recreate part objects on unrelated updates, and an object-identity dep
+  // re-ran the async render pipeline for identical content.
+  const settledSessionID = settledPart?.sessionID;
+  const settledMessageID = settledPart?.messageID;
+  const settledPartID = settledPart?.id;
  const domCacheKey = React.useMemo<DetachedMarkdownDomKey | null>(() => {
    // Streaming, unfinished, oversized, and identity-less Markdown continues
    // through the normal rendering pipeline and never retains detached DOM.
-    if (isStreaming || !settledPart || content.length === 0 || content.length > MARKDOWN_DOM_CACHE_MAX_SOURCE_CHARS) return null;
+    if (isStreaming || !settledSessionID || !settledMessageID || !settledPartID || content.length === 0 || content.length > MARKDOWN_DOM_CACHE_MAX_SOURCE_CHARS) return null;
+    // content.length is a cheap fingerprint: an edited or reverted part that
+    // re-materializes under the same id must not restore the old DOM.
    return {
-      scope: `${runtimeKey}\0${settledPart.sessionID}`,
-      id: `${settledPart.messageID}\0${settledPart.id}\0${imageMode}`,
+      scope: `${runtimeKey}\0${settledSessionID}`,
+      id: `${settledMessageID}\0${settledPartID}\0${imageMode}\0${content.length}`,
      locale,
      directory: effectiveDirectory,
    };
-  }, [content.length, effectiveDirectory, imageMode, isStreaming, locale, runtimeKey, settledPart]);
+  }, [content.length, effectiveDirectory, imageMode, isStreaming, locale, runtimeKey, settledSessionID, settledMessageID, settledPartID]);
  // Identity for the fade-in wrapper: a new part/message restarts the animation.
  const fadeKey = `markdown-${part?.id ? `part-${part.id}` : `message-${messageId}`}`;

@@ -77,10 +77,14 @@ export class DetachedMarkdownDomCache {
    const entry = session.get(entryKey);
    if (entry === undefined) return null;

+    // A mismatched probe (different locale or directory for the same part)
+    // must not destroy the entry — the matching renderer may still come for
+    // it. Only a real hit transfers ownership out of the cache.
+    if (entry.locale !== key.locale || entry.directory !== key.directory) return null;
+
    // A fragment is a move-only resource; taking it removes cache ownership.
    session.delete(entryKey);
    if (session.size === 0) this.sessions.delete(sessionKey);
-    if (entry.locale !== key.locale || entry.directory !== key.directory) return null;
    return entry.fragment;
  }

@@ -10,6 +10,8 @@ import { useArchivedAutoFolders } from '../folders/useArchivedAutoFolders';
 import { ProjectSessionSelectionEffect } from '../projects/useProjectSessionSelection';
 import type { WorktreeMetadata } from '@/types/worktree';
 import { useRecentSessionCollection, useSessionProjectCollection } from './sessionCollection';
+import { buildSessionBootstrapDemands } from './sessionBootstrapDemands';
+import { useChildStoreManager } from '@/sync/sync-context';
 import { createSessionOwnershipIndex } from '../sessions/sessionOwnership';
 import { useProjectSessionLists } from '../projects/useProjectSessionLists';
 import { useSessionSidebarSections } from '../projects/useSessionSidebarSections';
@@ -192,6 +194,26 @@ const VisibleSessionProjects: React.FC<SessionProjectCollectionProps> = ({ topol
    buildGroupSearchText,
    foldersMap,
  });
+
+  // Second bootstrap-demand owner: the layout-level useSessionListSync keeps
+  // every known directory alive at background priority even when the sidebar
+  // is hidden, but only the visible collection knows which projects and
+  // groups are EXPANDED. Without this owner, expanded projects bootstrapped
+  // serialized at background priority (one directory at a time) instead of
+  // concurrently at expanded priority.
+  const childStores = useChildStoreManager();
+  const expansionDemandOwner = `session-collection-expansion:${React.useId()}`;
+  React.useEffect(() => {
+    childStores.setBootstrapDemand(expansionDemandOwner, buildSessionBootstrapDemands({
+      projectSections,
+      activeProjectId: view.activeProjectId,
+      collapsedProjects: projectView.collapsedProjects,
+      collapsedGroups: projectView.collapsedGroups,
+      currentDirectory: null,
+      currentSessionDirectory: null,
+    }));
+    return () => childStores.clearBootstrapDemand(expansionDemandOwner);
+  }, [childStores, expansionDemandOwner, projectSections, projectView.collapsedProjects, projectView.collapsedGroups, view.activeProjectId]);
  const source = view.useGroupedSections ? sectionsForRender : flatSectionsForRender;
  const sectionsForSidebarRender = React.useMemo(() => view.showInlineArchived ? source : source.map((section) => (
    section.groups.some((group) => group.isArchivedBucket)
@@ -1884,12 +1884,22 @@ export const syncDesktopSettings = async (): Promise<void> => {
  // Each step is wrapped in try/catch so a failure in one side-effect (e.g.
  // a TypeError from writing to a contextBridge-protected global) doesn't
  // prevent server settings from reaching the Zustand store.
+  // Local changes sitting in the debounce buffer are not yet tracked as
+  // mutations (record() only stores while a request is in flight), so a GET
+  // racing the debounce window would briefly revert them. Reapply the
+  // pending buffer over every reconciled result.
+  const overlayPendingChanges = (settings: DesktopSettings): DesktopSettings => {
+    if (!_pendingSettingsChanges || !_pendingSettingsContext) return settings;
+    if (!isSettingsRuntimeContextCurrent(_pendingSettingsContext)) return settings;
+    return { ...settings, ..._pendingSettingsChanges };
+  };
+
  const applySettings = async (loadedSettings: DesktopSettings) => {
    if (!isSettingsRuntimeContextCurrent(context)) return;
-    let settings = _settingsMutationTracker.reconcile(loadedSettings, operation);
+    let settings = overlayPendingChanges(_settingsMutationTracker.reconcile(loadedSettings, operation));
    await waitForHydration();
    if (!isSettingsRuntimeContextCurrent(context)) return;
-    settings = _settingsMutationTracker.reconcile(loadedSettings, operation);
+    settings = overlayPendingChanges(_settingsMutationTracker.reconcile(loadedSettings, operation));
    const shouldPersistCraftGoalMigration = settings.draftStartersCraftGoalAdded !== true
      || settings.draftStartersScheduleTaskAdded !== true;
    // `autoSaveEnabled` is new to the settings backend. Until the server has a