Skip to content

fix(decoder): hand out every decoded byte, not only what left the window - #605

Merged
polaz merged 1 commit into
mainfrom
fix/#604-stream-output
Oct 18, 2026
Merged

polaz merged 1 commit into
mainfrom
fix/#604-stream-output

Conversation

@polaz

@polaz polaz commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Streaming decode hands the caller every decoded byte, not only bytes that have left the window, so a frame kept open and flushed delivers its output as it goes.
  • Fixes rsync --compress-choice=zstd with this library in place of libzstd: rsync keeps one frame per session and flushes it after every file, and every file used to fail its checksum.

Changes

  • DecodeBuffer keeps a delivered mark: reads copy out every undelivered byte oldest first, hashing as they go; the window stays as history, and only bytes both delivered and outside the window are released (as upstream copies each decoded block out in zdss_flush).
  • decode_from_to, the Read adapter, collect, collect_to_writer, can_collect, the caller-workspace decoder and ZSTD_decompressStream deliver output this way; inserted or primed history counts as delivered.
  • StreamingDecoder takes a frame's checksum in the call that hands over the last of its output, so the source stands past the frame.
  • decode_loop_z000033 gains a read mode (decode through Read in 64 KiB pieces).

Testing

  • Regression tests for the flushed-session pattern through the C ABI, the Read adapter and decode_from_to; fmt, clippy, nextest (default and caller-workspace), C ABI tests, doctests and rustdoc pass.
  • rsync --compress-choice=zstd with the library preloaded on both ends transfers files that pass rsync's verification on Linux x86_64; the same run fails on main.
  • Paired A/B of the read path on x86_64 (bench profile, interleaved, with a control arm): no regression.

Closes #604

Summary by CodeRabbit

  • Bug Fixes
    • Streaming reads now return decoded data as soon as it is available, including data from flushed frames that are not yet complete.
    • Decoded output is no longer held back while the decoder retains the history needed to process later data.
    • Reading into an exactly full buffer handles frame checksums correctly, with any incomplete checksum processing deferred to a later read.
  • Examples
    • Added a read-based decoding mode that collects output in fixed-size chunks.

The streaming paths handed the caller only decoded bytes that had left
the window, so until a frame ended its newest window of output stayed
inside the decoder. A frame kept open and flushed, as rsync keeps one
zstd frame per session and flushes it after every file, delivered none
of its last window, and every file failed rsync's checksum.

- The decode buffer keeps a delivered mark: a read copies out every byte
  not handed out yet, oldest first, as far as the target takes, hashing
  it as it goes; the window stays as history, and only bytes that are
  both handed out and outside the window are let go. Upstream copies
  each decoded block out of its window buffer the same way
  (zstd_decompress.c, zdss_flush).
- decode_from_to, the Read adapter, collect, collect_to_writer,
  can_collect, the caller-workspace decoder and ZSTD_decompressStream
  hand out output this way; history a caller inserted or primed itself
  counts as handed out.
- StreamingDecoder takes a frame's checksum in the call that hands over
  the last of its output, so the source stands past the frame, as
  upstream reads the checksum once the last block is flushed.
- decode_loop_z000033 gains a `read` mode that decodes through Read in
  64 KiB pieces.

Regression tests: a_session_flushed_per_file_decodes_file_by_file (the
rsync calling pattern through the C ABI),
a_flushed_frame_in_progress_hands_over_everything_so_far,
decode_from_to_hands_over_an_unfinished_frame.

Closes #604
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-18T20:54:06.256821Z bb95e89 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: structured-world/structured-zstd/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 51b4ee61-2427-4dd8-ba02-add07eaa3f35
📥 Commits

Reviewing files that changed from the base of the PR and between ec6bba0 and bb95e89.

📒 Files selected for processing: 9
  • c-api/src/tests/streams.rs
  • zstd/examples/decode_loop_z000033.rs
  • zstd/src/decoding/decode_buffer.rs
  • zstd/src/decoding/decode_buffer/tests.rs
  • zstd/src/decoding/frame_decoder.rs
  • zstd/src/decoding/frame_decoder/tests.rs
  • zstd/src/decoding/streaming_decoder.rs
  • zstd/src/decoding/streaming_decoder/tests.rs
  • zstd/src/decoding/workspace.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The decoder now returns decoded bytes while a frame is still in progress. It tracks delivered bytes separately from buffered window history. Tests cover frame decoding, streaming reads, and C API flushes.

Changes: Decoder output delivery

Layer / File(s) Summary
Track delivered bytes
zstd/src/decoding/decode_buffer.rs, zstd/src/decoding/decode_buffer/tests.rs
DecodeBuffer tracks the delivered prefix and returns only undelivered bytes. Window-preserving drains keep history in the buffer. Tests check delivery across partial writes and wrapped buffers.
Collect output during frame decoding
zstd/src/decoding/frame_decoder.rs, zstd/src/decoding/frame_decoder/tests.rs, zstd/src/decoding/workspace.rs
FrameDecoder and WorkspaceDecoder report undelivered bytes during frame decoding. Tests cover flushed, unfinished frames and targets that fill before all blocks are consumed.
Deliver output through streaming reads
zstd/src/decoding/streaming_decoder.rs, zstd/src/decoding/streaming_decoder/tests.rs, c-api/src/tests/streams.rs, zstd/examples/decode_loop_z000033.rs
StreamingDecoder handles an available frame checksum when its output buffer fills. Tests check reads before a frame ends and after a flush. The C API test checks each file-boundary flush, and the example adds a read mode.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant StreamingDecoder
  participant FrameDecoder
  participant DecodeBuffer
  StreamingDecoder->>FrameDecoder: Request decoded output
  FrameDecoder->>DecodeBuffer: Collect undelivered bytes
  DecodeBuffer-->>FrameDecoder: Return output and retain window history
  FrameDecoder-->>StreamingDecoder: Return decoded bytes
Loading
Merge Risk: ⚪ Minimal · up to bb95e

No confirmed issue remains that should block merging after normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main decoder change: delivering every decoded byte instead of only bytes that leave the history window.
Linked Issues check Passed Issue #604 is directly linked and active. The supplied PR evidence shows delivered-byte tracking in DecodeBuffer, immediate delivery through decode_from_to, StreamingDecoder::read, collection AP…
Out of Scope Changes check Passed The changes stay within issue #604. The decode_loop_z000033 read mode supports the affected StreamingDecoder::read path and its benchmark or regression validation. The production changes and tes…
Docstring Coverage Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 50 functions across 9 files.
Full details: Linked Issues check

Explanation

Issue #604 is directly linked and active. The supplied PR evidence shows delivered-byte tracking in DecodeBuffer, immediate delivery through decode_from_to, StreamingDecoder::read, collection APIs, can_collect, caller-workspace decoding, and ZSTD_decompressStream. It also shows checksum handling at delivery time, delivered history priming, bounded buffering of the window plus one block, and regression coverage for flushed unfinished frames through C ABI, Read, and decode_from_to. The reported existing-suite, rsync verification, and paired benchmark results support the remaining acceptance criteria.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #604. The decode_loop_z000033 read mode supports the affected StreamingDecoder::read path and its benchmark or regression validation. The production changes and tests all support immediate output delivery, history retention, checksum correctness, or validation of the flushed-frame regression. No unrelated change is demonstrated.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.23009% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
zstd/src/decoding/decode_buffer.rs 98.87% 1 Missing ⚠️
zstd/src/decoding/streaming_decoder.rs 91.66% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@polaz
polaz merged commit 15e4dec into main Oct 18, 2026
30 checks passed
@polaz
polaz deleted the fix/#604-stream-output branch October 18, 2026 21:39
@sw-release-bot sw-release-bot Bot mentioned this pull request Oct 18, 2026
polaz pushed a commit that referenced this pull request Oct 19, 2026
## 🤖 New release

* `structured-zstd`: 0.2.2 -> 0.2.3

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

##
[0.2.3](v0.2.2...v0.2.3)
- 2026-10-19

### Added

- *(c-api)* the experimental zstd.h and zdict.h surface
([#602](#602))

### Fixed

- *(decoder)* hand out every decoded byte, not only what left the window
([#605](#605))
- *(encoder)* take table pointers from the table, not a slice
([#598](#598))

### Testing

- *(ci)* prove the drop-in claim against upstream's suites, the
reference CLI and real consumers
([#610](#610))
- run the lsm tests in CI and fix the one that rotted
([#608](#608))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

Co-authored-by: sw-release-bot[bot] <255865126+sw-release-bot[bot]@users.noreply.github.com>
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.

fix(decoder): hand out every decoded byte, not only what left the window

1 participant