tools: Add semantic AI server for command intent classification and validation#1
Conversation
Co-authored-by: TheOriginalBytePlayer <[email protected]>
Co-authored-by: TheOriginalBytePlayer <[email protected]>
…mprove docs Co-authored-by: TheOriginalBytePlayer <[email protected]>
Co-authored-by: TheOriginalBytePlayer <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR implements a 64-bit C++ Semantic AI Server for FrameForge Studio that processes natural language commands through intent classification and validation. The server uses llama.cpp for LLM-based command interpretation and provides IPC communication via Named Pipes for integration with external applications.
Key Changes
- Command Schema System: Defines 15 verbs across 4 action groups (CAMERA_CONTROL, ACTOR_POSE, OBJECT_MGMT, SHOT_MGMT) with parameter specifications
- Intent Classification Engine: Llama-based NLP processing with temperature=0.1 for deterministic output, including fuzzy matching for typo correction (Levenshtein distance ≤2)
- Cross-platform IPC: Named Pipes implementation for Windows and Unix with bidirectional communication support
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 |
|---|---|
| tools/semantic-server/command-schema.h | Defines command schema, verbs, action groups, and parameter specifications with enum converters |
| tools/semantic-server/command-validator.h | Implements validation logic with fuzzy verb matching using Levenshtein distance |
| tools/semantic-server/intent-engine.h | Wraps llama.cpp for intent classification with system prompt and JSON extraction |
| tools/semantic-server/ipc-handler.h | Cross-platform Named Pipes implementation for Windows and Unix IPC |
| tools/semantic-server/semantic-server.cpp | Main server application with dual-mode operation (interactive CLI and IPC) |
| tools/semantic-server/test-validator.cpp | C++ unit tests for schema registry, validator, fuzzy matching, and enum conversions |
| tools/semantic-server/test_schema.py | Python tests for command schemas, JSON validation, and error formats |
| tools/semantic-server/test_client.py | Test client demonstrating Named Pipe communication |
| tools/semantic-server/README.md | User documentation with examples and integration guide |
| tools/semantic-server/IMPLEMENTATION_SUMMARY.md | Architecture overview and implementation details |
| tools/semantic-server/CMakeLists.txt | Build configuration for the semantic server and test executable |
| tools/CMakeLists.txt | Integration of semantic-server subdirectory into the build system |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Check if message is already JSON (from whisper or other source) | ||
| json input; | ||
| bool is_json_input = false; | ||
|
|
||
| try { | ||
| input = json::parse(message); | ||
| is_json_input = true; | ||
|
|
||
| // Check if it contains transcribed text | ||
| if (input.contains("text")) { | ||
| std::string text = input["text"].get<std::string>(); | ||
| auto result = intent_engine.process_input(text); | ||
|
|
||
| // Send validated command through pipe | ||
| json output = result.to_json(); | ||
| std::string response = output.dump(); | ||
| ipc_handler.send_message(response); | ||
|
|
||
| LOG_INF("Sent response: %s\n", response.c_str()); | ||
| } | ||
| } catch (...) { | ||
| // Not JSON, treat as plain text | ||
| is_json_input = false; | ||
| } | ||
|
|
||
| if (!is_json_input) { | ||
| // Process as plain text command | ||
| auto result = intent_engine.process_input(message); | ||
|
|
||
| // Send validated command through pipe | ||
| json output = result.to_json(); | ||
| std::string response = output.dump(); | ||
| ipc_handler.send_message(response); | ||
|
|
||
| LOG_INF("Sent response: %s\n", response.c_str()); | ||
| } |
There was a problem hiding this comment.
The nested if-else structure with is_json_input checking creates duplicated code for processing and sending responses (lines 157-166 and 173-182). This violates the DRY principle and makes maintenance harder. Consider extracting the response processing logic into a helper function to reduce duplication.
| if (pipe_handle == INVALID_HANDLE_VALUE) { | ||
| return false; | ||
| } | ||
|
|
||
| DWORD bytes_written; | ||
| BOOL success = WriteFile( | ||
| pipe_handle, | ||
| message.c_str(), | ||
| message.size(), | ||
| &bytes_written, | ||
| NULL | ||
| ); | ||
|
|
||
| return success && bytes_written == message.size(); |
There was a problem hiding this comment.
The pipe_handle may be invalid when send_message_windows is called if the client has disconnected between reads. This can lead to writing to an invalid handle. Consider checking if the pipe is still connected or handling the write failure more gracefully by returning false with appropriate logging.
| if (pipe_handle == INVALID_HANDLE_VALUE) { | |
| return false; | |
| } | |
| DWORD bytes_written; | |
| BOOL success = WriteFile( | |
| pipe_handle, | |
| message.c_str(), | |
| message.size(), | |
| &bytes_written, | |
| NULL | |
| ); | |
| return success && bytes_written == message.size(); | |
| // If we already know the handle is invalid, fail fast. | |
| if (pipe_handle == INVALID_HANDLE_VALUE) { | |
| return false; | |
| } | |
| // Sanity-check the handle type; if it's invalid, treat as failure. | |
| DWORD file_type = GetFileType(pipe_handle); | |
| if (file_type == FILE_TYPE_UNKNOWN && GetLastError() == ERROR_INVALID_HANDLE) { | |
| // Handle is no longer valid; normalize state and fail. | |
| CloseHandle(pipe_handle); | |
| pipe_handle = INVALID_HANDLE_VALUE; | |
| return false; | |
| } | |
| DWORD bytes_written = 0; | |
| BOOL success = WriteFile( | |
| pipe_handle, | |
| message.c_str(), | |
| static_cast<DWORD>(message.size()), | |
| &bytes_written, | |
| NULL | |
| ); | |
| if (!success) { | |
| DWORD error = GetLastError(); | |
| // If the pipe is broken or the other end disconnected, close and invalidate the handle. | |
| if (error == ERROR_BROKEN_PIPE || | |
| error == ERROR_NO_DATA || | |
| error == ERROR_PIPE_NOT_CONNECTED || | |
| error == ERROR_INVALID_HANDLE) { | |
| CloseHandle(pipe_handle); | |
| pipe_handle = INVALID_HANDLE_VALUE; | |
| } | |
| return false; | |
| } | |
| return bytes_written == static_cast<DWORD>(message.size()); |
| void stop() { | ||
| if (!running.exchange(false)) { | ||
| return; // Not running | ||
| } | ||
|
|
||
| // Signal the thread to stop | ||
| if (listener_thread.joinable()) { | ||
| listener_thread.join(); | ||
| } | ||
|
|
||
| #ifdef _WIN32 | ||
| if (pipe_handle != INVALID_HANDLE_VALUE) { | ||
| CloseHandle(pipe_handle); | ||
| pipe_handle = INVALID_HANDLE_VALUE; | ||
| } | ||
| #else | ||
| if (pipe_fd >= 0) { | ||
| close(pipe_fd); | ||
| pipe_fd = -1; | ||
| } | ||
| // Clean up the named pipe file | ||
| unlink(pipe_name.c_str()); | ||
| #endif |
There was a problem hiding this comment.
The order of operations in the stop() method is incorrect. The thread is joined before the pipe_handle is closed, but the thread may still be blocked on ReadFile when you try to join it. On Windows, you should either close the handle first to unblock the ReadFile (which will fail), or use overlapped I/O with a cancellation mechanism. The current implementation could cause the stop() method to hang indefinitely waiting for the thread to join.
| const int n_prompt = -llama_tokenize(vocab, prompt.c_str(), prompt.size(), NULL, 0, true, true); | ||
| if (n_prompt <= 0) { | ||
| return "{}"; // Return empty object on error | ||
| } |
There was a problem hiding this comment.
When tokenization fails (returns <= 0), the function returns an empty JSON object "{}", which will later be treated as invalid by the validator. However, this error is silent and provides no information about why tokenization failed. Consider logging an error message or returning a more descriptive error through the ValidationResult.
| } | ||
| } catch (...) { | ||
| // Not JSON, treat as plain text | ||
| is_json_input = false; |
There was a problem hiding this comment.
The variable is_json_input is set to true and then potentially set to false in the catch block, but the initial assignment to true happens before any JSON parsing attempt. This creates confusion about when the variable should be true vs false. Consider initializing to false and only setting to true after successful JSON parsing, which would also eliminate the need to set it to false in the catch block.
| is_json_input = false; |
| // Check if we have a complete JSON object | ||
| if (response.find('}') != std::string::npos) { | ||
| // Try to parse to see if it's complete | ||
| try { | ||
| json test = extract_json_from_text(response); | ||
| if (!test.is_null() && test.is_object()) { | ||
| break; // We have a complete JSON object | ||
| } | ||
| } catch (...) { | ||
| // Continue generating | ||
| } | ||
| } |
There was a problem hiding this comment.
The early stopping logic checks if a complete JSON object exists by finding the first '}' character. However, this can produce false positives if the JSON contains nested objects or strings with '}' characters. While the code then tries to parse the JSON to verify completeness, this approach may still stop prematurely in edge cases. Consider using a more robust JSON completeness check, such as tracking brace depth.
| try { | ||
| json test = extract_json_from_text(response); | ||
| if (!test.is_null() && test.is_object()) { | ||
| break; // We have a complete JSON object | ||
| } | ||
| } catch (...) { | ||
| // Continue generating |
There was a problem hiding this comment.
The catch-all exception handler silently continues generation when JSON parsing fails. This means the loop will continue generating tokens even if an exception occurs for a different reason (e.g., out of memory). Consider logging the exception or at least distinguishing between JSON parse errors and other exceptions.
| try { | |
| json test = extract_json_from_text(response); | |
| if (!test.is_null() && test.is_object()) { | |
| break; // We have a complete JSON object | |
| } | |
| } catch (...) { | |
| // Continue generating | |
| json test = extract_json_from_text(response); | |
| if (!test.is_null() && test.is_object()) { | |
| break; // We have a complete JSON object |
| } catch (...) { | ||
| return json(); | ||
| } |
There was a problem hiding this comment.
The catch-all exception handler returns an empty JSON object instead of a proper error. This makes it difficult to distinguish between successful parsing that returned an empty object and actual parsing failures. Consider returning a null JSON value or a specific error indicator.
| // Try to extract JSON from response if it's embedded in text | ||
| command = extract_json_from_text(llm_response); | ||
| if (command.is_null()) { | ||
| return ValidationResult::error("Failed to parse LLM response as JSON: " + std::string(e.what())); |
There was a problem hiding this comment.
The code catches json::parse_error specifically but then attempts to extract JSON from text and returns another generic error if that fails. The error message includes the original parse error details (e.what()), but if extract_json_from_text also fails, the original parse error information is lost. Consider preserving or combining both error messages for better debugging.
| // Try to extract JSON from response if it's embedded in text | |
| command = extract_json_from_text(llm_response); | |
| if (command.is_null()) { | |
| return ValidationResult::error("Failed to parse LLM response as JSON: " + std::string(e.what())); | |
| // Preserve the original parse error message | |
| const std::string initial_error = e.what(); | |
| // Try to extract JSON from response if it's embedded in text | |
| try { | |
| command = extract_json_from_text(llm_response); | |
| } catch (const json::parse_error & e2) { | |
| // Combine both error messages for better debugging | |
| return ValidationResult::error( | |
| "Failed to parse LLM response as JSON. " | |
| "Initial parse error: " + initial_error + | |
| " | Extracted JSON parse error: " + std::string(e2.what())); | |
| } | |
| if (command.is_null()) { | |
| return ValidationResult::error( | |
| "Failed to parse LLM response as JSON. " | |
| "Initial parse error: " + initial_error + | |
| " | Extracted JSON is null after fallback parsing."); |
| llama_context * ctx; | ||
| llama_model * model; | ||
| const llama_vocab * vocab; | ||
| llama_sampler * smpl; |
There was a problem hiding this comment.
The variable name 'smpl' is unclear and doesn't follow common naming conventions. Consider renaming to 'sampler' for better readability and maintainability.
|
@copilot apply changes based on the comments in this thread |
Description
Implements a 64-bit C++ semantic AI server that processes natural language commands through intent classification and JSON schema validation. Designed for real-time command interpretation with IPC support.
Core Components
command-schema.h): 15 verbs across 4 action groups (CAMERA_CONTROL, ACTOR_POSE, OBJECT_MGMT, SHOT_MGMT) with parameter specificationsintent-engine.h): Llama-based classification with temperature=0.1 for deterministic output, JSON extraction, early stopping on complete objectscommand-validator.h): Schema validation with Levenshtein distance ≤2 for fuzzy verb matching (handles common typos)ipc-handler.h): Cross-platform Named Pipes (Windows/Unix) for sub-millisecond latencysemantic-server.cpp): Dual-mode operation (interactive CLI for testing, IPC for production)Usage
Architecture
External audio transcription (e.g., whisper.cpp) → Named Pipe → Semantic Server → Validated JSON → Application
Testing
Build Integration
Added
tools/semantic-server/subdirectory to CMake. No changes to existing tools or libraries.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.