fix(tools/looker): support instructions, sources, and code interpreter for agent tool#2864
Conversation
900e7c3
into
googleapis:looker-agent
There was a problem hiding this comment.
Code Review
This pull request expands the looker-agent tool by adding support for the update operation and introducing new optional parameters: instructions, sources, and code_interpreter. The documentation and unit tests have been updated accordingly. Feedback focuses on a critical bug where default values for optional parameters would incorrectly overwrite existing agent data during updates, a request to maintain consistency with the project's direct type assertion pattern for parameters, and a suggestion to refactor duplicated request body construction logic.
| instructionsParameter := parameters.NewStringParameterWithDefault("instructions", "", "The instructions (system prompt) for the agent. Used for `create` and `update` operations.") | ||
| sourcesParameter := parameters.NewArrayParameterWithDefault("sources", []any{}, "Optional. A list of JSON-encoded data sources for the agent (e.g., ['{\"model\": \"my_model\", \"explore\": \"my_explore\"}']).", parameters.NewStringParameter("source", "A JSON-encoded source object with 'model' and 'explore' keys.")) | ||
| codeInterpreterParameter := parameters.NewBooleanParameterWithDefault("code_interpreter", false, "Optional. Enables Code Interpreter for this Agent. Used for `create` and `update` operations.") |
There was a problem hiding this comment.
There's a bug in the update operation logic caused by initializing optional parameters with default values. Because instructions, sources, and code_interpreter have defaults, they are always present in the mapParams. The update logic then always includes them in the request body, incorrectly overwriting existing agent properties with default values if the user doesn't specify them.
To fix this, you should:
- Define optional parameters without default values and mark them as not required. This will ensure they are only present if explicitly provided by the user.
- With that change, the update logic will work as intended, as it will only process parameters that are actually present in the request.
| instructionsParameter := parameters.NewStringParameterWithDefault("instructions", "", "The instructions (system prompt) for the agent. Used for `create` and `update` operations.") | |
| sourcesParameter := parameters.NewArrayParameterWithDefault("sources", []any{}, "Optional. A list of JSON-encoded data sources for the agent (e.g., ['{\"model\": \"my_model\", \"explore\": \"my_explore\"}']).", parameters.NewStringParameter("source", "A JSON-encoded source object with 'model' and 'explore' keys.")) | |
| codeInterpreterParameter := parameters.NewBooleanParameterWithDefault("code_interpreter", false, "Optional. Enables Code Interpreter for this Agent. Used for `create` and `update` operations.") | |
| instructionsParameter := parameters.NewStringParameterWithRequired("instructions", "The instructions (system prompt) for the agent. Used for create and update operations.", false) | |
| sourcesParameter := parameters.NewArrayParameterWithRequired("sources", "Optional. A list of JSON-encoded data sources for the agent (e.g., ['{\"model\": \"my_model\", \"explore\": \"my_explore\"}']).", false, parameters.NewStringParameter("source", "A JSON-encoded source object with 'model' and 'explore' keys.")) | |
| codeInterpreterParameter := parameters.NewBooleanParameterWithRequired("code_interpreter", "Optional. Enables Code Interpreter for this Agent. Used for create and update operations.", false) |
| var operation, agentId, name, instructions string | ||
| if v, ok := mapParams["operation"].(string); ok { | ||
| operation = v | ||
| } | ||
| if v, ok := mapParams["agent_id"].(string); ok { | ||
| agentId = v | ||
| } | ||
| if v, ok := mapParams["name"].(string); ok { | ||
| name = v | ||
| } | ||
| if v, ok := mapParams["instructions"].(string); ok { | ||
| instructions = v | ||
| } |
There was a problem hiding this comment.
The use of the comma-ok idiom for parameter access contradicts the project's established pattern. Direct type assertions are preferred for consistency as the parameter parsing framework's typed constructors guarantee safety. Please revert to direct type assertions for parameters that are guaranteed to be present.
References
- Within this project's parameter parsing framework, direct type assertions on parameter values are acceptable without a comma-ok check. The framework's typed constructors already guarantee type safety, and this is an established pattern for consistency.
| body := v4.WriteAgent{} | ||
| if name != "" { | ||
| body.Name = &name | ||
| } | ||
| if instructions != "" { | ||
| body.Context = &v4.Context{ | ||
| Instructions: &instructions, | ||
| } | ||
| } | ||
| if len(agentSources) > 0 { | ||
| body.Sources = &agentSources | ||
| } | ||
| if hasCodeInterpreter { | ||
| body.CodeInterpreter = &codeInterpreter | ||
| } |
There was a problem hiding this comment.
The logic for populating the v4.WriteAgent body is duplicated between the create and update cases. This should be refactored into a helper function to improve code clarity and maintainability. Note that while server lifecycle logic duplication is permitted in this repo, business logic like request body construction should remain DRY.
Description
This PR significantly enhances the
looker-agenttool by adding missing parameters required for effective agent management.In the initial implementation, only
nameanddescription(internal) were supported. However, crucial fields for Looker AI Agents such asinstructions(system prompt),sources(data models/explores), andcode_interpreterwere missing. This update fills those gaps and also introduces an update operation.Changes
instructions,sources(JSON-encoded array), andcode_interpreter(boolean) to bothcreateandupdateoperations.updateoperation to allow modification of existing agents.- Updated error handling to use
http.StatusBadRequestfor client-side validation errors.- Added safety checks for parameter access to prevent runtime panics.
Impact
The
looker-agenttool is now fully capable of managing the complete lifecycle and configuration of Looker AI Agents, enabling sophisticated analytical workflows.Verification