-
Notifications
You must be signed in to change notification settings - Fork 933
fix(agents-api): remove continue_
field + filter out name
field for non openai models
#1530
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
Conversation
…ld for non openai models
Claude finished @Ahmad-mtos's task —— View job PR Review Complete ✅I've reviewed this pull request for code quality, potential bugs, performance, security, and test coverage. SummaryThis PR removes the deprecated Code Quality & Best Practices ✅
Potential Issues & Bugs
|
Claude finished @Ahmad-mtos's task —— View job Changelog Bot Analysis CompleteActing as the changelog bot to review PR changes and create changelog entries if needed. Todo List:
SummaryUser-Visible Changes Detected: Yes - significant API changes found Changes Analyzed:
Actions Taken:
Note: The changelog entry has been created and committed locally. A rebase with the remote branch is needed before pushing. The changelog properly documents the API changes that will affect users of the Julep platform. |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
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.
Important
Looks good to me! 👍
Reviewed everything up to bc8431d in 1 minute and 47 seconds. Click for details.
- Reviewed
279
lines of code in12
files - Skipped
0
files when reviewing. - Skipped posting
12
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. documentation/advanced/chat.mdx:55
- Draft comment:
Removal of the 'continue_' field from the chat documentation is consistent with the backend changes. Ensure that all usage examples and client references have been updated accordingly. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. documentation/advanced/render.mdx:53
- Draft comment:
The 'continue_' field removal in the render documentation is clear and in line with the API change. Confirm that the example remains accurate without this field. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. src/agents-api/agents_api/activities/task_steps/prompt_step.py:49
- Draft comment:
The new filtering block removes the 'name' field for non-OpenAI models using a list comprehension. This approach is concise; however, consider ensuring that 'prompt' is always a list of dictionaries and that its usage downstream remains valid. You might also consider naming the filtered output differently if the original value is needed later. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. src/agents-api/agents_api/autogen/Chat.py:303
- Draft comment:
The 'continue_' field has been removed from both the Delta and Message models in Chat.py. This is a breaking change for clients expecting that field. Ensure that all consumers of the API are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is pointing out a breaking change due to the removal of a field in the models. It suggests ensuring that all consumers of the API are updated accordingly, which is a form of asking the author to ensure something. This violates the rule against asking the author to ensure behavior is intended or tested.
5. src/agents-api/agents_api/autogen/Tasks.py:884
- Draft comment:
The 'continue_' field has been removed from the PromptItem model in Tasks. Make sure that any systems relying on this field are adjusted for this breaking change. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure that systems relying on the removed field are adjusted, which violates the rule against asking the author to ensure compatibility or test changes. It doesn't provide a specific suggestion or ask for confirmation of intent, which would be acceptable.
6. src/integrations-service/integrations/autogen/Chat.py:285
- Draft comment:
The 'continue_' field removal from both Delta and Message models in the integrations service Chat module is consistent with changes elsewhere. Confirm that integration tests and client applications are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is asking the PR author to confirm that integration tests and client applications are updated, which violates the rule against asking for confirmation or ensuring testing. It does not provide a specific code suggestion or point out a specific issue with the code.
7. src/integrations-service/integrations/autogen/Tasks.py:850
- Draft comment:
The 'continue_' field has been removed from the PromptItem model in the integrations service Tasks module. Ensure that consumers are prepared for this removal. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to ensure that consumers are prepared for the removal of a field, which violates the rule against asking the author to ensure something. It does not provide a specific suggestion or ask for confirmation of intention.
8. src/llm-proxy/litellm-config.yaml:179
- Draft comment:
The updated comment clarifies that 'llama-3.1-70b-versatile' has been decommissioned. However, note that 'litellm_params' still reference 'groq/llama-3.1-70b-versatile'. Verify that this discrepancy is intentional or update the parameter accordingly. - Reason this comment was not posted:
Comment was on unchanged code.
9. src/schemas/create_agent_request.json:2383
- Draft comment:
New boolean fields 'auto_run_tools' and 'recall_tools' have been added, and 'tool_request' has been added to the event enum. Ensure that these additions are documented and that any existing clients are backwards–compatible with these schema changes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure documentation and backward compatibility, which falls under the rule of not asking the author to ensure or verify things. It doesn't provide a specific suggestion or point out a specific issue with the code.
10. src/schemas/create_task_request.json:2383
- Draft comment:
Similar to the agent schema, new fields 'auto_run_tools' and 'recall_tools' and the new event type 'tool_request' have been introduced. Confirm that documentation and client handling are updated to match these schema changes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to confirm that documentation and client handling are updated to match schema changes. This falls under the category of asking the author to ensure something is done, which is against the rules.
11. src/typespec/entries/models.tsp:81
- Draft comment:
The removal of the 'continue' property from the ChatMLMessage model in the Typespec file is in line with the updated API. Ensure that any dependent consumers of this model are informed of the change. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative and suggests ensuring that dependent consumers are informed, which violates the rules against making purely informative comments and asking the author to ensure something.
12. src/typespec/tsp-output/@typespec/openapi3/openapi-1.0.0.yaml:2822
- Draft comment:
The 'continue' property has been removed from several schema definitions in the OpenAPI spec. This aligns with the backend changes, but ensure that all API consumers update their references to this field. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure that all API consumers update their references to a removed field. This falls under the category of asking the author to ensure something is done, which violates the rules.
Workflow ID: wflow_RbCz2OMgxaP7RaxI
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
User description
continue_
field + filter outname
field for non openai modelsPR Type
Bug fix, Enhancement
Description
Remove deprecated
continue_
field from message modelsFilter out
name
field for non-OpenAI modelsUpdate documentation and schema definitions
Add new chat input properties for tool handling
Changes diagram
Changes walkthrough 📝
7 files
Filter name field for non-OpenAI models
Remove continue_ field from Delta and Message
Remove continue_ field from PromptItem
Remove continue_ field from Delta and Message
Remove continue_ field from PromptItem
Remove continue field from ChatMLMessage
Remove continue field from OpenAPI schema
2 files
Remove continue_ field from documentation
Remove continue_ field from documentation
1 files
Update Groq model comment
2 files
Add auto_run_tools and recall_tools properties
Add auto_run_tools and recall_tools properties