feat: Auto-convert builder charts to Raw SQL

[pulpdrew]

Summary

This PR adds automatic conversion from builder charts to sql charts when switching from builder mode to sql mode, so that users don't have to start with an empty SQL template.

The sql template is only overwritten when the chart was edited in builder mode more recently than in SQL mode.

This PR does not implement SQL --> Builder conversion (I'd like to do that in a followup).

Screenshots or video

Screen.Recording.2026-07-13.at.11.39.04.AM.mov

How to test on Vercel preview

This can be tested in the preview in Chart Explorer or in Dashboards.

References

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

[changeset-bot]

🦋 Changeset detected

Latest commit: 2149d8e

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	Minor
@hyperdx/app	Minor
@hyperdx/api	Minor
@hyperdx/otel-collector	Minor

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 13, 2026 4:03pm
hyperdx-storybook	Ready	Preview, Comment	Jul 13, 2026 4:03pm

[greptile-apps]

Greptile Summary

This PR automatically carries Builder charts into SQL mode. The main changes are:

• Macro-based SQL generation for supported chart types.
• Edit-order tracking to protect newer hand-written SQL.
• Shared chart conversion helpers and SQL formatter updates.
• Unit and end-to-end coverage for conversion workflows.

Confidence Score: 4/5

This is close, but the pending-source workflow should be fixed before merging.

• Metric filters now stay in the correct source scope.
• Normal conversion and edit-preservation paths have focused coverage.
• A mode switch made before source resolution still leaves the SQL editor empty or stale.

packages/app/src/components/DBEditTimeChartForm/useBuilderToSqlConversion.ts

Important Files Changed

Filename	Overview
packages/app/src/components/DBEditTimeChartForm/useBuilderToSqlConversion.ts	Adds conversion and edit-order tracking, but drops conversion when the source resolves after the mode switch.
packages/common-utils/src/core/builderToRawSql.ts	Adds macro-based SQL rendering and structured conversion errors.
packages/common-utils/src/core/renderChartConfig.ts	Keeps dashboard filters in the real metric source scope rather than outer CTE queries.
packages/app/tests/e2e/features/chart-explorer.spec.ts	Covers conversion and edit preservation, but not switching modes before source resolution.

_{Reviews (2): Last reviewed commit: "fix: Prevent extra in Metrics SQL" | Re-trigger Greptile}

[github-actions]

E2E Test Results

✅ All tests passed • 240 passed • 3 skipped • 1442s

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

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

🟡 Tier 3 — Standard

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

Why this tier:

• Diff size: 577 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: 8
• Production lines changed: 577 (+ 1799 in test files, excluded from tier calculation)
• Branch: drew/builder-sql-convert
• 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. No P0/P1 ship-blockers: no data-persistence loss, auth bypass, or injection reachable across a privilege boundary, and the happy path is sound. The findings below are correctness/reliability sharp edges and coverage gaps worth addressing before or shortly after merge.

🟡 P2 — recommended

• packages/common-utils/src/sqlFormatter.ts:37 — The space-collapsing regexes run over the entire formatted string with no string-literal awareness, so a filter value such as 'errorMap (x)' (inlined via SqlString.escape before format()) is silently rewritten to 'errorMap(x)', altering the saved-and-executed SQL and any SQL rendered elsewhere in the app.
  • Fix: Mask single/double-quoted literals and backtick identifiers before applying the combinator/$__ replacements, then restore them, and add tests asserting literal contents are preserved verbatim.
  • _{correctness, security, testing}
• packages/app/src/components/DBEditTimeChartForm/useBuilderToSqlConversion.ts:113 — Conversion is edge-triggered on the exact builder→SQL transition; if the async tableSource is still undefined at that instant the effect shows a "requires a source" error and returns, and because usePrevious has already advanced to sql it never retries when the source resolves.
  • Fix: Track a pending-conversion ref (or gate the transition as consumed only once tableSource is non-null) so a late source load still triggers generation.
  • _{julik-frontend-races, reliability}
• packages/app/src/components/DBEditTimeChartForm/useBuilderToSqlConversion.ts:167 — applyResult guards only on requestId, configType, and last-edited mode, not on source identity or component liveness, so an in-flight generation can write a template built against a since-changed tableSource, or call setValue/notifications.show after the editor has unmounted.
  • Fix: Capture the source identity at generation time and re-check it in applyResult, and return an effect cleanup that cancels the pending resolution on unmount.
  • _{julik-frontend-races, adversarial}
• packages/common-utils/src/core/renderChartConfig.ts:806 — isRenderingRawSqlTemplate is hand-threaded through ~9 render helper call sites; a missed site silently emits concrete SQL instead of a macro template rather than failing, and a prior follow-up commit already had to patch one omitted site.
  • Fix: Thread a small render-context object (or the relevant chartConfig slice) through these helpers so the flag is carried implicitly.
  • _{maintainability, kieran-typescript}
• packages/common-utils/src/core/builderToRawSql.ts:168 — renderBuilderConfigAsSqlTemplate returns a {isError} discriminated union but also throws on unbound/unsubstituted params, so its Promise<RenderedSqlTemplate> type hides a rejection path a future caller could miss.
  • Fix: Fold the invariant failures into the error branch, or make the rejection contract explicit in the type/docs.
  • _{kieran-typescript, reliability, testing}
• packages/app/src/components/DBEditTimeChartForm/useBuilderToSqlConversion.ts:80 — The hook's async orchestration (request-ID supersession, hand-edit recheck, tableSource race), inlineParams escaping with adversarial input, and the new sqlFormatter regexes' false-positive/literal behavior are all untested; only the pure classifyFormEdit helper has coverage.
  • Fix: Add renderHook-level tests for the race/guard paths plus unit tests for identifier/string escaping and formatter false positives.
  • _{testing, kieran-typescript, julik-frontend-races, reliability, correctness, security}

🔵 P3 nitpicks (10)

• packages/common-utils/src/core/builderToRawSql.ts:43 — Identifier escaping replaces backticks but not backslashes, so a value ending in \ (e.g. a with[].name CTE name) can break out of the backtick quoting; impact is limited to self-authored, self-executed SQL.
  • Fix: Escape backslash before backtick in the Identifier branch.
• packages/common-utils/src/core/builderToRawSql.ts:33 — inlineParams types params as Record<string, any>, disabling type checking though every use works with unknown.
  • Fix: Change the type to Record<string, unknown>.
• packages/app/src/components/DBEditTimeChartForm/useBuilderToSqlConversion.ts:182 — The .catch calls showError unconditionally, so a superseded or mode-changed generation that later rejects still pops a red error toast.
  • Fix: Apply the same requestId/configType guard used in applyResult before showing the error.
• packages/app/src/components/DBEditTimeChartForm/useBuilderToSqlConversion.ts:172 — The success notification omits a stable id (unlike the error notification), so repeated conversions stack duplicate green toasts.
  • Fix: Give the success notification a fixed id.
• packages/app/src/components/DBEditTimeChartForm/useBuilderToSqlConversion.ts:131 — Hand-written SQL is overwritten with no confirmation or undo when any builder field is edited after the SQL edit and the user switches back to SQL.
  • Fix: Prompt for confirmation (or preserve/diff) before regenerating over SQL that was hand-edited later than the last builder edit.
• packages/common-utils/src/core/builderToRawSql.ts:160 — A user-set fixed granularity may not survive conversion because template mode always emits $__timeInterval, which resolves to the auto/dashboard interval; verify against a line chart with an explicit granularity.
  • Fix: Preserve the explicit granularity in the generated template, or confirm the SQL-mode interval is re-derived from it.
• packages/app/src/components/DBEditTimeChartForm/useBuilderToSqlConversion.ts:88 — lastEditedModeRef and requestIdRef are never reset when the mounted form is reused for a different chart via a values reset (no remount), so edit history can carry over.
  • Fix: Reset both refs (and cancel any in-flight request) in an effect keyed on a stable chart identity.
• packages/common-utils/src/sqlFormatter.ts:4 — The combinator suffix list is duplicated across two regexes, and RAW_SQL_DISPLAY_TYPES / the databaseName === '' CTE sentinel are each restated in multiple places, inviting drift.
  • Fix: Extract the suffix alternation, the raw-SQL-display-type predicate, and the CTE check into single shared definitions.
• packages/app/src/components/DBEditTimeChartForm/__tests__/useBuilderToSqlTemplate.test.ts:1 — The test file name does not match its source module useBuilderToSqlConversion.ts, breaking the directory's 1:1 test-to-source naming convention.
  • Fix: Rename to useBuilderToSqlConversion.test.ts.
• packages/common-utils/src/core/builderToRawSql.ts:80 — The conversion is UI-only; no MCP/agent tool exposes renderBuilderConfigAsSqlTemplate, so agents cannot perform the builder→SQL conversion users get.
  • Fix: Consider a small MCP tool (or extending the dashboard tile tools) that reuses this function.

---

Reviewers (11): correctness, security, adversarial, julik-frontend-races, kieran-typescript, reliability, testing, maintainability, project-standards, agent-native, learnings-researcher.

Testing gaps: No renderHook-level coverage of the conversion effect (request-ID supersession, unmount-in-flight, tableSource-late/source-swap races); inlineParams escaping never exercised with backticks/quotes/injection-shaped values; new sqlFormatter combinator/parametric/macro regexes lack false-positive and string-literal-preservation tests; renderBuilderConfigAsSqlTemplate throw paths and convertToNumber/TableChartConfig (moved to common-utils) are untested at their new location.