Phase 21: Add flux_generate tool to MCP server - #40
Conversation
Add flux_generate as the 7th tool to the MCP server, enabling LLMs to generate FTL programs from natural language requirements via the MCP protocol. The tool wraps the existing GenerationLoop with proper error handling for missing API keys and invalid parameters. - Tool definition in build_tools() with full inputSchema - Handler handle_flux_generate() using tokio::runtime::Runtime for async - Input: requirement (required), requirement_type, provider, model, max_iterations - Output: GenerationResult as JSON - API key errors returned as tool errors (isError: true), no panics - Tests: mcp_flux_generate_in_tools_list, mcp_flux_generate_missing_api_key - Updated tools_list test to assert 7 tools - Added mcp-integration.md with full tool documentation Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates a new Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Hallo, danke für den Pull Request. Die Ergänzung des flux_generate-Tools ist eine tolle Erweiterung für den MCP-Server. Der Code ist gut strukturiert und die asynchrone Ausführung über Tokio ist sinnvoll umgesetzt.
Ich habe einige Anmerkungen, die hauptsächlich die Konsistenz, Wartbarkeit und Leistung betreffen:
- Es gibt eine Inkonsistenz bei den Werten für
requirement_typezwischen der Implementierung, dem Tool-Schema und der Dokumentation. - Die Funktion
handle_flux_generatekönnte durch Auslagerung der Logik in eine Hilfsfunktion lesbarer gestaltet werden. - Das wiederholte Erstellen einer Tokio-Runtime in
handle_flux_generatekönnte zu Leistungseinbußen führen. - In den Tests gibt es etwas Redundanz und eine Prüfungsbedingung könnte spezifischer formuliert werden.
Details dazu finden Sie in den Kommentaren zu den einzelnen Codezeilen. Insgesamt ist dies ein solider Beitrag!
| "type": "object", | ||
| "properties": { | ||
| "requirement": { "type": "string", "description": "Natural language description of the desired FTL program" }, | ||
| "requirement_type": { "type": "string", "description": "Type of requirement: translate, explain, optimize, refactor", "default": "translate" }, |
There was a problem hiding this comment.
Das Schema für requirement_type ist inkonsistent mit der Implementierung in flux-ftl/src/llm.rs. Das Schema beschreibt die Werte translate, explain, optimize, refactor, während die Implementierung translate, optimize, invent, discover erwartet. Dies sollte korrigiert werden, um die tatsächliche Funktionalität widerzuspiegeln. Bitte denken Sie daran, auch die Dokumentation in flux-ftl/mcp-integration.md entsprechend zu aktualisieren.
| "requirement_type": { "type": "string", "description": "Type of requirement: translate, explain, optimize, refactor", "default": "translate" }, | |
| "requirement_type": { "type": "string", "description": "Type of requirement: translate, optimize, invent, discover", "default": "translate" }, |
| fn handle_flux_generate(stdout: &std::io::Stdout, id: Value, args: &Value) { | ||
| let requirement = match args.get("requirement").and_then(|v| v.as_str()) { | ||
| Some(s) => s, | ||
| None => { | ||
| send_tool_error(stdout, id, "Missing required argument: requirement"); | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| let requirement_type = args | ||
| .get("requirement_type") | ||
| .and_then(|v| v.as_str()) | ||
| .unwrap_or("translate"); | ||
| let provider = args | ||
| .get("provider") | ||
| .and_then(|v| v.as_str()) | ||
| .unwrap_or("anthropic"); | ||
| let model = args.get("model").and_then(|v| v.as_str()); | ||
| let max_iterations = args | ||
| .get("max_iterations") | ||
| .and_then(|v| v.as_u64()) | ||
| .unwrap_or(5) as u32; | ||
|
|
||
| // Parse requirement type | ||
| let req_type = match RequirementType::from_str_loose(requirement_type) { | ||
| Ok(t) => t, | ||
| Err(e) => { | ||
| send_tool_error(stdout, id, &format!("Invalid requirement_type: {}", e)); | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| // Parse provider | ||
| let llm_provider = match LlmProvider::from_str_loose(provider) { | ||
| Ok(p) => p, | ||
| Err(e) => { | ||
| send_tool_error(stdout, id, &format!("Invalid provider: {}", e)); | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| // Build config from environment | ||
| let mut config = match LlmConfig::from_env(llm_provider, model.map(String::from)) { | ||
| Ok(c) => c, | ||
| Err(e) => { | ||
| send_tool_error(stdout, id, &format!("Configuration error: {}", e)); | ||
| return; | ||
| } | ||
| }; | ||
| config.max_iterations = max_iterations; | ||
|
|
||
| // Create generation loop | ||
| let gen_loop = match GenerationLoop::new(config) { | ||
| Ok(l) => l, | ||
| Err(e) => { | ||
| send_tool_error( | ||
| stdout, | ||
| id, | ||
| &format!("Failed to create generation loop: {}", e), | ||
| ); | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| let request = GenerateRequest { | ||
| requirement: requirement.to_string(), | ||
| requirement_type: req_type, | ||
| context: None, | ||
| examples: Vec::new(), | ||
| }; | ||
|
|
||
| // Run async generation loop | ||
| let rt = match tokio::runtime::Runtime::new() { | ||
| Ok(rt) => rt, | ||
| Err(e) => { | ||
| send_tool_error( | ||
| stdout, | ||
| id, | ||
| &format!("Failed to create async runtime: {}", e), | ||
| ); | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| let result = match rt.block_on(gen_loop.generate(&request)) { | ||
| Ok(r) => r, | ||
| Err(e) => { | ||
| send_tool_error(stdout, id, &format!("Generation failed: {}", e)); | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| let json = match serde_json::to_string(&result) { | ||
| Ok(j) => j, | ||
| Err(e) => { | ||
| send_tool_error(stdout, id, &format!("Serialization error: {}", e)); | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| send_tool_result(stdout, id, &json); | ||
| } |
There was a problem hiding this comment.
Die Funktion handle_flux_generate ist sehr lang und enthält viel repetitiven Code für die Fehlerbehandlung. Dies beeinträchtigt die Lesbarkeit und Wartbarkeit. Ich schlage vor, die Logik in eine separate Funktion handle_flux_generate_inner auszulagern, die ein Result zurückgibt. Dadurch kann der ?-Operator für eine prägnantere Fehlerbehandlung verwendet werden.
Die neue handle_flux_generate_inner Funktion könnte so aussehen:
fn handle_flux_generate_inner(args: &Value) -> Result<String, String> {
let requirement = args
.get("requirement")
.and_then(|v| v.as_str())
.ok_or_else(|| "Missing required argument: requirement".to_string())?;
let requirement_type = args
.get("requirement_type")
.and_then(|v| v.as_str())
.unwrap_or("translate");
let provider = args
.get("provider")
.and_then(|v| v.as_str())
.unwrap_or("anthropic");
let model = args.get("model").and_then(|v| v.as_str());
let max_iterations = args
.get("max_iterations")
.and_then(|v| v.as_u64())
.unwrap_or(5) as u32;
let req_type = RequirementType::from_str_loose(requirement_type)
.map_err(|e| format!("Invalid requirement_type: {}", e))?;
let llm_provider = LlmProvider::from_str_loose(provider)
.map_err(|e| format!("Invalid provider: {}", e))?;
let mut config = LlmConfig::from_env(llm_provider, model.map(String::from))
.map_err(|e| format!("Configuration error: {}", e))?;
config.max_iterations = max_iterations;
let gen_loop = GenerationLoop::new(config)
.map_err(|e| format!("Failed to create generation loop: {}", e))?;
let request = GenerateRequest {
requirement: requirement.to_string(),
requirement_type: req_type,
context: None,
examples: Vec::new(),
};
let rt = tokio::runtime::Runtime::new()
.map_err(|e| format!("Failed to create async runtime: {}", e))?;
let result = rt
.block_on(gen_loop.generate(&request))
.map_err(|e| format!("Generation failed: {}", e))?;
serde_json::to_string(&result).map_err(|e| format!("Serialization error: {}", e))
}Sie müssten diese Funktion dann über der handle_flux_generate definieren.
fn handle_flux_generate(stdout: &std::io::Stdout, id: Value, args: &Value) {
match handle_flux_generate_inner(args) {
Ok(result_json) => send_tool_result(stdout, id, &result_json),
Err(error_message) => send_tool_error(stdout, id, &error_message),
}
}| let rt = match tokio::runtime::Runtime::new() { | ||
| Ok(rt) => rt, | ||
| Err(e) => { | ||
| send_tool_error( | ||
| stdout, | ||
| id, | ||
| &format!("Failed to create async runtime: {}", e), | ||
| ); | ||
| return; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Das Erstellen einer neuen tokio::runtime::Runtime bei jedem Aufruf von handle_flux_generate kann zu Leistungseinbußen führen, da dies ein relativ aufwändiger Vorgang ist. Obwohl die Netzwerklatenz zum LLM wahrscheinlich dominiert, wäre es für eine bessere Leistung und Ressourcennutzung vorteilhaft, die Runtime einmal zu initialisieren und wiederzuverwenden. Eine Möglichkeit wäre, die Runtime mit once_cell oder lazy_static global zu erstellen oder die main-Funktion als async zu deklarieren und die Runtime von Tokio verwalten zu lassen.
| #[test] | ||
| fn mcp_flux_generate_in_tools_list() { | ||
| let resp = send_jsonrpc("tools/list", None); | ||
|
|
||
| let tools = resp["result"]["tools"].as_array().expect("tools should be array"); | ||
| assert_eq!(tools.len(), 7); | ||
|
|
||
| let gen_tool = tools.iter().find(|t| t["name"] == "flux_generate"); | ||
| assert!(gen_tool.is_some(), "flux_generate should be in tools list"); | ||
|
|
||
| let gen_tool = gen_tool.unwrap(); | ||
| assert!(gen_tool["description"].as_str().unwrap().contains("Generate")); | ||
| let schema = &gen_tool["inputSchema"]; | ||
| assert!(schema["properties"]["requirement"].is_object()); | ||
| assert!(schema["properties"]["requirement_type"].is_object()); | ||
| assert!(schema["properties"]["provider"].is_object()); | ||
| assert!(schema["properties"]["model"].is_object()); | ||
| assert!(schema["properties"]["max_iterations"].is_object()); | ||
| let required = schema["required"].as_array().unwrap(); | ||
| assert!(required.iter().any(|r| r == "requirement")); | ||
| } |
There was a problem hiding this comment.
Dieser Test mcp_flux_generate_in_tools_list überschneidet sich stark mit dem Test mcp_tools_list. Beide Tests rufen tools/list auf und prüfen die Anwesenheit und das Schema von flux_generate. Um die Redundanz zu reduzieren und die Wartung der Tests zu vereinfachen, könnten die spezifischen Prüfungen für flux_generate aus diesem Test in den mcp_tools_list-Test integriert werden. Dadurch wären alle tools/list-bezogenen Prüfungen an einem Ort gebündelt.
| text.to_lowercase().contains("error") | ||
| || text.to_lowercase().contains("api") | ||
| || text.to_lowercase().contains("key"), |
There was a problem hiding this comment.
Die Überprüfung der Fehlermeldung ist recht allgemein. Sie prüft nur auf das Vorhandensein von "error", "api" oder "key". Der Test wäre aussagekräftiger und robuster, wenn er auf eine spezifischere Zeichenkette prüfen würde, die in der erwarteten Fehlermeldung vorkommt, z. B. "missing api key". Dies stellt sicher, dass der korrekte Fehlergrund gemeldet wird.
| text.to_lowercase().contains("error") | |
| || text.to_lowercase().contains("api") | |
| || text.to_lowercase().contains("key"), | |
| text.to_lowercase().contains("missing api key"), |
1 participant
Summary
flux_generatefür LLM-gesteuerte FTL-GenerierungTest plan
Closes #34
🤖 Generated with Claude Code