Skip to content

fix(live): forward thinking config#5905

Open
he-yufeng wants to merge 2 commits into
google:mainfrom
he-yufeng:fix/live-thinking-config-forwarding
Open

fix(live): forward thinking config#5905
he-yufeng wants to merge 2 commits into
google:mainfrom
he-yufeng:fix/live-thinking-config-forwarding

Conversation

@he-yufeng
Copy link
Copy Markdown
Contributor

Summary

  • forward GenerateContentConfig.thinking_config into LiveConnectConfig before opening Gemini live sessions
  • keep existing live_connect_config.thinking_config untouched when the request config has no thinking config
  • add a regression test for Gemini.connect()

Fixes #5805.

To verify

  • $env:PYTHONPATH=(Resolve-Path src).Path; python -m pytest tests\unittests\models\test_google_llm.py -q
  • $env:PYTHONPATH=(Resolve-Path src).Path; python -m py_compile src\google\adk\models\google_llm.py tests\unittests\models\test_google_llm.py
  • python -m pyink --check src\google\adk\models\google_llm.py tests\unittests\models\test_google_llm.py
  • git diff --check

Note: pylint on the full google_llm.py module still reports pre-existing warnings unrelated to this change, so I did not treat that as a blocker.

@adk-bot adk-bot added the live [Component] This issue is related to live, voice and video chat label May 30, 2026
@adk-bot
Copy link
Copy Markdown
Collaborator

adk-bot commented May 30, 2026

Response from ADK Triaging Agent

Hello @he-yufeng, thank you for creating this PR!

This PR looks great and has a clear explanation and associated issue. To help the reviewers merge your PR more efficiently, could you please provide the following information as per our contribution guidelines:

  • Summary of passed pytest results: Please run the tests locally and paste a summary of the successful test runs (e.g., pytest tests/unittests/models/test_google_llm.py).
  • Logs or screenshot demonstrating the fix: Since this is a bug fix for live session connection configs, providing logs showing the successful session establishment with the forwarded thinking_config would be very helpful.

Thank you for your contribution!

@he-yufeng he-yufeng force-pushed the fix/live-thinking-config-forwarding branch from c295c81 to 77138dd Compare June 4, 2026 04:50
@he-yufeng
Copy link
Copy Markdown
Contributor Author

Rebased this onto current main and reran the focused model tests locally.

Pytest summary:

PYTHONUTF8=1 PYTHONPATH=src uv run pytest tests\unittests\models\test_google_llm.py -q --basetemp .tmp\pytest-5905-20260604c -p no:cacheprovider
# 62 passed, 7 warnings in 3.12s

I also ran the file-level checks:

uv run ruff check src\google\adk\models\google_llm.py tests\unittests\models\test_google_llm.py
uv run pyink --check src\google\adk\models\google_llm.py tests\unittests\models\test_google_llm.py
python -m py_compile src\google\adk\models\google_llm.py tests\unittests\models\test_google_llm.py
git diff --check

For the live-session evidence, I used a mocked live client so no real API key or network call was needed. The captured call into aio.live.connect showed the request-level thinking config being copied into the live config:

live.connect called: True
forwarded thinking_config: include_thoughts=None thinking_budget=128 thinking_level=None
same object: True

@rohityan rohityan self-assigned this Jun 5, 2026
@adk-bot
Copy link
Copy Markdown
Collaborator

adk-bot commented Jun 5, 2026

I will use the adk-pr-analyze skill to retrieve, inspect, and analyze Pull Request 5905. Let's first read the skill's instruction file.I will now run the PR/CLA verification script in read-only mode to check if the contributor of PR 5905 has signed the Google CLA.Since the user/environment denied permission for running terminal commands, I will use the custom python tool fetch_github_pr specifically designed to fetch details for PR 5905 directly.I will view the contents of the python script .agents/skills/adk-pr-analyze/scripts/triage_pr.py in read-only mode using the view_file tool to understand what checks it performs, so we can verify the CLA and pull request details ourselves.Since the command execution sandbox restricts running python scripts directly and only permits gh and git commands, I will use a clean gh command to retrieve the status check details for PR 5905 (specifically checking if the CLA verification passed).I will now use the gh command to retrieve the complete, line-by-line diff of the Pull Request 5905.I will fetch the details of the referenced Issue #5805 using fetch_github_issue to understand the root cause and why this fix was requested.I will view the contents of the local baseline file src/google/adk/models/google_llm.py around line 440 to 480 to verify the original code context and evaluate the patch.I will view the tests file tests/unittests/models/test_google_llm.py around lines 820-890 to inspect the test suite's alignment with writing standards.I will view the beginning of google_llm.py to verify that from __future__ import annotations and other coding standards/imports are correctly placed.I will view lines 410 to 445 of google_llm.py to see how other fields like system_instruction are forwarded to the live_connect_config.I will create a structured Pull Request Analysis Report and save it as an artifact in the workspace. This document provides a deep, comprehensive overview of the PR's value, justification, code formatting, style alignment, and testing quality.I have successfully analyzed PR 5905 and generated a detailed, premium analysis report!

📝 Summary of Analysis

  • Google CLA Status: ✅ Checked and verified. The cla/google status check passed successfully (SUCCESS), meaning the contributor is fully cleared.
  • Core Issue Fixed: Fixes Issue #5805 where thinking_config on an Agent's content configuration was silently ignored in realtime live/streaming sessions.
  • Code Modification: Safely forwards the thinking_config inside google_llm.py under Gemini.connect().
  • Test Suite Quality: Adds test_connect_forwards_thinking_config in test_google_llm.py conforming to AAA testing standards with inline mock locality. It also performs clean-up on neighboring tests to eliminate unused imports and unused variables.

The full, structured report has been saved as an artifact at:
👉 pr_5905_analysis.md

Recommendation: Approve — This is a well-designed, extremely clean, non-intrusive, and robust fix that solves the target issue perfectly.

@rohityan
Copy link
Copy Markdown
Collaborator

rohityan commented Jun 5, 2026

Hi @he-yufeng , Thank you for your contribution! We appreciate you taking the time to submit this pull request.

@rohityan rohityan added request clarification [Status] The maintainer need clarification or more information from the author needs review [Status] The PR/issue is awaiting review from the maintainer and removed request clarification [Status] The maintainer need clarification or more information from the author labels Jun 5, 2026
@rohityan rohityan requested a review from GWeale June 5, 2026 22:06
@rohityan
Copy link
Copy Markdown
Collaborator

rohityan commented Jun 5, 2026

Hi @GWeale , can you please review this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

live [Component] This issue is related to live, voice and video chat needs review [Status] The PR/issue is awaiting review from the maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

thinking_config silently ignored in Runner.run_live() for streaming models

3 participants