fix: stringify JSON search sidebar filters

[mfroembgen]

Summary

• Detect JSON-backed source columns and render sidebar filter keys as ClickHouse string expressions, for example toString(ResourceAttributes.\k8s`.`namespace`.`name`), instead of JSON map access or a fixed .:String` suffix.
• Canonicalize stale URL/saved sidebar filters that still use JSON map access once source columns load.
• Reuse the same JSON string rendering for search URL filters, sidebar value queries, load-more queries, and distribution queries.
• Prioritize active and pinned fields when fetching initial facet values so selected combinations show their matching options before Load more.
• Add unit coverage for JSON filter serialization, hydration, stale persisted filter canonicalization, and metadata value/distribution SQL rendering.

Why toString(...)

Testing against ClickHouse 26.5.1.882 with JSON(max_dynamic_types=8, max_dynamic_paths=64) columns showed that .:String is only safe for JSON paths whose active dynamic type is actually String. Integer, boolean, and mixed-type paths can silently return no values with .:String, while toString(<JSON path>) returned usable values for all of these real path shapes:

• ResourceAttributes.k8s.namespace.name (string)
• ResourceAttributes.cloud.account.id (integer)
• LogAttributes.endOfBatch (boolean)
• LogAttributes.level (mixed integer/string)

The same toString(...) expression also worked for distribution/grouping queries.

Manual verification

• Verified directly against ClickHouse 26.5.1.882, where ResourceAttributes and LogAttributes are JSON(max_dynamic_types=8, max_dynamic_paths=64) columns.
• Metadata-style query over a live data window returned non-empty values for string, int, bool, and mixed JSON paths with toString(...).
• Distribution-style grouping returned values for toString(ResourceAttributes.\cloud`.`account`.`id`)andtoString(LogAttributes.`endOfBatch`)`.
• Verified a local dashboard flow against a live API: stale ResourceAttributes['k8s.namespace.name'] filters were canonicalized, results loaded without the ClickHouse arrayElement JSON error, Show Distribution loaded, and selected/pinned facets refreshed with matching values plus Load more.

Automated verification

• mise exec node@22.16.0 -- node .yarn/releases/yarn-4.13.0.cjs lint:fix
• mise exec node@22.16.0 -- node ../../.yarn/releases/yarn-4.13.0.cjs ci:lint in packages/common-utils
• mise exec node@22.16.0 -- node ../../.yarn/releases/yarn-4.13.0.cjs ci:unit in packages/common-utils
• mise exec node@22.16.0 -- node .yarn/releases/yarn-4.13.0.cjs workspace @hyperdx/app jest packages/app/src/components/__tests__/DBSearchPageFilters.test.tsx packages/app/src/components/DBSearchPageFilters/utils.test.ts packages/app/src/__tests__/searchFilters.test.ts --runInBand --coverage=false
• mise exec node@22.16.0 -- bash -lc 'corepack enable >/dev/null 2>&1 || true; make dev-e2e FILE=filter-key-edge-cases' (18 passed)

References

Fixes #2549.
Related to #2482 and #2537.

[changeset-bot]

🦋 Changeset detected

Latest commit: 8b35ce0

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

This PR includes changesets to release 4 packages

Name	Type
@hyperdx/app	Patch
@hyperdx/common-utils	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]

@mfroembgen is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

[greptile-apps]

Greptile Summary

This PR fixes JSON-backed sidebar filters in the search page by switching from .:String typed subcolumn expressions to toString(...) wrappers, which correctly handles integer, boolean, and mixed-type JSON paths in ClickHouse 26.5. It also canonicalizes stale persisted filters on load and fixes stale load-more results being applied after a query scope change.

• Core expression change: renderJsonStringSubcolumn renamed to renderJsonStringExpression, emitting toString(col.\path`)instead ofcol.`path`.:String. New parseRenderedJsonStringExpressionreverses the form for MV rollup key extraction ingetAllKeyValues`.
• Canonicalization: canonicalizeFilterQuery migrates saved/URL filters in the old bracket-form or typed-subcolumn style to the new canonical form the first time the source columns load, preventing stale state from breaking queries.
• UX fixes: FilterGroup accordion expansion logic refactored so empty facets auto-expand to surface the Load More button without overriding a manual collapse; a loadMoreGenerationRef discards in-flight load-more results when the query scope changes.

Confidence Score: 4/5

Safe to merge for standard OTel deployments; one edge case in the new expression parser affects underscore-prefixed JSON column names.

The new parseClickHouseIdentifier function uses isIdentifierStart which excludes _, while quoteIdentifierIfNeeded (and ClickHouse itself) allow _ as a valid bare-identifier first character. For any JSON column whose name starts with _, parseRenderedJsonStringExpression silently returns undefined, causing getAllKeyValues to classify the key as rollupColumn = 'NativeColumn' and return no MV rollup results. Standard OTel column names are unaffected, but users with underscore-prefixed JSON columns in MV-backed setups would see Load More return empty values.

packages/common-utils/src/core/metadata.ts — isIdentifierStart needs || char === '_' to match the set of characters accepted by quoteIdentifierIfNeeded.

Important Files Changed

Filename	Overview
packages/common-utils/src/core/metadata.ts	Replaces .:String typed subcolumn expressions with toString(...) wrappers, adds renderJsonStringExpression / stripClickHouseJsonTypeSuffix as exported helpers, and adds parseRenderedJsonStringExpression for reversing the new form in getAllKeyValues. Contains a bug: isIdentifierStart excludes _, breaking round-trip parsing for underscore-prefixed JSON column names.
packages/app/src/components/DBSearchPageFilters/utils.ts	Extends toQuotedClickHouseKeyExpression to accept a jsonColumns option and emit toString(...) expressions for JSON-column paths; moves quoteIdentifierIfNeeded to common-utils; adds cleanClickHouseExpression support for generic ClickHouse type suffixes via the new stripClickHouseJsonTypeSuffix helper.
packages/app/src/searchFilters.tsx	Adds canonicalizeFilterQuery for migrating stale bracket-form or typed-subcolumn filter persisted state to the new toString(...) canonical form; threads jsonColumns through escapeFilterStateKeys and useSearchPageFilterState.
packages/app/src/components/DBSearchPageFilters/hooks.ts	Threads jsonColumns into both raw-table and MV facet pipelines; adds loadMoreGenerationRef to discard stale load-more results after scope changes; fixes the missing setLoadMoreLoadingKeys clear that was previously uncovered by a missing try/finally.
packages/app/src/DBSearchPage.tsx	Fetches JSON column names earlier via useJsonColumns, canonicalizes stale URL/saved-search filters via a new canonicalizedFilters memo + effect, and passes jsonColumns down to the sidebar filter state hook.
packages/app/src/components/DBSearchPageFilters.tsx	Refactors FilterGroup expansion state from a boolean to a nullable override + derived boolean so empty facets auto-expand for Load More without fighting manual collapse; wires handleLoadMore to pin the group open and passes jsonColumns through toQuotedClickHouseKeyExpression calls.

_{Reviews (22): Last reviewed commit: "fix: stringify JSON search sidebar filte..." | Re-trigger Greptile}

[github-actions]

🔴 P0/P1 -- must fix

• packages/app/src/components/DBSearchPageFilters/utils.ts:25 -- cleanClickHouseExpression strips backticks but never reverses the backslash-doubling quoteJsonPathSegment applies, so canonicalizeFilterQuery is non-idempotent for a JSON path segment containing a backslash or backtick; the DBSearchPage memo/effect then re-setValues an ever-growing filter each render and crashes with "Maximum update depth exceeded".
  • Fix: Make cleanClickHouseExpression the exact inverse of quoteJsonPathSegment (halve doubled backslashes and undouble backticks), or reuse the metadata-side parseRenderedJsonStringExpression, so escape∘unescape reaches a fixed point.
  • _{adversarial, correctness, maintainability, kieran-typescript}

🟡 P2 -- recommended

• packages/app/src/DBSearchPage.tsx:1315 -- The canonicalization effect snapshots live form state via getValues() into setSearchedConfig, so when useJsonColumns resolves asynchronously after the user has typed an unsubmitted where/select, it commits and executes that draft and rewrites the URL.
  • Fix: Update only the filters field from the already-searched searchedConfig, not the live form, so canonicalization cannot pick up unsubmitted edits.
  • _{adversarial, julik-frontend-races}
• packages/app/src/components/DBSearchPageFilters.tsx:945 -- The shouldExpandForLoadMore effect only ever calls setExpanded(true) and re-fires on every false→true transition (e.g. live-tail refetch toggling optionsLoading while extraFacetKeys resets), re-expanding a facet group the user just collapsed.
  • Fix: Make auto-expand one-shot per key (track already-auto-expanded keys in a ref) so it never re-asserts after a manual collapse.
  • _{adversarial, julik-frontend-races}
• packages/common-utils/src/core/metadata.ts:185 -- The new security-sensitive JSON-path parsers (parseRenderedJsonStringExpression, parseClickHouseIdentifier, parseRenderedJsonPath, stripClickHouseJsonTypeSuffix) are only exercised through happy-path callers, with no direct negative or adversarial-escaping coverage.
  • Fix: Add direct tests for malformed inputs (unterminated toString(/backtick, unquoted segments, empty path) and round-trip cases with doubled backticks and backslashes.
  • _{kieran-typescript, testing}

🔵 P3 nitpicks (6)

• packages/common-utils/src/core/metadata.ts:2217 -- parseRenderedJsonStringExpression uses bracket indexing innerExpression[parsedColumn.endIdx] while the rest of the module uses charAt(), relying on noUncheckedIndexedAccess being off.
  • Fix: Use charAt() for consistency and undefined-safety.
• packages/common-utils/src/core/metadata.ts:60 -- quoteJsonPathSegment was newly exported but has no consumer outside metadata.ts, and four low-level quoting helpers now widen the published @hyperdx/common-utils API surface.
  • Fix: Keep quoteJsonPathSegment module-private and confirm the other helper exports are intended public contract.
• packages/app/src/components/DBSearchPageFilters/utils.ts:5 -- The imported quoteIdentifierIfNeeded unquotes and escapes backslashes before quoting, diverging from the removed SqlString.escapeId version for already-quoted or backslash-containing flat column names.
  • Fix: Add a regression assertion that flat-column quoting still emits identical SQL for special-character column names.
• packages/app/src/components/DBSearchPageFilters/hooks.ts:140 -- Call sites pass { jsonColumns: jsonColumns ?? [] } even though toQuotedClickHouseKeyExpression already defaults options.jsonColumns ?? [] internally, duplicating the empty-array default in ~6 places.
  • Fix: Pass jsonColumns directly and let one layer own the default.
• packages/app/src/components/DBSearchPageFilters/hooks.ts:130 -- The escapedKeysToFetch memo gates only on isColumnsLoading, so if useColumns resolves before useJsonColumns (still []), JSON sub-keys are briefly escaped as invalid map-access SQL until the memo re-runs.
  • Fix: Also gate the memo on jsonColumns having loaded before fetching JSON-backed keys.
• packages/common-utils/src/core/metadata.ts:1695 -- getKeyValues rollup parsing only recognizes the new toString(...) form, so legacy .:String keys that were never canonicalized fall through parseKeyPath and mis-split into rollupColumn/rollupKey, degrading facet value suggestions.
  • Fix: Recognize and strip the legacy .:String typed-subcolumn form in the keyExpressions parse step.

---

Reviewers (8): correctness, security, adversarial, julik-frontend-races, kieran-typescript, testing, maintainability, api-contract.

Testing gaps:

• DBSearchPage canonicalizedFilters memo/effect has no coverage of its loop-avoidance guard or its non-overwrite behavior.
• canonicalizeFilterQuery non-sql early bail, length-mismatch bail, dateTimeColumns argument, and ordering-difference skip paths are untested.
• No adversarial round-trip tests for JSON path segments containing backslashes, backticks, closing parens, or newlines.

[github-actions]

Deep Review

✅ No critical issues found. The core mechanics of this change hold up under review: the new toString(<column>.\seg`.`seg`)JSON key rendering is correctly escaped (backslash-then-backtick doubling inside backtick-quoted identifiers, column names gated throughquoteIdentifierIfNeeded), filter values remain parametrized rather than concatenated, canonicalizeFilterQuery is guarded against dropping unrelated compound predicates (areSameFilterQueriesround-trip check + length guard) and is idempotent, and theDBSearchPagecanonicalization effect is protected from an update loop by itsJSON.stringify` equality check. No SQL-injection path is introduced. The recommendations below are test-coverage gaps for newly added behavior.

🟡 P2 -- recommended

• packages/app/src/DBSearchPage.tsx:1316 -- the new canonicalizeFilterQuery useMemo + useEffect that rewrites persisted filters and calls setValue('filters', …) and setSearchedConfig(…) has no component-level test; the only DBSearchPage test mocks useJsonColumns to [], so the effect body never runs.
  • Fix: Add an integration test that mounts DBSearchPage with a JSON column and a legacy-form persisted filter, asserting the filter is rewritten to the string-expression form and that setSearchedConfig fires once (no re-canonicalization loop).

🔵 P3 nitpicks (4)

• packages/app/src/searchFilters.tsx:67 -- every canonicalizeFilterQuery test passes a single-element filter array, leaving the index-based areSameFilterQueries comparison and per-element rewrite path unexercised for multiple filters.
  • Fix: Add a case with two JSON sidebar filters plus one mixed JSON + non-JSON filter, asserting all rewrite correctly and order is preserved.
• packages/app/src/searchFilters.tsx:108 -- the final canonicalFilters.length === filters.length ? … : filters defensive fallback's false branch is never tested.
  • Fix: Add coverage for a re-serialization that changes filter count, or annotate the branch as an intentionally-untested safety net.
• packages/common-utils/src/core/metadata.ts:185 -- parseRenderedJsonStringExpression round-trip is tested for bare and backslash-escaped paths but not for a segment containing a literal (doubled) backtick, so the backtick-reversal branch of parseClickHouseIdentifier is uncovered.
  • Fix: Add a getAllKeyValues case whose key expression contains a doubled-backtick segment and assert query_params carry the un-escaped single-backtick key.
• packages/common-utils/src/core/metadata.ts:209 -- no test drives a JSON top-level column name that itself contains special characters, so the column-identifier escaping side of renderJsonStringExpression is unverified end-to-end.
  • Fix: Add a rendering test for a special-character JSON column name asserting the column identifier is backtick-quoted, alongside a segment containing a raw backtick to lock in the injection-boundary guarantee.

---

Reviewers (4): security, testing, project-standards, learnings-researcher (plus orchestrator verification of canonicalizeFilterQuery, the DBSearchPage effect, and the metadata.ts render/parse round-trip).

Testing gaps:

• DBSearchPage.tsx canonicalization effect: no component-level test exercises the setValue + setSearchedConfig side effects or the null short-circuit.
• canonicalizeFilterQuery: no multi-filter or length-mismatch-fallback coverage.
• JSON string rendering: backtick-doubling round-trip and special-character column names are untested for the neutralization/escaping guarantee.
• Query-performance note (residual risk, not a defect): wrapping JSON paths in toString(...) instead of the typed .:String subcolumn is a deliberate, manually-verified correctness fix; its effect on ClickHouse index/skip-index usage on large datasets is not covered by automated tests.

[greptile-apps]

Add _ to isIdentifierStart to match the set of bare-identifier first characters accepted by quoteIdentifierIfNeeded and ClickHouse itself. Without this, parseClickHouseIdentifier silently fails for underscore-prefixed column names such as _my_json_col, causing parseRenderedJsonStringExpression to return undefined and getAllKeyValues to fall back to rollupColumn = 'NativeColumn' — making Load More return empty results for those columns in MV-backed setups.

Suggested change

	const isIdentifierStart = (char: string): boolean =>
	(char >= 'A' && char <= 'Z') || (char >= 'a' && char <= 'z');
	const isIdentifierStart = (char: string): boolean =>
	(char >= 'A' && char <= 'Z') || (char >= 'a' && char <= 'z') || char === '_';