From 57aa25f2311bde591059a1f37326ba6997074aba Mon Sep 17 00:00:00 2001 From: "my.nguyen" Date: Tue, 8 Sep 2026 18:16:28 +0700 Subject: [PATCH] fix(gooddata-eval): three false-negative sources in eval scoring Capture every multipart response part, not just text/visualization/ alertProposal, and render them into what the judge and the simulated user see. Normalize whitespace around MAQL punctuation before comparing, in one shared module the agentic and non-agentic metric comparators both use, and keep quoted literals out of every rewrite. Read the fixture's anomaly granularity, assert it, and stop telling the simulated user to refuse one, which deadlocked ANOMALY alert items. jira: QA-29230 risk: low Co-Authored-By: Claude Opus 5 (1M context) --- .../gooddata_eval/core/agentic/_catalog.py | 38 ++++++ .../gooddata_eval/core/agentic/alert_skill.py | 68 ++++++++-- .../core/agentic/conversation.py | 7 +- .../core/agentic/general_question.py | 3 +- .../gooddata_eval/core/agentic/guardrail.py | 3 +- .../gooddata_eval/core/agentic/kda_skill.py | 3 +- .../core/agentic/metric_skill.py | 74 ++--------- .../core/agentic/visualization.py | 6 +- .../src/gooddata_eval/core/chat/render.py | 47 +++++++ .../src/gooddata_eval/core/chat/sse_client.py | 36 +++++- .../gooddata_eval/core/evaluators/_maql.py | 103 +++++++++++++++ .../core/evaluators/_text_utils.py | 7 +- .../core/evaluators/metric_skill.py | 5 +- .../src/gooddata_eval/core/models.py | 2 + .../tests/test_agentic_alert_skill.py | 91 ++++++++++++++ .../tests/test_agentic_metric_skill.py | 50 +------- .../gooddata-eval/tests/test_chat_render.py | 118 ++++++++++++++++++ .../tests/test_maql_normalize.py | 106 ++++++++++++++++ 18 files changed, 630 insertions(+), 137 deletions(-) create mode 100644 packages/gooddata-eval/src/gooddata_eval/core/chat/render.py create mode 100644 packages/gooddata-eval/src/gooddata_eval/core/evaluators/_maql.py create mode 100644 packages/gooddata-eval/tests/test_chat_render.py create mode 100644 packages/gooddata-eval/tests/test_maql_normalize.py diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/_catalog.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/_catalog.py index 3d2555cdb..603c2811e 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/_catalog.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/_catalog.py @@ -2,6 +2,41 @@ from __future__ import annotations from dataclasses import dataclass, field +from enum import Enum + + +class AnomalyDetectionGranularity(str, Enum): + """Detection intervals an anomaly alert accepts. + + Mirrors gen-ai's enum of the same name; `StrEnum` is unavailable on the 3.10 floor, so + the `str` mixin carries the comparison against the raw tool argument. + """ + + HOUR = "HOUR" + DAY = "DAY" + WEEK = "WEEK" + MONTH = "MONTH" + QUARTER = "QUARTER" + YEAR = "YEAR" + + @classmethod + def parse(cls, value: object) -> AnomalyDetectionGranularity | None: + """Coerce a fixture value, or None when the fixture states none. + + Raises ValueError on an unknown interval: fixtures are hand-written, and a typo has + to fail before the run spends an API call rather than score the item against an + interval the product cannot produce. + """ + if value is None: + return None + candidate = str(value).strip().upper() + if not candidate: + return None + try: + return cls(candidate) + except ValueError: + expected = ", ".join(member.value for member in cls) + raise ValueError(f"Invalid granularity {value!r}; expected one of {expected}.") from None @dataclass @@ -30,6 +65,8 @@ class CatalogMetricAlert: """Attribute filters applied to the alert condition.""" attributes: list | None = None """Expected group-by attributes; ``None`` means the fixture states no expectation.""" + granularity: AnomalyDetectionGranularity | None = None + """Detection interval for an ANOMALY alert (DAY/WEEK/MONTH/...). Not a date filter.""" @classmethod def from_dict(cls, d: dict) -> CatalogMetricAlert: @@ -49,4 +86,5 @@ def from_dict(cls, d: dict) -> CatalogMetricAlert: recipients=recipients, filters=d.get("filters"), attributes=d.get("attributes"), + granularity=AnomalyDetectionGranularity.parse(d.get("granularity")), ) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py index e389bf559..cb1addf3d 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.py @@ -11,7 +11,7 @@ from gooddata_sdk import GoodDataSdk -from gooddata_eval.core.agentic._catalog import CatalogMetricAlert +from gooddata_eval.core.agentic._catalog import AnomalyDetectionGranularity, CatalogMetricAlert from gooddata_eval.core.agentic._trace_linker import ( RunIdentity, RunTraceContext, @@ -21,6 +21,7 @@ submit_trace_scoring, utc_now, ) +from gooddata_eval.core.chat.render import render_answer_text from gooddata_eval.core.chat.sse_client import ChatClient from gooddata_eval.core.config import ReasoningEffort from gooddata_eval.core.models import ( @@ -199,6 +200,22 @@ def _check_attributes(expected: CatalogMetricAlert, actual_args: dict) -> bool: return exp_ids == act_ids +def _check_granularity(expected: CatalogMetricAlert, actual_args: dict) -> bool: + """Compare the ANOMALY detection interval when the fixture states one. + + ``None`` means unasserted, mirroring ``attributes``: only the ANOMALY items carry a + ``Granularity``, and every other item must stay unaffected. The expectation is already + canonical by the time it lands here; the tool argument is a raw string, so only that + side needs folding. + """ + if expected.granularity is None: + return True + actual = actual_args.get("granularity") + if not actual: + return False + return str(actual).strip().upper() == expected.granularity.value + + def _check_metric(expected: CatalogMetricAlert, actual_args: dict) -> bool: if not expected.metric_id: return True @@ -330,20 +347,38 @@ def generate_simulated_alert_response( ) elif filters == []: filters_rule = ( - "5. Your alert must have NO filters and NO date/time window — it evaluates over all time. " - "If the agent asks which time period each check should cover, or offers a choice such as " - "'last Day / Week / Month', do NOT pick one: reply that you want no date filter at all, " - "all time. Never invent a period, a granularity or an 'evaluate each run on a X basis' " - "instruction the goal did not ask for.\n" + "5. Your alert must have NO filters and NO date/time window on the metric — it evaluates " + "over all time. If the agent asks which time period each check should cover, or offers a " + "choice such as 'last Day / Week / Month', do NOT pick one: reply that you want no date " + "filter at all, all time.\n" ) else: filters_rule = ( "5. Ask only for the filters your original request implies — do not invent an evaluation " - "period, granularity or date window that was not requested. If the agent offers a choice " + "period or date window that was not requested. If the agent offers a choice " "such as 'last Day / Week / Month' that your request never mentioned, say you do not want " "a date window.\n" ) + if operator == "ANOMALY": + # The fallback keeps the conversation alive when the fixture names no interval -- an + # anomaly alert cannot be created without one. It is deliberately NOT mirrored into + # `expected.granularity`: `_check_granularity` asserts only what the fixture stated, + # and scoring an item against an interval it never asked for is the defect this rule + # exists to undo. + granularity = (expected.granularity or AnomalyDetectionGranularity.DAY).value + anomaly_rule = ( + "7. This is an ANOMALY alert. Anomaly detection REQUIRES a time granularity, and that " + f"granularity is NOT a date filter. State it in your first reply and repeat it whenever " + f"asked: use {granularity} granularity. Rule 5 constrains filters on the metric only — it " + "never applies to this detection interval, so never refuse to give one.\n" + ) + else: + anomaly_rule = ( + "7. Do not invent an evaluation period, a granularity or an 'evaluate each run on a X " + "basis' instruction your goal never asked for.\n" + ) + original_request = f'Your original request to the agent was: "{question}"\n' if question else "" system_prompt = ( @@ -367,8 +402,7 @@ def generate_simulated_alert_response( " Do not wait for the agent to ask — state it alongside the metric and condition answers.\n" + filters_rule + f"6. Proactively state how often you want to be alerted in your first reply: {trigger_request}. " - " Repeat it if the agent proposes a different cadence.\n" - "Reply concisely and directly." + " Repeat it if the agent proposes a different cadence.\n" + anomaly_rule + "Reply concisely and directly." ) messages: list = [{"role": "system", "content": system_prompt}] @@ -408,6 +442,7 @@ class AlertEvaluation: metric_correct: bool recipients_correct: bool attributes_correct: bool = True + granularity_correct: bool = True @property def strict_pass(self) -> bool: @@ -421,6 +456,7 @@ def strict_pass(self) -> bool: self.metric_correct, self.recipients_correct, self.attributes_correct, + self.granularity_correct, ] ) @@ -539,6 +575,10 @@ def _normalize_expected_output(expected: dict) -> CatalogMetricAlert: filters = _normalize_expected_filters(expected) attributes = _normalize_expected_attributes(expected) + granularity = AnomalyDetectionGranularity.parse( + _case_insensitive_get(expected, "granularity", "detection granularity") + ) + return CatalogMetricAlert( operator=operator, threshold=threshold, @@ -549,6 +589,7 @@ def _normalize_expected_output(expected: dict) -> CatalogMetricAlert: recipients=recipients, filters=filters, attributes=attributes, + granularity=granularity, ) @@ -647,6 +688,8 @@ def _run_once(conv_id: str) -> AlertRunResult: 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) # Stop if agent gave a completely empty response (stuck) if not response_text and not chat_result.tool_call_events: break @@ -670,6 +713,7 @@ def _run_once(conv_id: str) -> AlertRunResult: metric_correct=tool_called and _check_metric(expected, actual_args), recipients_correct=tool_called and _check_recipients(expected, actual_args, sdk=sdk), attributes_correct=tool_called and _check_attributes(expected, actual_args), + granularity_correct=tool_called and _check_granularity(expected, actual_args), ) return AlertRunResult( conversation_id=conv_id, @@ -716,6 +760,7 @@ def _run_once(conv_id: str) -> AlertRunResult: r.eval.metric_correct, r.eval.recipients_correct, r.eval.attributes_correct, + r.eval.granularity_correct, ] ), ) @@ -791,6 +836,7 @@ def _write_scores(ctx: RunTraceContext) -> None: "metric_correct": ev.metric_correct, "recipients_correct": ev.recipients_correct, "attributes_correct": ev.attributes_correct, + "granularity_correct": ev.granularity_correct, } with ctx.observe(pt, run_idx) as tid: for score_name, value in strict_checks.items(): @@ -838,6 +884,7 @@ def _write_scores(ctx: RunTraceContext) -> None: "metric_correct": ev.metric_correct, "recipients_correct": ev.recipients_correct, "attributes_correct": ev.attributes_correct, + "granularity_correct": ev.granularity_correct, "actual_alert_arguments": best.actual_alert_arguments, "latency_breakdown": build_latency_breakdown(best.tool_call_events, best.reasoning_step_events), } @@ -849,7 +896,8 @@ def _write_scores(ctx: RunTraceContext) -> None: f"threshold_correct={ev.threshold_correct}, trigger_correct={ev.trigger_correct}, " f"filters_correct={ev.filters_correct}, metric_correct={ev.metric_correct}, " f"recipients_correct={ev.recipients_correct}, " - f"attributes_correct={ev.attributes_correct}. " + f"attributes_correct={ev.attributes_correct}, " + f"granularity_correct={ev.granularity_correct}. " f"Actual args: {best.actual_alert_arguments}" ) exc.reasoning_steps = best.reasoning_steps 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 2461fcd3a..919e4022c 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py @@ -22,6 +22,7 @@ ) 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.chat.render import render_answer_text from gooddata_eval.core.chat.sse_client import ChatClient from gooddata_eval.core.config import ReasoningEffort from gooddata_eval.core.models import ( @@ -214,7 +215,7 @@ def _check_output_correct(turn: TurnDefinition, chat_result: ChatResult) -> bool Returns None when expected_output is absent (presence check only). """ - from gooddata_eval.core.agentic.metric_skill import _normalize_maql # noqa: PLC0415 + from gooddata_eval.core.evaluators._maql import normalize_maql # noqa: PLC0415 otype = turn.expected_output_type expected = turn.expected_output @@ -256,7 +257,7 @@ def _check_output_correct(turn: TurnDefinition, chat_result: ChatResult) -> bool metric_result = _extract_metric_result(chat_result.tool_call_events or []) if not metric_result: return False - return _normalize_maql(metric_result.get("maql", "")) == _normalize_maql(expected.get("maql", "")) + return normalize_maql(metric_result.get("maql", "")) == normalize_maql(expected.get("maql", "")) return None @@ -451,6 +452,8 @@ def run_agentic_conversation( 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: break if clarification_turns >= max_clarification_turns: diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/general_question.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/general_question.py index 73e0de9cf..ae40fafd5 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/general_question.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/general_question.py @@ -15,6 +15,7 @@ submit_trace_scoring, utc_now, ) +from gooddata_eval.core.chat.render import render_answer_text from gooddata_eval.core.chat.sse_client import ChatClient from gooddata_eval.core.config import ReasoningEffort from gooddata_eval.core.evaluators._llm_judge import JudgeResponseError, LLMJudge, score_run @@ -112,7 +113,7 @@ def _run_single_general_question( item_started = time.monotonic() agent_started = time.monotonic() chat_result = client.send_message(conversation_id, question, user_context=user_context) - actual_output = (chat_result.text_response or "").strip() + actual_output = render_answer_text(chat_result) agent_elapsed = time.monotonic() - agent_started log_timer( f"[timer] general_question {conversation_id} GoodData response complete after " diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/guardrail.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/guardrail.py index 03b178c98..ff709c61d 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/guardrail.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/guardrail.py @@ -14,6 +14,7 @@ submit_trace_scoring, utc_now, ) +from gooddata_eval.core.chat.render import render_answer_text from gooddata_eval.core.chat.sse_client import ChatClient from gooddata_eval.core.config import ReasoningEffort from gooddata_eval.core.evaluators._llm_judge import JudgeResponseError, LLMJudge, score_run @@ -107,7 +108,7 @@ def _run_single_guardrail( and the remaining K-1) cannot drift -- they had already duplicated the whole body once. """ chat_result = client.send_message(conversation_id, question) - actual_output = (chat_result.text_response or "").strip() + actual_output = render_answer_text(chat_result) verdict = score_run(judge, input=question, expected_output=expected_output, actual_output=actual_output) return GuardrailResult( conversation_id=conversation_id, diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/kda_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/kda_skill.py index 0fb17ffde..9eda50d68 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/kda_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/kda_skill.py @@ -16,6 +16,7 @@ submit_trace_scoring, utc_now, ) +from gooddata_eval.core.chat.render import render_answer_text from gooddata_eval.core.chat.sse_client import ChatClient from gooddata_eval.core.config import ReasoningEffort from gooddata_eval.core.models import ( @@ -286,7 +287,7 @@ def _accumulate(result: ChatResult) -> None: response_id = chat_result.response_id or response_id _accumulate(chat_result) create_args, execute_result = _extract_kda_calls(chat_result.tool_call_events or []) - response_text = (chat_result.text_response or "").strip() + response_text = render_answer_text(chat_result) turn_completed = chat_result.stream_ended and bool(response_text) if create_args is not None: # This turn's own time -- the turn that called create, not any earlier 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 0479a9140..7d8f18454 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 @@ -20,8 +20,10 @@ submit_trace_scoring, utc_now, ) +from gooddata_eval.core.chat.render import render_answer_text from gooddata_eval.core.chat.sse_client import ChatClient from gooddata_eval.core.config import ReasoningEffort +from gooddata_eval.core.evaluators._maql import normalize_maql from gooddata_eval.core.models import ( AgenticAssertionError, AgenticEvalOutcome, @@ -40,76 +42,16 @@ _DEFAULT_K = 1 _DEFAULT_MAX_ITERATIONS = 7 -_IFNULL_RE = re.compile(r"IFNULL\s*\([^,]+,\s*0\)", re.IGNORECASE) -_SELECT_WRAP_RE = re.compile(r"^\s*\(\s*SELECT\s*\{([^}]+)\}\s*\)\s*$", re.IGNORECASE) -_INNER_SELECT_RE = re.compile(r"\(\s*SELECT\s*\{([^}]+)\}\s*\)", re.IGNORECASE) -# Matches whichever comes first: a {type/id} identifier reference or a quoted string -# literal -- both are case-sensitive data and must survive casefolding untouched. -# Everything else in MAQL (keywords, operators, numbers, punctuation) carries no -# case-sensitive meaning, per the MAQL reference (SELECT/BY/WHERE/FOR PREVIOUS/etc. -# are case-insensitive; only {..} identifiers and quoted literal values are not). -# Feeds _normalize_maql, the scoring comparator (_best_maql_match) -- do not widen this -# to handle \X escapes without confirming MAQL literals actually support backslash -# escaping (unconfirmed; see PR #1760 review). A wrong guess here silently changes -# maql_correct for the whole eval dataset, not just a hint. _no_where_clause_hint() -# below has its own, separately-scoped regex for that reason. -_PROTECTED_RE = re.compile(r"\{[^}]*\}|\"[^\"]*\"|'[^']*'") - - -def _strip_outer_parens(s: str) -> str: - """Strip one balanced layer of outer () if they wrap the entire expression.""" - if not (s.startswith("(") and s.endswith(")")): - return s - depth = 0 - for i, ch in enumerate(s): - if ch == "(": - depth += 1 - elif ch == ")": - depth -= 1 - if depth == 0 and i < len(s) - 1: - return s # Closing paren found before end — not a simple outer wrapper - return s[1:-1].strip() - - -def _casefold_outside_protected(s: str) -> str: - """Lowercase MAQL keywords/operators while preserving case-sensitive {type/id} - identifiers and quoted string literal values (e.g. WHERE {label/x} = "Active").""" - parts = [] - last = 0 - for m in _PROTECTED_RE.finditer(s): - parts.append(s[last : m.start()].lower()) - parts.append(m.group(0)) - last = m.end() - parts.append(s[last:].lower()) - return "".join(parts) - - -def _normalize_maql(maql: str) -> str: - """Semantic normalisation: strip whitespace, unwrap IFNULL/SELECT wrappers, casefold keywords.""" - if not maql: - return "" - m = maql.strip() - m = _IFNULL_RE.sub( - lambda mo: _strip_outer_parens(mo.group(0).split(",")[0].strip()[len("IFNULL(") :].strip()), - m, - ) - m = _SELECT_WRAP_RE.sub(r"{\1}", m) - m = _INNER_SELECT_RE.sub(r"{\1}", m) - m = re.sub(r"\{\s+", "{", m) - m = re.sub(r"\s+\}", "}", m) - m = re.sub(r"\s+", " ", m) - return _casefold_outside_protected(m.strip()) - def _best_maql_match(actual_maql: str, expected_outputs: list[dict]) -> tuple[bool, str]: """Try actual MAQL against every candidate; return (matched, best_expected_maql). First match wins. First candidate is used for error reporting when none match. """ - normalized_actual = _normalize_maql(actual_maql) + normalized_actual = normalize_maql(actual_maql) for candidate in expected_outputs: expected_maql = candidate.get("maql", "") - if normalized_actual == _normalize_maql(expected_maql): + if normalized_actual == normalize_maql(expected_maql): return True, expected_maql return False, expected_outputs[0].get("maql", "") if expected_outputs else "" @@ -122,9 +64,9 @@ class SimulatedResponseError(RuntimeError): """ -# Separate from _PROTECTED_RE on purpose: this one only feeds a same-turn LLM-prompt hint -# (see _no_where_clause_hint), never the scoring comparator, so it can afford to consume -# \X escape sequences inside quoted literals without risking maql_correct semantics. +# Separate from evaluators._maql._PROTECTED_RE on purpose: this one only feeds a same-turn +# LLM-prompt hint (see _no_where_clause_hint), never the scoring comparator, so it can afford +# to consume \X escape sequences inside quoted literals without risking maql_correct semantics. _HINT_PROTECTED_RE = re.compile(r"\{[^}]*\}|\"(?:[^\"\\]|\\.)*\"|'(?:[^'\\]|\\.)*'") @@ -342,6 +284,8 @@ def _execute_single_metric_run( metric_result = candidate break response_text = (chat_result.text_response or "").strip() + if not response_text: + response_text = render_answer_text(chat_result) if not response_text and not chat_result.tool_call_events: break if _iteration >= max_iterations - 1: diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/visualization.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/visualization.py index 85978d824..12faaaffb 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/visualization.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/visualization.py @@ -19,6 +19,7 @@ submit_trace_scoring, utc_now, ) +from gooddata_eval.core.chat.render import render_answer_text from gooddata_eval.core.chat.sse_client import ChatClient from gooddata_eval.core.config import ReasoningEffort from gooddata_eval.core.evaluators.visualization import ( @@ -207,12 +208,13 @@ def _execute_single_run( viz_produced = bool(current_result.created_visualizations and current_result.created_visualizations.objects) if viz_produced: break - if not current_result.text_response: + response_text = render_answer_text(current_result) + if not response_text: break if iteration >= max_iterations - 1: break - follow_up = generate_simulated_response(current_result.text_response, simulated_response_guide) + follow_up = generate_simulated_response(response_text, simulated_response_guide) current_result = client.send_message(conversation_id, follow_up) skill_activated = _check_visualization_skill_activated(all_tool_call_events) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/chat/render.py b/packages/gooddata-eval/src/gooddata_eval/core/chat/render.py new file mode 100644 index 000000000..ca3f3d278 --- /dev/null +++ b/packages/gooddata-eval/src/gooddata_eval/core/chat/render.py @@ -0,0 +1,47 @@ +# (C) 2026 GoodData Corporation +"""Render a chat turn's non-text parts into the prose an evaluator can read.""" + +import json + +from gooddata_eval.core.models import ChatResult + +_MAX_UNHANDLED_PART_CHARS = 2000 + + +def render_search_results(part: dict) -> str: + """Render one ``searchResults`` part as the list of objects it resolved.""" + objects = part.get("objects") or [] + if not objects: + return "" + lines = [] + for obj in objects: + title = str(obj.get("title") or "").strip() + obj_id = str(obj.get("id") or "").strip() + obj_type = str(obj.get("type") or "").strip() + label = f"{title} ({obj_type}/{obj_id})" if obj_id else title + description = str(obj.get("description") or "").strip() + lines.append(f"- {label}: {description}" if description else f"- {label}") + requested = str(part.get("requestedObjectType") or "object").strip() + return f"Search results ({len(objects)} {requested}):\n" + "\n".join(lines) + + +def render_unhandled_part(part: dict) -> str: + """Best-effort rendering of a part gd-eval does not model, truncated to a sane size.""" + ptype = str(part.get("type") or "unknown") + body = json.dumps({k: v for k, v in part.items() if k != "type"}, ensure_ascii=False) + if len(body) > _MAX_UNHANDLED_PART_CHARS: + body = body[:_MAX_UNHANDLED_PART_CHARS] + "… (truncated)" + return f"[{ptype}]\n{body}" + + +def render_answer_text(result: ChatResult) -> str: + """Everything the agent said this turn: prose plus the content-bearing parts. + + Alert proposals are excluded -- ``agentic.alert_skill.render_alert_proposal`` renders + those, and importing it here would close a cycle. Returns "" for a turn that produced + nothing, so callers can keep using falsiness as their "the agent is stuck" signal. + """ + chunks = [(result.text_response or "").strip()] + chunks += [render_search_results(part) for part in result.search_results] + chunks += [render_unhandled_part(part) for part in result.unhandled_parts] + return "\n\n".join(chunk for chunk in chunks if chunk) 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 55c14fb6b..27ca91d26 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 @@ -35,6 +35,20 @@ _RETRYABLE_STATUS_CODES: frozenset[int] = frozenset({429, 502, 503, 504}) _METADATA_SYNC_MARKER = "METADATA_SYNC_IN_PROGRESS" +_KNOWN_PART_TYPES: frozenset[str] = frozenset( + { + "text", + "visualization", + "dashboard", + "dashboardPatch", + "kda", + "whatIf", + "searchResults", + "alertProposal", + "clarifyingQuestions", + } +) + class ChatError(RuntimeError): """Non-retryable error reported by the chat SSE stream. @@ -124,6 +138,8 @@ class _SseAccumulator: viz_reasoning_parts: list[str] = field(default_factory=list) visualizations: list[dict[str, Any]] = field(default_factory=list) alert_proposals: list[dict[str, Any]] = field(default_factory=list) + search_results: list[dict[str, Any]] = field(default_factory=list) + unhandled_parts: list[dict[str, Any]] = field(default_factory=list) tool_call_events: list[dict[str, Any]] = field(default_factory=list) call_id_to_event_index: dict[str, int] = field(default_factory=dict) reasoning_steps: list[dict[str, Any]] = field(default_factory=list) @@ -151,13 +167,20 @@ def _handle_multipart(content: dict[str, Any], acc: _SseAccumulator) -> None: if t: acc.text_parts.append(t) acc.viz_reasoning_parts.append(t) - elif ptype == "visualization" and part.get("visualization"): - acc.visualizations.append(part["visualization"]) + elif ptype == "visualization": + if part.get("visualization"): + acc.visualizations.append(part["visualization"]) elif ptype == "alertProposal": # Record the part even when the server could not resolve the proposal payload # (``alertProposal: null``) — its mere presence is the confirmation signal, and # the reader falls back to a default CTA. acc.alert_proposals.append(part.get("alertProposal") or {}) + elif ptype == "searchResults": + acc.search_results.append(part) + else: + if ptype not in _KNOWN_PART_TYPES: + _log.warning("unknown multipart part type %r; captured as an unhandled part", ptype) + acc.unhandled_parts.append(part) def _handle_reasoning(content: dict[str, Any], acc: _SseAccumulator) -> None: @@ -198,9 +221,18 @@ def _handle_tool_result(content: dict[str, Any], acc: _SseAccumulator) -> None: def _build_chat_result(acc: _SseAccumulator) -> ChatResult: + if not acc.text_parts and (acc.search_results or acc.unhandled_parts): + _log.warning( + "assistant turn produced no text part; %d non-text part(s) captured (%d searchResults, %d unhandled)", + len(acc.search_results) + len(acc.unhandled_parts), + len(acc.search_results), + len(acc.unhandled_parts), + ) payload: dict[str, Any] = { "textResponse": "\n".join(acc.text_parts) or None, "alertProposals": acc.alert_proposals, + "searchResults": acc.search_results, + "unhandledParts": acc.unhandled_parts, "toolCallEvents": acc.tool_call_events, "reasoningStepCount": len(acc.reasoning_steps), "reasoningSteps": [step["summary"] for step in acc.reasoning_steps], diff --git a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/_maql.py b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/_maql.py new file mode 100644 index 000000000..fef1d2139 --- /dev/null +++ b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/_maql.py @@ -0,0 +1,103 @@ +# (C) 2026 GoodData Corporation +"""MAQL normalisation shared by every metric comparator.""" + +import re +from typing import Callable + +_IFNULL_RE = re.compile(r"IFNULL\s*\(([^,]+),\s*0\)", re.IGNORECASE) +_SELECT_WRAP_RE = re.compile(r"^\s*\(\s*SELECT\s*\{([^}]+)\}\s*\)\s*$", re.IGNORECASE) +_INNER_SELECT_RE = re.compile(r"\(\s*SELECT\s*\{([^}]+)\}\s*\)", re.IGNORECASE) +# Matches whichever comes first: a {type/id} identifier reference or a quoted string +# literal -- both are case-sensitive data and must survive casefolding untouched. +# Everything else in MAQL (keywords, operators, numbers, punctuation) carries no +# case-sensitive meaning, per the MAQL reference (SELECT/BY/WHERE/FOR PREVIOUS/etc. +# are case-insensitive; only {..} identifiers and quoted literal values are not). +# Feeds normalize_maql, the scoring comparator (_best_maql_match) -- do not widen this +# to handle \X escapes without confirming MAQL literals actually support backslash +# escaping (unconfirmed; see PR #1760 review). A wrong guess here silently changes +# maql_correct for the whole eval dataset, not just a hint. _no_where_clause_hint has its +# own, separately-scoped regex for that reason. +_PROTECTED_RE = re.compile(r"\{[^}]*\}|\"[^\"]*\"|'[^']*'") + + +_MASK_RE = re.compile(r"\x00(\d+)\x00") + + +def _mask_quoted_literals(s: str) -> tuple[str, list[str]]: + """Replace quoted string literals with placeholders, returning the text and the literals. + + A literal's contents are data: the IFNULL/SELECT unwrapping and brace-whitespace + rewrites below must not reach inside one. Splitting on _PROTECTED_RE rather than on + quotes alone means a quote character inside a {type/id} reference cannot open a bogus + literal. Brace regions are deliberately left in place -- normalising the whitespace + inside {..} is exactly what those rewrites are for. + """ + literals: list[str] = [] + + def _replace(match: re.Match) -> str: + token = match.group(0) + if token[0] not in "\"'": + return token + literals.append(token) + return f"\x00{len(literals) - 1}\x00" + + return _PROTECTED_RE.sub(_replace, s), literals + + +def _unmask_quoted_literals(s: str, literals: list[str]) -> str: + """Put the literals from _mask_quoted_literals back, verbatim.""" + return _MASK_RE.sub(lambda m: literals[int(m.group(1))], s) + + +def _strip_outer_parens(s: str) -> str: + """Strip one balanced layer of outer () if they wrap the entire expression.""" + if not (s.startswith("(") and s.endswith(")")): + return s + depth = 0 + for i, ch in enumerate(s): + if ch == "(": + depth += 1 + elif ch == ")": + depth -= 1 + if depth == 0 and i < len(s) - 1: + return s # Closing paren found before end — not a simple outer wrapper + return s[1:-1].strip() + + +def apply_outside_protected(s: str, fn: Callable[[str], str]) -> str: + """Run ``fn`` on the text between ``{..}`` / ``".."`` / ``'..'`` regions only, + preserving case-sensitive {type/id} identifiers and quoted string literal values.""" + parts: list[str] = [] + last = 0 + for m in _PROTECTED_RE.finditer(s): + parts.append(fn(s[last : m.start()])) + parts.append(m.group(0)) + last = m.end() + parts.append(fn(s[last:])) + return "".join(parts) + + +def _tighten_punctuation(seg: str) -> str: + """Drop whitespace next to parens, commas and operators -- it carries no meaning in MAQL.""" + seg = re.sub(r"\s*\(\s*", "(", seg) + seg = re.sub(r"\s*\)", ")", seg) + seg = re.sub(r"\s*,\s*", ",", seg) + seg = re.sub(r"\s*([-+*/=<>])\s*", r"\1", seg) + return seg + + +def normalize_maql(maql: str) -> str: + """Semantic normalisation: strip whitespace, unwrap IFNULL/SELECT wrappers, casefold + keywords, tighten punctuation.""" + if not maql: + return "" + m, literals = _mask_quoted_literals(maql.strip()) + m = _IFNULL_RE.sub(lambda mo: _strip_outer_parens(mo.group(1).strip()), m) + m = _SELECT_WRAP_RE.sub(r"{\1}", m) + m = _INNER_SELECT_RE.sub(r"{\1}", m) + m = re.sub(r"\{\s+", "{", m) + m = re.sub(r"\s+\}", "}", m) + m = re.sub(r"\s+", " ", m) + normalized = apply_outside_protected(m.strip(), str.lower) + normalized = apply_outside_protected(normalized, _tighten_punctuation) + return _unmask_quoted_literals(normalized, literals) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/_text_utils.py b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/_text_utils.py index ff14438b7..356a829fa 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/_text_utils.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/_text_utils.py @@ -1,11 +1,10 @@ # (C) 2026 GoodData Corporation """Shared text-extraction helpers for text-answer evaluators.""" +from gooddata_eval.core.chat.render import render_answer_text from gooddata_eval.core.models import ChatResult def extract_text(chat_result: ChatResult) -> str: - """Extract the agent's text response, stripping whitespace.""" - if chat_result.text_response: - return chat_result.text_response.strip() - return "" + """Extract the agent's answer: prose plus any content-bearing non-text part.""" + return render_answer_text(chat_result) diff --git a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/metric_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/metric_skill.py index 622399273..9c2ebb7fa 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/evaluators/metric_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/evaluators/metric_skill.py @@ -1,6 +1,7 @@ # (C) 2026 GoodData Corporation """Evaluator for metric_skill: agent must create the correct metric via create_metric tool call.""" +from gooddata_eval.core.evaluators._maql import normalize_maql from gooddata_eval.core.evaluators.base import ItemEvaluation from gooddata_eval.core.models import ChatResult, DatasetItem, build_latency_breakdown @@ -47,8 +48,8 @@ def evaluate(self, item: DatasetItem, chat_result: ChatResult) -> ItemEvaluation expected_maql = expected.get("maql", "") expected_format = expected.get("format", "") - maql_correct = actual_maql == expected_maql - format_correct = actual_format == expected_format + maql_correct = normalize_maql(actual_maql) == normalize_maql(expected_maql) + format_correct = actual_format.strip() == expected_format.strip() passed = maql_correct and format_correct return ItemEvaluation( diff --git a/packages/gooddata-eval/src/gooddata_eval/core/models.py b/packages/gooddata-eval/src/gooddata_eval/core/models.py index 824e77c3e..a1f1d5165 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/models.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/models.py @@ -213,6 +213,8 @@ class ChatResult(BaseModel): # step emits ONLY this part (no text part), so its `cta` is the only "the agent is asking # a question" signal the simulated-user loops can key off. alert_proposals: list[dict] = Field(default_factory=list, alias="alertProposals") + search_results: list[dict] = Field(default_factory=list, alias="searchResults") + unhandled_parts: list[dict] = Field(default_factory=list, alias="unhandledParts") tool_call_events: list[ToolCallEvent] = Field(default_factory=list, alias="toolCallEvents") reasoning_step_count: int = Field(default=0, alias="reasoningStepCount") reasoning_steps: list[str] = Field(default_factory=list, alias="reasoningSteps") diff --git a/packages/gooddata-eval/tests/test_agentic_alert_skill.py b/packages/gooddata-eval/tests/test_agentic_alert_skill.py index 246b90e58..cf812609d 100644 --- a/packages/gooddata-eval/tests/test_agentic_alert_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_alert_skill.py @@ -4,11 +4,13 @@ from unittest.mock import MagicMock, patch import pytest +from gooddata_eval.core.agentic._catalog import AnomalyDetectionGranularity from gooddata_eval.core.agentic.alert_skill import ( AlertEvaluation, AlertSkillAssertionError, _check_attributes, _check_filters, + _check_granularity, _check_recipients, _check_trigger, _deep_subset, @@ -686,6 +688,7 @@ def test_evaluate_agentic_alert_skill_returns_reasoning_steps_on_pass(): "metric_correct": True, "recipients_correct": True, "attributes_correct": True, + "granularity_correct": True, "actual_alert_arguments": {"operator": "GREATER_THAN", "threshold": 500}, "latency_breakdown": [], } @@ -725,6 +728,7 @@ def test_evaluate_agentic_alert_skill_attaches_reasoning_steps_to_exception_on_f "metric_correct": False, "recipients_correct": False, "attributes_correct": False, + "granularity_correct": False, "actual_alert_arguments": {}, "latency_breakdown": [], } @@ -905,3 +909,90 @@ def test_alert_evaluation_attributes_correct_defaults_to_true(): ) assert ev.attributes_correct is True assert ev.strict_pass is True + + +# --- ANOMALY granularity ---------------------------------------------------------------- + + +def test_sim_user_supplies_the_granularity_an_anomaly_alert_needs(): + prompt = _sim_user_prompt({"Operator": "ANOMALY", "Granularity": "DAY", "Time window/Filters": "None (All time)"}) + assert "use DAY granularity" in prompt + assert "never refuse to give one" in prompt + + +def test_sim_user_is_not_told_to_refuse_a_granularity_for_an_anomaly_alert(): + prompt = _sim_user_prompt({"Operator": "ANOMALY", "Time window/Filters": "None (All time)"}) + assert "invent" not in prompt.lower() + + +def test_sim_user_still_refuses_an_unrequested_granularity_for_a_normal_alert(): + prompt = _sim_user_prompt({"Operator": "GREATER_THAN", "Time window/Filters": "None (All time)"}) + assert "Do not invent an evaluation period, a granularity" in prompt + assert "no date filter at all" in prompt + + +def test_sim_user_falls_back_to_day_when_an_anomaly_item_states_no_granularity(): + prompt = _sim_user_prompt({"Operator": "ANOMALY"}) + assert "use DAY granularity" in prompt + + +def test_normalize_expected_output_reads_granularity(): + assert _normalize_expected_output({"Operator": "ANOMALY", "Granularity": "WEEK"}).granularity == "WEEK" + assert _normalize_expected_output({"Operator": "ANOMALY"}).granularity is None + + +def test_granularity_is_not_mistaken_for_a_filter(): + expected = _normalize_expected_output( + {"Operator": "ANOMALY", "Granularity": "DAY", "Time window/Filters": "None (All time)"} + ) + assert expected.filters == [] + assert expected.granularity == "DAY" + + +def test_granularity_is_asserted_when_the_fixture_states_one(): + expected = _normalize_expected_output({"Operator": "ANOMALY", "Granularity": "DAY"}) + assert _check_granularity(expected, {"granularity": "DAY"}) is True + assert _check_granularity(expected, {"granularity": "MONTH"}) is False + assert _check_granularity(expected, {}) is False + + +def test_granularity_comparison_is_case_insensitive(): + expected = _normalize_expected_output({"Operator": "ANOMALY", "Granularity": "day"}) + assert _check_granularity(expected, {"granularity": "DAY"}) is True + + +def test_granularity_is_unasserted_when_the_fixture_states_none(): + expected = _normalize_expected_output({"Operator": "GREATER_THAN"}) + assert _check_granularity(expected, {"granularity": "MONTH"}) is True + assert _check_granularity(expected, {}) is True + + +def test_a_mismatched_granularity_fails_strict_pass(): + ev = AlertEvaluation( + alert_created=True, + operator_correct=True, + threshold_correct=True, + trigger_correct=True, + filters_correct=True, + metric_correct=True, + recipients_correct=True, + granularity_correct=False, + ) + assert ev.strict_pass is False + + +def test_granularity_is_canonicalised_to_the_enum(): + expected = _normalize_expected_output({"Operator": "ANOMALY", "Granularity": " week "}) + assert expected.granularity is AnomalyDetectionGranularity.WEEK + + +def test_an_unknown_granularity_is_rejected_before_the_run_starts(): + with pytest.raises(ValueError, match="Invalid granularity"): + _normalize_expected_output({"Operator": "ANOMALY", "Granularity": "fortnight"}) + + +def test_every_gen_ai_interval_is_accepted(): + for value in ("HOUR", "DAY", "WEEK", "MONTH", "QUARTER", "YEAR"): + assert AnomalyDetectionGranularity.parse(value.lower()) is AnomalyDetectionGranularity(value) + assert AnomalyDetectionGranularity.parse(None) is None + assert AnomalyDetectionGranularity.parse(" ") is None diff --git a/packages/gooddata-eval/tests/test_agentic_metric_skill.py b/packages/gooddata-eval/tests/test_agentic_metric_skill.py index e2bc5ff56..bc36fe4f0 100644 --- a/packages/gooddata-eval/tests/test_agentic_metric_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_metric_skill.py @@ -14,7 +14,6 @@ _delete_metric, _extract_metric_result, _no_where_clause_hint, - _normalize_maql, evaluate_agentic_metric_skill, generate_simulated_response, run_agentic_metric_skill, @@ -137,14 +136,6 @@ def test_extract_metric_result_skips_an_empty_payload(): assert _extract_metric_result(calls) == {"metric_id": "m2"} -def test_normalize_maql_strips_whitespace(): - assert _normalize_maql(" SELECT { metric/foo } ") == "select {metric/foo}" - - -def test_normalize_maql_removes_select_wrapper(): - assert _normalize_maql("(SELECT {metric/abc})") == "{metric/abc}" - - def test_no_where_clause_hint_is_empty_when_a_candidate_has_a_where_clause(): assert _no_where_clause_hint(['SELECT {metric/foo} WHERE {label/status} = "active"']) == "" @@ -187,8 +178,9 @@ def test_no_where_clause_hint_ignores_where_inside_a_literal_with_an_escaped_quo """CodeRabbit finding on PR #1760: an escaped quote inside a quoted literal ended the protected-span match early, leaking the rest of the literal's text -- including a standalone WHERE -- as unprotected. Uses _HINT_PROTECTED_RE (escape-aware), kept - separate from the shared _PROTECTED_RE that feeds the maql_correct comparator (PR - #1760 review, Henry) -- see test_normalize_maql_does_not_consume_escape_sequences.""" + separate from evaluators._maql._PROTECTED_RE, which feeds the maql_correct comparator + (PR #1760 review, Henry) -- see + test_maql_normalize.test_does_not_consume_escape_sequences.""" maql = 'SELECT {metric/x} = "Jane\\"s store WHERE something"' assert _no_where_clause_hint([maql]) != "" @@ -282,42 +274,6 @@ def test_generate_simulated_response_prompt_handles_a_clarifying_question(monkey assert "no filter is needed" in sent_prompt -def test_normalize_maql_is_case_insensitive_for_keywords(): - """Regression test for a live-reproduced bug: 'FOR PREVIOUS(...)' vs - 'FOR Previous(...)' scored as a mismatch even though MAQL keywords are - case-insensitive -- a semantically identical agent answer failed the eval - purely on keyword casing.""" - actual = "SELECT {metric/active_card_count_-_txn_-_cutcgco} FOR PREVIOUS({label/process_date.year})" - expected = "SELECT {metric/active_card_count_-_txn_-_cutcgco}\n FOR Previous({label/process_date.year})" - assert _normalize_maql(actual) == _normalize_maql(expected) - - -def test_normalize_maql_preserves_identifier_case(): - # {type/id} references are real, case-sensitive ids -- must never be casefolded. - assert "Mixed_Case_Id" in _normalize_maql("SELECT {metric/Mixed_Case_Id}") - - -def test_normalize_maql_preserves_quoted_literal_case(): - """The bug this guards against: naively lowercasing everything outside {..} - would also lowercase quoted WHERE-clause literal values, which are real, - case-sensitive data -- not keywords. Two literals differing only in case - must NOT be treated as equal; that would be a false positive.""" - assert _normalize_maql('WHERE {label/status} = "Active"') != _normalize_maql('WHERE {label/status} = "active"') - - -def test_normalize_maql_does_not_consume_escape_sequences(): - """PR #1760 review (Henry): _PROTECTED_RE feeds this comparator (via - _casefold_outside_protected), so it must NOT treat \\X as an escape sequence unless - MAQL literals are confirmed to support backslash escaping (unconfirmed). A `\\"` - inside a literal must still end that literal at the next real quote -- not swallow - everything up to the following quoted value, which would leave a real keyword like - AND uncasefolded and a later literal's case wrongly casefolded.""" - maql = 'SELECT {metric/x} WHERE {label/path} = "C:\\" AND {label/y} = "Active"' - normalized = _normalize_maql(maql) - assert "and {label/y}" in normalized # AND is a keyword outside the literal -- casefolded - assert '"Active"' in normalized # the second literal's case is untouched -- not "active" - - def test_metric_run_result_fields(): r = MetricRunResult( conversation_id="c1", diff --git a/packages/gooddata-eval/tests/test_chat_render.py b/packages/gooddata-eval/tests/test_chat_render.py new file mode 100644 index 000000000..cebaa83d2 --- /dev/null +++ b/packages/gooddata-eval/tests/test_chat_render.py @@ -0,0 +1,118 @@ +# (C) 2026 GoodData Corporation. All rights reserved. +# SPDX-License-Identifier: LicenseRef-GoodData-Enterprise +"""Multipart capture (sse_client) and rendering (render).""" + +import json + +import pytest +from gooddata_eval.core.chat.render import render_answer_text, render_search_results +from gooddata_eval.core.chat.sse_client import _KNOWN_PART_TYPES, parse_sse_lines +from gooddata_eval.core.models import ChatResult + +_SEARCH_PART = { + "type": "searchResults", + "requestedObjectType": "metric", + "keywords": ["key driver analysis"], + "objects": [ + {"id": "total_net_revenue", "type": "metric", "title": "Total Net Revenue", "score": 0.9}, + {"id": "order_count", "type": "metric", "title": "Order Count", "description": "Orders placed"}, + ], +} + + +def _multipart_lines(*parts: dict) -> list[str]: + return [ + "data: " + json.dumps({"item": {"role": "assistant", "content": {"type": "multipart", "parts": list(parts)}}}), + "", + ] + + +# --- capture --------------------------------------------------------------------------- + + +def test_search_results_part_is_captured_alongside_the_text_part(): + lines = _multipart_lines({"type": "text", "text": "Found **10 metrics**:"}, _SEARCH_PART) + result = parse_sse_lines(lines) + assert result.text_response == "Found **10 metrics**:" + assert len(result.search_results) == 1 + assert result.search_results[0]["objects"][0]["title"] == "Total Net Revenue" + + +def test_a_turn_whose_only_part_is_non_text_still_carries_its_answer(): + result = parse_sse_lines(_multipart_lines(_SEARCH_PART)) + assert result.text_response is None + assert render_answer_text(result) != "" + assert "Total Net Revenue" in render_answer_text(result) + + +def test_an_unmodelled_part_type_is_kept_not_dropped(): + result = parse_sse_lines(_multipart_lines({"type": "kda", "kda": {"drivers": ["price"]}})) + assert len(result.unhandled_parts) == 1 + assert result.unhandled_parts[0]["type"] == "kda" + assert "kda" in render_answer_text(result) + + +def test_an_unresolved_visualization_part_is_not_treated_as_content(): + result = parse_sse_lines(_multipart_lines({"type": "visualization", "visualization": None})) + assert result.unhandled_parts == [] + assert render_answer_text(result) == "" + + +def test_alert_proposal_part_still_goes_to_its_own_field(): + result = parse_sse_lines(_multipart_lines({"type": "alertProposal", "alertProposal": {"cta": "Create"}})) + assert result.alert_proposals == [{"cta": "Create"}] + assert result.unhandled_parts == [] + + +def test_known_part_types_matches_the_documented_gen_ai_union(): + gen_ai_message_part_union = { + "text", + "visualization", + "dashboard", + "dashboardPatch", + "kda", + "whatIf", + "searchResults", + "alertProposal", + "clarifyingQuestions", + } + assert gen_ai_message_part_union == _KNOWN_PART_TYPES + + +# --- rendering ------------------------------------------------------------------------- + + +def test_render_search_results_lists_the_objects_the_judge_must_grade(): + rendered = render_search_results(_SEARCH_PART) + assert "Search results (2 metric):" in rendered + assert "- Total Net Revenue (metric/total_net_revenue)" in rendered + assert "- Order Count (metric/order_count): Orders placed" in rendered + assert "0.9" not in rendered + + +def test_render_search_results_of_an_empty_panel_is_empty(): + assert render_search_results({"type": "searchResults", "objects": [], "keywords": []}) == "" + + +def test_render_answer_text_of_a_silent_turn_is_empty(): + assert render_answer_text(ChatResult()) == "" + + +def test_render_answer_text_joins_prose_and_parts(): + result = parse_sse_lines(_multipart_lines({"type": "text", "text": "Found 2:"}, _SEARCH_PART)) + rendered = render_answer_text(result) + assert rendered.startswith("Found 2:") + assert "Total Net Revenue" in rendered + + +def test_a_huge_unmodelled_part_is_truncated(): + result = parse_sse_lines(_multipart_lines({"type": "whatIf", "blob": "x" * 50_000})) + rendered = render_answer_text(result) + assert "(truncated)" in rendered + assert len(rendered) < 3_000 + + +@pytest.mark.parametrize("ptype", ["dashboard", "dashboardPatch", "kda", "whatIf", "clarifyingQuestions"]) +def test_no_known_part_type_is_silently_dropped(ptype): + result = parse_sse_lines(_multipart_lines({"type": ptype, "payload": {"a": 1}})) + assert result.unhandled_parts, f"{ptype} was dropped" diff --git a/packages/gooddata-eval/tests/test_maql_normalize.py b/packages/gooddata-eval/tests/test_maql_normalize.py new file mode 100644 index 000000000..c1a9434cd --- /dev/null +++ b/packages/gooddata-eval/tests/test_maql_normalize.py @@ -0,0 +1,106 @@ +# (C) 2026 GoodData Corporation. All rights reserved. +# SPDX-License-Identifier: LicenseRef-GoodData-Enterprise +"""Tests for the shared MAQL normaliser (core/evaluators/_maql.py).""" + +import pytest +from gooddata_eval.core.evaluators._maql import normalize_maql + +# Pairs that are the SAME MAQL and must compare equal. +EQUIVALENT_PAIRS = [ + pytest.param( + "SELECT {metric/sales_order_revenue} - " + "(SELECT {metric/sales_order_revenue} FOR PREVIOUS ({label/transaction_date.month}))", + "SELECT {metric/sales_order_revenue} - " + "(SELECT {metric/sales_order_revenue} FOR PREVIOUS({label/transaction_date.month}))", + id="qa29226-b6fe30e7-space-before-paren", + ), + pytest.param("FOR PREVIOUS ({label/x})", "FOR PREVIOUS({label/x})", id="space-before-paren"), + pytest.param("THIS(QUARTER, -1)", "THIS(QUARTER,-1)", id="space-after-comma"), + pytest.param("{metric/a} / {metric/b}", "{metric/a}/{metric/b}", id="spaces-around-operator"), + pytest.param('{label/y} = "2025"', '{label/y}="2025"', id="spaces-around-equals"), +] + + +@pytest.mark.parametrize(("left", "right"), EQUIVALENT_PAIRS) +def test_whitespace_around_punctuation_is_not_semantic(left, right): + assert normalize_maql(left) == normalize_maql(right) + + +def test_punctuation_inside_a_quoted_literal_survives(): + maql = 'SELECT {metric/x} WHERE {label/account} = "Acme (Inc), Ltd"' + assert '"Acme (Inc), Ltd"' in normalize_maql(maql) + + +def test_whitespace_inside_a_quoted_literal_is_not_collapsed(): + assert normalize_maql('WHERE {label/x} = "A B"') != normalize_maql('WHERE {label/x} = "A B"') + + +def test_whitespace_outside_literals_is_still_collapsed(): + assert normalize_maql("SELECT {metric/a}\n\n BY {label/b}") == normalize_maql("SELECT {metric/a} BY {label/b}") + + +def test_distinct_quoted_literals_still_differ(): + assert normalize_maql('WHERE {label/x} = "A (1)"') != normalize_maql('WHERE {label/x} = "A(1)"') + + +def test_ifnull_tolerates_whitespace_before_its_paren(): + assert normalize_maql("IFNULL ({metric/a}, 0)") == normalize_maql("IFNULL({metric/a}, 0)") + + +def test_wrapper_syntax_inside_a_quoted_literal_is_not_unwrapped(): + maql = 'WHERE {label/x} = "IFNULL({metric/y}, 0)"' + assert '"IFNULL({metric/y}, 0)"' in normalize_maql(maql) + + +def test_braces_inside_a_quoted_literal_keep_their_whitespace(): + maql = 'WHERE {label/x} = "a { b }"' + assert '"a { b }"' in normalize_maql(maql) + assert normalize_maql(maql) != normalize_maql('WHERE {label/x} = "a {b}"') + + +def test_strips_whitespace(): + assert normalize_maql(" SELECT { metric/foo } ") == "select {metric/foo}" + + +def test_removes_select_wrapper(): + assert normalize_maql("(SELECT {metric/abc})") == "{metric/abc}" + + +def test_is_case_insensitive_for_keywords(): + """Regression test for a live-reproduced bug: 'FOR PREVIOUS(...)' vs + 'FOR Previous(...)' scored as a mismatch even though MAQL keywords are + case-insensitive -- a semantically identical agent answer failed the eval + purely on keyword casing.""" + actual = "SELECT {metric/active_card_count_-_txn_-_cutcgco} FOR PREVIOUS({label/process_date.year})" + expected = "SELECT {metric/active_card_count_-_txn_-_cutcgco}\n FOR Previous({label/process_date.year})" + assert normalize_maql(actual) == normalize_maql(expected) + + +def test_preserves_identifier_case(): + # {type/id} references are real, case-sensitive ids -- must never be casefolded. + assert "Mixed_Case_Id" in normalize_maql("SELECT {metric/Mixed_Case_Id}") + + +def test_preserves_quoted_literal_case(): + """The bug this guards against: naively lowercasing everything outside {..} + would also lowercase quoted WHERE-clause literal values, which are real, + case-sensitive data -- not keywords. Two literals differing only in case + must NOT be treated as equal; that would be a false positive.""" + assert normalize_maql('WHERE {label/status} = "Active"') != normalize_maql('WHERE {label/status} = "active"') + + +def test_does_not_consume_escape_sequences(): + """PR #1760 review (Henry): _PROTECTED_RE feeds this comparator, so it must NOT treat + \\X as an escape sequence unless MAQL literals are confirmed to support backslash + escaping (unconfirmed). A `\\"` inside a literal must still end that literal at the + next real quote -- not swallow everything up to the following quoted value, which + would leave a real keyword like AND uncasefolded and a later literal's case wrongly + casefolded.""" + maql = 'SELECT {metric/x} WHERE {label/path} = "C:\\" AND {label/y} = "Active"' + normalized = normalize_maql(maql) + assert "and {label/y}" in normalized # AND is a keyword outside the literal -- casefolded + assert '"Active"' in normalized # the second literal's case is untouched -- not "active" + + +def test_empty_input(): + assert normalize_maql("") == ""