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
13 changes: 13 additions & 0 deletions .changeset/json-viewer-key-order.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@hyperdx/app': patch
---

feat: choose how property keys are ordered in the row side panel

The JSON viewer sorts keys alphabetically, which is the right default for wide
ClickHouse `Map(...)` columns like `ProfileEvents` but hides the stored column
order. The properties view options menu now offers "Sort keys A–Z", "Sort keys
Z–A", and "Original order"; the choice persists with the viewer's other options.
Array elements keep their index order in every mode. In "Original order",
integer-like keys (e.g. HTTP status codes) still list first in ascending order,
since parsed JavaScript objects order keys that way.
57 changes: 56 additions & 1 deletion packages/app/src/components/DBRowJsonViewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,21 @@ import {
IconCheck,
IconCopy,
IconFilter,
IconList,
IconMinus,
IconPlus,
IconSearch,
IconSettings,
IconSortAscendingLetters,
IconSortDescendingLetters,
IconTextWrap,
} from '@tabler/icons-react';

import HyperJson, {
DEFAULT_KEY_ORDER,
FormatLeafValue,
GetLineActions,
KeyOrder,
LineAction,
} from '@/components/HyperJson';
import { useMaterializedAliasColumnsOption } from '@/hooks/useMaterializedAliasColumnsOption';
Expand Down Expand Up @@ -133,6 +138,9 @@ type ViewerOptions = {
whiteSpace?: 'pre' | 'pre-wrap';
tabulate: boolean;
filterBlanks: boolean;
// Absent for options stored before sorting was configurable; falls back to
// the alphabetical default.
keyOrder?: KeyOrder;
};

const VIEWER_OPTIONS_KEY = 'hdx_json_viewer_options';
Expand All @@ -142,8 +150,31 @@ const DEFAULT_VIEWER_OPTIONS: ViewerOptions = {
whiteSpace: 'pre-wrap',
tabulate: true,
filterBlanks: false,
keyOrder: DEFAULT_KEY_ORDER,
};

const KEY_ORDER_OPTIONS: {
value: KeyOrder;
label: string;
icon: React.ReactNode;
}[] = [
{
value: 'asc',
label: 'Sort keys A–Z',
icon: <IconSortAscendingLetters size={14} />,
},
{
value: 'desc',
label: 'Sort keys Z–A',
icon: <IconSortDescendingLetters size={14} />,
},
{
value: 'original',
label: 'Original order',
icon: <IconList size={14} />,
},
];

/**
* Migrates old `lineWrap` boolean to `whiteSpace` enum.
*
Expand Down Expand Up @@ -239,6 +270,7 @@ function HyperJsonMenu({
const [showMaterializedAliasColumns, setShowMaterializedAliasColumns] =
useMaterializedAliasColumnsOption();
const effectiveWhiteSpace = jsonOptions.whiteSpace ?? 'pre-wrap';
const effectiveKeyOrder = jsonOptions.keyOrder ?? DEFAULT_KEY_ORDER;

return (
<Group>
Expand Down Expand Up @@ -285,7 +317,7 @@ function HyperJsonMenu({
</UnstyledButton>
<Menu width={240} withinPortal={false}>
<Menu.Target>
<UnstyledButton>
<UnstyledButton data-testid="json-viewer-options-menu">
<IconSettings size={14} />
</UnstyledButton>
</Menu.Target>
Expand Down Expand Up @@ -361,6 +393,29 @@ function HyperJsonMenu({
Show materialized and alias columns
</Menu.Item>
)}
<Menu.Divider />
<Menu.Label lh={1} py={6}>
Property order
</Menu.Label>
{KEY_ORDER_OPTIONS.map(({ value, label, icon }) => (
<Menu.Item
key={value}
data-testid={`json-viewer-key-order-${value}`}
lh="1"
py={8}
leftSection={icon}
rightSection={
effectiveKeyOrder === value ? (
<IconCheck size={14} className="ps-2" />
) : null
}
onClick={() =>
setJsonOptions({ ...jsonOptions, keyOrder: value })
}
>
{label}
</Menu.Item>
))}
</Menu.Dropdown>
</Menu>
</Group>
Expand Down
39 changes: 32 additions & 7 deletions packages/app/src/components/HyperJson.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,30 @@ export type FormatLeafValue = (arg0: {
value: unknown;
}) => React.ReactNode | undefined;

/**
* How object keys are ordered at every level of the tree.
*
* `original` keeps the object's own key order — for ClickHouse `Map(...)`
* columns, physical storage order — except that integer-like keys (`"200"`,
* `Map(UInt64, …)` keys) always come first in ascending numeric order. JS
* objects order keys that way, so the true order is already gone once the
* query result is parsed.
*/
export type KeyOrder = 'asc' | 'desc' | 'original';

export const DEFAULT_KEY_ORDER: KeyOrder = 'asc';

// Store common state in an atom so that it can be shared between components
// to avoid prop drilling
type HyperJsonAtom = {
normallyExpanded: boolean;
keyOrder: KeyOrder;
getLineActions?: GetLineActions;
formatLeafValue?: FormatLeafValue;
};
const hyperJsonAtom = atom<HyperJsonAtom>({
normallyExpanded: false,
keyOrder: DEFAULT_KEY_ORDER,
});

const ValueRenderer = React.memo(
Expand Down Expand Up @@ -273,6 +288,7 @@ const Line = React.memo(
<div
ref={ref}
data-testid="json-viewer-line"
data-depth={nestedLevel}
onClick={handleToggle}
className={cx(styles.line, {
[styles.nestedLine]: nestedLevel > 0,
Expand Down Expand Up @@ -358,6 +374,8 @@ function TreeNode({

const originalLength = React.useMemo(() => Object.keys(data).length, [data]);

const { keyOrder } = useAtomValue(hyperJsonAtom);

// ClickHouse hands back `Map(...)` keys in physical storage order, which reads
// as random for wide maps like `ProfileEvents`. Sorting here (rather than
// upstream) covers every nesting level for free, since TreeNode recurses, and
Expand All @@ -366,13 +384,15 @@ function TreeNode({
const entries = React.useMemo(() => {
const raw = Object.entries(data);
// Arrays are index-keyed — reordering them would change the data.
if (isArray(data)) {
if (isArray(data) || keyOrder === 'original') {

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.

🔵 minor — 'Original order' isn't the stored order when keys look like integers

In JS, Object.entries always lists integer-like keys (e.g. "200", "404", "0") first, in ascending numeric order, no matter how they were inserted. Rows reach the viewer through JSON.parse, so a ClickHouse Map(...) with numeric-string keys (status codes, ports, shard IDs) comes out sorted under 'original' and doesn't match the physical storage order that the KeyOrder doc comment and the menu label promise. Two fixes: keep the ordered entry list from the raw response (e.g. a Map or an entries array), or say in the doc comment and the changeset that integer-like keys are still listed numerically first.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

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.

🔵 minor — "Original order" is not the stored order when keys look like integers

Object.entries always lists integer-like keys (e.g. "404", "200", or a Map(UInt64, …)) first in ascending numeric order, so for those maps "Original order" quietly shows them sorted, not in ClickHouse storage or JSON Body order. Either keep the original key order upstream (an entries array or a Map built before converting to an object) or narrow the KeyOrder doc comment and the menu wording so they don't promise the data's original order.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Valid. The integer-key reordering happens when the ClickHouse JSON result is parsed into an object, before HyperJson sees the data, so keeping the true order would mean changing the query/parse path. That's outside this PR. In 3a0f1e9 I narrowed the KeyOrder doc comment and the changeset to state the exception. I kept the "Original order" label, since it's accurate for string keys, which is the common case.

return raw;
}
return raw.sort(([a], [b]) =>
a.localeCompare(b, undefined, { numeric: true }),
const direction = keyOrder === 'desc' ? -1 : 1;
return raw.sort(
([a], [b]) =>
direction * a.localeCompare(b, undefined, { numeric: true }),
);
}, [data]);
}, [data, keyOrder]);

const visibleLines = React.useMemo(() => {
return isExpanded ? entries : entries.slice(0, MAX_TREE_NODE_ITEMS);
Expand Down Expand Up @@ -430,6 +450,7 @@ type HyperJsonProps = {
normallyExpanded?: boolean;
tabulate?: boolean;
whiteSpace?: 'pre' | 'pre-wrap';
keyOrder?: KeyOrder;
getLineActions?: GetLineActions;
formatLeafValue?: FormatLeafValue;
};
Expand All @@ -439,16 +460,20 @@ const HyperJson = ({
normallyExpanded = false,
tabulate = false,
whiteSpace = 'pre-wrap',
keyOrder = DEFAULT_KEY_ORDER,
getLineActions,
formatLeafValue,
}: HyperJsonProps) => {
const isEmpty = React.useMemo(() => Object.keys(data).length === 0, [data]);

const initialValues = React.useMemo(
() => ({ normallyExpanded, keyOrder, getLineActions, formatLeafValue }),
[normallyExpanded, keyOrder, getLineActions, formatLeafValue],
);

return (
<Provider>
<HydrateAtoms
initialValues={{ normallyExpanded, getLineActions, formatLeafValue }}
>
<HydrateAtoms initialValues={initialValues}>
<div
className={cx(styles.container, {
[styles.withTabulate]: tabulate,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import React from 'react';
import { Provider } from 'jotai';
import { MantineProvider } from '@mantine/core';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

import { DBRowJsonViewer } from '@/components/DBRowJsonViewer';
import { RowSidePanelContext } from '@/components/DBRowSidePanel';

jest.mock('next/router', () => ({
__esModule: true,
default: { push: jest.fn() },
}));

jest.mock('@/useFormatTime', () => ({
useFormatTime: () => (time: unknown) => String(time),
FormatTime: jest.fn(() => null),
}));

const VIEWER_OPTIONS_KEY = 'hdx_json_viewer_options';

// Options written before the sort setting existed — no `keyOrder` key.
const LEGACY_STORED_OPTIONS = {
normallyExpanded: true,
whiteSpace: 'pre-wrap',
tabulate: true,
filterBlanks: false,
};

const data = { zebra: 'z', alpha: 'a', Mango: 'm' };

// env="test" mounts the menu dropdown without a transition to wait on. The
// viewer options atom is module-scoped, so each render gets its own jotai store;
// the atom then reads localStorage when it mounts in that store.
const renderViewer = () =>
render(
<Provider>
<MantineProvider env="test">
<RowSidePanelContext value={{}}>
<DBRowJsonViewer data={data} />
</RowSidePanelContext>
</MantineProvider>
</Provider>,
);

const renderedKeys = (container: HTMLElement) =>

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.

🔵 minor — renderedKeys test helper copied word-for-word from HyperJson.test.tsx

The same .keyContainer > .key query helper is already in packages/app/src/components/__tests__/HyperJson.test.tsx:45. Move it to a shared test util (or export it) so the two suites can't drift apart when the HyperJson markup changes.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not changing here: it's a two-line query, and pulling it into a shared test util is the kind of hoisting the review guidance says to keep out of a feature PR.

Array.from(container.querySelectorAll('.keyContainer > .key')).map(el =>
el.textContent?.trim(),
);

const openOptionsMenu = async () => {
await userEvent.click(screen.getByTestId('json-viewer-options-menu'));
await screen.findByRole('menu');
};

const selectKeyOrder = async (value: 'asc' | 'desc' | 'original') => {
await openOptionsMenu();
await userEvent.click(screen.getByTestId(`json-viewer-key-order-${value}`));
};

const storedKeyOrder = () =>
JSON.parse(localStorage.getItem(VIEWER_OPTIONS_KEY) ?? '{}').keyOrder;

describe('DBRowJsonViewer key order', () => {
beforeEach(() => {
localStorage.clear();
});

it('sorts alphabetically when stored options predate the setting', async () => {
localStorage.setItem(
VIEWER_OPTIONS_KEY,
JSON.stringify(LEGACY_STORED_OPTIONS),
);
const { container } = renderViewer();

expect(renderedKeys(container)).toEqual(['alpha', 'Mango', 'zebra']);

await openOptionsMenu();
expect(
screen.getByTestId('json-viewer-key-order-asc').querySelector('.ps-2'),
).toBeInTheDocument();
});

it('restores a stored order on render', async () => {
localStorage.setItem(
VIEWER_OPTIONS_KEY,
JSON.stringify({ ...LEGACY_STORED_OPTIONS, keyOrder: 'original' }),
);
const { container } = renderViewer();

expect(renderedKeys(container)).toEqual(['zebra', 'alpha', 'Mango']);
});

it('switches between the stored order and reverse alphabetical', async () => {
const { container } = renderViewer();

expect(renderedKeys(container)).toEqual(['alpha', 'Mango', 'zebra']);

await selectKeyOrder('original');
expect(renderedKeys(container)).toEqual(['zebra', 'alpha', 'Mango']);
expect(storedKeyOrder()).toBe('original');

await selectKeyOrder('desc');
expect(renderedKeys(container)).toEqual(['zebra', 'Mango', 'alpha']);
expect(storedKeyOrder()).toBe('desc');

await selectKeyOrder('asc');
expect(renderedKeys(container)).toEqual(['alpha', 'Mango', 'zebra']);
expect(storedKeyOrder()).toBe('asc');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('migrateViewerOptions', () => {
whiteSpace: undefined,
tabulate: true,
filterBlanks: false,
keyOrder: 'asc',
});
});

Expand All @@ -48,6 +49,8 @@ describe('migrateViewerOptions', () => {
whiteSpace: 'pre-wrap',
tabulate: true,
filterBlanks: false,
// Migration fills in the defaults for options added later.
keyOrder: 'asc',
});
});

Expand Down
25 changes: 25 additions & 0 deletions packages/app/src/components/__tests__/HyperJson.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,31 @@ describe('HyperJson key ordering', () => {
]);
});

it('reverses the order with keyOrder="desc"', () => {
const { container } = renderWithMantine(
<HyperJson data={{ zebra: 1, alpha: 2, Mango: 3 }} keyOrder="desc" />,
);

expect(renderedKeys(container)).toEqual(['zebra', 'Mango', 'alpha']);
});

it('keeps the incoming order with keyOrder="original"', () => {
const { container } = renderWithMantine(
<HyperJson
data={{ ProfileEvents: { Seek: 59, FileOpen: 2, Query: 1 } }}
keyOrder="original"
normallyExpanded
/>,
);

expect(renderedKeys(container)).toEqual([
'ProfileEvents',
'Seek',
'FileOpen',
'Query',
]);
});

it('orders numeric suffixes naturally rather than lexicographically', () => {
const { container } = renderWithMantine(
<HyperJson data={{ key10: 1, key2: 2, key1: 3 }} />,
Expand Down
Loading
Loading