Dashboard flat table: server-supplied totals row (tfoot) via the cross-tab's marginal-totals machinery #5846

Activity

1. os-zhuang

   self-assigned this

   on Aug 23, 2026
2. os-zhuang commented on Aug 23, 2026

   

   os-zhuang

   on Aug 23, 2026

   ContributorAuthor

   More actions

   Claim: direct-dispatch (hotcrm seat acting cross-repo under maintainer instruction)
   Session: session_61f28fb9-8e31-4a6f-b4d8-10f749a092d2
   Branch: claude/issue-5846-table-totals-row
   Worktree: objectui-issue-5846
   File surface: packages/plugin-dashboard/src/DatasetWidget.tsx + its tests; the dataset query path it issues (read the executor contract, edit only what the widget owns); stop on breach; explain in the report
   Container & model: M, mode:subagent, model: opus — Clause-②: no (widget query + rendering; the analytics executor's own contract is not edited)
   Serial constraints cleared: the DatasetWidget.tsx serial queue is drained — #5834 (62c0fa1) and #5848 (1d9100e) both merged; base must include BOTH. No other open PR touches the file (REST pulls listing checked at claim time).

   Authorization — maintainer ruling, verbatim (未译): 「我觉得合理的需求都应该开发」
3. os-zhuang commented on Aug 23, 2026

   

   os-zhuang

   on Aug 23, 2026

   ContributorAuthor

   More actions

   {
     "issue": 5846,
     "status": "done",
     "branch": "claude/issue-5846-table-totals-row",
     "pr": "https://github.com/objectstack-ai/objectui/pull/5864",
     "premise_still_valid": true,
     "summary": "The flat dataset `table` branch of DatasetWidget.tsx now requests the server's `[]` marginal grouping and renders it as a `tfoot` under the console's existing `dashboard.total` label (en `Total`, zh `总计`) — no new string minted. Every figure is the server's own aggregate over the whole selection (a sum sums; avg/min/max report the whole-set aggregate); nothing is recombined client-side, and no footer is rendered when the executor answers no `[]` grouping or answers it with no row. The row lives in `tfoot`, OUTSIDE `entries`/`orderedEntries` entirely rather than as a special case in the comparator, so exemption from sorting, position below the `—` null bucket, and non-participation in the (row,index) drill pairing all fall out of the placement. PREMISE VERIFIED at the source rather than taken from #5827's report: `DatasetExecutor.execute` validates each grouping as a subset of the SELECTED dimensions, so `[]` is legal for a flat selection with no server change — objectstack's own dataset-executor.test.ts already pins `dimensions:['region'] + totals:{groupings:[[]]}` returning `[{dimensions:[],rows:[…]}]`, and ObjectStackAdapter.queryDataset already declares and returns `totals`. MEASURED COST on the real pipeline (compileDataset → AnalyticsService NativeSQL → SQL → sql.js SQLite): statements 1→2, the added one being `SELECT SUM(…),AVG(…) FROM t` with the same WHERE and no GROUP BY — 9.57ms beside the primary's 48.79ms at 100k rows, median wall 45.97→55.17ms (+20%); +28% at 10k, +19% at 1M. Shipped UNGATED and with no widget-schema change: the cross-tab has requested THREE such groupings unconditionally since #1753, so one is strictly less than what already ships on the sibling branch of the same file — and gating would have meant declaring a query-affecting `showTotals` key in @objectstack/spec (DashboardWidgetOptionsSchema is `.passthrough()` for presentation extras but declares every key that reaches the query), a cross-repo widening for a 20% saving. ONE JUDGEMENT CALL beyond the ruling, flagged in open_questions: no grouping is requested (and no footer rendered) for a table truncated by `options.limit`, because the executor drops `limit` for a totals query by design and a top-N list under a whole-set total is exactly the reconciliation the card exists to give the reader.",
     "tests": "Union re-run AFTER the final commit, at a2c9d307f; every exit code captured by redirecting to a file first, never read after a pipe. `pnpm --filter '@object-ui/plugin-dashboard^...' build` → dependency closure built (last package packages/fields, `built in 4.62s`), exit 0. `pnpm exec vitest run packages/plugin-dashboard/` → `Test Files 79 passed (79)` / `Tests 741 passed (741)`, exit 0 (new file DatasetWidget.tableTotalsRow.test.tsx = 11 cases; DatasetWidget.test.tsx's `does NOT request totals for a flat table` was UPDATED, not deleted — it pinned the absence this card removes — and split into the grand-total-only request plus the options.limit gate). `pnpm --filter @object-ui/plugin-dashboard type-check` → `tsc --noEmit && tsc -p tsconfig.test.json`, no diagnostics, exit 0. `pnpm --filter @object-ui/plugin-dashboard lint` → `381 problems (0 errors, 381 warnings)`, exit 0; of those, 5 are this diff's and all are in TEST files (4 × the `(URL as any)` export stub copied verbatim from the sibling tablePolish file, 1 × a mock.calls cast) — measured, not asserted: DatasetWidget.tsx itself lints at 12 warnings / 0 errors both at this HEAD and with the file reverted to origin/main. `node scripts/check-changeset-presence.mjs` → `3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)`, exit 0. `node scripts/check-changeset-no-major.mjs` → `No changeset declares a 'major' bump.`, exit 0. `node scripts/check-control-bytes.mjs` → `OK (scanned 4876 tracked text file(s); skipped 85 binary)`, exit 0. `pnpm run check:i18n-keys` → `Every in-scope call-site key resolves against the en pack (2928 keys)`, exit 0. `pnpm run check:i18n-drift` → `No en value changed in this range.`, exit 0. REVERSE VERIFICATION, direction stated before the run: reverting ONLY DatasetWidget.tsx to origin/main should redden the 7 new cases asserting the footer or the request plus the one updated case in DatasetWidget.test.tsx, and leave green the 4 pinning what did not move (the cross-tab's three groupings, the options.limit gate, and the two 'executor answered nothing' cases, which pass trivially when no footer exists at all) plus its sibling limit case. OBSERVED `Tests 8 failed | 59 passed (67)` and the eight failures are the eight predicted; the ablation script's own one-line summary said '6 + 1', an arithmetic slip in the summary — the enumerated per-case prediction, which is what the run was scored against, matched 8 for 8. Mutation confirmed ON DISK by anchored greps, never by an editor exit code: injected `wantsFlatTotals` 4→0 and `dataset-table-total-row` 1→0, deleted origin/main spelling `const totalsGroupings = isMatrix ? [rowDims, [colDim], []] : undefined;` 0→1. Restore leg confirmed the same way (4 / 1 / 0) plus an empty `git status --porcelain` for the file, re-ran green at 67/67. No rebuild stands between edit and run — both suites import the component by relative source path (`../DatasetWidget`), not through a package exports→dist hop — and the script carried `trap … EXIT INT TERM`, which did fire. The COST measurement is a separate read-only script (scratchpad, nothing written into the shared objectstack checkout) importing objectstack's built dist by absolute path at objectstack HEAD a2ec37707. DECLARED NARROWINGS: (1) the shared verification lock is inoperable on this host (`os-verify-lock.sh --status` → `CANNOT OPERATE THIS LOCK … a stock macOS does not ship flock`), so commands ran directly rather than through the entry point; (2) repo-wide `check:*` scanning left to CI; (3) `check:eager-closure` is a BROKEN GAUGE in a fresh worktree — it reads apps/console/dist/eager-closure.json, written only by a vite build of the console, and prints `This is a broken gauge, not a passing budget` when absent (exit 2); this diff adds no imports, so the closure is unchanged and CI owns the reading. NOT verified in a running browser — the same honest limit #5834 and #5848 declared. CI at report time: in_progress (report delivered at draft-PR time per the dispatch contract).",
     "open_questions": [
       {
         "question": "Should a flat table truncated by `options.limit` (a top-N) show the server's WHOLE-SET total, or none at all? The ruling settles the aggregate semantics but not this case, and the executor drops `limit` for a totals query by design, so no 'total of the rows shown' exists without the client re-aggregation the card forbids.",
         "options": [
           "A — omit the footer and do not request the grouping (shipped): the reader can always reconcile the row against the rows above it, and a top-N table pays no extra query at all.",
           "B — render the whole-set total under the truncated list, matching what the cross-tab already does with its own `limit`: a 'Total' that is the total of the DATA, not of the view.",
           "C — render it with a distinguishing label (e.g. 'Total (all rows)'), which needs a NEW string and so runs into objectstack#10292's territory the card told this PR not to enter."
         ],
         "recommendation": "A, as shipped, and it is a one-line flip to B if the maintainer reads 'Total' as a property of the dataset rather than of the table. A is recommended on the card's own justification — the row exists so the reader does not have to add the column up — which a figure they cannot reconcile actively defeats; it is also the only option that does not spend the measured query cost on a widget whose total would need a caveat. C is declined for the reason the card names: a new hardcoded string is precisely what it asked not to mint."
       }
     ],
     "out_of_scope_findings": []
   }
4. os-zhuang commented on Aug 23, 2026

   

   os-zhuang

   on Aug 23, 2026

   ContributorAuthor

   More actions

   PM review — ACCEPT, including both judgement calls. PR #5864. Merge on CI green.

   The premise was verified at the source, not inherited

   My dispatch flagged "totalsGroupings works for flat tables" as second-hand. The dev went to the executor: DatasetExecutor.execute validates groupings as subsets of the selected dimensions, so [] is legal with no server change, and objectstack's own dataset-executor.test.ts already pins the flat-selection [[]] case. The premise is now a cited fact.

   Call 1 accepted: shipped ungated, with a measurement instead of a shrug

   The card said "if the cost is non-trivial, gate it". The dev measured on the real pipeline (compileDataset → NativeSQL → sql.js): the added statement is a single no-GROUP-BY aggregate — 9.57ms beside the primary's 48.79ms at 100k rows (+20% median wall; +28% at 10k, +19% at 1M). The argument for ungated is structural, not dismissive: the sibling cross-tab branch of the same file has requested three such groupings unconditionally since #1753, so one is strictly less than what already ships; and gating would mean declaring a query-affecting key in @objectstack/spec (DashboardWidgetOptionsSchema declares every key that reaches the query) — a cross-repo schema widening to save ~10ms. Accepted: the absolute cost is trivial, the relative comparison is apt, and the alternative is more contract for less value.

   Call 2 accepted: no totals under a limit-truncated table (option A)

   The invariant chain is airtight: the executor drops limit for totals by design, so "total of the rows shown" cannot exist without the client re-aggregation the card forbids — the only honest figures are whole-set-total (unreconcilable against a top-N view) or nothing. A footer the reader cannot reconcile against the rows above it defeats the row's whole purpose. wantsFlatTotals gates request AND render in one predicate, so the two cannot disagree. Option B is a one-line flip if the maintainer reads "Total" as a property of the dataset rather than the view; option C was correctly declined (a new hardcoded string is objectstack#10292's territory, which the card fenced off).

   Structure verified in the diff

   • tfoot lives outside entries/orderedEntries — sorting exemption, below-null-bucket position, and drill non-participation all fall out of placement rather than being special-cased in the comparator.
   • Label is the console's existing dashboard.total key (en Total, zh 总计) — no string minted.
   • The prior test pinning "does NOT request totals for a flat table" was updated, not deleted — it pinned the exact absence this card removes, and it split into the new request-shape pin plus the limit gate.

   Verification integrity

   Predicted 8 red / observed 8 red under ablation, with per-case enumeration as the scoring basis; the ablation script's own summary line said "6+1" — an arithmetic slip declared rather than smoothed over, which is worth more than a tidy summary. Lint delta measured (5 new warnings, all in test files, component file identical at 12 before/after). On-disk mutation confirmation both legs; 67/67 restored.

   Disposition

   Ready + merge queue on CI green. The PM runs the visual half in the overlay after merge, closing the browser-verification gap all three table cards declared.
5. os-zhuang

   closed this as completedin #5864on Aug 23, 2026
6. os-zhuang

   removed their assignment

   on Aug 23, 2026