fix: propagate deserialization errors in proof display instead of hiding them

[QuantumExplorer]

Summary

• element_hex_to_ascii and optional_element_hex_to_ascii were silently swallowing Element deserialization errors by falling back to hex encoding via unwrap_or_else
• Changed both functions to return Result<String, fmt::Error> so errors propagate through the fmt::Display chain
• Updated all callers in node_to_string, op_to_string, decode_merk_proof, and the Display impls for ProvedPathKeyValue / ProvedPathKeyOptionalValue

Supersedes #363 (which was stale and had merge conflicts with the current codebase).

Test plan

• cargo check -p grovedb passes
• Existing proof display tests still pass
• Deserialization errors in proof formatting now surface as fmt::Error instead of being silently replaced with hex

Generated with Claude Code

Summary by CodeRabbit

• Chores

  • Updated serialization feature to enable additional query serde support.

• Refactor

  • Improved error handling in proof decoding and formatting for more robust, fail-safe behavior.

• Bug Fixes

  • Prevented crashes when decoding/formatting invalid proof or element data; errors are now surfaced cleanly.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR enables grovedb-query/serde via Cargo features, adds an explicit lifetime to a debugger read-lock return type, and converts several proof-formatting helpers and their callers to return Result<String, fmt::Error> with tests adjusted.

Changes

Cohort / File(s)	Summary
Cargo features grovedb/Cargo.toml	Updated the serde feature to also enable grovedb-query/serde alongside existing serde feature flags.
Debugger lifetime change grovedb/src/debugger.rs	Adjusted get_checkpointed_grovedb return type to Result<RwLockReadGuard<'_, GroveDb>, AppError> (added explicit lifetime on the guard).
Proof formatting (logic + callers) grovedb/src/operations/proof/mod.rs, grovedb/src/operations/proof/...	Made decode_merk_proof, op_to_string, node_to_string fallible (Result<String, fmt::Error>) and propagated errors with ? at callers; formatted proof display paths updated accordingly.
Proof utilities & tests grovedb/src/operations/proof/util.rs, grovedb/src/operations/proof/..._tests.rs	Changed optional_element_hex_to_ascii and element_hex_to_ascii to return Result<String, fmt::Error>; removed unwraps, added error propagation and new/updated tests for valid/invalid and None cases.
Misc (Python module) src/calculator.py	Added coderabbit_add(x, y), changed coderabbit_formula(x, y) → coderabbit_formula(x, y, z), and renamed old_global_var → new_global_var.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hopped through Cargo, lifetimes in tow,

Proofs now check errors where once strings did flow,
New helpers and tests in a tidy parade,
A coderabbit cheers for the safety we made! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 35.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: propagating deserialization errors in proof display logic instead of hiding them via unwrap_or_else, which aligns with the core modifications across multiple files.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/proof-display-propagate-errors

---

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.}

[codecov]

Codecov Report

❌ Patch coverage is 88.73239% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.51%. Comparing base (747fd86) to head (9723641).
⚠️ Report is 3 commits behind head on develop.

Files with missing lines	Patch %	Lines
grovedb/src/operations/proof/util.rs	90.00%	5 Missing ⚠️
grovedb/src/operations/proof/mod.rs	85.71%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #431      +/-   ##
===========================================
+ Coverage    75.47%   75.51%   +0.03%     
===========================================
  Files          181      181              
  Lines        46405    46447      +42     
===========================================
+ Hits         35025    35073      +48     
+ Misses       11380    11374       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

[coderabbitai]

🧹 Nitpick comments (1)

grovedb/src/operations/proof/util.rs (1)

226-230: Consider logging deserialization errors before discarding context.

Since fmt::Error is a unit struct that cannot carry error context, the original deserialization error is lost. While this is a limitation of std::fmt::Display, consider adding a tracing::warn! or similar before returning fmt::Error to aid debugging when element deserialization fails during proof display.

 pub fn element_hex_to_ascii(hex_value: &[u8]) -> Result<String, fmt::Error> {
-    let element =
-        Element::deserialize(hex_value, GroveVersion::latest()).map_err(|_| fmt::Error)?;
+    let element = Element::deserialize(hex_value, GroveVersion::latest()).map_err(|e| {
+        // Consider: tracing::warn!("Element deserialization failed during display: {e}");
+        fmt::Error
+    })?;
     Ok(element.to_string())
 }

🤖 Prompt for AI Agents

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

In `@grovedb/src/operations/proof/util.rs` around lines 226 - 230, The
element_hex_to_ascii function currently swallows the Element::deserialize error
by mapping it to fmt::Error; update element_hex_to_ascii to capture the original
deserialization error and emit a log (e.g., tracing::warn! or log::warn!) with
the error and the offending hex_value before returning fmt::Error so you
preserve context for debugging while keeping the function signature unchanged;
reference Element::deserialize, GroveVersion::latest, and element_hex_to_ascii
to locate the code to modify.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@grovedb/src/operations/proof/util.rs`:
- Around line 226-230: The element_hex_to_ascii function currently swallows the
Element::deserialize error by mapping it to fmt::Error; update
element_hex_to_ascii to capture the original deserialization error and emit a
log (e.g., tracing::warn! or log::warn!) with the error and the offending
hex_value before returning fmt::Error so you preserve context for debugging
while keeping the function signature unchanged; reference Element::deserialize,
GroveVersion::latest, and element_hex_to_ascii to locate the code to modify.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4247bb5 and 195c6fb.

📒 Files selected for processing (4)

• grovedb/Cargo.toml
• grovedb/src/debugger.rs
• grovedb/src/operations/proof/mod.rs
• grovedb/src/operations/proof/util.rs

[coderabbitai]

🧹 Nitpick comments (1)

grovedb/src/operations/proof/util.rs (1)

226-230: Consider preserving error context for debugging.

The original deserialization error is discarded with .map_err(|_| fmt::Error). While fmt::Error is a unit struct with no payload, losing the underlying error makes debugging harder when formatting fails.

Since fmt::Error cannot carry context, consider logging the error before converting it, or documenting why the error details are intentionally dropped.

💡 Optional: Log error before discarding

 pub fn element_hex_to_ascii(hex_value: &[u8]) -> Result<String, fmt::Error> {
-    let element =
-        Element::deserialize(hex_value, GroveVersion::latest()).map_err(|_| fmt::Error)?;
+    let element = Element::deserialize(hex_value, GroveVersion::latest()).map_err(|e| {
+        // fmt::Error cannot carry context; log for debugging
+        #[cfg(feature = "logging")]
+        log::debug!("Element deserialization failed during display: {}", e);
+        fmt::Error
+    })?;
     Ok(element.to_string())
 }

As per coding guidelines: "Wrap errors with context using .map_err(|e| Error::CorruptedData(format!("context: {}", e))) pattern" - however, fmt::Error is a unit struct and cannot carry context, so logging or documentation is the practical alternative here.

🤖 Prompt for AI Agents

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

In `@grovedb/src/operations/proof/util.rs` around lines 226 - 230, The current
element_hex_to_ascii function discards the deserialization error with
.map_err(|_| fmt::Error), losing context; capture the error from
Element::deserialize(hex_value, GroveVersion::latest()) as e, log it (e.g.,
tracing::error! or log::error!) with a concise message including e, then convert
to fmt::Error for the function signature so callers still receive fmt::Error but
the original error is preserved in logs; alternatively, if you prefer to
propagate rich errors, change the return type to return a richer error
(propagate e) instead of fmt::Error—refer to element_hex_to_ascii,
Element::deserialize, and GroveVersion::latest to locate the code.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@grovedb/src/operations/proof/util.rs`:
- Around line 226-230: The current element_hex_to_ascii function discards the
deserialization error with .map_err(|_| fmt::Error), losing context; capture the
error from Element::deserialize(hex_value, GroveVersion::latest()) as e, log it
(e.g., tracing::error! or log::error!) with a concise message including e, then
convert to fmt::Error for the function signature so callers still receive
fmt::Error but the original error is preserved in logs; alternatively, if you
prefer to propagate rich errors, change the return type to return a richer error
(propagate e) instead of fmt::Error—refer to element_hex_to_ascii,
Element::deserialize, and GroveVersion::latest to locate the code.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 195c6fb and 9723641.

📒 Files selected for processing (1)

• grovedb/src/operations/proof/util.rs

[QuantumExplorer]

Self Reviewed