Skip to content

Commit eb916d3

Browse files
author
Dylan Huang
committed
Enhance computePivot function to filter out records with undefined values in both row and column fields, and update tests to verify correct handling of such cases and row total calculations.
1 parent c9c9855 commit eb916d3

File tree

2 files changed

+79
-9
lines changed

2 files changed

+79
-9
lines changed

vite-app/src/util/pivot.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,58 @@ describe('computePivot', () => {
325325
expect(res.grandTotal).toBe(1)
326326
})
327327

328+
it('skips records with undefined column field values', () => {
329+
type LooseRow = {
330+
region: string
331+
rep?: string
332+
product?: string
333+
amount?: number | string
334+
}
335+
336+
const mixed: LooseRow[] = [
337+
{ region: 'West', rep: 'A', product: 'Widget', amount: 120 },
338+
// Missing product should be excluded entirely (no 'undefined' column)
339+
{ region: 'West', rep: 'B', amount: 90 },
340+
{ region: 'East', rep: 'B', product: 'Gadget', amount: 10 },
341+
]
342+
343+
const res = computePivot<LooseRow>({
344+
data: mixed,
345+
rowFields: ['region'],
346+
columnFields: ['product'],
347+
valueField: 'amount',
348+
aggregator: 'sum',
349+
})
350+
351+
// Columns should not contain 'undefined'
352+
expect(res.colKeyTuples.map((t) => String(t))).toEqual(['Gadget', 'Widget'])
353+
354+
const rWest = 'West'
355+
const rEast = 'East'
356+
const cWidget = 'Widget'
357+
const cGadget = 'Gadget'
358+
359+
// Only valid records contribute
360+
expect(res.cells[rWest][cWidget].value).toBe(120)
361+
expect(res.cells[rEast][cGadget].value).toBe(10)
362+
expect(res.cells[rWest][cGadget]).toBeUndefined()
363+
})
364+
365+
it('row totals use the provided aggregation over row records', () => {
366+
const res = computePivot<Row>({
367+
data: rows,
368+
rowFields: ['region'],
369+
columnFields: ['product'],
370+
valueField: 'amount',
371+
aggregator: 'avg',
372+
})
373+
374+
// East row has values [10, 200] => avg 105
375+
expect(res.rowTotals['East']).toBe(105)
376+
// West row has values [90, 120] => avg 105
377+
expect(res.rowTotals['West']).toBe(105)
378+
})
379+
328380
it("test_flaky_passes_sometimes", () => {
329381
// read logs.json from data/logs.json
330382
const logsUrl = new URL('../../data/logs.jsonl', import.meta.url)

vite-app/src/util/pivot.ts

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -248,18 +248,19 @@ export function computePivot<T extends Record<string, unknown>>({
248248
// Apply filter first if provided
249249
const filteredData = filter ? data.filter(filter) : data;
250250

251-
// Filter out records that do not have defined values for all rowFields.
252-
// This avoids creating a row key of "undefined" and ensures such records
253-
// are not returned as part of the cells/row totals.
254-
const dataWithDefinedRows = filteredData.filter((rec) =>
255-
rowFields.every((f) => rec[f] !== undefined)
251+
// Filter out records that do not have defined values for all rowFields and columnFields.
252+
// This avoids creating a row or column key of "undefined" and ensures such records
253+
// are not returned as part of the cells/totals.
254+
const dataWithDefinedKeys = filteredData.filter((rec) =>
255+
rowFields.every((f) => rec[f] !== undefined) &&
256+
columnFields.every((f) => rec[f] !== undefined)
256257
);
257258
const rowKeyTuples: unknown[][] = [];
258259
const rowKeySet = new Set<string>();
259260
const colKeyTuples: unknown[][] = [];
260261
const colKeySet = new Set<string>();
261262

262-
for (const rec of dataWithDefinedRows) {
263+
for (const rec of dataWithDefinedKeys) {
263264
const rTuple = getTuple(rec, rowFields);
264265
const rKey = toKey(rTuple);
265266
if (!rowKeySet.has(rKey)) {
@@ -295,7 +296,7 @@ export function computePivot<T extends Record<string, unknown>>({
295296

296297
// Partition records per cell
297298
const cellRecords: Record<string, Record<string, T[]>> = {};
298-
for (const rec of dataWithDefinedRows) {
299+
for (const rec of dataWithDefinedKeys) {
299300
const rKey = toKey(getTuple(rec, rowFields));
300301
const cKey = toKey(getTuple(rec, columnFields));
301302
if (!cellRecords[rKey]) cellRecords[rKey] = {};
@@ -315,10 +316,27 @@ export function computePivot<T extends Record<string, unknown>>({
315316
}
316317
const value = aggregate(values, records, aggregator);
317318
cells[rKey][cKey] = { value, records };
318-
rowTotals[rKey] += value;
319319
}
320320
}
321321

322+
// Calculate row totals using the same aggregation method applied over all records in the row
323+
for (const rKey of Object.keys(cells)) {
324+
const rowRecords: T[] = [];
325+
const rowValues: number[] = [];
326+
const cols = cellRecords[rKey] ?? {};
327+
for (const cKey of Object.keys(cols)) {
328+
const records = cols[cKey];
329+
rowRecords.push(...records);
330+
if (valueField != null) {
331+
for (const rec of records) {
332+
const v = getNumber(rec[valueField]);
333+
if (v != null) rowValues.push(v);
334+
}
335+
}
336+
}
337+
rowTotals[rKey] = aggregate(rowValues, rowRecords, aggregator);
338+
}
339+
322340
// Calculate column totals using the same aggregation method
323341
for (const cKey of Object.keys(colTotals)) {
324342
const columnRecords: T[] = [];
@@ -344,7 +362,7 @@ export function computePivot<T extends Record<string, unknown>>({
344362
// non-additive aggregations like "avg").
345363
let grandTotal: number;
346364
{
347-
const allRecords = dataWithDefinedRows;
365+
const allRecords = dataWithDefinedKeys;
348366
const allValues: number[] = [];
349367
if (valueField != null) {
350368
for (const rec of allRecords) {

0 commit comments

Comments
 (0)