refactor(agents-api): Minor refactors to typespec types #465
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Diwank Tomer <[email protected]>
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 744e2ef in 1 minute and 16 seconds
More details
- Looked at
2792lines of code in58files - Skipped
2files when reviewing. - Skipped posting
8drafted comments based on config settings.
1. agents-api/agents_api/autogen/Agents.py:13
- Draft comment:
Removingextra="allow"fromConfigDictmight cause issues if there are unexpected fields in the input data. Ensure that this change is intentional and that the system does not rely on this flexibility. This applies to other similar changes in the file. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is speculative and asks the author to ensure the change is intentional, which is against the rules. The removal ofextra="allow"is a code change, but the comment does not provide a definite issue or actionable feedback. It is more of a cautionary note rather than a necessary code change suggestion.
I might be overlooking the potential impact of removingextra="allow", which could be significant if the system relies on it. However, the comment does not provide a definite issue or solution, making it speculative.
While the removal ofextra="allow"could have implications, the comment does not provide a clear, actionable issue. It remains speculative and does not align with the rules for comments.
The comment should be removed as it is speculative and does not provide a definite issue or actionable feedback.
2. agents-api/agents_api/autogen/Agents.py:29
- Draft comment:
Ensure that themax_lengthandpatternconstraints align with the expected data for this field. This applies to other similar changes in the file. - Reason this comment was not posted:
Confidence changes required:30%
Themax_lengthandpatternconstraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
3. agents-api/agents_api/autogen/Chat.py:250
- Draft comment:
Ensure that themax_lengthandpatternconstraints align with the expected data for this field. This applies to other similar changes in the file. - Reason this comment was not posted:
Confidence changes required:30%
Themax_lengthandpatternconstraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
4. agents-api/agents_api/autogen/Docs.py:32
- Draft comment:
Ensure that themax_lengthconstraint aligns with the expected data for this field. This applies to other similar changes in the file. - Reason this comment was not posted:
Confidence changes required:30%
Themax_lengthandpatternconstraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
5. agents-api/agents_api/autogen/Executions.py:110
- Draft comment:
Ensure that themax_lengthandpatternconstraints align with the expected data for this field. This applies to other similar changes in the file. - Reason this comment was not posted:
Confidence changes required:30%
Themax_lengthandpatternconstraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
6. agents-api/agents_api/autogen/Tools.py:43
- Draft comment:
Ensure that themax_lengthandpatternconstraints align with the expected data for this field. This applies to other similar changes in the file. - Reason this comment was not posted:
Confidence changes required:30%
Themax_lengthandpatternconstraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
7. agents-api/agents_api/autogen/Users.py:25
- Draft comment:
Ensure that themax_lengthandpatternconstraints align with the expected data for this field. This applies to other similar changes in the file. - Reason this comment was not posted:
Confidence changes required:30%
Themax_lengthandpatternconstraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
8. typespec/common/scalars.tsp:15
- Draft comment:
Ensure that themaxLengthandpatternconstraints align with the expected data for this scalar. This applies to other similar changes in the file. - Reason this comment was not posted:
Confidence changes required:30%
Themax_lengthandpatternconstraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
Workflow ID: wflow_R4A59dapLtK85TgP
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
… types Signed-off-by: Diwank Tomer <[email protected]>
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 8ab6dea in 1 minute and 37 seconds
More details
- Looked at
4483lines of code in89files - Skipped
2files when reviewing. - Skipped posting
7drafted comments based on config settings.
1. sdks/ts/src/api/models/Tasks_ErrorWorkflowStep.ts:9
- Draft comment:
Thekind_property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency. - Reason this comment was not posted:
Marked as duplicate.
2. sdks/ts/src/api/models/Tasks_EvaluateStep.ts:10
- Draft comment:
Thekind_property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency. - Reason this comment was not posted:
Marked as duplicate.
3. sdks/ts/src/api/models/Tasks_ForeachStep.ts:10
- Draft comment:
Thekind_property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency. - Reason this comment was not posted:
Marked as duplicate.
4. sdks/ts/src/api/models/Tasks_GetStep.ts:9
- Draft comment:
Thekind_property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency. - Reason this comment was not posted:
Marked as duplicate.
5. sdks/ts/src/api/models/Tasks_IfElseWorkflowStep.ts:23
- Draft comment:
Thekind_property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency. - Reason this comment was not posted:
Marked as duplicate.
6. sdks/ts/src/api/models/Tasks_LogStep.ts:10
- Draft comment:
Thekind_property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency. - Reason this comment was not posted:
Marked as duplicate.
7. sdks/ts/src/api/models/Tasks_ParallelStep.ts:17
- Draft comment:
Thekind_property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_fjLwHc1BgoVtIjoV
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.
| /** | ||
| * The kind of step | ||
| */ | ||
| readonly kind_: "embed"; |
There was a problem hiding this comment.
The kind_ property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency.
whiterabbit1983
merged commit 1c071ec
into
dev-tasks-disable-routes
2 of 5 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Diwank Tomer [email protected]
Summary:
Refactored typespec types in
agents-apifor improved type safety and consistency across Python and TypeScript models and schemas.Key points:
agents-apifor improved type safety and consistency.extra="allow"fromConfigDictinagents-api/agents_api/autogen/*models.max_lengthconstraints to string fields in models likeAgent,CreateAgentRequest,JobStatus.sdks/ts/src/api/models/*andsdks/ts/src/api/schemas/*to align with updated typespec definitions.ChatMLMessage,Tasks_PromptStep, etc.typespec/common/scalars.tspto includemaxLengthandpatternconstraints for scalars likeidentifierSafeUnicodeandvalidPythonIdentifier.Generated with ❤️ by ellipsis.dev