-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix:Additional plugins for evals #4236
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -455,3 +455,47 @@ async def mock_generate_inferences_side_effect( | |
| mock_generate_inferences.assert_called_once() | ||
| called_with_content = mock_generate_inferences.call_args.args[3] | ||
| assert called_with_content.parts[0].text == "message 1" | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_generates_inferences_with_custom_plugins( | ||
| self, mocker, mock_runner, mock_session_service | ||
| ): | ||
| """Tests that custom plugins are merged with built-in plugins.""" | ||
| from google.adk.plugins.base_plugin import BasePlugin | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's generally good practice to place all import statements at the top of the file for better readability and consistency, unless there's a specific reason (like avoiding circular imports or very heavy imports only needed in rare paths). Moving this import to the top of the file would align with standard Python style guidelines. |
||
|
|
||
| mock_agent = mocker.MagicMock() | ||
| mock_user_sim = mocker.MagicMock(spec=UserSimulator) | ||
|
|
||
| # Mock user simulator will stop immediately | ||
| async def get_next_user_message_side_effect(*args, **kwargs): | ||
| return NextUserMessage(status=UserSimulatorStatus.STOP_SIGNAL_DETECTED) | ||
|
|
||
| mock_user_sim.get_next_user_message = mocker.AsyncMock( | ||
| side_effect=get_next_user_message_side_effect | ||
| ) | ||
|
|
||
| # Create a custom plugin | ||
| custom_plugin = mocker.MagicMock(spec=BasePlugin) | ||
| custom_plugin.name = "custom_test_plugin" | ||
|
|
||
| await EvaluationGenerator._generate_inferences_from_root_agent( | ||
| root_agent=mock_agent, | ||
| user_simulator=mock_user_sim, | ||
| plugins=[custom_plugin], | ||
| ) | ||
|
|
||
| # Verify Runner was created with merged plugins | ||
| # Built-in plugins: _RequestIntercepterPlugin, EnsureRetryOptionsPlugin | ||
| # Custom plugins: custom_test_plugin | ||
| runner_call_args = mock_runner.call_args | ||
| assert runner_call_args is not None | ||
| plugins_passed = runner_call_args[1]["plugins"] | ||
| assert len(plugins_passed) == 3, ( | ||
| "Expected 3 plugins: 2 built-in + 1 custom" | ||
| ) | ||
|
|
||
| # Verify built-in plugins are present | ||
| plugin_names = [p.name for p in plugins_passed] | ||
| assert "request_intercepter_plugin" in plugin_names | ||
| assert "ensure_retry_options" in plugin_names | ||
| assert "custom_test_plugin" in plugin_names | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -256,6 +256,53 @@ async def test_perform_inference_with_case_ids( | |
| ) | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_perform_inference_with_custom_plugins( | ||
| eval_service, | ||
| dummy_agent, | ||
| mock_eval_sets_manager, | ||
| mocker, | ||
| ): | ||
| """Tests that custom plugins are passed through to EvaluationGenerator.""" | ||
| from google.adk.plugins.base_plugin import BasePlugin | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| eval_set = EvalSet( | ||
| eval_set_id="test_eval_set", | ||
| eval_cases=[ | ||
| EvalCase(eval_id="case1", conversation=[], session_input=None), | ||
| ], | ||
| ) | ||
| mock_eval_sets_manager.get_eval_set.return_value = eval_set | ||
|
|
||
| # Create a custom plugin | ||
| custom_plugin = mocker.MagicMock(spec=BasePlugin) | ||
| custom_plugin.name = "custom_test_plugin" | ||
|
|
||
| # Mock the EvaluationGenerator call to verify plugins are passed | ||
| mock_generate_inferences = mocker.patch( | ||
| "google.adk.evaluation.local_eval_service.EvaluationGenerator._generate_inferences_from_root_agent", | ||
| return_value=[], | ||
| ) | ||
|
|
||
| inference_request = InferenceRequest( | ||
| app_name="test_app", | ||
| eval_set_id="test_eval_set", | ||
| inference_config=InferenceConfig( | ||
| parallelism=1, plugins=[custom_plugin] | ||
| ), | ||
| ) | ||
|
|
||
| results = [] | ||
| async for result in eval_service.perform_inference(inference_request): | ||
| results.append(result) | ||
|
|
||
| # Verify that plugins were passed to EvaluationGenerator | ||
| mock_generate_inferences.assert_called_once() | ||
| call_kwargs = mock_generate_inferences.call_args.kwargs | ||
| assert "plugins" in call_kwargs | ||
| assert call_kwargs["plugins"] == [custom_plugin] | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_perform_inference_eval_set_not_found( | ||
| eval_service, | ||
|
|
||
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
pluginsfield inInferenceConfigis currently typed aslist[Any]. WhileBasePluginis conditionally imported forTYPE_CHECKING, this creates a type inconsistency with downstream functions (e.g., inevaluation_generator.py) that expectlist[BasePlugin]. For better type safety and consistency, consider importingBasePlugindirectly (not underTYPE_CHECKING) and typing this field aslist[BasePlugin]. This would allow Pydantic to perform runtime validation and ensure type alignment throughout the codebase.