feat(FN-1544): add viewport-gated loading and lightweight memo comparison
- Add viewport-gated fetching to TaskCard component so only visible cards load their data - Implement lightweight memo comparison to prevent unnecessary re-renders - Add lazy enable gates to useSessionFiles and useTaskDiffStats hooks with caching - Add comprehensive tests for useSessionFiles and useTaskDiffStats hooks - Document viewport-gated loading patterns in memory and dashboard-load performance docs
This commit is contained in:
@@ -22,6 +22,31 @@
|
||||
- Debug logging via `process.env.FUSION_DEBUG_AI` helps diagnose AI session issues
|
||||
- When testing `console.warn` calls that expect multiple substrings in a single concatenated string, use `expect(mock.calls[0][0]).toMatch(/substring1/)` pattern instead of `expect.stringContaining()` on multiple arguments
|
||||
|
||||
## FN-1544: Viewport-Gated Card Metadata Loading
|
||||
|
||||
When optimizing dashboard performance for large task sets:
|
||||
- **Viewport gating pattern**: Use IntersectionObserver with `rootMargin: "200px"` to prefetch data just before cards become visible
|
||||
- **Lazy enable option**: Add `{ enabled?: boolean }` parameter to hooks (default `true` for backward compatibility)
|
||||
- **Disabled state**: Return stable empty state without triggering fetches when `enabled: false`
|
||||
- **In-memory caching**: Use TTL-based caching (e.g., 30 seconds) to avoid repeated fetches during rerenders
|
||||
- **Cache key format**: `"taskId:projectId"` for separate caching per task/project context
|
||||
- **Cache hit behavior**: Return immediately without loading flicker — don't set loading state on cache hit
|
||||
- **Export test helpers**: Export `__test_clearCache()` functions for test isolation
|
||||
- **Lightweight comparisons**: Replace `JSON.stringify` in memo comparators with field-by-field comparisons (e.g., `areAttachmentsEqual`, `areCommentsEqual`)
|
||||
|
||||
Example from `useTaskDiffStats`:
|
||||
```typescript
|
||||
// Cache keyed by taskId:projectId
|
||||
const diffStatsCache = new Map<string, { stats: DiffStats; expiresAt: number }>();
|
||||
|
||||
// Hook returns immediately on cache hit
|
||||
if (cached) {
|
||||
setStats(cached);
|
||||
setLoading(false);
|
||||
return;
|
||||
}
|
||||
```
|
||||
|
||||
## 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.
|
||||
|
||||
@@ -138,3 +138,104 @@ SCAN agents
|
||||
- `packages/core/src/db.test.ts` - Update expected index list
|
||||
- `packages/core/src/run-audit.test.ts` - Update expected index list
|
||||
- `packages/core/src/__tests__/task-documents.test.ts` - Update expected index list
|
||||
|
||||
---
|
||||
|
||||
# Dashboard Card Rendering Performance (FN-1544)
|
||||
|
||||
**Date:** 2026-04-10
|
||||
**Task:** FN-1544
|
||||
|
||||
## Executive Summary
|
||||
|
||||
Performance analysis identified that `TaskCard` components triggered expensive network requests (`session-files` and `diff` endpoints) eagerly on initial render, causing sluggish board/list views with large task sets. Additionally, the memo comparator used high-cost `JSON.stringify` on attachments and comments arrays.
|
||||
|
||||
## Issues Identified
|
||||
|
||||
### Issue 1: Eager Session Files Fetching (HIGH)
|
||||
|
||||
**Problem:** Every `TaskCard` unconditionally called `useSessionFiles` hook, which fetches from `/api/tasks/:id/session-files`. With many cards visible or nearly-visible, this created a flood of network requests on initial render.
|
||||
|
||||
**Solution:** Added an `enabled` parameter to `useSessionFiles` (default `true` for backward compatibility). The hook returns stable empty state when disabled without triggering fetches.
|
||||
|
||||
### Issue 2: Eager Diff Stats Fetching (HIGH)
|
||||
|
||||
**Problem:** Every `TaskCard` for done tasks unconditionally called `useTaskDiffStats` hook, which fetches from `/api/tasks/:id/diff`. This caused repeated fetches during rerenders.
|
||||
|
||||
**Solution:**
|
||||
1. Added an `enabled` parameter to `useTaskDiffStats` (default `true` for backward compatibility)
|
||||
2. Added short-lived in-memory caching (30-second TTL) keyed by `taskId:projectId` to avoid repeated fetches during rerenders
|
||||
3. Cache hits return immediately without loading flicker
|
||||
|
||||
### Issue 3: High-Cost Memo Comparison (MEDIUM)
|
||||
|
||||
**Problem:** `areTaskCardPropsEqual` used `JSON.stringify(attachments)` and `JSON.stringify(comments)` for comparison. For tasks with many attachments or comments, this serialized entire arrays on every render cycle.
|
||||
|
||||
**Solution:** Replaced `JSON.stringify` with lightweight field-by-field comparison functions:
|
||||
- `areAttachmentsEqual()` - compares attachment counts and metadata fields (filename, mimeType, size)
|
||||
- `areCommentsEqual()` - compares comment counts and metadata fields (author, content, createdAt)
|
||||
|
||||
## Mitigation Pattern: Viewport-Gated Card Metadata Loading
|
||||
|
||||
The key mitigation uses the existing `isInViewport` state with a 200px margin:
|
||||
|
||||
```typescript
|
||||
// TaskCard.tsx - Hook calls are gated on viewport visibility
|
||||
const { files: sessionFiles, loading: sessionFilesLoading } = useSessionFiles(
|
||||
task.id,
|
||||
task.worktree,
|
||||
task.column,
|
||||
projectId,
|
||||
{ enabled: isInViewport }, // Only fetch when card is visible
|
||||
);
|
||||
|
||||
const { stats: diffStats } = useTaskDiffStats(
|
||||
task.id,
|
||||
task.column,
|
||||
task.mergeDetails?.commitSha,
|
||||
projectId,
|
||||
{ enabled: isInViewport }, // Only fetch when card is visible
|
||||
);
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
- Offscreen cards don't trigger fetches
|
||||
- Cards entering viewport trigger fetches as expected
|
||||
- No new polling loops or background timers
|
||||
- Preserves existing card behaviors (badges, file counts, drag/drop, etc.)
|
||||
|
||||
## Cache Implementation Details
|
||||
|
||||
### useTaskDiffStats Cache
|
||||
|
||||
```typescript
|
||||
const diffStatsCache = new Map<string, { stats: DiffStats; expiresAt: number }>();
|
||||
const CACHE_TTL_MS = 30_000; // 30 seconds
|
||||
```
|
||||
|
||||
- Key format: `"taskId:projectId"`
|
||||
- Entries expire after TTL to ensure freshness
|
||||
- Cache is checked before initiating fetch - returns immediately on hit
|
||||
- Export `__test_clearDiffStatsCache()` for testing
|
||||
|
||||
## Files Modified
|
||||
|
||||
- `packages/dashboard/app/hooks/useSessionFiles.ts` - Added `enabled` option
|
||||
- `packages/dashboard/app/hooks/useTaskDiffStats.ts` - Added `enabled` option and caching
|
||||
- `packages/dashboard/app/hooks/__tests__/useSessionFiles.test.ts` - Tests for `enabled` option
|
||||
- `packages/dashboard/app/hooks/__tests__/useTaskDiffStats.test.ts` - Tests for `enabled` option and caching
|
||||
- `packages/dashboard/app/components/TaskCard.tsx` - Viewport-gated hook calls, lightweight memo comparison
|
||||
- `packages/dashboard/app/components/TaskCard.test.tsx` - Fixed pre-existing test expectations
|
||||
|
||||
## Impact Assessment
|
||||
|
||||
- **Initial render:** Significant reduction in network requests as offscreen cards don't fetch
|
||||
- **Rerenders:** Reduced CPU usage from eliminating `JSON.stringify` on large arrays
|
||||
- **Cache efficiency:** Repeated renders of the same task use cached diff stats instead of refetching
|
||||
- **User experience:** Board/list views feel more responsive, especially with many tasks
|
||||
|
||||
## Key Learnings
|
||||
|
||||
1. **Viewport gating is effective** - Using IntersectionObserver with a margin lets us fetch just-in-time without visible delay
|
||||
2. **Caching with TTL prevents staleness** - 30-second cache balances freshness with reduced network overhead
|
||||
3. **Lightweight comparisons outperform serialization** - Field-by-field comparison is O(n) vs JSON.stringify's O(n) + allocation overhead
|
||||
|
||||
@@ -298,7 +298,8 @@ describe("TaskCard mission badge", () => {
|
||||
expect(badge).not.toBeNull();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(badge?.textContent).toContain("Database Optimiza...");
|
||||
// MAX_MISSION_TITLE_LENGTH is 12, so first 9 chars + "..."
|
||||
expect(badge?.textContent).toContain("Database ...");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -325,8 +326,8 @@ describe("TaskCard mission badge", () => {
|
||||
expect(badge).not.toBeNull();
|
||||
|
||||
await waitFor(() => {
|
||||
// MAX_MISSION_TITLE_LENGTH is 20, so first 17 chars + "..."
|
||||
expect(badge?.textContent).toContain("This Is A Very Lo...");
|
||||
// MAX_MISSION_TITLE_LENGTH is 12, so first 9 chars + "..."
|
||||
expect(badge?.textContent).toContain("This Is A...");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -121,6 +121,49 @@ function areTaskDependenciesEqual(previous: string[], next: string[]): boolean {
|
||||
return previous.every((dependency, index) => dependency === next[index]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Lightweight comparison for attachment metadata (not file content).
|
||||
* Compares counts and top-level fields that affect card rendering.
|
||||
*/
|
||||
function areAttachmentsEqual(previous: Task["attachments"], next: Task["attachments"]): boolean {
|
||||
if (!previous && !next) return true;
|
||||
if (!previous || !next) return false;
|
||||
if (previous.length !== next.length) return false;
|
||||
|
||||
// Compare attachment metadata that affects card rendering
|
||||
return previous.every((att, i) => {
|
||||
const nextAtt = next[i];
|
||||
if (!nextAtt) return false;
|
||||
// Compare fields that affect the card's visual state
|
||||
return (
|
||||
att.filename === nextAtt.filename &&
|
||||
att.mimeType === nextAtt.mimeType &&
|
||||
att.size === nextAtt.size
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Lightweight comparison for comments.
|
||||
* Compares counts and top-level fields that affect card rendering.
|
||||
*/
|
||||
function areCommentsEqual(previous: Task["comments"], next: Task["comments"]): boolean {
|
||||
if (!previous && !next) return true;
|
||||
if (!previous || !next) return false;
|
||||
if (previous.length !== next.length) return false;
|
||||
|
||||
// Compare comment metadata that affects card rendering
|
||||
return previous.every((comment, i) => {
|
||||
const nextComment = next[i];
|
||||
if (!nextComment) return false;
|
||||
return (
|
||||
comment.author === nextComment.author &&
|
||||
comment.content === nextComment.content &&
|
||||
comment.createdAt === nextComment.createdAt
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
// Keep this comparator aligned with the fields TaskCard renders directly and the
|
||||
// task metadata that influences child badge freshness/subscriptions.
|
||||
function areTaskCardPropsEqual(previous: TaskCardProps, next: TaskCardProps): boolean {
|
||||
@@ -164,8 +207,8 @@ function areTaskCardPropsEqual(previous: TaskCardProps, next: TaskCardProps): bo
|
||||
previousTask.missionId === nextTask.missionId &&
|
||||
previousTask.assignedAgentId === nextTask.assignedAgentId &&
|
||||
previousTask.mergeRetries === nextTask.mergeRetries &&
|
||||
JSON.stringify(previousTask.attachments ?? []) === JSON.stringify(nextTask.attachments ?? []) &&
|
||||
JSON.stringify(previousTask.comments ?? []) === JSON.stringify(nextTask.comments ?? []) &&
|
||||
areAttachmentsEqual(previousTask.attachments, nextTask.attachments) &&
|
||||
areCommentsEqual(previousTask.comments, nextTask.comments) &&
|
||||
areTaskDependenciesEqual(previousTask.dependencies, nextTask.dependencies) &&
|
||||
areTaskStepsEqual(previousTask.steps, nextTask.steps) &&
|
||||
areTaskBadgeInfosEqual(previousTask.prInfo, nextTask.prInfo) &&
|
||||
@@ -448,8 +491,24 @@ function TaskCardComponent({
|
||||
}, [hasGitHubBadge, isInViewport, subscribeToBadge, task.id, unsubscribeFromBadge]);
|
||||
|
||||
const liveBadgeData = badgeUpdates.get(task.id);
|
||||
const { files: sessionFiles, loading: sessionFilesLoading } = useSessionFiles(task.id, task.worktree, task.column, projectId);
|
||||
const { stats: diffStats } = useTaskDiffStats(task.id, task.column, task.mergeDetails?.commitSha, projectId);
|
||||
|
||||
// Viewport-gated session files fetching - only fetch when card is visible
|
||||
const { files: sessionFiles, loading: sessionFilesLoading } = useSessionFiles(
|
||||
task.id,
|
||||
task.worktree,
|
||||
task.column,
|
||||
projectId,
|
||||
{ enabled: isInViewport },
|
||||
);
|
||||
|
||||
// Viewport-gated diff stats fetching - only fetch when card is visible
|
||||
const { stats: diffStats } = useTaskDiffStats(
|
||||
task.id,
|
||||
task.column,
|
||||
task.mergeDetails?.commitSha,
|
||||
projectId,
|
||||
{ enabled: isInViewport },
|
||||
);
|
||||
|
||||
// Get fresh batch data if available
|
||||
const batchData = useMemo(() => getFreshBatchData(task.id, projectId), [task.id, projectId]);
|
||||
|
||||
@@ -70,4 +70,54 @@ describe("useSessionFiles", () => {
|
||||
expect(result.current.files).toEqual([]);
|
||||
expect(mockFetchSessionFiles).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe("enabled option", () => {
|
||||
it("fetches when enabled is true (default)", async () => {
|
||||
mockFetchSessionFiles.mockResolvedValueOnce(["file.ts"]);
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useSessionFiles("FN-123", "/repo/.worktrees/kb-123", "in-progress", undefined, { enabled: true }),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
expect(result.current.files).toEqual(["file.ts"]);
|
||||
expect(mockFetchSessionFiles).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("fetches when enabled is not specified (default)", async () => {
|
||||
mockFetchSessionFiles.mockResolvedValueOnce(["file.ts"]);
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useSessionFiles("FN-123", "/repo/.worktrees/kb-123", "in-progress"),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
expect(result.current.files).toEqual(["file.ts"]);
|
||||
expect(mockFetchSessionFiles).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not fetch when enabled is false", async () => {
|
||||
const { result } = renderHook(() =>
|
||||
useSessionFiles("FN-123", "/repo/.worktrees/kb-123", "in-progress", undefined, { enabled: false }),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
expect(result.current.files).toEqual([]);
|
||||
expect(mockFetchSessionFiles).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns stable state (loading: false) when disabled", async () => {
|
||||
const { result } = renderHook(() =>
|
||||
useSessionFiles("FN-123", "/repo/.worktrees/kb-123", "in-progress", undefined, { enabled: false }),
|
||||
);
|
||||
|
||||
// Immediately check (before any async)
|
||||
expect(result.current.loading).toBe(false);
|
||||
expect(result.current.files).toEqual([]);
|
||||
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
expect(result.current.files).toEqual([]);
|
||||
expect(mockFetchSessionFiles).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { renderHook, waitFor } from "@testing-library/react";
|
||||
import { useTaskDiffStats } from "../useTaskDiffStats";
|
||||
import { useTaskDiffStats, __test_clearDiffStatsCache } from "../useTaskDiffStats";
|
||||
import * as api from "../../api";
|
||||
|
||||
vi.mock("../../api", () => ({
|
||||
@@ -12,6 +12,7 @@ const mockFetchTaskDiff = vi.mocked(api.fetchTaskDiff);
|
||||
describe("useTaskDiffStats", () => {
|
||||
beforeEach(() => {
|
||||
mockFetchTaskDiff.mockReset();
|
||||
__test_clearDiffStatsCache();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -159,4 +160,271 @@ describe("useTaskDiffStats", () => {
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
expect(result.current.stats).toBeNull();
|
||||
});
|
||||
|
||||
describe("enabled option", () => {
|
||||
it("fetches when enabled is true (default)", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValueOnce({
|
||||
files: [],
|
||||
stats: { filesChanged: 1, additions: 2, deletions: 3 },
|
||||
});
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useTaskDiffStats("FN-123", "done", "abc1234", undefined, { enabled: true }),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
expect(result.current.stats).toEqual({ filesChanged: 1, additions: 2, deletions: 3 });
|
||||
expect(mockFetchTaskDiff).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("fetches when enabled is not specified (default)", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValueOnce({
|
||||
files: [],
|
||||
stats: { filesChanged: 1, additions: 2, deletions: 3 },
|
||||
});
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useTaskDiffStats("FN-123", "done", "abc1234", undefined),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
expect(result.current.stats).toEqual({ filesChanged: 1, additions: 2, deletions: 3 });
|
||||
expect(mockFetchTaskDiff).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not fetch when enabled is false", async () => {
|
||||
const { result } = renderHook(() =>
|
||||
useTaskDiffStats("FN-123", "done", "abc1234", undefined, { enabled: false }),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
expect(result.current.stats).toBeNull();
|
||||
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns stable state (loading: false) when disabled", async () => {
|
||||
const { result } = renderHook(() =>
|
||||
useTaskDiffStats("FN-123", "done", "abc1234", undefined, { enabled: false }),
|
||||
);
|
||||
|
||||
// Immediately check (before any async)
|
||||
expect(result.current.loading).toBe(false);
|
||||
expect(result.current.stats).toBeNull();
|
||||
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
expect(result.current.stats).toBeNull();
|
||||
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("respects enabled flag changes", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValueOnce({
|
||||
files: [],
|
||||
stats: { filesChanged: 5, additions: 10, deletions: 2 },
|
||||
});
|
||||
|
||||
const { result, rerender } = renderHook(
|
||||
({ enabled }) => useTaskDiffStats("FN-123", "done", "abc1234", undefined, { enabled }),
|
||||
{ initialProps: { enabled: true } },
|
||||
);
|
||||
|
||||
// Fetch should happen initially
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
expect(result.current.stats).toEqual({ filesChanged: 5, additions: 10, deletions: 2 });
|
||||
|
||||
// Toggle enabled off - should not refetch
|
||||
mockFetchTaskDiff.mockClear();
|
||||
rerender({ enabled: false });
|
||||
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("caching", () => {
|
||||
beforeEach(() => {
|
||||
// Clear cache before each caching test to ensure isolation
|
||||
__test_clearDiffStatsCache();
|
||||
mockFetchTaskDiff.mockClear();
|
||||
});
|
||||
|
||||
it("returns cached stats without making a fetch", async () => {
|
||||
// First render - fetches and caches
|
||||
mockFetchTaskDiff.mockResolvedValueOnce({
|
||||
files: [],
|
||||
stats: { filesChanged: 10, additions: 50, deletions: 5 },
|
||||
});
|
||||
|
||||
const { result: first } = renderHook(() =>
|
||||
useTaskDiffStats("FN-CACHE-1", "done", "abc1234", undefined),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(first.current.loading).toBe(false));
|
||||
expect(first.current.stats).toEqual({ filesChanged: 10, additions: 50, deletions: 5 });
|
||||
expect(mockFetchTaskDiff).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Second render with same taskId - should use cache
|
||||
mockFetchTaskDiff.mockClear();
|
||||
|
||||
const { result: second } = renderHook(() =>
|
||||
useTaskDiffStats("FN-CACHE-1", "done", "abc1234", undefined),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(second.current.loading).toBe(false));
|
||||
// Should return cached value, not new value
|
||||
expect(second.current.stats).toEqual({ filesChanged: 10, additions: 50, deletions: 5 });
|
||||
// No additional fetch
|
||||
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns cached stats immediately without loading flicker", async () => {
|
||||
// Pre-populate cache by doing an initial fetch
|
||||
mockFetchTaskDiff.mockResolvedValueOnce({
|
||||
files: [],
|
||||
stats: { filesChanged: 7, additions: 30, deletions: 3 },
|
||||
});
|
||||
|
||||
const { result: first } = renderHook(() =>
|
||||
useTaskDiffStats("FN-CACHE-2", "done", "abc1234", undefined),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(first.current.loading).toBe(false));
|
||||
expect(first.current.stats).toEqual({ filesChanged: 7, additions: 30, deletions: 3 });
|
||||
|
||||
// Second hook instance - cache hit should be immediate
|
||||
mockFetchTaskDiff.mockClear();
|
||||
|
||||
const { result: second } = renderHook(() =>
|
||||
useTaskDiffStats("FN-CACHE-2", "done", "abc1234", undefined),
|
||||
);
|
||||
|
||||
// Cache hit - no loading state, no fetch
|
||||
expect(second.current.loading).toBe(false);
|
||||
expect(second.current.stats).toEqual({ filesChanged: 7, additions: 30, deletions: 3 });
|
||||
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("caches stats separately per task ID", async () => {
|
||||
// Pre-populate cache for FN-100
|
||||
mockFetchTaskDiff.mockResolvedValueOnce({
|
||||
files: [],
|
||||
stats: { filesChanged: 5, additions: 25, deletions: 2 },
|
||||
});
|
||||
|
||||
const { result: first } = renderHook(() =>
|
||||
useTaskDiffStats("FN-100", "done", "abc1234", undefined),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(first.current.loading).toBe(false));
|
||||
expect(first.current.stats).toEqual({ filesChanged: 5, additions: 25, deletions: 2 });
|
||||
|
||||
// Fetch for FN-200 (note: cache already has FN-100 entry)
|
||||
mockFetchTaskDiff.mockResolvedValueOnce({
|
||||
files: [],
|
||||
stats: { filesChanged: 15, additions: 100, deletions: 10 },
|
||||
});
|
||||
|
||||
const { result: second } = renderHook(() =>
|
||||
useTaskDiffStats("FN-200", "done", "def5678", undefined),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(second.current.loading).toBe(false));
|
||||
expect(second.current.stats).toEqual({ filesChanged: 15, additions: 100, deletions: 10 });
|
||||
|
||||
// Both should have been fetched (FN-100 was in cache from first test's beforeEach, but this test's beforeEach cleared it)
|
||||
expect(mockFetchTaskDiff).toHaveBeenCalledTimes(2);
|
||||
|
||||
// Each should have its own cached value
|
||||
mockFetchTaskDiff.mockClear();
|
||||
|
||||
const { result: cached1 } = renderHook(() =>
|
||||
useTaskDiffStats("FN-100", "done", "abc1234", undefined),
|
||||
);
|
||||
const { result: cached2 } = renderHook(() =>
|
||||
useTaskDiffStats("FN-200", "done", "def5678", undefined),
|
||||
);
|
||||
|
||||
expect(cached1.current.stats).toEqual({ filesChanged: 5, additions: 25, deletions: 2 });
|
||||
expect(cached2.current.stats).toEqual({ filesChanged: 15, additions: 100, deletions: 10 });
|
||||
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("caches stats separately per project ID", async () => {
|
||||
// Pre-populate cache for task without projectId
|
||||
mockFetchTaskDiff.mockResolvedValueOnce({
|
||||
files: [],
|
||||
stats: { filesChanged: 3, additions: 15, deletions: 1 },
|
||||
});
|
||||
|
||||
const { result: first } = renderHook(() =>
|
||||
useTaskDiffStats("FN-PROJ", "done", "abc1234", undefined),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(first.current.loading).toBe(false));
|
||||
expect(first.current.stats).toEqual({ filesChanged: 3, additions: 15, deletions: 1 });
|
||||
|
||||
// Fetch same task with different projectId
|
||||
mockFetchTaskDiff.mockResolvedValueOnce({
|
||||
files: [],
|
||||
stats: { filesChanged: 8, additions: 40, deletions: 4 },
|
||||
});
|
||||
|
||||
const { result: second } = renderHook(() =>
|
||||
useTaskDiffStats("FN-PROJ", "done", "abc1234", "proj-1"),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(second.current.loading).toBe(false));
|
||||
expect(second.current.stats).toEqual({ filesChanged: 8, additions: 40, deletions: 4 });
|
||||
|
||||
// Both should have been fetched
|
||||
expect(mockFetchTaskDiff).toHaveBeenCalledTimes(2);
|
||||
|
||||
// Cache should have both entries
|
||||
mockFetchTaskDiff.mockClear();
|
||||
|
||||
const { result: cachedNoProj } = renderHook(() =>
|
||||
useTaskDiffStats("FN-PROJ", "done", "abc1234", undefined),
|
||||
);
|
||||
const { result: cachedWithProj } = renderHook(() =>
|
||||
useTaskDiffStats("FN-PROJ", "done", "abc1234", "proj-1"),
|
||||
);
|
||||
|
||||
expect(cachedNoProj.current.stats).toEqual({ filesChanged: 3, additions: 15, deletions: 1 });
|
||||
expect(cachedWithProj.current.stats).toEqual({ filesChanged: 8, additions: 40, deletions: 4 });
|
||||
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("clears cache via __test_clearDiffStatsCache", async () => {
|
||||
// Pre-populate cache
|
||||
mockFetchTaskDiff.mockResolvedValueOnce({
|
||||
files: [],
|
||||
stats: { filesChanged: 5, additions: 25, deletions: 2 },
|
||||
});
|
||||
|
||||
const { result: first } = renderHook(() =>
|
||||
useTaskDiffStats("FN-CLEAR", "done", "abc1234", undefined),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(first.current.loading).toBe(false));
|
||||
expect(first.current.stats).toEqual({ filesChanged: 5, additions: 25, deletions: 2 });
|
||||
|
||||
// Clear cache
|
||||
__test_clearDiffStatsCache();
|
||||
|
||||
// Next fetch should not hit cache
|
||||
mockFetchTaskDiff.mockClear();
|
||||
mockFetchTaskDiff.mockResolvedValueOnce({
|
||||
files: [],
|
||||
stats: { filesChanged: 99, additions: 999, deletions: 99 },
|
||||
});
|
||||
|
||||
const { result: second } = renderHook(() =>
|
||||
useTaskDiffStats("FN-CLEAR", "done", "abc1234", undefined),
|
||||
);
|
||||
|
||||
await waitFor(() => expect(second.current.loading).toBe(false));
|
||||
// Should fetch fresh value
|
||||
expect(second.current.stats).toEqual({ filesChanged: 99, additions: 999, deletions: 99 });
|
||||
expect(mockFetchTaskDiff).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8,11 +8,39 @@ interface UseSessionFilesResult {
|
||||
loading: boolean;
|
||||
}
|
||||
|
||||
export function useSessionFiles(taskId: string, worktree: string | undefined, column: string, projectId?: string): UseSessionFilesResult {
|
||||
interface UseSessionFilesOptions {
|
||||
/** Enable fetching when true (default). Suppresses fetches for offscreen cards. */
|
||||
enabled?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetches session files for tasks with active worktrees.
|
||||
*
|
||||
* @param taskId - Task identifier
|
||||
* @param worktree - Worktree path (undefined = no worktree)
|
||||
* @param column - Current task column
|
||||
* @param projectId - Optional project identifier
|
||||
* @param options.enabled - When false, no fetch is made and returns empty/stable state
|
||||
*/
|
||||
export function useSessionFiles(
|
||||
taskId: string,
|
||||
worktree: string | undefined,
|
||||
column: string,
|
||||
projectId?: string,
|
||||
options: UseSessionFilesOptions = {},
|
||||
): UseSessionFilesResult {
|
||||
const enabled = options.enabled ?? true;
|
||||
const [files, setFiles] = useState<string[]>([]);
|
||||
const [loading, setLoading] = useState(false);
|
||||
|
||||
useEffect(() => {
|
||||
// Disabled state: return stable empty state without fetching
|
||||
if (!enabled) {
|
||||
setFiles([]);
|
||||
setLoading(false);
|
||||
return;
|
||||
}
|
||||
|
||||
if (!taskId || !worktree || !ACTIVE_COLUMNS.has(column)) {
|
||||
setFiles([]);
|
||||
setLoading(false);
|
||||
@@ -40,7 +68,7 @@ export function useSessionFiles(taskId: string, worktree: string | undefined, co
|
||||
}
|
||||
|
||||
void load();
|
||||
}, [taskId, worktree, column, projectId]);
|
||||
}, [taskId, worktree, column, projectId, enabled]);
|
||||
|
||||
return { files, loading };
|
||||
}
|
||||
|
||||
@@ -12,6 +12,54 @@ interface UseTaskDiffStatsResult {
|
||||
loading: boolean;
|
||||
}
|
||||
|
||||
interface UseTaskDiffStatsOptions {
|
||||
/** Enable fetching when true (default). Suppresses fetches for offscreen cards. */
|
||||
enabled?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Cache for diff stats to avoid repeated fetches during rerenders.
|
||||
* Key format: "taskId:projectId"
|
||||
* Entries expire after the TTL to ensure freshness.
|
||||
*/
|
||||
const diffStatsCache = new Map<string, { stats: DiffStats; expiresAt: number }>();
|
||||
const CACHE_TTL_MS = 30_000; // 30 seconds
|
||||
|
||||
function getCacheKey(taskId: string, projectId?: string): string {
|
||||
return `${taskId}:${projectId ?? ""}`;
|
||||
}
|
||||
|
||||
function getCachedStats(taskId: string, projectId?: string): DiffStats | null {
|
||||
const key = getCacheKey(taskId, projectId);
|
||||
const entry = diffStatsCache.get(key);
|
||||
|
||||
if (!entry) return null;
|
||||
|
||||
// Check expiration
|
||||
if (Date.now() > entry.expiresAt) {
|
||||
diffStatsCache.delete(key);
|
||||
return null;
|
||||
}
|
||||
|
||||
return entry.stats;
|
||||
}
|
||||
|
||||
function setCachedStats(taskId: string, projectId: string | undefined, stats: DiffStats): void {
|
||||
const key = getCacheKey(taskId, projectId);
|
||||
diffStatsCache.set(key, {
|
||||
stats,
|
||||
expiresAt: Date.now() + CACHE_TTL_MS,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Clears all entries from the diff stats cache.
|
||||
* Exported for testing purposes.
|
||||
*/
|
||||
export function __test_clearDiffStatsCache(): void {
|
||||
diffStatsCache.clear();
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetches diff stats for a done task that has a merge commit SHA.
|
||||
*
|
||||
@@ -20,17 +68,32 @@ interface UseTaskDiffStatsResult {
|
||||
* hook the card falls back to `mergeDetails.filesChanged`, which is
|
||||
* computed at merge time via `git show --shortstat` and can differ from the
|
||||
* diff endpoint's count when the merge includes changes from multiple branches.
|
||||
*
|
||||
* @param taskId - Task identifier
|
||||
* @param column - Current task column
|
||||
* @param commitSha - Merge commit SHA (undefined = no merge yet)
|
||||
* @param projectId - Optional project identifier
|
||||
* @param options.enabled - When false, no fetch is made and returns empty/stable state
|
||||
*/
|
||||
export function useTaskDiffStats(
|
||||
taskId: string,
|
||||
column: string,
|
||||
commitSha: string | undefined,
|
||||
projectId?: string,
|
||||
options: UseTaskDiffStatsOptions = {},
|
||||
): UseTaskDiffStatsResult {
|
||||
const enabled = options.enabled ?? true;
|
||||
const [stats, setStats] = useState<DiffStats | null>(null);
|
||||
const [loading, setLoading] = useState(false);
|
||||
|
||||
useEffect(() => {
|
||||
// Disabled state: return stable empty state without fetching
|
||||
if (!enabled) {
|
||||
setStats(null);
|
||||
setLoading(false);
|
||||
return;
|
||||
}
|
||||
|
||||
// Only fetch for done tasks with a recorded merge commit
|
||||
if (!taskId || column !== "done" || !commitSha) {
|
||||
setStats(null);
|
||||
@@ -38,6 +101,14 @@ export function useTaskDiffStats(
|
||||
return;
|
||||
}
|
||||
|
||||
// Check cache first - return immediately without loading flicker
|
||||
const cached = getCachedStats(taskId, projectId);
|
||||
if (cached) {
|
||||
setStats(cached);
|
||||
setLoading(false);
|
||||
return;
|
||||
}
|
||||
|
||||
let cancelled = false;
|
||||
|
||||
async function load() {
|
||||
@@ -46,6 +117,8 @@ export function useTaskDiffStats(
|
||||
const data = await fetchTaskDiff(taskId, undefined, projectId);
|
||||
if (!cancelled) {
|
||||
setStats(data.stats);
|
||||
// Store in cache
|
||||
setCachedStats(taskId, projectId, data.stats);
|
||||
}
|
||||
} catch {
|
||||
if (!cancelled) {
|
||||
@@ -63,7 +136,7 @@ export function useTaskDiffStats(
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [taskId, column, commitSha, projectId]);
|
||||
}, [taskId, column, commitSha, projectId, enabled]);
|
||||
|
||||
return { stats, loading };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user