refactor: Implement tasks list#173
Conversation
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new tasks/list endpoint. The implementation, however, contains several critical bugs that need to be addressed. These include incorrect filtering logic that fails to apply any filters, a type error in the in-memory store that would cause runtime failures, and a missing return statement in a utility function that breaks timestamp validation. Additionally, there are medium-severity issues related to parameter validation, handling of default values, and code duplication in generated types. I've provided detailed comments and suggestions to fix these issues.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new tasks/list endpoint, which is a great addition. My review focuses on the implementation details of this new feature. I've found a critical issue where the code would crash due to an undefined variable, as well as several other bugs related to pagination logic, parameter validation, and data handling. These issues could lead to incorrect API behavior, such as wrong total counts, incorrect history slicing, and overly restrictive filtering. I've provided detailed comments and suggestions to address these points.
guglielmo-san
changed the title
|
thoughts on this: #172 (comment) |
| if(params.lastUpdatedAfter !== undefined && !isValidUnixTimestampMs(params.lastUpdatedAfter)){ | ||
| return false; | ||
| } | ||
| const terminalStates: string[] = ["completed", "failed", "canceled", "rejected"]; |
There was a problem hiding this comment.
Is there a particular reason why only the terminal states are available for querying? The spec indicates that all task states are available for query.
4 participants
Description
The purpose of this PR is implementing the
tasks/listapi on the server side as for documentation.New
types.tshas been generated to include the parameters for the tasks/listThank you for opening a Pull Request!
Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
CONTRIBUTINGGuide.fix:which represents bug fixes, and correlates to a SemVer patch.feat:represents a new feature, and correlates to a SemVer minor.feat!:, orfix!:,refactor!:, etc., which represent a breaking change (indicated by the!) and will result in a SemVer major.Fixes #172 🦕