Skip to content

Preserve spendable metadata - #9

Merged
praveenperera merged 2 commits into
masterfrom
preserve-spendable-metadata
Jun 18, 2026
Merged

Preserve spendable metadata#9
praveenperera merged 2 commits into
masterfrom
preserve-spendable-metadata

Conversation

@praveenperera

@praveenperera praveenperera commented Jun 17, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Added label import functionality that preserves metadata about the spendable field format, distinguishing between omitted, boolean, and string representations.
  • Documentation

    • Updated changelog documenting the new label import capability.

Add a parsing path that reports whether output labels included an explicit spendable field, so callers can distinguish omitted values from explicit booleans.

Preserve output  field presence and representation
when parsing labels so callers can distinguish omitted values
from explicit booleans while keeping default parsing behavior
unchanged.
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 830229a7-cd9d-44f8-93dd-695fac57ae94

📥 Commits

Reviewing files that changed from the base of the PR and between e7d13af and 2315548.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • src/label.rs
  • src/lib.rs
  • src/serde_util.rs

📝 Walkthrough

Walkthrough

Adds Labels::try_from_str_with_metadata, a new public JSONL parsing method that returns a ParsedLabels struct bundling normalized Labels with per-output spendable field metadata. Three new public types (ParsedLabels, OutputSpendableField, SpendableFieldValue) and a custom Deserialize impl are introduced in src/lib.rs. The existing deserialize_string_or_bool in src/serde_util.rs is refactored to delegate to the new SpendableFieldValue type.

Changes

Spendable Metadata API

Layer / File(s) Summary
Public spendable metadata types and custom Deserialize
src/lib.rs
Adds ParsedLabels, OutputSpendableField, and SpendableFieldValue (Omitted/Boolean/String variants) with an explicit_value() helper. Adds a custom Deserialize for SpendableFieldValue accepting JSON booleans or case-insensitive string booleans, erroring on unrecognized strings.
deserialize_string_or_bool refactored to delegate to SpendableFieldValue
src/serde_util.rs
Replaces the local untagged StringOrBool enum and manual string parsing with delegation to SpendableFieldValue::deserialize, mapping Boolean/String variants to bool and treating Omitted as unreachable.
try_from_str_with_metadata implementation, internal helpers, tests, and changelog
src/label.rs, CHANGELOG.md
Adds internal ParsedLabelLine/ParsedOutputRecord structs for type-tagged JSON deserialization. Implements Labels::try_from_str_with_metadata iterating JSONL lines, collecting Labels and per-output OutputSpendableField metadata into ParsedLabels. Adds unit tests for omitted, boolean, string, and mixed inputs, and documents the new API in CHANGELOG.md.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 Hop! A new field emerges from the JSON stream,
Omitted, Boolean, String — each one a gleam.
The rabbit inspects each label with care,
Preserving the metadata lurking in there.
Now try_from_str_with_metadata leads the way,
No spendable secret shall ever go astray! 🥕

🚥 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 'Preserve spendable metadata' directly and clearly describes the main objective of the PR, which adds functionality to preserve spendable metadata when parsing output labels.
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.

✏️ 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 preserve-spendable-metadata

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

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

@praveenperera
praveenperera force-pushed the preserve-spendable-metadata branch from 9e7c45c to 0df0f9f Compare June 17, 2026 18:26
@praveenperera
praveenperera requested a review from Copilot June 17, 2026 18:28
@praveenperera

Copy link
Copy Markdown
Member Author

@greptileai review

@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds Labels::try_from_str_with_metadata, a new parsing entry point that preserves whether each output's spendable field was omitted, a JSON boolean, or a JSON string boolean — information that is lost by the normalizing try_from_str.

  • Introduces ParsedLabels, OutputSpendableField, and SpendableFieldValue as public types; SpendableFieldValue carries its own Deserialize impl and becomes the shared primitive reused by the existing serde_util::deserialize_string_or_bool.
  • Adds a private ParsedLabelLine / ParsedOutputRecord pair in label.rs that drives the metadata-aware parse path, with tests covering all three spendable variants and a mixed-type JSONL fixture.

Confidence Score: 5/5

Safe to merge — the new parse path is additive, the existing try_from_str and file-based imports are untouched, and all three spendable variants are covered by tests.

The change introduces a well-scoped parallel parse path with no modifications to existing behavior. SpendableFieldValue is the single source of truth for spendable deserialization, reused by the existing serde_util helper. The always-Some return for output metadata is deliberate and tested. No regressions are evident in the existing test suite paths.

No files require special attention.

Important Files Changed

Filename Overview
src/lib.rs Adds ParsedLabels, OutputSpendableField, and SpendableFieldValue public types; clean implementation with correct Default, Deserialize, and explicit_value() semantics
src/label.rs Adds try_from_str_with_metadata plus ParsedLabelLine/ParsedOutputRecord; the always-Some return from into_label_and_spendable is intentional to capture Omitted state; tests cover all three spendable variants
src/serde_util.rs Refactored to delegate to SpendableFieldValue::deserialize; Omitted branch is correctly marked unreachable since serde only invokes this deserializer when the field is present
CHANGELOG.md Adds changelog entry for try_from_str_with_metadata under Unreleased

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[JSONL input string] --> B{Which parser?}
    B -->|try_from_str| C[serde_json::from_str per line\ninto Label enum]
    B -->|try_from_str_with_metadata| D[serde_json::from_str per line\ninto ParsedLabelLine enum]

    C --> E[Labels]

    D --> F{Label type?}
    F -->|tx / addr / pubkey / input / xpub| G[Label variant, spendable = None]
    F -->|output| H[ParsedOutputRecord]

    H --> I[SpendableFieldValue\nOmitted / Boolean / String]
    I -->|explicit_value\n.unwrap_or true| J[OutputRecord\nspendable: bool]
    I --> K[OutputSpendableField\nref_ + value]

    G --> L[parsed_labels Vec]
    J --> L
    K --> M[output_spendable Vec]

    L --> N[ParsedLabels\nlabels + output_spendable]
    M --> N
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 string] --> B{Which parser?}
    B -->|try_from_str| C[serde_json::from_str per line\ninto Label enum]
    B -->|try_from_str_with_metadata| D[serde_json::from_str per line\ninto ParsedLabelLine enum]

    C --> E[Labels]

    D --> F{Label type?}
    F -->|tx / addr / pubkey / input / xpub| G[Label variant, spendable = None]
    F -->|output| H[ParsedOutputRecord]

    H --> I[SpendableFieldValue\nOmitted / Boolean / String]
    I -->|explicit_value\n.unwrap_or true| J[OutputRecord\nspendable: bool]
    I --> K[OutputSpendableField\nref_ + value]

    G --> L[parsed_labels Vec]
    J --> L
    K --> M[output_spendable Vec]

    L --> N[ParsedLabels\nlabels + output_spendable]
    M --> N
Loading

Reviews (3): Last reviewed commit: "Parse labels with output spendable metad..." | Re-trigger Greptile

Comment thread src/serde_util.rs Outdated
Comment thread src/label.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an API for importing BIP329 JSONL while preserving the original spendable field presence and JSON representation for output records (omitted vs boolean vs string-boolean), which is otherwise normalized away by OutputRecord.

Changes:

  • Introduce ParsedLabels, OutputSpendableField, and SpendableFieldValue to expose output spendable metadata.
  • Add Labels::try_from_str_with_metadata() to return normalized labels plus captured output spendable metadata, along with tests.
  • Reuse SpendableFieldValue parsing in the spendable field deserializer for consistent handling of string/boolean forms.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/lib.rs Adds new public metadata-carrying types (ParsedLabels, OutputSpendableField, SpendableFieldValue) and parsing helpers.
src/label.rs Adds try_from_str_with_metadata() and tests to preserve output spendable field metadata during import.
src/serde_util.rs Refactors string-or-bool parsing to use the new SpendableFieldValue deserialization logic.
CHANGELOG.md Documents the new metadata-preserving import API.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/label.rs Outdated
Comment thread src/serde_util.rs Outdated
Comment on lines +8 to +13
// keep normal output parsing aligned with metadata-aware parsing
let value = <SpendableFieldValue as serde::Deserialize>::deserialize(deserializer)?;

match StringOrBool::deserialize(deserializer)? {
StringOrBool::Bool(b) => Ok(b),
StringOrBool::String(s) => match s.to_ascii_lowercase().as_str() {
"true" => Ok(true),
"false" => Ok(false),
string => {
let msg = format!("Invalid boolean string: {string}");
Err(serde::de::Error::custom(msg))
}
},
}
value
.explicit_value()
.ok_or_else(|| serde::de::Error::custom("missing spendable value"))
@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds Labels::try_from_str_with_metadata and supporting types (ParsedLabels, OutputSpendableField, SpendableFieldValue) so callers can distinguish whether an output's spendable field was omitted, a JSON boolean, or a JSON string boolean. serde_util::deserialize_string_or_bool is refactored to delegate to the new SpendableFieldValue deserializer, keeping both parse paths aligned; three new tests cover all three spendable field variants.

Confidence Score: 4/5

Safe to merge — the new parsing path correctly handles all three spendable field representations, and the serde_util refactor preserves the existing deserialization behaviour for OutputRecord.

The double-parse approach in try_from_str_with_metadata is sound and the three new tests cover the intended cases. The unreachable error branch in serde_util and the absence of a file-based metadata variant are the only rough edges.

src/serde_util.rs (dead ok_or_else arm) and src/label.rs (no try_from_file_with_metadata counterpart) deserve a quick second look before merging.

Important Files Changed

Filename Overview
src/lib.rs Adds ParsedLabels, OutputSpendableField, and SpendableFieldValue with a custom Deserialize impl that correctly maps JSON booleans/strings to typed variants and reserves Omitted for the default-attribute path only.
src/label.rs Adds try_from_str_with_metadata which double-parses each output line to capture raw spendable field state; logic is correct but a file-based counterpart is missing.
src/serde_util.rs Refactored to delegate to SpendableFieldValue::deserialize; behavior is unchanged but the ok_or_else error arm for Omitted is unreachable.
CHANGELOG.md Documents the new try_from_str_with_metadata method under Unreleased/Added.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[JSONL input string] --> B{try_from_str_with_metadata}
    B --> C[Trim & iterate lines]
    C --> D[serde_json::from_str as Label]
    D -->|parse error| E[Return ParseError]
    D --> F{Label::Output?}
    F -->|No| G[Append to labels vec]
    F -->|Yes| H[serde_json::from_str as OutputSpendableMetadata]
    H -->|parse error| E
    H --> I{spendable field in JSON?}
    I -->|Absent| J[SpendableFieldValue::Omitted]
    I -->|JSON bool| K[SpendableFieldValue::Boolean]
    I -->|JSON string| L{valid 'true'/'false'?}
    L -->|Yes| M[SpendableFieldValue::String]
    L -->|No| E
    J & K & M --> N[Push OutputSpendableField ref_+value]
    N --> G
    G --> O[Return ParsedLabels labels + output_spendable vec]
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 string] --> B{try_from_str_with_metadata}
    B --> C[Trim & iterate lines]
    C --> D[serde_json::from_str as Label]
    D -->|parse error| E[Return ParseError]
    D --> F{Label::Output?}
    F -->|No| G[Append to labels vec]
    F -->|Yes| H[serde_json::from_str as OutputSpendableMetadata]
    H -->|parse error| E
    H --> I{spendable field in JSON?}
    I -->|Absent| J[SpendableFieldValue::Omitted]
    I -->|JSON bool| K[SpendableFieldValue::Boolean]
    I -->|JSON string| L{valid 'true'/'false'?}
    L -->|Yes| M[SpendableFieldValue::String]
    L -->|No| E
    J & K & M --> N[Push OutputSpendableField ref_+value]
    N --> G
    G --> O[Return ParsedLabels labels + output_spendable vec]
Loading

Comments Outside Diff (1)

  1. src/label.rs, line 64-79 (link)

    P2 No file-based variant of the metadata-preserving parser

    try_from_str_with_metadata has no corresponding try_from_file_with_metadata. The existing try_from_file is the primary entry point for most importers (wallets loading a .jsonl file on disk), so callers who want the spendable-field metadata but load from a file path must first read the whole file into a String manually before they can call the new method. A try_from_file_with_metadata mirror would make the new API symmetric and avoid that boilerplate.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (2): Last reviewed commit: "Preserve output spendable metadata" | Re-trigger Greptile

Comment thread src/serde_util.rs Outdated
Refactor label parsing to preserve output-level spendable metadata while normalizing Label records. Introduces a serde-tagged ParsedLabelLine enum and ParsedOutputRecord to deserialize each JSONL line by type, extract Label instances, and separately collect OutputSpendableField entries for outputs. Updates try_from_str_with_metadata to iterate lines, convert parsed variants into labels and optional spendable metadata, and accumulate both. Adjusts serde_util deserializer to return the explicit boolean value for present spendable fields (and mark omitted as unreachable). Adds a unit test verifying mixed label types and output spendable metadata handling. Also updates imports to support the new types.
@praveenperera
praveenperera merged commit 91d124b into master Jun 18, 2026
10 checks passed
@praveenperera
praveenperera deleted the preserve-spendable-metadata branch June 18, 2026 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants