diff --git a/.changeset/report-export-column-inference-all-rows.md b/.changeset/report-export-column-inference-all-rows.md new file mode 100644 index 0000000000..5864cd70c4 --- /dev/null +++ b/.changeset/report-export-column-inference-all-rows.md @@ -0,0 +1,5 @@ +--- +"@objectstack/plugin-reports": patch +--- + +Report CSV and HTML exports now infer their columns from **every** row of the result set, not from the first 50. Column inference sampled `rows.slice(0, 50)` while the projection it produced was applied to all rows, so when a report declared 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: the CSV was well-formed, every row had the same arity, and nothing marked a column as inferred rather than declared, so a recipient of a scheduled report attachment could not tell. 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 — and the sampled prefix is the query's first page in its own `orderBy`, so for a report sorted by status or created date the sample correlated with exactly the column it dropped. Both affected renderers are fixed (`csv`, which is also the `default:` format branch, and `html_table`); `json` was never affected. Already-inferred columns keep their position and late-appearing ones are appended, so an export that was correct before is byte-identical now. diff --git a/packages/plugins/plugin-reports/src/report-service.test.ts b/packages/plugins/plugin-reports/src/report-service.test.ts index 70d1b8c521..b2f351bf8e 100644 --- a/packages/plugins/plugin-reports/src/report-service.test.ts +++ b/packages/plugins/plugin-reports/src/report-service.test.ts @@ -98,9 +98,65 @@ describe('renderReport', () => { expect(JSON.parse(out)).toEqual([{ a: 1 }]); }); - it('auto-detects fields from first 50 rows when none specified', () => { + it('auto-detects fields from the rows when none specified', () => { const out = renderReport([{ a: 1 }, { b: 2 }], 'csv'); - expect(out.split('\r\n')[0]).toMatch(/a|b/); + expect(out.split('\r\n')[0].split(',')).toEqual(['a', 'b']); + }); + + // [#11774] Column inference sampled only `rows.slice(0, 50)` while the + // projection was applied to ALL rows, so a key whose first occurrence was at + // row 51+ never reached the header and its values were dropped from every + // row that carried it - a well-formed CSV of uniform arity with nothing + // marking the loss. Sparse columns are the normal shape of report output, + // and the first 50 rows are the query's first page IN ITS SORT ORDER, so + // the sample is precisely the correlated case. + describe('a column whose first row is past the old 50-row sample boundary', () => { + const LATE_ROW = 55; + const LATE_COL = 'escalation_note'; + const LATE_VALUE = 'breached SLA'; + + /** 60 rows sharing `id` + `status`; only row 55 carries LATE_COL. */ + function sparseRows(): Array> { + return Array.from({ length: 60 }, (_v, i) => ({ + id: `l${String(i).padStart(2, '0')}`, + status: 'open', + ...(i === LATE_ROW ? { [LATE_COL]: LATE_VALUE } : {}), + })); + } + + it('csv: reaches the header, and its value reaches the row that carries it', () => { + const out = renderReport(sparseRows(), 'csv'); + const lines = out.split('\r\n'); + const header = lines[0].split(','); + + expect(header).toContain(LATE_COL); + // Identity and order, not arity: already-inferred columns keep their + // position and the late one is appended. + expect(header).toEqual(['id', 'status', LATE_COL]); + + // The value arrives, under its own column - `toContain(LATE_COL)` alone + // would pass on an all-empty column. + const col = header.indexOf(LATE_COL); + expect(lines[1 + LATE_ROW].split(',')[col]).toBe(LATE_VALUE); + // ...and only in that row. + expect(lines[1].split(',')[col]).toBe(''); + }); + + it('html_table: reaches the header, and its value reaches the row that carries it', () => { + const out = renderReport(sparseRows(), 'html_table'); + // `]*>` would also match `` and swallow the first cell — + // require a space or the closing angle right after the tag name. + const header = [...out.matchAll(/]*)?>(.*?)<\/th>/g)].map(m => m[1]); + const body = [...out.matchAll(/((?:]*>.*?<\/td>)+)<\/tr>/g)] + .map(m => [...m[1].matchAll(/]*>(.*?)<\/td>/g)].map(c => c[1])); + + expect(header).toContain(LATE_COL); + expect(header).toEqual(['id', 'status', LATE_COL]); + + const col = header.indexOf(LATE_COL); + expect(body[LATE_ROW][col]).toBe(LATE_VALUE); + expect(body[0][col]).toBe(''); + }); }); }); @@ -210,6 +266,27 @@ describe('ReportService', () => { expect(stored.last_run_at).toBe(now.toISOString()); }); + it('run: a field first appearing past row 50 still reaches the export body', async () => { + // [#11774] The scheduled-attachment path end to end: the saved report + // declares no `query.fields`, so the header is inferred from the result + // set. Only row 55 carries `escalation_note`. + engine._tables['lead'] = Array.from({ length: 60 }, (_v, i) => ({ + id: `l${String(i).padStart(2, '0')}`, + status: 'open', + ...(i === 55 ? { escalation_note: 'breached SLA' } : {}), + })); + const r = await svc.saveReport({ + name: 'Open', object: 'lead', query: { filter: { status: 'open' } }, format: 'csv', + }, CTX); + const result = await svc.run(r.id, CTX); + + expect(result.rowCount).toBe(60); + const lines = result.body.split('\r\n'); + const header = lines[0].split(','); + expect(header).toContain('escalation_note'); + expect(lines[1 + 55].split(',')[header.indexOf('escalation_note')]).toBe('breached SLA'); + }); + it('run: throws REPORT_NOT_FOUND for unknown id', async () => { await expect(svc.run('nope', CTX)).rejects.toThrow(/REPORT_NOT_FOUND/); }); diff --git a/packages/plugins/plugin-reports/src/report-service.ts b/packages/plugins/plugin-reports/src/report-service.ts index 469076fead..ed400dec60 100644 --- a/packages/plugins/plugin-reports/src/report-service.ts +++ b/packages/plugins/plugin-reports/src/report-service.ts @@ -118,10 +118,28 @@ function escapeCsvCell(v: unknown): string { return s; } +/** + * Column set for an export, in first-seen order. + * + * [#11774] EVERY row, never a sample. The projection these columns drive is + * applied to ALL rows, so inferring them from a prefix made any key whose + * first occurrence fell past the boundary vanish from the header *and* from + * every row that carried it — silently, in a well-formed export of uniform + * arity with nothing marking a column as inferred rather than declared. The + * prefix was also not a random sample: it is the query's first page in its + * `orderBy`, so for a report sorted by status or date it correlates with + * exactly the sparse column it drops. + * + * Cost is one `Object.keys` pass per row, and is bounded twice over: the row + * array is already capped by `Math.min(query.limit, maxRows)` before it gets + * here, and both renderers already walk the same array once per row per + * column. Inference is therefore a fraction of the render it feeds, not a new + * order of magnitude on the export path. + */ function pickFields(rows: any[], explicit?: string[]): string[] { if (explicit && explicit.length > 0) return explicit; const seen = new Set(); - for (const r of rows.slice(0, 50)) { + for (const r of rows) { if (r && typeof r === 'object') for (const k of Object.keys(r)) seen.add(k); } return Array.from(seen);