Fix: Group By on ratio charts

[jordan-simonovski]

Problem

Ratio charts (As Ratio) couldn't be meaningfully grouped:

• Adding a Group By collapsed the chart to a single line — all groups in a time bucket overwrote each other in the multi-series merge.
• On metric sources, the Group By field offered no attribute autocomplete, so you couldn't discover keys like app.tenant_id or http.route while typing.

Root cause

Ratio runs the numerator and denominator as two separate queries and merges them. Two bugs:

1. The merge keyed rows by time bucket only, so different groups at the same bucket clobbered one another.
2. computeResultSetRatio dropped every non-value column, discarding the group dimension even if it had survived the merge.

Separately, metric sources have no single from.tableName (they fan out to per-type tables),input had no table to fetch attribute keys from.

Changes

packages/common-utils/src/clickhouse/index.ts

• Merge now keys by (time bucket + group dimensions) instead of timestamp alone, so grouped series don't collapse. Ungrouped charts are unaffected (key falls back to the timestamp/fixed key).
• computeResultSetRatio now identifies the two operands by numeric type and carries every other column through (timestamp + group dimensions), emitting one series per group.
• Grouped ratios use share-of-total semantics: each group's denominator is the total of thel groups in that time bucket, so the grouped lines are each group's contribution to theoverall ratio and sum to the ungrouped value (e.g. each tenant's share of the overall error rate). Ungrouped ratios are unchanged (one row/bucket → bucket total is that row's denominator).
• A group absent from the (filtered) numerator — e.g. a tenant with zero errors — contribut

packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx

• The chart-level Group By input now builds a metric-aware tableConnection (resolving the selected metric's table via the shared getMetricTableName helper + metricName), so attribute autocomplete works while
  typing on metric ratio/multi-series charts — matching the per-series inputs. Non-metric sou

Why share-of-total

"Group by" applied to a ratio has two plausible meanings: each group's own rate (per-group)n to the overall rate (share-of-total). We chose share-of-total so a grouped error-rate
decomposes the blended rate — the lines add up to the number you see ungrouped — which matcdown the 22% by tenant" chart. (Per-group rates are still available by not using ratio mode,or via a future toggle if needed.)

Testing

• New unit tests in clickhouse.test.ts:
  • grouped ratio = each group's share of the per-bucket total, and a bucket's contribution
  • zero-error group → 0% (not N/A);
  • existing non-grouped ratio tests unchanged (with/without timestamp; insufficient-column
• Full common-utils unit suite green (1367); editor form tests green; lint + tsc clean.
• Changeset added (@hyperdx/common-utils patch).

Scope

Only affects metric ratio charts — the only config path that splits into the merge/ratio coatio or non-metric charts.

[changeset-bot]

🦋 Changeset detected

Latest commit: d7bc84b

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

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview, Comment	Jul 3, 2026 1:35am
hyperdx-storybook	Ready	Preview, Comment	Jul 3, 2026 1:35am

[github-actions]

🟡 Tier 3 — Standard

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

Why this tier:

• Diff size: 395 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: 395 (+ 329 in test files, excluded from tier calculation)
• Branch: jordansimonovski/bugfix-ratio-grouping
• Author: jordan-simonovski

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

[greptile-apps]

Greptile Summary

This PR fixes Group By behavior for ratio charts. The main changes are:

• Preserves group dimensions when merging split metric query results.
• Computes grouped ratios as each group's share of the bucket total.
• Handles missing numerator and denominator rows in grouped ratio data.
• Uses intersected metric field metadata for chart-level Group By autocomplete.
• Adds unit tests for grouped ratios, same-alias operands, and field intersection.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
packages/common-utils/src/clickhouse/index.ts	Updates ratio result shaping and split-result merging so grouped ratio rows keep their dimensions and compute against bucket totals.
packages/common-utils/src/tests/clickhouse.test.ts	Adds tests for grouped ratio output, missing split rows, non-time-series grouping, merge behavior, and same-alias operands.
packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx	Builds chart-level Group By autocomplete inputs from the current source and series state.
packages/app/src/components/DBEditTimeChartForm/utils.ts	Adds helper logic to derive metric table connections and request common fields for global Group By expressions.
packages/app/src/hooks/useMetadata.tsx	Adds optional intersection behavior when loading fields from multiple table connections.
packages/app/src/components/SQLEditor/SQLInlineEditor.tsx	Passes the field-intersection option through to metadata loading for inline SQL autocomplete.

_{Reviews (7): Last reviewed commit: "Merge branch 'main' into jordansimonovsk..." | Re-trigger Greptile}

[github-actions]

E2E Test Results

✅ All tests passed • 223 passed • 3 skipped • 1542s

Status	Count
✅ Passed	223
❌ Failed	0
⚠️ Flaky	4
⏭️ Skipped	3

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

Deep Review

Scope: Group By support for ratio charts — split-query merge/ratio logic in common-utils plus metric-aware Group By autocomplete in the app editor. 9 files, ~1 core module + React hooks/components.
Intent (advisory): Fix grouped ratio charts collapsing to one line; add metric attribute autocomplete for the chart-level Group By.

The happy path — a grouped ratio with string dimensions and two valid split result sets — is correct and well covered by the new unit tests. Findings below are edge-case robustness, one autocomplete reliability regression on the new intersect path, and test-coverage gaps. No data-corruption, crash, or security issue is introduced on the happy path.

✅ No critical (P0/P1) issues found.

🟡 P2 — recommended

• packages/app/src/hooks/useMetadata.tsx:273 — On the new intersect path, a single series' rejected field fetch resolves to [], which collapses the whole intersection to empty, so one transient error blanks the entire chart-level Group By autocomplete and react-query caches that empty result under the intersect key.
  • Fix: Intersect only over successfully-fetched connections (exclude rejected sub-arrays) or surface the fetch error, rather than treating a rejected fetch as "field absent."
  • _{adversarial, correctness}
• packages/common-utils/src/clickhouse/index.ts:576 — const [numeratorName, denominatorName] = operandNames.filter(Boolean) drops the index-aligned '' placeholders, so if one split resolves no numeric value column the surviving name shifts into the numerator slot and denominatorName becomes undefined, which computeResultSetRatio then throws on for the whole chart; the string destructuring type also hides this undefined at compile time.
  • Fix: Assert exactly two non-empty operand names when isRatio is true before destructuring, or preserve positional alignment instead of relying on filter(Boolean).
  • _{maintainability, kieran-typescript, correctness}
• packages/common-utils/src/__tests__/clickhouse.test.ts:439 — The core fixed scenario — a grouped ratio that is also time-series (group dimension plus a real timestamp column) — is only verified piecemeal; mergeResultSets tests cover group-without-timestamp or timestamp-without-group, and useMultipleAllFields intersect:true and the ChartEditorControls wiring have no test through their own layers.
  • Fix: Add a mergeResultSets test with both a timestamp and a group column (isTimeSeries:true, isRatio:true) exercising the hasGroupCols hash-key branch, plus a useMultipleAllFields({intersect:true}) hook test including a rejected connection.
  • _{testing, correctness, adversarial}

🔵 P3 nitpicks (5)

• packages/common-utils/src/clickhouse/index.ts:417 — denominator.name.replace(/__\d+$/, '') strips the synthetic collision suffix but also mangles a legitimate value alias that genuinely ends in __<digits>, mislabeling the ratio column (display-only, key stays internally consistent).
  • Fix: Strip only the exact __${splitIdx} suffix that the merge appended, tracked per rename, instead of a generic regex.
• packages/common-utils/src/clickhouse/index.ts:503 — Operand identification via inferNumericColumn(meta)?.[0] assumes the value column is the first numeric column; this holds today only because group-by columns are either positioned after the value or array-typed, an unguarded implicit invariant that would silently break if select ordering changed.
  • Fix: Derive the operand from the known select position, or add an assertion/comment documenting the value-column-position invariant.
• packages/common-utils/src/clickhouse/index.ts:433 — With no timestamp column (Table/Number tiles) every row shares the __all__ bucket, so a grouped ratio becomes each group's share of the grand total rather than its own rate; this is intended and test-locked but easy to hit unexpectedly.
  • Fix: Confirm the share-of-grand-total semantic is desired for Table/Number grouped ratios, or key the bucket by group dimensions for per-group rates.
• packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx:110 — useWatch({ control, name: 'series' }) typically returns a new array reference each render, so the downstream useMemo and useMultipleAllFields query key may churn on unrelated series-field edits.
  • Fix: Memoize on a stable derived key (e.g. the metricType/metricName tuples) rather than the whole watched series array.
• packages/common-utils/src/clickhouse/index.ts:434 — Sibling functions use two different magic sentinels (__all__ vs __FIXED_TIMESTAMP__) for the same "single fixed bucket" concept, and the value column is re-inferred in two passes.
  • Fix: Extract a shared named constant and reuse the value-column name computed in the first pass.

---

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

Testing gaps:

• No coverage for a numeric group-by dimension or the same-alias collision rename combined with grouping.
• The intersectFields prop is threaded through three layers (buildGroupByConnectionProps → SQLInlineEditor → useMultipleAllFields) but only the two endpoints are unit tested; the middle wiring is unverified.
• mergeResultSets with isRatio:true and a split lacking a numeric value column (the operandNames.filter(Boolean) edge) is untested.

Non-blocking notes: computeResultSetRatio now does two passes over the data and objectHash-per-row keying on grouped sets; both are correctness-required and bounded by chart query limits, not a concern at normal sizes. The operands-less fallback branch of computeResultSetRatio is reachable only from direct unit tests, not production callers.