Skip to content

Fix scatter collapse after repeated box zoom#101

Merged
masenf merged 11 commits into
mainfrom
codex/fix-scatter-view-collapse-on-zoom
Jul 21, 2026
Merged

Fix scatter collapse after repeated box zoom#101
masenf merged 11 commits into
mainfrom
codex/fix-scatter-view-collapse-on-zoom

Conversation

@FarhanAliRaza

@FarhanAliRaza FarhanAliRaza commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Motivation

Repeated anisotropic box zoom followed by Zoom Out could collapse a scatter plot into a horizontal band.

A box zoom can narrow X and Y by very different factors. Zoom Out previously expanded both axes geometrically without respecting each axis's original range, so the less-zoomed axis could grow far beyond its home span while the other axis was still zoomed in. That extreme aspect-ratio mismatch visually flattened the point cloud.

Description

  • Cap each axis independently at its original span during zoom-out. Once an axis reaches its home span it stops expanding, while a more deeply zoomed axis can continue zooming out. (js/src/53_interaction.js)
  • Add the exact repeated box-zoom reproduction to examples/demo-advance.ipynb.
  • Add a Chromium regression that performs three anisotropic box zooms and four zoom-outs, then verifies X remains zoomed while Y stops exactly at its home domain. (tests/test_zoom_precision.py)
  • Regenerate the tracked anywidget and standalone browser bundles from the JS source. (python/xy/static/index.js, python/xy/static/standalone.js)

The earlier GPU-coordinate precision workaround was removed because it was not needed for this failure.

Testing

  • node js/build.mjs --check — generated static bundles are fresh.
  • uv run ruff check tests/test_zoom_precision.py — passed.
  • uv run ruff format --check tests/test_zoom_precision.py — passed.
  • uv run pytest tests/test_zoom_precision.py tests/test_static_client_security.py tests/test_scatter.py tests/test_components.py -q — 187 passed.
  • Chrome verification using the 20,000-point notebook reproduction:
    • after three anisotropic box zooms: X 125×, Y 1.953×
    • after four zoom-outs: X 7.8125×, Y
    • before the fix, Y reached 0.122×—eight times wider than its home span
    • the point cloud remained two-dimensional and no console errors were emitted

Closes #87

@codspeed-hq

codspeed-hq Bot commented Jul 20, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 97 untouched benchmarks
⏩ 1 skipped benchmark1


Comparing codex/fix-scatter-view-collapse-on-zoom (1dfaeda) with main (09b5873)

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

@FarhanAliRaza FarhanAliRaza changed the title Fix scatter precision after repeated box zoom Fix scatter collapse after repeated box zoom Jul 20, 2026
@FarhanAliRaza FarhanAliRaza added valid Issue is validated. This pr actually fixes the issue. Needs Code Review. and removed codex labels Jul 20, 2026

@masenf masenf left a comment

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.

when zooming out, if the zoom level is already at 100%, the pan is reset as well. when this happens, it leaves the previous selection in place including the dot and tooltip.

Screen.Recording.2026-07-20.at.2.54.03.PM.mov

probably a task for another PR, but i'm wondering if we should let the outer page scroll when the chart is already zoomed out to mitigate ENG-10492 which is a bit annoying when browsing the docs. idk if it would be more jarring for a user actively zooming out to hit 100% and then suddenly scroll down the page.

The tooltip stored only the cursor's screen position, so any view change
that happened without a pointermove — modebar Reset View, dblclick home,
wheel zoom, linked views — left it floating at a stale location showing
values for a point that was no longer there.

Anchor it in data space instead (matplotlib's data-coordinate-annotation
contract): record the hovered point's coordinates at pick time, reproject
them on every draw so the tooltip rides its point through pans, zooms,
and reset animations, and clip it when the anchor leaves the plot rect
rather than clamping to an edge that misrepresents where the point is.
The kernel's exact pick reply sharpens the f32-decoded anchor to f64
(§16), and every hide path clears the anchor so a later draw cannot
resurrect a dismissed tooltip.
@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes scatter collapse after repeated anisotropic box zooms. The main changes are:

  • Independent home-span limits for each axis during zoom-out.
  • Data-space tooltip positioning across view changes.
  • A browser test for repeated box zoom and zoom-out.
  • Cleaner Chromium shutdown and regenerated browser bundles.
  • Updated examples and interaction documentation.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.

T-Rex T-Rex Logs

What T-Rex did

  • Executed the general-contract-validation-proof to compare the current and final domains for X and Y.
  • The final Y domain is [-1.0388, 1.0387999999999995], confirming the expected y-range after validation.
  • The final X domain is [-0.2713599999999998, 0.2713599999999998], showing the narrowed horizontal range.
  • Recorded before-and-after videos demonstrating the complete repeated box-zoom and zoom-out sequence on the standalone HTML, client code, and styles.
  • Reached the maximum agent steps and noted that substantive validation tasks are complete; artifacts should be retained for review.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
js/src/53_interaction.js Limits each axis to its home span during zoom-out and clears tooltip state on explicit dismissal paths.
tests/test_zoom_precision.py Adds browser coverage for repeated anisotropic box zoom followed by zoom-out.
js/src/52_tooltip.js Stores tooltip positions in data space and reprojects them when the view changes.
js/src/50_chartview.js Repositions visible tooltips after chart redraws.
js/src/54_kernel.js Updates tooltip positions with exact coordinates returned by the kernel.
python/xy/_chromium.py Requests orderly browser shutdown before falling back to process termination.
python/xy/static/index.js Regenerates the anywidget bundle with the zoom and tooltip changes.
python/xy/static/standalone.js Regenerates the standalone bundle with the zoom and tooltip changes.

Reviews (6): Last reviewed commit: "fix(export): shut Chromium down via CDP ..." | Re-trigger Greptile

…w-collapse-on-zoom

# Conflicts:
#	examples/demo-advance.ipynb
#	js/src/53_interaction.js
#	python/xy/static/index.js
#	python/xy/static/standalone.js
…encoded linear mapping

interaction.md §5 gains the zoom-limit rules this branch introduced (the
§16 precision floor on zoom-in and the per-axis home-span stop on
zoom-out); a new §7 specifies the tooltip's data-coordinate anchoring
contract. design-dossier.md §16 records that linear axes stay
offset-encoded through the vertex transform (only log axes decode).
renderer-architecture.md module inventory resynced with the code.
masenf
masenf previously approved these changes Jul 21, 2026
@masenf

masenf commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

i busted a test try to merge origin/main, will push a patch to this branch

masenf and others added 4 commits July 21, 2026 12:24
…anchor

_zoomAxisRange's zoom-out guard snapped an axis to its home *range* once it
reached home span, discarding the translation from a cursor-anchored zoom
chain. A zoom-in-high then zoom-out-low pans a free (pan-enabled) axis; the
snap yanked it back to home. Cap the span at home while preserving the anchor
instead, matching the spec (zoom-out stops at home span, §7.5). Positional
containment remains the mechanism that pins locked axes to home.

Also fix the zoom-precision probe's finite check: view.view now nests a
`ranges` object (per-axis pan/zoom, #117), so Object.values(current) included a
non-number. Check the numeric view coordinates directly.
…lose

Chromium helper processes can outlive the terminated main process and
write into the temp profile while rmtree walks it, failing CI teardown
with ENOTEMPTY. Retry the cleanup, then abandon leftovers.
…cleanup

SIGTERM killed the browser process out from under its helper processes,
which kept flushing into the profile's Default/ while TemporaryDirectory
cleanup walked it, failing CI teardown with ENOTEMPTY. Browser.close asks
Chromium for an orderly exit — it flushes profile state and reaps its
helpers before the main process exits — so waiting on the process leaves
the profile quiescent. Also drop the crashpad handler, a detached process
that writes into the profile on its own schedule. Signals remain only as
escalation if the browser fails to exit.
@masenf
masenf merged commit eddb2c1 into main Jul 21, 2026
29 checks passed
Alek99 added a commit that referenced this pull request Jul 21, 2026
Conflict in spec/api/interaction.md was additive on both sides: main's
zoom-limit boundaries (#101) and this branch's axis-band scoping
paragraph plus the Back/Forward modebar buttons. Kept both and refreshed
the _zoomAxisRange line citation, which the merge shifted.
masenf added a commit that referenced this pull request Jul 21, 2026
Reconcile the updatePayload re-home fix with main's Reflex events /
cross-filtering work (#113) and the box-zoom scatter-collapse fix (#101).

- js/src/56_animation.js: auto-merged — the re-home block (clear view0 before
  deriving the new home) and main's `_updatePickable()` refactor are separate
  hunks of updatePayload and coexist. The full-payload path still routes
  through updatePayload; main's new XYChart.jsx onPayload re-pins a navigated
  chart via the restore contract, while a dependent (never-navigated) chart —
  the §4 detail histogram — takes the re-home, which is what fixes zoom-out.
- python/xy/static/{index,standalone}.js: regenerated from merged js/src.
- tests/reflex_adapter/test_assets.py: took main's assertion — the wrapper now
  fits the live spec via `eventSpec(data.spec, cbRef.current)`, superseding the
  stale-literal fix carried on this branch.
- spec/design/reflex-integration.md §4: keep the re-home note, reworded to
  defer viewport preservation to main's restore contract (the home must be the
  new spec's extents in both cases).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

valid Issue is validated. This pr actually fixes the issue. Needs Code Review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scatter view collapses to a horizontal line after repeated box-zoom + zoom-out

2 participants