Filter function calls #380
Conversation
Users of the ADK can now define the tools they want to filter through the config struct when creating a new llmagent.
The api key has been completely deleted from existence.
Summary of ChangesHello @cs168898, 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 introduces a significant enhancement to the LLM agent configuration by allowing developers to define a static filter for tools. This feature enables fine-grained control over an agent's capabilities, supporting scenarios such as restricting tool access based on user roles or premium features. The filtering is applied during agent initialization, providing a clear mechanism to manage tool availability. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a filtering mechanism for tools in LLM agents, which is a great addition for controlling agent capabilities. The implementation is mostly solid, with a new Filter field in the agent configuration and corresponding logic to apply it. The new test case effectively validates the filtering of a disallowed tool.
My main feedback is around the filter's logic and implementation. The current approach acts as a strict allowlist, which can be inconvenient for use cases like tool prohibition. I've suggested changing it to a denylist-by-default behavior, which should be more intuitive. I also noticed some code duplication in the filtering logic that could be refactored for better maintainability. Additionally, there's a minor formatting issue.
Overall, this is a valuable feature, and with a few adjustments, it will be even better.
internal/llminternal/base_flow.go
Outdated
| tools := []tool.Tool{} | ||
| baseTools := Reveal(llmAgent).Tools | ||
|
|
||
| if hasFilter { | ||
| for _, t := range baseTools { | ||
| if use, ok := filter[t.Name()]; ok && use { | ||
| tools = append(tools, t) | ||
| } | ||
| } | ||
| } else { | ||
| tools = append(tools, baseTools...) | ||
| } | ||
|
|
||
| // run processors for tools. | ||
| tools := Reveal(llmAgent).Tools | ||
| for _, toolSet := range Reveal(llmAgent).Toolsets { | ||
|
|
||
| tsTools, err := toolSet.Tools(icontext.NewReadonlyContext(ctx)) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to extract tools from the tool set %q: %w", toolSet.Name(), err) | ||
| } | ||
|
|
||
| tools = append(tools, tsTools...) | ||
| // if filter contains something then filter accordingly | ||
| if hasFilter { | ||
| for _, tool := range tsTools { | ||
| use, found := filter[tool.Name()] | ||
| if !found { | ||
| continue | ||
| } | ||
| if use { | ||
| tools = append(tools, tool) | ||
| } | ||
| } | ||
| } else { | ||
| // else just append the original toolset | ||
| tools = append(tools, tsTools...) | ||
| } | ||
| } | ||
|
|
||
| return toolPreprocess(ctx, req, tools) |
There was a problem hiding this comment.
The current filter implementation acts as a strict allowlist. This means if a filter map is provided, only tools explicitly set to true are included. This can be cumbersome if the user wants to disallow only a few tools, as they would have to list all other tools as true. A more intuitive approach for tool prohibition is to treat the filter as a denylist by default: if a tool is not in the filter, it's allowed.
Additionally, the logic for filtering baseTools and tools from Toolsets is duplicated. This can be refactored for better readability and maintainability by first collecting all tools and then applying the filter once.
Here is a suggested refactoring that implements the denylist behavior and removes code duplication:
// run processors for tools and filter
var allTools []tool.Tool
allTools = append(allTools, Reveal(llmAgent).Tools...)
for _, toolSet := range Reveal(llmAgent).Toolsets {
tsTools, err := toolSet.Tools(icontext.NewReadonlyContext(ctx))
if err != nil {
return fmt.Errorf("failed to extract tools from the tool set %q: %w", toolSet.Name(), err)
}
allTools = append(allTools, tsTools...)
}
if !hasFilter {
return toolPreprocess(ctx, req, allTools)
}
var filteredTools []tool.Tool
for _, t := range allTools {
// If a tool is not in the filter, it is allowed by default.
// This makes the filter act as a denylist, which is more intuitive for prohibiting tools.
if use, ok := filter[t.Name()]; !ok || use {
filteredTools = append(filteredTools, t)
}
}
return toolPreprocess(ctx, req, filteredTools)
internal/llminternal/agent.go
Outdated
|
|
||
| Tools []tool.Tool | ||
| Toolsets []tool.Toolset | ||
| Filter map[string]bool |
There was a problem hiding this comment.
There's a minor formatting issue here. A tab character (\t) is used for alignment instead of spaces. It's better to use spaces for consistent formatting with the rest of the struct fields.
| Filter map[string]bool | |
| Filter map[string]bool |
The current implementation now ignores the value of the Filter map and if the tool is NOT found in the Filter map, it will be included into the available tools to use. Also cleaned up tab and spacing issues
1 participant
Filter Function Calls
Developers using ADK-go can now add filters when they create a new LLM agent as such:
The Filter field accepts a map where the
Keyis the tool's name and theValueis a boolean on whether the tool is allowed or not.True = Tool is allowed.
False = Tool is NOT allowed.
Configstruct.Statestruct.base_flow.goNotes
I have decided to start on developer declared filter first as the use cases can include tool prohibition based on user roles.
For example, if a user has no premium access account, developers could create a conditional check and initialize the agent without access to specific tools.
The current implementation will impose a permanent tool filter on all requests after agent initialization, meaning we cannot change the filter once the agent has been initialized.
TODO
This PR references this Issue