Files
docmost/docs/superpowers/plans/2026-04-18-hide-property-persistence-bug.md

15 KiB

Hide-Property Persistence Bug Fix Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: Stop hidden column toggles from silently reappearing when another view-config mutation or a websocket-driven refetch lands between the toggle and the 300 ms debounced persist.

Architecture: Two narrow fixes in the client — nothing server-side. (1) The blind setColumnVisibility(derivedColumnVisibility) sync effect in use-base-table.ts is scoped to run only when the user switches views, so refetches of the same view can't overwrite pending local toggles. (2) The sort/filter mutations in base-toolbar.tsx merge in the live table state (visibility / order / widths) instead of spreading stale activeView.config — so saving a sort no longer clobbers a not-yet-persisted hide.

Tech Stack: React 18, TanStack Table v8, TanStack Query v5, Mantine, TypeScript.


Background — why this happens

useBaseTable keeps columnVisibility as local React state, seeded from the persisted view config via a useMemo-derived value (derivedColumnVisibility). Two effects currently do an unconditional re-sync:

// apps/client/src/features/base/hooks/use-base-table.ts:223-229
useEffect(() => {
  setColumnOrder(derivedColumnOrder);
}, [derivedColumnOrder]);

useEffect(() => {
  setColumnVisibility(derivedColumnVisibility);
}, [derivedColumnVisibility]);

Any mutation that calls queryClient.setQueryData(["bases", baseId], ...) — or queryClient.invalidateQueries at use-base-socket.ts:254 on ws base:property:* / base:view:* events — minted a new activeView.config reference. That recomputes derivedColumnVisibility, and the effect slams the local state back to whatever the server currently has.

Persistence is debounced 300 ms by persistViewConfig. During that window, any of these triggers a stomp:

  1. A concurrent sort / filter mutation from the same user. handleSortsChange / handleFiltersChange in base-toolbar.tsx:93-120 spread activeView.config (stale — still has old hiddenPropertyIds) and mutate immediately, without going through the debounced path. The onMutate optimistic write, the onSuccess server write, and the ws echo's invalidateQueries each re-trigger the effect.
  2. Another client sending any base:property:* / base:view:* ws event.
  3. The optimistic / success writes from our OWN persistViewConfig mutation when the server-side state hasn't persisted yet (e.g., response races).

Symptom: user toggles a column hidden → column disappears → column reappears within ~instant to a few hundred ms → reload confirms the toggle was never saved.

Deterministic repro (no second client needed):

  1. Open a base with ≥ 3 non-primary columns and no sorts.
  2. In the toolbar, open "Hide fields" and toggle column X off. Column vanishes.
  3. Immediately (well under 300 ms) open the Sort popover and add a sort.
  4. Column X reappears.
  5. Reload: X is visible; sort is saved; hide was lost.

hiddenPropertyIds wins semantics check: use-base-table.ts:117-143 — correct (hidden list is checked first, then legacy visiblePropertyIds, then default all-visible). That isn't the bug.


File Structure

Modified files:

  • apps/client/src/features/base/hooks/use-base-table.ts — gate the re-sync effect behind a view-id ref; export a new buildViewConfigFromTable helper used by both the debounced persist and the toolbar's direct mutations.
  • apps/client/src/features/base/components/base-toolbar.tsxhandleSortsChange / handleFiltersChange build the full config from live table state (not activeView.config) before mutate.

No new files. No server-side changes. No new deps.


Task 1: Verify the repro against the current build

  • Step 1: Build the client to make sure the branch is clean before changing anything.
pnpm nx run client:build

Expected: build succeeds.

  • Step 2: Ask the user to run the deterministic repro above.

(Per CLAUDE.md the agent must not run pnpm dev.) User should confirm:

  • Column X disappears, then reappears within ~300 ms of adding the sort.
  • Reload shows X visible, hide lost.

If the repro does NOT reproduce for the user, stop the plan here and ask for actual repro steps — the fixes below target this specific chain.


Task 2: Extract buildViewConfigFromTable helper

Files:

  • Modify: apps/client/src/features/base/hooks/use-base-table.ts

  • Step 1: Add the helper near the other build functions (above useBaseTable, after buildColumnPinning)

// Serializes the live react-table state into a persisted ViewConfig.
// Sort/filter toolbar mutations and the debounced `persistViewConfig`
// both go through this so a direct mutation (e.g. adding a sort) can't
// clobber a pending hide/reorder/resize by reading stale `activeView.config`.
export function buildViewConfigFromTable(
  table: Table<IBaseRow>,
  base: ViewConfig | undefined,
  overrides: Partial<ViewConfig> = {},
): ViewConfig {
  const state = table.getState();

  const sorts = state.sorting.map((s) => ({
    propertyId: s.id,
    direction: (s.desc ? "desc" : "asc") as "asc" | "desc",
  }));

  const propertyWidths: Record<string, number> = {};
  Object.entries(state.columnSizing).forEach(([id, width]) => {
    if (id !== "__row_number") propertyWidths[id] = width;
  });

  const propertyOrder = state.columnOrder.filter((id) => id !== "__row_number");

  const hiddenPropertyIds = Object.entries(state.columnVisibility)
    .filter(([id, visible]) => id !== "__row_number" && !visible)
    .map(([id]) => id);

  return {
    ...base,
    sorts,
    propertyWidths,
    propertyOrder,
    hiddenPropertyIds,
    visiblePropertyIds: undefined,
    ...overrides,
  };
}
  • Step 2: Replace the inline serialization inside persistViewConfig (lines roughly 267-297 of the current file) with a call to the helper.

Before:

persistTimerRef.current = setTimeout(() => {
  const state = table.getState();
  const sorts = state.sorting.map((s) => ({ ... }));
  // ...lots of inline serialization...
  const config: ViewConfig = {
    ...activeView.config,
    sorts,
    propertyWidths,
    propertyOrder,
    hiddenPropertyIds,
    visiblePropertyIds: undefined,
  };
  updateViewMutation.mutate({ viewId: activeView.id, baseId: base.id, config });
}, 300);

After:

persistTimerRef.current = setTimeout(() => {
  const config = buildViewConfigFromTable(table, activeView.config);
  updateViewMutation.mutate({ viewId: activeView.id, baseId: base.id, config });
}, 300);
  • Step 3: Build
pnpm nx run client:build

Expected: success.

  • Step 4: Commit
git add apps/client/src/features/base/hooks/use-base-table.ts
git commit -m "refactor(base): extract buildViewConfigFromTable helper"

Task 3: Route sort/filter mutations through the helper

Files:

  • Modify: apps/client/src/features/base/components/base-toolbar.tsx

handleSortsChange and handleFiltersChange currently read activeView.config directly, which reflects the server-persisted state, not the user's pending local changes. They must build the new config from table.getState() + the new sort or filter.

  • Step 1: Import the helper and accept the full view config for merging filters

Add at the top of the file:

import { buildViewConfigFromTable } from "@/features/base/hooks/use-base-table";
  • Step 2: Rewrite handleSortsChange

Before:

const handleSortsChange = useCallback(
  (newSorts: ViewSortConfig[]) => {
    if (!activeView) return;
    updateViewMutation.mutate({
      viewId: activeView.id,
      baseId: base.id,
      config: { ...activeView.config, sorts: newSorts },
    });
  },
  [activeView, base.id, updateViewMutation],
);

After:

const handleSortsChange = useCallback(
  (newSorts: ViewSortConfig[]) => {
    if (!activeView) return;
    const config = buildViewConfigFromTable(table, activeView.config, {
      sorts: newSorts,
    });
    updateViewMutation.mutate({
      viewId: activeView.id,
      baseId: base.id,
      config,
    });
  },
  [activeView, base.id, table, updateViewMutation],
);
  • Step 3: Rewrite handleFiltersChange the same way

Before:

const handleFiltersChange = useCallback(
  (newConditions: FilterCondition[]) => {
    if (!activeView) return;
    const filter: FilterGroup | undefined =
      newConditions.length > 0
        ? { op: "and", children: newConditions }
        : undefined;
    const { filter: _drop, ...rest } = activeView.config ?? {};
    updateViewMutation.mutate({
      viewId: activeView.id,
      baseId: base.id,
      config: filter ? { ...rest, filter } : rest,
    });
  },
  [activeView, base.id, updateViewMutation],
);

After:

const handleFiltersChange = useCallback(
  (newConditions: FilterCondition[]) => {
    if (!activeView) return;
    const filter: FilterGroup | undefined =
      newConditions.length > 0
        ? { op: "and", children: newConditions }
        : undefined;
    // `filter: undefined` in overrides removes the filter key; the helper's
    // spread-then-overrides order means `undefined` wins over any base filter.
    const config = buildViewConfigFromTable(table, activeView.config, {
      filter,
    });
    updateViewMutation.mutate({
      viewId: activeView.id,
      baseId: base.id,
      config,
    });
  },
  [activeView, base.id, table, updateViewMutation],
);
  • Step 4: Build
pnpm nx run client:build

Expected: success.

  • Step 5: Commit
git add apps/client/src/features/base/components/base-toolbar.tsx
git commit -m "fix(base): merge live table state into sort and filter mutations"

Task 4: Gate the sync effect behind view-id change

Files:

  • Modify: apps/client/src/features/base/hooks/use-base-table.ts

Within the same view, local state is the source of truth and the debounced persist flushes it. The sync effect must only run when the user switches to a different view (where the server's config is authoritative).

  • Step 1: Replace both sync effects

Before (around lines 220-229):

const [columnOrder, setColumnOrder] = useState<ColumnOrderState>(derivedColumnOrder);
const [columnVisibility, setColumnVisibility] = useState<VisibilityState>(derivedColumnVisibility);

useEffect(() => {
  setColumnOrder(derivedColumnOrder);
}, [derivedColumnOrder]);

useEffect(() => {
  setColumnVisibility(derivedColumnVisibility);
}, [derivedColumnVisibility]);

After:

const [columnOrder, setColumnOrder] = useState<ColumnOrderState>(derivedColumnOrder);
const [columnVisibility, setColumnVisibility] = useState<VisibilityState>(derivedColumnVisibility);

// Re-seed from server only when the user switches views. Within the same
// view, local state is the source of truth — the debounced persist flushes
// it. Without this guard, any ws-driven `invalidateQueries(["bases", baseId])`
// or concurrent view mutation lands a new `derivedColumnVisibility`
// reference and the effect would overwrite a pending hide/reorder toggle
// before `persistViewConfig` has a chance to flush it.
const lastSyncedViewIdRef = useRef<string | undefined>(activeView?.id);
useEffect(() => {
  const currentViewId = activeView?.id;
  if (currentViewId !== lastSyncedViewIdRef.current) {
    lastSyncedViewIdRef.current = currentViewId;
    setColumnOrder(derivedColumnOrder);
    setColumnVisibility(derivedColumnVisibility);
  }
}, [activeView?.id, derivedColumnOrder, derivedColumnVisibility]);

Note: derivedColumnOrder and derivedColumnVisibility remain in the dependency array so the effect reads the current derived values when a view switch happens. The ref-guarded branch only fires on id change.

  • Step 2: Build
pnpm nx run client:build

Expected: success.

  • Step 3: Commit
git add apps/client/src/features/base/hooks/use-base-table.ts
git commit -m "fix(base): only re-seed column state when view identity changes"

Task 5: USER manual verification — scripted steps

⚠️ Do not run pnpm dev as an agent. Hand off to the user with the cases below.

Ask the user to run through these. Each should now behave correctly:

  • Repro from Task 1 — hide then add sort

    1. Open a base with ≥ 3 non-primary columns.
    2. Hide column X via the popover.
    3. Within 300 ms, add a sort via the Sort popover.
    4. Expected: column X stays hidden. Sort indicator shows. Reload: both persisted.
  • Hide then change filter

    1. Hide column Y.
    2. Immediately add a filter condition.
    3. Expected: column Y stays hidden, filter applied. Reload: both persisted.
  • Hide all / show all

    1. Click "Hide all" in the popover. All non-primary columns disappear.
    2. Reload. Expected: same state.
    3. Click "Show all". Reload. Expected: all columns visible.
  • View switch

    1. Hide column Z on view A.
    2. Switch to view B (no hide). Expected: Z visible in view B.
    3. Switch back to view A. Expected: Z hidden.
  • WebSocket reconcile doesn't stomp

    1. Hide column W.
    2. From another browser / incognito session on the same base, add a new property.
    3. In the first window, the new property appears visible, W stays hidden.
  • Primary property can't be hidden (regression check)

    1. Open the popover — the primary column's switch is disabled (already enforced by enableHiding: !property.isPrimary at use-base-table.ts:87). Confirm.

Task 6: Final commit check + handoff

  • Step 1: Confirm branch state
git status
git log --oneline main..HEAD

Expected: clean tree, 3 new commits atop the CSV-export branch (refactor, fix sort/filter merge, fix sync effect).

  • Step 2: Trigger superpowers:finishing-a-development-branch to choose merge/PR.

Out of scope (explicitly not fixing here)

  • hiddenFieldCount badge dep in base-toolbar.tsx:88-91 — works by accident today (re-renders produce fresh getState() references). Low-impact cosmetic risk; leave alone unless it manifests.
  • Legacy visiblePropertyIds migration. Views created before hiddenPropertyIds existed may show new properties as hidden by default in buildColumnVisibility. No reports of this; migration would be a separate plan.
  • Batching handleHideAll/handleShowAll into a single table.setColumnVisibility(map) call instead of iterating toggleVisibility. React 18 batches these anyway; not a bug.
  • Server-side zod schema. viewConfigSchema already accepts hiddenPropertyIds: [] correctly; no change needed.