fix: treat a confirmed dialog as confirmed, and stop pinning a prepared model

scripts/guardrails-baseline.json

@@ -20,7 +20,7 @@
     "long_functions": 2
   },
   "src/matteloop/jobs/models/session.py": {
-    "lines": 439,
+    "lines": 437,
     "long_functions": 1
   },
   "src/matteloop/jobs/render.py": {

src/matteloop/jobs/models/session.py

@@ -220,14 +220,12 @@ def fetch(
     def remove(self, model_id: str) -> bool:
         with self._lock:
             spec = self._catalog.get(model_id)
-            if (self._active_spec is not None and self._active_spec.id == spec.id) or (
-                spec.id in self._cleanup_ids
-            ):
+            if spec.id in self._cleanup_ids:
                 raise AppError(
                     ErrorCode.MODEL_IN_USE,
                     "model-cache",
                     "error.model.in-use",
-                    f"model {spec.id!r} owns the active segmentation session",
+                    f"model {spec.id!r} is pending segmentation cleanup",
                     "close-or-switch-model",
                 )
             self._require_open()

src/matteloop/ui/model_manager/_controller.py

@@ -139,8 +139,6 @@ def _removal_block_reason(self, entry: ModelEntry) -> str | None:
             return "Cannot remove a model while a job is running."
         if self._manager is None:
             return "Model removal is unavailable in this runtime."
-        if self._manager.active_id == entry.model_id:
-            return "Cannot remove the model used by the active session."
         return None
 
     def _download_requested(self, value: object) -> None:
@@ -196,7 +194,9 @@ def _delete_outdated_requested(self) -> None:
             QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
             QMessageBox.StandardButton.No,
         )
-        if answer is not QMessageBox.StandardButton.Yes:
+        # QMessageBox.question returns an int, so identity with StandardButton
+        # does not hold.
+        if answer != QMessageBox.StandardButton.Yes:
             return
         self._start_obsolete_removal(manager)
 
@@ -213,10 +213,7 @@ def _redownload_outdated_requested(self) -> None:
             return
         if self._remove_thread is not None:
             return
-        if (
-            _confirm_redownload(self.dialog, entries)
-            is not QMessageBox.StandardButton.Yes
-        ):
+        if _confirm_redownload(self.dialog, entries) != QMessageBox.StandardButton.Yes:
             return
         cancel = CancellationState()
         self._batch = {entry.model_id: entry for entry in entries}
@@ -373,7 +370,9 @@ def _obsolete_removal_failed(self, error: object) -> None:
     def _remove_requested(self, value: object) -> None:
         if not isinstance(value, ModelEntry) or not value.cached:
             return
-        if self._removal_block_reason(value) is not None:
+        reason = self._removal_block_reason(value)
+        if reason is not None:
+            self.dialog.set_message(reason)
             return
         if self._remove_thread is not None:
             return
@@ -385,7 +384,7 @@ def _remove_requested(self, value: object) -> None:
             QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
             QMessageBox.StandardButton.No,
         )
-        if answer is not QMessageBox.StandardButton.Yes:
+        if answer != QMessageBox.StandardButton.Yes:
             return
         manager = self._manager
         if manager is None:

src/matteloop/ui/workspace_controller.py

@@ -85,4 +85,4 @@ def _confirm_pinned_delete(self) -> bool:
             QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
             QMessageBox.StandardButton.No,
         )
-        return answer is QMessageBox.StandardButton.Yes
+        return answer == QMessageBox.StandardButton.Yes

tests/jobs/models/test_session.py

@@ -413,20 +413,17 @@ def test_task9_rejects_custom_options_and_unimplemented_prompts(
     assert exc.value.code is ErrorCode.MODEL_PREPARATION_INVALID
 
 
-def test_remove_rejects_active_model_then_removes_exact_inactive_artifact(
-    tmp_path: Path,
-) -> None:
+def test_remove_active_and_inactive_model_weights(tmp_path: Path) -> None:
     manager, _downloader, _clients, _events = _manager(tmp_path)
-    result = manager.prepare("u2net", {})
-    assert result.artifact_path is not None
+    inactive = manager.prepare("u2net", {})
+    active = manager.prepare("u2netp", {})
+    assert inactive.artifact_path is not None
+    assert active.artifact_path is not None
 
-    with pytest.raises(AppError) as exc:
-        manager.remove("u2net")
-    assert exc.value.code is ErrorCode.MODEL_IN_USE
-
-    manager.prepare("u2netp", {})
     assert manager.remove("u2net") is True
-    assert not result.artifact_path.exists()
+    assert not inactive.artifact_path.exists()
+    assert manager.remove("u2netp") is True
+    assert not active.artifact_path.exists()
     assert manager.remove("u2net") is False
 
 

tests/ui/test_model_manager.py

@@ -5,9 +5,9 @@
 from threading import Event, get_ident
 
 import pytest
-from PySide6.QtCore import QUrl
+from PySide6.QtCore import QTimer, QUrl
 from PySide6.QtGui import QDesktopServices
-from PySide6.QtWidgets import QMessageBox
+from PySide6.QtWidgets import QApplication, QMessageBox
 
 from matteloop.core.errors import AppError, ErrorCode
 from matteloop.core.parameters import ParameterState
@@ -192,7 +192,7 @@ def test_model_manager_removes_only_confirmed_selected_weight_in_background(
     monkeypatch.setattr(
         QMessageBox,
         "question",
-        lambda *_args, **_kwargs: QMessageBox.StandardButton.Yes,
+        lambda *_args, **_kwargs: int(QMessageBox.StandardButton.Yes),
     )
     reported: list[str] = []
     monkeypatch.setattr(
@@ -224,7 +224,73 @@ def test_model_manager_removes_only_confirmed_selected_weight_in_background(
     controller.close()
 
 
-def test_model_manager_blocks_active_and_running_job_removal(
+def test_model_manager_removes_weight_after_real_confirmation(
+    tmp_path: Path, qtbot
+) -> None:
+    target = _model_path(tmp_path, "u2netp")
+    target.parent.mkdir(parents=True)
+    target.write_bytes(b"cached-weight")
+    manager = FakeModelRemovalService(targets={"u2netp": target})
+    controller = ModelManagerController(
+        ReducerStore(AppState(model_available=True)),
+        catalog=ModelCatalog.load_resource(),
+        cache_root=tmp_path,
+        manager=manager,
+    )
+    qtbot.addWidget(controller.dialog)
+    controller.open()
+    controller.dialog.model_list.setCurrentRow(
+        next(
+            index
+            for index, entry in enumerate(controller.dialog.entries)
+            if entry.model_id == "u2netp"
+        )
+    )
+
+    def click_active_modal_button() -> None:
+        app = QApplication.instance()
+        assert isinstance(app, QApplication)
+        modal = app.activeModalWidget()
+        if not isinstance(modal, QMessageBox):
+            return
+        yes_button = next(
+            (
+                button
+                for button in modal.buttons()
+                if modal.buttonRole(button) == QMessageBox.ButtonRole.YesRole
+            ),
+            None,
+        )
+        if yes_button is not None:
+            yes_button.click()
+            return
+        if manager.removed != ["u2netp"]:
+            return
+        ok_button = next(
+            (
+                button
+                for button in modal.buttons()
+                if modal.buttonRole(button) == QMessageBox.ButtonRole.AcceptRole
+            ),
+            None,
+        )
+        if ok_button is not None:
+            ok_button.click()
+
+    modal_timer = QTimer(controller.dialog)
+    modal_timer.setInterval(10)
+    modal_timer.timeout.connect(click_active_modal_button)
+    modal_timer.start()
+    try:
+        controller.dialog.remove_button.click()
+        qtbot.waitUntil(lambda: manager.removed == ["u2netp"], timeout=5000)
+        qtbot.waitUntil(lambda: not target.exists(), timeout=5000)
+    finally:
+        modal_timer.stop()
+    controller.close()
+
+
+def test_model_manager_allows_active_and_blocks_running_job_removal(
     tmp_path: Path, qtbot
 ) -> None:
     target = _model_path(tmp_path, "u2netp")
@@ -247,8 +313,7 @@ def test_model_manager_blocks_active_and_running_job_removal(
         )
     )
 
-    assert controller.dialog.remove_button.isEnabled() is False
-    assert "active session" in controller.dialog.remove_button.toolTip()
+    assert controller.dialog.remove_button.isEnabled() is True
 
     running = ReducerStore(
         AppState(
@@ -283,6 +348,12 @@ def test_model_manager_blocks_active_and_running_job_removal(
 
     assert running_controller.dialog.remove_button.isEnabled() is False
     assert "job is running" in running_controller.dialog.remove_button.toolTip()
+    selected = running_controller.dialog.selected_entry
+    assert selected is not None
+    running_controller._remove_requested(selected)
+    assert running_controller.dialog._message.text() == (
+        "Cannot remove a model while a job is running."
+    )
     controller.close()
     running_controller.close()
 
@@ -529,7 +600,7 @@ def test_model_manager_reports_and_deletes_outdated_copy_alongside_current_weigh
         QMessageBox,
         "question",
         lambda _parent, title, message, *_args: (
-            confirmation.append((title, message)) or QMessageBox.StandardButton.Yes
+            confirmation.append((title, message)) or int(QMessageBox.StandardButton.Yes)
         ),
     )
     controller.dialog.delete_outdated_button.click()
@@ -570,7 +641,7 @@ def test_model_manager_deletes_outdated_directory_without_touching_other_cache(
     monkeypatch.setattr(
         QMessageBox,
         "question",
-        lambda *_args, **_kwargs: QMessageBox.StandardButton.Yes,
+        lambda *_args, **_kwargs: int(QMessageBox.StandardButton.Yes),
     )
 
     controller.dialog.delete_outdated_button.click()
@@ -644,7 +715,7 @@ def test_model_manager_confirms_the_download_size_before_re_downloading_outdated
         QMessageBox,
         "question",
         lambda _parent, title, body, *_args: (
-            confirmation.append((title, body)) or QMessageBox.StandardButton.No
+            confirmation.append((title, body)) or int(QMessageBox.StandardButton.No)
         ),
     )
 
@@ -691,7 +762,7 @@ def test_model_manager_re_downloads_each_outdated_weight_then_deletes_its_old_co
     monkeypatch.setattr(
         QMessageBox,
         "question",
-        lambda *_args, **_kwargs: QMessageBox.StandardButton.Yes,
+        lambda *_args, **_kwargs: int(QMessageBox.StandardButton.Yes),
     )
 
     controller.dialog.redownload_outdated_button.click()
@@ -757,7 +828,7 @@ def record(message: str) -> None:
     monkeypatch.setattr(
         QMessageBox,
         "question",
-        lambda *_args, **_kwargs: QMessageBox.StandardButton.Yes,
+        lambda *_args, **_kwargs: int(QMessageBox.StandardButton.Yes),
     )
 
     controller.dialog.redownload_outdated_button.click()
@@ -792,7 +863,7 @@ def test_model_manager_cancel_stops_the_batch_and_keeps_the_rest_outdated(
     monkeypatch.setattr(
         QMessageBox,
         "question",
-        lambda *_args, **_kwargs: QMessageBox.StandardButton.Yes,
+        lambda *_args, **_kwargs: int(QMessageBox.StandardButton.Yes),
     )
 
     controller.dialog.redownload_outdated_button.click()
@@ -840,7 +911,7 @@ def test_model_manager_escape_cancels_a_running_download_instead_of_hiding_the_d
     monkeypatch.setattr(
         QMessageBox,
         "question",
-        lambda *_args, **_kwargs: QMessageBox.StandardButton.Yes,
+        lambda *_args, **_kwargs: int(QMessageBox.StandardButton.Yes),
     )
     controller.dialog.redownload_outdated_button.click()
     qtbot.waitUntil(lambda: manager.in_flight, timeout=5000)
@@ -877,7 +948,7 @@ def test_model_manager_ignores_dismissal_while_a_removal_runs(
     monkeypatch.setattr(
         QMessageBox,
         "question",
-        lambda *_args, **_kwargs: QMessageBox.StandardButton.Yes,
+        lambda *_args, **_kwargs: int(QMessageBox.StandardButton.Yes),
     )
     controller.dialog.delete_outdated_button.click()
     qtbot.waitUntil(lambda: manager.obsolete_calls == 1, timeout=5000)
@@ -923,7 +994,7 @@ def test_model_manager_stops_the_batch_at_the_first_failed_download(
     monkeypatch.setattr(
         QMessageBox,
         "question",
-        lambda *_args, **_kwargs: QMessageBox.StandardButton.Yes,
+        lambda *_args, **_kwargs: int(QMessageBox.StandardButton.Yes),
     )
 
     controller.dialog.redownload_outdated_button.click()
@@ -974,7 +1045,7 @@ def test_model_manager_close_cancels_the_running_download_before_waiting(
     monkeypatch.setattr(
         QMessageBox,
         "question",
-        lambda *_args, **_kwargs: QMessageBox.StandardButton.Yes,
+        lambda *_args, **_kwargs: int(QMessageBox.StandardButton.Yes),
     )
     controller.dialog.redownload_outdated_button.click()
     qtbot.waitUntil(lambda: manager.in_flight, timeout=5000)
@@ -1089,7 +1160,7 @@ def test_model_manager_refuses_re_download_while_a_job_runs(
     monkeypatch.setattr(
         QMessageBox,
         "question",
-        lambda *_args, **_kwargs: QMessageBox.StandardButton.Yes,
+        lambda *_args, **_kwargs: int(QMessageBox.StandardButton.Yes),
     )
 
     controller.dialog.redownload_outdated_button.click()
@@ -1128,7 +1199,7 @@ def fail_after_external_removal() -> int:
     monkeypatch.setattr(
         QMessageBox,
         "question",
-        lambda *_args, **_kwargs: QMessageBox.StandardButton.Yes,
+        lambda *_args, **_kwargs: int(QMessageBox.StandardButton.Yes),
     )
 
     controller.dialog.delete_outdated_button.click()