feat(FN-1734): merge fusion/fn-1734

.fusion/memory.md

@@ -76,6 +76,16 @@ if (cached) {
 }
 ```
 
+## FN-1734: Polling Hook Loading Contract
+
+When implementing polling hooks that fetch data periodically (e.g., health metrics, status updates):
+- **Loading contract**: `loading` should be `true` ONLY for initial data fetch, NOT during background polling
+- **Background polling pattern**: Use a ref (`initialLoadCompleteRef`) to track if initial load is done; only set `loading: true` when `!initialLoadCompleteRef.current`
+- **Component behavior**: Components consuming these hooks should show skeleton only when there's genuinely no data (not just when `loading` is true during refresh)
+- **Why it matters**: Setting `loading` to true on every poll causes skeleton flicker and scroll position resets, degrading UX
+
+Reference: `useProjectHealth` in `packages/dashboard/app/hooks/useProjectHealth.ts` demonstrates this pattern.
+
 ## Conventions
 
 - When mocking function types with Vitest for the build (tsc), use `vi.fn().mockResolvedValue(x) as unknown as T` instead of `vi.fn<Parameters<T>, ReturnType<T>>()`. The generic syntax works at runtime but fails during `tsc` build.

packages/dashboard/app/components/ProjectOverview.tsx

@@ -183,8 +183,15 @@ export function ProjectOverview({
     onSelectProject(project);
   }, [onSelectProject, recentProjectIds]);
 
+  // Determine if we need to show skeleton
+  // Show skeleton for initial load if:
+  // 1. Projects list is still loading, OR
+  // 2. Projects exist but we haven't fetched health data yet (healthLoading with no data)
+  // Don't show skeleton during background health polling when health data already exists
+  const needsInitialSkeleton = loading || (healthLoading && projects.length > 0 && Object.keys(healthMap).length === 0);
+
   // Show skeleton while loading
-  if (loading || healthLoading) {
+  if (needsInitialSkeleton) {
     return <ProjectGridSkeleton />;
   }
 

packages/dashboard/app/components/__tests__/ProjectOverview.test.tsx

@@ -2,22 +2,28 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
 import { render, screen, fireEvent, waitFor } from "@testing-library/react";
 import { ProjectOverview } from "../ProjectOverview";
 import type { ProjectInfo, ProjectHealth } from "@fusion/core";
+import { useProjectHealth } from "../../hooks/useProjectHealth";
+
+// Default mock implementation
+function createDefaultHealthMap(projectIds: string[]): Record<string, ProjectHealth> {
+  return projectIds.reduce((acc, id) => {
+    acc[id] = {
+      projectId: id,
+      status: "active" as const,
+      activeTaskCount: 5,
+      inFlightAgentCount: 2,
+      totalTasksCompleted: 100,
+      totalTasksFailed: 3,
+      updatedAt: new Date().toISOString(),
+    };
+    return acc;
+  }, {} as Record<string, ProjectHealth>);
+}
 
 // Mock the hooks
 vi.mock("../../hooks/useProjectHealth", () => ({
   useProjectHealth: vi.fn((projectIds: string[]) => ({
-    healthMap: projectIds.reduce((acc, id) => {
+    healthMap: createDefaultHealthMap(projectIds),
-      acc[id] = {
-        projectId: id,
-        status: "active",
-        activeTaskCount: 5,
-        inFlightAgentCount: 2,
-        totalTasksCompleted: 100,
-        totalTasksFailed: 3,
-        updatedAt: new Date().toISOString(),
-      } as ProjectHealth;
-      return acc;
-    }, {} as Record<string, ProjectHealth>),
     loading: false,
     error: null,
     refresh: vi.fn(),
@@ -75,6 +81,14 @@ const noop = () => {};
 describe("ProjectOverview", () => {
   beforeEach(() => {
     vi.clearAllMocks();
+    // Reset mock to default state
+    vi.mocked(useProjectHealth).mockImplementation((projectIds: string[]) => ({
+      healthMap: createDefaultHealthMap(projectIds),
+      loading: false,
+      error: null,
+      refresh: vi.fn(),
+      refreshProject: vi.fn(),
+    }));
   });
 
   it("renders without crashing with projects", () => {
@@ -423,4 +437,93 @@ describe("ProjectOverview", () => {
       expect(screen.getByLabelText("Sort projects")).toBeDefined();
     });
   });
+
+  describe("FN-1734: Health polling scroll position regression", () => {
+    it("shows project cards when health hook is in loading state but healthMap has existing data", () => {
+      // This is the key regression test for FN-1734:
+      // When background polling refreshes health, loading becomes true but we
+      // should NOT show skeleton if we already have health data
+      vi.mocked(useProjectHealth).mockReturnValue({
+        healthMap: {
+          proj_1: {
+            projectId: "proj_1",
+            status: "active",
+            activeTaskCount: 5,
+            inFlightAgentCount: 2,
+            totalTasksCompleted: 100,
+            totalTasksFailed: 3,
+            updatedAt: new Date().toISOString(),
+          },
+        },
+        loading: false,
+        error: null,
+        refresh: vi.fn(),
+        refreshProject: vi.fn(),
+      });
+
+      const { container } = render(
+        <ProjectOverview
+          projects={[makeProject({ id: "proj_1", name: "Test Project" })]}
+          onSelectProject={noop}
+          onAddProject={noop}
+          onPauseProject={noop}
+          onResumeProject={noop}
+          onRemoveProject={noop}
+        />
+      );
+
+      // Project card should be visible
+      expect(screen.getByTestId("project-card-proj_1")).toBeDefined();
+      // Skeleton should NOT be shown (we have existing health data)
+      expect(screen.queryByTestId("project-grid-skeleton")).toBeNull();
+      // Grid should be rendered
+      expect(container.querySelector(".project-grid")).not.toBeNull();
+    });
+
+    it("shows skeleton only when projects exist but no health data has been fetched", () => {
+      // When loading prop is true AND we have no health data yet,
+      // skeleton should be shown
+      render(
+        <ProjectOverview
+          projects={[makeProject({ id: "proj_1", name: "Test Project" })]}
+          loading={true} // Projects are loading
+          onSelectProject={noop}
+          onAddProject={noop}
+          onPauseProject={noop}
+          onResumeProject={noop}
+          onRemoveProject={noop}
+        />
+      );
+
+      // Skeleton should be shown (projects loading)
+      expect(screen.getByTestId("project-grid-skeleton")).toBeDefined();
+    });
+
+    it("shows skeleton when health hook is loading with no existing health data", () => {
+      // When health hook returns loading=true AND we have no health data yet,
+      // skeleton should be shown even if loading prop is false
+      vi.mocked(useProjectHealth).mockReturnValue({
+        healthMap: {}, // Empty - no health data yet
+        loading: true, // Health is still loading
+        error: null,
+        refresh: vi.fn(),
+        refreshProject: vi.fn(),
+      });
+
+      render(
+        <ProjectOverview
+          projects={[makeProject({ id: "proj_1", name: "Test Project" })]}
+          loading={false}
+          onSelectProject={noop}
+          onAddProject={noop}
+          onPauseProject={noop}
+          onResumeProject={noop}
+          onRemoveProject={noop}
+        />
+      );
+
+      // Skeleton should be shown (health loading with no data)
+      expect(screen.getByTestId("project-grid-skeleton")).toBeDefined();
+    });
+  });
 });

packages/dashboard/app/hooks/__tests__/useProjectHealth.test.ts

@@ -248,4 +248,63 @@ describe("useProjectHealth", () => {
 
     pending.resolve(createHealth("p1"));
   });
+
+  it("does not set loading to true during background polling refreshes", async () => {
+    // This is a regression test for FN-1734: polling refreshes should NOT
+    // set loading=true, as that would cause UI flicker and scroll position resets
+    vi.useFakeTimers();
+    mockFetchProjectHealth.mockResolvedValue(createHealth("p1"));
+
+    const { result } = renderUseProjectHealth(["p1"]);
+
+    // Flush initial effects
+    await act(async () => {
+      await Promise.resolve();
+    });
+
+    // Initial load should be complete - loading should be false
+    expect(result.current.loading).toBe(false);
+    expect(result.current.healthMap.p1).toEqual(createHealth("p1"));
+
+    // Capture loading state before polling
+    const loadingBeforePolling = result.current.loading;
+
+    // Advance timer for polling refresh (10 seconds)
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(10_000);
+      await Promise.resolve();
+    });
+
+    // After polling refresh: loading should still be false
+    // (Regression: this was the bug - loading was set to true on every refresh)
+    expect(result.current.loading).toBe(false);
+    expect(loadingBeforePolling).toBe(false);
+
+    // Health data should still be present
+    expect(result.current.healthMap.p1).toEqual(createHealth("p1"));
+  });
+
+  it("handles projectIds transition from empty to non-empty correctly", async () => {
+    // This tests that switching from no projects to having projects works correctly
+    const { result, rerender } = renderUseProjectHealth([]);
+
+    await waitFor(() => {
+      expect(result.current.loading).toBe(false);
+    });
+    expect(result.current.healthMap).toEqual({});
+
+    // Now switch to having projects
+    rerender({ ids: ["p1"] });
+
+    // Should start loading again for the new project
+    await waitFor(() => {
+      expect(result.current.loading).toBe(true);
+    });
+
+    await waitFor(() => {
+      expect(result.current.loading).toBe(false);
+    });
+
+    expect(result.current.healthMap.p1).toEqual(createHealth("p1"));
+  });
 });

packages/dashboard/app/hooks/useProjectHealth.ts

@@ -5,7 +5,7 @@ import { fetchProjectHealth } from "../api";
 export interface UseMultiProjectHealthResult {
   /** Map of project ID to health data */
   healthMap: Record<string, ProjectHealth | null>;
-  /** Loading state */
+  /** Loading state - true only for initial load, false during background polling */
   loading: boolean;
   /** Error if any */
   error: string | null;
@@ -20,21 +20,39 @@ const BATCH_SIZE = 5; // Number of concurrent health fetches
 
 /**
  * Hook for fetching health metrics for multiple projects.
- * 
+ *
  * Automatically polls every 10 seconds when the ProjectOverview is visible.
  * Stops polling when component unmounts.
  * Fetches health in batches to avoid overwhelming the server.
+ *
+ * Loading behavior: `loading` is true only during the initial fetch.
+ * Background polling updates do NOT set `loading` to true, so the UI
+ * keeps previously loaded data visible during refreshes. This prevents
+ * skeleton flicker and scroll position resets during periodic updates.
  */
 export function useProjectHealth(projectIds: string[]): UseMultiProjectHealthResult {
   const [healthMap, setHealthMap] = useState<Record<string, ProjectHealth | null>>({});
-  const [loading, setLoading] = useState(false);
+  const [loading, setLoading] = useState(true); // Start true for initial load
   const [error, setError] = useState<string | null>(null);
   const intervalRef = useRef<NodeJS.Timeout | null>(null);
   const abortRef = useRef<AbortController | null>(null);
+  // Track if we've completed the initial load
+  const initialLoadCompleteRef = useRef(false);
 
+  /**
+   * Refresh health data for all projects.
+   * This is called both for initial load and for background polling.
+   * Background polling does NOT set loading=true to avoid UI flicker.
+   */
   const refresh = useCallback(async () => {
+    // Handle empty projectIds - clear health state and complete initial load
     if (projectIds.length === 0) {
       setHealthMap({});
+      // Mark initial load complete (there's nothing to fetch)
+      if (!initialLoadCompleteRef.current) {
+        initialLoadCompleteRef.current = true;
+      }
+      setLoading(false);
       return;
     }
 
@@ -44,16 +62,20 @@ export function useProjectHealth(projectIds: string[]): UseMultiProjectHealthRes
     }
     abortRef.current = new AbortController();
 
-    try {
+    // Determine if this is the initial load
+    const isInitial = !initialLoadCompleteRef.current;
+    if (isInitial) {
       setLoading(true);
-      setError(null);
+    }
+    setError(null);
 
+    try {
       // Fetch health in batches
       const newHealthMap: Record<string, ProjectHealth | null> = {};
-      
+
       for (let i = 0; i < projectIds.length; i += BATCH_SIZE) {
         const batch = projectIds.slice(i, i + BATCH_SIZE);
-        
+
         // Fetch this batch concurrently
         const batchResults = await Promise.allSettled(
           batch.map(async (id) => {
@@ -77,12 +99,15 @@ export function useProjectHealth(projectIds: string[]): UseMultiProjectHealthRes
       }
 
       setHealthMap(newHealthMap);
+      initialLoadCompleteRef.current = true;
     } catch (err) {
       if (err instanceof Error && err.name === "AbortError") {
         // Ignore abort errors
         return;
       }
       setError(err instanceof Error ? err.message : "Failed to fetch health data");
+      // Mark initial load complete even on error so we don't stay in loading state
+      initialLoadCompleteRef.current = true;
     } finally {
       setLoading(false);
     }
@@ -102,7 +127,10 @@ export function useProjectHealth(projectIds: string[]): UseMultiProjectHealthRes
 
   // Initial fetch and when project IDs change
   useEffect(() => {
-    refresh();
+    // Reset initial load state when projectIds changes
+    initialLoadCompleteRef.current = false;
+
+    void refresh();
 
     return () => {
       if (abortRef.current) {
@@ -122,7 +150,7 @@ export function useProjectHealth(projectIds: string[]): UseMultiProjectHealthRes
 
     // Start new polling interval
     intervalRef.current = setInterval(() => {
-      refresh();
+      void refresh();
     }, POLL_INTERVAL_MS);
 
     return () => {