docs: Signal record components and DirtyState event - #419
Conversation
📝 WalkthroughWalkthroughThe documentation adds Signal record component APIs and validation semantics, and documents a new ChangesSignal record components
DirtyState event
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. |
|
| Filename | Overview |
|---|---|
| advanced/signal-protocol.mdx | Documents Signal record components and now clearly describes conversion as normalized re-encoding rather than a lossless round trip. |
| concepts/events.mdx | Documents the new DirtyState event and its relationship to existing clean and resync behavior. |
Reviews (2): Last reviewed commit: "docs: address review feedback on record-..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@advanced/signal-protocol.mdx`:
- Line 1238: Rewrite the dense introductory sentence around the SessionRecord
and SenderKeyRecord components into several concise sentences, separating the
caller use cases, the definition of components, the additive nature of the API,
and the canonical round-trip guarantee. Preserve all existing technical details
and the serialize()/deserialize() behavior.
In `@concepts/events.mdx`:
- Around line 2000-2017: Revise the DirtyState documentation to use concise,
single-idea sentences and active, second-person wording. Split the Emitted
description and the Fields explanations as needed, replacing third-person
references such as “the client” or “a handler” with “you” where appropriate.
Preserve the existing protocol behavior, event ordering, and RawNode guidance
while simplifying the long Note into shorter 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: b80ab62e-f4a1-48ff-a4bc-97e1b97223ca
📒 Files selected for processing (2)
advanced/signal-protocol.mdxconcepts/events.mdx
|
|
||
| ## Record components | ||
|
|
||
| For callers that need to interchange or inspect session and sender-key record state without depending on the generated protobuf schema directly — custom store implementations, migration tooling, offline debugging — `wacore-libsignal` exposes owned, validated projections of `SessionRecord` and `SenderKeyRecord` called **components**. This is purely additive: the protobuf-backed `serialize()`/`deserialize()` path is unchanged, and a canonical record round-trips losslessly through `into_components()` → `from_components()` → `serialize()`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Split this dense sentence for readability.
This single sentence packs the use cases, the "components" definition, the additive guarantee, and the round-trip chain together. Break it into a few sentences (one idea each) to match the docs style.
As per coding guidelines: "Keep sentences concise — one idea per sentence in documentation".
🤖 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 `@advanced/signal-protocol.mdx` at line 1238, Rewrite the dense introductory
sentence around the SessionRecord and SenderKeyRecord components into several
concise sentences, separating the caller use cases, the definition of
components, the additive nature of the API, and the canonical round-trip
guarantee. Preserve all existing technical details and the
serialize()/deserialize() behavior.
Source: Coding guidelines
| **Emitted:** When the server sends an `<ib><dirty type="..." timestamp="...">` marker, telling the client one of its cached protocol domains is stale server-side. | ||
|
|
||
| ```rust | ||
| #[derive(Debug, Clone, Serialize, bon::Builder)] | ||
| #[non_exhaustive] | ||
| pub struct DirtyState { | ||
| pub dirty_type: DirtyType, | ||
| pub timestamp: Option<u64>, | ||
| } | ||
| ``` | ||
|
|
||
| **Fields:** | ||
| - `dirty_type` - The stale domain, mirroring `wacore::iq::dirty::DirtyType`: `AccountSync`, `Groups`, `SyncdAppState`, `NewsletterMetadata`, or `Other(String)` for a wire value the client doesn't otherwise recognize. | ||
| - `timestamp` - `Option<u64>`, `None` if the `<dirty>` stanza omitted the `timestamp` attribute. | ||
|
|
||
| <Note> | ||
| This is a pure observability hook — it does not replace or gate the client's built-in handling. The client always sends the matching `<clean>` IQ (throttled behind offline-sync completion for `Groups`/`NewsletterMetadata`, per `WAWebHandleDirtyBits`) and, for `SyncdAppState`, re-syncs all app-state collections, exactly as it did before this event existed. `DirtyState` fires first, right before that built-in work starts, so a handler can refresh its own domain-specific derived state (e.g. invalidate a local groups cache) without parsing raw `<ib>` stanzas via [`RawNode`](#raw-stanza-events) or racing the client's own resync. | ||
| </Note> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Apply documentation guidelines for conciseness and perspective.
The current description contains long, complex sentences and uses the third person (e.g., "telling the client", "so a handler can"). As per coding guidelines, documentation in .mdx files must use the active voice, the second person ("you"), and keep sentences concise (one idea per sentence).
📝 Proposed refactor for documentation clarity
-**Emitted:** When the server sends an `<ib><dirty type="..." timestamp="...">` marker, telling the client one of its cached protocol domains is stale server-side.
+**Emitted:** When the server sends an `<ib><dirty type="..." timestamp="...">` marker. This tells your client that a cached protocol domain is stale on the server.
```rust
#[derive(Debug, Clone, Serialize, bon::Builder)]
#[non_exhaustive]
pub struct DirtyState {
pub dirty_type: DirtyType,
pub timestamp: Option<u64>,
}Fields:
-- dirty_type - The stale domain, mirroring wacore::iq::dirty::DirtyType: AccountSync, Groups, SyncdAppState, NewsletterMetadata, or Other(String) for a wire value the client doesn't otherwise recognize.
-- timestamp - Option<u64>, None if the <dirty> stanza omitted the timestamp attribute.
+- dirty_type - The stale domain. This mirrors wacore::iq::dirty::DirtyType: AccountSync, Groups, SyncdAppState, NewsletterMetadata, or Other(String) for a wire value your client doesn't otherwise recognize.
+- timestamp - An Option<u64>. This is None if the <dirty> stanza omitted the timestamp attribute.
📝 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.
| **Emitted:** When the server sends an `<ib><dirty type="..." timestamp="...">` marker, telling the client one of its cached protocol domains is stale server-side. | |
| ```rust | |
| #[derive(Debug, Clone, Serialize, bon::Builder)] | |
| #[non_exhaustive] | |
| pub struct DirtyState { | |
| pub dirty_type: DirtyType, | |
| pub timestamp: Option<u64>, | |
| } | |
| ``` | |
| **Fields:** | |
| - `dirty_type` - The stale domain, mirroring `wacore::iq::dirty::DirtyType`: `AccountSync`, `Groups`, `SyncdAppState`, `NewsletterMetadata`, or `Other(String)` for a wire value the client doesn't otherwise recognize. | |
| - `timestamp` - `Option<u64>`, `None` if the `<dirty>` stanza omitted the `timestamp` attribute. | |
| <Note> | |
| This is a pure observability hook — it does not replace or gate the client's built-in handling. The client always sends the matching `<clean>` IQ (throttled behind offline-sync completion for `Groups`/`NewsletterMetadata`, per `WAWebHandleDirtyBits`) and, for `SyncdAppState`, re-syncs all app-state collections, exactly as it did before this event existed. `DirtyState` fires first, right before that built-in work starts, so a handler can refresh its own domain-specific derived state (e.g. invalidate a local groups cache) without parsing raw `<ib>` stanzas via [`RawNode`](#raw-stanza-events) or racing the client's own resync. | |
| </Note> | |
| **Emitted:** When the server sends an `<ib><dirty type="..." timestamp="...">` marker. This tells your client that a cached protocol domain is stale on the server. | |
🤖 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 `@concepts/events.mdx` around lines 2000 - 2017, Revise the DirtyState
documentation to use concise, single-idea sentences and active, second-person
wording. Split the Emitted description and the Fields explanations as needed,
replacing third-person references such as “the client” or “a handler” with “you”
where appropriate. Preserve the existing protocol behavior, event ordering, and
RawNode guidance while simplifying the long Note into shorter statements.
Source: Coding guidelines
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd9a30aed0
ℹ️ 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".
|
|
||
| ## Record components | ||
|
|
||
| For callers that need to interchange or inspect session and sender-key record state without depending on the generated protobuf schema directly — custom store implementations, migration tooling, offline debugging — `wacore-libsignal` exposes owned, validated projections of `SessionRecord` and `SenderKeyRecord` called **components**. This is purely additive: the protobuf-backed `serialize()`/`deserialize()` path is unchanged, and a canonical record round-trips losslessly through `into_components()` → `from_components()` → `serialize()`. |
There was a problem hiding this comment.
Avoid promising a lossless component round trip
The lossless-round-trip claim does not hold for canonical records containing an active counter lease: as documented below, into_components() advances the chain to the lease ceiling and can drop a chain that cannot be advanced, so reconstructing and serializing the record does not preserve its original state or bytes. This matters especially for the custom-store and migration callers introduced here; describe the conversion as a safe normalized projection instead of a lossless round trip.
Useful? React with 👍 / 👎.
| - `timestamp` - `Option<u64>`, `None` if the `<dirty>` stanza omitted the `timestamp` attribute. | ||
|
|
||
| <Note> | ||
| This is a pure observability hook — it does not replace or gate the client's built-in handling. The client always sends the matching `<clean>` IQ (throttled behind offline-sync completion for `Groups`/`NewsletterMetadata`, per `WAWebHandleDirtyBits`) and, for `SyncdAppState`, re-syncs all app-state collections, exactly as it did before this event existed. `DirtyState` fires first, right before that built-in work starts, so a handler can refresh its own domain-specific derived state (e.g. invalidate a local groups cache) without parsing raw `<ib>` stanzas via [`RawNode`](#raw-stanza-events) or racing the client's own resync. |
There was a problem hiding this comment.
Do not promise that handlers run before the built-in resync
This ordering does not prevent a race for the default Bot API: EventDelivery::Concurrent spawns closure callbacks on separate tasks, so dispatch returns and the built-in clean/resync work can start before a DirtyState callback runs or finishes. Only an inline struct-based EventHandler executes before dispatch returns, so the note should not broadly promise that handlers can refresh derived state without racing the client's resync.
Useful? React with 👍 / 👎.
…State event ordering
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Summary
Documents the new public API surface added in oxidezap/whatsapp-rust#1062 ("feat(core): expose signal record components and dirty events"), merged to
maintoday.advanced/signal-protocol.mdx— new "Record components" section coveringSessionRecordComponents/SenderKeyRecordComponentsand the related component types, thefrom_components/into_componentsconversions onSessionRecord/SenderKeyRecord, import validation rules (sender-chain completeness, receiver-chain private-key rejection), export normalization (counter-lease advance before export, bounded archived-state/sender-key-state truncation), the newhas_usable_sender_chaincheck, and the secret-redactingDebugoutput on all*Componentstypes. Placed alongside the existing session/sender-key record content, cross-linking into the existing "Protocol safety limits" and "Flush scheduling" sections rather than duplicating them.concepts/events.mdx— addsDirtyStateto theEventenum listing and a new### DirtyStatesubsection (modeled on the neighboringOfflineSyncCompletedentry) documenting when it fires (<ib><dirty type="..." timestamp="...">from the server), its fields (dirty_type: DirtyType,timestamp: Option<u64>), the fullDirtyTypevariant set, and that it's a pure observability hook — the client's built-in clean/resync IQ handling is unchanged.No breaking changes were introduced by the source PR, so no existing doc content needed correction — this is purely additive.
Test plan
mint broken-links/ local preview (not run in this session — no local Mintlify environment)🤖 Generated with Claude Code
Generated by Claude Code
Summary by cubic
Adds docs for the Signal record components API and the
DirtyStateevent. Docs-only; clarifies validation, normalized import/export rules, and event ordering with no runtime changes.advanced/signal-protocol.mdx: New “Record components” section documentingSessionRecordComponents/SenderKeyRecordComponents,from_components/into_components, import validation (complete sender chain; reject receiver-chain private keys; accept raw or canonical pubkeys and export canonical), export normalization (advance counter leases, drop sender chains too stale to advance, bounded truncation),has_usable_sender_chain, and redactedDebug.concepts/events.mdx: AddsDirtyStatetoEvent, including when it fires (<ib><dirty ...>), that it fires before built-in clean/resync, fields (dirty_type,timestamp), fullDirtyTypevariants (AccountSync,Groups,SyncdAppState,NewsletterMetadata,Other(String)), and that it’s an observability hook only.Written for commit 9cb05e1. Summary will update on new commits.
Summary by CodeRabbit
New Features
DirtyStateevent with its type and optional timestamp.Documentation