fix(plugin-reports): infer export columns from every row, not the first 50

[claude]

Fixes#11774

The defect

pickFields() in packages/plugins/plugin-reports/src/report-service.ts inferred an export's columns from rows.slice(0, 50), while the projection those columns drive was applied to all rows. With no explicit query.fields, any key whose first occurrence fell at row 51 or later was absent from the header and dropped from every row that carried it.

The export gave no signal: well-formed CSV, uniform arity across rows, nothing marking a column as inferred rather than declared. A recipient of a scheduled report attachment could not tell. And the sampled prefix is not a random sample — it is the query's first page in its own orderBy, so for a report sorted by status or created date it correlated with exactly the sparse column it dropped. Sparse columns are the normal shape of report output: an optional field, a formula only some records satisfy, a lookup that resolves for a subset.

Both affected renderers are fixed — renderCsv (which is also renderReport's default: branch) and renderHtmlTable. renderJson emits rows as-is and was never affected.

Route taken, and why

Two routes restore the same stated contract ("the export carries every field present in the result set"). I took scan every row, not project from the saved report's declared column set:

• ReportQuery.fields is optional in packages/spec/src/contracts/report-service.ts. For a saved report that declares none there is no declared column set to project from, so that route would have to either make fields required or derive columns from the object's schema through a new ReportEngine capability — a new declared surface either way, which this card does not carry (triage's premise-first stop). Route rejected on that ground rather than on cost.
• renderReport is exported from the package and takes (rows, format, fields?) with no access to a saved report at all, so the declared-set route cannot reach that surface even in principle.

Cost, measured

The card names the real concern: scanning every row is O(rows × keys) on the export path. Measured (node, same shapes, 100 iters, the sparse key on the last row):

rows × keys	sample-50	full scan	delta	renderCsv body pass	scan as % of the render it feeds
1 000 × 20	0.111 ms	0.841 ms	+0.73 ms	2.515 ms	29.0 %
5 000 × 20	0.052 ms	4.228 ms	+4.18 ms	12.870 ms	32.5 %
5 000 × 50	0.120 ms	12.763 ms	+12.64 ms	36.675 ms	34.5 %
50 000 × 50	0.110 ms	129.808 ms	+129.70 ms	432.651 ms	30.0 %

The cost is bounded twice over. The row array reaching the renderer is already capped by Math.min(query.limit ?? 1000, maxRows) (maxRows defaults to 5000), and both renderers already make an O(rows × cols) pass over that same array. Inference is a stable ~30 % of the render it feeds across every shape measured — a fraction of work the export path already does unconditionally, not a new order of magnitude. At the service's own default cap that is +4.2 ms.

First-seen column order is preserved and late-appearing columns append, so an export that was already correct is byte-identical.

Pin

Written test-first, with the expected failure signature predicted in writing before the first run — and the run matched it exactly (3 failed | 76 passed, AssertionError: expected [ 'id', 'status' ] to include 'escalation_note' on all three).

Fixture: 60 rows sharing id + status, with only row 55 carrying escalation_note — a key first appearing past the old sample boundary. Three tests, covering both renderers and the end-to-end service path:

• renderReport(..., 'csv') — the named late column reaches the header, and 'breached SLA' arrives in row 55 under that column's index, and nowhere else.
• renderReport(..., 'html_table') — the same two facts against parsed <th> / <td> cells.
• ReportService.run() over a saved report declaring noquery.fields — the scheduled-attachment path end to end.

Identities are pinned, not counts: each asserts the named column and its value, plus toEqual(['id', 'status', 'escalation_note']) for order. That mattered in practice — the order assertion caught a bug in the test's own <th> extraction (<th[^>]*> also matches <thead>), which a toContain-only pin would have hidden. toHaveLength(3) would have missed it too.

The pre-existing 'auto-detects fields from first 50 rows when none specified' asserted toMatch(/a|b/) — an alternation that passes either way. It is renamed and strengthened to toEqual(['a', 'b']); it was never part of the red.

Verification

Test Files 5 passed (5) · Tests 79 passed (79) — pnpm --filter @objectstack/plugin-reports test, at 0a163296c7.
tsc --noEmit clean, exit 0 — pnpm --filter @objectstack/plugin-reports typecheck.

Gates derived at 0a163296c7 on a clean tree via node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no path arguments). 17 of 18 green with exit codes captured before any pipe. The 18th, check:type-check-debt, refused to measure on this worktree (its documented unbuilt-closure refusal — "41 workspace dependencies ... have no built type entry point on disk"); it is recorded as NOT MEASURED, not as a pass. See the report comment on #11774 for the full per-gate verdict lines and the narrowing evidence.

Contract review

Clause ② assessed against the diff, not inherited: NO. The whole source diff is one hunk inside the module-private pickFields (referenced nowhere outside the file); the file's exported symbol list is byte-identical base vs head (7 exports, same signatures); src/index.ts and packages/spec/ are untouched. Nothing here accepts or rejects — pickFields has no refusal path, no error code, no schema — so no accept/reject surface moves and no public surface widens. What changes is that the export now carries columns it was already contractually required to carry.

Generated by Claude Code

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see

• the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 2 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json f7eff23ed4b19933e0543b2997185212bc0a7761 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from a759d90db7dd8741cf015b53de9a8cb49deddedb — the merge of head 0a163296c76b0a5f486222c2ecc0f55c3020419c into base f7eff23ed4b19933e0543b2997185212bc0a7761, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a759d90db7dd8741cf015b53de9a8cb49deddedb && git checkout a759d90db7dd8741cf015b53de9a8cb49deddedb
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f7eff23ed4b19933e0543b2997185212bc0a7761 0a163296c76b0a5f486222c2ecc0f55c3020419c && git checkout -B drift-repro f7eff23ed4b19933e0543b2997185212bc0a7761 && git merge --no-ff 0a163296c76b0a5f486222c2ecc0f55c3020419c
node scripts/docs-audit/affected-docs.mjs --json f7eff23ed4b19933e0543b2997185212bc0a7761

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

[os-sam]

PM review — PASS. Every load-bearing claim re-measured on the merge-base, not inherited

domain:services seat (session session_01APWX2AwT3a4xDcjPCe8bk4). Merge-base ce2b9d2e59, head 0a163296c7. Three-dot throughout (git diff BASE...HEAD) — two-dot pulls in main-only commits and has misled a file list on this board before.

Diff shape. 3 files: the changeset, the test, and one 20-line hunk in report-service.ts. Fenced surfaces (packages/spec/, content/docs/releases/, docs/adr/, .claude/, skills/, AGENTS.md, CLAUDE.md) — zero hits, and the zero is real: a positive control through the same grep on the same file list hits the two plugin-reports paths.

⭐ Clause-② — NO, judged from the live gate text, and not on the reason it would be tempting to give

Re-read live on origin/main this round rather than recalled:

clause ②, NOT encoded and deliberately not: a card that changes contract accept/reject behaviour or widens the public surface is also claude-fable-5. That is judged from the card's CONTENT … An ordinary-looking surface (one package's source file) is the NORMAL shape of a clause-② card.

⚠️ So "it's one plugin file" is not a reason — the gate names that shape as the normal disguise. The reasons that do hold, each measured:

claim	measurement
pickFields is module-private	git grep pickFields on head → 3 hits, all in its own file: declaration :139, renderCsv:149, renderHtmlTable:166. Positive control on the same channel (renderReport) hits 4 files, so the narrow result is not a dead pattern.
no public surface moves	exported symbol list base vs head — 7 exports both sides, diff exit 0, signatures identical. pickFields is not among them.
nothing accepts or rejects	pickFields has no refusal path, no error code, no schema.
the contract text is unchanged	ReportQuery.fields stays optional in packages/spec. The code moves toward the stated contract, not the contract toward the code.

⚠️ The one gate that refused — narrowing verified rather than accepted

check:type-check-debt exited 1 with its documented unbuilt-closure refusal and is reported as NOT MEASURED, not as a pass. That is the correct record, and the narrowing behind it checks out:

• @objectstack/plugin-reports appears zero times in scripts/check-type-check-coverage.mjs at the merge-base — so --re-measure re-measures a population this package is not in. ⚠️ Positive control on the same channel and same ref lists five packages that are ledgered (cloud-connection, core, hono, knowledge-ragflow, metadata), so the zero is a real zero.
• The export-list invariance measured above closes the second half: no other package's ledgered count can shift either.
• The half a new test file actually moves — the structural check:type-check-coverage — ran green, and CI runs the farm regardless.

os-verify-lock returned VERDICT queue-timeout (exit 99) on the dist/index.d.ts comparison. Recorded as not measured rather than as a result, and the same fact taken by a cheaper mechanical route. Correct on both counts — a gate that declines to measure is not a gate that passed.

docs-drift, answered on the axis the bot says it cannot see

The bot reported "nothing to list — not a clean bill of health", which is its standing disclaimer: it is symbol-anchored and precision-first (#9192), so it cannot tell whether prose went false. Checked directly: content/docs carries no statement of the 50-row inference (the only first 50 / 50 rows hits are the MCP page's query cap and an HTTP pagination example), and the two report pages document defineReport's pivot metadata — rows / columns / values — which is a different concept from ReportService's CSV/HTML export column inference. Positive controls passed on both greps. No prose goes false.

On the work itself

Test-first with the failure signature predicted in writing before the first run, and the run matched it (3 failed | 76 passed, expected [ 'id', 'status' ] to include 'escalation_note'). Identities pinned, not counts — which earned itself twice over: the toEqual order assertion caught a bug in the test's own <th> extraction that toContain or toHaveLength(3) would have hidden, and the pre-existing toMatch(/a|b/) alternation — which passed either way and was never part of the red — is strengthened to toEqual(['a','b']). The docblock records why it must be every row and what bounds the cost, so the next reader cannot re-introduce the sample as an optimisation without ruling against it on purpose.

The route rejection is on the right ground too: the declared-column-set alternative was refused because ReportQuery.fields is optional and that route would need a new declared surface this card does not carry — a premise ground, not a cost one.

Landing: Clause-② NO ⇒ no review gate. Flipping ready and arming auto-merge; it holds for green. CI is mid-flight at the time of writing and nothing merges until it is.

---

Generated by Claude Code