fix(row-panel): emit Map['key'] subscript for Map columns in mergePath

[alex-fedotyev]

Summary

mergePath in packages/app/src/utils.ts converted numeric path segments to 1-based array subscripts ([N+1]) regardless of whether the parent column was a Map or an Array. On a Map(String, String) column this produced SQL like LogAttributes[2], which ClickHouse rejects with Illegal types of arguments: Map(String, String), UInt8 for function arrayElement. The grid row "expand" view failed for any row whose attribute path included a numeric-looking key under a Map column.

The function now accepts a mapColumns argument alongside jsonColumns. For Map-typed parents, all sub-keys render as string-key subscripts (Map['1']) regardless of whether the key looks numeric. Array columns keep their existing 1-based array-index behavior.

Confirmed impact

I traced the original report back to production session data. Two sessions in the 10-day window covered by HDX-4369 hit the exact ClickHouse error above. Both stack traces resolve to the ClickHouse query client in the chunked _app-*.js bundle, consistent with the bug class: an autocomplete / sidebar interaction on the search page produces a query that subscripts a Map column with a numeric token.

Reproduction patterns observed:

• Search view with no where clause set, user clicks an attribute in the filter sidebar; autocomplete fetch fires and ClickHouse rejects with the arrayElement type error.
• Search view filtered by TraceId = '...' with a Map-bracket field in the selected columns; same error.

Low absolute frequency in the sampled window, but the failure class matches a broader pattern of user-reported "grid expand keeps failing" symptoms, so the cheap fix is worth shipping. Full per-session detail (timestamps, user identities, session IDs, source IDs, exact URLs) is on the Linear ticket.

Adjacent failures in the same RUM window look superficially similar but are NOT addressed by this PR: two cases of subscripting a String column (e.g. SomeStringCol['...']) and one case of subscripting a JSON column. Different bug class, different fix; flagging for context.

What this PR changes

mergePath now accepts a third mapColumns argument. The three direct call sites thread it through:

• useAutoCompleteOptions derives mapColumns from the field list via the new deriveMapColumnsFromFields helper, matching on the canonical JSDataType.Map so wrapped Map types (LowCardinality(Map(...)), Nullable(Map(...))) are detected too.
• DBRowJsonViewer accepts an optional mapColumns prop, threaded by DBRowDataPanel and DBRowOverviewPanel from the result-set metadata via a new getMapColumnNames helper.
• DBSearchPageFilters uses a new useMapColumns hook (mirrors useJsonColumns).

The same-bug-class sibling in DBSearchPageFilters/utils.ts (toClickHouseKeyExpression, the "Load more" SQL path) is fixed in this PR by threading the parsed base name as mapColumns into mergePath. parseMapFieldName already proves the base column is a Map.

mergePath's Map and string-key branches now escape backslash and single-quote before interpolating into single-quoted SQL strings, so a Map sub-key like it's produces Map['it\'s'] instead of malformed SQL.

The deep-review pass on the previous revision flagged a residual leak through buildJSONExtractQuery. When a Map column sub-value is itself a JSON-parseable string, HyperJson promotes the child context to isInParsedJson=true with parsedJsonRootPath=[MapCol, key]. The inner mergePath call in buildJSONExtractQuery did not receive mapColumns, so a numeric sub-key fell through the default branch and re-introduced Map[N+1]. The latest commit threads mapColumns through buildJSONExtractQuery and its four call sites in DBRowJsonViewer (Add to Filters, Search, Chart, Toggle Column).

Explained test case for the reviewer

The shortest path from "this PR's claim" to "this PR's fix" is one test in packages/app/src/__tests__/utils.test.ts:

it("emits Map['1'] for a numeric segment under a Map column", () => {
  expect(mergePath(['LogAttributes', '1'], [], ['LogAttributes']))
    .toBe("LogAttributes['1']");
});

On main this test fails with LogAttributes[2]. That's the exact subscript that produced the ClickHouse error in the production sessions. After this PR the test passes because mergePath consults mapColumns (the third argument) and renders the string-key subscript instead of the array-index subscript.

The same shape applies to the other layers:

// DBSearchPageFilters/utils.test.ts: the "Load more" SQL path
expect(toClickHouseKeyExpression('LogAttributes.1', 'String'))
  .toBe("LogAttributes['1'] AS \"LogAttributes.1\"");

// DBRowJsonViewer.test.tsx: the JSON-extract path through a Map column
expect(buildJSONExtractQuery(
  ['LogAttributes', '1', 'foo'],
  ['LogAttributes', '1'],
  [], 'JSONExtractString',
  ['LogAttributes'],
)).toBe("JSONExtractString(LogAttributes['1'], 'foo')");

Each one is a regression test for a specific surface where a Map column meets a numeric-looking sub-key. If the threading drops at any call site, the corresponding test trips.

Test plan

• New tests in packages/app/src/__tests__/utils.test.ts covering:
  • The failing repro from the issue body: mergePath(['LogAttributes', '1'], [], ['LogAttributes']) returns LogAttributes['1'], not LogAttributes[2]
  • Non-numeric Map sub-key (unchanged behavior)
  • Multi-segment Map path
  • Array column with numeric key (inverse: unchanged behavior)
  • JSON-vs-Map precedence
  • Default (Array / unknown column) cases unchanged
  • SQL escaping for single quote, backslash, and both combined (Map and default branches)
• New tests in packages/app/src/components/DBSearchPageFilters/utils.test.ts covering:
  • Numeric-looking Map sub-key on the "Load more" path: LogAttributes.1 -> LogAttributes['1']
  • Multi-segment property path starting with a numeric segment
• New tests in packages/app/src/hooks/__tests__/useAutoCompleteOptions.test.tsx covering deriveMapColumnsFromFields:
  • Top-level Map columns
  • Wrapped Map types via jsType (LowCardinality(Map(...)), Nullable(Map(...)))
  • Nested fields excluded
  • Non-Map columns excluded
  • Undefined and empty inputs
• New tests in packages/app/src/components/DBRowJsonViewer.test.tsx covering buildJSONExtractQuery with mapColumns:
  • Numeric Map sub-key holding parsed JSON: emits JSONExtractString(LogAttributes['1'], 'foo')
  • Deeply nested numeric Map sub-key
  • Non-numeric Map sub-key (regression)
  • Empty mapColumns falls back to array-index (regression, covers Array(JSON))
• yarn jest for affected files (208 passed)
• make ci-lint clean
• yarn tsc --noEmit clean
• Reproduction confirmed against the production session data referenced in HDX-4369

Deferred follow-up

packages/api/src/controllers/ai.ts has three mergePath calls with the same bug class on the AI Summary / AI Assistant backend path. Fixing it crosses into a second package and would push this PR to Tier 3, so I split it out: #2367 (tracked as HDX-4402).

Fixes HDX-4369.

[changeset-bot]

🦋 Changeset detected

Latest commit: c57614c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@hyperdx/app	Patch
@hyperdx/api	Patch
@hyperdx/otel-collector	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview, Comment	Jun 3, 2026 2:56pm
hyperdx-storybook	Ready	Preview, Comment	Jun 3, 2026 2:56pm

[github-actions]

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

• Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats

• Production files changed: 8
• Production lines changed: 203 (+ 304 in test files, excluded from tier calculation)
• Branch: alex/HDX-4369-mergepath-map-subscript
• Author: alex-fedotyev

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

[github-actions]

E2E Test Results

✅ All tests passed • 197 passed • 3 skipped • 1318s

Status	Count
✅ Passed	197
❌ Failed	0
⚠️ Flaky	2
⏭️ Skipped	3

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

Deep Review

✅ No critical issues found. This change is additive (a defaulted third mapColumns argument), threads correctly through all five app-package mergePath call sites, and is well covered by unit tests. The Map/Array/JSON branch precedence and the escapeSqlSingleQuoted ordering (backslash before quote) are correct. No P0/P1 regression is introduced on a reachable path. The findings below are recommendations and nits.

🟡 P2 -- recommended

• packages/app/src/components/DBRowJsonViewer.tsx:60 -- buildJSONExtractQuery now escapes baseColumn via mergePath but still interpolates each nestedPath segment into jsonPathArgs (p => '${p}') unescaped, so a parsed-JSON sub-key containing a single quote produces malformed SQL on the Add-to-Filters / Search / Chart / Toggle-column actions; the escaping fix is half-applied within a function this PR modifies (pre-existing line, in-scope to complete).
  • Fix: Apply the same single-quote/backslash escaper to each nestedPath segment when building jsonPathArgs, exporting the helper from utils.ts.
  • _{security, correctness, adversarial}
• packages/app/src/components/DBRowDataPanel.tsx:233 -- getMapColumnNames classifies Map columns by raw type-string prefix ('Map' || startsWith('Map(')) while the sibling derivations deriveMapColumnsFromFields and useMapColumns classify by canonical JSDataType.Map, leaving three non-equivalent detection paths for one concept (the divergent wrapped-type crash they describe is not reachable, since ClickHouse rejects LowCardinality(Map(...)) / Nullable(Map(...)) column definitions, but the inconsistency is a maintenance hazard).
  • Fix: Consolidate Map detection into one helper that runs through convertCHDataTypeToJSType, or document why the result-set path can safely rely on string matching.
  • _{adversarial, testing, maintainability}
• packages/app/src/components/DBRowJsonViewer.test.tsx -- the new mapColumns threading through getLineActions (its mergePath call, the four buildJSONExtractQuery call sites, and the useCallback dependency) is validated only at the pure-function level; no render-level test passes mapColumns, so dropping the prop at any call site would go uncaught.
  • Fix: Add a component test that renders with mapColumns=['LogAttributes'] and a numeric-looking sub-key, asserting the action callbacks receive the string-subscript form.
  • _{testing, kieran-typescript}

🔵 P3 nitpicks (4)

• packages/app/src/components/DBSearchPageFilters/utils.ts:159 -- the comment states parseMapFieldName "guarantees parsed.baseName is a Map," but that function is purely syntactic and has no column-type knowledge; the Map treatment is enforced by callers, not the parser.
  • Fix: Reword the comment to state the Map assumption is caller-enforced, and note that a non-Map numeric dot-form key would be mis-rendered if one ever reached this path.
  • _{correctness, adversarial}
• packages/app/src/utils.ts:993 -- the HDX-4369 rationale block (with the verbatim ClickHouse error text) is duplicated nearly word-for-word across six files plus the changeset, so any future correction must be edited in every copy.
  • Fix: Keep the authoritative rationale on mergePath and reduce each call-site comment to a one-line pointer.
  • _{maintainability}
• packages/app/src/components/DBRowJsonViewer.tsx:332 -- buildJSONExtractQuery (5 positional args) and mergePath (3 args) now carry two adjacent interchangeable string[] params (jsonColumns, mapColumns) with no type-level guard against transposition, and three call sites must spell out the default 'JSONExtractString' just to reach the trailing mapColumns slot.
  • Fix: Group the trailing column/function parameters into an options object so the two string[] arguments can no longer be silently swapped.
  • _{kieran-typescript, maintainability}
• packages/app/src/utils.ts:1014 -- escapeSqlSingleQuoted escapes single quotes as \' whereas the existing escapeString in common-utils/src/filters.ts doubles them (''); both are valid ClickHouse but the convention now diverges, and the JSON-column branch still leaves backticks unescaped (pre-existing, unchanged).
  • Fix: Align the new escaper with the established escapeString convention, or note the intentional divergence.
  • _{correctness, security}

---

Reviewers (6): correctness, security, adversarial, testing, maintainability, kieran-typescript.

Testing gaps:

• useMapColumns hook has no direct test (consistent with useJsonColumns, which is also only mocked).
• getMapColumnNames has no wrapped-type case; the inputs that would expose the detection divergence are untested.

[alex-fedotyev]

Pushed a584d181 addressing the deep-review feedback. Summary of what's in this revision:

Same-bug-class sibling in toClickHouseKeyExpression: the "Load more" SQL path now threads parsed.baseName as mapColumns into mergePath. parseMapFieldName already proves the base is a Map, so this is a one-line addition. Two new test cases in DBSearchPageFilters/utils.test.ts cover numeric-looking Map sub-keys (LogAttributes.1 and LogAttributes.42.foo).

SQL escaping in mergePath: the Map and default-branch string-key arms now escape backslash and single-quote via a small escapeSqlSingleQuoted helper. Five new test cases in utils.test.ts cover it's, back\slash, both combined, the default branch, and the no-op sanity case on numeric segments.

useAutoCompleteOptions Map detection: switched from raw type-string matching (which missed LowCardinality(Map(...)) and Nullable(Map(...))) to the canonical f.jsType === JSDataType.Map. Extracted deriveMapColumnsFromFields as an exported helper so a regression here trips a unit test. Five new test cases cover wrapped Map types, nested-field exclusion, non-Map exclusion, and undefined/empty inputs.

buildJSONExtractQuery invariant: added a comment explaining why this mergePath call intentionally omits mapColumns (gated on isInParsedJson so the root is always a JSON column). No behavior change.

Deferred: the three mergePath calls in packages/api/src/controllers/ai.ts:116-132 are the same bug class on the AI backend path. Threading mapColumns there crosses into a second package and would push this PR to Tier 3, so I split it out as #2367.

Verification:

• yarn jest for affected files: 182 passed
• make ci-lint: clean
• yarn tsc --noEmit: clean
• Predicted tier: still Tier 2 (192 prod lines, 252 test lines, single-layer)

[alex-fedotyev]

Pushed f4a1ac74. Two things in this revision:

1. Closed the deep-review P2 on buildJSONExtractQuery. The reviewer flagged that HDX-4369 still reproduces through the parsed-JSON path: when a Map column has a sub-value that is itself a JSON-parseable string, HyperJson promotes the child to isInParsedJson=true with parsedJsonRootPath=[MapCol, key], and the inner mergePath call did not receive mapColumns. A numeric sub-key fell through to Map[N+1]. I confirmed the leak with a node repro:

Map[1] with JSON child:  JSONExtractString(LogAttributes[2], 'foo')      // broken
Map[42][bar][baz]:       JSONExtractString(LogAttributes[43], 'bar', ...) // broken
Map[config] (control):   JSONExtractString(LogAttributes['config'], 'foo') // ok, non-numeric key

buildJSONExtractQuery now takes a mapColumns parameter; the four call sites in DBRowJsonViewer (Add to Filters, Search, Chart, Toggle Column) thread it through. The stale comment that claimed the gate guaranteed a JSON column was rewritten to describe what parsedJsonRootPath[0] actually is.

Four new tests in DBRowJsonViewer.test.tsx pin the behavior: numeric Map sub-key, deeply nested numeric Map sub-key, non-numeric Map sub-key (regression), and empty mapColumns (regression for the Array(JSON) case).

2. PR body now carries the production-impact context. I traced HDX-4369 back to its production session data; the patterns match a sidebar-attribute click on the search view and a TraceId-filtered search with a Map-bracket field in the selected columns. Both stacks resolve to the ClickHouse query client in the chunked _app-*.js bundle, consistent with the bug class. The PR body now has the high-level impact summary, an "explained test case" walkthrough so the reviewer doesn't have to map test names to surfaces, and the deferred AI controller follow-up (#2367 / HDX-4402) is restated. Full per-session detail (timestamps, identities, session IDs, source IDs) is on the Linear ticket.

Verification on this revision:

• yarn jest on the five affected suites: 208 passed
• make ci-lint: clean
• yarn tsc --noEmit: clean
• Predicted tier: still Tier 2 (still single-layer, prod-line count unchanged)