Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ui/packages/@quent/components/src/dag/DAGNodeInfoPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from '@quent/hooks';
import { DataText } from '../ui/data-text';
import { thinScrollbarClass } from '../ui/thin-scroll';
import { inferFieldFormatter } from '@quent/utils';
import { inferFieldFormatter, isNumericValue } from '@quent/utils';
import { DataFlowMatrix } from './DataFlowMatrix';

export const DAGNodeInfoPanel = ({ isDark = false }: { isDark?: boolean }) => {
Expand Down Expand Up @@ -86,7 +86,7 @@ export const DAGNodeInfoPanel = ({ isDark = false }: { isDark?: boolean }) => {
<div className="ml-2 flex flex-col gap-0.5">
{value.map((item, i) => (
<DataText key={i} className="text-muted-foreground whitespace-pre-line">
{item}
{String(item)}
</DataText>
))}
</div>
Expand All @@ -95,7 +95,7 @@ export const DAGNodeInfoPanel = ({ isDark = false }: { isDark?: boolean }) => {
<div className="flex items-center justify-between">
<DataText className="capitalize">{key.replace(/_/g, ' ')}:</DataText>
<DataText className="text-muted-foreground ml-1">
{typeof value === 'number' ? inferFieldFormatter(key)(value) : String(value)}
{isNumericValue(value) ? inferFieldFormatter(key)(value) : String(value)}
</DataText>
</div>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,6 @@ describe('parseCustomStatistics', () => {
expect(parseCustomStatistics(op)).toEqual([{ key: 'label', value: 'hello' }]);
});

it('unwraps a boolean tagged value', () => {
const op = makeOperator({ enabled: makeTagged('Bool', true) });
expect(parseCustomStatistics(op)).toEqual([{ key: 'enabled', value: true }]);
});

it('produces null for a null-valued tagged entry', () => {
const op = makeOperator({ missing: null });
expect(parseCustomStatistics(op)).toEqual([{ key: 'missing', value: null }]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ function expanded(
groups: Record<string, { id: string; label?: string }>,
itemId: string,
statisticName: string,
value: number | string | null
value: number | bigint | string | null
): StatGroupExpandedRow {
const normalized: Record<string, { id: string; label: string }> = {};
for (const [k, v] of Object.entries(groups)) {
Expand Down Expand Up @@ -156,6 +156,21 @@ describe('buildPivotedRows row clustering', () => {
expect(fordHybrid?.aggs.get('price')?.count).toBe(2);
});

it('aggregates bigint stat values (large U64/I64) without throwing', () => {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const rows: StatGroupExpandedRow[] = [
expanded({ brand: { id: 'Ford' }, fuel: { id: 'Hybrid' } }, 'car-1', 'output_rows', 1000n),
expanded({ brand: { id: 'Ford' }, fuel: { id: 'Hybrid' } }, 'car-2', 'output_rows', 3000n),
];
const out = buildPivotedRows(rows, [brandIdx, fuelIdx], true);
const agg = out[0]?.aggs.get('output_rows');
expect(agg?.sum).toBe(4000);
expect(agg?.mean).toBe(2000);
expect(agg?.min).toBe(1000);
expect(agg?.max).toBe(3000);
expect(agg?.count).toBe(2);
expect(agg?.isNumeric).toBe(true);
});

it('preserves first-appearance order across both columns of the hierarchy', () => {
// `buildPivotedRows` merges rows that share a full group-key path, so
// duplicate (brand, fuel) pairs collapse to one output row. What we
Expand Down
9 changes: 9 additions & 0 deletions ui/packages/@quent/components/src/pivot-table/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ describe('formatStatValue', () => {
it('converts a string value using String()', () => {
expect(formatStatValue('hello', 'label')).toBe('hello');
});

it('formats a bigint value (large U64/I64 stat) via the field formatter', () => {
expect(formatStatValue(1073741824n, 'spill_bytes')).toBe('1.00 GiB');
expect(formatStatValue(1500n, 'output_rows')).toBe('1.50 k');
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

// ---- isNumericValue --------------------------------------------------------
Expand All @@ -170,6 +175,10 @@ describe('isNumericValue', () => {
expect(isNumericValue(42)).toBe(true);
});

it('returns true for a bigint', () => {
expect(isNumericValue(42n)).toBe(true);
});

Comment thread
coderabbitai[bot] marked this conversation as resolved.
it('returns false for null', () => {
expect(isNumericValue(null)).toBe(false);
});
Expand Down
62 changes: 42 additions & 20 deletions ui/packages/@quent/components/src/pivot-table/utils.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import { inferFieldFormatter } from '@quent/utils';
import { inferFieldFormatter, isNumericValue } from '@quent/utils';
import type { StatValue, ContinuousPaletteName } from '@quent/utils';
import { continuousColor } from '@quent/utils';

// Re-exported for consumers that still import it from here; defined in `@quent/utils`
export { isNumericValue };

import type {
StatGroupExpandedRow,
GroupKeyEntry,
Expand All @@ -23,7 +27,7 @@ export interface GroupIndexDef {
getLabel: (row: StatGroupExpandedRow) => string;
}

export function formatNumericStat(n: number | null, statName: string): string {
export function formatNumericStat(n: number | bigint | null, statName: string): string {
if (n === null) return '-';
return inferFieldFormatter(statName)(n);
}
Expand All @@ -43,16 +47,12 @@ export function itemHasId(items: Iterable<string>, target: ReadonlySet<string>):

export function formatStatValue(value: StatValue, statName: string): string {
if (value === null || value === undefined) return '-';
if (typeof value === 'number') return formatNumericStat(value, statName);
if (isNumericValue(value)) return formatNumericStat(value, statName);
if (typeof value === 'boolean') return value ? 'true' : 'false';
if (Array.isArray(value)) return value.join(', ');
return String(value);
}

export function isNumericValue(v: StatValue): v is number {
return typeof v === 'number';
}

// --- color gradient ---

export function gradientBg(
Expand Down Expand Up @@ -173,7 +173,7 @@ export function getSortValue(
if (!isAgg) {
const v = row.values.get(stat);
if (v === undefined) return null;
return isNumericValue(v) ? v : null;
return isNumericValue(v) ? Number(v) : null;

@coderabbitai coderabbitai Bot Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file outline =="
ast-grep outline ui/packages/@quent/components/src/pivot-table/utils.ts --view expanded || true

echo
echo "== relevant lines =="
sed -n '140,310p' ui/packages/@quent/components/src/pivot-table/utils.ts

echo
echo "== search for isNumericValue and related callers =="
rg -n "isNumericValue|parseJsonWithBigInt|Number\\(v\\)|pivot-table" ui/packages/@quent/components/src -S

Repository: rapidsai/quent

Length of output: 10680


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== utils.ts around numeric formatting and aggregation =="
sed -n '1,140p' ui/packages/@quent/components/src/pivot-table/utils.ts
sed -n '240,290p' ui/packages/@quent/components/src/pivot-table/utils.ts

echo
echo "== pivot-table types =="
sed -n '1,220p' ui/packages/@quent/components/src/pivot-table/types.ts

echo
echo "== search for StatValue and isNumericValue definitions =="
rg -n "type StatValue|interface StatValue|export .*isNumericValue|function isNumericValue|const isNumericValue" ui/packages/@quent -S

Repository: rapidsai/quent

Length of output: 11009


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== isNumericValue / formatter definitions in `@quent/utils` =="
rg -n "export .*isNumericValue|function isNumericValue|const isNumericValue|inferFieldFormatter|format.*bigint|StatValue" ui/packages/@quent/utils/src -S

echo
echo "== candidate files =="
fd -a "dagTypes.ts|.*utils.*" ui/packages/@quent/utils/src

echo
echo "== relevant source snippets =="
sed -n '1,140p' ui/packages/@quent/utils/src/dagTypes.ts
sed -n '1,220p' ui/packages/@quent/utils/src/index.ts

Repository: rapidsai/quent

Length of output: 8520


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== getSortValue usages =="
rg -n "getSortValue\\(" ui/packages/@quent/components/src -S

echo
echo "== sort logic around pivoted rows =="
rg -n "sortInfo|compare|localeCompare|getSortValue|aggMode" ui/packages/@quent/components/src/pivot-table -S

echo
echo "== relevant chunks from pivot-table components =="
sed -n '1,260p' ui/packages/@quent/components/src/pivot-table/GroupedDataTable.tsx
sed -n '1,260p' ui/packages/@quent/components/src/pivot-table/PivotedStatTable.tsx

Repository: rapidsai/quent

Length of output: 26159


Keep pivot-table stats as bigint through sort and aggregation
StatValue already includes bigint, but both Number(v) in getSortValue and Number(row.value) in the aggregation bucket silently narrow large u64 values. That can mis-order rows and produce wrong sums/means/min/max once values exceed Number.MAX_SAFE_INTEGER; use bigint-aware handling instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ui/packages/`@quent/components/src/pivot-table/utils.ts at line 176, Update
getSortValue and the aggregation bucket’s row.value handling to preserve bigint
values through sorting and aggregation instead of coercing them with Number. Add
bigint-aware comparisons and sum/mean/min/max calculations, while retaining
existing behavior for numeric non-bigint values and returning the appropriate
StatValue type.

Source: Path instructions

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This too, but if you want I can go back and do the pivot table stuff

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johallar, yes please—updating the pivot-table path in this PR would be best, since it is part of the BigInt-support objective. In particular, preserve exact bigint values for sorting and aggregate operations so large u64 statistics remain correctly ordered and calculated, and add regression coverage for values beyond Number.MAX_SAFE_INTEGER.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol, comment intended for @cmatzenbach

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johallar Ok, just pushed up a fix for this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I see the agg fixes, but the line that this comment is on will still affect sorting won't it?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

@cmatzenbach cmatzenbach Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, should've mentioned it - I was thinking we could address this in a follow-up PR, as that would require cascading changes to gradientBg and the column range comparisons in PivotedStatsTable.tsx. Here would be the three areas where we are currently accepting precision loss, which would require that refactor:

  1. getSortValue line 176 - sort order for non-aggregated rows with large bigints
  2. getSortValue agg path - agg.sum, agg.min, agg.max come from PivotedRowAgg which stores number | null, so even though we now compute them more accurately as bigints, they're converted to number before being stored and returned here
  3. gradientBg color scaling - uses the number sort value to position a value on a color gradient, so large bigints that lost precision in getSortValue get slightly wrong colors

I'm not quite sure how large those changes would be, so I thought keeping it separate might make sense.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 that works! Let's just track in an issue so we don't lose track of it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done - logged in #475

}
const agg = row.aggs.get(stat);
if (!agg || !agg.isNumeric) return null;
Expand All @@ -197,7 +197,7 @@ type Accumulator = {
keys: GroupKeyEntry[];
rowKey: string;
values: Map<string, StatValue>;
aggBuckets: Map<string, { nums: number[]; count: number }>;
aggBuckets: Map<string, { nums: number[]; bigints: bigint[]; count: number }>;
itemIds: Set<string>;
itemScopeIds: Map<string, string>;
itemType: string;
Expand Down Expand Up @@ -251,11 +251,13 @@ export function buildPivotedRows(
} else {
let bucket = group.aggBuckets.get(row.statisticName);
if (!bucket) {
bucket = { nums: [], count: 0 };
bucket = { nums: [], bigints: [], count: 0 };
group.aggBuckets.set(row.statisticName, bucket);
}
bucket.count++;
if (isNumericValue(row.value)) {
if (typeof row.value === 'bigint') {
bucket.bigints.push(row.value);
} else if (typeof row.value === 'number') {
bucket.nums.push(row.value);
}
}
Expand All @@ -266,17 +268,37 @@ export function buildPivotedRows(
const aggs = new Map<string, PivotedRowAgg>();
if (isAggregating) {
for (const [stat, bucket] of group.aggBuckets) {
const hasNum = bucket.nums.length > 0;
const sum = hasNum ? bucket.nums.reduce((a, b) => a + b, 0) : null;
const mean = hasNum ? sum! / bucket.nums.length : null;
const min = hasNum ? Math.min(...bucket.nums) : null;
const max = hasNum ? Math.max(...bucket.nums) : null;
const onlyBigints = bucket.bigints.length > 0 && bucket.nums.length === 0;
const allNums = onlyBigints
? bucket.bigints.map(Number)
: [...bucket.nums, ...bucket.bigints.map(Number)];
const hasNum = allNums.length > 0;

let sum: number | null = null;
let min: number | null = null;
let max: number | null = null;
let mean: number | null = null;
let stdev: number | null = null;
if (mean !== null && bucket.nums.length > 1) {
const variance =
bucket.nums.reduce((acc, v) => acc + (v - mean) ** 2, 0) / (bucket.nums.length - 1);
stdev = Math.sqrt(variance);

if (hasNum) {
if (onlyBigints) {
// Use bigint arithmetic for sum/min/max to avoid precision loss
sum = Number(bucket.bigints.reduce((a, b) => a + b, 0n));
min = Number(bucket.bigints.reduce((a, b) => (a < b ? a : b)));
max = Number(bucket.bigints.reduce((a, b) => (a > b ? a : b)));
} else {
sum = allNums.reduce((a, b) => a + b, 0);
min = Math.min(...allNums);
max = Math.max(...allNums);
}
mean = sum / allNums.length;
if (allNums.length > 1) {
const variance =
allNums.reduce((acc, v) => acc + (v - mean!) ** 2, 0) / (allNums.length - 1);
stdev = Math.sqrt(variance);
}
}

aggs.set(stat, { sum, mean, min, max, stdev, count: bucket.count, isNumeric: hasNum });
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
useSetHighlightedNodeIds,
} from '@quent/hooks';
import { parseCustomStatistics } from '../lib/queryBundle.utils';
import { inferFieldFormatter } from '@quent/utils';
import { inferFieldFormatter, isNumericValue } from '@quent/utils';
import { DataText } from '../ui/data-text';
import { NodeFlowBar } from './NodeFlowBar';

Expand Down Expand Up @@ -124,7 +124,7 @@ export const QueryPlanNode = memo(({ data }: { data: QueryPlanNodeData }) => {
const formattedColorFieldValue =
colorFieldValue === null
? null
: typeof colorFieldValue === 'number'
: isNumericValue(colorFieldValue)
? inferFieldFormatter(colorField!)(colorFieldValue)
: String(colorFieldValue);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,22 @@ describe('computeEdgeColoring', () => {
expect(result.values.get('e2')).toBe(40);
});

it('returns continuous coloring for bigint values (coerced to number)', () => {
// Bigint stats stay continuous (coerced to number), not categorical.
const edges = [
makeEdge('e1', [{ key: 'rows', value: 10n }]),
makeEdge('e2', [{ key: 'rows', value: 40n }]),
];
const result = computeEdgeColoring(edges, 'rows', 'light');
expect(result?.type).toBe('continuous');
if (result?.type !== 'continuous') return;
expect(result.min).toBe(10);
expect(result.max).toBe(40);
expect(result.values.get('e1')).toBe(10);
expect(result.values.get('e2')).toBe(40);
expect(typeof result.values.get('e1')).toBe('number');
});

it('returns categorical coloring for string values', () => {
const edges = [
makeEdge('e1', [{ key: 'type', value: 'hash' }]),
Expand Down Expand Up @@ -231,6 +247,19 @@ describe('computeEdgeWidthConfig', () => {
expect(result!.values.get('e3')).toBe(12);
});

it('coerces bigint values (large U64/I64) into the number width scale', () => {
const edges = [
makeEdge('e1', [{ key: 'rows', value: 5n }]),
makeEdge('e2', [{ key: 'rows', value: 20n }]),
];
const result = computeEdgeWidthConfig(edges, 'rows');
expect(result).not.toBeNull();
expect(result!.min).toBe(5);
expect(result!.max).toBe(20);
expect(result!.values.get('e1')).toBe(5);
expect(typeof result!.values.get('e1')).toBe('number');
});

it('returns equal min/max for a single edge', () => {
const edges = [makeEdge('e1', [{ key: 'rows', value: 7 }])];
const result = computeEdgeWidthConfig(edges, 'rows');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import type { DAGNode, DAGEdge, NodeColoring, EdgeWidthConfig, EdgeColoring } from './types';
import { parseCustomStatistics } from '../../lib/queryBundle.utils';
import { getActivePalette, type PaletteTheme } from '@quent/utils';
import { getActivePalette, isNumericValue, type PaletteTheme } from '@quent/utils';

export function computeNodeColoring(
nodes: DAGNode[],
Expand All @@ -19,11 +19,12 @@ export function computeNodeColoring(
});
if (!entries.length) return null;

if (entries.every(e => typeof e.value === 'number')) {
const nums = entries.map(e => e.value as number);
// handle potential bigints
if (entries.every(e => isNumericValue(e.value))) {
Comment thread
johallar marked this conversation as resolved.
const nums = entries.map(e => Number(e.value));
return {
type: 'continuous',
values: new Map(entries.map(e => [e.id, e.value as number])),
values: new Map(entries.map(e => [e.id, Number(e.value)])),
min: Math.min(...nums),
max: Math.max(...nums),
};
Expand Down Expand Up @@ -53,11 +54,12 @@ export function computeEdgeColoring(
});
if (!entries.length) return null;

if (entries.every(e => typeof e.value === 'number')) {
const nums = entries.map(e => e.value as number);
// handle potential bigints
if (entries.every(e => isNumericValue(e.value))) {
const nums = entries.map(e => Number(e.value));
return {
type: 'continuous',
values: new Map(entries.map(e => [e.id, e.value as number])),
values: new Map(entries.map(e => [e.id, Number(e.value)])),
min: Math.min(...nums),
max: Math.max(...nums),
};
Expand All @@ -79,8 +81,8 @@ export function computeEdgeWidthConfig(edges: DAGEdge[], field: string | null):

const entries = edges.flatMap(edge => {
const stat = (edge.portStats ?? []).find(s => s.key === field);
if (typeof stat?.value !== 'number') return [];
return [{ id: edge.id, value: stat.value }];
if (stat?.value == null || !isNumericValue(stat.value)) return [];
return [{ id: edge.id, value: Number(stat.value) }];
});
if (!entries.length) return null;

Expand Down
4 changes: 3 additions & 1 deletion ui/packages/@quent/utils/src/dagTypes.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import type { DynamicValue } from './types/index';

// Pure-data types for DAG coloring, width configuration, and node/edge shapes.
// These are kept in @quent/utils to avoid circular dependencies between
// @quent/hooks (which holds DAG atoms) and @quent/components (which holds DAG rendering).
Expand Down Expand Up @@ -59,7 +61,7 @@ export const DAG_LAYOUT_DIRECTION = {

export type DagLayoutDirection = (typeof DAG_LAYOUT_DIRECTION)[keyof typeof DAG_LAYOUT_DIRECTION];

export type StatValue = string | number | boolean | null | string[];
export type StatValue = DynamicValue | null;

export interface DAGNode {
id: string;
Expand Down
Loading