From 75c7915c31c786660daf702112ded61e5c8df97d Mon Sep 17 00:00:00 2001 From: Jason Farrar Date: Mon, 14 Sep 2026 20:39:56 +0100 Subject: [PATCH 1/3] fix(debug): retry incomplete debug worker on session restore and log failures On session restore, non-success rows with missing debug paths (NULL debug_json_path) caused the Open JSON/Excel buttons in DebugInfoDialog to remain permanently greyed out. This happened when the debug worker was cancelled or failed before completing in a prior session, and the stale NULL paths persisted in gui.db. - Restart the debug worker on session restore for any non-success rows whose debug_status is not 'done' or 'error'. - Log update_debug_info() failures so silent SQL UPDATE failures are visible in stderr instead of leaving buttons permanently disabled. --- .../presenters/statement_result_presenter.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/openstan/presenters/statement_result_presenter.py b/src/openstan/presenters/statement_result_presenter.py index 986a6cd..d8f249f 100644 --- a/src/openstan/presenters/statement_result_presenter.py +++ b/src/openstan/presenters/statement_result_presenter.py @@ -463,6 +463,16 @@ def load_results_from_db(self, batch_id: str, project_id: str) -> None: else: self.failure_model.add_row(row) + # Re-run the debug worker for any non-success rows that lack complete + # debug data (e.g. the worker was cancelled or failed in a prior session). + has_incomplete_debug = any( + r.debug_status not in ("done", "error") + for r in rows + if r.result != "SUCCESS" + ) + if has_incomplete_debug and self.project_path is not None: + self.__start_debug_worker(batch_id) + # --------------------------------------------------------------------------- # Public: import lifecycle — called by StanPresenter # --------------------------------------------------------------------------- @@ -670,9 +680,14 @@ def __on_debug_entry_done( ) -> None: """Persist debug result for one row and update the open dialog if any.""" status = "done" if debug_json_path is not None else "error" - self.result_model.update_debug_info( + ok, msg = self.result_model.update_debug_info( result_id, status, debug_json_path, debug_excel_path ) + if not ok: + print( + f"WARNING: update_debug_info failed for {result_id}: {msg}", + file=sys.stderr, + ) self._debug_done_count += 1 From 4904e4fc39b520b06ac11450963a9f0c554e0cbd Mon Sep 17 00:00:00 2001 From: Jason Farrar Date: Mon, 14 Sep 2026 21:22:16 +0100 Subject: [PATCH 2/3] fix(debug): skip already-complete rows when re-running debug worker __start_debug_worker was resetting ALL non-success rows to 'pending' and reprocessing them, which overwrote valid debug_json_path values from prior sessions. Filter to only rows whose debug_status is not 'done' or 'error' so completed debug data is preserved. --- .../presenters/statement_result_presenter.py | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/openstan/presenters/statement_result_presenter.py b/src/openstan/presenters/statement_result_presenter.py index d8f249f..5b799ae 100644 --- a/src/openstan/presenters/statement_result_presenter.py +++ b/src/openstan/presenters/statement_result_presenter.py @@ -630,7 +630,12 @@ def __on_view_debug_info(self) -> None: # --------------------------------------------------------------------------- def __start_debug_worker(self, batch_id: str) -> None: - """Collect all non-success rows and start DebugWorker off-thread.""" + """Collect non-success rows that need debug work and start DebugWorker. + + Only rows whose ``debug_status`` is not already ``"done"`` or + ``"error"`` are re-processed. This prevents overwriting valid + debug paths from a previous run (e.g. on session restore). + """ if self.project_path is None: print( "WARNING: Cannot start debug worker — project path is not set.", @@ -647,20 +652,33 @@ def __start_debug_worker(self, batch_id: str) -> None: n_success = self.success_model.row_count non_success_ids = all_result_ids[n_success:] - # Mark all non-success rows as 'pending' in the DB - for rid in non_success_ids: + # Only process rows that still need debug work — skip rows that + # already completed successfully in a prior session. + needs_debug = [ + (rid, row) + for rid, row in zip(non_success_ids, non_success) + if row.debug_status not in ("done", "error") + ] + if not needs_debug: + return + + debug_ids = [rid for rid, _ in needs_debug] + debug_rows = [row for _, row in needs_debug] + + # Mark only the incomplete rows as 'pending' in the DB + for rid in debug_ids: self.result_model.update_debug_info(rid, "pending", None) self._debug_cancel = threading.Event() self._debug_worker_done = False self._debug_done_count = 0 - self._debug_total_count = len(non_success) + self._debug_total_count = len(debug_rows) self.__update_debug_button_label() worker = DebugWorker( - rows=non_success, - result_ids=non_success_ids, + rows=debug_rows, + result_ids=debug_ids, batch_id=batch_id, project_path=self.project_path, cancel_event=self._debug_cancel, From 223f7f1f08557bfc80bcc7fd8994c30bb5f19d08 Mon Sep 17 00:00:00 2001 From: Jason Farrar Date: Tue, 15 Sep 2026 09:28:20 +0100 Subject: [PATCH 3/3] fix(debug): log pending-status update failures and add regression tests - Log update_debug_info() failures when marking rows as 'pending' in __start_debug_worker, matching the logging added for 'done' updates. - Add 6 unit tests covering the session-restore debug worker retry: triggers on None/pending status, skips done/error, respects project_path guard, handles mixed statuses. --- .../presenters/statement_result_presenter.py | 7 +- tests/unit/test_statement_result_presenter.py | 260 ++++++++++++++++++ 2 files changed, 266 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_statement_result_presenter.py diff --git a/src/openstan/presenters/statement_result_presenter.py b/src/openstan/presenters/statement_result_presenter.py index 5b799ae..01224b4 100644 --- a/src/openstan/presenters/statement_result_presenter.py +++ b/src/openstan/presenters/statement_result_presenter.py @@ -667,7 +667,12 @@ def __start_debug_worker(self, batch_id: str) -> None: # Mark only the incomplete rows as 'pending' in the DB for rid in debug_ids: - self.result_model.update_debug_info(rid, "pending", None) + ok, msg = self.result_model.update_debug_info(rid, "pending", None) + if not ok: + print( + f"WARNING: update_debug_info(pending) failed for {rid}: {msg}", + file=sys.stderr, + ) self._debug_cancel = threading.Event() self._debug_worker_done = False diff --git a/tests/unit/test_statement_result_presenter.py b/tests/unit/test_statement_result_presenter.py new file mode 100644 index 0000000..f25650c --- /dev/null +++ b/tests/unit/test_statement_result_presenter.py @@ -0,0 +1,260 @@ +""" +test_statement_result_presenter.py — unit tests for StatementResultPresenter. + +Focuses on the session-restore debug worker retry logic (issue #212): +when ``load_results_from_db`` finds non-success rows with incomplete +debug status, it should trigger ``__start_debug_worker``. +""" + +from pathlib import Path +from unittest.mock import MagicMock, patch +from uuid import uuid4 + +from PySide6.QtSql import QSqlDatabase + +from openstan.models.statement_result_model import ( + FailureResultModel, + ReviewResultModel, + StatementResultModel, + SuccessResultModel, +) +from openstan.presenters.statement_result_presenter import StatementResultPresenter +from tests.unit.conftest import seed_session_and_project + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _seed_result_rows( + db: QSqlDatabase, + batch_id: str, + queue_id: str, + project_id: str, + statuses: list[str | None], +) -> list[str]: + """Insert result rows into statement_result with given debug statuses. + + Returns the list of result_ids in insertion order. + """ + import sqlite3 + + result_ids: list[str] = [] + with sqlite3.connect(db.databaseName()) as conn: + for status in statuses: + rid = uuid4().hex + conn.execute( + "INSERT INTO statement_result " + "(result_id, batch_id, queue_id, project_id, result, file_path, " + " debug_json_path, debug_excel_path, debug_status, deleted, created) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 0, datetime('now'))", + ( + rid, + batch_id, + queue_id, + project_id, + "REVIEW", + f"/tmp/statement_{rid[:8]}.pdf", + None, + None, + status, + ), + ) + result_ids.append(rid) + conn.commit() + return result_ids + + +def _make_presenter(gui_db: QSqlDatabase) -> StatementResultPresenter: + """Build a minimal StatementResultPresenter with mocked view.""" + success_model = SuccessResultModel() + review_model = ReviewResultModel() + failure_model = FailureResultModel() + result_model = StatementResultModel(gui_db) + + # Stub out models that aren't exercised in these tests + payload_model = MagicMock() + queue_model = MagicMock() + batch_model = MagicMock() + + view = MagicMock() + # The view needs setModel and clicked signals for init wiring + view.success_table = MagicMock() + view.review_table = MagicMock() + view.failure_table = MagicMock() + view.buttonCloseResults = MagicMock() + view.buttonAbandonBatch = MagicMock() + view.buttonViewDebugInfo = MagicMock() + view.buttonCommitBatch = MagicMock() + view.labelStatementsProcessed = MagicMock() + view.results_tabs = MagicMock() + view.progressBar = MagicMock() + + return StatementResultPresenter( + success_model=success_model, + review_model=review_model, + failure_model=failure_model, + result_model=result_model, + payload_model=payload_model, + queue_model=queue_model, + batch_model=batch_model, + view=view, + ) + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + + +class TestLoadResultsFromDbDebugRetry: + """Verify that load_results_from_db triggers the debug worker for + rows with incomplete debug status.""" + + def test_triggers_worker_when_rows_have_none_status( + self, gui_db: QSqlDatabase + ) -> None: + """Rows with debug_status=None (never processed) should trigger worker.""" + _session_id, project_id, _ = seed_session_and_project(gui_db) + batch_id = uuid4().hex + queue_id = uuid4().hex + + _seed_result_rows( + gui_db, + batch_id, + queue_id, + project_id, + statuses=[None, None, None], + ) + + presenter = _make_presenter(gui_db) + presenter.project_path = Path("/tmp/test_project") + + with patch.object( + presenter, "_StatementResultPresenter__start_debug_worker" + ) as mock_start: + presenter.load_results_from_db(batch_id, project_id) + + mock_start.assert_called_once_with(batch_id) + + def test_triggers_worker_when_rows_have_pending_status( + self, gui_db: QSqlDatabase + ) -> None: + """Rows with debug_status='pending' (incomplete) should trigger worker.""" + _session_id, project_id, _ = seed_session_and_project(gui_db) + batch_id = uuid4().hex + queue_id = uuid4().hex + + _seed_result_rows( + gui_db, + batch_id, + queue_id, + project_id, + statuses=["pending", "pending"], + ) + + presenter = _make_presenter(gui_db) + presenter.project_path = Path("/tmp/test_project") + + with patch.object( + presenter, "_StatementResultPresenter__start_debug_worker" + ) as mock_start: + presenter.load_results_from_db(batch_id, project_id) + + mock_start.assert_called_once_with(batch_id) + + def test_no_worker_when_all_done(self, gui_db: QSqlDatabase) -> None: + """Rows with debug_status='done' should NOT trigger the worker.""" + _session_id, project_id, _ = seed_session_and_project(gui_db) + batch_id = uuid4().hex + queue_id = uuid4().hex + + _seed_result_rows( + gui_db, + batch_id, + queue_id, + project_id, + statuses=["done", "done"], + ) + + presenter = _make_presenter(gui_db) + presenter.project_path = Path("/tmp/test_project") + + with patch.object( + presenter, "_StatementResultPresenter__start_debug_worker" + ) as mock_start: + presenter.load_results_from_db(batch_id, project_id) + + mock_start.assert_not_called() + + def test_no_worker_when_all_error(self, gui_db: QSqlDatabase) -> None: + """Rows with debug_status='error' should NOT trigger the worker.""" + _session_id, project_id, _ = seed_session_and_project(gui_db) + batch_id = uuid4().hex + queue_id = uuid4().hex + + _seed_result_rows( + gui_db, + batch_id, + queue_id, + project_id, + statuses=["error", "error"], + ) + + presenter = _make_presenter(gui_db) + presenter.project_path = Path("/tmp/test_project") + + with patch.object( + presenter, "_StatementResultPresenter__start_debug_worker" + ) as mock_start: + presenter.load_results_from_db(batch_id, project_id) + + mock_start.assert_not_called() + + def test_no_worker_when_project_path_is_none(self, gui_db: QSqlDatabase) -> None: + """Worker should not start if project_path is not set.""" + _session_id, project_id, _ = seed_session_and_project(gui_db) + batch_id = uuid4().hex + queue_id = uuid4().hex + + _seed_result_rows( + gui_db, + batch_id, + queue_id, + project_id, + statuses=[None, None], + ) + + presenter = _make_presenter(gui_db) + # project_path stays None (default) + + with patch.object( + presenter, "_StatementResultPresenter__start_debug_worker" + ) as mock_start: + presenter.load_results_from_db(batch_id, project_id) + + mock_start.assert_not_called() + + def test_mixed_statuses_triggers_worker(self, gui_db: QSqlDatabase) -> None: + """A mix of done/error/pending rows should trigger worker (pending needs work).""" + _session_id, project_id, _ = seed_session_and_project(gui_db) + batch_id = uuid4().hex + queue_id = uuid4().hex + + _seed_result_rows( + gui_db, + batch_id, + queue_id, + project_id, + statuses=["done", "error", "pending", None], + ) + + presenter = _make_presenter(gui_db) + presenter.project_path = Path("/tmp/test_project") + + with patch.object( + presenter, "_StatementResultPresenter__start_debug_worker" + ) as mock_start: + presenter.load_results_from_db(batch_id, project_id) + + mock_start.assert_called_once_with(batch_id)