[iceboundrock]

Spike for #256: before M1 builds to_batches() and the PyCapsule stream, can the contract's two stream error paths be built as written (§15.1: Python reads raise the LTSeq class; §16.4: the C stream ends with EIO/EINVAL and a message that starts with the class name), what does a Rust iterator under to_batches() cost, and does closing the reader stop the execution (OD-X6, kept by the owner "subject to feasibility verification")?

The findings, setup and questions for the owner are in benchmarks/stream_errors/REPORT.md; raw results are in benchmarks/stream_errors/results/.

Answers

Question	Answer
Step 1: Python reads of a reader over a Rust iterator	Keep the class. read_next_batch, iteration and read_all raise the LTSeq class and message unchanged, after the batches before the error, for six injected classes and four real engine errors (integer // by zero, INT64_MIN // -1, a failed cast, a Parquet file deleted after read_parquet)
Step 2: a C stream with LTSeq's own errno and message	Works with pyarrow: EIO gives OSError, EINVAL gives ArrowInvalid, and the message is exactly "CastError: ...". DuckDB 2.0.0 and Polars 2.1.0 carry the message but raise one class of their own whatever the errno. arrow-rs's exporter cannot do this, so M1 needs its own callbacks; the spike's capsule.rs is a starting point
Step 3: cost	No measurable cost at 8,192-row batches with Python idle. 1.1-1.4x at 1,024-row batches in a Python loop, and up to 2.3x through DuckDB. With another Python thread busy, about three GIL waits per batch (17 ms against 5 ms today); DuckDB reading to_batches() takes 28-45x as long as DuckDB reading the table
OD-X6: close()	Not for either reader pyarrow's Python API builds; yes for a Cython subclass. from_batches's reader keeps the class but ignores close() and leaving a with block (dropping the last reference does stop it). A reader imported from the C stream stops on close() but loses the class. A pyarrow.RecordBatchReader subclass compiled in Cython, sharing from_batches's C++ reader and closing the Rust iterator in close(), does both: typed errors on every read, and close() or a with exit stop the execution

So OD-X6 and §15.1's error sentence can both hold with pyarrow 26.0.1. The price is the native subclass's: a second extension module built by Cython as C++20 and linked against pyarrow's libarrow/libarrow_python (maturin does not build it, so the wheel build needs another step and every platform a C++20 toolchain), and an ABI tie to one pyarrow major version, because it relies on RecordBatchReader's layout from lib.pxd, which is not a documented API. Readers derived from it, such as reader.cast(...), are plain pyarrow readers whose close() does not stop the execution. It was tested on Linux x86-64 only.

REPORT.md's recommendation: keep §15.1 and OD-X6 as written, build the native subclass in M1 (option (e)), and ask pyarrow upstream to forward close() to the iterator (option (d)) so the subclass can go once that pyarrow is the minimum. Narrowing the Resources line to "dropping the reader stops it" (a) or giving up the class on Python reads (b) would weaken a requirement the spike shows can be met. If the owner rejects the C++ build and the pyarrow pin, an LTSeq reader class that is not a pyarrow.RecordBatchReader (c) keeps both requirements at the cost of §15.1's return type. The report also asks whether §16.4 should point DuckDB and Polars at the table itself instead of t.to_batches(), the slowest path measured, and what budget M1 has for the iterator.

What is in the PR

• src/stream_spike/ behind a new stream_spike cargo feature (no shipped build carries it):
  • reader.rs: _SpikeBatches, a PyO3 iterator over one execution, with three GIL modes. detach is ADR 0016's rule; attached is a measurement lower bound, not a candidate; poll takes a ready batch without releasing the GIL.
  • capsule.rs: a C stream with its own get_next and get_last_error.
  • classify.rs: the §20.1 class of an execution error, from the bottom of DataFusion's error chain. It handles LocalFiles' Generic wrapper around a missing file.
  • A counter of live executions, so tests can tell whether close() or dropping a reader stopped one.
• py-ltseq/ltseq/_stream_spike.py: stand-ins for the §20.1 classes with the contract's bases (the real classes do not exist yet), plus the helpers to_batches, Stream and live.
• benchmarks/stream_errors/: probe.py (every error source against every read path: pyarrow, DuckDB, Polars, plus close and drop), bench.py ([Bench] v0.5 feasibility measurements: price six contract commitments before building them #251's m3 pipeline over the R3 input, idle and with a busy Python thread, one process per cell), results, README and REPORT. _closing_reader.pyx is the native subclass, and closing_reader.py compiles it against the installed pyarrow under target/; the probe leaves its columns out when Cython or a C++20 compiler is missing.
• py-ltseq/tests/test_stream_spike.py: holds the spike to the report's outcomes, so a pyarrow, DuckDB or Polars upgrade that changes one fails here. It skips on a default build, except a test that the default build refuses the spike; the native-subclass tests also skip without Cython.
• CI runs clippy and the Rust unit tests with --features stream_spike. Its Python build uses default features, so the Python spike tests skip there. The default build changes only by cfg'd lines in lib.rs and the feature in Cargo.toml.

How it was tested

• cargo clippy --all-targets -- -D warnings, with and without --features stream_spike: clean (first commit; CI runs both on the head).
• cargo test --features stream_spike --lib stream_spike::: 6 passed.
• Spike build (maturin develop --features stream_spike, CPython 3.13.18, pyarrow 26.0.1), pytest py-ltseq/tests/test_stream_spike.py: 154 passed, 1 skipped (the default-build test), including the real overflow and the native subclass at 1 and 4 partitions; the same after deleting the cached Cython build so it compiled from scratch.
• Default build: pytest py-ltseq/tests/test_stream_spike.py test_arrow_boundary.py test_streaming.py test_m0_prototypes.py: all pass except test_duckdb_reads_ltseq_directly. That test predates this change and fails only because DuckDB 2.0.0, installed in the local throwaway venv, removed fetchone(); the project's bench group pins DuckDB 1.5.5 or later.
• pyright py-ltseq/ltseq and the new test file: 0 errors.
• bench.py produced the timing results: release build, CPython 3.13.5, pyarrow 26.0.1, DuckDB 2.0.0, Polars 2.1.0, one 8-CPU VM. probe.py was rerun after the native subclass and the overflow were added, on a debug build; it measures no time. The report's caveats cover noise and the stand-in classes.

Closes #256

[iceboundrock]

AI Code Review — Round 1

Reviewed HEAD: 2b29fa9c9f14dabc566d43cbf65550ce60108d5c against base main (merge base b357449aa14aae9c8c91c5458edd24daa155845b)

Findings

R1-F1 [blocked] benchmarks/stream_errors/REPORT.md:228-257 — The claimed incompatibility between typed errors and close() is disproven

Required resolution:

Correct the report and PR description's categorical claim that no pyarrow.RecordBatchReader can satisfy both requirements. With the same pyarrow 26.0.1, I built a small Cython RecordBatchReader subclass using pyarrow's shipped .pxd declarations: allocate the subclass with new, assign its native reader field from the from_batches reader, retain the Rust iterator, and override close() to call iterator.close(). Inherited read_next_batch(), iteration and read_all() preserve all four requested exception classes; close() and context-manager exit reduce spike.live() from 1 to 0. The initialized subclass also works with pa.table, DuckDB and Polars. Include and reproduce this native-subclass alternative in the feasibility analysis, explain its additional Cython/C++ build and pyarrow ABI coupling, and revise the recommendation accordingly. The two tested factory constructions demonstrate limitations of those constructions, not a need to weaken the contract.

R1-F2 [non-blocked] benchmarks/stream_errors/REPORT.md:52-61 — A real execution overflow exists despite the report claiming otherwise

Required resolution:

Correct the claim that ArithmeticOverflowError has no execution error today, including corresponding statements in README.md and src/stream_spike/mod.rs. A table containing x = INT64_MIN and y = -1, derived with z=lambda r: r.x // r.y, raises the spike's ArithmeticOverflowError with 'Overflow happened on: -9223372036854775808 / -1'. I verified all three Python reads and both pyarrow C-stream consumers at 1 and 4 partitions. Add this real overflow to the probe and regression coverage, regenerate the probe results, and update the real-error counts and coverage discussion.

Spec

The prototype demonstrates class preservation through the Rust iterator, native EIO/EINVAL propagation, consumer behavior and iterator cost as requested. Its from_batches close() limitation is reproducible. However, the OD-X6 feasibility conclusion overgeneralizes that limitation and excludes a working native-subclass alternative; the reported absence of a real overflow source is also incorrect.

Standards

The spike is feature-gated and leaves the public API unchanged. The default execution path releases the GIL, uses ordered_df(), and stays on the Rust/DataFusion query path. No public behavior change requires shipped-stub updates. The research conclusions need correction to meet the repository's requirement to distinguish demonstrated facts from broader conclusions.

Assessment

The tested runtime prototype behaves as documented for its chosen constructions. The primary readiness issue is the report: it recommends narrowing a contract requirement based on an impossibility claim contradicted by a working counterexample. Another fix round is needed before this feasibility evidence should guide M1.

Observations

The supplied PR context matches the bound HEAD and base main. All supplied CI checks were pending, and the controller ran no validation commands. The workflow compiles and tests the Rust spike, but its Python extension build uses default features, so the Python spike cases are skipped there; they were exercised manually during this review. No prior findings or deferred follow-up issues were supplied.

Verification

Read the full bound diff, repository guidance, issue, PR context, checks, contract sections and related execution/Arrow code. A fresh debug build with stream_spike and pyo3/extension-module succeeded. Rust spike tests: 6 passed. Python spike tests against that build: 90 passed, 1 skipped. Related Arrow-boundary and streaming tests: 49 passed; the remaining existing DuckDB test was run separately and failed because DuckDB 2.0.0 removed fetchone(), as the PR already reports. Available default extension: 1 spike-refusal test passed, 90 feature cases skipped. Both committed Markdown result files exactly match rendering their JSON inputs. Independently verified the real floor-division overflow matrix and built a scratch Cython subclass proving typed reads and effective close()/context-manager exit can coexist. The release rebuild exceeded its 10-minute limit; final validation used the successful debug rebuild, and benchmark timing cells were not rerun. git diff --check passed, the worktree remained clean at the bound HEAD, and no started build process remained running.

Summary

Two findings: one blocking correction to the OD-X6 feasibility conclusion and one limited correction to real-overflow evidence. The prototype's existing test matrix passes, but its report is not yet reliable enough to justify changing the contract.

Needs another fix round: YES

[iceboundrock]

AI Code Review — Round 2

Reviewed HEAD: 737ba6e5c546453cac1ae4f2ede8956c8bd9947c against base main (merge base b357449aa14aae9c8c91c5458edd24daa155845b)

Findings

R2-F1 [blocked] PR description, Answers and recommendation — PR description still recommends weakening the contract based on a disproven incompatibility

Required resolution:

Correct the PR description's claim that OD-X6 and typed Python errors cannot both hold for a pyarrow.RecordBatchReader, and its recommendation to narrow the resource requirement. The revised report and native-subclass prototype demonstrate both together, subject to Cython/C++ build and pyarrow ABI costs. Summarize that alternative and align the recommendation with the report. This is the unresolved PR-description portion of R1-F1.

Spec

The prototype and report address the requested error paths, consumer behavior, cost measurement and close feasibility. The PR description still contradicts the feasibility evidence and could misdirect the contract decision.

Standards

The feature-gated spike leaves the default public API unchanged. The contradictory PR description does not meet the repository's requirement that research conclusions distinguish demonstrated facts from broader claims.

Assessment

The report and regression coverage now include the native subclass and the real overflow. The remaining incorrect top-level recommendation must be corrected before this evidence is ready to guide M1.

Observations

R1-F1's report, prototype and coverage portions are addressed, but its PR-description portion remains open as R2-F1. R1-F2 is addressed by the real-overflow probe, regenerated results, updated discussion and regression tests. The supplied PR head and base match the review binding. CI builds the Python extension without the spike feature, so its Python spike cases are skipped there; the prior review reports those cases were run manually. The native subclass was tested only on Linux against pyarrow 26.0.1, as the report notes.

Verification

Inspected the bound changes, issue, PR context, previous review, checks, repository guidance, prototype and committed results. All supplied CI checks were pending; the controller ran no validation commands. cargo test --offline --features stream_spike --lib stream_spike:: passed all 6 tests after setting the Python library path; the initial attempt timed out during compilation and the next could not locate the Python shared library. Python spike tests could not be rerun because the available Python environment lacks pyarrow. git diff --check passed and the worktree remained clean. Benchmark timings were not rerun.

Summary

One blocking finding remains: the PR description retains the disproven incompatibility conclusion and outdated recommendation. The report and real-overflow evidence have otherwise been revised.

Needs another fix round: YES

[iceboundrock]

AI Code Review — Round 3

Reviewed HEAD: 737ba6e5c546453cac1ae4f2ede8956c8bd9947c against base main (merge base b357449aa14aae9c8c91c5458edd24daa155845b)

Findings

None.

Spec

The feature-gated prototype and report address the requested Python and C-stream error paths, consumer behavior, iterator cost, and reader-close feasibility. The reported native-subclass result is qualified by its build, ABI, platform and concurrency limitations.

Standards

The spike follows the repository's feature-gating and execution conventions without changing the default public API. The report separates measured outcomes from limitations and recommendations.

Assessment

The recorded evidence supports the spike's conclusions, with the noted limitations. No issue requiring a fix in this PR was identified.

Observations

R2-F1 no longer applies: the current PR description describes the native subclass, its Cython/C++ and pyarrow ABI costs, and recommends retaining the requirements in alignment with the report. R1-F2's real-overflow probe, results and regression coverage are present. The Python spike tests are skipped by the default CI build; the PR reports a separate feature-enabled run. The native subclass was tested on Linux only, as documented.

Verification

Inspected the full bound diff, issue, PR description and review history, checks, repository guidance, related prototype code and recorded results. The supplied head and base match the binding. macOS and memory checks passed; both Linux build-and-test checks were pending when read. git diff --check passed. cargo test --offline --features stream_spike --lib stream_spike:: passed 6 tests after setting the Python shared-library path; the first attempt could not locate that library. Python tests could not be rerun because the available Python lacks pyarrow. Benchmark timings were not rerun. The worktree remained clean.

Summary

No findings. The previously outstanding PR-description finding has been resolved, and the spike's remaining limitations are documented.

Needs another fix round: NO