Skip to content

Rationalize SourceStream vs Volumes with IByteSource abstraction#1053

Draft
adamhathcock with Copilot wants to merge 6 commits into
masterfrom
copilot/rationalize-sourcestream-volumes
Draft

Rationalize SourceStream vs Volumes with IByteSource abstraction#1053
adamhathcock with Copilot wants to merge 6 commits into
masterfrom
copilot/rationalize-sourcestream-volumes

Conversation

Copilot AI commented Nov 29, 2025

Copy link
Copy Markdown
Contributor
  • Explore the codebase to understand SourceStream and Volume implementations
  • Analyze how different archive formats (Zip, Rar, 7Zip, Tar) use these abstractions
  • Understand the distinction between split archives, multi-volume archives, and SOLID archives
  • Create IByteSource interface to clarify the "stream of bytes" abstraction:
    • IByteSource: Core interface for byte sources
    • FileByteSource: File-based implementation
    • StreamByteSource: Stream-based implementation
  • Add documentation comments to clarify the relationships between abstractions:
    • Updated IVolume with comprehensive documentation
    • Updated Volume with documentation explaining the layered architecture
    • Updated SourceStream with documentation for split vs volume modes
    • Updated SharpCompressStream with documentation explaining its role as a buffering/position-tracking wrapper
    • Updated IStreamStack with comprehensive documentation explaining its role in enabling stream stacking
  • Update FORMATS.md with Stream and Volume Architecture section
  • Refactor SourceStream to use IByteSource internally:
    • SourceStream now maintains a List<IByteSource> instead of separate file/stream lists
    • Added new constructor that accepts IByteSource directly
    • Existing constructors wrap FileInfo/Stream in appropriate IByteSource implementations
    • Cleaner separation between raw byte sources and archive-specific Volume semantics
  • Analyzed SharpCompressStream: Does not need structural changes - serves buffering/position tracking purpose
  • Analyzed IStreamStack: Essential infrastructure for stream stacking - 30+ implementations, heavily used. Documented its purpose and relationship to other abstractions.
  • Ensure existing tests continue to pass (869 tests passed)
  • Code review: Fixed documentation issues
  • Security scan: No vulnerabilities found
Original prompt

This section details on the original issue you should resolve

<issue_title>Rationalize SourceStream vs Volumes</issue_title>
<issue_description>SourceStream now handles multiple files and streams while the intention of Volume was to represent that.

Having both abstractions seems wrong yet both are providing value.</issue_description>

<agent_instructions>Provide a plan for this considering the current implementations of Volumes and SourceStream as well as if formats have internal streams of bytes</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@adamhathcock I probably need to create an abstraction that a stream of bytes isn't the same as a File/Stream.

7Zip can have a contiguous stream of bytes for a file or files.
Rar can have a contiguous stream of bytes for files (SOLID).
A stream of bytes can be across Files/Streams</comment_new>


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits November 29, 2025 13:33
… architecture

Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Copilot AI changed the title [WIP] Rationalize SourceStream and Volume abstractions Rationalize SourceStream vs Volumes with IByteSource abstraction Nov 29, 2025
Copilot AI requested a review from adamhathcock November 29, 2025 13:52
@adamhathcock

Copy link
Copy Markdown
Owner

@copilot this is a good start. Does source stream need to stay as it is? Refactor more using the new sources. This may need to touch source stream and volumes.

Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>

Copilot AI commented Nov 30, 2025

Copy link
Copy Markdown
Contributor Author

@copilot this is a good start. Does source stream need to stay as it is? Refactor more using the new sources. This may need to touch source stream and volumes.

Refactored SourceStream to use IByteSource internally in f2cde3a:

  • SourceStream now maintains a List<IByteSource> instead of separate List<FileInfo> and List<Stream>
  • Added new constructor accepting IByteSource directly for flexibility
  • Existing FileInfo/Stream constructors wrap inputs in FileByteSource/StreamByteSource
  • Made the internal Sources property internal to avoid exposing implementation details

All 869 tests pass.

@adamhathcock

Copy link
Copy Markdown
Owner

@copilot should SharpCompressStream change to account for any new architecture? Think and refactor

…rchitecture

Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>

Copilot AI commented Nov 30, 2025

Copy link
Copy Markdown
Contributor Author

@copilot should SharpCompressStream change to account for any new architecture? Think and refactor

After analysis, SharpCompressStream does not need structural changes. It serves a different purpose than IByteSource:

  • IByteSource: Source identification (where bytes come from - file or stream)
  • SharpCompressStream: Stream wrapping (how bytes are read - buffering, position tracking)

These are orthogonal concerns. Streams from IByteSource.OpenRead() can be wrapped by SharpCompressStream for buffering.

Added documentation in b2b9e4e clarifying SharpCompressStream's role in the architecture alongside IByteSource and SourceStream.

@adamhathcock

Copy link
Copy Markdown
Owner

@copilot analyze IStreamStack and see if it's needed. Document and refactor if needed

@adamhathcock

Copy link
Copy Markdown
Owner

@copilot resume

Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>

Copilot AI commented Nov 30, 2025

Copy link
Copy Markdown
Contributor Author

@copilot resume

Analyzed IStreamStack in fb3d326:

Conclusion: IStreamStack is essential infrastructure - it's implemented by 30+ stream classes (all compression streams, crypto streams, etc.) and is heavily used via its extension methods (SetBuffer, Rewind, StackSeek, GetPosition).

Purpose: Enables coordinated buffering and position tracking across hierarchical stream layers (e.g., raw stream → buffering → decompression → CRC validation).

Action taken: Added comprehensive documentation explaining:

  • Its role in the stream stack pattern
  • Relationship to IByteSource, SourceStream, SharpCompressStream
  • The extension methods it enables

No structural refactoring needed - the interface is well-designed for its purpose.

@adamhathcock

Copy link
Copy Markdown
Owner

@copilot resume

Copilot AI commented Dec 1, 2025

Copy link
Copy Markdown
Contributor Author

@copilot resume

All work is complete. The PR includes:

Changes made:

  • Created IByteSource interface with FileByteSource and StreamByteSource implementations
  • Refactored SourceStream to use IByteSource internally
  • Added comprehensive documentation to IVolume, Volume, SourceStream, SharpCompressStream, and IStreamStack
  • Updated FORMATS.md with Stream and Volume Architecture section

Analysis completed:

  • SharpCompressStream: No changes needed - serves buffering/position tracking purpose orthogonal to IByteSource
  • IStreamStack: Essential infrastructure with 30+ implementations - documented its role

Verification:

  • All 869 tests pass
  • Build succeeds with no warnings

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.

Rationalize SourceStream vs Volumes

2 participants