fix(core): preserve Uint8Array in document fields for CBOR round-trip

[jchris]

Summary

• Fixed sanitizeDocumentFields() to return Uint8Array as-is instead of iterating numeric keys and producing a plain object. DAG-CBOR natively supports byte strings — the sanitizer just needs to not destroy the type.
• Added 6 regression tests covering: empty, small, 256-byte, nested, array-of-objects, and multi-field Uint8Array documents.

Fixes the known bug from fireproof-legacy.txt where Uint8Array stored in docs got converted to {0: 1, 1: 2, ...} plain objects.

Test plan

• All 6 new Uint8Array round-trip tests pass
• All 1625 core tests pass (0 regressions)
• pnpm build passes

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Binary data (Uint8Array) is now preserved unchanged during storage and retrieval, preventing accidental conversion to plain objects and protecting data integrity.

• Tests

  • Added automated tests that verify round-trip preservation of binary values at top-level and nested fields, including byte-for-byte equality and preservation of sibling non-binary data.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 14942202-695b-429f-b3f9-22a12ad9c259

📥 Commits

Reviewing files that changed from the base of the PR and between a2270c2 and b5e83d2.

📒 Files selected for processing (2)

• core/base/crdt-helpers.ts
• core/tests/fireproof/uint8array-roundtrip.test.ts

✅ Files skipped from review due to trivial changes (1)

• core/tests/fireproof/uint8array-roundtrip.test.ts

---

Walkthrough

Adds a Uint8Array type check to sanitizeDocumentFields so binary Uint8Array values are returned unchanged during document sanitization, and adds tests verifying round-trip preservation for top-level and nested Uint8Array fields through the Fireproof database.

Changes

Cohort / File(s)	Summary
Sanitization change core/base/crdt-helpers.ts	Added an isUint8Array import and a branch in sanitizeDocumentFields to detect and return Uint8Array instances unchanged, preventing them from being reconstructed as plain objects.
Tests: Uint8Array round-trip core/tests/fireproof/uint8array-roundtrip.test.ts	New Vitest suite with two tests that create a Database, store and retrieve documents to assert top-level and nested Uint8Array fields remain Uint8Array instances and match original bytes; ensures cleanup after each test.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main fix: preserving Uint8Array in document fields for CBOR serialization. It accurately summarizes the primary change in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jchris/fix-uint8array-roundtrip

---

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 and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

core/tests/fireproof/uint8array-roundtrip.test.ts (1)

27-28: Minor: redundant new Uint8Array() wrapping.

After asserting toBeInstanceOf(Uint8Array) on line 27, the wrap on line 28 is unnecessary—you can compare directly:

-    expect(new Uint8Array(doc.payload)).toEqual(data);
+    expect(doc.payload).toEqual(data);

This pattern repeats in other tests (lines 38, 47, 59-60, 71-72). Not blocking, but simplifies assertions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/tests/fireproof/uint8array-roundtrip.test.ts` around lines 27 - 28, The
tests redundantly wrap doc.payload in new Uint8Array after already asserting it
is a Uint8Array; update assertions to compare doc.payload directly to data
(e.g., replace expect(new Uint8Array(doc.payload)).toEqual(data) with
expect(doc.payload).toEqual(data)), and apply the same change for the other
occurrences noted (the assertions around lines referencing doc.payload and data
in this test file).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@core/tests/fireproof/uint8array-roundtrip.test.ts`:
- Around line 27-28: The tests redundantly wrap doc.payload in new Uint8Array
after already asserting it is a Uint8Array; update assertions to compare
doc.payload directly to data (e.g., replace expect(new
Uint8Array(doc.payload)).toEqual(data) with expect(doc.payload).toEqual(data)),
and apply the same change for the other occurrences noted (the assertions around
lines referencing doc.payload and data in this test file).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f3c4e25e-1acf-43d9-9e96-83b6e5fabed5

📥 Commits

Reviewing files that changed from the base of the PR and between 2c4dad9 and a2270c2.

📒 Files selected for processing (2)

• core/base/crdt-helpers.ts
• core/tests/fireproof/uint8array-roundtrip.test.ts

[mabels]

there is. cement ./src/coerce-binary.ts
21:export function isUint8Array(value: unknown): value is Uint8Array {

[mabels]

why wee need that all test -- one should be enough --- and one who show that this was wrong?