refactor(api): rustdoc for entry types, must_use, root re-exports, and constructor consistency - #1089
Conversation
…d constructor consistency Ergonomics pass over the public surface, no behavior changes. - Document Client and Bot, the two entry points, including how to build them and where run/send/events live. - Mark BotHandle, BotBuilder and ClientBuilder #[must_use]; dropping a BotHandle silently aborts the bot, and a dropped builder does nothing. - Re-export Polls, PollOptionResult, PollVoteCiphertext, MexGraphQLError and EncType from the crate root, like their feature siblings. - Drop the get_ prefix from Client::get_push_name/get_pn/get_lid so they match the surrounding accessors. - Take &Jid/&[Jid] in AcceptGroupInviteV4Iq, BatchGetGroupInfoIq and GetGroupProfilePicturesIq constructors, per the IqSpec convention. - Seal SendOptions and EditOptions with #[non_exhaustive] and add with_* setters so fields can be added without breaking consumers. - Explain when to use Groups::query_info (cached, send oriented) versus Groups::get_metadata (full, always over the network).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 49 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR renames client identity accessors, adds fluent send-option setters, changes group IQ constructors to borrow inputs, documents public APIs, adds ChangesAPI migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
| Filename | Overview |
|---|---|
| src/bot.rs | Adds comprehensive rustdoc to Bot (with a no_run quickstart example) and BotBuilder (#[must_use]); BotHandle gains #[must_use] with correct drop-aborts message. |
| src/client.rs | Adds comprehensive rustdoc to Client covering lifecycle, event bus, and send API; no logic changes. |
| src/client/accessors.rs | Renames get_push_name/get_pn/get_lid to push_name/pn/lid and adds rustdoc; internal require_pn updated correctly. |
| src/client/builder.rs | ClientBuilder gains #[must_use] with the conventional 'call .build()' message; no other changes. |
| src/send/mod.rs | Adds #[non_exhaustive] + #[must_use] with_* setters to SendOptions and EditOptions; internal EditOptions struct-literal at line 2843 stays valid (inside crate). All accessor renames applied. |
| src/features/groups.rs | Adds rustdoc distinguishing query_info (cached, send-path) from get_metadata (network, full); IqSpec call sites updated to pass references. |
| src/lib.rs | Adds root re-exports: EncType, MexGraphQLError, PollOptionResult, PollVoteCiphertext, Polls. No removed exports. |
| wacore/src/iq/groups.rs | AcceptGroupInviteV4Iq::new, BatchGetGroupInfoIq::new, and GetGroupProfilePicturesIq::new/with_type changed from owned types to &Jid/&[Jid] with internal clone/to_vec; tests updated. |
| src/message/msg_secret.rs | Accessor renames applied across several address-mode dispatch branches; logic unchanged. |
| tests/e2e/src/lib.rs | All get_pn/get_lid/get_push_name call sites in the shared test helper updated to the new names. |
Reviews (2): Last reviewed commit: "docs(groups): correct query_info cache a..." | Re-trigger Greptile
📦 Binary size report
.text per crate
Baseline: |
The with_* setters consume self and return Self, so discarding the result silently drops the configured value.
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 `@src/features/groups.rs`:
- Around line 370-382: The documentation for Groups::query_info incorrectly
implies stale cache entries are automatically revalidated. Update its
cache-semantics description to state that CachePreferred returns cached
GroupInfo immediately, while the persisted-phash/not-modified request occurs
only on a cache miss or explicit refresh; retain the existing guidance about
get_metadata and query_info_with_freshness.
- Around line 625-635: Update the get_metadata documentation to describe
participant phone-number resolution as best-effort: known LID→PN mappings
populate phone numbers, while unknown mappings remain unset. Remove wording that
promises complete or fully resolved phone numbers, while preserving the rest of
the metadata and caching behavior description.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 94822dd2-ecb2-4ca2-b765-7f08756f71ec
📒 Files selected for processing (36)
src/bot.rssrc/client.rssrc/client/accessors.rssrc/client/builder.rssrc/client/messaging.rssrc/features/comments.rssrc/features/events.rssrc/features/groups.rssrc/features/polls.rssrc/features/reaction.rssrc/handlers/call.rssrc/history_sync.rssrc/lib.rssrc/message/msg_secret.rssrc/message/receive.rssrc/message/tests.rssrc/send/mod.rssrc/voip/facade.rstests/e2e/src/lib.rstests/e2e/tests/app_state.rstests/e2e/tests/chat_actions.rstests/e2e/tests/community.rstests/e2e/tests/groups.rstests/e2e/tests/lid_sessions.rstests/e2e/tests/media.rstests/e2e/tests/memory_soak.rstests/e2e/tests/prekey_sessions.rstests/e2e/tests/privacy_tokens.rstests/e2e/tests/profile.rstests/e2e/tests/profile_picture.rstests/e2e/tests/receipts.rstests/e2e/tests/retry_dm_multidevice.rstests/e2e/tests/session_reuse.rstests/e2e/tests/status.rswacore/src/iq/groups.rswacore/src/stanza/call.rs
…wording query_info returns a cached entry as-is; the phash request happens only on a miss. get_metadata backfills participant phone numbers on a best-effort basis, so an unmapped LID stays unset.
Summary
An ergonomics pass over the public API surface: the two entry types (
ClientandBot) had no rustdoc at all, a few builders could be dropped silently with no warning, and a handful of types and constructors drifted from the conventions their siblings already follow. No behavior changes, only surface.Changes
ClientandBot. What each one is, how to get one (ClientBuildervsBot::builder()), and pointers to the lifecycle (run/connect/disconnect), the event bus, and sending.Botalso gets a short quick-start example.#[must_use]onBotHandle,BotBuilderandClientBuilder. Dropping aBotHandleaborts the bot, which is easy to do by accident, so the message says so; the builders get the usual "call.build()".Polls,PollOptionResult,PollVoteCiphertext,MexGraphQLErrorandEncTypeare now re-exported from the crate root alongsideGroups/Contacts/Mex.get_prefix fromClient::get_push_name/get_pn/get_lid→push_name/pn/lid, matching the neighbouring accessors. Breaking; all workspace call sites updated.&Jid/&[Jid]in IqSpec constructors.AcceptGroupInviteV4Iq::newtook ownedJid/String, andBatchGetGroupInfoIq::new/GetGroupProfilePicturesIq::newtookVec<Jid>, while their siblings take references. They now take references and clone internally, peragent_docs/protocol_architecture.md.GetGroupProfilePicturesIq::with_typemoved withnewso the type stays internally consistent. Breaking for direct IqSpec users.SendOptionsandEditOptionswith#[non_exhaustive]plus chainablewith_*setters (SendOptions::default().with_message_id(id)).bonis feature-gated behindpluginsin this crate, so hand-rolled setters were the option that does not pull a new unconditional dependency. Breaking for out-of-crate struct-literal construction; e2e call sites updated.Groups::query_infovsGroups::get_metadata. Not renamed, only explained:query_inforeturns the slim, phash-refreshed, cachedArc<GroupInfo>the send path needs, whileget_metadataalways goes to the network for the fullGroupMetadata.Validation
e2e was not run locally; CI covers it.
Generated by Claude Code