Skip to content

Merge Gemini tool mode into caller-supplied ToolConfig instead of replacing it - #738

Open
PratikDhanave wants to merge 2 commits into
microsoft:mainfrom
PratikDhanaveFork:gemini-merge-toolconfig
Open

Merge Gemini tool mode into caller-supplied ToolConfig instead of replacing it#738
PratikDhanave wants to merge 2 commits into
microsoft:mainfrom
PratikDhanaveFork:gemini-merge-toolconfig

Conversation

@PratikDhanave

Copy link
Copy Markdown
Contributor

What

When a tool mode is set, buildParams in provider/geminiprovider/agent.go assigned a brand-new genai.ToolConfig holding only the FunctionCallingConfig, replacing any ToolConfig the caller had already supplied through the GenerateContentConfig escape hatch. genai.ToolConfig also carries RetrievalConfig (Vertex grounding) and IncludeServerSideToolInvocations; those were silently dropped.

This mutates the existing ToolConfig in place instead: it only overrides FunctionCallingConfig, preserving the other caller-set fields.

Why

GenerateContentConfig is documented as the pass-through for custom genai parameters, and the config is already copied wholesale into cfg via *cfg = p. Overriding a single sub-field rather than clobbering the whole pointer matches how the rest of buildParams merges options (e.g. it appends to cfg.Tools rather than replacing it). It also mirrors the .NET/Python providers, where tool-mode selection sets the function-calling mode without discarding other tool-config settings a caller supplied for grounding/retrieval.

Testing

Added TestToolModeMergesCallerToolConfig to the canonical agent_test.go. It drives a run with a caller ToolConfig{RetrievalConfig: ...}, a function tool, and WithToolMode(required), then inspects the outgoing request: functionCallingConfig.mode == ANY is applied and retrievalConfig.languageCode is preserved. The test fails before this change (retrievalConfig dropped) and passes after. go build ./..., go vet, and go test ./provider/geminiprovider/... are green.

Copilot AI review requested due to automatic review settings July 24, 2026 03:47
@PratikDhanave
PratikDhanave requested a review from a team as a code owner July 24, 2026 03:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes how the Gemini provider applies tool-mode selection so it no longer overwrites a caller-supplied genai.ToolConfig, preserving other ToolConfig fields (e.g., RetrievalConfig for Vertex grounding) while still enforcing the requested function-calling mode.

Changes:

  • Update buildParams to merge tool-mode settings into an existing ToolConfig instead of replacing the whole struct.
  • Add a regression test ensuring RetrievalConfig is preserved when WithToolMode(required) is applied.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
provider/geminiprovider/agent.go Merges tool-mode function-calling config into existing ToolConfig instead of clobbering it.
provider/geminiprovider/agent_test.go Adds a test to verify caller-supplied RetrievalConfig is preserved when tool mode is set.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +247 to +250
if cfg.ToolConfig == nil {
cfg.ToolConfig = &genai.ToolConfig{}
}
cfg.ToolConfig.FunctionCallingConfig = fc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 61f3eef: I now shallow-clone the caller-supplied ToolConfig struct before overriding FunctionCallingConfig, so we no longer mutate the caller's aliased pointer (from the shallow *cfg = p copy) while still preserving their other fields like RetrievalConfig.

@github-actions github-actions Bot added the parity-approved Go API consistency review found no parity issues label Jul 24, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

buildParams replaced cfg.ToolConfig wholesale when a tool mode was set,
dropping any RetrievalConfig or IncludeServerSideToolInvocations the
caller passed through GenerateContentConfig. Mutate the existing
ToolConfig so only FunctionCallingConfig is overridden.
@PratikDhanave
PratikDhanave force-pushed the gemini-merge-toolconfig branch from 61f3eef to d8c9bc9 Compare July 24, 2026 09:30
@github-actions

Copy link
Copy Markdown
Contributor

Parity Review: ✅ No Issues Found

This PR fixes a bug in the unexported buildParams method of the Gemini provider — no exported Go APIs are added or changed.

What changed: When a tool mode is set, the code now merges FunctionCallingConfig into any caller-supplied ToolConfig (shallow-cloning the struct first) instead of creating a brand-new ToolConfig that silently drops other fields like RetrievalConfig (Vertex grounding) and IncludeServerSideToolInvocations.

Cross-repo consistency: This change mirrors the behavior of the .NET and Python providers, where tool-mode selection sets the function-calling mode without discarding other tool-config settings a caller supplied for grounding/retrieval. The Go provider was diverging from upstream; this PR corrects that divergence.

Scope: Only unexported implementation code is touched (buildParams is unexported). No exported types, option structs, builder patterns, or user-visible behavior contracts are altered.

Conclusion: The parity-approved label already present is correct and should remain. No public-api-change label is warranted.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Go API Consistency Review Agent · 19.1 AIC · ⌖ 5.57 AIC · ⊞ 5.9K ·

@gdams
gdams added this pull request to the merge queue Jul 29, 2026
@gdams
gdams removed this pull request from the merge queue due to a manual request Jul 29, 2026
@gdams
gdams enabled auto-merge July 29, 2026 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parity-approved Go API consistency review found no parity issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants