-
Notifications
You must be signed in to change notification settings - Fork 13
Fix support for models that do support inbuilt function calling. #151
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
Signed-off-by: Palaniappan R <[email protected]>
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.
Pull Request Overview
This PR removes support for inbuilt tool calling in models and refactors related code accordingly.
- Explicitly assigns a CPU device fallback in the FAISS vectorstore.
- Removes the inbuilt_tool_calling parameter from API router and agent instantiations.
- Renames and refactors the agent method to choose_tools in the RetrieverGraph.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| backend/src/vectorstores/faiss.py | Explicitly assigns {"device": "cpu"} when use_cuda is False, ensuring a clear device configuration. |
| backend/src/prompts/prompt_templates.py | Removes the redundant tool_rephrase_prompt_template to simplify the prompt logic. |
| backend/src/api/routers/graphs.py | Removes the inbuilt_tool_calling parameter in line with the PR goal. |
| backend/src/agents/retriever_graph.py | Renames the agent method to choose_tools and removes the inbuilt tool calling branch. |
Comments suppressed due to low confidence (2)
backend/src/vectorstores/faiss.py:34
- Consider adding a comment explaining why 'cpu' is explicitly assigned in the else clause to clarify the intent and improve code readability.
model_kwargs = {"device": "cuda"} if use_cuda else {"device": "cpu"}
backend/src/agents/retriever_graph.py:123
- The renaming of the method from 'agent' to 'choose_tools' improves clarity; please ensure that any inline documentation or references to the old name are updated accordingly.
def choose_tools(self, state: AgentState) -> dict[str, list[str]]:
5c880f5 to
c805f15
Compare
Signed-off-by: Palaniappan R <[email protected]>
Signed-off-by: Palaniappan R <[email protected]>
Signed-off-by: Palaniappan R <[email protected]>
Signed-off-by: Palaniappan R <[email protected]>
Signed-off-by: Palaniappan R <[email protected]>
Signed-off-by: Palaniappan R <[email protected]>
luarss
left a comment
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.
LGTM
No description provided.