Skip to content

Conversation

joshsny
Copy link
Contributor

@joshsny joshsny commented Oct 17, 2025

we want to get rid of these as a concept, and just have task runs and agent types

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

14 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +69 to +71
stage = params.get("stage")
if stage:
qs = qs.filter(runs__stage=stage)
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: filtering by runs__stage will match ANY run with that stage, not just the latest run

this changes the behavior from the previous code which used a subquery to filter by the stage of the latest run only

potential issues:

  1. tasks with old runs in a stage will still appear even if their latest run is in a different stage
  2. duplicate tasks if multiple runs have the same stage value (needs .distinct())

suggested fix:

Suggested change
stage = params.get("stage")
if stage:
qs = qs.filter(runs__stage=stage)
stage = params.get("stage")
if stage:
from django.db.models import OuterRef, Subquery
latest_run = TaskRun.objects.filter(task=OuterRef("pk")).order_by("-created_at").values("stage")[:1]
qs = qs.annotate(latest_stage=Subquery(latest_run)).filter(latest_stage=stage)
Prompt To Fix With AI
This is a comment left during a code review.
Path: products/tasks/backend/api.py
Line: 69:71

Comment:
**logic:** filtering by `runs__stage` will match ANY run with that stage, not just the latest run

this changes the behavior from the previous code which used a subquery to filter by the stage of the **latest run only**

potential issues:
1. tasks with old runs in a stage will still appear even if their latest run is in a different stage
2. duplicate tasks if multiple runs have the same stage value (needs `.distinct()`)

suggested fix:

```suggestion
        stage = params.get("stage")
        if stage:
            from django.db.models import OuterRef, Subquery
            latest_run = TaskRun.objects.filter(task=OuterRef("pk")).order_by("-created_at").values("stage")[:1]
            qs = qs.annotate(latest_stage=Subquery(latest_run)).filter(latest_stage=stage)
```

How can I resolve this? If you propose a fix, please make it concise.

@joshsny joshsny force-pushed the array/remove-workflows branch from 3085234 to e589bac Compare October 20, 2025 08:53
@joshsny joshsny enabled auto-merge (squash) October 21, 2025 18:51
Copy link
Contributor

Migration SQL Changes

Hey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:

products/tasks/backend/migrations/0011_taskrun_stage_alter_task_current_stage_and_more.py

BEGIN;
--
-- Add field stage to taskrun
--
ALTER TABLE "posthog_task_run" ADD COLUMN "stage" varchar(100) NULL;
--
-- Alter field current_stage on task
--
-- (no-op)
--
-- Alter field workflow on task
--
-- (no-op)
--
-- Alter field current_stage on taskrun
--
-- (no-op)
COMMIT;

Copy link
Contributor

🔍 Migration Risk Analysis

We've analyzed your migrations for potential risks.

Summary: 0 Safe | 1 Needs Review | 0 Blocked

⚠️ Needs Review

May have performance impact

tasks.0011_taskrun_stage_alter_task_current_stage_and_more
  └─ #1 ✅ AddField
     Adding nullable field is safe
     model: taskrun, field: stage
  └─ #2 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: task, field: current_stage, field_type: ForeignKey
  └─ #3 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: task, field: workflow, field_type: ForeignKey
  └─ #4 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: taskrun, field: current_stage, field_type: ForeignKey

@joshsny joshsny merged commit 1120f5b into master Oct 21, 2025
162 of 163 checks passed
@joshsny joshsny deleted the array/remove-workflows branch October 21, 2025 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants