diag(send): measure which term invalidates each group-path device memo

[jlucaso1]

Summary

PRs #1283 and #1285 bounded the cost of a device-memo hit and of a memo miss, and both had to force the outcome to do it. Neither could say which outcome a client takes once the group is warm. An external profile of a client at 763aea9 implied the answer was "miss, on all 30 of 30 sends", with resolve_skdm_targets_memoized at 117,483 Ir/send at 512 members — ~10% of that process's profiled instructions.

Measured here, the answer is the opposite: 30 hits out of 30, on both memos, at 8, 32, 128 and 512 members. filter_skdm_targets costs 0 Ir/send at 512 because it never runs, and resolve_group_devices_uncached costs 1. The external profile is not reproducible against this code, and the batch closes as a characterization: nothing was optimized, and nothing needed to be.

The deliverable is the instrumentation that made that answerable, and it is per-term on purpose. The group memo has three validity terms plus a scoped re-stamp, the SKDM memo has four stale terms plus an entry-absent condition; an aggregate miss count cannot tell a metadata refresh from an in-place cold flip from an entry that was never stored, and it cannot separate cause from consequence either — the SKDM memo compares the Arc the group memo returned, so a group-memo recompute forces skdm_targets.miss_devices regardless of its own terms.

Numbers below are from this container, not from CodSpeed and not from the machine that produced the external profile. Absolute values are not comparable across machines; every comparison here is against a baseline measured on the same host, in the same session.

Changes

• src/client/device_memo_stats.rs (new) — Client::device_memo_stats() -> DeviceMemoStats, always on, no feature gate, per agent_docs/observability.md's rule. Per-term counts for both resolvers, plus DeviceMemoStats::since for scoping a window without a reset (a reset would race a concurrent send). Counters are two arrays indexed by the outcome discriminant, so recording is one indexed relaxed fetch_add with no branch on the variant; exhaustive const fn slot maps tie the array lengths to the enums, so adding a variant anywhere — including after the current last one — stops the crate compiling instead of indexing off the end on the send path. Not #[cfg(test)] like dm_devices_memo_recomputes: a test counter answers the question in a fixture, and the question is what a deployed client gets.
• skdm_memo_entry_is_valid → skdm_memo_entry_stale_term — same four terms, same short-circuit order, now naming the first that failed instead of returning bool. No memo semantics change; the test helper calls the same predicate as before.
• resolve_group_devices_memoized — each exit records its own term. No change to which branch is taken.
• Four tests in src/send/mod.rs — the regime scenario in PN and LID mode, the server-paced variant, and one that drives each miss term and checks it is reported as itself.
• GroupSendFixture gains a LID mode — participants LID-addressed, lid_to_pn_map populated, pairs durably in the LID-PN cache. PR bench(client): measure the group send the client crate actually pays #1283 named LID as the largest gap in its coverage, and it is the mode the external profile ran.
• bench_support: GroupSendHarness::memo_stats() — one accessor, so the hit rate is readable at group sizes no unit test builds. No new benchmark target and no new CodSpeed work: the existing shard already carries four 512-member fixtures, and a hit rate is not a thing a benchmark should measure.

Cost

Method as in #1283: callgrind (valgrind 3.22) over a driver built from the committed bench_support fixture, per-iteration Ir = (Ir at K − Ir at 1) / (K − 1), minimum of the repetitions, spread stated. K = 10001 for the resolver (it does not advance the sender-key chain, so the rotation threshold does not bound it) — at 512 members the setup is 2.78G Ir and drifts ~100K between runs, which at K=1001 is ±200 Ir of noise on a 4,400 Ir measurement and at K=10001 is ±20. Baseline is 139b315, this branch's base at the time, rebuilt and measured in the same session.

The hit rate — the number this batch exists for

30 consecutive warm sends per group size, after one cold send and one priming warm send, counters read over the window:

group_size	group memo	SKDM memo
8	30 hit / 0 restamp / 0 miss	30 hit / 0 miss
32	30 hit / 0 restamp / 0 miss	30 hit / 0 miss
128	30 hit / 0 restamp / 0 miss	30 hit / 0 miss
512	30 hit / 0 restamp / 0 miss	30 hit / 0 miss

Not one miss, of any term, at any size. not_stored is 0 too, so no send left the next one unable to hit.

What a warm send at 512 members actually pays (baseline build, inclusive Ir/send)

symbol	external profile @512	here @512
resolve_skdm_targets_memoized	117,483	2,879
└ resolve_group_devices_uncached	47,338	1
└ filter_skdm_targets chain	11,967	0
get_user_devices_owned (per-member fan-out)	—	2
set_sender_key_status_for_devices	10,298	19
GroupInfo::phone_jid_for_lid_user	11,547	absent (PN fixture)
whole send	—	524,121

The three zeros are the finding: they are what a memo hit is. filter_skdm_targets does not appear because the SKDM memo short-circuits before it; resolve_group_devices_uncached and get_user_devices_owned do not appear because the group memo short-circuits before them.

The memo-hit resolve is flat, with the counters in

group_size	8	32	128	512
Ir/resolve, memo hit (min)	4,435	4,431	4,431	4,433

Spread ≤ 21 Ir at every size. #1285's shape reproduces; the absolute level differs because the machine does.

What the counters cost

	baseline 139b315	this branch	delta
SKDM resolve, memo hit @8	4,419 (4,419–4,427)	4,435 (4,435–4,435)	+16
SKDM resolve, memo hit @512	4,408 (4,408–4,429)	4,433 (4,433–4,452)	+25
whole warm send @512	523,543 (523,543–523,940)	523,778 (523,778–523,802)	+235, below the baseline's own ±397 spread

+16 Ir on the tightest thing the counters sit inside; at whole-send scale the effect is not separable from run-to-run noise. Two relaxed fetch_adds per send. Binary size: +576 B (+0.01%), per the size gate on this PR. CodSpeed flags no regression on any of the four shards.

One thing worth recording, because the first revision got it wrong by 8×. Classifying the outcome into an enum and then matching on it a second time to act cost 126 Ir per resolve, not 16 — the second dispatch, plus moving the SKDM memo entry (an owned clone of a five-field tuple carrying a Jid and a Vec<Jid>) into a temporary just to classify it first. Recording on the branch that decided cut it to 16. Measured both ways, on the same host, in the same session; the shape of the instrumentation mattered more than the atomics did.

Checked and not changed

set_sender_key_status_for_devices — 19 Ir/send at 512, not 10,298. Leave it. The Vec<String> the external profile pointed at is never built on a warm send: exclude_own_devices filters before to_string, and a warm send's SKDM list is our own companion and nothing else, so the filter yields an empty vector and the function returns early. Its whole cost is that one filtered element. The external figure is roughly 512 × 20 Ir, i.e. what the filter scan alone would cost if the list carried every member — which is a redistributing send, not a warm one. Nothing to optimize.

GroupInfo::phone_jid_for_lid_user — it lives entirely inside the miss. It is called only from resolve_group_devices_uncached (once per participant mapping in, once per resolved device mapping back), which the group memo skips on a hit. It is therefore never a target of its own while the memo hits, and repeat_lid_group_sends_hit_both_device_memos_on_every_send shows a LID group hits on all 30. If it ever shows up in a profile, the finding is that the memo missed, and the fix belongs there.

unchanged_for — neither re-stamp nor recompute; it does not run. 0 re-stamps across every window measured, and the symbol does not appear in the send profile at all, because the generation never moves in the steady state. Its cost is consequently not on the steady-state bill, and this PR does not quote one. Test case 4 of each_miss_term_is_reported_as_itself pins that an unrelated write re-stamps rather than recomputing, and that the SKDM memo still hits behind it.

A stale SKDM entry is deliberately left in place when the new targets are not memoizable. It can never become valid again — the map generation only moves forward, the map Arc is replaced wholesale on a rebuild, and the device-set Weak keeps the old allocation alive so no ptr::eq can spuriously match. Clearing it would add a cache write on the miss path and change no outcome. What that does mean is that not_stored guarantees only that the next call cannot hit, not that it reports miss_absent; the counter documents that, and the forget-path test is the worked example (miss_map_generation, then miss_map).

The group_devices_memo capacity (64) is not the suspect. miss_absent is 0 across every window; it is the counter that would report an eviction, and it stays at 0.

ptr::eq(memo.group_info, group_info) is not the suspect either, as the batch already suspected: miss_group_info is 0 in regime, and case 2 of each_miss_term_is_reported_as_itself proves the counter can fire by publishing a fresh Arc into the group cache.

Every writer of device_topology.current(), enumerated so nobody re-derives it: DeviceRegistryCache::insert and ::invalidate (fused, so update_device_list{,s}_guarded, patch_device_add, invalidate_device_cache_guarded, the canonical-flip cleanups and the usync publication all record by construction); the one direct record_registry on the migration-failure path in migrate_device_registry_on_lid_discovery; LidPnCache::add_guarded (record([lid, pn])); and LidPnCache::clear (record_global, which poisons the scoped fast path). DeviceRegistryCache::promote deliberately records nothing — it is a cache fill for an answer the DB already gave.

A server-paced client does not invalidate by construction — but the reason is conditional, and worth knowing. On the receive path the only writer a steady-state message reaches is the LID↔PN learn (cache_lid_pn_from_message, for every message whose sender carries both identifiers). It exits at can_skip_relearn before any write once the pair is durably persisted and resolvable in both directions. So a warm client that has synced its mappings answers 30 inbound messages with 30 memo hits, which is what a_send_answering_an_inbound_group_message_still_hits_both_memos asserts. If persistence ever fails, or a pair is only half-resolvable, every inbound message re-records [lid, pn] and every send after it recomputes both memos — the counters would show group_devices.miss_topology tracking the inbound rate exactly, which is the signature to look for. The other receive-path writer, schedule_unknown_device_sync, is gated on an unknown device and deduped per user, so it does not fire in regime. That test covers the topology-visible half of a receive only; decryption, dispatch and receipts are not modelled, and a regression that made some other part of the receive path write topology would not be caught by it.

Nothing was optimized, and no memo key was touched. The batch's rule was instrumentation first, and the instrumentation says there is nothing to fix on this path.

Validation

• cargo fmt --all
• cargo clippy -p whatsapp-rust --all-targets --features bench-harness -- -D warnings — clean. The rest of the workspace matrix is left to CI (whatsapp-rust-voip-cli cannot build here: its alsa-sys build dependency is absent, same as bench(client): measure the group send the client crate actually pays #1283 and bench(client): address the remaining review comments on #1283 #1285).
• cargo test -p whatsapp-rust --lib — 1,650 passed, 0 failed, including the four new tests. tests/report_coverage.rs passes (the counters name no growable type, so they are correctly outside memory_report()). The e2e crate is not built locally (disk); CI covers it and is green.
• The regime tests were checked against a wrong assumption before they were trusted: the first draft asserted a 30/30 hit rate starting one send after the cold send and failed at 29/30, which is how the cold send turned out to take the force_skdm path and never consult the memos at all. The window now starts where bench_support's fixture starts, and the reason is in the test.
• each_miss_term_is_reported_as_itself drives four different invalidation causes and asserts the term, not a rate — including that a forget takes two resolves (miss_map_generation then miss_map, via the single-flight branch) and that an unrelated topology write re-stamps rather than recomputing.
• Every figure above is the minimum of 2–3 callgrind repetitions with the spread quoted, measured against a baseline built from 139b315 on the same host in the same session.

Review round

Seven bot findings, six fixed and one answered in-thread:

• The const index guard only pinned the last variant, so a variant added after it would still index off the end. Now exhaustive const fn matches — adding a variant anywhere fails to compile. record_* still indexes by as usize, so the measured cost above is unchanged.
• A resolver call whose device resolution returned Err recorded no SKDM outcome, so calls() was not one per call and hit_rate() could read healthy over a shrinking denominator. Added SkdmTargetsMemoOutcome::ResolveFailed / SkdmTargetsMemoStats::resolve_failed.
• Fixing that made hit_rate's own doc wrong in turn — a failed call skipped filter_skdm_targets too, by not getting that far — so it now says memo hits over every resolver call, and why keeping failures in the denominator is the point.
• not_stored claimed the next call would be a miss_absent. Corrected to what it actually guarantees (see "Checked and not changed"); the code is right, the doc was not — in two places, the counter and observability.md, the second only caught on the follow-up round.
• observability.md counted four SKDM terms against five miss counters; it now names the four stale terms plus the entry-absent condition.
• The unrelated-user literal used an NPA of 555, which is not the reserved fictional NANP format.

CI

Green: Clippy, Build & Test, Build & Lint (all features), Feature Matrix, Test Stable (MSRV), E2E, Rustdoc, wasm32 release, all three Miri gates, Format, Cargo Deny, Binary Size, and all four CodSpeed shards plus the performance analysis.

Semver Checks (informational) is red and continue-on-error: true. Confirmed pre-existing rather than assumed: the job checks -p wacore -p wacore-binary -p waproto, and this PR touches none of those crates. Every item it names is the same set #1283 and #1285 saw — mex_operations structs/fields/modules, the BinaryError::UnexpectedFormatByte variant, and the removed simd feature — measured against the last published release rather than this PR's base. The only public surface here is additive (a new module and its types, one new Client method), and the one rename is pub(crate).

One earlier Clippy failure was an infrastructure flake, not code: sccache-action aborted at setup with a TLS self-signed certificate error, so clippy never executed. It did not recur.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8a1e51f4-1571-4335-9a06-38557ea03da9

📥 Commits

Reviewing files that changed from the base of the PR and between 7c4f21b and 274e9e7.

📒 Files selected for processing (1)

• agent_docs/observability.md

---

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Added device memo statistics for group and direct-message resolution.
  • Statistics include hits, misses, restamps, bypasses, invalidation causes, and non-stored outcomes.
  • Added snapshot differencing, call counts, hit/served rates, display formatting, and benchmark visibility.

• Documentation

  • Expanded observability guidance and clarified benchmark coverage, cold-resolution costs, and steady-state validation.

• Tests

  • Added coverage for memo outcomes, invalidation causes, addressing modes, warm sends, and empty-state rates.

Walkthrough

The PR adds always-on statistics for group-device and SKDM memo resolution. Memo paths record typed hit, miss, restamp, bypass, and non-stored outcomes. It also adds PN/LID validation tests, benchmark access, and observability documentation.

Changes

Device memo observability

Layer / File(s)	Summary
Statistics contract and client wiring src/client/device_memo_stats.rs, src/client.rs, src/client/lifecycle.rs	Adds atomic outcome counters, public statistics types, rates, snapshot differencing, display output, exports, and client initialization.
Group-device memo outcome recording src/client/device_registry.rs	Records bypass, hit, restamp, group-info, topology, and absent-entry outcomes during memo resolution.
SKDM outcome tracking and send validation src/send/mod.rs	Returns typed stale terms, records SKDM outcomes, controls target-set storage, and expands PN/LID fixtures and regression tests.
Observability documentation and benchmark access agent_docs/observability.md, src/bench_support.rs, benches/client_group_send.rs	Documents the fourth measurement surface and exposes memo statistics from the group-send benchmark harness.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GroupSendFixture
  participant SKDMMemo
  participant DeviceRegistry
  participant DeviceMemoCounters
  GroupSendFixture->>SKDMMemo: resolve memoized targets
  SKDMMemo->>DeviceRegistry: resolve group devices
  DeviceRegistry-->>SKDMMemo: return group memo outcome
  SKDMMemo->>DeviceMemoCounters: record SKDM outcome
  DeviceMemoCounters-->>GroupSendFixture: provide memo statistics

Loading

Possibly related PRs

• oxidezap/whatsapp-rust#840: Introduced the group-device memo lookup flow instrumented by this PR.
• oxidezap/whatsapp-rust#985: Modified SKDM memo validation extended with typed stale-outcome tracking here.
• oxidezap/whatsapp-rust#1283: Added related group-send benchmark and SKDM memo-resolution paths.

Suggested labels: api-design

Suggested reviewers: greptile-apps

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 identifies the main change: measuring the invalidation term for group-path device memos.
Description check	✅ Passed	The description directly explains the instrumentation, tests, measurements, costs, and validation for device memo outcomes.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/group-send-memo-hitrate-ft6dga

---

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

The PR adds always-on, per-term instrumentation for the group-device and SKDM memos without changing their memoization semantics. The previously uncounted device-resolution error path now records resolve_failed, preserving the one-outcome-per-call contract.

• Adds cumulative memo statistics, scoped snapshot subtraction, rates, and display formatting.
• Records group-device and SKDM outcomes at the branches that determine them.
• Adds PN/LID fixtures and steady-state, server-paced, and invalidation-term coverage.
• Exposes memo statistics through the benchmark harness and documents the observability surface.

Confidence Score: 5/5

The PR appears safe to merge.

The previously omitted failed-resolution path now records resolve_failed, and no blocking failure remains.

Important Files Changed

Filename	Overview
src/client/device_memo_stats.rs	Introduces per-client group-device and SKDM outcome counters, snapshots, deltas, rates, and formatting with exhaustive enum-to-slot compile-time checks.
src/send/mod.rs	Records each SKDM lookup outcome, including prerequisite resolution failures, and expands PN/LID memo behavior tests.
src/client/device_registry.rs	Adds branch-local accounting for group-device memo hits, re-stamps, misses, and bypasses without changing resolver decisions.
src/client.rs	Registers the new statistics module, exports its public reporting types, and stores counters on each client.
src/client/lifecycle.rs	Initializes the per-client memo counters during client construction.
src/bench_support.rs	Exposes memo snapshots through the group-send benchmark harness.
benches/client_group_send.rs	Updates benchmark guidance to explain where memo outcome frequency is measured.
agent_docs/observability.md	Documents the new always-on memo statistics surface and interpretation of each outcome.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Resolve SKDM targets] --> B[Resolve group devices]
    B -->|Error| C[Record resolve_failed]
    B -->|Success| D{SKDM memo enabled?}
    D -->|No| E[Record bypassed]
    D -->|Yes| F{Memo entry state}
    F -->|Absent| G[Record miss_absent]
    F -->|Stale| H[Record first stale term]
    F -->|Valid| I[Record hit and return memoized targets]
    E --> J[Filter SKDM targets]
    G --> J
    H --> J
    J --> K{Targets memoizable?}
    K -->|Yes| L[Store memo]
    K -->|No| M[Record not_stored]
    L --> N[Return resolved targets]
    M --> N
    C --> O[Return None]

Loading

_{Reviews (4): Last reviewed commit: "docs(perf): describe hit_rate as memo hi..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 3

🤖 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/bench_support.rs`:
- Around line 191-193: Update the documentation near Self::warm_send to replace
the unclear phrase “which outcome a client in regime takes” with wording that
states “which outcome a client takes in a given regime,” preserving the
surrounding benchmark guidance.

In `@src/client/device_memo_stats.rs`:
- Around line 144-149: Replace the last-variant assertions in the const block
with exhaustive const-evaluated matches for GroupDevicesMemoOutcome and
SkdmTargetsMemoOutcome, assigning every enum variant its expected array index.
Ensure adding any variant requires updating the match and fails compilation if
omitted, while leaving the as usize indexing in record_group_devices and
record_* unchanged.

In `@src/send/mod.rs`:
- Line 3613: Update the string literal passed to
fixture.client.device_topology.record in the topology fixture from the invalid
555 NPA format to the repository-standard fictional NANP value 12025550111,
preserving its use as the topology cache key.

🪄 Autofix

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: 9ada4112-ba5f-4ed1-a044-bda5dd72db4b

📥 Commits

Reviewing files that changed from the base of the PR and between 8e1b502 and 331aaa5.

📒 Files selected for processing (8)

• agent_docs/observability.md
• benches/client_group_send.rs
• src/bench_support.rs
• src/client.rs
• src/client/device_memo_stats.rs
• src/client/device_registry.rs
• src/client/lifecycle.rs
• src/send/mod.rs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 331aaa5c4a

ℹ️ 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".

[chatgpt-codex-connector]

Clear stale entries before recording not_stored

When an existing memo becomes stale and the newly resolved targets contain a non-own device, this branch records not_stored but leaves the old entry in skdm_warm_memo. The next lookup therefore sees Some(memo) and reports the applicable stale term (the forget-path test already demonstrates MissMapGeneration followed by MissMap), not the promised miss_absent; this makes the public diagnostic misleading when consumers use not_stored to distinguish deliberate non-storage from eviction. Remove the stale entry here or revise the counter's documented semantics.

AGENTS.md reference: AGENTS.md:L59-L59

Useful? React with 👍 / 👎.

[jlucaso1]

Right that the doc overpromised; fixed there rather than in the code, because clearing the entry buys nothing.

A stale entry left in place cannot come back to life. The map generation is monotonic, the map Arc is replaced wholesale on a rebuild, and the device-set Weak keeps the old allocation alive precisely so a new Arc can never land on the same address — that is the ABA guard GroupDevicesMemo documents. So the entry stays stale until something overwrites it, and the failure mode a clear would prevent (serving a stale needs_skdm) is not reachable. Clearing would add a cache write on the miss path and change no outcome.

What was wrong was the claim, so not_stored now documents what it actually guarantees — the next call cannot hit — and says explicitly that when a stale entry was already there the next call reports whichever term is still failing, not miss_absent. Your read of the forget-path test is exactly that case, and it is now the example in the comment. SkdmTargetsMemoOutcome::MissAbsent lost the same wrong sentence.

Your sibling comment about the Err arm was a real hole in the API contract and is fixed as you described: SkdmTargetsMemoOutcome::ResolveFailed is recorded there, so calls() is one per call again and hit_rate() cannot look healthy over a denominator that quietly shrank.

---

Generated by Claude Code

[chatgpt-codex-connector]

Account for failed SKDM resolver calls

When resolve_group_devices_memoized returns an error—for example after a failed cache-backed device resolution—execution reaches the outer Err arm before any SKDM outcome is recorded, because all new counters are inside the success arm. Consequently SkdmTargetsMemoStats::calls() is not one counter per resolver call as its public API claims, and hit_rate() can appear healthy while attempted group sends are failing before memo lookup; add an error/aborted outcome or explicitly scope the API to successful prerequisite resolutions.

AGENTS.md reference: AGENTS.md:L59-L59

Useful? React with 👍 / 👎.

[github-actions]

📦 Binary size report

Metric	main	PR	Δ
bin size (stripped)	10.08 MiB	10.08 MiB	+576 B (+0.01%) 🔺
bin .text	8.07 MiB	8.07 MiB	+576 B (+0.01%) 🔺
bin allocated (text+data+bss)	10.08 MiB	10.08 MiB	0
llvm-lines wacore	533,726	533,726	0
llvm-lines wacore copies	17,428	17,428	0
llvm-lines whatsapp-rust lib	766,615	767,656	+1,041 (+0.14%) 🔺
llvm-lines whatsapp-rust lib copies	23,863	23,873	+10 (+0.04%) 🔺
deps crates (Cargo.lock)	462	462	0

.text per crate

Crate	main	PR	Δ
.text whatsapp_rust	1.86 MiB	1.86 MiB	+556 B (+0.03%) 🔺
.text wacore	692.31 KiB	692.31 KiB	0
.text wacore_binary	81.61 KiB	81.61 KiB	0
.text wacore_libsignal	178.98 KiB	178.98 KiB	0
.text wacore_appstate	22.35 KiB	22.35 KiB	0
.text wacore_noise	20.94 KiB	20.94 KiB	0
.text waproto	1.74 MiB	1.74 MiB	0
.text whatsapp_rust_sqlite_storage	540.56 KiB	540.56 KiB	0
.text whatsapp_rust_tokio_transport	40.49 KiB	40.49 KiB	0
.text whatsapp_rust_ureq_http_client	12.68 KiB	12.68 KiB	0
.text std	994.83 KiB	994.83 KiB	0
.text other deps	1.90 MiB	1.90 MiB	0

Baseline: 8e1b50291 (latest main run) · Head: e0203c2f8 · Graphs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b63882f231

ℹ️ 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".

[chatgpt-codex-connector]

Stop promising an absent miss after non-storage

When a stale SKDM memo already exists and a newly resolved non-own target set cannot be stored, the implementation deliberately retains that stale entry, so the next lookup reports whichever stale term still fails rather than miss_absent. Fresh evidence after the earlier review is that the updated SkdmTargetsMemoStats::not_stored documentation now explicitly describes this behavior, but this observability guide still promises the opposite, which can cause operators to misclassify deliberate non-storage as eviction; update this guide to match the public counter semantics.

AGENTS.md reference: AGENTS.md:L59-L59

Useful? React with 👍 / 👎.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

agent_docs/observability.md (1)

114-122: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Reconcile the SKDM term count with miss_absent.

DeviceMemoStats exposes five SKDM miss counters: miss_absent, miss_devices, miss_map, miss_map_generation, and miss_sender. This section lists only four SKDM validity terms and omits the absent-entry condition, while it explicitly includes entry present for group memos. State that the SKDM memo has four stale terms plus the entry-absent condition, or list all five conditions.

Proposed documentation fix

- and the SKDM memo has four
- (device `Arc`, sender-key-map `Arc`, map generation, sending identity).
+ and the SKDM memo has four stale terms
+ (device `Arc`, sender-key-map `Arc`, map generation, sending identity),
+ plus the entry-absent condition recorded by `miss_absent`.

🤖 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 `@agent_docs/observability.md` around lines 114 - 122, Update the SKDM memo
description to account for all five miss counters: four stale-validity terms
(device Arc, sender-key-map Arc, map generation, and sending identity) plus the
entry-absent condition. Keep the existing group memo terms and explanation of
dependent miss_devices behavior unchanged.

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

Outside diff comments:
In `@agent_docs/observability.md`:
- Around line 114-122: Update the SKDM memo description to account for all five
miss counters: four stale-validity terms (device Arc, sender-key-map Arc, map
generation, and sending identity) plus the entry-absent condition. Keep the
existing group memo terms and explanation of dependent miss_devices behavior
unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2229b96c-f2d3-4a60-88b7-0d8e87eefbcb

📥 Commits

Reviewing files that changed from the base of the PR and between b63882f and 7c4f21b.

📒 Files selected for processing (1)

• agent_docs/observability.md

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 274e9e732b

ℹ️ 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".

[chatgpt-codex-connector]

Describe hit_rate as the memo-hit share

When prerequisite device resolution fails, the call never reaches filter_skdm_targets, but resolve_failed is included in calls() and excluded from the numerator. Therefore this method does not return the documented share of calls that skipped the filter; it returns memo hits divided by all resolver attempts. In failure-heavy workloads, consumers relying on this description will misinterpret the metric, so document the failure-inclusive memo-hit semantics instead.

AGENTS.md reference: AGENTS.md:L59-L59

Useful? React with 👍 / 👎.

Dismissed because a newer commit was pushed; Greptile will re-review the current head.