fix: allow deleting predictions imported without model_version#9737
fix: allow deleting predictions imported without model_version#9737kcw2034 wants to merge 2 commits into
Conversation
👷 Deploy request for heartex-docs pending review.Visit the deploys page to approve it
|
👷 Deploy request for label-studio-docs-new-theme pending review.Visit the deploys page to approve it
|
✅ Deploy Preview for label-studio-playground canceled.
|
✅ Deploy Preview for label-studio-storybook canceled.
|
…ersion Issue HumanSignal#9717 — imported predictions whose model_version was missing could not be deleted via DELETE /api/projects/<pk>/model-versions. Three legacy import paths defaulted the field to the literal string 'undefined' while other paths stored NULL, and the FE echoed those values back to a delete endpoint that rejected anything falsy. Backend - Three import sites (data_import/api.py × 2, tasks/serializers.py) now store None instead of 'undefined' when the field is missing from input. - DELETE /api/projects/<pk>/model-versions accepts JSON null, empty string, and the legacy 'undefined' string as "no model version" and routes them through a single isnull-aware delete path. - Project.delete_predictions(None) sweeps NULL / '' / 'undefined' together so callers do not have to know about the pre-migration data layout. - Migration tasks/0062 backfills 'undefined' rows to NULL. Frontend - PredictionsList renders "No model version" with an explanatory tooltip for falsy and legacy-'undefined' values; the stale === "undefined" comparison is gone. Tests - projects/tests/test_api.py adds TestDeletePredictionsAPI covering specific version, JSON null, empty string, legacy 'undefined', mixed null-ish sweep, and missing-key 400. Related: review of upstream PR HumanSignal#9735. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
0868637 to
24c45ae
Compare
… tests - Migration 0062 now runs the UPDATE in 1000-row batches with atomic=False so it does not hold locks for the full duration on large Prediction tables. - Add TestImportPredictionsModelVersionDefault to verify the three import sites store NULL (not 'undefined') when model_version is missing/empty, closing the loop between import and the delete endpoint fix. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Addressed review feedback: #1 #2 Title prefix — changed #3 Migration batching — #5 Import-site integration test — added #4 Final state: 12/12 new tests pass (6 delete + 3 import + 3 existing model-versions). Branch is 2 commits ahead of |
Summary
Resolves #9717. Predictions imported without a
model_version(e.g., from cloud storage or JSON files lacking the field) could not be deleted because the DELETE endpoint rejected falsy values and the import layer stored the literal string'undefined'instead ofNULL.This is an alternative to #9735 that fixes the root cause (import-side) rather than masking it in the API.
Why not #9735
#9735 patches the DELETE endpoint to treat the literal string
'undefined'and'null'as null-version. That works, but:'undefined'string originates from three BE import sites (data_import/api.py×2,tasks/serializers.py) defaulting to'undefined'whenmodel_versionis missing. The frontend just echoes it back.'undefined'model would lose unrelated predictions.'undefined'vs'') is left unresolved.What this PR does
Backend
'undefined'— three call sites now useitem.get('model_version') or None, so future imports without a version land asNULL.null/\"\"/'undefined'as "no model version" via a single normalization step, then routes through anisnull-aware delete.Project.delete_predictions(None)sweepsNULL∪''∪'undefined'so callers don't need to know about the pre-migration data layout. This is robust during rolling deployments where some workers may still write the old value.tasks/0062backfills existing'undefined'rows toNULL. Reverse is intentionally a no-op (can't distinguish migrated rows from genuinely-NULL ones).Frontend
PredictionsList.jsxremoves the=== \"undefined\"magic-string comparison. Falsy + legacy'undefined'now both display "No model version" with an explanatory tooltip.Tests
projects/tests/test_api.py::TestDeletePredictionsAPIcovers six scenarios: specific version, JSON null, empty string, legacy'undefined'string, mixed null-ish sweep, and missing-key 400.Test plan
pytest label_studio/projects/tests/test_api.py::TestDeletePredictionsAPI— 6/6 passpytest label_studio/projects/tests/test_api.py::TestProjectModelVersionsAPI— 3/3 pass (no regression)showmigrations taskslists0062_backfill_undefined_model_versionmodel_version, verify they delete cleanlyFiles changed (7, +149 / -18)
data_import/api.py(×2)'undefined'on missing fieldtasks/serializers.pyprojects/api.pyprojects/models.pydelete_predictions(None)sweeps null-ishtasks/migrations/0062_*.pyprojects/tests/test_api.pyweb/apps/labelstudio/.../PredictionsList.jsx🤖 Generated with Claude Code