Merge pull request #824 from Wibias/fix/gui-ux-bugs

fix(gui): stop revisit flashes and layout shift across dashboard tabs

Author: Wibias

gui/src/client-resource.ts

@@ -40,6 +40,12 @@ type Store<T> = {
   /** Store-level visibilitychange handler, installed only while this store polls. */
   visibilityListener: (() => void) | null;
   generation: number;
+  /**
+   * Set when `setClientResourceData` publishes while nobody is subscribed (session-cache
+   * seed). The first subscribe must quiet-revalidate; otherwise mount never fetches and
+   * seeded rows stay indefinitely stale with `lastAttemptOk: true`.
+   */
+  seedNeedsRevalidate: boolean;
 };
 
 /**
@@ -80,6 +86,7 @@ function getStore<T>(key: string): Store<T> {
       inflightOwner: null,
       visibilityListener: null,
       generation: 0,
+      seedNeedsRevalidate: false,
     };
     stores.set(key, store);
   }
@@ -223,6 +230,8 @@ async function runFetch<T>(
   try {
     const data = await fetcher(controller.signal);
     if (gen !== store.generation || controller.signal.aborted) return;
+    // Cleared on settle (not at subscribe) so StrictMode's aborted first mount still revalidates.
+    store.seedNeedsRevalidate = false;
     store.snapshot = {
       data,
       error: undefined,
@@ -233,6 +242,7 @@ async function runFetch<T>(
     };
   } catch (error) {
     if (gen !== store.generation || controller.signal.aborted) return;
+    store.seedNeedsRevalidate = false;
     store.snapshot = {
       ...store.snapshot,
       // Normalize so a loader that rejects with `undefined` still reads as a failure.
@@ -298,9 +308,12 @@ function subscribeResource<T>(
   store.fetcherByListener.set(onStoreChange, fetcher);
   store.subscriberCount++;
 
-  // Cold start only — keep cached data across transient 0→1 resubscribe gaps.
-  if (store.subscriberCount === 1 && store.snapshot.data === undefined) {
-    void runFetch(store, fetcher, { replaceInflight: true, owner: onStoreChange });
+  // Cold start, or a pre-subscribe seed that still needs a network check. Keep
+  // cached data across transient 0→1 resubscribe gaps when neither applies.
+  if (store.subscriberCount === 1) {
+    if (store.snapshot.data === undefined || store.seedNeedsRevalidate) {
+      void runFetch(store, fetcher, { replaceInflight: true, owner: onStoreChange });
+    }
   }
   recomputePoll(store);
 
@@ -328,7 +341,7 @@ function subscribeResource<T>(
 }
 
 /** Module-level fetch cache with useSyncExternalStore subscriptions (no fetch in useEffect). */
-export interface ClientResourceOptions {
+export interface ClientResourceOptions<T = unknown> {
   pollMs?: number;
   enabled?: boolean;
   /**
@@ -337,18 +350,34 @@ export interface ClientResourceOptions {
    * happening off-screen — a restarting server, for instance.
    */
   pauseWhenHidden?: boolean;
+  /**
+   * Optional seed applied once before the first subscribe (session-cache revisit).
+   * Lives in the store — not a render-time ref — so React Compiler / react-hooks/refs
+   * stay quiet while the mount fetch still quiet-revalidates via `seedNeedsRevalidate`.
+   */
+  initialData?: T;
+}
+
+/** Seed an empty, unsubscribed store. No-ops when data already exists or someone is listening. */
+function seedClientResourceIfEmpty<T>(key: string, data: T): void {
+  const store = getStore<T>(key);
+  if (store.subscriberCount !== 0 || store.snapshot.data !== undefined) return;
+  setClientResourceData(key, data);
 }
 
 export function useClientResource<T>(
   key: string,
   fetcher: (signal: AbortSignal) => Promise<T>,
-  options?: ClientResourceOptions,
+  options?: ClientResourceOptions<T>,
 ): ResourceSnapshot<T> & { refresh: (opts?: { forceLoading?: boolean }) => void } {
   const enabled = options?.enabled !== false;
   const pollMs = options?.pollMs;
   // Default true: a background tab has nobody reading the paint. Opt out for polls that
   // must keep running while hidden, such as waiting for a restarted server to answer.
   const pauseWhenHidden = options?.pauseWhenHidden !== false;
+  if (enabled && options?.initialData !== undefined) {
+    seedClientResourceIfEmpty(key, options.initialData);
+  }
   const fetcherRef = useRef(fetcher);
   // Sync latest fetcher every commit. No dep array on purpose: inline fetchers are
   // reallocated every render; listing them would re-subscribe forever.
@@ -411,7 +440,7 @@ export function useKeyedClientResource<T>(
   key: string,
   deps: readonly unknown[],
   load: (signal: AbortSignal) => Promise<T>,
-  options?: ClientResourceOptions,
+  options?: ClientResourceOptions<T>,
 ): ResourceSnapshot<T> & { refresh: (opts?: { forceLoading?: boolean }) => void } {
   const resource = useClientResource(key, load, options);
   const prevDepsRef = useRef<readonly unknown[] | null>(null);
@@ -447,6 +476,9 @@ export function setClientResourceData<T>(key: string, data: T) {
     hasSucceeded: true,
     lastAttemptOk: true,
   };
+  // Only pre-subscribe seeds need a follow-up fetch. Live publishers (mutation
+  // results) already hold the fresh value and must not schedule a redundant GET.
+  store.seedNeedsRevalidate = store.subscriberCount === 0;
   emit(store);
 }
 

gui/src/components/CodexAccountPool.tsx

@@ -11,7 +11,6 @@ import CodexPoolStrategySetting from "./CodexPoolStrategySetting";
 import { useCodexAutoSwitch } from "../hooks/useCodexAutoSwitch";
 import { readJsonIfOk } from "../fetch-json";
 import { CodexAccountPoolCards, CodexAccountPoolReauthBanner } from "./codex-account-pool-cards";
-import { DataSurfaceStatus } from "./data-surface";
 import { CodexAccountSwitchModal } from "./codex-account-switch-modal";
 import { CodexAccountResetModal } from "./codex-account-reset-modal";
 import { CodexAccountPoolLoadStates, CodexAccountPoolMainCard, CodexAccountPoolPageHead } from "./codex-account-pool-main-card";
@@ -57,10 +56,6 @@ export default function CodexAccountPool({ apiBase, accountModeState = null, ban
   const ownController = useCodexAccountPool(apiBase, !injectedController);
   const controller = injectedController ?? ownController;
   const { accounts, activeId, loadState, switchingId, pauseUpdatingId, pausingExhausted, load } = controller;
-  // The controller owns the visible progress signal. A forced quota refresh keeps rows on screen
-  // and can take ~1s (longer when the server has to refill per-account quota), so without this the
-  // wait is invisible; `refreshingQuota` below stays local because it only guards its own button.
-  const { refreshing } = controller;
   const [confirm, setConfirm] = useState<CodexAccountEntry | null>(null);
   const [showAdd, setShowAdd] = useState(false);
   const [reauthId, setReauthId] = useState<string | null>(null);
@@ -277,12 +272,6 @@ export default function CodexAccountPool({ apiBase, accountModeState = null, ban
         onRetry={() => { void load(); }}
       />
 
-      {/* Revalidation over existing rows. The cold branch above already owns a live region, so
-          this only renders once rows are on screen, keeping one announcement per transition. */}
-      {refreshing && accounts.length > 0 && (
-        <DataSurfaceStatus live={loadState !== "error"}>{t("common.loading")}</DataSurfaceStatus>
-      )}
-
       {!(loadState === "loading" && accounts.length === 0) && (
         <>
           <CodexAccountPoolMainCard

gui/src/components/apikeys-workspace/ApiKeysWorkspace.tsx

@@ -39,6 +39,8 @@ export interface ApiKeysWorkspaceProps {
   copied: boolean;
   filteredModels: ExternalModelRow[];
   modelsLoading: boolean;
+  /** Quiet revalidation / retry over rows already on screen — not a skeleton. */
+  modelsRefreshing?: boolean;
   modelsLoadFailed: boolean;
   modelCount: number;
   hasModelData: boolean;
@@ -76,6 +78,7 @@ export default function ApiKeysWorkspace({
   copied,
   filteredModels,
   modelsLoading,
+  modelsRefreshing = false,
   modelsLoadFailed,
   modelCount,
   hasModelData,
@@ -420,6 +423,7 @@ export default function ApiKeysWorkspace({
                 <ApiKeysModelsPanel
                   filteredModels={filteredModels}
                   modelsLoading={modelsLoading}
+                  modelsRefreshing={modelsRefreshing}
                   modelsLoadFailed={modelsLoadFailed}
                   modelCount={modelCount}
                   hasModelData={hasModelData}

gui/src/components/section-tabs.tsx

@@ -8,8 +8,8 @@
  * The active tab follows the scroll position, so the strip reports where you are rather
  * than only where you last clicked.
  */
-import { useEffect, useRef, useState } from "react";
-import { sectionAnchorId, sectionAnchorPrefix } from "../section-anchors";
+import { useCallback, useEffect, useRef, useState } from "react";
+import { SECTION_TAB_SCROLL_LOCK_MS, sectionAnchorId, sectionAnchorPrefix } from "../section-anchors";
 
 export interface SectionTabItem {
   id: string;
@@ -28,7 +28,41 @@ export function SectionTabs({
   ariaLabel: string;
 }) {
   const [active, setActive] = useState(items[0]?.id ?? "");
-  const stripRef = useRef<HTMLDivElement | null>(null);
+  /** While set, scroll-spy ignores intermediate sections during smooth scroll-to-click. */
+  const scrollLockRef = useRef<string | null>(null);
+  const scrollLockTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+
+  const clearScrollLock = useCallback(() => {
+    scrollLockRef.current = null;
+    if (scrollLockTimerRef.current !== null) {
+      clearTimeout(scrollLockTimerRef.current);
+      scrollLockTimerRef.current = null;
+    }
+  }, []);
+
+  /** Timeout path: drop the click lock and re-read the visible section. */
+  const expireScrollLock = useCallback(() => {
+    clearScrollLock();
+    // If the user interrupted smooth scroll, the target may never cross the observer threshold —
+    // without a resync `active` would stay on the clicked tab while another section is on screen.
+    // Prefer the heading nearest the sticky-strip reading line (not only those with top <= 120),
+    // so a destination that stopped mid-viewport still wins over an off-screen prior heading.
+    let bestId: string | null = null;
+    let bestDistance = Number.POSITIVE_INFINITY;
+    const readingLine = 72;
+    for (const item of items) {
+      const node = document.getElementById(sectionAnchorId(scope, item.id));
+      if (!node) continue;
+      const distance = Math.abs(node.getBoundingClientRect().top - readingLine);
+      if (distance < bestDistance) {
+        bestDistance = distance;
+        bestId = item.id;
+      }
+    }
+    if (bestId) setActive(bestId);
+  }, [clearScrollLock, items, scope]);
+
+  useEffect(() => () => clearScrollLock(), [clearScrollLock]);
 
   // Follow the scroll position. `rootMargin` biases the observer toward the top of the
   // viewport so the heading you are reading wins, not whatever is technically centred.
@@ -41,6 +75,16 @@ export function SectionTabs({
 
     const observer = new IntersectionObserver(
       entries => {
+        const locked = scrollLockRef.current;
+        if (locked) {
+          const lockedNode = document.getElementById(sectionAnchorId(scope, locked));
+          const lockedVisible = entries.some(entry => entry.isIntersecting && entry.target === lockedNode);
+          if (lockedVisible) {
+            clearScrollLock();
+            setActive(locked);
+          }
+          return;
+        }
         const visible = entries
           .filter(entry => entry.isIntersecting)
           .sort((a, b) => a.boundingClientRect.top - b.boundingClientRect.top)[0];
@@ -52,19 +96,21 @@ export function SectionTabs({
     );
     for (const node of nodes) observer.observe(node);
     return () => observer.disconnect();
-  }, [items, scope]);
+  }, [clearScrollLock, items, scope]);
 
   const go = (id: string) => {
     const target = document.getElementById(sectionAnchorId(scope, id));
     if (!target) return;
+    scrollLockRef.current = id;
+    if (scrollLockTimerRef.current !== null) clearTimeout(scrollLockTimerRef.current);
+    scrollLockTimerRef.current = setTimeout(expireScrollLock, SECTION_TAB_SCROLL_LOCK_MS);
     setActive(id);
     // `scroll-margin-top` on the target keeps the heading clear of the pinned strip.
     target.scrollIntoView({ behavior: "smooth", block: "start" });
   };
 
   return (
     <div
-      ref={stripRef}
       className="page-tabs section-tabs"
       role="tablist"
       aria-label={ariaLabel}

gui/src/components/storage-workspace/StorageWorkspace.tsx

@@ -169,18 +169,18 @@ export default function StorageWorkspace({ report, locale }: StorageWorkspacePro
           </div>
         ) : (
           <div className="stw-overview">
-            <div className="usage-cards">
-              <div className="stat">
-                <div className="muted">{t("storage.card.total")}</div>
-                <div className="stat-value">{formatBytes(report.total.bytes, locale)}</div>
+            <div className="stw-summary">
+              <div className="stw-summary-card">
+                <div className="stw-summary-label">{t("storage.card.total")}</div>
+                <div className="stw-summary-value">{formatBytes(report.total.bytes, locale)}</div>
               </div>
-              <div className="stat">
-                <div className="muted">{t("storage.card.files")}</div>
-                <div className="stat-value">{report.total.fileCount.toLocaleString(locale)}</div>
+              <div className="stw-summary-card">
+                <div className="stw-summary-label">{t("storage.card.files")}</div>
+                <div className="stw-summary-value">{report.total.fileCount.toLocaleString(locale)}</div>
               </div>
-              <div className="stat">
-                <div className="muted">{t("storage.card.home")}</div>
-                <div className="stat-value mono stw-home-path" title={report.codexHome}>{report.codexHome}</div>
+              <div className="stw-summary-card">
+                <div className="stw-summary-label">{t("storage.card.home")}</div>
+                <div className="stw-summary-value mono stw-home-path" title={report.codexHome}>{report.codexHome}</div>
               </div>
             </div>
 

gui/src/data-surface.ts

@@ -43,6 +43,8 @@ export type DataSurfaceOptions<T> = {
   enabled?: boolean;
   /** Forwarded to the resource layer; see ClientResourceOptions.pauseWhenHidden. */
   pauseWhenHidden?: boolean;
+  /** Forwarded to the resource layer; see ClientResourceOptions.initialData. */
+  initialData?: T;
 };
 
 /**

gui/src/pages/ApiKeys.tsx

@@ -11,7 +11,7 @@ import {
 import { readSessionListCache, writeSessionListCache } from "../session-list-cache";
 import { createBoundedFetch } from "../bounded-fetch";
 import { useDataSurface } from "../data-surface";
-import { DataSurfaceSkeleton, DataSurfaceStatus } from "../components/data-surface";
+import { DataSurfaceSkeleton } from "../components/data-surface";
 import ApiKeysWorkspace from "../components/apikeys-workspace/ApiKeysWorkspace";
 import {
   DEFAULT_ENDPOINTS,
@@ -92,6 +92,8 @@ export default function ApiKeys({ apiBase }: { apiBase: string }) {
   // turn stale client state into an apparently authoritative empty auth table.
   const keysCacheKey = `ocx.apikeys.list.v2:${apiBase}`;
   const modelsCacheKey = `ocx.apikeys.models.v1:${apiBase}`;
+  const keysResourceKey = `api-keys:${apiBase}`;
+  const modelsResourceKey = `api-models:${apiBase}`;
   // A cache entry is arbitrary parsed JSON. Trusting it would reintroduce exactly
   // what the network path refuses: an empty matrix rendering as an authoritative
   // "no rules" table, or a row without `usage` throwing on first render.
@@ -164,16 +166,16 @@ export default function ApiKeys({ apiBase }: { apiBase: string }) {
   // Keys and models intentionally remain independent resources: a slow catalog must never
   // block endpoint/key management, and each cache key retains its own session seed.
   const keysResource = useDataSurface<CachedKeysShape>(
-    `api-keys:${apiBase}`,
+    keysResourceKey,
     [apiBase],
     fetchKeys,
-    { isEmpty: data => data.keys.length === 0 },
+    { isEmpty: data => data.keys.length === 0, initialData: cachedKeys ?? undefined },
   );
   const modelsResource = useDataSurface<ExternalModelRow[]>(
-    `api-models:${apiBase}`,
+    modelsResourceKey,
     [apiBase],
     fetchModels,
-    { isEmpty: models => models.length === 0 },
+    { isEmpty: models => models.length === 0, initialData: cachedModels ?? undefined },
   );
   const keysState = keysResource.state;
   const modelsState = modelsResource.state;
@@ -374,7 +376,10 @@ export default function ApiKeys({ apiBase }: { apiBase: string }) {
   const subtitleParts = t("api.subtitle").split("{authHeader}");
 
   return (
-    <section className="api-page">
+    <section
+      className="api-page"
+      aria-busy={keysState.refreshing || modelsState.refreshing || undefined}
+    >
       <div className="page-head">
         <h2>{t("api.title")}</h2>
       </div>
@@ -399,20 +404,6 @@ export default function ApiKeys({ apiBase }: { apiBase: string }) {
         </>
       ) : (
         <>
-          {/* Keys and models revalidate independently and can be in flight together. Only one
-              region may announce per transition, so keys take precedence and models steps down to
-              visual-only while keys is speaking. */}
-          {keysState.refreshing && keysData && (
-            <DataSurfaceStatus live={!keysState.showError}>{t("api.activeKeysLoading")}</DataSurfaceStatus>
-          )}
-          {/* `modelsState.data` alone misses the session-cached case: after a
-              failure the rows on screen come from `cachedModels`, and a retry
-              then ran with no visible progress at all. */}
-          {modelsState.refreshing && (modelsState.data !== undefined || cachedModels !== null) && (
-            <DataSurfaceStatus live={!modelsState.showError && !(keysState.refreshing && keysData)}>
-              {t("api.modelsLoading")}
-            </DataSurfaceStatus>
-          )}
           <ApiKeysWorkspace
         keys={keys}
         attributionSince={attributionSince}
@@ -429,6 +420,8 @@ export default function ApiKeys({ apiBase }: { apiBase: string }) {
         copied={copied}
         filteredModels={filteredModels}
         modelsLoading={modelsState.showSkeleton && !modelsState.data && !cachedModels}
+        // Only announce progress on a retry after failure — quiet warm revisits stay silent.
+        modelsRefreshing={modelsState.refreshing && modelsState.showError && (modelsState.data !== undefined || cachedModels !== null)}
         modelsLoadFailed={modelsState.showError}
         modelCount={models.length}
         // `readSessionListCache` answers `null`, not `undefined`, when it has