Uml diagram #52
Uml diagram #52
Conversation
mend
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive UML diagram functionality via MCP, including support for UML class creation, manipulation, and example implementations. It also introduces workflow generation capabilities from amt-compose projects and includes several complete project structures for demonstration.
- Added UML class diagram tools (add/remove/update) with attributes, methods, and associations
- Implemented tasklist MCP client for generating workflow diagrams from amt-compose projects
- Added complete amt-compose example project with WIT interfaces and build system
Reviewed Changes
Copilot reviewed 48 out of 53 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| glsp-mcp-server/src/mcp/tools.rs | Added UML class manipulation tools and duplicate tool definitions |
| glsp-mcp-server/src/backend.rs | Implemented UML tools with incomplete method definitions |
| tasklist_mcp_client/ | Complete MCP client for workflow generation from amt-compose projects |
| workspace/amt/simple/ | Full amt-compose project structure with WIT interfaces and build system |
| workspace/diagrams/ | Generated diagram files from GLSP tool usage |
| composer/ | WIT composition example project with math/user interfaces |
Files not reviewed (1)
- glsp-web-client/package-lock.json: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let mut tools = Vec::new(); | ||
| tools.push(Tool { | ||
| name: "remove_uml_class".to_string(), |
There was a problem hiding this comment.
There are two separate tools vectors being created and the first one is never used. The first vector (lines 89-139) creates UML tools but is discarded when the second vector is created at line 140.
Copilot uses AI. Check for mistakes.
| let mut tools = vec![ | ||
| Tool { | ||
| name: "create_diagram".to_string(), |
There was a problem hiding this comment.
Duplicate tool definitions exist - the UML tools are defined twice (once starting at line 89 and again integrated into the main tools vector). The first definition should be removed or integrated properly.
Copilot uses AI. Check for mistakes.
| "update_component_group" => self.update_component_group(params.arguments).await, | ||
| "get_component_group_interfaces" => { | ||
| self.get_component_group_interfaces(params.arguments).await | ||
| _ => Err(anyhow::anyhow!(format!("Unknown tool: {}", params.name))) |
There was a problem hiding this comment.
This error case was changed from returning a CallToolResult with is_error: Some(true) to returning an Err, which changes the error handling behavior and may break existing clients expecting the previous format.
| _ => Err(anyhow::anyhow!(format!("Unknown tool: {}", params.name))) | |
| _ => Ok(CallToolResult { | |
| content: vec![TextContent { | |
| content_type: "text".to_string(), | |
| text: format!("Unknown tool: {}", params.name), | |
| }], | |
| is_error: Some(true), | |
| }) |
Copilot uses AI. Check for mistakes.
| if let Some(s) = item.as_str() { | ||
| serde_json::json!({"name": s, "returnType": "", "visibility": "public"}) | ||
| } else if let Some(obj) = item.as_object() { | ||
| serde_json::Value::Object(obj.clone()) | ||
| } else { | ||
| serde_json::json!({"name": "method", "returnType": "", "visibility": "public"}) | ||
| } | ||
| }).collect::<Vec<_>>() |
There was a problem hiding this comment.
The if-else chain is incomplete and doesn't properly return values. Each branch should be wrapped in a proper return structure or assigned to a variable.
| if let Some(s) = item.as_str() { | |
| serde_json::json!({"name": s, "returnType": "", "visibility": "public"}) | |
| } else if let Some(obj) = item.as_object() { | |
| serde_json::Value::Object(obj.clone()) | |
| } else { | |
| serde_json::json!({"name": "method", "returnType": "", "visibility": "public"}) | |
| } | |
| }).collect::<Vec<_>>() | |
| match item { | |
| serde_json::Value::String(s) => { | |
| serde_json::json!({"name": s, "returnType": "", "visibility": "public"}) | |
| } | |
| serde_json::Value::Object(obj) => { | |
| serde_json::Value::Object(obj.clone()) | |
| } | |
| _ => { | |
| serde_json::json!({"name": "method", "returnType": "", "visibility": "public"}) | |
| } | |
| } | |
| }).collect::<Vec<_>>() |
Copilot uses AI. Check for mistakes.
| use regex::Regex; | ||
|
|
||
| pub fn extract_diagram_id(text: &str) -> Option<String> { | ||
| let re = Regex::new(r"(?:diagram ID|with ID): ([a-f0-9\-]+)").ok()?; |
There was a problem hiding this comment.
Regex compilation should be cached using lazy_static or once_cell since this function may be called multiple times with the same pattern.
Copilot uses AI. Check for mistakes.
| } | ||
|
|
||
| pub fn extract_node_id(text: &str) -> Option<String> { | ||
| let re = Regex::new(r"ID: ([a-f0-9\-]+)").ok()?; |
There was a problem hiding this comment.
Regex compilation should be cached using lazy_static or once_cell since this function may be called multiple times with the same pattern.
Copilot uses AI. Check for mistakes.
| let re = Regex::new(r"ID:\s*([a-f0-9-]{36})") | ||
| .context("Failed to compile UUID regex")?; |
There was a problem hiding this comment.
Regex compilation should be cached using lazy_static or once_cell as noted in the comment on line 239, but the implementation still compiles on each call.
Copilot uses AI. Check for mistakes.
- Document current repository health and dependencies - Include recent commit history and toolchain versions - Outline project structure and current features - Define next steps and future enhancements
- STATUS.md: UML diagram branch status report - STATUS-main.md: Main branch status report - STATUS-ralf-main.md: Ralf GLSP MCP main branch (v0.2.0) - STATUS-pre-commit-debugging.md: Pre-commit hooks & debugging branch Each report includes: - Current repository health and dependencies - Recent commit history (last 10 commits) - Project structure and features - Development environment details - Build targets and performance metrics - Next steps and future enhancements
added uml diagram functionality via mcp