Skip to content

Commit e18efdc

Browse files
committed
fix(gooddata-eval): persist skill_routing credit across conversation turns
_activated_skills() only looked at the current turn's tool calls, so skill_routing was recomputed from scratch each turn. The platform keeps a skill active once set_skills is called, so an agent correctly omits a redundant set_skills call on a later turn that reuses the same skill -- but the scorer forced skill_routing=False whenever no skill was activated *this* turn, regardless of whether it was already active. Found via scripts/authoring/debug_conversation.py replaying analyst-explores-dynamic-currency-conversion (gdc-mic-ai-evaluation repo): turns t4/t5 both ran create_adhoc_visualization/create_metric successfully against an already-active skill, yet scored FAIL solely on this. Track activated skills in a running set across the whole conversation instead of resetting it every turn.
1 parent 8a7cf06 commit e18efdc

2 files changed

Lines changed: 64 additions & 1 deletion

File tree

packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,10 @@ def run_agentic_conversation(
300300
response_id: str | None = None
301301
conversation_tool_call_events: list[ToolCallEvent] = []
302302
conversation_reasoning_step_events: list[ReasoningStepEvent] = []
303+
# Skills activated by any turn so far. The platform keeps a skill active across
304+
# turns once set -- an agent correctly reuses the already-active skill without
305+
# re-issuing set_skills, so routing credit must not require a fresh call every turn.
306+
activated_skills_so_far: set[str] = set()
303307
# Every send_message() call (across every logical turn AND every clarification
304308
# sub-turn within it) restarts call_ts/ts near 0 -- these run across the whole
305309
# conversation, not reset per logical turn, so every one of those calls shifts them.
@@ -375,7 +379,8 @@ def run_agentic_conversation(
375379
current_message = _get_sim_user_response(response_text, resolved_turn, resolved_expected)
376380

377381
activated = _activated_skills(all_tool_calls)
378-
skill_routing = turn.expected_skill in activated if activated else False
382+
activated_skills_so_far |= set(activated)
383+
skill_routing = turn.expected_skill in activated_skills_so_far
379384
output_present = _check_output_present(resolved_turn, final_result) if final_result else False
380385
output_correct = (
381386
_check_output_correct(resolved_turn, final_result) if (final_result and output_present) else None

packages/gooddata-eval/tests/test_agentic_conversation.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,64 @@ def test_run_agentic_conversation_deletes_every_unique_metric_across_turns():
329329
assert deleted == [("ws1", "extra"), ("ws1", "shared")]
330330

331331

332+
def test_run_agentic_conversation_skill_routing_persists_across_turns():
333+
"""A skill activated in an earlier turn stays credited when a later turn reuses it
334+
without re-issuing set_skills -- the platform keeps a skill active once set, so an
335+
agent correctly omits a redundant set_skills call. Requiring a fresh call every turn
336+
produced false FAILs on turns that did the right thing (found via
337+
debug_conversation.py replaying analyst-explores-dynamic-currency-conversion,
338+
turns t4/t5: create_adhoc_visualization/create_metric both ran and succeeded, but
339+
skill_routing was False solely because set_skills wasn't repeated)."""
340+
mock_client = MagicMock()
341+
mock_client.create_conversation.return_value = "conv-1"
342+
mock_client.send_message.side_effect = [
343+
_metric_turn_result([_skills_tc("metric"), _create_metric_tc("m1")]),
344+
_metric_turn_result([_create_metric_tc("m2")]), # no set_skills -- skill already active
345+
]
346+
347+
with (
348+
patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=mock_client),
349+
patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"),
350+
):
351+
result = run_agentic_conversation(
352+
host="http://host/api/v1/actions/workspaces/ws1/ai",
353+
token="tok",
354+
workspace_id="ws1",
355+
fixture=_two_metric_turn_fixture(),
356+
)
357+
358+
assert result.turn_results[0].skill_routing is True
359+
assert result.turn_results[1].skill_routing is True
360+
361+
362+
def test_run_agentic_conversation_skill_routing_false_when_skill_never_activated():
363+
"""Guard against the fix being too lenient: a skill that no turn ever activated
364+
must still fail routing, not be credited by the cumulative-set change."""
365+
mock_client = MagicMock()
366+
mock_client.create_conversation.return_value = "conv-1"
367+
mock_client.send_message.return_value = _metric_turn_result([_create_metric_tc("m1")])
368+
369+
fixture = ConversationFixture(
370+
id="test-never-activated",
371+
expected_skills=["metric"],
372+
turns=[
373+
TurnDefinition(turn_id="t1", message="Create x", expected_skill="metric", expected_output_type="metric"),
374+
],
375+
)
376+
with (
377+
patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=mock_client),
378+
patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"),
379+
):
380+
result = run_agentic_conversation(
381+
host="http://host/api/v1/actions/workspaces/ws1/ai",
382+
token="tok",
383+
workspace_id="ws1",
384+
fixture=fixture,
385+
)
386+
387+
assert result.turn_results[0].skill_routing is False
388+
389+
332390
def test_run_agentic_conversation_deletes_metrics_even_when_a_later_turn_raises():
333391
mock_client = MagicMock()
334392
mock_client.create_conversation.return_value = "conv-1"

0 commit comments

Comments
 (0)