diff --git a/apps/autocal/config/locales/en_US.json b/apps/autocal/config/locales/en_US.json index 213312e..2ad5857 100644 --- a/apps/autocal/config/locales/en_US.json +++ b/apps/autocal/config/locales/en_US.json @@ -684,6 +684,7 @@ "col_range_status": "Range/status" }, "review": { + "checking": "Reading and verifying recordings in the background…", "load_selection": "Load saved analysis…", "confirm_stable": "This interval was checked for stability", "apply_window": "Confirm interval and recompute", diff --git a/apps/autocal/config/locales/zh_CN.json b/apps/autocal/config/locales/zh_CN.json index 7264be3..46e86ff 100644 --- a/apps/autocal/config/locales/zh_CN.json +++ b/apps/autocal/config/locales/zh_CN.json @@ -684,6 +684,7 @@ "col_range_status": "范围／状态" }, "review": { + "checking": "正在后台读取并校验记录，请稍候…", "load_selection": "载入历史分析…", "confirm_stable": "此时间段已检查稳定", "apply_window": "确认选中时间段并重算", diff --git a/apps/autocal/src/orisys_autocal/multipoint.py b/apps/autocal/src/orisys_autocal/multipoint.py index f334492..d497919 100644 --- a/apps/autocal/src/orisys_autocal/multipoint.py +++ b/apps/autocal/src/orisys_autocal/multipoint.py @@ -326,6 +326,20 @@ class Campaign: raise ValueError("the solved pose is outside the stage travel; it is stored but not commanded") return {key: float(pose[key]) for key in ("x", "y", "yaw", "pitch", "z")} + def store_solved_pose(self, site_id: str, pose: dict) -> None: + """Update the current site pose without rewriting past repeat records.""" + if self.is_active and site_id != self.pending_alignment_site: + raise RuntimeError("cannot change a pose during an active trial") + site = self.sites[self._index(site_id)] + record = None if self._document is None else next( + (row for row in self._document["sites"] if row["site_id"] == site_id), None) + if site.pose == pose and (record is None or record["pose"] == pose): + return + site.pose = dict(pose) + if record is not None: + record["pose"] = site.pose + self._save() + def set_force_rotation(self, values: list[float]) -> None: """Store the force-sensor-to-tool rotation. There is no default matrix.""" self._rotation = parse_rotation(list(values)) @@ -623,6 +637,13 @@ class Campaign: elapsed = (time.perf_counter_ns() - self._hold_started_ns) / 1e9 return max(0.0, self._hold_s - elapsed) + @property + def pending_alignment_site(self) -> str: + """The next point awaiting alignment between recorded trials.""" + if self.phase != "liftoff" or self._single_site: + return "" + return self.sites[self._site_index].site_id + def progress(self) -> dict: site = self.sites[self._site_index] if self.sites and self._site_index < len(self.sites) else None step = self._steps[self._step_index] if self._step_index < len(self._steps) else None diff --git a/apps/autocal/src/orisys_autocal/preferences.py b/apps/autocal/src/orisys_autocal/preferences.py index ac28efe..32c988f 100644 --- a/apps/autocal/src/orisys_autocal/preferences.py +++ b/apps/autocal/src/orisys_autocal/preferences.py @@ -274,7 +274,6 @@ _MERGED_BLOCKS = ( "safety_limits", "sensor_setup", "sequence_profile", - "press_axis", ) diff --git a/apps/autocal/src/orisys_autocal/session_review.py b/apps/autocal/src/orisys_autocal/session_review.py index 7bd82ad..2b66caf 100644 --- a/apps/autocal/src/orisys_autocal/session_review.py +++ b/apps/autocal/src/orisys_autocal/session_review.py @@ -520,6 +520,61 @@ def review_session(directory: Path, *, selections: dict | None = None) -> dict: } +def reselect_review(result: dict, selections: dict) -> dict: + """Adjust loaded hold traces; saving still verifies all acquired files afresh.""" + from copy import deepcopy + + updated = dict(result) + updated["sites"] = [{**site, "windows": [dict(window) for window in site["windows"]]} + for site in result["sites"]] + used = set() + for site in updated["sites"]: + for window in site["windows"]: + key = f"{window['trial_id']}:{window['step_index']}" + choice = selections.get(key) + start, end = window["hold_start_host_mono_ns"], window["hold_end_host_mono_ns"] + verified = False + if choice is not None: + used.add(key) + start, end = int(choice["t_start_host_mono_ns"]), int(choice["t_end_host_mono_ns"]) + verified = choice["stability_verified"] is True + if not window["hold_start_host_mono_ns"] <= start < end <= window["hold_end_host_mono_ns"]: + raise ValueError(f"selection outside recorded hold: {key}") + trace = [(stamp, value) for stamp, _, value in window["trace"] if start <= stamp < end] + raw = np.array([value for _, value in trace], dtype=float) + if verified and raw.size < 2: + raise ValueError(f"not enough valid samples to confirm: {key}") + status = window["status"] + if status in {"candidate", "insufficient_samples"}: + status = "candidate" if raw.size >= 2 else "insufficient_samples" + window.update( + t_start_host_mono_ns=start, t_end_host_mono_ns=end, + duration_s=(end - start) / 1e9, samples=int(raw.size), + raw_mean=float(raw.mean()) if raw.size else None, + raw_std=float(raw.std()) if raw.size else None, + raw_drift_per_s=float(np.polyfit( + np.array([(stamp - start) / 1e9 for stamp, _ in trace]), raw, 1)[0]) + if raw.size >= 2 and len({stamp for stamp, _ in trace}) >= 2 else None, + stability_verified=verified, status=status, + ) + training = [w for w in site["windows"] if w["branch"] in {"zero", "load"} + and w["status"] == "candidate"] + site.update(fit=_fit(training), + confirmed_fit=_fit([w for w in training if w["stability_verified"]]), + quality=_quality(site["windows"])) + if updated.get("source_format") == MULTIPOINT_FORMAT: + site.update(_multipoint_site(site, site["windows"])) + site["blockers"] = sorted({ + *[b for b in site["blockers"] if not b.startswith("unverified_window:") and b != "no_fit"], + *[f"unverified_window:{w['step_index']}" for w in site["windows"] if not w["stability_verified"]], + *(["no_fit"] if site["fit"] is None else []), + }) + if used != set(selections): + raise ValueError("selection refers to an unavailable hold window") + updated["selections"] = deepcopy(selections) + return updated + + def load_review(directory: Path, review_path: Path) -> dict: """Recompute saved selections only when the acquired inputs still match.""" saved = json.loads(review_path.read_text(encoding="utf-8")) diff --git a/apps/autocal/src/orisys_autocal/ui/multipoint_panel.py b/apps/autocal/src/orisys_autocal/ui/multipoint_panel.py index ee1873b..90a9c12 100644 --- a/apps/autocal/src/orisys_autocal/ui/multipoint_panel.py +++ b/apps/autocal/src/orisys_autocal/ui/multipoint_panel.py @@ -353,8 +353,7 @@ class MultipointPanel(QWidget): def _store_solved_pose(self, site_id: str, pose: dict) -> None: """Record a fresh solve of a point without retaking the point itself.""" - site = next(item for item in self.campaign.sites if item.site_id == site_id) - site.pose = pose + self.campaign.store_solved_pose(site_id, pose) def _require_same_point(self, site, point, normal) -> None: """Refuse a point the current map no longer puts where the record says. diff --git a/apps/autocal/src/orisys_autocal/ui/record_overview_dialog.py b/apps/autocal/src/orisys_autocal/ui/record_overview_dialog.py index afde61f..ee4775a 100644 --- a/apps/autocal/src/orisys_autocal/ui/record_overview_dialog.py +++ b/apps/autocal/src/orisys_autocal/ui/record_overview_dialog.py @@ -8,7 +8,7 @@ from bisect import bisect_left, bisect_right from orisys_app_shared.i18n import t from PySide6.QtCharts import QChart, QChartView, QLineSeries, QScatterSeries, QValueAxis -from PySide6.QtCore import Qt, QPointF +from PySide6.QtCore import Qt, QPointF, Slot from PySide6.QtGui import QColor, QPainter from PySide6.QtWidgets import ( QComboBox, QDialog, QFileDialog, QHBoxLayout, QHeaderView, QLabel, @@ -24,6 +24,7 @@ from ..record_diagnostics import trial_diagnostics from ..sensor.uv_outline import recorded_outline from .session_review_dialog import SessionReviewDialog from .chart_helpers import add_value_axes, clear_chart +from .review_job import ReviewJob class RecordOverviewPage(QWidget): @@ -32,6 +33,7 @@ class RecordOverviewPage(QWidget): def __init__(self, parent=None): super().__init__(parent) self.overview = None + self._job = None self.directory = Path.cwd() self.content_layout = QVBoxLayout(self) self.open_button = QPushButton(self) @@ -66,8 +68,31 @@ class RecordOverviewPage(QWidget): self.load_session(Path(path)) def load_session(self, directory: Path): + if self._job is not None: + return + self._loading_directory = directory + def read(): + review = review_session(directory) + diagnostics = {trial["trial_id"]: trial_diagnostics(directory, trial) + for trial in review["trials"]} + return review, diagnostics + self.open_button.setEnabled(False) + self.message.setText(t("review.checking")) + self.message.show() + self._job = ReviewJob(read, self._loaded) + self._job.start() + + @Slot(object, object) + def _loaded(self, value, error): + self._job = None + self.open_button.setEnabled(True) + if error is not None: + self.message.setText(t("calibration.error").format(detail=error)) + self.message.show() + return + directory = self._loading_directory try: - overview = RecordOverviewDialog(directory, self) + overview = RecordOverviewDialog(directory, self, review=value[0], diagnostics=value[1]) except (OSError, ValueError, KeyError, TypeError) as exc: self.message.setText(t("calibration.error").format(detail=exc)) self.message.show() @@ -95,11 +120,13 @@ class RecordOverviewDialog(QDialog): if self.isWindow(): super().reject() - def __init__(self, directory: Path, parent=None): + def __init__(self, directory: Path, parent=None, *, review=None, diagnostics=None): super().__init__(parent) self.directory = directory - self.review = review_session(directory) - self.diagnostics = {trial["trial_id"]: trial_diagnostics(directory, trial) for trial in self.review["trials"]} + self._job = None + self.review = review if review is not None else review_session(directory) + self.diagnostics = diagnostics if diagnostics is not None else { + trial["trial_id"]: trial_diagnostics(directory, trial) for trial in self.review["trials"]} self.outline, self.outline_state = recorded_outline(self.review) self._uv_bounds = None self._uv_label = None @@ -520,25 +547,39 @@ class RecordOverviewDialog(QDialog): self._refresh() def _save(self): - try: - path = save_overview(self.review, self.plan) + review, plan = self.review, self.plan + self._start_job("save", lambda: save_overview(review, plan)) + + @Slot(object, object) + def _job_finished(self, value, error): + self._job = None + self.setEnabled(True) + if error is not None: + self.message.setText(t("calibration.error").format(detail=error)) + self.message.show() + elif self._job_kind == "save": self._dirty = False self._show_response() self.message.setText(t("overview.saved")) self.message.show() - self.message.setToolTip(str(path)) - except (OSError, ValueError, KeyError, TypeError) as exc: - self.message.setText(t("calibration.error").format(detail=exc)) - self.message.show() + self.message.setToolTip(str(value)) + else: + self.review, self.plan = value + self._dirty = False + self._refresh() + + def _start_job(self, kind, action): + if self._job is not None: + return + self._job_kind = kind + self.setEnabled(False) + self.message.setText(t("review.checking")) + self.message.show() + self._job = ReviewJob(action, self._job_finished) + self._job.start() def _load(self): path, _ = QFileDialog.getOpenFileName(self, t("overview.load"), str(self.directory / "derived"), "JSON (*.json)") if path: - try: - review, plan = load_overview(self.directory, Path(path)) - self.review, self.plan = review, plan - self._dirty = False - self._refresh() - except (OSError, ValueError, KeyError, TypeError) as exc: - self.message.setText(t("calibration.error").format(detail=exc)) - self.message.show() + directory = self.directory + self._start_job("load", lambda: load_overview(directory, Path(path))) diff --git a/apps/autocal/src/orisys_autocal/ui/review_job.py b/apps/autocal/src/orisys_autocal/ui/review_job.py new file mode 100644 index 0000000..47d3edf --- /dev/null +++ b/apps/autocal/src/orisys_autocal/ui/review_job.py @@ -0,0 +1,27 @@ +"""Run recording reads and integrity checks without blocking Qt's event loop.""" + +from PySide6.QtCore import QObject, QRunnable, QThreadPool, Signal + + +class _Result(QObject): + ready = Signal(object, object) + + +class ReviewJob(QRunnable): + def __init__(self, action, receiver): + super().__init__() + self.action = action + self.result = _Result() + self.result.ready.connect(receiver) + + def start(self): + QThreadPool.globalInstance().start(self) + + def run(self): + try: + value = self.action() + except Exception as exc: + # Worker failures must return to Qt so controls are always restored. + self.result.ready.emit(None, str(exc)) + else: + self.result.ready.emit(value, None) diff --git a/apps/autocal/src/orisys_autocal/ui/session_review_dialog.py b/apps/autocal/src/orisys_autocal/ui/session_review_dialog.py index fc6a175..8162e69 100644 --- a/apps/autocal/src/orisys_autocal/ui/session_review_dialog.py +++ b/apps/autocal/src/orisys_autocal/ui/session_review_dialog.py @@ -6,14 +6,15 @@ from pathlib import Path from orisys_app_shared.i18n import t from PySide6.QtCharts import QChart, QChartView, QLineSeries, QScatterSeries -from PySide6.QtCore import Qt, QPointF +from PySide6.QtCore import Qt, QPointF, Slot from PySide6.QtGui import QColor, QPainter from PySide6.QtWidgets import ( QComboBox, QDialog, QHBoxLayout, QHeaderView, QLabel, QPushButton, QTableWidget, QTableWidgetItem, QVBoxLayout, QDoubleSpinBox, QCheckBox, QFileDialog, QTabWidget, QWidget, ) -from ..session_review import review_session, save_review, load_review +from ..session_review import review_session, save_review, load_review, reselect_review +from .review_job import ReviewJob from .chart_helpers import add_value_axes, clear_chart @@ -30,6 +31,8 @@ class SessionReviewDialog(QDialog): "#holdInspection QPushButton {min-height: 22px; padding: 3px 10px;}") self.review = review if review is not None else review_session(directory) self.directory = directory + self._job = None + self._job_kind = "" layout = QVBoxLayout(self) self.notice = QLabel(t("review.notice"), self) self.notice.setWordWrap(True) @@ -227,9 +230,7 @@ class SessionReviewDialog(QDialog): } row = self.table.currentRow() try: - updated = review_session(self.directory, selections=selections) - if updated["input_sha256"] != self.review["input_sha256"]: - raise ValueError("recorded inputs changed since this review was opened") + updated = reselect_review(self.review, selections) self.review = updated self._show_site() self.table.selectRow(row) @@ -242,11 +243,8 @@ class SessionReviewDialog(QDialog): path, _ = QFileDialog.getOpenFileName(self, t("review.load_selection"), str(self.directory / "derived"), "JSON (*.json)") if path: - try: - self.review = load_review(self.directory, Path(path)) - self._show_site() - except (OSError, ValueError, KeyError, TypeError) as exc: - self.message.setText(str(exc)) + directory = self.directory + self._start_job("load", lambda: load_review(directory, Path(path))) def _show_uv(self, selected: int) -> None: clear_chart(self.uv_chart) @@ -280,8 +278,28 @@ class SessionReviewDialog(QDialog): return "—" if value is None else f"{value:.6g}" def _save(self) -> None: - try: - path = save_review(self.review) - self.message.setText(t("review.saved").format(path=path)) - except (OSError, ValueError) as exc: - self.message.setText(t("calibration.error").format(detail=exc)) + review = self.review + self._start_job("save", lambda: save_review(review)) + + def _start_job(self, kind, action): + if self._job is not None: + return + self._job_kind = kind + self._job = ReviewJob(action, self._job_finished) + self.message.setText(t("review.checking")) + for button in (self.save_button, self.load_button, self.apply_button): + button.setEnabled(False) + self._job.start() + + @Slot(object, object) + def _job_finished(self, value, error): + self._job = None + for button in (self.save_button, self.load_button, self.apply_button): + button.setEnabled(True) + if error is not None: + self.message.setText(t("calibration.error").format(detail=error)) + elif self._job_kind == "load": + self.review = value + self._show_site() + else: + self.message.setText(t("review.saved").format(path=value)) diff --git a/apps/autocal/tests/review_test_helpers.py b/apps/autocal/tests/review_test_helpers.py new file mode 100644 index 0000000..8550357 --- /dev/null +++ b/apps/autocal/tests/review_test_helpers.py @@ -0,0 +1,12 @@ +"""Bounded event-loop waits for recording review tests.""" + +import time + +from PySide6.QtTest import QTest + + +def wait_review(widget): + deadline = time.monotonic() + 10 + while widget._job is not None and time.monotonic() < deadline: + QTest.qWait(10) + assert widget._job is None, "background review did not finish" diff --git a/apps/autocal/tests/test_multipoint_align.py b/apps/autocal/tests/test_multipoint_align.py index 91c2d05..1af4075 100644 --- a/apps/autocal/tests/test_multipoint_align.py +++ b/apps/autocal/tests/test_multipoint_align.py @@ -183,6 +183,7 @@ def test_a_point_cannot_be_aligned_in_the_middle_of_a_run(make_window, tmp_path) def test_next_site_can_be_aligned_between_trials(make_window, tmp_path): + import json window = make_window() panel = window.calibration_panel.multipoint _add_site(panel, window.finger_stack, site_id="P1") @@ -199,9 +200,18 @@ def test_next_site_can_be_aligned_between_trials(make_window, tmp_path): panel._selected = "P2" panel._refresh() assert panel.align_button.isEnabled() + previous = json.loads((panel.campaign.directory / "sites/P1/repeat-001/session.json").read_text()) + window.finger_panel.sensor_z_spin.setValue(12.0) panel._align() assert len(emitted) == 1 + manifest = json.loads((panel.campaign.directory / "session.json").read_text()) + assert manifest["sites"][1]["pose"]["z"] == emitted[0]["z"] + panel.campaign.confirm_liftoff() + repeat = json.loads((panel.campaign.directory / "sites/P2/repeat-001/session.json").read_text()) + assert repeat["pose"] == manifest["sites"][1]["pose"] + assert json.loads((panel.campaign.directory / "sites/P1/repeat-001/session.json").read_text()) == previous panel._selected = "P1" panel._align() assert len(emitted) == 1 + panel.campaign.stop("test finished") diff --git a/apps/autocal/tests/test_preferences_isolation.py b/apps/autocal/tests/test_preferences_isolation.py index 019730d..481c2ee 100644 --- a/apps/autocal/tests/test_preferences_isolation.py +++ b/apps/autocal/tests/test_preferences_isolation.py @@ -15,10 +15,23 @@ from __future__ import annotations from pathlib import Path +import pytest + APP_ROOT = Path(__file__).resolve().parents[1] INSTALLED = APP_ROOT / "preference.json" +@pytest.mark.parametrize("stored", [{"calibrated": True}, {"zero_mm": 12.5, "calibrated": True}]) +def test_loading_incomplete_press_calibration_does_not_invent_measurements(stored): + import json + from orisys_autocal import preferences + from orisys_autocal.motion.pose_move import PressAxis + + Path(preferences.path()).write_text(json.dumps({"press_axis": stored}), encoding="utf-8") + loaded = preferences.load_preferences() + assert PressAxis.from_settings(loaded["press_axis"]).target_for(20) is None + + def test_the_preference_path_is_redirected_under_the_scratch_root(): from orisys_autocal.preferences import path diff --git a/apps/autocal/tests/test_record_overview.py b/apps/autocal/tests/test_record_overview.py index 78cf133..95486c6 100644 --- a/apps/autocal/tests/test_record_overview.py +++ b/apps/autocal/tests/test_record_overview.py @@ -5,6 +5,8 @@ import json from copy import deepcopy import pytest + +from review_test_helpers import wait_review from PySide6.QtWidgets import QPushButton from PySide6.QtGui import QAction @@ -155,11 +157,13 @@ def test_overview_gui_plan_click_trial_filter_and_saved_roundtrip(qt_app, overvi qt_app.processEvents() assert dialog.grab().save(str(tmp_path / "overview-response.png")) dialog.findChild(QPushButton, "overview_save").click() + wait_review(dialog) saved = next(root.glob("derived/*/review.json")) dialog.findChild(QAction, "overview_clear_plan").trigger() assert dialog.table.rowCount() == 3 monkeypatch.setattr(record_overview_dialog.QFileDialog, "getOpenFileName", lambda *args: (str(saved), "")) dialog.findChild(QAction, "overview_load").trigger() + wait_review(dialog) assert dialog.table.rowCount() == 4 dialog.close() @@ -174,6 +178,7 @@ def test_calibration_review_action_opens_records_tab(make_window, overview_sessi window.calibration_panel.review_button.click() assert window.workspace.current_key() == "records" page = window.workspace.page("records") + wait_review(page) assert len(page.overview.rows) == 3 assert not page.overview.isWindow() window.close() @@ -192,10 +197,13 @@ def test_records_tab_browse_error_and_reload(make_window, overview_session, monk assert page.overview is None and page.path_label.text() monkeypatch.setattr(QFileDialog, "getExistingDirectory", lambda *args: str(root)) page.open_button.click() + wait_review(page) first = page.overview page.load_session(root / "not-a-session") + wait_review(page) assert page.overview is first and page.message.text() page.open_button.click() + wait_review(page) assert page.overview is not first and not page.message.text() window.show() qt_app.processEvents() diff --git a/apps/autocal/tests/test_review_job.py b/apps/autocal/tests/test_review_job.py new file mode 100644 index 0000000..a033277 --- /dev/null +++ b/apps/autocal/tests/test_review_job.py @@ -0,0 +1,86 @@ +"""Recording integrity work must leave the UI thread free and report failures.""" + +import threading + +from PySide6.QtCore import QObject, QTimer, Slot, QCoreApplication, QEvent, QThreadPool +from PySide6.QtTest import QTest + +from orisys_autocal.ui.review_job import ReviewJob + + +def test_review_job_runs_off_thread_and_keeps_qt_responsive(qt_app): + started, release = threading.Event(), threading.Event() + main_thread = threading.get_ident() + results, ticks = [], [] + + class Receiver(QObject): + @Slot(object, object) + def receive(self, value, error): + results.append((value, error, threading.get_ident())) + + receiver = Receiver() + def action(): + started.set() + assert release.wait(3) + return threading.get_ident() + + job = ReviewJob(action, receiver.receive) + job.start() + try: + assert started.wait(2) + QTimer.singleShot(0, lambda: ticks.append(True)) + QTest.qWait(20) + assert ticks == [True] and results == [] + finally: + release.set() + for _ in range(100): + if results: + break + QTest.qWait(10) + assert len(results) == 1 + assert results[0][0] != main_thread + assert results[0][1] is None and results[0][2] == main_thread + + +def test_unexpected_review_job_failure_returns_to_receiver(qt_app): + results = [] + class Receiver(QObject): + @Slot(object, object) + def receive(self, value, error): + results.append((value, error)) + receiver = Receiver() + def action(): + raise RuntimeError("decoder failed") + job = ReviewJob(action, receiver.receive) + job.start() + for _ in range(100): + if results: + break + QTest.qWait(10) + assert results == [(None, "decoder failed")] + + +def test_review_job_can_finish_after_its_receiver_is_destroyed(qt_app): + started, release, finished = threading.Event(), threading.Event(), threading.Event() + results = [] + class Receiver(QObject): + @Slot(object, object) + def receive(self, value, error): + results.append((value, error)) + receiver = Receiver() + def action(): + started.set() + assert release.wait(3) + finished.set() + return "done" + job = ReviewJob(action, receiver.receive) + job.start() + try: + assert started.wait(2) + receiver.deleteLater() + QCoreApplication.sendPostedEvents(receiver, QEvent.Type.DeferredDelete) + finally: + release.set() + assert QThreadPool.globalInstance().waitForDone(3000) + qt_app.processEvents() + assert finished.is_set() and results == [] diff --git a/apps/autocal/tests/test_session_review.py b/apps/autocal/tests/test_session_review.py index 7d09b55..e4f7b6a 100644 --- a/apps/autocal/tests/test_session_review.py +++ b/apps/autocal/tests/test_session_review.py @@ -6,6 +6,8 @@ import csv import json import pytest + +from review_test_helpers import wait_review from orisys_autocal.calibration_session import write_json from orisys_autocal.session_review import review_session, save_review @@ -120,6 +122,7 @@ def test_review_dialog_shows_saved_windows_charts_and_new_export(qt_app, session dialog.uv_chart.series()[0].clicked.emit(dialog.uv_chart.series()[0].at(0)) assert dialog.sites.currentIndex() == 0 dialog.save_button.click() + wait_review(dialog) assert len(list(session.glob("derived/*/review.json"))) == 1 dialog.show() qt_app.processEvents() @@ -177,11 +180,13 @@ def test_operator_confirms_windows_in_dialog_and_reloads_them(qt_app, session, m assert dialog.review["sites"][0]["windows"][row]["stability_verified"] assert dialog.review["sites"][0]["confirmed_fit"] is not None dialog.save_button.click() + wait_review(dialog) saved = next(session.glob("derived/*/review.json")) fresh = session_review_dialog.SessionReviewDialog(session) monkeypatch.setattr(session_review_dialog.QFileDialog, "getOpenFileName", lambda *args: (str(saved), "")) fresh.load_button.click() + wait_review(fresh) assert fresh.review["sites"] == dialog.review["sites"] dialog.close() fresh.close() diff --git a/apps/autocal/tests/test_session_review_multipoint.py b/apps/autocal/tests/test_session_review_multipoint.py index 0a6e716..8edd772 100644 --- a/apps/autocal/tests/test_session_review_multipoint.py +++ b/apps/autocal/tests/test_session_review_multipoint.py @@ -8,7 +8,9 @@ from pathlib import Path import pytest -from orisys_autocal.session_review import review_session, save_review +from review_test_helpers import wait_review + +from orisys_autocal.session_review import review_session, save_review, reselect_review REFERENCE_HEADER = ( "step_index", "branch", "target_load_n", "t_start_host_mono_ns", "t_end_host_mono_ns", @@ -33,6 +35,58 @@ CAPTURE_SESSION_ID = 42 BRANCHES = ["zero", "load", "load", "unload", "unload"] +def test_window_adjustment_uses_loaded_traces_and_matches_fresh_review(tmp_path, monkeypatch): + import orisys_autocal.session_review as module + root = _session(tmp_path / "recording") + original = review_session(root) + w = original["sites"][0]["windows"][0] + selections = {f"{w['trial_id']}:{w['step_index']}": { + "t_start_host_mono_ns": w["hold_start_host_mono_ns"] + 10_000_000, + "t_end_host_mono_ns": w["hold_end_host_mono_ns"], "stability_verified": True}} + expected = review_session(root, selections=selections) + monkeypatch.setattr(module, "_rows", lambda *args: pytest.fail("window edit read a file")) + monkeypatch.setattr(module, "_sha256", lambda *args: pytest.fail("window edit hashed a file")) + adjusted = reselect_review(original, selections) + assert adjusted == expected + assert original["selections"] == {} and not w["stability_verified"] + assert adjusted["sites"][0]["windows"][0]["trace"] is w["trace"] + + +def test_saving_adjusted_review_still_rejects_modified_video(tmp_path): + root = _session(tmp_path / "recording") + adjusted = reselect_review(review_session(root), {}) + video = next(root.glob("sites/*/*/capture/raw_0001.mp4")) + video.write_bytes(b"different recorded video") + with pytest.raises(ValueError, match="recorded inputs changed"): + save_review(adjusted) + assert not (root / "derived").exists() + + +def test_background_save_reports_changed_video_and_recovers_controls(qt_app, tmp_path, monkeypatch): + import threading + from PySide6.QtWidgets import QLabel + from orisys_autocal.ui.session_review_dialog import SessionReviewDialog + root = _session(tmp_path / "recording") + dialog = SessionReviewDialog(root) + main_thread = threading.get_ident() + updates = [] + set_text = QLabel.setText + def checked_set_text(label, text): + if label is dialog.message: + updates.append(threading.get_ident()) + set_text(label, text) + monkeypatch.setattr(QLabel, "setText", checked_set_text) + next(root.glob("sites/*/*/capture/raw_0001.mp4")).write_bytes(b"changed") + dialog.save_button.click() + assert not dialog.save_button.isEnabled() and not dialog.apply_button.isEnabled() + wait_review(dialog) + assert "recorded inputs changed" in dialog.message.text() + assert dialog.save_button.isEnabled() and dialog.apply_button.isEnabled() + assert len(updates) >= 2 and set(updates) == {main_thread} + assert not (root / "derived").exists() + dialog.close() + + def _write(path: Path, header, rows) -> None: path.parent.mkdir(parents=True, exist_ok=True) with path.open("w", newline="", encoding="utf-8") as handle: