diff --git a/python_app/gui/app_window.py b/python_app/gui/app_window.py index 92422e3..a27b630 100644 --- a/python_app/gui/app_window.py +++ b/python_app/gui/app_window.py @@ -285,7 +285,6 @@ class AppWindow( self._bscan_cpp_replay_window = bscan_cpp_replay_window self._bscan_history_by_combo = {} self._bscan_depth_axis_by_combo = {} - self._bscan_history_floor_collection_id = 0 self._bscan_render_signature = None # Writer into the processor's input ring, used to re-feed retained sweeps so the # whole visible B-scan is recomputed. Opened lazily, only while acquisition is diff --git a/python_app/gui/controllers/app_window_config/live_processing_mixin.py b/python_app/gui/controllers/app_window_config/live_processing_mixin.py index 41e0148..8fd3c14 100644 --- a/python_app/gui/controllers/app_window_config/live_processing_mixin.py +++ b/python_app/gui/controllers/app_window_config/live_processing_mixin.py @@ -511,7 +511,6 @@ class AppWindowLiveProcessingMixin: def _clear_history_mode_caches(self) -> None: """Drop cached render state for pass-through, B-scan, and GPR views.""" - self._bscan_history_floor_collection_id = 0 self._clear_bscan_plot_history() if hasattr(self, "_bscan_plot"): self._bscan_plot.clear() diff --git a/python_app/gui/controllers/app_window_pipeline_mixin.py b/python_app/gui/controllers/app_window_pipeline_mixin.py index 5cbd2c7..12c4bcb 100644 --- a/python_app/gui/controllers/app_window_pipeline_mixin.py +++ b/python_app/gui/controllers/app_window_pipeline_mixin.py @@ -689,7 +689,6 @@ class AppWindowPipelineMixin: def _reset_runtime_history(self) -> None: """Reset runtime history and B-scan caches.""" self._replace_runtime_history(retained_raw=[], retained_pre=[], retained_result=[]) - self._bscan_history_floor_collection_id = 0 self._discard_bscan_replay_results() self._clear_history_mode_caches() self._update_history_indicator() diff --git a/python_app/gui/controllers/app_window_plot/bscan_plot_mixin.py b/python_app/gui/controllers/app_window_plot/bscan_plot_mixin.py index de2eb11..1213ca2 100644 --- a/python_app/gui/controllers/app_window_plot/bscan_plot_mixin.py +++ b/python_app/gui/controllers/app_window_plot/bscan_plot_mixin.py @@ -16,17 +16,19 @@ def _result_tail( *, result_history: list[ResultCollection], history_limit: int, - floor_collection_id: int, ) -> list[ResultCollection]: - """Return filtered and de-duplicated result-history tail for B-scan usage.""" - filtered = [ - collection - for collection in result_history[-history_limit:] - if int(collection.collection_id) > int(floor_collection_id) - ] + """Return the de-duplicated newest `history_limit` entries for B-scan usage. + + Selection is purely positional. An earlier version also dropped entries below a + `collection_id` floor, which cannot work here: ids are neither dense (the results + ring overwrites unread slots) nor monotonic across a run boundary (the C++ side + numbers from 1 again). Given the floor was derived from the first entry of this + very slice, the comparison provably removed nothing when ids ascend, and removed + exactly the newest frames when they do not. + """ unique_reversed_tail: list[ResultCollection] = [] seen_keys: set[tuple[int, int]] = set() - for collection in reversed(filtered): + for collection in reversed(result_history[-history_limit:]): key = (int(collection.collection_id), int(collection.monotonic_ns)) if key in seen_keys: continue @@ -43,13 +45,11 @@ def build_bscan_signature( subtract_mean_ascan_enabled: bool, result_history: list[ResultCollection], history_limit: int, - floor_collection_id: int, ) -> tuple[object, ...]: """Build deterministic signature used to detect B-scan cache invalidation.""" result_tail = _result_tail( result_history=result_history, history_limit=history_limit, - floor_collection_id=floor_collection_id, ) return ( int(history_limit), @@ -61,7 +61,6 @@ def build_bscan_signature( float(live_config.bscan_start_freq_mhz), float(live_config.bscan_stop_freq_mhz), bool(subtract_mean_ascan_enabled), - int(floor_collection_id), tuple((int(collection.collection_id), int(collection.monotonic_ns), len(collection.blocks)) for collection in result_tail), ) @@ -82,7 +81,6 @@ def apply_mean_ascan_subtraction( def rebuild_bscan_history_from_results( result_history: list[ResultCollection], history_limit: int, - floor_collection_id: int, stats: dict[str, object] | None = None, ) -> tuple[dict[tuple[int, int], deque[np.ndarray]], dict[tuple[int, int], np.ndarray]]: """Rebuild B-scan history and depth axes from processed result payloads. @@ -98,7 +96,6 @@ def rebuild_bscan_history_from_results( result_tail = _result_tail( result_history=result_history, history_limit=history_limit, - floor_collection_id=floor_collection_id, ) axis_resets = 0 without_bscan = 0 @@ -305,7 +302,6 @@ class AppWindowBscanPlotMixin: def _sync_bscan_history_from_results(self) -> None: """Rebuild B-scan history cache when live params or inputs changed.""" - self._advance_bscan_floor_to_display_window() signature = self._bscan_signature() if signature == self._bscan_render_signature: return @@ -317,13 +313,12 @@ class AppWindowBscanPlotMixin: live_config = self._live_processing_config() # Must read the same source the rebuild will, or the cache decides nothing # changed while the image would in fact be built from different collections. - result_history, from_replay = self._bscan_source_collections() + result_history, _from_replay = self._bscan_source_collections() return build_bscan_signature( live_config=live_config, subtract_mean_ascan_enabled=bool(self._bscan_subtract_mean_ascan.isChecked()), result_history=result_history, history_limit=self._bscan_display_window_scans(), - floor_collection_id=0 if from_replay else self._bscan_history_floor_collection_id, ) def _bscan_source_collections(self) -> tuple[list[ResultCollection], bool]: @@ -332,7 +327,7 @@ class AppWindowBscanPlotMixin: A completed replay is the better source: it is exactly `window` long and every entry went through the processor with the same settings. The runtime history is not usable right after one, because results whose ids it never held are appended - out of order and the positional floor then cuts them away again. + out of order, leaving the deque unsorted for the rest of the session. """ replayed = getattr(self, "_bscan_replay_results", None) if replayed: @@ -343,20 +338,16 @@ class AppWindowBscanPlotMixin: """Recompute B-scan history cache from results history buffer.""" result_history, from_replay = self._bscan_source_collections() window = self._bscan_display_window_scans() - # A replay already selected the window; re-applying the floor would cut it again. - floor_collection_id = 0 if from_replay else int(self._bscan_history_floor_collection_id) stats: dict[str, object] = {} history_by_combo, depth_axis_by_combo = rebuild_bscan_history_from_results( result_history=result_history, history_limit=window, - floor_collection_id=floor_collection_id, stats=stats, ) self._bscan_history_by_combo = history_by_combo self._bscan_depth_axis_by_combo = depth_axis_by_combo self._log_bscan_window_shortfall( window=window, - floor_collection_id=floor_collection_id, result_history_len=len(result_history), from_replay=from_replay, history_by_combo=history_by_combo, @@ -367,7 +358,6 @@ class AppWindowBscanPlotMixin: self, *, window: int, - floor_collection_id: int, result_history_len: int, from_replay: bool, history_by_combo: dict[tuple[int, int], deque[np.ndarray]], @@ -375,9 +365,9 @@ class AppWindowBscanPlotMixin: ) -> None: """Explain at DEBUG why the image holds fewer columns than were requested. - Four independent causes produce the same symptom, so each is reported as its + Three independent causes produce the same symptom, so each is reported as its own number rather than a single verdict: - * `result history` / `tail` — the run simply produced fewer sweeps; + * `tail` — the run simply produced fewer sweeps than were asked for; * `without_bscan` — frames processed in another mode carry no bscan payload; * `axis_resets` — a changed depth axis restarted the deque, dropping every column gathered before it (the usual cause after a partial replay); @@ -394,7 +384,7 @@ class AppWindowBscanPlotMixin: self._log_debug( f"B-scan window not filled: rendered={rendered} of requested={window}. " f"source={'replay' if from_replay else 'result history'} len={result_history_len}, " - f"above floor(id>{floor_collection_id})={stats.get('tail')}, " + f"newest-{window} tail={stats.get('tail')}, " f"of those without a bscan payload={stats.get('without_bscan')}, " f"depth-axis resets={stats.get('axis_resets')}. " f"Per combo: {per_combo or 'none'}." @@ -483,36 +473,6 @@ class AppWindowBscanPlotMixin: self._bscan_depth_axis_by_combo.clear() self._bscan_render_signature = None - def _advance_bscan_floor_to_display_window(self) -> None: - """Clamp B-scan source history to the active display window. - - The window counts RETAINED ENTRIES, so the floor is read off the n-th - newest entry rather than computed as `latest_id - window`. Collection ids - are not dense: the results ring overwrites unread slots when the producer - outruns the GUI poll loop, so the GUI keeps ids like 1..50, 81..130, ... - Subtracting the window from the newest id would then span far fewer than - `window` entries — asking for 150 sweeps yielded 87. - - Recomputed unconditionally rather than ratcheted upwards: widening the - window in stopped mode must be able to LOWER the floor and bring older - frames back into view. A stale floor cannot survive this way either, so - the previous special case for collection ids restarting on a new C++ run - is no longer needed. - """ - history = self._result_history - if not history: - return - - window = self._bscan_display_window_scans() - if len(history) <= window: - self._bscan_history_floor_collection_id = 0 - return - - # `_result_tail` keeps entries with `collection_id > floor`, so sit the - # floor one below the oldest entry that still fits in the window. - oldest_visible = history[len(history) - window] - self._bscan_history_floor_collection_id = max(0, int(oldest_visible.collection_id) - 1) - def _ensure_phase_view_box(self) -> pg.ViewBox: """Create or return secondary right-axis ViewBox for phase curves.""" plot_item = self._bscan_plot.getPlotItem() diff --git a/python_app/gui/controllers/app_window_plot/bscan_replay_mixin.py b/python_app/gui/controllers/app_window_plot/bscan_replay_mixin.py index 691ed64..543cec2 100644 --- a/python_app/gui/controllers/app_window_plot/bscan_replay_mixin.py +++ b/python_app/gui/controllers/app_window_plot/bscan_replay_mixin.py @@ -128,9 +128,9 @@ class AppWindowBscanReplayMixin: # The two histories are fed by different rings that drop independently, so the # sweeps we re-sent only partly overlap the results already on record. The # non-overlapping ones get appended to the deque even though their ids are old, - # which breaks its ordering — and the positional floor then cuts exactly those - # off again. Observed: 300 re-sent, 300 recovered, 145 drawn. What came back is - # by construction the right count and uniformly processed, so use it directly. + # so its newest `window` entries are a mix of freshly and stale-processed + # frames. What came back is by construction the right count and uniformly + # processed, so use it directly. self._bscan_replay_results = replayed # Replayed collections keep their original ids and timestamps, so the render diff --git a/python_app/gui/controllers/app_window_snapshot_mixin.py b/python_app/gui/controllers/app_window_snapshot_mixin.py index f18f123..de7e876 100644 --- a/python_app/gui/controllers/app_window_snapshot_mixin.py +++ b/python_app/gui/controllers/app_window_snapshot_mixin.py @@ -241,7 +241,6 @@ class AppWindowSnapshotMixin: retained_pre=retained_pre, retained_result=retained_result, ) - self._bscan_history_floor_collection_id = 0 self._discard_bscan_replay_results() self._clear_history_mode_caches() diff --git a/python_app/tests/test_bscan_history_window.py b/python_app/tests/test_bscan_history_window.py index c23461d..f7a1e2b 100644 --- a/python_app/tests/test_bscan_history_window.py +++ b/python_app/tests/test_bscan_history_window.py @@ -2,12 +2,13 @@ The B-scan used to be pinned to the C++ replay window (~50 sweeps) by two separate mechanisms: the render-side history limit and a monotonically rising -`_bscan_history_floor_collection_id`. Widening only the first would have changed -nothing, because the floor kept filtering older collections out for good. +`collection_id` floor. Widening only the first would have changed nothing, because +the floor kept filtering older collections out for good. -These tests pin the two properties that make the stopped-mode review work: the -window follows acquisition state, and the floor is recomputed (not ratcheted) so a -widened window can bring already-discarded frames back into view. +The floor is gone: selection is positional, which is the only criterion that holds +when ids are sparse (the results ring drops) or restart from 1 (a new C++ run). +These tests pin the observable consequences — how many columns end up on screen — +rather than any internal counter. """ from __future__ import annotations @@ -22,6 +23,7 @@ os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") from PyQt6.QtWidgets import QApplication # noqa: E402 from python_app.gui.app_window import AppWindow # noqa: E402 +from python_app.gui.runtime.history import record_result_history # noqa: E402 from python_app.models.dataset_model import ( # noqa: E402 ComboKey, ResultBlock, @@ -60,12 +62,12 @@ class BscanDisplayWindowTest(unittest.TestCase): self.w = _window self._original_is_running = self.w._supervisor.is_running self.w._result_history.clear() - self.w._bscan_history_floor_collection_id = 0 + self.w._bscan_render_signature = None def tearDown(self) -> None: self.w._supervisor.is_running = self._original_is_running self.w._result_history.clear() - self.w._bscan_history_floor_collection_id = 0 + self.w._bscan_render_signature = None def _set_running(self, running: bool) -> None: self.w._supervisor.is_running = lambda: running @@ -86,44 +88,73 @@ class BscanDisplayWindowTest(unittest.TestCase): self.w._bscan_history_window.setValue(300) self.assertEqual(self.w._bscan_display_window_scans(), 300) - def test_widening_the_window_lowers_the_floor(self) -> None: - # The regression this whole change exists for: a run leaves the floor high, - # and widening the window afterwards must pull it back down. + def test_widening_the_window_brings_older_frames_back(self) -> None: + # The regression this whole change exists for: a live run renders 50 columns, + # and widening the window after Stop must reach back over the retained history + # rather than stay pinned to whatever the live path last drew. self._fill_history(300) self._set_running(True) - self.w._advance_bscan_floor_to_display_window() - raised_floor = self.w._bscan_history_floor_collection_id - self.assertEqual(raised_floor, 300 - self.w._bscan_cpp_replay_window) + self.w._sync_bscan_history_from_results() + self.assertEqual( + len(self.w._bscan_history_by_combo[(0, 0)]), self.w._bscan_cpp_replay_window + ) self._set_running(False) self.w._bscan_history_window.setValue(300) - self.w._advance_bscan_floor_to_display_window() - self.assertEqual(self.w._bscan_history_floor_collection_id, 0) + self.w._sync_bscan_history_from_results() + self.assertEqual(len(self.w._bscan_history_by_combo[(0, 0)]), 300) - def test_floor_survives_collection_ids_restarting(self) -> None: - # A new C++ run restarts ids from 1. The unconditional recompute must not - # leave a stale high floor that hides the whole fresh run. + def test_restarted_collection_ids_do_not_hide_the_fresh_run(self) -> None: + # Regression: Start after a stopped review made the image melt to 0 columns and + # then snap back to 50. A new C++ run numbers from 1, so entries that are newest + # by position carry the smallest ids; any id-based cut-off derived from the old + # run rejected exactly them. self._fill_history(300) - self._set_running(True) - self.w._advance_bscan_floor_to_display_window() - self.assertGreater(self.w._bscan_history_floor_collection_id, 0) + self._set_running(False) + self.w._bscan_history_window.setValue(300) + self.w._sync_bscan_history_from_results() - self.w._result_history.clear() - self._fill_history(5) - self.w._advance_bscan_floor_to_display_window() - self.assertEqual(self.w._bscan_history_floor_collection_id, 0) + self._set_running(True) + window = self.w._bscan_cpp_replay_window + for fresh in range(1, window + 1): + self.w._result_history.append(_bscan_result(fresh)) + self.w._sync_bscan_history_from_results() + self.assertEqual(len(self.w._bscan_history_by_combo[(0, 0)]), window) def test_window_counts_entries_not_collection_ids(self) -> None: # The results ring overwrites unread slots when the producer outruns the GUI - # poll loop, so retained collection ids are sparse. A floor of - # `latest_id - window` then spans far fewer than `window` entries: this is - # exactly the case where asking for 150 sweeps rendered only 87. + # poll loop, so retained collection ids are sparse. Counting in ids rather than + # in entries is exactly the case where asking for 150 sweeps rendered only 87. self._fill_history(300, id_step=3) self._set_running(False) self.w._bscan_history_window.setValue(150) self.w._sync_bscan_history_from_results() self.assertEqual(len(self.w._bscan_history_by_combo[(0, 0)]), 150) + def test_reprocessed_results_replace_rather_than_duplicate_columns(self) -> None: + # The processor's recompute is triggered by feeding sweeps back through it, and + # it republishes them under their ORIGINAL ids. Two independent mechanisms keep + # that from doubling every column, and neither may be confused with the removed + # `collection_id` floor: a threshold cannot tell a duplicate from its original, + # since they share the id. Deduplication is by key equality. + self._set_running(False) + self.w._bscan_history_window.setValue(300) + self._fill_history(200) + self.w._sync_bscan_history_from_results() + self.assertEqual(len(self.w._bscan_history_by_combo[(0, 0)]), 200) + + # Intake: a recomputed collection replaces the entry holding the same key. + for collection in list(self.w._result_history): + record_result_history(self.w._result_history, _bscan_result(collection.collection_id)) + self.assertEqual(len(self.w._result_history), 200) + + # Render: a duplicate that reached the deque by another path is still collapsed, + # keeping the newer of the two. + self.w._result_history.append(_bscan_result(200)) + self.w._bscan_render_signature = None + self.w._sync_bscan_history_from_results() + self.assertEqual(len(self.w._bscan_history_by_combo[(0, 0)]), 200) + def test_rebuild_renders_the_full_widened_window(self) -> None: self._fill_history(300) self._set_running(False) diff --git a/python_app/tests/test_bscan_replay.py b/python_app/tests/test_bscan_replay.py index ef18c5c..e205a3b 100644 --- a/python_app/tests/test_bscan_replay.py +++ b/python_app/tests/test_bscan_replay.py @@ -225,9 +225,9 @@ class ReplayGuardTest(unittest.TestCase): `_pre_history` and `_result_history` come from two rings that drop independently, so the re-sent sweeps only partly overlap the recorded results. - The non-overlapping ones get appended to the deque under old ids, breaking its - ordering, and the positional floor then cuts exactly those away. Rendering from - the replayed set instead sidesteps the whole problem. + The non-overlapping ones get appended to the deque under old ids, leaving its + newest `window` entries a mix of freshly and stale-processed frames. Rendering + from the replayed set instead sidesteps the whole problem. """ window = 30 replayed = [_bscan_result(cid) for cid in range(9000, 9000 + window)] diff --git a/python_app/tests/test_processing.py b/python_app/tests/test_processing.py index c27a7b4..fcbb899 100644 --- a/python_app/tests/test_processing.py +++ b/python_app/tests/test_processing.py @@ -219,7 +219,7 @@ class RebuildBscanHistoryTest(unittest.TestCase): self._bscan_collection(1, (0, 0), [1.0, 2.0], [10.0, 20.0]), self._bscan_collection(2, (0, 0), [1.0, 2.0], [11.0, 21.0]), ] - by_combo, axes = rebuild_bscan_history_from_results(history, history_limit=10, floor_collection_id=0) + by_combo, axes = rebuild_bscan_history_from_results(history, history_limit=10) self.assertEqual(len(by_combo[(0, 0)]), 2) self.assertTrue(np.array_equal(axes[(0, 0)], np.array([1.0, 2.0], dtype=np.float32))) @@ -229,7 +229,7 @@ class RebuildBscanHistoryTest(unittest.TestCase): self._bscan_collection(2, (0, 0), [1.0, 2.0], [1.0, 2.0], kind=2), # wrong kind self._bscan_collection(3, (0, 0), [1.0, 2.0, 3.0], [1.0, 2.0]), # size mismatch ] - by_combo, _ = rebuild_bscan_history_from_results(history, history_limit=10, floor_collection_id=0) + by_combo, _ = rebuild_bscan_history_from_results(history, history_limit=10) self.assertEqual(by_combo, {}) def test_depth_axis_change_resets_history(self) -> None: @@ -237,17 +237,21 @@ class RebuildBscanHistoryTest(unittest.TestCase): self._bscan_collection(1, (0, 0), [1.0, 2.0], [10.0, 20.0]), self._bscan_collection(2, (0, 0), [1.0, 2.0, 3.0], [11.0, 21.0, 31.0]), # new depth axis ] - by_combo, axes = rebuild_bscan_history_from_results(history, history_limit=10, floor_collection_id=0) + by_combo, axes = rebuild_bscan_history_from_results(history, history_limit=10) self.assertEqual(len(by_combo[(0, 0)]), 1) # reset on axis change; only the latest sweep remains self.assertEqual(axes[(0, 0)].shape, (3,)) - def test_floor_collection_id_excludes_older(self) -> None: + def test_selection_is_positional_not_by_collection_id(self) -> None: + # Ids are neither dense nor monotonic across a run boundary, so the tail is + # taken by position only. Here the newest two entries carry the SMALLEST ids; + # an id-based cut-off would have dropped exactly them. history = [ - self._bscan_collection(1, (0, 0), [1.0], [10.0]), - self._bscan_collection(2, (0, 0), [1.0], [20.0]), + self._bscan_collection(cid, (0, 0), [1.0], [float(cid)]) + for cid in (98, 99, 100, 1, 2) ] - by_combo, _ = rebuild_bscan_history_from_results(history, history_limit=10, floor_collection_id=1) - self.assertEqual(len(by_combo[(0, 0)]), 1) # only collection_id > 1 + by_combo, _ = rebuild_bscan_history_from_results(history, history_limit=3) + self.assertEqual(len(by_combo[(0, 0)]), 3) + self.assertEqual([sweep[0] for sweep in by_combo[(0, 0)]], [100.0, 1.0, 2.0]) # --------------------------------------------------------------------------- #