Skip to content

docs(arrow-convert): document and doctest the ArrowData read-back API - #3084

Merged
trunk-io[bot] merged 1 commit into
mainfrom
claude/dreamy-bardeen-v852wi-arrow-convert-doctests
Aug 11, 2026
Merged

docs(arrow-convert): document and doctest the ArrowData read-back API#3084
trunk-io[bot] merged 1 commit into
mainfrom
claude/dreamy-bardeen-v852wi-arrow-convert-doctests

Conversation

@phil-opp

@phil-opp phil-opp commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Issue

ArrowData's TryFrom<&ArrowData> implementations (libraries/arrow-convert/src/from_impls.rs) are the primary public API for reading received Arrow data back into Rust values — the read-back counterpart to IntoArrow. IntoArrow has a documented, compile-checked doctest (lib.rs), but the read-back side had none, and its non-obvious, load-bearing contract was only discoverable by reading the source:

  • scalar conversions (u8, f32, String, &str, chrono types) require exactly one element and no nulls;
  • slice / Vec conversions accept any length but reject any nulls.

Fix

Expand the ArrowData doc comment to describe both conversion shapes and their length/null contracts, with a compile-checked doctest that:

  • round-trips a scalar (u8), a String, and a Vec<i32> via try_into(), and
  • shows that a multi-element array cannot be read as a scalar (returns Err).

Documentation only — no behavior change. This is a good fit for the "prefer compile-checked doctests" documentation goal: the doctest locks the contract against future drift.

Validation

  • cargo test -p dora-arrow-convert --doc — 2 passed (the new ArrowData doctest plus the existing IntoArrow one).
  • cargo fmt --all -- --check and cargo clippy -p dora-arrow-convert -- -D warnings clean.

🤖 This is a machine-generated pull request opened by Claude Code as part of an automated code-review pass. Please review carefully before merging.

🤖 Generated with Claude Code


Generated by Claude Code

`ArrowData`'s `TryFrom<&ArrowData>` implementations are the primary public
API for reading received Arrow data back into Rust values, paired with
`IntoArrow` for the send direction. `IntoArrow` carries a documented,
compile-checked doctest, but the read-back side had none, and its
non-obvious, load-bearing contract was only discoverable from the source:
scalar conversions (`u8`, `f32`, `String`, ...) require exactly one
element and no nulls, while slice/`Vec` conversions accept any length but
reject nulls.

Expand the `ArrowData` doc comment to describe both conversion shapes and
their length/null contracts, with a compile-checked doctest that
round-trips a scalar, a string, and a `Vec`, and shows that a
multi-element array cannot be read as a scalar. No behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SD4dBVimzepwKSh2b9F8aG
@trunk-io

trunk-io Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

😎 Merged successfully - details.

phil-opp commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Automated review by Claude — this is a fully automated review with no human in the loop.

I reviewed this diff and found no issues. It is a docs-only change adding a doc comment plus a compile-checked doctest for ArrowData's TryFrom<&ArrowData> read-back API; no behavior change. The doctest correctly exercises the scalar single-element rule and the Vec/any-length rule, and asserts that a multi-element array cannot be read back as a scalar.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

@phil-opp the Trunk merge queue failed for this PR.

See the Trunk merge-status comment for details.

Posted as a new comment so GitHub sends an email — Trunk's sticky comment is edited in place and won't trigger a notification.

Copy link
Copy Markdown
Collaborator Author

These fault_tolerance_tests failures don't originate from this PR. #3084 only adds documentation and a doctest to libraries/extensions/arrow-convert — it doesn't touch binaries/daemon.

The failing Test (ubuntu-latest) ran against the Trunk merge-queue batch branch (533dc2a), which bundles this PR together with others ahead of it in the queue. The panic locations (binaries/daemon/src/lib.rs:8289 / 8327 / 8377 / 8407) sit ~20 lines below where these tests actually live on main — e.g. wedged_user_input_node_is_a_finish_straggler's assert is at lib.rs:8268 here — so the batch contains another PR that shifted the file and changed finish_stragglers classification behavior.

Verified locally on current main: all four named tests, and the full fault_tolerance_tests suite (55 tests), pass:

test result: ok. 55 passed; 0 failed; 0 ignored; 0 measured; 149 filtered out

Nothing to fix on this PR — it should re-test clean once the queue re-batches it without the offending sibling.


Generated by Claude Code

@trunk-io
trunk-io Bot merged commit 45f8c2f into main Aug 11, 2026
27 checks passed
@trunk-io
trunk-io Bot deleted the claude/dreamy-bardeen-v852wi-arrow-convert-doctests branch August 11, 2026 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants