Skip to content

refactor!: API ergonomics — &[Jid], #[non_exhaustive], PartialEq/Eq - #457

Merged
jlucaso1 merged 2 commits into
mainfrom
refactor/api-ergonomics
Mar 28, 2026
Merged

refactor!: API ergonomics — &[Jid], #[non_exhaustive], PartialEq/Eq#457
jlucaso1 merged 2 commits into
mainfrom
refactor/api-ergonomics

Conversation

@jlucaso1

@jlucaso1 jlucaso1 commented Mar 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

BREAKING: Improves public API ergonomics across features.

&[Jid] instead of Vec<Jid> (HIGH)

  • Status::send_text, send_image, send_video, send_raw, revoke now take &[Jid] instead of Vec<Jid>
  • Callers no longer forced to allocate a Vec — can pass &[jid1, jid2] or &existing_vec
  • Internal send_status_message updated accordingly

#[non_exhaustive] on public enums (MEDIUM)

Adding variants to these enums is no longer a semver-breaking change:

  • StatusPrivacySetting, PresenceStatus, PinDuration, RevokeType
  • GroupType, ChatStateType
  • NewsletterVerification, NewsletterState, NewsletterRole

PartialEq/Eq on public structs (MEDIUM)

Users can now compare these types directly:

  • GroupMetadata, GroupParticipant, CreateGroupResult
  • SendResult
  • NewsletterMetadata, NewsletterVerification, NewsletterState, NewsletterRole
  • CreateCommunityOptions, CreateCommunityResult, CommunitySubgroup
  • LinkSubgroupsResult, UnlinkSubgroupsResult

Not included (deferred)

  • Serialize/Deserialize: Requires enabling the serde feature on wacore-binary for Jid — left for a separate PR
  • Builder pattern consistency: Low severity, deferred
  • Error type context: Low severity, deferred

Test plan

  • cargo clippy --all --tests — zero warnings
  • cargo fmt --all — clean
  • No e2e tests use the status API directly

Summary by CodeRabbit

  • New Features / Improvements

    • Status-sending APIs now accept slice references for recipients, simplifying call sites and reducing ownership requirements.
    • Documentation example updated to demonstrate the new slice-based usage.
  • Chores

    • Several public types now support value-based equality comparisons.
    • Multiple enums marked forward-compatible to allow safe future extension.

BREAKING: Status::send_* methods now take `&[Jid]` instead of `Vec<Jid>`,
removing the forced allocation on callers. Existing code using `vec![...]`
can switch to `&[...]` or pass `&vec`.

Also:
- Add #[non_exhaustive] to 9 public enums so new variants aren't breaking
- Add PartialEq/Eq to 10+ public structs (GroupMetadata, SendResult,
  NewsletterMetadata, community types, etc.)
- Add PartialEq/Eq to NewsletterVerification, NewsletterState, NewsletterRole
@coderabbitai

coderabbitai Bot commented Mar 28, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 19b5a62d-5f2a-499d-ab6b-717100337e64

📥 Commits

Reviewing files that changed from the base of the PR and between fce2c16 and 2e0723f.

📒 Files selected for processing (1)
  • src/features/newsletter.rs

📝 Walkthrough

Walkthrough

Added #[non_exhaustive] to several public enums and expanded PartialEq, Eq derives on multiple public types. Several recipient parameters changed from Vec<Jid> to borrowed &[Jid] in status/send APIs; example doc updated accordingly.

Changes

Cohort / File(s) Summary
Enum non-exhaustive markers
src/features/chatstate.rs, src/features/community.rs, src/features/newsletter.rs, src/features/presence.rs, src/features/status.rs, src/send.rs
Added #[non_exhaustive] to public enums (ChatStateType, GroupType, NewsletterMessageType, NewsletterVerification, NewsletterState, NewsletterRole, PresenceStatus, StatusPrivacySetting, PinDuration, RevokeType) to prevent exhaustive matching.
Derived equality on public types
src/features/community.rs, src/features/groups.rs, src/features/newsletter.rs, src/send.rs
Added PartialEq and Eq to multiple structs/enums (CreateCommunityOptions, CreateCommunityResult, CommunitySubgroup, LinkSubgroupsResult, UnlinkSubgroupsResult, GroupMetadata, GroupParticipant, CreateGroupResult, NewsletterMetadata, NewsletterVerification, NewsletterState, NewsletterRole, SendResult) enabling value equality checks.
Recipient API signatures
src/features/status.rs, src/send.rs
Changed recipient parameters from owned Vec<Jid> to borrowed slices &[Jid] for: Status::send_text, send_image, send_video, send_raw, revoke, and Client::send_status_message; adjusted call sites/doc example to pass &recipients.
Minor call-site iteration fix
src/send.rs
Adjusted recipient resolution to clone jid where needed when iterating borrowed recipients.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hop through code with joyful cheer,
Non-exhaustive doors now open near,
Slices of Jids I lightly bear,
Equality blossoms everywhere —
A tiny rabbit, changelog clear!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically summarizes the three main changes: API parameter ergonomics (&[Jid]), forward-compatibility marking (#[non_exhaustive]), and trait derivations (PartialEq/Eq).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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 refactor/api-ergonomics

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Mar 28, 2026

Copy link
Copy Markdown

🐰 Bencher Report

Branchrefactor/api-ergonomics
Testbedubuntu-latest

🚨 1 Alert

BenchmarkMeasure
Units
ViewBenchmark Result
(Result Δ%)
Upper Boundary
(Limit %)
libsignal_benchmark::session_optimization_group::bench_decrypt_with_previous_session previous_session:setup_with_archived_sessions()Instructions
instructions x 1e3
📈 plot
🚷 threshold
🚨 alert (🔔)
47.13 x 1e3
(+9.30%)Baseline: 43.12 x 1e3
45.28 x 1e3
(104.09%)

Click to view all benchmark results
BenchmarkInstructionsBenchmark Result
instructions
(Result Δ%)
Upper Boundary
instructions
(Limit %)
binary_benchmark::attr_parser_group::bench_attr_parser attr_lookup:setup_attr_marshaled()📈 view plot
🚷 view threshold
6,197.00
(-5.41%)Baseline: 6,551.71
6,879.30
(90.08%)
binary_benchmark::child_iteration_group::bench_get_children_by_tag📈 view plot
🚷 view threshold
524,304.00
(-27.23%)Baseline: 720,532.44
756,559.06
(69.30%)
binary_benchmark::jid_optimization_group::bench_jid_to_owned_access jid_access:setup_jid_heavy_marshaled()📈 view plot
🚷 view threshold
20,868.00
(-5.71%)Baseline: 22,131.34
23,237.91
(89.80%)
binary_benchmark::marshal_group::bench_marshal_allocating📈 view plot
🚷 view threshold
98,202.00
(-16.92%)Baseline: 118,194.92
124,104.66
(79.13%)
binary_benchmark::marshal_group::bench_marshal_auto_allocating📈 view plot
🚷 view threshold
98,230.00
(-10.19%)Baseline: 109,374.64
114,843.38
(85.53%)
binary_benchmark::marshal_group::bench_marshal_auto_huge_bytes_allocating📈 view plot
🚷 view threshold
532,948.00
(-0.11%)Baseline: 533,528.60
560,205.03
(95.13%)
binary_benchmark::marshal_group::bench_marshal_auto_long_string📈 view plot
🚷 view threshold
15,870.00
(-4.75%)Baseline: 16,660.89
17,493.93
(90.72%)
binary_benchmark::marshal_group::bench_marshal_auto_many_children_allocating📈 view plot
🚷 view threshold
14,715,201.00
(-8.05%)Baseline: 16,002,795.90
16,802,935.70
(87.58%)
binary_benchmark::marshal_group::bench_marshal_exact_allocating📈 view plot
🚷 view threshold
118,358.00
(-20.72%)Baseline: 149,284.51
156,748.73
(75.51%)
binary_benchmark::marshal_group::bench_marshal_exact_huge_bytes_allocating📈 view plot
🚷 view threshold
534,378.00
(-0.11%)Baseline: 534,949.39
561,696.86
(95.14%)
binary_benchmark::marshal_group::bench_marshal_exact_long_string📈 view plot
🚷 view threshold
17,919.00
(-4.24%)Baseline: 18,712.21
19,647.82
(91.20%)
binary_benchmark::marshal_group::bench_marshal_exact_many_children_allocating📈 view plot
🚷 view threshold
28,066,347.00
(-21.75%)Baseline: 35,865,959.41
37,659,257.38
(74.53%)
binary_benchmark::marshal_group::bench_marshal_huge_bytes_allocating📈 view plot
🚷 view threshold
533,387.00
(-0.11%)Baseline: 533,967.60
560,665.97
(95.13%)
binary_benchmark::marshal_group::bench_marshal_long_string📈 view plot
🚷 view threshold
15,843.00
(-7.42%)Baseline: 17,113.00
17,968.64
(88.17%)
binary_benchmark::marshal_group::bench_marshal_many_children_allocating📈 view plot
🚷 view threshold
14,716,627.00
(-8.04%)Baseline: 16,003,914.74
16,804,110.48
(87.58%)
binary_benchmark::marshal_group::bench_marshal_reusing_buffer📈 view plot
🚷 view threshold
107,945.00
(-13.44%)Baseline: 124,706.79
130,942.13
(82.44%)
binary_benchmark::marshal_group::bench_marshal_reusing_buffer_vec_writer📈 view plot
🚷 view threshold
98,302.00
(-10.18%)Baseline: 109,446.64
114,918.98
(85.54%)
binary_benchmark::roundtrip_group::bench_roundtrip large:setup_large_marshaled()📈 view plot
🚷 view threshold
90,974.00
(-5.39%)Baseline: 96,152.96
100,960.60
(90.11%)
binary_benchmark::roundtrip_group::bench_roundtrip small:setup_small_marshaled()📈 view plot
🚷 view threshold
7,378.00
(-3.47%)Baseline: 7,643.06
8,025.21
(91.94%)
binary_benchmark::roundtrip_group::bench_roundtrip_auto large:setup_large_marshaled()📈 view plot
🚷 view threshold
91,005.00
(-1.81%)Baseline: 92,678.11
97,312.01
(93.52%)
binary_benchmark::roundtrip_group::bench_roundtrip_auto small:setup_small_marshaled()📈 view plot
🚷 view threshold
7,401.00
(+0.36%)Baseline: 7,374.12
7,742.83
(95.59%)
binary_benchmark::roundtrip_group::bench_roundtrip_exact large:setup_large_marshaled()📈 view plot
🚷 view threshold
106,790.00
(-1.54%)Baseline: 108,463.11
113,886.26
(93.77%)
binary_benchmark::roundtrip_group::bench_roundtrip_exact small:setup_small_marshaled()📈 view plot
🚷 view threshold
8,913.00
(+0.30%)Baseline: 8,886.12
9,330.43
(95.53%)
binary_benchmark::unmarshal_group::bench_unmarshal large:setup_large_marshaled()📈 view plot
🚷 view threshold
41,989.00
(-8.03%)Baseline: 45,656.19
47,939.00
(87.59%)
binary_benchmark::unmarshal_group::bench_unmarshal small:setup_small_marshaled()📈 view plot
🚷 view threshold
2,717.00
(-4.04%)Baseline: 2,831.49
2,973.07
(91.39%)
binary_benchmark::unpack_group::bench_unpack_compressed📈 view plot
🚷 view threshold
556,092.00
(+2.34%)Baseline: 543,377.13
570,545.99
(97.47%)
binary_benchmark::unpack_group::bench_unpack_uncompressed📈 view plot
🚷 view threshold
771.00
(-0.30%)Baseline: 773.34
812.01
(94.95%)
libsignal_benchmark::conversation_group::bench_full_dm_conversation full:setup_conversation_data()📈 view plot
🚷 view threshold
27,604,463.00
(-0.35%)Baseline: 27,701,417.90
29,086,488.79
(94.90%)
libsignal_benchmark::dm_group::bench_dm_decrypt_first_message decrypt_prekey:setup_dm_with_first_message()📈 view plot
🚷 view threshold
5,544,828.00
(-0.06%)Baseline: 5,547,916.49
5,825,312.32
(95.19%)
libsignal_benchmark::dm_group::bench_dm_encrypt_first_message first_msg:setup_dm_session()📈 view plot
🚷 view threshold
175,061.00
(-1.36%)Baseline: 177,478.43
186,352.35
(93.94%)
libsignal_benchmark::dm_group::bench_dm_encrypt_subsequent_message subsequent:setup_established_dm_session()📈 view plot
🚷 view threshold
175,710.00
(-1.43%)Baseline: 178,257.66
187,170.55
(93.88%)
libsignal_benchmark::dm_group::bench_dm_session_establishment setup:setup_dm_users()📈 view plot
🚷 view threshold
17,354,982.00
(+0.43%)Baseline: 17,280,268.24
18,144,281.65
(95.65%)
libsignal_benchmark::group_messaging_group::bench_group_create_distribution_message create:setup_group_sender()📈 view plot
🚷 view threshold
298,417.00
(+0.57%)Baseline: 296,734.29
311,571.00
(95.78%)
libsignal_benchmark::group_messaging_group::bench_group_decrypt_message decrypt:setup_group_with_encrypted_message()📈 view plot
🚷 view threshold
12,627,094.00
(+0.25%)Baseline: 12,595,395.99
13,225,165.79
(95.48%)
libsignal_benchmark::group_messaging_group::bench_group_encrypt_message encrypt:setup_group_with_distribution()📈 view plot
🚷 view threshold
719,597.00
(+0.39%)Baseline: 716,805.43
752,645.70
(95.61%)
libsignal_benchmark::session_optimization_group::bench_decrypt_with_previous_session previous_session:setup_with_archived_sessions()📈 view plot
🚷 view threshold
🚨 view alert (🔔)
47,129.00
(+9.30%)Baseline: 43,120.30
45,276.31
(104.09%)

libsignal_benchmark::session_optimization_group::bench_message_key_eviction eviction:setup_message_key_eviction()📈 view plot
🚷 view threshold
15,561,842.00
(+0.00%)Baseline: 15,561,762.40
16,339,850.52
(95.24%)
libsignal_benchmark::session_optimization_group::bench_out_of_order_decryption out_of_order:setup_out_of_order_messages()📈 view plot
🚷 view threshold
5,378,778.00
(-1.87%)Baseline: 5,481,455.24
5,755,528.00
(93.45%)
libsignal_benchmark::session_optimization_group::bench_promote_matching_session promote:setup_promote_matching_session()📈 view plot
🚷 view threshold
312,188.00
(-62.05%)Baseline: 822,532.97
863,659.61
(36.15%)
libsignal_benchmark::signature_group::bench_key_generation keygen📈 view plot
🚷 view threshold
2,830,547.00
(+0.18%)Baseline: 2,825,349.30
2,966,616.77
(95.41%)
libsignal_benchmark::signature_group::bench_signature_creation sign:setup_keypair_with_message()📈 view plot
🚷 view threshold
3,452,844.00
(-0.54%)Baseline: 3,471,626.10
3,645,207.40
(94.72%)
libsignal_benchmark::signature_group::bench_signature_verification verify:setup_keypair_with_message()📈 view plot
🚷 view threshold
125,458,568.00
(+0.09%)Baseline: 125,345,258.53
131,612,521.46
(95.32%)
reporting_token_benchmark::content_extraction_group::bench_content_extraction extended:setup_extended_message()📈 view plot
🚷 view threshold
12,002.00
(+1.56%)Baseline: 11,817.80
12,408.69
(96.72%)
reporting_token_benchmark::content_extraction_group::bench_content_extraction simple:setup_simple_message()📈 view plot
🚷 view threshold
3,930.00
(+2.53%)Baseline: 3,832.88
4,024.53
(97.65%)
reporting_token_benchmark::full_generation_group::bench_full_token_generation extended:setup_full_gen_extended()📈 view plot
🚷 view threshold
87,073.00
(-0.81%)Baseline: 87,786.85
92,176.19
(94.46%)
reporting_token_benchmark::full_generation_group::bench_full_token_generation simple:setup_full_gen_simple()📈 view plot
🚷 view threshold
79,022.00
(-1.00%)Baseline: 79,824.23
83,815.44
(94.28%)
reporting_token_benchmark::key_derivation_group::bench_key_derivation📈 view plot
🚷 view threshold
50,375.00
(-1.09%)Baseline: 50,932.53
53,479.15
(94.20%)
reporting_token_benchmark::message_encoding_group::bench_message_encoding extended:setup_extended_message()📈 view plot
🚷 view threshold
5,956.00
(+3.25%)Baseline: 5,768.51
6,056.94
(98.33%)
reporting_token_benchmark::message_encoding_group::bench_message_encoding simple:setup_simple_message()📈 view plot
🚷 view threshold
2,227.00
(+4.63%)Baseline: 2,128.44
2,234.87
(99.65%)
reporting_token_benchmark::token_calculation_group::bench_token_calculation📈 view plot
🚷 view threshold
21,920.00
(+0.01%)Baseline: 21,917.79
23,013.68
(95.25%)
🐰 View full continuous benchmarking report in Bencher

@jlucaso1

Copy link
Copy Markdown
Collaborator Author

@claude review

@claude

claude Bot commented Mar 28, 2026

Copy link
Copy Markdown

Claude finished @jlucaso1's task in 45s —— View job


Code Review: API Ergonomics Improvements

  • Read changed files
  • Provide review feedback

Verdict: LGTM — clean, well-scoped breaking change. A few minor observations below.


&[Jid] → slice migration (status + send)

Clean improvement. The .clone() added at src/send.rs:331 is the correct consequence of iterating over &[Jid] instead of owned Vec<Jid> — the else branch now borrows from the slice so it needs an explicit clone to push into resolved_recipients. No concerns here.

The doc example update (vec![...] → [...] + &recipients) is correct and consistent with the new signatures.

#[non_exhaustive] additions

All 8 enums are good candidates. One thing to note: ChatStateType, PresenceStatus, StatusPrivacySetting, and PinDuration use StringEnum derive — worth verifying that #[non_exhaustive] doesn't interfere with that proc macro's generated match arms. Since this is on the defining crate side (not downstream), the generated code should be fine, but something to keep in mind if StringEnum generates exhaustive matches externally.

PartialEq/Eq derives

All struct fields in the annotated types are themselves Eq-compatible (Jid, String, Option<String>, Vec<Jid>, bool, u32, i64), so the derives are sound. GroupMetadata is the largest struct getting these traits (20+ fields) — equality checks will be field-by-field, which is fine for correctness but callers should be aware it's not free for hot paths.

Benchmark alert

The Bencher alert on bench_decrypt_with_previous_session (+9.30%) is unrelated to this PR — the change is purely at the type/signature level with no algorithmic impact on the libsignal benchmarks. Likely CI noise.

Suggestion (non-blocking)

NewsletterMessageType in src/features/newsletter.rs is conspicuously absent from the #[non_exhaustive] treatment while the other three newsletter enums got it. If it's intentional (e.g., stable/frozen enum), no issue — but if it was an oversight, it might be worth adding for consistency.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant