fix(collab): report the bytes sweepBlobs actually reclaimed

[BIMvoice]

sweepBlobs returned a byte count that did not match what it actually reclaimed. Found on a never-raised branch; merges clean.

RED — surgical, restoring only if (ok) freed += 1; void freed; return decision.reclaimBytes;:

expected 4, received 7    (test/blob-gc.test.ts:122)

packages/collab 226 pass / 2 skipped; packages/collab-server re-run after rebuild, 182 pass. check-api-surface ✅ (42 packages, 4197 exports), check-unused-locals ✅.

No neighbour regression is possible here: there is no in-repo production caller of sweepBlobs — only tests. The changeset's "a caller using the return value…" is phrased conditionally and is accurate rather than an overclaim.

One thing for your call before merge: dropByteLengths is a new required field on the exported SweepDecision interface. Any external code constructing a SweepDecision literal breaks at typecheck — and this ships as patch. Either the field should be optional or the bump should reflect it.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

@louistrue, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 54 minutes

Limit details: You’ve used all 2 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2b349898-b5dd-4a6c-985b-4526cf43fc09

📥 Commits

Reviewing files that changed from the base of the PR and between fe38b33 and 4e6beb4.

📒 Files selected for processing (3)

• .changeset/blob-gc-overstated-reclaim.md
• packages/collab/src/geometry/gc.ts
• packages/collab/test/blob-gc.test.ts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

Viewer benchmark

⚠ 1 metric(s) exceeded the regression threshold (advisory only, not blocking).

01_Snowdon_Towers_Sample_Structural(1).ifc

Baseline recorded 2026-07-01T20:31:05.538Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.

Metric	Current	Baseline	Delta	Threshold	Status
firstBatchWaitMs	1686ms	2905ms	-42.0%	+50%	✅
firstVisibleGeometryMs	2251ms	3652ms	-38.4%	+50%	✅
streamCompleteMs	2671ms	3598ms	-25.8%	+50%	✅
spatialReadyMs	1549ms	1032ms	+50.1%	+50%	❌
metadataCompleteMs	1856ms	3063ms	-39.4%	+50%	✅
totalWallClockMs	2800ms	3700ms	-24.3%	+50%	✅

AC20-FZK-Haus.ifc

Baseline recorded 2026-07-01T20:30:59.972Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.

Metric	Current	Baseline	Delta	Threshold	Status
firstBatchWaitMs	349ms	1075ms	-67.5%	+50%	✅
firstVisibleGeometryMs	1168ms	1572ms	-25.7%	+50%	✅
streamCompleteMs	831ms	1980ms	-58.0%	+50%	✅
spatialReadyMs	974ms	915ms	+6.4%	+50%	✅
metadataCompleteMs	1059ms	1392ms	-23.9%	+50%	✅
totalWallClockMs	1400ms	3300ms	-57.6%	+50%	✅

Refresh the baseline from a CI run: dispatch the Benchmark workflow with record_baseline, download the benchmark-baseline artifact, and commit baseline.json (see tests/benchmark/README.md).

[louistrue]

Reviewed by reading and running, not with the CLI. Nothing to change. Recording what I checked, since "no findings" is only worth something if you know what was looked for.

The fix is covered, verified by mutation

Baseline first: 228 passed / 2 skipped. Then restored the pre-fix behaviour exactly as the body describes:

- if (ok) freed += decision.dropByteLengths[hash] ?? 0;
+ if (ok) freed += 1; void freed; return decision.reclaimBytes;

× does not report bytes as reclaimed when the underlying delete() fails
1 failed | 227 passed | 2 skipped

So the assertion is real and it fails for the right reason.

The ?? 0 is genuinely defensive, not papering over a live gap

That was my main suspicion — a ?? 0 on a lookup is often hiding a case where the two collections can disagree, and then a missing entry silently under-reports as a zero-byte blob, which is indistinguishable from a real zero.

They cannot disagree here. Both drop.push(hash) sites (gc.ts:156-158 and :163-165) set dropByteLengths[hash] and add to reclaim in the same three lines, in both branches. There is no path that appends to drop without recording a length.

And no NaN path

BlobMeta.byteLength is number, not optional (blob-store.ts:26), and the metaFromGet fallback always populates it from bytes.byteLength. So reclaim += meta.byteLength cannot produce NaN from anything in-repo. The only way in is an external options.metaProvider returning a value that lies about its own type at runtime, which is a TS-boundary concern rather than a defect here.

On the interface question you raised

You flagged dropByteLengths as a new required field on the exported SweepDecision and asked for a call. I think you are right to raise it and right that it is a real break — but the blast radius argument in the body holds: there is no in-repo production caller of sweepBlobs, only tests, so nothing internal constructs a SweepDecision literal.

The one thing I would add: making it optional to avoid the break would be worse, because then dropByteLengths[hash] ?? 0 silently returns 0 for every hash when an older caller omits the field, and sweepBlobs reports zero bytes reclaimed while deleting everything. That is a quieter failure than a typecheck error, and it is the same defect this PR fixes. Required is the right call.