Laundry-list UX batch: Calculating chip, editable metadata, Crop-as-live-ROI, movie-editor overlay scoping - #44
Merged
Merged
Conversation
Follow-ups from the item-#1 review: (a) a second selector on the same navigator (add_navigation_selector_and_signal_plot, as the add_selector toolbar action does) proves closing one signal window leaves the sibling registered with no stray selector_removed - the old survive test was vacuous with the single-selector fixture; (b) the 5-D chained-navigator close proves the COMPOSITE deregisters (selector_removed carries the composite id), the upstream time composite survives, and both inner settle timers are cancelled. 7 passed x2 runs (timer-race flake check). (cherry picked from commit 4a850ee68616788c52c686027dbb0dea96fb3fb1)
Backend emits {type:'window_computing', window_id, computing} around long
computes: the progressive nav fill (threaded + distributed), streamed VI
(stream_progressive_to_plot), and the initial lazy-signal compute. Every
start has a guaranteed stop (try/finally or the _on_plot_ready terminal
branch); a new window_computing context manager in lifecycle.py wraps the
pattern. Renderer shows a centered translucent pulse chip (pointer-events
none - the chunk fill stays visible and interactive) debounced 300 ms so
short computes never flash it; clears on finish/fail/close/node-switch.
This commit is the backend emitters + chip UI + tests; the session.py stop
emit and the renderer kernel wiring (protocol.ts/SpyDEContext.tsx) land in
the wave-2 shared-kernel wiring commit alongside two sibling features'
hunks. Also carries the update_functions.py comment repoint to
spyde/external (from bcc6eca's review). (Laundry item #3; Opus-reviewed:
SHIP - start/stop balanced on all paths incl. cancelled futures, off-main
emits safe on the locked stdout channel.)
(cherry picked from commit d70cef275a94ccd193e88c0077e2d96f13389f7c)
…e axes)
Double-click an Instrument-metadata value -> inline EditableCell (the
axes-table component) -> set_metadata action resolves (group,prop) to the
hyperspy key server-side via METADATA_WIDGET_CONFIG (no key on the wire),
writes tree.root.metadata (root is what the panel displays, consistent
from any window incl. derived nodes), re-emits to all tree windows.
YAML key:-bearing cells are editable; derived (Dtype/Dim.) + Dataset stay
read-only (enforced backend-side). Numeric coercion rejects garbage AND
non-finite (nan/inf); edits survive .zspy save->reload (tested with a
real fresh hs.load).
Review found + fixed: the cell pre-filled the units-suffixed DISPLAY
string ('200.0 kV') so numeric edits silently no-opd; now the emit carries
a raw unit-free value map (editable: {group:{prop:raw}}) and the cell uses
the same value/display split as axes cells. 11 tests. (Laundry item #5;
Opus-reviewed: FIX-FIRST -> fixed; session.py/protocol.ts/SpyDEContext.tsx
hunks land in the wave-2 wiring commit.)
(cherry picked from commit b4a39cca5ea269b9dafe0c961e59cc110bdf508a)
Crop (laundry #4): the caret opens crop_open -> a draggable/resizable rectangle covers the frame; fields sync bidirectionally with the widget (widget primary, via the existing spyde:figure_event flow - no kernel changes); Crop applies from the LIVE widget bounds (clamped to frame, full-frame = no-op with NO redundant tree node); widget tears down on apply/close/cancel/window close/node switch/tree close. Center Zero Beam (laundry #10): the search box never showed because czb_set_region drew a static add_squares MarkerGroup (decoration, not an input) and the default half_square_width=0 removed even that. Now a real draggable rectangle widget appears immediately (full-frame at hw=0), drags re-centre/clamp it (pyxem's search is frame-centred), and czb_run reads the live widget (hw=0 -> kwarg omitted = search everywhere). Review blocker fixed: anyplotlib Widget.set() fires pointer_move even on a no-change write, so both drag handlers re-entered themselves (measured 428 handler runs / stack depth ~1988 per JS drag frame, surviving only via a bumped recursionlimit + swallowed RecursionError). Hard per-tree re-entrancy guards added; once-per-event tests drive the REAL _update_from_js path. Node-switch (show_tree_node) and tree-close teardown gaps closed. 30+40+14 tests green. (Laundry #4+#10; Opus-reviewed: FIX-FIRST -> fixed.) (cherry picked from commit 6d5f703c72e5d5db96b577e150bf72f00c0d360b)
… zoom One renderer defect caused both symptoms (laundry #6+#13): the editor mounts a second iframe for the SAME fig_id while the MDI window's iframe stays mounted; both register in iframeRefs on one key, last-write-wins, so state pushes land on only one - tile detail going to the hidden MDI iframe left the editor blurry, and widget replays going to the MDI iframe painted movie overlays outside the editor. Fix: a movieEditorClaim store (useSyncExternalStore) - while the editor holds a window, MDIArea unmounts it entirely (one consumer per fig_id); on close the claim clears, the window remounts and replayState repaints clean current state. Backend teardown gaps closed: _clear_movie_sessions now routes through the same _teardown_session as movie_close (was cancel- flag only - widgets leaked on report New/Open/close), report_remove_cell gains a movie branch, and MovieGate auto-closes when the report closes or its cell disappears. Teardown is idempotent (double-fire safe). Overlay persistence in cell.movie.annotations is untouched - reopen restores. No new tile code: the editor scrubs the IDENTICAL Plot/Plot2D as the MDI window, so anyplotlib's tile loop (enable_tile/update_tile_source) just works once only one iframe consumes the pushes; zoom persists across scrub. +7 tests incl. tile-backend-engaged hardening from review. (Opus-reviewed: SHIP.) (cherry picked from commit ba85ea8a6f4fcd9e657d0bc83d0ec11a0020a8ed)
Serial Playwright spec covering all 13 items via screenshots (the pixels are the test, per CLAUDE.md). Groups A/B/C by app instance. Shots land in electron/laundry_visual_shots/. 14/14 passed in the verification run. (cherry picked from commit 5b8374619dc8d4f336da045ad761671dee7ed397)
…ing) The hunks four already-committed features share in the three kernel files, committed together so each lands coherently: - session.py: _on_plot_ready emits the window_computing stop (d70cef2's Calculating chip - the one guaranteed terminal point for the initial lazy compute); the two inline metadata-emit blocks are replaced by _session_axes._emit_metadata (b4a39cc's editable metadata - the emit now carries the raw editable map on load/type-change too). - protocol.ts: WindowComputingMessage (d70cef2), SelectorRemovedMessage (d4058b1), MetadataMessage.editable raw-value map (b4a39cc), IoThroughputMessage (0e46fa8) + union entries. - SpyDEContext.tsx: computingWindows Set<number> (+ WINDOW_COMPUTING reducer, clears on WINDOW_CLOSED/SIGNAL_TREE switch), SELECTOR_REMOVED row pruning, metadataEditable map, io_throughput CustomEvent re-broadcast. tsc clean (tsconfig.node + tsconfig.web). (cherry picked from commit 01131d9e669a7deb87e65fda1b39ab070da9bf30)
…heck Running laundry_visual.spec.ts against the subset that actually landed turned up two problems in the spec itself. 1. "#8 MB/s HUD" tests the cold-read throughput meter (laundry 0e46fa8), which is NOT in this batch, so it failed -- and because the describe block is serial its failure also stopped "#6/#13 editor" (which IS in this batch) from running at all. Removed, with a note to restore it alongside the feature. 2. "#13 zoom persists on scrub" never verified anything. It read the visible span by scraping numeric axis tick labels, got NaN both times, and the comparison was wrapped in `if (Number.isFinite(...))` -- so the assertion silently never ran, for as long as the test has existed, while the test name claimed the coverage. Attempts to give it a real handle on the view state (anyplotlib's _aplModel; the figure page's __apl_viewStateJson / __apl_panelDiag, both confirmed present in a normal MDI figure frame by probe) all still read NaN from inside the editor. Rather than leave a green test that checks nothing, the claim is now an explicit test.fixme naming the gap, and the enclosing test was renamed to drop the coverage it does not provide. What #6/#13 DOES verify is unchanged and real: the overlay stays inside the editor, the source MDI window is hidden while it is open and restored after, and the editor reopens clean. Left the decisive clue in the comment: screenshot 21-movie-editor-overlay.png shows the editor figure rendering axis ticks, so the tick fallback should find them and the failure is in resolving the nested spyde-fig:// frame, not in the page content. 13 passed, 1 fixme.
Member
Author
Real-app verification: 13 passed, 1 fixme
Clue left for whoever picks it up: What is genuinely verified
Not verified
Screenshots in |
Center Zero Beam now draws its search box as soon as the wizard opens (c22e1a1), rather than drawing nothing until a half-width was typed. The test sampled yellow pixels, filled the half-width field, and required the count to INCREASE -- exactly backwards once the box is already there and a smaller half-width makes it smaller. That is the shard-3 e2e failure on this branch; shard 3 is green on the branches that do not touch CZB. Asserts the "drawn on open" half only, which IS the behaviour change. The shrink half is deliberately not asserted here and the reason is worth recording: ui_fixes runs serially, so by test 9 the DP also carries a yellow line-profile ROI from an earlier test, and countYellowPixels() counts both colours by design -- the ROI's pixels swamp the box's, so the total need not fall when the box shrinks. Two earlier attempts to assert it here failed for exactly that reason (first with half-width 30, which on this fixture's 32x32 DP clips back to the full frame anyway; then with 6). The full contract -- appears at full frame, shrinks with half-width, tears down on close -- is covered on a CLEAN app in laundry_visual.spec.ts "#10 CZB Automatic", which passes.
Member
Author
CI green (21/21) after
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The clean UX subset of
feat/laundry-list-2026-07-22, cherry-picked onto post-#37 main.79f01900aac8916d7df86c22e1a1095a3aeb71664c0d1e6baTwo corrections to the original triage
01131d9(shared-kernel wiring) is a prerequisite, not an optional extra. It was filed as "maybe"; without it the renderer does not compile —metadataEditableandcomputingWindowsare declared there and consumed by the Calculating chip and the metadata editor. It is small and self-contained: 2 renderer files +session.py, 129 lines.d4058b1(deregister a signal plot''s navigator selector on window close) is omitted — it conflicted on cherry-pick because it already landed as Deregister a signal plot's navigator selector on window close #38.Deliberately not included
The data-access commits (
5bfd9a4integration frame cache,6811f3ccache-aware classification + 1 GB budget,9303093contiguous-block span read + GPU region sum) are superseded by #42, which does the same four things inspyde/array_cache/with parity tests and real-app numbers. The one idea that looked complementary — the contiguous-block span read for the uncached edge of a 1-D span — was measured and does not pay on this hardware:Windows'' prefetcher already treats contiguous frame-by-frame memmap access as sequential, and the single big slice needs a 512 MB contiguous allocation, which is why the cold case regresses. An explicit
os.readinto a buffer was 2.4× slower than the memmap.Also deferred as planned: the env-gated MRC sequential-read port (
f41013d), the Storage Benchmark UI (8a8b5ff), the data-access benchmark suite, and the MB/s HUD (0e46fa8).Verification
tscclean; 1643 passed, 2 skipped, 1 xfailed. These are five UI features, so headless green is not verification —laundry_visual.spec.tsrun and screenshots to follow in a comment.