microsoft / microsoft/regorus

RVM: partial object codegen gaps (constant-key, multi-level, duplicate-key)

Open
#719 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
346
Forks
75
Avg merge
1d 3h
Merged PRs (30d)
15

Description

Summary

The RVM compiler/codegen has gaps in partial object rule support. These are pre-existing issues uncovered while fixing #712 (which addresses the interpreter-side bug).

In OPA v1, p[k] if { ... } is a partial object rule (key → true). The RVM previously misclassified it as PartialSet. PR #718 corrects the classification to PartialObject, but the codegen only handles the simple variable-key case correctly.

Problem 1: Constant-key partial objects

p["fixed"] if { input.enabled }

The constant key "fixed" gets baked into the rule path by utils.rs path construction. When classified as PartialObject, the rule produces {"fixed": true} via ObjectSet, but leaf lookup at data.test.p.fixed returns the whole object instead of just true.

Expected: data.test.p.fixedtrue
Actual: data.test.p.fixed{"fixed": true} (or lookup fails)

Problem 2: Multi-level bracket keys

p[a][b] if {
  some a, obj in input.nested
  some b, _ in obj
}

The compiler only keeps one key expression (compiler/rules.rs:353-355) and emits one ObjectSet (compiler/queries.rs:148-196). The outer key is lost, producing a flat object instead of nested.

Expected: {"app": {"read": true, "write": true}, "ops": {"deploy": true}}
Actual: {"read": true, "write": true, "deploy": true} (flattened)

Problem 3: Duplicate key conflict resolution

PartialObject semantics require consistent handling of duplicate keys. OPA raises an error on conflicting values for the same key. Regorus currently silently overwrites (last-writer-wins).

Locked-down tests (skipped in PR #718):

  • partial_object_duplicate_key_last_wins — single rule, two iterations produce same key with different values
  • partial_object_duplicate_paths_same_key_different_values_conflict — two rules producing same key with conflicting values

Problem 4: Non-string static-prefix partial objects

p[1][k] if { ... }
p[true][k] if { ... }
p[null][k] if { ... }

PR #718 added is_string_literal() to allow string static prefixes (e.g., p["cfg"][k]), but non-string scalar literals (numbers, booleans, null) are rejected with a misleading "nested bracket keys unsupported" error. These are valid static prefixes and should be handled like string prefixes.

Also: raw-string prefixes (p[`cfg`][k]) pass is_string_literal() validation but fail later in path extraction, which only handles Expr::String, not Expr::RawString.

Problem 5: Undefined key/value materialization

The RVM incorrectly materializes entries with undefined keys or undefined values instead of skipping them.

Locked-down tests (skipped in PR #718):

  • partial_object_undefined_key_skipped / partial_object_undefined_key_skips_iteration — key expression evaluates to undefined; entry should be skipped
  • partial_object_undefined_value_skips_iteration — value expression evaluates to undefined; entry should be skipped
  • partial_object_mixed_undefined_key_value_cases_skip_bad_iterations — mixed cases

Problem 6: Vacuous truth in every body

p[k] if {
  every x in input.items { x > 0 }
  k := "valid"
}

When input.items is an empty array, every is vacuously true, so the rule should fire. The RVM currently handles this incorrectly for certain groupings.

Locked-down test (skipped in PR #718):

  • partial_object_every_vacuous_truth_collects_empty_arrays

Current Mitigation (PR #718)

PR #718 adds compiler errors for unsupported patterns (constant-key and multi-level), causing graceful fallback to the interpreter. The simple variable-key case (p[k] if) works correctly in the RVM.

Long-term Fix

A proper fix would involve:

  • Logical rule paths (separating static path from dynamic keys)
  • Support for multi-key ObjectSetPath or equivalent
  • Proper virtual_data lookup for partial object sub-paths
  • Correct undefined propagation (skip entries with undefined key or value)
  • Duplicate key conflict detection (error on conflicting values)
  • Non-string and raw-string literal prefix support
Relevant files
  • src/languages/rego/compiler/rules.rs — rule type classification, validate_partial_object_shape()
  • src/languages/rego/compiler/queries.rs — ObjectSet emission
  • src/languages/rego/compiler/utils.rs / src/utils.rs — rule path construction
  • src/rvm/vm/virtual_data.rs — leaf lookup logic
  • src/rvm/vm/dispatch.rs — ObjectSet instruction implementation
  • src/rvm/vm/rules.rs — rule frame setup and result aggregation
  • tests/rvm/rego/cases/partial_object_rules.yaml — skipped tests documenting expected behavior

Related: #712, #718

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the skipped cases in tests/rvm/rego/cases/partial_object_rules.yaml, then read validate_partial_object_shape() in src/languages/rego/compiler/rules.rs and ObjectSet emission in src/languages/rego/compiler/queries.rs. Trace path handling, lookup, and aggregation through the listed utils.rs, virtual_data.rs, dispatch.rs, and rvm/vm/rules.rs files. Done means the documented constant-key, multi-level, duplicate-key, undefined, literal-prefix, and vacuous-truth cases behave as expected in the RVM.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.