[teeohhem]

Summary

PromQL in MCP runs on three backends (a Prometheus endpoint, ClickHouse's Prometheus HTTP API, and the ClickHouse table function), but the existing tests mock Prometheus. This adds tests that run real queries, so a backend that drifts or a broken image shows up in CI.

Integration tests (packages/api)

• docker-compose.ci.yml gets a pinned Prometheus that remote-reads default.metrics_ts from ClickHouse. All three backends then read the same seeded samples.
• promqlBackends.int.test.ts saves 14 PromQL tile types through clickstack_save_dashboard. It checks that every backend returns the same results for each tile, for clickstack_promql (range and instant), and for a dashboard variable. It also asserts known values, and checks from the outgoing requests that each run really used the backend it's named for.
• promqlLargeResults.int.test.ts returns 12,000 series and checks:
  • the 10,000-row table cap;
  • limit=10000 sent on table instant queries only;
  • the 50,000-char response budget.
• Known bug: line, number, pie and bar tiles with many series go over the budget (about 1.3M chars) because seriesLabels isn't trimmed. Those cases are it.failing, so they flag themselves once it's fixed.
• seedTimeSeriesTagsTable accepts per-series samples.
• New slot port HDX_CI_PROMETHEUS_PORT (19100 + slot).

Image smoke (scripts/ci/smoke-all-in-one.sh --promql-mcp)

The new mode does this on the auth all-in-one image:

1. Boot with ENABLE_PROMQL=true and an INGESTION_API_KEY.
2. Send one OTLP gauge and wait until it's remote-written into metrics_ts.
3. Register a user and add a PromQL source.
4. Query the gauge over /api/mcp with clickstack_promql and a saved PromQL tile (clickstack_query_tiles must report ok with data).

docker-build.yml now loads the auth image and runs this step. The smoke script was added to the job's path filter, so this PR runs it.

What I could and couldn't verify locally

• The integration suites pass (10 tests, the 4 it.failing included), and six neighbouring PromQL suites still pass (196 tests).
• I couldn't build the all-in-one image locally: Next.js timed out generating static pages on a shared Docker VM. I ran the script's PromQL section as-is against the dev stack instead. It passes, fails on a missing metric, and reports tool errors.
• Not yet checked in the image itself: ENABLE_PROMQL and INGESTION_API_KEY wiring. This PR's "Build All-in-One Image" job is the first real run.

How to reproduce

Starting point: a local checkout with Docker.

On this branch:

1. make dev-int-build, then make dev-int FILE="promqlBackends promqlLargeResults". All 10 tests pass.
2. Optional: build the auth image (docker buildx build -f docker/hyperdx/Dockerfile --target all-in-one-auth --load -t aio:local ... with the build contexts from docker-build.yml), then run scripts/ci/smoke-all-in-one.sh aio:local --promql-mcp.

How to test on Vercel preview

N/A — non-UI change.

References

• Linear Issue: HDX-5302
• Related PRs: feat: Run PromQL tiles from MCP query_tile #3528, refactor: Share PromQL chart planning and rows between app and MCP #3529, feat: Add the clickstack_promql MCP tool and PromQL prompt docs #3531, fix: refuse Prometheus endpoint responses over 8 MB #3541

🤖 Generated with Claude Code

[changeset-bot]

⚠️ No Changeset found

Latest commit: 147770c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a 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 7:47pm UTC
hyperdx-storybook	Ready	Preview	Nov 9, 2026 7:47pm UTC

[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: 7
• Production lines changed: 165 (+ 582 in test files, excluded from tier calculation)
• Branch: claude/mcp-promql-smoke-tests
• Author: teeohhem

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]

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]

🔵 minor — mcp() doesn't treat JSON-RPC protocol errors as failures

For a JSON-RPC error response (unknown tool, invalid request, auth error from the transport), .result.isError // false evaluates to false. The check passes and mcp prints .error.message as if it were the tool result. The run only fails later, when the caller's jq '.result.data | length' can't parse that text. That failure has no ::error:: annotation and no container logs. Fix: also call fail when .error is present, e.g. jq -e '(.error == null) and ((.result.isError // false) == false)'.

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

[github-actions]

🔵 minor — The PromQL/MCP smoke never runs on PRs that change the MCP/PromQL code it tests

This step tests API behaviour (/api/mcp, clickstack_promql, PromQL tiles). The job's PR path filter deliberately leaves out packages/api/**, on the grounds that those changes "can't break the bundling layers". That reasoning holds for the old canary smoke but not for this one. A PR that breaks PromQL over MCP passes PR CI and only fails on the push to main. Fix: run this smoke in a job that triggers on packages/api/**, or document in the workflow that it only gates main.

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

[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

2 posted as inline comment(s) on the changed lines. 1 listed below.

Findings outside the changed lines

1 minor

• 🔵 packages/api/src/mcp/__tests__/promqlBackends.int.test.ts:527 — Cross-backend equality test passes when every backend returns zero rows → toMatchObject({ rows: expect.any(Number) }) also accepts rows: 0. Eight of the 14 tiles ('line with two expressions', 'line, unaligned', 'stacked bar', 'number, mean/default reducer', 'pie by job', 'bar by instance, instant', 'table, range') have no other value check. If the shared MCP PromQL path drops the data for a display type, all three backends return {rows:0,data:[]}, the results still match, and the test passes. Fix: assert rows > 0 (or data.length > 0) on the Prometheus result for every tile before comparing the backends.

---

_{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 test- and CI-only change (new PromQL integration tests, a CI Prometheus service that remote-reads ClickHouse, a new worktree slot port, a fixture extension for per-series samples, and a --promql-mcp smoke mode). No production application code is modified. The new cross-backend test harness is strong: onBackend asserts, from the captured fetch calls, that each named backend actually routed its PromQL queries where expected, and the table-function path asserts no Prometheus HTTP call was made. Port ranges do not collide (HDX_CI_API_PORT 19000–19099 vs HDX_CI_PROMETHEUS_PORT 19100–19199 across slots 0–99). The findings below are minor robustness and maintainability notes; none block merge.

🟡 P2 -- recommended

• packages/api/src/mcp/__tests__/promqlLargeResults.int.test.ts:922 -- the 10,000-row cap, limit=10000, and 50,000-char budget are only exercised against the mocked Prometheus-endpoint backend, so a trimming regression on the ClickHouse-HTTP or table-function response paths would not be caught.
  • Fix: Parameterize the large-result cases across all three backends, or add a short note documenting why endpoint-only coverage is sufficient here.
  • _{testing, maintainability}

🔵 P3 nitpicks (4)

• scripts/ci/smoke-all-in-one.sh:1118 -- mcp() only inspects .result.isError, so a JSON-RPC-level error response (where .result is absent) is printed as if it were tool output; the release gate currently catches this only via each caller's downstream rows/hasData check and set -e.
  • Fix: In mcp(), treat a present top-level .error or an absent .result as a failure, independent of .result.isError.
• scripts/ci/smoke-all-in-one.sh:1075 -- the login/password POST omits -f, so an HTTP error on login is not surfaced there and is only detected by the subsequent -f call to /me.
  • Fix: Add -f (or --fail-with-body) to the login curl for consistent fail-fast behavior.
• packages/api/src/fixtures.ts:404 -- when a caller passes per-series samples but omits withSamples: true, the samples are silently ignored while the all-or-none guard still runs, which is a quiet footgun.
  • Fix: Throw or auto-enable sample writing when any series supplies samples, so the two flags cannot disagree silently.
• docker-compose.ci.yml:220 -- prometheus uses depends_on: ch-server without a readiness condition, so it can start before ClickHouse's remote-read handler is serving.
  • Fix: Rely on the existing per-test seeding/retries (already the case) or add a condition: service_healthy with a ClickHouse healthcheck to make the dependency explicit.

---

Reviewers (8): correctness, testing, maintainability, project-standards, kieran-typescript, reliability, adversarial, previous-comments.

Testing gaps:

• The it.failing cases document a real, unfixed MCP-response-budget overflow (multi-series line/number/pie/bar tiles exceed the 50,000-char budget because seriesLabels is not trimmed alongside data); this is pre-existing product behavior surfaced — not introduced — by this diff, and should be tracked as a follow-up so the markers convert to passing it once the trimmer is fixed.
• Prior PR comments were automated only (changeset-bot, vercel-bot); there was no human review feedback to verify. No changeset is included, which is acceptable for a test/CI-only change that ships no runtime behavior.

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

E2E Test Results

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

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

Tests ran across 4 shards in parallel.

View full report →