Skip to content

Commit 24c45ae

Browse files
kcw2034claude
andcommitted
fix(predictions): allow deleting predictions imported without model_version
Issue #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 #9735. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent ffd4fb8 commit 24c45ae

7 files changed

Lines changed: 149 additions & 18 deletions

File tree

label_studio/data_import/api.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ def _create_memory_efficient(self, project):
549549
project_id=project.id,
550550
result=Prediction.prepare_prediction_result(item.get('result'), project),
551551
score=item.get('score'),
552-
model_version=item.get('model_version', 'undefined'),
552+
model_version=item.get('model_version') or None,
553553
)
554554
)
555555
all_task_ids.add(task_id)
@@ -623,7 +623,7 @@ def _create_legacy(self, project):
623623
project_id=project.id,
624624
result=Prediction.prepare_prediction_result(item.get('result'), project),
625625
score=item.get('score'),
626-
model_version=item.get('model_version', 'undefined'),
626+
model_version=item.get('model_version') or None,
627627
)
628628
)
629629
except Exception as e:

label_studio/projects/api.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -881,13 +881,19 @@ def get(self, request, *args, **kwargs):
881881

882882
def delete(self, request, *args, **kwargs):
883883
project = self.get_object()
884-
model_version = request.data.get('model_version', None)
885884

886-
if not model_version:
885+
if 'model_version' not in request.data:
887886
raise RestValidationError('model_version param is required')
887+
model_version = request.data.get('model_version')
888888

889-
count = project.delete_predictions(model_version=model_version)
889+
# Normalize representations of "no model version" to None. JSON null
890+
# already arrives as None; the empty string is the model field default,
891+
# and the literal string 'undefined' is legacy data from older imports
892+
# (see migration that backfills it to NULL).
893+
if model_version in (None, '', 'undefined'):
894+
model_version = None
890895

896+
count = project.delete_predictions(model_version=model_version)
891897
return Response(data=count)
892898

893899

label_studio/projects/models.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -797,20 +797,24 @@ def should_none_model_version(self, model_version):
797797

798798
def delete_predictions(self, model_version=None):
799799
"""
800-
Deletes the predictions based on the provided model version.
801-
If no model version is provided, it deletes all the predictions for this project.
800+
Deletes predictions for this project filtered by model version.
802801
803-
:param model_version: Identifier of the model version (default is None)
802+
:param model_version: If None, deletes predictions whose model_version
803+
is NULL, empty, or the legacy placeholder 'undefined' (predictions
804+
imported without a version label — see the data migration that
805+
backfills 'undefined' to NULL). Otherwise filters by exact match.
804806
:type model_version: str, optional
805807
:return: Dictionary with count of deleted predictions
806808
:rtype: dict
807809
"""
808-
params = {'project': self}
809-
810-
if model_version:
811-
params.update({'model_version': model_version})
812-
813-
predictions = Prediction.objects.filter(**params)
810+
if model_version is None:
811+
predictions = Prediction.objects.filter(project=self).filter(
812+
Q(model_version__isnull=True)
813+
| Q(model_version='')
814+
| Q(model_version='undefined')
815+
)
816+
else:
817+
predictions = Prediction.objects.filter(project=self, model_version=model_version)
814818

815819
with transaction.atomic():
816820
# If we are deleting specific model_version then we need

label_studio/projects/tests/test_api.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,91 @@ def test_extended(self):
9090
assert response.json()['static'][1]['count'] == 1
9191
assert response.json()['static'][2]['model_version'] == 'model_1'
9292
assert response.json()['static'][2]['count'] == 2
93+
94+
95+
class TestDeletePredictionsAPI(APITestCase):
96+
"""Covers the DELETE /api/projects/<pk>/model-versions endpoint.
97+
98+
Regression coverage for issue #9717: predictions imported without a
99+
model_version (stored as NULL, empty string, or the legacy 'undefined'
100+
placeholder) must be deletable.
101+
"""
102+
103+
def setUp(self):
104+
self.project = ProjectFactory()
105+
self.user = self.project.created_by
106+
self.task = TaskFactory(project=self.project)
107+
self.client.force_authenticate(user=self.user)
108+
self.url = f'/api/projects/{self.project.id}/model-versions'
109+
110+
def _prediction_count(self, **filters):
111+
return self.task.predictions.filter(**filters).count()
112+
113+
def test_delete_by_specific_version(self):
114+
PredictionFactory(task=self.task, model_version='v1')
115+
PredictionFactory(task=self.task, model_version='v1')
116+
PredictionFactory(task=self.task, model_version='v2')
117+
118+
response = self.client.delete(self.url, data={'model_version': 'v1'}, format='json')
119+
120+
assert response.status_code == 200
121+
assert self._prediction_count(model_version='v1') == 0
122+
assert self._prediction_count(model_version='v2') == 1
123+
124+
def test_delete_null_version_via_json_null(self):
125+
PredictionFactory(task=self.task, model_version=None)
126+
PredictionFactory(task=self.task, model_version='v1')
127+
128+
response = self.client.delete(self.url, data={'model_version': None}, format='json')
129+
130+
assert response.status_code == 200
131+
assert self._prediction_count(model_version__isnull=True) == 0
132+
assert self._prediction_count(model_version='v1') == 1
133+
134+
def test_delete_null_version_via_empty_string(self):
135+
PredictionFactory(task=self.task, model_version='')
136+
PredictionFactory(task=self.task, model_version='v1')
137+
138+
response = self.client.delete(self.url, data={'model_version': ''}, format='json')
139+
140+
assert response.status_code == 200
141+
assert self._prediction_count(model_version='') == 0
142+
assert self._prediction_count(model_version='v1') == 1
143+
144+
def test_delete_legacy_undefined_string(self):
145+
"""Pre-migration rows have model_version='undefined'. Migration 0062
146+
backfills these to NULL, but the API still accepts the legacy string
147+
so it works during a rolling deployment.
148+
"""
149+
PredictionFactory(task=self.task, model_version='undefined')
150+
PredictionFactory(task=self.task, model_version='v1')
151+
152+
response = self.client.delete(self.url, data={'model_version': 'undefined'}, format='json')
153+
154+
assert response.status_code == 200
155+
assert self._prediction_count(model_version='undefined') == 0
156+
assert self._prediction_count(model_version='v1') == 1
157+
158+
def test_delete_null_version_groups_legacy_representations(self):
159+
"""A single null-version delete request should sweep NULL, empty, and
160+
the legacy 'undefined' placeholder together so callers don't need to
161+
know about the pre-migration data layout.
162+
"""
163+
PredictionFactory(task=self.task, model_version=None)
164+
PredictionFactory(task=self.task, model_version='')
165+
PredictionFactory(task=self.task, model_version='undefined')
166+
PredictionFactory(task=self.task, model_version='v1')
167+
168+
response = self.client.delete(self.url, data={'model_version': None}, format='json')
169+
170+
assert response.status_code == 200
171+
assert self.task.predictions.exclude(model_version='v1').count() == 0
172+
assert self._prediction_count(model_version='v1') == 1
173+
174+
def test_delete_requires_model_version_key(self):
175+
PredictionFactory(task=self.task, model_version='v1')
176+
177+
response = self.client.delete(self.url, data={}, format='json')
178+
179+
assert response.status_code == 400
180+
assert self._prediction_count(model_version='v1') == 1
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
"""Backfill legacy `Prediction.model_version='undefined'` rows to NULL.
2+
3+
Older code paths (`data_import/api.py`, `tasks/serializers.py`) defaulted the
4+
`model_version` field to the literal string `'undefined'` when an imported
5+
prediction did not specify one. That created two distinct "no version"
6+
representations (NULL and the string 'undefined') and caused the predictions
7+
delete endpoint to fail. The application layer now uses NULL as the canonical
8+
representation, and this migration normalizes existing rows.
9+
10+
The reverse migration is intentionally a no-op: we cannot tell migrated rows
11+
apart from genuinely-NULL rows after the fact.
12+
"""
13+
14+
from django.db import migrations
15+
16+
17+
def backfill_undefined_to_null(apps, schema_editor):
18+
Prediction = apps.get_model('tasks', 'Prediction')
19+
Prediction.objects.filter(model_version='undefined').update(model_version=None)
20+
21+
22+
class Migration(migrations.Migration):
23+
dependencies = [
24+
('tasks', '0061_task_project_file_upload_idx_async'),
25+
]
26+
operations = [
27+
migrations.RunPython(
28+
backfill_undefined_to_null,
29+
reverse_code=migrations.RunPython.noop,
30+
),
31+
]

label_studio/tasks/serializers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,7 @@ def add_predictions(self, task_predictions):
620620
)
621621
prediction_score = None
622622

623-
last_model_version = prediction.get('model_version', 'undefined')
623+
last_model_version = prediction.get('model_version') or None
624624
db_predictions.append(
625625
Prediction(
626626
task=self.db_tasks[i],

web/apps/labelstudio/src/pages/Settings/PredictionsSettings/PredictionsList.jsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,15 @@ const VersionCard = ({ version, selected, onSelect, editable, onDelete }) => {
5555
[version, onDelete],
5656
);
5757

58+
const hasNoVersion = !version.model_version || version.model_version === "undefined";
59+
5860
return (
5961
<div className={rootClass.toClassName()}>
6062
<div>
6163
<div className={rootClass.elem("title").toClassName()}>
62-
{version.model_version}
63-
{version.model_version === "undefined" && (
64-
<Tooltip title="Model version is undefined. Likely means that model_version field was missing when predictions were imported.">
64+
{hasNoVersion ? "No model version" : version.model_version}
65+
{hasNoVersion && (
66+
<Tooltip title="No model version on these predictions. Usually means the model_version field was missing when predictions were imported.">
6567
<IconInfoOutline className={cn("help-icon").toClassName()} width="14" height="14" />
6668
</Tooltip>
6769
)}

0 commit comments

Comments
 (0)