[Feat] ChatViewModel 및 TyprWriterFlow 작성 - #86
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthrough채팅 상태 모델과 Changes채팅 흐름
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
Collaborator
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
feature/chat/src/main/java/com/kikidan/chat/ChatViewModel.kt (2)
105-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value[P3] 변수 섀도잉과 메서드명 오타를 정리하십시오.
Line 108의 지역 변수
result가 Line 105 람다 파라미터result를 가립니다. 이름을 분리하십시오. 또한onSteamingDone은onStreamingDone의 오타입니다(Line 138, 179).♻️ 제안 수정
is ChatStreamEvent.Start -> { - val result = onStreamingStart(placeholder, event) - streamConversationId = result.first - assistantMessageId = result.second + val (startedConversationId, startedAssistantId) = + onStreamingStart(placeholder, event) + streamConversationId = startedConversationId + assistantMessageId = startedAssistantId }Also applies to: 179-179
🤖 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 `@feature/chat/src/main/java/com/kikidan/chat/ChatViewModel.kt` around lines 105 - 110, Rename the local `result` inside the `ChatStreamEvent.Start` branch of the `.transform` lambda to avoid shadowing the lambda parameter, and update all references to it. Correct the misspelled `onSteamingDone` method references to `onStreamingDone` at both call sites.
33-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win[P2]
conversationId와 작성 중input의 SavedStateHandle 적용을 검토하십시오.프로세스가 종료되면 현재 대화 식별자와 작성 중 입력값이 사라집니다. 두 값은 복원 대상입니다.
SavedStateHandle로conversationId를 받아 초기 상태에 반영하고,input저장도 함께 검토하십시오.이 지적은 코딩 가이드 기준입니다. As per path instructions: "상태 보존이 필요한 값은 SavedStateHandle 사용을 검토하세요."
🤖 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 `@feature/chat/src/main/java/com/kikidan/chat/ChatViewModel.kt` around lines 33 - 38, Update the ChatViewModel state initialization and load flow to use SavedStateHandle for the restorable conversationId, applying its saved value to the initial ChatState and persisting updates when load receives a new ID. Also persist and restore the in-progress input value through the same handle, keeping the existing state updates intact.Source: Path instructions
feature/chat/src/test/java/com/kikidan/chat/ChatViewModelTest.kt (1)
344-385: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win[P2]
onSendClick경로 테스트가 없습니다.전송 테스트는 모두
onSuggestionClick을 사용합니다.onSendClick의 입력 초기화 순서와 공백 입력 처리는 검증되지 않습니다. 다음 두 케이스를 추가하십시오.
- 스트리밍 중
onSendClick호출 후input이 유지되는지 확인합니다.- 공백만 입력한 상태에서
onSendClick호출 시sendCallCount가 0인지 확인합니다.테스트 코드를 작성해 드릴까요?
🤖 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 `@feature/chat/src/test/java/com/kikidan/chat/ChatViewModelTest.kt` around lines 344 - 385, Add tests covering the onSendClick path in ChatViewModelTest: verify that invoking onSendClick during streaming preserves the current input, and verify that onSendClick with whitespace-only input does not call the repository, keeping sendCallCount at zero. Reuse the existing viewModel, FakeChatRepository, and vm.test setup.
🤖 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 `@feature/chat/src/main/java/com/kikidan/chat/ChatViewModel.kt`:
- Around line 143-152: Update localUserMessage creation to use a pending status
instead of MessageStatus.COMPLETED, then transition it to COMPLETED when
ChatStreamEvent.Start is received. In the Throwable catch block, mark the
optimistic message as failed while preserving the existing streaming state reset
and error side effect so the resend UI can appear.
- Around line 65-70: Update onSendClick and send so input normalization and
clearing occur inside send: trim the content once on entry, return without
clearing or sending when it is blank or while streaming, and use the same
trimmed value for the placeholder and sendChatMessage. Remove the premature
state.copy(input = "") from onSendClick.
- Around line 74-84: Update startNewConversation and the send() streaming flow
so an in-progress response from the previous conversation cannot update the new
conversation state: either cancel the active streaming job when starting a new
conversation, or validate the captured stream/conversation identifiers before
applying terminal events such as Done. Ensure stale events are ignored after the
conversation is reset.
In `@feature/chat/src/main/java/com/kikidan/chat/TypewriterFlow.kt`:
- Around line 8-11: TypewriterFlow의 KDoc 설명에서 잘못된 따옴표를 제거하여 `전체 텍스"를`을 올바른 문구로
수정하십시오.
In `@feature/chat/src/test/java/com/kikidan/chat/ChatViewModelTest.kt`:
- Around line 150-153: Update the failure message in the streamingText
monotonicity assertion to include the current streaming text value separately
from the previous value; replace the duplicated chatState interpolation while
preserving the existing startsWith check and assertion behavior.
---
Nitpick comments:
In `@feature/chat/src/main/java/com/kikidan/chat/ChatViewModel.kt`:
- Around line 105-110: Rename the local `result` inside the
`ChatStreamEvent.Start` branch of the `.transform` lambda to avoid shadowing the
lambda parameter, and update all references to it. Correct the misspelled
`onSteamingDone` method references to `onStreamingDone` at both call sites.
- Around line 33-38: Update the ChatViewModel state initialization and load flow
to use SavedStateHandle for the restorable conversationId, applying its saved
value to the initial ChatState and persisting updates when load receives a new
ID. Also persist and restore the in-progress input value through the same
handle, keeping the existing state updates intact.
In `@feature/chat/src/test/java/com/kikidan/chat/ChatViewModelTest.kt`:
- Around line 344-385: Add tests covering the onSendClick path in
ChatViewModelTest: verify that invoking onSendClick during streaming preserves
the current input, and verify that onSendClick with whitespace-only input does
not call the repository, keeping sendCallCount at zero. Reuse the existing
viewModel, FakeChatRepository, and vm.test setup.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c738278e-b79f-4cd8-929b-3607efcf582a
⛔ Files ignored due to path filters (3)
app/build.gradle.ktsis excluded by!**/*.gradle.ktsfeature/chat/build.gradle.ktsis excluded by!**/*.gradle.ktssettings.gradle.ktsis excluded by!**/*.gradle.kts
📒 Files selected for processing (8)
feature/chat/src/main/AndroidManifest.xmlfeature/chat/src/main/java/com/kikidan/chat/ChatViewModel.ktfeature/chat/src/main/java/com/kikidan/chat/TypewriterFlow.ktfeature/chat/src/main/java/com/kikidan/chat/model/ChatSideEffect.ktfeature/chat/src/main/java/com/kikidan/chat/model/ChatState.ktfeature/chat/src/test/java/com/kikidan/chat/ChatViewModelTest.ktfeature/chat/src/test/java/com/kikidan/chat/FakeChatRepository.ktfeature/chat/src/test/java/com/kikidan/chat/TypewriterFlowTest.kt
Comment on lines
+8
to
+11
| /** | ||
| * 도착 속도(네트워크)와 표시 속도(화면)를 분리한다. | ||
| * 업스트림 청크를 버퍼에 쌓고 tick마다 조금씩 잘라 지금까지 보여줄 전체 텍스"를 방출한다. | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
[P3] KDoc 오타를 수정하십시오.
전체 텍스"를에 잘못된 따옴표가 있습니다.
✏️ 제안 수정
- * 업스트림 청크를 버퍼에 쌓고 tick마다 조금씩 잘라 지금까지 보여줄 전체 텍스"를 방출한다.
+ * 업스트림 청크를 버퍼에 쌓고 tick마다 조금씩 잘라 지금까지 보여줄 전체 텍스트를 방출한다.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * 도착 속도(네트워크)와 표시 속도(화면)를 분리한다. | |
| * 업스트림 청크를 버퍼에 쌓고 tick마다 조금씩 잘라 지금까지 보여줄 전체 텍스"를 방출한다. | |
| */ | |
| /** | |
| * 도착 속도(네트워크)와 표시 속도(화면)를 분리한다. | |
| * 업스트림 청크를 버퍼에 쌓고 tick마다 조금씩 잘라 지금까지 보여줄 전체 텍스트를 방출한다. | |
| */ |
🤖 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 `@feature/chat/src/main/java/com/kikidan/chat/TypewriterFlow.kt` around lines 8
- 11, TypewriterFlow의 KDoc 설명에서 잘못된 따옴표를 제거하여 `전체 텍스"를`을 올바른 문구로 수정하십시오.
Comment on lines
+150
to
+153
| assertTrue( | ||
| "streamingText 단조 증가 실패: '$chatState' → '$chatState'", | ||
| currentChatState.streamingText.startsWith(chatState.streamingText), | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
[P3] 단조 증가 실패 메시지에 현재 값이 빠졌습니다.
$chatState가 두 번 사용되어 이전 값만 출력됩니다.
💚 제안 수정
assertTrue(
- "streamingText 단조 증가 실패: '$chatState' → '$chatState'",
+ "streamingText 단조 증가 실패: '${chatState.streamingText}' → '${currentChatState.streamingText}'",
currentChatState.streamingText.startsWith(chatState.streamingText),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| assertTrue( | |
| "streamingText 단조 증가 실패: '$chatState' → '$chatState'", | |
| currentChatState.streamingText.startsWith(chatState.streamingText), | |
| ) | |
| assertTrue( | |
| "streamingText 단조 증가 실패: '${chatState.streamingText}' → '${currentChatState.streamingText}'", | |
| currentChatState.streamingText.startsWith(chatState.streamingText), | |
| ) |
🤖 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 `@feature/chat/src/test/java/com/kikidan/chat/ChatViewModelTest.kt` around
lines 150 - 153, Update the failure message in the streamingText monotonicity
assertion to include the current streaming text value separately from the
previous value; replace the duplicated chatState interpolation while preserving
the existing startsWith check and assertion behavior.
oungsi2000
force-pushed
the
feat/77-chat-viewmodel
branch
from
August 8, 2026 05:21
c2a9587 to
7917a1a
Compare
oungsi2000
force-pushed
the
feat/77-chat-viewmodel
branch
from
August 8, 2026 08:46
3d72226 to
e690c2a
Compare
oungsi2000
marked this pull request as ready for review
August 9, 2026 07:59
TypewriterFlowTest의 취소 테스트에서 runTest 기본 디스패처를 StandardTestDispatcher가 아닌 UnconfinedTestDispatcher로 오인해 job이 시작되기 전에 취소되던 버그를 runCurrent() 추가로 수정.
ChatSideEffect.ShowMessage를 제거하고, 예외 객체를 직접 전달하는 Error와 스트리밍 전용 에러인 ShowStreamingErrorMessage로 세분화하여 에러 처리 로직을 개선한다. 채팅 스트림에서 ChatStreamEvent.Error 이벤트를 직접 처리하도록 로직을 추가한다. 사용자 메시지 생성 시 초기 상태를 PENDING에서 COMPLETED로 변경하여 낙관적 업데이트 UI를 조정한다. 또한, 테스트 코드를 포함한 feature/chat 모듈 전반에 ktlint 포맷팅을 적용하고 코드 구조를 정리한다.
기존의 `ChatPhase` enum과 `streamingText` 필드를 `StreamingChatState` sealed interface로 통합한다. 상태에 따라 필요한 데이터(`streamingText`)를 `Typing` 상태 내부에 캡슐화하여, Idle이나 Thinking 상태에서 불필요한 데이터에 접근하는 것을 방지하고 상태 전이를 명확하게 표현한다. - `ChatContract.kt`를 제거하고 모델들을 `model` 패키지로 이동 - `ChatViewModel` 내 상태 업데이트 및 메시지 전송 로직을 변경된 상태 모델에 맞춰 수정 - `StreamingChatState.Typing`인 경우에만 `streamingText`를 참조하도록 보장하여 안정성 개선
ChatViewModel의 인덴트와 메서드 체이닝 줄바꿈을 일관되게 수정한다. ChatState 및 하위 데이터 클래스에 trailing comma를 추가하여 가독성을 개선하고 향후 변경 시 diff 노이즈를 방지한다.
`send()` 함수에서 스트리밍 중인 경우 요청을 무시하는 가드 로직이 반대로 되어 있어, `Idle` 상태가 아닐 때 리턴하도록 수정한다. 테스트 코드의 상태 검증 로직을 `ChatPhase`에서 `StreamingChatState` 기반으로 변경하고, 가독성을 위해 헬퍼 메서드 명칭과 테스트 이름을 정리한다.
ChatViewModel의 send 메서드 내 스트리밍 시작 및 종료 처리 로직을 onStreamingStart, onSteamingDone으로 분리하여 가독성을 개선했다. 기존에 존재하던 할당량(quota) 초과 가드 로직은 제거되었다. TypewriterFlow는 MutableStateFlow 대신 StringBuffer를 사용하도록 변경하여 문자열 결합 효율을 높였으며, CATCH_UP_DIVISOR 값을 32로 조정하여 실제 기기 체감에 맞춰 타이핑 방출 속도를 최적화했다. 테스트 코드에서는 불필요한 문자열 템플릿과 공백을 정리했다.
ChatSuggestion의 카테고리 인자가 String에서 ChatCategory 타입으로 변경됨에 따라 테스트 데이터를 수정한다. 잔여 할당량이 0일 때 메시지 전송을 차단하던 로직의 테스트 케이스를 삭제한다. TypewriterFlowTest에 ExperimentalCoroutinesApi 사용을 위한 @OptIn 설정을 추가한다.
메시지 플레이스홀더를 생성할 때 `trim()`을 호출하여 입력된 텍스트의 불필요한 앞뒤 공백을 제거하도록 수정한다.
todakun.feature 플러그인을 사용하여 개별 모듈에서 반복되는 빌드 설정을 통합 관리한다. 이로 인해 중복되는 Android SDK 버전 설정, 컴파일 옵션 및 Hilt, Orbit, Compose 관련 공통 의존성 선언을 삭제하여 빌드 스크립트를 간소화한다.
suggestions와 messages의 타입을 List 대신 PersistentList로 변경한다. Jetpack Compose에서 일반 List 인터페이스는 안정성(Stability)을 보장할 수 없어 불필요한 리컴포지션이 발생할 수 있으므로, 명시적으로 불변 컬렉션을 사용하여 성능을 최적화한다. Co-Authored-By: Claude <noreply@anthropic.com>
suggestions와 messages를 PersistentList로 변경하여 UI 상태의 안정성을 확보하고 Compose 리컴포지션 최적화를 도모한다. 기존의 standard List 대신 toPersistentList()와 persistentListOf()를 사용하도록 전환한다.
SavedStateHandle을 도입하여 conversationId와 미전송 입력값(input)을 유지한다. 전송 로직에서 공백 입력을 차단하고, 전송 시작 즉시 입력창을 비워 UX를 개선한다. 로컬 유저 메시지의 초기 상태를 GENERATING으로 변경하고, 스트리밍 실패 시 해당 메시지에 FAILED 상태를 반영하도록 에러 핸들링을 보완한다. 스트리밍 중 중복 전송 요청은 무시하되, 그 사이 입력된 텍스트는 초기화되지 않고 보존된다.
oungsi2000
force-pushed
the
feat/77-chat-viewmodel
branch
from
August 10, 2026 12:47
502ac0f to
986ca0d
Compare
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.
관련 이슈
#77
작업 내용
변경사항 / 상세
중점 리뷰사항
스크린샷 (선택)
Summary by CodeRabbit
새로운 기능
테스트