feat: Show warnings for invalid promql variable usage

[pulpdrew]

Summary

This PR improves support for variables in promql by warnings to the promql input when variables are used incorrectly (eg. a non-existent variable is referenced, a macro is used, or the variable is not surrounded by quotes).

Note that this requires enabling the following feature toggles:

(In the UI)

• NEXT_PUBLIC_ENABLE_PROMQL=true
• NEXT_PUBLIC_ENABLE_DASHBOARD_VARIABLES=true

(In the API)

• ENABLE_PROMQL=true

Screenshots or video

How to test

• Create a PromQL source, the default.metrics_ts timeseries table should exist after enabling promql
• Create a dashboard and add a variable that queries instance IDs ResourceAttributes['service.instance.id'] from the metrics gauge table
• Create a promql tile that references a variable, eg. http_client_duration_milliseconds_count{instance=~"$Instance"}

References

• Linear Issue: Closes HDX-5064
• Related PRs:

[changeset-bot]

🦋 Changeset detected

Latest commit: 769ec36

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	Aug 26, 2026 6:19pm
hyperdx-storybook	Ready	Preview	Aug 26, 2026 6:19pm

[github-actions]

E2E Test Results

✅ All tests passed • 317 passed • 1 skipped • 1403s

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

Tests ran across 4 shards in parallel.

View full report →

[greptile-apps]

Greptile Summary

Adds language-aware PromQL variable validation and displays its errors and warnings in the PromQL editor.

• Warns about unknown, unquoted, and unavailable variable references.
• Rejects SQL-only variable macros in PromQL.
• Adds unit and browser coverage for the new validation behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
packages/common-utils/src/variables.ts	Adds PromQL-specific variable and macro validation while limiting existing SQL checks to SQL templates.
packages/app/src/components/PromQLEditor/PromQLEditor.tsx	Connects shared variable validation to the PromQL editor's warning, error, and issue-indicator UI.
packages/common-utils/src/tests/variables.test.ts	Covers canonical PromQL references, unknown variables, unquoted regex expansions, unsupported macros, and missing variable scope.
packages/app/tests/e2e/features/dashboard-filter-variables.spec.ts	Exercises the new PromQL validation messages and verifies that correcting a reference clears the indicator.

_{Reviews (4): Last reviewed commit: "feat: Show warnings for invalid promql v..." | Re-trigger Greptile}

[github-actions]

🟡 Tier 3 — Standard

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

Why this tier:

• 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: 2
• Production lines changed: 101 (+ 159 in test files, excluded from tier calculation)
• Branch: drew/promql-variable-warnings
• Author: pulpdrew

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]

Deep Review

✅ No critical issues found. This is an additive, well-tested change: it introduces PromQL variable-reference validation where none existed before, and cleanly gates the pre-existing SQL/Lucene checks behind language branches without regressing them. The findings below are recommendations, led by one acknowledged-but-unaddressed gap.

🟡 P2 -- recommended

• packages/common-utils/src/variables.ts:1265 -- The new PromQL check filters only on !r.inStringLiteral, which carries no matcher-operator awareness, so a regex-expanding reference in a quoted equality matcher (up{service="$service"}) is silently accepted; Prometheus matches the expanded (api|web) literally under =, returning an empty or wrong series with no indicator. This is the prior greptile P1 the author acknowledged; the current diff does not address it.
  • Fix: Record the preceding matcher operator (=/!=/=~/!~) when scanning references, and warn when a regex-format reference sits in a quoted value whose operator is = or !=, keeping the canonical =~"$service" form warning-free.
  • _{correctness, adversarial, testing, previous-comments}
• packages/common-utils/src/variables.ts:1235 -- Per-language validation is implemented as inline if (language === 'sql') / if (language === 'promql') blocks even though LanguageSettings already centralizes per-language behavior (defaultFormat, disableMacros, escapeRegexForLiteral), so adding a language means editing scattered conditionals with no compile-time exhaustiveness guard.
  • Fix: Move the per-language validation rules and messages onto LanguageSettings (e.g. a validation hook and message fields) so each language is defined in one place.
  • _{maintainability, kieran-typescript}
• packages/api/src/routers/external-api/v2/utils/dashboards.ts:1409 -- The validation is a pure exported function but is only invoked from the editor UI; validateDashboardTiles (used by the MCP saveDashboard/patchDashboard tools) never calls it, so an agent creating the same tile gets no equivalent warning a human would see.
  • Fix: Call validateVariableReferencesInTemplate per tile inside the dashboard save/patch validation path and return its errors/warnings to the agent; file as a follow-up if out of scope for this PR.
  • _agent-native

🔵 P3 nitpicks (4)

• packages/common-utils/src/variables.ts:1221 -- The macro-error message uses language === 'promql' ? … : <Lucene wording>, so any future disableMacros language would silently emit Lucene-worded text despite the generic settings.disableMacros guard.
  • Fix: Carry the macro-misuse message (or a template) on LanguageSettings rather than an else-means-Lucene assumption.
• packages/common-utils/src/variables.ts:440 -- SETTINGS_BY_LANGUAGE and languageSettings() add a Map copy plus an unreachable defensive throw over the already-exhaustive Record<TemplateLanguage, LanguageSettings>; the three near-identical names (LANGUAGE_SETTINGS, SETTINGS_BY_LANGUAGE, languageSettings) also force the reader to disambiguate the same concept. The Map is justified (it silences security/detect-object-injection), but the wrapper is heavier than needed.
  • Fix: Keep a single typed accessor and drop the redundant Map, or use a targeted lint-disable on direct Record access.
• packages/common-utils/src/__tests__/variables.test.ts:1810 -- The metric-name-placement test asserts only warnings.toHaveLength(1) rather than message content, and no PromQL test exercises multiple/mixed references (the const [{ name }] = … first-reference selection and dedup path).
  • Fix: Assert the message text for ${service}_total, and add a mixed-reference case such as up{a=~"$svc", b=~$env}.
• .changeset/ninety-rockets-rest.md:6 -- The changeset body lowercases the product term as promql.
  • Fix: Capitalize to PromQL per the repo's proper-noun casing convention.

---

Reviewers (9): correctness, adversarial, kieran-typescript, testing, maintainability, previous-comments, project-standards, agent-native, learnings-researcher.

Testing gaps:

• No unit test pins the quoted-equality case up{service="$service"} — the exact silent false-negative from the top P2, nor documents it as an intentional non-catch.
• No regression test asserting Lucene still emits its macro error and skips both the SQL and PromQL checks after the settings.disableMacros refactor.
• No PromQL coverage for combined issues in one template (unknown variable + macro + unquoted regex reference).

Dropped during re-grading: an adversarial finding that languageSettings()'s throw could crash the editor render — verified unreachable, since SearchConditionLanguage narrows to exactly TemplateLanguage via the language = 'sql' default, so no well-typed caller can trigger it.

[MikeShi42]

nit: would it make more sense to only warn here? Not sure why a user would deliberately do it, but also why not just warn in that case