refactor(chat): decompose ChatPage into composables#1989
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChatPage is decomposed into composables for message display, search, gestures, virtualization, session restoration, plan snapshots, and composer submission. Obsolete scroll code and related declarations/tests are removed, while async operations gain epoch, token, and session-view guards. ChangesChatPage decomposition
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ChatPage
participant useSessionRestore
participant SessionStore
participant MessageStore
ChatPage->>useSessionRestore: schedule session restore
useSessionRestore->>SessionStore: apply session summary
useSessionRestore->>MessageStore: load messages
useSessionRestore->>useSessionRestore: validate epoch and active view
useSessionRestore-->>ChatPage: expose restored state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/renderer/src/pages/chat-page/useDisplayMessages.ts (1)
45-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAlign the new feature-local composables with the renderer module-location rule.
All seven modules are introduced under
pages/chat-page, outside the listed responsibility directories. Relocate them to an approved directory, or update the repository rule if feature-local page modules are intentional.
src/renderer/src/pages/chat-page/useDisplayMessages.ts#L45-L45: relocateuseDisplayMessages.src/renderer/src/pages/chat-page/useChatSearch.ts#L31-L31: relocateuseChatSearch.src/renderer/src/pages/chat-page/useListGestures.ts#L31-L31: relocateuseListGestures.src/renderer/src/pages/chat-page/useMessageVirtualization.ts#L38-L38: relocateuseMessageVirtualization.src/renderer/src/pages/chat-page/useSessionRestore.ts#L29-L29: relocateuseSessionRestore.src/renderer/src/pages/chat-page/usePlanFloatLifecycle.ts#L32-L32: relocateusePlanFloatLifecycle.src/renderer/src/pages/chat-page/useComposerSubmit.ts#L84-L84: relocateuseComposerSubmit.As per coding guidelines, “In renderer app code, organize modules under
components/,stores/,views/,i18n/, andlib/according to their responsibility.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/src/pages/chat-page/useDisplayMessages.ts` at line 45, Feature-local composables are placed outside the renderer’s approved responsibility directories. Relocate useDisplayMessages in src/renderer/src/pages/chat-page/useDisplayMessages.ts#L45-L45, useChatSearch in src/renderer/src/pages/chat-page/useChatSearch.ts#L31-L31, useListGestures in src/renderer/src/pages/chat-page/useListGestures.ts#L31-L31, useMessageVirtualization in src/renderer/src/pages/chat-page/useMessageVirtualization.ts#L38-L38, useSessionRestore in src/renderer/src/pages/chat-page/useSessionRestore.ts#L29-L29, usePlanFloatLifecycle in src/renderer/src/pages/chat-page/usePlanFloatLifecycle.ts#L32-L32, and useComposerSubmit in src/renderer/src/pages/chat-page/useComposerSubmit.ts#L84-L84 into appropriate components/, stores/, views/, i18n/, or lib/ directories according to responsibility, updating all imports; alternatively, update the repository rule only if feature-local page modules are intentionally supported.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture/chatpage-decomposition/plan.md`:
- Around line 31-32: 更新该架构拆分计划中的回滚说明，移除“任一步回滚不影响其他步骤”的无条件表述；明确 composable
提取提交存在依赖关系，必须按提交的逆序回滚，或同时回滚所有依赖该 composable 的后续提交，避免破坏导入和编排。
In `@src/renderer/src/pages/chat-page/useComposerSubmit.ts`:
- Around line 154-176: Update prepareFilesForCurrentModel to capture the active
provider/model identity before awaiting getCapabilities, then recheck it against
the current selection after the await and before filtering or notifying. If the
selection changed, discard the stale capability result and retry using the
current selection, preserving the existing behavior for unchanged selections and
failures.
In `@src/renderer/src/pages/chat-page/useDisplayMessages.ts`:
- Around line 413-440: The message-building flow should not append an assistant
placeholder after adding the fallback streaming row. Track whether the `else if`
branch that pushes `toStreamingMessage` for `messageStore.streamingBlocks` ran,
and only evaluate the pending or generating placeholder branches when it did
not.
In `@src/renderer/src/pages/chat-page/useListGestures.ts`:
- Around line 219-223: Update resetIntentForSessionChange to reset the complete
gesture lifecycle, including cancelling or clearing the old session’s timer and
resetting isListGestureActive and isListScrolling, in addition to the existing
intent fields. Ensure no pending old-session callback can affect the new
scroll-controller session.
In `@src/renderer/src/pages/chat-page/useSessionRestore.ts`:
- Around line 54-70: Update restoreSessionMessages to handle rejections from
both loadMessagesForSession and pendingInputStore.loadPendingInputs. Catch
failures from the primary restore without leaving runRestore’s promise rejected,
and add rejection handling to the pendingInputsPromise chain while preserving
the requestId guard for onSecondaryStateReady and reporting or recovering from
each failure.
---
Nitpick comments:
In `@src/renderer/src/pages/chat-page/useDisplayMessages.ts`:
- Line 45: Feature-local composables are placed outside the renderer’s approved
responsibility directories. Relocate useDisplayMessages in
src/renderer/src/pages/chat-page/useDisplayMessages.ts#L45-L45, useChatSearch in
src/renderer/src/pages/chat-page/useChatSearch.ts#L31-L31, useListGestures in
src/renderer/src/pages/chat-page/useListGestures.ts#L31-L31,
useMessageVirtualization in
src/renderer/src/pages/chat-page/useMessageVirtualization.ts#L38-L38,
useSessionRestore in
src/renderer/src/pages/chat-page/useSessionRestore.ts#L29-L29,
usePlanFloatLifecycle in
src/renderer/src/pages/chat-page/usePlanFloatLifecycle.ts#L32-L32, and
useComposerSubmit in
src/renderer/src/pages/chat-page/useComposerSubmit.ts#L84-L84 into appropriate
components/, stores/, views/, i18n/, or lib/ directories according to
responsibility, updating all imports; alternatively, update the repository rule
only if feature-local page modules are intentionally supported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b4475df8-033a-449a-91a2-43f7667319f5
📒 Files selected for processing (15)
docs/architecture/chatpage-decomposition/plan.mddocs/architecture/chatpage-decomposition/spec.mddocs/architecture/chatpage-decomposition/tasks.mdsrc/renderer/src/composables/message/types.tssrc/renderer/src/composables/message/useMessageScroll.tssrc/renderer/src/pages/ChatPage.vuesrc/renderer/src/pages/chat-page/useChatSearch.tssrc/renderer/src/pages/chat-page/useComposerSubmit.tssrc/renderer/src/pages/chat-page/useDisplayMessages.tssrc/renderer/src/pages/chat-page/useListGestures.tssrc/renderer/src/pages/chat-page/useMessageVirtualization.tssrc/renderer/src/pages/chat-page/usePlanFloatLifecycle.tssrc/renderer/src/pages/chat-page/useSessionRestore.tstest/renderer/composables/chat/chatScrollArchitecture.test.tstest/renderer/composables/useMessageScroll.test.ts
💤 Files with no reviewable changes (4)
- test/renderer/composables/useMessageScroll.test.ts
- src/renderer/src/composables/message/useMessageScroll.ts
- src/renderer/src/composables/message/types.ts
- test/renderer/composables/chat/chatScrollArchitecture.test.ts
Collaborator
Author
Review responseThis PR is a behavior-zero-drift decomposition — every concern was moved line-for-line out of Actionable comments — verified as pre-existing, intentionally preserved (no change)All five describe behavior that already existed on the base and was faithfully carried over. "Fixing" any of them here would introduce the exact behavior drift this refactor is meant to avoid; they're better raised as separate hardening PRs.
Doc / structure comments — addressed
Validation
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ChatPage.vue<script setup>into seven focused composables undersrc/renderer/src/pages/chat-page/, each owning one concern and its own race-guard epoch/token. Main file drops to ~1710 lines (−44%).sessionRestoreRequestId,attachmentFilterToken,isChatPageActive) with self-contained closures inside the owning composable.composables/message/useMessageScroll.ts(339 lines, zero references, old vue-virtual-scroller path) + its orphan test + theScrollInfotype.onSubmit/onQueueSubmit/onSteer/onCommandSubmit) had their duplicated guards collapsed into a singlecanSubmitNow()with identical semantics.Composables extracted
useSessionRestorerestoreRequestIdepoch,canWriteSessionView, deferred startup restoreuseDisplayMessagesdisplayMessageCache,assistantRenderKeyByMessageId,pendingAssistantPlaceholderuseMessageVirtualizationpendingMeasureQueue, rAF flushuseListGesturesisListScrollingusePlanFloatLifecycleplanSnapshotClearTimersuseChatSearchlib/chatSearch)useComposerSubmitattachmentFilterTokenNotable notes
useAssistantPlaceholderwas merged intouseDisplayMessagesrather than split out: its renderKey handoff writes the sameassistantRenderKeyByMessageIdmap the conversion cache reads, and its show/hide depends onhasFirstStreamingContent/ephemeralRateLimitBlock. A single owner keeps the boundary clean. Decision recorded in the spec.refto a plain object, so the float would vanish instantly instead of lingering 1200ms after a session completes. Fixed infix(chat): restore plan float linger reactivity; everything else verified drift-free.useChatScrollController,chatScrollState,chatScrollOperationArbiter,useMessageWindow) is unchanged — only ChatPage's glue calls were regrouped.pnpm run i18n).Test plan
pnpm run typecheck(node + web)pnpm run format:check/pnpm run lint/pnpm run i18nChatPage.test.ts— 82/82 passtest:renderer— the 18 failures are pre-existing ondev(Settings/Provider deeplink + sidepanel), reproduced on a cleanorigin/devcheckout; unrelated to this branch.Architecture
See
docs/architecture/chatpage-decomposition/spec.md,plan.md,tasks.md.Summary by CodeRabbit
Architecture
Bug Fixes
Documentation
Tests