feat(bindings_node): return catch-up counts as number (not BigInt) + completed flag

[tylerhawkes]

Stacked on #3881 (base branch tyler/lifecycle-catchup-hardening) — review/merge that first.

Returns the node catchUpToLive counts as number (napi u32) instead of BigInt, and adds the completed flag (now that core CatchUpSummary carries it, from #3881).

BigInt is a permanent tax on every JS/TS consumer — it can't be mixed with number, breaks JSON.stringify, and is awkward in TS — while a single catch-up run's counts comfortably fit a number. Doing this before the release avoids a breaking type change for every agent codebase later.

Verification

• Full napi release build succeeds (just node check)

🤖 Generated with Claude Code

Note

Return catch-up counts as number and add completed flag to Node CatchUpSummary

Changes the messages and conversations fields in the Node-exposed CatchUpSummary from BigInt to u32 (JavaScript number), and adds a new completed: bool field forwarded from the Rust source struct.

Risk: any Node consumers reading messages or conversations as BigInt will break.

📊 Macroscope summarized 989e08f. 1 file reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

[claude]

Claude finished @tylerhawkes's task —— View job

---

Code Review - 2026-07-21 22:00 UTC

Overall Assessment: ✅ LGTM - The changes are clean, well-justified, and properly implemented.

Review Summary

This PR converts catch-up counts from BigInt (napi i64) to number (napi u32) and adds the completed flag. The implementation is correct and the rationale is sound.

Detailed Findings

Code Quality: ✅ Excellent

• Clean conversion using as u32 casts in the From trait implementation (bindings/node/src/client/catch_up.rs:20-21)
• Proper field addition for completed flag (line 22)
• Clear documentation explaining the behavior (lines 12-13)

Potential Issues: ⚠️ One consideration

The as u32 cast is lossy if core counts ever exceed u32::MAX (4,294,967,295). However:

• ✅ The PR description correctly justifies this: "a single catch-up run's counts comfortably fit a number"
• ✅ In practice, processing 4B+ messages/conversations in one catch-up run is unrealistic
• ✅ The trade-off (avoiding BigInt ergonomics) is reasonable for the domain

If you wanted to be defensive, you could add saturating casts or a debug assertion, but this is likely overkill:

pub messages: u32.saturating_from(summary.messages),  // or .min(u32::MAX as u64) as u32

Performance: ✅ Neutral-to-positive

• No performance impact on the Rust side
• JS consumers benefit from avoiding BigInt overhead

Security: ✅ No concerns

• No security implications in this change

Test Coverage: ⚠️ Implicitly covered

• Core CatchUpSummary has extensive tests in xmtp_mls/src/subscriptions/catch_up.rs (lines 607-734)
• Node binding layer has no new behavior to test beyond the type conversion
• The change is verified by just node check (builds successfully)
• Consider: If the Node.js test suite exercises catch_up_to_live, the types are already validated there

Recommendations

1. Optional: Add a comment at the cast sites explaining the u64→u32 overflow is acceptable for this domain
2. Merge when ready - the base PR (feat(mls,bindings): bound resume_streams + return partial catch-up summary on deadline #3881) should merge first as noted

API Design Note

Good call doing this before release. Forcing JS consumers to use BigInt for counts would be painful and unnecessary here.

---

Tasks completed:

• Read changed file (bindings/node/src/client/catch_up.rs)
• Analyze code quality and best practices
• Check for potential bugs or issues
• Review performance and security considerations
• Assess test coverage
• Post review feedback

---

[macroscopeapp]

🟡 Medium client/catch_up.rs:20

summary.messages as u32 and summary.conversations as u32 silently truncate when the u64 count exceeds u32::MAX, returning a wrapped (possibly zero) value to JavaScript instead of the true number of newly persisted items. Use a checked conversion that errors on overflow, or keep a 64-bit representation such as BigInt so the full range is preserved.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @bindings/node/src/client/catch_up.rs around line 20:

`summary.messages as u32` and `summary.conversations as u32` silently truncate when the `u64` count exceeds `u32::MAX`, returning a wrapped (possibly zero) value to JavaScript instead of the true number of newly persisted items. Use a checked conversion that errors on overflow, or keep a 64-bit representation such as `BigInt` so the full range is preserved.

[macroscopeapp]

Approvability

Verdict: Needs human review

1 blocking correctness issue found. Unresolved review comment identifies silent truncation when casting u64 to u32, which could return incorrect counts to JavaScript callers. This correctness concern warrants human review.

^{You can customize Macroscope's approvability policy. Learn more.}