[bot] Merge master/f26babd9 into rel/dev - #1747
Conversation
_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>
…itive fix(gooddata-eval): make MAQL comparison case-insensitive for keywords
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## rel/dev #1747 +/- ##
===========================================
+ Coverage 80.14% 80.15% +0.01%
===========================================
Files 272 272
Lines 19101 19111 +10
===========================================
+ Hits 15309 15319 +10
Misses 3792 3792 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 Automated PR to perform merge from master into rel/dev with changes up to f26babd (created by https://github.com/gooddata/gooddata-python-sdk/actions/runs/32369777052).