Skip to content

Fix how the string argument is treated (Fixed Bytes instead of string)#71

Open
scx1332 wants to merge 2 commits into
mainfrom
fd-agent/cli-string-parse-fixedbytes-return
Open

Fix how the string argument is treated (Fixed Bytes instead of string)#71
scx1332 wants to merge 2 commits into
mainfrom
fd-agent/cli-string-parse-fixedbytes-return

Conversation

@scx1332
Copy link
Copy Markdown
Member

@scx1332 scx1332 commented May 8, 2026

This pull request was initialized by file-diff-engine for a claude-backed agent task.

Branch: fd-agent/cli-string-parse-fixedbytes-return
Base branch: main
Task runner: claude
Model: opus
Pull request completion mode: AutoReady
Completion behavior: this task pull request starts as a draft and will be marked ready for review after the agent run completes successfully.

Task:
fix

fn parse_cli_string_value(value: &str) -> Result<FixedBytes<128>>

this function should return this FixedBytes argument instead of string to match our attribute value type

…Result<FixedBytes<128>> th

fix

fn parse_cli_string_value(value: &str) -> Result<FixedBytes<128>>

this function should return this FixedBytes argument instead of string to match our attribute value type
@scx1332
Copy link
Copy Markdown
Member Author

scx1332 commented May 8, 2026

Plan

Fix parse_cli_string_value to return Result<FixedBytes<128>> instead of Result<String> so its output matches the on-chain attribute value type (bytes32[4] / FixedBytes<128>).

Changes (all in crates/arkiv-cli/src/main.rs)

  1. Import FixedBytes from alloy_primitives.
  2. Add pack_string_attr_value(&[u8]) -> Result<FixedBytes<128>> helper: validates that the byte slice is at most 128 bytes and packs it left-aligned, zero-padded into a FixedBytes<128>. Reused by both CLI and JSON code paths.
  3. Change parse_cli_string_value(value: &str) -> Result<FixedBytes<128>> so it parses quotes/escapes as before but produces a FixedBytes<128> via the helper. The 128-byte ceiling is now enforced at parse time.
  4. Change BatchAttributeValue::String { string: FixedBytes<128> } and add a custom deserialize_with that takes a JSON string and packs it via the same helper, preserving the existing JSON batch shape ({"name": "title", "string": "the answer"}).
  5. Update build_attribute so the String arm passes string.as_slice() (already 128-byte packed) into Attribute::string(name, ...). The constructor's chunk-of-32 logic happily handles a pre-packed 128-byte slice and produces the same on-chain bytes32[4].

Verification

  • cargo build -p arkiv-cli
  • cargo test -p arkiv-cli if any tests are present
  • Manually re-check the CLI parse path: unquoted, single-/double-quoted, escapes, oversized (>128 bytes) string — last one should now error during parse.

No other crates reference parse_cli_string_value or BatchAttributeValue::String.

The CLI string parser now produces the natural on-chain attribute value
type (FixedBytes<128>) instead of a String, matching the Attribute.value
shape. The 128-byte ceiling is enforced at parse time via a shared
pack_string_attr_value helper, which is also used by a custom serde
deserializer for BatchAttributeValue::String so JSON batch files keep
accepting plain strings.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@scx1332
Copy link
Copy Markdown
Member Author

scx1332 commented May 8, 2026

Summary report

Fix landed in commit c2a47a6.

What changed

All edits in crates/arkiv-cli/src/main.rs:

  1. Imported FixedBytes from alloy_primitives.
  2. Added pack_string_attr_value(bytes: &[u8]) -> Result<FixedBytes<128>> — a shared helper that rejects inputs over 128 bytes and packs the rest into a left-aligned, zero-padded FixedBytes<128>. This mirrors the on-chain Attribute { valueType: ATTR_STRING, value: bytes32[4] } layout that arkiv_bindings::Attribute::string produces internally.
  3. Changed parse_cli_string_value(value: &str) -> Result<FixedBytes<128>> (was Result<String>). The quote/escape parsing logic is preserved verbatim; only the final return goes through the new helper, so the 128-byte limit is now enforced at parse time instead of inside Attribute::string.
  4. Changed the BatchAttributeValue::String variant's string field from String to FixedBytes<128> and added a #[serde(deserialize_with = "de_string_attr_value")] adapter so JSON batch files keep their existing shape ({"name": "title", "string": "the answer"}) — the adapter feeds the JSON string through the same pack_string_attr_value helper.
  5. Updated build_attribute so the String arm passes string.as_slice() (now 128 packed bytes) into Attribute::string(name, ...). The constructor's chunks(32) loop produces the same on-chain bytes32[4] either way, so wire-format output is unchanged.

Verification

  • cargo build -p arkiv-cli — passes.
  • cargo clippy -p arkiv-cli -- -D warnings — clean.
  • cargo test -p arkiv-cli — runs (crate has no unit tests today; nothing regressed).

Notes

  • JSON batch consumers (e.g. scripts/fixtures/attributes-all-types.json) keep working without changes; only the in-memory representation of the parsed value changed.
  • The user-visible error for an oversized string now surfaces during CLI/JSON parsing (attribute string value exceeds 128 bytes (N bytes)) rather than later inside Attribute::string.
  • No other crates reference parse_cli_string_value or BatchAttributeValue::String.

@scx1332 scx1332 marked this pull request as ready for review May 8, 2026 20:38
@scx1332 scx1332 changed the title Agent task: fix fn parse_cli_string_value(value: &str) -> Result<FixedBytes<128>> this funct Fix how the string argument is treated (Fixed Bytes instead of string) May 8, 2026
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.

1 participant