docs: document Client::device_memo_stats() - #518
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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 |
jlucaso1
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4518b82444
ℹ️ 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".
| pub fn device_memo_stats(&self) -> DeviceMemoStats | ||
| ``` | ||
|
|
||
| Per-term hit/miss counts for the two device-list memos the group-send path depends on — the group-devices memo and the SKDM-targets memo — cumulative since the client was built. Always on, no feature gate: recording is one indexed relaxed atomic add per resolver call, and the reporting types are dropped by LTO in a binary that never calls this method. |
There was a problem hiding this comment.
The second sentence combines feature availability, runtime instrumentation cost, and LTO behavior into one long statement. Split these into separate sentences so readers can scan each guarantee independently, as required by the repository's one-idea-per-sentence documentation rule.
AGENTS.md reference: AGENTS.md:L24-L25
Useful? React with 👍 / 👎.
Addresses review feedback on #518: the availability/cost/LTO guarantees were bundled into one sentence, against AGENTS.md's one-idea-per-sentence rule. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E4eR5JAKYPgnJFNb3rsU8a
|
@claude solve conflicts |
whatsapp-rust#1292 added a new always-on, no-feature-gate diagnostic: per-term hit/miss counters for the group-devices and SKDM-targets memos on the group-send path, exposed as Client::device_memo_stats() returning DeviceMemoStats (public in whatsapp_rust::client, not re-exported from the crate root). Documents it alongside stats() / memory_report() / resource_report() in the API client reference's Diagnostics section. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E4eR5JAKYPgnJFNb3rsU8a
Addresses review feedback on #518: the availability/cost/LTO guarantees were bundled into one sentence, against AGENTS.md's one-idea-per-sentence rule. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E4eR5JAKYPgnJFNb3rsU8a
538f660 to
faf87a9
Compare
jlucaso1
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faf87a9590
ℹ️ 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".
| Storage, transport, and HTTP reports are supplied by the trait implementations behind `Client` — see [`DeviceStore::resource_report`](/api/store#resource_report), [`Transport::resource_report`](/api/transport#resource_report), and [`HttpClient::resource_report`](/api/http-client#resource_report). `AllocSnapshot`, `StorageResourceReport`, `TransportResourceReport`, and `HttpResourceReport` are re-exported from `wacore::stats`; all four are also re-exported from the `whatsapp_rust` crate root. | ||
| </Note> | ||
|
|
||
| ### device_memo_stats |
There was a problem hiding this comment.
Adding this method leaves the Diagnostics introduction at line 2366 inaccurate: it still says there are exactly “Three on-Client surfaces” and enumerates only stats(), memory_report(), and resource_report(). Include device_memo_stats() there so the section summary remains accurate and readers using it to choose a diagnostic API can discover the new counters.
Useful? React with 👍 / 👎.
| |-------|------|-------------| | ||
| | `hits` | `u64` | Memo hit | | ||
| | `miss_absent` | `u64` | No entry — first send for this group, or an eviction | | ||
| | `miss_devices` | `u64` | The resolved device-set `Arc` differs from the memoized one — the cascade term a group-memo recompute always forces | |
There was a problem hiding this comment.
miss_devices outcome
A group-memo recomputation does not always force miss_devices: when the SKDM memo has no entry, such as on the first send or after its eviction, the immediately preceding miss_absent definition says that lookup is classified as miss_absent, and there is no memoized device-set Arc to compare. Limit this claim to recomputations performed while an SKDM entry exists so readers do not misclassify normal cold-start misses.
Useful? React with 👍 / 👎.
| pub fn device_memo_stats(&self) -> DeviceMemoStats | ||
| ``` | ||
|
|
||
| Per-term hit/miss counts for the two device-list memos the group-send path depends on — the group-devices memo and the SKDM-targets memo — cumulative since the client was built. Always on, no feature gate. Recording is one indexed relaxed atomic add per resolver call. The reporting types are dropped by LTO in a binary that never calls this method. |
There was a problem hiding this comment.
not_stored increment
The one-add-per-resolver-call cost claim does not hold when a resolved target set cannot be memoized: that call records its ordinary lookup outcome and also increments the supplemental not_stored counter. The later statement that calls() excludes not_stored because it is not a lookup outcome confirms that the counter is not mutually exclusive with the per-call outcome, so document this exceptional second relaxed atomic add rather than presenting one add as unconditional.
Useful? React with 👍 / 👎.
| | `bypassed` | `u64` | Call didn't consult the memo | | ||
| | `resolve_failed` | `u64` | The device resolution this call depends on returned `Err`, so no memo term was evaluated | | ||
|
|
||
| `calls()` sums every field except `not_stored` (which describes the store, not a lookup outcome). `hit_rate()` returns `hits / calls()`, `resolve_failed` included in the denominator on purpose — folding failed resolutions out would let a client whose group sends are failing upstream read a healthy rate — or `None` before the first call. |
There was a problem hiding this comment.
This sentence combines the calls() aggregation rule, the hit_rate() formula, the rationale for including failures, and the pre-first-call return value. Split these guarantees into separate sentences so each API behavior conveys one idea and remains easy to scan.
AGENTS.md reference: AGENTS.md:L25-L25
Useful? React with 👍 / 👎.
- Diagnostics intro now mentions device_memo_stats as the fourth always-on surface, answering a different question than the other three. - The atomic-add cost note now accounts for the extra not_stored add a non-memoizable resolution also pays. - The chained-memo paragraph now qualifies that a group-memo recompute only forces skdm_targets.miss_devices when an SKDM entry already exists; with none yet, that call reports miss_absent instead. - Split the hit_rate()/calls() paragraph into one idea per sentence. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E4eR5JAKYPgnJFNb3rsU8a
2 participants
Summary
oxidezap/whatsapp-rust#1292added a new always-on, no-feature-gate diagnostic: per-term hit/miss counters for the two device-list memos on the group-send path (group-devices memo and SKDM-targets memo), exposed asClient::device_memo_stats() -> DeviceMemoStats.DeviceMemoStats,GroupDevicesMemoStats, andSkdmTargetsMemoStatsare newpubtypes inwhatsapp_rust::client.Documented it in
api/client.mdx's "Diagnostics" section alongsidestats()/memory_report()/resource_report(), with the full field tables for both stat structs,calls()/served_rate()/hit_rate()/since()semantics, and a usage example. Noted that these types aren't re-exported from the crate root, unlike the other diagnostics types in that section.The rest of #1292 (internal instrumentation wiring in
device_registry.rs/send/mod.rs, benchmark/fixture changes) has no public surface beyonddevice_memo_stats()itself.Note: this PR originally also carried a fix for
PlatformType::WAIL's mapping inconcepts/authentication.mdx(fromwhatsapp-rust#1293), but#517landed that same fix onmainfirst — rebased this branch ontomainand dropped the now-redundant commit to resolve the conflict.Test plan
device_memo_stats()/DeviceMemoStats/GroupDevicesMemoStats/SkdmTargetsMemoStatsinsrc/client.rsandsrc/client/device_memo_stats.rsonmain(0c76269) against the new doc section, field by field.pubinwhatsapp_rust::clientbut absent fromsrc/lib.rs's re-export list, unlikeStatsSnapshot/MemoryReport/ResourceReport.Ref: oxidezap/whatsapp-rust#1292