Order tasks desc by default - #18
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR changes the default ordering behavior for the list_with_join method in the TaskRepository from ascending ("asc") to descending ("desc").
- Changes the
order_directionparameter default value from"asc"to"desc"
Comments suppressed due to low confidence (1)
agentex/src/domain/repositories/task_repository.py:45
- The docstring is incomplete and does not document the
order_directionparameter, including its default value. With the change from 'asc' to 'desc', it's especially important to document this parameter so API consumers understand the default ordering behavior. Consider adding documentation fororder_by,order_direction,limit, andpage_numberparameters.
"""
List Tasks with custom filters that may require joining tables.
Args:
- task_filters (dict[str, ColumnPrimitiveValue | Sequence[ColumnPrimitiveValue]] | None): Filters on the task table itself.
Keys are column names. Values are either the value to match, or a list of values to match.
- agent_id (str | None): Filter tasks by agent ID using the join table
- agent_name (str | None): Filter tasks by agent name
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| agent_name: str | None = None, | ||
| order_by: str | None = None, | ||
| order_direction: Literal["asc", "desc"] = "asc", | ||
| order_direction: Literal["asc", "desc"] = "desc", |
There was a problem hiding this comment.
Changing the default order_direction from 'asc' to 'desc' is a breaking change for API consumers. The service layer method list_tasks in task_service.py (lines 184-190) calls list_with_join without specifying order_direction, which means all existing API calls to the /tasks endpoint will now return results in descending order instead of ascending. This affects the behavior of the public API endpoint without any corresponding update to API documentation or version bump. Consider either: (1) explicitly passing 'asc' in the service layer to maintain backward compatibility, or (2) documenting this as a breaking change with appropriate API versioning.
| order_direction: Literal["asc", "desc"] = "desc", | |
| order_direction: Literal["asc", "desc"] = "asc", |
There was a problem hiding this comment.
I believe this is fine, the other alternative is making this change in TaskService which calls this method, but we don't propagate sort order beyond this (or to the API) so there's not much difference.
Changes the TaskRespository's list tasks method to order by desc, by default. This makes the most recently created tasks appear at the beginning of the list, which is more intuitive imo and more convenient for the frontend.
Ideally, we would expose this as a parameter in the
/tasksendpoint, but then we should do that change across all bulk get endpoints, and that would require an SDK change. I think this is okay for now