feat: remove special handling of builtin::rag tool #1015
Conversation
facebook-github-bot
added
the
CLA Signed
ehhuang
changed the title
ehhuang
changed the title
ehhuang
changed the title
ehhuang
force-pushed
the
pr1014
branch
3 times, most recently
from
17c3d05 to
01cc4c0
Compare
ehhuang
changed the title
ehhuang
requested review from
ashwinb,
yanxi0830,
hardikjshah,
dltn,
raghotham,
dineshyv,
vladimirivic,
sixianyi0721 and
terrytangyuan
as code owners
| @@ -466,5 +503,8 @@ def test_rag_and_code_agent(llama_stack_client, agent_config): | |||
| documents=docs, | |||
| ) | |||
| logs = [str(log) for log in EventLogger().log(response) if log is not None] | |||
| logs_str = "".join(logs) | |||
| logs_str = "\n".join(logs) | |||
| print(logs_str) | |||
| @@ -381,92 +379,6 @@ async def _run( | |||
| if documents: | |||
| await self.handle_documents(session_id, documents, input_messages, tool_defs) | |||
|
|
|||
| if RAG_TOOL_GROUP in toolgroups and len(input_messages) > 0: | |||
There was a problem hiding this comment.
Wondering what happens if we explicitly specify the builtin::rag toolgroups now?
I'm wondering if there's a way we can specify how to force retrieve v.s. retrieve based on model tool call.
There was a problem hiding this comment.
There's a knowledge_search tool being exposed (see list_runtime_tools). The model chooses which tools to call among all the tools. The user can include in instructions that they would like certain tools called. The tool_choice option will also support this, coming next.
| tools=[ | ||
| tool for tool in tool_defs.values() if tool_to_group.get(tool.tool_name, None) != RAG_TOOL_GROUP | ||
| ], | ||
| tools=[tool for tool in tool_defs.values()], |
There was a problem hiding this comment.
For builtin tools we do not have a description in the system prompt besides
Environment: ipython
Tools: brave_search, wolfram_alpha
for all other tools , we have a prompt saying here is the tool and the description of when to use it.
Now, we are neither explicitly calling the tool (sine this was never a builtin tool that the model was trained with) nor does the model know when to invoke this since its not in the client tools for which we include descriptions.
Thus, its not clear how this will get invoked if at all. Can you test our RAG examples to see if this works ?
There was a problem hiding this comment.
But the client_sdk tests do have rag in multiple tests and it seems like they passed ? So what am i missing here ?
There was a problem hiding this comment.
Yea by removing this logic to remove RAG_TOOL_GROUP before, we actually are passing the rag tool to model now, with descriptions. So the model does call it. And you're right the client_tests do test it.
|
Also reminder - we will need to update getting_started and other places where we have RAG agent. |
Summary: Test Plan: Summary: Test Plan:
Summary:
Lets the model decide which tool it needs to call to respond to a query.
Test Plan:
Stack created with Sapling. Best reviewed with ReviewStack.