refactor(web): ChatView's send preparation is pure functions in ChatView.logic#22734

merged2
refactor/chatview-draft-environment-routing
refactor/chatview-send-plan
opened3d agobyJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 3d ago

onSend is about 1,200 lines. Its preparation was written inline, and some of it twice:

  • copying what the composer held at send time;
  • planning the dispatch: worktree base branch, compact-before-send, and queueing;
  • binding attachment ids for context records;
  • the inline-context fallback for older servers;
  • the optimistic row's attachments and the title fallbacks;
  • resolving one thread per model for a multiple-model send.

These are now pure functions:

  • In ChatView.logic.ts: snapshotComposerSend, planComposerTurn, boundAttachmentIds, withMessageContext, optimisticMessageAttachments and sendTitleFallbackLabels. All have tests except the last two.
  • In chat/chatViewSend.ts: resolveMultipleModelTargets, which lives there because composerProviderState pulls in picker components. ChatView still validates each target's text with the composer in selection order and stops at the first problem, as before.

The two places that put a failed send back into the draft now call one restoreComposerDraft helper.

The dispatch itself stays in ChatView. That covers uploads, the dock transition, the multiple-model start loop, the turn start, and failure recovery. Moving any of it would need a 30-field input object, which wouldn't make it easier to read. This layer adds no useStableCallback calls or layout-effect ref assignments, and doesn't change render boundaries.

previous layerthis layer
ChatView.tsx lines7,3727,258

Tests: vp test run src/reactCompilerCoverage.test.ts for ChatView and every new file (0 bailouts), vp test run src/components/ChatView.logic.test.ts (5 new cases), tsc-rs --noEmit for apps/web, and oxlint on the changed files.

🤖 Generated with Claude Code

Checks

All checks passed

Added 2 commits

  • refactor(web): ChatView's send preparation is pure functions in ChatView.logic35322b3
  • refactor(web): multiple-model send targets resolve in resolveMultipleModelTargets9fdc654
Synced from GitHub

Thread transfer impact

⚠️ The latest CI run did not produce a thread transfer result for 9fdc654.

This comment will update automatically after the next completed run.

Synced from GitHub

Approvability

Verdict: Approved at 9fdc654

Macroscope's review found this PR approvable — This is a component-local refactor that extracts existing ChatView send-preparation logic into pure helpers and consolidates draft recovery, while leaving uploads and dispatch behavior in place. Added unit coverage exercises the key extracted decisions, and no product defaults, schemas, or static-analysis settings change.

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

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: fba68e0f-0a1a-4d6b-9cc7-40fd658f2007
📥 Commits

Reviewing files that changed from the base of the PR and between 7483af891cd7a34bc9f3c661a49d3af59b42574b and 9fdc654554c96a2d9bbb78957cb34a7dd6d129d4.

📒 Files selected for processing: 4
  • apps/web/src/components/ChatView.logic.test.ts
  • apps/web/src/components/ChatView.logic.ts
  • apps/web/src/components/ChatView.tsx
  • apps/web/src/components/chat/chatViewSend.ts

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

📝 Walkthrough

The chat send flow now captures composer state in a shared snapshot, plans dispatch and context handling through shared helpers, resolves multiple-model targets centrally, and uses common attachment, optimistic-message, title-fallback, and draft-restoration logic.

Changes: Composer send flow

Layer / File(s)Summary
Composer snapshot and dispatch helpers
apps/web/src/components/ChatView.logic.ts, apps/web/src/components/ChatView.logic.test.ts
Added helpers to snapshot composer content, plan worktree and queue behavior, bind attachment IDs, construct message context, project optimistic attachments, and derive title fallback labels. Tests cover snapshot expiration, dispatch planning, context formats, and attachment ID selection.
Multi-model target resolution and draft restoration
apps/web/src/components/chat/chatViewSend.ts
Added target resolution for selected models, including provider and model checks, prompt formatting, and interaction-mode selection. Added restoration of captured composer content after a failed send.
Chat send flow integration
apps/web/src/components/ChatView.tsx
Updated single-thread and multi-model sends to use shared snapshot, planning, target-resolution, context, attachment, title, and draft-restoration helpers. Removed duplicated inline send logic.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to 9fdc6

The reviewed send paths retain the expected terminal-context and attachment behavior. No actionable merge-blocking issue remains after normal checks.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description explains the refactor, its scope within ChatView, the retained dispatch behavior, and the verification performed. It does not provide the required Scope and approval section, and the P…Add the required Problem and Scope and approval sections. State the expected behavior and reproduction context, then link the triaged issue or maintainer approval with the approval comment, or explain why this focused refactor qualifies for…
Passed Passed checks: 3 passed
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely identifies the main change: moving ChatView send preparation into pure functions in ChatView.logic.
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 refactor, its scope within ChatView, the retained dispatch behavior, and the verification performed. It does not provide the required Scope and approval section, and the Problem content is not labeled with the template heading.

Resolution

Add the required Problem and Scope and approval sections. State the expected behavior and reproduction context, then link the triaged issue or maintainer approval with the approval comment, or explain why this focused refactor qualifies for an exemption.

✨ 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.