π :: (#954) AI μ±λ΄ κΈ°λ₯ ꡬν#955
Hidden character warning
Conversation
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review detailsβοΈ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: π Files selected for processing (4)
π WalkthroughWalkthroughThis PR adds a full chatbot feature: network API/models/datasource, a data-layer repository, a Compose-based chat UI (screen, input bar, message bubbles, suggestion chips, typing indicator), a ViewModel managing chat state, new chatbot icons, and app navigation/bottom-bar integration including a new ChangesChatbot Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ChatBotScreen
participant ChatBotViewModel
participant ChatBotRepository
participant NetworkChatBotDataSource
participant ChatBotApiService
ChatBotScreen->>ChatBotViewModel: sendQuestion()
ChatBotViewModel->>ChatBotViewModel: append user message, isLoading=true
ChatBotViewModel->>ChatBotRepository: askQuestion(question)
ChatBotRepository->>NetworkChatBotDataSource: askQuestion(request)
NetworkChatBotDataSource->>ChatBotApiService: askQuestion(request)
ChatBotApiService-->>NetworkChatBotDataSource: ChatBotAnswerResponse
NetworkChatBotDataSource-->>ChatBotRepository: ChatBotAnswerResponse
ChatBotRepository-->>ChatBotViewModel: answer
ChatBotViewModel-->>ChatBotScreen: updated state (message, isLoading=false)
sequenceDiagram
participant BottomNavigationBar
participant DmsApp
participant ChatBotRoute
BottomNavigationBar->>DmsApp: onNavigate(ChatBotScreenNav)
DmsApp->>ChatBotRoute: render NavDisplay entry
ChatBotRoute->>DmsApp: onInputFocusChanged(isFocused)
DmsApp->>DmsApp: update isChatInputFocused, toggle bottom bar visibility
Possibly related PRs
Suggested reviewers: π₯ Pre-merge checks | β 5β Passed checks (5 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
π§Ή Nitpick comments (2)
feature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotTypingIndicator.kt (1)
17-35: π Maintainability & Code Quality | π΅ Trivial | β‘ Quick winConsider animating the typing dots.
The dots currently render statically; an infinite pulsing/bouncing animation (e.g.,
rememberInfiniteTransition) would better communicate "typing" activity to users.π€ 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/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotTypingIndicator.kt` around lines 17 - 35, The ChatBotTypingIndicator composable currently renders static dots, so update it to animate the typing indicator instead of showing fixed circles. Use ChatBotTypingIndicator and its three-dot loop to add an infinite pulsing or bouncing effect with a Compose animation API such as rememberInfiniteTransition, and apply the animated state to each dot so the indicator clearly conveys ongoing typing activity.feature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/ChatBotScreen.kt (1)
121-131: π Performance & Scalability | π΅ Trivial | β‘ Quick winUse
items(..., key = ...)instead offorEach + itemfor the message list.Building
LazyColumncontent withstate.messages.forEach { item { ... } }gives Compose no stable identity for each row, so any mutation tomessagesforces full re-evaluation/recomposition of all items and disables efficient diffing. Preferitems(state.messages, key = { it.id })(assumingChatBotMessagehas/gets a stable id) for typing bubble too.β»οΈ Suggested refactor
- state.messages.forEach { message -> - item { - ChatBotMessageItem(message = message) - } - } + items( + items = state.messages, + key = { it.id }, + ) { message -> + ChatBotMessageItem(message = message) + }π€ 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/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/ChatBotScreen.kt` around lines 121 - 131, Replace the `state.messages.forEach { item { ... } }` pattern in `ChatBotScreen` with `items(...)` so each chat row has stable identity and Compose can diff efficiently. Update the `LazyColumn` content to use `items(state.messages, key = { it.id })` for `ChatBotMessageItem`, and give the typing bubble (`ChatBotTypingItem`) a stable keyed item as well if it is part of the list. Use the existing `state.messages` and `ChatBotMessage` model to locate the change.
π€ 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/src/main/kotlin/team/aliens/dms/android/feature/chatbot/viewmodel/ChatBotViewModel.kt`:
- Around line 45-73: The `askQuestion` call inside `ChatBotViewModel` is wrapped
in `runCatching`, which swallows `CancellationException` and incorrectly treats
coroutine cancellation as a normal failure. Update this flow to preserve
cancellation by either rethrowing `CancellationException` explicitly or
replacing `runCatching` with a `try/catch` around
`chatBotRepository.askQuestion(question)` that rethrows cancellation before
handling real errors in the `onFailure`-equivalent path.
---
Nitpick comments:
In
`@feature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/ChatBotScreen.kt`:
- Around line 121-131: Replace the `state.messages.forEach { item { ... } }`
pattern in `ChatBotScreen` with `items(...)` so each chat row has stable
identity and Compose can diff efficiently. Update the `LazyColumn` content to
use `items(state.messages, key = { it.id })` for `ChatBotMessageItem`, and give
the typing bubble (`ChatBotTypingItem`) a stable keyed item as well if it is
part of the list. Use the existing `state.messages` and `ChatBotMessage` model
to locate the change.
In
`@feature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotTypingIndicator.kt`:
- Around line 17-35: The ChatBotTypingIndicator composable currently renders
static dots, so update it to animate the typing indicator instead of showing
fixed circles. Use ChatBotTypingIndicator and its three-dot loop to add an
infinite pulsing or bouncing effect with a Compose animation API such as
rememberInfiniteTransition, and apply the animated state to each dot so the
indicator clearly conveys ongoing typing activity.
πͺ 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: d340e95b-00c2-47e7-b14d-48db0f61acd6
π Files selected for processing (23)
app/src/main/kotlin/team/aliens/dms/android/app/navigation/BottomMenu.ktapp/src/main/kotlin/team/aliens/dms/android/app/navigation/BottomNavigationBar.ktapp/src/main/kotlin/team/aliens/dms/android/app/navigation/DmsNavKeys.ktapp/src/main/kotlin/team/aliens/dms/android/app/ui/DmsApp.ktcore/design-system/src/main/java/team/aliens/dms/android/core/designsystem/foundation/DmsIcon.ktcore/design-system/src/main/res/drawable/ic_chatbot.xmlcore/design-system/src/main/res/drawable/ic_chatbot_fill.xmldata/src/main/kotlin/team/aliens/dms/android/data/chatbot/di/ChatBotDataModule.ktdata/src/main/kotlin/team/aliens/dms/android/data/chatbot/repository/ChatBotRepository.ktdata/src/main/kotlin/team/aliens/dms/android/data/chatbot/repository/ChatBotRepositoryImpl.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/ChatBotScreen.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotInputBar.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotMessageBubble.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotSuggestionChip.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotTypingIndicator.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/viewmodel/ChatBotState.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/viewmodel/ChatBotViewModel.ktnetwork/src/main/kotlin/team/aliens/dms/android/network/chatbot/apiservice/ChatBotApiService.ktnetwork/src/main/kotlin/team/aliens/dms/android/network/chatbot/datasource/NetworkChatBotDataSource.ktnetwork/src/main/kotlin/team/aliens/dms/android/network/chatbot/datasource/NetworkChatBotDataSourceImpl.ktnetwork/src/main/kotlin/team/aliens/dms/android/network/chatbot/di/ChatBotNetworkModule.ktnetwork/src/main/kotlin/team/aliens/dms/android/network/chatbot/model/ChatBotAnswerResponse.ktnetwork/src/main/kotlin/team/aliens/dms/android/network/chatbot/model/ChatBotQuestionRequest.kt
| runCatching { | ||
| chatBotRepository.askQuestion(question) | ||
| }.onSuccess { answer -> | ||
| _state.update { | ||
| it.copy( | ||
| isLoading = false, | ||
| messages = it.messages + ChatBotMessage( | ||
| text = answer, | ||
| isUser = false, | ||
| ), | ||
| ) | ||
| } | ||
| }.onFailure { throwable -> | ||
| Log.e( | ||
| "ChatBot", | ||
| "request failed: ${throwable::class.java.simpleName}, message=${throwable.message}", | ||
| throwable, | ||
| ) | ||
|
|
||
| _state.update { | ||
| it.copy( | ||
| isLoading = false, | ||
| messages = it.messages + ChatBotMessage( | ||
| text = "λ΅λ³μ λΆλ¬μ€μ§ λͺ»νμ΄μ. μ μ ν λ€μ μλν΄ μ£ΌμΈμ.", | ||
| isUser = false, | ||
| ), | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
π©Ί Stability & Availability | π Major | β‘ Quick win
Rethrow CancellationException from runCatching.
runCatching wraps a suspend call and catches all Throwables, including CancellationException. If viewModelScope is cancelled mid-request (e.g., ViewModel cleared), the cancellation is swallowed and turned into a normal failure path, which continues to update state and log as if it were a real error instead of allowing the coroutine to stop β a documented anti-pattern for runCatching + suspend calls.
π‘οΈ Proposed fix to rethrow cancellation
runCatching {
chatBotRepository.askQuestion(question)
+ }.onFailure { throwable ->
+ if (throwable is kotlinx.coroutines.CancellationException) throw throwable
}.onSuccess { answer ->Or more idiomatically, replace runCatching with a try { ... } catch (e: CancellationException) { throw e } catch (e: Throwable) { ... } block.
π 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.
| runCatching { | |
| chatBotRepository.askQuestion(question) | |
| }.onSuccess { answer -> | |
| _state.update { | |
| it.copy( | |
| isLoading = false, | |
| messages = it.messages + ChatBotMessage( | |
| text = answer, | |
| isUser = false, | |
| ), | |
| ) | |
| } | |
| }.onFailure { throwable -> | |
| Log.e( | |
| "ChatBot", | |
| "request failed: ${throwable::class.java.simpleName}, message=${throwable.message}", | |
| throwable, | |
| ) | |
| _state.update { | |
| it.copy( | |
| isLoading = false, | |
| messages = it.messages + ChatBotMessage( | |
| text = "λ΅λ³μ λΆλ¬μ€μ§ λͺ»νμ΄μ. μ μ ν λ€μ μλν΄ μ£ΌμΈμ.", | |
| isUser = false, | |
| ), | |
| ) | |
| } | |
| } | |
| runCatching { | |
| chatBotRepository.askQuestion(question) | |
| }.onFailure { throwable -> | |
| if (throwable is kotlinx.coroutines.CancellationException) throw throwable | |
| }.onSuccess { answer -> | |
| _state.update { | |
| it.copy( | |
| isLoading = false, | |
| messages = it.messages + ChatBotMessage( | |
| text = answer, | |
| isUser = false, | |
| ), | |
| ) | |
| } | |
| }.onFailure { throwable -> | |
| Log.e( | |
| "ChatBot", | |
| "request failed: ${throwable::class.java.simpleName}, message=${throwable.message}", | |
| throwable, | |
| ) | |
| _state.update { | |
| it.copy( | |
| isLoading = false, | |
| messages = it.messages + ChatBotMessage( | |
| text = "λ΅λ³μ λΆλ¬μ€μ§ λͺ»νμ΄μ. μ μ ν λ€μ μλν΄ μ£ΌμΈμ.", | |
| isUser = false, | |
| ), | |
| ) | |
| } | |
| } |
π€ 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/src/main/kotlin/team/aliens/dms/android/feature/chatbot/viewmodel/ChatBotViewModel.kt`
around lines 45 - 73, The `askQuestion` call inside `ChatBotViewModel` is
wrapped in `runCatching`, which swallows `CancellationException` and incorrectly
treats coroutine cancellation as a normal failure. Update this flow to preserve
cancellation by either rethrowing `CancellationException` explicitly or
replacing `runCatching` with a `try/catch` around
`chatBotRepository.askQuestion(question)` that rethrows cancellation before
handling real errors in the `onFailure`-equivalent path.
2 participants
κ°μ
μμ μ¬ν
μΆκ° λ‘ ν λ§
Summary by CodeRabbit
New Features
Bug Fixes