feat: add segment-based streaming ZIP writer, ZIP64 modes; replace legacy writer #2723

strantalis · 2025-09-12T16:52:33Z

Proposed Changes

Introduce SegmentWriter for deterministic, out-of-order segment writing and true streaming output.
Add CRC32 combine (crc32combine) to compute payload CRC without buffering entire payload.
Implement zip primitives (headers, central directory, EOCD) with ZIP64 mode control (Auto/Always/Never) and signed data descriptors.
Replace legacy tdf3 writer; remove old writer tests and reader/writer harness.
Update reader tests and add ZIP64 mode tests and benchmarks.
Integrate new writer into SDK TDF creation; track total size from streamed segments + finalize bytes.
Adjust TDF tests’ expected sizes and tolerances to match new ZIP layout.

Notes:

Small archives may differ by ~40–60 bytes vs. legacy due to conditional ZIP64 extras and data descriptor signature; output remains ZIP-spec compliant.

Checklist

I have added or updated unit tests
I have added or updated integration tests (if appropriate)
I have added or updated documentation

Testing Instructions

gemini-code-assist

Summary of Changes

Hello @strantalis, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the TDF archive writing mechanism to support modern streaming and out-of-order data processing paradigms. By introducing a segment-based writer and advanced ZIP64 capabilities, it enhances the SDK's ability to handle large files efficiently and reliably. The changes improve performance, reduce memory footprint during TDF creation, and ensure compliance with ZIP standards for diverse use cases.

Highlights

Segment-based Streaming ZIP Writer: Introduced a new SegmentWriter for deterministic, out-of-order segment writing, enabling true streaming output for TDF files. This allows segments to be written and processed as they become available, rather than requiring all data upfront.
CRC32 Combine Functionality: Added a CRC32CombineIEEE utility to efficiently compute the total CRC32 checksum across multiple segments without needing to buffer the entire payload, which is crucial for streaming operations.
Enhanced ZIP64 Mode Control: Implemented robust control over ZIP64 modes (Auto, Always, Never) and support for signed data descriptors, ensuring proper handling of large files and compliance with ZIP specifications.
Legacy Writer Replacement: The previous tdf3 writer implementation has been entirely replaced by the new SegmentWriter, simplifying the archive writing logic and improving flexibility.
Updated SDK Integration and Testing: The SDK's TDF creation process now integrates the new SegmentWriter. Corresponding reader tests have been updated, and new benchmarks and tests for ZIP64 modes and various segment writing patterns have been added to validate performance and correctness.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Segments flow, a stream untold, ZIP64, stories unfold. No more wait, no buffer's hold, Data's journey, brave and bold.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a significant and valuable improvement by replacing the legacy ZIP writer with a new segment-based streaming writer. The new implementation supports out-of-order segment writing, true streaming, and configurable ZIP64 modes, which is a great enhancement for handling large files and improving flexibility. The code is well-structured with new primitives, interfaces, and comprehensive tests, including benchmarks.

I've identified a few areas for improvement: a potential bug in segment cleanup logic, some non-deterministic test behavior, and opportunities to refine test and benchmark code. Additionally, there's a minor typo in a test filename (sdk/internal/archive/segment_writier_test.go should be sdk/internal/archive/segment_writer_test.go).

Overall, this is a solid contribution. Addressing the identified issues will further strengthen the robustness and correctness of the new writer.

sdk/internal/archive/segment_writer.go

sdk/internal/archive/benchmark_test.go

sdk/internal/archive/crc32combine_test.go

sdk/internal/archive/segment_writier_test.go

…eplace legacy writer - Introduce SegmentWriter for deterministic, out-of-order segment writing and true streaming output. - Add CRC32 combine (crc32combine) to compute payload CRC without buffering entire payload. - Implement zip primitives (headers, central directory, EOCD) with ZIP64 mode control (Auto/Always/Never) and signed data descriptors. - Replace legacy tdf3 writer; remove old writer tests and reader/writer harness. - Update reader tests and add ZIP64 mode tests and benchmarks. - Integrate new writer into SDK TDF creation; track total size from streamed segments + finalize bytes. - Adjust TDF tests’ expected sizes and tolerances to match new ZIP layout. Notes: Small archives may differ by ~40–60 bytes vs. legacy due to conditional ZIP64 extras and data descriptor signature; output remains ZIP-spec compliant.

strantalis · 2025-09-12T22:13:44Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a significant and well-executed refactoring of the archive writing logic. The new SegmentWriter provides a robust solution for deterministic, out-of-order segment writing and true streaming output, which is a great improvement over the legacy writer. The implementation correctly uses crc32combine to avoid buffering payloads, and the new ZIP primitives and configuration options make the system more flexible and maintainable. The accompanying tests and benchmarks are comprehensive. I have a few minor suggestions regarding code comments, use of deprecated APIs, and a filename typo, which are detailed in the review comments. Overall, this is an excellent contribution.

sdk/internal/archive/benchmark_test.go

sdk/internal/archive/crc32combine_test.go

sdk/internal/archive/segment_writier_test.go

dmihalcik-virtru · 2025-09-15T11:52:38Z

sdk/internal/archive/benchmark_test.go

+				right++
+			}
+		}
+	case "random", "mixed":


Suggestion: Insrtead of

What's the suggestion?

jentfoo

Overall looks good. Some of the zip logic could benefit from fuzz testing, but I didn't see anything concerning.

jentfoo · 2025-09-15T15:20:56Z

sdk/internal/archive/segment_writer.go

+	sw.finalized = true
+
+	// Return the final bytes
+	result := make([]byte, buffer.Len())


Since buffer is constructed in function (and as far as I see not retained), why not just returned buffer.Bytes() directly?

strantalis requested review from a team as code owners September 12, 2025 16:52

github-actions bot added comp:sdk A software development kit, including library, for client applications and inter-service communicati size/xl labels Sep 12, 2025

gemini-code-assist bot reviewed Sep 12, 2025

View reviewed changes

strantalis force-pushed the feature/archive-writer branch from 2badaa9 to 7bfa948 Compare September 12, 2025 16:56

recommended fixes

51140af

gemini-code-assist bot reviewed Sep 12, 2025

View reviewed changes

strantalis changed the title ~~feat(archive): add segment-based streaming ZIP writer, ZIP64 modes; replace legacy writer~~ feat: add segment-based streaming ZIP writer, ZIP64 modes; replace legacy writer Sep 12, 2025

strantalis added 2 commits September 12, 2025 18:19

address gemini feedback and remove deprecated rand.Seed

51dde06

update package comments

a260dce

strantalis requested review from jentfoo and sujankota September 12, 2025 22:31

dmihalcik-virtru reviewed Sep 15, 2025

View reviewed changes

jentfoo approved these changes Sep 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: add segment-based streaming ZIP writer, ZIP64 modes; replace legacy writer #2723

feat: add segment-based streaming ZIP writer, ZIP64 modes; replace legacy writer #2723

Uh oh!

strantalis commented Sep 12, 2025 •

edited

Loading

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

strantalis commented Sep 12, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmihalcik-virtru Sep 15, 2025

Uh oh!

jrschumacher Sep 15, 2025

Uh oh!

jentfoo left a comment

Uh oh!

jentfoo Sep 15, 2025

Uh oh!

Uh oh!

feat: add segment-based streaming ZIP writer, ZIP64 modes; replace legacy writer #2723

Are you sure you want to change the base?

feat: add segment-based streaming ZIP writer, ZIP64 modes; replace legacy writer #2723

Uh oh!

Conversation

strantalis commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Checklist

Testing Instructions

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

strantalis commented Sep 12, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmihalcik-virtru Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

jrschumacher Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

jentfoo left a comment

Choose a reason for hiding this comment

Uh oh!

jentfoo Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

strantalis commented Sep 12, 2025 •

edited

Loading