-
Notifications
You must be signed in to change notification settings - Fork 62
Add RVM/interpreter incompatibility tests and compiler error detection#666
Add RVM/interpreter incompatibility tests and compiler error detection #666anakrish wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds new regression/compatibility coverage between the Rego interpreter and the RVM, while tightening the compiler to explicitly reject a few currently-unsupported Rego patterns (instead of compiling them incorrectly).
Changes:
- Added 12 new
tests/rvm/rego/cases/*.yamlsuites covering interpreter/RVM edge cases (aggregates, deref, defaults, every, functions, planner IR, ref-heads, walk, etc.). - Reduced blanket OPA folder skipping by replacing most folder-level skips with targeted "compiler limitation" detection and a curated list of known RVM runtime mismatches.
- Added compiler errors for unsupported patterns (function defaults, comprehension-in-default, and multi-value ref-head rules).
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rvm/rego/cases/aggregates.yaml | New aggregate-on-rule-result cases (sum/count/min/max) |
| tests/rvm/rego/cases/base_and_virtual_docs.yaml | New base vs virtual doc interaction/suffix lookup cases (mostly skipped) |
| tests/rvm/rego/cases/data_deref.yaml | New dynamic data deref/path computation cases |
| tests/rvm/rego/cases/default_keyword.yaml | New advanced default keyword cases (some skipped) |
| tests/rvm/rego/cases/every_quantifier.yaml | New every quantifier cases including known-bug coverage |
| tests/rvm/rego/cases/function_advanced.yaml | New advanced function scenarios (recursion, composition, comprehensions) |
| tests/rvm/rego/cases/partial_doc_constants.yaml | New constant partial doc cases, including known compiler limitation expectations |
| tests/rvm/rego/cases/partial_object_doc.yaml | New partial object doc cases, including constant bracket-key limitations |
| tests/rvm/rego/cases/planner_ir.yaml | New IR edge cases, else-chain and multi-definition rules |
| tests/rvm/rego/cases/ref_heads.yaml | New ref-head/multi-key head coverage (currently some cases are skipped) |
| tests/rvm/rego/cases/type_builtins.yaml | New type_name and is_* builtin cases |
| tests/rvm/rego/cases/walk_builtin.yaml | New walk builtin cases expecting compiler rejection |
| tests/opa.rs | Reworks OPA conformance runner skip logic: fewer folder skips, more targeted skips |
| src/languages/rego/compiler/error.rs | Adds new compiler error variants for unsupported patterns |
| src/languages/rego/compiler/program.rs | Validates unsupported default-rule patterns before default evaluation |
| src/languages/rego/compiler/rules.rs | Adds multi-value ref-head detection and errors (but currently incomplete for p[q].r) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
320f440 to
712d24c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
712d24c to
b3a0902
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b3a0902 to
71e5eb9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
71e5eb9 to
000716a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot
AI
Apr 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The header says multi-value ref heads currently "flatten" results, but this PR adds explicit compiler rejections for multi-value ref heads (MultiValueRefHeadUnsupported) and the test cases below assert compilation failures. Consider updating the header comment to reflect the current behavior (compile-time unsupported/error) so future readers aren’t misled about runtime semantics.
Copilot
AI
Apr 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is marked skip: true but doesn’t document why it’s skipped (unlike other skipped cases in these suites). Adding a short reason (e.g., known RVM/interpreter mismatch or unsupported operator shape) would make it easier to re-enable later and track progress.
- Add 12 new YAML test files covering RVM/interpreter edge cases: aggregates, base/virtual docs, data deref, default keyword, every quantifier, advanced functions, partial doc constants, partial objects, planner IR, ref heads, type builtins, walk builtin - Add compiler errors for unsupported patterns: - FunctionDefaultsUnsupported: default f(x) := value - ComprehensionInDefaultUnsupported: default p := [x | ...] - MultiValueRefHeadUnsupported: p[q].r or p[q] contains r - Trim OPA_TODO_FOLDERS from 14 blanket-skipped folders to 1 (withkeyword) by replacing folder-level skips with: - compiler_limitation_reason(): auto-skips on 11 known compiler error patterns (with keyword, walk, function defaults, comprehension defaults, multi-value ref heads, undefined variable, unknown function, etc.) - KNOWN_RVM_MISMATCH_NOTES: 32 entries for runtime RVM bugs that compile successfully but produce incorrect results (every false-case, base/virtual merge, suffix lookup, dynamic cross-rule bracket lookup, etc.) - Net effect: 290 more OPA tests now actively validate RVM correctness
000716a to
661fac7
Compare
Add 12 new YAML test files covering RVM/interpreter edge cases: aggregates, base/virtual docs, data deref, default keyword, every quantifier, advanced functions, partial doc constants, partial objects, planner IR, ref heads, type builtins, walk builtin
Add compiler errors for unsupported patterns:
Trim OPA_TODO_FOLDERS from 14 blanket-skipped folders to 1 (withkeyword) by replacing folder-level skips with:
Net effect: 290 more OPA tests now actively validate RVM correctness