fix(app): pulse the list-bar and histogram charts while they refresh by Harshul1484 · Pull Request #3229 · hyperdxio/hyperdx

Harshul1484 · 2026-09-25T22:07:29Z

Summary

A follow-up to #3205, for the Services dashboard. Two chart types there already keep their previous data during a refresh (placeholderData: prev => prev) but give no sign that new data is loading: the "Top 20 Most Time Consuming Endpoints/Queries" list-bar charts and the "Request Latency" histogram. They now pulse while the refetch runs, like the categorical charts fixed in #3211.

DBListBarChart pulses the bar list, and its empty state, while isPlaceholderData is true.
DBHistogramChart does the same through a new className prop on HistogramChart, which is applied to its ResponsiveContainer.
DBHistogramChart's loading state now shows only before the first data arrives (isLoading && !data), as DBListBarChart and DBBarChart already do. useQueriedChartConfig also reports isLoading while the materialized-view check for the new range runs, which briefly replaced the kept histogram with "Loading Chart Data...".
DBListBarChart is also used by the LLM token-cost charts and the endpoint side panel's performance chart, which now pulse the same way.

Tests

DBListBarChart.test.tsx:
- the bars pulse during a refresh and stop once fresh data loads;
- the empty state pulses during a refresh, but not for a fresh empty result.
DBHistogramChart.test.tsx:
- the histogram stays on screen and pulses while a refresh loads, and stops once fresh data loads;
- the empty state pulses during a refresh, but not for a fresh empty result;
- the loading state still shows before the first data.
Without the source changes, 4 of the 9 new tests fail. The ones that pass either way are the guards: no pulse once fresh, and the loading state before the first data.

How to test on Vercel preview

Preview routes: /services

Steps:

Open the Services dashboard on the demo trace source. On the "HTTP Service" tab, switch "Request Latency" to "Display as Histogram".
Click "Refresh dashboard".
Verify the "Request Latency" histogram and the "Top 20 Most Time Consuming Endpoints" list keep their data on screen and pulse while they reload, and stop pulsing once the new data appears.

References

Checks run locally:

jest --findRelatedTests for both charts: 33 passed across 4 suites.
eslint on the changed files: 0 errors, no new warnings.
prettier --check.
tsc --noEmit for packages/app.

changeset-bot · 2026-09-25T22:07:34Z

🦋 Changeset detected

Latest commit: 8e7222e

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

This PR includes changesets to release 3 packages

Name	Type
@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 · 2026-09-25T22:07:35Z

@Harshul1484 is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

github-actions · 2026-09-25T22:09:12Z

PR Review

✅ No issues found.

greptile-apps · 2026-09-25T22:10:24Z

Confidence Score: 4/5

[Low risk] Adds visual feedback while charts refresh.

The refresh behavior appears sound, but the explicit empty-state and component-size requirements should be satisfied before merging.

Findings

Ad-hoc empty states ▶
Component files exceed size limit ▶

Summary

The PR keeps list-bar and histogram results visible and pulses them during refresh, with tests for loading and empty states.

The newly added tests check that fresh empty results do not pulse.
The empty-state implementation still needs to follow the repository’s component requirement.

_{Reviews (2) · Last reviewed commit: "test(app): check a fresh empty chart res..."}

github-actions · 2026-09-25T22:14:03Z

Deep Review

✅ No critical issues found.

This is a small, well-scoped follow-up that adds effect-pulse refresh feedback to DBListBarChart and DBHistogramChart, keyed off react-query's isPlaceholderData, with matching tests. No correctness, security, data, or contract regressions were identified in the diff. The state transitions are coherent: when isLoading is true with placeholder data present, react-query reports isPlaceholderData true, so the kept chart both renders (via the new isLoading && !data gate) and pulses.

🔵 P3 nitpicks (3)

packages/app/src/components/DBHistogramChart.tsx:1 — the file is now 308 lines and DBListBarChart.tsx is 309, both over the AGENTS.md "keep component files under 300 lines" guideline; this repeats a prior review comment on the PR that remains unaddressed.
- Fix: extract the inner HistogramChart/ListBar presentational components (or the tooltip) into sibling files to bring both back under the guideline, or confirm the guideline is advisory for a marginal overage.
packages/app/src/components/DBHistogramChart.tsx:303 — the chart pulse is keyed on isPlaceholderData alone, whereas sibling charts (DBBarChart, DBPieChart, DBNumberChart) key it on isLoading || isPlaceholderData; the behavior is equivalent in practice here but the divergence is a consistency snag for future readers.
- Fix: align on isLoading || isPlaceholderData for the chart pulse to match the established pattern, or leave a short comment explaining why isPlaceholderData alone is sufficient given the new isLoading && !data gate.
packages/app/src/components/__tests__/DBHistogramChart.test.tsx:161 — the refresh tests assert against document.querySelector('.recharts-responsive-container'), coupling the test to a Recharts-internal DOM class that could change on a library upgrade.
- Fix: accept the coupling as low-risk, or target a stable app-owned wrapper/data-testid instead of the library class.

Reviewers (7): correctness, testing, maintainability, project-standards, kieran-typescript, julik-frontend-races, previous-comments.

Testing gaps: No test covers the list-bar chart pulsing while isLoading is true with kept data (only the isPlaceholderData-true path is asserted); the histogram covers this case but the list-bar does not.

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

greptile-apps · 2026-09-25T22:26:05Z

+        <div
+          className={cx(
+            'd-flex h-100 w-100 align-items-center justify-content-center text-muted',
+            { 'effect-pulse': isPlaceholderData },
+          )}


Ad-hoc empty states The updated empty-result branches in this chart and DBListBarChart still render inline <div> elements. The repository guide requires EmptyState for all empty/no-data states. Please use it in both charts while preserving the refresh pulse behavior; this requirement must be satisfied before merging.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Every chart component (DBBarChart, DBPieChart, DBNumberChart, DBTableChart, DBTimeChart and these two) renders the same inline "No data found within time range." block today, so switching them to EmptyState is a separate, codebase-wide change; this PR only adds the pulse class to the existing element.

You're right: the inline empty state predates this PR and is shared consistently across the chart components. Requiring EmptyState only in these two charts would expand the change and make the implementation inconsistent, so this is not a blocking issue for this PR. I'll withdraw the finding; a codebase-wide empty-state migration can be handled separately.

fix(app): pulse the list-bar and histogram charts while they refresh

bc3e38e

github-actions Bot added the external label Sep 25, 2026

Harshul1484 mentioned this pull request Sep 25, 2026

Dashboard tiles other than line/bar time charts show no sign they are refreshing #3205

Open

greptile-apps Bot reviewed Sep 25, 2026

View reviewed changes

Comment thread packages/app/src/components/DBHistogramChart.tsx

test(app): check a fresh empty chart result doesn't pulse

8e7222e

greptile-apps Bot reviewed Sep 25, 2026

View reviewed changes

Navigation Menu

fix(app): pulse the list-bar and histogram charts while they refresh - #3229

fix(app): pulse the list-bar and histogram charts while they refresh#3229
Harshul1484 wants to merge 2 commits into
hyperdxio:mainfrom
Harshul1484:fix/services-tiles-refresh-pulse

Harshul1484 commented Sep 25, 2026 •

edited

Loading

Uh oh!

changeset-bot Bot commented Sep 25, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Sep 25, 2026

Uh oh!

github-actions Bot commented Sep 25, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Sep 25, 2026 •

edited

Loading

Uh oh!

Uh oh!

github-actions Bot commented Sep 25, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Sep 25, 2026

Uh oh!

Harshul1484 Sep 26, 2026

Uh oh!

greptile-apps Bot Sep 26, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Harshul1484 commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How to test on Vercel preview

References

Uh oh!

changeset-bot Bot commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

vercel Bot commented Sep 25, 2026

Uh oh!

github-actions Bot commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review

Uh oh!

greptile-apps Bot commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 4/5

Findings

Summary

Uh oh!

Uh oh!

github-actions Bot commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deep Review

Uh oh!

greptile-apps Bot Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

Harshul1484 Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Harshul1484 commented Sep 25, 2026 •

edited

Loading

changeset-bot Bot commented Sep 25, 2026 •

edited

Loading

github-actions Bot commented Sep 25, 2026 •

edited

Loading

greptile-apps Bot commented Sep 25, 2026 •

edited

Loading

github-actions Bot commented Sep 25, 2026 •

edited

Loading