Fix mobile remote session lag caused by hidden thread outline refreshes - #1214
Conversation
wjin17
changed the title
|
🚨 SLOP COP 🚨 · I am SlopCop. I am reviewing this pull request for security, code quality, performance, architecture, and product behavior. |
| // clients from rebuilding an invisible outline on every appended-event batch. | ||
| const outlineQuery = useThreadConversationOutline(threadId, { | ||
| enabled: timelineRows.length > 0, | ||
| enabled: tocVisible && timelineRows.length > 0, |
There was a problem hiding this comment.
🚨 slopcop/review — The query gate uses a different width box than CSS.
host.clientWidth includes 24 pixels of horizontal padding. The @container rule checks the content-box width.
This difference enables the outline query at host widths from 896 through 919 pixels. CSS still hides the table of contents.
I reproduced the issue with a 900-pixel host. Its content width was 876 pixels, and CSS hid the table of contents. However, JavaScript mounted the table-of-contents button.
Realtime events can therefore rebuild a hidden outline in this range. Please measure the content-box width and add boundary tests.
A 900-pixel padded host must disable the query. A 920-pixel padded host must enable the query.
SawyerHood
left a comment
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
ELI5
This patch stops phones from requesting a menu that they cannot show. It works on phones, but it misses a narrow split-pane range.
Finding
I found one P2 issue. The JavaScript gate and the CSS breakpoint use different width boxes.
JavaScript includes 24 pixels of host padding. CSS checks only the content width.
Host widths from 896 through 919 pixels still request the outline. CSS hides the table of contents in that range.
I reproduced this issue with a 900-pixel host and a 876-pixel content box. CSS hid the table of contents, but JavaScript mounted its button.
Please use the content-box width for the query gate. Add boundary tests for 900 and 920-pixel padded hosts.
Other checks
The security review found no issue. The codebase search found no duplicate table-of-contents visibility helper.
The focused test passed all 20 tests. The app type check and git diff --check also passed.
The browser test made no outline request with a 390-pixel pane. A 1,120-pixel pane made the expected request.
I posted this result as a comment review. I did not approve the pull request or request changes.
`clientWidth` counts the scroll overlay's 24px of horizontal padding, but the `@container` rule resolves against the content box. Host widths from 896 through 919px therefore enabled the outline query while CSS still hid the table of contents, so realtime events kept rebuilding a hidden outline in that split-pane band. Subtract the computed horizontal padding before comparing against the breakpoint, and cover both boundaries with padded-host tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Addressed the SlopCop finding in 1344b2e. The gate now measures the container-query width. Added two boundary tests with a padded host: a 900px host stays disabled, and a 920px host enables the query. The 900px test fails against the previous Validation: |
2 participants
Summary
Root cause
The thread TOC is hidden below its 56rem container breakpoint, but its outline query was enabled whenever timeline rows existed. Because realtime
events-appendednotifications invalidate that query, mobile clients repeatedly fetched and rebuilt the full conversation outline even though they could not display it. Remote browser sessions paid both the server projection cost and tunnel latency for every request.Performance
I ran a controlled A/B test through the same phone and bb Connect tunnel using two fresh threads per variant. Every run used Claude Code's default model and the same prompt: 20 sequential tool calls followed by a fixed-length response. Both variants stored 110 events across the two runs.
mainThe individual upstream runs made 108 and 100 outline requests; both patched runs made zero. Run duration remained effectively equal because model generation dominates wall time.
During the original remote-session diagnosis, a 39.1-hour traffic capture contained 1,184 outline requests, 4.61 MB of compressed responses, and 62.26 seconds of aggregate origin request time, with a 198.9 ms p95. This change eliminates that request path whenever the TOC is hidden by the compact-pane breakpoint.
Validation
pnpm exec turbo run test --filter=@bb/app --force— 324 files and 2,447 tests passedpnpm exec turbo run test --filter=@bb/app --force -- src/components/thread/toc/ThreadTableOfContents.test.tsx— 20 tests passedpnpm exec turbo run typecheck --filter=@bb/apppnpm exec turbo run lint --filter=@bb/app— 0 errorspnpm exec turbo run build --filter=@bb/appgit diff --checkThe new regression fails against the previous enablement condition (
enabled: trueat 400px) and passes with this change (enabled: false).