Skip to content

feat: Add a line interpolation display setting (smooth, linear, step) - #3393

Open
pulpdrew wants to merge 3 commits into
mainfrom
drew/line-interpolation
Open

pulpdrew wants to merge 3 commits into
mainfrom
drew/line-interpolation

Conversation

@pulpdrew

@pulpdrew pulpdrew commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR adds a display setting for line chart interpolation mode. Smooth (default), Linear (default for stacked line charts), and step are options. This setting has been added across the UI, External API, and MCP.

This also fixes another small bug where selecting stacked bar or line chart type in the chart editor would not update the persisted tile's display type.

Screenshots or video

Screenshot 2026-10-16 at 2 14 27 PM Screenshot 2026-10-16 at 2 14 32 PM

How to test on Vercel preview

On chart explorer or a dashboard, create a line or stacked line chart, and change the interpolation setting.

References

  • Linear Issue:
  • Related PRs:

@changeset-bot

changeset-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: d0fdce2

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/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

vercel Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
hyperdx-oss Ready Ready Preview Oct 16, 2026 7:16pm UTC
hyperdx-storybook Ready Ready Preview Oct 16, 2026 7:16pm UTC

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

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 125 in total (1.69%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issues were found.

Summary

Adds smooth, linear, and step interpolation across chart settings, the External API, and MCP.

  • Line charts let authors choose how the points connect.
  • Dashboard APIs accept and return line interpolation choices.
  • The chart editor saves the display type picked in its preview.

Reviews (6) · Last reviewed commit: "style: Make Line interpolation label con..." · Reviewed by Greptile

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

E2E Test Results

✅ All tests passed • 432 passed • 1 skipped • 1651s

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

Tests ran across 4 shards in parallel.

View full report →

Comment thread packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx Outdated
@pulpdrew
pulpdrew marked this pull request as ready for review October 16, 2026 18:28
@github-actions github-actions Bot added the review/tier-4 Critical — deep review + domain expert sign-off label Oct 16, 2026
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

🔴 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:

  • Critical-path files (3) — tenancy, public API, or shipped database config:
    • packages/api/src/routers/external-api/v2/dashboards.ts
    • packages/api/src/routers/external-api/v2/utils/alertChartConfig.ts
    • packages/api/src/routers/external-api/v2/utils/dashboards.ts
  • 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: 13
  • Production lines changed: 249 (+ 326 in test files, excluded from tier calculation)
  • Critical-path lines changed: 41
  • Branch: drew/line-interpolation
  • Author: pulpdrew

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

Comment thread packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx Outdated
Comment thread packages/app/src/components/ChartDisplaySettingsDrawer.tsx Outdated
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

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.

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

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

Findings outside the changed lines

1 minor
  • 🔵 packages/app/src/components/ChartDisplaySettingsDrawer.tsx:601 — Line interpolation control is keyed on the saved display type, not the one actually drawn when compareToPreviousPeriod is on → DBTimeChart now forces DisplayType.Line when compareToPreviousPeriod is set (DBTimeChart.tsx:650), so a stacked_line tile with compare on is drawn smooth while the drawer still shows 'Linear' as checked. A stacked_bar tile with compare on is drawn as a line but has no interpolation control at all. Fix: derive showLineInterpolation and defaultLineInterpolation from the effective display type (compareToPreviousPeriod ? Line : displayType, using the drawer's live form value). Better still, put that rule in one shared helper that both DBTimeChart and the drawer call.

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

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

This is a cleanly scoped, well-engineered change. The lineInterpolation field is threaded consistently through the shared schema, form state, renderer, external v2 API (zod + OpenAPI JSDoc + generated openapi.json), and MCP tool schemas, with the internal↔external round-trip handled symmetrically for line and stacked_line (and correctly omitted for stacked_bar). Defaults are centralized in getDefaultLineInterpolation (smooth for line, linear for stacked line) and the field is optional everywhere, so there is no breaking change for existing API clients. Classifying lineInterpolation as evaluation-inert in alertChartConfig.ts is correct — it is render-only and does not affect alert evaluation.

Both prior review comments appear resolved by this diff:

  • The "chart changes skip discard warning" concern is addressed — setDisplayType now calls setValue('displayType', type, { shouldDirty: true }) in EditTimeChartForm.tsx.
  • The "compared periods get stacked together" concern is addressed by DBTimeChart.tsx moving the comparison override into the shared displayType useMemo, which forces DisplayType.Line whenever compareToPreviousPeriod is set in both controlled and uncontrolled modes.

🟡 P2 — recommended

  • packages/app/src/components/DBTimeChart.tsx:647 — The comparison-forces-Line behavior now applies in controlled (setDisplayType-provided) mode, a user-facing behavior change over the prior useEffect; confirm a test exercises "stacked type + compareToPreviousPeriod" in controlled preview mode so this does not silently regress later.
    • Fix: Add a DBTimeChart test asserting that a stacked displayType with compareToPreviousPeriod renders as a line in controlled mode.
🔵 P3 nitpicks (2)
  • packages/app/src/HDXMultiSeriesTimeChart.tsx:1441 — Selecting smooth interpolation on a stacked line re-enables the monotone-curve band overlap that the previously hardcoded linear guard avoided; the default stays linear, so this is an intentional opt-in, but there is no in-UI hint about the trade-off.
    • Fix: Consider a short helper/description note on the interpolation control that smooth stacking can cause adjacent bands to overlap.
  • packages/app/src/components/ChartDisplaySettingsDrawer.tsx:335 — The SegmentedControl onChange runs LineInterpolationSchema.safeParse on a value that can only ever be one of the control's own options, so the guard is dead defensive code.
    • Fix: Call onChange(v as LineInterpolation) directly, or drop the guard, since the options are statically typed.

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

Testing gaps: controlled-mode compareToPreviousPeriod → forced-Line behavior; editor Tabs remap preserving a stacked sub-type when re-selecting the Time Series tab.

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

@pulpdrew
pulpdrew force-pushed the drew/line-interpolation branch from 60b4e8e to 54f8b1d Compare October 16, 2026 18:40
Comment thread packages/app/src/components/ChartDisplaySettingsDrawer.tsx Outdated
Comment thread packages/app/src/components/ChartDisplaySettingsDrawer.tsx
Comment thread packages/app/src/components/DBTimeChart.tsx
Comment thread packages/api/src/routers/external-api/v2/utils/dashboards.ts

This branch was successfully deployed

2 active deployments
Preview – hyperdx-oss — d0fdce24 Deployed Oct 16, 2026 by vercel[bot]
Preview – hyperdx-storybook — d0fdce24 Deployed Oct 16, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-4 Critical — deep review + domain expert sign-off

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant