refactor(mpp-idea): reuse CodeReviewViewModel from mpp-ui#9
Conversation
- Simplify IdeaCodeReviewViewModel by extending CodeReviewViewModel from mpp-ui - Remove duplicate IdeaCodeReviewModels.kt, use shared models from mpp-ui - Update IdeaCodeReviewContent.kt to use shared model types - Fix dependency configuration in build.gradle.kts and settings.gradle.kts - All CodeReview features now inherited from base class: - Plan generation (generateModificationPlan) - Fix generation (generateFixes, proceedToGenerateFixes) - Lint analysis (runLint, analyzeModifiedCode) - Issue tracking (loadIssueForCommit) - Test discovery (findRelatedTests) - File viewer support (openFile, closeFileViewer)
WalkthroughThis PR refactors the mpp-idea module to use centralized Maven artifacts parameterized by Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
| } | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Since this ViewModel implements Disposable, consider ensuring any coroutines/resources owned by the base CodeReviewViewModel are cancelled or cleared on dispose (or pass a scope tied to the project/component lifecycle) to avoid leaks after the toolwindow closes. (Guideline: no_memory_leaks)
🤖 Was this useful? React with 👍 or 👎
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/codereview/IdeaCodeReviewViewModel.kt (2)
4-6: Double‑check lifecycle wiring between IntelliJ,CoroutineScope, andCodeReviewViewModelThe class now delegates almost everything to
CodeReviewViewModeland only passesworkspace = createWorkspaceFromProject(project)into the base constructor. The IntelliJ‑providedcoroutineScopeis stored but not fed into the base class or otherwise used here.If
CodeReviewViewModelsupports injecting an external scope (or other lifecycle hooks), you probably want it to run on the IntelliJ/plugin scope rather than on an internally created default scope, so its jobs are tied to the plugin lifecycle. If the base class already manages its own scoped lifecycle independently, consider dropping thecoroutineScopeparameter from this class to avoid confusion about which scope actually controls cleanup.Also, the new
jewelRendererandloggerlook appropriate for IntelliJ‑specific adaptation.Also applies to: 24-29, 31-31, 33-34
52-58: Consider explicitly tying coroutine scope cancellation todispose()(or clarify that base VM handles it)
dispose()currently only logs and the comment says “The parent class cleanup will happen when the scope is cancelled”, but this class doesn’t cancel any scope and doesn’t call into the base ViewModel indispose().If there is an IntelliJ‑managed
CoroutineScopethat should bound the lifetime ofCodeReviewViewModelwork, consider either:
- Passing that scope into
CodeReviewViewModel(if supported) and cancelling it from the IntelliJ lifecycle (e.g., here indispose()), or- Explicitly delegating to a base‑class cleanup method if one exists.
If the base class already binds its scope to an appropriate lifecycle (e.g., internal
viewModelScopethat is closed elsewhere), it might be clearer to update the comment and/or remove the unusedcoroutineScopeparameter sodispose()doesn’t suggest extra cleanup that never happens here.Also applies to: 24-26
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
mpp-idea/build.gradle.kts(3 hunks)mpp-idea/settings.gradle.kts(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/codereview/IdeaCodeReviewContent.kt(9 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/codereview/IdeaCodeReviewModels.kt(0 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/codereview/IdeaCodeReviewViewModel.kt(1 hunks)
💤 Files with no reviewable changes (1)
- mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/codereview/IdeaCodeReviewModels.kt
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
Use
expect/actualfor platform-specific code in KMP projects (e.g., file I/O on JVM/JS/Wasm)
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/codereview/IdeaCodeReviewContent.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/codereview/IdeaCodeReviewViewModel.kt
🧠 Learnings (6)
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: For IntelliJ IDEA Plugin build, use `cd mpp-idea && ../gradlew compileKotlin` instead of `./gradlew :mpp-idea:compileKotlin` from root
Applied to files:
mpp-idea/build.gradle.ktsmpp-idea/settings.gradle.kts
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Applies to {mpp-core,mpp-ui}/**/*.kt : Check export first in mpp-core and mpp-ui; if some functions are not working well with CLI (TypeScript), verify JsExport usage
Applied to files:
mpp-idea/build.gradle.ktsmpp-idea/settings.gradle.kts
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Applies to **/src/{androidMain,desktopMain}/**/*.kt : For Compose (Desktop/Android), use `AutoDevColors` from `cc.unitmesh.devins.ui.compose.theme` or `MaterialTheme.colorScheme`
Applied to files:
mpp-idea/build.gradle.ktsmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/codereview/IdeaCodeReviewContent.kt
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Use `cd mpp-idea && ../gradlew test --tests "cc.unitmesh.devins.idea.renderer.JewelRendererTest"` for running standalone JUnit 5 tests that do not require IntelliJ Platform
Applied to files:
mpp-idea/build.gradle.ktsmpp-idea/settings.gradle.kts
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Use `./gradlew :mpp-ui:generateI18n4kFiles` for internationalization
Applied to files:
mpp-idea/settings.gradle.kts
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Do not run `./gradlew clean`. Clean only the specific module instead (e.g., `./gradlew :mpp-core:clean`)
Applied to files:
mpp-idea/settings.gradle.kts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (241)
- GitHub Check: Build (223)
🔇 Additional comments (10)
mpp-idea/build.gradle.kts (2)
27-39: Verify repository configuration and artifact resolution strategy.Adding
mavenLocal()(line 28) suggests artifacts are published to the local Maven repository. Please clarify:
- How are the mpp-ui and mpp-core artifacts published to mavenLocal? This should be documented or automated in the parent build.
- Is
mavenLocal()necessary, or can this use composite builds instead (like theincludeBuild("..")approach in settings.gradle.kts)?
45-47: No group ID declaration needed in mpp-ui—the review comment is based on a flawed assumption.The review incorrectly assumes mpp-ui should declare
group = "AutoDev-Intellij"in its build.gradle.kts. However:
- mpp-ui is not a published library: It's a desktop/WASM application with no publishing configuration. The group ID is never meant to be declared there.
- Substitution rules are correct: The
mpp-idea/settings.gradle.ktscorrectly maps the Maven coordinateAutoDev-Intellij:mpp-uito the local project:mpp-ui. The group ID"AutoDev-Intellij"is used only in the substitution mapping, not as a project declaration.- mpp-core is correctly configured: Confirmed at line 16 of
mpp-core/build.gradle.kts:group = "cc.unitmesh"✓- Build succeeds because configuration is correct: The dependency substitutions work as intended.
No action needed.
mpp-idea/settings.gradle.kts (2)
21-26: Documentation of group IDs is clear.The comments explaining where each group ID comes from are helpful for maintainability. However, these should be verified against the actual module definitions (see the verification script in the prior comment).
30-33: The substitution rules for mpp-codegraph and mpp-viewer are necessary and correct.These modules are not over-specified.
mpp-codegraphis a direct dependency of bothmpp-uiandmpp-core, whilempp-vieweris a direct dependency ofmpp-ui. When substituting published artifacts with local projects in Gradle, you must also substitute their transitive dependencies to ensure the IDE plugin uses local versions instead of fetching published artifacts. The existing substitution rules are a standard and required pattern for multi-module projects.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/codereview/IdeaCodeReviewViewModel.kt (2)
15-22: KDoc clearly describes the new delegation toCodeReviewViewModelThe documentation accurately reflects the new responsibilities (workspace creation, renderer, Disposable) and that core behavior lives in the shared base ViewModel. No changes needed here.
36-48: Workspace creation helper looks sound for IntelliJ projectsUsing
project.basePathandproject.nameto build aWorkspaceviaDefaultWorkspace.create(orcreateEmptywhenbasePathis null) is a reasonable mapping from IntelliJProject→ sharedWorkspace. This keeps IntelliJ‑specific concerns localized in the IDEA module while reusing the sharedWorkspaceabstraction.No functional issues spotted here.
Also applies to: 4-6
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/codereview/IdeaCodeReviewContent.kt (4)
19-22: LGTM! Shared types properly imported.The migration from Idea-prefixed types to shared mpp-ui types is clean and aligns with the PR's goal of removing code duplication.
74-74: LGTM! Function signatures correctly updated.The type changes from Idea-prefixed types (
IdeaCommitInfo,IdeaDiffFileInfo,IdeaAIAnalysisProgress) to shared types (CommitInfo,DiffFileInfo,AIAnalysisProgress) are mechanical and correct.Also applies to: 129-129, 179-179, 258-258, 318-318
340-340: LGTM! AnalysisStage enum integration is correct.The usage of the shared
AnalysisStageenum values (IDLE, RUNNING_LINT, ANALYZING_LINT, GENERATING_PLAN, WAITING_FOR_USER_INPUT, GENERATING_FIX, COMPLETED, ERROR) is consistent and the control flow logic for button states and loading indicators is sound.Also applies to: 362-369, 372-374
43-43: The selectCommit API usage is correct—no changes needed.The
selectCommit(index)call at line 43 properly uses single-select behavior. The method signatureselectCommit(index: Int, toggle: Boolean = false)defaults totoggle=false, which setsnewSelection = setOf(index), replacing the entire selection with a single commit. The state maintainsSet<Int>for consistency with the shared ViewModel design, but the IDEA UI intentionally enforces single-selection by omitting the toggle parameter. This differs fromCodeReviewSideBySideViewin the common code, which explicitly passestoggle = isToggleto support multi-select when needed—both patterns are correct for their respective contexts.
Summary
This PR refactors
IdeaCodeReviewViewModelto extend the commonCodeReviewViewModelfrommpp-ui, eliminating code duplication and ensuring feature parity.Changes
IdeaCodeReviewViewModel.kt
CodeReviewViewModelfrommpp-uiWorkspaceadapter from IntelliJ'sProjectIdeaCodeReviewModels.kt
mpp-ui:CodeReviewStateinstead ofIdeaCodeReviewStateCommitInfoinstead ofIdeaCommitInfoDiffFileInfoinstead ofIdeaDiffFileInfoAIAnalysisProgressinstead ofIdeaAIAnalysisProgressAnalysisStageinstead ofIdeaAnalysisStageIdeaCodeReviewContent.kt
mpp-uiBuild Configuration
build.gradle.ktssettings.gradle.ktsFeatures Now Available (inherited from CodeReviewViewModel)
loadCommitHistory()/loadMoreCommits()- Load git commits with infinite scrollselectCommit()- Select commits and load their diffsstartAnalysis()- Run AI code review analysisgenerateModificationPlan()- Generate AI modification plangenerateFixes()/proceedToGenerateFixes()- Generate AI fixesanalyzeModifiedCode()- Analyze modified code rangesrunLint()- Run linting on modified filesfindRelatedTests()- Find related test filesopenFile()/closeFileViewer()- File viewer supportloadIssueForCommit()- Load issue info from trackersrefresh()- Refresh dataTesting
./gradlew :mpp-idea:build --no-configuration-cachePull Request opened by Augment Code with guidance from the PR author
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.