diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py index a87338df3..fadc22b52 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py @@ -6,7 +6,7 @@ import json import re from dataclasses import dataclass, field -from typing import Literal +from typing import ClassVar, Literal from gooddata_sdk import GoodDataSdk from pydantic import BaseModel @@ -70,6 +70,23 @@ class TurnResult(BaseModel): def skill_success(self) -> bool: return self.skill_routing and self.output_present and self.no_error + # Reported per turn in detail["turns"]. A name listed here that no longer exists on the + # model raises rather than silently emitting a stale key, which a hand-written dict + # literal of the same fields would not -- and model_dump deep-copies activated_skills, + # so a caller mutating the returned dict cannot reach back into this TurnResult. + _DETAIL_FIELDS: ClassVar[set[str]] = { + "turn_id", + "expected_skill", + "skill_routing", + "output_present", + "output_correct", + "activated_skills", + } + + def detail(self) -> dict: + """The subset of this result reported in detail["turns"] for one conversation turn.""" + return self.model_dump(include=self._DETAIL_FIELDS) + def _resolve_refs( expected_output: dict | None, @@ -430,17 +447,7 @@ def _conversation_detail(result: ConversationResult) -> dict: return { "full_skill_coverage": result.full_skill_coverage, "total_clarification_turns": result.total_clarification_turns, - "turns": [ - { - "turn_id": tr.turn_id, - "expected_skill": tr.expected_skill, - "skill_routing": tr.skill_routing, - "output_present": tr.output_present, - "output_correct": tr.output_correct, - "activated_skills": tr.activated_skills, - } - for tr in result.turn_results - ], + "turns": [tr.detail() for tr in result.turn_results], "latency_breakdown": build_latency_breakdown(result.tool_call_events, result.reasoning_step_events), } diff --git a/packages/gooddata-eval/tests/test_agentic_conversation.py b/packages/gooddata-eval/tests/test_agentic_conversation.py index f28deb124..4413ad86f 100644 --- a/packages/gooddata-eval/tests/test_agentic_conversation.py +++ b/packages/gooddata-eval/tests/test_agentic_conversation.py @@ -97,6 +97,34 @@ def test_turn_result_skill_success(): assert r.skill_success is True +def _turn_result() -> TurnResult: + return TurnResult( + turn_id="t1", + expected_skill="visualization", + skill_routing=True, + output_present=True, + no_error=True, + activated_skills=["visualization"], + clarification_turns_used=0, + output_correct=None, + ) + + +def test_turn_result_detail_copies_activated_skills(): + """A caller mutating the returned dict must not reach back into the TurnResult.""" + r = _turn_result() + d = r.detail() + d["activated_skills"].append("mutated") + assert r.activated_skills == ["visualization"] + + +def test_turn_result_detail_fields_all_exist_on_the_model(): + """_DETAIL_FIELDS is a hand-listed subset, so a renamed field must fail here rather + than silently drop a key from every report.""" + assert set(TurnResult.model_fields) >= TurnResult._DETAIL_FIELDS + assert set(_turn_result().detail()) == TurnResult._DETAIL_FIELDS + + def test_resolve_refs_no_refs(): assert _resolve_refs({"key": "value"}, {}) == {"key": "value"}