chore: clean up Arrow device export

[0ax1]

Cleanups from a review of the Arrow device export path, no behavior change:

[0ax1]

@claude review

[github-actions]

Claude finished @0ax1's task in 2m 47s —— View job

---

PR Review: clean: simplify Arrow device export internals

• Gather PR context and diff
• Review arrow_binary.cu wrap-check removal
• Review canonical.rs inlining and doc changes
• Review plan_builder.rs PType::try_from change
• Post review feedback

I reviewed all three files against the codebase. This is a clean, well-justified refactor with no behavior change. No blocking issues found. Details below.

---

vortex-cuda/kernels/src/arrow_binary.cu — ✅ correct

Removing the end < offset wrap check is sound. At line 56 size > INT32_MAX is already rejected, so size ≤ 2³¹−1, and offset is a uint32_t (< 2³²). Both are widened to uint64_t, so end = offset + size < 2³³ — it cannot wrap a u64, and end ≥ offset always holds. The check was genuinely dead, and the new comment documents why. The remaining end > data_buffer_lens[buffer_index] bound is preserved.

vortex-cuda/src/dynamic_dispatch/plan_builder.rs — ✅ correctness improvement

cast.scalar_fn().as_::<Cast>() returns &Cast::Options which is &DType. The old .as_ptype() (dtype_impl.rs:180) panics via vortex_panic! on any non-primitive DType; the new PType::try_from(..) (ptype.rs:840) returns Err instead, and the let-else returns false. Since this is a predicate (is_dyn_dispatch_cast_compatible) that can be evaluated for a cast targeting a non-primitive type, this turns a potential panic into graceful rejection, and now mirrors the source-side PType::try_from guard three lines above. Good change.

vortex-cuda/src/arrow/canonical.rs — ✅ faithful inlining + doc fixes

• The list_layout_parts 5-tuple helper and export_list_layout_with_child indirection are inlined into export_list / export_list_layout. I confirmed both removed helpers had no other callers, and export_list_layout correctly stays pub(super) since arrow/list_view.rs still calls it (lines 75/382/427).
• The doc-comment swap is now correct: export_list_view takes a ListViewArray and export_list takes a standard ListArray — previously the "standard list" doc sat on the list-view function.
• The expanded RebuiltListViewChild and export_binary docs match the actual behavior (transient Dict-wrapped children from the take-based rebuild; cuDF's arrow_to_cudf_type rejecting both Binary and BinaryView).

---

Minor (non-blocking) note

The new export_arrow_validity_buffer doc asserts "every path is backed by a 4-byte-padded, tail-zeroed allocation." This is a useful contract to write down, but it's only as accurate as the device-copy layer's tail-zeroing guarantee — worth a quick double-check that no current or future path returns a non-padded BufferHandle::slice(..) that violates it, since cuDF's word-sized mask reads depend on it. Not a defect in this diff; just flagging the invariant now lives in a doc comment rather than an assertion.

No tests were added, which is appropriate for a no-behavior-change cleanup. Note I was unable to run cargo build/clippy for vortex-cuda in this environment (CUDA toolchain), so I relied on source inspection — please ensure CI's GPU build passes.
· clean/arrow-device-export-simplifications

[0ax1]

@claude keep all comments concise e.g.

/// cuDF's Arrow Device import currently rejects both Arrow `Binary` and Arrow `BinaryView`
/// (unlike `Utf8View`, which it accepts as strings), so standard `Binary` is exported as the
/// layout other Arrow Device consumers accept most widely. This path keeps conversion on the
/// CUDA stream by building `i32` offsets from view sizes and gathering inline/out-of-line view
/// bytes into one contiguous values buffer.
async fn export_binary(

commit and push

[github-actions]

Claude finished @0ax1's task in 2m 16s —— View job

---

Tighten doc comments in this PR

• Review the doc comments added/expanded in the diff
• Condense the two verbose blocks (export_arrow_validity_buffer, RebuiltListViewChild) to the dense single-paragraph style
• Commit changes locally
• Push to branch — blocked: read-only review workflow (token has no write access)

---

I condensed the two doc comments the PR expanded into multi-paragraph blocks, matching the dense single-paragraph style of the export_binary comment you cited. No content was dropped — same invariants, fewer words.

I committed the change locally, but the push was rejected (403 Permission to vortex-data/vortex.git denied to github-actions[bot]). This is a read-only review workflow, so I can't push to your branch. Please apply the patch below yourself.

Patch (git apply)

diff --git a/vortex-cuda/src/arrow/canonical.rs b/vortex-cuda/src/arrow/canonical.rs
--- a/vortex-cuda/src/arrow/canonical.rs
+++ b/vortex-cuda/src/arrow/canonical.rs
@@ -680,12 +680,10 @@ fn gather_binary_values(
 
 /// Export Vortex validity as an Arrow validity byte buffer on the CUDA device.
 ///
-/// Returns `None` for the buffer when Arrow can omit validity because all rows are valid.
-///
-/// Every returned buffer is backed by an allocation padded to a 4-byte multiple with zeroed
-/// padding so cuDF's word-sized mask reads stay in bounds: the fast path through the device
-/// copy's tail zeroing, the other paths through their own padded allocations. Bits at positions
-/// `>= len + arrow_offset` within the final data byte are unspecified, as Arrow permits.
+/// Returns `None` when Arrow can omit validity because all rows are valid. Every returned buffer
+/// is padded to a 4-byte multiple with zeroed padding so cuDF's word-sized mask reads stay in
+/// bounds. Bits at positions `>= len + arrow_offset` in the final data byte are unspecified, as
+/// Arrow permits.
 pub(super) async fn export_arrow_validity_buffer(
     validity: Validity,
     len: usize,
@@ -855,13 +853,9 @@ pub(super) enum ListChildExport {
     /// Preserve concrete child layouts, such as dictionaries,
     /// so exported data matches the schema.
     PreserveConcreteLayout,
-    /// Canonicalize temporary encodings introduced by the host ListView
-    /// rebuild, while still preserving rebuilt dictionary children.
-    ///
-    /// This is not equivalent to `export_array`: the take-based rebuild wraps
-    /// children in transient encodings (for example `take` returns `Dict`
-    /// arrays nested inside struct fields) that the pre-computed export schema
-    /// does not include, so they must be canonicalized away before export.
+    /// Canonicalize transient encodings from the take-based host ListView rebuild (for example
+    /// `Dict` arrays nested in struct fields) that the pre-computed export schema omits, while
+    /// still preserving rebuilt dictionary children.
     RebuiltListViewChild,
 }

The export_binary and export_list_view comments were already concise, so I left them as-is. I did not run cargo fmt/clippy (comment-only change; lines stay within the 100-col limit, and the CUDA toolchain isn't available here).

[codspeed-hq]

Merging this PR will degrade performance by 19.77%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 5 improved benchmarks
❌ 105 regressed benchmarks
✅ 1418 untouched benchmarks
⏩ 10 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	Simulation	compare[48]	213 µs	300.7 µs	-29.16%
❌	Simulation	compare[50]	227.7 µs	319.2 µs	-28.66%
❌	Simulation	compare[49]	228.2 µs	317.7 µs	-28.19%
❌	Simulation	compare[44]	207.5 µs	287.8 µs	-27.89%
❌	Simulation	compare[46]	218.5 µs	302.5 µs	-27.77%
❌	Simulation	compare[47]	223.5 µs	309.4 µs	-27.75%
❌	Simulation	compare[40]	190.7 µs	263.5 µs	-27.64%
❌	Simulation	compare[44]	212.2 µs	292.4 µs	-27.44%
❌	Simulation	compare[45]	218.9 µs	301 µs	-27.28%
❌	Simulation	compare[43]	209.2 µs	287.7 µs	-27.26%
❌	Simulation	compare[42]	204.6 µs	281.1 µs	-27.23%
❌	Simulation	compare[40]	195.6 µs	268.4 µs	-27.1%
❌	Simulation	compare[43]	214.2 µs	292.6 µs	-26.79%
❌	Simulation	compare[42]	209.4 µs	286 µs	-26.76%
❌	Simulation	compare[41]	204.5 µs	279.2 µs	-26.75%
❌	Simulation	compare[41]	209.3 µs	284 µs	-26.29%
❌	Simulation	compare[31]	157.7 µs	213.8 µs	-26.21%
❌	Simulation	compare[39]	199.9 µs	270.8 µs	-26.19%
❌	Simulation	compare[38]	195.5 µs	264.6 µs	-26.12%
❌	Simulation	compare[36]	187.3 µs	252.6 µs	-25.87%
...	...	...	...	...	...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing clean/arrow-device-export-simplifications (6e35e91) with develop (3731d5b)}

Footnotes

1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[0ax1]

License error is unrelated to my PR.