feat(collections): introduce Object storage abstraction (foundation) by anakrish · Pull Request #57 · anakrish/regorus

anakrish · 2026-06-01T02:26:08Z

First in a small series that introduces opaque storage types for Value variants so future backends can swap in without rippling through call sites. The design also lets multiple backends coexist at runtime — different Object instances in the same process can use different implementations (e.g. a lazy DB-backed object for input, inline small-map objects for SARIF location records, regular sorted maps elsewhere) and interoperate through the same opaque type.

The design and rationale are written up in docs/value/object.md. It covers the encapsulation choice, the explicit iter vs iter_sorted split, the cursor for resumable traversal, why Ord is defined against sorted iteration, and the concrete workloads driving the abstraction (SARIF small-object pressure, Kubernetes admission, Azure Policy aliases and case-insensitive compare, external/lazy data sources, eval-time arenas, host-language FFI). Precedents: serde_json::Map, toml::Table, simdjson DOM.

Layout: the new type lives at src/value/object.rs; src/value.rs is moved to src/value/mod.rs to make room. Re-exported as regorus::Object.

Follow-ups (separate PRs):

Object migration (payload swap + call sites)
Set foundation + migration
Array, String, and Bindings abstractions

anakrish · 2026-06-01T02:34:25Z

@copilot review this pr using all the review skills in this repo. Launch each skill in a separate agent and report the findings here. Do not make any commits to the branch.

Copilot · 2026-06-01T02:39:46Z

@copilot review this pr using all the review skills in this repo. Launch each skill in a separate agent and report the findings here. Do not make any commits to the branch.

Completed review using both review skills in separate agents:

code-review: 0 findings (0 critical/high/medium/low). No material issues found.
deep-review: 0 findings (0 critical/high/medium/low). Low-risk, additive abstraction change; no correctness/safety/API/resource-limit regressions identified.

Both reports were generated and saved in-agent:

/tmp/code-review-report.md
/tmp/deep-review-report.md

No commits were made.

anakrish · 2026-06-01T19:08:22Z

@copilot Review this PR using all the review skills in this repo. Use a separate agent for each skill.

Copilot

Pull request overview

Introduces an opaque Object storage type wrapping BTreeMap<Value, Value> to allow alternative backends (lazy, inline small-map, alias-aware, etc.) to be swapped or coexist without changing call sites. This is the foundation PR; payload swap and call-site migration follow in subsequent PRs.

Changes:

New src/value/object.rs module with Object, Iter, IterMut, IntoIter, and ObjectCursor (resumable iteration), plus serde, Ord/PartialOrd (defined against iter_sorted for storage-independent comparison), and Index impls.
Re-exports Object from regorus and converts src/value.rs → src/value/mod.rs to host the new submodule; tests added in src/value/tests.rs.
Updates src/scheduler.rs to qualify Expr::Array/Expr::Object patterns to disambiguate from the new top-level Object re-export (where use crate::* + use crate::ast::Expr::* were both in scope).
New design doc docs/value/object.md.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/value/object.rs	New opaque `Object` wrapper around `BTreeMap`, with curated API, resumable cursor, hand-written `Ord` over sorted iter, serde (canonical sorted output), and iterator types.
src/value/mod.rs	Wires in new `object` submodule and tests; re-exports `Object`/iterator types (and `ObjectCursor` under `rvm`).
src/value/tests.rs	Unit tests: oracle parity with `BTreeMap`, dup-key semantics, accessor coverage, serde roundtrip, cursor coverage, `Ord` invariants.
src/lib.rs	Adds `Object` to public re-exports.
src/scheduler.rs	Qualifies `Expr::Array`/`Expr::Object` patterns to avoid clash with newly re-exported `Object`.
docs/value/object.md	Design rationale, precedents, and target use cases for the abstraction.

anakrish

Both lib.rs and scheduler.rs contain unrelated changes and should not be part of this PR.

anakrish · 2026-06-01T19:15:15Z

+#[cfg(test)]
+mod tests;
+
+#[allow(unused_imports)] // surface for downstream PRs; cursor used internally in PR1b


Remove reference to PR1b

Done in 54d8b66 — comment now reads just // surface for downstream PRs.

Copilot · 2026-06-01T19:37:33Z

@copilot Review this PR using all the review skills in this repo. Use a separate agent for each skill.

Completed. I ran both review skills in separate agents and collected the results:

code-review: 1 Medium finding at src/value/object.rs (serde round-trip is lossy for non-string keys: keys are stringified on serialize and come back as Value::String on deserialize).
deep-review: 0 findings (no additional correctness/security/API issues flagged in that pass).

No commits were made.

Add an opaque Object type for the key→value storage backing Value::Object. It exposes a small set of methods (get, insert, remove, iter, iter_sorted, cursor, serde) and keeps the backing store private, so future representations -- inline small-map, hash-backed, lazy, arena, FFI-callback -- can plug in without touching the call sites that name this type. Nothing in the engine uses Object yet. Value::Object still wraps Rc<BTreeMap<Value, Value>>; the payload swap and call-site migration come in the next PR. Object stands on its own unit tests in the meantime. docs/value/object.md walks through the design, the precedents it follows (serde_json::Map, toml::Table, simdjson DOM), and the concrete workloads the abstraction is meant to unlock. A matching Set abstraction follows in a separate PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

anakrish · 2026-06-01T20:32:03Z

Re the code-review finding (lossy serde round-trip for non-string keys): this is pre-existing behavior, not a regression in this PR. Value's Serialize impl on main (src/value.rs lines 96–108 of acf7f7a) already stringifies non-string Object keys via serde_json::to_string(k) for the same JSON-shape reason (JSON keys must be strings). The Object::serialize impl in this PR lifts that block verbatim into src/value/object/serde.rs so the next PR can collapse the duplicate copy in value.rs. The new test object_serialize_non_string_keys_and_deterministic pins the stringification behavior so a future backend swap cannot silently change it.

Builds on #57. Swap Value::Object's payload from Rc<BTreeMap<Value, Value>> to Rc<Object> and migrate all call sites to the Object API. as_object / as_object_mut keep their names but return &Object / &mut Object. The mutable accessor handles Rc::make_mut internally, so callers no longer do it themselves. Object grows into_value() and From<Object> for Value. Value's serializer now delegates to Object::serialize, dropping a duplicate non-string-key stringification path. RVM IterationState::Object is rewritten around ObjectCursor: O(log n) steps over a shared Rc<Object>, no eager pair snapshot. Snapshot independence is preserved by Rc copy-on-write; setup_next_iteration advances the cursor inline and advance() becomes a no-op for this variant. A new iteration_state_object_is_snapshot_independent_of_source test covers CoW against a mutated alias. Value::Set still wraps Rc<BTreeSet<Value>>; the matching Set abstraction and its swap ship in follow-up PRs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot started work on behalf of anakrish June 1, 2026 02:34 View session

Copilot finished work on behalf of anakrish June 1, 2026 03:28

anakrish force-pushed the storage-abstraction-object-foundation branch 9 times, most recently from cad5fe8 to dcaf050 Compare June 1, 2026 19:06

anakrish requested a review from Copilot June 1, 2026 19:07

Copilot started reviewing on behalf of anakrish June 1, 2026 19:08 View session

Copilot AI reviewed Jun 1, 2026

View reviewed changes

Copilot started work on behalf of anakrish June 1, 2026 19:09 View session

anakrish commented Jun 1, 2026

View reviewed changes

anakrish force-pushed the storage-abstraction-object-foundation branch 2 times, most recently from 54d8b66 to 1016c83 Compare June 1, 2026 19:51

Copilot finished work on behalf of anakrish June 1, 2026 20:00

anakrish force-pushed the storage-abstraction-object-foundation branch 2 times, most recently from 553e431 to 98873b8 Compare June 1, 2026 20:14

anakrish force-pushed the storage-abstraction-object-foundation branch from 98873b8 to 595029d Compare June 1, 2026 20:16

anakrish mentioned this pull request Jun 1, 2026

refactor(value): migrate Value::Object to Object storage abstraction #58

Merged

anakrish merged commit 32c702a into main Jun 1, 2026
58 checks passed

anakrish mentioned this pull request Jun 2, 2026

feat(value): introduce Set storage abstraction #60

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(collections): introduce Object storage abstraction (foundation) - #57

feat(collections): introduce Object storage abstraction (foundation)#57
anakrish merged 1 commit into
mainfrom
storage-abstraction-object-foundation

anakrish commented Jun 1, 2026 •

edited

Loading

Uh oh!

anakrish commented Jun 1, 2026

Uh oh!

Copilot AI commented Jun 1, 2026

Uh oh!

anakrish commented Jun 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

anakrish left a comment

Uh oh!

anakrish Jun 1, 2026

Uh oh!

anakrish Jun 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Jun 1, 2026

Uh oh!

anakrish commented Jun 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

anakrish commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anakrish commented Jun 1, 2026

Uh oh!

Copilot AI commented Jun 1, 2026

Uh oh!

anakrish commented Jun 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

anakrish left a comment

Choose a reason for hiding this comment

Uh oh!

anakrish Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

anakrish Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Jun 1, 2026

Uh oh!

anakrish commented Jun 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anakrish commented Jun 1, 2026 •

edited

Loading