Skip to content

Commit 94dddaa

Browse files
Tomkessclaude
andcommitted
fix(gooddata-eval): make MAQL comparison case-insensitive for keywords
_normalize_maql/_best_maql_match compare an agent's generated MAQL against expected_output.maql via exact string equality after whitespace/wrapper normalization -- but MAQL keywords (SELECT, FOR PREVIOUS, WHERE, BY, ...) are case-insensitive at the query-engine level (confirmed against the MAQL reference), while the comparison itself was fully case-sensitive. Reproduced live in gdc-mic-ai-evaluation, post the #1718 fix: fixture "Create a metric for the prior-year value of Active cards" expects SELECT {metric/active_card_count_-_txn_-_cutcgco} FOR Previous({label/process_date.year}) Agent produced, verbatim: SELECT {metric/active_card_count_-_txn_-_cutcgco} FOR PREVIOUS({label/process_date.year}) Byte-identical except FOR PREVIOUS vs FOR Previous -- scored as a fail. First fix attempt considered and rejected: lowercase everything outside {type/id} braces. That's wrong -- WHERE-clause literal values are ALSO outside braces (e.g. WHERE {label/status} = "Active") and are real, case-sensitive data, not keywords; blindly folding them would create a new false-positive risk (two genuinely different filter values scored as equal). Actual fix: per the MAQL reference, every literal value is quoted and every identifier lives inside {..} -- both are exhaustively structural markers, so protecting text inside either while casefolding everything else needs no keyword list at all (which would risk being incomplete against MAQL's large vocabulary: SELECT, BY, WHERE, HAVING, FOR PREVIOUS/NEXT/EACH, WITHOUT PF, TOP/BOTTOM, WITHIN, RANK family, RUNSUM family, IFNULL, CASE/WHEN, 15+ math functions, ...). Added _casefold_outside_protected(), applied as the final step in _normalize_maql. Tests added: - keyword case-insensitivity on the exact reproduced case (FOR PREVIOUS vs FOR Previous) - identifier case preserved ({metric/Mixed_Case_Id} untouched) - quoted literal case preserved AND still distinguishes real differences (WHERE x = "Active" vs WHERE x = "active" must stay a genuine mismatch -- this is the test that would have caught the rejected first draft) Updated the one existing test whose expected value assumed no case normalization ever happens (SELECT -> select). Full gooddata-eval suite: 274 passed, 9 pre-existing unrelated failures (missing openai extra in this test env; two unrelated test files) -- identical count to before this change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 47b3216 commit 94dddaa

2 files changed

Lines changed: 45 additions & 3 deletions

File tree

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

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@
2525
_IFNULL_RE = re.compile(r"IFNULL\s*\([^,]+,\s*0\)", re.IGNORECASE)
2626
_SELECT_WRAP_RE = re.compile(r"^\s*\(\s*SELECT\s*\{([^}]+)\}\s*\)\s*$", re.IGNORECASE)
2727
_INNER_SELECT_RE = re.compile(r"\(\s*SELECT\s*\{([^}]+)\}\s*\)", re.IGNORECASE)
28+
# Matches whichever comes first: a {type/id} identifier reference or a quoted string
29+
# literal -- both are case-sensitive data and must survive casefolding untouched.
30+
# Everything else in MAQL (keywords, operators, numbers, punctuation) carries no
31+
# case-sensitive meaning, per the MAQL reference (SELECT/BY/WHERE/FOR PREVIOUS/etc.
32+
# are case-insensitive; only {..} identifiers and quoted literal values are not).
33+
_PROTECTED_RE = re.compile(r"\{[^}]*\}|\"[^\"]*\"|'[^']*'")
2834

2935

3036
def _strip_outer_parens(s: str) -> str:
@@ -42,8 +48,21 @@ def _strip_outer_parens(s: str) -> str:
4248
return s[1:-1].strip()
4349

4450

51+
def _casefold_outside_protected(s: str) -> str:
52+
"""Lowercase MAQL keywords/operators while preserving case-sensitive {type/id}
53+
identifiers and quoted string literal values (e.g. WHERE {label/x} = "Active")."""
54+
parts = []
55+
last = 0
56+
for m in _PROTECTED_RE.finditer(s):
57+
parts.append(s[last : m.start()].lower())
58+
parts.append(m.group(0))
59+
last = m.end()
60+
parts.append(s[last:].lower())
61+
return "".join(parts)
62+
63+
4564
def _normalize_maql(maql: str) -> str:
46-
"""Semantic normalisation: strip whitespace, unwrap IFNULL/SELECT wrappers."""
65+
"""Semantic normalisation: strip whitespace, unwrap IFNULL/SELECT wrappers, casefold keywords."""
4766
if not maql:
4867
return ""
4968
m = maql.strip()
@@ -56,7 +75,7 @@ def _normalize_maql(maql: str) -> str:
5675
m = re.sub(r"\{\s+", "{", m)
5776
m = re.sub(r"\s+\}", "}", m)
5877
m = re.sub(r"\s+", " ", m)
59-
return m.strip()
78+
return _casefold_outside_protected(m.strip())
6079

6180

6281
def _best_maql_match(actual_maql: str, expected_outputs: list[dict]) -> tuple[bool, str]:

‎packages/gooddata-eval/tests/test_agentic_metric_skill.py‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020

2121
def test_normalize_maql_strips_whitespace():
22-
assert _normalize_maql(" SELECT { metric/foo } ") == "SELECT {metric/foo}"
22+
assert _normalize_maql(" SELECT { metric/foo } ") == "select {metric/foo}"
2323

2424

2525
def test_normalize_maql_removes_select_wrapper():
@@ -59,6 +59,29 @@ def test_generate_simulated_response_prompt_preserves_maql_fidelity(monkeypatch)
5959
assert call_kwargs["max_tokens"] >= 300
6060

6161

62+
def test_normalize_maql_is_case_insensitive_for_keywords():
63+
"""Regression test for a live-reproduced bug: 'FOR PREVIOUS(...)' vs
64+
'FOR Previous(...)' scored as a mismatch even though MAQL keywords are
65+
case-insensitive -- a semantically identical agent answer failed the eval
66+
purely on keyword casing."""
67+
actual = "SELECT {metric/active_card_count_-_txn_-_cutcgco} FOR PREVIOUS({label/process_date.year})"
68+
expected = "SELECT {metric/active_card_count_-_txn_-_cutcgco}\n FOR Previous({label/process_date.year})"
69+
assert _normalize_maql(actual) == _normalize_maql(expected)
70+
71+
72+
def test_normalize_maql_preserves_identifier_case():
73+
# {type/id} references are real, case-sensitive ids -- must never be casefolded.
74+
assert "Mixed_Case_Id" in _normalize_maql("SELECT {metric/Mixed_Case_Id}")
75+
76+
77+
def test_normalize_maql_preserves_quoted_literal_case():
78+
"""The bug this guards against: naively lowercasing everything outside {..}
79+
would also lowercase quoted WHERE-clause literal values, which are real,
80+
case-sensitive data -- not keywords. Two literals differing only in case
81+
must NOT be treated as equal; that would be a false positive."""
82+
assert _normalize_maql('WHERE {label/status} = "Active"') != _normalize_maql('WHERE {label/status} = "active"')
83+
84+
6285
def test_metric_run_result_fields():
6386
r = MetricRunResult(
6487
conversation_id="c1",

0 commit comments

Comments
 (0)