-
Notifications
You must be signed in to change notification settings - Fork 1
feat(mpp-vscode): implement CodeLens with Tree-sitter and fix CodeFence parsing #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ce parsing ## CodeLens Implementation - Add Tree-sitter based code element parser (code-element-parser.ts) - Support 7 languages: TypeScript, JavaScript, Python, Java, Kotlin, Go, Rust - Implement AutoComment, AutoTest, AutoMethod actions - Add prompt templates for code generation ## CodeFence Fix - Rewrite codeFence.ts to match mpp-core's CodeFence.parseAll logic - Add preProcessDevinBlock to convert ```devin blocks to <devin> tags - Handle <devin>, <thinking>, and <!-- walkthrough_start --> tags - Fix issue where devin instructions showed extra CodeBlockRenderer ## Other Changes - Remove auto-add current file feature from ChatInput - Add getCurrentModelConfig to ChatViewProvider for CodeLens actions - Add web-tree-sitter and @unit-mesh/treesitter-artifacts dependencies
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds CodeLens features and commands, a Tree-sitter-backed code-element parser with regex fallback, LLM-powered auto-actions (auto-comment, auto-test, auto-method), prompt templates, ChatView APIs, WASM handling for grammars, enhanced CodeFence parsing, build/packaging updates, and multiple webview UI/CSS adjustments removing active-file auto-add and DevIn duplication. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VSCodeExt as Extension
participant Parser as CodeElementParser
participant LLM as LLMService
participant Diff as DiffManager
participant Webview as ChatView/Webview
User->>VSCodeExt: Open document
VSCodeExt->>Parser: parseDocument(document)
Parser-->>VSCodeExt: CodeElement[] (structures/methods)
VSCodeExt->>User: Render CodeLens items
User->>VSCodeExt: Click CodeLens (e.g., autoTest)
VSCodeExt->>Parser: build ActionContext (element, code)
VSCodeExt->>LLM: send generated prompt (stream response)
LLM-->>VSCodeExt: streaming code block
VSCodeExt->>Diff: compute/apply edits (insert/replace/append)
VSCodeExt->>Webview: sendCodeContext (optional)
VSCodeExt-->>User: Show progress / complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (10)
mpp-vscode/webview/src/utils/codeFence.ts (1)
241-274: Add missing language aliases for full mpp-core parity.The fallback
|| languageIdhandles many cases, but these common aliases from mpp-core'slookupFileExtwon't map correctly:const extensionMap: Record<string, string> = { javascript: 'js', + js: 'js', typescript: 'ts', + ts: 'ts', python: 'py', + py: 'py', java: 'java', kotlin: 'kt', + kt: 'kt', rust: 'rs', + rs: 'rs', go: 'go', cpp: 'cpp', + 'c++': 'cpp', c: 'c', csharp: 'cs', + 'c#': 'cs', ruby: 'rb', + rb: 'rb', php: 'php', swift: 'swift', shell: 'sh', bash: 'sh', zsh: 'sh', + sh: 'sh', json: 'json', yaml: 'yaml', yml: 'yml', xml: 'xml', html: 'html', css: 'css', scss: 'scss', markdown: 'md', md: 'md', sql: 'sql', diff: 'diff', - patch: 'patch' + patch: 'patch', + devin: 'devin', + plantuml: 'puml', + puml: 'puml', + toml: 'toml', + dockerfile: 'dockerfile' };mpp-vscode/src/providers/codelens-provider.ts (2)
96-129: Consider preserving action order from configuration.Converting
displayItemsto aSetloses the user's configured order fromcodelens.items. If the order in which actions appear matters to users, consider using an array with a membership check instead:private buildCodeLensGroups( elements: CodeElement[], - displayItems: Set<CodeLensAction>, + displayItems: CodeLensAction[], document: vscode.TextDocument ): vscode.CodeLens[][] { const groups: vscode.CodeLens[][] = []; + const displaySet = new Set(displayItems); for (const element of elements) { const codeLenses: vscode.CodeLens[] = []; - for (const action of displayItems) { + for (const action of displayItems) { + if (!displaySet.has(action)) continue;This preserves the config array order while maintaining efficient lookup.
192-202: Non-null assertion is safe but could be more defensive.The
lens.command!assertion at line 194 is safe becausecreateCodeLensalways creates lenses with commands. However, for robustness against future changes:private buildCollapsedCodeLens(group: vscode.CodeLens[]): vscode.CodeLens { const [first] = group; - const commands = group.map(lens => lens.command!); + const commands = group.map(lens => lens.command).filter((cmd): cmd is vscode.Command => !!cmd);mpp-vscode/src/actions/auto-actions.ts (3)
60-65: Cancellation check happens too late — LLM request may complete before token is checked.The cancellation token is only checked after
streamMessagecompletes. If the user cancels during streaming, the request continues until completion. Consider passing the token tostreamMessageor checking it within the streaming callback.- await llmService.streamMessage(prompt, (chunk) => { + await llmService.streamMessage(prompt, (chunk) => { + if (token.isCancellationRequested) return; response += chunk; progress.report({ message: 'Generating...' }); });
136-139: Empty catch block swallows all errors, not just file-not-found.The catch block is intended to handle non-existent files, but it will also silently ignore permission errors, encoding issues, or other I/O failures.
try { const existingDoc = await vscode.workspace.openTextDocument(testFileUri); existingContent = existingDoc.getText(); - } catch { /* File doesn't exist */ } + } catch (e) { + // Expected for new files; log unexpected errors + if (e instanceof Error && !e.message.includes('cannot open')) { + log(`Note: Could not read existing test file: ${e.message}`); + } + }
239-248:findContainingClassmay return incorrect class for nested or adjacent classes.The function returns the last class declaration before the element position, but doesn't verify the element is actually inside that class's body. This could return a wrong class for code positioned after a class ends.
This is a known limitation of regex-based approaches. For higher accuracy, consider using Tree-sitter's parent traversal when available.
mpp-vscode/src/prompts/prompt-templates.ts (1)
114-119:parseCodeBlockdoesn't use thelanguageparameter.The function accepts a
languageparameter but never uses it. Either remove the unused parameter or use it to validate/filter code blocks by language.-export function parseCodeBlock(response: string, language?: string): string { +export function parseCodeBlock(response: string, _language?: string): string { const codeBlockRegex = /```(?:\w+)?\n([\s\S]*?)```/g; const matches = [...response.matchAll(codeBlockRegex)]; if (matches.length > 0) return matches[0][1].trim(); return response.trim(); }Or use it to prefer matching language blocks:
export function parseCodeBlock(response: string, language?: string): string { if (language) { const langRegex = new RegExp(`\`\`\`${language}\\n([\\s\\S]*?)\`\`\``, 'g'); const langMatches = [...response.matchAll(langRegex)]; if (langMatches.length > 0) return langMatches[0][1].trim(); } // fallback to any code block const codeBlockRegex = /```(?:\w+)?\n([\s\S]*?)```/g; const matches = [...response.matchAll(codeBlockRegex)]; if (matches.length > 0) return matches[0][1].trim(); return response.trim(); }mpp-vscode/src/providers/code-element-parser.ts (3)
211-217: Static state with instance-level logging may cause confusing log output.The parser instance and languages are static (shared across all
CodeElementParserinstances), butlogis an instance property. DuringdoInitialize, the first instance'slogfunction will be used, even if later instances have different loggers.Consider making
logstatic or passing a logger todoInitialize:export class CodeElementParser { private static parserInstance: any = null; private static ParserClass: any = null; private static languages: Map<string, Language> = new Map(); private static initPromise: Promise<void> | null = null; + private static log: (message: string) => void = console.log; - constructor(private log: (message: string) => void) {} + constructor(log: (message: string) => void) { + CodeElementParser.log = log; + }
222-245: Failed initialization is not retryable — rejected promise is cached forever.If
doInitializethrows,initPromiseremains set to the rejected promise. All subsequent calls toinitialize()will await this rejected promise and fail, even if the underlying issue is transient.private async doInitialize(): Promise<void> { try { CodeElementParser.ParserClass = await getParser(); await CodeElementParser.ParserClass.init(); CodeElementParser.parserInstance = new CodeElementParser.ParserClass(); this.log('Tree-sitter parser initialized'); } catch (error) { this.log(`Failed to initialize tree-sitter: ${error}`); + CodeElementParser.initPromise = null; // Allow retry throw error; } }
567-605: Go and Rust regex fallbacks don't parse struct/type definitions.The tree-sitter profiles for Go and Rust include struct/type queries, but the regex fallbacks only parse functions. This means CodeLens won't appear on structs when tree-sitter is unavailable.
If struct CodeLens is important, consider adding regex patterns:
// For Go (add before functionRegex): const structRegex = /^type\s+(\w+)\s+struct\s*{/gm; // For Rust (add before functionRegex): const structRegex = /^\s*(pub\s+)?struct\s+(\w+)/gm;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
mpp-vscode/package.json(3 hunks)mpp-vscode/src/actions/auto-actions.ts(1 hunks)mpp-vscode/src/commands/codelens-commands.ts(1 hunks)mpp-vscode/src/extension.ts(2 hunks)mpp-vscode/src/prompts/prompt-templates.ts(1 hunks)mpp-vscode/src/providers/chat-view.ts(2 hunks)mpp-vscode/src/providers/code-element-parser.ts(1 hunks)mpp-vscode/src/providers/codelens-provider.ts(1 hunks)mpp-vscode/webview/src/App.tsx(0 hunks)mpp-vscode/webview/src/components/ChatInput.tsx(0 hunks)mpp-vscode/webview/src/components/TopToolbar.tsx(1 hunks)mpp-vscode/webview/src/utils/codeFence.ts(2 hunks)
💤 Files with no reviewable changes (2)
- mpp-vscode/webview/src/components/ChatInput.tsx
- mpp-vscode/webview/src/App.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Build and run MPP CLI with: `cd mpp-ui && npm run build && npm run start`
Applied to files:
mpp-vscode/package.json
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Build MPP Core with: `cd /Volumes/source/ai/autocrud && ./gradlew :mpp-core:assembleJsPackage`
Applied to files:
mpp-vscode/package.json
🧬 Code graph analysis (7)
mpp-vscode/src/commands/codelens-commands.ts (2)
mpp-vscode/src/providers/code-element-parser.ts (1)
CodeElement(26-32)mpp-vscode/src/actions/auto-actions.ts (4)
ActionContext(24-29)executeAutoComment(34-88)executeAutoTest(93-156)executeAutoMethod(161-210)
mpp-vscode/src/providers/chat-view.ts (1)
mpp-ui/src/jsMain/typescript/design-system/theme-helpers.ts (1)
codeBlock(127-137)
mpp-vscode/src/extension.ts (2)
mpp-vscode/src/providers/codelens-provider.ts (1)
AutoDevCodeLensProvider(24-226)mpp-vscode/src/commands/codelens-commands.ts (1)
CodeLensCommands(12-253)
mpp-vscode/webview/src/utils/codeFence.ts (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/devins/parser/CodeFence.kt (1)
languageId(7-348)
mpp-vscode/src/providers/code-element-parser.ts (3)
mpp-core/src/commonMain/kotlin/cc/unitmesh/devins/document/DocumentRegistry.kt (1)
getParser(106-109)mpp-codegraph/src/wasmJsMain/kotlin/cc/unitmesh/codegraph/parser/wasm/TreeSitterInterop.kt (2)
rootNode(748-793)nodeType(576-744)mpp-core/src/commonMain/kotlin/cc/unitmesh/devins/token/DevInsToken.kt (1)
offset(140-148)
mpp-vscode/src/actions/auto-actions.ts (2)
mpp-vscode/src/providers/code-element-parser.ts (1)
CodeElement(26-32)mpp-vscode/src/prompts/prompt-templates.ts (10)
LANGUAGE_COMMENT_MAP(36-47)AutoDocContext(7-14)generateAutoDocPrompt(49-65)parseCodeBlock(114-119)AutoTestContext(16-26)getTestFramework(121-135)generateAutoTestPrompt(67-88)getTestFilePath(137-166)AutoMethodContext(28-34)generateAutoMethodPrompt(90-112)
mpp-vscode/src/providers/codelens-provider.ts (1)
mpp-vscode/src/providers/code-element-parser.ts (1)
CodeElement(26-32)
🪛 ast-grep (0.40.0)
mpp-vscode/webview/src/utils/codeFence.ts
[warning] 78-78: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(devinStartRegex.source, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 84-84: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(thinkingStartRegex.source, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 90-90: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(walkthroughStartRegex.source, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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 (30)
mpp-vscode/webview/src/components/TopToolbar.tsx (2)
13-18: LGTM! Clean removal of auto-add props.The interface simplification is clear and aligns with the PR objective to remove the auto-add current file feature. The remaining props cover the essential file management operations.
20-22: Component signature correctly updated.The destructuring matches the updated interface. All call sites have been verified and properly updated—the ChatInput.tsx usage passes only the four expected props (
selectedFiles,onAddFile,onRemoveFile,onClearFiles) with no references to the removedautoAddCurrentFileoronToggleAutoAddprops.mpp-vscode/webview/src/utils/codeFence.ts (6)
12-27: LGTM!The
CodeBlockinterface correctly mirrors mpp-core'sCodeFenceclass fields, and the regex patterns match those defined in the Kotlin reference implementation.
33-54: LGTM with minor behavioral note.The implementation correctly handles the devin block preprocessing. The regex differs slightly from mpp-core (uses
(?:\n|$)instead of requiring a trailing\n), which actually provides better edge-case handling when the devin block is at end-of-content without a trailing newline.
77-94: Static analysis warning is a false positive.The ast-grep ReDoS warnings on these lines are false positives. The regex patterns are derived from module-level constants (
/<devin>/,/<thinking>/,<!--\s*walkthrough_start\s*-->), not from user input. These are simple patterns with no catastrophic backtracking risk. Creating global regex variants from.sourceis a standard JavaScript pattern.
60-153: LGTM!The
parseCodeBlocksfunction correctly implements the multi-phase parsing logic from mpp-core'sparseAll:
- Pre-processes devin blocks
- Finds and sorts all special tags by position
- Processes tags in order, parsing intermediate content as markdown
- Handles completeness detection for unclosed tags
The logic for updating
currentIndexand extracting tag content is correct.
155-236: LGTM!The
parseMarkdownContentfunction correctly parses standard markdown content with code fences, handling:
- Text accumulation before code blocks
- Language ID extraction from fence markers
- Proper completion status for closed vs unclosed blocks
276-314: LGTM!The
getDisplayNamefunction provides a clean mapping from language IDs to human-readable names, with appropriate fallback handling. The inclusion of special block types (thinking,walkthrough,devin,plan,mermaid) aligns well with the parser's capabilities.mpp-vscode/package.json (3)
47-71: LGTM! CodeLens commands are well-structured.The command definitions follow VSCode extension conventions with clear naming patterns (
autodev.codelens.*) and descriptive titles.
126-164: LGTM! Configuration schema is well-designed.The configuration properties provide good user control with:
- Boolean toggle for enabling/disabling
- Enum-based display mode with helpful descriptions
- Flexible array for customizing which actions to show
The defaults (
quickChat,autoTest,autoComment) are sensible for common use cases.
227-230: Update express to the latest major version.Express is outdated; the package.json specifies ^4.18.2 while the latest version is 5.2.1. Consider upgrading to express v5 to ensure current security patches and features. The other dependencies (@unit-mesh/treesitter-artifacts, web-tree-sitter, cors) are already at their latest versions.
Likely an incorrect or invalid review comment.
mpp-vscode/src/extension.ts (3)
13-14: LGTM!The imports are correctly structured for the new CodeLens functionality.
139-145: LGTM! CodeLens registration is properly structured.The provider is correctly registered for multiple languages using the document selector pattern, and command disposables are spread into subscriptions.
148-155: LGTM! Configuration change listener is well-implemented.The listener correctly refreshes CodeLens when any
autodev.codelensconfiguration changes, ensuring the UI stays in sync with user preferences.mpp-vscode/src/providers/chat-view.ts (2)
231-262: LGTM! Well-structured CodeLens integration method.The
sendCodeContextmethod properly:
- Formats code context with markdown code blocks
- Ensures the webview is visible before posting
- Sends both the formatted message and the original context for flexibility
1087-1104: LGTM! Clean model config accessor.The method safely handles null cases and provides a well-typed return value for CodeLens actions to access the current model configuration.
mpp-vscode/src/commands/codelens-commands.ts (5)
12-17: LGTM! Clean dependency injection pattern.The constructor properly receives callbacks for accessing the chat view provider and model config, avoiding tight coupling between modules.
22-80: LGTM! Command registration is well-structured.All CodeLens commands are properly registered with appropriate command IDs matching those declared in
package.json.
144-161: LGTM! AutoComment handler is well-implemented.Properly validates model config availability before executing and correctly constructs the
ActionContextfor the executor.
210-227: LGTM! Show menu implementation is clean.The quick pick menu correctly transforms commands into selectable items and executes the chosen action with its arguments.
232-252: LGTM! Code context builder is well-structured.The method creates a properly typed context object that matches the expected interface in
ChatViewProvider.sendCodeContext.mpp-vscode/src/providers/codelens-provider.ts (3)
16-22: LGTM! Clean type definition.The
CodeLensActionunion type provides type safety for action filtering and command creation.
45-91: LGTM! Robust provider implementation.The
provideCodeLensesmethod includes:
- Configuration-based enable/disable check
- File size guard to prevent performance issues
- Proper cancellation token handling
- Error handling with logging
207-221: LGTM! Comprehensive test file detection.The regex patterns cover common test file naming conventions across multiple languages and frameworks.
mpp-vscode/src/actions/auto-actions.ts (2)
229-237: Signature extraction may not work correctly for arrow functions or multi-line parameter lists.The heuristic stops at
{or:, which works for simple cases but may truncate signatures with default parameter objects or type annotations containing these characters.Consider whether this edge case matters for your use case. For example:
const fn = (opts: { x: number }) => { ... } // Stops at first ':'
250-254: LGTM!The offset-based replacement logic is correct for substituting the method body content.
mpp-vscode/src/prompts/prompt-templates.ts (1)
152-154: Rust has empty test suffix, resulting in no test file path change.For Rust,
testSuffixis an empty string, so the generated path will be identical to the source file. This doesn't align with Rust conventions where tests are typically in the same file or in atests/directory.Is Rust's inline test convention intended here? If so, consider returning
nullor the same path with a comment, or generating tests in atests/subdirectory.mpp-vscode/src/providers/code-element-parser.ts (3)
45-60: MemoizedQuery caches query per instance, but queries are language-dependent.The
MemoizedQuerycaches a single compiled query, but tree-sitter queries are compiled against a specificLanguage. If the same query string is used with different languages (which doesn't happen here), it would return an incorrectly cached query. This is currently safe since each profile has its own query instances.The current usage pattern is correct since each language profile creates its own
MemoizedQueryinstances.
607-624:findBlockRangedoesn't account for braces inside strings or comments.The brace-counting logic will be confused by code like:
function example() { const s = "{ not a block }"; // } also not real }This is a known limitation of regex-based fallback. Ensure this is acceptable or document it as a limitation when Tree-sitter is unavailable.
626-643: LGTM!The indentation-based block detection for Python correctly handles empty lines and stops at de-indented code. This is a reasonable heuristic for the regex fallback.
| setTimeout(() => { | ||
| chatView.sendMessage('Explain this code in detail, including:\n1. What it does\n2. How it works\n3. Any potential issues or improvements'); | ||
| }, 300); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Race condition: setTimeout does not guarantee message ordering.
Using setTimeout to delay sending the explain/optimize message creates a race condition. If sendCodeContext takes longer than 300ms (e.g., due to webview initialization), the follow-up message may arrive before the code context, confusing the user or causing issues.
Consider awaiting the code context operation or using a more deterministic approach:
- chatView.sendCodeContext(codeContext);
-
- setTimeout(() => {
- chatView.sendMessage('Explain this code in detail, including:\n1. What it does\n2. How it works\n3. Any potential issues or improvements');
- }, 300);
+ chatView.sendCodeContext(codeContext);
+ // Send the explain prompt - the webview should queue messages appropriately
+ await chatView.sendMessage('Explain this code in detail, including:\n1. What it does\n2. How it works\n3. Any potential issues or improvements');Note: This requires sendCodeContext to return a Promise or the webview to handle message ordering. Alternatively, add a callback/event mechanism to know when the context is rendered.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In mpp-vscode/src/commands/codelens-commands.ts around lines 115-117, the
follow-up message is delayed with setTimeout which creates a race condition if
sendCodeContext or webview initialization takes longer than 300ms; replace the
timeout with a deterministic flow: have sendCodeContext return a Promise (or add
a callback/event acknowledgement from the webview) and only call
chatView.sendMessage after that Promise resolves or the acknowledgement is
received, removing the setTimeout and ensuring message ordering is preserved.
| export function getTestFilePath(sourcePath: string, language: string): string { | ||
| const pathParts = sourcePath.split('/'); | ||
| const fileName = pathParts.pop() || ''; | ||
| const dirPath = pathParts.join('/'); | ||
| const extIndex = fileName.lastIndexOf('.'); | ||
| const baseName = extIndex > 0 ? fileName.substring(0, extIndex) : fileName; | ||
| const ext = extIndex > 0 ? fileName.substring(extIndex) : ''; | ||
|
|
||
| const testSuffixes: Record<string, string> = { | ||
| typescript: '.test.ts', | ||
| javascript: '.test.js', | ||
| typescriptreact: '.test.tsx', | ||
| javascriptreact: '.test.jsx', | ||
| java: 'Test.java', | ||
| kotlin: 'Test.kt', | ||
| python: '_test.py', | ||
| go: '_test.go', | ||
| rust: '', | ||
| csharp: 'Tests.cs', | ||
| }; | ||
|
|
||
| const testSuffix = testSuffixes[language] || `.test${ext}`; | ||
|
|
||
| if (language === 'java' || language === 'kotlin') { | ||
| const testDir = dirPath.replace('/src/main/', '/src/test/'); | ||
| return `${testDir}/${baseName}${testSuffix}`; | ||
| } | ||
| if (language === 'python') return `${dirPath}/test_${baseName}.py`; | ||
| return `${dirPath}/${baseName}${testSuffix}`; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Path separator / won't work correctly on Windows.
The function uses forward slashes for path manipulation, but VSCode on Windows may provide paths with backslashes. Use path.sep or normalize paths to handle cross-platform compatibility.
+import * as path from 'path';
+
export function getTestFilePath(sourcePath: string, language: string): string {
- const pathParts = sourcePath.split('/');
+ const normalizedPath = sourcePath.replace(/\\/g, '/');
+ const pathParts = normalizedPath.split('/');
const fileName = pathParts.pop() || '';
const dirPath = pathParts.join('/');
// ... rest of functionAlternatively, use Node's path module for proper cross-platform handling.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In mpp-vscode/src/prompts/prompt-templates.ts around lines 137 to 166, the
function manipulates paths using literal '/' which breaks on Windows; replace
manual string splitting and concatenation with Node's path utilities
(require/import path) and normalize inputs with path.normalize or path.sep-aware
logic: use path.parse or path.basename/dirname to get dir and filename, use
path.join to build test file path, and use path.relative/replace with
path.join('/src','main') -> path.join('/src','test') or path.sep-aware
replacement for the Java/Kotlin case so behavior remains the same across
platforms.
| dispose() { | ||
| this._onDidChangeCodeLenses.dispose(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat mpp-vscode/src/providers/code-element-parser.ts | head -150Repository: phodal/auto-dev-sketch
Length of output: 3915
🏁 Script executed:
wc -l mpp-vscode/src/providers/code-element-parser.tsRepository: phodal/auto-dev-sketch
Length of output: 117
🏁 Script executed:
cat mpp-vscode/src/providers/code-element-parser.ts | tail -200Repository: phodal/auto-dev-sketch
Length of output: 7672
🏁 Script executed:
grep -n "class CodeElementParser\|dispose\|private\|this\._" mpp-vscode/src/providers/code-element-parser.tsRepository: phodal/auto-dev-sketch
Length of output: 1795
🏁 Script executed:
grep -n "CodeElementParser\|parser\|dispose" mpp-vscode/src/providers/codelens-provider.ts | head -50Repository: phodal/auto-dev-sketch
Length of output: 380
🏁 Script executed:
sed -n '1,50p' mpp-vscode/src/providers/codelens-provider.tsRepository: phodal/auto-dev-sketch
Length of output: 1284
🏁 Script executed:
sed -n '200,240p' mpp-vscode/src/providers/codelens-provider.tsRepository: phodal/auto-dev-sketch
Length of output: 534
🏁 Script executed:
sed -n '222,270p' mpp-vscode/src/providers/code-element-parser.tsRepository: phodal/auto-dev-sketch
Length of output: 1450
🏁 Script executed:
sed -n '235,260p' mpp-vscode/src/providers/code-element-parser.tsRepository: phodal/auto-dev-sketch
Length of output: 856
🏁 Script executed:
rg -i "dispose|cleanup|delete|release" mpp-vscode/src/providers/code-element-parser.tsRepository: phodal/auto-dev-sketch
Length of output: 48
🏁 Script executed:
rg -i "tree.sitter|wasm.*cleanup|language.*cleanup" mpp-vscode/src --type typescriptRepository: phodal/auto-dev-sketch
Length of output: 100
🏁 Script executed:
head -20 mpp-vscode/src/providers/code-element-parser.ts | grep -i importRepository: phodal/auto-dev-sketch
Length of output: 229
Add dispose() method to CodeElementParser and call it from CodeLensProvider.dispose().
The CodeElementParser initializes and caches Tree-sitter resources (Parser instance and Language WASM modules) in static fields but provides no cleanup mechanism. When CodeLensProvider is disposed, the parser resources remain in memory. Add a static dispose() method to CodeElementParser to clean up these resources, and invoke it from CodeLensProvider.dispose():
// In CodeElementParser
static dispose(): void {
CodeElementParser.parserInstance = null;
CodeElementParser.ParserClass = null;
CodeElementParser.languages.clear();
CodeElementParser.initPromise = null;
}
// In CodeLensProvider.dispose()
dispose() {
this._onDidChangeCodeLenses.dispose();
CodeElementParser.dispose();
}🤖 Prompt for AI Agents
mpp-vscode/src/providers/codelens-provider.ts lines 223-225: add a static
dispose() method to CodeElementParser that clears its cached Tree-sitter
resources (set parser instance and ParserClass to null, clear the languages
map/set, reset any initPromise to null) and then call
CodeElementParser.dispose() from CodeLensProvider.dispose() after disposing
_onDidChangeCodeLenses so the parser and WASM modules are released when the
provider is disposed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds CodeLens functionality to mpp-vscode using Tree-sitter for accurate code parsing, and fixes CodeFence parsing issues with devin instruction blocks. The implementation supports 7 programming languages and provides AI-powered actions (AutoComment, AutoTest, AutoMethod, Quick Chat, Explain, Optimize) directly above functions and classes. Additionally, it removes the auto-add current file feature from the chat interface.
Key changes:
- Tree-sitter based code element parser with fallback regex parsing for 7 languages
- CodeLens provider with configurable actions and display modes
- Rewritten CodeFence parser to handle special block types (devin, thinking, walkthrough)
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
mpp-vscode/webview/src/utils/codeFence.ts |
Rewritten parser to handle special block types and fix devin instruction rendering |
mpp-vscode/webview/src/components/TopToolbar.tsx |
Removed auto-add current file UI components |
mpp-vscode/webview/src/components/ChatInput.tsx |
Removed auto-add current file state and logic |
mpp-vscode/webview/src/App.tsx |
Removed activeFile state management |
mpp-vscode/src/providers/codelens-provider.ts |
New CodeLens provider with configurable actions and display modes |
mpp-vscode/src/providers/code-element-parser.ts |
New Tree-sitter based parser with regex fallback for 7 languages |
mpp-vscode/src/providers/chat-view.ts |
Added getCurrentModelConfig and sendCodeContext methods |
mpp-vscode/src/prompts/prompt-templates.ts |
New prompt templates for AutoComment, AutoTest, AutoMethod actions |
mpp-vscode/src/extension.ts |
CodeLens provider and command registration for supported languages |
mpp-vscode/src/commands/codelens-commands.ts |
Command handlers for all CodeLens actions |
mpp-vscode/src/actions/auto-actions.ts |
Action executors using LLMService for code generation |
mpp-vscode/package.json |
Added dependencies, commands, and configuration for CodeLens feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, async (progress, token) => { | ||
| const llmService = new LLMService(config); | ||
|
|
||
| let response = ''; | ||
| await llmService.streamMessage(prompt, (chunk) => { | ||
| response += chunk; | ||
| progress.report({ message: 'Generating...' }); | ||
| }); | ||
|
|
||
| if (token.isCancellationRequested) return; |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cancellation token is not properly checked during the streaming process. The token is only checked after llmService.streamMessage completes, but the streaming operation itself is not cancellable. If a user cancels the operation during streaming, the LLM service will continue processing until completion, wasting resources.
Consider passing the cancellation token to streamMessage or adding periodic checks within the stream callback to respect cancellation requests.
| await llmService.streamMessage(prompt, (chunk) => { | ||
| response += chunk; | ||
| progress.report({ message: 'Generating...' }); | ||
| }); |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same cancellation token issue exists here. The token is checked after streaming completes, but the LLM service continues processing even if the user cancels. This applies to all three auto-action functions (AutoComment, AutoTest, AutoMethod).
| await llmService.streamMessage(prompt, (chunk) => { | |
| response += chunk; | |
| progress.report({ message: 'Generating...' }); | |
| }); | |
| const abortController = new AbortController(); | |
| const cancellationListener = token.onCancellationRequested(() => { | |
| abortController.abort(); | |
| }); | |
| try { | |
| await llmService.streamMessage( | |
| prompt, | |
| (chunk) => { | |
| response += chunk; | |
| progress.report({ message: 'Generating...' }); | |
| }, | |
| { abortSignal: abortController.signal } | |
| ); | |
| } catch (err) { | |
| if (abortController.signal.aborted) { | |
| // Cancelled by user, exit early | |
| return; | |
| } | |
| throw err; | |
| } finally { | |
| cancellationListener.dispose(); | |
| } |
| getCurrentModelConfig(): { provider: string; model: string; apiKey: string; temperature?: number; maxTokens?: number; baseUrl?: string } | undefined { | ||
| if (!this.configWrapper) return undefined; | ||
|
|
||
| const activeConfig = this.configWrapper.getActiveConfig(); | ||
| if (!activeConfig) return undefined; | ||
|
|
||
| return { | ||
| provider: activeConfig.provider, | ||
| model: activeConfig.model, | ||
| apiKey: activeConfig.apiKey || '', | ||
| temperature: activeConfig.temperature, | ||
| maxTokens: activeConfig.maxTokens, | ||
| baseUrl: activeConfig.baseUrl | ||
| }; | ||
| } |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getCurrentModelConfig() method returns a plain object type instead of the ModelConfig type from mpp-core. This creates a type inconsistency since getModelConfig in codelens-commands.ts expects to return ModelConfig | undefined.
Consider changing the return type to match the imported ModelConfig type, or create a proper type alias for the configuration object to ensure type safety across the codebase.
| const commands = group.map(lens => lens.command!); | ||
|
|
||
| return new vscode.CodeLens(first.range, { | ||
| title: '$(autodev-icon) $(chevron-down)', |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The buildCollapsedCodeLens method uses the icon name $(autodev-icon) which is likely not defined in VS Code's built-in icon set. This will result in the icon not displaying or showing as plain text.
Use a valid Codicon name from VS Code's icon set (e.g., $(sparkle), $(wand), $(lightbulb), etc.) or define a custom icon in package.json if needed.
| title: '$(autodev-icon) $(chevron-down)', | |
| title: '$(sparkle) $(chevron-down)', |
| }); | ||
| } | ||
|
|
||
| const functionRegex = /^\s*(export\s+)?(async\s+)?(?:function\s+(\w+)|(?:const|let|var)\s+(\w+)\s*=\s*(?:async\s+)?\([^)]*\)\s*=>)/gm; |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TypeScript/JavaScript function regex on line 479 won't correctly match arrow functions with complex parameter types or destructuring. The pattern \([^)]*\) will fail on nested parentheses like (options: { name: string; getValue: () => number }).
This is a known limitation of regex-based parsing, but consider documenting this limitation or improving the regex to handle at least simple nested cases, or use a character counting approach instead of [^)]*.
| CodeElementParser.parserInstance = new CodeElementParser.ParserClass(); | ||
| this.log('Tree-sitter parser initialized'); | ||
| } catch (error) { | ||
| this.log(`Failed to initialize tree-sitter: ${error}`); |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If doInitialize() fails and throws an error, the initPromise is never cleared, causing all subsequent initialization attempts to return the same rejected promise. This leaves the parser permanently broken until the extension is reloaded.
Consider resetting CodeElementParser.initPromise = null in the catch block of doInitialize() to allow retry attempts, or handle the initialization failure more gracefully.
| this.log(`Failed to initialize tree-sitter: ${error}`); | |
| this.log(`Failed to initialize tree-sitter: ${error}`); | |
| CodeElementParser.initPromise = null; |
| "@unit-mesh/treesitter-artifacts": "^1.7.7", | ||
| "cors": "^2.8.5", | ||
| "express": "^4.18.2", | ||
| "web-tree-sitter": "^0.25.10", |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The web-tree-sitter package requires WASM files to be loaded at runtime. The WASM path resolution in code-element-parser.ts uses require.resolve() which may not work correctly in a bundled extension environment.
Consider:
- Ensuring WASM files from
@unit-mesh/treesitter-artifactsare copied to the dist folder during build - Using
vscode.Uri.joinPath(context.extensionUri, 'wasm', ...)for path resolution instead ofrequire.resolve() - Testing the CodeLens feature in a packaged extension to verify WASM loading works correctly
| }, | ||
| { | ||
| "command": "autodev.codelens.autoMethod", | ||
| "title": "AutoDev: Auto Method" |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The command autodev.codelens.showMenu is registered in codelens-commands.ts and used in codelens-provider.ts (line 198), but it's not declared in the commands section of package.json. While VS Code allows undeclared commands for internal use, it's better practice to declare all commands for discoverability and consistency.
Add the command declaration:
{
"command": "autodev.codelens.showMenu",
"title": "AutoDev: Show CodeLens Menu"
}| "title": "AutoDev: Auto Method" | |
| "title": "AutoDev: Auto Method" | |
| }, | |
| { | |
| "command": "autodev.codelens.showMenu", | |
| "title": "AutoDev: Show CodeLens Menu" |
mpp-vscode/src/extension.ts
Outdated
| // Register for supported languages | ||
| const supportedLanguages = [ | ||
| 'typescript', 'javascript', 'typescriptreact', 'javascriptreact', | ||
| 'python', 'java', 'kotlin', 'go', 'rust', 'csharp', 'cpp', 'c' |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The supported languages list in extension.ts includes 'csharp', 'cpp', and 'c', but these languages are not defined in the LANGUAGE_PROFILES map in code-element-parser.ts. This means CodeLens will be registered for these languages but will fail silently when trying to parse documents, returning empty arrays.
Either remove these languages from the supported list, or add corresponding language profiles in code-element-parser.ts.
| 'python', 'java', 'kotlin', 'go', 'rust', 'csharp', 'cpp', 'c' | |
| 'python', 'java', 'kotlin', 'go', 'rust' |
| python: '_test.py', | ||
| go: '_test.go', | ||
| rust: '', | ||
| csharp: 'Tests.cs', | ||
| }; | ||
|
|
||
| const testSuffix = testSuffixes[language] || `.test${ext}`; | ||
|
|
||
| if (language === 'java' || language === 'kotlin') { | ||
| const testDir = dirPath.replace('/src/main/', '/src/test/'); | ||
| return `${testDir}/${baseName}${testSuffix}`; | ||
| } | ||
| if (language === 'python') return `${dirPath}/test_${baseName}.py`; |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an inconsistency in Python test file naming. Line 152 defines the test suffix as '_test.py' (underscore before "test"), but line 164 generates the test file path as test_${baseName}.py (underscore after "test").
This creates contradictory naming conventions. For example, if the source file is calculator.py:
- Line 152 suggests the test file should be
calculator_test.py - Line 164 generates
test_calculator.py
Python's pytest convention typically uses test_*.py format. Consider standardizing to use test_ prefix consistently, or update line 152 to match line 164's logic.
Eliminates duplicate DevIn rendering, tool rerun button, and timeline item headers for a cleaner UI and improved code structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mpp-vscode/webview/src/components/sketch/SketchRenderer.tsx (1)
1-12: Update the file header documentation.The header comment still mentions "DevIn -> DevInRenderer" on line 11, but DevInRenderer is no longer used in this file. DevIn blocks now return
nulland are handled elsewhere.Apply this diff to update the documentation:
/** * SketchRenderer - Main content renderer * * Parses LLM response content and dispatches to appropriate sub-renderers: * - Markdown/Text -> MarkdownRenderer * - Code -> CodeBlockRenderer * - Diff -> DiffRenderer * - Thinking -> ThinkingRenderer * - Terminal -> TerminalRenderer * - Mermaid -> MermaidRenderer - * - DevIn -> DevInRenderer + * - DevIn -> Handled by ToolCallRenderer (not rendered here) */
♻️ Duplicate comments (3)
mpp-vscode/src/providers/code-element-parser.ts (3)
235-245: ResetinitPromiseon initialization failure to allow retries.If
doInitialize()throws,initPromiseremains set to the rejected promise. All subsequentinitialize()calls return this rejected promise, leaving the parser permanently broken until extension reload.Apply this diff to allow retry on failure:
private async doInitialize(): Promise<void> { try { CodeElementParser.ParserClass = await getParser(); await CodeElementParser.ParserClass.init(); CodeElementParser.parserInstance = new CodeElementParser.ParserClass(); this.log('Tree-sitter parser initialized'); } catch (error) { this.log(`Failed to initialize tree-sitter: ${error}`); + CodeElementParser.initPromise = null; throw error; } }
479-479: Known limitation: Arrow function regex fails on nested parentheses.The pattern
\([^)]*\)cannot match parameters with nested parentheses like(options: { getValue: () => number }). Since this is a fallback when Tree-sitter is unavailable, consider documenting this limitation.
550-550: Java/Kotlin method regex misses common patterns.The regex requires
{at the end of the signature line. This misses abstract methods ending with;, Kotlin single-expression functions using=, and methods where the brace is on the next line.
🧹 Nitpick comments (3)
mpp-vscode/webview/src/components/sketch/ToolCallRenderer.tsx (1)
10-17: Clean up unusedonActionprop or re‑wire it
ToolCallRendererPropsstill exposesonAction, but the component now only destructurestoolCalland never usesonAction. This leaves a dead, misleading prop in the public interface: callers can passonActionbut it will have no effect.Either:
- remove
onActionfromToolCallRendererProps, and from all call sites, or- re‑introduce the relevant UI/handler wiring if you still want an action hook (e.g., rerun).
mpp-vscode/src/providers/code-element-parser.ts (2)
45-60: Memoization assumes single-language usage per query instance.The
MemoizedQuerycaches the compiled query from the firstquery(language)call. If the sameMemoizedQueryinstance is called with differentLanguageobjects, it returns a query compiled for the wrong language.In practice, this is safe because each
LanguageProfileinLANGUAGE_PROFILEShas its own query instances. However, adding a comment or keying the cache by language would make this contract explicit.class MemoizedQuery { private readonly queryStr: string; - private compiledQuery: Query | undefined; + private compiledQueries: Map<Language, Query> = new Map(); constructor(queryStr: string) { this.queryStr = queryStr; } query(language: Language): Query { - if (this.compiledQuery) { - return this.compiledQuery; + const cached = this.compiledQueries.get(language); + if (cached) { + return cached; } - this.compiledQuery = language.query(this.queryStr); - return this.compiledQuery; + const compiled = language.query(this.queryStr); + this.compiledQueries.set(language, compiled); + return compiled; } }
393-399: Capture index ordering is implicit and fragile.The code assumes
captures[0]is the block node andcaptures[1]is the identifier node. This depends on the order captures appear in the query patterns, which isn't enforced by tree-sitter. If a query pattern changes capture order, this silently breaks.Consider using capture names to look up nodes:
return matches.flatMap(match => { - let blockNode = match.captures[0]?.node; - const idNode = match.captures[1]?.node; + // Find captures by name pattern - block capture vs name capture + let blockNode = match.captures.find(c => + c.name.startsWith('definition.') || c.name.endsWith('_declaration') || c.name.endsWith('-body') + )?.node; + const idNode = match.captures.find(c => + c.name.startsWith('name.') || c.name.endsWith('-name') || c.name === 'type_identifier' + )?.node; if (!blockNode || !idNode) { return []; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
mpp-vscode/src/commands/codelens-commands.ts(1 hunks)mpp-vscode/src/providers/code-element-parser.ts(1 hunks)mpp-vscode/src/providers/codelens-provider.ts(1 hunks)mpp-vscode/webview/src/App.css(1 hunks)mpp-vscode/webview/src/App.tsx(1 hunks)mpp-vscode/webview/src/components/Timeline.css(2 hunks)mpp-vscode/webview/src/components/Timeline.tsx(0 hunks)mpp-vscode/webview/src/components/sketch/MarkdownRenderer.css(4 hunks)mpp-vscode/webview/src/components/sketch/SketchRenderer.css(0 hunks)mpp-vscode/webview/src/components/sketch/SketchRenderer.tsx(1 hunks)mpp-vscode/webview/src/components/sketch/ToolCallRenderer.css(1 hunks)mpp-vscode/webview/src/components/sketch/ToolCallRenderer.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- mpp-vscode/webview/src/components/sketch/SketchRenderer.css
- mpp-vscode/webview/src/components/Timeline.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- mpp-vscode/src/providers/codelens-provider.ts
- mpp-vscode/src/commands/codelens-commands.ts
- mpp-vscode/webview/src/App.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to mpp-ui/src/jsMain/typescript/**/*.{ts,tsx} : In CLI/TUI (TypeScript), use `mpp-ui/src/jsMain/typescript/design-system/` and import `semanticInk` / `semanticChalk`
Applied to files:
mpp-vscode/webview/src/components/sketch/SketchRenderer.tsx
🧬 Code graph analysis (1)
mpp-vscode/src/providers/code-element-parser.ts (1)
mpp-codegraph/src/wasmJsMain/kotlin/cc/unitmesh/codegraph/parser/wasm/TreeSitterInterop.kt (2)
rootNode(748-793)nodeType(576-744)
⏰ 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 (223)
- GitHub Check: Build (241)
🔇 Additional comments (13)
mpp-vscode/webview/src/App.css (1)
114-130: UI refinement in loading indicator: spacing and sizing adjustments look good.The reduction in
.loading-indicatorpadding (20px → 10px) and.loading-spinnerdimensions (16px → 12px) are proportional refinements that tighten the loading UI, consistent with the leaner webview design mentioned in the PR objectives.mpp-vscode/webview/src/components/sketch/MarkdownRenderer.css (5)
8-16: Verify that the reduced heading margins don't compress readability.Lines 9, 14–16: Heading margins reduced from
0.5em 0.25emto0.25em 0.15em, plus a new:first-childrule setsmargin-top: 0. This tightens vertical rhythm, but ensure the reduction maintains visual separation between headings and surrounding content. Confirm this aligns with the CodeFence parsing fix intent.
26-36: Margin adjustments across block elements are consistent and appropriate.Lines 26–36: Paragraphs now use
0.25emmargin (reduced from prior implicit values), with:first-childand:last-childselectors to eliminate top/bottom spacing at boundaries. Similar pattern applied to lists, code blocks, and blockquotes. This creates tighter, more predictable vertical rhythm.
39-50: First-child/last-child margin resets consistently applied to lists.Lines 44–50: Added
:first-childand:last-childselectors to.md-listto remove excess spacing at list boundaries. Combined with the margin reduction on line 40, this creates consistent spacing when lists appear at the start or end of rendered content.
86-100: Code block margin resets help eliminate extra vertical space from CodeFence parsing.Lines 87, 94–100: Similar margin tightening and
:first-child/:last-childresets applied to.md-pre. This change directly aligns with the PR objective of fixing "extra CodeBlockRenderer" rendering. Tighter spacing around code blocks should reduce visual clutter when CodeFence blocks are parsed and rendered.
111-125: Blockquote spacing adjustments complete the vertical rhythm refinement.Lines 112, 119–125: Blockquote margins reduced and
:first-child/:last-childselectors added, following the same pattern as other block elements. Ensures blockquotes integrate smoothly into the tightened layout.mpp-vscode/webview/src/components/Timeline.css (2)
23-28: Confirm assistant message layout change produces intended spacing.The gap: 0 override removes default flex item spacing within assistant message containers, while margin: 8px 4px adds external spacing. Verify this combination produces the intended visual alignment with Timeline.tsx content rendering.
95-102: Verify left alignment of tool-call and terminal items.Removing margin-left: 16px (to 0) eliminates left indentation from tool-call and terminal items. Confirm visual alignment and hierarchy are preserved with the parent .timeline-item gap: 8px and the simplified rendering path.
mpp-vscode/webview/src/components/sketch/ToolCallRenderer.css (1)
99-107: Editor-themed styling for tool output looks consistentSwitching to
--vscode-editor-background/--vscode-editor-foregroundand adding apanel-border-colored border makes the tool output block visually consistent with editor content while still remaining visually separated from the outer container. No functional or obvious UX regressions here.mpp-vscode/webview/src/components/sketch/SketchRenderer.tsx (1)
119-122: The devin case handling is correct and properly architected.DevIn blocks are indeed handled through ToolCallRenderer, but at the backend parsing layer: the Kotlin
ToolCallParserextracts devin blocks from the LLM response and converts them toToolCallobjects before they reach the frontend. By the time the Timeline component receives items, devin blocks are already transformed intotool_calltype items and rendered byToolCallRenderer. Returningnullin SketchRenderer prevents rendering duplicate content from the extracted blocks.mpp-vscode/src/providers/code-element-parser.ts (3)
11-18: LGTM!The lazy loading pattern for the CommonJS module is appropriate. The synchronous
requirewrapped in async allows consistent usage patterns throughout the class.
296-302: LGTM!The
require.resolveapproach for locating bundled WASM files is appropriate for VSCode extensions. The error is logged before returning null, providing sufficient debugging information.
626-643: LGTM!The indentation-based block detection correctly handles Python's syntax. Empty lines are properly skipped, and the block ends when returning to the original indentation level.
| private findBlockRange(document: vscode.TextDocument, startPos: vscode.Position): vscode.Range { | ||
| const text = document.getText(); | ||
| let offset = document.offsetAt(startPos); | ||
| while (offset < text.length && text[offset] !== '{') { | ||
| offset++; | ||
| } | ||
| if (offset >= text.length) { | ||
| return new vscode.Range(startPos, startPos); | ||
| } | ||
| let depth = 1; | ||
| offset++; | ||
| while (offset < text.length && depth > 0) { | ||
| if (text[offset] === '{') depth++; | ||
| else if (text[offset] === '}') depth--; | ||
| offset++; | ||
| } | ||
| return new vscode.Range(startPos, document.positionAt(offset)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brace counting doesn't skip string literals or comments.
The block range finder counts all { and } characters, including those inside string literals and comments. This can produce incorrect ranges for code like:
function foo() {
const s = "{ not a block }";
}Since this is fallback parsing, this is acceptable, but worth documenting.
- Add missing autodev.codelens.showMenu command declaration - Remove unsupported languages (csharp, cpp, c) from CodeLens registration - Fix Python test file naming to use consistent test_*.py format - Implement robust WASM file bundling and path resolution - Add copy-wasm.js script to copy WASM files during build - Update build script to include web-tree-sitter external and copy:wasm - Implement multi-strategy path resolution (extension path + node_modules fallback) - Add .vscodeignore to ensure WASM files are packaged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
mpp-vscode/src/providers/code-element-parser.ts (4)
482-516: TS/JS regex fallback misses arrow functions with complex parameter listsThe
functionRegexfallback uses\([^)]*\)to match parameters, which fails on nested parentheses (e.g., typed/destructured params like(options: { name: string; get: () => number })). This is a known limitation, but it will cause some valid functions to be skipped in regex mode.If you want to improve coverage, consider a more tolerant pattern (e.g., a non‑greedy
.*?up to=>/{) or explicitly documenting this limitation nearparseTypeScriptRegex.
571-585: Java/Kotlin regex ignores abstract methods, brace-on-next-line, and Kotlin expression bodiesThe Java/Kotlin
methodRegexrequires a{on the same line as the signature and assumes a block body. It will miss:
- Abstract methods ending with
;- Methods formatted with the opening brace on the next line
- Kotlin single‑expression functions using
=If you want broader coverage in fallback mode, consider making the terminator more flexible:
- const methodRegex = /^\s*(public|private|protected)?\s*(static|final|abstract)?\s*(\w+)\s+(\w+)\s*\([^)]*\)\s*{/gm; + // Match methods ending with '{', ';', or '=' (possibly with whitespace/newlines before) + const methodRegex = + /^\s*(public|private|protected)?\s*(static|final|abstract)?\s*(\w+)\s+(\w+)\s*\([^)]*\)\s*(\{|\=|;)/gm;
628-645: Brace counting infindBlockRangedoes not skip strings/comments
findBlockRangecounts every{and}character, including those in string literals and comments. This can produce incorrect ranges in edge cases and was already noted earlier.Given this is only used in regex fallback mode, it’s likely acceptable, but consider:
- Adding a short comment to document the limitation, or
- Enhancing the scanner to skip over basic string/comment forms if this becomes user-visible.
243-252: Initialization failure permanently poisonsinitPromise, preventing retriesIf
doInitialize()throws,CodeElementParser.initPromiseremains set to the rejected promise. Subsequent calls toinitialize()will just await the same rejected promise, so Tree-sitter initialization is never retried until the extension reloads. This was already called out in a previous review.You can allow retry attempts by clearing
initPromisein the catch block:private async doInitialize(): Promise<void> { try { CodeElementParser.ParserClass = await getParser(); await CodeElementParser.ParserClass.init(); CodeElementParser.parserInstance = new CodeElementParser.ParserClass(); this.log('Tree-sitter parser initialized'); } catch (error) { this.log(`Failed to initialize tree-sitter: ${error}`); + CodeElementParser.initPromise = null; throw error; } }
🧹 Nitpick comments (2)
mpp-vscode/scripts/copy-wasm.js (1)
18-48: Consider failing the build (or setting a non‑zero exit code) when required WASM files are missingRight now missing artifacts only emit warnings and the script always exits successfully. If the extension relies on these grammars for core features, it may be better to fail fast during the build instead of degrading at runtime via regex fallbacks.
For example, you could treat “no files copied” as an error:
let copiedCount = 0; for (const file of wasmFiles) { const sourcePath = path.join(sourceDir, file); const targetPath = path.join(targetDir, file); @@ } -console.log(`\nCopied ${copiedCount}/${wasmFiles.length} WASM files to dist/wasm/`); +console.log(`\nCopied ${copiedCount}/${wasmFiles.length} WASM files to dist/wasm/`); +if (copiedCount === 0) { + console.error('No WASM files were copied; failing build.'); + process.exitCode = 1; +}Adjust the condition if only a subset of languages is truly required.
mpp-vscode/src/providers/code-element-parser.ts (1)
45-60: MemoizedQuery caches a single Query instance per query string, not per Language
MemoizedQuerycurrently stores onecompiledQueryand reuses it across calls, even ifquery(language)is invoked with differentLanguageinstances (e.g.,typescriptvstypescriptreactgrammars). In Tree-sitter, queries are conceptually bound to a specificLanguage; reusing oneQueryfor multiple languages is brittle.Not a correctness bug in practice if the grammars share node sets, but for robustness you may want per-language caching, e.g.:
class MemoizedQuery { - private readonly queryStr: string; - private compiledQuery: Query | undefined; + private readonly queryStr: string; + private compiledQueries = new Map<Language, Query>(); @@ - query(language: Language): Query { - if (this.compiledQuery) { - return this.compiledQuery; - } - this.compiledQuery = language.query(this.queryStr); - return this.compiledQuery; - } + query(language: Language): Query { + const existing = this.compiledQueries.get(language); + if (existing) { + return existing; + } + const compiled = language.query(this.queryStr); + this.compiledQueries.set(language, compiled); + return compiled; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
mpp-vscode/.vscodeignore(1 hunks)mpp-vscode/package.json(4 hunks)mpp-vscode/scripts/copy-wasm.js(1 hunks)mpp-vscode/src/extension.ts(2 hunks)mpp-vscode/src/prompts/prompt-templates.ts(1 hunks)mpp-vscode/src/providers/code-element-parser.ts(1 hunks)mpp-vscode/src/providers/codelens-provider.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- mpp-vscode/.vscodeignore
🚧 Files skipped from review as they are similar to previous changes (4)
- mpp-vscode/src/extension.ts
- mpp-vscode/src/providers/codelens-provider.ts
- mpp-vscode/package.json
- mpp-vscode/src/prompts/prompt-templates.ts
🧰 Additional context used
🧬 Code graph analysis (1)
mpp-vscode/src/providers/code-element-parser.ts (1)
mpp-codegraph/src/wasmJsMain/kotlin/cc/unitmesh/codegraph/parser/wasm/TreeSitterInterop.kt (2)
rootNode(748-793)nodeType(576-744)
| private buildBlock( | ||
| rootNode: SyntaxNode, | ||
| memoizedQuery: MemoizedQuery, | ||
| language: Language, | ||
| elementType: CodeElementType, | ||
| autoSelectInsideParent: string[], | ||
| document: vscode.TextDocument | ||
| ): CodeElement[] { | ||
| try { | ||
| const query = memoizedQuery.query(language); | ||
| const matches = query.matches(rootNode); | ||
|
|
||
| return matches.flatMap(match => { | ||
| let blockNode = match.captures[0]?.node; | ||
| const idNode = match.captures[1]?.node; | ||
|
|
||
| if (!blockNode || !idNode) { | ||
| return []; | ||
| } | ||
|
|
||
| // Handle autoSelectInsideParent | ||
| if (autoSelectInsideParent.length > 0) { | ||
| for (const nodeType of autoSelectInsideParent) { | ||
| if (blockNode.parent?.type === nodeType) { | ||
| blockNode = blockNode.parent; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const nameRange = new vscode.Range( | ||
| new vscode.Position(idNode.startPosition.row, idNode.startPosition.column), | ||
| new vscode.Position(idNode.endPosition.row, idNode.endPosition.column) | ||
| ); | ||
|
|
||
| const bodyRange = new vscode.Range( | ||
| new vscode.Position(blockNode.startPosition.row, blockNode.startPosition.column), | ||
| new vscode.Position(blockNode.endPosition.row, blockNode.endPosition.column) | ||
| ); | ||
|
|
||
| return [{ | ||
| type: elementType, | ||
| name: idNode.text, | ||
| nameRange, | ||
| bodyRange, | ||
| code: blockNode.text | ||
| }]; | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and examine the buildBlock method
fd "code-element-parser.ts" --type fRepository: phodal/auto-dev-sketch
Length of output: 113
🏁 Script executed:
# Check the structure of the file and look at the buildBlock method
wc -l mpp-vscode/src/providers/code-element-parser.tsRepository: phodal/auto-dev-sketch
Length of output: 117
🏁 Script executed:
# Read the buildBlock method to confirm the current implementation
sed -n '402,448p' mpp-vscode/src/providers/code-element-parser.ts | cat -nRepository: phodal/auto-dev-sketch
Length of output: 1857
🏁 Script executed:
# Search for Go language query configuration
rg "go" mpp-vscode/src/providers/ -i -A 5 -B 5 | head -100Repository: phodal/auto-dev-sketch
Length of output: 4341
🏁 Script executed:
# Look for query definitions, especially for Go methods
fd "query" mpp-vscode/ -type fRepository: phodal/auto-dev-sketch
Length of output: 236
🏁 Script executed:
# Search for method_declaration pattern
rg "method_declaration" mpp-vscode/ -rRepository: phodal/auto-dev-sketch
Length of output: 479
🏁 Script executed:
# Find the Go methodQuery definition
rg "go.*methodQuery|methodQuery.*method_declaration" mpp-vscode/ -A 10 -B 2Repository: phodal/auto-dev-sketch
Length of output: 48
🏁 Script executed:
# Look for the complete Go language configuration block
rg "go:" mpp-vscode/src/providers/code-element-parser.ts -A 30Repository: phodal/auto-dev-sketch
Length of output: 882
🏁 Script executed:
# Check if there's a methodQuery for Go with method_declaration
rg "method_declaration" mpp-vscode/ -B 5 -A 5Repository: phodal/auto-dev-sketch
Length of output: 1745
🏁 Script executed:
# Verify tree-sitter capture syntax and what the API provides
rg "match\.captures" mpp-vscode/src/providers/code-element-parser.ts -B 3 -A 3Repository: phodal/auto-dev-sketch
Length of output: 309
🏁 Script executed:
# Check the MemoizedQuery class to understand capture structure
rg "class MemoizedQuery|interface.*capture" mpp-vscode/src/providers/ -A 10Repository: phodal/auto-dev-sketch
Length of output: 866
🏁 Script executed:
# Check the tree-sitter imports and how captures are used elsewhere
rg "capture" mpp-vscode/src/providers/code-element-parser.ts -B 2 -A 2Repository: phodal/auto-dev-sketch
Length of output: 241
🏁 Script executed:
# Look for any TypeScript type definitions or documentation about captures
rg "web-tree-sitter|tree-sitter" mpp-vscode/ --type json -A 5Repository: phodal/auto-dev-sketch
Length of output: 824
🏁 Script executed:
# Check package.json for tree-sitter version
cat mpp-vscode/package.json | grep -A 5 -B 5 "tree-sitter"Repository: phodal/auto-dev-sketch
Length of output: 917
🏁 Script executed:
# Verify tree-sitter 0.25.10 capture API - check if there are any comments/examples in code
rg "captures\[" mpp-vscode/src/ -B 5 -A 5Repository: phodal/auto-dev-sketch
Length of output: 957
🏁 Script executed:
# Check if there's any existing use of capture names
rg "\.name" mpp-vscode/src/providers/code-element-parser.ts | head -20Repository: phodal/auto-dev-sketch
Length of output: 48
Go methods are incorrectly labeled with receiver names instead of method names due to brittle capture indexing
The buildBlock method assumes match.captures[0] is always the block node and match.captures[1] is the name node. This works for Java, Kotlin, and Rust queries which produce exactly two captures, but fails for Go. The Go method_declaration query captures three nodes—@method-body, @receiver-struct-name, and @method-name—causing idNode to become the receiver struct name instead of the method name.
The fix should derive nodes from capture names instead of relying on positional indices:
private buildBlock(
@@
): CodeElement[] {
try {
const query = memoizedQuery.query(language);
const matches = query.matches(rootNode);
return matches.flatMap(match => {
- let blockNode = match.captures[0]?.node;
- const idNode = match.captures[1]?.node;
+ if (match.captures.length === 0) {
+ return [];
+ }
+
+ const captures = match.captures as Array<{ node: SyntaxNode; name: string }>;
+
+ // Prefer captures that clearly denote the block/body, fall back to first capture
+ let blockNode =
+ captures.find(c =>
+ c.name === 'definition.class' ||
+ c.name === 'definition.method' ||
+ c.name === 'type_declaration' ||
+ c.name === 'function-body' ||
+ c.name === 'method-body'
+ )?.node ?? captures[0].node;
+
+ // Prefer captures that clearly denote the identifier/name
+ const idNode =
+ captures.find(c =>
+ c.name.includes('name.') || // e.g. name.definition.method
+ c.name === 'type_identifier' ||
+ c.name === 'type-name' ||
+ c.name === 'function-name' ||
+ c.name === 'method-name'
+ )?.node ?? captures[1]?.node;
if (!blockNode || !idNode) {
return [];
}
@@
const bodyRange = new vscode.Range(
new vscode.Position(blockNode.startPosition.row, blockNode.startPosition.column),
new vscode.Position(blockNode.endPosition.row, blockNode.endPosition.column)
);
@@
});
} catch (error) {
this.log(`Error building block: ${error}`);
return [];
}
}This preserves current behavior for existing queries while fixing Go method parsing and making the code resilient to future query changes.
Update mppVersion in gradle.properties and add new media assets and backup files for the upcoming release.
Summary
This PR implements CodeLens functionality for mpp-vscode and fixes the CodeFence parsing issue where devin instructions showed an extra CodeBlockRenderer.
Changes
CodeLens Implementation
code-element-parser.ts)prompt-templates.ts)CodeFence Fix
codeFence.tsto match mpp-core'sCodeFence.parseAlllogicpreProcessDevinBlockto convert ```devin blocks to<devin>tags<devin>,<thinking>, and<!-- walkthrough_start -->tagsOther Changes
getCurrentModelConfigto ChatViewProvider for CodeLens actionsweb-tree-sitterand@unit-mesh/treesitter-artifactsdependenciesNew Files
mpp-vscode/src/providers/code-element-parser.ts- Tree-sitter based code parsermpp-vscode/src/providers/codelens-provider.ts- CodeLens providermpp-vscode/src/commands/codelens-commands.ts- CodeLens command handlersmpp-vscode/src/actions/auto-actions.ts- AutoComment, AutoTest, AutoMethodmpp-vscode/src/prompts/prompt-templates.ts- Prompt templatesTesting
cd mpp-vscode/webview && npm run buildcd mpp-vscode && npm run build:extensionSummary by CodeRabbit
New Features
Changes
Chores
UI
✏️ Tip: You can customize this high-level summary in your review settings.