From 23532485ff1d40c4295c214184ab580a7fd599d1 Mon Sep 17 00:00:00 2001 From: Peter Tomko Date: Mon, 24 Aug 2026 22:04:38 +0200 Subject: [PATCH 1/2] Use a TurnDetail dataclass instead of a dict literal in _conversation_detail Addresses hkad98's PR #1750 review comment: the per-turn subset reported in detail["turns"] was a bare dict literal with no type checking. TurnDetail mirrors the same 6 fields; asdict() at the boundary keeps the output shape (and the detail: dict contract) unchanged. --- .../core/agentic/conversation.py | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) 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..4c6c03b43 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py @@ -5,7 +5,7 @@ import json import re -from dataclasses import dataclass, field +from dataclasses import asdict, dataclass, field from typing import Literal from gooddata_sdk import GoodDataSdk @@ -426,19 +426,33 @@ def run_agentic_conversation( ) +@dataclass +class TurnDetail: + """The subset of a TurnResult reported in detail["turns"] for one conversation turn.""" + + turn_id: str + expected_skill: str + skill_routing: bool + output_present: bool + output_correct: bool | None + activated_skills: list[str] + + 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, - } + asdict( + TurnDetail( + 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 ], "latency_breakdown": build_latency_breakdown(result.tool_call_events, result.reasoning_step_events), From 25494ea7be179264ad494ea90750af3500f2bc57 Mon Sep 17 00:00:00 2001 From: Peter Tomko Date: Tue, 25 Aug 2026 10:49:02 +0200 Subject: [PATCH 2/2] Move turn detail dict-building onto TurnResult.detail() Per hkad98's review comment on #1757: replace the separate TurnDetail dataclass + _conversation_detail's asdict() call with a detail() method on TurnResult itself, since it already owns every field being reported. Output shape (detail["turns"]) is unchanged. --- .../core/agentic/conversation.py | 39 +++++++------------ 1 file changed, 13 insertions(+), 26 deletions(-) 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 4c6c03b43..f409593a9 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py @@ -5,7 +5,7 @@ import json import re -from dataclasses import asdict, dataclass, field +from dataclasses import dataclass, field from typing import Literal from gooddata_sdk import GoodDataSdk @@ -70,6 +70,17 @@ class TurnResult(BaseModel): def skill_success(self) -> bool: return self.skill_routing and self.output_present and self.no_error + def detail(self) -> dict: + """The subset of this result reported in detail["turns"] for one conversation turn.""" + return { + "turn_id": self.turn_id, + "expected_skill": self.expected_skill, + "skill_routing": self.skill_routing, + "output_present": self.output_present, + "output_correct": self.output_correct, + "activated_skills": self.activated_skills, + } + def _resolve_refs( expected_output: dict | None, @@ -426,35 +437,11 @@ def run_agentic_conversation( ) -@dataclass -class TurnDetail: - """The subset of a TurnResult reported in detail["turns"] for one conversation turn.""" - - turn_id: str - expected_skill: str - skill_routing: bool - output_present: bool - output_correct: bool | None - activated_skills: list[str] - - def _conversation_detail(result: ConversationResult) -> dict: return { "full_skill_coverage": result.full_skill_coverage, "total_clarification_turns": result.total_clarification_turns, - "turns": [ - asdict( - TurnDetail( - 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), }