fix(dashboards): match the number-tile background sparkline to the displayed value#2501
Conversation
…query The big number on a number tile is a single aggregate (`convertToNumberChartConfig` drops `groupBy`), but the background sparkline kept any `groupBy` the tile carried over from a prior Line display type. It then plotted only the first group's trend behind a value that aggregates every group. Extract the sparkline's query derivation into a pure `buildSparklineTimeConfig` helper and drop `groupBy` there, mirroring the value query's strip, so the trend reflects the same single series as the value it sits behind. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 281e7d2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
review/tier-2
🔵 Tier 2 — Low RiskSmall, isolated change with no API route or data model modifications. Why this tier:
Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns. Stats
|
Greptile SummaryThis PR fixes a data-correctness bug in the
Confidence Score: 5/5Safe to merge — the change is a targeted query-shape correction with no effect on rendering, layout, or other tile types. The fix is minimal and surgical: one new helper function, a one-line replacement of the existing useMemo body, and tests that exercise both the helper in isolation and the full component pipeline. The isBuilderChartConfig guard correctly limits the delete to configs that actually carry groupBy, and the outer component already gates on the same guard so the inner path will always be a builder config in practice. No regressions to other tile types are possible. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[NumberTileBackgroundChart\nreceives config with possible residual groupBy] -->|isBuilderChartConfig check| B[NumberTileBackgroundChartInner]
B --> C["buildSparklineTimeConfig(config)"]
C --> D[Strip display-only fields\nbackgroundChart / color / colorRules / numberFormat]
D --> E[Set displayType = Line\ngranularity = config.granularity ?? 'auto']
E --> F{isBuilderChartConfig?}
F -->|yes| G[delete timeConfig.groupBy]
F -->|no| H[no groupBy to remove]
G --> I[timeConfig: single-series query shape]
H --> I
I --> J["convertToTimeChartConfig(timeConfig)"]
J --> K["useQueriedChartConfig(queriedConfig)"]
K --> L[Single-series data matches\nnumber tile aggregate value]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[NumberTileBackgroundChart\nreceives config with possible residual groupBy] -->|isBuilderChartConfig check| B[NumberTileBackgroundChartInner]
B --> C["buildSparklineTimeConfig(config)"]
C --> D[Strip display-only fields\nbackgroundChart / color / colorRules / numberFormat]
D --> E[Set displayType = Line\ngranularity = config.granularity ?? 'auto']
E --> F{isBuilderChartConfig?}
F -->|yes| G[delete timeConfig.groupBy]
F -->|no| H[no groupBy to remove]
G --> I[timeConfig: single-series query shape]
H --> I
I --> J["convertToTimeChartConfig(timeConfig)"]
J --> K["useQueriedChartConfig(queriedConfig)"]
K --> L[Single-series data matches\nnumber tile aggregate value]
Reviews (2): Last reviewed commit: "Merge branch 'main' into alex/HDX-1360-n..." | Re-trigger Greptile |
E2E Test Results✅ All tests passed • 216 passed • 3 skipped • 1544s
Tests ran across 4 shards in parallel. |
alex-fedotyev
requested review from
a team and
teeohhem
and removed request for
a team
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
|
@alex-fedotyev mind resolving the merge conflict please? Thanks! |
Resolve conflict in NumberTileBackgroundChart.test.tsx: keep the expanded import set the render and helper tests need, and adopt the path-alias import (@/components/...) that #2506 now requires in place of the relative path.
2 participants
Follow-up to #2489. The background trend sparkline that #2489 added to number tiles can plot the wrong data when a tile carries a leftover
groupBy.Summary
The big number on a number tile is a single aggregate:
convertToNumberChartConfigstripsgroupByfrom its query. The background sparkline builds its own query from the tile config but keptgroupBy, so a tile that still carried one queried multiple series and the renderer plotted only the first. The faint trend behind the value then belonged to a single group while the value itself aggregated every group.A residual
groupByis reachable in normal use: switching a grouped Line chart to the Number display type only hides the group-by input, it does not clear the value, so a saved Number tile can still carry it.The fix extracts the sparkline's query derivation into a small pure helper,
buildSparklineTimeConfig, and dropsgroupBythere, mirroring the value query's strip.granularityis still kept (defaulting toauto) so the trend stays bucketed. There is no visual or layout change: the sparkline renders exactly as before, it just plots the same single series as the value it sits behind.Changes
buildSparklineTimeConfig(config)from the inlineuseMemoinNumberTileBackgroundChartand stripgroupByalongside the existing display-only fields.groupBy, plus unit tests for the helper.Test plan
nx run @hyperdx/app:ci:lint(lint + tsc)nx run @hyperdx/app:ci:unit(2069 passed, 0 failures)NumberTileBackgroundChartwith a tile that has a residualgroupByand asserts the config passed touseQueriedChartConfigdrops it, exercising the realbuildSparklineTimeConfigthenconvertToTimeChartConfigpath rather than the helper in isolation.[ui-check: allow] Data-correctness fix: it changes which series the sparkline queries, not how anything is drawn. There is no visual, layout, color, or state surface to capture, and the new render test proves the issued query is correct.
[viewport: allow] No layout change.