From 97c4555600842418caa3514a3ea9930be3a3ebc5 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev Date: Fri, 27 Feb 2026 13:21:24 -0800 Subject: [PATCH 1/2] refactor: extract deltaChartUtils and PropertyComparisonChart from DBDeltaChart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1830 Pure refactor — zero behavior change. Extracts inline utility functions and the PropertyComparisonChart component from DBDeltaChart.tsx (601 lines) into separate files, establishing the file structure for the Event Deltas improvement series. New files: - deltaChartUtils.ts: flattenData, getPropertyStatistics, mergeValueStatisticsMaps - PropertyComparisonChart.tsx: PropertyComparisonChart, HDXBarChartTooltip, layout constants (CHART_WIDTH, CHART_HEIGHT, CHART_GAP, PAGINATION_HEIGHT) DBDeltaChart.tsx shrinks from 601 to 344 lines (imports replace inline code). Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/app/src/components/DBDeltaChart.tsx | 269 +----------------- .../components/PropertyComparisonChart.tsx | 135 +++++++++ .../app/src/components/deltaChartUtils.ts | 110 +++++++ 3 files changed, 257 insertions(+), 257 deletions(-) create mode 100644 packages/app/src/components/PropertyComparisonChart.tsx create mode 100644 packages/app/src/components/deltaChartUtils.ts diff --git a/packages/app/src/components/DBDeltaChart.tsx b/packages/app/src/components/DBDeltaChart.tsx index e1cd43368b..9c5e486134 100644 --- a/packages/app/src/components/DBDeltaChart.tsx +++ b/packages/app/src/components/DBDeltaChart.tsx @@ -1,13 +1,4 @@ -import { memo, useEffect, useMemo, useState } from 'react'; -import { withErrorBoundary } from 'react-error-boundary'; -import { - Bar, - BarChart, - ResponsiveContainer, - Tooltip, - XAxis, - YAxis, -} from 'recharts'; +import { useEffect, useMemo, useState } from 'react'; import { ClickHouseQueryError } from '@hyperdx/common-utils/dist/clickhouse'; import { ChartConfigWithDateRange, @@ -20,255 +11,19 @@ import { useElementSize } from '@mantine/hooks'; import { isAggregateFunction } from '@/ChartUtils'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { getFirstTimestampValueExpression } from '@/source'; -import { - getChartColorError, - getChartColorSuccess, - truncateMiddle, -} from '@/utils'; import { SQLPreview } from './ChartSQLPreview'; - -import styles from '../../styles/HDXLineChart.module.scss'; - -/* - * Response Data is like... -{ - Timestamp: "", - Map: { - "property": value, - } -} - -- Flatten -- Count Property Occurences -- Pick most common properties -- Count values for most common properties - -- Merge both sets of properties? one property? - */ - -// TODO: doesn't work for empty objects? -// https://stackoverflow.com/a/19101235 -function flattenData(data: Record) { - const result: Record = {}; - function recurse(cur: Record, prop: string) { - if (Object(cur) !== cur) { - result[prop] = cur; - } else if (Array.isArray(cur)) { - let l; - for (let i = 0, l = cur.length; i < l; i++) - recurse(cur[i], prop + '[' + i + ']'); - if (l == 0) result[prop] = []; - } else { - let isEmpty = true; - for (const p in cur) { - isEmpty = false; - recurse(cur[p], prop ? prop + '.' + p : p); - } - if (isEmpty && prop) result[prop] = {}; - } - } - recurse(data, ''); - return result; -} - -function getPropertyStatistics(data: Record[]) { - const flattened = data.map(flattenData); - const propertyOccurences = new Map(); - - const MIN_PROPERTY_OCCURENCES = 5; - const commonProperties = new Set(); - - flattened.forEach(item => { - Object.entries(item).forEach(([key, value]) => { - const count = propertyOccurences.get(key) || 0; - propertyOccurences.set(key, count + 1); - - if (count + 1 >= MIN_PROPERTY_OCCURENCES) { - commonProperties.add(key); - } - }); - }); - - // property -> (value -> count) - const valueOccurences = new Map>(); - flattened.forEach(item => { - Object.entries(item).forEach(([key, value]) => { - if (commonProperties.has(key)) { - let valuesMap = valueOccurences.get(key); - if (!valuesMap) { - valuesMap = new Map(); - valueOccurences.set(key, valuesMap); - } - - const valueCount = valuesMap.get(value) || 0; - valuesMap.set(value, valueCount + 1); - } - }); - }); - - const percentageOccurences = new Map>(); - valueOccurences.forEach((valuesMap, property) => { - const percentageMap = new Map(); - valuesMap.forEach((valueCount, value) => { - percentageMap.set( - value, - (valueCount / (propertyOccurences.get(property) ?? 0)) * 100, - ); - }); - percentageOccurences.set(property, percentageMap); - }); - - return { - // valueOccurences, - percentageOccurences, - // commonProperties, - // propertyOccurences, - }; -} - -function mergeValueStatisticsMaps( - outlierValues: Map, // value -> count - inlierValues: Map, -) { - const mergedArray: { - name: string; - outlierCount: number; - inlierCount: number; - }[] = []; - // Collect all value names for this property - // we sort them so timestamps are ordered - const allValues = Array.from( - new Set([...outlierValues.keys(), ...inlierValues.keys()]), - ).sort(); - - allValues.forEach(value => { - const count1 = outlierValues.get(value) || 0; - const count2 = inlierValues.get(value) || 0; - mergedArray.push({ - name: value, - outlierCount: count1, - inlierCount: count2, - }); - }); - - return mergedArray; -} - -const HDXBarChartTooltip = withErrorBoundary( - memo((props: any) => { - const { active, payload, label, title } = props; - if (active && payload && payload.length) { - return ( -
-
- {title && ( - - {title} - - )} - - {label.length === 0 ? Empty String : label} - - {payload - .sort((a: any, b: any) => b.value - a.value) - .map((p: any) => ( -
- {p.name}: {p.value.toFixed(2)}% -
- ))} -
-
- ); - } - return null; - }), - { - onError: console.error, - fallback: ( -
- An error occurred while rendering the tooltip. -
- ), - }, -); - -function PropertyComparisonChart({ - name, - outlierValueOccurences, - inlierValueOccurences, -}: { - name: string; - outlierValueOccurences: Map; - inlierValueOccurences: Map; -}) { - const mergedValueStatistics = mergeValueStatisticsMaps( - outlierValueOccurences, - inlierValueOccurences, - ); - - return ( -
- - {truncateMiddle(name, 32)} - - - - {/* */} - - - } - allowEscapeViewBox={{ y: true }} - /> - - - - -
- ); -} - -// Layout constants for dynamic grid calculation. -// CHART_WIDTH is the minimum chart width used to determine how many columns fit; actual rendered -// width expands to fill the container (charts use width: '100%' inside a CSS grid). -// CHART_HEIGHT must match PropertyComparisonChart's outer div height. -// CHART_GAP is used both in the column/row formula and as the CSS grid gap. -const CHART_WIDTH = 340; // minimum column width threshold (px) -const CHART_HEIGHT = 120; // must match PropertyComparisonChart outer div height (px) -const CHART_GAP = 16; // px; used in grid gap and layout math -// Space reserved for the pagination row: Pagination control (~32px) + top padding (16px). -// Always reserved (even when pagination is hidden via visibility:hidden) so rows count is stable. -const PAGINATION_HEIGHT = 48; +import { + getPropertyStatistics, + mergeValueStatisticsMaps, +} from './deltaChartUtils'; +import { + CHART_GAP, + CHART_HEIGHT, + CHART_WIDTH, + PAGINATION_HEIGHT, + PropertyComparisonChart, +} from './PropertyComparisonChart'; export default function DBDeltaChart({ config, diff --git a/packages/app/src/components/PropertyComparisonChart.tsx b/packages/app/src/components/PropertyComparisonChart.tsx new file mode 100644 index 0000000000..af03ed3a78 --- /dev/null +++ b/packages/app/src/components/PropertyComparisonChart.tsx @@ -0,0 +1,135 @@ +import { memo } from 'react'; +import { withErrorBoundary } from 'react-error-boundary'; +import { + Bar, + BarChart, + ResponsiveContainer, + Tooltip, + XAxis, + YAxis, +} from 'recharts'; +import { Text } from '@mantine/core'; + +import { + getChartColorError, + getChartColorSuccess, + truncateMiddle, +} from '@/utils'; + +import { mergeValueStatisticsMaps } from './deltaChartUtils'; + +import styles from '../../styles/HDXLineChart.module.scss'; + +// Layout constants for dynamic grid calculation. +// CHART_WIDTH is the minimum chart width used to determine how many columns fit; actual rendered +// width expands to fill the container (charts use width: '100%' inside a CSS grid). +// CHART_HEIGHT must match PropertyComparisonChart's outer div height. +// CHART_GAP is used both in the column/row formula and as the CSS grid gap. +export const CHART_WIDTH = 340; // minimum column width threshold (px) +export const CHART_HEIGHT = 120; // must match PropertyComparisonChart outer div height (px) +export const CHART_GAP = 16; // px; used in grid gap and layout math +// Space reserved for the pagination row: Pagination control (~32px) + top padding (16px). +// Always reserved (even when pagination is hidden via visibility:hidden) so rows count is stable. +export const PAGINATION_HEIGHT = 48; + +const HDXBarChartTooltip = withErrorBoundary( + memo((props: any) => { + const { active, payload, label, title } = props; + if (active && payload && payload.length) { + return ( +
+
+ {title && ( + + {title} + + )} + + {label.length === 0 ? Empty String : label} + + {payload + .sort((a: any, b: any) => b.value - a.value) + .map((p: any) => ( +
+ {p.name}: {p.value.toFixed(2)}% +
+ ))} +
+
+ ); + } + return null; + }), + { + onError: console.error, + fallback: ( +
+ An error occurred while rendering the tooltip. +
+ ), + }, +); + +export function PropertyComparisonChart({ + name, + outlierValueOccurences, + inlierValueOccurences, +}: { + name: string; + outlierValueOccurences: Map; + inlierValueOccurences: Map; +}) { + const mergedValueStatistics = mergeValueStatisticsMaps( + outlierValueOccurences, + inlierValueOccurences, + ); + + return ( +
+ + {truncateMiddle(name, 32)} + + + + + + } + allowEscapeViewBox={{ y: true }} + /> + + + + +
+ ); +} diff --git a/packages/app/src/components/deltaChartUtils.ts b/packages/app/src/components/deltaChartUtils.ts new file mode 100644 index 0000000000..f22ec17175 --- /dev/null +++ b/packages/app/src/components/deltaChartUtils.ts @@ -0,0 +1,110 @@ +/** + * Utility functions for DBDeltaChart. + * Pure helpers with no React dependencies — safe to import from tests. + */ + +// Recursively flattens nested objects/arrays into dot-notation keys. +// Empty objects produce an empty {} entry; empty arrays produce an empty [] entry. +// Based on https://stackoverflow.com/a/19101235 +export function flattenData(data: Record) { + const result: Record = {}; + function recurse(cur: Record, prop: string) { + if (Object(cur) !== cur) { + result[prop] = cur; + } else if (Array.isArray(cur)) { + let l; + for (let i = 0, l = cur.length; i < l; i++) + recurse(cur[i], prop + '[' + i + ']'); + if (l == 0) result[prop] = []; + } else { + let isEmpty = true; + for (const p in cur) { + isEmpty = false; + recurse(cur[p], prop ? prop + '.' + p : p); + } + if (isEmpty && prop) result[prop] = {}; + } + } + recurse(data, ''); + return result; +} + +export function getPropertyStatistics(data: Record[]) { + const flattened = data.map(flattenData); + const propertyOccurences = new Map(); + + const MIN_PROPERTY_OCCURENCES = 5; + const commonProperties = new Set(); + + flattened.forEach(item => { + Object.entries(item).forEach(([key, value]) => { + const count = propertyOccurences.get(key) || 0; + propertyOccurences.set(key, count + 1); + + if (count + 1 >= MIN_PROPERTY_OCCURENCES) { + commonProperties.add(key); + } + }); + }); + + // property -> (value -> count) + const valueOccurences = new Map>(); + flattened.forEach(item => { + Object.entries(item).forEach(([key, value]) => { + if (commonProperties.has(key)) { + let valuesMap = valueOccurences.get(key); + if (!valuesMap) { + valuesMap = new Map(); + valueOccurences.set(key, valuesMap); + } + + const valueCount = valuesMap.get(value) || 0; + valuesMap.set(value, valueCount + 1); + } + }); + }); + + const percentageOccurences = new Map>(); + valueOccurences.forEach((valuesMap, property) => { + const percentageMap = new Map(); + valuesMap.forEach((valueCount, value) => { + percentageMap.set( + value, + (valueCount / (propertyOccurences.get(property) ?? 0)) * 100, + ); + }); + percentageOccurences.set(property, percentageMap); + }); + + return { + percentageOccurences, + }; +} + +export function mergeValueStatisticsMaps( + outlierValues: Map, // value -> count + inlierValues: Map, +) { + const mergedArray: { + name: string; + outlierCount: number; + inlierCount: number; + }[] = []; + // Collect all value names for this property + // we sort them so timestamps are ordered + const allValues = Array.from( + new Set([...outlierValues.keys(), ...inlierValues.keys()]), + ).sort(); + + allValues.forEach(value => { + const count1 = outlierValues.get(value) || 0; + const count2 = inlierValues.get(value) || 0; + mergedArray.push({ + name: value, + outlierCount: count1, + inlierCount: count2, + }); + }); + + return mergedArray; +} From ac4ca010100bc303e082e372c0036d80f1ad5a64 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev Date: Fri, 27 Feb 2026 15:16:50 -0800 Subject: [PATCH 2/2] fix: use CHART_HEIGHT constant instead of hardcoded 120 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses code review feedback — the PropertyComparisonChart outer div now uses the CHART_HEIGHT constant defined in the same file, preventing the values from drifting apart. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/app/src/components/PropertyComparisonChart.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/components/PropertyComparisonChart.tsx b/packages/app/src/components/PropertyComparisonChart.tsx index af03ed3a78..0114d451c4 100644 --- a/packages/app/src/components/PropertyComparisonChart.tsx +++ b/packages/app/src/components/PropertyComparisonChart.tsx @@ -85,7 +85,7 @@ export function PropertyComparisonChart({ ); return ( -
+
{truncateMiddle(name, 32)}