diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/_conversation_context.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/_conversation_context.py new file mode 100644 index 000000000..665d3681c --- /dev/null +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/_conversation_context.py @@ -0,0 +1,295 @@ +# (C) 2026 GoodData Corporation. All rights reserved. +"""What the conversation evaluator needs to judge context, not just output presence. + +Three questions get asked of every assistant reply that did not produce the turn's +expected output: + +1. What kind of reply is it -- a confirmation request ("Should I create this metric?"), + a question to the user, or a turn that ended without doing anything (a stall)? +2. For a question: did the assistant ask for something the conversation had already + established? That is the lost-context signal, graded by a binary LLM judge. +3. What does the simulated user say back? In ``context`` mode it never sees the turn's + expected output -- only the fixture's set answers and the conversation itself -- so a + lost context can no longer be repaired silently by a user who knows the answer key. + +Only information that is IN the conversation counts. A question about something the +assistant could have looked up in the workspace, but that nobody mentioned, is graded as +legitimate: whether the assistant should have looked it up is a separate check. +""" + +from __future__ import annotations + +import hashlib +import json +import os +import re +from dataclasses import dataclass +from typing import Literal + +from gooddata_eval.core.config import judge_model +from gooddata_eval.core.evaluators._llm_judge import JudgeResponseError, LLMJudge, _message_content +from gooddata_eval.core.models import ChatResult + +ReplyKind = Literal["confirmation", "question", "no_action"] + +NUDGE_MESSAGE = "Please go ahead and do it." +CONFIRMATION_REPLY = "Yes, go ahead." +# Asked to pick from a list (which dashboard to bind an alert to), a real user picks one. +NO_ANSWER_REPLY = "Use your best judgement and go ahead. If you need me to pick from a list, take the first option." + +# Part of the metric and alert skills' designed flow, not a clarification. +_CONFIRMATION_RE = re.compile( + r"\b(?:should|shall|may|can)\s+i\s+(?:go\s+ahead|proceed|create|save|update|apply|add|set\s+(?:it|this)\s+up)" + r"|\bdo\s+you\s+want\s+me\s+to\s+(?:go\s+ahead|proceed|create|save|update|apply)" + r"|\bwould\s+you\s+like\s+me\s+to\s+(?:go\s+ahead|proceed|create|save|update|apply)" + r"|\bplease\s+confirm\b|\bconfirm\s+(?:if|whether|that)\b", + re.IGNORECASE, +) + +# Checked after _CONFIRMATION_RE, so "please confirm" stays a confirmation. +_REQUEST_RE = re.compile( + r"\bplease\s+(?:select|choose|pick|specify|provide|tell\s+me|let\s+me\s+know|clarify|share)\b" + r"|\breply\s+with\b|\b(?:select|choose|pick)\s+(?:one|a|an|the)\b", + re.IGNORECASE, +) + +_MESSAGE_CHARS = 800 +_TRANSCRIPT_MESSAGES = 40 + + +def has_structured_question(chat_result: ChatResult) -> bool: + """gen-ai's own signal that the reply is a question: a ``clarifyingQuestions`` part.""" + return any(part.get("type") == "clarifyingQuestions" for part in chat_result.unhandled_parts or []) + + +def classify_reply(chat_result: ChatResult, rendered_text: str) -> ReplyKind: + """Kind of a reply that did not produce the expected output. + + The structured ``clarifyingQuestions`` part is authoritative when present. Without it + the text decides: a confirmation phrase or an alert proposal is a confirmation, a + question mark is a question, and anything else -- including an empty reply -- is a turn + that stopped without acting. + """ + if has_structured_question(chat_result): + return "question" + if chat_result.alert_proposals: + return "confirmation" + text = (rendered_text or "").strip() + if not text: + return "no_action" + if _CONFIRMATION_RE.search(text): + return "confirmation" + if "?" in text or _REQUEST_RE.search(text): + return "question" + return "no_action" + + +@dataclass +class TranscriptEntry: + role: Literal["user", "assistant"] + text: str + + +def clip(text: str, limit: int = _MESSAGE_CHARS) -> str: + text = " ".join((text or "").split()) + return text if len(text) <= limit else text[: limit - 1] + "…" + + +def summarize_visualizations(chat_result: ChatResult) -> str: + """One line per chart the reply showed: what it plotted and how it was filtered. + + Charts carry most of what a conversation establishes ("the Apparel filter", "by + quarter"), and they are invisible in the text part. The judge needs them to know what + was already settled. + """ + vizzes = chat_result.created_visualizations + objects = getattr(vizzes, "objects", None) or [] + lines = [] + for viz in objects: + fields = [getattr(f, "using", f) for f in (viz.query.fields or {}).values()] + filters = [ + {k: v for k, v in f.items() if k in ("type", "using", "state", "from", "to", "top", "bottom")} + for f in (viz.query.filter_by or {}).values() + if isinstance(f, dict) + ] + lines.append(f"[chart {viz.type or ''}: fields={fields} filters={filters}]") + return " ".join(lines) + + +def render_transcript(entries: list[TranscriptEntry]) -> str: + recent = entries[-_TRANSCRIPT_MESSAGES:] + return "\n".join(f"{e.role.upper()}: {clip(e.text)}" for e in recent) + + +_JUDGE_SYSTEM = """\ +You grade one question that an analytics assistant asked its user. You get: +- CONVERSATION BEFORE: everything said before the user's latest message (may be empty), +- LATEST MESSAGE: what the user just asked, +- QUESTION: the assistant's reply, in which it asks the user something instead of doing it. + +Decide whether the information the QUESTION asks for already appears in CONVERSATION BEFORE: +stated by the user, or determined by an assistant answer -- for example which metric was meant, +which filters apply, which item came out weakest or biggest, what a chart showed, which object +was created. + +Rules: +- The LATEST MESSAGE does not count. A reference in it ("it", "that second one", "those + brands") only points at something that must appear in CONVERSATION BEFORE. +- If CONVERSATION BEFORE is empty, the answer is false. +- Information that exists in the workspace but was never mentioned in the conversation does not + count. +- Do not judge whether asking was useful, necessary or well phrased. + +Examples: +- BEFORE: user "Net sales by country." / assistant "Here is net sales by country." LATEST: "Add + order count to it." QUESTION: "Which chart should I add order count to?" -> true (the chart + just made). +- BEFORE: user "Top 10 products by revenue." / assistant "Here are the top 10 products." + LATEST: "Remove the limit." QUESTION: "Which limit do you mean?" -> true (the top 10). +- BEFORE: (empty) LATEST: "Show revenue by month." QUESTION: "Which revenue metric: gross or + net?" -> false (nothing earlier says which). + +Return a JSON object with exactly two keys: + "already_in_conversation": true or false + "reasoning": one sentence naming the information and where it appears, or that it does not +""" + +_JUDGE_USER = "CONVERSATION BEFORE:\n{history}\n\nLATEST MESSAGE:\n{latest}\n\nQUESTION:\n{question}" + + +def judge_prompt(history: str, latest: str, question: str) -> str: + """The user prompt the clarification judge reads, also recorded for auditing.""" + return _JUDGE_USER.format(history=history or "(empty)", latest=latest, question=question) + + +@dataclass(frozen=True) +class ClarificationVerdict: + lost_context: bool | None # None when the judge could not be run or gave no verdict + reasoning: str + error: str | None = None + + +class _ClarificationLLM(LLMJudge): + """``LLMJudge``'s client, temperature handling and retry, with this judge's own prompt and + verdict key. The key names the question asked, so a 0/1 cannot be read the wrong way round.""" + + def __init__(self, model: str | None = None) -> None: + super().__init__([], model=model) + self._system_prompt = _JUDGE_SYSTEM + + def already_in_conversation(self, prompt: str) -> tuple[bool, str]: + messages = [{"role": "system", "content": self._system_prompt}, {"role": "user", "content": prompt}] + raw = None + for _attempt in range(2): + raw = _message_content(self._create_completion(messages)) + if isinstance(raw, str) and raw.strip(): + break + if not (isinstance(raw, str) and raw.strip()): + raise JudgeResponseError(f"clarification judge {self.model!r} returned an empty body twice") + try: + data = json.loads(raw) + except json.JSONDecodeError as exc: + raise JudgeResponseError(f"clarification judge returned unparseable JSON: {raw!r}") from exc + value = data.get("already_in_conversation") if isinstance(data, dict) else None + if not isinstance(value, bool): + raise JudgeResponseError(f"clarification judge returned no boolean verdict: {raw!r}") + return value, str(data.get("reasoning", "")) + + +class ClarificationJudge: + """Binary judge: did the assistant ask for something the conversation already established? + + Temperature 0 and a JSON verdict, on ``LLMJudge``'s client. Verdicts are cached per prompt, + so a conversation that repeats the same question does not pay for it twice. + """ + + def __init__(self, model: str | None = None) -> None: + self._model = model + self._llm: _ClarificationLLM | None = None + self._cache: dict[str, ClarificationVerdict] = {} + self._warned = False + + @property + def model_name(self) -> str: + """The judge model, without building a client (which needs an API key).""" + return self._llm.model if self._llm is not None else (self._model or judge_model()) + + def judge(self, history: str, latest: str, question: str) -> ClarificationVerdict: + prompt = judge_prompt(history, latest, question) + key = hashlib.sha256(prompt.encode()).hexdigest() + if key in self._cache: + return self._cache[key] + try: + if self._llm is None: + self._llm = _ClarificationLLM(self._model) + try: + found, reasoning = self._llm.already_in_conversation(prompt) + verdict = ClarificationVerdict(lost_context=found, reasoning=reasoning) + except JudgeResponseError as exc: + verdict = ClarificationVerdict(lost_context=None, reasoning="", error=str(exc)) + except Exception as exc: # noqa: BLE001 -- a grading fault must not fail the agent's conversation + if not self._warned: + print(f"[JUDGE] clarifications left unjudged: {exc}") + self._warned = True + verdict = ClarificationVerdict(lost_context=None, reasoning="", error=str(exc)) + self._cache[key] = verdict + return verdict + + +_SIM_USER_MODEL = "gpt-4o" + + +def _chat(prompt_system: str, prompt_user: str) -> str | None: + try: + from openai import OpenAI # noqa: PLC0415 + except ImportError: + return None + api_key = os.environ.get("OPENAI_API_KEY") + if not api_key: + return None + try: + response = OpenAI(api_key=api_key).chat.completions.create( + model=_SIM_USER_MODEL, + messages=[{"role": "system", "content": prompt_system}, {"role": "user", "content": prompt_user}], + temperature=0, + max_tokens=200, + ) + except Exception as exc: + print(f"[SIM-USER] reply failed: {exc}") + return None + content = response.choices[0].message.content if response.choices else None + return content.strip() if content else None + + +def reply_from_facts(transcript: str, question: str, facts: list[str]) -> str: + """A legitimate question the fixture has no unused set answer for. + + The reply may use only the fixture's facts for this turn -- never the expected output -- + and falls back to "use your best judgement" when the facts do not cover the question. + """ + if not facts: + return NO_ANSWER_REPLY + reply = _chat( + "You play a business user talking to an analytics assistant. Answer its question briefly, " + "using ONLY the facts listed below. Never invent requirements. If the facts do not answer " + f"the question, reply exactly: {NO_ANSWER_REPLY}", + f"Conversation so far:\n{transcript}\n\nThe assistant now asks:\n{question}\n\n" + "Facts you know:\n" + "\n".join(f"- {f}" for f in facts), + ) + return reply or NO_ANSWER_REPLY + + +def reply_restating(transcript: str, question: str) -> str: + """The assistant asked for something the conversation already established. + + The turn is already failed; restating keeps the rest of the conversation measurable, + which is what a real user would do too. + """ + reply = _chat( + "You play a business user talking to an analytics assistant. The assistant just asked for " + "something that was already said or found earlier in the conversation. Reply briefly, " + "starting with 'As I said earlier,' and repeat only that information, taken from the " + "conversation. Do not add anything new.", + f"Conversation so far:\n{transcript}\n\nThe assistant now asks:\n{question}", + ) + return reply or "As I said earlier in this conversation — please use what we already established." 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 98a5066ed..f501c5a9c 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py @@ -1,16 +1,45 @@ # (C) 2026 GoodData Corporation. All rights reserved. -"""Agentic conversation evaluation runner (multi-turn, multi-skill).""" +"""Agentic conversation evaluation runner (multi-turn, multi-skill). + +Two modes decide what a pass means: + +- ``legacy``: a turn passes when the expected skill is active and an output of the + expected type appears. The simulated user is primed with the turn's expected output. +- ``context``: a turn additionally has to match its expected output given everything the + conversation established, must not ask for information the conversation already holds + (graded by a binary judge), and must not stall. The simulated user answers from the + fixture's set answers and the conversation only, never from the expected output. + +Both modes compute every score, so a dataset can run in ``legacy`` while the ``context`` +scores are collected alongside it; the mode only picks which verdict raises. +""" from __future__ import annotations import json +import os import re from dataclasses import dataclass, field +from datetime import date from typing import ClassVar, Literal from gooddata_sdk import GoodDataSdk -from pydantic import BaseModel, Field - +from pydantic import BaseModel, Field, ValidationError + +from gooddata_eval.core.agentic._conversation_context import ( + CONFIRMATION_REPLY, + NUDGE_MESSAGE, + ClarificationJudge, + TranscriptEntry, + classify_reply, + clip, + judge_prompt, + render_transcript, + reply_from_facts, + reply_restating, + summarize_visualizations, +) +from gooddata_eval.core.agentic._gate import log_gate_scores from gooddata_eval.core.agentic._trace_linker import ( RunIdentity, RunTraceContext, @@ -20,15 +49,18 @@ submit_trace_scoring, utc_now, ) -from gooddata_eval.core.agentic.alert_skill import render_alert_proposal -from gooddata_eval.core.agentic.metric_skill import _delete_metric, _extract_created_metric_ids, _extract_metric_result +from gooddata_eval.core.agentic.alert_skill import _delete_alert, render_alert_proposal +from gooddata_eval.core.agentic.metric_skill import _delete_metric, _extract_metric_result from gooddata_eval.core.chat.render import render_answer_text -from gooddata_eval.core.chat.sse_client import ChatClient +from gooddata_eval.core.chat.sse_client import ChatClient, ChatError, TurnIncompleteError from gooddata_eval.core.config import ReasoningEffort +from gooddata_eval.core.evaluators._deep_subset import deep_subset +from gooddata_eval.core.evaluators._maql import normalize_maql from gooddata_eval.core.models import ( AgenticAssertionError, AgenticEvalOutcome, ChatResult, + CreatedVisualization, ReasoningStepEvent, ToolCallEvent, shift_and_index_events, @@ -39,15 +71,35 @@ check_viz_type, get_dimension_uri_set, get_metric_uri_set, + normalized_filters, + resolve_alias_to_uri, ) _REF_PATTERN = re.compile(r"\$ref:([\w_]+)\.([\w_]+)") _DEFAULT_MAX_CLARIFICATION_TURNS = 7 +ConversationMode = Literal["legacy", "context"] +CONVERSATION_MODE_ENV_VAR = "GD_EVAL_CONVERSATION_MODE" + + +def resolve_conversation_mode(mode: str | None = None) -> ConversationMode: + """The explicit mode, else ``GD_EVAL_CONVERSATION_MODE``, else ``legacy``.""" + raw = (mode or os.environ.get(CONVERSATION_MODE_ENV_VAR, "") or "legacy").strip().lower() + if raw not in ("legacy", "context"): + raise ValueError(f"conversation mode must be 'legacy' or 'context', got {raw!r}") + return "context" if raw == "context" else "legacy" + class TurnDefinition(BaseModel): - """Definition of a single turn in a multi-turn conversation evaluation.""" + """Definition of a single turn in a multi-turn conversation evaluation. + + ``depends_on`` names the earlier turns this one cannot be answered without. It does not + change how the turn is graded; it selects which turns ``context_kept_rate`` is computed + over. ``set_answers`` are what the user replies, in order, when the assistant asks a + legitimate question on this turn -- the only knowledge the context-mode simulated user + has beyond the conversation itself. + """ turn_id: str message: str @@ -55,6 +107,10 @@ class TurnDefinition(BaseModel): expected_output_type: Literal["visualization", "tool_call", "metric"] = "visualization" expected_tool_name: str | None = None expected_output: dict | None = None + expected_output_alternatives: list[dict] = Field(default_factory=list) + expected_tool_args: dict | None = None + depends_on: list[str] = Field(default_factory=list) + set_answers: list[str] = Field(default_factory=list) class ConversationFixture(BaseModel): @@ -66,6 +122,21 @@ class ConversationFixture(BaseModel): turns: list[TurnDefinition] +ReplyRecordKind = Literal["confirmation", "question", "no_action", "turn_incomplete"] + + +class ClarificationRecord(BaseModel): + """One assistant reply that did not deliver the turn's output, and what the user said back.""" + + kind: ReplyRecordKind + agent_message: str + lost_context: bool | None = None + reasoning: str = "" + judge_error: str | None = None + judge_input: str = "" + reply: str = "" + + class TurnResult(BaseModel): """Evaluation result for a single conversation turn. @@ -81,6 +152,10 @@ class TurnResult(BaseModel): ``skill_routing=True`` with ``activated_skills=[]`` is the reused-skill case, not a scoring bug -- ``active_skills`` shows where the credit came from. + + ``skill_success`` is the legacy verdict. ``context_success`` also requires the output to + match the expected one (when the fixture states it), no clarification the judge found + to be lost context, and no stall. """ turn_id: str @@ -93,11 +168,43 @@ class TurnResult(BaseModel): active_skills: list[str] = Field(default_factory=list) clarification_turns_used: int = 0 output_correct: bool | None = None + mismatches: list[str] = Field(default_factory=list) + clarifications: list[ClarificationRecord] = Field(default_factory=list) + depends_on: list[str] = Field(default_factory=list) @property def skill_success(self) -> bool: return self.skill_routing and self.output_present and self.no_error + @property + def lost_context(self) -> bool: + return any(c.lost_context is True for c in self.clarifications) + + @property + def stalled(self) -> bool: + return any(c.kind in ("no_action", "turn_incomplete") for c in self.clarifications) + + @property + def context_success(self) -> bool: + return self.skill_success and self.output_correct is not False and not self.lost_context and not self.stalled + + def failure_reasons(self) -> list[str]: + """Why ``context_success`` is False, in reading order; empty when it is True.""" + reasons: list[str] = [] + if not self.no_error: + reasons.append("turn could not run") + elif not self.output_present: + reasons.append("no output of the expected type") + elif not self.skill_routing: + reasons.append(f"skill {self.expected_skill!r} not active") + if self.output_correct is False: + reasons.extend(self.mismatches or ["output does not match the expected output"]) + if self.lost_context: + reasons.append("asked for information the conversation already established") + if self.stalled: + reasons.append("ended a reply without the expected output and without asking anything") + return reasons + # 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, @@ -112,11 +219,18 @@ def skill_success(self) -> bool: # What skill_routing was judged against -- without it, a turn showing # skill_routing=True and activated_skills=[] looks like a scoring bug. "active_skills", + "clarification_turns_used", + "mismatches", + "clarifications", + "depends_on", } 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) + out = self.model_dump(include=self._DETAIL_FIELDS) + out["context_success"] = self.context_success + out["failure_reasons"] = self.failure_reasons() + return out def _resolve_refs( @@ -210,56 +324,168 @@ def _check_output_present(turn: TurnDefinition, chat_result: ChatResult) -> bool return False -def _check_output_correct(turn: TurnDefinition, chat_result: ChatResult) -> bool | None: - """Check output correctness against expected_output when defined. +def _expected_viz(expected: dict) -> CreatedVisualization | None: + """The chart an expected output describes, or None when it names no fields to compare. - Returns None when expected_output is absent (presence check only). + Fixtures wrap the chart as ``{"visualization": {...}}``; a bare chart is accepted too. + ``type`` may be empty, which means "any chart type". """ - from gooddata_eval.core.evaluators._maql import normalize_maql # noqa: PLC0415 + body = expected.get("visualization", expected) + if not isinstance(body, dict): + return None + query = body.get("query") + if not isinstance(query, dict) or not query.get("fields"): + return None + try: + return CreatedVisualization.model_validate({**body, "type": body.get("type") or ""}) + except ValidationError: + return None + +def _flat_viz_check(candidates: list[dict], chat_result: ChatResult) -> tuple[bool | None, list[str]]: + """The flat ``{"metrics": [uri, ...], "dimensions": [uri, ...]}`` form: the listed URIs + must all be in one of the charts. ``(None, [])`` when no candidate lists any.""" + flat = [c for c in candidates if isinstance(c.get("metrics"), list) or isinstance(c.get("dimensions"), list)] + if not flat: + return None, [] + objects = getattr(chat_result.created_visualizations, "objects", None) or [] + for cand in flat: + want_m, want_d = set(cand.get("metrics") or []), set(cand.get("dimensions") or []) + for act in objects: + if want_m <= get_metric_uri_set(act) and want_d <= get_dimension_uri_set(act): + return True, [] + return False, [f"expected {flat[0].get('metrics') or []} / {flat[0].get('dimensions') or []} in the chart"] + + +def _sort_signature(viz: CreatedVisualization) -> set[tuple[str, str, str]]: + """(sort type, sorted-by URI, direction) for each sort entry.""" + out: set[tuple[str, str, str]] = set() + for entry in viz.query.sort_by: + stype = str(entry.get("type") or "") + direction = str(entry.get("direction") or "").upper() + aliases = entry.get("metrics") if stype == "metric_sort" else [entry.get("by")] + for alias in aliases or []: + if isinstance(alias, str): + out.add((stype, resolve_alias_to_uri(alias, viz.query.fields), direction)) + return out + + +def _clamp_future_dates(viz: CreatedVisualization, today: date) -> CreatedVisualization: + """A copy whose absolute date filters end no later than ``today``. + + Nothing is recorded after today, so "2025 and 2026 so far" as 2025-01-01..2026-12-31 and as + 2025-01-01.. select the same data. Comparing the raw end dates would fail an answer + that states the period more precisely than the fixture does. + """ + filters = {} + for key, f in viz.query.filter_by.items(): + to = f.get("to") if isinstance(f, dict) and f.get("type") == "date_filter" else None + if isinstance(to, str) and re.fullmatch(r"\d{4}-\d{2}-\d{2}", to) and to > today.isoformat(): + f = {**f, "to": today.isoformat()} + filters[key] = f + return viz.model_copy(update={"query": viz.query.model_copy(update={"filter_by": filters})}) + + +def _viz_mismatches( + expected: CreatedVisualization, actual: CreatedVisualization, today: date | None = None +) -> list[str]: + """Every way ``actual`` differs from ``expected``; empty when they match. + + Metrics and dimensions compare as sets of URIs, so bucket order and local ids do not + matter. Filters compare per category and always: a chart with a filter nobody asked for + fails as surely as one missing a filter. Type and sort compare only when the expected + chart states them. + """ + today = today or date.today() + expected, actual = _clamp_future_dates(expected, today), _clamp_future_dates(actual, today) + out: list[str] = [] + exp_m, act_m = get_metric_uri_set(expected), get_metric_uri_set(actual) + if exp_m != act_m: + out.append(f"metrics: expected {sorted(exp_m)}, got {sorted(act_m)}") + exp_d, act_d = get_dimension_uri_set(expected), get_dimension_uri_set(actual) + if exp_d != act_d: + out.append(f"dimensions: expected {sorted(exp_d)}, got {sorted(act_d)}") + if not check_viz_type(expected, actual): + out.append(f"type: expected {expected.type!r}, got {actual.type!r}") + scores = check_filters(expected, actual, today) + exp_f, act_f = normalized_filters(expected, today), normalized_filters(actual, today) + for category, ok in (("date", scores.date_ok), ("ranking", scores.ranking_ok), ("attribute", scores.attribute_ok)): + if not ok: + out.append(f"{category} filters: expected {exp_f[category]}, got {act_f[category]}") + if expected.query.sort_by: + exp_s, act_s = _sort_signature(expected), _sort_signature(actual) + # An expected entry without a direction matches either direction. + if not all(any(a[:2] == e[:2] and (not e[2] or a[2] == e[2]) for a in act_s) for e in exp_s): + out.append(f"sort: expected {sorted(exp_s)}, got {sorted(act_s)}") + return out + + +# `IN ("x")` with one value is `= "x"`: both spellings appear in agent output for the same filter. +_SINGLE_IN_RE = re.compile(r'(? str: + return _SINGLE_IN_RE.sub(r"=\1", normalize_maql(maql)) + + +def _tool_calls_named(tool_call_events: list[ToolCallEvent], name: str | None) -> list[ToolCallEvent]: + return [tc for tc in tool_call_events if name is None or tc.function_name == name] + + +def _check_output_correct( + turn: TurnDefinition, chat_result: ChatResult, turn_tool_calls: list[ToolCallEvent] | None = None +) -> tuple[bool | None, list[str]]: + """Whether the turn's output matches its expected output, and how it differs if not. + + Returns ``(None, [])`` when the fixture states nothing to compare. The expected output + and each of ``expected_output_alternatives`` are candidates; matching any one is a pass. + ``turn_tool_calls`` are every tool call of the turn, across its clarification rounds -- + the tool whose arguments are checked may have run before the final reply. + """ + tool_calls = turn_tool_calls if turn_tool_calls is not None else (chat_result.tool_call_events or []) + candidates = [c for c in [turn.expected_output, *turn.expected_output_alternatives] if c] otype = turn.expected_output_type - expected = turn.expected_output - if not expected: - return None if otype == "visualization": - from gooddata_eval.core.models import CreatedVisualization # noqa: PLC0415 - - vizzes = chat_result.created_visualizations - if not vizzes: - return False - objects = getattr(vizzes, "objects", None) + expected_vizzes = [v for v in (_expected_viz(c) for c in candidates) if v is not None] + if not expected_vizzes: + return _flat_viz_check(candidates, chat_result) + objects = getattr(chat_result.created_visualizations, "objects", None) or [] if not objects: - return False - viz = objects[0] - results: list[bool] = [] - if "viz_type" in expected or "type" in expected: - try: - exp_viz = CreatedVisualization.model_validate(expected.get("visualization", expected)) - results.append(check_viz_type(exp_viz, viz)) - except Exception: - pass - if expected.get("metrics"): - actual_uris = get_metric_uri_set(viz) - results.append(all(m in actual_uris for m in expected["metrics"])) - if expected.get("dimensions"): - actual_uris = get_dimension_uri_set(viz) - results.append(all(d in actual_uris for d in expected["dimensions"])) - if "filters" in expected: - try: - exp_viz = CreatedVisualization.model_validate(expected.get("visualization", expected)) - results.append(check_filters(exp_viz, viz).all_ok) - except Exception: - pass - return all(results) if results else None + return False, ["no visualization to compare"] + best: list[str] | None = None + for exp in expected_vizzes: + for act in objects: + diff = _viz_mismatches(exp, act) + if not diff: + return True, [] + if best is None or len(diff) < len(best): + best = diff + return False, best or [] if otype == "metric": - metric_result = _extract_metric_result(chat_result.tool_call_events or []) + maqls: list[str] = [m for m in (c.get("maql") for c in candidates) if isinstance(m, str) and m] + if not maqls: + return None, [] + metric_result = _extract_metric_result(tool_calls) if not metric_result: - return False - return normalize_maql(metric_result.get("maql", "")) == normalize_maql(expected.get("maql", "")) + return False, ["no created metric to compare"] + actual = _canonical_maql(metric_result.get("maql", "")) + if any(_canonical_maql(m) == actual for m in maqls): + return True, [] + return False, [f"maql: expected {maqls[0]!r}, got {metric_result.get('maql', '')!r}"] - return None + if otype == "tool_call": + if not turn.expected_tool_args: + return None, [] + calls = _tool_calls_named(tool_calls, turn.expected_tool_name) + args = [tc.parsed_arguments() or {} for tc in calls] + if any(deep_subset(turn.expected_tool_args, a) for a in args): + return True, [] + got = args[-1] if args else None + return False, [f"tool arguments: expected subset {turn.expected_tool_args}, got {got}"] + + return None, [] def _get_sim_user_response(agent_message: str, turn: TurnDefinition, expected_output: dict | None) -> str: @@ -329,7 +555,14 @@ def _get_sim_user_response(agent_message: str, turn: TurnDefinition, expected_ou @dataclass class ConversationResult: - """Outcome of a multi-turn, multi-skill conversation evaluation.""" + """Outcome of a multi-turn, multi-skill conversation evaluation. + + ``conversation_success`` is the legacy verdict, ``context_success`` the context one; see + the module docstring. ``context_kept_rate`` is the share of context-dependent turns + (those with ``depends_on``, else every turn after the first) that passed in context + terms, and ``turns_before_first_break`` how many turns passed before the first one that + did not. + """ conversation_id: str turn_results: list[TurnResult] @@ -341,6 +574,102 @@ class ConversationResult: response_id: str | None = None tool_call_events: list[ToolCallEvent] = field(default_factory=list) reasoning_step_events: list[ReasoningStepEvent] = field(default_factory=list) + context_success: bool = False + context_kept_rate: float | None = None + turns_before_first_break: int = 0 + lost_context_clarifications: int = 0 + stalled_turns: int = 0 + mode: ConversationMode = "legacy" + judge_model: str | None = None + + +def _metric_creations(tool_call_events: list[ToolCallEvent]) -> list[tuple[str, bool]]: + """(metric_id, created_new) for every successful create_metric call. + + ``create_metric`` is an upsert: on an existing id it replaces that metric and reports + ``created_new: false``. A result without the flag counts as a creation. + """ + out: list[tuple[str, bool]] = [] + for tc in tool_call_events: + if tc.function_name != "create_metric" or not tc.result: + continue + result_data = tc.parsed_result() + if not isinstance(result_data, dict): + continue + data = result_data.get("data", result_data) + if not isinstance(data, dict) or data.get("isError"): + continue + metric_id = data.get("metric_id") + if metric_id: + out.append((metric_id, data.get("created_new") is not False)) + return out + + +def _created_alert_ids(tool_call_events: list[ToolCallEvent]) -> list[str]: + """Ids of the alerts (automations) ``create_metric_alert`` calls created.""" + ids: list[str] = [] + for tc in tool_call_events: + if tc.function_name != "create_metric_alert" or not tc.result: + continue + result_data = tc.parsed_result() + if not isinstance(result_data, dict): + continue + nested = result_data.get("data") + alert_id = result_data.get("id") or (nested.get("id") if isinstance(nested, dict) else None) + if isinstance(alert_id, str) and alert_id and alert_id not in ids: + ids.append(alert_id) + return ids + + +class _WorkspaceObjects: + """What a conversation wrote to the workspace, for cleanup once it ends. + + Deferred to the end because a later turn may ``$ref`` a metric an earlier turn created. + A metric the agent updated in place (``created_new: false``) existed before the + conversation, so it is reported rather than deleted. + """ + + def __init__(self) -> None: + self.metrics: list[str] = [] + self.updated_metrics: list[str] = [] + self.alerts: list[str] = [] + + def record(self, tool_call_events: list[ToolCallEvent]) -> None: + for metric_id, created_new in _metric_creations(tool_call_events): + if created_new: + if metric_id not in self.metrics and metric_id not in self.updated_metrics: + self.metrics.append(metric_id) + elif metric_id not in self.metrics and metric_id not in self.updated_metrics: + self.updated_metrics.append(metric_id) + for alert_id in _created_alert_ids(tool_call_events): + if alert_id not in self.alerts: + self.alerts.append(alert_id) + + def delete(self, sdk: GoodDataSdk, workspace_id: str) -> None: + for alert_id in self.alerts: + _delete_alert(sdk, workspace_id, alert_id) + for metric_id in self.metrics: + _delete_metric(sdk, workspace_id, metric_id) + for metric_id in self.updated_metrics: + print(f"[CLEANUP] Metric {metric_id} existed before the conversation and was updated in place; kept.") + + +def _context_summary( + turn_results: list[TurnResult], fixture: ConversationFixture +) -> tuple[bool, float | None, int, int, int]: + """(context_success, context_kept_rate, turns_before_first_break, lost_context, stalled).""" + context_success = bool(turn_results) and all(tr.context_success for tr in turn_results) + labelled = {t.turn_id for t in fixture.turns if t.depends_on} + dependent = [tr for tr in turn_results if tr.turn_id in labelled] if labelled else turn_results[1:] + rate = (sum(tr.context_success for tr in dependent) / len(dependent)) if dependent else None + before_break = len(turn_results) + for i, tr in enumerate(turn_results): + if not tr.context_success: + before_break = i + break + lost = sum(1 for tr in turn_results for c in tr.clarifications if c.lost_context is True) + stalled = sum(1 for tr in turn_results if tr.stalled) + return context_success, rate, before_break, lost, stalled def run_agentic_conversation( @@ -352,13 +681,24 @@ def run_agentic_conversation( initial_conversation_id: str | None = None, reasoning_effort: ReasoningEffort | None = None, agent_id: str | None = None, + mode: ConversationMode | None = None, + clarification_judge: ClarificationJudge | None = None, + fresh_conversation_per_turn: bool = False, ) -> ConversationResult: """Run a multi-turn, multi-skill conversation evaluation (no K-runs). A single conversation is used for all turns in the fixture. Each turn may trigger up to *max_clarification_turns* additional rounds of simulated-user replies before the agent produces the expected output. + + ``fresh_conversation_per_turn`` sends every turn into a new conversation instead, so the + agent sees no history. It is the no-memory baseline a context score is calibrated + against: context-dependent turns are expected to fail there. """ + resolved_mode = resolve_conversation_mode(mode) + if fresh_conversation_per_turn and initial_conversation_id is not None: + raise ValueError("fresh_conversation_per_turn needs conversations this run creates itself") + judge = clarification_judge if clarification_judge is not None else ClarificationJudge() client = ChatClient( host=host, token=token, workspace_id=workspace_id, reasoning_effort=reasoning_effort, agent_id=agent_id ) @@ -369,14 +709,12 @@ def run_agentic_conversation( total_steps = 0 conversation_id: str = "" owns_conversation = False - # Metrics created during this conversation, deleted after it completes so they do - # not persist in the (shared) workspace and get reused by a later test. Deferred to - # the end — a later turn may $ref a metric an earlier turn created. - created_metric_ids: list[str] = [] + created = _WorkspaceObjects() reasoning_steps: list[str] = [] response_id: str | None = None conversation_tool_call_events: list[ToolCallEvent] = [] conversation_reasoning_step_events: list[ReasoningStepEvent] = [] + transcript: list[TranscriptEntry] = [] # The skills active right now, mirroring the platform's own state machine: set_skills # REPLACES the active set rather than adding to it -- verified against the gen-ai # service's skill registry, and stated in the tool's own description. So a turn that @@ -405,9 +743,18 @@ def run_agentic_conversation( conversation_id = client.create_conversation() owns_conversation = True - for turn in fixture.turns: + for turn_index, turn in enumerate(fixture.turns): + if fresh_conversation_per_turn and turn_index > 0: + client.delete_conversation(conversation_id) + conversation_id = client.create_conversation() + transcript = [] + active_skills = set() try: resolved_expected = _resolve_refs(turn.expected_output, turn_outputs) + resolved_alternatives = [ + _resolve_refs(a, turn_outputs) or {} for a in turn.expected_output_alternatives + ] + resolved_tool_args = _resolve_refs(turn.expected_tool_args, turn_outputs) except ValueError as exc: print(f"[SKIP] turn '{turn.turn_id}': {exc}") turn_results.append( @@ -423,18 +770,40 @@ def run_agentic_conversation( # read as "nothing was active", which is a different claim. active_skills=sorted(active_skills), output_correct=False, + mismatches=[str(exc)], + depends_on=list(turn.depends_on), ) ) continue - resolved_turn = turn.model_copy(update={"expected_output": resolved_expected}) + resolved_turn = turn.model_copy( + update={ + "expected_output": resolved_expected, + "expected_output_alternatives": resolved_alternatives, + "expected_tool_args": resolved_tool_args, + } + ) clarification_turns = 0 all_tool_calls: list[ToolCallEvent] = [] current_message = turn.message final_result: ChatResult | None = None + records: list[ClarificationRecord] = [] + answers_left = list(turn.set_answers) for _iter in range(max_clarification_turns + 1): - chat_result = client.send_message(conversation_id, current_message) + transcript.append(TranscriptEntry("user", current_message)) + incomplete = False + try: + chat_result = client.send_message(conversation_id, current_message) + except TurnIncompleteError as exc: + chat_result = exc.partial_result or ChatResult() + incomplete = True + except ChatError as exc: + # The stream can die after create_metric ran server-side; keep what the + # accumulator saw so the cleanup still knows what to delete. + if exc.partial_result is not None: + created.record(exc.partial_result.tool_call_events or []) + raise final_result = chat_result total_steps += chat_result.reasoning_step_count turn_offset, tool_index_offset, reasoning_index_offset = shift_and_index_events( @@ -444,26 +813,75 @@ def run_agentic_conversation( reasoning_index_offset=reasoning_index_offset, ) all_tool_calls.extend(chat_result.tool_call_events or []) + created.record(chat_result.tool_call_events or []) conversation_tool_call_events.extend(chat_result.tool_call_events or []) conversation_reasoning_step_events.extend(chat_result.reasoning_step_events or []) reasoning_steps.extend(chat_result.reasoning_steps or []) response_id = chat_result.response_id or response_id - if _check_output_present(resolved_turn, chat_result): - break - response_text = (chat_result.text_response or "").strip() if not response_text and chat_result.alert_proposals: response_text = render_alert_proposal(chat_result.alert_proposals[-1]) if not response_text: response_text = render_answer_text(chat_result) - if not response_text and not chat_result.tool_call_events: + transcript.append( + TranscriptEntry( + "assistant", " ".join(p for p in (response_text, summarize_visualizations(chat_result)) if p) + ) + ) + + if _check_output_present(resolved_turn, chat_result): + if incomplete: + records.append(ClarificationRecord(kind="turn_incomplete", agent_message=clip(response_text))) break - if clarification_turns >= max_clarification_turns: + + kind: ReplyRecordKind = "turn_incomplete" if incomplete else classify_reply(chat_result, response_text) + if ( + resolved_mode == "legacy" + and not incomplete + and not response_text + and not chat_result.tool_call_events + ): + records.append(ClarificationRecord(kind="no_action", agent_message="")) break + record = ClarificationRecord(kind=kind, agent_message=clip(response_text)) + if kind == "question": + # transcript ends [..., user: current_message, assistant: this reply]. + history, latest = render_transcript(transcript[:-2]), current_message + record.judge_input = judge_prompt(history, latest, response_text) + verdict = judge.judge(history, latest, response_text) + record.lost_context = verdict.lost_context + record.reasoning = verdict.reasoning + record.judge_error = verdict.error + # A second stall in the same turn ends it: one push is what a user gives. + second_stall = ( + resolved_mode == "context" + and kind in ("no_action", "turn_incomplete") + and any(r.kind in ("no_action", "turn_incomplete") for r in records) + ) + if clarification_turns >= max_clarification_turns or second_stall: + records.append(record) + break + + if resolved_mode == "legacy": + reply = _get_sim_user_response(response_text, resolved_turn, resolved_expected) + elif kind == "confirmation": + reply = CONFIRMATION_REPLY + elif kind in ("no_action", "turn_incomplete"): + reply = NUDGE_MESSAGE + elif record.lost_context is True and not any(r.lost_context is True for r in records): + # Once per turn, so a judge mistake cannot loop the user on restating. + reply = reply_restating(render_transcript(transcript[:-1]), response_text) + elif answers_left: + reply = answers_left.pop(0) + else: + reply = reply_from_facts(render_transcript(transcript[:-1]), response_text, turn.set_answers) + record.reply = clip(reply) + records.append(record) + clarification_turns += 1 total_clarification_turns += 1 - current_message = _get_sim_user_response(response_text, resolved_turn, resolved_expected) + current_message = reply # `declared` is what THIS turn's final set_skills call asked for (None when it # made no call); `active_skills` is what is actually active during the turn. No @@ -477,9 +895,10 @@ def run_agentic_conversation( active_skills = set(declared) skill_routing = turn.expected_skill in active_skills output_present = _check_output_present(resolved_turn, final_result) if final_result else False - output_correct = ( - _check_output_correct(resolved_turn, final_result) if (final_result and output_present) else None - ) + output_correct: bool | None = None + mismatches: list[str] = [] + if final_result and output_present: + output_correct, mismatches = _check_output_correct(resolved_turn, final_result, all_tool_calls) # Capture metric output for $ref resolution in subsequent turns. if final_result and turn.expected_output_type == "metric": @@ -487,11 +906,6 @@ def run_agentic_conversation( if metric_data: turn_outputs[turn.turn_id] = metric_data - # Track every metric created this turn (any turn may create one) for cleanup. - for metric_id in _extract_created_metric_ids(all_tool_calls): - if metric_id not in created_metric_ids: - created_metric_ids.append(metric_id) - turn_results.append( TurnResult( turn_id=turn.turn_id, @@ -503,14 +917,16 @@ def run_agentic_conversation( active_skills=sorted(active_skills), clarification_turns_used=clarification_turns, output_correct=output_correct, + mismatches=mismatches, + clarifications=records, + depends_on=list(turn.depends_on), ) ) finally: if owns_conversation and conversation_id: client.delete_conversation(conversation_id) - for metric_id in created_metric_ids: - _delete_metric(sdk, workspace_id, metric_id) + created.delete(sdk, workspace_id) client.close() # Not derived from TurnResult.activated_skills: that field carries only each turn's FINAL @@ -518,6 +934,8 @@ def run_agentic_conversation( # been activated. See ever_declared_skills' declaration above. full_skill_coverage = set(fixture.expected_skills).issubset(ever_declared_skills) conversation_success = all(tr.skill_success for tr in turn_results) + context_success, kept_rate, before_break, lost, stalled = _context_summary(turn_results, fixture) + judged = any(c.kind == "question" for tr in turn_results for c in tr.clarifications) return ConversationResult( conversation_id=conversation_id, @@ -530,13 +948,28 @@ def run_agentic_conversation( response_id=response_id, tool_call_events=conversation_tool_call_events, reasoning_step_events=conversation_reasoning_step_events, + context_success=context_success, + context_kept_rate=kept_rate, + turns_before_first_break=before_break, + lost_context_clarifications=lost, + stalled_turns=stalled, + mode=resolved_mode, + judge_model=judge.model_name if judged else None, ) def _conversation_detail(result: ConversationResult) -> dict: return { + "mode": result.mode, "full_skill_coverage": result.full_skill_coverage, + "conversation_success": result.conversation_success, + "context_success": result.context_success, + "context_kept_rate": result.context_kept_rate, + "turns_before_first_break": result.turns_before_first_break, + "lost_context_clarifications": result.lost_context_clarifications, + "stalled_turns": result.stalled_turns, "total_clarification_turns": result.total_clarification_turns, + "judge_model": result.judge_model, "turns": [tr.detail() for tr in result.turn_results], **timeline_detail(result.tool_call_events, result.reasoning_step_events), } @@ -562,16 +995,20 @@ def evaluate_agentic_conversation( run_metadata_extra: dict | None = None, reasoning_effort: ReasoningEffort | None = None, submit_trace_link: SubmitTraceLink = run_trace_link_inline, + mode: ConversationMode | None = None, + clarification_judge: ClarificationJudge | None = None, ) -> AgenticEvalOutcome: """Run conversation evaluation, log to Langfuse, and raise on failure. - Returns the conversation's outcome (reasoning_steps, conversation_id, response_id) as - an AgenticEvalOutcome on success; on failure the same three values are attached to the - raised exception as + ``mode`` (default: ``GD_EVAL_CONVERSATION_MODE``, else ``legacy``) picks which verdict + raises; both are always scored. Returns the conversation's outcome (reasoning_steps, + conversation_id, response_id) as an AgenticEvalOutcome on success; on failure the same + three values are attached to the raised exception as ``.reasoning_steps``/``.conversation_id``/``.response_id`` (mirrors the `conversation_id`-on-exception idiom in `ChatClient.ask()`) so callers can retrieve them either way. """ + resolved_mode = resolve_conversation_mode(mode) langfuse, window_start = open_trace_window(langfuse) result = run_agentic_conversation( host=host, @@ -582,7 +1019,10 @@ def evaluate_agentic_conversation( initial_conversation_id=initial_conversation_id, reasoning_effort=reasoning_effort, agent_id=agent_id, + mode=resolved_mode, + clarification_judge=clarification_judge, ) + passed = result.context_success if resolved_mode == "context" else result.conversation_success if langfuse is not None and dataset_item_id: # Pinned on the calling thread: a deferred poll must not widen its query window. @@ -590,6 +1030,7 @@ def evaluate_agentic_conversation( # Resolved here, not inside the task: deferring it would make the queued task hold # the whole fixture until the drain. ds_name = dataset_name or fixture.dataset_name + failed_turns = {tr.turn_id: tr.failure_reasons() for tr in result.turn_results if not tr.context_success} def _write_scores(ctx: RunTraceContext) -> None: @@ -599,8 +1040,11 @@ def _write_scores(ctx: RunTraceContext) -> None: 0, conversation_id=result.conversation_id, output={ + "mode": result.mode, "conversation_success": result.conversation_success, + "context_success": result.context_success, "full_skill_coverage": result.full_skill_coverage, + "failed_turns": failed_turns, }, ) as tid: ctx.score( @@ -609,12 +1053,30 @@ def _write_scores(ctx: RunTraceContext) -> None: value=float(result.conversation_success), data_type="BOOLEAN", ) + ctx.score( + tid, + name="context_success", + value=float(result.context_success), + data_type="BOOLEAN", + ) ctx.score( tid, name="full_skill_coverage", value=float(result.full_skill_coverage), data_type="BOOLEAN", ) + if result.context_kept_rate is not None: + ctx.score(tid, name="context_kept_rate", value=result.context_kept_rate, data_type="NUMERIC") + ctx.score( + tid, name="turns_before_first_break", value=result.turns_before_first_break, data_type="NUMERIC" + ) + ctx.score( + tid, + name="lost_context_clarifications", + value=result.lost_context_clarifications, + data_type="NUMERIC", + ) + ctx.score(tid, name="stalled_turns", value=result.stalled_turns, data_type="NUMERIC") # One turn per fixture turn, plus every simulated-user round the agent triggered. # The clarification count alone hides how much of the conversation the fixture # asked for, so the comparison needs the total. @@ -631,6 +1093,8 @@ def _write_scores(ctx: RunTraceContext) -> None: value=result.total_clarification_turns, data_type="NUMERIC", ) + # Reports read gate_passed first; it must be the verdict this run raised on. + log_gate_scores(ctx, tid, gate=None, pass_at_k=passed, pass_power_k=passed) for tr in result.turn_results: ctx.score( tid, @@ -638,12 +1102,15 @@ def _write_scores(ctx: RunTraceContext) -> None: value=float(tr.skill_success), data_type="BOOLEAN", ) + strict_checks = { + "conversation_success": result.conversation_success, + "full_skill_coverage": result.full_skill_coverage, + } + if resolved_mode == "context": + strict_checks["context_success"] = result.context_success ctx.quality( tid, - strict_checks={ - "conversation_success": result.conversation_success, - "full_skill_coverage": result.full_skill_coverage, - }, + strict_checks=strict_checks, latency_sec=pt.latency if pt else None, cost_usd=pt.total_cost if pt else None, ) @@ -673,13 +1140,18 @@ def _write_scores(ctx: RunTraceContext) -> None: detail = _conversation_detail(result) - if not result.conversation_success: - failed_turns = [tr for tr in result.turn_results if not tr.skill_success] - exc = ConversationAssertionError( - f"Conversation assertion failed. " - f"full_skill_coverage={result.full_skill_coverage}. " - f"Failed turns: {[t.turn_id for t in failed_turns]}" - ) + if not passed: + if resolved_mode == "context": + failed = {tr.turn_id: tr.failure_reasons() for tr in result.turn_results if not tr.context_success} + message = f"Conversation assertion failed (context). Failed turns: {failed}" + else: + legacy_failed = [tr.turn_id for tr in result.turn_results if not tr.skill_success] + message = ( + f"Conversation assertion failed. " + f"full_skill_coverage={result.full_skill_coverage}. " + f"Failed turns: {legacy_failed}" + ) + exc = ConversationAssertionError(message) exc.reasoning_steps = result.reasoning_steps exc.conversation_id = result.conversation_id exc.response_id = result.response_id diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py index 275609e84..b0c2578e8 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py @@ -205,7 +205,8 @@ def _extract_created_metric_ids(tool_call_events: list[ToolCallEvent]) -> list[s """Ids of every metric created by ``create_metric`` calls (a turn may create more than one). Used for cleanup so no created metric leaks — unlike ``_extract_metric_result``, which - returns only the first result for MAQL evaluation. Shared with conversation evaluation. + returns only the first result for MAQL evaluation. ``create_metric`` is an upsert: a result + with ``created_new: false`` replaced a metric that existed before, so it is not listed. """ metric_ids: list[str] = [] for tc in tool_call_events: @@ -215,7 +216,9 @@ def _extract_created_metric_ids(tool_call_events: list[ToolCallEvent]) -> list[s if not result_data: continue data = result_data.get("data", result_data) - metric_id = data.get("metric_id") if isinstance(data, dict) else None + if not isinstance(data, dict) or data.get("created_new") is False: + continue + metric_id = data.get("metric_id") if metric_id and metric_id not in metric_ids: metric_ids.append(metric_id) return metric_ids diff --git a/packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py b/packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py index 2127c88e3..bf2d602b2 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py @@ -38,6 +38,8 @@ # server bugs. A genuinely deterministic 500 still terminates, just after the bounded backoff. _RETRYABLE_STATUS_CODES: frozenset[int] = frozenset({429, 500, 502, 503, 504}) _METADATA_SYNC_MARKER = "METADATA_SYNC_IN_PROGRESS" +# `reason` values gen-ai puts on the error event of a turn that finished without an answer. +_TURN_INCOMPLETE_REASONS: frozenset[str] = frozenset({"max_iterations", "max_tokens", "content_filter"}) # Stands in for the `id` a persisted visualization would carry, on the fallback path # where the agent's create_adhoc_visualization call failed and only its arguments survive. _ADHOC_VIZ_ID = "adhoc-visualization-not-persisted" @@ -72,17 +74,29 @@ def __init__( status_code: int | None = None, detail: str | None = None, partial_result: ChatResult | None = None, + reason: str | None = None, ) -> None: super().__init__(message) self.status_code = status_code self.detail = detail self.partial_result = partial_result + self.reason = reason class TransientChatError(ChatError): """Retryable transient error: gen-ai temporarily unavailable or still syncing metadata.""" +class TurnIncompleteError(ChatError): + """The agent ended its turn without a final answer, e.g. at its iteration limit. + + gen-ai reports this as a 502 carrying a ``reason``, which the status code alone would + route to the transient retry. It is not transient: resending the question repeats the + same work, and on a conversation it also puts the question into the history twice. + Callers see it as the agent's own failure to answer. + """ + + class TurnTimeoutError(ChatError): """The agent exceeded a wall-clock budget -- either the per-turn one or the per-item one spanning every turn of a conversation. Not retryable: a slow turn stays slow, and @@ -383,6 +397,11 @@ def parse_sse_lines(lines: Iterable[str]) -> ChatResult: code = event_data.get("statusCode") detail = event_data.get("detail") message = f"SSE error {code}: {detail}" + reason = event_data.get("reason") + if reason in _TURN_INCOMPLETE_REASONS: + raise TurnIncompleteError( + message, status_code=code, detail=detail, partial_result=_build_chat_result(acc), reason=reason + ) if code in _RETRYABLE_STATUS_CODES: raise TransientChatError( message, status_code=code, detail=detail, partial_result=_build_chat_result(acc) diff --git a/packages/gooddata-eval/tests/test_agentic_conversation.py b/packages/gooddata-eval/tests/test_agentic_conversation.py index f9fa0c746..e720808a2 100644 --- a/packages/gooddata-eval/tests/test_agentic_conversation.py +++ b/packages/gooddata-eval/tests/test_agentic_conversation.py @@ -1,18 +1,46 @@ # (C) 2026 GoodData Corporation. All rights reserved. # SPDX-License-Identifier: LicenseRef-GoodData-Enterprise +import json as _json +import sys +import types +from datetime import date from unittest.mock import MagicMock, patch import pytest +from gooddata_eval.core.agentic import _conversation_context as ctx_mod +from gooddata_eval.core.agentic._conversation_context import ( + CONFIRMATION_REPLY, + NO_ANSWER_REPLY, + NUDGE_MESSAGE, + ClarificationJudge, + ClarificationVerdict, + _ClarificationLLM, + classify_reply, + judge_prompt, + reply_from_facts, + reply_restating, +) from gooddata_eval.core.agentic.conversation import ( ConversationAssertionError, ConversationFixture, + ConversationResult, TurnDefinition, TurnResult, + _canonical_maql, + _check_output_correct, + _created_alert_ids, + _expected_viz, _get_sim_user_response, + _metric_creations, _resolve_refs, evaluate_agentic_conversation, + resolve_conversation_mode, run_agentic_conversation, ) +from gooddata_eval.core.chat.render import render_answer_text +from gooddata_eval.core.chat.sse_client import ChatError as ChatError_ +from gooddata_eval.core.chat.sse_client import TurnIncompleteError +from gooddata_eval.core.evaluators._llm_judge import JudgeResponseError from gooddata_eval.core.models import ChatResult, ToolCallEvent @@ -126,7 +154,7 @@ 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 + assert set(_turn_result().detail()) == TurnResult._DETAIL_FIELDS | {"context_success", "failure_reasons"} def test_resolve_refs_no_refs(): @@ -946,8 +974,16 @@ def test_evaluate_agentic_conversation_returns_reasoning_steps_on_pass(): assert outcome.conversation_id == "conv-1" assert outcome.response_id == "resp-1" assert outcome.detail == { + "mode": "legacy", "full_skill_coverage": True, + "conversation_success": True, + "context_success": True, + "context_kept_rate": None, + "turns_before_first_break": 1, + "lost_context_clarifications": 0, + "stalled_turns": 0, "total_clarification_turns": 0, + "judge_model": None, "turns": [ { "turn_id": "t1", @@ -957,6 +993,12 @@ def test_evaluate_agentic_conversation_returns_reasoning_steps_on_pass(): "output_correct": None, "activated_skills": ["visualization"], "active_skills": ["visualization"], + "clarification_turns_used": 0, + "mismatches": [], + "clarifications": [], + "depends_on": [], + "context_success": True, + "failure_reasons": [], } ], "latency_breakdown": [], @@ -1011,8 +1053,16 @@ def test_evaluate_agentic_conversation_attaches_reasoning_steps_to_exception_on_ assert exc_info.value.conversation_id == "conv-1" assert exc_info.value.response_id == "resp-2" assert exc_info.value.detail == { + "mode": "legacy", "full_skill_coverage": False, + "conversation_success": False, + "context_success": False, + "context_kept_rate": None, + "turns_before_first_break": 0, + "lost_context_clarifications": 0, + "stalled_turns": 1, "total_clarification_turns": 0, + "judge_model": None, "turns": [ { "turn_id": "t1", @@ -1022,6 +1072,25 @@ def test_evaluate_agentic_conversation_attaches_reasoning_steps_to_exception_on_ "output_correct": None, "activated_skills": ["other_skill"], "active_skills": ["other_skill"], + "clarification_turns_used": 0, + "mismatches": [], + "clarifications": [ + { + "kind": "no_action", + "agent_message": "Here is something else", + "lost_context": None, + "reasoning": "", + "judge_error": None, + "judge_input": "", + "reply": "", + } + ], + "depends_on": [], + "context_success": False, + "failure_reasons": [ + "no output of the expected type", + "ended a reply without the expected output and without asking anything", + ], } ], "latency_breakdown": [], @@ -1131,3 +1200,748 @@ def _capture(_submit, _identity, **kwargs): assert scores["clarification_turns"] == 1 assert scores["turns"] == 2 # 1 fixture turn + 1 clarification round assert scores["steps"] == 5 + + +def _viz( + metric: str = "metric/gross_margin_percent", + dim: str = "label/dim_store.store_format", + filters: dict | None = None, + sort: list[dict] | None = None, +) -> dict: + query: dict = {"fields": {"m": {"using": metric}, "d": {"using": dim}}, "filter_by": filters or {}} + if sort: + query["sort_by"] = sort + return {"type": "bar_chart", "query": query, "metrics": ["m"], "view_by": ["d"]} + + +_EXPRESS = { + "f": {"type": "attribute_filter", "using": "label/dim_store.store_format", "state": {"include": ["Express"]}} +} + + +def _tool(name: str, args: dict | None = None, result: object = None) -> dict: + return { + "functionName": name, + "functionArguments": _json.dumps(args or {}), + "result": _json.dumps(result) if result is not None else None, + } + + +def _result( + text: str | None = "Done", viz: dict | None = None, tools: list[dict] | None = None, **extra: object +) -> ChatResult: + payload: dict = {"textResponse": text, "toolCallEvents": tools or []} + if viz is not None: + payload["createdVisualizations"] = {"objects": [viz]} + payload.update(extra) + return ChatResult.model_validate(payload) + + +def _viz_turn(turn_id: str = "t1", expected: dict | None = None, **kw: object) -> TurnDefinition: + return TurnDefinition( + turn_id=turn_id, + message=kw.pop("message", "chart it"), + expected_skill="visualization", + expected_output={"visualization": expected} if expected is not None else None, + **kw, + ) + + +def test_output_correct_reads_the_wrapped_visualization_and_passes_an_identical_chart() -> None: + ok, diff = _check_output_correct(_viz_turn(expected=_viz(filters=_EXPRESS)), _result(viz=_viz(filters=_EXPRESS))) + assert (ok, diff) == (True, []) + + +def test_output_correct_fails_a_chart_that_dropped_an_established_filter() -> None: + ok, diff = _check_output_correct(_viz_turn(expected=_viz(filters=_EXPRESS)), _result(viz=_viz())) + assert ok is False + assert any(d.startswith("attribute filters") for d in diff) + + +def test_output_correct_fails_a_filter_nobody_asked_for() -> None: + ok, diff = _check_output_correct(_viz_turn(expected=_viz()), _result(viz=_viz(filters=_EXPRESS))) + assert ok is False + assert any(d.startswith("attribute filters") for d in diff) + + +def test_output_correct_fails_a_different_metric() -> None: + ok, diff = _check_output_correct(_viz_turn(expected=_viz()), _result(viz=_viz(metric="metric/units_sold"))) + assert ok is False + assert diff[0].startswith("metrics") + + +def test_output_correct_accepts_any_listed_alternative() -> None: + turn = _viz_turn(expected=_viz(filters=_EXPRESS), expected_output_alternatives=[{"visualization": _viz()}]) + assert _check_output_correct(turn, _result(viz=_viz()))[0] is True + + +def test_output_correct_checks_sort_only_when_expected_states_it() -> None: + sort = [{"type": "metric_sort", "direction": "DESC", "metrics": ["m"]}] + assert _check_output_correct(_viz_turn(expected=_viz()), _result(viz=_viz(sort=sort)))[0] is True + ok, diff = _check_output_correct(_viz_turn(expected=_viz(sort=sort)), _result(viz=_viz())) + assert ok is False + assert diff[0].startswith("sort") + + +def test_output_correct_sort_without_a_direction_matches_either_direction() -> None: + by_units = [{"type": "metric_sort", "metrics": ["m"]}] + asc = [{"type": "metric_sort", "direction": "ASC", "metrics": ["m"]}] + assert _check_output_correct(_viz_turn(expected=_viz(sort=by_units)), _result(viz=_viz(sort=asc)))[0] is True + desc = [{"type": "metric_sort", "direction": "DESC", "metrics": ["m"]}] + assert _check_output_correct(_viz_turn(expected=_viz(sort=desc)), _result(viz=_viz(sort=asc)))[0] is False + + +def test_output_correct_is_unknown_when_nothing_is_expected() -> None: + assert _check_output_correct(_viz_turn(), _result(viz=_viz())) == (None, []) + + +def test_output_correct_compares_tool_arguments_across_the_turn() -> None: + turn = TurnDefinition( + turn_id="a", + message="alert me", + expected_skill="alert", + expected_output_type="tool_call", + expected_tool_name="create_metric_alert", + expected_tool_args={"metric_id": "units_sold", "threshold": 25}, + ) + call = ToolCallEvent.model_validate(_tool("create_metric_alert", {"metric_id": "units_sold", "threshold": 25})) + wrong = ToolCallEvent.model_validate(_tool("create_metric_alert", {"metric_id": "units_sold", "threshold": 250})) + assert _check_output_correct(turn, _result(), [call])[0] is True + assert _check_output_correct(turn, _result(), [wrong])[0] is False + + +def test_output_correct_compares_maql_for_a_metric_turn() -> None: + turn = TurnDefinition( + turn_id="m", + message="make it", + expected_skill="metric", + expected_output_type="metric", + expected_output={"maql": "SELECT {metric/a} / {metric/b}"}, + ) + good = ToolCallEvent.model_validate( + _tool("create_metric", result={"data": {"metric_id": "x", "maql": "SELECT {metric/a}/{metric/b}"}}) + ) + bad = ToolCallEvent.model_validate( + _tool("create_metric", result={"data": {"metric_id": "x", "maql": "SELECT {metric/a}"}}) + ) + assert _check_output_correct(turn, _result(), [good])[0] is True + assert _check_output_correct(turn, _result(), [bad])[0] is False + + +@pytest.mark.parametrize( + ("result", "kind"), + [ + (_result(text="Formula: SELECT 1. Should I create this metric?"), "confirmation"), + (_result(text=None, alertProposals=[{"cta": "Create?"}]), "confirmation"), + (_result(text="Which revenue metric do you mean?"), "question"), + (_result(text="Please select one of the five options by number, name, or ID."), "question"), + (_result(text="Reply with the number of the dashboard to use."), "question"), + (_result(text="Let me check what that maps to."), "no_action"), + (_result(text=None), "no_action"), + (_result(text="Pick one", unhandledParts=[{"type": "clarifyingQuestions"}]), "question"), + ], +) +def test_classify_reply(result: ChatResult, kind: str) -> None: + assert classify_reply(result, result.text_response or render_answer_text(result)) == kind + + +def test_resolve_conversation_mode(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("GD_EVAL_CONVERSATION_MODE", raising=False) + assert resolve_conversation_mode() == "legacy" + monkeypatch.setenv("GD_EVAL_CONVERSATION_MODE", "context") + assert resolve_conversation_mode() == "context" + assert resolve_conversation_mode("legacy") == "legacy" + with pytest.raises(ValueError, match="legacy"): + resolve_conversation_mode("strict") + + +class _FakeJudge: + """Grades every question with a fixed verdict and records what it was shown.""" + + model_name = "fake-judge" + + def __init__(self, lost_context: bool | None) -> None: + self.lost_context = lost_context + self.calls: list[tuple[str, str, str]] = [] + + def judge(self, history: str, latest: str, question: str) -> ClarificationVerdict: + self.calls.append((history, latest, question)) + return ClarificationVerdict(lost_context=self.lost_context, reasoning="because") + + +_SKILLS = _tool("set_skills", {"skill_names": ["visualization"]}) + + +def _run( + replies: list, turns: list[TurnDefinition], mode: str = "context", judge: _FakeJudge | None = None, **kw: object +) -> tuple[ConversationResult, MagicMock]: + client = MagicMock() + client.create_conversation.return_value = "conv-1" + client.send_message.side_effect = replies + fixture = ConversationFixture(id="c", expected_skills=["visualization"], turns=turns) + with ( + patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=client), + patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"), + ): + result = run_agentic_conversation( + host="h", token="t", workspace_id="ws", fixture=fixture, mode=mode, clarification_judge=judge, **kw + ) + return result, client + + +def test_context_mode_fails_a_turn_that_asks_for_what_the_conversation_established() -> None: + turns = [ + _viz_turn("t1", expected=_viz()), + _viz_turn("t2", expected=_viz(filters=_EXPRESS), message="its margin?", depends_on=["t1"]), + ] + judge = _FakeJudge(lost_context=True) + with patch( + "gooddata_eval.core.agentic.conversation.reply_restating", return_value="As I said earlier, Express." + ) as restate: + result, client = _run( + [ + _result(viz=_viz(), tools=[_SKILLS]), + _result(text="Which store format do you mean?"), + _result(viz=_viz(filters=_EXPRESS)), + ], + turns, + judge=judge, + ) + t2 = result.turn_results[1] + assert t2.skill_success is True # the legacy verdict still passes it + assert t2.lost_context is True + assert t2.context_success is False + assert "asked for information the conversation already established" in t2.failure_reasons() + assert result.conversation_success is True + assert result.context_success is False + assert result.context_kept_rate == 0.0 + assert result.turns_before_first_break == 1 + assert result.lost_context_clarifications == 1 + restate.assert_called_once() + assert client.send_message.call_args_list[2].args[1] == "As I said earlier, Express." + history, latest, question = judge.calls[0] + assert "chart it" in history + assert latest == "its margin?" + assert "its margin?" in t2.clarifications[0].judge_input + assert question == "Which store format do you mean?" + + +def test_context_mode_sends_the_set_answer_to_a_legitimate_question() -> None: + turns = [_viz_turn("t1", expected=_viz(), message="revenue by format", set_answers=["Gross Margin Percent."])] + result, client = _run( + [_result(text="Which metric do you mean?"), _result(viz=_viz(), tools=[_SKILLS])], + turns, + judge=_FakeJudge(lost_context=False), + ) + assert client.send_message.call_args_list[1].args[1] == "Gross Margin Percent." + assert result.turn_results[0].context_success is True + assert result.turn_results[0].clarifications[0].reply == "Gross Margin Percent." + + +def test_context_mode_answers_a_confirmation_with_yes_and_does_not_count_it_against_the_turn() -> None: + turns = [_viz_turn("t1", expected=_viz())] + judge = _FakeJudge(lost_context=True) + result, client = _run( + [_result(text="Should I create this chart?"), _result(viz=_viz(), tools=[_SKILLS])], turns, judge=judge + ) + assert client.send_message.call_args_list[1].args[1] == CONFIRMATION_REPLY + assert judge.calls == [] + assert result.turn_results[0].context_success is True + + +def test_context_mode_records_an_iteration_limit_as_a_stall_and_nudges() -> None: + turns = [_viz_turn("t1", expected=_viz())] + incomplete = TurnIncompleteError("SSE error 502", status_code=502, reason="max_iterations") + result, client = _run([incomplete, _result(viz=_viz(), tools=[_SKILLS])], turns, judge=_FakeJudge(False)) + turn = result.turn_results[0] + assert client.send_message.call_args_list[1].args[1] == NUDGE_MESSAGE + assert turn.output_present is True + assert turn.stalled is True + assert turn.context_success is False + assert result.stalled_turns == 1 + + +def test_context_mode_never_primes_the_simulated_user_with_the_expected_output() -> None: + turns = [_viz_turn("t1", expected=_viz())] + with patch("gooddata_eval.core.agentic.conversation._get_sim_user_response") as legacy_sim: + _run([_result(text="Which metric?"), _result(viz=_viz(), tools=[_SKILLS])], turns, judge=_FakeJudge(False)) + legacy_sim.assert_not_called() + + +def test_legacy_mode_keeps_the_primed_simulated_user_and_the_legacy_verdict() -> None: + turns = [_viz_turn("t1", expected=_viz(filters=_EXPRESS))] + with patch("gooddata_eval.core.agentic.conversation._get_sim_user_response", return_value="Express") as sim: + result, _ = _run( + [_result(text="Which format?"), _result(viz=_viz(), tools=[_SKILLS])], + turns, + mode="legacy", + judge=_FakeJudge(False), + ) + sim.assert_called_once() + assert result.conversation_success is True # legacy ignores the dropped filter + assert result.context_success is False # ...which context mode reports + assert result.turn_results[0].output_correct is False + + +def test_context_kept_rate_is_over_labelled_turns_only() -> None: + turns = [ + _viz_turn("t1", expected=_viz()), + _viz_turn("t2", expected=_viz(), depends_on=["t1"]), + _viz_turn("t3", expected=_viz(filters=_EXPRESS)), + ] + good = _result(viz=_viz(), tools=[_SKILLS]) + result, _ = _run([good, good, good], turns, judge=_FakeJudge(False)) + # t3 fails but is not labelled context-dependent, so the rate is t2 alone. + assert result.context_kept_rate == 1.0 + assert result.turns_before_first_break == 2 + assert result.context_success is False + + +def test_fresh_conversation_per_turn_gives_every_turn_an_empty_history() -> None: + turns = [_viz_turn("t1", expected=_viz()), _viz_turn("t2", expected=_viz())] + good = _result(viz=_viz(), tools=[_SKILLS]) + _, client = _run([good, good], turns, judge=_FakeJudge(False), fresh_conversation_per_turn=True) + assert client.create_conversation.call_count == 2 + assert client.delete_conversation.call_count == 2 + + +def test_cleanup_deletes_created_metrics_and_alerts_but_keeps_a_metric_updated_in_place() -> None: + turns = [_viz_turn("t1", expected=_viz())] + tools = [ + _SKILLS, + _tool("create_metric", result={"data": {"metric_id": "new_one", "maql": "SELECT 1", "created_new": True}}), + _tool("create_metric", result={"data": {"metric_id": "old_one", "maql": "SELECT 2", "created_new": False}}), + _tool("create_metric_alert", result={"id": "alert-1"}), + ] + with ( + patch("gooddata_eval.core.agentic.conversation._delete_metric") as del_metric, + patch("gooddata_eval.core.agentic.conversation._delete_alert") as del_alert, + ): + _run([_result(viz=_viz(), tools=tools)], turns, judge=_FakeJudge(False)) + assert [c.args[2] for c in del_metric.call_args_list] == ["new_one"] + assert [c.args[2] for c in del_alert.call_args_list] == ["alert-1"] + + +def test_cleanup_still_runs_for_a_metric_created_before_the_stream_died() -> None: + turns = [_viz_turn("t1", expected=_viz())] + partial = _result(tools=[_tool("create_metric", result={"data": {"metric_id": "orphan", "maql": "SELECT 1"}})]) + with ( + patch("gooddata_eval.core.agentic.conversation._delete_metric") as del_metric, + pytest.raises(ChatError_), + ): + _run([ChatError_("stream died", partial_result=partial)], turns, judge=_FakeJudge(False)) + assert [c.args[2] for c in del_metric.call_args_list] == ["orphan"] + + +def test_context_mode_pushes_a_stalled_turn_once_then_ends_it() -> None: + turns = [_viz_turn("t1", expected=_viz())] + stall = _result(text="Let me look into that.") + result, client = _run([stall, stall, stall], turns, judge=_FakeJudge(False)) + assert client.send_message.call_count == 2 + assert [c.kind for c in result.turn_results[0].clarifications] == ["no_action", "no_action"] + assert result.turn_results[0].context_success is False + + +@pytest.mark.parametrize(("mode", "gate"), [("legacy", True), ("context", False)]) +def test_scores_carry_both_verdicts_and_gate_on_the_mode(mode: str, gate: bool) -> None: + """A turn that shows the chart but drops an established filter: legacy passes it, context does not.""" + client = MagicMock() + client.create_conversation.return_value = "conv-1" + client.send_message.side_effect = [_result(viz=_viz(), tools=[_SKILLS])] + fixture = ConversationFixture( + id="c", expected_skills=["visualization"], turns=[_viz_turn("t1", expected=_viz(filters=_EXPRESS))] + ) + captured: dict = {} + + def _capture(_submit: object, _identity: object, **kwargs: object) -> None: + captured["write_scores"] = kwargs["write_scores"] + + with ( + patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=client), + patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"), + patch("gooddata_eval.core.agentic.conversation.submit_trace_scoring", _capture), + ): + try: + evaluate_agentic_conversation( + host="h", + token="t", + workspace_id="ws", + fixture=fixture, + langfuse=MagicMock(), + dataset_item_id="item-1", + mode=mode, + clarification_judge=_FakeJudge(False), + ) + raised = False + except ConversationAssertionError: + raised = True + assert raised is not gate + + ctx = MagicMock() + captured["write_scores"](ctx) + scores = {c.kwargs["name"]: c.kwargs["value"] for c in ctx.score.call_args_list} + assert scores["conversation_success"] == 1.0 + assert scores["context_success"] == 0.0 + assert scores["gate_passed"] is gate + assert scores["turns_before_first_break"] == 0 + + +def test_output_correct_treats_an_end_date_after_today_as_today() -> None: + """ "2025 and 2026 so far": the year end and today select the same data.""" + so_far = { + "f": {"type": "date_filter", "using": "dataset/transaction_date", "from": "2025-01-01", "to": "2026-12-31"} + } + today = { + "f": {"type": "date_filter", "using": "dataset/transaction_date", "from": "2025-01-01", "to": "2026-09-30"} + } + earlier = { + "f": {"type": "date_filter", "using": "dataset/transaction_date", "from": "2025-01-01", "to": "2026-06-30"} + } + with patch("gooddata_eval.core.agentic.conversation.date") as fake_date: + fake_date.today.return_value = date(2026, 9, 30) + assert _check_output_correct(_viz_turn(expected=_viz(filters=so_far)), _result(viz=_viz(filters=today)))[0] + assert not _check_output_correct(_viz_turn(expected=_viz(filters=so_far)), _result(viz=_viz(filters=earlier)))[ + 0 + ] + + +def _llm_returning(body: str) -> _ClarificationLLM: + llm = _ClarificationLLM.__new__(_ClarificationLLM) + llm.model = "fake" + llm._system_prompt = "" + choice = MagicMock() + choice.message.content = body + llm._create_completion = lambda messages: MagicMock(choices=[choice]) + return llm + + +def test_clarification_llm_reads_the_named_boolean() -> None: + assert _llm_returning('{"already_in_conversation": true, "reasoning": "t2 said so"}').already_in_conversation( + "p" + ) == (True, "t2 said so") + assert ( + _llm_returning('{"already_in_conversation": false, "reasoning": "no"}').already_in_conversation("p")[0] is False + ) + + +@pytest.mark.parametrize("body", ['{"score": 0}', '{"already_in_conversation": "yes"}', "not json", ""]) +def test_clarification_llm_refuses_anything_but_a_boolean_verdict(body: str) -> None: + with pytest.raises(JudgeResponseError): + _llm_returning(body).already_in_conversation("p") + + +def test_judge_prompt_marks_an_empty_history() -> None: + assert "CONVERSATION BEFORE:\n(empty)" in judge_prompt("", "Sort it", "By what?") + + +def test_context_mode_restates_at_most_once_per_turn() -> None: + turns = [_viz_turn("t1", expected=_viz(), set_answers=["Gross Margin Percent."])] + with patch( + "gooddata_eval.core.agentic.conversation.reply_restating", return_value="As I said earlier, X." + ) as restate: + _, client = _run( + [_result(text="Which one?"), _result(text="Which one, again?"), _result(viz=_viz(), tools=[_SKILLS])], + turns, + judge=_FakeJudge(lost_context=True), + ) + restate.assert_called_once() + assert client.send_message.call_args_list[2].args[1] == "Gross Margin Percent." + + +def test_output_correct_treats_a_single_value_in_as_equals() -> None: + turn = TurnDefinition( + turn_id="m", + message="make it", + expected_skill="metric", + expected_output_type="metric", + expected_output={"maql": 'SELECT {metric/a} WHERE {label/b} = "Digital"'}, + ) + same = ToolCallEvent.model_validate( + _tool( + "create_metric", + result={"data": {"metric_id": "x", "maql": 'SELECT {metric/a} WHERE {label/b} IN ("Digital")'}}, + ) + ) + two = ToolCallEvent.model_validate( + _tool( + "create_metric", + result={"data": {"metric_id": "x", "maql": 'SELECT {metric/a} WHERE {label/b} IN ("Digital", "Online")'}}, + ) + ) + assert _check_output_correct(turn, _result(), [same])[0] is True + assert _check_output_correct(turn, _result(), [two])[0] is False + + +def test_an_iteration_limit_after_the_chart_was_made_keeps_the_chart() -> None: + turns = [_viz_turn("t1", expected=_viz())] + partial = _result(viz=_viz(), tools=[_SKILLS]) + incomplete = TurnIncompleteError("SSE error 502", status_code=502, reason="max_iterations", partial_result=partial) + result, client = _run([incomplete], turns, judge=_FakeJudge(False)) + turn = result.turn_results[0] + assert client.send_message.call_count == 1 + assert turn.output_present is True and turn.output_correct is True + assert turn.stalled is True + + +def test_a_fresh_conversation_starts_with_no_active_skills() -> None: + turns = [_viz_turn("t1", expected=_viz()), _viz_turn("t2", expected=_viz())] + result, _ = _run( + [_result(viz=_viz(), tools=[_SKILLS]), _result(viz=_viz())], + turns, + judge=_FakeJudge(False), + fresh_conversation_per_turn=True, + ) + assert result.turn_results[1].skill_routing is False + + +def test_a_question_on_the_last_allowed_round_is_still_judged() -> None: + turns = [_viz_turn("t1", expected=_viz())] + judge = _FakeJudge(lost_context=True) + result, _ = _run([_result(text="Which chart?")], turns, judge=judge, max_clarification_turns=0) + assert result.turn_results[0].clarifications[0].lost_context is True + assert result.lost_context_clarifications == 1 + + +def test_not_in_is_not_rewritten_as_equals() -> None: + assert "not in" in _canonical_maql('SELECT {metric/a} WHERE {label/b} NOT IN ("x")') + assert _canonical_maql('SELECT {metric/a} WHERE {label/b} IN ("x")') == _canonical_maql( + 'SELECT {metric/a} WHERE {label/b} = "x"' + ) + + +def test_the_flat_expected_shape_is_still_checked() -> None: + turn = TurnDefinition( + turn_id="t", + message="m", + expected_skill="visualization", + expected_output={"metrics": ["metric/gross_margin_percent"], "dimensions": ["label/dim_store.store_format"]}, + ) + assert _check_output_correct(turn, _result(viz=_viz()))[0] is True + assert _check_output_correct(turn, _result(viz=_viz(metric="metric/units_sold")))[0] is False + + +# --- clarification judge ------------------------------------------------------------- + + +def _judge_with(llm: MagicMock) -> ClarificationJudge: + judge = ClarificationJudge(model="fake") + judge._llm = llm + return judge + + +def test_judge_reads_the_verdict_and_caches_it_per_prompt() -> None: + llm = MagicMock() + llm.already_in_conversation.return_value = (True, "t1 named it") + judge = _judge_with(llm) + first = judge.judge("USER: chart it", "Add Units Sold", "Which chart?") + second = judge.judge("USER: chart it", "Add Units Sold", "Which chart?") + assert first == second == ClarificationVerdict(lost_context=True, reasoning="t1 named it") + llm.already_in_conversation.assert_called_once() + judge.judge("", "Add Units Sold", "Which chart?") + assert llm.already_in_conversation.call_count == 2 + + +def test_an_unreadable_verdict_leaves_the_clarification_unjudged() -> None: + llm = MagicMock() + llm.already_in_conversation.side_effect = JudgeResponseError("no boolean") + verdict = _judge_with(llm).judge("", "x", "y") + assert verdict.lost_context is None + assert "no boolean" in verdict.error + + +def test_a_provider_fault_leaves_clarifications_unjudged_and_is_announced_once( + capsys: pytest.CaptureFixture[str], +) -> None: + llm = MagicMock() + llm.already_in_conversation.side_effect = RuntimeError("401 bad key") + judge = _judge_with(llm) + assert judge.judge("", "a", "q1").lost_context is None + assert judge.judge("", "b", "q2").error == "401 bad key" + assert capsys.readouterr().out.count("left unjudged") == 1 + + +def test_the_judge_needs_no_api_key_until_it_is_asked(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + judge = ClarificationJudge() + assert judge.model_name + assert judge.judge("", "a", "q").error is not None + + +def test_the_clarification_llm_uses_its_own_prompt(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + _fake_openai(monkeypatch, MagicMock()) + llm = _ClarificationLLM(model="gpt-4o") + assert "already_in_conversation" in llm._system_prompt + assert llm.model == "gpt-4o" + + +def test_the_clarification_llm_retries_an_empty_body_once() -> None: + llm = _llm_returning("") + calls: list[int] = [] + choice = MagicMock() + choice.message.content = '{"already_in_conversation": false, "reasoning": "r"}' + bodies = iter([MagicMock(choices=[]), MagicMock(choices=[choice])]) + llm._create_completion = lambda messages: calls.append(1) or next(bodies) + assert llm.already_in_conversation("p") == (False, "r") + assert len(calls) == 2 + + +# --- simulated user (context mode) ------------------------------------------------------- + + +def _fake_openai(monkeypatch: pytest.MonkeyPatch, openai_cls: MagicMock) -> MagicMock: + """Stand in for the optional openai package, whether or not it is installed.""" + monkeypatch.setitem(sys.modules, "openai", types.SimpleNamespace(OpenAI=openai_cls)) + return openai_cls + + +def _openai_returning(content: str | None) -> MagicMock: + response = MagicMock() + response.choices = [MagicMock()] + response.choices[0].message.content = content + client = MagicMock() + client.chat.completions.create.return_value = response + return MagicMock(return_value=client) + + +def test_reply_from_facts_without_facts_needs_no_llm(monkeypatch: pytest.MonkeyPatch) -> None: + openai_cls = _fake_openai(monkeypatch, MagicMock()) + assert reply_from_facts("USER: x", "Which?", []) == NO_ANSWER_REPLY + openai_cls.assert_not_called() + + +def test_reply_from_facts_answers_from_the_facts(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + openai_cls = _fake_openai(monkeypatch, _openai_returning(" Sales Order Revenue. ")) + assert reply_from_facts("USER: revenue by month", "Which revenue?", ["Sales Order Revenue."]) == ( + "Sales Order Revenue." + ) + prompt = openai_cls.return_value.chat.completions.create.call_args.kwargs["messages"][1]["content"] + assert "Sales Order Revenue." in prompt and "Which revenue?" in prompt + + +@pytest.mark.parametrize("failure", ["no_key", "provider_error", "empty_body"]) +def test_simulated_user_falls_back_when_the_llm_gives_nothing(monkeypatch: pytest.MonkeyPatch, failure: str) -> None: + if failure == "no_key": + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + openai_cls = _openai_returning("unused") + else: + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + openai_cls = _openai_returning(None) + if failure == "provider_error": + openai_cls.return_value.chat.completions.create.side_effect = RuntimeError("503") + _fake_openai(monkeypatch, openai_cls) + assert reply_from_facts("h", "q", ["fact"]) == NO_ANSWER_REPLY + assert reply_restating("h", "q").startswith("As I said earlier") + + +def test_reply_restating_returns_the_llm_restatement(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + _fake_openai(monkeypatch, _openai_returning("As I said earlier, Express.")) + assert reply_restating("USER: weakest? ASSISTANT: Express", "Which format?") == "As I said earlier, Express." + + +def test_simulated_user_reports_a_missing_openai_package(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setitem(sys.modules, "openai", None) + assert ctx_mod._chat("s", "u") is None + + +# --- turn verdict and content-check edges ------------------------------------------------- + + +def test_failure_reasons_name_an_unrun_turn_and_an_inactive_skill() -> None: + unrun = _turn_result().model_copy(update={"no_error": False}) + assert unrun.failure_reasons()[0] == "turn could not run" + wrong_skill = _turn_result().model_copy(update={"skill_routing": False}) + assert wrong_skill.failure_reasons() == ["skill 'visualization' not active"] + + +def test_expected_viz_is_none_for_a_shape_it_cannot_read() -> None: + assert _expected_viz({"visualization": "not a chart"}) is None + assert _expected_viz({"visualization": {"query": {"fields": {"m": 1}}}}) is None + assert _expected_viz({"visualization": {"query": {}}}) is None + + +def test_mismatches_name_the_dimension_and_the_type() -> None: + expected = {**_viz(), "type": "line_chart"} + ok, diff = _check_output_correct(_viz_turn(expected=expected), _result(viz=_viz(dim="label/dim_brand.brand_name"))) + assert ok is False + assert any(d.startswith("dimensions") for d in diff) + assert any(d.startswith("type") for d in diff) + + +def test_output_correct_without_anything_to_compare_against() -> None: + assert _check_output_correct(_viz_turn(expected=_viz()), _result()) == (False, ["no visualization to compare"]) + metric_turn = TurnDefinition( + turn_id="m", + message="make it", + expected_skill="metric", + expected_output_type="metric", + expected_output={"maql": "SELECT 1"}, + ) + assert _check_output_correct(metric_turn, _result(), []) == (False, ["no created metric to compare"]) + no_maql = metric_turn.model_copy(update={"expected_output": {"title": "x"}}) + assert _check_output_correct(no_maql, _result(), []) == (None, []) + + +def test_cleanup_ignores_tool_results_it_cannot_read() -> None: + unreadable = [ + ToolCallEvent.model_validate(_tool("create_metric", result=["not", "a", "dict"])), + ToolCallEvent.model_validate(_tool("create_metric", result={"data": {"isError": True}})), + ToolCallEvent.model_validate(_tool("create_metric_alert", result=["nope"])), + ToolCallEvent.model_validate(_tool("create_metric_alert", result={"data": {"id": "a-nested"}})), + ] + assert _metric_creations(unreadable) == [] + assert _created_alert_ids(unreadable) == ["a-nested"] + + +def test_fresh_conversation_per_turn_refuses_a_conversation_it_did_not_create() -> None: + fixture = ConversationFixture(id="c", expected_skills=["visualization"], turns=[_viz_turn("t1")]) + with ( + patch("gooddata_eval.core.agentic.conversation.ChatClient"), + patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"), + pytest.raises(ValueError, match="creates itself"), + ): + run_agentic_conversation( + host="h", + token="t", + workspace_id="ws", + fixture=fixture, + initial_conversation_id="given", + fresh_conversation_per_turn=True, + ) + + +def test_context_kept_rate_is_scored_when_there_are_dependent_turns() -> None: + client = MagicMock() + client.create_conversation.return_value = "conv-1" + client.send_message.side_effect = [_result(viz=_viz(), tools=[_SKILLS]), _result(viz=_viz())] + fixture = ConversationFixture( + id="c", + expected_skills=["visualization"], + turns=[_viz_turn("t1", expected=_viz()), _viz_turn("t2", expected=_viz(), depends_on=["t1"])], + ) + captured: dict = {} + with ( + patch("gooddata_eval.core.agentic.conversation.ChatClient", return_value=client), + patch("gooddata_eval.core.agentic.conversation.GoodDataSdk"), + patch( + "gooddata_eval.core.agentic.conversation.submit_trace_scoring", + lambda _s, _i, **kw: captured.update(write=kw["write_scores"]), + ), + ): + evaluate_agentic_conversation( + host="h", + token="t", + workspace_id="ws", + fixture=fixture, + langfuse=MagicMock(), + dataset_item_id="i", + mode="context", + clarification_judge=_FakeJudge(False), + ) + ctx = MagicMock() + captured["write"](ctx) + scores = {c.kwargs["name"]: c.kwargs["value"] for c in ctx.score.call_args_list} + assert scores["context_kept_rate"] == 1.0 + assert scores["turns_before_first_break"] == 2 diff --git a/packages/gooddata-eval/tests/test_agentic_metric_skill.py b/packages/gooddata-eval/tests/test_agentic_metric_skill.py index 73282e6f5..59b2fdff6 100644 --- a/packages/gooddata-eval/tests/test_agentic_metric_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_metric_skill.py @@ -1,5 +1,6 @@ # (C) 2026 GoodData Corporation. All rights reserved. # SPDX-License-Identifier: LicenseRef-GoodData-Enterprise +import json import sys import types from contextlib import ExitStack, contextmanager @@ -12,6 +13,7 @@ MetricSkillAssertionError, SimulatedResponseError, _delete_metric, + _extract_created_metric_ids, _extract_metric_result, _no_where_clause_hint, evaluate_agentic_metric_skill, @@ -853,3 +855,18 @@ def _capture(_submit, _identity, **kwargs): # they are counts, and a float reads as though a fraction of a turn were possible. assert isinstance(scores["turns"], int) assert isinstance(scores["steps"], int) + + +def test_extract_created_metric_ids_skips_a_metric_updated_in_place() -> None: + """create_metric is an upsert; deleting an updated metric would delete one that existed before.""" + + def call(mid: str, created_new: bool) -> ToolCallEvent: + return ToolCallEvent.model_validate( + { + "functionName": "create_metric", + "functionArguments": "{}", + "result": json.dumps({"data": {"metric_id": mid, "created_new": created_new}}), + } + ) + + assert _extract_created_metric_ids([call("new", True), call("old", False)]) == ["new"] diff --git a/packages/gooddata-eval/tests/test_sse_client.py b/packages/gooddata-eval/tests/test_sse_client.py index ae44fd9d1..c109ac391 100644 --- a/packages/gooddata-eval/tests/test_sse_client.py +++ b/packages/gooddata-eval/tests/test_sse_client.py @@ -4,7 +4,13 @@ import httpx import pytest from gooddata_eval.core.chat import sse_client as sse_mod -from gooddata_eval.core.chat.sse_client import ChatClient, ChatError, TransientChatError, parse_sse_lines +from gooddata_eval.core.chat.sse_client import ( + ChatClient, + ChatError, + TransientChatError, + TurnIncompleteError, + parse_sse_lines, +) from gooddata_eval.core.models import DatasetItem, ReasoningStepEvent, ToolCallEvent, build_latency_breakdown @@ -491,6 +497,30 @@ def test_parse_sse_lines_metadata_sync_marker_in_malformed_json_is_transient(): parse_sse_lines(["data: {bad json METADATA_SYNC_IN_PROGRESS"]) +@pytest.mark.parametrize("reason", ["max_iterations", "max_tokens", "content_filter"]) +def test_parse_sse_lines_turn_incomplete_is_not_transient(reason: str) -> None: + """gen-ai's "no final answer" error is a 502 with a reason; retrying it would resend the question.""" + lines = [ + 'data: {"item": {"role": "assistant", "content": {"type": "text", "text": "Let me check."}}}', + f'data: {{"statusCode": 502, "detail": "iteration limit", "reason": "{reason}"}}', + ] + with pytest.raises(TurnIncompleteError) as ei: + parse_sse_lines(lines) + assert not isinstance(ei.value, TransientChatError) + assert ei.value.reason == reason + assert ei.value.partial_result is not None + assert ei.value.partial_result.text_response == "Let me check." + + +@pytest.mark.parametrize("reason", [None, "unknown"]) +def test_parse_sse_lines_502_without_a_definite_reason_stays_transient(reason: str | None) -> None: + payload = {"statusCode": 502, "detail": "LLM provider is not responding"} + if reason: + payload["reason"] = reason + with pytest.raises(TransientChatError): + parse_sse_lines([f"data: {json.dumps(payload)}"]) + + def test_parse_sse_lines_non_retryable_status_is_chat_error_not_transient(): with pytest.raises(ChatError) as ei: parse_sse_lines(['data: {"statusCode": 400, "detail": "bad"}'])