Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ Shared utilities come from `adit-radis-shared` package (accounts, token auth, co
Analysis operations follow a Job -> Task pattern (similar to ADIT):

- An **AnalysisJob** contains multiple **AnalysisTasks**
- Status flow: `UNVERIFIED` -> `PREPARING` -> `PENDING` -> `IN_PROGRESS` -> `SUCCESS`/`WARNING`/`FAILURE`
- Status flow: `UNVERIFIED` -> `PREPARING` -> `PENDING` -> `IN_PROGRESS` -> `SUCCESS`/`WARNING`/`FAILURE`/`CANCELED`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document the cancellation paths accurately.

CANCELED can follow CANCELING. It can also result when a job has no remaining tasks or only canceled tasks. The linear flow in Line 71 implies that CANCELED only follows IN_PROGRESS.

Proposed documentation update
- Status flow: `UNVERIFIED` -> `PREPARING` -> `PENDING` -> `IN_PROGRESS` -> `SUCCESS`/`WARNING`/`FAILURE`/`CANCELED`
+ Completion flow: `UNVERIFIED` -> `PREPARING` -> `PENDING` -> `IN_PROGRESS` -> `SUCCESS`/`WARNING`/`FAILURE`
+ Cancellation outcome: `CANCELING`, jobs with no tasks, and jobs with only canceled tasks become `CANCELED`.

As per coding guidelines, “Maintain clear structure of agent descriptions, capabilities, and usage in AGENTS.md”.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- Status flow: `UNVERIFIED` -> `PREPARING` -> `PENDING` -> `IN_PROGRESS` -> `SUCCESS`/`WARNING`/`FAILURE`/`CANCELED`
- Completion flow: `UNVERIFIED` -> `PREPARING` -> `PENDING` -> `IN_PROGRESS` -> `SUCCESS`/`WARNING`/`FAILURE`
- Cancellation outcome: `CANCELING`, jobs with no tasks, and jobs with only canceled tasks become `CANCELED`.
🤖 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 `@AGENTS.md` at line 71, Update the status flow documentation in AGENTS.md to
show that CANCELED may follow CANCELING and may result when a job has no
remaining tasks or only canceled tasks, rather than implying it only follows
IN_PROGRESS. Preserve the existing transitions and clearly separate these
cancellation outcomes while maintaining the document’s current structure.

Source: Coding guidelines

- Jobs automatically update state based on task completion
- Email notifications sent on job completion
- Background workers (Procrastinate) process tasks from `default` and `llm` queues
Expand Down
17 changes: 13 additions & 4 deletions radis/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,23 +110,32 @@ def update_job_state(self) -> bool:
has_success = self.tasks.filter(status=AnalysisTask.Status.SUCCESS).exists()
has_warning = self.tasks.filter(status=AnalysisTask.Status.WARNING).exists()
has_failure = self.tasks.filter(status=AnalysisTask.Status.FAILURE).exists()
has_canceled = self.tasks.filter(status=AnalysisTask.Status.CANCELED).exists()

# An "All tasks ..." message would be untrue when some tasks were canceled instead.
if has_failure:
self.status = AnalysisJob.Status.FAILURE
self.message = (
"Some tasks failed." if (has_success or has_warning) else "All tasks failed."
"Some tasks failed."
if (has_success or has_warning or has_canceled)
else "All tasks failed."
)

elif has_warning:
self.status = AnalysisJob.Status.WARNING
self.message = (
"Some tasks have warnings." if has_success else "All tasks have warnings."
"Some tasks have warnings."
if (has_success or has_canceled)
else "All tasks have warnings."
)
elif has_success:
self.status = AnalysisJob.Status.SUCCESS
self.message = "All tasks succeeded."
self.message = "Some tasks were canceled." if has_canceled else "All tasks succeeded."
elif has_canceled:
self.status = AnalysisJob.Status.CANCELED
self.message = "All tasks were canceled."
else:
# at least one of success, warnings or failures must be > 0
# at least one of success, warnings, failures or cancellations must be > 0
raise AssertionError(f"Invalid task status of {self}.")

self.ended_at = timezone.now()
Expand Down
71 changes: 61 additions & 10 deletions radis/core/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,67 @@ def test_job_update_job_state_all_tasks_fail(self):
assert job.message == "All tasks failed."
assert job.ended_at is not None

@pytest.mark.django_db
def test_job_update_job_state_all_tasks_canceled(self):
# Status is PENDING, not CANCELING, so this reaches the final evaluation, where
# an all-canceled job raised AssertionError before the canceled branch existed.
user = UserFactory.create()
job = ExtractionJobFactory.create(owner=user, status=AnalysisJob.Status.PENDING)

ExtractionTaskFactory.create(job=job, status=AnalysisTask.Status.CANCELED)
ExtractionTaskFactory.create(job=job, status=AnalysisTask.Status.CANCELED)

result = job.update_job_state()
job.refresh_from_db()

assert result is True
assert job.status == AnalysisJob.Status.CANCELED
assert job.message == "All tasks were canceled."
assert job.ended_at is not None

@pytest.mark.django_db
def test_job_update_job_state_canceled_task_does_not_mask_failure(self):
# The canceled branch is last in the chain, so it must not shadow a failure.
user = UserFactory.create()
job = ExtractionJobFactory.create(owner=user, status=AnalysisJob.Status.PENDING)

ExtractionTaskFactory.create(job=job, status=AnalysisTask.Status.CANCELED)
ExtractionTaskFactory.create(job=job, status=AnalysisTask.Status.FAILURE)

job.update_job_state()
job.refresh_from_db()

assert job.status == AnalysisJob.Status.FAILURE
assert job.message == "Some tasks failed."

@pytest.mark.django_db
def test_job_update_job_state_success_and_canceled_does_not_claim_all_succeeded(self):
user = UserFactory.create()
job = ExtractionJobFactory.create(owner=user, status=AnalysisJob.Status.PENDING)

ExtractionTaskFactory.create(job=job, status=AnalysisTask.Status.SUCCESS)
ExtractionTaskFactory.create(job=job, status=AnalysisTask.Status.CANCELED)

job.update_job_state()
job.refresh_from_db()

assert job.status == AnalysisJob.Status.SUCCESS
assert job.message == "Some tasks were canceled."

@pytest.mark.django_db
def test_job_update_job_state_warning_and_canceled_does_not_claim_all_warned(self):
user = UserFactory.create()
job = ExtractionJobFactory.create(owner=user, status=AnalysisJob.Status.PENDING)

ExtractionTaskFactory.create(job=job, status=AnalysisTask.Status.WARNING)
ExtractionTaskFactory.create(job=job, status=AnalysisTask.Status.CANCELED)

job.update_job_state()
job.refresh_from_db()

assert job.status == AnalysisJob.Status.WARNING
assert job.message == "Some tasks have warnings."

@pytest.mark.django_db
def test_job_update_job_state_tasks_still_pending(self):
user = UserFactory.create()
Expand Down Expand Up @@ -381,16 +442,6 @@ def test_job_get_mail_context_default(self):
context = job.get_mail_context()
assert context == {} # Default implementation returns empty dict

@pytest.mark.django_db
def test_job_update_job_state_with_only_canceled_tasks(self):
user = UserFactory.create()
job = ExtractionJobFactory.create(owner=user, status=AnalysisJob.Status.PENDING)
ExtractionTaskFactory.create(job=job, status=AnalysisTask.Status.CANCELED)
ExtractionTaskFactory.create(job=job, status=AnalysisTask.Status.CANCELED)

with pytest.raises(AssertionError, match="Invalid task status"):
job.update_job_state()

@pytest.mark.django_db
def test_job_update_job_state_consecutive_calls(self):
user = UserFactory.create()
Expand Down