fix(desktop): restore recent channel sorting

[wesbillman]

Summary

• batch desktop last-message queries into groups of 128 so workspaces with larger channel counts stay within the relay's explicit-channel limit
• propagate timestamp query failures instead of replacing the sidebar's cached recency with all-null data
• advance channel recency from live message events so Recent ordering updates without waiting for the next refresh

Root cause

The desktop sent one explicit #h filter per channel in a single /query. The relay rejects a request with more than 128 aggregate explicit channel values. The desktop swallowed that rejection and returned no timestamps, so Recent correctly fell back to A–Z for every channel.

Testing

• cargo test --manifest-path desktop/src-tauri/Cargo.toml last_message_filters_stay_within_relay_channel_cap --lib
• cd desktop && pnpm test (5,132 passed)
• cd desktop && pnpm exec biome check src/features/channels/useLiveChannelUpdates.ts src/features/channels/lib/channelRecency.ts src/features/channels/lib/channelRecency.test.mjs tests/e2e/channel-sort.spec.ts
• cd desktop && pnpm check:file-sizes
• cargo fmt --all -- --check
• cd desktop && pnpm build:e2e followed by pnpm exec playwright test tests/e2e/channel-sort.spec.ts --project=smoke --workers=1 against a dedicated static server (4 passed)

Manual test

In a workspace with more than 128 channels, choose Channels → Sort → Recent. Channels should order by latest message instead of A–Z. While Recent is selected, a new message in a visible channel should move that channel to the top immediately.

[wesbillman]

[jedwards27]

Verdict: REQUEST CHANGES
Reviewed: 2e7583bf5ad5926ca32367af9954bc79d108e42d..c8d980d1dc9ce09fa1562977a5d94bf5c4674fdb (exact head c8d980d1dc9ce09fa1562977a5d94bf5c4674fdb)
Risk: high — this changes live message recency across renderer cache updates and batched relay queries.

Blocking: an in-flight refresh can roll back a newer live recency update

updateChannelLastMessageAt correctly rejects an older individual live event (desktop/src/features/channels/lib/channelRecency.ts:40-49), but refresh settlement does not preserve that invariant. applyLastMessages replaces each current value unconditionally (desktop/src/features/channels/hooks.ts:283-293), including when the refresh settles through hooks.ts:394-417 after a live update.

The reachable ordering is:

1. get_channels reads relay timestamp T1;
2. the live listener receives T2 and advances the displayed channel (desktop/src/features/channels/useLiveChannelUpdates.ts:248-255);
3. the older refresh settles and replaces T2 with T1.

An exact-head executable repro through the shipped helpers observed T2=2026-01-01T00:02:00.000Z followed by stale settlement T1=2026-01-01T00:01:00.000Z. Above 128 channels this window grows because native timestamp batches execute sequentially (desktop/src-tauri/src/commands/channels.rs:159-166). The visible channel can therefore move to the top and then fall backward until the next 60-second refresh. The new E2E emits only after initial loading (desktop/e2e/channel-sort.spec.ts:149-159), so it does not cover this overlap.

Please merge fetched timestamps monotonically with the displayed cache at settlement while preserving the intended authoritative absence/null behavior, and add a deterministic causal regression: start refresh with T1, apply live T2, complete refresh, assert displayed recency remains T2. Exercise both the not-modified (payload.channels === null) and full-list settlement paths.

Contracts traced

The relay’s aggregate explicit-#h cap is 128 (crates/buzz-relay/src/handlers/req.rs:36-42,1099-1114); the candidate correctly chunks 257 filters as [128,128,1]. Batch failures propagate before frontend cache mutation (desktop/src-tauri/src/commands/channels.rs:159-167,352-362), preserving cached recency. Community query-client isolation and live-listener disposal remain intact. No additional material finding was found in those reviewed paths.

Validation at exact clean head

• cd desktop && pnpm test: 5,132/5,132 passed.
• Focused Tauri batching regression: 1 passed; focused channelRecency + hooks suites: 15 passed.
• pnpm build:e2e plus channel-sort.spec.ts --project=smoke --workers=1: 4/4 passed; causal mutation removing the live-recency call made test 02 fail as expected, then the clean tree was restored.
• A local just desktop-ci run completed renderer checks, all 5,132 TS tests, formatting, and renderer build, but timed out at 600 seconds while cold-compiling desktop-tauri-check; this is not claimed as a full local pass.
• GitHub checks at this exact head were successful for Desktop Core, all four smoke shards, integration shards, Windows Rust, macOS Desktop Build, Rust Lint, Desktop Release Candidate, and DCO.

Manual/native evidence: screenshots from the browser smoke journey showed coherent menu semantics and normal Recent ordering. No real native GUI/harness run was performed, so browser mocks do not establish native behavior at 128/129 channels or native failure states.

Residual risk: actual multi-request native behavior and partial-failure UI behavior above the relay cap remain unexercised. The reproduced stale-settlement race is independently sufficient to block this head.

— :bot: Jude’s code review agent

[wesbillman]

Addressed the blocking stale-settlement race at 449950a88.

The review was correct: a live T2 update could land while get_channels held older T1, then refresh settlement could overwrite T2. Refresh settlement now merges recency monotonically against both the displayed cache and the request-start snapshot. This covers full-list, hashless retry, and not-modified response paths while preserving authoritative-null behavior when no concurrent update occurred.

Added causal regression coverage for T1 → live T2 → stale settlement T1 and the null/newer-refresh boundary cases. Validation: Desktop unit suite 5,145/5,145, focused recency/hooks 17/17, Playwright channel-sort 4/4, targeted Rust batching 1/1, Biome, typecheck, file-size gate, E2E build, and Rust fmt all pass locally.

[jedwards27]

:bot: Jude’s code review agent — REQUEST CHANGES

Reviewed: 3c228b1082a93aca302c7b6a67ec274c51ed5eaf..449950a88f9234491d4c23a79e77a8275168e44e
Risk: high — this changes live-message/refresh concurrency at the renderer↔Tauri relay boundary.

The implementation now appears to close the stale-refresh race: it records displayed recency at request start, merges against display state at settlement, preserves a concurrent newer timestamp, and still permits authoritative null to clear unchanged stale recency (desktop/src/features/channels/hooks.ts:364-438, desktop/src/features/channels/lib/channelRecencyMerge.ts:12-38). I found no second implementation defect; 128-channel batching, fail-fast timestamp-query propagation/cache preservation, and live Recent reordering remain sound.

Blocking: the regression test does not exercise the production settlement paths

desktop/src/features/channels/lib/channelRecency.test.mjs:66-77 calls mergeConcurrentChannelRecency directly. Its responseShape loop changes only the assertion label; it never drives either production call site at hooks.ts:392-396 or hooks.ts:429-433. We replaced both call sites with identity behavior returning the refreshed value—the integration-seam mutation that restores the stale-settlement defect—and the entire Desktop suite still passed 5,142/5,142. The helper test is causal only at the helper boundary: disabling its merge produced 2/5 focused failures.

For a high-risk async-ordering fix, that leaves the shipped wiring unguarded. Add a deterministic query-level regression that defers getChannels, injects live T2 into the React Query cache after request-start T0, settles stale T1, and asserts final T2/order through the actual query flow. Cover matching not-modified and authoritative full-list; also cover the mismatched/null→hashless fallback and unchanged authoritative-null clearing. Mutation-bypass each settlement merge and require the corresponding row to fail.

Exact-head validation

• cd desktop && pnpm test — 5,142 passed.
• just desktop-tauri-test — passed; one intentional performance test ignored.
• cd desktop && pnpm check && pnpm typecheck — passed with existing warnings/infos.
• cd desktop && pnpm build — passed with chunk warnings.
• cd desktop && pnpm build:e2e && pnpm exec playwright test tests/e2e/channel-sort.spec.ts --project=smoke --workers=1 — 4/4 passed; proves ordinary live Recent reorder, not refresh overlap.
• Helper mutation — focused suite failed 2/5; restored suite passed 5/5.
• Production-wiring mutation — full Desktop suite remained 5,142/5,142, confirming the blocker.
• Hosted checks observed at this head: macOS build, Windows Rust, both integration shards, release candidate, Rust lint, DCO, and smoke shards 1–2 passed; Desktop Core and smoke shards 3–4 were still running at final inspection.
• Reviewer’s direct raw Tauri invocation failed only because sidecar stubs were absent; canonical just desktop-tauri-test created them and passed.

Manual/native evidence: no native GUI launch; shared-host policy prohibits it without explicit opt-in. Browser mock evidence does not prove native 128/129-channel batching/failure behavior.

Residual risk: native large-channel/failure states remain unexercised, but the merge blocker is specifically the missing causal production-flow regression. Please do not merge this head until that test bites the real call path.

[wesbillman]

Addressed the production-wiring coverage blocker at ee7d56f33.

The review was correct: the previous "full-list/not-modified" regression called the merge helper twice with identical inputs and never exercised either production settlement path. The query orchestration is now an exported production function used directly by useChannelsQuery, and the new tests execute it through a real QueryClient.fetchQuery with deferred getChannels settlement:

• matching not-modified: T0 request → live T2 cache update → stale T1 settlement
• authoritative full list: same causal interleaving
• mismatched null → deferred hashless retry: same causal interleaving
• unchanged authoritative absence still clears recency

Mutation gate: bypassing the normal production merge fails the first two rows; bypassing the hashless-fallback merge fails the retry row. The previous misleading response-shape loop is gone.

Validation at committed head ee7d56f33: Desktop unit suite 5,146/5,146, focused query/recency 21/21, TypeScript, Biome, differential file-size gate, and git diff --check pass. Independent fresh review found no blocker and reproduced per-seam mutation sensitivity.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

REQUEST CHANGES: Forum “Recent” still loses authoritative recency on reload/refetch.

The prior stale-settlement and production-wiring blockers are resolved at exact head ee7d56f33dd73ee28b97656482264378f3c99955, but a fresh cross-surface review found one remaining defect in the feature.

AppSidebar offers Recent sorting for Forums, and live recency deliberately treats forum posts/comments as human-visible activity via CHANNEL_MESSAGE_EVENT_KINDS. However, the native refresh in desktop/src-tauri/src/commands/channels.rs:334-343 queries only kinds 9 and 40002; it omits forum post/comment kinds 45001 and 45003.

A forum containing only forum events therefore receives no lastMessageAt on cold load. If a live forum event first moves it under Recent, a later successful refresh supplies authoritative absence and clears that timestamp when no event races the request. The forum then falls back to alphabetical order. The current E2E checks Forums only with their sort preference unset/A–Z, so it cannot catch this.

Please make the native recency query cover the same human-visible channel activity contract, at least kinds 45001 and 45003, and add a Forum=Recent reload/refetch regression. Existing stream batching, error propagation, monotonic settlement, and causal mutation coverage remain sound.

[jedwards27]

Reviewed exact head ee7d56f33dd73ee28b97656482264378f3c99955 against base 3c228b1082a93aca302c7b6a67ec274c51ed5eaf. The previous causal-coverage blocker is resolved. I found no material defect.

The production query path now delegates to refreshChannelsQuery, and the regression tests drive that function through a real React Query QueryClient with deferred relay completion. They cover matching not-modified, direct authoritative full-list, hash-mismatch/hashless fallback, and authoritative absence. Crucially, two independent merge-bypass mutations caused the expected production-seam regressions: bypassing the fallback merge failed the hashless retry case (15 pass / 1 fail), while bypassing the normal settlement merge failed matching not-modified and direct full-list (14 pass / 2 fail). This proves the tests no longer pass while the real wiring is broken.

Behavior traced at this head:

• live T2 recency survives stale T1 settlement for not-modified, full-list, and fallback paths;
• authoritative absence still clears stale recency;
• relay rejection happens before snapshot/cache settlement, preserving cached display state;
• persisted snapshot/hash data remains authoritative rather than contaminated by live-merged timestamps;
• the unchanged native path batches last-message filters at 128 and remains serial/fail-fast, while member-count failure degrades independently.

Validation on clean, matching-head trees:

• focused channel recency/hooks tests: 21/21 pass;
• just desktop-ci: rc 0, including 5,146/5,146 renderer tests, build, Tauri fmt/check, and full Tauri workspace tests (one intentional performance test ignored);
• pnpm build:e2e plus focused channel-sort.spec.ts on an isolated port: 4/4 pass; inspected artifacts showed Recent selection, live movement to top, independent groups, and DM sorting;
• current GitHub checks are all successful or intentionally skipped, and the PR is mergeable.

Residual risk: no real native GUI/harness journey was run on this shared host. Browser-mocked E2E does not independently prove a live relay's 128/129 boundary or native failure rendering. The native batching implementation and full native test gates were inspected/run, and this head's delta is limited to query orchestration extraction and regression coverage, so that residual risk is acceptable.

Any new head requires re-review.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Verdict: CLEAR at exact head f9003a02af9035431bf9045f921d5db921c94718. I found no remaining material code or product defect in 3c228b1082a93aca302c7b6a67ec274c51ed5eaf..f9003a02af9035431bf9045f921d5db921c94718.

The Forum Recent blocker is resolved. The native recency filter now covers the same four human-visible activity kinds used by the renderer (9, 40002, 45001, 45003), so forum posts and comments remain authoritative through cold load, refresh, and reload. The new native contract test asserts the exact filter, and the browser regression seeds Forums=Recent, verifies the non-alphabetical authoritative order, reloads, and verifies it again.

I also rechecked the earlier fixes: 128-channel relay batching, fail-fast timestamp-query errors, monotonic refresh settlement across not-modified/full-list/hashless-fallback paths, authoritative-null clearing, and production-path causal coverage remain intact.

Validation on a clean detached worktree whose HEAD matched the reviewed SHA:

• just desktop-tauri-test: passed the full Tauri workspace; the new native kind-contract test passed (one intentional performance test ignored).
• Focused production refresh/recency suites: 21/21 passed.
• pnpm build:e2e plus focused channel-sort.spec.ts: 5/5 passed.
• Mutation check: removing authoritative watercooler forum recency made test 04 fail with alphabetical [announcements, watercooler]; the exact head was restored and confirmed clean.
• git diff --check 3c228b108..HEAD: clean.

At submission time, 9 hosted checks had passed, 8 were still running, and none had failed. Those pending checks are a merge gate, not a code-review finding. The two old changes-requested reviews target obsolete heads and may still need dismissal/re-review by their author or a maintainer.

[jedwards27]

Verdict: APPROVE

Reviewed: 3c228b1082a93aca302c7b6a67ec274c51ed5eaf..f9003a02af9035431bf9045f921d5db921c94718

The remaining Forum Recent defect is resolved. Native recency queries now use the same human-visible activity contract as renderer live updates—kinds 9, 40002, 45001, and 45003—so forum posts/comments remain authoritative across cold load and refresh. The filter still contributes one #h query per channel with limit: 1, preserves 128-filter batching, and retains fail-fast timestamp-query behavior before cache settlement.

I rechecked the earlier concurrency and cache contracts at this exact head: newer live T2 survives stale T1 settlement through matching not-modified, authoritative full-list, and hashless-fallback flows; unchanged authoritative absence still clears stale recency; relay timestamp failure preserves cached state; and snapshot/hash authority is not contaminated by display-only live merges. Relay ordering is newest-first, with a defensive maximum reduction over returned timestamps.

Causal and exact-head evidence

• Reverting the native kind set to [9, 40002] made last_message_filter_covers_all_human_visible_activity_kinds fail specifically for missing 45001/45003; restoration produced a clean full Tauri workspace pass (2,686 passed / 0 failed / 18 ignored in the main package, with one intentional performance test ignored).
• Bypassing the hashless-fallback production merge failed its regression (15 passed / 1 failed); bypassing normal not-modified/full-list settlement failed both corresponding regressions (14 passed / 2 failed). The restored focused control passed 21/21.
• Full just desktop-ci passed on a clean matching head: checks, build, 5,146/5,146 renderer tests, Tauri fmt/check, and the full Tauri workspace.
• pnpm build:e2e plus isolated channel-sort.spec.ts passed 5/5, including Forum Recent authoritative ordering across reload, live stream movement, independent groups, persistence, and DM sorting.
• git diff --check passed. Immediately before this review, the remote head still matched f9003a02af9035431bf9045f921d5db921c94718, the PR was mergeable, and all applicable hosted checks had completed successfully.

No material defect remains in the reviewed range.

Residual risk: no real relay-backed native GUI journey was run. Browser E2E and native filter-shape mutation prove the rendered and relay-query sides separately rather than their full live IPC composition. For this narrow change, the causal production-seam tests, full native gate, and hosted matrix make that residual acceptable.

Any head movement invalidates this approval.

— :bot: Jude’s code review agent

[jedwards27]

Verdict: APPROVE

Reviewed: base cd0d33f08507d07c8e8b8511bba92290c046ef03, exact head 0e75e1ade03cf5ac4ee38a4094ee8b59fc6da943.

The parent-aware merge preserves the complete Recent contract while adopting the base branch's extracted native channels/fetch.rs structure. I found no material defect in the new delta or affected renderer↔Tauri↔relay behavior.

The native query still uses the canonical human-visible activity kinds (9, 40002, 45001, 45003), emits one indexed #h filter per channel with limit: 1, and chunks at the relay's 128-filter cap. Recency-batch failure aborts settlement and preserves cached ordering. Successful settlement keeps a newer concurrent live timestamp while authoritative absence clears unchanged stale recency. Live stream/forum activity advances recency before self/mute/notification filtering. Member polling, open-directory discovery, and identity-scoped pending-owner state retain their ownership boundaries.

Channels and Forums keep independently persisted Recent preferences scoped by identity/relay. Targeted browser evidence covered A–Z defaults, Recent persistence/reload, immediate live stream reordering, authoritative Forum Recent ordering across reload, Channels/Forums isolation, and independent DM behavior.

Exact-head evidence

• Causal native mutation 128 → 1024 made last_message_filters_stay_within_relay_channel_cap fail with [257] instead of [128, 128, 1]; restoration left a clean tree.
• make desktop-test: 5,204 passed, 0 failed.
• make desktop-tauri-test: 2,797 passed, 0 failed, 19 ignored across 16 targets.
• Targeted channel-sort.spec.ts: 5/5 passed; inspected Recent screenshot SHA-256: d2c9678750d42b87082730c085b18aa053196c3f924cebfc3036de0e4dc57b91.
• pnpm check, pnpm build, Tauri cargo check, and git diff --check passed on clean matching-head trees.
• All applicable hosted checks completed successfully immediately before submission.

Residual risk: browser E2E directly emits live stream activity, while live Forum advancement and query failure/empty states are established through shared canonical-kind/cache paths and contract/unit tests rather than separate browser emissions. No relay-backed native GUI journey exercised the full IPC composition at the 128/129 boundary. The causal native mutation, full renderer/native suites, targeted browser journey, and hosted matrix make that residual acceptable for this narrow merge-resolution delta.

Any new head invalidates this approval.

— :bot: Jude’s code review agent