Skip to content
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/google/adk/evaluation/evaluation_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
from .eval_case import SessionInput
from .eval_set import EvalSet
from .request_intercepter_plugin import _RequestIntercepterPlugin
from .simulation.user_simulator import BaseUserSimulatorConfig
from .simulation.user_simulator import Status as UserSimulatorStatus
from .simulation.user_simulator import UserSimulator
from .simulation.user_simulator_provider import UserSimulatorProvider
Expand Down Expand Up @@ -263,6 +264,7 @@ async def generate_responses(
agent_module_path: str,
repeat_num: int = 3,
agent_name: str = None,
user_simulator_config: Optional[BaseUserSimulatorConfig] = None,
) -> list[EvalCaseResponses]:
"""Returns evaluation responses for the given dataset and agent.

Expand All @@ -273,12 +275,19 @@ async def generate_responses(
usually done to remove uncertainty that a single run may bring.
agent_name: The name of the agent that should be evaluated. This is
usually the sub-agent.
user_simulator_config: Optional configuration for the user simulator.
Only relevant for eval cases that use a `conversation_scenario` (which
are driven by `LlmBackedUserSimulator`); ignored for static
conversations. Pass an `LlmBackedUserSimulatorConfig` to override the
user-simulation model, max invocations, or custom instructions.
"""
results = []

for eval_case in eval_set.eval_cases:
# assume only static conversations are needed
user_simulator = UserSimulatorProvider().provide(eval_case)
user_simulator = UserSimulatorProvider(
user_simulator_config=user_simulator_config
).provide(eval_case)

responses = []
for _ in range(repeat_num):
response_invocations = await EvaluationGenerator._process_query(
Expand Down
45 changes: 45 additions & 0 deletions tests/unittests/evaluation/test_evaluation_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@

from google.adk.evaluation.app_details import AgentDetails
from google.adk.evaluation.app_details import AppDetails
from google.adk.evaluation.eval_case import EvalCase
from google.adk.evaluation.eval_set import EvalSet
from google.adk.evaluation.evaluation_generator import _LiveSession
from google.adk.evaluation.evaluation_generator import EvaluationGenerator
from google.adk.evaluation.request_intercepter_plugin import _RequestIntercepterPlugin
from google.adk.evaluation.simulation.llm_backed_user_simulator import LlmBackedUserSimulatorConfig
from google.adk.evaluation.simulation.user_simulator import NextUserMessage
from google.adk.evaluation.simulation.user_simulator import Status as UserSimulatorStatus
from google.adk.evaluation.simulation.user_simulator import UserSimulator
Expand Down Expand Up @@ -594,6 +597,48 @@ async def mock_generate_inferences_side_effect(
called_with_content = mock_generate_inferences.call_args.args[3]
assert called_with_content.parts[0].text == "message 1"


class TestGenerateResponses:
"""Test cases for EvaluationGenerator.generate_responses method."""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We don't need this new class. The newly added test case can be a part of the existing class TestGenerateInferencesFromRootAgent

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.

Thank you for the feedback @ankursharmas !

The newly added test exercises generate_responses rather than _generate_inferences_from_root_agent. It seems to me that test_evaluation_generator.py mostly follows a one-class-per-method convention, where each test class targets a single method. For example:

Test class Method under test
TestConvertEventsToEvalInvocation convert_events_to_eval_invocations
TestGetAppDetailsByInvocationId _get_app_details_by_invocation_id
TestGenerateInferencesForSingleUserInvocation _generate_inferences_for_single_user_invocation
TestGenerateInferencesForSingleUserInvocationLive _generate_inferences_for_single_user_invocation_live

Following that, wouldn't a separate TestGenerateResponses class for generate_responses fit better here?

While looking at this I also noticed that test_generates_inferences_with_user_simulator_live lives in TestGenerateInferencesFromRootAgent even though it tests _generate_inferences_from_root_agent_live, so the convention is already a bit mixed there.

On a separate note: there was a careless merge on my side which I have now fixed.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah makes sense! Thank you for pointing that out.

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.

@ankursharmas do you consider the conversation resolved?

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.

@ankursharmas this PR is currently blocked by this open thread. Let me know if you need any further changes from my side; otherwise, could you please resolve the conversation?


@pytest.mark.asyncio
async def test_generate_responses_forwards_llm_backed_user_simulator_config(
self, mocker
):
"""Tests that an LlmBackedUserSimulatorConfig is forwarded to the provider verbatim."""
mock_provider_cls = mocker.patch(
"google.adk.evaluation.evaluation_generator.UserSimulatorProvider"
)
mocker.patch(
"google.adk.evaluation.evaluation_generator.EvaluationGenerator._process_query",
new_callable=mocker.AsyncMock,
return_value=[],
)

user_simulator_config = LlmBackedUserSimulatorConfig(
model="test-model",
max_allowed_invocations=5,
)
eval_set = EvalSet(
eval_set_id="test_set",
eval_cases=[EvalCase(eval_id="case_0", conversation=[])],
)

await EvaluationGenerator.generate_responses(
eval_set=eval_set,
agent_module_path="some.agent.module",
repeat_num=1,
user_simulator_config=user_simulator_config,
)

mock_provider_cls.assert_called_once_with(
user_simulator_config=user_simulator_config
)
assert (
mock_provider_cls.call_args.kwargs["user_simulator_config"]
is user_simulator_config
)

@pytest.mark.asyncio
async def test_generates_inferences_with_user_simulator_live(
self, mocker, mock_runner, mock_session_service
Expand Down
Loading