Skip to content

ios: deflate the text entries in the shared scan zip - #87

Open
AidenJohnston wants to merge 4 commits into
t3/ios-mvffrom
t3/41-zip-deflate
Open

AidenJohnston wants to merge 4 commits into
t3/ios-mvffrom
t3/41-zip-deflate

Conversation

@AidenJohnston

@AidenJohnston AidenJohnston commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

Part of #68

Base branch: t3/ios-mvf. This PR depends on no other PR.

What changed

The zip that "Share scan" offers stored every file uncompressed. In the 4.1 field test run 1, that zip held about 20 MB of text: the 100 Hz motion CSVs under streams/, plus manifest.json and scene.json. #68 measured gzip taking the CSVs from 19.7 MB to 8.0 MB.

ZipWriter now tries DEFLATE on every entry except JPEGs. It uses Apple's Compression framework: a compression_stream with COMPRESSION_ZLIB writes raw RFC 1951 DEFLATE, which is exactly what ZIP method 8 stores, so the change adds no dependency.

  • The encoder streams through a fixed 64 KiB buffer and stops as soon as its output is no smaller than the input. An entry is therefore written deflated only when that is strictly smaller. Anything else (random bytes, a one-byte file, an empty file) stays stored.
  • A deflated entry gets method 8 and the compressed size in both the local header and the central directory. The CRC-32 and the uncompressed size still describe the raw data. The version needed stays 2.0, which APPNOTE requires for deflate.
  • Names ending in .jpg, .jpeg, .heic or .png skip the attempt, because those formats are already compressed. The packet only writes .jpg photos today.
  • The import is guarded by #if canImport(Compression). Without the framework, every entry is stored as before.

KeyframeStore.zipPacket is the only caller, and its call is unchanged. It builds the zip in the detached save task, and the upload sends scene.json alone, so the upload screen doesn't wait on the extra compression.

Why

The motion streams are text and deflate to well under half their size. The photos make up about 83% of the bundle, but this PR leaves them alone. A lower JPEG quality or a smaller image for the shared copy has to wait until someone measures what reconstruction needs, which is why this PR only covers part of #68. The image size must also keep matching the intrinsics. A HEIC switch would break the photos/<id>.jpg names the packet uses.

The saving that #68's numbers suggest is roughly 12 MB on run 1, from 119.5 MB to about 108 MB. No sample packet is on hand to measure the new writer's output. The phone's encoder runs at a fixed zlib level, close to gzip's default.

Verification

  • iOS CI on 8d2a6a1 (after merging t3/ios-mvf e62be65) passed: HouseScanKit tests with -warnings-as-errors, the app build, the project drift check and the Simulator UI tests. Run 36313813163. The first head, 5150321, also passed (run 36296274160).
  • ZipWriterTests (9 tests) passed on the Mac mini with -warnings-as-errors before the push:
    • textEntryIsDeflated: a synthetic CSV is written with method 8, comes out under half its size, has the raw data's CRC and size, and inflates back to the original with NSData.decompressed(using: .zlib).
    • largeEntriesCrossOutputChunks: an entry whose deflated form spans several 64 KiB output chunks round-trips, and 400 KB of noise is stored.
    • incompressibleEntriesAreStored: JPEG names (including an upper-case .JPG), random bytes, a one-byte entry and an empty entry stay method 0 with their bytes unchanged.
    • A test parser walks the central directory and each local header, and fails if the two disagree on method, CRC or sizes.
    • unzipAcceptsArchive (/usr/bin/unzip -t, which checks each CRC after inflating) and the streamed-vs-in-memory and determinism tests include a deflated CSV entry.
  • KeyframeStore.zipPacket (ZipWriter.write) is the only call site outside the tests. No server file changed. Python's zipfile reads method 8, and the server's bundle check only looks at entry names.

Only a device can verify these: the real zip size and the time added to the save task on a full scan, and opening a new bundle with the Files app or the share targets.

Files touched

  • ios/HouseScanKit/Sources/HouseScanKit/Scene/ZipWriter.swift
  • ios/HouseScanKit/Tests/HouseScanKitTests/ZipWriterTests.swift

Risk

Compressing costs CPU time in the background save. Besides the loaded entry, memory holds only the compressed output, which is never allowed to reach the entry's size, and a 64 KiB buffer. Photos skip the encoder, so they add neither cost. Deflated bytes come from Apple's encoder, so an archive is byte-identical for the same inputs only on the same OS build.

Checklist

  • The scope is focused; related changes belong together
  • I explained what changed and why
  • N/A: before/after images. Nothing on screen changes.
  • N/A: video. There is no motion or interaction change.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • ZIP bundles now compress eligible files when compression produces a smaller result. JPEG, HEIC, and PNG files, along with files that do not benefit from compression, remain uncompressed.
    • This can reduce bundle sizes while preserving the original contents of files. If compression is unavailable or cannot be completed, files are stored without compression.

ZipWriter stored every entry (method 0). The scan bundle's motion CSVs,
manifest and scene are text, about a sixth of a 4.1 bundle, and deflate
well. Each entry except JPEGs now goes through Apple's Compression
framework (COMPRESSION_ZLIB writes raw RFC 1951 DEFLATE, which is ZIP
method 8) and is written deflated only when that is strictly smaller;
anything else stays stored. The CRC and uncompressed size still describe
the raw data. Without the Compression framework every entry is stored.

Tests parse the central directory and local headers: a CSV entry is
method 8, under half its size and inflates back to the original; JPEGs,
random bytes, one-byte and empty entries stay stored. The unzip -t test
now covers a deflated entry.

Part of #68.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 42fd66c9-f495-4ffa-8283-ffbb172ef714

📥 Commits

Reviewing files that changed from the base of the PR and between d9abb8e and aa5999e.

📒 Files selected for processing (2)
  • ios/HouseScanKit/Sources/HouseScanKit/Scene/ZipWriter.swift
  • ios/HouseScanKit/Tests/HouseScanKitTests/ZipWriterTests.swift

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

Walkthrough

ZipWriter now attempts raw DEFLATE for entries that are not already compressed by filename suffix. It uses compressed output only when smaller than the source and records the selected method and sizes in ZIP headers. Tests cover compressed entries, stored entries, and ZIP metadata.

Changes

ZIP entry compression

Layer / File(s) Summary
Compression selection and ZIP metadata
ios/HouseScanKit/Sources/HouseScanKit/Scene/ZipWriter.swift
The writer adds raw DEFLATE support and skips selected pre-compressed filename suffixes. It uses compressed data only when smaller than the original, and records the selected method and sizes in ZIP headers.
Compression and storage tests
ios/HouseScanKit/Tests/HouseScanKitTests/ZipWriterTests.swift
Tests parse ZIP entries and check local and central directory metadata, compression round trips, and stored entries for pre-compressed filenames and non-shrinking inputs.

Priority: ➖ Normal

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

Change: Feature

Suggested reviewers: samgu-nrx

Merge Risk: ⚪ Minimal · up to aa599

The change reduces compressible ZIP entries while preserving stored entries when compression is unsuitable. No concrete merge-blocking issue remains; full-scan size, save time, and Files compatibility still warrant device validation.

Security Architecture Review

Security architecture risk: 🔵 Low · up to aa599

The change is confined to scan archive encoding. Completed-file publication and the separate JSON upload path limit exposure. Compatibility with external archive readers remains unverified; no introduced security vulnerability was established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The established exposure is an individual scan's locally generated shared archive. The inspected flow does not propagate ZIP compression into the placement-service request or confer additional authority.

Trust Boundaries and Controls

  • observed — The changed helper encodes caller-provided bytes; it does not extract archives or interpret entry content. Existing name, duplicate and ZIP-size checks execute before entry emission.

Resilience and Maintainability Implications

  • observed — Input pointers remain inside withUnsafeBytes, output is copied into owned Data, and deferred cleanup releases the buffer and initialized compression stream. The current bundle caller waits for prior work and suppresses stale publication.

Hardening Proposals

  • proposed — Validate a representative mixed-method archive with actual downstream ingestion and sharing readers before rollout. Their implementations were unavailable, so compatibility remains an open contract check rather than an observed failure.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 56.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding DEFLATE compression for text entries in the shared scan ZIP.
Description check ✅ Passed The description covers the required sections, explains the behavior and rationale, documents verification and risks, and marks non-applicable UI media items with reasons.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@github-actions github-actions Bot added the size:M 30-99 effective changed lines. label Sep 27, 2026

@AidenJohnston AidenJohnston left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Adversarial review of 5150321 (U18, Part of #68), against base t3/ios-mvf.

Must fix

None found.

Should fix

None found. The items under Notes are optional tightening.

Notes

  1. No compiler has seen this Swift yet (gh pr checks 87: iOS build skipping, because the PR is a draft; PR Size passed; CodeRabbit skipped the draft). My mental compile passed:

    • compression_encode_buffer(UnsafeMutablePointer<UInt8>, Int, UnsafePointer<UInt8>, Int, UnsafeMutableRawPointer?, compression_algorithm) -> Int matches the call at ZipWriter.swift:213.
    • The typed withUnsafeMutableBytes/withUnsafeBytes closures and bindMemory(to:) return types line up. There is no exclusivity conflict (out is mutated, data is only read), and neither #if branch has unreachable code.
    • The new members are private, and the public signatures didn't change. NSData.decompressed(using: .zlib) is available because the package floor is macOS 26.

    Before merging, get a green swift test --package-path ios/HouseScanKit -Xswiftc -warnings-as-errors run. Marking the PR ready triggers it, or workflow_dispatch on the branch does.

  2. The fallback and the test disagree off Apple platforms. Scenario: HouseScanKit is ever built where Compression is missing (for example the Linux corelibs). The writer falls back to stored as designed (ZipWriter.swift:202). But textEntryIsDeflated asserts method == 8 unconditionally (ZipWriterTests.swift:108), and NSData.decompressed(using:) doesn't exist in swift-corelibs-foundation, so that test file wouldn't compile there. Today Package.swift declares iOS and macOS only and CI runs on macos-26, so this can't happen yet. If Linux ever matters, wrap the test in the same #if canImport(Compression).

  3. "Byte-identical" now holds per OS build only (ZipWriter.swift:67-68). The deflate output comes from Apple's encoder, and its bytes can change between iOS or macOS releases. deterministicWithoutDate and streamedArchiveMatchesInMemory still hold, because both sides run on the same machine. Nothing hashes scan.zip itself (the manifest hashes the files inside it), so nothing breaks. The doc comment could say "on the same OS".

  4. Correctness against #68 and the 4.1 evidence. The fix covers the lossless part of #68, deflating the text entries:

    • Run 1 had 19.7 MB of motion CSVs, and #68 measured gzip taking them to 8.0 MB. Photos stay stored, so the roughly 12 MB estimate is plausible.
    • "Part of #68" is correct. JPEG quality and pixel size are untouched, as the brief requires, and wait on U16's measurement.
    • .heic and .png are also skipped. The packet writes neither, so nothing changes.
  5. Header fields checked against APPNOTE.

    • Method 8 goes in both the local header and the central directory.
    • The compressed size is the deflated body, and the uncompressed size and CRC-32 describe the raw data.
    • Version needed stays 20, and bit 3 (data descriptor) is not set, which is right because the sizes are known before the header is written.
    • written advances by body.count, so the offsets and the end record stay consistent.
    • The capacity is one byte under the input, and the 0-on-overflow return gives a strictly smaller result or stored. The count > 1 guard avoids Data(count: -1) on empty entries.
  6. Flows around it.

    • ZipWriter.write has one caller: KeyframeStore.zipPacket (KeyframeStore.swift:217), called from ScanEngine.writePacket (ScanEngine+Packet.swift:287). It runs inside Task.detached in saveBundle (ScanEngine.swift:1497-1519), and the upload never waits on it.
    • The only visible effect is that "Share scan" appears a little later, while about 20 MB is compressed. At zlib speeds on an A15 that should be well under a second. Only a device can confirm it.
    • Every reader of the bundle I found handles method 8: /usr/bin/unzip in ios/Tools/check-app-scene.sh, Python zipfile in the packet/ validator and in experiments/evals on beta, and server/api.py _open_bundle on t3/server, which sizes with file_size, the uncompressed size.
    • Nothing on any branch parses the zip by hand. No JS reader exists and no stored-only parser exists.
  7. Test parser. The offsets in parse match APPNOTE: CDH method@10, crc@16, sizes@20/24, name length@28, offset@42; LFH method@8, crc@14, sizes@18/22; EOCD count@10, offset@16.

    • The 4096-byte LCG noise can't deflate below 4095 bytes (a stored block alone costs 5 extra bytes), so the "stays stored" check is sound.
    • The 2000-row CSV (about 110 KB) spans several deflate blocks, and the fixture now puts it through unzip -t.
  8. Scope. Only ZipWriter.swift and ZipWriterTests.swift changed. There are no project.yml or project.pbxproj edits, no signing changes and no new dependency. Compression is a system framework.

    • The branch is behind t3/ios-mvf by 5 commits, but none of them touch these files or their callers, and a test merge is clean.
    • No other open PR touches ZipWriter or KeyframeStore.
  9. Public-repo check. The diff, the PR body and the commit message contain no names, meter numbers, addresses, emails, private/ content or scan data. The fixtures are synthetic.

  10. PR body. It has Part of #68, the base, which checks I made and which need CI or a device, the files touched, the dependency on no other PR, and the attribution line. No sample packet was on hand, so there are no measured before/after sizes. Running one scan on a phone after merging would confirm the saving.

Verdict: ready-with-notes

🤖 Generated with Claude Code

@AidenJohnston

Copy link
Copy Markdown
Collaborator Author

Verdict: ready-with-notes

  • The review listed no must-fix items. The branch is still the single commit 5150321, so nothing changed after the review.
  • The iOS workflow run dispatched on head 5150321 passed: HouseScanKit tests with warnings as errors, the app build, and the full-flow UI test in the Simulator (https://github.com/SamGu-NRX/house-scanning-master/actions/runs/36296274160).
  • I re-read the diff and it is sound. Method 8 and the compressed size go into both the local header and the central directory, while the CRC and the uncompressed size still describe the raw data. An entry is only kept deflated when that is strictly smaller. JPEGs are never compressed. ZipWriter's public API is unchanged, so its only caller, KeyframeStore.zipPacket, needs no edit.
  • These checks can only be done on a phone: the real zip size and the extra save time on a full scan, and opening a new bundle in Files or a share target.

🤖 Generated with Claude Code

@AidenJohnston AidenJohnston left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Adversarial review of #87 (U18, t3/41-zip-deflate at 5150321, base t3/ios-mvf, part of #68)

I read the full new ZipWriter.swift and ZipWriterTests.swift, the only caller (KeyframeStore.zipPacket, called from ScanEngine+Packet.writePacket inside the detached saveBundle task), the server's bundle reader (server/api.py _open_bundle on beta/t3/server), #68 and the public run 1/2/3 analyses.

Must fix

None.

Should fix

  1. The PR body's Verification section is out of date. It says "the package has not been compiled or tested yet". The manually started iOS workflow run 36296274160 on head 5150321 passed: the Test HouseScanKit step compiled ZipWriterTests.swift under -warnings-as-errors, and textEntryIsDeflated, incompressibleEntriesAreStored, unzipAcceptsArchive (/usr/bin/unzip -t, which inflates and checks each CRC), streamedArchiveMatchesInMemory and deterministicWithoutDate all passed, with 383 tests in total. The build, the committed-project drift check and the Simulator UI flow also passed. Move those items from "only CI or a device can verify" to "verified", and cite the run. Otherwise a merger can't tell the Swift has been built at all.

Notes

  1. ios/HouseScanKit/Sources/HouseScanKit/Scene/ZipWriter.swift:76-77: the write doc comment still says memory holds "one file and the central directory". deflated (:207) now allocates an output buffer of data.count - 1 bytes next to the loaded file, so a deflatable entry briefly needs about twice its size. Run 1's largest CSV was about 10 MB, and a LiDAR lidar/mesh.ply could be larger. The PR body's Risk section says this, but the doc comment doesn't. A one-line doc fix is enough.
  2. Correctness checks that held up.
    • With COMPRESSION_ZLIB, the encoder writes raw RFC 1951 data with no header (method 8). The unzip -t test proves this with an independent inflater, not only with Apple's round trip.
    • The CRC and the uncompressed size are computed from the raw data (:123-124). The compressed size and the method appear in both headers (:134/:138 and :151/:155). Version needed 2.0 is correct for deflate.
    • With a capacity of count - 1, the encoder returns 0 on overflow, so the noise, one-byte and empty entries fall back to stored. The noise test would have caught a truncated result.
    • The typed withUnsafe*Bytes closures avoid the deprecated overloads. The new helpers are private, and the public signatures are unchanged.
    • KeyframeStore.zipPacket at ios/HouseScan/Runtime/KeyframeStore.swift:230 is the only call site, and nothing else in ios/, web/ or experiments/ parses the zip at the byte level. The packet manifest hashes the files on disk, not zip bodies, so it is unaffected.
  3. Server compatibility. _open_bundle in server/api.py (on beta) uses zipfile, which reads method 8. It already catches zlib.error, and its size cap sums info.file_size, which is still the uncompressed size, so the limit means the same thing. No server change is needed.
  4. Scope. The PR only reaches the lossless part of #68 (about 12 MB of the 119.5 MB on run 1). Photos are still about 83% of the bundle, so this does not make run 1's 119.5 MB share or its slow Drive upload noticeably better. The PR says so and correctly leaves the JPEG quality or size for U16. The added .jpeg/.heic/.png skips go slightly beyond the brief (.jpg only), but they're harmless and aren't a product decision. No project.yml/project.pbxproj edits and no unrelated changes.
  5. Branch state. The branch is 41 commits behind t3/ios-mvf. None of those commits touch ZipWriter.swift, its tests, KeyframeStore.swift or ScanEngine+Packet.swift, and git merge-tree shows a clean merge.
  6. CI. The iOS check on the PR shows "skipping", because draft PRs don't trigger the iOS job. See Should fix 1 for the manually started run on the head. PR size check passed. CodeRabbit skipped the draft.
  7. Device-only. The real zip size and the extra save time before "Share scan" becomes available are both unmeasured. shareableScan is set only after writePacket returns, so any deflate time delays the button. With about 20 MB of text at the default zlib level, that is probably well under a second, but it should be timed on the next device test.
  8. Public-repo rules. The diff, the PR body and the commit message contain no names, meter numbers, addresses, private/ content or scan data. The test fixtures are synthetic. The PR body has Part of #68, the base, dependencies, the files touched and the attribution line.

Verdict: ready-with-notes

🤖 Generated with Claude Code

@codemetrics-ai

Copy link
Copy Markdown

Note

Review skipped

Codemetrics is installed on this repository, but it hasn't been added to your team yet.

To enable AI code reviews for this repo, go to your dashboard and add house-scanning-master to your team's repositories.

@SamGu-NRX

Copy link
Copy Markdown
Owner

@greptileai

@greptile-apps

greptile-apps Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Adds compression to the scan archive writer.

The PR appears safe to merge; no actionable issue was established.

Fix All in Claude CodeFindings

  1. P2 Large compression buffer ▶
Fix with agent prompt
### Issue 1
ios/HouseScanKit/Sources/HouseScanKit/Scene/ZipWriter.swift:206-207
The writer loads each entry into memory, then allocates another buffer almost as large before checking whether compression helps. A long LiDAR scan can include binary depth frames and a mesh, so saving its ZIP can use substantially more memory and may fail under memory pressure. A bounded or streaming compression path would preserve the writer’s one-file-at-a-time memory goal.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Summary

The PR makes the shared scan ZIP deflate entries when doing so reduces their size, while retaining stored entries for images and data that do not shrink.

  • Updates local and central-directory methods and sizes for compressed entries.
  • Adds round-trip, multi-chunk, stored-entry, and standard-unzip test coverage.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Load ZIP entry] --> B{Precompressed image?}
  B -- Yes --> S[Store with method 0]
  B -- No --> C[Attempt DEFLATE]
  C --> D{Output smaller than input?}
  D -- No --> S
  D -- Yes --> E[Write method 8 and compressed size]
  S --> F[Write ZIP headers and body]
  E --> F
Loading

Reviews (1) · Last reviewed commit: "Merge t3/ios-mvf into t3/41-zip-deflate"

Comment on lines +206 to +207
let capacity = data.count - 1
var out = Data(count: capacity)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Large compression buffer

The writer loads each entry into memory, then allocates another buffer almost as large before checking whether compression helps. A long LiDAR scan can include binary depth frames and a mesh, so saving its ZIP can use substantially more memory and may fail under memory pressure. A bounded or streaming compression path would preserve the writer’s one-file-at-a-time memory goal.

Prompt To Fix With AI
This is a comment left during a code review.
Path: ios/HouseScanKit/Sources/HouseScanKit/Scene/ZipWriter.swift
Line: 206-207

Comment:
**Large compression buffer**

The writer loads each entry into memory, then allocates another buffer almost as large before checking whether compression helps. A long LiDAR scan can include binary depth frames and a mesh, so saving its ZIP can use substantially more memory and may fail under memory pressure. A bounded or streaming compression path would preserve the writer’s one-file-at-a-time memory goal.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex

AidenJohnston and others added 2 commits September 27, 2026 05:48
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ized one

Greptile noted the deflate attempt allocated a second buffer almost as large as each entry. The encoder now streams through 64 KiB, so the only added memory is the compressed output, and stops once that output stops being a saving. The write() and byte-identical doc comments say what they now mean, and the deflate tests are behind the same canImport(Compression) as the code.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@AidenJohnston

Copy link
Copy Markdown
Collaborator Author

Updated for review. Head is now 8d2a6a1.

  • Base: merged t3/ios-mvf at e62be65 in 5f63c65. There were no conflicts.
  • Greptile P2, large compression buffer (ZipWriter.swift:206-207): fixed in 8d2a6a1. deflated now runs a compression_stream through a fixed 64 KiB buffer. It used to allocate an output buffer almost the size of the entry. The only extra memory is now the compressed output, and the encoder stops as soon as that output reaches the input's size. The new test largeEntriesCrossOutputChunks covers output that spans several chunks, plus 400 KB of noise that must stay stored.
  • Review note, the write doc still said memory holds "one file and the central directory": fixed in 8d2a6a1. It now also mentions the compressed copy.
  • Review note, "byte-identical" holds only on the same OS build: the archive doc now says so (8d2a6a1).
  • Review note, the deflate tests assume the Compression framework: textEntryIsDeflated and the new test are now under #if canImport(Compression), the same guard as the code (8d2a6a1).
  • Should fix, the Verification section was out of date: I updated the PR body with the runs below.

CI: the iOS build passed on 8d2a6a1, including HouseScanKit with -warnings-as-errors, the app build, the drift check and the UI tests: https://github.com/SamGu-NRX/house-scanning-master/actions/runs/36313813163. Before the push, ZipWriterTests (9 tests) also passed on the Mac mini with -warnings-as-errors.

The real zip size and the save time still need a device scan.

🤖 Generated with Claude Code

@SamGu-NRX

Copy link
Copy Markdown
Owner

@codex review

Please review current head 8d2a6a13abf1f6abb4d328a0b5c74620b0ee295f.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 27, 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-09-27T11:15:28.090081Z 8d2a6a1 Manual request
ℹ️ 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.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 8d2a6a13ab

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

ZipWriter.write's doc comment conflicted: the base now writes to a temporary file and moves it over the old archive, and this branch says memory also holds the compressed copy. The comment keeps both.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@SamGu-NRX

Copy link
Copy Markdown
Owner

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@SamGu-NRX

Copy link
Copy Markdown
Owner

@greptileai review

Please review the current head aa5999e040b37a93ddfc91065c6954cabef59c8e against this PR's actual base. Catch-up review requested by Sam after enabling BaseScanning in Greptile.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 effective changed lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants