U12 part 6: Board re-rendered every column on every state change — one inline arrow, measured with a memo-comparator probe (#2528)

## 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.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## 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.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-28 23:23:07 -07:00
committed by GitHub
parent da0351857e
commit 7003dc9803
3 changed files with 56 additions and 20 deletions

View File

@@ -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.

View File

@@ -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, (taskId: string) => 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}

View File

@@ -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" }),