test(platform-value): improve coverage for pointer, bytes_36, path operations, and diff

[QuantumExplorer]

Summary

Adds 119 tests across 4 files in rs-platform-value covering real logic.

File	New Tests	What's covered
pointer.rs	30	RFC 6901 pointer navigation, tilde escapes, array indices, take()
types/bytes_36.rs	33	Conversions, hashing, ordering, string encoding (follows bytes_20/32 pattern)
inner_value_at_path.rs	31	Path parsing, remove/set/get at nested paths, array indices
patch/diff.rs	25	RFC 6902 diff generation, tilde/slash escaping, nested maps/arrays

Test plan

• cargo test -p platform-value --lib passes
• cargo fmt --all clean
• CI

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Expanded test coverage for value path manipulation, retrieval, and modification operations.
  • Added tests for patch diff functionality including RFC6901 path escaping and patch application round-trips.
  • Increased test coverage for JSON pointer operations and token unescaping.
  • Added comprehensive tests for Bytes36 type including construction, conversion, serialization, and ordering semantics.

[coderabbitai]

📝 Walkthrough

Walkthrough

Comprehensive unit test coverage was added to four modules in the rs-platform-value crate, validating path manipulation APIs, diff/patch RFC6901 behavior, JSON pointer operations, and Bytes36 type conversions and serialization. No public APIs were modified.

Changes

Cohort / File(s)	Summary
Path Manipulation Tests packages/rs-platform-value/src/inner_value_at_path.rs	Added 355 lines of tests for is_array_path parsing (underscore names, zero indices, bracket patterns, numeric bounds) and Value path APIs (remove_value_at_path, remove_optional_value_at_path, set_value_at_full_path, get_value_at_path, get_optional_value_at_path, set_value_at_path, remove_values_matching_path) covering success/error cases and nested/array traversal.
Diff & Patch Tests packages/rs-platform-value/src/patch/diff.rs	Added 333 lines of tests validating RFC6901 path escaping (~→~0, /→~1), empty-patch equivalence for identical documents, map-diff scenarios (added/removed/modified keys), array-diff operations (append, removal, replacement), nested diffs, round-trip patch application, null handling, and mixed-type replacements.
JSON Pointer Tests packages/rs-platform-value/src/pointer.rs	Added 238 lines of tests covering pointer parsing rules (leading /, empty pointer, missing paths), nested traversal through maps/arrays, RFC6901 token unescaping, array index bounds validation, mutating access via pointer_mut, and take() behavior for scalar/null/array values.
Bytes36 Type Tests packages/rs-platform-value/src/types/bytes_36.rs	Added 370 lines of tests for construction/conversion (fixed arrays, vector round-trips), TryFrom<Value> validation, accessor APIs, trait behavior (Hash consistency, Ord semantics), string conversion (Base64/Base58/Hex), Default values, serde round-trips (JSON and bincode), and equality/inequality assertions.

Estimated Code review effort

🎯 3 (Moderate) | ⏱️ ~28 minutes

Poem

🐰 Hop, hop—the tests arrive in bulk!
Four modules now wear coverage silk,
Paths parsed and pointers dance with glee,
Diffs patched perfectly, let us see!
Bytes tamed, assertions now flow free,
Quality's bounty for all to see! 🌟

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the primary change: adding comprehensive test coverage across four files (pointer, bytes_36, path operations, and diff) in the platform-value crate.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/platform-value-paths-and-types

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[thepastaclaw]

Review Gate

Commit: a1f2cd44

• ✅ Debounce: 151m ago (need 30m)

• ⏳ CI checks: build failure: PR title

• ✅ CodeRabbit review: comment found

• ⏳ Off-peak hours: peak window (5am-11am PT) — currently 06:15 AM PT Tuesday

• Run review now (check to override)

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

packages/rs-platform-value/src/types/bytes_36.rs (1)

531-536: Potential clippy warning: .clone() on Copy type in test.

Bytes36 is Copy, so this may trigger clippy::clone_on_copy depending on workspace lint settings.

Suggested change (keep test intent + stay clippy-clean)

     #[test]
+    #[allow(clippy::clone_on_copy)]
     fn clone_is_equal() {
         let b = Bytes36::new([77u8; 36]);
         let c = b.clone();
         assert_eq!(b, c);
     }

As per coding guidelines, "Follow rustfmt defaults and keep code clippy-clean for Rust modules" and "Ensure Rust code passes clippy linter checks via cargo clippy --workspace".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/rs-platform-value/src/types/bytes_36.rs` around lines 531 - 536, The
test clone_is_equal creates a clone of a Copy type (Bytes36) which can trigger
clippy::clone_on_copy; change the test to avoid calling .clone() by using a
direct copy instead (e.g., replace the `let c = b.clone();` usage with a copy
assignment `let c = b;`) while keeping the same assertions; locate the test
function named clone_is_equal and update the creation/assertion around
Bytes36::new([77u8; 36]) accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/rs-platform-value/src/inner_value_at_path.rs`:
- Around line 812-844: The tests miss indexed-array path cases and the removal
logic in remove_values_matching_path incorrectly checks bounds and uses unwrap
on array indexing; add unit tests exercising indexed paths (e.g., call
remove_values_matching_path("items[0].key") and an out-of-range index like
"items[99]") to assert correct removal and that out-of-range returns empty
without panicking, and fix the implementation in remove_values_matching_path by
correcting the bound check (use array.len() <= number_index or compare using
number_index >= array.len() appropriately) and replace direct indexing + unwrap
with safe access (array.get(number_index)) and handle None by returning an empty
vec.

In `@packages/rs-platform-value/src/types/bytes_36.rs`:
- Around line 373-379: The test hash_different_values asserts a property that
different values must hash differently, which is not guaranteed; update the test
to assert the guaranteed hashing behavior instead: verify that a value and its
identical clone/hash of itself produce equal hashes (use Bytes36::new,
compute_hash and e.g. a.clone() or a copy) or otherwise remove the
non-guaranteed inequality check so the test only asserts that equal values hash
equally via compute_hash(&a) and compute_hash(&a_clone).

---

Nitpick comments:
In `@packages/rs-platform-value/src/types/bytes_36.rs`:
- Around line 531-536: The test clone_is_equal creates a clone of a Copy type
(Bytes36) which can trigger clippy::clone_on_copy; change the test to avoid
calling .clone() by using a direct copy instead (e.g., replace the `let c =
b.clone();` usage with a copy assignment `let c = b;`) while keeping the same
assertions; locate the test function named clone_is_equal and update the
creation/assertion around Bytes36::new([77u8; 36]) accordingly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3de41e25-e99f-4784-90e1-79db2b42a4a0

📥 Commits

Reviewing files that changed from the base of the PR and between 8a83981 and a1f2cd4.

📒 Files selected for processing (4)

• packages/rs-platform-value/src/inner_value_at_path.rs
• packages/rs-platform-value/src/patch/diff.rs
• packages/rs-platform-value/src/pointer.rs
• packages/rs-platform-value/src/types/bytes_36.rs

[codecov]

Codecov Report

❌ Patch coverage is 99.74425% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.74%. Comparing base (8a83981) to head (a1f2cd4).
⚠️ Report is 6 commits behind head on v3.1-dev.

Files with missing lines	Patch %	Lines
packages/rs-platform-value/src/types/bytes_36.rs	99.11%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           v3.1-dev    #3437      +/-   ##
============================================
+ Coverage     80.66%   80.74%   +0.08%     
============================================
  Files          2852     2852              
  Lines        285371   286153     +782     
============================================
+ Hits         230190   231055     +865     
+ Misses        55181    55098      -83     

Components	Coverage Δ
dpp	73.28% <ø> (ø)
drive	82.49% <ø> (ø)
drive-abci	86.70% <ø> (ø)
sdk	36.55% <ø> (ø)
dapi-client	79.06% <ø> (ø)
platform-version	∅ <ø> (∅)
platform-value	92.00% <99.74%> (+1.21%)	⬆️
platform-wallet	76.09% <ø> (ø)
drive-proof-verifier	55.26% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.