Preserve optional output spendability

[praveenperera]

Summary by CodeRabbit

• Breaking Changes
  • Output records now preserve whether spendable was explicitly provided or omitted. Missing values remain absent in exported data, while explicit true/false values are kept as-is.
  • The effective spendable behavior is unchanged when the field is omitted.
• Bug Fixes
  • Fixed parsing and export so output labels no longer fill in a default spendable value when it was not present in the source data.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 50f5880e-34eb-4c0a-b0bc-df45ff8f7ea9

📥 Commits

Reviewing files that changed from the base of the PR and between b2a0823 and bb3aca3.

📒 Files selected for processing (4)

• CHANGELOG.md
• src/label.rs
• src/lib.rs
• src/serde_util.rs

---

📝 Walkthrough

Walkthrough

This PR changes OutputRecord.spendable from bool to Option<bool> to distinguish omitted values from explicit ones. The deserializer, accessor method, label parsing logic, tests, and changelog are updated to reflect this optional representation and round-trip behavior.

Changes

Optional spendable field

Layer / File(s)	Summary
Optional deserializer src/serde_util.rs	deserialize_string_or_bool replaced with deserialize_optional_string_or_bool, returning Option<bool> instead of a required bool.
Field and accessor update src/lib.rs	OutputRecord.spendable becomes Option<bool> with skip_serializing_if; spendable() accessor now returns unwrap_or(true).
Label parsing changes src/label.rs	into_label_and_spendable stores parsed value as Option instead of defaulting to true; related doc comment updated.
Tests and changelog src/label.rs, CHANGELOG.md	Test assertions updated for Some/None spendable cases and export omission behavior; CHANGELOG documents the breaking change.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

• bitcoinppl/bip329#9: Introduces the metadata-preserving SpendableFieldValue/try_from_str_with_metadata path that shares the same omitted-vs-explicit spendable semantics used in this PR.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: preserving optional spendability for outputs.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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 spendable-optional

---

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

[greptile-apps]

Greptile Summary

This PR changes OutputRecord.spendable from bool to Option<bool> so that omitted spendable fields round-trip faithfully (as None) while explicitly provided booleans are preserved exactly as given. The spendable() helper method retains the BIP-329 default semantic of true via unwrap_or(true).

• The public field type is now Option<bool>, which is a breaking API change documented in the changelog; callers that previously read or constructed spendable directly must update their code.
• Serialization correctly omits the field when None (via skip_serializing_if = "Option::is_none"), and the custom deserializer deserialize_optional_string_or_bool handles present fields that are JSON booleans or string booleans.
• Tests are expanded to assert both the raw spendable field value and the SpendableFieldValue metadata, including a new round-trip export test that verifies the field is omitted when absent.

Confidence Score: 5/5

Safe to merge; the change is well-scoped and all affected code paths are covered by updated tests.

The type widening is straightforward: absent fields map to None, explicit booleans/string-booleans map to Some(v), and the public spendable() accessor preserves the BIP-329 default-true semantic via unwrap_or(true). Serde attributes (default, skip_serializing_if, custom deserializer) are applied correctly and are consistent with each other. The unreachable! branch that existed in the old deserializer is cleanly removed because explicit_value() handles all cases. New round-trip and metadata tests cover the omitted, false, and string-true cases explicitly.

No files require special attention.

Important Files Changed

Filename	Overview
src/lib.rs	Core type change: OutputRecord.spendable widened to Option<bool> with correct serde attributes; spendable() accessor preserves default-true semantic via unwrap_or(true)
src/serde_util.rs	Renamed and updated deserializer returns Option<bool> by delegating to SpendableFieldValue::explicit_value(); unreachable branch removed correctly
src/label.rs	Updated into_label_and_spendable to pass explicit_value() directly; tests expanded to verify None vs Some(false) distinctions in both parse paths
CHANGELOG.md	Breaking change properly documented under Unreleased

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[JSONL input] --> B{Field present?}
    B -- No --> C["spendable = None\n(serde default)"]
    B -- Yes --> D[deserialize_optional_string_or_bool]
    D --> E{JSON type?}
    E -- bool --> F["SpendableFieldValue::Boolean(v)"]
    E -- string --> G["SpendableFieldValue::String(v)"]
    E -- other --> H[Deserialization Error]
    F --> I["explicit_value() → Some(v)"]
    G --> I
    C --> J["OutputRecord.spendable: Option<bool>"]
    I --> J
    J --> K{serialize?}
    K -- None --> L[Field omitted from output JSON]
    K -- Some(v) --> M["spendable: v in output JSON"]
    J --> N["spendable() method"]
    N --> O["unwrap_or(true) → bool"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[JSONL input] --> B{Field present?}
    B -- No --> C["spendable = None\n(serde default)"]
    B -- Yes --> D[deserialize_optional_string_or_bool]
    D --> E{JSON type?}
    E -- bool --> F["SpendableFieldValue::Boolean(v)"]
    E -- string --> G["SpendableFieldValue::String(v)"]
    E -- other --> H[Deserialization Error]
    F --> I["explicit_value() → Some(v)"]
    G --> I
    C --> J["OutputRecord.spendable: Option<bool>"]
    I --> J
    J --> K{serialize?}
    K -- None --> L[Field omitted from output JSON]
    K -- Some(v) --> M["spendable: v in output JSON"]
    J --> N["spendable() method"]
    N --> O["unwrap_or(true) → bool"]

Loading

_{Reviews (1): Last reviewed commit: "Preserve optional output spendability" | Re-trigger Greptile}