docs(chat-store): document the session-wide arrival feed - #515
Conversation
Covers messages_by_arrival/messages_by_arrival_in_range and the new ArrivalCursor type: paging contract, why it orders by seq instead of timestamp_ms, and the watermark pitfall the PR's tests pin (a deleted message's seq can be reused by the next arrival).
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
📝 WalkthroughWalkthroughThe ChatStore documentation adds session-wide arrival-ordered message feeds with optional half-open timestamp filters. It defines ChangesArrival-ordered message feeds
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
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 |
![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 `@api/chat-store.mdx`:
- Around line 226-228: Update the new documentation sections around
messages_by_arrival and the related sections to address the reader directly in
second person and use active voice. Format all API and type references,
including messages_by_arrival, messages, MessageCursor, and ArrivalCursor, as
inline code; apply the same style to the additional referenced sections.
- Line 241: Qualify the `seq` reset statement in the saved-watermark
explanation: say it restarts at 1 only when clearing the chat empties the entire
table and the schema permits SQLite rowid reuse; otherwise, acknowledge that
rows from other chats can keep the sequence from resetting.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7cadfd25-0d84-4159-9e82-6d5f457edf1b
📒 Files selected for processing (1)
api/chat-store.mdx
CodeRabbit correctly caught that clear-chat/delete-for-me delete rows scoped to one chat, not the whole per-device messages table (verified against ClearChatUpdate's delete_chat_rows call in store.rs) — so "restarts at 1" only holds when that chat happened to hold every remaining row. The general mechanism (freeing the table's max rowid for reuse) doesn't depend on that special case, so it's what the watermark pitfall actually rests on.
![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: dc9e0e6390
ℹ️ 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".
| } | ||
| ``` | ||
|
|
||
| Stopping at a saved watermark instead skips messages, silently. SQLite hands out the rowid backing `seq` as `max(rowid) + 1`, so deleting the newest message gives its number to the next arrival, and clearing a chat entirely restarts at 1 — both routine, since delete-for-me and clear-chat go through this store. A message that lands at or below a remembered `seq` after either reads as already seen and never surfaces again under a watermark comparison. |
There was a problem hiding this comment.
When any other chat still contains a message, clearing this chat does not restart seq at 1: SQLite chooses one more than the maximum rowid across the entire messages table. The reset occurs only when the operation empties that table, so qualify this as clearing the only remaining chat (or all messages) rather than presenting every routine clear-chat as a reset.
Useful? React with 👍 / 👎.
| ) -> Result<Vec<StoredMessage>>; | ||
| ``` | ||
|
|
||
| The session-wide feed: every chat interleaved, newest arrival first. This is the read a reconciliation consumer wants — "everything that landed since I last looked, across all chats" — which `chats` plus `messages` can otherwise only answer by paging every thread. `messages_by_arrival` is `messages_by_arrival_in_range` with no bounds; the ranged form additionally restricts the feed to a half-open wall-clock window, `since <= timestamp < until`, so adjacent windows tile without double-counting or dropping a row. Either bound may be `None`. A `limit` of zero, or negative (which SQLite reads as unbounded), returns nothing rather than the whole table. |
There was a problem hiding this comment.
The new overview describes what “a reconciliation consumer” wants instead of addressing the reader. Rewrite this as direct guidance such as “Use this read when you want…” so the section follows the repository’s required second-person voice.
AGENTS.md reference: AGENTS.md:L24-L24
Useful? React with 👍 / 👎.
Both verified against storages/chat-store/src:
- The wall-clock window's tiling guarantee ("adjacent windows tile
without gaps/dupes") only holds for a single scan. store.rs's ack
handler rewrites an outgoing row's timestamp_ms in place after insert,
independent of seq, so a row can cross a window boundary between two
queries taken at different times.
- The dedup loop's (chat_jid, id) comparison assumes chat_jid is stable.
lid.rs's merge_split_chat does `UPDATE OR IGNORE messages SET chat_jid
= ?` when healing a PN/LID split — its own comment notes rowids (seq)
survive the update — so a message can reappear under a different
chat_jid after a merge and get misread as new.
Also rephrased the feed's opening sentence to second person per this
repo's AGENTS.md style guideline, which Codex correctly cited.
2 participants
Summary
Documents the new session-wide arrival-ordered message API added in oxidezap/whatsapp-rust#1284:
ChatStore::messages_by_arrival/messages_by_arrival_in_range— a keyset-paginated feed over every chat interleaved by arrival order, for a reconciliation consumer that wants "everything that landed since I last looked" without an N+1 walk of the chat list.ArrivalCursortype, and why it's a distinct type fromMessageCursor(the two feeds sort by different keys).Changes to
api/chat-store.mdxmessages_by_arrival/messages_by_arrival_in_range: paging contract (re-enter at the head, compare by(chat_jid, id), never stop at a rememberedseq), why it orders by arrival instead oftimestamp_ms(history-sync backfill lands old messages at new arrival positions), the tombstone/mutation boundary withsubscribe(), and the wall-clock window's scan-cost caveat.ArrivalCursorentry under Types, next toMessageCursor.Straight documentation of the new public API surface; no other behavior changed in this PR.
Generated by Claude Code
Summary by cubic
Documents the session-wide, arrival-ordered message feed and
ArrivalCursorinchat-store, with clear paging and ordering rules. Shows how to pull everything that landed across all chats without N+1 per-thread paging.New Features
messages_by_arrival/messages_by_arrival_in_range: newest-first, keyset-paginated byArrivalCursor.seq(nottimestamp_ms); ranged windows are half-open; compare by(chat_jid, id);limit <= 0returns nothing.Caveats
seq(rowid) after per-chat deletes (delete-for-me/clear-chat), so watermarking byseqcan skip new arrivals.timestamp_mscan be corrected after insert and cross a window boundary.chat_jidis not stable across PN/LID merges; normalize before(chat_jid, id)deduping to avoid misreading a rekeyed row as new.Written for commit 742b1bd. Summary will update on new commits.