+
${this._getVisiblePointCount()} points
${this._isZoomedIn ? html`· Zoomed in` : ''}
diff --git a/packages/core/src/components/scatter-plot/scatter-plot.zoom-indicator.test.ts b/packages/core/src/components/scatter-plot/scatter-plot.zoom-indicator.test.ts
index 2cf2f643..1e59eafc 100644
--- a/packages/core/src/components/scatter-plot/scatter-plot.zoom-indicator.test.ts
+++ b/packages/core/src/components/scatter-plot/scatter-plot.zoom-indicator.test.ts
@@ -70,13 +70,13 @@ describe('scatterplot zoom indicator (#343)', () => {
vi.restoreAllMocks();
});
- it('renders the point count and zoom label as spaced presentation items', async () => {
+ it('renders the point count and zoom label as a polite status', async () => {
const plot = await makePlot();
const host = plot._interactionHost();
host.onTransform(d3.zoomIdentity.scale(2));
await plot.updateComplete;
- const chip = plot.shadowRoot?.querySelector('.plot-indicator');
+ const chip = plot.shadowRoot?.querySelector('[role="status"][aria-live="polite"]');
expect(chip?.querySelector('.point-count')?.textContent).toBe('1 points');
expect(chip?.querySelector('.zoom-indicator')?.textContent).toBe('· Zoomed in');
expect(Array.from(chip?.children ?? []).map((child) => child.className)).toEqual([
From fb660dd6610b7ae2ea82c2a43bf4160fd3790069 Mon Sep 17 00:00:00 2001
From: Florin Senoner <23100806+FlorinSenoner@users.noreply.github.com>
Date: Thu, 6 Aug 2026 14:15:39 +0200
Subject: [PATCH 4/4] fix(scatterplot): harden zoom indicator boundary
---
apps/web/tests/zoom-indicator.spec.ts | 18 +-------
.../changes/show-zoom-indicator/design.md | 12 +++---
.../specs/scatterplot-zoom-indicator/spec.md | 14 +++++-
openspec/changes/show-zoom-indicator/tasks.md | 7 +++
.../scatter-plot/scatter-plot.styles.ts | 1 -
.../components/scatter-plot/scatter-plot.ts | 20 +++++----
.../scatter-plot.zoom-indicator.test.ts | 43 ++++++++++++-------
7 files changed, 67 insertions(+), 48 deletions(-)
diff --git a/apps/web/tests/zoom-indicator.spec.ts b/apps/web/tests/zoom-indicator.spec.ts
index afd67d47..f98022a1 100644
--- a/apps/web/tests/zoom-indicator.spec.ts
+++ b/apps/web/tests/zoom-indicator.spec.ts
@@ -17,31 +17,17 @@ test.describe('scatterplot zoom indicator (#343)', () => {
};
const pointCountChip = plot.getByRole('status');
await expect(pointCountChip).toHaveAttribute('aria-live', 'polite');
- const pointCount = pointCountChip.locator('.point-count');
- const zoomMarker = pointCountChip.locator('.zoom-indicator');
- await expect(zoomMarker).toHaveCount(0);
+ await expect(pointCountChip).toHaveText(/^\s*\d+ points\s*$/);
await page.mouse.move(center.x, center.y);
await page.mouse.wheel(0, -500);
await expect
.poll(() => plot.evaluate((element: any) => element._transform.k))
.toBeGreaterThan(1);
- await expect(pointCount).toHaveText(/^\d+ points$/);
- await expect(zoomMarker).toHaveText('· Zoomed in');
- const chipSpacing = await pointCountChip.evaluate((chip) => {
- const count = chip.querySelector('.point-count')?.getBoundingClientRect();
- const marker = chip.querySelector('.zoom-indicator')?.getBoundingClientRect();
- return {
- actual: count && marker ? marker.left - count.right : 0,
- configured: Number.parseFloat(getComputedStyle(chip).columnGap),
- };
- });
- expect(chipSpacing.actual).toBeGreaterThan(0);
- expect(chipSpacing.actual).toBeCloseTo(chipSpacing.configured, 1);
+ await expect(pointCountChip).toHaveText(/^\s*\d+ points · Zoomed in\s*$/);
await page.mouse.dblclick(center.x, center.y);
await expect.poll(() => plot.evaluate((element: any) => element._transform.k)).toBe(1);
- await expect(zoomMarker).toHaveCount(0);
await expect(pointCountChip).toHaveText(/^\s*\d+ points\s*$/);
});
});
diff --git a/openspec/changes/show-zoom-indicator/design.md b/openspec/changes/show-zoom-indicator/design.md
index f754f146..e5349f6d 100644
--- a/openspec/changes/show-zoom-indicator/design.md
+++ b/openspec/changes/show-zoom-indicator/design.md
@@ -8,7 +8,7 @@ Issue #343 requires a visible signal specifically for zooming in. The current `z
**Goals:**
-- Show `Zoomed in` next to the existing point count while the active D3 scale is greater than `1`.
+- Show `Zoomed in` next to the existing point count while the active D3 scale is meaningfully greater than `1`, ignoring near-identity floating-point residue from symmetric wheel gestures.
- Remove the marker once reset reaches identity.
- Announce zoom and reset status changes to assistive technology without moving focus.
- Preserve the non-reactive `_transform` performance invariant by scheduling Lit updates only when the boolean zoomed-in state changes.
@@ -24,23 +24,23 @@ Issue #343 requires a visible signal specifically for zooming in. The current `z
### Reuse the existing point-count chip
-The marker will render as `N points · Zoomed in` inside the existing bottom-left `.plot-indicator`. The chip reuses the component's established `role="status" aria-live="polite"` pattern so its changing content is announced without focus movement. A separate overlay was rejected because it would need new collision rules for no additional user value.
+The marker will render as the single text run `N points · Zoomed in` inside the existing bottom-left `.plot-indicator`. The chip reuses the component's established `role="status" aria-live="polite"` pattern so its changing content is announced without focus movement. A separate overlay was rejected because it would need new collision rules for no additional user value.
### Store only a reactive boolean boundary
-The scatterplot will add a reactive `_isZoomedIn` boolean. The host callback will continue assigning every transform to the plain `_transform` field, then assign `t.k > 1` to the boolean. Lit's default change detection ignores equal boolean values, so it enqueues a render once when zoom crosses above identity and once when reset reaches identity, rather than once per D3 frame.
+The scatterplot will add a reactive `_isZoomedIn` boolean. The host callback will continue assigning every transform to the plain `_transform` field, then compare the scale against identity plus a small fixed tolerance before assigning the boolean. The tolerance treats multiplicative wheel round-trip residue as identity without suppressing a perceptible zoom. Lit's default change detection ignores equal boolean values, so it enqueues a template render once when zoom crosses above identity and once when reset reaches identity, rather than once per D3 frame. Because the marker is light-DOM text and the controller already renders transformed plot content imperatively, marker-only updates will be excluded from the `updated()` path that redraws WebGL and rebuilds selection overlays.
Keeping the boolean on the scatterplot host, rather than the interaction controller, preserves the controller's role as a generic transform dispatcher and keeps rendering state beside the template that consumes it.
### Verify both state propagation and user-visible behavior
-A focused jsdom test will drive the real host bridge, assert the count and marker render as a polite status, and observe actual plot rendering across the no-repeat-update boundary for additional `k > 1` transforms. A Playwright regression will locate the chip by its status role, wheel over the real Explore scatterplot, verify the complete `N points · Zoomed in` presentation, double-click reset, and assert the status returns to the point count alone.
+A focused jsdom test will drive the real host bridge, assert the exact count/marker text renders as a polite status, observe `isUpdatePending` across repeated same-side transforms, verify marker-only state changes do not redraw WebGL, and cover near-identity wheel residue. A Playwright regression will locate the chip by its status role, wheel over the real Explore scatterplot, verify the complete `N points · Zoomed in` presentation, double-click reset, and assert the status returns to the point count alone.
## Risks / Trade-offs
-- **Floating-point values during reset could keep the marker visible until the transition finishes** → This is intentional: the plot remains non-identity until D3 emits the final `k === 1` transform.
+- **Multiplicative wheel accumulation can finish a symmetric gesture a few ULPs above `1`** → Treat scales within a small fixed tolerance of identity as identity; the exact reset transform remains covered as well.
- **A marker appended to the count chip is less prominent than a dedicated badge** → The count chip is persistent, unobtrusive, and has no collision risk; the text remains visible throughout the zoomed state.
-- **Reactive state could regress zoom performance if equal values enqueue updates** → Exercise consecutive same-side transforms and assert that no additional plot render runs.
+- **Reactive state could regress zoom performance** → Exercise consecutive same-side transforms through Lit's scheduling signal and independently assert that marker-only updates do not enter the WebGL/overlay redraw path.
## Migration Plan
diff --git a/openspec/changes/show-zoom-indicator/specs/scatterplot-zoom-indicator/spec.md b/openspec/changes/show-zoom-indicator/specs/scatterplot-zoom-indicator/spec.md
index 32436e58..88c5ecbd 100644
--- a/openspec/changes/show-zoom-indicator/specs/scatterplot-zoom-indicator/spec.md
+++ b/openspec/changes/show-zoom-indicator/specs/scatterplot-zoom-indicator/spec.md
@@ -2,7 +2,7 @@
### Requirement: Scatterplot indicates a zoomed-in view
-The scatterplot SHALL append the text `Zoomed in` to its existing visible point-count indicator whenever the active view scale is greater than the identity scale of `1`. The scatterplot SHALL NOT show that marker at identity scale, while zoomed out below identity, or for translation alone at identity scale. The point-count indicator SHALL expose its changing content as a polite status message so assistive technology can announce zoom and reset changes without moving focus.
+The scatterplot SHALL append the text `Zoomed in` to its existing visible point-count indicator whenever the active view scale is greater than the identity scale of `1` by more than the implementation's small numerical tolerance. The scatterplot SHALL NOT show that marker at identity scale, for near-identity floating-point residue, while zoomed out below identity, or for translation alone at identity scale. The point-count indicator SHALL expose its changing content as a polite status message so assistive technology can announce zoom and reset changes without moving focus.
#### Scenario: User zooms in with the wheel
@@ -20,6 +20,11 @@ The scatterplot SHALL append the text `Zoomed in` to its existing visible point-
- **WHEN** the existing reset behavior returns the view to identity scale `1`
- **THEN** the point-count indicator no longer includes `Zoomed in`
+#### Scenario: Symmetric wheel gesture returns near identity
+
+- **WHEN** accumulated wheel transforms leave the scale only within the numerical tolerance above `1`
+- **THEN** the point-count indicator does not include `Zoomed in`
+
#### Scenario: Assistive technology receives zoom-state changes
- **WHEN** the point-count indicator changes between identity and zoomed-in content
@@ -33,7 +38,7 @@ The scatterplot SHALL append the text `Zoomed in` to its existing visible point-
### Requirement: Zoom indication preserves transform rendering performance
-The scatterplot MUST keep the full D3 transform non-reactive and SHALL derive a separate reactive boolean for the zoomed-in boundary. Transform updates SHALL schedule marker-related Lit rendering only when that boolean changes.
+The scatterplot MUST keep the full D3 transform non-reactive and SHALL derive a separate reactive boolean for the zoomed-in boundary. Transform updates SHALL schedule marker-related Lit rendering only when that boolean changes. Marker-only Lit updates SHALL NOT redraw WebGL content or rebuild selection overlays already handled by the imperative zoom path.
#### Scenario: Zoom gesture emits multiple zoomed-in frames
@@ -44,3 +49,8 @@ The scatterplot MUST keep the full D3 transform non-reactive and SHALL derive a
- **WHEN** a reset transition emits zoomed-in frames followed by its final identity frame
- **THEN** the reactive zoom-indicator state changes once on the final identity frame
+
+#### Scenario: Zoom-indicator state crosses its boundary
+
+- **WHEN** a transform changes only the reactive zoom-indicator state
+- **THEN** the status text updates without an additional WebGL redraw or selection-overlay rebuild
diff --git a/openspec/changes/show-zoom-indicator/tasks.md b/openspec/changes/show-zoom-indicator/tasks.md
index 8ce26e9f..cce4e0f4 100644
--- a/openspec/changes/show-zoom-indicator/tasks.md
+++ b/openspec/changes/show-zoom-indicator/tasks.md
@@ -16,3 +16,10 @@
- [x] 3.1 Repeat the original browser reproduction and confirm the marker appears above identity and disappears after reset with no new relevant console errors.
- [x] 3.2 Run the affected core and Playwright test projects, then run `pnpm precommit`.
- [x] 3.3 Validate the OpenSpec change and review the final diff for issue-only scope.
+
+## 4. Adversarial Review Follow-up
+
+- [x] 4.1 Rewrite the boundary-scheduling regression around Lit's `isUpdatePending` signal and add a failing regression showing marker-only updates must not redraw WebGL.
+- [x] 4.2 Add a failing regression for near-identity floating-point residue from symmetric wheel accumulation.
+- [x] 4.3 Render the count and conditional marker as one exact text run and remove the marker-only gap styling and query-hook spans.
+- [x] 4.4 Run focused component and browser tests, strict OpenSpec validation, and the repository precommit checks.
diff --git a/packages/core/src/components/scatter-plot/scatter-plot.styles.ts b/packages/core/src/components/scatter-plot/scatter-plot.styles.ts
index 1f5b9ddc..0470dc90 100644
--- a/packages/core/src/components/scatter-plot/scatter-plot.styles.ts
+++ b/packages/core/src/components/scatter-plot/scatter-plot.styles.ts
@@ -140,7 +140,6 @@ const scatterplotStylesCore = css`
z-index: var(--z-overlay);
display: flex;
align-items: center;
- gap: 0.25rem;
height: 2rem;
padding: 0 0.625rem;
box-sizing: border-box;
diff --git a/packages/core/src/components/scatter-plot/scatter-plot.ts b/packages/core/src/components/scatter-plot/scatter-plot.ts
index 7e67a932..c9d5e030 100644
--- a/packages/core/src/components/scatter-plot/scatter-plot.ts
+++ b/packages/core/src/components/scatter-plot/scatter-plot.ts
@@ -83,6 +83,10 @@ const VIRTUALIZATION_PADDING = 100;
const HIT_TEST_SEARCH_RADIUS_PX = 15;
const POINT_RADIUS_SIZE_DIVISOR = 3;
+// D3 wheel zoom accumulates scale multiplicatively, so a symmetric round trip
+// can finish a few ULPs above identity even though the view is visually reset.
+const ZOOM_IDENTITY_EPSILON = 1e-6;
+
/** Default number of bins for numeric→categorical materialization. Mirrors
* materializeVisualizationData's `defaultBinCount = 10` default. */
const DEFAULT_NUMERIC_BIN_COUNT = 10;
@@ -866,12 +870,13 @@ export class ProtspaceScatterplot extends LitElement {
this._webglRenderer?.invalidateStyleCache();
this._renderPlot();
}
- // Render for other changes
- const selectionKeys = ['selectedProteinIds', 'highlightedProteinIds'];
+ // These keys affect only the template or are rendered by the selection block
+ // above. Zoom transforms already redraw through the interaction controller's RAF.
+ const noAdditionalRenderKeys = ['selectedProteinIds', 'highlightedProteinIds', '_isZoomedIn'];
const changedKeys = Array.from(changedProperties.keys()).map(String);
- const onlySelectionChanged =
- changedKeys.length > 0 && changedKeys.every((k) => selectionKeys.includes(k));
- if (!onlySelectionChanged) {
+ const onlyNoAdditionalRenderKeysChanged =
+ changedKeys.length > 0 && changedKeys.every((k) => noAdditionalRenderKeys.includes(k));
+ if (!onlyNoAdditionalRenderKeysChanged) {
this._renderPlot();
this._updateSelectionOverlays();
}
@@ -1226,7 +1231,7 @@ export class ProtspaceScatterplot extends LitElement {
resolveSlotsToIds: (slots) => this._slotsToInteractiveIds(slots),
onTransform: (t) => {
this._transform = t;
- this._isZoomedIn = t.k > 1;
+ this._isZoomedIn = t.k > 1 + ZOOM_IDENTITY_EPSILON;
this._connectorOverlay.updateZoomScale(t.k);
},
onSelect: (ids, clearVisual) => this._commitSelection(ids, clearVisual),
@@ -1984,8 +1989,7 @@ export class ProtspaceScatterplot extends LitElement {
${this.data
? html`
- ${this._getVisiblePointCount()} points
- ${this._isZoomedIn ? html`· Zoomed in` : ''}
+ ${`${this._getVisiblePointCount()} points${this._isZoomedIn ? ' · Zoomed in' : ''}`}
`
: ''}
diff --git a/packages/core/src/components/scatter-plot/scatter-plot.zoom-indicator.test.ts b/packages/core/src/components/scatter-plot/scatter-plot.zoom-indicator.test.ts
index 1e59eafc..7ea9e4e4 100644
--- a/packages/core/src/components/scatter-plot/scatter-plot.zoom-indicator.test.ts
+++ b/packages/core/src/components/scatter-plot/scatter-plot.zoom-indicator.test.ts
@@ -26,10 +26,12 @@ import './scatter-plot';
type ZoomIndicatorInternals = HTMLElement & {
data: VisualizationData;
selectedAnnotation: string;
+ isUpdatePending: boolean;
updateComplete: Promise
;
firstUpdated(): void;
_interactionHost(): PlotInteractionHost;
_renderPlot(): void;
+ _updateSelectionOverlays(): void;
};
function makeData(): VisualizationData {
@@ -77,36 +79,47 @@ describe('scatterplot zoom indicator (#343)', () => {
await plot.updateComplete;
const chip = plot.shadowRoot?.querySelector('[role="status"][aria-live="polite"]');
- expect(chip?.querySelector('.point-count')?.textContent).toBe('1 points');
- expect(chip?.querySelector('.zoom-indicator')?.textContent).toBe('· Zoomed in');
- expect(Array.from(chip?.children ?? []).map((child) => child.className)).toEqual([
- 'point-count',
- 'zoom-indicator',
- ]);
+ expect(chip?.textContent?.trim()).toBe('1 points · Zoomed in');
});
- it('schedules rendering only when the zoomed-in boundary changes', async () => {
+ it('schedules a Lit update only when the zoomed-in boundary changes', async () => {
const plot = await makePlot();
const host = plot._interactionHost();
- const renderPlot = vi.spyOn(plot, '_renderPlot').mockImplementation(() => {});
host.onTransform(d3.zoomIdentity.scale(2));
+ expect(plot.isUpdatePending).toBe(true);
await plot.updateComplete;
- expect(renderPlot).toHaveBeenCalledTimes(1);
- renderPlot.mockClear();
host.onTransform(d3.zoomIdentity.scale(3));
- await plot.updateComplete;
- expect(renderPlot).not.toHaveBeenCalled();
+ expect(plot.isUpdatePending).toBe(false);
host.onTransform(d3.zoomIdentity.translate(30, 20));
+ expect(plot.isUpdatePending).toBe(true);
await plot.updateComplete;
- expect(renderPlot).toHaveBeenCalledTimes(1);
- expect(plot.shadowRoot?.querySelector('.zoom-indicator')).toBeNull();
+ expect(plot.shadowRoot?.querySelector('[role="status"]')?.textContent?.trim()).toBe('1 points');
- renderPlot.mockClear();
host.onTransform(d3.zoomIdentity.scale(0.5));
+ expect(plot.isUpdatePending).toBe(false);
+ });
+
+ it('does not redraw plot content for a zoom-indicator-only update', async () => {
+ const plot = await makePlot();
+ const renderPlot = vi.spyOn(plot, '_renderPlot').mockImplementation(() => {});
+ const updateOverlays = vi.spyOn(plot, '_updateSelectionOverlays').mockImplementation(() => {});
+
+ plot._interactionHost().onTransform(d3.zoomIdentity.scale(2));
await plot.updateComplete;
+
expect(renderPlot).not.toHaveBeenCalled();
+ expect(updateOverlays).not.toHaveBeenCalled();
+ });
+
+ it('treats floating-point wheel residue as identity', async () => {
+ const plot = await makePlot();
+
+ plot._interactionHost().onTransform(d3.zoomIdentity.scale(1.0000000000000002));
+ await plot.updateComplete;
+
+ expect(plot.shadowRoot?.querySelector('[role="status"]')?.textContent?.trim()).toBe('1 points');
});
});