Skip to content

test(desktop): await thread scroll anchor - #3174

Merged
wesbillman merged 2 commits into
mainfrom
grumplestiltzkin/thread-focus-scroll-flake
Aug 6, 2026
Merged

test(desktop): await thread scroll anchor#3174
wesbillman merged 2 commits into
mainfrom
grumplestiltzkin/thread-focus-scroll-flake

Conversation

@cameronhotchkies

Copy link
Copy Markdown
Collaborator

Why

The focus/split E2E test could capture the thread root before its programmatic middle-thread scroll had settled, then incorrectly report a scroll-restoration failure.

What

  • Poll until the requested middle-thread scroll position is applied
  • Require the captured anchor to intersect the thread viewport and differ from the root
  • Preserve the existing focus-to-split-to-focus viewport assertions

Risk Assessment

Low — test-only synchronization change with no production behavior changes.

References

Generated with Codex

@cameronhotchkies
cameronhotchkies marked this pull request as ready for review July 27, 2026 20:10
@cameronhotchkies
cameronhotchkies requested a review from a team as a code owner July 27, 2026 20:10
@dophsquare dophsquare added the triage-ready Appropriate for agentic review label Jul 28, 2026
Co-authored-by: npub1x4hk035p3p9q39a3fcrd2fe30lpkrhr5dwe0cqzzjphxyyh8m0gsq4vqap <356f67c681884a0897b14e06d527317fc361dc746bb2fc0042906e6212e7dbd1@buzz.block.builderlab.xyz>
Signed-off-by: npub1x4hk035p3p9q39a3fcrd2fe30lpkrhr5dwe0cqzzjphxyyh8m0gsq4vqap <356f67c681884a0897b14e06d527317fc361dc746bb2fc0042906e6212e7dbd1@buzz.block.builderlab.xyz>
@cameronhotchkies
cameronhotchkies force-pushed the grumplestiltzkin/thread-focus-scroll-flake branch from f9de570 to 0f89a41 Compare July 30, 2026 18:25

@atishpatel atishpatel 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.

Approving — test-only synchronization fix, verified locally.

What I checked

  • Reviewed the diff: topVisibleMessageIdscrollToMiddleVisibleMessage, which (a) sets the 40% scroll inside the poll and re-checks element.scrollTop landed within 1px in the same synchronous evaluate, (b) requires the captured row to intersect the viewport on both edges, (c) rejects the thread root. That is exactly the failure mode in the linked run: the anchor was captured at scrollTop === 0, so the root became the anchor and the later restoration assertion legitimately failed on an illegitimate anchor.
  • Not masking a product bug: the position check is synchronous with the assignment, so the anchor is genuinely a ~40% row. If restoration were broken (or the app snapped back to bottom asynchronously afterward), the preserved toBeInViewport() assertions after focus→split→focus would still fail. The three original viewport/highlight assertions are untouched.
  • Rebased the diff cleanly onto origin/main (96ae141) in a scratch worktree — the branch is 159 commits behind but thread-focus-mode.spec.ts has no conflicting change on main since cca88390.
  • pnpm exec tsc --noEmit: clean. pnpm check (biome + file-size/px-text/pubkey guards): clean, no new findings.
  • playwright test --project=smoke thread-focus-mode.spec.ts --repeat-each=8: 16/16 passed, no flaky, on the rebased tree.

Nits, non-blocking

  • expect.poll with a mutating callback is slightly unusual (the poll body performs the scroll rather than just observing it). It's correct here — the retry is the re-scroll — but a one-line comment saying so would help the next reader.
  • The anchorId closure write plus the post-poll if (!anchorId) throw is redundant with the poll predicate; harmless.

Nothing blocking. Please rebase/let the merge queue bring it up to date before landing.

@wesbillman
wesbillman enabled auto-merge (squash) August 6, 2026 14:56
@wesbillman
wesbillman merged commit 9213090 into main Aug 6, 2026
26 checks passed
@wesbillman
wesbillman deleted the grumplestiltzkin/thread-focus-scroll-flake branch August 6, 2026 15:04
wpfleger96 pushed a commit that referenced this pull request Aug 6, 2026
…-agents-nav

* origin/main:
  Refine agent runtime controls (#5026)
  test(desktop): await thread scroll anchor (#3174)
  Improve desktop mobile pairing flow (#5024)
  feat(desktop): show selected community in rail (#5000)
  fix(desktop): stop rate-limited reconnect backfill from tearing down the authenticated socket (#4990)
  fix(desktop): skip native notifications outside app bundles (#5004)
  ci: prove the relay-driven mesh lifecycle — discover, join, infer, deny — with real nodes (#3862)
  fix(desktop): virtualize channel member lists (#4991)
  fix(desktop): enforce owner-only access in internal builds (#4053)
  test(desktop): match attachment button label (#4993)
  fix(acp): pace observer telemetry at 1/s with per-channel batch envelopes (#4917)
  fix(desktop): enable the content security policy (#4614)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
wpfleger96 pushed a commit that referenced this pull request Aug 6, 2026
…n-surface

* origin/main: (44 commits)
  fix(buzz-agent): classify read timeouts distinctly in LLM error messages (#4959)
  Refine agent runtime controls (#5026)
  test(desktop): await thread scroll anchor (#3174)
  Improve desktop mobile pairing flow (#5024)
  feat(desktop): show selected community in rail (#5000)
  fix(desktop): stop rate-limited reconnect backfill from tearing down the authenticated socket (#4990)
  fix(desktop): skip native notifications outside app bundles (#5004)
  ci: prove the relay-driven mesh lifecycle — discover, join, infer, deny — with real nodes (#3862)
  fix(desktop): virtualize channel member lists (#4991)
  fix(desktop): enforce owner-only access in internal builds (#4053)
  test(desktop): match attachment button label (#4993)
  fix(acp): pace observer telemetry at 1/s with per-channel batch envelopes (#4917)
  fix(desktop): enable the content security policy (#4614)
  fix(mobile): merge relay recounts with locally seen thread replies (#4633)
  fix(desktop): enable message editing in Inbox (#2198)
  relay: fuzz WebSocket 1012 restart-close timing on graceful drain (BUZZ_DRAIN_JITTER_MS) (#4542)
  fix(desktop): outline the selected community (#4969)
  fix(desktop): clamp thread panel to channel surface (#4965)
  style(messages): increase username contrast (#4948)
  fix(desktop): rename generic attachment action from 'Attach image' to 'Attach file' (#2381) (#4304)
  ...

# Conflicts:
#	desktop/src-tauri/src/commands/mod.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-ready Appropriate for agentic review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants