Overhaul for streaming data directly from zip (optimization)

[frheault]

(Similar description to trx-rs PR)

Proposed modifications to allow fair comparison for benchmarking across languages, the biggest modification is to up/down-cast to uint32 offsets since it is not completely illegal. Some of the benchmark files add offsets as uint64. I believe this is mostly to smooth operations between languages, I personally think that virtually no one will create a tractogram with 40M streamlines with 100 points each, but it is possible.

I apologize for the formatting, I believe my VScode did some automatic formatting, @mattcieslak if you could tell me what standard/linter/tool to use (or maybe I should manually revert the identical lines?).

I believe some unit testing is needed to verify if everything is alright, but I tested single language round-trip (load/save) and between language compatibility. Then I benchmarked on big files: https://github.com/tee-ar-ex/trx-manuscript-2026-benchmark

[Copilot]

Pull request overview

This PR updates AnyTrxFile::_create_from_pointer()’s handling of groups/ entries to allow loading group membership arrays stored in additional integer dtypes by converting them into the internal uint32 representation, enabling more consistent cross-language benchmarking/interoperability.

Changes:

• Allow groups/ arrays with integer dtypes beyond uint32 (e.g., uint64, int64, int32, uint16, etc.) by casting to uint32.
• Add a guard intended to prevent unsafe downcasting when NB_STREAMLINES exceeds 32-bit capacity.
• Materialize group arrays to owned memory before storing in trx.groups.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

❌ Patch coverage is 53.05439% with 561 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.69%. Comparing base (abe6709) to head (9b3a330).

Files with missing lines	Patch %	Lines
src/legacy_io.cpp	29.67%	429 Missing ⚠️
src/trx.cpp	72.11%	116 Missing ⚠️
include/trx/trx.tpp	71.87%	9 Missing ⚠️
tests/test_trx_gs_consistency.cpp	90.90%	5 Missing and 1 partial ⚠️
tests/test_trx_anytrxfile.cpp	98.27%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #49      +/-   ##
==========================================
- Coverage   87.85%   83.69%   -4.16%     
==========================================
  Files          15       17       +2     
  Lines        7269     8833    +1564     
  Branches      997     1285     +288     
==========================================
+ Hits         6386     7393    +1007     
- Misses        883     1439     +556     
- Partials        0        1       +1     

Flag	Coverage Δ
linux	82.66% <52.45%> (-5.20%)	⬇️
macos	83.53% <53.06%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Suppressed comments (6)

src/trx.cpp:934

• StreamingZipWriter::add_file_stream_begin truncates out.tellp() into a uint32 offset. If the archive grows past 4GiB, offsets wrap and the central directory becomes invalid. Add a size/offset guard (or implement Zip64).

    void add_file_stream_begin(const std::string& name) {
        Entry e;
        e.name = name;
        e.offset = static_cast<uint32_t>(out.tellp());
        e.size = 0; // Will be updated later
        e.crc = 0;  // Will be updated later
        e.flags = 8;

src/trx.cpp:960

• StreamingZipWriter::stream_data accumulates a streamed entry size in a uint32. Once the streamed file exceeds 4GiB, the addition wraps and produces a corrupted ZIP. Add an overflow guard and fail fast with a clear message (or implement Zip64).

    void stream_data(const char* data, size_t size) {
        if (size > 0) {
            out.write(data, size);
            entries.back().size += static_cast<uint32_t>(size);
            entries.back().crc = calculate_crc32(entries.back().crc, reinterpret_cast<const unsigned char*>(data), size);
        }

src/trx.cpp:973

• StreamingZipWriter::finalize truncates the central-directory offset (out.tellp()) into uint32 and the entry count into uint16 (via later casts). If the archive grows too large, this silently produces an invalid ZIP. Add a guard before writing the central directory/EOCD (or implement Zip64).

    void finalize() {
        uint32_t cd_offset = static_cast<uint32_t>(out.tellp());
        for (const auto& e : entries) {

src/trx.cpp:188

• read_le32 shifts promoted int values (from uint8_t) by 16/24 bits. If the high byte is >= 0x80, the expression (ptr[3] << 24) can overflow a signed int, which is undefined behavior. Cast each byte to uint32_t before shifting to keep the operations unsigned.

static uint32_t read_le32(const uint8_t* ptr) {
    return ptr[0] | (ptr[1] << 8) | (ptr[2] << 16) | (ptr[3] << 24);
}

src/trx.cpp:473

• header.json is read with zip_fread() but the return value is ignored (and zip_stat_index() isn’t checked). On a short read or error, this can parse uninitialized/partial data. Also, if sb.size==0, writing to &header_str[0] is undefined. Check zip_stat_index() and zip_fread() results before parsing.

          zip_stat_t sb;
          zip_stat_index(zf.get(), header_idx, 0, &sb);
          std::string header_str(sb.size, ' ');
          zip_fread(hz.get(), &header_str[0], sb.size);
          std::string err;

src/trx.cpp:906

• StreamingZipWriter stores entry offsets/sizes in uint32 (classic ZIP). For large TRX archives, out.tellp() and/or entry size can exceed UINT32_MAX; the current code truncates via static_cast<uint32_t>, producing a corrupted archive with no error. At minimum, detect this and throw a clear error (or implement Zip64 / fall back to libzip for large outputs).

This issue also appears in the following locations of the same file:

• line 928
• line 955
• line 971

    void add_file(const std::string& name, const char* data, size_t size) {
        Entry e;
        e.name = name;
        e.offset = static_cast<uint32_t>(out.tellp());
        e.size = static_cast<uint32_t>(size);
        e.crc = calculate_crc32(0, reinterpret_cast<const unsigned char*>(data), size);
        e.flags = 0;

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 17 changed files in this pull request and generated 7 comments.

Suppressed comments (3)

src/trx.cpp:2

• std::vector<uint8_t> only guarantees 1-byte alignment. Reinterpreting its storage as float*, double*, or Eigen::half* and writing through those pointers is undefined behavior on platforms that require alignment (can crash on ARM). Prefer writing via std::memcpy per-element into the byte buffer, or allocate a properly-aligned typed buffer (e.g., std::vector<float>/std::vector<double>/std::vector<Eigen::half>) and then serialize/copy its bytes.
  include/trx/trx.h:46
• This introduces a json type alias in the global namespace and inside namespace trx. Exporting json globally can easily collide with other translation units/libraries that define their own json alias/type, and it also makes using ::json; compile even when the library intended trx::json. Recommend removing the global using json = ...; and keeping the alias only within namespace trx (or behind a dedicated trx::json name).

using json = json11::Json;

namespace trx {
namespace fs = std::filesystem;
using json = json11::Json;
}

include/trx/legacy_io.h:16

• This header uses std::shared_ptr but does not include <memory>. Relying on indirect includes is fragile and can break builds depending on include order. Add #include <memory> to this header.

    std::shared_ptr<trx::AnyTrxFile> original_trx;