[karl-power]

Summary

Moves PromQL query planning and result shaping into common-utils so the app and MCP's query_tile share one copy instead of two that can drift. Follow-up to the review on #3528.

• New common-utils/src/core/promqlChart.ts holds what was in the app's promqlChartQuery.ts and ChartUtils.tsx: resolving granularity and alignment, building the instant and range queries, parsing results, building chart, table and histogram rows, and reducing series.
• getTimeChartGranularity, the auto bucket counts (80 for time charts, 245 for heatmaps) and MAX_TABLE_ROWS move to common-utils/src/core/utils.ts.
• The app keeps only its Prometheus fetch calls. Its behaviour is unchanged; the row builder tests moved to common-utils as they were.
• MCP's promqlTile.ts now calls the shared code, so PromQL table tiles return a column per label (capped at 10,000 rows, with limit sent on instant queries), histogram heatmaps return rows keyed by le, and heatmap auto steps match the app. Time chart and reduced tiles return the same rows as before.
• A result a chart can't draw (a string, or a histogram without le) throws PromqlChartError, which MCP reports as a user error.

How to test

1. Test steps described in feat: Run PromQL tiles from MCP query_tile #3528 still work.

References

• Linear Issue: HDX-5302
• Related PRs: feat: Run PromQL tiles from MCP query_tile #3528 (base of this PR), feat: read and write PromQL tiles and label filters in MCP #3516, feat: describe PromQL sources in MCP #3496

[changeset-bot]

🦋 Changeset detected

Latest commit: fe3ecd0

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

This PR includes changesets to release 3 packages

Name	Type
@hyperdx/api	Patch
@hyperdx/app	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
hyperdx-oss	Ready	Preview	Nov 9, 2026 5:24pm UTC
hyperdx-storybook	Ready	Preview	Nov 9, 2026 5:24pm UTC

[github-actions]

Duplicate code analysis

No new duplicate code introduced by this PR.

---

jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 110 in total (1.21%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

[github-actions]

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

• 🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.
• 🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.
• Anything about surrounding code, or asking you to widen the change (hoist a helper,
  dedupe with another file, fix other call sites): never fix here, whatever the
  severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

3 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 3 minor

None could be anchored to changed lines; they are listed below.

Findings outside the changed lines

3 minor

• 🔵 packages/common-utils/src/core/promql.ts:129 — promqlStep is now unused outside its own tests → The app's fetchRangeExpression was the last production caller. It is gone, and the new code calls the newly exported promqlStepSeconds instead. Delete promqlStep and move its cases in core/__tests__/promql.test.ts over to promqlStepSeconds, so there is a single step helper.
• 🔵 packages/common-utils/src/core/promqlChart.ts:2085 — PromqlResultData restates the API's promqlQueryDataSchema by hand → packages/api/src/controllers/promql.ts:527-543 already defines this payload as a Zod discriminated union, and PromqlQueryData is inferred from it. The two definitions already disagree: the string result is unknown here and [number, string] there. Move the Zod schema into common-utils, derive PromqlResultData with z.infer, and import it in the API controller, as the REQUIRED DRY convention asks.
• 🔵 packages/cli/src/shared/tileConfig.ts:262 — CLI keeps a copy of getTimeChartGranularity whose @source path is now wrong → This PR moves getTimeChartGranularity (with its maxBuckets parameter) into @hyperdx/common-utils/dist/core/utils, a module this file already imports from. The @source packages/app/src/ChartUtils.tsx tag now points at a file that no longer defines the function. Delete the local copy and import the shared function. Pass maxBuckets as the fourth argument and undefined for minGranularitySeconds, then run yarn ratchet:update.

---

_{Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.}

[github-actions]

Deep Review

✅ No critical issues found.

This is a behavior-preserving refactor that consolidates two drifting copies of PromQL chart planning and row-shaping into one shared module (packages/common-utils/src/core/promqlChart.ts), consumed by the app and the MCP query_tile path. Direct verification of the moved logic against the pre-refactor code found the transformations equivalent, the extracted constants and helpers correctly re-pointed, and the prior crash report resolved.

Prior feedback — resolved. The reported crash (Cannot read properties of undefined (reading 'length') on large instant clickstack_promql results) is fixed in two independent layers, either of which closes it:

• packages/api/src/mcp/tools/query/promqlTile.ts:84 — the non-reducer branch of toPromqlTileResult now spreads the seriesLabels key only when promqlSeriesLabels is present, so table/histogram tiles no longer emit seriesLabels: undefined.
• packages/api/src/utils/trimToolResponse.ts:296 — trimObject filters undefined-valued entries before the size math, so JSON.stringify(value).length can no longer run on undefined. A regression test was added in packages/api/src/utils/__tests__/trimToolResponse.test.ts.

The jscpd duplicate-code advisory names clones in packages/api/src/routers/api/prometheus.ts, which is not part of this PR's diff — not attributable to these changes.

🟡 P2 — recommended

• packages/common-utils/src/core/promqlChart.ts:51 — getPromqlChartRange widened the alignment gate from isRangeQuery(config) to !displayTypeSupportsInstantQuery(config) || isRangeQuery(config) and swapped the alignment helper; direct reading shows this is a no-op for the app's existing call sites (table/reduced configs, which support instant queries) but it is now the single shared path for every MCP tile display type, so the date-range-alignment behavior across display types is worth an explicit test to lock the equivalence in.
  • Fix: Add unit coverage asserting resolved granularity and aligned date range for each display type (line, table, number, heatmap) with alignDateRangeToGranularity both set and unset.

🔵 P3 nitpicks (2)

• packages/common-utils/src/core/promqlChart.ts:286 — the samples generator builds new Map(Object.entries(metric)) for every series even though callers only read via .get(label); the old code indexed metric[label] directly, so this adds a per-series allocation (bounded by the row cap, so not material).
  • Fix: Index metric directly with a key-presence check, or reuse the labelColumns set, to avoid the per-series Map construction.
• packages/common-utils/src/core/promqlChart.ts:382 — toPromqlChartResult types heatmap loosely as { mode?: string } rather than reusing the shared config's heatmap type, weakening the compile-time guarantee on mode === 'histogram'.
  • Fix: Reference the existing heatmap config type instead of an inline shape.

---

Reviewers (1): direct-analysis (correctness, previous-comments, maintainability, performance).

Testing gaps: The moved common-utils suite (+512 lines) and updated MCP suite (+60 lines) carry the behavior forward and cover the new PromqlChartError and trimToolResponse paths; the main untested surface is getPromqlChartRange's cross-display-type alignment behavior now that it is the shared path (see the P2 above).

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}

[github-actions]

🔴 Tier 4 — Critical

Touches authentication, tenancy data models, the public API or shipped database config — or substantially changes the query rendering engine, background tasks, the OTel pipeline, image build, or release CI.

Why this tier:

• Large diff: 1156 production lines changed (threshold: 1000)
• Cross-layer change: touches frontend (packages/app) + backend (packages/api) + shared utils (packages/common-utils)

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats

• Production files changed: 12
• Production lines changed: 1156 (+ 877 in test files, excluded from tier calculation)
• Branch: claude/hdx-5302-mcp-promql-shared-chart
• Author: karl-power

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

[github-actions]

E2E Test Results

✅ All tests passed • 464 passed • 1 skipped • 1157s

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

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

🔵 minor — The CLI's ported getTimeChartGranularity now duplicates this function, and its @source tag is stale

packages/cli/src/shared/tileConfig.ts:261-275 is a copy tagged @source packages/app/src/ChartUtils.tsx (getTimeChartGranularity), and it hardcodes 80. That function no longer exists in ChartUtils.tsx; the canonical version is now in common-utils/core/utils. Have the CLI import getTimeChartGranularity and TIME_CHART_AUTO_GRANULARITY_BUCKETS from @hyperdx/common-utils/dist/core/utils, delete the port, and run yarn ratchet:update.

_{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}

[pulpdrew]

Hit a bug while testing the next PR up the stack:

Bug: large instant clickstack_promql results crash with Cannot read properties of undefined (reading 'length')

Repro: run clickstack_promql with queryType: "instant" and an expression that returns over 50k characters. For example, increase(hyperdx_alerts_webhook_deliveries_total[$__range]) over a two-week range returns 130 series with 28 labels each. The same expression wrapped in sum by (outcome) works, because its result is small.

Cause: introduced in 5796ff7.

• An instant query builds a table tile. toPromqlTableRows never sets promqlSeriesLabels, so the non-reducer branch of toPromqlTileResult (promqlTile.ts) returns { ...rows, seriesLabels: undefined }.
• When a response is over 50k characters, formatQueryResult calls trimToolResponse. trimObject then runs JSON.stringify(value).length on each key. JSON.stringify(undefined) returns undefined, so it throws.
• Before 5796ff7, seriesLabels was always Object.fromEntries(...), so this path couldn't hit it.

[karl-power]

Hit a bug while testing the next PR up the stack

Whooops, thanks for finding that. Should be fixed now in f9abb1e