Merge pull request #1976 from transformerlab/fix/job-delete-and-cli-commands

Fix job delete endpoint and add lab job delete CLI commands

Author: aliasaria

.agents/skills/transformerlab-cli/SKILL.md

@@ -505,7 +505,7 @@ Provider configs (`api_token`, `api_key`, `ssh_key_path`) contain secrets. If th
 3. **Use `--format json`** when you need to parse output, but be prepared to fall back to pretty output parsing if it doesn't work
 4. **`--no-interactive` on `task queue` silently uses the DEFAULT provider (Local).** There is no `--provider` flag. To target a specific provider, you must drive the interactive prompts (see "Selecting a provider" below).
 5. **`task add` has no `--yes` flag** — pipe `echo "y"` to confirm: `echo "y" | lab task add ./my-task`
-6. **Skip confirmation on destructive commands:** use `--no-interactive` for `provider delete`, and `--yes` / `-y` for `model delete` / `dataset delete` (the flag names differ — verify with `--help`)
+6. **Skip confirmation on destructive commands:** use `--no-interactive` for `provider delete`, `job delete`, and `job delete-all`; use `--yes` / `-y` for `model delete` / `dataset delete` (the flag names differ — verify with `--help`)
 7. **Never use `job monitor`** — it launches a TUI that blocks; use `job list` + `job task-logs` instead
 8. **Never use `task interactive`** unless the user specifically requests an interactive session
 9. **`job task-logs --follow`** streams continuously and blocks until the job finishes — use when the user wants real-time monitoring
@@ -662,6 +662,8 @@ This applies to launching jobs, fetching logs, checking cluster status, and ever
 | `lab job artifacts <id>` | List job artifacts | Yes |
 | `lab job download <id>` | Download artifacts (`--file` for glob) | Yes |
 | `lab job stop <id>` | Stop a running job | Yes |
+| `lab job delete <id>` | Delete a job (`--no-interactive` to skip prompt) | Yes |
+| `lab job delete-all` | Delete all jobs in the current experiment (`--no-interactive` to skip prompt) | Yes |
 | `lab provider list` | List compute providers | No |
 | `lab provider info <id>` | Show provider details | No |
 | `lab provider add` | Add a new provider | No |

api/localprovider_pyproject.toml

@@ -32,7 +32,7 @@ dependencies = [
   "soundfile==0.13.1",
   "tensorboardX==2.6.2.2",
   "timm==1.0.15",
-  "transformerlab==0.1.28",
+  "transformerlab==0.1.29",
   "transformerlab-inference==0.2.52",
   "transformers==4.57.1",
   "wandb==0.23.1",

api/pyproject.toml

@@ -25,7 +25,7 @@ dependencies = [
   "python-dotenv==1.1.0",
   "python-multipart==0.0.20",
   "sqlalchemy[asyncio]==2.0.40",
-  "transformerlab==0.1.28",
+  "transformerlab==0.1.29",
   "transformerlab-inference==0.2.52",
   "uvicorn==0.35.0",
   "watchfiles==1.0.5",

api/test/api/test_experiment_jobs.py

@@ -30,11 +30,11 @@ def test_job_creation_and_management(client):
     assert resp.status_code in (200, 404)
 
     # Test job delete
-    resp = client.get("/experiment/alpha/jobs/delete/1")
+    resp = client.delete("/experiment/alpha/jobs/1")
     assert resp.status_code in (200, 404)
 
     # Test delete all jobs
-    resp = client.get("/experiment/alpha/jobs/delete_all")
+    resp = client.delete("/experiment/alpha/jobs/delete_all")
     assert resp.status_code == 200
 
 

api/test/api/test_job_delete_service.py

@@ -0,0 +1,101 @@
+"""Tests for job_service.job_delete and job_delete_all.
+
+These regression tests cover the bug where:
+- job_delete swallowed exceptions (silent failure, stale cache)
+- The per-job cache was never invalidated after delete, so terminal-status
+  jobs (COMPLETE/FAILED/STOPPED) cached for 7 days continued to appear in
+  /jobs/list with their pre-delete status.
+"""
+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from transformerlab.services import job_service
+
+
+@pytest.mark.asyncio
+async def test_job_delete_invalidates_cache():
+    """After job_delete the per-job cache key must be cleared."""
+    fake_job = MagicMock()
+    fake_job.delete = AsyncMock()
+
+    with (
+        patch.object(job_service, "_resolve_full_job_id", AsyncMock(return_value="abc123")),
+        patch.object(job_service.Job, "get", AsyncMock(return_value=fake_job)),
+        patch.object(job_service.cache, "invalidate", AsyncMock()) as mock_invalidate,
+        patch.object(job_service.cache, "delete", AsyncMock()) as mock_delete_cache,
+    ):
+        await job_service.job_delete("abc123", experiment_id="exp1")
+
+    fake_job.delete.assert_awaited_once()
+    mock_invalidate.assert_awaited_once_with("job:abc123")
+    mock_delete_cache.assert_awaited_once_with(job_service._job_cache_key("abc123"))
+
+
+@pytest.mark.asyncio
+async def test_job_delete_propagates_filenotfound():
+    """job_delete must NOT swallow FileNotFoundError so the API can return 404."""
+    with (
+        patch.object(job_service, "_resolve_full_job_id", AsyncMock(return_value=None)),
+        patch.object(
+            job_service.Job,
+            "get",
+            AsyncMock(side_effect=FileNotFoundError("no such job")),
+        ),
+    ):
+        with pytest.raises(FileNotFoundError):
+            await job_service.job_delete("missing", experiment_id="exp1")
+
+
+@pytest.mark.asyncio
+async def test_job_delete_all_invalidates_cache_for_each_job():
+    """job_delete_all must invalidate the per-job cache for every job."""
+    fake_experiment = MagicMock()
+    fake_experiment.delete_all_jobs = AsyncMock()
+
+    with (
+        patch.object(
+            job_service,
+            "_list_experiment_job_ids",
+            AsyncMock(return_value=["jobA", "jobB", "jobC"]),
+        ),
+        patch.object(job_service, "Experiment", return_value=fake_experiment),
+        patch.object(job_service.cache, "invalidate", AsyncMock()) as mock_invalidate,
+        patch.object(job_service.cache, "delete", AsyncMock()) as mock_delete_cache,
+    ):
+        deleted = await job_service.job_delete_all(experiment_id="exp1")
+
+    assert deleted == 3
+    fake_experiment.delete_all_jobs.assert_awaited_once()
+    assert mock_invalidate.await_count == 3
+    assert mock_delete_cache.await_count == 3
+    invalidated_tags = {call.args[0] for call in mock_invalidate.await_args_list}
+    assert invalidated_tags == {"job:jobA", "job:jobB", "job:jobC"}
+
+
+@pytest.mark.asyncio
+async def test_job_delete_all_handles_empty_experiment():
+    """job_delete_all on an empty experiment returns 0 and doesn't error."""
+    fake_experiment = MagicMock()
+    fake_experiment.delete_all_jobs = AsyncMock()
+
+    with (
+        patch.object(job_service, "_list_experiment_job_ids", AsyncMock(return_value=[])),
+        patch.object(job_service, "Experiment", return_value=fake_experiment),
+        patch.object(job_service.cache, "invalidate", AsyncMock()) as mock_invalidate,
+    ):
+        deleted = await job_service.job_delete_all(experiment_id="exp1")
+
+    assert deleted == 0
+    mock_invalidate.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+async def test_job_delete_all_returns_zero_for_none_experiment():
+    """Passing None as experiment_id should return 0 without side effects."""
+    with patch.object(job_service.cache, "invalidate", AsyncMock()) as mock_invalidate:
+        deleted = await job_service.job_delete_all(experiment_id=None)
+
+    assert deleted == 0
+    mock_invalidate.assert_not_awaited()

api/test/api/test_jobs.py

@@ -21,12 +21,12 @@ def test_jobs_list(client):
 
 
 def test_jobs_delete_all(client):
-    resp = client.get("/experiment/1/jobs/delete_all")
+    resp = client.delete("/experiment/1/jobs/delete_all")
     assert resp.status_code == 200
     data = resp.json()
-    assert "message" in data or data == []
-    if "message" in data:
-        assert isinstance(data["message"], str)
+    assert data.get("message") == "OK"
+    assert "deleted" in data
+    assert isinstance(data["deleted"], int)
 
 
 def test_jobs_get_by_id(client):
@@ -35,10 +35,16 @@ def test_jobs_get_by_id(client):
 
 
 def test_jobs_delete_by_id(client):
-    resp = client.get("/experiment/1/jobs/delete/1")
+    resp = client.delete("/experiment/1/jobs/1")
     assert resp.status_code in (200, 404)
 
 
+def test_jobs_delete_by_id_returns_404_when_missing(client):
+    """A nonexistent job id should now return 404 (not the previous silent 200)."""
+    resp = client.delete("/experiment/1/jobs/nonexistent-job-id-xyz")
+    assert resp.status_code == 404
+
+
 def test_jobs_get_template(client):
     resp = client.get("/experiment/1/jobs/template/1")
     assert resp.status_code in (200, 404)

api/transformerlab/routers/experiment/jobs.py

@@ -2,6 +2,7 @@
 import csv
 from fnmatch import fnmatch
 import json
+import logging
 import os
 import posixpath
 from typing import List, Optional
@@ -41,6 +42,8 @@
 from transformerlab.services import asset_version_service
 
 
+logger = logging.getLogger(__name__)
+
 router = APIRouter(prefix="/jobs", tags=["train"])
 
 
@@ -123,9 +126,14 @@ async def jobs_get_all(
     return jobs
 
 
-@router.get("/delete/{job_id}")
-async def job_delete(job_id: str, experimentId: str):
-    await job_service.job_delete(job_id, experiment_id=experimentId)
+async def _job_delete_handler(job_id: str, experimentId: str) -> dict:
+    try:
+        await job_service.job_delete(job_id, experiment_id=experimentId)
+    except FileNotFoundError:
+        raise HTTPException(status_code=404, detail=f"Job {job_id} not found in experiment {experimentId}")
+    except Exception as e:
+        logger.exception("Failed to delete job (job_id=%s, experiment=%s)", job_id, experimentId)
+        raise HTTPException(status_code=500, detail=f"Failed to delete job {job_id}: {e}")
     return {"message": "OK"}
 
 
@@ -174,10 +182,23 @@ async def stop_job(job_id: str, experimentId: str):
     return {"message": "OK"}
 
 
-@router.get("/delete_all")
+async def _job_delete_all_handler(experimentId: str) -> dict:
+    try:
+        deleted = await job_service.job_delete_all(experiment_id=experimentId)
+    except Exception as e:
+        logger.exception("Failed to delete all jobs (experiment=%s)", experimentId)
+        raise HTTPException(status_code=500, detail=f"Failed to delete all jobs: {e}")
+    return {"message": "OK", "deleted": deleted}
+
+
+@router.delete("/delete_all")
 async def job_delete_all(experimentId: str):
-    await job_service.job_delete_all(experiment_id=experimentId)
-    return {"message": "OK"}
+    return await _job_delete_all_handler(experimentId)
+
+
+@router.delete("/{job_id}")
+async def job_delete(job_id: str, experimentId: str):
+    return await _job_delete_handler(job_id, experimentId)
 
 
 @router.get("/{job_id}")

api/transformerlab/services/job_service.py

@@ -347,18 +347,51 @@ async def job_count_running():
     return await Job.count_running_jobs()
 
 
-async def job_delete_all(experiment_id):
-    if experiment_id is not None:
-        experiment = Experiment(experiment_id)
-        await experiment.delete_all_jobs()
+async def job_delete_all(experiment_id) -> int:
+    """Soft-delete all jobs in an experiment.
+
+    Returns the number of jobs that existed when the operation started so the
+    caller can report progress. Per-job cache entries are invalidated so terminal
+    statuses cached for 7 days don't keep deleted jobs visible in /jobs/list.
+    """
+    if experiment_id is None:
+        return 0
+
+    job_ids = await _list_experiment_job_ids(experiment_id)
+    if not job_ids:
+        return 0
+
+    experiment = Experiment(experiment_id)
+    await experiment.delete_all_jobs()
+
+    for job_id in job_ids:
+        try:
+            await cache.invalidate(f"job:{job_id}")
+            await cache.delete(_job_cache_key(job_id))
+        except Exception:
+            logger.exception("Failed to invalidate cache after delete_all (job_id=%s)", job_id)
+
+    return len(job_ids)
 
 
 async def job_delete(job_id, experiment_id):
+    """Soft-delete a job (sets status to DELETED).
+
+    Raises FileNotFoundError if the job directory does not exist; other exceptions
+    propagate so the caller can return a meaningful HTTP error.
+    """
+    resolved_id = await _resolve_full_job_id(str(job_id), str(experiment_id))
+    actual_id = resolved_id or str(job_id)
+
+    job = await Job.get(actual_id, experiment_id)
+    await job.delete()
+
+    # Invalidate cache so subsequent listings/reads see the DELETED status.
     try:
-        job = await Job.get(job_id, experiment_id)
-        await job.delete()
-    except Exception as e:
-        print(f"Error deleting job {job_id}: {e}")
+        await cache.invalidate(f"job:{actual_id}")
+        await cache.delete(_job_cache_key(actual_id))
+    except Exception:
+        logger.exception("Failed to invalidate cache after job delete (job_id=%s)", actual_id)
 
 
 async def job_update_job_data_insert_key_value(job_id, key, value, experiment_id):