fix: auto-append /v1 to embedding API base URL#6909
fix: auto-append /v1 to embedding API base URL#6909Yaohua-Leo wants to merge 6 commits intoAstrBotDevs:masterfrom
Conversation
- Add tool_call_timeout parameter to _execute_handoff method - Pass run_context.tool_call_timeout to ctx.tool_loop_agent - Add unit test to verify tool_call_timeout is correctly passed - Fixes AstrBotDevs#6711: SubAgent MCP tool call timeout now respects configured timeout The SubAgent handoff execution was using the default 60-second timeout instead of the configured tool_call_timeout from provider settings. This change ensures that SubAgent MCP tool calls respect the user's configured timeout settings.
- Restore test_collect_handoff_image_urls_filters_extensionless_missing_event_file - Fix test_collect_handoff_image_urls_keeps_extensionless_existing_event_file assertion - Keep new test_execute_handoff_passes_tool_call_timeout_to_tool_loop_agent
- Pass run_context.tool_call_timeout directly to ctx.tool_loop_agent - Remove unnecessary local variable assignment - Addresses review feedback from Sourcery AI
- Automatically append /v1 to embedding_api_base if not already present - Fixes issue where embedding providers like SiliconFlow fail without /v1 suffix Fixes AstrBotDevs#6855
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness of API interactions by ensuring that embedding API base URLs are correctly formatted with the '/v1' suffix, preventing potential connection issues with certain providers. Additionally, it improves agent execution by properly propagating tool call timeout settings, allowing for more controlled and reliable tool usage. These changes collectively contribute to a more stable and predictable system behavior. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
/v1auto-append logic assumes the version segment is at the end of the URL; if a user configures anembedding_api_basethat already includes deeper paths (e.g./v1/embeddingsor another versioned path), this will silently produce incorrect URLs—consider parsing the URL path and only adding/v1when there is no path or only/. - When passing
tool_call_timeout=run_context.tool_call_timeoutintotool_loop_agent, this now hard-depends on that attribute being present on allrun_contextinstances; if that’s not guaranteed, consider using a default viagetattr(run_context, 'tool_call_timeout', <default>)to avoid potentialAttributeErrors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `/v1` auto-append logic assumes the version segment is at the end of the URL; if a user configures an `embedding_api_base` that already includes deeper paths (e.g. `/v1/embeddings` or another versioned path), this will silently produce incorrect URLs—consider parsing the URL path and only adding `/v1` when there is no path or only `/`.
- When passing `tool_call_timeout=run_context.tool_call_timeout` into `tool_loop_agent`, this now hard-depends on that attribute being present on all `run_context` instances; if that’s not guaranteed, consider using a default via `getattr(run_context, 'tool_call_timeout', <default>)` to avoid potential `AttributeError`s.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to pass a tool_call_timeout to the agent's tool execution loop, ensuring that tool calls can respect a specified timeout. This is implemented by adding the tool_call_timeout argument to the tool_loop_agent call within the _execute_handoff function, and a new unit test has been added to verify this functionality. Additionally, the PR standardizes the OpenAI embedding API base URL by ensuring it always ends with /v1. There is no feedback to provide on the review comments as none were submitted.
- Parse URL to check if path exists before appending /v1 - Avoids incorrect URLs when user configures paths like /v1/embeddings - Addresses Sourcery review feedback Fixes AstrBotDevs#6855
Summary
Changes
Fixes #6855
Summary by Sourcery
Ensure embedding API base URLs are normalized and propagate tool call timeouts through subagent handoffs.
Bug Fixes:
Enhancements:
Tests: