-
Notifications
You must be signed in to change notification settings - Fork 895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(typespec): Add types for computer-use tools #772
Conversation
Signed-off-by: Diwank Singh Tomer <diwank.singh@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to 44e254f in 50 seconds
More details
- Looked at
1360
lines of code in10
files - Skipped
1
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. agents-api/agents_api/autogen/Chat.py:13
- Draft comment:
ChosenToolCall
is removed from imports but still referenced in the code. Ensure all references are updated or re-import it. - Reason this comment was not posted:
Marked as duplicate.
2. agents-api/agents_api/autogen/Entries.py:13
- Draft comment:
ChosenToolCall
is removed from imports but still referenced in the code. Ensure all references are updated or re-import it. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment seems incorrect because there is no reference toChosenToolCall
in the updated code. The code has been updated to use other classes instead. Therefore, the comment does not reflect the current state of the code and should be removed.
I might be missing some context or other parts of the codebase whereChosenToolCall
is still used, but based on the provided code, it seems to be fully replaced.
The task is to focus on the provided diff and code, and based on that, the comment is not applicable.
The comment should be deleted because it incorrectly states thatChosenToolCall
is still referenced in the code, which is not the case in the provided code.
3. integrations-service/integrations/autogen/Chat.py:13
- Draft comment:
ChosenToolCall
is removed from imports but still referenced in the code. Ensure all references are updated or re-import it. - Reason this comment was not posted:
Marked as duplicate.
4. integrations-service/integrations/autogen/Entries.py:13
- Draft comment:
ChosenToolCall
is removed from imports but still referenced in the code. Ensure all references are updated or re-import it. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is incorrect becauseChosenToolCall
is not referenced anywhere in the code after the import was removed. The code has been updated to use other classes instead. Therefore, the comment should be deleted as it is not applicable.
I might have missed a subtle reference or alias that could indirectly useChosenToolCall
. However, given the context, it seems unlikely.
The review of the entire file shows no evidence ofChosenToolCall
being used, so the comment is likely incorrect.
The comment should be deleted becauseChosenToolCall
is not referenced in the code after its import was removed.
Workflow ID: wflow_qzjEsJ87oMFVg6I5
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Signed-off-by: Diwank Singh Tomer <diwank.singh@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 0db5d91 in 31 seconds
More details
- Looked at
64
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. agents-api/agents_api/autogen/openapi_model.py:321
- Draft comment:
EnsureBaseChosenToolCall
is compatible with all usages ofChatMLContent
. This change should be verified in the context of the codebase. - Reason this comment was not posted:
Confidence changes required:50%
The change fromChosenToolCall
toBaseChosenToolCall
inChatMLContent
is consistent with the PR description. However, it's important to ensure thatBaseChosenToolCall
is compatible with all usages ofChatMLContent
. This change should be verified in the context of the codebase.
2. agents-api/tests/test_docs_routes.py:139
- Draft comment:
Theawait asyncio.sleep(0.5)
is a workaround for a timing issue. Consider documenting this or addressing the root cause if possible. This is also applicable on line 161. - Reason this comment was not posted:
Confidence changes required:50%
The addition ofawait asyncio.sleep(0.5)
in the test functions is a workaround for a timing issue. This should be documented or addressed more robustly if possible.
Workflow ID: wflow_SYe9QOPrWD7I1N8p
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Signed-off-by: Diwank Singh Tomer diwank.singh@gmail.com
Important
This pull request adds new tool types for computer-use tools, including computer actions, text editor commands, and bash commands, with updates across models and TypeSpec files.
ChosenComputer20241022
,ChosenTextEditor20241022
, andChosenBash20241022
inChat.py
andEntries.py
.tool_calls
inMessageModel
to include new tool types.Computer20241022Def
,TextEditor20241022Def
, andBash20241022Def
inTools.py
.BaseChosenToolCall
to handle new tool types.anthropic.tsp
for new tool definitions.models.tsp
to include new tool types inToolType
enum.openapi-1.0.0.yaml
.This description was created by for 0db5d91. It will automatically update as commits are pushed.