docs(configuration): document parameter required/default semantics and the default: null trap - #3823
Open
AndyFooBlah wants to merge 1 commit into
Open
Conversation
Parameters are required by default, and providing a default silently makes one optional, but the interaction between the two fields was only implied by two table cells. Adds an "Optional Parameters" section with the full matrix and the two ways to make a parameter optional. Also documents that default: null does not work. YAML parses it (and the equivalent default: ~ or a bare default:) to a null value, which is indistinguishable from the field being absent, so the parameter stays required and omitted calls fail at invocation time. required: false is the working spelling. An explicit default: "" is contrasted as a real default, since only null is ignored. Behavior verified against CheckParamRequired and ParseParams in internal/util/parameters/parameters.go, exercised through the YAML parse path for every row of the matrix and all three null spellings. Assisted by Claude. Part of googleapis#3639
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the documentation in docs/en/documentation/configuration/tools/_index.md to add a new section on 'Optional Parameters'. It explains the default behavior of parameters, the two ways to make them optional (using default or required: false), and warns that default: null does not make a parameter optional. There are no review comments, so no feedback is provided.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Documents how
requiredanddefaultinteract on tool parameters, and thedefault: nulltrap.Parameters are required by default, and supplying a
defaultsilently makes one optional — the existing table implies both in single cells (Default to true,If provided, required will be false), but not the interaction, and not the failure mode. Adds an Optional Parameters section under Specifying Parameters with:default: <value>vsrequired: false) and how they differ for an omitted argument;required×defaultmatrix;default: nulldoes not make a parameter optional.On that last point: YAML parses
default: null— and equallydefault: ~or a baredefault:— to a null value, whichCheckParamRequiredcannot distinguish from the field being absent:So the parameter stays required, and an agent call that omits the argument fails at invocation time with
parameter "..." is required. Writingdefault: nullto mean "optional, no value when omitted" is a natural reading that produces a runtime error instead;required: falseis the working spelling. The section also contrastsdefault: "", which is a real default — only null is ignored.Docs only; no behavior change.
Verification
Every row of the matrix was exercised through the real YAML parse path (
yaml.UnmarshalContextintoparameters.Parameters, thenManifest()andParseParamswith the argument omitted) rather than read off the source, including all three null spellings:manifest.Requiredrequiredomitted, nodefaulttrueparameter "p" is requiredrequired: true, nodefaulttrueparameter "p" is requiredrequired: false, nodefaultfalsenilrequired: true+default: fallbackfalserequiredomitted +default: fallbackfalserequiredomitted +default: ""false""required: true+default: nulltrueparameter "p" is requiredrequiredomitted +default: nulltrueparameter "p" is requiredrequiredomitted +default: ~trueparameter "p" is requiredrequiredomitted + baredefault:trueparameter "p" is requiredThe
{{< notice warning >}}shortcode used here is already used elsewhere in the same page.PR Checklist
!if this involves a breaking changeIssue Reference
Part of #3639 🦕 — this is gap (2) of the three in that issue, which @anubhav756 and @averikitsch both green-lit as a self-contained first step. Gap (1) is with @wangauone for
cloud-gemini-data-analytics-query, and gap (3) was answered as not feasible, so I've left the issue open rather than auto-closing it.