Sign up
View stargazers13Sign up

refactor(web): ChatView's terminal actions live in useChatTerminalActions#23063

merged1
refactor/split2-chatview-send
refactor/split2-chatview-terminal
opened2d agobyJJulius Marminge
2 files changed
View on GitHubTwo-way sync enabledPR last imported 2d ago

Drawer and right-panel terminal actions, project scripts, and chat code blocks move into chat/useChatTerminalActions.ts, with the terminal store selectors, terminal session actions, and the last-invoked-script storage they use.

One small cleanup: runShellCommand called the script runner through a ref kept fresh by a layout effect. The runner is already a stable callback that reads the latest render, so it now calls it directly and the layout-effect ref assignment is gone.

BeforeAfter
ChatView.tsx lines5,7005,420

Tests: tsc-rs --noEmit for apps/web, oxlint on the changed files, vp test run src/components/ChatView.logic.test.ts, and the React Compiler coverage test for every changed file (0 bailouts). ChatView stays in the coverage test's slow set.

Part of the ChatView split (7,258 → 3,975 lines across the stack).

Opus 5.5, Claude Code.

🤖 Generated with Claude Code

Checks

All checks passed
MmacroscopeappBot2d ago
Synced from GitHub

Approvability

Verdict: Approved at 156d60f

Macroscope's review found this PR approvable — This is a focused refactor that moves ChatView’s existing terminal lifecycle, script execution, and preview actions into a dedicated hook while preserving the existing call-site wiring and side effects. No product defaults, schemas, deployment behavior, or static-analysis configuration are changed.

No code changes detected at 089570e. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot2d ago
Synced from GitHub

Review in Change Stack →

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: 191cd967-bbb5-4ebc-ad69-fe508372462c
📥 Commits

Reviewing files that changed from the base of the PR and between 156d60f8c23ea2ffb07c2ce0a6df52bd74ebc21f and 089570e0a46fc41404d2402da2d54826d55380d9.

📒 Files selected for processing: 1
  • apps/web/src/components/ChatView.tsx

Limit details: You’ve used all 10 included reviews currently available.

📝 Walkthrough

Terminal and project-script actions move from ChatView into useChatTerminalActions. ChatView supplies the hook inputs and uses its returned terminal callbacks.

Changes: Chat terminal actions

Layer / File(s)Summary
Centralize terminal actions
apps/web/src/components/chat/useChatTerminalActions.ts		The new hook manages terminal creation, splitting, activation, closure, script and shell-command execution, preview opening, context insertion, focus, access checks, and failure reporting.
Connect ChatView to the hook
apps/web/src/components/ChatView.tsx		ChatView connects the hook to active thread, project, panel, terminal, permission, focus, launch-context, and error state. Local terminal action logic and related helpers are removed.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to 08957

This refactor moves terminal and project-script actions out of ChatView into a dedicated hook. Inspection found no behavior change or defect, so it looks safe to merge.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description explains the implementation and lists focused verification, but it omits the required Problem and Scope and approval sections. It also does not provide issue or maintainer approval det…Add a Problem section with the user or engineering problem and expected behavior. Add a Scope and approval section with the triaged issue or discussion link and explicit maintainer approval, or explain why an exemption applies. Keep the exi…
Passed Passed checks: 3 passed
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main refactor: moving ChatView terminal actions into useChatTerminalActions.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the implementation and lists focused verification, but it omits the required Problem and Scope and approval sections. It also does not provide issue or maintainer approval details.

Resolution

Add a Problem section with the user or engineering problem and expected behavior. Add a Scope and approval section with the triaged issue or discussion link and explicit maintainer approval, or explain why an exemption applies. Keep the existing Change and Verification details.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
📝 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

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Comment @coderabbitai help to get the list of available commands.

Added 1 commit

  • refactor(web): ChatView's terminal actions live in useChatTerminalActions089570e
Ggithub-actionsBot2d ago
Synced from GitHub

Thread transfer impact

⚠️ The latest CI run did not produce a thread transfer result for 089570e.

This comment will update automatically after the next completed run.