docs: cover PR #1058's history-sync visitor API and MsgSecretEntry breaking change - #416
Conversation
…eaking change Documents the new process_history_sync_bytes_with_record_visitor / _with_record_sink streaming API, the MemoryReport history-sync fields, and the MsgSecretEntry/put_msg_secret signature change (Arc<str> fields, fixed-size MessageSecret) from whatsapp-rust PR #1058. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FzZuNPQtjj2r89hH45ccDn
📝 WalkthroughWalkthroughThe documentation updates define fixed-size message-secret types, add streaming history-sync visitor and sink APIs, describe allocation optimizations, and extend ChangesAPI documentation updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
| Filename | Overview |
|---|---|
| api/store.mdx | Updates put_msg_secret signature to &[u8; 32], adds MsgSecretEntry struct definition with Arc fields and MessageSecret type, adds get_msg_secret return-type asymmetry note, and expands the migration callout to cover both struct constructors and trait implementors. |
| api/wacore.mdx | Adds HistoryMsgSecretRecordVisitor trait and the two new process_history_sync_bytes_with_record_* free functions below the existing filtered entry point, with accurate where-clause signatures and clear prose distinguishing closure vs. trait variants; adds an allocation-optimizations bullet citing measured numbers. |
| api/client.mdx | Adds history_sync_tasks (CollectionStats), history_sync_tasks_peak (u64), and history_sync_payload_bytes_peak (u64) to the MemoryReport field table; updates the Display description to mention the new In-flight history sync section. |
| concepts/architecture.mdx | Adds item 11 to the RAM optimization layers list covering the streaming visitor/sink path, with matching bench numbers and cross-links to the API reference; consistent in style with items 8–10. |
Reviews (2): Last reviewed commit: "docs: address review feedback on PR #105..." | Re-trigger Greptile
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 467e23f510
ℹ️ 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".
| } | ||
| ``` | ||
|
|
||
| `chat`, `sender`, and `msg_id` are `Arc<str>` rather than `String` so buffered batch inserts can clone entries cheaply, and `secret` is the fixed-size `MessageSecret` array rather than `Vec<u8>` — an invalid-length secret is unrepresentable instead of needing a runtime check. This is a breaking change for custom backends constructing `MsgSecretEntry` directly: build the JID/ID fields with `Arc::from(...)`/`.into()` and pass a `[u8; 32]` for `secret`. The SQLite table itself is unchanged: |
There was a problem hiding this comment.
Split the migration guidance into concise sentences
This paragraph combines both field-type changes, two separate rationales, compatibility impact, and migration instructions in long compound sentences. This conflicts with the applicable AGENTS.md requirements to “Keep sentences concise” and use “one idea per sentence,” making the breaking-change guidance harder for custom-backend authors to scan. Split each type change, rationale, and required migration step into its own sentence.
Useful? React with 👍 / 👎.
| 8. **Secret-presence pre-scan** — before running buffa decode on each `HistorySyncMsg`, a shallow varint walk checks whether the message carries `message_secret` at any level (`WebMessageInfo.message_secret` or `Message.message_context_info.message_secret`). Messages without a secret (the majority in production blobs) are discarded immediately — no struct decode, no allocation. The scan mirrors protobuf merge semantics so repeated field occurrences and malformed bytes are handled identically to a full decode | ||
| 9. **Shared conversation id** — `HistoryMsgSecretRecord.chat_id` is `Arc<str>`, allocated once per conversation and reference-counted into every record within that conversation (10k clones → 500 on the bench fixture). `msg_id` uses `CompactString` (inline for typical 20–22 char WA IDs) and `secret` uses `SecretBytes` (inline for secrets ≤32 bytes) | ||
| 10. **Filter-before-materialize retention hook** — `process_history_sync_bytes_filtered` runs a caller-supplied retention predicate against a borrowed `HistoryMsgSecretRecordRef` (a zero-copy view into the not-yet-built record) before allocating the owned, heap-backed `HistoryMsgSecretRecord`. A record the predicate rejects is discarded without ever being materialized. `process_history_sync_bytes` — and `process_history_sync` itself — keep their existing accept-all behavior by wrapping the filtered entry point with an always-`true` predicate, so callers that don't own a retention policy are unaffected. Bench (synthetic 500-conversation blob, upstream PR's rejection-heavy fixture): allocation churn 21.61 MiB → 14.00 MiB, allocation count ~84k → ~26k. The reduction scales with how much of the record set the predicate rejects — the default accept-all behavior sees none of it. See [`wacore` — history_sync types](/api/wacore#history_sync-types) for the exported signatures | ||
| 11. **Streaming record-visitor path** — `process_history_sync_bytes_with_record_visitor` (closure-based) and `process_history_sync_bytes_with_record_sink` (trait-based, via `HistoryMsgSecretRecordVisitor`) go further than the filter hook above: the caller builds its own storage row directly from the borrowed `HistoryMsgSecretRecordRef`, so the owned `HistoryMsgSecretRecord` is never allocated at all, even for accepted records. The visitor trait's optional `reserve`/`retained_item_size` hooks let a capacity-aware caller (e.g. a batched SQL insert buffer) size its own collection up front. Bench (allocator-instrumented synthetic history extraction): 20.20 MiB → 14.43 MiB allocated (-28.6%); CodSpeed history stream-drain memory: 243.8 KB → 115.2 KB (2.1× less). See [`wacore` — history_sync types](/api/wacore#history_sync-types) |
There was a problem hiding this comment.
Point the visitor link at the generated anchor
The destination heading is history_sync types, so the generated slug preserves the underscore and replaces the space, producing #history_sync-types; the repository follows the same behavior for anchors such as #passkey-linking-shortcake_passkey. Consequently, clicking this newly added link does not jump to the documented signatures. Change this link and the new same-page link in api/wacore.mdx to use #history_sync-types.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/client.mdx`:
- Line 1973: Rewrite the `CollectionStats` and `MemoryReport` explanation as
short, separate sentences, with one technical idea per sentence. Remove the
chained “and” construction and dangling clause while preserving the details
about byte summation, `Display`, and the in-flight history sync section.
In `@api/store.mdx`:
- Line 406: Rewrite the documentation paragraph around MsgSecretEntry using
short, single-idea sentences. Explain the Arc<str> fields, fixed-size
MessageSecret secret, and unchanged SQLite table separately, then address the
breaking change directly in second person by instructing users to construct
JID/ID fields with Arc::from(...) or .into() and provide a [u8; 32] secret.
In `@api/wacore.mdx`:
- Line 338: Update the documentation sentence describing the
wacore::history_sync streaming visitor path to address the reader directly using
“you” instead of referring to “callers,” while preserving the existing meaning
and technical symbols.
- Line 366: Rewrite the documentation around
process_history_sync_bytes_with_record_visitor and
process_history_sync_bytes_with_record_sink into concise sentences, with one
idea per sentence. Address the reader directly using “you” instead of “the
caller,” while preserving the distinctions between closures, visitors, retained
byte accounting, capacity hooks, and when to use
process_history_sync_bytes_filtered.
- Line 820: Update the streaming visitor/sink documentation sentence to use
concise, single-idea sentences and address the reader as “you” instead of “the
caller.” Preserve the existing API names and benchmark results while splitting
the explanation into clearer sentences.
In `@concepts/architecture.mdx`:
- Line 528: Rewrite the “Streaming record-visitor path” description into concise
sentences, with one idea per sentence and second-person phrasing such as “you
build” instead of “the caller builds.” Preserve all technical details, symbols,
metrics, and links while splitting the current long sentence into readable
statements.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: be838bf9-da99-4dab-aca1-5b0be563e50b
📒 Files selected for processing (4)
api/client.mdxapi/store.mdxapi/wacore.mdxconcepts/architecture.mdx
| } | ||
| ``` | ||
|
|
||
| `chat`, `sender`, and `msg_id` are `Arc<str>` rather than `String` so buffered batch inserts can clone entries cheaply, and `secret` is the fixed-size `MessageSecret` array rather than `Vec<u8>` — an invalid-length secret is unrepresentable instead of needing a runtime check. This is a breaking change for custom backends constructing `MsgSecretEntry` directly: build the JID/ID fields with `Arc::from(...)`/`.into()` and pass a `[u8; 32]` for `secret`. The SQLite table itself is unchanged: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Split sentences and use the second person ("you") for instructions.
The explanation combines multiple concepts (string types, cloning, array size, runtime checks) into a long run-on sentence and does not use the second-person perspective for the breaking change instruction. As per coding guidelines, documentation must keep sentences concise (one idea per sentence) and use the second person ("you") and active voice.
♻️ Proposed rewrite for clarity and compliance
-`chat`, `sender`, and `msg_id` are `Arc<str>` rather than `String` so buffered batch inserts can clone entries cheaply, and `secret` is the fixed-size `MessageSecret` array rather than `Vec<u8>` — an invalid-length secret is unrepresentable instead of needing a runtime check. This is a breaking change for custom backends constructing `MsgSecretEntry` directly: build the JID/ID fields with `Arc::from(...)`/`.into()` and pass a `[u8; 32]` for `secret`. The SQLite table itself is unchanged:
+The `chat`, `sender`, and `msg_id` fields are `Arc<str>` rather than `String`. This allows buffered batch inserts to clone entries cheaply. The `secret` field uses the fixed-size `MessageSecret` array rather than `Vec<u8>`. This makes an invalid-length secret unrepresentable. You no longer need to perform a runtime length check. This is a breaking change if you construct `MsgSecretEntry` directly in a custom backend. You must build the JID/ID fields with `Arc::from(...)` or `.into()`. You must also pass a `[u8; 32]` for `secret`. The SQLite table itself is unchanged:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `chat`, `sender`, and `msg_id` are `Arc<str>` rather than `String` so buffered batch inserts can clone entries cheaply, and `secret` is the fixed-size `MessageSecret` array rather than `Vec<u8>` — an invalid-length secret is unrepresentable instead of needing a runtime check. This is a breaking change for custom backends constructing `MsgSecretEntry` directly: build the JID/ID fields with `Arc::from(...)`/`.into()` and pass a `[u8; 32]` for `secret`. The SQLite table itself is unchanged: | |
| The `chat`, `sender`, and `msg_id` fields are `Arc<str>` rather than `String`. This allows buffered batch inserts to clone entries cheaply. The `secret` field uses the fixed-size `MessageSecret` array rather than `Vec<u8>`. This makes an invalid-length secret unrepresentable. You no longer need to perform a runtime length check. This is a breaking change if you construct `MsgSecretEntry` directly in a custom backend. You must build the JID/ID fields with `Arc::from(...)` or `.into()`. You must also pass a `[u8; 32]` for `secret`. The SQLite table itself is unchanged: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/store.mdx` at line 406, Rewrite the documentation paragraph around
MsgSecretEntry using short, single-idea sentences. Explain the Arc<str> fields,
fixed-size MessageSecret secret, and unchanged SQLite table separately, then
address the breaking change directly in second person by instructing users to
construct JID/ID fields with Arc::from(...) or .into() and provide a [u8; 32]
secret.
Source: Coding guidelines
- Note that get_msg_secret/_with_ts still return Vec<u8> (SQLite BLOB has no length constraint), unlike the fixed-size write path (Greptile). - Clarify that overriding the defaulted put_msg_secret also requires the new [u8; 32] signature, not just MsgSecretEntry construction (Greptile). - Rewrite the new prose into concise, second-person sentences per this repo's AGENTS.md style guide (CodeRabbit). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FzZuNPQtjj2r89hH45ccDn
Summary
Documents oxidezap/whatsapp-rust#1058 ("perf(core): reduce history sync and decrypt allocation churn"). I audited the full diff and split its changes into what's already covered by this site vs. what's genuinely new internal-only detail:
MsgSecretEntry/MsgSecretStore::put_msg_secret— breaking change.chat/sender/msg_idare nowArc<str>(wereString) andsecretis now the fixed-sizeMessageSecret([u8; 32]) instead ofVec<u8>/a raw slice. Updated the struct definition, theput_msg_secretsignature, and added a note inapi/store.mdxtelling custom-backend authors what to change.process_history_sync_bytes_with_record_visitor(closure) andprocess_history_sync_bytes_with_record_sink(via the newHistoryMsgSecretRecordVisitortrait) let a caller build its own storage row straight from the borrowedHistoryMsgSecretRecordRef, skipping the ownedHistoryMsgSecretRecordallocation entirely — a step beyond the existingprocess_history_sync_bytes_filteredpredicate this site already documents in depth. Added the new functions/trait toapi/wacore.mdx'shistory_synctypes section, a matching bullet in its "Allocation optimizations" list, and item 11 inconcepts/architecture.mdx's "RAM optimization layers" list (same style as items 8–10 covering the sibling history-sync optimizations), all citing the PR's measured numbers (20.20 MiB → 14.43 MiB allocated, -28.6%; CodSpeed stream-drain 243.8 KB → 115.2 KB, 2.1× less).Client::memory_report()gains three fields.history_sync_tasks: CollectionStats,history_sync_tasks_peak: u64, andhistory_sync_payload_bytes_peak: u64, plus a new--- In-flight history sync ---section inMemoryReport'sDisplayoutput. Added to theMemoryReportfields table inapi/client.mdx.Not documented (per the PR's own "additive" framing, these are deep internal
wacore::libsignalperf primitives —OwnedCiphertextMessage/message_decrypt_owned,aes_256_cbc_decrypt_in_place,hmac_sha256_two_part, and two newSignalCryptoProvidermethods — none of which this site documents at that level of granularity today; there's no existing section enumerating individual libsignal session-cipher or crypto-provider functions to extend). Also leftchangelog/untouched per instructions, since that's human-authored.Test plan
#history_sync-types,#ram-optimization-layers,#memory_report) resolve correctly🤖 Generated with Claude Code
Generated by Claude Code
Summary by cubic
Docs now cover the history-sync streaming visitor API, the
MsgSecretEntry/put_msg_secrettype changes, and the new history-sync fields inClient::memory_report()fromwhatsapp-rustPR #1058. Clarifies how to process records without allocating owned structs and what custom backends must update, including read-path return types.New Features
process_history_sync_bytes_with_record_visitor,process_history_sync_bytes_with_record_sink, andHistoryMsgSecretRecordVisitorto build rows directly fromHistoryMsgSecretRecordRefand skip the owned allocation (−28.6% allocated bytes measured).history_sync_tasks: CollectionStats,history_sync_tasks_peak: u64, andhistory_sync_payload_bytes_peak: u64, plus the new “In-flight history sync” section inMemoryReport’sDisplay.Migration
MsgSecretEntrynow usesArc<str>forchat/sender/msg_idandMessageSecret([u8; 32]) forsecret.put_msg_secrettakes&[u8; 32](this applies even if you override the defaulted method). UseArc::from(...)/.into()for IDs and pass a[u8; 32]secret. Reads are unchanged:get_msg_secret/get_msg_secret_with_tsstill returnVec<u8>since the SQLiteBLOBhas no length constraint. Storage schema is unchanged.Written for commit e8dbfd7. Summary will update on new commits.
Summary by CodeRabbit