test(dashboards): cover release-marker scoping by the tile's own filter by teeohhem · Pull Request #2905 · hyperdxio/hyperdx

teeohhem · 2026-08-13T21:10:13Z

Test-only. Adds the E2E coverage that should have caught the second bug in #2894, now that #2894 has merged.

Why

A review comment on #2894 pointed out that the release-marker scope read chart.config.where, but timeseries charts don't use it — they keep their filter in each series' aggCondition. That was right, and the consequence was worse than a wasted query: the markers came back spanning every service in the source, none matched a line on an ungrouped chart, and resolveAnnotationSeries dropped all of them. Filter a tile to one service and you saw no markers.

The E2E suite was green through all of it. Its scoping test uses setGlobalFilter, a dashboard filter, which travels as filters and was already applied. Nothing exercised the tile's own filter — the field users actually type into.

The fix landed on #2894 with unit coverage only. This closes the loop at the level that would have prevented the mistake.

What

One test that filters only the tile, no dashboard filter, and asserts the filtered service's two releases are drawn and the other service's is not.

ChartEditorComponent.setSeriesWhere reaches a series' Where. It switches to Lucene explicitly for two reasons: the mode is sticky in localStorage, so a previous spec can leave it on SQL, and series-where-input only exists on the Lucene branch — SearchWhereInput's SQL branch renders a CodeMirror that carries no test id. Lookups are scoped to the tile-editor modal, since the dashboard behind it renders the same markup.

addTileWithSource grows an optional third argument. Its two other callers (dashboard-template-import.spec.ts, lucene-autocomplete.spec.ts) pass two, so their behaviour is byte-identical.

No app-code changes

Three files, all under packages/app/tests/e2e/.

An earlier revision of this branch added a data-testid="series-where" wrapper, because SearchWhereInput forwards a test id only on its Lucene branch. #2902 has since landed data-testid="series-where-input", a where-language-switch test id and a switchWhereToLucene helper, so the hook is no longer needed and the helper is built on those instead.

Verification

Re-proved it's a real regression test, not a tautology. With aggConditionScopeFilter temporarily stubbed to return undefined — reproducing the pre-fix behaviour of reading only the statement-level where — the new test fails as:

Expected: 2
Received: 0
  9 × locator resolved to 0 elements
  at expect(dashboardPage.getAnnotationMarkers()).toHaveCount(2)
     release-markers.spec.ts:170

which is exactly the user-visible symptom. Stub reverted; suite green.

make dev-e2e FILE=release-markers — 4/4 passing.
make dev-e2e FILE=dashboard-template-import — 11/11 passing (other addTileWithSource caller).
make dev-e2e FILE=lucene-autocomplete — 4/4 passing (owns the test ids and helper this reuses).
make ci-lint — 0 errors, 590 warnings, unchanged.

No changeset: test-only, and #2894 already carries the feature's.

🤖 Generated with Claude Code

changeset-bot · 2026-08-13T21:10:18Z

⚠️ No Changeset found

Latest commit: d4d0403

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel · 2026-08-13T21:10:20Z

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

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview	Aug 14, 2026 7:20pm
hyperdx-storybook	Ready	Preview	Aug 14, 2026 7:20pm

greptile-apps · 2026-08-13T21:12:53Z

Greptile Summary

The PR adds dashboard E2E coverage for release-marker scoping through a tile’s series-level filter.

Adds a page-object helper that switches the series WHERE editor to Lucene and fills its condition.
Extends tile creation to accept an optional series filter.
Verifies that only releases belonging to the tile-filtered service are rendered.

Confidence Score: 3/5

The PR is not yet safe to merge because the previously reported delimiter-based series mismatch and kind-biased annotation cap remain unresolved.

Series matching still splits keys on a delimiter that may occur inside service names, and annotation kinds are still concatenated before the global cap, allowing one kind to hide another.

Files Needing Attention: packages/app/src/ChartUtils.tsx; packages/app/src/components/charts/chartAnnotations.tsx

Important Files Changed

Filename	Overview
packages/app/tests/e2e/components/ChartEditorComponent.ts	Adds a modal-scoped helper for setting the series-level WHERE condition in Lucene mode.
packages/app/tests/e2e/features/release-markers.spec.ts	Adds regression coverage asserting release markers honor the tile’s own service filter.
packages/app/tests/e2e/page-objects/DashboardPage.ts	Allows tile creation to set an optional series-level filter before running and saving the chart.

_{Reviews (2): Last reviewed commit: "test(dashboards): cover release-marker s..." | Re-trigger Greptile}

github-actions · 2026-08-13T21:18:16Z

Deep Review

✅ No critical issues found.

This is a test-only diff: one new E2E spec plus supporting page-object infrastructure (setSeriesWhere, a tileEditor locator getter, and an optional seriesWhere argument on addTileWithSource). The app-code testids it drives (series-where-input, where-language-switch, chart-name-input) already exist in the base. Reviewers independently confirmed the locator scoping is unambiguous, the assertions form a genuine (non-tautological) regression guard, and the no-changeset claim is correct per the repo's AGENTS.md test exemption. No P0/P1/P2 issues.

🔵 P3 nitpicks (4)

packages/app/tests/e2e/components/ChartEditorComponent.ts:126 — setSeriesWhere only ever switches to Lucene, so a tile filter stored as a SQL aggCondition has no E2E coverage; a regression on the SQL branch would go undetected.
- Fix: Add a sibling assertion or helper path that exercises the SQL aggCondition branch, or note the Lucene-only scope explicitly.
- _correctness
packages/app/tests/e2e/features/release-markers.spec.ts:174 — the count assertion cannot fully distinguish "tile filter applied" from "markers coincidentally suppressed," since both yield the same marker count in some states.
- Fix: Add an assertion that the tile's Lucene filter round-trips (e.g. reopen the tile editor and confirm the filter persists) to prove the filter drove the result.
- _correctness
packages/app/tests/e2e/components/ChartEditorComponent.ts:109 — setSeriesWhere/tileEditor have no fast-fail guard when invoked outside a dashboard tile-editor context; the getter resolves to an empty locator and the call hangs to the default timeout instead of erroring clearly.
- Fix: Assert the tile-editor dialog is visible at the top of setSeriesWhere so misuse fails fast with a readable message.
- _{maintainability}
packages/app/tests/e2e/page-objects/DashboardPage.ts:597 — the third optional positional seriesWhere argument diverges from the destructured-options pattern used by sibling helpers and will not scale if more tile-level fields are added later.
- Fix: Consider an options-object signature if additional optional tile fields are anticipated; fine as-is for a single param.
- _{maintainability}

Reviewers (5): correctness, testing, maintainability, kieran-typescript, project-standards.

Testing gaps:

Tile filters stored as a SQL aggCondition are not exercised — the helper switches to Lucene unconditionally.
The new spec verifies marker count/labels but not that the tile's filter round-trips, so "filter applied" and "markers suppressed" are not fully disambiguated.

The existing spec scoped its chart with a dashboard-wide filter, which travels as `filters` and was already applied to the releases query — so it stayed green while the tile's *own* filter was ignored entirely. A time chart keeps that filter in its series' `aggCondition`, and nothing exercised it. What slipped through was worse than a wasted query: the markers came back spanning every service in the source, none matched a line on an ungrouped chart, and `resolveAnnotationSeries` dropped all of them. Filter a tile to one service and you saw no markers at all. Add a test that filters only the tile, asserting the filtered service's two releases are drawn and the other service's is not. `setSeriesWhere` needs no app-code hook: it reuses the `series-where-input` and `where-language-switch` test ids and the `switchWhereToLucene` helper. It sets the language explicitly because the mode is sticky in localStorage — a previous spec can leave it on SQL — and because `series-where-input` only exists on the Lucene branch, SearchWhereInput's SQL branch rendering a CodeMirror that carries no test id. Lookups are scoped to the tile-editor modal, since the dashboard behind it renders the same markup. `addTileWithSource` takes the filter as an optional third argument; its other callers pass two and are unchanged.

github-actions · 2026-08-14T19:16:26Z

🔵 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: 0
Production lines changed: 0 (+ 78 in test files, excluded from tier calculation)
Branch: claude/release-markers-tile-filter-e2e
Author: teeohhem

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

github-actions · 2026-08-14T19:23:28Z

E2E Test Results

✅ All tests passed • 296 passed • 1 skipped • 1075s

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

Tests ran across 4 shards in parallel.

View full report →

pulpdrew

LGTM

vercel Bot deployed to Preview – hyperdx-storybook August 13, 2026 21:12 View deployment

greptile-apps Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread packages/app/src/ChartUtils.tsx

Comment thread packages/app/src/components/charts/chartAnnotations.tsx

vercel Bot deployed to Preview – hyperdx-oss August 13, 2026 21:15 View deployment

teeohhem force-pushed the tom/release-markers branch from 319c3c3 to e30ecc3 Compare August 14, 2026 13:49

teeohhem changed the base branch from tom/release-markers to main August 14, 2026 19:15

teeohhem force-pushed the claude/release-markers-tile-filter-e2e branch from 58dd53f to d4d0403 Compare August 14, 2026 19:15

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

vercel Bot deployed to Preview – hyperdx-storybook August 14, 2026 19:19 View deployment

vercel Bot deployed to Preview – hyperdx-oss August 14, 2026 19:20 View deployment

pulpdrew approved these changes Aug 14, 2026

View reviewed changes

teeohhem merged commit 28113dc into main Aug 14, 2026
27 checks passed

teeohhem deleted the claude/release-markers-tile-filter-e2e branch August 14, 2026 19:38

Navigation Menu

Uh oh!

test(dashboards): cover release-marker scoping by the tile's own filter - #2905

test(dashboards): cover release-marker scoping by the tile's own filter#2905
teeohhem merged 1 commit into
mainfrom
claude/release-markers-tile-filter-e2e

teeohhem commented Aug 13, 2026 •

edited

Loading

Uh oh!

changeset-bot Bot commented Aug 13, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Aug 13, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Aug 13, 2026 •

edited

Loading

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 13, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 14, 2026

Uh oh!

github-actions Bot commented Aug 14, 2026

Uh oh!

pulpdrew left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

teeohhem commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

No app-code changes

Verification

Uh oh!

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

Uh oh!

⚠️ No Changeset found

Uh oh!

vercel Bot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Deep Review

Uh oh!

github-actions Bot commented Aug 14, 2026

🔵 Tier 2 — Low Risk

Uh oh!

github-actions Bot commented Aug 14, 2026

E2E Test Results

Uh oh!

pulpdrew left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

teeohhem commented Aug 13, 2026 •

edited

Loading

changeset-bot Bot commented Aug 13, 2026 •

edited

Loading

vercel Bot commented Aug 13, 2026 •

edited

Loading

greptile-apps Bot commented Aug 13, 2026 •

edited

Loading

github-actions Bot commented Aug 13, 2026 •

edited

Loading