Skip to content

Commit f26babd

Browse files
authored
Merge pull request #1719 from gooddata/fix/normalize-maql-case-insensitive
fix(gooddata-eval): make MAQL comparison case-insensitive for keywords
2 parents 588b985 + 94dddaa commit f26babd

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
@@ -21,7 +21,7 @@
2121

2222

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

2626

2727
def test_normalize_maql_removes_select_wrapper():
@@ -61,6 +61,29 @@ def test_generate_simulated_response_prompt_preserves_maql_fidelity(monkeypatch)
6161
assert call_kwargs["max_tokens"] >= 300
6262

6363

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

0 commit comments

Comments
 (0)