Skip to content

fix(dashboard): draw an isolated time-chart series beyond the line-render cap#2719

Open
alex-fedotyev wants to merge 1 commit into
mainfrom
alex/timechart-isolate-beyond-cap
Open

fix(dashboard): draw an isolated time-chart series beyond the line-render cap#2719
alex-fedotyev wants to merge 1 commit into
mainfrom
alex/timechart-isolate-beyond-cap

Conversation

@alex-fedotyev

Copy link
Copy Markdown
Contributor

Fix a case where isolating or filtering a dashboard time-chart series to one that ranks beyond the per-chart line-render cap left the chart empty.

Summary

getVisibleLineData applied the HARD_LINES_LIMIT cap before the legend selection, so selecting a series ranked past the cap (via isolate, checkbox filter, or table search) sliced it out and drew nothing, even though the legend and stats still listed it. Apply the selection first, then cap: an explicitly chosen series always draws, and an oversized manual selection stays bounded by the cap.

Test plan

  • eslint + tsc clean
  • unit: getVisibleLineData covers the no-selection cap, selection-beats-cap, and the oversized-selection bound (HDXMultiSeriesTimeChart.test.ts)

[ui-check: allow] Selection-ordering fix in a pure helper; no visual or theme-dependent change, and the behavior is covered by unit tests.

getVisibleLineData applied HARD_LINES_LIMIT before the legend selection, so
isolating or filtering to a series ranked past the cap sliced it out and left
the chart empty while the legend and stats still listed it. Apply the
selection first, then cap, so a chosen series always draws; an oversized
manual selection is still bounded by the cap. Update the unit test that
pinned the old drop-if-over-limit behavior and add selection-beats-cap and
oversized-selection cases.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 23, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Jul 23, 2026 4:10pm
hyperdx-storybook Ready Ready Preview, Comment Jul 23, 2026 4:10pm

Request Review

@github-actions github-actions Bot added the review/tier-2 Low risk — AI review + quick human skim label Jul 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats
  • Production files changed: 1
  • Production lines changed: 23 (+ 31 in test files, excluded from tier calculation)
  • Branch: alex/timechart-isolate-beyond-cap
  • Author: alex-fedotyev

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

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes rendering of explicitly selected time-chart series beyond the normal line cap.

  • Filters by the active legend selection before applying HARD_LINES_LIMIT.
  • Adds unit coverage for uncapped selection priority and oversized-selection bounding.
  • Adds a patch changeset for @hyperdx/app.

Confidence Score: 4/5

The oversized-selection axis mismatch should be fixed before merging because invisible capped-out series can distort the displayed chart.

Rendering now caps the filtered selection, but axis bounds continue to scan the uncapped selected data, allowing values from lines that are not drawn to determine the visible scale.

packages/app/src/HDXMultiSeriesTimeChart.tsx

Important Files Changed

Filename Overview
packages/app/src/HDXMultiSeriesTimeChart.tsx Reorders selection and capping correctly for isolated series, but leaves axis-domain calculation inconsistent with the bounded rendered set.
packages/app/src/tests/HDXMultiSeriesTimeChart.test.ts Covers the helper’s new ordering and cap behavior, but not axis bounds for oversized selections.
.changeset/timechart-isolate-beyond-cap.md Accurately documents the isolated-series rendering fix and patch release.

Fix All in Claude Code Fix All in Conductor Fix All in Cursor Fix All in Codex

Reviews (1): Last reviewed commit: "fix(dashboard): draw an isolated series ..." | Re-trigger Greptile

Comment on lines +520 to +522
return lineData
.filter(ld => selectedSeriesNames.has(getSeriesDisplayName(ld)))
.slice(0, HARD_LINES_LIMIT);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Capped lines still affect axis

When more than HARD_LINES_LIMIT series are selected and a capped-out series has an extreme value, rendering excludes that series but the domain calculation still scans it, causing invisible data to expand the y-axis and compress the visible lines.

Knowledge Base Used: App Components and Charts

Fix in Claude Code Fix in Conductor Fix in Cursor Fix in Codex

@github-actions

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

The change reorders getVisibleLineData to apply the legend/isolate/search selection before the HARD_LINES_LIMIT cap, so an explicitly chosen series that ranks past the cap still draws while an oversized manual selection stays bounded. The logic is a small pure helper, correct across the no-selection, empty-selection, selection-beats-cap, and oversized-selection paths, and the common in-range selection case is behaviorally unchanged. Display-name matching semantics are untouched by this diff. Unit coverage in HDXMultiSeriesTimeChart.test.ts exercises each behavioral branch.

🔵 P3 nitpicks (1)
  • packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts:124 — coverage pins the single-beyond-limit and select-everything extremes but not a mixed selection combining in-range and beyond-cap series whose total stays under the cap.
    • Fix: add a case selecting one in-range and one beyond-cap series and assert both return in lineData order.

Reviewers (1): direct review across correctness, testing, maintainability, project-standards, performance, and TypeScript lenses.

Testing gaps:

  • The multi-agent reviewer fan-out could not run: the sandboxed shell is non-functional (bwrap: Can't create file at /home/.mcp.json on every command, including echo), so git diff/git show and the per-persona sub-agents were unavailable. Findings above come from a direct read of the changed source and tests, not the standard git-scoped persona pipeline.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 241 passed • 1 skipped • 990s

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

Tests ran across 4 shards in parallel.

View full report →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-2 Low risk — AI review + quick human skim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant