refactor(events): model ServerAck.class as Option, document payload stability

[jlucaso1]

What

Follow-up to #989. Two small, related cleanups on the event API surface while the crate is still pre-1.0 and breaking changes are cheap.

1. ServerAck.class: plain String becomes an Option

ServerAck.class used an empty-string sentinel (unwrap_or_default()) when the server ack omits the class attribute. Every other payload in events.rs models a maybe-absent field as an Option, so this was the lone exception. Now a missing class is None instead of "", and the parser drops the unwrap_or_default().

2. Document the pre-1.0 event stability policy

Added a stability note to the Event enum doc (and a pointer in AGENTS.md):

• the enum is #[non_exhaustive], so match arms keep a _ catch-all;
• payload structs stay constructible (not #[non_exhaustive]) so the crate can build them across the wacore / whatsapp-rust boundary without constructors;
• consumers should read the fields they need or keep a .. rest when destructuring, since a payload may gain fields in a 0.x minor;
• a maybe-absent field is always an Option, never a sentinel;
• sealing payloads behind #[non_exhaustive] + constructors is deferred to the 1.0 API freeze.

Why not seal the payloads now

#[non_exhaustive] on a wacore payload struct blocks struct-literal construction from whatsapp-rust (a separate crate), which would force a constructor for every event payload. Pre-1.0 that cost buys little, since fields can still be added with a minor bump. The policy records the decision instead.

Breaking

ServerAck.class is now an Option. Consumers match Some("message") instead of "message". Only landed in #989, so nothing released depends on the old shape.

Verification

• cargo fmt --check and cargo clippy -p whatsapp-rust -p wacore --tests clean.
• test_ack_dispatches_server_ack_event updated for the Option shape and green.
• ack_miss_path_does_not_heap_allocate green — the change stays inside the interest-gated block, so the perf(client): borrow the ack id instead of allocating a String per stanza #827 allocation-free miss path is untouched.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

📝 Walkthrough

Walkthrough

Alright, let's be direct here. ServerAck.class moves from a required String to Option<String>, so it now correctly represents "no class attribute" instead of faking it with an empty string. The producer, tests, and docs are updated to match this cleaner contract.

Changes

ServerAck class optionality

Layer / File(s)	Summary
Type and stability docs wacore/src/types/events.rs	ServerAck.class changes from String to Option<String>; adds pre-1.0 stability documentation for the Event enum on handling optional payload fields.
Ack handler and tests src/client/node_io.rs, src/client/tests.rs	handle_ack_response now sets class only when the attribute exists (no more unwrap_or_default()); tests updated to check as_deref() == Some(...) and is_none() instead of empty-string comparisons.
Contributor convention docs AGENTS.md	Adds a convention requiring Option<T> for maybe-absent event payload fields and clarifies pre-1.0 payload backward-compatibility expectations.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• oxidezap/whatsapp-rust#978: Also touches handle_ack_response in src/client/node_io.rs, overlapping at the same function though for a different concern (waiter cancellation-safety).

Suggested labels: `api-design`, `breaking-change`

Look, this is a small but important fix — data models need to reflect reality, not paper over missing data with empty strings. That's the kind of precision we need. Ship it.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main changes: ServerAck.class becomes Option and event payload stability is documented.
Description check	✅ Passed	The description directly matches the code changes and explains the Option transition plus the new stability guidance.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/whatsapp-rust-pr-review-wjppqi

---

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.}

[greptile-apps]

Greptile Summary

This PR makes two focused cleanups to the event API surface: it models ServerAck.class as Option<String> (replacing an empty-string sentinel), and it adds an explicit pre-1.0 stability policy doc to the Event enum and AGENTS.md. Both the parser and tests are updated consistently.

• ServerAck.class is now Option<String>: the parser drops unwrap_or_default(), and all test assertions use .as_deref() / .is_none() correctly.
• A # Stability (pre-1.0) section is added to the Event enum doc, capturing the #[non_exhaustive] enum / constructible-struct tradeoff and codifying the Option<T> convention for absent fields.

Confidence Score: 5/5

Safe to merge — the type change is small, complete, and all call sites in the codebase are updated.

Every place that previously accessed ServerAck.class as a String has been updated: the parser in node_io.rs correctly produces Option, and both test assertions use .as_deref() / .is_none(). A grep over all Rust files confirms no stale .class == '...' comparisons remain. The doc and AGENTS.md addition are accurate and add value without introducing risk.

No files require special attention.

Important Files Changed

Filename	Overview
wacore/src/types/events.rs	Changes ServerAck.class from String to Option and adds the pre-1.0 stability policy doc to the Event enum; both the type and doc comment are accurate and internally consistent.
src/client/node_io.rs	Drops unwrap_or_default() from the class attribute read, cleanly propagating Option from node.get_attr("class").
src/client/tests.rs	Updates both test assertions for ServerAck.class — present-class uses .as_deref() == Some("message") and absent-class uses .is_none(); both are idiomatic and correct.
AGENTS.md	Adds the event-payload convention bullet (Option, no sentinels, pre-1.0 constructibility) cross-linked to the Event doc; accurately reflects the code changes.

_{Reviews (1): Last reviewed commit: "refactor(events): model ServerAck.class ..." | Re-trigger Greptile}

[github-actions]

📦 Binary size report

Metric	main	PR	Δ
bin size (stripped)	10.81 MiB	10.81 MiB	0
bin .text	8.81 MiB	8.81 MiB	0
bin allocated (text+data+bss)	10.81 MiB	10.81 MiB	0
llvm-lines wacore	504,323	504,323	0
llvm-lines wacore copies	17,278	17,278	0
llvm-lines whatsapp-rust lib	757,829	757,825	-4 (-0.00%) 🔽
llvm-lines whatsapp-rust lib copies	24,586	24,586	0
deps crates (Cargo.lock)	466	466	0

.text per crate

Crate	main	PR	Δ
.text whatsapp_rust	1.63 MiB	1.63 MiB	-15 B (-0.00%) 🔽
.text wacore	531.47 KiB	531.47 KiB	0
.text wacore_binary	157.70 KiB	157.70 KiB	0
.text wacore_libsignal	178.73 KiB	178.73 KiB	0
.text wacore_appstate	156.45 KiB	156.45 KiB	0
.text wacore_noise	26.05 KiB	26.05 KiB	0
.text waproto	1.60 MiB	1.60 MiB	0
.text whatsapp_rust_sqlite_storage	512.98 KiB	512.98 KiB	0
.text whatsapp_rust_tokio_transport	43.61 KiB	43.61 KiB	0
.text whatsapp_rust_ureq_http_client	10.47 KiB	10.47 KiB	0
.text std	1.00 MiB	1.00 MiB	0
.text other deps	2.95 MiB	2.95 MiB	0

Baseline: 376ac1b31 (latest main run) · Head: a1abb6ba1 · Graphs

[cubic-dev-ai]

No issues found across 4 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

<sub>Auto-approved: Refactored ServerAck.class to Option for consistency; added stability docs.

Re-trigger cubic</sub>