fix(core): job canceled during a running task no longer ends as SUCCESS - #256
fix(core): job canceled during a running task no longer ends as SUCCESS#256samuelvkwong wants to merge 2 commits into
Conversation
A worker holds its AnalysisJob instance in memory for the whole task run. When the user cancels meanwhile, update_job_state evaluated the final status against the stale in-memory status, skipped the CANCELING branch, and overwrote the cancel with SUCCESS/WARNING/FAILURE. Two layers of fix in update_job_state: - refresh the status from the DB before evaluating, so the normal case (cancel during a long task) settles to CANCELED - write the final status with a guarded queryset update that excludes CANCELING/CANCELED rows, so a cancel landing in the remaining sliver between refresh and write also wins; zero updated rows settles the job to CANCELED and skips the finished mail Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesAnalysis job cancellation handling
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
radis/core/models.py (1)
97-107: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winGuard the PENDING and IN_PROGRESS intermediate saves against a concurrent cancel.
refresh_from_db()at line 89 only establishes an old memory snapshot;self.save()is still a full-row update if called withoutupdate_fields. Between thetasks.filter().exists()checks and these saves, another request can writeCANCELING, and these saved rows can overwrite it back toPENDINGorIN_PROGRESS, losing the cancellation intent. Use the same conditionalobjects.exclude(status__in=[CANCELING, CANCELED]).update(status=...)pattern as the final write so canceling always wins.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@radis/core/models.py` around lines 97 - 107, The PENDING and IN_PROGRESS branches in the job status update flow can overwrite a concurrent cancellation because self.save() performs a full-row update. Replace each intermediate self.status assignment and self.save() with a conditional AnalysisJob.objects.exclude(status__in=[CANCELING, CANCELED]).update(status=...) using the same pattern as the final write, while preserving the existing early returns.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@radis/core/models.py`:
- Around line 97-107: The PENDING and IN_PROGRESS branches in the job status
update flow can overwrite a concurrent cancellation because self.save() performs
a full-row update. Replace each intermediate self.status assignment and
self.save() with a conditional
AnalysisJob.objects.exclude(status__in=[CANCELING, CANCELED]).update(status=...)
using the same pattern as the final write, while preserving the existing early
returns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2167a8f3-f6c5-4060-95aa-14fd3e112bc5
📒 Files selected for processing (2)
radis/core/models.pyradis/core/tests/test_models.py
|
Saw the issue with incorrect job status after cancellation in Radis. I have robust state-machine logic for managing agent task lifecycles. |
3 participants
Problem
Canceling an analysis job while a task is running frequently ends with the job in SUCCESS instead of CANCELED.
A worker loads its
AnalysisJobinstance once at task start and holds it in memory for the whole task run (minutes for LLM batches). If the user cancels during that window, the cancel view writesCANCELINGto the DB — but the worker's stale instance still saysIN_PROGRESS. When the task finishes,update_job_state()re-queries the tasks but checksself.status == CANCELINGagainst the stale in-memory value, skips the cancel branch, and the final evaluation overwritesCANCELINGwithSUCCESS(canceled tasks aren't counted at all, so even a 9-of-10-canceled job reports "All tasks succeeded").Affects every
AnalysisJobsubclass (extractions, subscriptions, and the upcoming labeling app).Fix
Two layers in
update_job_state():CANCELING/CANCELEDrows. Zero updated rows means a concurrent cancel won; the job is then settled toCANCELEDand the finished mail is skipped.Tests
test_update_job_state_on_stale_instance_respects_concurrent_cancel— reproduces the reported bug (failed with SUCCESS before the fix)test_final_state_write_guarded_against_cancel_landing_after_refresh— pins the sliver-window guard (failed before the guarded write)Both written test-first and watched fail. Full
core+extractions+subscriptionssuites pass (310 tests).🤖 Generated with Claude Code
Summary by CodeRabbit