feat: Add system tool calls to the chat endpoint #1179
Conversation
WalkthroughThis PR introduces a new utility function, Changes
Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts belowEmoji Descriptions:
Interact with the Bot:
|
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
| model_response = await eval_tool_calls( | ||
| litellm.acompletion, {"system"}, developer.id, **{**settings, **params} | ||
| ) |
There was a problem hiding this comment.
The eval_tool_calls function is called with a set containing only 'system' as tool types, which may incorrectly restrict tool evaluation. The set should include all relevant tool types being used.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| model_response = await eval_tool_calls( | |
| litellm.acompletion, {"system"}, developer.id, **{**settings, **params} | |
| ) | |
| model_response = await eval_tool_calls( | |
| litellm.acompletion, {"system", "function"}, developer.id, **{**settings, **params} | |
| ) |
| response: ModelResponse | CustomStreamWrapper | None = None | ||
| done = False | ||
| while not done: | ||
| response: ModelResponse | CustomStreamWrapper = await func(**kwargs) |
There was a problem hiding this comment.
Potential memory leak in eval_tool_calls() - the response variable is reassigned in the loop but previous responses are not properly cleaned up/closed when streaming
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| response: ModelResponse | CustomStreamWrapper | None = None | |
| done = False | |
| while not done: | |
| response: ModelResponse | CustomStreamWrapper = await func(**kwargs) | |
| if response: | |
| await response.close() | |
| response: ModelResponse | CustomStreamWrapper | None = None | |
| done = False | |
| while not done: | |
| response: ModelResponse | CustomStreamWrapper = await func(**kwargs) |
|
|
||
| connection_pool = getattr(app.state, "postgres_pool", None) | ||
| tool_handler = partial(tool_handler, connection_pool=connection_pool) | ||
| arguments["developer_id"] = str(developer_id) |
There was a problem hiding this comment.
Type error in call_tool() - developer_id is converted to string but some query handlers expect UUID type, causing potential runtime errors
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| arguments["developer_id"] = str(developer_id) | |
| arguments["developer_id"] = developer_id if isinstance(developer_id, uuid.UUID) else uuid.UUID(str(developer_id)) |
CI Feedback 🧐(Feedback updated until commit 459aa61)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
whiterabbit1983
force-pushed
the
f/chat-system-tool-calls
branch
from
459aa61 to
ea7b342
Compare
| model_response = await eval_tool_calls( | ||
| litellm.acompletion, {"system"}, developer.id, **{**settings, **params} | ||
| ) |
There was a problem hiding this comment.
The eval_tool_calls function is called with an incorrect argument - passing a set {'system'} instead of a list of system messages, which will likely cause runtime errors.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| model_response = await eval_tool_calls( | |
| litellm.acompletion, {"system"}, developer.id, **{**settings, **params} | |
| ) | |
| model_response = await eval_tool_calls( | |
| litellm.acompletion, ["system"], developer.id, **{**settings, **params} | |
| ) |
| tool_args = json.loads(tool.function.arguments) | ||
| tool_response = await call_tool(developer_id, tool_name, tool_args) |
There was a problem hiding this comment.
Missing error handling for invalid JSON in tool.function.arguments. If JSON parsing fails, it will raise an unhandled exception.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| tool_args = json.loads(tool.function.arguments) | |
| tool_response = await call_tool(developer_id, tool_name, tool_args) | |
| tool_name = tool.function.name | |
| try: | |
| tool_args = json.loads(tool.function.arguments) | |
| except json.JSONDecodeError: | |
| raise ValueError(f"Invalid JSON in tool arguments: {tool.function.arguments}") |
EntelligenceAI PR Summary
The recent update introduces a new utility function,
eval_tool_calls, intools.pyto enhance the evaluation of tool calls within chat sessions. Thechatfunction inchat.pyhas been updated to utilize this utility, replacing direct calls tolitellm.acompletion. This change aims to improve code modularity and streamline the integration of tool responses in chat sessions.