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 intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
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.
anakrish
force-pushed
the
rvm-interp-incompat
branch
2 times, most recently
from
320f440 to
712d24c
Compare
There was a problem hiding this comment.
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.
anakrish
force-pushed
the
rvm-interp-incompat
branch
from
712d24c to
b3a0902
Compare
There was a problem hiding this comment.
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.
anakrish
force-pushed
the
rvm-interp-incompat
branch
from
b3a0902 to
71e5eb9
Compare
There was a problem hiding this comment.
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.
anakrish
force-pushed
the
rvm-interp-incompat
branch
from
71e5eb9 to
000716a
Compare
There was a problem hiding this comment.
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.
| # Ref Heads (Multi-Key Rule Heads) Test Suite | ||
| # Tests rules with chained bracket references in the head, e.g. foo[x][y] := value | ||
| # These patterns are a known compiler limitation: multi-value ref heads are unsupported. | ||
| # The RVM compiler currently only extracts the outermost bracket index, | ||
| # losing intermediate keys and producing flat instead of nested results. | ||
|
|
There was a problem hiding this comment.
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.
| allowed_actions contains "write" | ||
| result := input.action in allowed_actions | ||
| query: data.test.result | ||
| want_result: true |
There was a problem hiding this comment.
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.
| want_result: true | |
| want_result: true | |
| # Skipped due to a known RVM/interpreter mismatch when evaluating `in` against a partial set rule result. |
- 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
anakrish
force-pushed
the
rvm-interp-incompat
branch
from
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