feat(sidebar): persist independent section sorting - #206
Conversation
a73c35a to
a164e5e
Compare
|
AI review report — Carl (implementation/coordinator) and Princess Donut (adversarial reviewer) are AI agents. Round 1: blocked. Donut reviewed
Integration completed: rebased and pushed Validation at clean PR remains draft. Remaining gates: both blockers, the specific retry UX decision, final-head Donut pass, required hosted checks and eligible GitHub/CODEOWNER approval. No GitHub approval review or merge submitted. Integration follow-up: Princess Donut subsequently passed the bounded rebase delta at |
Add per-section Recent and A–Z choices with optimistic save recovery and bounded verified live activity. Preserve saved sidebar placement during startup and align shared menu edge corners. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Count newly supported diff messages in Recent history and live ordering. Exercise session-owned save errors through Projects now that main no longer has Home. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Defer icon sizing and optical alignment to the shared menu follow-up. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Keep saved viewport restoration behind settled preferences while retaining the bounded roster reveal. Route only sidebar preference events through the sorting fixture handler, and align delayed-scroll readiness and cleanup with the current startup flow. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Stub jsdom's missing scrollIntoView method and wait for the real reveal callback before leaving the exact-target journey. Keep native scrolling and geometry acceptance in browser coverage. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Extend the existing sorting journey across light/dark and narrow/intermediate/wide layouts. Check all corners of the submenu trigger and both radio choices against the shared pill token after integrating the main-owned menu. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Align the sorting-owned projection expectations with main's always-present New message entry point. Preserve the mainline projection and its tests unchanged. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Record the desktop-compatible whole-record persistence contract and deterministically cover an intervening different-section save being lost while both mutations confirm. Keep runtime behavior and the wire format unchanged. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
7dc5e16 to
92c7384
Compare
|
AI review report — Carl (implementation/coordinator) and Princess Donut (adversarial reviewer) are AI agents. Bounded follow-up at
No ready transition, GitHub approval review or merge. Follow-up is event-driven, not monitored on a timer. |
Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
AI review/fix update — Recent lifecycle correction deliveredCarl and Princess Donut are AI agents. Donut’s read-only review of local Fixed in Merge Validation: the new session regression failed before the fix ( Final-head code-review PASS: Princess Donut independently reviewed clean |
Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Preserve the persistent app-owned sidebar and move section sorting, startup presentation, and Recent recovery into its existing owner. Retain both focus and scroll regression assertions and both broker capabilities. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Scope plugin status assertions to the main surface now that the sidebar persists across pages. Wait for the observed wheel scrollend before capturing the exact-navigation reading baseline; preserve the existing anchor tolerance and product behavior. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Preserve the sidebar sorting icon alongside hosted-community action icons. Retain main's settings and profile additions and the branch's browser synchronization repairs. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Observe scrollend before each real wheel gesture and wait before measuring the next distance. Keep full-row viewport and scroll-progress assertions unchanged; avoid queued native input carrying the row offscreen after the loop exits. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Compose independent section sorting with confirmed mute/read preferences, preserving the persistent row menu, authenticated mute publication, shared write queue and cancellation fences. Retain one native wheel-completion barrier with all Settings assertions. Add combined projection, queued mutation, cancellation and reload regressions. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Compose Recent/A–Z section sorting with confirmed lifecycle visibility and removal. Preserve fresh row permissions, confirmation focus, empty navigation, and mainline deployment/community-admin changes. Keep both bounded relay query models and share validated fixture publication handling so sorting and lifecycle can run together. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Source review clear of blocking findings at ab25dd2af0f7f491f340f2d5bf2487fa78802135 against 223028bbb59273dae9c109675b6b7f3141d13340. Two non-blocking P3 follow-ups are recorded inline. COMMENT only, not approval or merge readiness. The explicitly accepted cross-device whole-record LWW limitation remains accepted, not fixed.
Reviewed the section menu → session-owned optimistic queue → transport/broker → encrypted preference/read-back path, activity admission and cancellation, personal-group composition, and cold/warm persistent-sidebar lifecycle. Initial concerns were narrowed after tracing the alternate personal-group owner and existing preference-error Retry UI; neither warrants a blocking review.
Merge gate: required CI is red. Run 36077499072, attempt 1 failed Chromium's Beta warm-switch measurement: 200.9 ms against <200 ms; CI required failed accordingly. The logs establish the failure, not whether it is a reproducible product regression. JavaScript passed (327 files / 3,577 tests), Rust/tool integration and all four browser-journey shards passed; Windows native was skipped. Checkout logs pin tested merge 289b025f43ea178f69becb3b86ce80c0582a80ae with older base 6fa0e9a63690e7eab413f9e5c11be7a05a4f6c4f, so those results do not validate the reviewed base. Diagnose the measurement and obtain successful required checks on the final head/base before merge; do not simply relax the ceiling.
Source-only Blox review plus existing hosted evidence. No checkout, install, build, tests, browser execution or CI reruns were performed here. Packaged-native behavior, live-relay writes, physical interaction and multi-device acceptance remain unverified.
| if ( | ||
| !Object.values( | ||
| sidebarPreferences.queries.snapshot().data?.sort ?? {}, | ||
| ).includes("recent") |
There was a problem hiding this comment.
[P3, non-blocking] Ignore removed-group overrides when deciding Recent demand
Set a personal group to Recent, remove that group through ChannelGroupsDialog, and reopen with all displayed sections at A–Z. Full preference reads intentionally retain the old section:<id> key, but this raw-value check (and useSidebarStartup.ts:69) still demands activity. A failed read then shows “Sections sorted by Recent may be out of date” despite there being no such visible section; cold reveal can also wait up to its 1.5-second budget unnecessarily. The conversation remains usable and the notice is dismissible, so this is a follow-up rather than a merge blocker.
Derive demand from the actual current section set, including personal groups from channelKit, while preserving unknown keys in the encrypted record. Do not filter only against preferences.sections: valid personal groups are supplied by a different owner. Cover removed-group/all-visible-A–Z demand and preserve live personal-group Recent behavior.
| /> | ||
| </span> | ||
| )} | ||
| {preferences.sortWritable && ( |
There was a problem hiding this comment.
[P3, non-blocking] Disable Sort until an initial preference snapshot exists
If the first preference read fails, startup correctly reveals the conversation list and shows the existing preference warning with Retry. This capability-only condition also exposes Sort, but setSort() rejects immediately when snapshot.data is absent; setSectionSort() then swallows that rejection and closes the menu without applying the choice. Later refresh failures retain data and do not have this problem.
Hide/disable the affordance until preferences.sortWritable && !!preferences.data, or give it an explicit unavailable reason. Preserve the existing Retry recovery and retained-data behavior. This is a misleading control in an already-signposted recoverable error state, not an unsupported-host defect.
Use mainline wheel-completion barriers for message navigation and Settings while retaining the explicit Settings scroll-progress assertion. Preserve incoming channel-members behavior and unchanged sorting, lifecycle, and opening performance contracts. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No blocking source findings at be889c6fec69238f4555eb782029380e6dfdd1a2 against 31859908f88025bf24a7be4f5219cf22d3139cfd. This is a COMMENT review, not approval or live-product acceptance.
- Integration: production sorting files are unchanged from the prior source-clear
ab25dd2a. Reviewed the incoming UI seams, retained activity/session and encrypted mutation/lifetime contracts, and both browser-test conflict resolutions. The shared wheel-completion helper preserves the movement/anchor assertions. Existing P3s remain nonblocking: removed-group Recent demand and Sort enabled without loaded preferences. The explicitly accepted cross-device whole-record LWW limitation remains accepted, not fixed. - CI: the previous red required-check gate is now green in run 36080697284, attempt 1. Actual checkout was synthetic merge
4da6198eb647daec83a0b31e05cc5691080e8b02, merging this head into2983b2f4b62673733419c6c52ed0e4dfbcc153d1, not the recorded base above. Its six differences from source head are profile-related paths. On that tested tree: 3,580 Vitest tests, 634 browser journeys across Chromium/WebKit, and 7 measurements passed; selected browser cases had no skips, retries or flaky outcomes. All five sorting journeys passed in both engines. Three documented WebKit local-only cases remain outside hosted selection; Windows native validation was skipped. - Performance caveat: warm switches were 187.8, 186.2, 179.0 and 168.3 ms. These pass the existing <200ms ceiling but all miss the <100ms target. I inspected the raw timing/frame evidence; it does not establish runner contention or a PR-caused regression. This large-sidebar measurement uses no saved Recent selection, so it is not direct Recent-enabled performance acceptance. No budget or assertion weakening was found in the reviewed changes.
Source review and existing hosted evidence only; no independent test reruns, native app execution or live-relay/multi-device acceptance this round. Eligible reviewer/CODEOWNER approval and a fresh merge preflight remain separate gates. No approval or merge performed.
|
AI-generated update — Carl, acting for Taylor Ho. @wesbillman Both non-blocking follow-ups are implemented and pushed on
These fixes are not in the merged PR. #206 merged at Validation at No hosted checks are reported for this follow-up head yet. Current-head independent review and native/live-relay behavior are unverified. The accepted whole-record last-write-wins contract is unchanged. |
Overview
Category: new-feature
User Impact: Users can choose Recent or A–Z independently for each sidebar section and keep those choices after reopening Buzz.
Problem: Sidebar sections only have alphabetical ordering, making it harder to return to active conversations without changing how every other section is organized.
Solution: Add a per-section Sort menu with persisted choices, immediate feedback, and explicit rollback/retry when saving fails. Recent uses bounded, verified activity reads plus live updates, while a bounded startup presentation gate preserves saved placement without blocking the open conversation.
This is the focused sorting slice, integrated with
mainat119195ea331de33c8480bab180df0091ca8e9421, not the older combined sidebar branch. Group/star editing, section CRUD/reordering, and sibling integration remain separate. The two-parent merge preserves main’s persistent app-owned sidebar, navigation, New message, session actions, delayed viewport restoration, and Todos; sorting presentation now belongs tofeatures/channel-navigation/ChannelSidebar.tsx, not the conversation page. Recent includes kind-40008 diff messages in both historical and live activity. Sorting inherits the shared pill-shaped menu rows merged in #209; this branch no longer overrides shared menu radii. The section ellipsis and separate create buttons remain unchanged, and the Sort glyph remains 14px.Accepted persistence contract: section choices are independently selectable, not conflict-safe across devices. The desktop-compatible encrypted
channel-sortrecord remains whole-record last-write-wins: near-simultaneous saves can silently overwrite another section’s choice even when both saves report success. The owner accepted that limitation for this PR; the new documentation and deterministic regression test describe it, not a concurrency fix.Changes
File changes
dev/relay-broker.mjs
Expose purpose-bound background activity reads and a narrow, serialized sort-write endpoint with bounded head reads, publication acceptance, and read-back confirmation.
dev/sidebar-preferences.mjs
Decode the additional encrypted sort coordinate alongside existing group and star preferences.
dev/sidebar-sort-broker.test.mjs
Cover the production broker's filter boundaries, signed writes, rejection paths, admission behavior, and publication confirmation.
dev/sidebar-sort.mjs
Validate sort intents and update the requested section while retaining unrelated fields present in the record read. Document that neither the broker-local queue nor requested-field read-back guarantees preservation against another device’s concurrent whole-record publication.
dev/sidebar-sort.test.mjs
Cover intent validation, encryption, fields retained from the record read, fresh-head reads, and failure/confirmation behavior. Add a deterministic actual-mutator race proving both saves can succeed while one section’s choice is lost under the accepted LWW contract.
docs/channels.md
Document desktop-compatible whole-record LWW persistence, independently selectable versus conflict-safe choices, and the limitations of local serialization and read-back.
src/bundled/channels/Channels.module.css
Keep each section's actions in the existing compact header layout.
src/features/channel-navigation/ChannelSidebar.tsx
Wire independent Sort menus, session-owned save errors and retry, Recent-activity Retry/Dismiss, displayed personal-group IDs, and startup presentation into main’s persistent sidebar.
ChannelsPage.tsxis unchanged from integrated main. Keep saved viewport restoration behind settled preferences even when the bounded reveal exposes rows earlier.src/bundled/channels/sidebar-sections.test.ts
Cover section independence, stable tie-breaking, missing activity, retained placement, and main’s always-present empty DMs section for New message.
src/bundled/channels/sidebar-sections.ts
Sort each section independently while preserving existing visibility, grouping, and starred placement.
src/bundled/channels/useSidebarPreferences.ts
Expose sort mutations and error dismissal through the existing sidebar preference hook.
src/bundled/channels/useSidebarStartup.test.tsx
Exercise the real hook's cold/warm presentation, bounded waiting, failures, and session lifetime behavior.
src/bundled/channels/useSidebarStartup.ts
Coordinate sidebar reveal with saved preferences, names, activity, and unread settlement; retain warm-session presentation without delaying indefinitely.
src/features/relay/channel-activity-session.test.ts
Cover activity demand, live admission, cancellation, session cleanup, failure → last Recent cleared → re-entry → fresh failure/recovery, and retained selections/activity at the integration owner.
src/features/relay/channel-activity.test.ts
Cover bounded batches, monotonic live/history reconciliation, stale completion fencing, failures, and diff-message activity.
src/features/relay/channel-activity.ts
Own the bounded activity projection independently of unread state and roster/access readiness. Retire loading/error status when demand is cancelled without discarding good activity values.
src/features/relay/contracts.ts
Expose verified last activity and initial activity status on existing channel contracts.
src/features/relay/live.ts
Include forum activity in the existing live channel subscription while retaining main's diff-message support.
src/features/relay/session.ts
Connect activity demand, verified live events, roster visibility, and teardown to the session-owned projection.
src/features/relay/sidebar-preferences-store.ts
Own optimistic sort choices, serialized writes, per-section rollback/retry, and supersession across page navigation.
src/features/relay/sidebar-preferences.test.ts
Extend preference projection expectations for the sort coordinate.
src/features/relay/sidebar-preferences.ts
Define normalized fixed/custom section sort keys and the narrow mutation contract.
src/features/relay/sidebar-sorting-store.test.ts
Cover overlapping choices, stale refreshes, independent failures, supersession, and lifetime cleanup.
src/features/relay/transport.ts
Advertise host capabilities, verify bounded activity responses, and route narrow sort intents to the broker.
src/shared/design-system/icons/index.ts
Expose the Phosphor sorting glyph through the existing icon gateway.
tests/browser/fixture.mjs
Add opt-in signed sorting/personal-group data and narrowly scoped injected-failure accounting for real broker journeys.
src/features/sessions/SessionMessageTarget.test.tsx
Preserve main’s focus assertion and the feature’s explicit scroll assertion at the reveal boundary; do not weaken either parent’s coverage.
tests/browser/navigation-scroll-intent.spec.mjs
Observe the current pending-details signal and await held requests during teardown. Preserve all four exact viewport contracts.
tests/browser/navigation-sorting.spec.mjs
Add browser journeys for independent optimistic sorting and recovery across page exit/reload, cold saved presentation, and personal-group persistence. Assert the shared pill corners on the Sort trigger and radio choices across light/dark themes and narrow/intermediate/wide viewports.
tests/browser/policy-relay.mjs
Model the exact bounded activity and three-coordinate preference reads used by the production broker.
tests/browser/sidebar-unread.spec.mjs
Establish the bounded sidebar reveal before measuring scroll and await held routes during cleanup, preserving the existing unread/session assertions.
Reproduction Steps
Validation and current review status
Current head:
602179040fb324564ddb87af3bb580bbd72f10d6; integrated main:119195ea331de33c8480bab180df0091ca8e9421. Ready for review; final-head adversarial review and relevant validation passed; remaining hosted CI pending.b704fdbadds the Recent warning with Retry/Dismiss;0c2291dretires the failure when the last Recent section becomes A–Z while preserving good activity values.6021790integrates main without rewriting existing feature history. Section sort/startup/recovery wiring moved into main’s persistent sidebar owner. The conversation page equals main. Both broker capabilities, both lifecycle/persistence docs, and main focus plus feature scroll assertions are preserved.idle, receivederror; all other 3,108 tests passed. At final clean 6021790, 313 files / 3,362 Vitest tests passed (52.80s wall; 282.73s summed test execution).navigation-sorting,navigation-sidebar,navigation-scroll-intent,new-messageandsidebar-unreadfiles, both Chromium and WebKit, real production app/broker with isolated signed fixtures. Recovery now asserts A–Z warning removal, dismissal/re-entry, retained Recent selection and exact restored activity row order. Persistent sidebar/page transitions and original viewport contracts remain covered.CI requiredis not yet reported. Old green checks are not reused as this head’s evidence.Princess Donut (AI reviewer) passed exact clean head 6021790, including the lifecycle correction, persistent-sidebar integration and combined feature: no concrete code-review blockers found. She reviewed source and SHA-labelled validation logs without rerunning tests. The accepted whole-record LWW limitation remains accepted, not technically fixed. Her code-review pass does not attest hosted CI or merge readiness.
Moved to ready for review after exact-head Donut review and relevant validation passed; no unresolved UX decision remains. Remaining merge gates: all required hosted checks, eligible GitHub approval/CODEOWNER coverage, and fresh final merge preflight. DCO already passes. No approval review or merge performed. Native and live-relay/multi-device behavior remain unverified. No timers or CI polling.
Review/fix evidence: #206 (comment) . Earlier reports: #206 (comment) and #206 (comment) .
Earlier implementation validation (historical snapshots, not current-head runs)
Earlier head:
a73c35ab3e002a898e4faec35516f7a9d29ee4c5; integrated main:842c1d3bac29f42a029e383759642ef1fc6705a2.86aa1e4, 58 browser cases passed across Chromium and WebKit in the completenavigation-sorting,navigation-session-menu,navigation-sidebar,new-message,navigation-scroll-intent,sidebar-unread, andtodosfiles (1.4m local). This includes shared menu geometry, independent/personal-group persisted sorting, failure rollback/retry, cold startup, session context actions, New message, scroll intent, unread, and Todos integration.86aa1e4to current head is a pinned-formatter line wrap plus an optional trailing argument comma inChannelsPage.tsx. The preceding hosted JavaScript job failed on that exact formatting difference before running its tests. The full-treebiome check --error-on-warnings .now passes locally; the unrelated schema-version notice is informational.86aa1e4: 15 files / 94 tests passed. No broad test suite was rerun solely for the formatting-only follow-up.Commands (Hermit active):
Regression history: delayed scroll restoration was verified fail-then-pass before this integration: all eight scroll cases failed with early restoration, then passed after preserving main’s preference-settlement boundary (expected final positions 900/1800/0/1800). The current complete-file rerun retains those contracts. The rebase preserves both main’s
ListChecksIconand sorting’sArrowsDownUpIcon; sorting-owned test expectations now retain main’s empty DMs section rather than changing upstream behavior.Browser-test scope: three sorting scenarios run in both engines (six cases); none removed. They prove browser menu interaction, production-broker encryption/publication/read-back and reload, page-unmount recovery, and visible cold-start presentation. The shared pill-corner matrix extends an existing journey rather than adding cases. Combinatorial ordering, mutation races, validation, and lifetime matrices remain in unit tests. No broader before/after performance claim is made. Timings are local macOS arm64/Node 24.18.0 measurements, not hosted CI.
Evidence boundary: browser write/reload and failure journeys use the production frontend/broker against an isolated signed relay fixture, not live-community preference writes. Native behavior, live multi-device writes, and integration with the separate grouping work remain unverified. Prior independent review covered an earlier snapshot, not this latest integration. Hosted CI remains a separate gate.
Screenshots / Demos
Two real built-app captures show A–Z selected and Recent selected after save and reload, in dark theme with the teal accent. Both Chromium and WebKit captures were generated from pre-rebase head
a73c35a(not regenerated for current head6021790; these historical captures predate the persistent app-owned sidebar layout); the shared images are Chromium, tightly cropped to 430×390 with synthetic fixture data. They use #209’s shared pill-shaped menu styling and retain the section ellipsis. These are two states of the finished implementation, not a synthetic UI mockup or before/after code comparison.A–Z selected
Recent saved and restored after reload
Originating channel:
b9ab2a04-14c4-440d-8c82-aebfbc1caa68(task thread).Prepared by Carl, an AI agent, at Taylor Ho's request. Originally opened as a draft; now ready for review under the authorized review gates. No GitHub approval review or merge performed.