Adversarial review: fix scoped-filter replay, browse async seams, edge detection#22
Merged
Merged
Conversation
…olver injectable Adversarial review findings in the GUI-state -> ProcessingState -> replay path, each pinned by tests in the new tests/test_processing_scope_seams.py: - Scope replay ordering (the high-severity finding): committed roi_filter_ops/mask_filter_ops always replayed before geometric_ops, so a quick-selection or ROI-scoped filter committed after a flip/rotate landed at the mirrored location (frozen geometry captured in display coordinates, replayed in the raw frame). Commit paths now stamp after_geometric_ops (the geometric-op count at commit time) and processing_state_from_gui interleaves scope steps at that position. Legacy entries without the key keep the historical pre-geometric position so old sidecars replay byte-identically. - ROI-scoped image arithmetic now freezes geometry + position at commit (parity with scoped filters); legacy roi_id-only entries keep live resolution after geometric_ops. - operand_resolver is forwarded into the nested roi/mask recursion, so scoped image arithmetic honours injected in-memory resolvers instead of falling back to disk I/O. - Malformed frozen_geometry/frozen_mask snapshots now block export: _roi_from_frozen_geometry validates the kind and probes rasterisation (ROI.to_mask silently rasterises unknown kinds to all-False), and missing_roi_references reports unreconstructible snapshots; apply-time skips warn instead of being silent. - rotate_arbitrary's warning no longer claims ROI steps "have been skipped" (they never were); it now fires only for live-resolving roi steps after the rotation, the genuinely frame-ambiguous case. Co-Authored-By: Claude Fable 5 <[email protected]>
…calibration Adversarial stress findings, pinned by tests/test_edge_detection_adversarial.py: - A constant non-zero image produced an ALL-TRUE edge mask: Sobel leaves an O(eps*|data|) float-cancellation residue (~2e-16 for a flat 3.7) that defeated the strict "magnitude > 0.0" guard, making the percentile cut ~2e-16. The guard is now a relative floor scaled by data magnitude and the pixel-size division, so a genuine 1e-12-amplitude step is still detected while flat images of any value stay empty. - ROI-restricted Canny over-detected ~2x (432 vs 218 edge pixels on identical data): skimage computes quantile thresholds over the whole gradient-magnitude array, so near-zero gradient outside a small ROI diluted the cut. Percentile thresholds are now computed over the restricted region (mirroring skimage's smoothed ndi.sobel magnitude) and passed to skimage as absolute values. - gradient_magnitude was 2x the true physical slope: skimage's normalised Sobel/Scharr kernels respond 2.0 on a unit-slope ramp (two-pixel difference span, no /2). Responses are halved before the pixel-size division so the docstring's promise of real dz/dx, dz/dy holds. Orientation and all relative/normalised outputs are unchanged. Co-Authored-By: Claude Fable 5 <[email protected]>
…bility Adversarial review of the async browse path, pinned by the new tests/test_browse_worker_seams.py and test_browse_metadata_display.py: - Worker emit guarantees: render_scan_image was unguarded in ThumbnailLoader, FolderThumbnailLoader, and ChannelPreviewLoader's per-plane loop (and ChannelLoader) — a render exception escaped work() with zero or partial emits, leaving cards/panels stuck on their loading placeholder forever (QThreadPool swallows the exception). Failures now emit null-QImage placeholders / None slots, one per owed render. - Orphaned card-build queue: appearance changes (colormap/channel/ align-rows) replace _load_token to drop in-flight renders, which also cancelled the timer-sliced card build — the tail cards of a large folder were never constructed and the background thumbnail timer polled forever. The build now has its own _card_build_token, reset only when the grid re-renders; _rerender_scan_thumbnails also keeps not-yet-built entries pending so they thumbnail with the new appearance once built. - Navigation-failure consistency: a failed folder index left _current_dir on the failed path while the old folder's entries stayed displayed — refresh() retargeted the failed folder and the next navigation pushed it onto Back history. current_dir is now restored to the rendered folder, the failed push is dropped, and the breadcrumb resynced. Re-renders also announce their selection reset via selection_changed(0). - Cache durability: _process_file cached load_error items keyed by (path, mtime, size), pinning transient read failures (network hiccup, file locked by the acquisition software) as permanently broken until the file's mtime changed. Only healthy items and the unrecognised-file None are cached now; failures retry on the next visit. - STM vs qPlus display chain verified end-to-end (item -> SxmFile unit conversions -> card/info-panel strings): a constant-delta-f setpoint can never surface as a pA current, negative delta-f survives, and error items degrade to explicit "?" placeholders. Co-Authored-By: Claude Fable 5 <[email protected]>
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.
Summary
Fixes for 10 confirmed bugs found in an adversarial seam review of the recent work (async browse loading, quick selections / scoped processing, edge detection, metadata caching), plus three new adversarial test suites that pin every finding and the guarantees that already held. Suite: 2273 passed, 0 xfailed.
Processing scopes / replay (
state.py,gui_adapter.py, viewer mixins)geometric_ops, so a filter committed after a flip/rotate landed at the mirrored location. Commit paths now stampafter_geometric_opsand the adapter interleaves scope steps at their commit position. Legacy sidecars (no key) replay byte-identically.operand_resolveris forwarded into nested roi/mask recursion (scoped arithmetic no longer falls back to disk I/O past an injected resolver).rotate_arbitrarywarning no longer claims ROI steps were "skipped" (they never were); it fires only for the genuinely frame-ambiguous live-roi-after-rotation case.Edge detection (
edge_detection.py)> 0.0; a 1e-12 step is still detected).gradient_magnitudeis now the true physical slope (skimage kernels respond 2.0 per unit slope; halved before pixel-size scaling).Browse async / cache (
workers.py,thumbnail_grid.py,indexing.py)current_dirto the folder actually displayed (was: refresh retargeted the failed path, Back history corrupted).(path, mtime, size), so they never recovered until mtime changed).Test plan
test_processing_scope_seams.py,test_edge_detection_adversarial.py,test_browse_worker_seams.py(registered in conftest GUI gating), plustest_browse_metadata_display.pyand additions totest_browse_cache.py.🤖 Generated with Claude Code