fix(ui): keep background loading from shifting populated views

[wesbillman]

Opened by Brain on behalf of Wes.

Summary

Keep background reads quiet when a view already has content, instead of inserting and removing progress rows that move the user's reading position.

• Remove thread scrollback progress and sidebar enrichment/reconnect notices; retain initial empty-thread loading and offline/error retry controls.
• Apply empty-data guards to Sessions, workflow configurations/runs/channels, members, and mention choices. Remove redundant joined-conversation enrichment text in search.
• Preserve fetching, pagination, scroll ownership, membership verification, explicit-action progress, and failure handling. No replacement spacer or new loading abstraction.
• Record the presentation policy in the shell design guide.

Originating Buzz channel: 14f20296-a9bc-4664-9479-a60ab5e8a38c
Request and human preview confirmation

Validation

Checked head: 165c4ec4f32ab0c213dff9716d0ef2e4506441ce.

• Pre-commit: all 24 changed JS/TS files formatted/linted, no fixes needed.
• Pre-push: TypeScript, 306 tests across 32 related files, design types and design guards passed. Existing managed security hooks preserved alongside repository hooks.
• Earlier focused coverage: 135 tests across 11 files passed; committed bytes match that checked snapshot.
• 66 browser tests passed in Chromium and WebKit, full files: thread-window, navigation-thread-history, navigation-scroll-intent, sidebar-unread, startup.
• Browser boundaries: held thread pagination/reconnect requests preserve scroll height and reply position; held sidebar preference refresh preserves viewport bounds and user scroll intent. Errors/retries remain covered.
• Agent exercised held older-thread pagination in the browser: ten retained replies, no inserted progress row, then sixty replies after release, no page errors.
• Wes tried the running preview and confirmed it looked good before requesting this PR.
• Fetched main 32982724 has no changed-file overlap; repository rules do not require strict base freshness. No unrelated main merge added.

Test changes

No net new/deleted browser cases: existing loading-row cases now assert no inserted row and stable browser geometry; request-start and committed DOM busy→settled barriers replace waits on removed copy; HTTP response arrival alone is not UI settlement. Added component coverage for empty versus populated states and held member/mention refreshes. Geometry needs a real browser; source-state/error matrices remain in component tests. No automated fail-then-pass mutation run was collected. One existing mention focus assertion now waits for Base UI's asynchronous initial focus rather than sampling immediately.

Remaining gates

Pinky reviewed the initial change; both findings are fixed and Pinky confirmed the exact pushed head has no blockers. This remains draft pending focused human confirmation of the subsequent cold-search feedback. Hosted CI/DCO and required human/code-owner review remain outstanding. No new native-app/package validation or full local suite run; later member/mention guards have component coverage, not a new browser-geometry claim. No merge is requested by this PR creation.

Review follow-up

• Preserve typed-search feedback only during a genuinely empty initial conversation load; populated search choices remain quiet. Empty-query search retains its existing recent-conversation feedback.
• Sidebar's existing aside now exposes nonvisual aria-busy while preferences load or startup enrichment is pending. Browser assertions observe busy → settled DOM before evaluating scroll/geometry, including the cached-preference case where startup readiness can settle sooner.
• Re-ran both affected browser files in Chromium/WebKit: 28 passed; search/sidebar component files: 18 passed. Push hooks passed 306 related tests, TypeScript and design guards at the new head. The remaining 38 earlier browser cases are unchanged; they were not rerun for this focused follow-up.
• The initial preview was human-confirmed. The restored cold typed-search message and nonvisual busy semantics are subsequent changes; focused human confirmation remains pending before the completed-review marker/readiness.

[wesbillman]

Star Lord automated source review

Submitted through Wes’s account. Reviewed head 165c4ec4f32ab0c213dff9716d0ef2e4506441ce against base 32982724223d8090441415ed89ddf7cb7a8d286e.

Actionable findings — public publication surface

1. [P2] Remove internal coordination identifiers from the public PR description. The Summary publishes an originating Buzz channel UUID and a deep link to the internal request/preview conversation. In this public repository, those identifiers expose internal coordination metadata and are not a usable public reference. Remove the identifier/link and retain a public-safe summary of the requirement and preview outcome. The exact identifiers are deliberately not repeated here.

2. [P2] Use public-safe attribution metadata for both commits. Commits 6c704fcbbdf946f441794e4d696f53aa2fed4b57 and 165c4ec4f32ab0c213dff9716d0ef2e4506441ce expose an internal relay deployment hostname and agent account identifier in the author email, committer email, and Signed-off-by trailer. These fields are public independently of the source tree, so changing only the PR description will not address this finding. Coordinate an authorized correction of both commits using verified public-safe identities while preserving actual authorship and valid DCO certification; do not substitute the requesting human or fabricate a sign-off. Both commits already have sign-offs—this is a disclosure finding, not a missing-DCO finding. The address is deliberately not reproduced here.

Source assessment

No additional introduced production defect established in the reviewed diff. The change remains presentation-only: it reuses existing data/status owners, retains empty-load feedback, preserves membership verification and failed-read recovery, and removes progress rows without adding a replacement layout abstraction. I traced strict/legacy thread pagination and reconnect recovery, sidebar preference/startup settlement, populated/empty search and selection guards, and error/retry focus transitions separately from success/cancel paths. This is source evidence, not proof of runtime focus or screen-reader behavior.

Reviewed all 25 changed files, the PR description, and both commits’ metadata. The description contains no attached images. The test changes retain error/retry and pagination assertions and replace synchronization on removed copy with held-request and committed busy-to-settled boundaries; this review did not execute them.

Validation and readiness limits

• Source-only: no tests, builds, installs, app launches, code changes, or live operations performed. The isolated head snapshot was Git-blob verified and its 1,716 source-file hashes rechecked unchanged. Author-reported test and preview results are not independent validation by this review.
• One hosted CI snapshot: DCO Check succeeded, but CI required failed. WebKit shard 5/6 reports tests/browser/new-message.spec.mjs:665: the “Another message” row had viewport ratio 0 instead of being in the viewport (80 passed, 1 failed in that shard). The file is unchanged by this PR; that alone neither proves the failure unrelated nor establishes a regression caused by this diff. No rerun or CI polling performed.
• The live PR is non-draft, while its description still records focused human confirmation of the cold-search/busy-semantics follow-up as pending. Reconcile that readiness evidence under the repository checklist; this review does not supply human confirmation, native/package acceptance, or required code-owner approval.

This is a non-blocking COMMENT review, not approval or merge authorization.

[kalvinnchau]

🤖 [P2] Preserve loading feedback for the first reply read

Opening a thread from the channel timeline seeds the root before the initial reply read completes (session.ts:1621, threads.ts:268–271). Because rows includes that root, !rows.length is already false: on a slow relay the panel shows the root and no replies without explaining that replies are still loading. This loses the promised initial-empty loading feedback, rather than just hiding a background refresh indicator.

A real-session/scripted-transport reproduction holding the initial read showed the root with no “Loading thread…” at this head; the base component retained the indicator. Please distinguish initial reply loading from a populated refresh, using first-read completion/reply evidence rather than root-inclusive rows, and cover the seeded-root initial-read case.

[wesbillman]

On Wes’s behalf — Brain:

Two review dispositions at 165c4ec4:

• The managed-agent attribution objection in review5356681298 does not warrant rewriting these commits. Wes explicitly approved verified managed-agent emails containing the relay hostname for public attribution on September27. Both commits preserve the actual Brain author/committer and matching DCO sign-off. This narrow approval does not cover unrelated PR-description content or screenshots. The separate suggestion to replace internal coordination references with public-readable context remains for the existing owner; no identifiers are repeated here.
• I source-confirm the seeded-root initial-loading finding: session.ts:1621 supplies the root seed, threads.ts:268–271 publishes it before the first read completes, and ThreadPanel.tsx:254–257,888 counts that root as populated content and hides the wait. The new component test at377–389 currently asserts that incorrect transition. Repair the initial-reply boundary and its regression coverage while retaining quiet populated refresh/scrollback; do not change the fetching owner merely to fix presentation. The other reviewer’s held-read runtime result is attributed, not my execution.

The existing owner retains implementation. This initial-loading defect is separate from the WebKit sent-message visibility CI failure; #412 passing the latter at another head/base does not resolve this finding or establish causation. No approval, source edit, rerun, readiness change, or merge by this maintenance pass.

[wpfleger96]

🤖 hey @wesbillman, I think this needs two fixes before merge. I like the direction overall. The guards are small predicates on the existing owners, there's no replacement spacer or new loading abstraction, and the Sessions, workflow configs/runs/channels, members, and typed-search guards all keep their error and retry branches. Member additions also stay disabled until the fresh roster read succeeds.

Blocking: the first reply read of a thread opened from the timeline has no loading feedback. This is Kalvin's inline finding, which Brain has already confirmed, and I got the same result from the source. rows includes the seeded root (ThreadPanel.tsx:254-257), so snapshot.status === "loading" && !rows.length at :888 is already false during the initial read. On a slow relay the panel shows just the root, which reads as "no replies". That goes against the PR's own "initial empty loads still explain the wait". The new component case "shows initial thread loading only until content is available" asserts that exact transition (seed the root, expect no "Loading thread…"), so it needs to flip along with the fix. It should also cover a seeded root plus a held first read.

Blocking: in the cached offline state, the sidebar stays aria-busy="true" forever (inline comment on ChannelSidebar.tsx). Busy is preferences.status === "loading" || startup.updating. On a cached reload, the cachedOnly session's discover() returns early (store.ts:973), so the roster stays idle. That keeps namesReady false in useSidebarStartup.ts, and updating = ready && !settled stays true. When the handshake fails, service.ts keeps the same session and only publishes the error, so nothing can clear busy short of a successful reconnect. The new "Offline · Showing saved conversations" status and its Retry connection button end up inside a region that tells assistive tech to hold off on updates. That's the moment the announcement matters most. At base this wasn't reachable, because "Updating sidebar details…" was gated on !cached. The fix I'd suggest: derive busy only from work that is actually pending (or gate it on !cached the way the old notice was), and keep the offline status outside any busy region. startup.spec.mjs's cached-offline reload case could assert the aside isn't busy once Retry connection shows up.

Also worth fixing (lower risk)

• Explicit retries now have no visible progress on populated views, even though the description says explicit-action progress is preserved. Clicking Retry connection publishes connecting without the error, so the whole offline notice disappears. Nothing shows until the connection succeeds or the 8s deadline brings the notice back, and the composer stays disabled with no stated reason. Before this PR, that window showed "Reconnecting…". Both Retry thread buttons behave the same way: threads.ts clears error when it enters loading, so the row or button goes away, and with rows present nothing replaces it. Some in-place pending state on the control the user just clicked would keep the layout still and still answer "did my click do anything?".
• AGENTS.md asks for fail-then-pass evidence on the changed browser cases, and the description says none was collected. It would be good to have that for the geometry assertions that replaced the removed copy.
• CI: the red WebKit 5/6 shard is new-message.spec.mjs:289 (:665, viewport ratio 0). That same test fails in WebKit 5/6 on the main pushes 73f2a7cb and d7414652, and this PR doesn't touch that file, so I'm not attributing it here.

[wpfleger96]

🤖 In the cached offline state, startup.updating never settles. The cachedOnly session's discover() returns early (store.ts:973), so the roster stays idle, namesReady stays false, and updating stays true after the handshake fails. That leaves the offline status and Retry connection below permanently inside aria-busy="true". I'd gate this on !cached (like the old "Updating sidebar details…" notice) or derive it from work that's actually pending, and keep the offline status outside any busy region.

[wesbillman]

On Wes’s behalf — Brain:

Source-confirmed at 165c4ec4f32ab0c213dff9716d0ef2e4506441ce. The cached-only owner retains an idle roster (store.ts:145,973; exposed by session.ts:782,801), so restored content makes ready true while namesReady keeps startup.updating true (useSidebarStartup.ts:67–86). A failed handshake retains that owner (service.ts:280–285), leaving the enclosing aside busy after the offline status/Retry appears.

This is a semantic accessibility defect: ARIA permits assistive technology to defer changes inside a busy region. The earlier observation that buttons remain in Chromium’s accessibility tree does not establish that the status will be announced. No native screen-reader reproduction claimed here.

Keep offline/error recovery outside any indefinitely busy region and derive busy from actual pending work. Preserve the held-preference → rendered-settlement barrier: cached preferences in a connected session are not the same as connection.cached, which denotes the cached-only owner. Do not remove that coverage or change session/network ownership merely to clear this attribute. Extend the existing cached-offline startup scenario to assert busy has settled when Retry is visible, plus held-read and successful-recovery coverage. The original implementer retains this repair; this corroborates the existing finding rather than introducing a duplicate one.