-
Notifications
You must be signed in to change notification settings - Fork 295
Added initial llms.txt #261
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
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @ivanmkc, 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 new, comprehensive documentation file, 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
|
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.
Code Review
This pull request introduces llms.txt, a new comprehensive documentation file for the Go Agent Development Kit (ADK). The file provides a good overview of the ADK's features with code examples. My review focuses on improving the correctness and robustness of these Go code snippets. I've identified several critical issues, including potential panics from unsafe type assertions and ignored errors, which could mislead users. I've provided suggestions to fix these issues by adding proper error handling and safe type checks. Additionally, I've pointed out some formatting inconsistencies and minor clarity improvements in the documentation to enhance readability.
|
I'll address some of the AI-comments. The llms.txt isn't supposed to be runnable code but I'll add more err checks so the AI-user of llms.txt isn't biased to write code that ignores errors. |
2b6df20 to
4318870
Compare
4318870 to
1181e84
Compare
|
How do I get @gemini-code-assist to re-review? |
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.
Code Review
This pull request introduces llms.txt, a comprehensive documentation file for the Go Agent Development Kit (ADK). The file provides a good overview of the ADK's features with code examples. My review focuses on ensuring the correctness and clarity of this new documentation. I've identified several Go code snippets that would fail to compile due to incorrect API usage, particularly around handling genai.Part and other struct fields. I have provided suggestions to fix these examples. Additionally, I've pointed out some minor formatting and consistency issues to improve the document's readability.
| type updateUserPreferenceResult struct { | ||
| Status string `json:"status"` | ||
| UpdatedPreference string `json:"updated_preference"` | ||
| } |
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.
The updateUserPreferenceResult struct is missing the Message field, but the updateUserPreference function attempts to set it on lines 454 and 465. This will cause a compilation error. Please add the Message field to the struct definition.
type updateUserPreferenceResult struct {
Status string `json:"status"`
UpdatedPreference string `json:"updated_preference"`
Message string `json:"message,omitempty"`
}
| func onBeforeModelGuardrail(ctx agent.CallbackContext, req *model.LLMRequest) (*model.LLMResponse, error) { | ||
| for _, content := range req.Contents { | ||
| for _, part := range content.Parts { | ||
| if strings.Contains(part.Text, "finance") { | ||
| // By returning a non-nil response, we override the LLM call. | ||
| return &model.LLMResponse{ | ||
| Content: genai.NewContentFromText( | ||
| "I'm sorry, but I cannot discuss financial topics.", | ||
| genai.RoleModel, | ||
| ), | ||
| }, nil | ||
| } | ||
| } | ||
| } | ||
| // Returning nil allows the default LLM call to proceed. | ||
| return nil, nil | ||
| } |
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.
The code example for onBeforeModelGuardrail incorrectly tries to access part.Text. A genai.Part is an interface, and to access its text content, you need to perform a type assertion to genai.Text. This will cause a compilation error as-is.
func onBeforeModelGuardrail(ctx agent.CallbackContext, req *model.LLMRequest) (*model.LLMResponse, error) {
for _, content := range req.Contents {
for _, part := range content.Parts {
if text, ok := part.(genai.Text); ok && strings.Contains(string(text), "finance") {
// By returning a non-nil response, we override the LLM call.
return &model.LLMResponse{
Content: genai.NewContentFromText(
"I'm sorry, but I cannot discuss financial topics.",
genai.RoleModel,
),
}, nil
}
}
}
// Returning nil allows the default LLM call to proceed.
return nil, nil
}
| func onAfterModel(ctx agent.CallbackContext, resp *model.LLMResponse, respErr error) (*model.LLMResponse, error) { | ||
| if respErr != nil || resp == nil { | ||
| return nil, respErr // Pass through errors | ||
| } | ||
| // Modify the response text | ||
| originalText := resp.Content.Parts[0].Text | ||
| modifiedText := strings.ReplaceAll(originalText, "joke", "funny story") | ||
| resp.Content.Parts[0].Text = modifiedText | ||
| return resp, nil | ||
| } |
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 example has a similar issue to the onBeforeModelGuardrail example. resp.Content.Parts[0] is a genai.Part interface and does not have a .Text field. You need to perform a type assertion to get the text, and you cannot directly assign a new string to it. You should create a new genai.Text part and replace it in the slice. This code will not compile as written.
func onAfterModel(ctx agent.CallbackContext, resp *model.LLMResponse, respErr error) (*model.LLMResponse, error) {
if respErr != nil || resp == nil || resp.Content == nil || len(resp.Content.Parts) == 0 {
return nil, respErr // Pass through errors
}
// Modify the response text
if originalText, ok := resp.Content.Parts[0].(genai.Text); ok {
modifiedText := strings.ReplaceAll(string(originalText), "joke", "funny story")
resp.Content.Parts[0] = genai.Text(modifiedText)
}
return resp, nil
}
llms.txt
Outdated
| documentPart, err := ctx.Artifacts().Load(ctx, args.DocumentName) | ||
| if err != nil { | ||
| return processDocumentResult{Status: "error", Message: fmt.Sprintf("Document not found: %v", err)} | ||
| } |
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.
The ctx.Artifacts().Load() function returns a *artifact.LoadResponse struct. The code incorrectly assigns this to documentPart and then doesn't use it, which would cause a compilation error. The loaded artifact part should be accessed via loadResp.Part.
loadResp, err := ctx.Artifacts().Load(ctx, args.DocumentName)
if err != nil {
return processDocumentResult{Status: "error", Message: fmt.Sprintf("Document not found: %v", err)}
}
_ = loadResp.Part // The loaded document part can be used for analysis.
llms.txt
Outdated
|
|
||
| ## 2. Multi-Agent Systems | ||
|
|
||
| Complex tasks can be broken down by composing multiple agents. `WorkflowAgents` orchestrate the execution flow of their `sub_agents`. |
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.
For consistency with Go's naming conventions and the code examples in this document (e.g., agent.Config's SubAgents field), it would be clearer to refer to sub_agents as SubAgents in the descriptive text.
Complex tasks can be broken down by composing multiple agents. `WorkflowAgents` orchestrate the execution flow of their `SubAgents`.
llms.txt
Outdated
| } | ||
| ``` | ||
|
|
||
| **`agent.ReadonlyContext`** |
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.
|
|
||
| For a complete and detailed API reference, please visit the official Go documentation. | ||
|
|
||
| - [:octicons-arrow-right-24: View Go API Docs](https://pkg.go.dev/google.golang.org/adk) No newline at end of file |
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.
149f28f to
f432ecd
Compare
f432ecd to
4d3c54c
Compare
|
Let me review the comments again. |
|
We are planning some breaking changes to the API surface. Would it be possible to come back to this after some of those changes land? |
|
@rakyll Of course, ping me when that happens. The eventual goal is that llms.txt will update with each breaking version. |
Initial llms.txt which is a condensed version of adk-docs, specific for Golang.
This is an initial, baseline version which will be benchmarked against future versions (or not using a llms.txt).
Equivalent to: https://github.com/google/adk-python/blob/main/llms-full.txt
This will need updating when the API changes.