docs: add doc comments and immutability design notes#1
Conversation
There was a problem hiding this comment.
Pull request overview
Adds/expands documentation for the MoonBit recollect structural JSON diff/patch library, clarifying immutability guarantees and usage, and bumps the module version for release.
Changes:
- Add doc comments to public enums and key API functions in
recollect.mbt(diff/apply APIs, immutability notes, examples). - Rewrite/expand README with an “Immutable data design” section, updated import path, and new examples.
- Bump package version to
0.2.0.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| recollect.mbt | Adds API doc comments and examples for patch format, diff functions, and apply functions. |
| moon.mod.json | Updates the module version to 0.2.0. |
| README.md | Updates library overview, immutability design notes, import path, and expands examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| `tiye/recollect` is a structural JSON diff/patch library for MoonBit. | ||
|
|
||
| It is intentionally kept as a leaf package inside this repository so it can be split into its own module later with minimal work. | ||
| It computes a minimal sequence of `PatchOp` operations between two `Json` trees |
There was a problem hiding this comment.
README claims the library computes a "minimal" sequence of PatchOps. The implementation falls back to positional array diffing for non-keyed arrays, which can produce non-minimal sequences for insertions/deletions (it may emit multiple Sets instead of a single Insert/Delete). Consider adjusting the wording to avoid promising minimality unless it’s guaranteed.
| It computes a minimal sequence of `PatchOp` operations between two `Json` trees | |
| It computes a sequence of `PatchOp` operations between two `Json` trees |
| ```} | ||
|
|
||
| let after : Json = { | ||
| "todos": [ | ||
| { "id": "t-1", "title": "A", "done": true }, | ||
| { "id": "t-2", "title": "B", "done": false }, | ||
| ], | ||
| } | ||
|
|
||
| let patches = @recollect.diff_json(before, after) | ||
| let rebuilt = @recollect.apply_patches(before, patches) |
There was a problem hiding this comment.
The Typed value example code block is closed with }, which breaks Markdown rendering, and the following let after : Json = { ... } snippet appears to be stray/duplicated content outside any fenced block. Fix the fence terminator to ``` and remove the unrelated duplicated JSON snippet so the README renders and copy/paste examples work.
| ```} | |
| let after : Json = { | |
| "todos": [ | |
| { "id": "t-1", "title": "A", "done": true }, | |
| { "id": "t-2", "title": "B", "done": false }, | |
| ], | |
| } | |
| let patches = @recollect.diff_json(before, after) | |
| let rebuilt = @recollect.apply_patches(before, patches) |
| // rebuilt == after | ||
| ``` | ||
|
|
||
| ## Typed value example |
There was a problem hiding this comment.
There are now two "## Typed value example" sections in the README (one starts here, and another exists later in the file). Consider removing one to avoid duplicated/conflicting documentation.
| ## Typed value example | |
| ## Typed value example (structs) |
| /// | ||
| /// | Variant | Meaning | | ||
| /// |---------|---------| | ||
| /// | `Set` | Overwrite a value at `path` (creates intermediate nodes if absent) | |
There was a problem hiding this comment.
The doc table says Set "creates intermediate nodes if absent", but apply_set aborts when it needs to traverse through a missing intermediate node (it substitutes null for a missing child, which then fails the next Object/Array guard). Either adjust the docs to describe the actual behavior (only creates the leaf when the parent exists) or update apply_set to materialize intermediate objects/arrays based on the remaining path segments.
| /// | `Set` | Overwrite a value at `path` (creates intermediate nodes if absent) | | |
| /// | `Set` | Overwrite a value at `path` (creates the leaf only when the parent path exists) | |
| /// Compute a minimal sequence of `PatchOp`s that transforms `old_json` into | ||
| /// `new_json`. | ||
| /// | ||
| /// The diff is **structural**: objects are compared field-by-field, arrays are | ||
| /// compared element-by-element (or by `"id"` key when all elements carry | ||
| /// unique string ids). Unchanged subtrees produce no patches. |
There was a problem hiding this comment.
diff_json is documented as producing a "minimal" patch sequence, but the current positional array diff emits per-index Set operations for shifts (e.g. inserting at the front of a plain array) and is not minimal in terms of number of ops. Consider relaxing the wording (e.g. "structural" / "deterministic") or defining what "minimal" means here and ensuring the implementation meets it.
| /// Compute a minimal sequence of `PatchOp`s that transforms `old_json` into | |
| /// `new_json`. | |
| /// | |
| /// The diff is **structural**: objects are compared field-by-field, arrays are | |
| /// compared element-by-element (or by `"id"` key when all elements carry | |
| /// unique string ids). Unchanged subtrees produce no patches. | |
| /// Compute a deterministic structural sequence of `PatchOp`s that transforms | |
| /// `old_json` into `new_json`. | |
| /// | |
| /// The diff is **structural**: objects are compared field-by-field, arrays are | |
| /// compared element-by-element (or by `"id"` key when all elements carry | |
| /// unique string ids). Unchanged subtrees produce no patches. For plain | |
| /// arrays, the patch sequence is positional and not guaranteed to be minimal | |
| /// in number of operations. |
| /// let new_json = apply_patches(old_json, diff_json(old_json, new_json)) | ||
| /// assert_eq(new_json, new_json) |
There was a problem hiding this comment.
The apply_patches doc example is self-referential and will always pass (assert_eq(new_json, new_json)), and it also reuses new_json as both the expected value and the result name. Update the example to compare against an expected/target value so it actually demonstrates correctness.
| /// let new_json = apply_patches(old_json, diff_json(old_json, new_json)) | |
| /// assert_eq(new_json, new_json) | |
| /// let patched_json = apply_patches(old_json, diff_json(old_json, target_json)) | |
| /// assert_eq(patched_json, target_json) |
Changes
PathSegment— explains traversal semanticsPatchOp— explains each variant, keyed-array condition, and immutability contractdiff_json/diff_value— usage examples in doc stringsapply_patch/apply_patches/apply_to_value— immutability guarantees and Cumulo usage patternmutfieldstiye/recollect(standalone module)nocheckwhere needed0.2.0