Skip to content

feat(antd-rust): byte-range stream methods on the REST and gRPC clients - #317

Merged
Nic-dorman merged 1 commit into
mainfrom
nic/v2-1501-antd-rust-range
Oct 12, 2026
Merged

Nic-dorman merged 1 commit into
mainfrom
nic/v2-1501-antd-rust-range

Conversation

@Nic-dorman

Copy link
Copy Markdown
Member

Summary

Follow-up to #313 (review note 1): antd-rust gets the byte-range download methods antd-go has. #313 already regenerated antd-rust's protobuf code with the new offset / length fields; this adds the client surface.

Stacked on #313. The base is nic/v2-1425-antd-byte-range-download, so the diff is antd-rust only. Merge after #313. If #313 is merged with a merge commit, GitHub retargets this PR to main once that branch is deleted.

What changes

Methods, on both Client (REST) and GrpcClient:

  • data_stream_range(data_map, offset, length) and data_stream_public_range(address, offset, length).
  • length: Option<u64>, where None reads to the end. A length past the end is clamped by the daemon.

RangeStream (src/range.rs):

  • A Stream of Bytes with public offset, length and size: what the stream carries, after clamping, plus the object size.
  • A body shorter or longer than the announced length ends in an AntdError::Network instead of passing silently.

Errors:

  • New AntdError::RangeNotSatisfiable { size: Option<u64>, message }. On REST it comes from a 416, with the size from Content-Range: bytes */size. On gRPC it comes from OUT_OF_RANGE, with the size from the x-content-range metadata that feat(antd, antd-go): carry the object size on gRPC OUT_OF_RANGE #316 adds, or None without it.
  • A daemon that ignores the range is an error, never a silent whole-object read. On REST that's a 200 with no Content-Range; on gRPC, no x-content-range.
  • A zero length is rejected before any request is sent. A Content-Range that doesn't start at the requested offset is rejected too.

gRPC OUT_OF_RANGE is mapped only on the range calls. tonic also reports a local "decoded message length too large" error as OUT_OF_RANGE. A first version mapped the code generically, and the existing grpc_prepare_upload_recv_limit_is_enforced_and_configurable test caught it reading that error as a range error. Other calls keep OUT_OF_RANGE as AntdError::Grpc.

Docs: the README covers the methods, the error variant, and the gRPC mapping note.

Compatibility

Additive methods and a new public type. Adding an AntdError variant breaks exhaustive matches, so the next antd-rust release needs a minor version bump. No version is changed here; package bumps go in their own PR.

Test evidence

Unit tests: antd-rust passes 128 lib tests (18 new), plus doc and integration tests, with fmt, clippy -D warnings and doc clean.

  • RangeStream and parsing: the header forms, including u64 overflow becoming bytes=offset-; Content-Range parsing and its rejects; the size from an unsatisfied range; short, long and errored bodies.
  • REST (mockito): 206 private and public-to-end, 416 with the size, an ignored range, the zero-length check sending nothing, and a short body.
  • gRPC (in-process mock daemon): ranged private and public responses, clamping, OUT_OF_RANGE with and without the size, and an ignored range.

Devnet: On a 25-node devnet with a 4.5 GiB object (4,831,850,553 bytes), against a daemon built from #316 (so #313's ranges plus the gRPC size metadata), a checker using these methods passes all 30 checks (ALL PASS). Every byte was compared with the source file as it streamed.

  • Ranges, private on REST and gRPC: the first 100 bytes; straddles of the first two chunk boundaries and of an 8-chunk window boundary; the last chunk's edge; the last 100 bytes to the end; clamping past EOF; a straddle of 2^32; 1 MiB past 2^32; and the whole object to the end on each transport (4.8 GB, streamed with bounded memory).
  • Public ranges on both transports: first, last-chunk edge, and tail to the end.
  • 416 / OUT_OF_RANGE at offset = size → RangeNotSatisfiable { size: Some(4831850553) } on both transports.
  • Length 0 → BadRequest on both transports.

antd logged no ERROR or WARN lines.

🤖 Generated with Claude Code

antd now serves byte ranges on its data streams: a `Range` header over
REST, `offset` / `length` over gRPC. antd-go already exposes them, and
antd-rust's generated code already had the new fields. This adds the
client methods to match.

- Methods: `data_stream_range(data_map, offset, length)` and
  `data_stream_public_range(address, offset, length)` on both Client
  and GrpcClient. `length: None` reads to the end.
- Return type: a `RangeStream`, a Stream of Bytes that carries the
  range's `offset`, `length` and the object `size`. A body shorter or
  longer than the announced length ends in an error instead of passing
  silently.
- Errors: a new `AntdError::RangeNotSatisfiable { size, message }` for
  a range starting at or past the end. REST's 416 reads the size from
  `Content-Range: bytes */size`; gRPC reads it from the
  `x-content-range` metadata, when the daemon sends it.
- Ignored ranges: a daemon that ignores the range, i.e. one that
  predates range support, is an error, never a silent whole-object
  read. A zero length is rejected before any request is sent.

gRPC OUT_OF_RANGE maps to RangeNotSatisfiable only on the range calls.
tonic also reports a local "decoded message length too large" error as
OUT_OF_RANGE, and the existing recv-limit test caught a generic mapping
misreading it.

Adding a variant to AntdError breaks exhaustive matches, so the next
antd-rust release needs a minor version bump.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@dirvine dirvine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Six-seat independent review panel — PR #317 (antd-rust byte-range streams)

Reviewed head: 5ea8563e4487013ead12815998e4cb18578028fe (pinned in an isolated clone; re-verified at posting time). Base: nic/v2-1425-antd-byte-range-download — reviewed as a stacked delta on the base feature branch's proto regen (#313), per the PR's declared base.

Panel composition (degradation disclosed): The standard panel runs five Codex CLI seats plus one GLM-5.2 OpenRouter seat. Codex CLI's ChatGPT auth refresh token was revoked at dispatch time (non-interactive re-auth unavailable), so the five Codex seats ran as clearly-labelled API fallbacks via OpenRouter — each keeping its original review lens and receiving the full diff plus complete current sources (~90–115K prompt tokens each): seat 1 openai/gpt-5.1-codex, seat 2 anthropic/claude-sonnet-4.5, seat 3 openai/gpt-5.2-codex, seat 4 moonshotai/kimi-k2.5, seat 5 z-ai/glm-5.3. Seat 6 ran as specified: z-ai/glm-5.2 via OpenRouter. All seats were fresh, independent prompts requiring verdict / blockers / non-blocking findings / limitations with file:line citations; every substantive citation below was spot-verified against the working tree at the pinned head.

Coordinator verification (independent of the seats)

  • cargo test at the pinned head: all suites green, zero failures — 128 lib tests, the proto-drift check (generated_grpc_code_matches_daemon_protos), integration/example/doc suites, and all 18 range-related tests (7 range.rs unit, 6 REST integration, 4 gRPC integration, 1 doc-test) enumerated and passing.
  • CI green at the head at review time; no prior reviews on this PR.
  • antd-go parity confirmed: DataStreamRange/RangeReader in both Go transports use the same endpoints and semantics (Rust Option<u64> where Go uses a -1 sentinel); ffi/ has no coupling to antd-rust, so no FFI surface to mirror.
  • The RangeStream state machine was independently traced: every terminal path (inner error → range.rs:84-86, complete → :88-90, short/long body → :97-98) sets done before returning, so polls after a terminal item are None (:66-67); the waker is forwarded via self.inner.as_mut().poll_next(cx); dropping the stream drops the reqwest/tonic transport, cancelling the RPC — verified for both transports.

Per-seat verdicts

Seat Model (tool) Lens Verdict Key findings
1 gpt-5.1-codex (API fallback) Rust correctness & error handling APPROVE None reported (terse report: deep hidden reasoning, summary-only output — noted as low-transparency)
2 claude-sonnet-4.5 (API fallback) Stream lifecycle, backpressure, cancellation, cleanup APPROVE (substance)¹ 8 proposed blockers, all self-retracted on inspection (done-flag, waker forwarding, tonic/reqwest drop-cancellation all correct)
3 gpt-5.2-codex (API fallback) Public API surface & REST/gRPC consistency APPROVE-WITH-CONCERNS Semver follow-through; extreme-length REST↔gRPC asymmetry (verified)
4 kimi-k2.5 (API fallback) Tests & adversarial coverage REQUEST CHANGES² Untested REST 206-without-Content-Range branch; 5 test-coverage nits
5 glm-5.3 (API fallback) Compatibility, semver, docs, repo hygiene APPROVE-WITH-CONCERNS 8 findings: semver follow-through, docs completeness, antd-go parity divergences
6 glm-5.2 (as specified) Generalist APPROVE-WITH-CONCERNS 6 findings: mid-stream OUT_OF_RANGE gap, test gaps, semver

¹ Seat 2's header line said REQUEST CHANGES but its body concludes "No blockers. The code is correct." after retracting every proposed blocker; each retraction was independently verified against range.rs here. Recorded as a malformed verdict line; substance = no defects.
² Seat 4's single blocker is the same untested-branch gap seat 6 classified as a non-blocking follow-up — adjudicated below.

Blocking issues

None. No seat found a code-level defect anywhere in the diff.

Dissent adjudication (seat 4): the claimed blocker — the untested REST 206-without-Content-Range branch at antd-rust/src/client.rs:537-540 — is sustained as fact (the branch is genuinely unexercised) but overruled as a blocker. It is a coverage gap on a one-line defensive branch (ok_or_else(|| AntdError::Internal(...))); the branch's correctness was verified by three seats plus this review; the full suite is green at the pinned head; and no seat found a code defect. Under the panel protocol this is a follow-up, not a merge blocker.

Non-blocking findings (deduplicated, all spot-verified)

Release / semver

  1. AntdError is not #[non_exhaustive], so the new RangeNotSatisfiable variant (antd-rust/src/errors.rs:127) breaks downstream exhaustive matches. The deferred bump must be a minor (0.2.0 → 0.3.0), and the release PR should mirror the PartialUpload precedent: a "(new in 0.3.0)" marker on the README error-table row and an "Upgrading from 0.2.x" note. The PR body already acknowledges this — hold it to that at release time. (seats 3, 5, 6)
  2. Once the stacked base ships, state the minimum daemon version for the range methods — the repo convention is a "Requires antd >= X" doc line (e.g. antd-rust/src/client.rs:911, :937), and the README's "Tested against antd 0.12.x" line predates range support. (seat 5)

Documentation completeness

  1. Short/long-body integrity failures surface as AntdError::Network (antd-rust/src/range.rs:74-98), documented on the type (antd-rust/src/range.rs:20-23) but not in the two methods' doc comments or the README rows — worth one sentence each so downstream matchers aren't surprised. (seat 5)
  2. error_for_status maps any REST 416 to RangeNotSatisfiable (antd-rust/src/errors.rs:322-325), slightly broader than the variant doc's "from a range method" phrasing; benign while only range calls can 416, but a clarifying sentence would remove the drift. (seats 5, 6)
  3. Pre-existing, not introduced here: the README "Data (Immutable)" table omits the base data_stream* methods even though this PR adds their range counterparts to the same table (README.md:87-88). (seat 5)

Test gaps (follow-ups)

  1. No test exercises the REST 206-without-Content-Range branch (antd-rust/src/client.rs:537-540) or the 416-without-Content-Range size: None path (antd-rust/src/client.rs:551-555). (seats 4, 6)
  2. test_grpc_data_stream_range_validates_before_sending (antd-rust/src/grpc_tests.rs:1760-1766) asserts the BadRequest but cannot assert zero RPCs the way its REST counterpart does (antd-rust/src/tests.rs:1650 uses expect(0) + m.assert()), because the mock service exposes no request counter. (seats 4, 6)
  3. gRPC short-body/long-body paths have unit coverage (antd-rust/src/range.rs:294-307) but no integration test through mock_range_response; a last-byte-only range (offset = size−1, length 1) is untested in both transports; the mock always sends 1-byte frames, so multi-byte chunking is unexercised. (seats 4, 6)

Behavioural notes (theoretical; verified correct as written)

  1. A mid-stream OUT_OF_RANGE (after the RPC has opened) maps to AntdError::Grpc, not RangeNotSatisfiable (antd-rust/src/grpc_client.rs:107-110 deliberately keeps From<tonic::Status> unmapped, since tonic also uses that code for local message-size errors). The daemon validates range bounds at call time, so this is unreachable in practice. (seat 6)
  2. A chunk that crosses the announced length is discarded whole, including its valid prefix (antd-rust/src/range.rs:74-82); a well-behaved daemon sends exactly the requested bytes. (seats 2, 6)
  3. Extreme lengths diverge by transport by design: REST renders an overflowing offset + length − 1 as an open-ended bytes=offset- header (antd-rust/src/range.rs:121-129, documented and unit-tested), while gRPC passes length verbatim (antd-rust/src/grpc_client.rs:344-376) and relies on daemon-side handling. Both yield read-to-end semantics; noting for API-parity expectations. (seat 3)
  4. antd-go parity divergences, all Rust being stricter with no wire impact: Rust rejects a satisfied Content-Range not starting at the requested offset (antd-rust/src/range.rs:149-153) where Go does not; Rust rejects over-long bodies where Go's RangeReader documents short-body signalling only; Option<u64> versus Go's -1 sentinel for read-to-end and absent size. (seat 5)

Verdict

APPROVE. The diff introduces no defect found by any of the six seats. The implementation is correct across both transports (state machine, backpressure, cancellation and drop-safety verified; error mapping — 416/OUT_OF_RANGE, ignored-range, short/long-body — deliberate and consistent), the API surface is consistent between REST and gRPC and matches antd-go's semantics while being stricter, the test suite is proportionate (18 focused tests, all green at the pinned head), and the docs cover the load-bearing semantics. The findings above are documentation, test-coverage and release-hygiene follow-ups; none block this merge. Approval does not waive required checks — CI must stay green through merge.

@Nic-dorman
Nic-dorman changed the base branch from nic/v2-1425-antd-byte-range-download to main October 12, 2026 12:25
@Nic-dorman
Nic-dorman merged commit fa94499 into main Oct 12, 2026
5 checks passed
Nic-dorman added a commit that referenced this pull request Oct 13, 2026
Follow-ups from the #315–#317 reviews (drift scripts, range error tests and docs)
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