[HDX-5077] Move multi-series metric merge computation to ClickHouse

[wrn14897]

Why

HDX-5077 — prerequisite for metric formulas (HDX-4938), which need all metric series as columns of one relation so a formula can render as a final SELECT projection.

A metric chart with N series used to fan out into N ClickHouse queries whose result sets were merged node/browser-side in mergeResultSets / computeResultSetRatio. This resolves the long-standing TODOs ("generate union CTEs on the db side", "compute the ratio on the db side") by composing the per-series queries into one statement.

Stacked on #2858 (HDX-5076 regression baseline) — that suite was written against the old node-side merge and passes unchanged against this implementation, which is the parity gate.

How

New renderMultiSeriesMetricChartConfig in renderChartConfig.ts: each per-series branch is rendered exactly as the single-series path would (own CTE scaffolding, own physical table per metric type), but with internal aliases (__hdx_value, __hdx_group_{j}); branches are tagged with their series index, combined via UNION ALL, and pivoted back into one row per (group values, time bucket):

SELECT
  anyOrNullIf(__hdx_value, __hdx_series_idx = 0) AS "avg(metric.alpha)",
  anyOrNullIf(__hdx_value, __hdx_series_idx = 1) AS "avg(metric.beta)",
  __hdx_group_0 AS "ServiceName",
  __hdx_time_bucket
FROM (
  SELECT ..., 0 AS __hdx_series_idx FROM (<per-series query 0>)
  UNION ALL
  SELECT ..., 1 AS __hdx_series_idx FROM (<per-series query 1>)
)
GROUP BY __hdx_group_0, __hdx_time_bucket
ORDER BY __hdx_time_bucket
SETTINGS <hoisted, deduped>

• anyOrNullIf keeps missing rows NULL → rendered gap, never 0
• Ratio (seriesReturnType: 'ratio') divides in SQL: coalesce(v0, 0) / nullif(v1, 0); ratioMode: 'share_of_total' divides by sum(v1) OVER (PARTITION BY __hdx_time_bucket)
• Same-alias series keep the __{splitIndex} suffix; the suffix is stripped from the ratio label (parity)
• Gauge/sum branches project group-by dimensions as columns; histogram branches keep their Array group column; each class pads the other's columns (NULL / []) so mixed charts stay type-compatible without joining rows across classes (parity with the old merge, which never joined them)
• Per-branch SETTINGS are stripped (extractSettingsClauseFromEnd) and hoisted deduped — ClickHouse rejects SETTINGS on non-final UNION branches
• queryChartConfig renders once / queries once for every config; mergeResultSets, computeResultSetRatio, computeRatio + helpers deleted along with their unit suites (semantics now pinned by the int baseline)
• Latent bug fix: sum increase TopGroups ranking now strips group-by aliases (an aliased groupBy rendered AS "alias" inside tuple(...) — a syntax error; also required by the injected internal aliases), mirroring the strip in renderSeriesLimitCte

Side effects

• "View SQL" for multi-series metric charts now shows the full composed query (previously silently showed only series 0)
• One query round trip instead of N; no more O(rows) merge in the browser
• Value column meta types become the UNION supertype (e.g. Nullable(Float64)) — consumers resolve types via convertCHDataTypeToJSType, which already handles Nullable (asserted in the baseline suite)
• Group column display names for expression group-bys (e.g. map accesses) now use the expression text instead of ClickHouse's derived name (Attributes['host'] vs arrayElement(Attributes, 'host')) — legends use group values, so this only affects table headers

Test plan

• Parity gate: all 12 HDX-5076 baseline int tests pass unchanged against the new implementation (first run, no adjustments)
• New SQL snapshot unit tests for the composed query (pivot shape, ratio both modes, mixed-class padding, alias collision, settings hoisting, number shape)
• make ci-lint (incl. ratchet) and make ci-unit green repo-wide (5,550 tests)
• Int suites against docker CH 26.5: common-utils queryChartConfig (26/26) + sampleWeightedAggregations, api checkAlerts (268/268, 90 snapshots unchanged), api renderChartConfig metric suite (99/99, 28 snapshots unchanged), MCP queryTool (65/65)
• Pre-existing metadata.int.test.ts failures reproduce identically on unmodified main (environment-dependent, unrelated)

Issues

#2680

Out of scope (follow-ups)

Formula schema/UI (HDX-5078/5079), builderToRawSql multi-series support, external API v2 cross-source /charts/series fan-out, enabling chunking for metric charts.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview	Aug 12, 2026 6:30am
hyperdx-storybook	Ready	Preview	Aug 12, 2026 6:30am

[changeset-bot]

🦋 Changeset detected

Latest commit: 9aa6819

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

This PR includes changesets to release 4 packages

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

[greptile-apps]

Greptile Summary

The PR moves multi-series metric merging and ratio computation from application code into one composed ClickHouse query.

• Renders each metric series as an independently configured query branch.
• Combines branches with UNION ALL, then pivots values by series index and grouping keys.
• Hoists branch SETTINGS into the final statement and removes the former client-side merge helpers.
• Updates chart, CLI, integration, and snapshot coverage for the composed-query behavior.

Confidence Score: 4/5

The PR is not yet safe to merge because comma-bearing ClickHouse setting values still produce malformed composed SQL.

The final SETTINGS clause is still reconstructed with an unstructured comma split even though setting values are arbitrary quoted strings, so valid comma-bearing values can break every multi-series metric query using that setting.

Files Needing Attention: packages/common-utils/src/core/renderChartConfig.ts

Important Files Changed

Filename	Overview
packages/common-utils/src/core/renderChartConfig.ts	Adds the composed multi-series SQL renderer; the previously reported SETTINGS parser defect remains present in the hoisting path.
packages/common-utils/src/clickhouse/index.ts	Simplifies chart execution to render and issue one query instead of splitting and merging result sets client-side.
packages/common-utils/src/core/builderToRawSql.ts	Aligns builder-to-raw-SQL conversion with the composed multi-series rendering path.
packages/common-utils/src/tests/renderChartConfig.test.ts	Adds broad SQL-shape coverage for pivoting, ratios, mixed metric classes, aliases, and SETTINGS hoisting.
packages/common-utils/src/tests/queryChartConfig.int.test.ts	Preserves integration-level result-shape and multi-series parity coverage against the new single-query implementation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Config[Multi-series metric chart config] --> Branches[Render one SQL branch per series]
  Branches --> Strip[Extract trailing SETTINGS]
  Strip --> Union[Combine branches with UNION ALL]
  Union --> Pivot[Group and pivot by series index]
  Pivot --> Ratio[Compute ratio when configured]
  Ratio --> Hoist[Append deduplicated SETTINGS]
  Hoist --> CH[(ClickHouse)]
  CH --> Result[Single merged chart result]

Loading

_{Reviews (5): Last reviewed commit: "fix: preserve ClickHouse-derived group c..." | Re-trigger Greptile}

[github-actions]

Deep Review

✅ No critical issues found. No P0/P1 defects: the composed-query rewrite preserves the observable multi-series contract, dead node-side code is fully removed with no dangling importers, chSql parameter names are content-hashed so merging independently-rendered branch params is collision-safe, and the external API v2 /charts/series path fans out per single-series config so it is unaffected. The items below are recommended hardening and coverage gaps.

🟡 P2 — recommended

• packages/common-utils/src/core/renderChartConfig.ts:178 — mergeSettingsClauses splits each branch's hoisted SETTINGS body on ,, but string-valued settings such as additional_result_filter (visible in the snapshots as additional_result_filter = 'x != 2') hold SQL boolean expressions that can legitimately contain commas (e.g. ServiceName IN ('a','b')); such a value is split into malformed items, producing an invalid SETTINGS clause and a query ClickHouse rejects. This is a regression limited to the new multi-series composed path — the single-series path emits the clause verbatim without re-splitting.
  • Fix: Parse the settings body with quote/bracket awareness (reuse splitAndTrimWithBracket) instead of a raw split(','), or fail loudly on an unparseable shape.
  • _{security, testing, maintainability}
• packages/common-utils/src/core/renderChartConfig.ts:2453 — The share_of_total ratio denominator emits sum(...) OVER () (empty window) when there is no granularity, but every share_of_total test supplies granularity, so the global-window arm used by Table/Number ratio charts is never generated or executed; the deleted unit suite covered this exact case (grand-total division with no timestamp column).
  • Fix: Add an integration test with seriesReturnType: 'ratio', ratioMode: 'share_of_total', a group-by, and no granularity, asserting groups divide by the grand total.
  • _testing
• packages/common-utils/src/core/renderChartConfig.ts:2417 — The composed query's correctness rests on an undocumented positional invariant: the scalar branch is emitted as bare SELECT *, the outer pivot uses * EXCEPT (...) + GROUP BY ALL, and histogram branches pad exactly scalarGroupCount NULL columns — all of which only line up if renderSelect keeps projecting [value, groupCols…, bucket] and scalarGroupCount matches the scalar branch's actual group-column count; a future divergence surfaces only as a raw ClickHouse UNION column-count error, caught by no assertion in this function.
  • Fix: Add a runtime guard comparing each branch's projected group-column count, or a comment cross-linking renderSelect's projection order as the load-bearing contract.
  • _{maintainability, testing}

🔵 P3 nitpicks (3)

• packages/common-utils/src/core/renderChartConfig.ts:2464 — Series alias (and the composite ratioName) is interpolated unescaped into the AS "…" identifier via UNSAFE_RAW_SQL; an alias containing a " can break out of the quoting. This is pre-existing parity with the single-series path and not a new privilege boundary (the chart author already has raw-SQL authoring power), but the new ratio-label sink shares it.
  • Fix: Escape embedded double-quotes in the alias before interpolation, or constrain the alias schema to disallow quote/control characters, in both the single-series and composed sinks.
• packages/common-utils/src/core/renderChartConfig.ts:141 — The changeset states result shape and column naming are "unchanged," but composed value columns now carry meta type Nullable(Float64) instead of the aggregate's concrete type; this is invisible to all in-repo consumers (they normalize via convertCHDataTypeToJSType), yet the changeset wording is inaccurate.
  • Fix: Note the meta type widening to the Nullable supertype in the changeset text.
• packages/common-utils/src/core/renderChartConfig.ts:2265 — The new doc comment references mergeResultSets, a symbol deleted in this same PR; intentional as historical migration context, but the name no longer resolves for future readers grepping the codebase.
  • Fix: Reword to describe the prior node-side merge without naming the removed symbol, or mark it explicitly as removed.

---

Reviewers (8): correctness, adversarial, security, testing, maintainability, performance, api-contract, project-standards.

Testing gaps:

• Multi-series metric ratio/pivot/column-name contract lives only in queryChartConfig.int.test.ts, which jest.config.js excludes from the default run via testPathIgnorePatterns; the standard unit gate now exercises only generated-SQL snapshots — confirm CI runs the .int suite against a live ClickHouse.
• mergeSettingsClauses is tested only for dedup of identical clauses; the union-of-distinct-settings path (two branches contributing different items that must both survive) is unasserted.
• All-histogram grouped multi-series (hasScalarGroups=false, no scalar branch to win union names) and ExponentialHistogram/Summary multi-series configs are not exercised.
• No test covers a series alias containing a double-quote in either the single-series or composed path.

Note: the correctness, adversarial, and kieran-typescript sub-agents did not return before synthesis; their focus areas (UNION column alignment, ratio math, alias-collision suffixing, recursion termination, type narrowing) were independently verified during merge and no additional defects surfaced.

[github-actions]

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

• Diff size: 680 production lines changed (Tier 2 max: < 250)
• Cross-layer change: touches frontend (packages/app) + shared utils (packages/common-utils)

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats

• Production files changed: 5
• Production lines changed: 680 (+ 1125 in test files, excluded from tier calculation)
• Branch: warren/hdx-5077-metrics-ch-side-merge
• Author: wrn14897

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

[github-actions]

E2E Test Results

✅ All tests passed • 280 passed • 1 skipped • 1110s

Status	Count
✅ Passed	280
❌ Failed	0
⚠️ Flaky	0
⏭️ Skipped	1

Tests ran across 4 shards in parallel.

View full report →

[wrn14897]

E2E failure root cause + fix (9aa6819)

The Shard 3 kubernetes e2e failures were a real regression: the composed query re-aliased group-by columns to their expression text (ResourceAttributes['k8s.namespace.name']), but consumers read merged rows by the name a single-series query produces — for un-aliased expressions that is ClickHouse's derived name (arrayElement(ResourceAttributes, 'k8s.namespace.name'), see KubernetesDashboardPage.tsx). The rename blanked the namespace/pod cells, failing should show namespace metrics and should filter by namespace. External-API clients read these rows too, so the renderer must preserve the names rather than consumers adapting.

Derived names can't be reproduced node-side, so the fix stops renaming group columns entirely:

• branches render their group-by untouched (natural names preserved)
• gauge/sum wrappers use SELECT * and go first in the UNION (UNION ALL matches by position, names come from the first branch); histogram wrappers stay explicit with NULL pads whose names are never referenced
• the outer pivot passes group/bucket columns through via * EXCEPT (__hdx_value, __hdx_series_idx) and groups with GROUP BY ALL (ClickHouse ≥ 22.12) instead of naming them

Added two int regression tests pinning the contract (un-aliased expression group-bys keep the arrayElement(...) derived name; user-aliased ones keep the alias) — the gap in the HDX-5076 baseline was that its group-bys were plain columns, where derived name == expression text.

Local verification: 28/28 queryChartConfig int tests, 1756 unit tests, lint + ratchet clean. Letting CI confirm the k8s e2e.