[Mehedi Hasan (Mahdi-Hasan)]

Motivation & Context

Gemini chat and embedding constructors call load_settings() for the API key, Enterprise/Vertex mode, project, and location before using an injected genai.Client. None of these are needed for a caller-supplied client. An invalid ambient value still fails construction, e.g. GOOGLE_GENAI_USE_ENTERPRISE=not-a-bool raises ValueError even with a valid client.

Description & Review Guide

• What are the major changes?
  • New load_gemini_settings() in _sdk_client.py. With an injected client it loads a private model-only TypedDict (model, embedding_model). Otherwise it loads GoogleGeminiSettings as before.
  • RawGeminiChatClient and RawGeminiEmbeddingClient use it. load_settings skips override keys that aren't in the TypedDict, so the constructors pass the same arguments as before.
  • Docstrings for client now say which settings are not read.
• What is the impact of these changes?
  • With an injected client, mode and service URL come only from the SDK client. Explicit enterprise=/vertexai=/project=/location=/api_key= arguments are ignored on that path. Before, mode was only used as a fallback when the SDK client did not expose _api_client.vertexai.
  • Model resolution is unchanged: explicit model, then GOOGLE_MODEL / GOOGLE_EMBEDDING_MODEL, then the existing default or error.
  • Owned-client construction keeps its existing validation. A test covers this.
• What do you want reviewers to focus on?
  • Whether ignoring explicit auth/mode arguments on the injected path is acceptable, or whether you'd prefer they raise.

Related Issue

Fixes #9959

AI Assistance

• No material AI assistance was used.
• This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: implementation, tests, and this description. The issue's repro was re-run against a real genai.Client before and after the change.

Contribution Checklist

• The code builds clean without any errors or warnings
• All unit tests pass, and I have added new tests where possible
• The PR follows the Contribution Guidelines
• This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
• This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[Eduard van Valkenburg (eavanvalkenburg)]

/review

[github-actions]

MAF Automated Review — Iteration 1

Result: Findings reported
Scope: full PR (1 commit(s)): f6499d118251
Model: gpt-5.6-sol

Overview

The PR cleanly avoids parsing ambient connection settings when a caller supplies a Gemini SDK client, while retaining model resolution, owned-client validation, routing from exposed SDK state, and caller ownership. The new tests cover invalid ambient mode values and SDK clients whose private mode field is available. One compatibility gap remains: explicit mode no longer reaches the existing fallback when that private SDK field is absent, which can misclassify a Vertex client and alter request behavior.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: python/packages/gemini/agent_framework_gemini/_sdk_client.py

[github-actions]

When an injected client does not expose a boolean _api_client.vertexai, this model-only schema drops explicit enterprise/vertexai values before resolve_vertexai_mode() can use its existing fallback, so a Vertex client is classified as Developer API. That changes Vertex embedding batching and can send an unsupported multi-content request. Please keep skipping ambient connection settings, but preserve an explicitly supplied mode as the fallback when SDK state is unavailable.

[Mehedi Hasan (Mahdi-Hasan)]

Fixed in d37fe55. An injected client still skips the GOOGLE_* connection settings, but explicit enterprise/vertexai arguments are kept as the fallback. New tests cover an SDK client with no boolean _api_client.vertexai for chat and embeddings; they fail without the fix.

[Copilot]

🟡 Changes recommended

Explicit enterprise/vertexai overrides are not validated for injected clients, which can accept values like "false" and select the wrong service URL, while two messages also inaccurately describe model environment fallback.

2 open issues

• Validate explicit enterprise and vertexai overrides as booleans · New
• Clarify embedding model environment fallback for injected clients · New

_{Review effort: Lite}

[Mehedi Hasan (Mahdi-Hasan)]

Fixed in fd0c7a3. Explicit enterprise/vertexai values for an injected client now get the same bool check (and error message) an owned client gets from load_settings. Environment mode is still not read. Added tests for chat and embeddings that pass "false".

[Mehedi Hasan (Mahdi-Hasan)]

Fixed in fd0c7a3. The chat and embedding docstrings now say only connection settings are skipped, and that GOOGLE_MODEL / GOOGLE_EMBEDDING_MODEL still apply.