GUI read-model overhaul: server-assembled channel windows (Correct™ pagination + relay-signed bounds)

[tlongwell-block]

GUI Read-Model Overhaul: server-assembled channel windows

Problem

Messages that reliably appear via API/CLI were missing from GUI channels, and channel/thread opens took multiple seconds. Four stacked patches (#1473, #1478, #1483, #1486) each fixed a real bug and each revealed the next. The root disease: the timeline was a client-assembled soup of raw events paginated by author-controlled created_at — threads derived from the channel cache, ancestors spliced in by id, aux events backfilled by #e fan-out, and a "page until N visible rows" loop on top. Every merge path was a correctness hazard; every channel open was 3–10 round trips.

Deterministic reproducer (ancestor-island): a cached reply referencing an old root outside the contiguous window splices that root into the channel cache; scrollback then anchors its cursor on the island and pages backward from it, permanently stranding every row between the island and the real frontier. CLI sees them; the GUI never does. (parity-ancestor-island.spec.ts: 0/100 gap rows reachable on main after exhausted scroll.)

Approach

Paginate the channel by top-level rows, not raw events; the server assembles the page, the client renders it. No new endpoints — a /query filter extension with nostr semantics everywhere (normative spec: docs/nips/NIP-CW.md; docs/bridge-channel-window.md remains as the ratified contract record).

• Filter extension: top_level: true (+ include_summaries, include_aux) on POST /query
• Composite (created_at DESC, id ASC) keyset cursor — until + before_id, both or neither (400 on half); no timestamp-only fallback, killing the dense-second dup/loss class
• Relay-signed overlays, synthesized at query time, never stored, ingest-rejected from clients:
  • kind:39005 thread-summary per replied row (reply_count, participants — batched in one window-wide query, no per-root fan-out)
  • kind:39006 window-bounds, exactly one per response: {has_more, next_cursor} — the sole exhaustion authority; clients never infer "done" from row counts (limit+1 probe server-side; exact-multiple final pages lie)
• Aux closure: two-hop (reactions/edits/deletions targeting rows, then deletions targeting aux), doesn't consume the row budget
• Client: single get_channel_window command per page; cursor fed back verbatim from 39006; live updates via since: now subscription

Evidence

RED→GREEN flip, same fixture byte-identical in both states:

State	Tree	Result
main	relay + client @ main	0/100 gap rows reachable (39s, exhausted scroll)
this PR	relay + client from this branch	100/100 (18.4s, zero stranded rows)

The flip is CI-enforced: parity-ancestor-island.spec.ts runs under Desktop E2E Integration against the live relay (green in 20.3s on the tip — a real seed+scroll+assert execution, not a fast bail).

Full CI green on the final tree (run 28677545063; the tip SHA was subsequently amended for commit-trailer compliance only — tree byte-identical, git rev-parse HEAD^{tree} unchanged).

Suites on the assembled tip dfb1cd0: buzz-relay 462/462 unit, buzz-db 57/57 Postgres-backed (incl. dense-second tie paging, exact-multiple exhaustion, top-level predicate, summary join), live-wire interop e2e 25/25, desktop 1564/1564, tauri 873/873, mobile 395/1 skipped, focused window tests 17/17.

Formal verification

The frozen contract (v3, docs/bridge-channel-window.md, reviewer header is the durable record) carries a TLA+ invariant set (GuiRelaySync.tla). Two distinct sign-offs: the contract was ratified pre-implementation; a full theorem map then discharged every invariant against the assembled tip dfb1cd06, and was re-verified against the final tip after integration (spec-guardian sign-off; invariant-load-bearing files bit-identical between the two, additions strictly strengthen the discharge chain — see below) — Inv_NoPhantomEvents (relay-only overlay kinds rejected at ingest), Inv_ResumeIntervalComplete (limit+1 probe before kind filtering), Inv_NoSilentGap (signed, request-bound, atomic 39006), Inv_CursorSound (live overlay never mutates frozen pages), Inv_QuiescentConvergence (page-0 refetch on reconnect), Inv_CausalView (two-hop aux closure), and response atomicity. Known non-blocking gap: no retention floor on 39006 yet; oldest_retained can be added to its content JSON wire-break-free.

Landed after assembly (all reviewed + re-verified)

• NIP-CW (docs/nips/NIP-CW.md): the frozen contract promoted to a normative NIP.
• Thread read-model consumption (fix(desktop): consume relay thread read models): the thread panel consumes relay 39005 summaries via mergeThreadSummaries — a monotone merge (Math.max on counts/lastReplyAt, participant union capped slice(-3) favoring local knowledge). 39005 counts are a floor, not sole authority; deliberate, since the local reply cache can lead the relay summary. Also repaired a family of pre-existing scroll-anchor/deep-backfill spec failures present on the pre-assembly base.
• Reconnect self-heal paging (fix(desktop): self-heal older-history paging after reconnect): hasOlderMessages was a private latch reset only on channel change — after a reconnect refreshed the window, an exhausted pre-reconnect latch kept the scroll observer uninstalled and froze paging at page one (the founding complaint's second mechanism). Now derived reactively from the store tail's hasMore — the same signed 39006.has_more the parser reads, so there is no parallel derivation to drift. Closes the user-visible half of Inv_QuiescentConvergence; relay-reconnect.spec.ts goes green with no test edit.
• Parity spec defaults to the canonical :3000 relay (CI integration); BUZZ_E2E_RELAY_URL still overrides for isolated stacks.

Notes for reviewers

• The e2e bridge (desktop/src/testing/e2eBridge.ts) hand-mirrors the channel_window.rs Tauri command including mock-mode 39006 synthesis — a deliberate contract twin; a future contract change must touch both (flagged in commit bodies).
• next_cursor derives from the last raw retained row before event reconstruction: a page whose tail row fails reconstruction still advances the scan (skip-and-continue). Client accordingly never compares 39006 cursor to reconstructed rows.
• Includes a standalone fix for a pre-existing red on main: fresh-DB migration test expected [1,2] applied versions after feat: per-community workspace icon set by admins, served via NIP-11 #1463 added migration 0003.
• Supersedes the patch stack fix(desktop): stop timeline cap from evicting scrolled-in history mid-read #1473/fix(desktop): defer timeline trim past StrictMode probe #1478/fix(desktop): stop out-of-band ancestor merges from poisoning the scrollback cursor #1483/fix(desktop): hide non-contiguous islands until scrollback heals them #1486 line of work.

Branches merged (integration branch: overhaul/gui-read-model; assembled at dfb1cd0, final tip carries the post-assembly commits above)

• overhaul/relay-window-api @ e30a301 (relay: buzz-db window query + bridge dispatch + interop e2e)
• overhaul/client-core @ 6d6eed5 (client read model + cursor trust + e2e bridge handler + mock window contract; assembled tip dfb1cd0)
• overhaul/test-harness @ 2edc527 (ancestor-island parity gate, nonce-stable seeding, bridge URL threading)

Named follow-ups (out of scope here)

• parity-dense-second.spec.ts live-relay twin (the mock dense-second parity test is in; main's fix(read-path): reach complete threads, dense-second timelines, and all people in the GUI #1418 keyset fallback already drains the tied second, so the live twin is additive coverage, not a RED).
• Relay-ordered thread participants (current merge favors locally known, slice(-3)).
• 39006 retention floor (oldest_retained), wire-break-free addition.

[wesbillman]

I agree with the server-assembled, top-level-row window and composite cursor direction, but the desktop integration is not merge-safe yet. I found four correctness blockers:

1. Relay summaries are dead data, so channel thread affordances regress. parseChannelWindowResponse stores each 39005 in ChannelWindowPage.rows[].thread, and channelWindowThreadSummaries() can expose them, but that helper has no callers. flattenChannelWindowEvents() deliberately emits only raw rows/aux, and replies were removed from the channel cache. formatTimelineMessages / buildMainTimelineEntries therefore cannot derive the existing MessageThreadSummaryRow for top-level messages. Reply counts/participants/last-reply UI disappear even though the relay computes them. The unread/thread badge derivations in useChannelUnreadState likewise still consume only the reply-free formatted timeline.

2. Live broadcast replies disappear from the channel timeline. In useChannelSubscription.appendMessage, every content event with a parent returns through the thread-only branch before checking the ["broadcast", "1"] marker. But the new window contract explicitly defines depth-1 broadcast replies as top-level rows. A broadcast reply arriving live is written only to threadRepliesKey; it does not enter liveOverlay and remains absent from the timeline until a later page-zero refetch.

3. A partial live reply cache suppresses the authoritative thread fetch. The same live branch calls setQueryData(threadRepliesKey(...)) even when that thread query has never run. React Query now considers that newly created entry fresh for useThreadReplies' 60-second staleTime. Opening the thread during that interval can render only the live reply(s), without running the full paged getThreadReplies query. Live delivery must not masquerade as a complete/fresh subtree (track completeness separately, invalidate, or ensure opening always fetches authoritative data).

4. Retained live overlays eventually collide with authoritative older pages. replaceNewestChannelWindow removes a live row only when its ID appears in the refreshed page zero. If enough newer rows push that event below page zero, it remains in liveOverlay. Paging down later returns the same event authoritatively, and appendOlderChannelWindow throws on the overlap. Reconcile overlays by the authoritative interval/cursor chain (or remove matching overlays as every page is appended), with a regression covering: initial page → live row → enough new rows to push it out of refreshed page zero → paginate until that row.

There is also a weaker but real continuity concern: subscribeToChannelLive uses since: Date.now() only after the independent page-zero request. That creates a request/subscription handoff gap and excludes newly ingested backdated events (the exact class the PR says cannot be trusted by author timestamp). The old overlapping subscription avoided this. Establish the subscription boundary before/atomically with the snapshot, or overlap and dedupe.

The architecture is promising, and the composite keyset/server-owned exhaustion model is substantially better than the current client-derived cursor. But these are user-visible missing/incomplete-thread and stalled-pagination failures, so I recommend fixing them and adding focused client regressions before merge.

[wesbillman]

Relay-side review (Brain) — server half of the split review with Pinky

Verdict on the relay/server side: sound. I read the full relay/db diff, then verified against a fresh isolated stack (clean Postgres 15 + Redis + this branch's relay): all 4 new Postgres-gated channel_window tests pass, both new interop e2e window tests pass (test_channel_window_rows_overlays_and_exact_multiple_exhaustion, test_channel_window_rejects_half_cursor_and_client_overlay_kinds), and the full buzz-core/buzz-relay/buzz-db lib suites are green (193/462/79). CI's Backend Integration (relay e2e) is also green — the red checks are all desktop-side, consistent with the client blockers already filed.

Verified invariants (code + live):

• Keyset predicate created_at < $ts OR (created_at = $ts AND id > $id) matches the (created_at DESC, id ASC) total order, and is index-backed by idx_events_community_channel_created. bytea > is bytewise, as the spec requires.
• limit + 1 probe runs after all predicates (access, deleted_at, top-level, kinds) — same SQL statement, so no false has_more.
• next_cursor is the scan position captured before reconstruction filtering; next_cursor = Some ⇔ has_more holds by construction.
• 39005/39006 are in is_relay_only_kind, so client submission is rejected at ingest (e2e-pinned).
• Inaccessible/nonexistent channel → silent empty result, no rows, no bounds — no existence oracle, matching the spec's access-scoping section.
• Aux closure two-hop logic is correct (hop 2 keys off hop-1 event ids via mem::take, dedup by id, access check per event).
• The handled set means a window filter is never double-served by the feed/thread/catchall paths.
• The migration-count test change (vec![1,2] → vec![1,2,3]) is a legitimate fix: 0003_community_icon.sql already exists on main and main's assertion is stale — I reproduced the fresh-DB migration run and it applies 3 versions.

Two minor, non-blocking relay findings:

1. Spec-conformance gap on malformed before_id. NIP-CW §Request says a malformed cursor value "MUST cause rejection — it MUST NOT be ignored and demoted to a half cursor or a head request." But extract_before_id maps malformed → None, so top_level: true with a malformed before_id and no until becomes (None, None) — a silent head request, exactly the demotion the spec forbids. (Malformed before_id with until is fine: it becomes a half cursor and 400s.) One-line fix: on the window path, distinguish "key absent" from "key present but malformed" and 400 the latter. Impact is near-zero for the shipped client (it only echoes relay-issued cursors), but the doc is normative and says MUST.
2. Multiple window filters in one request each append rows + one 39006 into the same flat array. The d-tag request binding disambiguates the bounds overlays, and rows carry h tags, so cross-channel batching partitions cleanly — but two windows of the same channel at different cursors would have indistinguishable row sets. The shipped client sends one filter per request, so this is a doc footnote, not a defect.

I defer to Pinky's changes-requested review for the merge gate — his four desktop integration blockers are real and the client half is where the holes are. The relay contract itself (window query, cursor semantics, overlay synthesis, ingest rejection, exhaustion authority) is correct and well-tested, and the NIP-CW/bridge contract doc accurately describes what the relay actually does, modulo finding 1.

[tlongwell-block]

Thanks for the split review — the four desktop blockers and both relay findings were all real. Everything is addressed on the branch; head is 36b91e50. Response keyed to your numbering, with the regression evidence for each.

Desktop blockers (Pinky)

1. Relay 39005 summaries were dead data — fixed in 41901aef (fix(desktop): consume relay thread read models). channelWindowThreadSummaries() is now consumed at ChannelScreen.tsx:448; summaries flow through buildMainTimelineEntries/threadPanel.ts into the thread affordance rows, and useChannelUnreadState picks them up. Covered by new cases in threadPanel.test.mjs.

2. Live broadcast replies dropped from the timeline — fixed in 9a533a9e (merged via f448b7b6). appendMessage keeps dual routing deliberately: a parented event always enters the thread cache, and a ["broadcast","1"] reply now continues into the window-store live overlay instead of returning early — broadcast replies belong in both places per NIP-CW §Top-level.

One honest refinement to your framing, found while building the regression: at the reviewed tip the rendered timeline was masked by a second, unfiltered live subscriber (useLiveChannelUpdates writes every channel event into channel-messages), so the user-visible row was usually present — but only as an orphan cache write that any window-store rebuild erased. The durable bug was that the broadcast reply never reached the authoritative window store. The new e2e regression asserts exactly that invariant (live-broadcast-reply-timeline.spec.ts): RED at the reviewed tip, GREEN now. It includes a non-broadcast depth-1 control so a "merge everything" fix can't false-green it.

3. Live-written thread cache masquerading as fresh — fixed in 41901aef. useThreadReplies now has staleTime: 0 (useThreadReplies.ts:41), so opening a thread always runs the authoritative paged fetch; live writes can't suppress it.

4. Overlay/older-page collision throw — fixed in 9a533a9e. appendOlderChannelWindow now removes overlay entries whose IDs arrive in the appended authoritative page, so reconciliation happens on every page append, not just page-zero refresh. Two regressions in channelWindowStore.test.mjs: your exact scenario (live row → newer rows push it below page zero → paginate to it → reconcile, not throw), plus a sharper content-divergence case asserting the authoritative relay copy wins when the overlay holds a stale/optimistic version of the same ID. Both RED at the reviewed tip, GREEN now.

Subscription handoff gap (since: now after page-zero) — fixed in 9a533a9e. Order inverted: subscribe first, then once the live REQ reaches EOSE/readiness, page zero is invalidated and refetched. The live overlay covers the fetch interval and the authoritative response reconciles the overlap — no window between fetch and subscription.

Relay findings (Brain)

1. Malformed before_id silently demoted to a head request — fixed in 1b03128d. extract_before_id is now a three-state BeforeId enum (Absent / Valid / Malformed); Malformed → 400 on the window path and the general query path (same grammar; no reason to keep a silent-ignore anywhere). Regression added to test_channel_window_rejects_half_cursor_and_client_overlay_kinds: confirmed 200 (the bug) against a relay running the pre-fix binary, 400 after — plus the unit tests rewritten for the enum.

2. Same-channel multi-window ambiguity — agreed it's a doc footnote, not a defect. Queued as a NIP-CW follow-up alongside the other non-blocking notes (39006 oldest_retained retention floor, relay-ordered participants) rather than growing this PR.

A regression we introduced and fixed while responding (full disclosure)

The blocker-2 fix's post-EOSE page-zero refetch (9a533a9e, the subscription-handoff inversion) introduced a real regression that CI caught at the assembled tip: thread-unread.spec.ts went 13/13 → 5-6 failing. Root cause (bisected in code, deterministic): refreshNewestWindow() rebuilt channel-messages wholesale from the window store, but get_channel_window returns top-level rows + aux + 39005 summaries only — so live thread replies already in the cache were dropped by the refetch, killing the unread-badge derivation. Same mechanism also clobbered pending optimistic rows (a DM-open refetch raced the in-flight send).

Fixed in 36b91e50 (23 lines, hooks.ts only): the refetch now reconciles instead of clobbering — the flattened authoritative window is merged with retained prior rows, where retention is limited to pending optimistic rows and non-broadcast thread replies, and on any id collision the authoritative relay copy wins (retained rows are filtered out when the refetch returned the same id — so this cannot reintroduce the blocker-4 stale-shadow bug; broadcast depth-1 rows are excluded from retention entirely because they come from the authoritative window). Invariant: a live thread reply or pending optimistic row present before a page-zero refetch survives it; anything the relay returned authoritatively replaces what was retained. Verified independently by three of us at the identical tree, all --retries=0 fresh builds: thread-unread 13/13, the DM optimistic-row spec (mentions.spec.ts:1425) green 4/4 consecutive runs, both blocker regressions and channelWindowStore 14/14 still green.

Verification at 36b91e50 (assembled tip, not per-lane)

• CI green on the head SHA (both workflows)
• cargo test -p buzz-relay: 463 green; interop e2e 25/25 against a live relay
• Desktop: typecheck green, 1568/1568 unit (14/14 channelWindowStore), pnpm check green
• Playwright integration 108/108 against a fresh relay built at this tip (includes parity-ancestor-island); smoke suite green in CI at --retries=0-honest local runs for every spec the review touched
• The RED→GREEN matrix for blockers 2/4 was independently re-verified at the merged tip in a separate worktree, and the mock bridge's top-level predicate was corrected to mirror the relay's broadcast depth-1 classification (5d2482e2) — the harness had a latent contract divergence on exactly the event class blocker 2 concerns; the parity fix is isolated in its own commit so the specs' RED-at-tip stands independently.

Re-requesting review.

[tlongwell-block]

Head note: the branch was force-pushed 36b91e50 → eecc7784 to satisfy the DCO gate — three merge commits were missing sign-off trailers. Message-only rewrite via commit-tree: every tree is byte-identical (final tree 6884e907 unchanged, git diff between old and new head is empty), so all verification in the comment above applies to eecc7784 as-is. CI + DCO both green on the new head.