docs: migration of Go Sdk docs #2292
Conversation
Summary of ChangesHello @AnmolShukla2002, 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 focuses on improving the documentation for the Go SDK of the MCP Toolbox. The changes provide a more detailed introduction to the SDK's architecture and purpose, clarify the functionality of its core and integration-specific packages, and offer practical guidance for getting started. The primary impact is to significantly lower the barrier to entry for developers looking to integrate the MCP Toolbox Go SDK with their Gen AI applications by providing clear, runnable examples for popular frameworks. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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.
Code Review
This pull request is a great step towards improving the Go SDK documentation by migrating content and adding several useful code samples for various AI frameworks. The new documentation structure is clearer. However, I've found several issues within the new markdown files and Go code samples, including incorrect Go syntax, inconsistent and invalid model names, poor error handling, use of deprecated libraries, and some minor formatting and grammatical errors. I've provided detailed comments and suggestions to address these points, which should help improve the quality and correctness of the documentation and samples.
| for event, err := range r.Run(ctx, userID, session.ID(), userMsg, agent.RunConfig{ | ||
| StreamingMode: streamingMode, | ||
| }) { | ||
| if err != nil { | ||
| fmt.Printf("\nAGENT_ERROR: %v\n", err) | ||
| } else { | ||
| if event.LLMResponse.Content != nil { | ||
| for _, p := range event.LLMResponse.Content.Parts { | ||
| if streamingMode != agent.StreamingModeSSE || event.LLMResponse.Partial { | ||
| fmt.Print(p.Text) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The syntax for event, err := range r.Run(...) is not valid Go. The r.Run function returns a channel and an error separately, and range cannot be used on a multi-value return like this. You should first call the function, check the returned error, and then use range on the channel. Any errors during streaming will be part of the event struct from the channel.
| for event, err := range r.Run(ctx, userID, session.ID(), userMsg, agent.RunConfig{ | |
| StreamingMode: streamingMode, | |
| }) { | |
| if err != nil { | |
| fmt.Printf("\nAGENT_ERROR: %v\n", err) | |
| } else { | |
| if event.LLMResponse.Content != nil { | |
| for _, p := range event.LLMResponse.Content.Parts { | |
| if streamingMode != agent.StreamingModeSSE || event.LLMResponse.Partial { | |
| fmt.Print(p.Text) | |
| } | |
| } | |
| } | |
| } | |
| } | |
| eventCh, err := r.Run(ctx, userID, session.ID(), userMsg, agent.RunConfig{ | |
| StreamingMode: streamingMode, | |
| }) | |
| if err != nil { | |
| log.Fatalf("agent execution failed: %v", err) | |
| } | |
| for event := range eventCh { | |
| if event.Error != nil { | |
| fmt.Printf("\nAGENT_ERROR: %v\n", event.Error) | |
| } else { | |
| if event.LLMResponse.Content != nil { | |
| for _, p := range event.LLMResponse.Content.Parts { | |
| if streamingMode != agent.StreamingModeSSE || event.LLMResponse.Partial { | |
| fmt.Print(p.Text) | |
| } | |
| } | |
| } | |
| } | |
| } |
| }, | ||
| }, | ||
| } | ||
| genContentResp, _ := client.Models.GenerateContent(ctx, modelName, contents, config) |
There was a problem hiding this comment.
The error from client.Models.GenerateContent is being ignored. This is a critical operation, and any network or API errors should be handled to prevent the program from continuing in an unpredictable state.
| genContentResp, _ := client.Models.GenerateContent(ctx, modelName, contents, config) | |
| genContentResp, err := client.Models.GenerateContent(ctx, modelName, contents, config) | |
| if err != nil { | |
| log.Fatalf("GenerateContent failed: %v", err) | |
| } |
| log.Fatal("Could not invoke tool", err) | ||
| } | ||
|
|
||
| params.Messages = append(params.Messages, openai.ToolMessage(result.(string), toolCall.ID)) |
There was a problem hiding this comment.
The type assertion result.(string) is unsafe. If the Invoke method returns a value that is not a string, this will cause a panic. It's safer to use a type switch or fmt.Sprintf to convert the result to a string, as seen in other samples.
| params.Messages = append(params.Messages, openai.ToolMessage(result.(string), toolCall.ID)) | |
| params.Messages = append(params.Messages, openai.ToolMessage(fmt.Sprintf("%v", result), toolCall.ID)) |
| // Make initial chat completion request | ||
| completion, err := openAIClient.Chat.Completions.New(ctx, params) | ||
| if err != nil { | ||
| panic(err) |
There was a problem hiding this comment.
Using panic for recoverable errors like API calls is generally discouraged in Go as it causes an abrupt stop of the program. It would be better to handle the error gracefully, for instance by using log.Fatalf, which is used in the other examples. This comment applies to all uses of panic in this file.
| panic(err) | |
| log.Fatalf("Chat completion failed: %v", err) |
| model, err := gemini.NewModel(ctx, "gemini-2.5-flash", &genai.ClientConfig{ | ||
| APIKey: genaiKey, | ||
| }) |
There was a problem hiding this comment.
The model name gemini-2.5-flash appears to be incorrect as this model version does not exist. Please use a valid model name, for example gemini-1.5-flash-latest. Several other sample files in this PR also use incorrect or inconsistent model names that should be reviewed.
| model, err := gemini.NewModel(ctx, "gemini-2.5-flash", &genai.ClientConfig{ | |
| APIKey: genaiKey, | |
| }) | |
| model, err := gemini.NewModel(ctx, "gemini-1.5-flash-latest", &genai.ClientConfig{ | |
| APIKey: genaiKey, | |
| }) |
|
|
||
| // Start the conversation history. | ||
| messageHistory := []llms.MessageContent{ | ||
| llms.TextParts(llms.ChatMessageTypeHuman, "Find hotels in Basel with Basel in it's name."), |
There was a problem hiding this comment.
There's a grammatical error in the query string. It should be its (possessive) instead of it's (a contraction of 'it is').
| llms.TextParts(llms.ChatMessageTypeHuman, "Find hotels in Basel with Basel in it's name."), | |
| llms.TextParts(llms.ChatMessageTypeHuman, "Find hotels in Basel with Basel in its name."), |
| log.Fatalf("Failed to load tools: %v\nMake sure your Toolbox server is running and the tool is configured.", err) | ||
| } | ||
|
|
||
| genAITools := make([]*genai.Tool, len(tools)) |
There was a problem hiding this comment.
This line has incorrect indentation. Please remove the extra space at the beginning to adhere to standard Go formatting.
| genAITools := make([]*genai.Tool, len(tools)) | |
| genAITools := make([]*genai.Tool, len(tools)) |
| toolsMap[tool.Name()] = tool | ||
|
|
||
| } | ||
| question := "Find hotels in Basel with Basel in it's name " |
There was a problem hiding this comment.
There's a grammatical error in the query string. It should be its (possessive) instead of it's (a contraction of 'it is'). Also, there is a trailing space that can be removed.
| question := "Find hotels in Basel with Basel in it's name " | |
| question := "Find hotels in Basel with Basel in its name" |
|
|
||
| <!-- /TOC --> | ||
|
|
||
| ## Overview |
There was a problem hiding this comment.
This ## Overview heading is a duplicate of the one on line 10. This can be confusing for readers. Please consider merging the content of both sections under a single heading to improve clarity.
| This repository contains the Go SDK designed to seamlessly integrate the | ||
| functionalities of the [MCP | ||
| Toolbox](https://github.com/googleapis/genai-toolbox) into your Gen AI | ||
| applications. The SDK allow you to load tools defined in Toolbox and use them |
There was a problem hiding this comment.
There is a minor grammatical error here. Since 'SDK' is singular, the verb should be 'allows'.
| applications. The SDK allow you to load tools defined in Toolbox and use them | |
| applications. The SDK allows you to load tools defined in Toolbox and use them |
Related PRs : #2330