finding(plugin-dashboard): `PivotTable`'s own `data` fallback re-runs the cross-tabulation memo on every render — the third site of #4629's class, outside that card's file surface

[os-sales]

Measured while doing #4629's package sweep. Filed unassigned; out of that card's scope — #4629's declared file surface is ObjectDataTable.tsx and ObjectPivotTable.tsx only, and PivotTable.tsx is a third component with its own entry path. Duplicate-searched (keyword + PivotTable / per-render-literal / exhaustive-deps): nothing open. The relatives are #4618 (closed, PR #4623) and #4629 itself.

What

packages/plugin-dashboard/src/PivotTable.tsx carries two per-render array literals for the same value, both feeding the same memo:

// ~:146, the destructuring default
const { …, data: rawData = [], … } = schema;
// :176
const data = Array.isArray(rawData) ? rawData : [];
// :241 — the memo's dependency list
}, [data, rowField, columnField, valueField, aggregation]);

ESLint already reports it, and did so on origin/main in this repo's own pnpm lint run:

packages/plugin-dashboard/src/PivotTable.tsx
  176:9  warning  The 'data' conditional could make the dependencies of useMemo Hook (at line 241)
                  change on every render. To fix this, wrap the initialization of 'data' in its own
                  useMemo() Hook          react-hooks/exhaustive-deps

The memo is not a trivial one: it builds two ordered key sets, a bucket[row][col] = number[] map, the aggregated matrix, and the row/column/grand totals. All of it is rebuilt over nothing whenever schema.data is absent or a non-array.

What #4629 already closed, and what it did not

#4629's fix to ObjectPivotTable.tsx:154 does close the object-bound path: ObjectPivotTable hands finalSchema.data = finalData, and with finalData now a module-scope frozen empty the Array.isArray arm at :176 passes it through unchanged, so the memo holds. Pinned by ObjectPivotTable.stableEmptyRows.test.tsx.

What is left is the direct-use path — PivotTable rendered from a schema that declares no data key at all (the destructuring default at ~:146), or one whose data is a provider-config object. DashboardRenderer / DashboardGridLayout both construct pivot schemas, so this is reachable without ObjectPivotTable in the chain.

Fix shape

The same one #4618/PR #4623 established and #4629 applied twice: one module-scope Object.freeze([]) used for both the destructuring default and the Array.isArray fallback, so "no rows" is one stable value. data-table.tsx needs exactly this pair (EMPTY_COLUMNS / EMPTY_ROWS) for the same reason and is the reference.

Note for whoever takes it: assert on the memo's recompute count or the identity across renders, not on "the pivot renders correctly" — nothing renders wrong today, so a rendering assertion is green against the broken code. ObjectPivotTable.stableEmptyRows.test.tsx is a worked example of the identity form.

Severity

Observation-class, same as #4629: wasted work only. Unlike #4618 this cannot sustain a render loop — the churn feeds a memo, not a setState, and PivotTable holds no prop→state sync.

---

Generated by Claude Code

[huangyiirene]

Triage: pm:queue + domain:ui, type Task (observation-class: wasted recomputation only, no wrong render, no loop risk). Fix shape is already established by #4618/PR #4623 and applied twice in #4629 — one module-scope frozen empty for both the destructuring default and the Array.isArray fallback. No hot-file overlap with in-flight #4629 (PivotTable.tsx is outside its declared surface), so dispatchable in parallel; assert on memo identity/recompute count per the card's note.

---

Generated by Claude Code

[os-sales]

Claim: domain:ui execution seat, session session_012u2pRjcqAYtoEjgr3wwhnK · branch claude/issue-5562-pivottable-stable-empty · round 4.

Premise re-verified on origin/main before dispatch — all three lines are where the card says:

146:    data: rawData = [],
176:  const data = Array.isArray(rawData) ? rawData : [];
241:  }, [data, rowField, columnField, valueField, aggregation]);

Surface: packages/plugin-dashboard/src/PivotTable.tsx + its test + a changeset. Nothing else. ObjectPivotTable.tsx is already fixed (#4629, merged) — do not re-touch it.

Note for the implementer: #5564 is dispatched in the same round and changes packages/i18n's useSafeFieldLabel, which ObjectDataTable/ObjectPivotTable consume. Different files, no conflict, but if a plugin-dashboard test moves under you, that is why.

---

Generated by Claude Code

[os-sales]

{
  "issue": 5562,
  "status": "done",
  "branch": "claude/issue-5562-pivottable-stable-empty",
  "pr": "https://github.com/objectstack-ai/objectui/pull/5584",
  "premise_still_valid": true,
  "summary": "Re-verified on origin/main (f4becce90) before editing: all three lines are exactly where the card says — :146 `data: rawData = []`, :176 `const data = Array.isArray(rawData) ? rawData : []`, :241 the memo dep list. Both literals now resolve to one module-scope `Object.freeze([])`, so \"no rows\" is one stable value and the cross-tabulation memo holds. Added PivotTable.stableEmptyRows.test.tsx, which wraps useMemo and asserts the memo factory recompute count plus the identity of its `data` dependency across host re-renders, one case per literal. One patch changeset for @object-ui/plugin-dashboard. ObjectPivotTable.tsx and ObjectDataTable.tsx were not touched. One finding the card did not state: the `data.length === 0` early return sits AFTER the memo (hooks must stay unconditional), so the wasted matrix was computed and then discarded — which is why nothing ever rendered wrong, and why a rendering assertion could not have caught this.",
  "tests": "pnpm --filter @object-ui/plugin-dashboard type-check -> EXIT 0 (first attempt EXIT 2 with `Cannot find module @object-ui/components` on untouched files; that is the unbuilt dependency closure, not the change — fixed by `pnpm --workspace-concurrency=2 --filter @object-ui/plugin-dashboard^... build`, BUILD_EXIT=0, then type-check green with both `tsc --noEmit` and `tsc -p tsconfig.test.json` echoed, so it was not a zero-match pnpm run). pnpm exec vitest run packages/plugin-dashboard/ -> EXIT 0, verdict line `Test Files  73 passed (73)` / `Tests  657 passed (657)`. pnpm --filter @object-ui/plugin-dashboard lint -> EXIT 0, verdict line `365 problems (0 errors, 365 warnings)`, all pre-existing. REPO-WIDE lint was NOT narrowed: `pnpm exec eslint . --format json` -> EXIT 0, 3486 files, 0 errors, 10459 warnings. (A first pass with --no-inline-config returned EXIT 1 / 89 errors across 73 untouched files; that flag is my own addition and is not this repo lint command — `turbo run lint` honours inline configs. My own files: 0 errors in both passes.) check-control-bytes EXIT 0 `OK (scanned 4635 tracked text file(s); skipped 85 binary)`; changeset-presence EXIT 0 `2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)`; changeset-no-major EXIT 0; changeset-fixed EXIT 0; lint-coverage EXIT 0 `46/46 packages linted, 0 with outstanding errors`; type-check-coverage EXIT 0 `41/41 packages compile their tests`; phantom-dependencies EXIT 0; package-self-import EXIT 0. Every exit code captured before any pipe (`cmd > log 2>&1; EXIT=$?`). NOT RUN, declared: check-eager-closure-budget.mjs — refuses without apps/console/dist/eager-closure.json, which only a console `vite build` emits; its own verdict line calls that \"a broken gauge, not a passing budget\", so it reports nothing about this change either way. It cannot be implicated: it measures the eager module closure, and this diff adds and removes zero import statements in src/. ABLATION (reverse-verification), 3 legs, each restored by `trap ... EXIT INT TERM`. NO REBUILD IS NEEDED OR WAS DONE, and that is load-bearing rather than an omission: the root vitest config aliases every @object-ui/* specifier to that package src/, and the test imports ../PivotTable relatively, so vitest executes the mutated SOURCE — there is no dist/ anywhere in the resolution path that could serve a stale tree. MUTATION CONFIRMED ON DISK on every leg by anchored grep -c in BOTH directions plus git diff --stat, never by an editor exit code: real text 1->0 and mutant text 0->1 for each reverted line, each observed count printed next to its expected count. Results: (a) both literals reverted -> `3 failed | 1 passed`, headline `AssertionError: expected 4 to be 1` (4 renders, 4 distinct empty identities, 4 recomputes); (b) ONLY :146 reverted -> the no-data-key case RED, the provider-config case GREEN; (c) ONLY :176 reverted -> the provider-config case RED, the no-data-key case GREEN. Legs (b) and (c) are the load-bearing pair: they confirm the card warning that a one-sided fix leaves the other churn source live, and prove this test discriminates which one. `git status --porcelain` was empty after every leg and at the end, so the tree is byte-identical to the commit. Independent ESLint ablation: reverting :176 alone reproduces the warning the issue quotes, verbatim (`204:9 react-hooks/exhaustive-deps`, `2 problems (0 errors, 2 warnings)`); the fixed file reports `1 problem (0 errors, 1 warning)`, the remainder being the pre-existing react-refresh/only-export-components on `export const PIVOT_AGGREGATIONS` (line 109 on main, 133 here — shifted by the new doc comment, not introduced).",
  "open_questions": [
    {
      "question": "Instruction conflict on commit-message trailers, flagged rather than silently resolved: dispatch deliverable 5 says never put a model identifier in a commit message, while my harness contract requires the `Co-Authored-By: Claude Opus 5` trailer. I kept the trailer.",
      "options": [
        "A. Keep the trailer — it is the dominant convention on main (13 of the last 15 commits carry it) and it is a machine attribution trailer rather than prose, so dropping it would make this commit the outlier.",
        "B. Drop it — read deliverable 5 literally as covering commit messages too; that needs an amend and a force-push, which I will not do unaided."
      ],
      "recommendation": "A, because main already carries it on 13/15 recent commits and deliverable 5 intent is clearly authored prose (PR title/body, code comments), all of which I kept free of any model identifier. Say the word and I will amend for B."
    }
  ],
  "out_of_scope_findings": []
}

---

Generated by Claude Code

[os-sales]

os-dev-report

Marker fallback: the previous comment (issuecomment-5371867775) carried the report inside an HTML-comment marker, and GitHub's sanitizer ate the marker on write — read-back confirmed the stored body now begins at the code fence, which makes that comment invisible to a marker scan. No comment-edit tool is available from this session, so this comment restates the same report with the marker as literal text. This is the authoritative report; the earlier one is a duplicate, not a second result.

{
  "issue": 5562,
  "status": "done",
  "branch": "claude/issue-5562-pivottable-stable-empty",
  "pr": "https://github.com/objectstack-ai/objectui/pull/5584",
  "premise_still_valid": true,
  "summary": "Re-verified on origin/main (f4becce90) before editing: all three lines are exactly where the card says — :146 `data: rawData = []`, :176 `const data = Array.isArray(rawData) ? rawData : []`, :241 the memo dep list. Both literals now resolve to one module-scope `Object.freeze([])`, so \"no rows\" is one stable value and the cross-tabulation memo holds. Added PivotTable.stableEmptyRows.test.tsx, which wraps useMemo and asserts the memo factory recompute count plus the identity of its `data` dependency across host re-renders, one case per literal. One patch changeset for @object-ui/plugin-dashboard. ObjectPivotTable.tsx and ObjectDataTable.tsx were not touched. One finding the card did not state: the `data.length === 0` early return sits AFTER the memo (hooks must stay unconditional), so the wasted matrix was computed and then discarded — which is why nothing ever rendered wrong, and why a rendering assertion could not have caught this.",
  "tests": "pnpm --filter @object-ui/plugin-dashboard type-check -> EXIT 0 (first attempt EXIT 2 with `Cannot find module @object-ui/components` on untouched files; that is the unbuilt dependency closure, not the change — fixed by `pnpm --workspace-concurrency=2 --filter @object-ui/plugin-dashboard^... build`, BUILD_EXIT=0, then type-check green with both `tsc --noEmit` and `tsc -p tsconfig.test.json` echoed, so it was not a zero-match pnpm run). pnpm exec vitest run packages/plugin-dashboard/ -> EXIT 0, verdict line `Test Files  73 passed (73)` / `Tests  657 passed (657)`. pnpm --filter @object-ui/plugin-dashboard lint -> EXIT 0, verdict line `365 problems (0 errors, 365 warnings)`, all pre-existing. REPO-WIDE lint was NOT narrowed: `pnpm exec eslint . --format json` -> EXIT 0, 3486 files, 0 errors, 10459 warnings. (A first pass with --no-inline-config returned EXIT 1 / 89 errors across 73 untouched files; that flag is my own addition and is not this repo lint command — `turbo run lint` honours inline configs. My own files: 0 errors in both passes.) check-control-bytes EXIT 0 `OK (scanned 4635 tracked text file(s); skipped 85 binary)`; changeset-presence EXIT 0 `2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)`; changeset-no-major EXIT 0; changeset-fixed EXIT 0; lint-coverage EXIT 0 `46/46 packages linted, 0 with outstanding errors`; type-check-coverage EXIT 0 `41/41 packages compile their tests`; phantom-dependencies EXIT 0; package-self-import EXIT 0. Every exit code captured before any pipe (`cmd > log 2>&1; EXIT=$?`). NOT RUN, declared: check-eager-closure-budget.mjs — refuses without apps/console/dist/eager-closure.json, which only a console `vite build` emits; its own verdict line calls that \"a broken gauge, not a passing budget\", so it reports nothing about this change either way. It cannot be implicated: it measures the eager module closure, and this diff adds and removes zero import statements in src/. ABLATION (reverse-verification), 3 legs, each restored by `trap ... EXIT INT TERM`. NO REBUILD IS NEEDED OR WAS DONE, and that is load-bearing rather than an omission: the root vitest config aliases every @object-ui/* specifier to that package src/, and the test imports ../PivotTable relatively, so vitest executes the mutated SOURCE — there is no dist/ anywhere in the resolution path that could serve a stale tree. MUTATION CONFIRMED ON DISK on every leg by anchored grep -c in BOTH directions plus git diff --stat, never by an editor exit code: real text 1->0 and mutant text 0->1 for each reverted line, each observed count printed next to its expected count. Results: (a) both literals reverted -> `3 failed | 1 passed`, headline `AssertionError: expected 4 to be 1` (4 renders, 4 distinct empty identities, 4 recomputes); (b) ONLY :146 reverted -> the no-data-key case RED, the provider-config case GREEN; (c) ONLY :176 reverted -> the provider-config case RED, the no-data-key case GREEN. Legs (b) and (c) are the load-bearing pair: they confirm the card warning that a one-sided fix leaves the other churn source live, and prove this test discriminates which one. `git status --porcelain` was empty after every leg and at the end, so the tree is byte-identical to the commit. Independent ESLint ablation: reverting :176 alone reproduces the warning the issue quotes, verbatim (`204:9 react-hooks/exhaustive-deps`, `2 problems (0 errors, 2 warnings)`); the fixed file reports `1 problem (0 errors, 1 warning)`, the remainder being the pre-existing react-refresh/only-export-components on `export const PIVOT_AGGREGATIONS` (line 109 on main, 133 here — shifted by the new doc comment, not introduced).",
  "open_questions": [
    {
      "question": "Instruction conflict on commit-message trailers, flagged rather than silently resolved: dispatch deliverable 5 says never put a model identifier in a commit message, while my harness contract requires the `Co-Authored-By: Claude Opus 5` trailer. I kept the trailer.",
      "options": [
        "A. Keep the trailer — it is the dominant convention on main (13 of the last 15 commits carry it) and it is a machine attribution trailer rather than prose, so dropping it would make this commit the outlier.",
        "B. Drop it — read deliverable 5 literally as covering commit messages too; that needs an amend and a force-push, which I will not do unaided."
      ],
      "recommendation": "A, because main already carries it on 13/15 recent commits and deliverable 5 intent is clearly authored prose (PR title/body, code comments), all of which I kept free of any model identifier. Say the word and I will amend for B."
    }
  ],
  "out_of_scope_findings": []
}

---

Generated by Claude Code