docs: reflect active chat lane eviction fix - #410
Conversation
Document that per-chat lanes now survive capacity eviction while a message is in flight (evict_guard + QueuedChatMessage.lane_liveness), and note that this closes the specific chat-lane-eviction trigger previously described for the sender-key chain lock section.
📝 WalkthroughWalkthroughThe architecture documentation now describes unbounded per-chat channels, bounded lane-cache backpressure with a 5,000-lane default, and active-lane eviction protection. It also documents the guard’s relationship to sender-key chain locking. ChangesPer-Chat Lane Design
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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. |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: a5b3c8263e
ℹ️ 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(crate) chat_lanes: Cache<Jid, ChatLane>, | ||
| // ChatLane { enqueue_lock, queue_tx } | ||
| // Each queue: async_channel::bounded::<Arc<OwnedNodeRef>>(500) | ||
| // Each queue: async_channel::bounded::<QueuedChatMessage>(500) |
There was a problem hiding this comment.
In the referenced #1045 diff, create_chat_lane constructs this channel with async_channel::unbounded::<QueuedChatMessage>(), so the updated snippet still advertises a 500-message bounded queue and per-chat backpressure that do not exist. In backlog-heavy chats this misleads readers into expecting TrySendError::Full/cancelled acks and a per-lane memory cap, while the actual capacity limit only applies to the lane cache; please align this snippet and the surrounding capacity sentence with the Rust code.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — confirmed against handlers/message.rs in whatsapp-rust: the channel is async_channel::unbounded, and the real capacity limit (chat_lanes_capacity, default 5,000 per cache_config.rs) bounds the number of cached lanes, not messages per lane. Fixed in 4e6c7df.
Generated by Claude Code
The queue element type comment was updated in the previous commit but still claimed a bounded(500) channel with per-lane backpressure. The actual code (handlers/message.rs in whatsapp-rust) uses async_channel::unbounded. The real capacity limit is on the number of cached lanes (chat_lanes_capacity, default 5,000 per cache_config.rs), not on messages per lane. Addresses review feedback from @chatgpt-codex-connector on #410.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@concepts/architecture.mdx`:
- Line 536: Update the chat_lanes capacity explanation in the architecture
documentation to state that protected active lanes may temporarily exceed
chat_lanes_capacity, while only evictable idle lanes are removed. Also revise
the corresponding api/bot.mdx configuration description to define
chat_lanes_capacity as the cached-lane capacity, not combined
enqueue-lock/message-queue capacity.
🪄 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: 02a488bf-8fcb-4402-9d91-9ffb01b5d58c
📒 Files selected for processing (1)
concepts/architecture.mdx
Addresses CodeRabbit review feedback on #410: this file's description of chat_lanes_capacity conflicted with concepts/architecture.mdx's updated wording. Aligns both to describe it as the number of cached per-chat lanes (each pairing an enqueue lock with an unbounded queue), and notes it's a soft cap like the other coordination caches — an active lane is never evicted, so the map can briefly exceed capacity when every cached lane is in use.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Addresses CodeRabbit review feedback on #410: a lane with an in-flight message is protected from eviction, so the chat_lanes cache can briefly exceed chat_lanes_capacity when every cached lane happens to be active at once, rather than evicting a live lane. Mirrors the phrasing already used for session_locks_capacity/group_distribution_locks_capacity.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
1 participant
Summary
Updates
concepts/architecture.mdxto reflect whatsapp-rust#1045, "fix(message): preserve active chat lanes."Arc<OwnedNodeRef>toQueuedChatMessage { node, lane_liveness }. Added a note explaining that each queued message now carries a clone of the lane'senqueue_lock, and the cache'sevict_guardrefuses to evict a lane while any message is still in flight — closing a race where a lane could be capacity-evicted right after its worker dequeued a message, letting a later stanza for the same chat spawn a second worker.No other docs changes — this PR fixes a correctness bug in internal message-dispatch state and doesn't change any public API or CHANGELOG content.
Test plan
src/client.rs,src/client/lifecycle.rs,src/handlers/message.rs,src/client/tests.rs)Generated by Claude Code
Summary by cubic
Updates
concepts/architecture.mdxandapi/bot.mdxto document the v0.6 eviction guard that preserves active per‑chat lanes while a message is in flight, and corrects that per‑chat queues useasync_channel::unbounded.Aligns both pages to state
chat_lanes_capacityis a soft cap on the number of cached lanes (enqueue lock + unbounded queue), explainsQueuedChatMessage { node, lane_liveness }powering the guard, and notes the sender‑key chain lock’s prior lane‑eviction trigger is now closed (lock remains defense‑in‑depth).Written for commit b48d555. Summary will update on new commits.
Summary by CodeRabbit