๐ :: (#956) AI ์ฑ๋ด ์ฑํ ํ๋ฉด ์ฌ์ฉ์ฑ ๊ฐ์ #957
Hidden character warning
Conversation
|
Warning Review limit reached
Next review available in: 53 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 (1)
๐ WalkthroughWalkthroughDmsApp now derives bottom-bar visibility from the navigation back stack, ChatBotScreen uses IME-driven layout spacing, ChatBotMessageBubble renders markdown-styled text, and ChatBotTypingIndicator now animates three dots. ChangesChatbot and navigation updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels: ๐ฅ 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}
| PointHistoryRoute( | ||
| pointType = it.pointType, | ||
| onBackClick = { backStack.remove(PointHistoryScreenNav(it.pointType)) } | ||
| onBackClick = { backStack.remove(PointHistoryScreenNav(it.pointType)) }, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and canโt be posted inline due to platform limitations.
โ ๏ธ Outside diff range comments (1)
feature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/ChatBotScreen.kt (1)
100-155: ๐ฉบ Stability & Availability | ๐ก Minor | โก Quick winHandle IME insets for the persistent bottom bar
ChatBotInputBaronly appliesimePadding(), whileDmsAppkeeps the bottom bar visible with no bottom-window insets, so the chat input can be obscured or collide with the keyboard on edge-to-edge screens. Add bottom/IME inset handling at the scaffold or bottom-bar level.๐ค 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 100 - 155, ChatBotInputBar currently only relies on imePadding(), so the persistent bottom bar can still overlap the keyboard on edge-to-edge screens. Update the chat screen layout around ChatBotScreen and ChatBotInputBar to apply proper bottom/IME window inset handling at the scaffold or bottom-bar level, ensuring the input bar stays above the keyboard and respects system bottom insets when isInputFocused changes.
๐งน Nitpick comments (4)
app/src/main/kotlin/team/aliens/dms/android/app/ui/DmsApp.kt (1)
101-141: ๐ Maintainability & Code Quality | ๐ต Trivial | โก Quick winBottom-tab route set is duplicated in two different forms.
The route membership check at Lines 102-107 (
currentScreen in listOf(...)) and the removal filter at Lines 135-138 (it is HomeScreenNav || ...) encode the same "bottom-tab routes" concept twice, using different comparison styles (equality vs.is). If a new bottom-tab destination is added later, it's easy to update one list and forget the other, silently breaking bottom bar visibility or back-stack cleanup.โป๏ธ Suggested consolidation
+ val bottomTabRoutes = remember { + setOf(HomeScreenNav, ApplicationScreenNav, ChatBotScreenNav, MyPageScreenNav) + } val currentScreen = backStack.lastOrNull() - val shouldShowBottomBar = currentScreen in listOf( - HomeScreenNav, - ApplicationScreenNav, - ChatBotScreenNav, - MyPageScreenNav, - ) + val shouldShowBottomBar = currentScreen in bottomTabRoutesif (currentScreen != destination) { - backStack.removeAll { - it is HomeScreenNav || - it is ApplicationScreenNav || - it is ChatBotScreenNav || - it is MyPageScreenNav - } + backStack.removeAll { it in bottomTabRoutes } backStack.add(destination) }๐ค 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 `@app/src/main/kotlin/team/aliens/dms/android/app/ui/DmsApp.kt` around lines 101 - 141, The bottom-tab route set is duplicated in DmsApp between the `shouldShowBottomBar` membership check and the `onNavigate` back-stack cleanup in `BottomNavigationBar`. Consolidate this shared concept into a single reusable bottom-tab route helper or collection and use it both for the visibility check and the removal predicate so new routes only need to be added in one place.feature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotMessageBubble.kt (2)
34-39: ๐ Performance & Scalability | ๐ต Trivial | โก Quick winMarkdown conversion is recomputed on every recomposition.
text.toChatBotMarkdownText()runs its regex passes and rebuilds theAnnotatedStringon every recomposition ofChatBotMessageBubble, even thoughtextfor a given message is static once appended. Wrap it inremember(text)to avoid redundant regex work, especially since this bubble is re-created as items scroll in/out of aLazyColumn.โป๏ธ Proposed fix
Text( modifier = Modifier.padding(horizontal = 18.dp, vertical = 14.dp), text = if (isUser) { AnnotatedString(text) } else { - text.toChatBotMarkdownText() + remember(text) { text.toChatBotMarkdownText() } },๐ค 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/ChatBotMessageBubble.kt` around lines 34 - 39, The markdown parsing in ChatBotMessageBubble is recomputed on every recomposition because text.toChatBotMarkdownText() is called directly in the text assignment. Cache the converted AnnotatedString with remember(text) inside ChatBotMessageBubble so the conversion only reruns when the message text changes, while still keeping the existing isUser branch unchanged.
63-86: ๐ฏ Functional Correctness | ๐ต Trivial | ๐ค Low valueBullet conversion only recognizes
*prefixes, not-or numbered lists.LLM-generated markdown commonly uses
-for bullets and1.for ordered lists; those will render as raw literal characters instead of styled list markers. Given the feature is explicitly scoped to "bullets and bold," this is acceptable for now, but consider extending the regex to also match-\s+if answers commonly use dash bullets.๐ค 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/ChatBotMessageBubble.kt` around lines 63 - 86, The bullet conversion in toChatBotMarkdownText only handles "*" prefixes, so dash bullets still render as raw text; update the regex in String.toChatBotMarkdownText to also match "-" bullet prefixes (while keeping the current bold handling in AnnotatedString) so common LLM bullet formats are normalized consistently.feature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/ChatBotScreen.kt (1)
105-112: ๐ Maintainability & Code Quality | ๐ต Trivial | โก Quick winHardcoded bottom padding values are brittle given the new "always-visible" bottom bar behavior.
The
180.dp/220.dpbottom padding is a magic-number guess meant to clear the input bar and (per the sibling layer) the now-always-visible outer bottom navigation bar plus keyboard. If the actual heights of the input bar, bottom nav bar, or IME differ across devices/font scales, content will again be clipped or over-padded โ the exact bug this PR is fixing. Consider deriving this padding from measured component heights (onSizeChanged/onGloballyPositioned) or fromWindowInsets.ime/WindowInsets.navigationBarsinstead of fixed constants.๐ค 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 105 - 112, The bottom content padding in ChatBotScreen is using brittle magic numbers, so replace the fixed 180.dp/220.dp values with padding derived from actual UI measurements or insets. Update the contentPadding logic in ChatBotScreen to account for the input bar and always-visible bottom navigation bar using measured component heights (for example via onSizeChanged or onGloballyPositioned) and/or WindowInsets.ime plus WindowInsets.navigationBars, so the spacing stays correct across devices, font scales, and keyboard states.
๐ค 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.
Outside diff comments:
In
`@feature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/ChatBotScreen.kt`:
- Around line 100-155: ChatBotInputBar currently only relies on imePadding(), so
the persistent bottom bar can still overlap the keyboard on edge-to-edge
screens. Update the chat screen layout around ChatBotScreen and ChatBotInputBar
to apply proper bottom/IME window inset handling at the scaffold or bottom-bar
level, ensuring the input bar stays above the keyboard and respects system
bottom insets when isInputFocused changes.
---
Nitpick comments:
In `@app/src/main/kotlin/team/aliens/dms/android/app/ui/DmsApp.kt`:
- Around line 101-141: The bottom-tab route set is duplicated in DmsApp between
the `shouldShowBottomBar` membership check and the `onNavigate` back-stack
cleanup in `BottomNavigationBar`. Consolidate this shared concept into a single
reusable bottom-tab route helper or collection and use it both for the
visibility check and the removal predicate so new routes only need to be added
in one place.
In
`@feature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/ChatBotScreen.kt`:
- Around line 105-112: The bottom content padding in ChatBotScreen is using
brittle magic numbers, so replace the fixed 180.dp/220.dp values with padding
derived from actual UI measurements or insets. Update the contentPadding logic
in ChatBotScreen to account for the input bar and always-visible bottom
navigation bar using measured component heights (for example via onSizeChanged
or onGloballyPositioned) and/or WindowInsets.ime plus
WindowInsets.navigationBars, so the spacing stays correct across devices, font
scales, and keyboard states.
In
`@feature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotMessageBubble.kt`:
- Around line 34-39: The markdown parsing in ChatBotMessageBubble is recomputed
on every recomposition because text.toChatBotMarkdownText() is called directly
in the text assignment. Cache the converted AnnotatedString with remember(text)
inside ChatBotMessageBubble so the conversion only reruns when the message text
changes, while still keeping the existing isUser branch unchanged.
- Around line 63-86: The bullet conversion in toChatBotMarkdownText only handles
"*" prefixes, so dash bullets still render as raw text; update the regex in
String.toChatBotMarkdownText to also match "-" bullet prefixes (while keeping
the current bold handling in AnnotatedString) so common LLM bullet formats are
normalized consistently.
โน๏ธ Review info
โ๏ธ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 695814b8-de69-462b-8090-edf480c7d229
๐ Files selected for processing (3)
app/src/main/kotlin/team/aliens/dms/android/app/ui/DmsApp.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/ChatBotScreen.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotMessageBubble.kt
There was a problem hiding this comment.
๐งน Nitpick comments (2)
feature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotTypingIndicator.kt (2)
26-91: ๐ Maintainability & Code Quality | ๐ต Trivial | โก Quick winConsider a text alternative for screen readers.
The typing indicator conveys "AI is responding" purely visually via animated dots, with no
contentDescription/semantics for assistive technology.๐ค 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 26 - 91, The ChatBotTypingIndicator UI is purely visual and needs a screen-reader alternative. Add appropriate Compose semantics in ChatBotTypingIndicator and/or ChatBotTypingDot so assistive tech can announce that the AI is responding, using the existing ChatBotTypingIndicator and ChatBotTypingDot components as the place to attach the text alternative without changing the animation behavior.
42-76: ๐ Maintainability & Code Quality | ๐ต Trivial | โก Quick winPrefer
initialStartOffsetinstead of embedding delay into keyframe timestamps.The staggering is implemented by baking
delayMillisinto each keyframe's timestamp, duplicated across theoffsetYandalphaspecs. Compose providesinitialStartOffsetoninfiniteRepeatablespecifically for this staggered-start use case, which avoids the timestamp arithmetic and keeps each dot'skeyframesblock identical.โป๏ธ Simplify using initialStartOffset
val offsetY by infiniteTransition.animateFloat( initialValue = 0f, targetValue = -4f, animationSpec = infiniteRepeatable( animation = keyframes { durationMillis = 720 - 0f at delayMillis - -4f at delayMillis + 180 - 0f at delayMillis + 360 - 0f at 720 + 0f at 0 + -4f at 180 + 0f at 360 + 0f at 720 }, repeatMode = RepeatMode.Restart, + initialStartOffset = StartOffset(delayMillis, StartOffsetType.Delay), ), label = "ChatBotTypingDotOffset", )Verify this behaves as expected across API levels (
initialStartOffsetrequires Compose animation-core โฅ1.1.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/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotTypingIndicator.kt` around lines 42 - 76, The staggered animation in ChatBotTypingIndicatorโs rememberInfiniteTransition setup is hardcoded by adding delayMillis into each keyframe timestamp, and this duplicated arithmetic should be replaced with initialStartOffset on infiniteRepeatable. Update the offsetY and alpha animationSpec blocks so their keyframes stay identical and the per-dot delay is applied via initialStartOffset instead of timestamp offsets, keeping the animation behavior the same while simplifying the code. Ensure the change still works with the Compose animation-core version used by this module.
๐ค 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.
Nitpick comments:
In
`@feature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotTypingIndicator.kt`:
- Around line 26-91: The ChatBotTypingIndicator UI is purely visual and needs a
screen-reader alternative. Add appropriate Compose semantics in
ChatBotTypingIndicator and/or ChatBotTypingDot so assistive tech can announce
that the AI is responding, using the existing ChatBotTypingIndicator and
ChatBotTypingDot components as the place to attach the text alternative without
changing the animation behavior.
- Around line 42-76: The staggered animation in ChatBotTypingIndicatorโs
rememberInfiniteTransition setup is hardcoded by adding delayMillis into each
keyframe timestamp, and this duplicated arithmetic should be replaced with
initialStartOffset on infiniteRepeatable. Update the offsetY and alpha
animationSpec blocks so their keyframes stay identical and the per-dot delay is
applied via initialStartOffset instead of timestamp offsets, keeping the
animation behavior the same while simplifying the code. Ensure the change still
works with the Compose animation-core version used by this module.
โน๏ธ Review info
โ๏ธ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b4787d30-0c90-4f4e-8f90-b5af20891fe4
๐ Files selected for processing (3)
app/src/main/kotlin/team/aliens/dms/android/app/ui/DmsApp.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/ChatBotScreen.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/chatbot/ui/component/ChatBotTypingIndicator.kt
๐ค Files with no reviewable changes (1)
- app/src/main/kotlin/team/aliens/dms/android/app/ui/DmsApp.kt
2 participants
๊ฐ์
์์ ์ฌํญ
์ถ๊ฐ ๋ก ํ ๋ง
Summary by CodeRabbit
New Features
Bug Fixes