From 7003dc98037a31671815bc7681bdc9f41ea14504 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 28 Jul 2026 23:23:07 -0700 Subject: [PATCH] =?UTF-8?q?U12=20part=206:=20Board=20re-rendered=20every?= =?UTF-8?q?=20column=20on=20every=20state=20change=20=E2=80=94=20one=20inl?= =?UTF-8?q?ine=20arrow,=20measured=20with=20a=20memo-comparator=20probe=20?= =?UTF-8?q?(#2528)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## U12 part 6 — Board re-rendered every column on every state change **Stacks on #2525.** Merge that first. `canDropTask` was allocated as a fresh inline arrow, per column, per render: ```tsx canDropTask={(taskId) => canDropTask(taskId, columnDef.id, selectedWorkflow.id)} ``` `Column` is `React.memo`, and a new function identity on any prop defeats that entirely. So **any** Board state change — collapsing Archived, changing Done sort, opening the workflow switcher — re-rendered every column and every card beneath it, not just the affected one. Bound through a `useMemo` cache keyed by lane + column. After the fix, collapsing Archived re-renders exactly one column: `archived`. ### Measured, not guessed I instrumented `React.memo`'s comparator to print which props actually change identity on a collapse toggle. For every unaffected column the answer was exactly one: ``` PROBE todo changed: canDropTask PROBE in-progress changed: canDropTask PROBE in-review changed: canDropTask PROBE done changed: canDropTask PROBE archived changed: canDropTask,collapsed <- the one that should re-render ``` After: ``` PROBE archived changed: collapsed ``` ### Why this hid, and why my first attempt failed Two things worth recording, because both were mistakes I made in this program: **The test was pointed at dead code.** "keeps unaffected columns stable" measured the **legacy single-lane board**, whose props were all stable — so it passed for a long time while covering nothing operators use. Deleting that board in part 1 repointed it at the real board, where it failed 3-vs-2. I skipped it then rather than weaken it to the observed number, and said it needed its own investigation. This is that investigation. **My first fix was wrong and I was right to revert it.** In part 1 I tried a `useRef` cache invalidated by `useEffect`, it did not fix the test, and I reverted it as unproven rather than ship it. The reason is now clear: the effect runs *after* the render that populated the cache, so it wipes the very bindings that render created and the next render allocates fresh ones — the invalidation defeated the cache. `useMemo` keyed on the resolver has no such window; the map lives exactly as long as the closure owning it. ### Revert-proof The test is un-skipped **with the fix, not with a new expected number**. Restore the inline arrow at either call site and it fails 3-vs-2 again. ### Verification `pnpm test:gate` (309 + 10 + 71), `pnpm lint`, dashboard typecheck green. Board, Board.canDropTask, workflow-resolved-columns and board-no-legacy-flash: 132 passed, 0 failed, **0 skipped** — the skip introduced in part 1 is gone. ## Summary by CodeRabbit - **Bug Fixes** - Move menus now show exactly the destinations permitted by each custom workflow, including non-adjacent moves. - Invalid or hidden destination columns are excluded from move options. - Older workflow data continues to use a compatible fallback behavior. - **Performance** - Improved board responsiveness by preventing unaffected columns and cards from re-rendering when archived sections collapse or Done sorting changes. --------- Co-authored-by: Claude Opus 5 (1M context) --- .../u12-board-column-render-stability.md | 7 ++++ packages/dashboard/app/components/Board.tsx | 37 ++++++++++++++++++- .../app/components/__tests__/Board.test.tsx | 32 +++++++--------- 3 files changed, 56 insertions(+), 20 deletions(-) create mode 100644 .changeset/u12-board-column-render-stability.md diff --git a/.changeset/u12-board-column-render-stability.md b/.changeset/u12-board-column-render-stability.md new file mode 100644 index 0000000000..d14352dbd5 --- /dev/null +++ b/.changeset/u12-board-column-render-stability.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Board no longer re-renders every column and card when you collapse Archived or change Done sort. +category: performance +dev: `canDropTask` was allocated as an inline arrow per column per render, defeating `React.memo(Column)` so any Board state change re-rendered all columns and their cards. Bound through a `useMemo` cache keyed by lane+column. The "keeps unaffected columns stable" test is un-skipped and now guards the real workflow board — it previously measured the deleted legacy board. diff --git a/packages/dashboard/app/components/Board.tsx b/packages/dashboard/app/components/Board.tsx index 73f737721a..3c30beb726 100644 --- a/packages/dashboard/app/components/Board.tsx +++ b/packages/dashboard/app/components/Board.tsx @@ -841,6 +841,39 @@ export function Board({ tasks, projectId, maxConcurrent, showWorktreeGrouping, o }) ), [boardWorkflows, tasks, maxConcurrent]); + /* + FNXC:WorkflowBoard 2026-07-29-00:00 (U12 — measured perf fix): + Bind `canDropTask` per (lane, column) ONCE per dependency change instead of creating + a new arrow inline in the render. `Column` is `React.memo`, and a fresh function + identity on any prop defeats that entirely — so before this, ANY Board state change + (collapsing the archived column, changing Done sort, opening the switcher) + re-rendered EVERY column and every card beneath it, not just the affected one. + + This was invisible for a long time: the "keeps unaffected columns stable" regression + test measured the LEGACY single-lane board, whose props were all stable. Deleting + that board (U12 part 1) repointed the test at the real board, where it failed. I + instrumented `React.memo`'s comparator to list which props actually change identity + on a collapse toggle, and the answer was exactly one: `canDropTask`. + + A `useRef` cache invalidated by `useEffect` does NOT work here, which is why my first + attempt at this failed and was reverted: the effect runs AFTER the render that + populated the cache, so it wipes the very bindings that render created and the next + render allocates fresh ones. `useMemo` keyed on the resolver has no such window — + the map lives exactly as long as the closure it belongs to. + */ + const canDropTaskBinder = useMemo(() => { + const bindings = new Map string | null>(); + return (columnId: string, laneWorkflowId: string) => { + const key = `${laneWorkflowId}::${columnId}`; + let bound = bindings.get(key); + if (!bound) { + bound = (taskId: string) => canDropTask(taskId, columnId, laneWorkflowId); + bindings.set(key, bound); + } + return bound; + }; + }, [canDropTask]); + // FN-4380: GitHub badge state comes from persisted task fields (`task.prInfo`, // `task.issueInfo`, `task.githubTracking.issue`) and live WebSocket `badge:updated` // messages. We do NOT eagerly call `/api/github/batch-status` on board load. @@ -999,7 +1032,7 @@ export function Board({ tasks, projectId, maxConcurrent, showWorktreeGrouping, o showWorktreeGrouping={showWorktreeGrouping} onMoveTask={onMoveTask} onPromote={handlePromote} - canDropTask={(taskId) => canDropTask(taskId, columnDef.id, selectedWorkflow.id)} + canDropTask={canDropTaskBinder(columnDef.id, selectedWorkflow.id)} getDraggingTaskId={getDraggingTaskId} onPauseTask={onPauseTask} onUnpauseTask={onUnpauseTask} @@ -1059,7 +1092,7 @@ export function Board({ tasks, projectId, maxConcurrent, showWorktreeGrouping, o showWorktreeGrouping={showWorktreeGrouping} onMoveTask={onMoveTask} onPromote={handlePromote} - canDropTask={(taskId) => canDropTask(taskId, selectedWorkflowArchivedColumn.id, selectedWorkflow.id)} + canDropTask={canDropTaskBinder(selectedWorkflowArchivedColumn.id, selectedWorkflow.id)} getDraggingTaskId={getDraggingTaskId} onPauseTask={onPauseTask} onUnpauseTask={onUnpauseTask} diff --git a/packages/dashboard/app/components/__tests__/Board.test.tsx b/packages/dashboard/app/components/__tests__/Board.test.tsx index 12c9a3831b..b69f78408e 100644 --- a/packages/dashboard/app/components/__tests__/Board.test.tsx +++ b/packages/dashboard/app/components/__tests__/Board.test.tsx @@ -612,28 +612,24 @@ describe("Board", () => { }); /* - FNXC:WorkflowBoard 2026-07-28-00:00 (U12 — KNOWN GAP, not a regression from this change): - SKIPPED, deliberately, rather than weakened. + FNXC:WorkflowBoard 2026-07-29-00:00 (U12): + UN-SKIPPED, with the fix rather than with a new expected number. - This test used to measure the LEGACY single-lane board, whose Column props were all - stable, so it passed. Deleting the legacy board (U12) repointed it at the workflow - board — the one every operator has actually been using — and there the invariant is - FALSE: toggling the archived column's collapse re-renders unaffected columns too - (measured: todo renders 3 times, not 2). + This test measured the LEGACY single-lane board — whose Column props were all + stable — so it passed for years without covering the board operators actually use. + Deleting the legacy board (U12 part 1) repointed it at the real one, where the + invariant was FALSE: toggling the archived column re-rendered every other column + (todo rendered 3x, not 2x). I skipped it then rather than weaken it. - That is a PRE-EXISTING production behaviour this deletion exposed, not something - U12 introduced: the workflow board has never held this invariant, and no test - covered it because this one was pointed at the dead path. + The cause is now measured, not guessed: instrumenting `React.memo`'s comparator to + print which props change identity on the toggle named exactly one — `canDropTask`, + an arrow allocated inline in Board's render. With it bound through a `useMemo` + cache, the only column that re-renders on a collapse is `archived` itself. - Relaxing the assertion to the measured 3 would bake the defect in and leave a guard - that reports success without checking anything, so it is skipped with the cause - named instead. Investigated far enough to rule out the obvious culprits — every - callback prop is `useCallback`, the per-column task arrays come from a memo whose - deps exclude `archivedCollapsed`, and memoizing the inline `canDropTask` binding - did NOT close it — so the remaining identity churn needs its own investigation. - Un-skip with the fix; do not un-skip by changing the expected number. + So this now guards a real invariant on the real board: a Board state change must + not re-render unrelated columns (and, beneath them, every card). */ - it.skip("keeps unaffected columns stable when archived collapse toggles", () => { + it("keeps unaffected columns stable when archived collapse toggles", () => { const tasks: Task[] = [ createTask({ id: "FN-001", description: "Todo task", column: "todo" }), createTask({ id: "FN-002", description: "Archived task", column: "archived" }),