From fa3a16b0c4785daea9333cb98ee54565955f066c Mon Sep 17 00:00:00 2001 From: Lee Clement Date: Wed, 22 Jul 2026 16:55:56 -0230 Subject: [PATCH 1/7] feat: delete (soft) and restore training runs via SDK and CLI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the platform's external v2 trainings delete/restore routes: - rfapi: delete_version_training (POST .../v2/trainings/delete, optional trainingId with sole-run fallback) and restore_version_training (trainingId required — trashed runs are invisible to the sole-run resolver). - SDK: Version.delete_training(training_id=None) and Version.restore_training(training_id), mirroring Version.delete()/ restore() trash semantics: the run and its models hide from listings but stay restorable for 30 days before permanent cleanup. - CLI: `roboflow train delete / [--training-id]` and `roboflow train restore / --training-id`, with actionable hints for in-progress runs, alias-backed models, and multi-run versions. Deletion is soft-only on the public API by design; permanent deletion stays in the web UI's Trash view. Co-Authored-By: Claude Fable 5 --- roboflow/adapters/rfapi.py | 51 +++++++++++++ roboflow/cli/handlers/train.py | 126 ++++++++++++++++++++++++++++++++ roboflow/core/training.py | 18 +++++ roboflow/core/version.py | 44 +++++++++++ tests/cli/test_train_handler.py | 53 ++++++++++++++ tests/test_rfapi.py | 49 +++++++++++++ tests/test_training.py | 26 +++++++ 7 files changed, 367 insertions(+) diff --git a/roboflow/adapters/rfapi.py b/roboflow/adapters/rfapi.py index bdf8067e..a576b626 100644 --- a/roboflow/adapters/rfapi.py +++ b/roboflow/adapters/rfapi.py @@ -141,6 +141,57 @@ def stop_version_training(api_key: str, workspace_url: str, project_url: str, ve return response.json() if response.content else {"success": True} +def delete_version_training( + api_key: str, + workspace_url: str, + project_url: str, + version: str, + *, + training_id: Optional[str] = None, +): + """Move a terminal training run to the workspace Trash (soft delete). + + POST /{workspace}/{project}/{version}/v2/trainings/delete. The run and + every model it produced disappear from listings but stay restorable for + 30 days via ``restore_version_training`` or the Trash UI, after which + they are permanently deleted. The server refuses in-flight runs (stop or + cancel first) and the run backing the version's registered model. There + is no permanent-delete option on the public API. + + ``training_id`` selects a specific run on the version (MMPV); omit it to + target the version's sole run. + """ + url = f"{API_URL}/{workspace_url}/{project_url}/{version}/v2/trainings/delete?api_key={api_key}" + body: Dict[str, str] = {} + if training_id: + body["trainingId"] = training_id + response = requests.post(url, json=body) + if not response.ok: + raise RoboflowError(response.text) + return response.json() if response.content else {"inTrash": True} + + +def restore_version_training( + api_key: str, + workspace_url: str, + project_url: str, + version: str, + *, + training_id: str, +): + """Restore a trashed training run (and its models) back into listings. + + POST /{workspace}/{project}/{version}/v2/trainings/restore. + ``training_id`` is required — a trashed run is invisible to the sole-run + resolver. Fails while the parent project or version is itself in Trash. + """ + url = f"{API_URL}/{workspace_url}/{project_url}/{version}/v2/trainings/restore?api_key={api_key}" + response = requests.post(url, json={"trainingId": training_id}) + if not response.ok: + raise RoboflowError(response.text) + return response.json() if response.content else {"restored": True} + + def get_training_results(api_key: str, workspace_url: str, project_url: str, version: str): """Run-level training results bundle. diff --git a/roboflow/cli/handlers/train.py b/roboflow/cli/handlers/train.py index 8dd7c229..996ed530 100644 --- a/roboflow/cli/handlers/train.py +++ b/roboflow/cli/handlers/train.py @@ -175,6 +175,59 @@ def stop_training( _stop(args) +@train_app.command("delete") +def delete_training( + ctx: typer.Context, + target: Annotated[ + str, + typer.Argument(help="Training to delete as 'project/version'"), + ], + training_id: Annotated[ + Optional[str], + typer.Option( + "--training-id", + help=( + "Training id of the run to delete (versions can own several). Omit to target the version's sole run." + ), + ), + ] = None, +) -> None: + """Move a terminal training run to the workspace Trash (soft delete). + + The run and every model it produced disappear from listings but stay + restorable for 30 days ('roboflow train restore' or the web Trash view), + after which they are permanently deleted. In-flight runs are refused — + stop or cancel first — as is the run backing the version's registered + model. Permanent deletion is only available in the web UI's Trash view. + """ + args = ctx_to_args(ctx, target=target, training_id=training_id) + _delete(args) + + +@train_app.command("restore") +def restore_training( + ctx: typer.Context, + target: Annotated[ + str, + typer.Argument(help="Version the trashed training belongs to, as 'project/version'"), + ], + training_id: Annotated[ + str, + typer.Option( + "--training-id", + help="Training id of the trashed run to restore (required).", + ), + ], +) -> None: + """Restore a trashed training run (and its models) back into listings. + + Fails while the parent project or version is itself in Trash — restore + those first ('roboflow trash list' shows what is trashed). + """ + args = ctx_to_args(ctx, target=target, training_id=training_id) + _restore(args) + + @train_app.command("results") def training_results( ctx: typer.Context, @@ -553,6 +606,79 @@ def _stop(args): # noqa: ANN001 ) +def _delete(args): # noqa: ANN001 + from roboflow.adapters import rfapi + from roboflow.cli._output import output, output_error + + resolved = _resolve_train_target(args) + if resolved is None: + return + api_key, workspace_url, project_slug, version_str = resolved + + try: + result = rfapi.delete_version_training( + api_key, + workspace_url, + project_slug, + version_str, + training_id=getattr(args, "training_id", None), + ) + except rfapi.RoboflowError as exc: + msg = str(exc) + hint = None + if "in progress" in msg: + hint = "Stop or cancel the run first: 'roboflow train stop /'." + elif "registered model" in msg: + hint = "This run backs the version's registered model. Register another model first." + elif "MULTIPLE_TRAININGS" in msg: + hint = "This version owns several runs. Pass --training-id (see 'roboflow train results')." + output_error(args, msg, hint=hint, exit_code=3) + return + + output( + args, + {"status": "in_trash", "project": project_slug, "version": version_str, **(result or {})}, + text=( + f"Training moved to Trash for {project_slug} version {version_str}. " + "Restorable for 30 days via 'roboflow train restore'." + ), + ) + + +def _restore(args): # noqa: ANN001 + from roboflow.adapters import rfapi + from roboflow.cli._output import output, output_error + + resolved = _resolve_train_target(args) + if resolved is None: + return + api_key, workspace_url, project_slug, version_str = resolved + + try: + result = rfapi.restore_version_training( + api_key, + workspace_url, + project_slug, + version_str, + training_id=args.training_id, + ) + except rfapi.RoboflowError as exc: + msg = str(exc) + hint = None + if "not in trash" in msg.lower(): + hint = "Only trashed runs can be restored. 'roboflow trash list' shows what is trashed." + elif "in trash" in msg.lower(): + hint = "Restore the parent project/version first ('roboflow trash list')." + output_error(args, msg, hint=hint, exit_code=3) + return + + output( + args, + {"status": "restored", "project": project_slug, "version": version_str, **(result or {})}, + text=f"Training restored for {project_slug} version {version_str}.", + ) + + def _results(args): # noqa: ANN001 from roboflow.adapters import rfapi diff --git a/roboflow/core/training.py b/roboflow/core/training.py index 66880ddc..07a321e8 100644 --- a/roboflow/core/training.py +++ b/roboflow/core/training.py @@ -261,6 +261,24 @@ def stop(self): self.__api_key, self.workspace, self.project, self.version, training_id=self.training_id ) + def delete(self): + """Move this run to the workspace Trash (soft delete, DNA ``trainings.delete``). + + The run and every model it produced disappear from listings but stay + restorable for 30 days via :meth:`restore` or the Trash UI. The server + refuses in-flight runs (stop or cancel first) and the run backing the + version's registered model. + """ + return rfapi.delete_version_training( + self.__api_key, self.workspace, self.project, self.version, training_id=self.training_id + ) + + def restore(self): + """Restore this run from Trash (DNA ``trainings.restore``).""" + return rfapi.restore_version_training( + self.__api_key, self.workspace, self.project, self.version, training_id=self.training_id + ) + def __str__(self): return json.dumps( { diff --git a/roboflow/core/version.py b/roboflow/core/version.py index 4f5b940c..f9dba74f 100644 --- a/roboflow/core/version.py +++ b/roboflow/core/version.py @@ -876,6 +876,50 @@ def restore(self): parent_id=match.get("parentId"), ) + def delete_training(self, training_id: Optional[str] = None): + """ + Move one of this version's training runs to Trash (soft delete). + + The run and every model it produced disappear from listings but stay + restorable for 30 days via `Version.restore_training()` or the Trash + UI, after which they are permanently deleted. The server refuses + in-flight runs (stop or cancel first) and the run backing the + version's registered model. + + Args: + training_id: Training id of the run to delete (MMPV versions can + own several). Omit to target the version's sole run. + + Returns: + dict: Server response with `{trainingId, inTrash: True, alreadyInTrash}`. + """ + return rfapi.delete_version_training( + self.__api_key, + self.workspace, + self.project, + self.version, + training_id=training_id, + ) + + def restore_training(self, training_id: str): + """ + Restore one of this version's trashed training runs. + + Args: + training_id: Training id of the trashed run (required — trashed + runs are invisible to the sole-run fallback). + + Returns: + dict: Server response with `{trainingId, restored: True}`. + """ + return rfapi.restore_version_training( + self.__api_key, + self.workspace, + self.project, + self.version, + training_id=training_id, + ) + def __str__(self): """ String representation of version object. diff --git a/tests/cli/test_train_handler.py b/tests/cli/test_train_handler.py index 32503de0..3059eb14 100644 --- a/tests/cli/test_train_handler.py +++ b/tests/cli/test_train_handler.py @@ -483,6 +483,15 @@ def test_results_help(self) -> None: result = runner.invoke(app, ["train", "results", "--help"]) self.assertEqual(result.exit_code, 0) + def test_delete_help(self) -> None: + result = runner.invoke(app, ["train", "delete", "--help"]) + self.assertEqual(result.exit_code, 0) + self.assertIn("trash", result.output.lower()) + + def test_restore_help(self) -> None: + result = runner.invoke(app, ["train", "restore", "--help"]) + self.assertEqual(result.exit_code, 0) + class TestTrainCancelStopResults(unittest.TestCase): """_cancel / _stop / _results business logic.""" @@ -553,6 +562,50 @@ def test_stop_success(self, mock_stop: MagicMock) -> None: result = json.loads(out) self.assertEqual(result["status"], "stop_requested") + @patch("roboflow.adapters.rfapi.delete_version_training") + def test_delete_success(self, mock_delete: MagicMock) -> None: + from roboflow.cli.handlers.train import _delete + + mock_delete.return_value = {"trainingId": "t-1", "inTrash": True, "alreadyInTrash": False} + out = self._capture_stdout(_delete, self._args(training_id="t-1")) + + mock_delete.assert_called_once_with("test-key", "test-ws", "my-project", "3", training_id="t-1") + result = json.loads(out) + self.assertEqual(result["status"], "in_trash") + self.assertTrue(result["inTrash"]) + + @patch("roboflow.adapters.rfapi.delete_version_training") + def test_delete_in_progress_surfaces_hint(self, mock_delete: MagicMock) -> None: + from roboflow.adapters import rfapi + from roboflow.cli.handlers.train import _delete + + mock_delete.side_effect = rfapi.RoboflowError( + "This training is still in progress. Stop or cancel it before deleting it." + ) + buf = io.StringIO() + old = sys.stderr + sys.stderr = buf + try: + with self.assertRaises(SystemExit) as cm: + _delete(self._args(training_id=None)) + finally: + sys.stderr = old + self.assertEqual(cm.exception.code, 3) + err = json.loads(buf.getvalue()) + self.assertIn("in progress", err["error"]["message"]) + self.assertIn("train stop", err["error"].get("hint", "")) + + @patch("roboflow.adapters.rfapi.restore_version_training") + def test_restore_success(self, mock_restore: MagicMock) -> None: + from roboflow.cli.handlers.train import _restore + + mock_restore.return_value = {"trainingId": "t-1", "restored": True} + out = self._capture_stdout(_restore, self._args(training_id="t-1")) + + mock_restore.assert_called_once_with("test-key", "test-ws", "my-project", "3", training_id="t-1") + result = json.loads(out) + self.assertEqual(result["status"], "restored") + @patch("roboflow.adapters.rfapi.get_training_results") def test_results_nas_run(self, mock_get: MagicMock) -> None: from roboflow.cli.handlers.train import _results diff --git a/tests/test_rfapi.py b/tests/test_rfapi.py index 30acf5cd..5e3e3586 100644 --- a/tests/test_rfapi.py +++ b/tests/test_rfapi.py @@ -9,9 +9,11 @@ from roboflow.adapters.rfapi import ( RoboflowError, create_training_v2, + delete_version_training, get_train_recipe, get_training, list_trainings_for_version, + restore_version_training, upload_image, ) from roboflow.config import API_URL, DEFAULT_BATCH_NAME @@ -348,5 +350,52 @@ def test_get_training_raises_on_error(self): get_training(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION, training_id="missing") +class TestTrainingTrash(unittest.TestCase): + API_KEY = "test_api_key" + WORKSPACE = "test-ws" + PROJECT = "test-project" + VERSION = "3" + + @responses.activate + def test_delete_version_training_posts_training_id(self): + expected_url = ( + f"{API_URL}/{self.WORKSPACE}/{self.PROJECT}/{self.VERSION}/v2/trainings/delete?api_key={self.API_KEY}" + ) + responses.add( + responses.POST, + expected_url, + json={"trainingId": "t-1", "inTrash": True, "alreadyInTrash": False}, + status=200, + ) + + result = delete_version_training(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION, training_id="t-1") + + self.assertEqual(json.loads(responses.calls[0].request.body), {"trainingId": "t-1"}) + self.assertEqual(result, {"trainingId": "t-1", "inTrash": True, "alreadyInTrash": False}) + + @responses.activate + def test_delete_version_training_omits_training_id_for_sole_run(self): + expected_url = ( + f"{API_URL}/{self.WORKSPACE}/{self.PROJECT}/{self.VERSION}/v2/trainings/delete?api_key={self.API_KEY}" + ) + responses.add(responses.POST, expected_url, json={"inTrash": True}, status=200) + + delete_version_training(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION) + + self.assertEqual(json.loads(responses.calls[0].request.body), {}) + + @responses.activate + def test_restore_version_training_requires_training_id(self): + expected_url = ( + f"{API_URL}/{self.WORKSPACE}/{self.PROJECT}/{self.VERSION}/v2/trainings/restore?api_key={self.API_KEY}" + ) + responses.add(responses.POST, expected_url, json={"trainingId": "t-1", "restored": True}, status=200) + + result = restore_version_training(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION, training_id="t-1") + + self.assertEqual(json.loads(responses.calls[0].request.body), {"trainingId": "t-1"}) + self.assertEqual(result, {"trainingId": "t-1", "restored": True}) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_training.py b/tests/test_training.py index 89420c1d..949c9e96 100644 --- a/tests/test_training.py +++ b/tests/test_training.py @@ -117,5 +117,31 @@ def test_models_are_cached_until_refresh(self): self.assertEqual(get_training.call_count, 3) +class TestTrainingTrash(unittest.TestCase): + def test_delete_moves_run_to_trash(self): + training = Training("key", "ws", "proj", "1", {"trainingId": "training-1"}) + + with patch( + "roboflow.core.training.rfapi.delete_version_training", + return_value={"trainingId": "training-1", "inTrash": True, "alreadyInTrash": False}, + ) as delete: + result = training.delete() + + delete.assert_called_once_with("key", "ws", "proj", "1", training_id="training-1") + self.assertTrue(result["inTrash"]) + + def test_restore_brings_run_back(self): + training = Training("key", "ws", "proj", "1", {"trainingId": "training-1"}) + + with patch( + "roboflow.core.training.rfapi.restore_version_training", + return_value={"trainingId": "training-1", "restored": True}, + ) as restore: + result = training.restore() + + restore.assert_called_once_with("key", "ws", "proj", "1", training_id="training-1") + self.assertTrue(result["restored"]) + + if __name__ == "__main__": unittest.main() From 4a7a6c922b581fa440135d2a4cd05403128be7f6 Mon Sep 17 00:00:00 2001 From: Lee Clement Date: Fri, 24 Jul 2026 17:34:22 -0230 Subject: [PATCH 2/7] =?UTF-8?q?fix:=20review=20=E2=80=94=20strict=20traini?= =?UTF-8?q?ng=20ids=20and=20an=20actionable=20multi-run=20recovery=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - delete_version_training omits trainingId only when None and raises ValueError on blank/whitespace ids, so an empty selector can never fall back to soft-deleting the version's sole run; restore_version_training rejects blank ids likewise. - New `roboflow train list /` enumerates a version's runs (TRAINING_ID / STATUS / MODEL_TYPE / MODELS), and the MULTIPLE_TRAININGS hint on delete now points at it instead of `train results`, which cannot enumerate runs. Co-Authored-By: Claude Fable 5 --- roboflow/adapters/rfapi.py | 11 +++++-- roboflow/cli/handlers/train.py | 54 ++++++++++++++++++++++++++++++++- tests/cli/test_train_handler.py | 37 ++++++++++++++++++++++ tests/test_rfapi.py | 10 ++++++ 4 files changed, 108 insertions(+), 4 deletions(-) diff --git a/roboflow/adapters/rfapi.py b/roboflow/adapters/rfapi.py index a576b626..0d76ca35 100644 --- a/roboflow/adapters/rfapi.py +++ b/roboflow/adapters/rfapi.py @@ -158,12 +158,15 @@ def delete_version_training( cancel first) and the run backing the version's registered model. There is no permanent-delete option on the public API. - ``training_id`` selects a specific run on the version (MMPV); omit it to - target the version's sole run. + ``training_id`` selects a specific run on the version (MMPV); omit it + (``None``) to target the version's sole run. A blank id is rejected — on a + destructive call an empty selector must not silently fall back. """ + if training_id is not None and not str(training_id).strip(): + raise ValueError("training_id must be a non-empty string when provided") url = f"{API_URL}/{workspace_url}/{project_url}/{version}/v2/trainings/delete?api_key={api_key}" body: Dict[str, str] = {} - if training_id: + if training_id is not None: body["trainingId"] = training_id response = requests.post(url, json=body) if not response.ok: @@ -185,6 +188,8 @@ def restore_version_training( ``training_id`` is required — a trashed run is invisible to the sole-run resolver. Fails while the parent project or version is itself in Trash. """ + if not training_id or not str(training_id).strip(): + raise ValueError("training_id is required") url = f"{API_URL}/{workspace_url}/{project_url}/{version}/v2/trainings/restore?api_key={api_key}" response = requests.post(url, json={"trainingId": training_id}) if not response.ok: diff --git a/roboflow/cli/handlers/train.py b/roboflow/cli/handlers/train.py index 996ed530..15b13fdb 100644 --- a/roboflow/cli/handlers/train.py +++ b/roboflow/cli/handlers/train.py @@ -228,6 +228,23 @@ def restore_training( _restore(args) +@train_app.command("list") +def list_trainings( + ctx: typer.Context, + target: Annotated[ + str, + typer.Argument(help="Version whose trainings to list, as 'project/version'"), + ], +) -> None: + """List a version's training runs with their ids. + + An MMPV version may own several runs; use the TRAINING_ID column with + 'roboflow train delete/restore --training-id' or 'train cancel/stop'. + """ + args = ctx_to_args(ctx, target=target) + _list(args) + + @train_app.command("results") def training_results( ctx: typer.Context, @@ -631,7 +648,7 @@ def _delete(args): # noqa: ANN001 elif "registered model" in msg: hint = "This run backs the version's registered model. Register another model first." elif "MULTIPLE_TRAININGS" in msg: - hint = "This version owns several runs. Pass --training-id (see 'roboflow train results')." + hint = "This version owns several runs. Pass --training-id (see 'roboflow train list /')." output_error(args, msg, hint=hint, exit_code=3) return @@ -679,6 +696,41 @@ def _restore(args): # noqa: ANN001 ) +def _list(args): # noqa: ANN001 + from roboflow.adapters import rfapi + from roboflow.cli._output import output, output_error + from roboflow.cli._table import format_table + + resolved = _resolve_train_target(args) + if resolved is None: + return + api_key, workspace_url, project_slug, version_str = resolved + + try: + trainings = rfapi.list_trainings_for_version(api_key, workspace_url, project_slug, version_str) + except rfapi.RoboflowError as exc: + output_error(args, str(exc), exit_code=3) + return + + rows = [ + { + "trainingId": t.get("trainingId", ""), + "status": t.get("status", ""), + "modelType": t.get("modelType", ""), + "models": len(t.get("modelIds") or []), + } + for t in trainings + ] + table = format_table( + rows, + columns=["trainingId", "status", "modelType", "models"], + headers=["TRAINING_ID", "STATUS", "MODEL_TYPE", "MODELS"], + ) + if not rows: + table = "(No trainings on this version)" + output(args, {"trainings": trainings}, text=table) + + def _results(args): # noqa: ANN001 from roboflow.adapters import rfapi diff --git a/tests/cli/test_train_handler.py b/tests/cli/test_train_handler.py index 3059eb14..8d7e189f 100644 --- a/tests/cli/test_train_handler.py +++ b/tests/cli/test_train_handler.py @@ -488,6 +488,10 @@ def test_delete_help(self) -> None: self.assertEqual(result.exit_code, 0) self.assertIn("trash", result.output.lower()) + def test_list_help(self) -> None: + result = runner.invoke(app, ["train", "list", "--help"]) + self.assertEqual(result.exit_code, 0) + def test_restore_help(self) -> None: result = runner.invoke(app, ["train", "restore", "--help"]) self.assertEqual(result.exit_code, 0) @@ -595,6 +599,39 @@ def test_delete_in_progress_surfaces_hint(self, mock_delete: MagicMock) -> None: self.assertIn("in progress", err["error"]["message"]) self.assertIn("train stop", err["error"].get("hint", "")) + @patch("roboflow.adapters.rfapi.delete_version_training") + def test_delete_multiple_trainings_hint_points_to_train_list(self, mock_delete: MagicMock) -> None: + from roboflow.adapters import rfapi + from roboflow.cli.handlers.train import _delete + + mock_delete.side_effect = rfapi.RoboflowError( + '{"error":{"message":"multiple runs","code":"MULTIPLE_TRAININGS"}}' + ) + buf = io.StringIO() + old = sys.stderr + sys.stderr = buf + try: + with self.assertRaises(SystemExit): + _delete(self._args(training_id=None)) + finally: + sys.stderr = old + err = json.loads(buf.getvalue()) + self.assertIn("roboflow train list", err["error"].get("hint", "")) + + @patch("roboflow.adapters.rfapi.list_trainings_for_version") + def test_list_enumerates_training_ids(self, mock_list: MagicMock) -> None: + from roboflow.cli.handlers.train import _list + + mock_list.return_value = [ + {"trainingId": "t-1", "status": "finished", "modelType": "yolo26n", "modelIds": ["m1"]}, + {"trainingId": "t-2", "status": "stopped", "modelType": "yolo26n", "modelIds": []}, + ] + out = self._capture_stdout(_list, self._args()) + + mock_list.assert_called_once_with("test-key", "test-ws", "my-project", "3") + result = json.loads(out) + self.assertEqual([t["trainingId"] for t in result["trainings"]], ["t-1", "t-2"]) + @patch("roboflow.adapters.rfapi.restore_version_training") def test_restore_success(self, mock_restore: MagicMock) -> None: from roboflow.cli.handlers.train import _restore diff --git a/tests/test_rfapi.py b/tests/test_rfapi.py index 5e3e3586..4b0c468c 100644 --- a/tests/test_rfapi.py +++ b/tests/test_rfapi.py @@ -384,6 +384,16 @@ def test_delete_version_training_omits_training_id_for_sole_run(self): self.assertEqual(json.loads(responses.calls[0].request.body), {}) + def test_delete_version_training_rejects_blank_training_id(self): + for blank in ["", " "]: + with self.assertRaises(ValueError): + delete_version_training(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION, training_id=blank) + + def test_restore_version_training_rejects_blank_training_id(self): + for blank in ["", " "]: + with self.assertRaises(ValueError): + restore_version_training(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION, training_id=blank) + @responses.activate def test_restore_version_training_requires_training_id(self): expected_url = ( From 3c52cd93b543bd27c9ecb7455797c4f136748f3c Mon Sep 17 00:00:00 2001 From: Lee Clement Date: Mon, 27 Jul 2026 11:22:57 -0230 Subject: [PATCH 3/7] refactor: align delete/restore with the entity trash API pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The platform refactored training deletion to match project/version/workflow: HTTP DELETE on the resource and restore via the shared workspace trash route. - delete_version_training now issues DELETE /{ws}/{proj}/{ver}/v2/trainings/{training_id} (id required) and returns the shared { deleted, type, ..., trash: true } shape. - restore_version_training is gone; Training.restore, Version.restore_training, and 'roboflow train restore' go through restore_trash_item(..., "training", id) — the same route project/version/workflow restores use. - Sole-run resolution moves client-side (resolve_version_training_id): Version.delete_training() and 'roboflow train delete' without --training-id list the version's runs and only proceed when there is exactly one, keeping the MULTIPLE_TRAININGS error + 'roboflow train list' hint. - CLI restore hints now match the trash route's "not found in trash" message. Co-Authored-By: Claude Fable 5 --- roboflow/adapters/rfapi.py | 67 ++++++++++++++------------- roboflow/cli/handlers/train.py | 22 +++++---- roboflow/core/training.py | 6 +-- roboflow/core/version.py | 30 +++++++----- tests/cli/test_train_handler.py | 25 +++++----- tests/test_rfapi.py | 82 ++++++++++++++++++++------------- tests/test_training.py | 12 ++--- 7 files changed, 140 insertions(+), 104 deletions(-) diff --git a/roboflow/adapters/rfapi.py b/roboflow/adapters/rfapi.py index 0d76ca35..fd6307eb 100644 --- a/roboflow/adapters/rfapi.py +++ b/roboflow/adapters/rfapi.py @@ -141,40 +141,35 @@ def stop_version_training(api_key: str, workspace_url: str, project_url: str, ve return response.json() if response.content else {"success": True} -def delete_version_training( +def resolve_version_training_id( api_key: str, workspace_url: str, project_url: str, version: str, - *, training_id: Optional[str] = None, -): - """Move a terminal training run to the workspace Trash (soft delete). +) -> str: + """Resolve the training run a version-scoped call targets. - POST /{workspace}/{project}/{version}/v2/trainings/delete. The run and - every model it produced disappear from listings but stay restorable for - 30 days via ``restore_version_training`` or the Trash UI, after which - they are permanently deleted. The server refuses in-flight runs (stop or - cancel first) and the run backing the version's registered model. There - is no permanent-delete option on the public API. - - ``training_id`` selects a specific run on the version (MMPV); omit it - (``None``) to target the version's sole run. A blank id is rejected — on a - destructive call an empty selector must not silently fall back. + A supplied id is returned as-is (blank → ``ValueError``). When omitted, + the version's sole run is resolved via ``list_trainings_for_version``; + zero or multiple runs raise with the run ids so the caller can pick one. """ - if training_id is not None and not str(training_id).strip(): - raise ValueError("training_id must be a non-empty string when provided") - url = f"{API_URL}/{workspace_url}/{project_url}/{version}/v2/trainings/delete?api_key={api_key}" - body: Dict[str, str] = {} if training_id is not None: - body["trainingId"] = training_id - response = requests.post(url, json=body) - if not response.ok: - raise RoboflowError(response.text) - return response.json() if response.content else {"inTrash": True} + if not str(training_id).strip(): + raise ValueError("training_id must be a non-empty string when provided") + return training_id + trainings = list_trainings_for_version(api_key, workspace_url, project_url, version) + if not trainings: + raise RoboflowError(f"No training runs found for {project_url}/{version}.") + if len(trainings) > 1: + ids = ", ".join(str(t.get("trainingId") or t.get("id")) for t in trainings) + raise RoboflowError( + f"MULTIPLE_TRAININGS: version {project_url}/{version} owns several runs ({ids}); pass training_id." + ) + return str(trainings[0].get("trainingId") or trainings[0].get("id")) -def restore_version_training( +def delete_version_training( api_key: str, workspace_url: str, project_url: str, @@ -182,19 +177,27 @@ def restore_version_training( *, training_id: str, ): - """Restore a trashed training run (and its models) back into listings. + """Move a terminal training run to the workspace Trash (soft delete). + + DELETE /{workspace}/{project}/{version}/v2/trainings/{training_id} — the + same resource-DELETE pattern as project/version/workflow deletion, and the + same ``{deleted, type, ..., trash: true}`` response shape. The run and + every model it produced disappear from listings but stay restorable for + 30 days via ``restore_trash_item(..., "training", ...)`` or the Trash UI, + after which they are permanently deleted. The server refuses in-flight + runs (stop or cancel first) and the run backing the version's registered + model. There is no permanent-delete option on the public API. - POST /{workspace}/{project}/{version}/v2/trainings/restore. - ``training_id`` is required — a trashed run is invisible to the sole-run - resolver. Fails while the parent project or version is itself in Trash. + ``training_id`` is required (it is the resource path). Use + ``resolve_version_training_id`` to target a version's sole run. """ if not training_id or not str(training_id).strip(): raise ValueError("training_id is required") - url = f"{API_URL}/{workspace_url}/{project_url}/{version}/v2/trainings/restore?api_key={api_key}" - response = requests.post(url, json={"trainingId": training_id}) + url = f"{API_URL}/{workspace_url}/{project_url}/{version}/v2/trainings/{training_id}?api_key={api_key}" + response = requests.delete(url) if not response.ok: raise RoboflowError(response.text) - return response.json() if response.content else {"restored": True} + return response.json() if response.content else {"deleted": True} def get_training_results(api_key: str, workspace_url: str, project_url: str, version: str): @@ -1475,7 +1478,7 @@ def list_trash(api_key, workspace_url): def restore_trash_item(api_key, workspace_url, item_type, item_id, parent_id=None): """POST /{workspace}/trash/restore — restore an item from Trash. - `item_type` must be one of "project", "version", "workflow". + `item_type` must be one of "project", "version", "workflow", "training". `parent_id` is required when restoring a version (the parent project id). """ url = f"{API_URL}/{workspace_url}/trash/restore?api_key={api_key}" diff --git a/roboflow/cli/handlers/train.py b/roboflow/cli/handlers/train.py index 15b13fdb..c36f56c5 100644 --- a/roboflow/cli/handlers/train.py +++ b/roboflow/cli/handlers/train.py @@ -633,12 +633,19 @@ def _delete(args): # noqa: ANN001 api_key, workspace_url, project_slug, version_str = resolved try: + training_id = rfapi.resolve_version_training_id( + api_key, + workspace_url, + project_slug, + version_str, + getattr(args, "training_id", None), + ) result = rfapi.delete_version_training( api_key, workspace_url, project_slug, version_str, - training_id=getattr(args, "training_id", None), + training_id=training_id, ) except rfapi.RoboflowError as exc: msg = str(exc) @@ -672,17 +679,14 @@ def _restore(args): # noqa: ANN001 api_key, workspace_url, project_slug, version_str = resolved try: - result = rfapi.restore_version_training( - api_key, - workspace_url, - project_slug, - version_str, - training_id=args.training_id, - ) + result = rfapi.restore_trash_item(api_key, workspace_url, "training", args.training_id) except rfapi.RoboflowError as exc: msg = str(exc) hint = None - if "not in trash" in msg.lower(): + # The shared trash route reports a non-trashed id as "not found in + # trash"; the service-level guard says "not in trash". Match both + # before the parent-blocked case, which also mentions "in trash". + if "not found in trash" in msg.lower() or "not in trash" in msg.lower(): hint = "Only trashed runs can be restored. 'roboflow trash list' shows what is trashed." elif "in trash" in msg.lower(): hint = "Restore the parent project/version first ('roboflow trash list')." diff --git a/roboflow/core/training.py b/roboflow/core/training.py index 07a321e8..79e7309f 100644 --- a/roboflow/core/training.py +++ b/roboflow/core/training.py @@ -274,10 +274,8 @@ def delete(self): ) def restore(self): - """Restore this run from Trash (DNA ``trainings.restore``).""" - return rfapi.restore_version_training( - self.__api_key, self.workspace, self.project, self.version, training_id=self.training_id - ) + """Restore this run from Trash via the shared workspace trash-restore route.""" + return rfapi.restore_trash_item(self.__api_key, self.workspace, "training", self.training_id) def __str__(self): return json.dumps( diff --git a/roboflow/core/version.py b/roboflow/core/version.py index f9dba74f..ed16763e 100644 --- a/roboflow/core/version.py +++ b/roboflow/core/version.py @@ -888,37 +888,45 @@ def delete_training(self, training_id: Optional[str] = None): Args: training_id: Training id of the run to delete (MMPV versions can - own several). Omit to target the version's sole run. + own several). Omit to target the version's sole run — resolved + client-side; several runs raise with their ids listed. Returns: - dict: Server response with `{trainingId, inTrash: True, alreadyInTrash}`. + dict: Server response with `{deleted: True, type: "training", ..., trash: True}` + (the same shape as project/version/workflow deletion). """ + resolved_id = rfapi.resolve_version_training_id( + self.__api_key, + self.workspace, + self.project, + self.version, + training_id, + ) return rfapi.delete_version_training( self.__api_key, self.workspace, self.project, self.version, - training_id=training_id, + training_id=resolved_id, ) def restore_training(self, training_id: str): """ Restore one of this version's trashed training runs. + Goes through the shared workspace trash-restore route (the same one + project/version/workflow restores use) with `type: "training"`. + Args: training_id: Training id of the trashed run (required — trashed runs are invisible to the sole-run fallback). Returns: - dict: Server response with `{trainingId, restored: True}`. + dict: Server response from the trash restore route. """ - return rfapi.restore_version_training( - self.__api_key, - self.workspace, - self.project, - self.version, - training_id=training_id, - ) + if not training_id or not str(training_id).strip(): + raise ValueError("training_id is required") + return rfapi.restore_trash_item(self.__api_key, self.workspace, "training", training_id) def __str__(self): """ diff --git a/tests/cli/test_train_handler.py b/tests/cli/test_train_handler.py index 8d7e189f..d5398201 100644 --- a/tests/cli/test_train_handler.py +++ b/tests/cli/test_train_handler.py @@ -570,13 +570,13 @@ def test_stop_success(self, mock_stop: MagicMock) -> None: def test_delete_success(self, mock_delete: MagicMock) -> None: from roboflow.cli.handlers.train import _delete - mock_delete.return_value = {"trainingId": "t-1", "inTrash": True, "alreadyInTrash": False} + mock_delete.return_value = {"deleted": True, "type": "training", "trainingId": "t-1", "trash": True} out = self._capture_stdout(_delete, self._args(training_id="t-1")) mock_delete.assert_called_once_with("test-key", "test-ws", "my-project", "3", training_id="t-1") result = json.loads(out) self.assertEqual(result["status"], "in_trash") - self.assertTrue(result["inTrash"]) + self.assertTrue(result["trash"]) @patch("roboflow.adapters.rfapi.delete_version_training") def test_delete_in_progress_surfaces_hint(self, mock_delete: MagicMock) -> None: @@ -591,7 +591,7 @@ def test_delete_in_progress_surfaces_hint(self, mock_delete: MagicMock) -> None: sys.stderr = buf try: with self.assertRaises(SystemExit) as cm: - _delete(self._args(training_id=None)) + _delete(self._args(training_id="t-1")) finally: sys.stderr = old self.assertEqual(cm.exception.code, 3) @@ -600,13 +600,15 @@ def test_delete_in_progress_surfaces_hint(self, mock_delete: MagicMock) -> None: self.assertIn("train stop", err["error"].get("hint", "")) @patch("roboflow.adapters.rfapi.delete_version_training") - def test_delete_multiple_trainings_hint_points_to_train_list(self, mock_delete: MagicMock) -> None: - from roboflow.adapters import rfapi + @patch("roboflow.adapters.rfapi.list_trainings_for_version") + def test_delete_multiple_trainings_hint_points_to_train_list( + self, mock_list: MagicMock, mock_delete: MagicMock + ) -> None: from roboflow.cli.handlers.train import _delete - mock_delete.side_effect = rfapi.RoboflowError( - '{"error":{"message":"multiple runs","code":"MULTIPLE_TRAININGS"}}' - ) + # The sole-run resolution is client-side now: several runs abort the + # delete before any request is made, with the train-list hint. + mock_list.return_value = [{"trainingId": "t-1"}, {"trainingId": "t-2"}] buf = io.StringIO() old = sys.stderr sys.stderr = buf @@ -615,6 +617,7 @@ def test_delete_multiple_trainings_hint_points_to_train_list(self, mock_delete: _delete(self._args(training_id=None)) finally: sys.stderr = old + mock_delete.assert_not_called() err = json.loads(buf.getvalue()) self.assertIn("roboflow train list", err["error"].get("hint", "")) @@ -632,14 +635,14 @@ def test_list_enumerates_training_ids(self, mock_list: MagicMock) -> None: result = json.loads(out) self.assertEqual([t["trainingId"] for t in result["trainings"]], ["t-1", "t-2"]) - @patch("roboflow.adapters.rfapi.restore_version_training") + @patch("roboflow.adapters.rfapi.restore_trash_item") def test_restore_success(self, mock_restore: MagicMock) -> None: from roboflow.cli.handlers.train import _restore - mock_restore.return_value = {"trainingId": "t-1", "restored": True} + mock_restore.return_value = {"restored": True} out = self._capture_stdout(_restore, self._args(training_id="t-1")) - mock_restore.assert_called_once_with("test-key", "test-ws", "my-project", "3", training_id="t-1") + mock_restore.assert_called_once_with("test-key", "test-ws", "training", "t-1") result = json.loads(out) self.assertEqual(result["status"], "restored") diff --git a/tests/test_rfapi.py b/tests/test_rfapi.py index 4b0c468c..2b668c9b 100644 --- a/tests/test_rfapi.py +++ b/tests/test_rfapi.py @@ -13,7 +13,8 @@ get_train_recipe, get_training, list_trainings_for_version, - restore_version_training, + resolve_version_training_id, + restore_trash_item, upload_image, ) from roboflow.config import API_URL, DEFAULT_BATCH_NAME @@ -357,54 +358,73 @@ class TestTrainingTrash(unittest.TestCase): VERSION = "3" @responses.activate - def test_delete_version_training_posts_training_id(self): + def test_delete_version_training_deletes_the_training_resource(self): expected_url = ( - f"{API_URL}/{self.WORKSPACE}/{self.PROJECT}/{self.VERSION}/v2/trainings/delete?api_key={self.API_KEY}" - ) - responses.add( - responses.POST, - expected_url, - json={"trainingId": "t-1", "inTrash": True, "alreadyInTrash": False}, - status=200, + f"{API_URL}/{self.WORKSPACE}/{self.PROJECT}/{self.VERSION}/v2/trainings/t-1?api_key={self.API_KEY}" ) + payload = { + "deleted": True, + "type": "training", + "workspace": self.WORKSPACE, + "project": self.PROJECT, + "projectId": "ds-1", + "version": self.VERSION, + "trainingId": "t-1", + "trash": True, + } + responses.add(responses.DELETE, expected_url, json=payload, status=200) result = delete_version_training(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION, training_id="t-1") - self.assertEqual(json.loads(responses.calls[0].request.body), {"trainingId": "t-1"}) - self.assertEqual(result, {"trainingId": "t-1", "inTrash": True, "alreadyInTrash": False}) - - @responses.activate - def test_delete_version_training_omits_training_id_for_sole_run(self): - expected_url = ( - f"{API_URL}/{self.WORKSPACE}/{self.PROJECT}/{self.VERSION}/v2/trainings/delete?api_key={self.API_KEY}" - ) - responses.add(responses.POST, expected_url, json={"inTrash": True}, status=200) - - delete_version_training(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION) - - self.assertEqual(json.loads(responses.calls[0].request.body), {}) + self.assertEqual(result, payload) def test_delete_version_training_rejects_blank_training_id(self): for blank in ["", " "]: with self.assertRaises(ValueError): delete_version_training(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION, training_id=blank) - def test_restore_version_training_rejects_blank_training_id(self): + def test_resolve_version_training_id_returns_supplied_id_without_listing(self): + resolved = resolve_version_training_id(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION, "t-9") + self.assertEqual(resolved, "t-9") + + def test_resolve_version_training_id_rejects_blank_id(self): for blank in ["", " "]: with self.assertRaises(ValueError): - restore_version_training(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION, training_id=blank) + resolve_version_training_id(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION, blank) @responses.activate - def test_restore_version_training_requires_training_id(self): - expected_url = ( - f"{API_URL}/{self.WORKSPACE}/{self.PROJECT}/{self.VERSION}/v2/trainings/restore?api_key={self.API_KEY}" + def test_resolve_version_training_id_resolves_the_sole_run(self): + list_url = f"{API_URL}/{self.WORKSPACE}/{self.PROJECT}/{self.VERSION}/v2/trainings?api_key={self.API_KEY}" + responses.add(responses.GET, list_url, json={"trainings": [{"trainingId": "t-1"}]}, status=200) + + resolved = resolve_version_training_id(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION) + + self.assertEqual(resolved, "t-1") + + @responses.activate + def test_resolve_version_training_id_raises_when_the_version_owns_several_runs(self): + list_url = f"{API_URL}/{self.WORKSPACE}/{self.PROJECT}/{self.VERSION}/v2/trainings?api_key={self.API_KEY}" + responses.add( + responses.GET, + list_url, + json={"trainings": [{"trainingId": "t-1"}, {"trainingId": "t-2"}]}, + status=200, ) - responses.add(responses.POST, expected_url, json={"trainingId": "t-1", "restored": True}, status=200) - result = restore_version_training(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION, training_id="t-1") + with self.assertRaises(RoboflowError) as ctx: + resolve_version_training_id(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION) + self.assertIn("MULTIPLE_TRAININGS", str(ctx.exception)) + self.assertIn("t-2", str(ctx.exception)) + + @responses.activate + def test_restore_trash_item_restores_a_training(self): + expected_url = f"{API_URL}/{self.WORKSPACE}/trash/restore?api_key={self.API_KEY}" + responses.add(responses.POST, expected_url, json={"restored": True}, status=200) + + result = restore_trash_item(self.API_KEY, self.WORKSPACE, "training", "t-1") - self.assertEqual(json.loads(responses.calls[0].request.body), {"trainingId": "t-1"}) - self.assertEqual(result, {"trainingId": "t-1", "restored": True}) + self.assertEqual(json.loads(responses.calls[0].request.body), {"type": "training", "id": "t-1"}) + self.assertEqual(result, {"restored": True}) if __name__ == "__main__": diff --git a/tests/test_training.py b/tests/test_training.py index 949c9e96..e53aaf87 100644 --- a/tests/test_training.py +++ b/tests/test_training.py @@ -123,23 +123,23 @@ def test_delete_moves_run_to_trash(self): with patch( "roboflow.core.training.rfapi.delete_version_training", - return_value={"trainingId": "training-1", "inTrash": True, "alreadyInTrash": False}, + return_value={"deleted": True, "type": "training", "trainingId": "training-1", "trash": True}, ) as delete: result = training.delete() delete.assert_called_once_with("key", "ws", "proj", "1", training_id="training-1") - self.assertTrue(result["inTrash"]) + self.assertTrue(result["trash"]) - def test_restore_brings_run_back(self): + def test_restore_goes_through_the_shared_trash_route(self): training = Training("key", "ws", "proj", "1", {"trainingId": "training-1"}) with patch( - "roboflow.core.training.rfapi.restore_version_training", - return_value={"trainingId": "training-1", "restored": True}, + "roboflow.core.training.rfapi.restore_trash_item", + return_value={"restored": True}, ) as restore: result = training.restore() - restore.assert_called_once_with("key", "ws", "proj", "1", training_id="training-1") + restore.assert_called_once_with("key", "ws", "training", "training-1") self.assertTrue(result["restored"]) From beeab7def086fbc0daccea3a8c94e4192961ac39 Mon Sep 17 00:00:00 2001 From: Lee Clement Date: Mon, 27 Jul 2026 12:46:43 -0230 Subject: [PATCH 4/7] fix: train list reads the run id from the API's actual field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v2 trainings list serializes each run's id as `id` (the pre-existing serializeExternalTraining shape, shared with MCP), not `trainingId` — the CLI table rendered an empty TRAINING_ID column and the sole-run resolver's MULTIPLE_TRAININGS message listed "None". Read `id`, correct the docstring, and point the test fixtures at the real payload shape (the old fixtures used the wrong key, which is exactly what masked this). Co-Authored-By: Claude Fable 5 --- roboflow/adapters/rfapi.py | 6 +++--- roboflow/cli/handlers/train.py | 2 +- tests/cli/test_train_handler.py | 9 +++++---- tests/test_rfapi.py | 6 +++--- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/roboflow/adapters/rfapi.py b/roboflow/adapters/rfapi.py index fd6307eb..31f986aa 100644 --- a/roboflow/adapters/rfapi.py +++ b/roboflow/adapters/rfapi.py @@ -162,11 +162,11 @@ def resolve_version_training_id( if not trainings: raise RoboflowError(f"No training runs found for {project_url}/{version}.") if len(trainings) > 1: - ids = ", ".join(str(t.get("trainingId") or t.get("id")) for t in trainings) + ids = ", ".join(str(t.get("id")) for t in trainings) raise RoboflowError( f"MULTIPLE_TRAININGS: version {project_url}/{version} owns several runs ({ids}); pass training_id." ) - return str(trainings[0].get("trainingId") or trainings[0].get("id")) + return str(trainings[0].get("id")) def delete_version_training( @@ -228,7 +228,7 @@ def list_trainings_for_version(api_key: str, workspace_url: str, project_url: st GET /{ws}/{proj}/{version}/v2/trainings. MMPV versions return every run; SMPV versions return a single entry synthesized from ``version.train``. Returns the raw ``trainings`` array — each entry carries - ``{trainingId, status, modelType, modelGroup, modelIds, start}``. + ``{id, versionId, status, start, end, jobType, modelType, modelGroup, modelIds}``. """ url = f"{API_URL}/{workspace_url}/{project_url}/{version}/v2/trainings?api_key={api_key}" response = requests.get(url) diff --git a/roboflow/cli/handlers/train.py b/roboflow/cli/handlers/train.py index c36f56c5..c29d1334 100644 --- a/roboflow/cli/handlers/train.py +++ b/roboflow/cli/handlers/train.py @@ -718,7 +718,7 @@ def _list(args): # noqa: ANN001 rows = [ { - "trainingId": t.get("trainingId", ""), + "trainingId": t.get("id", ""), "status": t.get("status", ""), "modelType": t.get("modelType", ""), "models": len(t.get("modelIds") or []), diff --git a/tests/cli/test_train_handler.py b/tests/cli/test_train_handler.py index d5398201..d7d38b65 100644 --- a/tests/cli/test_train_handler.py +++ b/tests/cli/test_train_handler.py @@ -608,7 +608,7 @@ def test_delete_multiple_trainings_hint_points_to_train_list( # The sole-run resolution is client-side now: several runs abort the # delete before any request is made, with the train-list hint. - mock_list.return_value = [{"trainingId": "t-1"}, {"trainingId": "t-2"}] + mock_list.return_value = [{"id": "t-1"}, {"id": "t-2"}] buf = io.StringIO() old = sys.stderr sys.stderr = buf @@ -626,14 +626,15 @@ def test_list_enumerates_training_ids(self, mock_list: MagicMock) -> None: from roboflow.cli.handlers.train import _list mock_list.return_value = [ - {"trainingId": "t-1", "status": "finished", "modelType": "yolo26n", "modelIds": ["m1"]}, - {"trainingId": "t-2", "status": "stopped", "modelType": "yolo26n", "modelIds": []}, + {"id": "t-1", "status": "finished", "modelType": "yolo26n", "modelIds": ["m1"]}, + {"id": "t-2", "status": "stopped", "modelType": "yolo26n", "modelIds": []}, ] out = self._capture_stdout(_list, self._args()) mock_list.assert_called_once_with("test-key", "test-ws", "my-project", "3") result = json.loads(out) - self.assertEqual([t["trainingId"] for t in result["trainings"]], ["t-1", "t-2"]) + self.assertEqual([t["id"] for t in result["trainings"]], ["t-1", "t-2"]) + self.assertIn("t-1", out) @patch("roboflow.adapters.rfapi.restore_trash_item") def test_restore_success(self, mock_restore: MagicMock) -> None: diff --git a/tests/test_rfapi.py b/tests/test_rfapi.py index 2b668c9b..70a799f6 100644 --- a/tests/test_rfapi.py +++ b/tests/test_rfapi.py @@ -308,7 +308,7 @@ def test_create_training_v2_raises_on_error(self): @responses.activate def test_list_trainings_for_version_unwraps_trainings_key(self): - payload = {"trainings": [{"trainingId": "t-1"}, {"trainingId": "t-2"}]} + payload = {"trainings": [{"id": "t-1"}, {"id": "t-2"}]} responses.add(responses.GET, self.BASE_URL, json=payload, status=200) result = list_trainings_for_version(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION) @@ -395,7 +395,7 @@ def test_resolve_version_training_id_rejects_blank_id(self): @responses.activate def test_resolve_version_training_id_resolves_the_sole_run(self): list_url = f"{API_URL}/{self.WORKSPACE}/{self.PROJECT}/{self.VERSION}/v2/trainings?api_key={self.API_KEY}" - responses.add(responses.GET, list_url, json={"trainings": [{"trainingId": "t-1"}]}, status=200) + responses.add(responses.GET, list_url, json={"trainings": [{"id": "t-1"}]}, status=200) resolved = resolve_version_training_id(self.API_KEY, self.WORKSPACE, self.PROJECT, self.VERSION) @@ -407,7 +407,7 @@ def test_resolve_version_training_id_raises_when_the_version_owns_several_runs(s responses.add( responses.GET, list_url, - json={"trainings": [{"trainingId": "t-1"}, {"trainingId": "t-2"}]}, + json={"trainings": [{"id": "t-1"}, {"id": "t-2"}]}, status=200, ) From f49a6d0c1b5db3203b0a6b80e57ab3c65d3fcb6c Mon Sep 17 00:00:00 2001 From: Lee Clement Date: Mon, 27 Jul 2026 13:31:34 -0230 Subject: [PATCH 5/7] fix: blank training ids fail as structured CLI usage errors Review follow-up: a blank --training-id raised a bare ValueError past the CLI's error path, and restore forwarded whitespace ids to the API. - restore_trash_item validates item_id (shared by every trash restore). - train delete/restore map ValueError through output_error with exit code 2 and a --training-id hint. - Tests: structured error + exit code for blank delete/restore, blank Training.restore(), and blank restore_trash_item ids. Co-Authored-By: Claude Fable 5 --- roboflow/adapters/rfapi.py | 2 ++ roboflow/cli/handlers/train.py | 6 ++++++ tests/cli/test_train_handler.py | 34 +++++++++++++++++++++++++++++++++ tests/test_rfapi.py | 5 +++++ tests/test_training.py | 6 ++++++ 5 files changed, 53 insertions(+) diff --git a/roboflow/adapters/rfapi.py b/roboflow/adapters/rfapi.py index 31f986aa..e2631122 100644 --- a/roboflow/adapters/rfapi.py +++ b/roboflow/adapters/rfapi.py @@ -1481,6 +1481,8 @@ def restore_trash_item(api_key, workspace_url, item_type, item_id, parent_id=Non `item_type` must be one of "project", "version", "workflow", "training". `parent_id` is required when restoring a version (the parent project id). """ + if not item_id or not str(item_id).strip(): + raise ValueError("item_id is required") url = f"{API_URL}/{workspace_url}/trash/restore?api_key={api_key}" payload = {"type": item_type, "id": item_id} if parent_id is not None: diff --git a/roboflow/cli/handlers/train.py b/roboflow/cli/handlers/train.py index c29d1334..9520fe19 100644 --- a/roboflow/cli/handlers/train.py +++ b/roboflow/cli/handlers/train.py @@ -647,6 +647,9 @@ def _delete(args): # noqa: ANN001 version_str, training_id=training_id, ) + except ValueError as exc: + output_error(args, str(exc), hint="Pass a non-empty --training-id.", exit_code=2) + return except rfapi.RoboflowError as exc: msg = str(exc) hint = None @@ -680,6 +683,9 @@ def _restore(args): # noqa: ANN001 try: result = rfapi.restore_trash_item(api_key, workspace_url, "training", args.training_id) + except ValueError as exc: + output_error(args, str(exc), hint="Pass a non-empty --training-id.", exit_code=2) + return except rfapi.RoboflowError as exc: msg = str(exc) hint = None diff --git a/tests/cli/test_train_handler.py b/tests/cli/test_train_handler.py index d7d38b65..0edb087e 100644 --- a/tests/cli/test_train_handler.py +++ b/tests/cli/test_train_handler.py @@ -636,6 +636,40 @@ def test_list_enumerates_training_ids(self, mock_list: MagicMock) -> None: self.assertEqual([t["id"] for t in result["trainings"]], ["t-1", "t-2"]) self.assertIn("t-1", out) + @patch("roboflow.adapters.rfapi.delete_version_training") + def test_delete_blank_training_id_is_a_structured_usage_error(self, mock_delete: MagicMock) -> None: + from roboflow.cli.handlers.train import _delete + + buf = io.StringIO() + old = sys.stderr + sys.stderr = buf + try: + with self.assertRaises(SystemExit) as cm: + _delete(self._args(training_id=" ")) + finally: + sys.stderr = old + self.assertEqual(cm.exception.code, 2) + mock_delete.assert_not_called() + err = json.loads(buf.getvalue()) + self.assertIn("non-empty", err["error"]["message"]) + self.assertIn("--training-id", err["error"].get("hint", "")) + + def test_restore_blank_training_id_is_a_structured_usage_error(self) -> None: + from roboflow.cli.handlers.train import _restore + + buf = io.StringIO() + old = sys.stderr + sys.stderr = buf + try: + with self.assertRaises(SystemExit) as cm: + _restore(self._args(training_id=" ")) + finally: + sys.stderr = old + self.assertEqual(cm.exception.code, 2) + err = json.loads(buf.getvalue()) + self.assertIn("required", err["error"]["message"]) + self.assertIn("--training-id", err["error"].get("hint", "")) + @patch("roboflow.adapters.rfapi.restore_trash_item") def test_restore_success(self, mock_restore: MagicMock) -> None: from roboflow.cli.handlers.train import _restore diff --git a/tests/test_rfapi.py b/tests/test_rfapi.py index 70a799f6..d0106312 100644 --- a/tests/test_rfapi.py +++ b/tests/test_rfapi.py @@ -416,6 +416,11 @@ def test_resolve_version_training_id_raises_when_the_version_owns_several_runs(s self.assertIn("MULTIPLE_TRAININGS", str(ctx.exception)) self.assertIn("t-2", str(ctx.exception)) + def test_restore_trash_item_rejects_blank_ids(self): + for blank in ["", " ", None]: + with self.assertRaises(ValueError): + restore_trash_item(self.API_KEY, self.WORKSPACE, "training", blank) + @responses.activate def test_restore_trash_item_restores_a_training(self): expected_url = f"{API_URL}/{self.WORKSPACE}/trash/restore?api_key={self.API_KEY}" diff --git a/tests/test_training.py b/tests/test_training.py index e53aaf87..b8f32e89 100644 --- a/tests/test_training.py +++ b/tests/test_training.py @@ -130,6 +130,12 @@ def test_delete_moves_run_to_trash(self): delete.assert_called_once_with("key", "ws", "proj", "1", training_id="training-1") self.assertTrue(result["trash"]) + def test_restore_rejects_a_blank_training_id(self): + training = Training("key", "ws", "proj", "1", {"trainingId": " "}) + + with self.assertRaises(ValueError): + training.restore() + def test_restore_goes_through_the_shared_trash_route(self): training = Training("key", "ws", "proj", "1", {"trainingId": "training-1"}) From 352d717f8b84f09fa07057ceeac6fd01d32ccc57 Mon Sep 17 00:00:00 2001 From: Lee Clement Date: Mon, 27 Jul 2026 13:51:58 -0230 Subject: [PATCH 6/7] docs: strip internal platform jargon from new docstrings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit External SDK users have no context for "DNA" or "MMPV" — say what the commands and methods do in product terms instead. Co-Authored-By: Claude Fable 5 --- roboflow/cli/handlers/train.py | 2 +- roboflow/core/training.py | 2 +- roboflow/core/version.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/roboflow/cli/handlers/train.py b/roboflow/cli/handlers/train.py index 9520fe19..e3f9b4bb 100644 --- a/roboflow/cli/handlers/train.py +++ b/roboflow/cli/handlers/train.py @@ -238,7 +238,7 @@ def list_trainings( ) -> None: """List a version's training runs with their ids. - An MMPV version may own several runs; use the TRAINING_ID column with + A version may own several training runs; use the TRAINING_ID column with 'roboflow train delete/restore --training-id' or 'train cancel/stop'. """ args = ctx_to_args(ctx, target=target) diff --git a/roboflow/core/training.py b/roboflow/core/training.py index 79e7309f..f4ec7bc4 100644 --- a/roboflow/core/training.py +++ b/roboflow/core/training.py @@ -262,7 +262,7 @@ def stop(self): ) def delete(self): - """Move this run to the workspace Trash (soft delete, DNA ``trainings.delete``). + """Move this run to the workspace Trash (soft delete). The run and every model it produced disappear from listings but stay restorable for 30 days via :meth:`restore` or the Trash UI. The server diff --git a/roboflow/core/version.py b/roboflow/core/version.py index ed16763e..5bfdca10 100644 --- a/roboflow/core/version.py +++ b/roboflow/core/version.py @@ -887,8 +887,8 @@ def delete_training(self, training_id: Optional[str] = None): version's registered model. Args: - training_id: Training id of the run to delete (MMPV versions can - own several). Omit to target the version's sole run — resolved + training_id: Training id of the run to delete (a version can own + several runs). Omit to target the version's sole run — resolved client-side; several runs raise with their ids listed. Returns: From 2cae9870f46e5cdaae88df4618cf9812f365f88b Mon Sep 17 00:00:00 2001 From: Lee Clement Date: Tue, 28 Jul 2026 11:33:37 -0230 Subject: [PATCH 7/7] docs: version endpoints always serve the oldest remaining run's model The platform replaced the delete refusal for the serving run with a single rule: {project}/{version} serves the oldest surviving run's model. Deleting the serving run switches serving to the next-oldest run (versionAliasAction "repointed" + target) or stops it when none survives ("deleted"); restoring the oldest run hands serving back. Update docstrings and CLI help, drop the now-impossible "register another model first" hint, and print the switch or stop in train delete output. Co-Authored-By: Claude Fable 5 --- roboflow/cli/handlers/train.py | 23 +++++++++++++++----- roboflow/core/training.py | 8 +++++-- roboflow/core/version.py | 7 +++++-- tests/cli/test_train_handler.py | 37 +++++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 9 deletions(-) diff --git a/roboflow/cli/handlers/train.py b/roboflow/cli/handlers/train.py index e3f9b4bb..f2a69651 100644 --- a/roboflow/cli/handlers/train.py +++ b/roboflow/cli/handlers/train.py @@ -197,8 +197,10 @@ def delete_training( The run and every model it produced disappear from listings but stay restorable for 30 days ('roboflow train restore' or the web Trash view), after which they are permanently deleted. In-flight runs are refused — - stop or cancel first — as is the run backing the version's registered - model. Permanent deletion is only available in the web UI's Trash view. + stop or cancel first. The version's hosted endpoint always serves the + oldest remaining run's model, so deleting the serving run switches + serving to the next-oldest run, or stops it when none survives. + Permanent deletion is only available in the web UI's Trash view. """ args = ctx_to_args(ctx, target=target, training_id=training_id) _delete(args) @@ -655,19 +657,30 @@ def _delete(args): # noqa: ANN001 hint = None if "in progress" in msg: hint = "Stop or cancel the run first: 'roboflow train stop /'." - elif "registered model" in msg: - hint = "This run backs the version's registered model. Register another model first." elif "MULTIPLE_TRAININGS" in msg: hint = "This version owns several runs. Pass --training-id (see 'roboflow train list /')." output_error(args, msg, hint=hint, exit_code=3) return + alias_action = (result or {}).get("versionAliasAction") + if alias_action == "repointed": + alias_note = ( + f" Serving for '{project_slug}/{version_str}' switched to " + f"'{(result or {}).get('versionAliasTarget', 'the next-oldest model')}'." + ) + elif alias_action == "deleted": + alias_note = ( + f" No other model remains, so '{project_slug}/{version_str}' stops serving " + "until a new training completes or this run is restored." + ) + else: + alias_note = "" output( args, {"status": "in_trash", "project": project_slug, "version": version_str, **(result or {})}, text=( f"Training moved to Trash for {project_slug} version {version_str}. " - "Restorable for 30 days via 'roboflow train restore'." + f"Restorable for 30 days via 'roboflow train restore'.{alias_note}" ), ) diff --git a/roboflow/core/training.py b/roboflow/core/training.py index f4ec7bc4..96e02f7f 100644 --- a/roboflow/core/training.py +++ b/roboflow/core/training.py @@ -266,8 +266,12 @@ def delete(self): The run and every model it produced disappear from listings but stay restorable for 30 days via :meth:`restore` or the Trash UI. The server - refuses in-flight runs (stop or cancel first) and the run backing the - version's registered model. + refuses in-flight runs (stop or cancel first). The version's hosted + endpoint always serves the oldest remaining run's model: if this run + was serving, serving switches to the next-oldest run (the response + reports ``versionAliasAction: "repointed"`` and the new target) or + stops when no other run survives (``"deleted"``); restoring the + oldest run hands serving back. """ return rfapi.delete_version_training( self.__api_key, self.workspace, self.project, self.version, training_id=self.training_id diff --git a/roboflow/core/version.py b/roboflow/core/version.py index 5bfdca10..dee2cce2 100644 --- a/roboflow/core/version.py +++ b/roboflow/core/version.py @@ -883,8 +883,11 @@ def delete_training(self, training_id: Optional[str] = None): The run and every model it produced disappear from listings but stay restorable for 30 days via `Version.restore_training()` or the Trash UI, after which they are permanently deleted. The server refuses - in-flight runs (stop or cancel first) and the run backing the - version's registered model. + in-flight runs (stop or cancel first). The version's hosted endpoint + always serves the oldest remaining run's model: deleting the serving + run switches serving to the next-oldest run (`versionAliasAction: + "repointed"`) or stops it when no other run survives (`"deleted"`); + restoring the oldest run hands serving back. Args: training_id: Training id of the run to delete (a version can own diff --git a/tests/cli/test_train_handler.py b/tests/cli/test_train_handler.py index 0edb087e..fcb1517b 100644 --- a/tests/cli/test_train_handler.py +++ b/tests/cli/test_train_handler.py @@ -636,6 +636,43 @@ def test_list_enumerates_training_ids(self, mock_list: MagicMock) -> None: self.assertEqual([t["id"] for t in result["trainings"]], ["t-1", "t-2"]) self.assertIn("t-1", out) + @patch("roboflow.adapters.rfapi.delete_version_training") + def test_delete_notes_the_serving_switch(self, mock_delete: MagicMock) -> None: + from roboflow.cli.handlers.train import _delete + + mock_delete.return_value = { + "deleted": True, + "type": "training", + "trainingId": "t-1", + "trash": True, + "versionAliasAction": "repointed", + "versionAliasTarget": "test-ws/next-oldest-model", + } + args = self._args(training_id="t-1") + args.json = False + out = self._capture_stdout(_delete, args) + + self.assertIn("switched to", out) + self.assertIn("next-oldest-model", out) + + @patch("roboflow.adapters.rfapi.delete_version_training") + def test_delete_notes_the_serving_stop_when_no_model_remains(self, mock_delete: MagicMock) -> None: + from roboflow.cli.handlers.train import _delete + + mock_delete.return_value = { + "deleted": True, + "type": "training", + "trainingId": "t-1", + "trash": True, + "versionAliasAction": "deleted", + } + args = self._args(training_id="t-1") + args.json = False + out = self._capture_stdout(_delete, args) + + self.assertIn("stops serving", out) + self.assertIn("my-project/3", out) + @patch("roboflow.adapters.rfapi.delete_version_training") def test_delete_blank_training_id_is_a_structured_usage_error(self, mock_delete: MagicMock) -> None: from roboflow.cli.handlers.train import _delete