Open
Conversation
…put() is called after the fragment is detached
Contributor
📝 WalkthroughRelease Notes: Guard against IllegalStateException
Risks / Best practices
WalkthroughBuild output clearing now avoids touching activity-scoped ViewModel or calling fragment methods when the fragment is detached; KDoc for the shareable output interface documents this caller behavior. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt (1)
433-442: Avoid duplicate shared-state clears on the attached-fragment path.When attached, this now clears via
buildOutputViewModelhere and again insideBuildOutputFragment.clearOutput(). Consider using a single clear path to reduce redundant work.♻️ Proposed simplification
fun clearBuildOutput() { - // Always clear the persistent build output state via the bottom sheet's - // activity-scoped ViewModel. - buildOutputViewModel.clear() - - // `clearOutput()` on the fragment may touch additional UI state, so only call it when - // the fragment is currently attached to an activity. val fragment = pagerAdapter.buildOutputFragment if (fragment != null && fragment.isAdded && !fragment.isDetached) { fragment.clearOutput() + return } + buildOutputViewModel.clear() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt` around lines 433 - 442, The code currently always calls buildOutputViewModel.clear() and then, when the fragment is attached, also calls BuildOutputFragment.clearOutput(), causing duplicate clears; change the logic to use a single clear path: if pagerAdapter.buildOutputFragment is attached (fragment != null && fragment.isAdded && !fragment.isDetached) call fragment.clearOutput() (which should handle UI and shared-state clearing), otherwise call buildOutputViewModel.clear(); update references in EditorBottomSheet to only clear the ViewModel when the fragment is not attached.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt`:
- Around line 433-442: The code currently always calls
buildOutputViewModel.clear() and then, when the fragment is attached, also calls
BuildOutputFragment.clearOutput(), causing duplicate clears; change the logic to
use a single clear path: if pagerAdapter.buildOutputFragment is attached
(fragment != null && fragment.isAdded && !fragment.isDetached) call
fragment.clearOutput() (which should handle UI and shared-state clearing),
otherwise call buildOutputViewModel.clear(); update references in
EditorBottomSheet to only clear the ViewModel when the fragment is not attached.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 70980486-3d8f-4499-9ccc-97c82e5aa28c
📒 Files selected for processing (3)
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/ShareableOutputFragment.ktapp/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt
jomen-adfa
approved these changes
Mar 19, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt`:
- Around line 97-99: Remove the redundant call to buildOutputViewModel.clear()
in BuildOutputFragment to avoid racing with fresh append() activity from other
components (EditorBottomSheet already clears the shared BuildOutputViewModel and
calls fragment.clearOutput()); specifically, delete or disable the
buildOutputViewModel.clear() invocation inside the isAdded && !isDetached check
so the fragment no longer double-clears the shared state and conflicts with
EditorBottomSheet's clear logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b33cc994-5d9e-44fe-88c8-13d05acf274e
📒 Files selected for processing (1)
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt
Comment on lines
+97
to
+99
| if (isAdded && !isDetached) { | ||
| buildOutputViewModel.clear() | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid double-clearing shared build-output state here.
EditorBottomSheet already clears BuildOutputViewModel before calling fragment.clearOutput(). Clearing again in this fragment can race with fresh append() activity and wipe newly cached output/session state.
Suggested fix
override fun clearOutput() {
- // This fragment uses `activityViewModels()`, which relies on the fragment being attached.
- // `clearOutput()` can be triggered from build/service callbacks even after the fragment
- // has been detached, so we must guard before touching the activity-scoped ViewModel.
- if (isAdded && !isDetached) {
- buildOutputViewModel.clear()
- }
+ // Shared build-output state is cleared by the caller-side ViewModel owner
+ // (e.g., EditorBottomSheet). Keep fragment clear local to UI/editor state.
super.clearOutput()
}📝 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
| if (isAdded && !isDetached) { | |
| buildOutputViewModel.clear() | |
| } | |
| override fun clearOutput() { | |
| // Shared build-output state is cleared by the caller-side ViewModel owner | |
| // (e.g., EditorBottomSheet). Keep fragment clear local to UI/editor state. | |
| super.clearOutput() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt`
around lines 97 - 99, Remove the redundant call to buildOutputViewModel.clear()
in BuildOutputFragment to avoid racing with fresh append() activity from other
components (EditorBottomSheet already clears the shared BuildOutputViewModel and
calls fragment.clearOutput()); specifically, delete or disable the
buildOutputViewModel.clear() invocation inside the isAdded && !isDetached check
so the fragment no longer double-clears the shared state and conflicts with
EditorBottomSheet's clear logic.
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
2 participants
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.
BuildOutputFragment.clearOutput() was being called after the fragment is detached
The bottom sheet now clears the shared build-output state via its own ViewModel first