Implement a runs endpoint that can return workflow runs or tasks #1708

wintonzheng · 2025-02-03T19:54:53Z

Important

Add /runs endpoint to fetch workflow runs and tasks with pagination and status filtering.

New Functionality:
- Add get_all_runs() in client.py to fetch workflow runs and tasks with pagination and status filtering.
- Add /runs endpoint in agent_protocol.py to expose get_all_runs() via HTTP.
Behavior:
- Limits results to 100 runs (10 pages of 10 items each).
- Sorts results by created_at in descending order.
Error Handling:
- Logs SQLAlchemy errors in get_all_runs() and raises exceptions.
- Returns empty list if page number exceeds limit in /runs endpoint.

^{This description was created by}^{for fc38cd4. It will automatically update as commits are pushed.}

> [!IMPORTANT] > Add `/runs` endpoint to fetch workflow runs and tasks with pagination and status filtering, limited to 100 results. > > - **Endpoints**: > - Add `/runs` endpoint in `agent_protocol.py` to fetch workflow runs and tasks. > - Supports pagination and status filtering. > - Limits results to 100 runs. > - **Database**: > - Add `get_all_runs()` in `client.py` to query `WorkflowRunModel` and `TaskModel`. > - Combines results, sorts by `created_at`, and applies pagination. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for ad53637ef91536b9e47e53083f16f118ebb3e130. It will automatically update as commits are pushed.</sup>

ellipsis-dev

❌ Changes requested. Incremental review on fc38cd4 in 1 minute and 51 seconds

More details

Looked at 82 lines of code in 2 files
Skipped 0 files when reviewing.
Skipped posting 6 drafted comments based on config settings.

1. skyvern/forge/sdk/db/client.py:1391

Draft comment:
Pagination: Merging and sorting results in Python may be inefficient for larger datasets.
Reason this comment was not posted:
Marked as duplicate.

2. skyvern/forge/sdk/db/client.py:1421

Draft comment:
Ensure both WorkflowRun and Task have a 'created_at' attribute for correct sorting.
Reason this comment was not posted:
Comment did not seem useful.

3. skyvern/forge/sdk/routes/agent_protocol.py:438

Draft comment:
The /runs endpoint returns a union of WorkflowRun and Task. Consider adding a discriminator field for clients.
Reason this comment was not posted:
Marked as duplicate.

4. skyvern/forge/sdk/routes/agent_protocol.py:449

Draft comment:
Returning an empty list when page > 10 might confuse clients. Document this temporary behavior.
Reason this comment was not posted:
Confidence changes required: 33%
None

5. skyvern/forge/sdk/routes/agent_protocol.py:438

Draft comment:
The response_model union 'list[WorkflowRun | Task]' may lead to schema resolution issues. Consider using a discriminated union or a common base model with a type field for clearer OpenAPI documentation.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
While the comment raises a valid concern about OpenAPI schema clarity, it's more of a speculative suggestion rather than pointing out a definite issue. The code will work fine as is - it's just suggesting a potential documentation improvement. The comment also doesn't provide specific guidance on how to implement the suggested changes.
The comment could be correct that this might cause documentation issues, but without evidence that it's actually causing problems, this may be premature optimization.
The current code appears to be working, and FastAPI generally handles union types well. Without concrete evidence of documentation problems, this comment may be adding unnecessary complexity.
Delete the comment because it is speculative and not pointing out a definite issue that needs fixing. The suggestion is more of a "nice to have" documentation improvement rather than a required code change.

6. skyvern/forge/sdk/routes/agent_protocol.py:221

Draft comment:
There appears to be a redundant check for 'if not step' in the execute_agent_task_step endpoint. The step is already validated earlier in the branch, so this duplicate check could be removed to simplify the code.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_NFQGRn8T85akRFKc

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ellipsis-dev · 2025-02-03T19:57:00Z

skyvern/forge/sdk/routes/agent_protocol.py

+    analytics.capture("skyvern-oss-agent-runs-get")
+
+    # temporary limit to 100 runs
+    if page > 10:


Temporary limit: returning an empty list when page > 10 may surprise clients. Consider returning an HTTP error or a more informative response.

ellipsis-dev · 2025-02-03T19:57:00Z

skyvern/forge/sdk/db/client.py

+    ) -> list[WorkflowRun | Task]:
+        try:
+            async with self.Session() as session:
+                # temporary limit to 10 pages


The get_all_runs method retrieves page*page_size records from two separate queries, merges, sorts in Python, and then slices the result. This can be inefficient for large datasets. Consider handling pagination at the database level.

ellipsis-dev

❌ Changes requested. Reviewed everything up to fc38cd4 in 2 minutes and 34 seconds

More details

Looked at 82 lines of code in 2 files
Skipped 0 files when reviewing.
Skipped posting 7 drafted comments based on config settings.

1. skyvern/forge/sdk/routes/agent_protocol.py:452

Draft comment:
Fetching runs by applying a limit as page*page_size and then slicing the merged list in memory may impact performance. Consider applying pagination at the query level.
Reason this comment was not posted:
Comment looked like it was already resolved.

2. skyvern/forge/sdk/routes/agent_protocol.py:452

Draft comment:
Ensure that the model_dump() output meets the declared response_model schema. Verify that both WorkflowRun and Task models are compatible.
Reason this comment was not posted:
Comment did not seem useful.

3. skyvern/forge/sdk/routes/agent_protocol.py:449

Draft comment:
When page exceeds 10 the function returns an empty list. Consider replacing this temporary hack with a clear HTTP error or pagination metadata so clients know they've reached the limit.
Reason this comment was not posted:
Marked as duplicate.

4. skyvern/forge/sdk/routes/agent_protocol.py:438

Draft comment:
The response_model is defined as a union (list[WorkflowRun | Task]). This may lead to serialization/validation issues. Consider using a discriminated union or a clearer schema for mixed types.
Reason this comment was not posted:
Marked as duplicate.

5. skyvern/forge/sdk/routes/agent_protocol.py:1350

Draft comment:
The recursive call in _flatten_workflow_run_timeline may risk deep recursion with highly nested workflows. Consider an iterative approach or enforce a maximum recursion depth.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. skyvern/forge/sdk/routes/agent_protocol.py:568

Draft comment:
Ensure that the number and order of signed URLs match the artifacts list to avoid potential index errors when setting artifact.signed_url.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. skyvern/forge/sdk/routes/agent_protocol.py:1044

Draft comment:
For task generation caching, using an exact SHA256 hash for user prompts could be too rigid. Consider an approximate similarity search in future to account for minor variations in user input.
Reason this comment was not posted:
Confidence changes required: 50%
None

Workflow ID: wflow_irvhHDjAdZeE1GGp

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ellipsis-dev · 2025-02-03T19:57:45Z

skyvern/forge/sdk/routes/agent_protocol.py

+    analytics.capture("skyvern-oss-agent-runs-get")
+
+    # temporary limit to 100 runs
+    if page > 10:


Avoid duplicating the temporary page limit check. Both get_runs endpoint and get_all_runs DB method check if page > 10. Consider unifying this logic.

wintonzheng added the sync label Feb 3, 2025

ellipsis-dev bot reviewed Feb 3, 2025

View reviewed changes

msalihaltun approved these changes Feb 3, 2025

View reviewed changes

msalihaltun merged commit e0e8684 into main Feb 3, 2025
7 checks passed

msalihaltun deleted the salih/unified-runs-endpoint branch February 3, 2025 19:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement a runs endpoint that can return workflow runs or tasks #1708

Implement a runs endpoint that can return workflow runs or tasks #1708

wintonzheng commented Feb 3, 2025 •

edited by ellipsis-dev bot

Loading

ellipsis-dev bot left a comment

ellipsis-dev bot Feb 3, 2025

ellipsis-dev bot Feb 3, 2025

ellipsis-dev bot left a comment

ellipsis-dev bot Feb 3, 2025

Implement a runs endpoint that can return workflow runs or tasks #1708

Implement a runs endpoint that can return workflow runs or tasks #1708

Conversation

wintonzheng commented Feb 3, 2025 • edited by ellipsis-dev bot Loading

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

ellipsis-dev bot Feb 3, 2025

Choose a reason for hiding this comment

ellipsis-dev bot Feb 3, 2025

Choose a reason for hiding this comment

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

ellipsis-dev bot Feb 3, 2025

Choose a reason for hiding this comment

wintonzheng commented Feb 3, 2025 •

edited by ellipsis-dev bot

Loading