Synced from GitHub
refactor(web): ChatView's timeline banners render in ChatBannerLane#23101
Both timeline banners are memoized, but they re-rendered on every ChatView render anyway. ChatView built their onDismiss closures inline, and the React Compiler does not cache that part of ChatView's JSX, so each render handed the banners new handlers.
ChatBannerLane takes the plain values (the visible status, the banner keys, the thread id, the error and its class) plus the stable setters, and builds the two dismiss handlers itself. The lane still renders whenever ChatView does, because it gets a new element with equal props. Its compiled cache keeps both banners and their subtrees from re-rendering.
The provider banner's dismissed key and the error banner's dismiss tick stay in ChatView. hasTimelineTopBanner reads the same values to decide on the timeline's top fade.
Renders (median of 3):
| Interaction | before: total | after: total | before: banner renders | after: banner renders |
|---|---|---|---|---|
| Type 20 chars | 170 | 169 | 2 | 0 |
| Streaming reply (about 110 commits) | 2,889 | 2,866 | 58 | 0 |
| Thread switch and back | 2,824 | 1,999 | 36 | 0 |
| Right panel open and close | 485 | 481 | 8 | 0 |
"Banner renders" counts ProviderStatusBanner plus ThreadErrorBanner. Streaming and thread switch totals swing by about ±20% between runs, because response length and history loading vary. The per-component counts are the reliable signal.
| File | before | after |
|---|---|---|
| ChatView.tsx | 3,899 | 3,893 |
| chat/ChatBannerLane.tsx | 57 |
Tested:
- apps/web typecheck and lint on the touched files.
ThreadErrorBanner.test.tsxandProviderStatusBanner.test.tspass.- React Compiler: 0 bailouts on ChatView and ChatBannerLane. ChatView stays in the slow set.
- Hands-on in a dev build: the lane is still mounted above the timeline on server threads and empty when nothing is wrong. I could not produce a real provider-status or thread-error banner in the dev environment, so I did not click the dismiss buttons by hand.
🤖 Generated with Claude Code
Checks
- SuccessfulCodeRabbitGitHub
Review completed
- Successful
Skipped - Not affected
- Skipped
- Skipped
- Skipped
- SuccessfulRemove preview downloadGitHub
- Successful
- Skipped
- SuccessfulLabel PR sizeGitHub
- SuccessfulPrepare PR size configGitHub
- SuccessfulLabel PR 23101GitHub
- Successful
- SuccessfulCollect PR targetsGitHub
- Successful
- SuccessfulCheckGitHub
- Skipped
- SkippedRustGitHub
- SkippedRelease SmokeGitHub
- SuccessfulBuild and TypecheckGitHub
- SuccessfulTest WebGitHub
- SuccessfulTestGitHub
- SuccessfulLabel PR 23101GitHub
- Skipped
- SkippedDeploy web previewGitHub
- SuccessfulLabel PR sizeGitHub
- SuccessfulCollect PR targetsGitHub
- SkippedEAS PreviewGitHub
- SuccessfulPrepare PR size configGitHub
- SuccessfulTest Server 5GitHub
- SuccessfulTest Server 2GitHub
- SuccessfulTest Server 6GitHub
- SuccessfulTest Server 4GitHub
- SuccessfulNative fingerprint diffGitHub
- SuccessfulTest Server 8GitHub
- SuccessfulChangesGitHub
- SuccessfulTest Server 1GitHub
- SuccessfulTest Server 7GitHub
- SuccessfulTest Server 3GitHub
- SuccessfulLintGitHub
Added 1 commit
- refactor(web): ChatView's timeline banners render in ChatBannerLane54349ea
Synced from GitHub
Approvability
Verdict: Approved at 54349ea
Macroscope's review found this PR approvable — This refactor extracts the existing timeline banner markup and dismissal handlers into ChatBannerLane without changing their placement, inputs, or behavior. It introduces no new capability, schema, configuration, or production workflow.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
- Configuration used: Path: .coderabbit.config.ts
- Review profile: CHILL
- Plan: Team
- Run ID:
1e9aad87-38aa-4382-8b4e-0cf627e6f8dc
📥 Commits
Reviewing files that changed from the base of the PR and between 300d3ced33bc5b8afd5a823cd7bd27e9102db4c3 and 54349ea3e2f4799c90fdb701bfe12e55526bd618.
📒 Files selected for processing: 2
apps/web/src/components/ChatView.tsxapps/web/src/components/chat/ChatBannerLane.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.
📝 Walkthrough
ChatView now uses ChatBannerLane to render provider-status and thread-error banners. The new component receives banner state and callbacks, and handles provider and thread-error dismissals.
Changes: Chat banner rendering
Layer / File(s) Summary Extract and wire the chat banner lane
apps/web/src/components/chat/ChatBannerLane.tsx,apps/web/src/components/ChatView.tsxChatBannerLane renders the provider-status and thread-error banners. It handles their dismissal callbacks. ChatView passes the banner state, setup navigation, and error-management callbacks to the component. Priority: ⬇️ Low
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Refactor
Suggested reviewers:
t3dotgg
Merge Risk: ⚪ Minimal · up to 54349
The banner extraction preserves the existing behavior, with no identified issue blocking merge.
Pre-merge checks: Passed 3 · Failed 1
Failed Failed checks: 1 warning
Check name Status Explanation Resolution Description check Warning The description explains the change and includes detailed verification results, but it omits the required Scope and approval section. It also does not provide the required before-and-after screenshots… Add a Problem section with the expected behavior and problem context. Add a Scope and approval section with the linked issue or maintainer approval, or explain why this focused change qualifies for an exemption. Add clear before-and-after s… Passed Passed checks: 3 passed
Check name Status Explanation Title check Passed The title clearly and concisely describes the main change: moving ChatView timeline banner rendering into ChatBannerLane. Linked Issues check Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check
Explanation
The description explains the change and includes detailed verification results, but it omits the required Scope and approval section. It also does not provide the required before-and-after screenshots for this UI change. The problem statement is present but not clearly structured under the required Problem heading.
Resolution
Add a Problem section with the expected behavior and problem context. Add a Scope and approval section with the linked issue or maintainer approval, or explain why this focused change qualifies for an exemption. Add clear before-and-after screenshots, and include a recording if interaction behavior requires it.
✨ Finishing Touches
📝 Generate docstrings
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
- Commit to this branch
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.