diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/search_tool.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/search_tool.py index 1d19c2e89..90dc591cf 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/search_tool.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/search_tool.py @@ -43,22 +43,58 @@ def _tool_selection(tool_call_events: list[ToolCallEvent]) -> bool: return any(tc.function_name == "search_objects" for tc in tool_call_events) +# Arguments that do not determine search quality, so a fixture naming them must not +# fail the call. `limit` has a server-side default the agent is free to choose; +# `emit_widget` was renamed to `user_requested_search` in the tool schema. The +# single-shot evaluator (core/evaluators/search_tool.py) has ignored these since the +# rename -- this path did not, so every fixture written against the older signature +# scored tool_correctness=False while still passing on tool_selection alone. +_IGNORED_SEARCH_ARGS = frozenset({"limit", "emit_widget", "user_requested_search"}) + + +def _loosely_contains(expected: str, actual: str) -> bool: + """Case-insensitive containment in either direction; an empty string matches nothing.""" + if not expected.strip() or not actual.strip(): + return False + return expected.lower() in actual.lower() or actual.lower() in expected.lower() + + +def _list_matches(expected: list, actual: list, *, loose: bool) -> bool: + """Every expected element must appear in some actual element, and vice versa is not required. + + ``loose`` is substring rather than equality, because free text legitimately tokenizes + differently on the two sides: a fixture says ["abandonment", "rate", "3ds"] while the agent + searches the object's title as one phrase, ["Abandonment Rate - 3DS"]. Both name the same + object, and exact element equality calls that a failure. A list of controlled values + (``object_types``) is not free text, so it is compared exactly and case-sensitively, as the + single-shot evaluator does: "METRIC" is not "metric". + """ + if loose: + return all( + any(isinstance(a, str) and isinstance(e, str) and _loosely_contains(e, a) for a in actual) for e in expected + ) + return all(e in actual for e in expected) + + def _tool_correctness(tool_call_events: list[ToolCallEvent], expected_tool_call: dict) -> bool: """Return True if the search_objects call arguments match expected. - List fields (e.g. keywords, object_types) use subset matching: all expected - values must appear in the actual call, but the agent may include extras. + List fields (keywords, object_types) use subset matching: every expected value must + appear in the actual call and the agent may include extras. ``keywords`` match by + substring; every other list is compared exactly. """ for tc in tool_call_events: if tc.function_name == "search_objects": args = tc.parsed_arguments() or {} for key, exp_val in expected_tool_call.items(): + if key in _IGNORED_SEARCH_ARGS: + continue act_val = args.get(key) if isinstance(exp_val, list) and isinstance(act_val, list): - if not set(exp_val).issubset(set(act_val)): + if not _list_matches(exp_val, act_val, loose=key == "keywords"): return False elif isinstance(exp_val, str) and isinstance(act_val, str): - if exp_val.lower() not in act_val.lower() and act_val.lower() not in exp_val.lower(): + if not _loosely_contains(exp_val, act_val): return False elif exp_val != act_val: return False diff --git a/packages/gooddata-eval/tests/test_agentic_search_tool.py b/packages/gooddata-eval/tests/test_agentic_search_tool.py index e188e7b99..916877bdb 100644 --- a/packages/gooddata-eval/tests/test_agentic_search_tool.py +++ b/packages/gooddata-eval/tests/test_agentic_search_tool.py @@ -250,3 +250,57 @@ def test_evaluate_agentic_search_tool_attaches_reasoning_steps_to_exception_on_f "latency_breakdown": [], } assert len(exc_info.value.run_latencies_s) == exc_info.value.runs_effective == 1 + + +def test_tool_correctness_tokenized_keywords_match_a_title_phrase(): + """A fixture tokenizes; the agent searches the object's title as one phrase.""" + tcs = [_mock_tc("search_objects", {"keywords": ["Abandonment Rate - 3DS"], "object_types": ["metric"]})] + assert _tool_correctness(tcs, {"keywords": ["abandonment", "rate", "3ds"]}) is True + + +def test_tool_correctness_still_rejects_an_unrelated_keyword(): + tcs = [_mock_tc("search_objects", {"keywords": ["Abandonment Rate - 3DS"]})] + assert _tool_correctness(tcs, {"keywords": ["interchange"]}) is False + + +def test_tool_correctness_ignores_limit(): + """`limit` has a server-side default; the single-shot evaluator already ignores it.""" + tcs = [_mock_tc("search_objects", {"keywords": ["revenue"], "limit": 20})] + assert _tool_correctness(tcs, {"keywords": ["revenue"], "limit": 10}) is True + + +def test_tool_correctness_ignores_the_renamed_widget_argument(): + tcs = [_mock_tc("search_objects", {"keywords": ["revenue"], "user_requested_search": True})] + assert _tool_correctness(tcs, {"keywords": ["revenue"], "emit_widget": False}) is True + + +def test_tool_correctness_allows_extra_object_types(): + """Searching more broadly than the fixture asked is not a failure.""" + tcs = [_mock_tc("search_objects", {"keywords": ["revenue"], "object_types": ["metric", "fact", "attribute"]})] + assert _tool_correctness(tcs, {"keywords": ["revenue"], "object_types": ["metric"]}) is True + + +def test_tool_correctness_rejects_a_missing_object_type(): + tcs = [_mock_tc("search_objects", {"keywords": ["revenue"], "object_types": ["metric"]})] + assert _tool_correctness(tcs, {"keywords": ["revenue"], "object_types": ["dashboard"]}) is False + + +def test_tool_correctness_ignores_arguments_the_agent_adds(): + """compatible_with_afm and kda_eligible appeared after these fixtures were written.""" + tcs = [_mock_tc("search_objects", {"keywords": ["revenue"], "compatible_with_afm": "x", "kda_eligible": True})] + assert _tool_correctness(tcs, {"keywords": ["revenue"]}) is True + + +def test_tool_correctness_rejects_an_empty_search_term(): + """ "" is a substring of everything, so it must not satisfy an expected keyword.""" + tcs = [_mock_tc("search_objects", {"keywords": [""]})] + assert _tool_correctness(tcs, {"keywords": ["revenue"]}) is False + tcs = [_mock_tc("search_objects", {"keywords": ["revenue"]})] + assert _tool_correctness(tcs, {"keywords": [""]}) is False + + +def test_tool_correctness_compares_object_types_case_sensitively(): + """object_types holds controlled values, so "METRIC" is not "metric" -- the single-shot + evaluator already says so; only free-text keywords match loosely.""" + tcs = [_mock_tc("search_objects", {"keywords": ["revenue"], "object_types": ["METRIC"]})] + assert _tool_correctness(tcs, {"keywords": ["revenue"], "object_types": ["metric"]}) is False