From af862ad0d7f3dcf968a01ef20eb28d1371389955 Mon Sep 17 00:00:00 2001 From: Peter Tomko Date: Thu, 22 Oct 2026 10:36:28 +0200 Subject: [PATCH 1/2] fix(eval): agentic search_tool scored tool_correctness ~1% against valid calls The agentic and single-shot search_tool scorers disagree, and the agentic one -- which is what `--kind search_tool` dispatches to -- is the stale one. `core/evaluators/search_tool.py` already documents and applies the exclusion: # limit is optional with a server-side default; emit_widget was renamed to # user_requested_search in the tool schema -- neither affects search quality. `core/agentic/search_tool.py` never got it. It compares every key the fixture names, so a fixture written against the older tool signature fails on `limit` alone (fixtures say 10, the agent sends the server default 20). The second cause is list matching. Expected elements were compared with exact string equality, so a fixture that tokenizes -- ["abandonment", "rate", "3ds"] -- never matches an agent that searches the object's title as one phrase, ["Abandonment Rate - 3DS"]. Both name the same object. Measured over 143 recorded search_objects calls from a live corpus (12 distinct questions, October 2026): tool_correctness was true in 3 of 244 evaluations, ~1%. Every one of those 12 questions failed on `keywords` + `limit`, and 11 of 12 failed on nothing else. This changes two things in `_tool_correctness`: - ignore `limit`, `emit_widget` and `user_requested_search`, matching the single-shot evaluator's existing rationale - match list elements by case-insensitive substring in either direction rather than exact equality, so differing tokenization of the same object name passes Replayed against those same 143 calls, 11 of 12 questions now score correct on every run. The twelfth scores 1 of 7 and should: it phrases its search differently on every run, which is real instability rather than a scorer artifact. Nothing in the suite surfaced this, because an item passes on `tool_selection` alone -- pass@k stayed at 91-100% while `tool_correctness` sat at 1% and the derived quality_score was pinned at exactly 0.5 for every search_tool item. Co-Authored-By: Claude Opus 5 An empty search term matches nothing: "" is a substring of every string, so _loosely_contains would otherwise accept an empty agent keyword against any expected keyword, and the reverse. Co-Authored-By: Claude Sonnet 5.5 --- .../gooddata_eval/core/agentic/search_tool.py | 39 +++++++++++++-- .../tests/test_agentic_search_tool.py | 47 +++++++++++++++++++ 2 files changed, 82 insertions(+), 4 deletions(-) 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..1472d31ad 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,53 @@ 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) -> bool: + """Every expected element must appear in some actual element, and vice versa is not required. + + Substring rather than equality, because the two sides legitimately tokenize + differently: 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. + """ + return all( + any(isinstance(a, str) and isinstance(e, str) and _loosely_contains(e, a) for a 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 (e.g. keywords, object_types) use subset matching: every expected value + must appear in the actual call, by substring, and the agent may include extras. """ 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): 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..1a7b2aef0 100644 --- a/packages/gooddata-eval/tests/test_agentic_search_tool.py +++ b/packages/gooddata-eval/tests/test_agentic_search_tool.py @@ -250,3 +250,50 @@ 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 From 4358c43d5cef5b1fd4c89ddf774c9aba6ecb4b18 Mon Sep 17 00:00:00 2001 From: Peter Tomko Date: Thu, 22 Oct 2026 11:27:54 +0200 Subject: [PATCH 2/2] fix(eval): compare search_tool object_types exactly object_types holds controlled values, so the loose case-insensitive substring match that tokenized keywords need would accept "METRIC" for "metric". The single-shot evaluator already compares them case-sensitively; keywords stay loose, every other list is exact. Co-Authored-By: Claude Sonnet 5.5 --- .../gooddata_eval/core/agentic/search_tool.py | 27 +++++++++++-------- .../tests/test_agentic_search_tool.py | 7 +++++ 2 files changed, 23 insertions(+), 11 deletions(-) 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 1472d31ad..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 @@ -59,24 +59,29 @@ def _loosely_contains(expected: str, actual: str) -> bool: return expected.lower() in actual.lower() or actual.lower() in expected.lower() -def _list_matches(expected: list, actual: list) -> bool: +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. - Substring rather than equality, because the two sides legitimately tokenize - differently: 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. + ``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". """ - return all( - any(isinstance(a, str) and isinstance(e, str) and _loosely_contains(e, a) for a in actual) for e in expected - ) + 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: every expected value - must appear in the actual call, by substring, and 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": @@ -86,7 +91,7 @@ def _tool_correctness(tool_call_events: list[ToolCallEvent], expected_tool_call: continue act_val = args.get(key) if isinstance(exp_val, list) and isinstance(act_val, list): - if not _list_matches(exp_val, 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 not _loosely_contains(exp_val, act_val): diff --git a/packages/gooddata-eval/tests/test_agentic_search_tool.py b/packages/gooddata-eval/tests/test_agentic_search_tool.py index 1a7b2aef0..916877bdb 100644 --- a/packages/gooddata-eval/tests/test_agentic_search_tool.py +++ b/packages/gooddata-eval/tests/test_agentic_search_tool.py @@ -297,3 +297,10 @@ def test_tool_correctness_rejects_an_empty_search_term(): 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