perf(app): gate right-panel context and files tabs when inactive#854
Conversation
Wrap SessionContextTab and FilesTab in <Show when={sidePanelTab() === ...}>
to match the existing terminal-tab pattern. Inactive tab content fully
unmounts instead of staying live, so memos, effects, file.load(), and
platform.statPaths no longer run when the user is on another tab.
The tab strip itself is unaffected.
Refs #602
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 31 minutes and 22 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/pages/session/session-side-panel.tsx)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
There was a problem hiding this comment.
Code Review
This pull request modifies the SessionSidePanel component to conditionally render the FilesTab and SessionContextTab using the Show component based on the active tab state. This change likely improves performance by preventing unnecessary mounting of tab content when it is not visible. I have no feedback to provide as there are no review comments.
Perf delta summaryComparator: pass
|
Add source-grep contract tests in session-side-panel.test.tsx that assert each tab body (files, context, terminal) sits inside a Show wrapper keyed to its sidePanelTab id. Matches the project pattern in no-mode-picker.test.ts. Catches the most likely regressions: tab-id typos, copy-paste mistakes, and any future change that drops the Show wrap. A full render test for these wrappers would need to mock the entire SessionSidePanel context surface; source-grep covers the same regression shapes at much lower maintenance cost. Refs #602 Addresses P3 review feedback on #854
|
Re: P3 — added a source-grep contract test in This catches the regression shapes you flagged:
Also covers the terminal tab so it can't be removed alongside the new ones. What this does NOT cover: Kobalte Pushed as |
Summary
Wrap
SessionContextTabandFilesTabin<Show when={sidePanelTab() === ...}>so inactive right-panel tab content fully unmounts instead of staying live.Two-line change at
session-side-panel.tsx:411-413andsession-side-panel.tsx:543-545, matching the existing terminal pattern at line 531.Why
Area B right-panel audit (#602) found that Context and Files tab content stayed mounted while the user was on another tab, doing real work:
SessionContextTabrecomputesmetrics/breakdown/systemPromptmemos and queues arequestAnimationFrameon every message-length changeFilesTabrunsfile.load()for each previewable artifact and callsplatform.statPathswheneverprops.fileschanges — andprops.filesitself reacts to Review state, so this fires while users review diffsThis contributes to the typing-lag observation in #615. The other right-panel tabs (Status, Review inner, Terminal) already have explicit guards; Context and Files were the two arms that slipped through.
The fix is not "completely stop all Context-related work" — the tab strip itself, including any per-tab indicators, is unaffected. Only the tab body content unmounts.
Related Issue
Refs #602 (Area B right-panel rewrite)
Contributes to #615 (session UI typing lag)
Human Review Status
Pending
Review Focus
view().scroll("context")store; Files scroll persistence is not currently implemented and can be added separately if it becomes user-visible.Risk Notes
How To Verify
Manual desktop walk-through of all four tabs (Status / Files / Review / Context) deferred — change is purely additive control flow that mirrors the existing terminal pattern (line 531), and the active-tab render path is unchanged. Reviewer is welcome to request a
dev:desktoprecording if tab-switching dynamic verification is desired.Screenshots or Recordings
docs/design/preview/screenshots/app-shell.pngregenerated; no visible diff from baseline. Default app-shell snap does not exercise the right panel in an open state, so this confirms no regression to the default surface but does not visually demonstrate the gated tabs.Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.