Skip to content

fix: allow deleting predictions imported without model_version#9737

Open
kcw2034 wants to merge 2 commits into
HumanSignal:developfrom
kcw2034:fix/delete-predictions-null-model-version
Open

fix: allow deleting predictions imported without model_version#9737
kcw2034 wants to merge 2 commits into
HumanSignal:developfrom
kcw2034:fix/delete-predictions-null-model-version

Conversation

@kcw2034
Copy link
Copy Markdown

@kcw2034 kcw2034 commented May 11, 2026

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 of NULL.

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:

  1. The 'undefined' string originates from three BE import sites (data_import/api.py ×2, tasks/serializers.py) defaulting to 'undefined' when model_version is missing. The frontend just echoes it back.
  2. Magic-string handling at the API layer creates a foot-gun: a user with a legitimately-named 'undefined' model would lose unrelated predictions.
  3. The mixed data state (NULL vs 'undefined' vs '') is left unresolved.
  4. No regression tests.

What this PR does

Backend

  • Import sites stop defaulting to 'undefined' — three call sites now use item.get('model_version') or None, so future imports without a version land as NULL.
  • DELETE endpoint accepts JSON null / \"\" / 'undefined' as "no model version" via a single normalization step, then routes through an isnull-aware delete.
  • Project.delete_predictions(None) sweeps NULL'''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.
  • Migration tasks/0062 backfills existing 'undefined' rows to NULL. Reverse is intentionally a no-op (can't distinguish migrated rows from genuinely-NULL ones).

Frontend

  • PredictionsList.jsx removes the === \"undefined\" magic-string comparison. Falsy + legacy 'undefined' now both display "No model version" with an explanatory tooltip.

Tests

  • projects/tests/test_api.py::TestDeletePredictionsAPI covers 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 pass
  • pytest label_studio/projects/tests/test_api.py::TestProjectModelVersionsAPI — 3/3 pass (no regression)
  • Migration detected by Django: showmigrations tasks lists 0062_backfill_undefined_model_version
  • Manual: import predictions JSON without model_version, verify they delete cleanly

Files changed (7, +149 / -18)

File Purpose
data_import/api.py (×2) Stop storing 'undefined' on missing field
tasks/serializers.py Same
projects/api.py DELETE endpoint normalization
projects/models.py delete_predictions(None) sweeps null-ish
tasks/migrations/0062_*.py Backfill legacy data
projects/tests/test_api.py Regression coverage
web/apps/labelstudio/.../PredictionsList.jsx FE displays "No model version"

🤖 Generated with Claude Code

@kcw2034 kcw2034 requested a review from a team as a code owner May 11, 2026 05:24
@netlify
Copy link
Copy Markdown

netlify Bot commented May 11, 2026

👷 Deploy request for heartex-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 4c79132

@netlify
Copy link
Copy Markdown

netlify Bot commented May 11, 2026

👷 Deploy request for label-studio-docs-new-theme pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 4c79132

@netlify
Copy link
Copy Markdown

netlify Bot commented May 11, 2026

Deploy Preview for label-studio-playground canceled.

Name Link
🔨 Latest commit 4c79132
🔍 Latest deploy log https://app.netlify.com/projects/label-studio-playground/deploys/6a0173ce5aa0da0008d9cde4

@netlify
Copy link
Copy Markdown

netlify Bot commented May 11, 2026

Deploy Preview for label-studio-storybook canceled.

Name Link
🔨 Latest commit 4c79132
🔍 Latest deploy log https://app.netlify.com/projects/label-studio-storybook/deploys/6a0173ce8338be000815fe96

…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>
@kcw2034 kcw2034 force-pushed the fix/delete-predictions-null-model-version branch from 0868637 to 24c45ae Compare May 11, 2026 05:25
@kcw2034 kcw2034 changed the title fix(predictions): allow deleting predictions imported without model_version fix: allow deleting predictions imported without model_version May 11, 2026
… 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>
@kcw2034
Copy link
Copy Markdown
Author

kcw2034 commented May 11, 2026

Addressed review feedback:

#1 delete_predictions caller audit — verified. The Python codebase has exactly one caller of Project.delete_predictions(model_version=...): the DELETE endpoint in this PR (projects/api.py:896). The other 3 grep hits are ml_models.ModelRun.delete_predictions() (different class, no args) and unrelated UI label mappings. No caller relies on the previous "delete-all" semantics.

#2 Title prefix — changed fix(predictions):fix: to match repo convention.

#3 Migration batchingtasks/0062 now runs the UPDATE in 1000-row batches with atomic = False. The single long transaction is gone; each batch commits independently.

#5 Import-site integration test — added TestImportPredictionsModelVersionDefault (3 cases) covering missing / empty / explicit model_version through the /api/projects/<pk>/import/predictions endpoint. Verifies the DB row lands as NULL (or the given value), closing the loop between the import-side fix and the delete-side fix.

#4 Prediction.model_version field cleanup — intentionally left out. The current null=True, default='' combination is awkward, but a schema migration on the Prediction table on large installs is a separate risk class and deserves its own PR with focused review.

Final state: 12/12 new tests pass (6 delete + 3 import + 3 existing model-versions). Branch is 2 commits ahead of develop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Predictions from imported tasks cannot be deleted

1 participant