Skip to content

feat(read-state): server-owned read state for every client (buzz_v1) - #8639

Draft
baxen wants to merge 1 commit into
mainfrom
art/read-state-v1
Draft

baxen wants to merge 1 commit into
mainfrom
art/read-state-v1

Conversation

@baxen

@baxen baxen commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The relay becomes the source of truth for unread state. It keeps each person's read position per channel and thread, so web, desktop and mobile show the same dots and badges. Clients turn this on only when NIP-11 advertises buzz_v1, which stays behind the existing disabled-by-default buzz_v1_enabled flag. Relays without it keep NIP-RS.

This replaces #8264 and includes all of it.

API (/buzz/v1, NIP-98 auth, private to the caller):

  • GET /me/sidebar: unread, mention count, latest message and followed_threads for each joined channel, paged 20 channels at a time.
  • POST /me/read-state: mark-through intents, plus follow and unfollow. Batched.
  • POST /me/coverage: read or unread for specific message IDs. The Inbox uses it.
  • After a write commits, the relay publishes an ephemeral, relay-signed kind 24300 ping, p-tagged to the writer. It names the channels and threads that changed, so the writer's other devices refetch only those. Clients can't publish this kind.
  • Every response and ping carries a per-person read_revision (migration 0075). Clients use it to drop stale answers and to notice a missed ping.

Storage and ingest

  • Migration 0074 adds the arrival-order access paths: events.read_timeline and thread_metadata.received_at, each with a partial index, plus a mentions-per-channel index and follow columns on personal_read_frontiers. It does no backfill. On a large existing database, the four indexes must be prebuilt CONCURRENTLY first. The migration refuses to build them under write load and fails the deploy instead; see docs/buzz-v1-read-state.md § brownfield.
  • At ingest, each eligible message runs one bounded statement. It covers the author, mentioned members, the root's author and DM members. Nothing is written per channel member or per follower. Existing followers are touched at most once per refresh interval.
  • A 30-day unread floor applies. Mention walks start from the unfloored position, so old-author mentions still count.

Testing

  • Clippy is clean. Unit tests pass: 285 and 178. On PG16, scripts/postgres-test-run.sh passed 619/620 for buzz-db and 488/490 for buzz-relay, run at 678cbd397.
    • session_statement_timeouts_bound_writer_and_reader_sessions failed only under load and passed when rerun alone.
    • hanging_redis_peer_times_out_before_the_health_listener_binds also fails on unmodified main on this machine, so it's a timing problem in my environment.
    • The third failure is fixed below and passes on rerun.
    • The only change after that run is a one-line clippy borrow fix.
  • Pre-push hooks pass at 37d26fd69.
  • Fixed while cutting this: token_owner_refusals_are_the_fixed_nip_fi_denial_on_http (feat(relay): verify NIP-FI agent tokens and admit agents through owners #8401) expected an admitted agent to get 200 from /buzz/v1/me/sidebar. Read state refuses agents, which have no read state, so the test now expects a 403 with the read-state forbidden JSON. That still proves the request got past NIP-FI auth.
  • Live run on Oct 27 against a local relay at the code in this PR: Chromium and WebKit, the Tauri app and the iOS simulator on one identity. That covered cross-device read clearing, thread follow, unfollow and refollow, coverage, dropped pings and reconnects, offline replay, and a lost acknowledgement followed by a restart.

🤖 Generated with Claude Code

The relay keeps each person's read position per channel and thread, so
web, desktop and mobile show the same unread state. Clients read it with
GET /buzz/v1/me/sidebar, write with POST /me/read-state, and ask about
individual messages with POST /me/coverage. After a write commits, the
relay sends a relay-signed ephemeral ping (kind 24300, p-tagged to the
writer) to that person's other connections, which re-fetch only the
channels and threads it names.

- Arrival-order access paths (migration 0074): eligible timeline messages
  and thread replies are indexed by relay arrival time, plus mentions per
  channel; no backfill.
- Per-actor read_revision (migration 0075) on every response and ping, so
  clients drop older answers and detect a missed ping.
- Thread follows: authors, repliers and mentioned members follow at
  ingest (a root's author included); explicit follow/unfollow intents;
  automatic follows go dormant after the retention window. Sidebar rows
  list followed_threads (capped at 100).
- 30-day unread floor; mention walks start from the unfloored position.
- Coverage endpoint answers read/unread for specific message IDs.

Replaces #8264, which this includes.

Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz>
Co-authored-by: ART <ea39c1c8acf57180161ef4ba4ed68abf8589f5291956b909a5aa991fc66664ca@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Bradley Axen <baxen@squareup.com>

@TheSentinel454 TheSentinel454 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Review of #8639 at 37d26fd69dd24a5726619ecf29b32eadf0d73cad. These comments combine a DB-performance pass and a code review, each labeled as a recommended blocker, recommended, or optional.

The design fits docs/database.md: every unread count is computed when it is read, nothing is written per follower, and it adds no triggers, functions, foreign keys or backfill. One recommended blocker is the sticky-unfollow race in follows.rs. The main scale point is read-state GETs on the writer pool.

One more optional note: the PR body cites traffic numbers from an internal deployment, which may not belong in a public repo.

WHEN (excluded.through_message_id IS NOT NULL OR personal_read_frontiers.unfollowed)
AND excluded.through_timestamp > personal_read_frontiers.through_timestamp
THEN excluded.through_message_id ELSE personal_read_frontiers.through_message_id END,
unfollowed=false,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Recommended blocker: a concurrent ordinary reply can undo a sticky unfollow.

The snapshot filter at :174 keeps track of i.refollow. This ON CONFLICT ... DO UPDATE WHERE doesn't: under READ COMMITTED it re-checks the latest committed row, and personal_read_frontiers.unfollowed AND excluded.through_timestamp > personal_read_frontiers.through_timestamp is true for any later message.

Ordering that triggers it:

  1. A root author's (or DM member's) implicit follow is due for its daily refresh, or has no row yet.
  2. An unfollow intent updates the row but hasn't committed.
  3. An ordinary reply's ingest snapshot still sees the followed row (or no row) and selects it.
  4. The upsert waits, sees the committed unfollow, and since the reply's arrival minus 1 µs is later than the unfollow's now(), sets unfollowed=false.

So an acknowledged unfollow is reversed by a message that doesn't fall under either documented exception (posting in the thread, or a mention).

The window is narrow but real. Suggested fix: carry the re-follow permission into the conflict update and re-check it against the locked row. One way is to split the refollow rows from the refresh-only rows, so a refresh-only conflict never clears unfollowed.

Test: two transactions held at barriers, driven through the production ingest path. Assert that the unfollow survives an ordinary reply and that a later mention still re-follows.

('events', 'read_timeline', 'BOOLEAN NOT NULL DEFAULT false'),
('thread_metadata', 'received_at', 'TIMESTAMPTZ'),
('personal_read_frontiers', 'unfollowed',
$d$BOOLEAN NOT NULL DEFAULT false CHECK (NOT unfollowed OR root_id <> ''::bytea)$d$),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Optional: because of the inline CHECK, this ADD COLUMN scans all of personal_read_frontiers while holding ACCESS EXCLUSIVE. A constant-default column add is otherwise catalog-only.

On a local PG14 table with 3M rows, the plain add took 0.8 ms and the add with this CHECK took 358 ms. The scan also proves nothing: the new column is false on every existing row.

The blast radius is small today. The old relay doesn't write this table on ingest, the table only fills from v1 intents, and the migration's 5 s statement_timeout bounds it. The brownfield SQL in docs/buzz-v1-read-state.md has no statement timeout, though.

Per docs/database.md Rules 5 and 6, consider adding it as a separate ALTER TABLE ... ADD CONSTRAINT ... CHECK (...) NOT VALID, with VALIDATE later if wanted. Or drop it: follow() already rejects channel-only targets. Either way, the doc's "catalog-only" sentence needs updating.

r#"SELECT encode(e.id,'hex') AS id, e.channel_id,
CASE
WHEN e.pubkey=$2 THEN 'read'
WHEN e.read_timeline THEN

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Recommended: coverage ignores the thread side of a broadcast reply. A broadcast reply takes this read_timeline branch and never checks its thread frontier, but the sidebar also counts it on the thread row (docs/buzz-v1-read-state.md: "a broadcast reply also counts on its thread row").

Scenario: you follow a thread, receive a broadcast reply that mentions you, then mark only the channel through it. Coverage says read, while the sidebar still lists that reply and its mention on the thread. If Inbox uses coverage, the item disappears while the thread stays unread.

Either combine the two (unread in any counted scope), or document that coverage checks the timeline first. Add tests for timeline-read/thread-unread and the reverse.

let revision_range = last_revision.map(|last| (last - applied_count, last));
// After every commit and before the response, so a client that sees the
// response and then reconnects cannot miss a change it caused.
let ping_id = super::ping::send(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Recommended: the Redis ping has no deadline. The intents and the row refresh share the 8 s budget, but this best-effort ping is awaited after commit with none. publish_event awaits pool acquisition and PUBLISH, and the pool built in main.rs sets no timeouts.

A stalled Redis connection would hold up the HTTP response for intents that already committed, and delay the same-pod local delivery that follows.

Suggest a short explicit timeout around the publish, the same local-echo cleanup on timeout as on error, and then local fan-out and the response as usual. Test: a hanging Redis peer after a successful commit still returns the applied outcomes and delivers the local ping.

thread.newest = replies.newest;
thread.newest_at = replies.newest_at;
}
let mut conn = observability::acquire_writer(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Recommended before v1 is enabled at scale, not for this draft: every read-state GET runs on the writer pool with no concurrency limit of its own. That covers sidebar (:102), threads (here) and coverage (coverage.rs:65).

They share connections with ingest and auth, and each statement is allowed up to 2 s. A relay deploy reconnects every client, and each one loads its full sidebar. At the 100K–1M clients you mention, that can take every writer connection, so ingest fails before the database itself is busy.

Suggestions:

  • Cap concurrent v1 GETs and shed the overflow as 503 with Retry-After, which the contract already defines.
  • Jitter client reconnects.
  • Move GETs to the reader/replica. revision already lets a client drop a lagging reply, and POST returns its rows from the writer.

Separately, WriterOperation::SubscriptionHistory puts this load under subscription history in pool metrics. A dedicated operation label would make it visible.

it makes follow), whether or not the API is enabled, so follows exist once it
is; there are no stored unread counters.

The column additions are catalog-only but briefly take an exclusive lock on

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Optional: two deployment notes.

  1. With the inline CHECK on unfollowed, this column add isn't catalog-only. See the comment on 0074:34.
  2. docs/database.md puts index builds on a partitioned parent (events) outside the out-of-band envelope: they "need their own reviewed procedure". The per-partition procedure here looks correct to me. pg_get_indexdef shows ON ONLY for the parent, the parent becomes valid once every partition is attached, new monthly partitions inherit the index, and the partial indexes start empty. Deployments that apply schema out of band will still need it run as a separately reviewed operator step, and the PR description should note it under the review checklist's prebuild item.


`threads` lists the threads you follow that have unread replies, newest
arrival first (then `root_id`), at most 25. Threads past
the 25th are omitted with their mentions, and nothing signals the omission:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Optional, doc/contract drift:

  • threads_truncated is returned, so "nothing signals the omission" is no longer accurate.
  • An own broadcast reply advances both frontiers (asserted in follows_postgres_tests.rs:542), but the Following section says it reads only its thread.
  • The doc doesn't cover coverage, the read-state ping (relay signature, best-effort recovery), revision/range fields or followed_threads.

KIND_AGENT_TURN_METRIC,
KIND_MEMBER_ADDED_NOTIFICATION,
KIND_MEMBER_REMOVED_NOTIFICATION,
KIND_READ_STATE_CHANGED,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Optional: the new kind is registered in the privacy and provenance gates here, at :205 and in both ingest rejections, but no test exercises those contracts.

Suggested coverage through the real POST and local/Redis fan-out:

  • Only the actor in the same community receives the signed ping. Another actor, or the same key in another community, doesn't.
  • Kindless ID subscriptions are covered.
  • A client-authored ping is rejected over both WS and HTTP.

Removing any of these registrations should then fail a test.

.as_str();
assert!(through_message.contains("DROP COLUMN threads_through_timestamp"));
// 0074 adds the sticky thread unfollow and follow liveness.
let unfollowed_column = "unfollowed boolean not null default false \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Optional (docs/database.md Rule 4): these assertions match strings in the migration SQL. Rule 4 asks for a catalog-level parity assertion between desired state and migrations for touched tables and indexes ("A string match against schema.sql does not count"). events, thread_metadata, event_mentions and personal_read_frontiers aren't in admin_schema_parity_between_desired_state_and_migrations.

) -> Result<(StoredEvent, bool)> {
let result = insert_event_on(tx, event, channel_id).await?;
if result.1 {
if let Some((channel, place)) = crate::personal_read::place(event, channel_id, None) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Optional, so it's a deliberate decision: record_message runs on every eligible insert whether or not BUZZ_V1_ENABLED is set. Deploying this changes production ingest even with v1 off; the PR's local benchmark shows p99 going from 6.0 to 8.4 ms. Undoing it means a revert. That's reasonable for having follows ready when the API is enabled, but worth stating in the PR description.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants