fix(nav): skip the transient deepcopy placeholder on .data; e2e harness never fails blind - #126
Merged
Merged
Conversation
…ta — skip it The console-preview flavour of the second-signal IndexError (console_preview spec CI: 'NAV-DEBUG eager index RAISED: indices=[0, 0] data.shape=(1,) nav_shape=(6, 6)'). Every hyperspy signal operation (arithmetic, comparison, sum, deepcopy) goes through BaseSignal._deepcopy_with_new_data, which TRANSIENTLY rebinds self.data = None on the live signal object while it deep-copies — the data setter turns that into array([None], dtype=object), shape (1,). The math console evaluates user expressions (s1 + 0 — the eye-toggle live preview, re-run on every nav commit via NAV_CHANGE_HOOKS) against the SAME bound root-signal objects on the console thread, so a navigator update on the serial dispatcher thread can land inside that window and index the placeholder with 2-D nav coordinates. _pending_future_data cannot catch it: data[0] is None, not a future. Reproduced at 119 errors per 300 moves with a real 'root + 0' hammer thread. Fix at the read boundary, dispatcher model untouched (no locks, no generation counters): capture .data ONCE per read, skip the frame when the captured binding cannot satisfy the nav indices (_nav_readable_data — the last good frame stays up, exactly like the pending-future skip), and clamp + index that SAME captured reference (threaded through _prepare_nav_indices via the new optional data=) so a post-capture rebind still reads the coherent pre-swap array. The eager-branch ERROR tripwire stays: with the transient structurally excluded, anything that still fires it is a genuine new bug.
Three pins in test_nav_second_signal_race.py, all through the REAL serial dispatcher path on the eager 4-D fixture: - the parked array([None]) placeholder (exactly what _deepcopy_with_new_data leaves on .data mid-copy) is skipped with no ERROR log and the last good frame stays painted, and the SAME position paints once the window closes; - a direct update_from_navigation_selection call mid-window returns None instead of raising; - hammering 'root + 0' (the expression the console live preview evaluates against the live bound signal) on a side thread across 150 dispatcher moves never logs a nav-read ERROR (119/300 before the fix).
Main's sped_ag_grid failure reported only 'Target page, context or browser has been closed' — the app died and nothing about why reached the CI log (backend logging does not reach Playwright stdout unless SPYDE_LOG_LEVEL is set, and nothing dumped the harness logBuffer on failure). Three additions, no restructure: - launchApp defaults SPYDE_LOG_LEVEL=WARNING when neither the spec's env nor the shell set one, so backend warnings/errors tee to the stderr the harness already captures (app.py's env-gated tee). Specs that wait on INFO/DEBUG log lines all pass their own level already. - an explicit close listener: if the Electron process exits (or the window closes) while a test is mid-flight, console.error the exit code + the last ~60 backend log lines immediately, so 'browser has been closed' is always adjacent to the backend's last words. backend.tail(n) exposes the same tail to specs. - app.close() (the spec's finally) marks the close expected and attaches the tail to the test as a Playwright attachment, so any failure's report carries the backend's last words too. Verified: metadata_panel.spec.ts passes under the hardened harness; a scratch spec that killed the app mid-test printed the new '[harness] Electron process exited MID-TEST (code=null, signal=SIGTERM)' dump directly above the resulting timeout in the line reporter.
CSSFrancis
marked this pull request as ready for review
August 8, 2026 00:58
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.
Fixes the
console_previewe2e failure (theNAV-DEBUG eager index RAISED … second-signal IndexErrortripwire) at its mechanism, and makes the Playwright harness dump diagnostics instead of dying with a bare "browser has been closed".The race (reproduced at 119 errors / 300 nav moves, 0 after the fix): every hyperspy signal operation (
+, comparison,sum, deepcopy) passes throughBaseSignal._deepcopy_with_new_data, which transiently rebindsself.data = Noneon the live signal object — the data setter turns that intoarray([None], dtype=object), shape(1,), whileaxes_managerstill reports nav(6,6). The math console evaluates user expressions (s1 + 0; the eye-toggle live preview re-runs on every nav commit viaNAV_CHANGE_HOOKS) against the same bound root-signal objects on the console thread — so a dispatcher-thread nav read landing inside that window indexes the placeholder with 2-D nav coordinates. The pending-future guard can't catch it (data[0]isNone, not a future). Note this bites ANY console expression on a bound live signal, not just the preview.Fix — read boundary only; the serial-dispatcher/latest-wins design is untouched (no locks, no counters):
update_from_navigation_selectioncaptures.dataonce per read;_nav_readable_dataskips the frame when the captured binding can't satisfy the nav indices (last good frame stays up, exactly like the pending-future skip); the clamp (_prepare_nav_indices, new optionaldata=) and the eager index use that same captured reference, so even a post-capture rebind reads the coherent pre-swap array. The ERROR tripwire deliberately stays at ERROR: with the transient structurally excluded, anything that still fires it is a genuine new bug. One reviewer note: a size>1 object ndarray of futures would now skip rather than paint the loading checkerboard — per_pending_future_data's own docs that shape doesn't occur (futures come bare or as length-1 object arrays, both already skipped before this guard).New test:
test_nav_second_signal_race.py— parks the placeholder exactly as hyperspy does and drives the real dispatcher (no ERROR, last-good-frame retained, same position paints after the window closes); plus aroot + 0hammer × 150 dispatcher moves asserting zero ERRORs.Harness hardening (
electron/tests/_harness.cjs, +57 lines):launchAppdefaultsSPYDE_LOG_LEVEL=WARNING(audited: every spec that greps log lines sets its own level); an Electron process-exit listener reports mid-test death with exit code + the last ~60 backend log lines;app.close()marks expected closes and attaches the backend-log tail to the Playwright report on failures. Verified: typecheck green;metadata_panel.spec.tsgreen under the hardened harness; a scratch spec that killed the app mid-flight printed the new dump directly above the timeout (scratch deleted).Verification: new test +
test_console.py40 passed; console_preview/nav_cached_read/navigator_race batch 83 passed; nav-read regression batch (tiered classify, paint decouple, async cancel, region cap, shm, rebin) 47 passed;test_overlay_layers.py16 passed.Pre-existing, unrelated, noted for visibility:
dock_compact.spec.tsfails locally with a 6-px dock-body overflow on this box.