-
Notifications
You must be signed in to change notification settings - Fork 262
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
Docs: how to enable custom tools guide #1652
Conversation
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.
Left some comments on docs
docs/configuration/ai-analysis.rst
Outdated
description: "Perform a GET request to example.com" | ||
command: "curl -X GET https://example.com" | ||
- name: "curl_with_params" | ||
description: "Perform a GET request to example.com with query parameters" |
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.
I would give a description that helps the LLM understand when to use this (use this to xyz...)
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.
Updated descriptions
docs/configuration/ai-analysis.rst
Outdated
command: "kubectl drain {{ node_name }} --ignore-daemonsets --force --delete-emptydir-data" | ||
|
||
|
||
Adding Custom Tools to Holmes |
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.
Is this the same as the two examples above?
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.
Basically yes, but previous toolsets were just examples, but here we give a short guide on how to add a toolset
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.
Can you give full insttructions on the previous 2 examples as well? (And the exact yaml structure necessary - e.g. holmes.toolsets
and not just toolsets
)
So we will have
Example 1 ...
Example 2 ...
Example 3 ...
And each will show the full instructions
…dev/robusta into holmes-docs-enable-custom-tools
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.
Looking good. Left a few more comments. This should be the final review, I hope.
docs/configuration/ai-analysis.rst
Outdated
command: "kubectl drain {{ node_name }} --ignore-daemonsets --force --delete-emptydir-data" | ||
|
||
|
||
Adding Custom Tools to Holmes |
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.
Can you give full insttructions on the previous 2 examples as well? (And the exact yaml structure necessary - e.g. holmes.toolsets
and not just toolsets
)
So we will have
Example 1 ...
Example 2 ...
Example 3 ...
And each will show the full instructions
docs/configuration/ai-analysis.rst
Outdated
- No | ||
* - ``additional_instructions`` | ||
- string | ||
- Additional shell commands or processing instructions applied to the output of this tool. |
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.
Can you give an example of how this would be used?
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.
Updated the description
Toolset Fields | ||
-------------- | ||
|
||
.. list-table:: |
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.
this doesn't render right in the preview url - see https://docs.robusta.dev/holmes-docs-enable-custom-tools/configuration/ai-analysis.html#how-to-define-a-new-toolset
docs/configuration/ai-analysis.rst
Outdated
- "GITHUB_TOKEN" | ||
- command: "curl --version" | ||
tools: | ||
- name: "list_user_repos" |
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.
explain in a comment that parameters here are inferred
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.
Added comments
docs/configuration/ai-analysis.rst
Outdated
description: "Shows the most recent commits for a repository" | ||
command: "cd {{ repo_dir }} && git log -{{number_of_commits}} --oneline" | ||
|
||
- name: "get_repo_details" |
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.
explain in a comment that here you chose to be explicit about parameters - maybe show an example where this is actually useful - e.g. if you add a parameter that you want to be optional
docs/configuration/ai-analysis.rst
Outdated
tools: | ||
- name: "list_user_repos" | ||
description: "Lists all repositories for a GitHub user" | ||
command: "curl -H 'Authorization: token {{ github_token }}' https://api.github.com/users/{{ username }}/repos" |
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.
do you need to override the user_description so that the github_token is not shown there?
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.
This was changed as we removed variables section
…dev/robusta into holmes-docs-enable-custom-tools
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.
Left some final comments. Please fix then merge. No need for a further review.
Thanks!
docs/configuration/ai-analysis.rst
Outdated
- string | ||
- Instructions on how to install prerequisites required by the toolset. | ||
- No | ||
* - ``variables`` |
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.
This is no longer relevant, right?
docs/configuration/ai-analysis.rst
Outdated
- **Yes** | ||
* - ``command`` | ||
- string | ||
- A shell command template that the tool will execute. Can include variables and parameters using Jinja2 syntax (``{{ variable_name }}``). |
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.
Can include environment variables using ${ENV_VAR}
or parameters that the LLM will fill in, using Jinja2 syntax ({{ param_name }}
)
https://docs.robusta.dev/holmes-docs-enable-custom-tools/configuration/ai-analysis.html#toolsets-examples