-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix runconfig options endpoint #4264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix runconfig options endpoint #4264
Conversation
…ptions to /run_live endpoint Fixes google#4263 The /run_live WebSocket endpoint now accepts three new query parameters: - proactive_audio (bool, default=False): Enables Gemini Live's proactive audio feature - affective_dialog (bool, default=False): Enables emotional awareness in the model - session_resumption (bool, default=False): Enables session resumption mechanism These options are passed to the RunConfig and used when calling runner.run_live(). Co-authored-by: bashimr <1313979+bashimr@users.noreply.github.com>
Use `affective_dialog or None` for cleaner code instead of redundant conditional. Co-authored-by: bashimr <1313979+bashimr@users.noreply.github.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @bashimr, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Response from ADK Triaging Agent Hello @bashimr, thank you for creating this PR! I see that the CLA check has failed. Please make sure to sign the Contributor License Agreement (CLA) as described in our contribution guidelines. Also, could you please provide the logs or screenshots from your manual end-to-end tests? This will help the reviewers to better understand and verify your changes. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request exposes new configuration options on the /run_live WebSocket endpoint, allowing for more control over Gemini Live features like proactive audio and affective dialog. The implementation is straightforward, adding new query parameters and using them to construct the RunConfig. My review includes a suggestion to improve the clarity and correctness of handling one of the new boolean flags and a recommendation to strengthen the unit tests to cover the new logic.
| if proactive_audio | ||
| else None | ||
| ), | ||
| enable_affective_dialog=affective_dialog or None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expression affective_dialog or None is a bit ambiguous and could be clearer. It maps a False value from the query parameter to None for the RunConfig. If False and None have different meanings for enable_affective_dialog (e.g., False explicitly disables it, while None uses a default behavior), this could be a subtle bug.
Given the PR description states "All parameters default to False for backward compatibility", it seems more direct and readable to pass the boolean value as-is. This would pass False when the query parameter is false or omitted, which aligns better with the stated goal.
| enable_affective_dialog=affective_dialog or None, | |
| enable_affective_dialog=affective_dialog, |
| def test_run_live_accepts_new_config_options(test_app, test_session_info): | ||
| """Test that /run_live endpoint accepts proactive_audio, affective_dialog, and session_resumption query parameters.""" | ||
| # First, create a session to use | ||
| session_url = ( | ||
| f"/apps/{test_session_info['app_name']}/users/" | ||
| f"{test_session_info['user_id']}/sessions/{test_session_info['session_id']}" | ||
| ) | ||
| test_app.post(session_url, json={"state": {}}) | ||
|
|
||
| # The actual websocket test would require more elaborate setup. | ||
| # Here we verify that the endpoint accepts the new query parameters | ||
| # by checking FastAPI's route signature. | ||
| from fastapi import Query as FastApiQuery | ||
|
|
||
| # Find the /run_live route | ||
| run_live_route = None | ||
| for route in test_app.app.routes: | ||
| if hasattr(route, "path") and route.path == "/run_live": | ||
| run_live_route = route | ||
| break | ||
|
|
||
| assert run_live_route is not None, "/run_live route not found" | ||
|
|
||
| # Check that the endpoint function signature includes the new parameters | ||
| import inspect | ||
|
|
||
| endpoint_fn = run_live_route.endpoint | ||
| sig = inspect.signature(endpoint_fn) | ||
| params = sig.parameters | ||
|
|
||
| # Verify the new parameters exist | ||
| assert "proactive_audio" in params, "proactive_audio parameter not found" | ||
| assert "affective_dialog" in params, "affective_dialog parameter not found" | ||
| assert ( | ||
| "session_resumption" in params | ||
| ), "session_resumption parameter not found" | ||
|
|
||
| # Verify they have default values of False | ||
| assert params["proactive_audio"].default.default is False | ||
| assert params["affective_dialog"].default.default is False | ||
| assert params["session_resumption"].default.default is False | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is a good start as it verifies that the new query parameters are present in the endpoint's signature with the correct defaults. However, it doesn't test the core logic of this change, which is the correct creation of the RunConfig object based on these parameters.
I recommend adding a test that verifies the RunConfig is constructed as expected. You can achieve this by mocking runner.run_live and using the TestClient's websocket_connect to trigger the endpoint, then asserting on the run_config argument passed to the mock. This would provide much stronger guarantees that the feature works as intended.
Here is an example of what such a test could look like:
from unittest.mock import patch, MagicMock
from google.adk.agents.run_config import RunConfig
from google.genai import types
import asyncio
@patch("google.adk.cli.adk_web_server.AdkWebServer.get_runner_async")
def test_run_live_creates_correct_run_config(
mock_get_runner, test_app, test_session_info
):
"""Test that /run_live creates RunConfig with correct options."""
# Setup session
session_url = (
f"/apps/{test_session_info['app_name']}/users/"
f"{test_session_info['user_id']}/sessions/{test_session_info['session_id']}"
)
test_app.post(session_url, json={"state": {}})
# Mock the runner and its run_live method
mock_runner = MagicMock()
async def empty_agen(*args, **kwargs):
if False: yield
mock_runner.run_live.return_value = empty_agen()
mock_get_runner.return_value = mock_runner
# Test with all options enabled
with test_app.websocket_connect(
f"/run_live?app_name={test_session_info['app_name']}"
f"&user_id={test_session_info['user_id']}"
f"&session_id={test_session_info['session_id']}"
"&proactive_audio=true&affective_dialog=true&session_resumption=true"
) as websocket:
pass # Just need to establish connection to trigger the endpoint logic
mock_get_runner.assert_called_with(test_session_info["app_name"])
mock_runner.run_live.assert_called_once()
# Check the run_config passed to run_live
call_args, call_kwargs = mock_runner.run_live.call_args
run_config = call_kwargs.get("run_config")
assert isinstance(run_config, RunConfig)
assert isinstance(run_config.proactivity, types.ProactivityConfig)
assert run_config.proactivity.proactive_audio is True
assert run_config.enable_affective_dialog is True
assert isinstance(run_config.session_resumption, types.SessionResumptionConfig)
# Reset mock and test default values
mock_runner.reset_mock()
with test_app.websocket_connect(
f"/run_live?app_name={test_session_info['app_name']}"
f"&user_id={test_session_info['user_id']}"
f"&session_id={test_session_info['session_id']}"
) as websocket:
pass
mock_runner.run_live.assert_called_once()
call_args, call_kwargs = mock_runner.run_live.call_args
run_config = call_kwargs.get("run_config")
assert run_config.proactivity is None
assert run_config.enable_affective_dialog is None
assert run_config.session_resumption is None
Closes: #4263
2. Or, if no issue exists, describe the change:
Problem: The /run_live WebSocket endpoint hardcodes a minimal RunConfig with only response_modalities, preventing developers from using Gemini Live's proactive audio, affective dialog, and session resumption features through the standard ADK endpoint.
Solution: Expose three new optional query parameters that map to existing RunConfig options:
proactive_audio (bool, default=False) → Creates ProactivityConfig(proactive_audio=True)
affective_dialog (bool, default=False) → Sets enable_affective_dialog=True
session_resumption (bool, default=False) → Creates SessionResumptionConfig()
All parameters default to False for backward compatibility.
Testing Plan
Unit Tests:
I have added or updated unit tests for my change.
All unit tests pass locally.
Code
26 passed, 7 deselected (optional deps: a2a, rouge_score)
Manual End-to-End (E2E) Tests:
Connect to /run_live with new parameters:
Checklist
I have read the CONTRIBUTING.md document.
I have performed a self-review of my own code.
I have commented my code, particularly in hard-to-understand areas.
I have added tests that prove my fix is effective or that my feature works.
New and existing unit tests pass locally with my changes.
I have manually tested my changes end-to-end.
Any dependent changes have been merged and published in downstream modules.
Additional context
The infrastructure for these RunConfig options already exists in ADK - they are correctly passed to the LLM in basic.py. This change simply exposes them through the /run_live endpoint query parameters.