Skip to content

Commit 4e1cbda

Browse files
Tomkessclaude
andcommitted
fix(gooddata-eval): stop the metric-skill simulated user from dropping MAQL clauses
agentic_metric_skill's simulated-user reply (generate_simulated_response) is what keeps a multi-turn metric-creation conversation going after the agent asks a clarifying question -- it prompts an LLM to answer as the user, using the fixture's expected_output.maql as its only source of truth. The prompt told it to "reply briefly" with no instruction to preserve the MAQL's structure. In practice it would silently drop a WHERE/filter clause, or paraphrase a label id, whenever the agent's question didn't happen to ask about that part directly -- so a well-behaved agent, faithfully following the (already-wrong) simulated answer, still failed the eval. Reproduced live twice against a real gdc-mic-ai-evaluation fixture ("Create a metric for total ecommerce spend", expects SELECT {metric/spend_amount_-_cutcgco} WHERE {label/ecommerce_indicator_code} = "1"): 1. Simulated reply dropped "_code" off ecommerce_indicator_code, anchoring the agent on a sibling attribute that doesn't have that filter. 2. Simulated reply picked one of 3 metric options the agent offered and said "please proceed with that" -- never mentioning the WHERE clause that expected_output required, even though it had it in hand. Confirmed via a 5x-repeated A/B test that this is a prompt problem, not a model-capability one: swapping gpt-4o-mini for gpt-4o under the OLD prompt did not fix it (still dropped the clause); the NEW prompt fixes it on the ORIGINAL gpt-4o-mini (1/5 -> 5/5 runs preserving the exact filter). Fix: instruct the simulating LLM to (a) ensure every clause of the expected MAQL is eventually satisfied even if the agent's question didn't ask about it, (b) quote field/label identifiers verbatim rather than paraphrase them, and (c) proactively add a filter the agent's own offered options omitted. Also drop "reply briefly" and raise max_tokens 150->300, since brevity was part of what squeezed the filter clause out. This brings metric_skill's simulated-user prompt in line with alert_skill's generate_simulated_alert_response, which already passes structured facts + explicit "proactively tell the agent X" instructions rather than one freely-paraphrased string -- not a new pattern for this codebase. Added a regression test asserting the sent prompt preserves clause-fidelity language and the raised max_tokens. Full gooddata-eval suite: 272 passed (9 pre-existing unrelated failures, confirmed identical on clean master before this change -- missing openai extra in test env, and two unrelated test files). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 8ead00e commit 4e1cbda

2 files changed

Lines changed: 39 additions & 3 deletions

File tree

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,17 @@ def generate_simulated_response(agent_message: str, expected_output: dict) -> st
101101
prompt = (
102102
f"You are simulating a user in a conversation with a BI assistant that creates metrics. "
103103
f"The assistant said: '{agent_message}'. "
104-
f"The user originally asked to create a metric with MAQL: {expected_maql}. "
105-
f"Reply briefly as the user, providing any clarification the assistant needs."
104+
f"The user's ground-truth intended metric is exactly this MAQL: {expected_maql}. "
105+
f"Reply as the user. You MUST ensure every clause of that MAQL (including any WHERE/filter "
106+
f"conditions) is eventually satisfied, and quote field/label identifiers verbatim from it -- "
107+
f"never paraphrase or drop a clause, even if the assistant's question doesn't explicitly ask "
108+
f"about it. If the assistant's offered options omit a required filter, add it yourself."
106109
)
107110
try:
108111
response = client.chat.completions.create(
109112
model="gpt-4o-mini",
110113
messages=[{"role": "user", "content": prompt}],
111-
max_tokens=150,
114+
max_tokens=300,
112115
temperature=0,
113116
)
114117
except OpenAIError as exc:

packages/gooddata-eval/tests/test_agentic_metric_skill.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
# SPDX-License-Identifier: LicenseRef-GoodData-Enterprise
33
import os
44
import sys
5+
import types
56
from unittest.mock import MagicMock, patch
67

78
import pytest
@@ -25,6 +26,38 @@ def test_normalize_maql_removes_select_wrapper():
2526
assert _normalize_maql("(SELECT {metric/abc})") == "{metric/abc}"
2627

2728

29+
def test_generate_simulated_response_prompt_preserves_maql_fidelity(monkeypatch):
30+
"""Regression test for a live-reproduced bug: the old prompt ("reply briefly",
31+
no instruction to cover clauses the assistant didn't ask about) let the
32+
simulating LLM silently drop a MAQL's WHERE clause or paraphrase a label id --
33+
confirmed via a 5x-repeated A/B test (1/5 vs 5/5 fidelity) that this was the
34+
prompt, not the model (gpt-4o did not fix it under the old prompt either).
35+
"""
36+
monkeypatch.setenv("OPENAI_API_KEY", "test-key")
37+
mock_client = MagicMock()
38+
mock_response = MagicMock()
39+
mock_response.choices = [MagicMock(message=MagicMock(content="ok"))]
40+
mock_client.chat.completions.create.return_value = mock_response
41+
42+
# `openai` is an optional [llm-judge] extra, not installed in this test env --
43+
# inject a fake module rather than patching a real one (mirrors how the source
44+
# itself does `from openai import OpenAI` as a local, guarded import).
45+
fake_openai_module = types.SimpleNamespace(OpenAI=MagicMock(return_value=mock_client))
46+
monkeypatch.setitem(sys.modules, "openai", fake_openai_module)
47+
48+
expected_output = {"maql": 'SELECT {metric/spend_amount_-_cutcgco} WHERE {label/ecommerce_indicator_code} = "1"'}
49+
generate_simulated_response("Which base metric should I use?", expected_output)
50+
51+
call_kwargs = mock_client.chat.completions.create.call_args.kwargs
52+
sent_prompt = call_kwargs["messages"][0]["content"]
53+
54+
assert "verbatim" in sent_prompt
55+
assert "every clause" in sent_prompt
56+
assert "WHERE" in sent_prompt or "filter" in sent_prompt.lower()
57+
assert "reply briefly" not in sent_prompt.lower()
58+
assert call_kwargs["max_tokens"] >= 300
59+
60+
2861
def test_metric_run_result_fields():
2962
r = MetricRunResult(
3063
conversation_id="c1",

0 commit comments

Comments
 (0)