Skip to content

Commit 46a27e6

Browse files
committed
feat(gooddata-eval): add KDA-skill agentic evaluator
Adds kda_skill.py to gooddata-eval, evaluating the chatbot's create_key_driver_analysis/execute_key_driver_analysis tool calls against the agent_kda_skill Langfuse dataset. Scope is completion (kda_triggered -> executed -> success -> turn_completed), not per-field correctness -- per-field checks are computed and logged as kda_-prefixed informational scores for a follow-up ticket (QA-28699), and are None (not False) both when expected_output has no key for a field AND when that field's own precondition (kda_triggered, or executed+success for Summary) isn't met. Latency is measured directly by the harness, not re-derived from Langfuse after the fact: ChatResult.turn_wall_clock_sec is set by ChatClient around the SSE read itself (after the connection opens, excluding any transient retry's backoff sleep), stamped on both the successful result and any ChatError.partial_result so a turn that dies mid-stream after KDA already succeeded still gets a number. KdaRunResult.kda_turn_sec captures it in _run_once at the exact point create_args gets set -- atomic with the kda_triggered signal, not a separate step that can fail independently. Logged as the kda_wall_clock_sec Langfuse score; combo_report.py (gdc-nas) reads it directly, with no trace re-querying or observation-shape matching needed on that side anymore. Adds ChatResult.stream_ended (from the SSE response_ended event, derived -- not a raw server field, see sse_client.py's _RESPONSE_ENDED_EVENT comment). turn_completed requires both stream_ended AND a non-empty text_response -- a stream that ends cleanly but delivers nothing to the user isn't a completed turn either, and a turn cut off mid-answer can still emit partial, non-empty text before dying. Fixes from review: - kda_ prefix pass_at_k/pass_power_k Langfuse scores -- unprefixed, "pass_at_2" at k=2 collides with visualization.py's own score name that gdc-nas's combo_report.py.verdict() checks first, silently misfiling every KDA record as visualization once KDA_RUN_K=2 is ever set. - Relative (not absolute) tolerance for Summary's revenue-scale values by default; change is checked against reference_value's scale, not against itself. Also recognizes an explicit absolute_tolerance key -- every real agent_kda_skill dataset item uses it, which this module never read before, silently running ~3000x looser than the dataset author intended. Warns on any other *tolerance* key so a future typo surfaces instead of repeating. - Filters compared as canonicalized sets, not order-sensitive lists. - except Exception (not except ChatError) around send_message -- a stream cut off mid-turn raises a raw httpx transport error, which a narrower catch would let escape uncaught, skipping Langfuse scoring entirely for that run. - ChatError/TransientChatError carry partial_result so tool calls that already succeeded before a later, unrelated stream error aren't discarded and misreported as "the agent never called KDA at all". - turn_completed resets to False on an exception, so a crash on a later disambiguation iteration can't leave a stale True from an earlier iteration. - kda_disambiguated logged and immediately nulls the six *_correct informational fields: the simulated user reply names the acceptable candidate(s) drawn from expected_output itself, so those fields aren't an independent signal once a run went through disambiguation. - KDA-specific _is_asking_clarification instead of a heuristic shared with metric_skill.py/conversation.py, which had silently drifted apart; strips a leading "to clarify, " discourse marker before its substring checks, since that phrase means "in other words" in a final answer, not a request for one. - run_agentic_kda_skill rejects k < 1 -- the single initial run happens unconditionally regardless of k, so a bad env-driven KDA_RUN_K value (0, a typo, negative) previously ran silently once instead of surfacing the bad config, indistinguishable from a deliberate k=1. - Regression tests for find_traces_per_conversation's None-safety and _filters_match's isinstance guard. JIRA: QA-28800
1 parent 676da68 commit 46a27e6

7 files changed

Lines changed: 2028 additions & 7 deletions

File tree

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@
3030
evaluate_agentic_guardrail,
3131
run_agentic_guardrail,
3232
)
33+
from gooddata_eval.core.agentic.kda_skill import (
34+
AgenticKdaSummary,
35+
KdaEvaluation,
36+
KdaRunResult,
37+
KdaSkillAssertionError,
38+
evaluate_agentic_kda_skill,
39+
run_agentic_kda_skill,
40+
)
3341
from gooddata_eval.core.agentic.metric_skill import (
3442
AgenticMetricSummary,
3543
MetricRunResult,
@@ -56,6 +64,7 @@
5664
"AgenticAlertSummary",
5765
"AgenticGeneralQuestionSummary",
5866
"AgenticGuardrailSummary",
67+
"AgenticKdaSummary",
5968
"AgenticMetricSummary",
6069
"AgenticSearchSummary",
6170
"AgenticRunSummary",
@@ -69,6 +78,9 @@
6978
"GeneralQuestionResult",
7079
"GuardrailAssertionError",
7180
"GuardrailResult",
81+
"KdaEvaluation",
82+
"KdaRunResult",
83+
"KdaSkillAssertionError",
7284
"MetricRunResult",
7385
"MetricSkillAssertionError",
7486
"RunResult",
@@ -81,13 +93,15 @@
8193
"evaluate_agentic_conversation",
8294
"evaluate_agentic_general_question",
8395
"evaluate_agentic_guardrail",
96+
"evaluate_agentic_kda_skill",
8497
"evaluate_agentic_metric_skill",
8598
"evaluate_agentic_search_tool",
8699
"evaluate_agentic_visualization",
87100
"run_agentic_alert_skill",
88101
"run_agentic_conversation",
89102
"run_agentic_general_question",
90103
"run_agentic_guardrail",
104+
"run_agentic_kda_skill",
91105
"run_agentic_metric_skill",
92106
"run_agentic_search_tool",
93107
"run_agentic_visualization",

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

Lines changed: 689 additions & 0 deletions
Large diffs are not rendered by default.

packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py

Lines changed: 66 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,38 @@
2828
_log = logging.getLogger(__name__)
2929

3030
SSE_DATA_PREFIX = "data: "
31+
SSE_EVENT_PREFIX = "event: "
32+
# gen-ai's last event, only if at least one item was already emitted (conversations_controller.py).
33+
_RESPONSE_ENDED_EVENT = "response_ended"
3134

3235
_RETRYABLE_STATUS_CODES: frozenset[int] = frozenset({429, 502, 503, 504})
3336
_METADATA_SYNC_MARKER = "METADATA_SYNC_IN_PROGRESS"
3437

3538

3639
class ChatError(RuntimeError):
37-
"""Non-retryable error reported by the chat SSE stream."""
40+
"""Non-retryable error reported by the chat SSE stream.
3841
39-
def __init__(self, message: str, *, status_code: int | None = None, detail: str | None = None) -> None:
42+
``partial_result`` carries whatever the accumulator captured before this error fired
43+
(tool calls included) -- an error event ends the stream before ``_build_chat_result``
44+
ever runs, so without this a caller has no way to see, e.g., that KDA's own tool calls
45+
already succeeded before an unrelated later error (a failed final-summary generation)
46+
killed the turn. Callers must not assume it's complete: fields normally filled in only
47+
at the very end of the stream (``stream_ended``, in particular) reflect the state at
48+
the moment of the error, not a genuinely finished turn.
49+
"""
50+
51+
def __init__(
52+
self,
53+
message: str,
54+
*,
55+
status_code: int | None = None,
56+
detail: str | None = None,
57+
partial_result: ChatResult | None = None,
58+
) -> None:
4059
super().__init__(message)
4160
self.status_code = status_code
4261
self.detail = detail
62+
self.partial_result = partial_result
4363

4464

4565
class TransientChatError(ChatError):
@@ -109,6 +129,7 @@ class _SseAccumulator:
109129
reasoning_steps: list[dict[str, Any]] = field(default_factory=list)
110130
adhoc_viz_args: list[dict[str, Any]] = field(default_factory=list)
111131
response_id: str | None = None
132+
stream_ended: bool = False
112133

113134

114135
def _handle_text(content: dict[str, Any], acc: _SseAccumulator) -> None:
@@ -187,22 +208,46 @@ def _build_chat_result(acc: _SseAccumulator) -> ChatResult:
187208
}
188209
result = ChatResult.model_validate(payload)
189210
result.response_id = acc.response_id
211+
result.stream_ended = acc.stream_ended
190212
return result
191213

192214

193215
def parse_sse_lines(lines: Iterable[str]) -> ChatResult:
194216
"""Parse an SSE stream (iterable of decoded lines) into a ChatResult."""
195217
acc = _SseAccumulator()
196-
for raw_line in lines:
218+
current_event = "message" # SSE default in the absence of an explicit "event: " line
219+
it = iter(lines)
220+
while True:
221+
try:
222+
raw_line = next(it)
223+
except StopIteration:
224+
break
225+
except Exception as exc:
226+
# Only a failure from iterating `lines` itself (e.g. httpx.RemoteProtocolError/
227+
# ReadError from a connection drop mid-stream) is rescued here. A bug in the
228+
# processing below must propagate as-is, loudly -- catching it the same way
229+
# would blend a real parser bug into the same "error" bucket as a network blip,
230+
# with no statusCode payload to tell them apart later.
231+
raise ChatError(f"SSE stream error: {exc}", partial_result=_build_chat_result(acc)) from exc
197232
line = raw_line.decode("utf-8") if isinstance(raw_line, bytes) else raw_line
198-
if not line or line.startswith("event: ") or not line.startswith(SSE_DATA_PREFIX):
233+
if not line:
234+
current_event = "message" # blank line ends one event block per the SSE spec
235+
continue
236+
if line.startswith(SSE_EVENT_PREFIX):
237+
current_event = line[len(SSE_EVENT_PREFIX) :].strip()
238+
continue
239+
if not line.startswith(SSE_DATA_PREFIX):
240+
continue
241+
if current_event == _RESPONSE_ENDED_EVENT:
242+
acc.stream_ended = True
199243
continue
200244
data_str = line[len(SSE_DATA_PREFIX) :]
201245
if _METADATA_SYNC_MARKER in data_str:
202246
raise TransientChatError(
203247
f"SSE transient error: {_METADATA_SYNC_MARKER}",
204248
status_code=None,
205249
detail=None,
250+
partial_result=_build_chat_result(acc),
206251
)
207252
try:
208253
event_data = json.loads(data_str)
@@ -213,8 +258,10 @@ def parse_sse_lines(lines: Iterable[str]) -> ChatResult:
213258
detail = event_data.get("detail")
214259
message = f"SSE error {code}: {detail}"
215260
if code in _RETRYABLE_STATUS_CODES:
216-
raise TransientChatError(message, status_code=code, detail=detail)
217-
raise ChatError(message, status_code=code, detail=detail)
261+
raise TransientChatError(
262+
message, status_code=code, detail=detail, partial_result=_build_chat_result(acc)
263+
)
264+
raise ChatError(message, status_code=code, detail=detail, partial_result=_build_chat_result(acc))
218265
if event_data.get("responseId") and not acc.response_id:
219266
acc.response_id = event_data["responseId"]
220267
item = event_data.get("item")
@@ -293,9 +340,21 @@ def send_message(self, conversation_id: str, question: str) -> ChatResult:
293340
body["options"] = {"reasoningEffort": self._reasoning_effort}
294341

295342
def _do() -> ChatResult:
343+
# t0 here, not around send_message(): includes the request/connection/server
344+
# setup time a caller actually waits through, but still excludes
345+
# _retry_transient's backoff sleep between attempts (harness overhead, not
346+
# gen-ai's time), since each retry calls _do() -- and this timer -- fresh.
347+
t0 = time.monotonic()
296348
with self._client.stream("POST", url, json=body, headers=headers) as resp:
297349
resp.raise_for_status()
298-
return parse_sse_lines(resp.iter_lines())
350+
try:
351+
result = parse_sse_lines(resp.iter_lines())
352+
except ChatError as exc:
353+
if exc.partial_result is not None:
354+
exc.partial_result.turn_wall_clock_sec = time.monotonic() - t0
355+
raise
356+
result.turn_wall_clock_sec = time.monotonic() - t0
357+
return result
299358

300359
return _retry_transient(_do, is_retryable=_is_retryable_exc)
301360

packages/gooddata-eval/src/gooddata_eval/core/models.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ class ChatResult(BaseModel):
100100
reasoning_step_count: int = Field(default=0, alias="reasoningStepCount")
101101
conversation_id: str | None = Field(default=None, alias="conversationId")
102102
response_id: str | None = Field(default=None, alias="responseId")
103+
# Derived, not a raw server field -- see sse_client.py's _RESPONSE_ENDED_EVENT.
104+
stream_ended: bool = False
105+
# Set by ChatClient, not from the payload: wall-clock time of the SSE read itself,
106+
# excluding retry backoff.
107+
turn_wall_clock_sec: float | None = None
103108

104109

105110
class SummaryInput(BaseModel):

0 commit comments

Comments
 (0)