Conversation
…erPage - Add IdeaStructuredInfoPane component for TOC and entities display - Refactor IdeaKnowledgeContent to use resizable split panes - Use IdeaResizableSplitPane for horizontal layout (left/center/right) - Use IdeaVerticalResizableSplitPane for document content + structured info - Add navigateToEntity method to IdeaKnowledgeViewModel - Remove duplicate TOC from DocumentContentPanel (now in StructuredInfoPane)
WalkthroughThis pull request refactors the knowledge panel UI layout from a flat Row-based three-pane design to nested resizable split panes, extracts Table of Contents and Entities display into a new dedicated component, simplifies markdown text extraction logic, and adds entity navigation functionality to the knowledge view model. 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 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| } | ||
| } | ||
| return cell.getTextInNode(content).toString() | ||
| .replace("|", "") |
There was a problem hiding this comment.
Stripping all "|" characters can remove literal pipes that belong to cell content (e.g., escaped \|), reducing fidelity; the previous token-based extraction avoided this. Consider avoiding blanket pipe removal or relying on AST tokenization for clean text.
🤖 Was this useful? React with 👍 or 👎
| * Navigate to an entity in the document | ||
| */ | ||
| fun navigateToEntity(entity: cc.unitmesh.devins.document.Entity) { | ||
| val content = _state.value.documentContent ?: return |
There was a problem hiding this comment.
Entity navigation currently searches only documentContent; when the viewer is showing parsedContent (IdeaKnowledgeContent.kt: line 81), clicks will no-op. Consider also searching the parsed content to mirror the viewer’s source. (Guideline: null_safety)
🤖 Was this useful? React with 👍 or 👎
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/markdown/SimpleJewelMarkdown.kt (1)
744-751: Good simplification!The refactor from recursive text collection to direct string manipulation makes the code clearer and easier to maintain. The order of replacements is correct (
**before*), ensuring markdown artifacts are properly stripped.Optionally, consider adding strikethrough handling for consistency with the rest of the renderer:
private fun extractCellText(cell: ASTNode, content: String): String { return cell.getTextInNode(content).toString() .replace("|", "") .replace("`", "") + .replace("~~", "") .replace("**", "") .replace("*", "") .trim() }mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeViewModel.kt (1)
369-382: Consider handling multiple occurrences and edge cases.The implementation only navigates to the first occurrence of the entity name. If an entity appears multiple times in the document, subsequent occurrences are ignored. Additionally, the word-boundary regex (
\b) may not work correctly for entities with special characters or operators.Consider these improvements:
- Track and cycle through multiple occurrences if the same entity is clicked repeatedly.
- Log or notify when entity is not found rather than silently returning.
- Test with edge cases like entity names containing special characters, operators, or non-alphanumeric symbols.
Example improvement for finding the first occurrence with logging:
fun navigateToEntity(entity: cc.unitmesh.devins.document.Entity) { val content = _state.value.documentContent ?: return // Search for the entity name in the content val entityName = entity.name val pattern = Regex("\\b${Regex.escape(entityName)}\\b") val match = pattern.find(content) if (match != null) { val lineNumber = content.substring(0, match.range.first).count { it == '\n' } + 1 navigateToLine(lineNumber, entityName) + } else { + // Log or show notification that entity was not found in document + updateState { it.copy(error = "Entity '${entityName}' not found in document") } } }mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaStructuredInfoPane.kt (1)
63-66: Consider LazyColumn for large lists.For documents with many TOC items or entities (50+), the current
ColumnwithforEachmight cause performance issues and doesn't provide scroll state management. Consider wrapping the items in aLazyColumnwithitems()for better performance and built-in scrolling.Example for TOC section:
-Column(Modifier.fillMaxWidth(), verticalArrangement = Arrangement.spacedBy(2.dp)) { - toc.forEach { item -> IdeaTocItemRow(item, onTocSelected) } -} +LazyColumn( + Modifier.fillMaxWidth().heightIn(max = 300.dp), + verticalArrangement = Arrangement.spacedBy(2.dp) +) { + items(toc, key = { it.title }) { item -> + IdeaTocItemRow(item, onTocSelected) + } +}Similar pattern for the entities section.
Also applies to: 86-89
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.kt (1)
45-114: Consider persisting split ratios for better UX.Users will lose their preferred panel sizes when restarting the IDE. Consider persisting split ratios to
PropertiesComponentor similar state storage so users don't have to resize panels every session.Example approach:
// Save split ratio PropertiesComponent.getInstance(project).setValue("knowledge.leftSplitRatio", ratio.toString()) // Load split ratio val savedRatio = PropertiesComponent.getInstance(project) .getValue("knowledge.leftSplitRatio")?.toFloatOrNull() ?: 0.18f
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/markdown/SimpleJewelMarkdown.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.kt(4 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeViewModel.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaStructuredInfoPane.kt(1 hunks)
🧰 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/renderer/markdown/SimpleJewelMarkdown.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeViewModel.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaStructuredInfoPane.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.kt
🧠 Learnings (1)
📚 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/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.kt
⏰ 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). (3)
- GitHub Check: Agent
- GitHub Check: Build (241)
- GitHub Check: Build (223)
🔇 Additional comments (4)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaStructuredInfoPane.kt (2)
30-92: LGTM! Well-structured component with good UX patterns.The structured info pane is well-designed with:
- Clear separation between TOC and Entities sections
- Automatic expansion state reset when content changes (prevents stale UI state)
- Empty state handling with informative messages
- Proper use of AutoDevColors and JewelTheme
145-171: Recursive TOC rendering is appropriate for the use case.The recursive rendering of TOC items (line 169) is well-implemented with proper indentation based on level. While deeply nested hierarchies could theoretically cause issues, document TOCs rarely exceed 5-6 levels, making this approach safe and readable.
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.kt (2)
45-114: LGTM! Excellent layout refactor with resizable split panes.The nested split-pane layout is well-executed:
- Clear hierarchy: document list → (content + structured info) → AI chat
- Reasonable initial split ratios and constraints
- Proper integration of IdeaStructuredInfoPane with navigation callbacks
- Consistent use of
Modifier.fillMaxSize()throughoutThe refactor successfully separates concerns and provides users with flexible layout control.
296-306: LGTM! Clean removal of TOC rendering from DocumentContentPanel.The removal of the
onTocItemClickparameter and TOC rendering logic is consistent with the architectural change to centralize structured info (TOC + Entities) inIdeaStructuredInfoPane. The simplified signature makes the panel's responsibility clearer.
There was a problem hiding this comment.
Pull request overview
This PR synchronizes the IdeaKnowledgeContent UI with the DocumentReaderPage from mpp-ui, introducing a resizable split-pane layout and a new structured information pane component. The changes modernize the UI from fixed-width panels to a fully resizable layout while maintaining consistency across the multiplatform codebase.
Key Changes:
- Introduced
IdeaStructuredInfoPanecomponent with collapsible TOC and Entities sections, adapted from mpp-ui for Jewel theming - Refactored layout from fixed-width
Rowto nested resizable split panes (horizontal and vertical) - Added
navigateToEntity()method to support entity navigation from the structured info pane - Simplified markdown table cell text extraction logic
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| IdeaStructuredInfoPane.kt | New component displaying TOC and Entities in collapsible sections with Jewel UI theming |
| IdeaKnowledgeViewModel.kt | Added navigateToEntity() method for entity navigation support |
| IdeaKnowledgeContent.kt | Refactored from fixed Row layout to nested resizable split panes (horizontal + vertical) |
| SimpleJewelMarkdown.kt | Simplified table cell text extraction from recursive token collection to direct string manipulation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val content = _state.value.documentContent ?: return | ||
| // Search for the entity name in the content | ||
| val entityName = entity.name | ||
| val pattern = Regex("\\b${Regex.escape(entityName)}\\b") | ||
| val match = pattern.find(content) | ||
| if (match != null) { | ||
| val lineNumber = content.substring(0, match.range.first).count { it == '\n' } + 1 | ||
| navigateToLine(lineNumber, entityName) |
There was a problem hiding this comment.
The navigateToEntity function searches for the entity name in the document content, but ignores the entity.location field which already contains precise position information (page, line, anchor). Consider using entity.location.line directly when available, falling back to text search only when the location is not provided. This would be more efficient and accurate:
fun navigateToEntity(entity: cc.unitmesh.devins.document.Entity) {
val lineNum = entity.location.line
if (lineNum != null) {
navigateToLine(lineNum, entity.name)
} else {
// Fallback: search for the entity name in content
val content = _state.value.documentContent ?: return
val entityName = entity.name
val pattern = Regex("\\b${Regex.escape(entityName)}\\b")
val match = pattern.find(content)
if (match != null) {
val lineNumber = content.substring(0, match.range.first).count { it == '\n' } + 1
navigateToLine(lineNumber, entityName)
}
}
}| val content = _state.value.documentContent ?: return | |
| // Search for the entity name in the content | |
| val entityName = entity.name | |
| val pattern = Regex("\\b${Regex.escape(entityName)}\\b") | |
| val match = pattern.find(content) | |
| if (match != null) { | |
| val lineNumber = content.substring(0, match.range.first).count { it == '\n' } + 1 | |
| navigateToLine(lineNumber, entityName) | |
| val lineNum = entity.location.line | |
| if (lineNum != null) { | |
| navigateToLine(lineNum, entity.name) | |
| } else { | |
| val content = _state.value.documentContent ?: return | |
| val entityName = entity.name | |
| val pattern = Regex("\\b${Regex.escape(entityName)}\\b") | |
| val match = pattern.find(content) | |
| if (match != null) { | |
| val lineNumber = content.substring(0, match.range.first).count { it == '\n' } + 1 | |
| navigateToLine(lineNumber, entityName) | |
| } |
| item.title, | ||
| style = JewelTheme.defaultTextStyle.copy( | ||
| fontSize = 12.sp, | ||
| fontWeight = if (safeLevel == 1) FontWeight.SemiBold else FontWeight.Normal |
There was a problem hiding this comment.
The coerceAtLeast(1) affects both the padding calculation and the font weight check. This means a TOC item with level 0 or negative would be displayed with bold font (level 1 style) even though it's at an incorrect level. Consider separating these concerns:
@Composable
private fun IdeaTocItemRow(item: TOCItem, onTocSelected: (TOCItem) -> Unit) {
val safeLevel = item.level.coerceAtLeast(1)
Column {
Row(
Modifier.fillMaxWidth()
.clickable { onTocSelected(item) }
.padding(start = ((safeLevel - 1) * 12).dp, top = 4.dp, bottom = 4.dp, end = 4.dp),
verticalAlignment = Alignment.CenterVertically
) {
Text(
item.title,
style = JewelTheme.defaultTextStyle.copy(
fontSize = 12.sp,
fontWeight = if (item.level == 1) FontWeight.SemiBold else FontWeight.Normal
),
maxLines = 1,
overflow = TextOverflow.Ellipsis,
modifier = Modifier.weight(1f)
)
// ... rest of the code
}
item.children.forEach { child -> IdeaTocItemRow(child, onTocSelected) }
}
}This keeps the bold styling only for actual level 1 items, while still protecting against negative padding.
| fontWeight = if (safeLevel == 1) FontWeight.SemiBold else FontWeight.Normal | |
| fontWeight = if (item.level == 1) FontWeight.SemiBold else FontWeight.Normal |
| return cell.getTextInNode(content).toString() | ||
| .replace("|", "") | ||
| .replace("`", "") | ||
| .replace("**", "") | ||
| .replace("*", "") | ||
| .trim() |
There was a problem hiding this comment.
The simplified table cell extraction replaces all markdown formatting characters globally, which could incorrectly strip valid content. For example:
"Value: 3*4 = 12"would become"Value: 34 = 12"(valid asterisk removed)"Use | as separator"would become"Use as separator"(valid pipe removed)- Code snippets like
`func()`would lose the backticks
The previous implementation that recursively collected TEXT tokens was more accurate as it only extracted actual text content, preserving the semantic structure. If performance is a concern, consider using a more targeted approach that strips markdown formatting only at string boundaries or using the markdown parser's structure instead of string replacement.
| return cell.getTextInNode(content).toString() | |
| .replace("|", "") | |
| .replace("`", "") | |
| .replace("**", "") | |
| .replace("*", "") | |
| .trim() | |
| fun collectText(node: ASTNode): String { | |
| return when (node.type) { | |
| MarkdownTokenTypes.TEXT -> node.getTextInNode(content).toString() | |
| else -> node.children.joinToString("") { collectText(it) } | |
| } | |
| } | |
| return collectText(cell).trim() |
1 participant
Summary
Sync the
IdeaKnowledgeContentUI in mpp-idea with theDocumentReaderPagefrom mpp-ui to ensure UI consistency across platforms.Changes
New Components
IdeaCollapsibleSection: Expandable/collapsible card with headerIdeaTocItemRow: Recursive TOC item display with indentationIdeaEntityItemRow: Entity card with icon and descriptionLayout Refactoring
ViewModel Updates
navigateToEntity()method to support entity navigationCleanup
DocumentContentPanel(now inIdeaStructuredInfoPane)Layout Structure
Testing
Pull Request opened by Augment Code with guidance from the PR author
Summary by CodeRabbit
Release Notes
New Features
UI Improvements
✏️ Tip: You can customize this high-level summary in your review settings.