docs: reflect JID identity/equality changes from whatsapp-rust#1182 - #464
Conversation
Follow-up to whatsapp-rust#1182, which moved Jid/JidRef equality and Hash off the derived impls onto identity_agent()-routed hand-written ones. Update the notes explaining why a wire-decoded JID equals a text-parsed one to describe the structural guarantee instead of the decoder-only invariant that used to be the whole story.
Follow-up to whatsapp-rust#1182. sort_dedup_by_device now keys on Jid::identity_agent() (user + server + device + integrator + identity_agent) instead of the raw agent field, matching the JID's own hand-written PartialEq/Hash. Explain why both directions of the old raw-agent key were wrong: it could either merge two rendered-agent devices or fail to collapse two JIDs that are actually one device.
📝 WalkthroughWalkthroughDocumentation now describes normalized JID identity semantics across binary decoding, equality, hashing, and signal-protocol device deduplication. It also explains how ChangesJID identity semantics
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@advanced/binary-protocol.mdx`:
- Around line 127-131: Rewrite the documentation at advanced/binary-protocol.mdx
lines 127-131 and advanced/signal-protocol.mdx lines 2009-2013 using concise,
one-idea sentences. In binary-protocol.mdx, separate the normalization behavior,
equality and hashing behavior, integrator handling, and public identity_agent()
guidance. In signal-protocol.mdx, separate the deduplication key, raw agent
failure mode, and Bot/Interop exception. Make no behavioral or API changes.
- Around line 127-131: Update the documentation around Jid and JidRef equality
to describe identity_agent() consistently as public rather than private, and
replace “derived comparisons” with “manual comparisons” when explaining how
callers should apply the same identity rule.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 648563fc-18d5-4515-9c21-f419779be5b3
📒 Files selected for processing (2)
advanced/binary-protocol.mdxadvanced/signal-protocol.mdx
| Earlier versions kept a redundant copy in `agent`. If you decoded a JID off the wire, you got `agent` set to the domain byte; if you parsed the same JID from text, you got `agent` set to `0` — even though `Display` rendered both the same way. With `PartialEq`/`Hash` derived at the time, those two JIDs compared unequal and could hash to different values. | ||
|
|
||
| `agent` now stays `0` for these servers on both paths, so a wire-decoded JID and a text-parsed JID compare equal and hash the same. `agent` is only ever nonzero for `Bot`/`Interop`, which do render it. | ||
| `agent` now stays `0` for these servers on both paths, so a wire-decoded JID and a text-parsed JID compare equal and hash the same. But `PartialEq`/`Hash` no longer *rely* on that invariant holding everywhere either — `Jid` and `JidRef` implement both by hand, routed through a private `identity_agent()` that reads as `0` on any server where `Server::renders_agent()` is false, regardless of what the raw field holds. That closes a gap the decoder fix alone didn't: `swap_pn_lid_namespace` and similar code can still carry a nonzero `agent` across a namespace conversion, and equality treats the result the same as the clean JID either way. `agent` is only ever identity-relevant for `Bot`/`Interop`, which do render it. | ||
|
|
||
| `integrator` is *not* normalized the same way — it is folded into identity unconditionally, matching `is_same_chat_as`, since the field is never set outside `Interop` in practice. `Jid::identity_agent()` is public so code building its own key over a JID (sorting, deduplicating, indexing) can apply the same rule the derived comparisons use instead of reading `jid.agent` directly. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Use concise, one-idea sentences in both documentation updates.
advanced/binary-protocol.mdx#L127-L131: split normalization, equality/hashing, integrator behavior, and public API guidance into separate sentences.advanced/signal-protocol.mdx#L2009-L2013: split the deduplication key, raw-agent failure mode, and Bot/Interop exception into separate sentences.
As per coding guidelines, keep sentences concise — one idea per sentence in documentation.
📍 Affects 2 files
advanced/binary-protocol.mdx#L127-L131(this comment)advanced/signal-protocol.mdx#L2009-L2013
🤖 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 `@advanced/binary-protocol.mdx` around lines 127 - 131, Rewrite the
documentation at advanced/binary-protocol.mdx lines 127-131 and
advanced/signal-protocol.mdx lines 2009-2013 using concise, one-idea sentences.
In binary-protocol.mdx, separate the normalization behavior, equality and
hashing behavior, integrator handling, and public identity_agent() guidance. In
signal-protocol.mdx, separate the deduplication key, raw agent failure mode, and
Bot/Interop exception. Make no behavioral or API changes.
Source: Coding guidelines
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -A8 -B4 'identity_agent' wacore/binary/src/jid.rsRepository: oxidezap/whatsapp-rust-docs
Length of output: 286
🏁 Script executed:
#!/bin/bash
set -u
echo "Repository files matching binary/jid docs:"
git ls-files | rg -n '(^|/)advanced/binary-protocol\.mdx$|jid\.rs$' || true
echo
echo "Locate identity_agent in relevant tracked files:"
rg -n 'identity_agent|renders_agent|PartialEq|Hash|JidRef|identity' -g 'advanced/binary-protocol.mdx' -g '*.rs' . || trueRepository: oxidezap/whatsapp-rust-docs
Length of output: 5547
🏁 Script executed:
#!/bin/bash
set -u
# Inspect the referenced documentation section and nearby context.
sed -n '118,136p' advanced/binary-protocol.mdx | cat -n
printf '\n--- later JID section ---\n'
sed -n '538,552p' advanced/binary-protocol.mdx | cat -nRepository: oxidezap/whatsapp-rust-docs
Length of output: 4992
Make the identity_agent() API description internally consistent.
Line 131 says Jid::identity_agent() is public, while line 129 describes it as a private helper. Also replace “derived comparisons” with “manual comparisons” because the text says the implementations are hand-written.
🤖 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 `@advanced/binary-protocol.mdx` around lines 127 - 131, Update the
documentation around Jid and JidRef equality to describe identity_agent()
consistently as public rather than private, and replace “derived comparisons”
with “manual comparisons” when explaining how callers should apply the same
identity rule.
![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: bf91aea848
ℹ️ 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".
| Earlier versions kept a redundant copy in `agent`. If you decoded a JID off the wire, you got `agent` set to the domain byte; if you parsed the same JID from text, you got `agent` set to `0` — even though `Display` rendered both the same way. With `PartialEq`/`Hash` derived at the time, those two JIDs compared unequal and could hash to different values. | ||
|
|
||
| `agent` now stays `0` for these servers on both paths, so a wire-decoded JID and a text-parsed JID compare equal and hash the same. `agent` is only ever nonzero for `Bot`/`Interop`, which do render it. | ||
| `agent` now stays `0` for these servers on both paths, so a wire-decoded JID and a text-parsed JID compare equal and hash the same. But `PartialEq`/`Hash` no longer *rely* on that invariant holding everywhere either — `Jid` and `JidRef` implement both by hand, routed through a private `identity_agent()` that reads as `0` on any server where `Server::renders_agent()` is false, regardless of what the raw field holds. That closes a gap the decoder fix alone didn't: `swap_pn_lid_namespace` and similar code can still carry a nonzero `agent` across a namespace conversion, and equality treats the result the same as the clean JID either way. `agent` is only ever identity-relevant for `Bot`/`Interop`, which do render it. |
There was a problem hiding this comment.
When readers want to build custom sort or index keys, this sentence says identity_agent() is private, while the next paragraph explicitly presents Jid::identity_agent() as public for that exact use. This leaves the documented API contradictory; describe the Jid method as public here, or qualify which separate helper remains private.
Useful? React with 👍 / 👎.
| pub fn sort_dedup_by_device(jids: &mut Vec<Jid>); | ||
| ``` | ||
|
|
||
| `sort_dedup_by_device` keys on `Jid::identity_agent()` rather than the raw `agent` field, so its notion of "same device" matches exactly what `Jid`'s `PartialEq`/`Hash` already treat as equal (see [Binary Protocol](/advanced/binary-protocol#jid-encoding) for why the two fields differ). That has to hold in both directions: keying on the raw `agent` would let two JIDs that are actually one device — an inert agent byte on `Pn`/`Lid`/`Hosted`/`HostedLid`, same AD-JID, same Signal address — both survive the dedup and pick up two concurrent encryption jobs against one session; dropping `agent` from the key entirely would go too far the other way and silently collapse two genuinely distinct `@bot`/`@interop` devices, which *do* render it, losing a fan-out destination. |
There was a problem hiding this comment.
This added sentence combines the chosen key, its equality rationale, the duplicate-encryption consequence, and the Bot/Interop counterexample into one very long sentence. Split these into concise, one-idea sentences so readers can follow the two distinct failure modes and the page complies with the project's writing standard.
AGENTS.md reference: AGENTS.md:L25-L25
Useful? React with 👍 / 👎.
CodeRabbit review on #464: the note called identity_agent() "private" in one sentence, then said Jid::identity_agent() "is public" two sentences later, without distinguishing that these are two different items — a private module-level free function and a public method that wraps it. Clarify both are separate but related.
CodeRabbit and Codex both flagged the JID-equality note as running
multiple ideas per sentence, against this repo's documented style rule
("keep sentences concise — one idea per sentence", AGENTS.md). Break
the note into single-idea sentences without changing its content.
Codex review on PR #464 flagged the added explanation of why sort_dedup_by_device keys on identity_agent() as combining the key choice, equality rationale, duplicate-encryption consequence, and the Bot/Interop counterexample into one long sentence — against this repo's AGENTS.md style rule ('keep sentences concise — one idea per sentence'). Split it into single-idea sentences without changing the content.
1 participant
Summary
whatsapp-rust#1182 ("fix(jid): make agent identity, the AD form, and device dedup agree") changed how
Jid/JidRefequality,Hash, and device deduplication work. This PR updates the two docs pages that document that surface so they match the new behavior:advanced/binary-protocol.mdx— The note explaining why a wire-decoded JID equals a text-parsed one previously attributed this solely to the decoder invariant ("agentstays0").Jid/JidRefnow implementPartialEq/Hashby hand, routed through a privateidentity_agent()that structurally ignoresagenton any server where it isn't rendered — independent of whatever the raw field holds. Updated both the struct-level<Note>and theAD_JIDdecoding section to describe this as a structural guarantee rather than a decoder-only one, and to mention thatintegratoris not normalized the same way (folded into identity unconditionally, matchingis_same_chat_as) and thatJid::identity_agent()is now public.advanced/signal-protocol.mdx—sort_dedup_by_device's doc comment said it keys on "user + server + agent + device". It now keys onJid::identity_agent()(user + server + device + integrator + identity_agent) to matchJid's own equality rule. Added an explanation of why the old raw-agentkey was wrong in both directions: it could merge two devices that genuinely differ (@bot/@interopwith a rendered agent) or fail to collapse two JIDs that are actually the same device (an inert agent byte onPn/Lid/Hosted/HostedLid).Deliberately not included
Per instructions, changelog entries are human-authored only —
changelog/2026-06-10-phash-arena-sort.mdxstill references the pre-renamepush_ad_to/to_ad_stringfunction names (renamed topush_phash_form_to/to_phash_form_stringin #1182), but that file was left untouched.normalize_for_prekey_bundle, which #1182 removes entirely, was never documented in this repo, so there's nothing to update there.References
Generated by Claude Code
Summary by cubic
Update Binary and Signal protocol docs to match the new
Jid/JidRefidentity and device dedup rules fromwhatsapp-rust#1182. Clarifies hand-writtenPartialEq/Hash(privateidentity_agent(server, agent)helper and publicJid::identity_agent()), notesagentis ignored where not rendered andintegratoris folded, confirms parsed vs wire JIDs compare/hash equal even with a strayagent, switchessort_dedup_by_deviceto key onJid::identity_agent(), and splits the equality note into single-idea sentences per style.Written for commit 6645a25. Summary will update on new commits.
Summary by CodeRabbit