mirror of
https://github.com/thomasnordquist/MQTT-Explorer.git
synced 2026-09-12 01:23:31 +00:00
Add comprehensive chat debugging and fix documentation
Co-authored-by: thomasnordquist <7721625+thomasnordquist@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
co-authored by
thomasnordquist
thomasnordquist
parent
20befcd6fd
commit
bdbea92f7a
@@ -0,0 +1,346 @@
|
||||
# Chat Debugging and Fix Summary
|
||||
|
||||
## Problem Statement
|
||||
The AI Assistant chat was not working - users couldn't send messages and receive responses.
|
||||
|
||||
## Investigation Process
|
||||
|
||||
### Step 1: Initial Testing
|
||||
- Built the server successfully (no TypeScript errors)
|
||||
- Created Playwright test to investigate the issue
|
||||
- Started browser with server and captured console errors
|
||||
|
||||
### Step 2: Error Discovery
|
||||
Found browser console errors:
|
||||
```
|
||||
LLM Service: Invalid result from backend
|
||||
Error: No response from AI assistant
|
||||
```
|
||||
|
||||
The issue: Frontend was rejecting LLM responses that contained tool calls but no text.
|
||||
|
||||
### Step 3: Tool Call Flow Analysis
|
||||
Traced the complete message flow:
|
||||
1. User sends message
|
||||
2. Backend calls OpenAI LLM
|
||||
3. LLM returns tool calls (e.g., `get_topic`)
|
||||
4. Backend returns `{ response: "", toolCalls: [...] }`
|
||||
5. **BUG**: Frontend threw error because response was empty
|
||||
|
||||
### Step 4: Deep Debugging
|
||||
Added extensive logging to trace:
|
||||
- Message structure sent to OpenAI API
|
||||
- Tool call format and content
|
||||
- Conversation history building
|
||||
- Tool result handling
|
||||
|
||||
Discovered multiple issues:
|
||||
1. Frontend validation rejected empty responses
|
||||
2. Assistant messages missing `tool_calls` property
|
||||
3. Backend adding duplicate tool results
|
||||
4. Tool call format mismatch
|
||||
|
||||
## Root Causes
|
||||
|
||||
### Issue 1: Overly Strict Validation
|
||||
**Location**: `app/src/services/llmService.ts:722-725`
|
||||
|
||||
**Problem**:
|
||||
```typescript
|
||||
if (!result || !result.response) {
|
||||
throw new Error('No response from AI assistant')
|
||||
}
|
||||
```
|
||||
|
||||
When LLM returns tool calls, the response is empty (which is normal). The code treated this as an error.
|
||||
|
||||
**Fix**:
|
||||
```typescript
|
||||
if (!result || (!result.response && !result.toolCalls)) {
|
||||
throw new Error('No response from AI assistant')
|
||||
}
|
||||
```
|
||||
|
||||
### Issue 2: Missing tool_calls Property
|
||||
**Location**: `app/src/services/llmService.ts:748-750`
|
||||
|
||||
**Problem**:
|
||||
```typescript
|
||||
this.conversationHistory.push({
|
||||
role: 'assistant',
|
||||
content: assistantMessage,
|
||||
})
|
||||
```
|
||||
|
||||
The assistant message didn't include the `tool_calls` property that OpenAI requires. When the tool response message was sent in the next request, OpenAI rejected it because there was no preceding message with `tool_calls`.
|
||||
|
||||
**Fix**:
|
||||
```typescript
|
||||
this.conversationHistory.push({
|
||||
role: 'assistant',
|
||||
content: assistantMessage || '',
|
||||
tool_calls: toolCalls, // Required by OpenAI
|
||||
})
|
||||
```
|
||||
|
||||
### Issue 3: Duplicate Tool Messages
|
||||
**Location**: `src/server.ts:492-505`
|
||||
|
||||
**Problem**:
|
||||
The backend was adding tool results to the messages array:
|
||||
```typescript
|
||||
if (toolResults && toolResults.length > 0) {
|
||||
messagesWithToolResults = [
|
||||
...messages,
|
||||
...toolResults.map(result => ({
|
||||
role: 'tool',
|
||||
content: result.content,
|
||||
tool_call_id: result.tool_call_id,
|
||||
})),
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
But the frontend had ALREADY added them to the conversation history before sending. This resulted in:
|
||||
```
|
||||
[system, user, assistant+tool_calls, tool_result, tool_result]
|
||||
^^^^^^^^^^^ ^^^^^^^^^^^
|
||||
from frontend from backend (duplicate!)
|
||||
```
|
||||
|
||||
OpenAI rejected this because there were two tool messages for the same tool_call_id.
|
||||
|
||||
**Fix**:
|
||||
Removed the backend duplication entirely. The frontend manages the conversation history:
|
||||
```typescript
|
||||
// Frontend adds tool results to conversation history before sending
|
||||
const apiResponse = await llmClient.chat(messages) // Use messages directly
|
||||
```
|
||||
|
||||
### Issue 4: Tool Call Format
|
||||
**Location**: `app/src/services/llmService.ts:759`
|
||||
|
||||
**Problem**:
|
||||
OpenAI returns tool calls as:
|
||||
```javascript
|
||||
{
|
||||
id: "call_xxx",
|
||||
type: "function",
|
||||
function: {
|
||||
name: "get_topic",
|
||||
arguments: "{\"topic\":\"kitchen/lamp\"}"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
But `executeTool` expected:
|
||||
```javascript
|
||||
{
|
||||
id: "call_xxx",
|
||||
name: "get_topic",
|
||||
arguments: "{\"topic\":\"kitchen/lamp\"}"
|
||||
}
|
||||
```
|
||||
|
||||
**Fix**:
|
||||
Transform the format before execution:
|
||||
```typescript
|
||||
toolCalls.map((tc: any) => {
|
||||
const toolCall = {
|
||||
id: tc.id,
|
||||
name: tc.function?.name || tc.name,
|
||||
arguments: tc.function?.arguments || tc.arguments,
|
||||
}
|
||||
return this.executeTool(toolCall, currentNode)
|
||||
})
|
||||
```
|
||||
|
||||
## Changes Made
|
||||
|
||||
### Frontend (`app/src/services/llmService.ts`)
|
||||
```typescript
|
||||
// 1. Extended interface
|
||||
export interface LLMMessage {
|
||||
role: 'system' | 'user' | 'assistant' | 'tool'
|
||||
content: string
|
||||
tool_calls?: any[] // NEW
|
||||
tool_call_id?: string // NEW
|
||||
}
|
||||
|
||||
// 2. Fixed validation
|
||||
if (!result || (!result.response && !result.toolCalls)) {
|
||||
throw new Error('No response from AI assistant')
|
||||
}
|
||||
|
||||
// 3. Include tool_calls in assistant message
|
||||
this.conversationHistory.push({
|
||||
role: 'assistant',
|
||||
content: assistantMessage || '',
|
||||
tool_calls: toolCalls, // NEW
|
||||
})
|
||||
|
||||
// 4. Include tool_call_id in tool messages
|
||||
this.conversationHistory.push({
|
||||
role: 'tool' as any,
|
||||
content: toolResult.content,
|
||||
tool_call_id: toolResult.tool_call_id, // NEW
|
||||
})
|
||||
|
||||
// 5. Transform tool call format
|
||||
const toolCall = {
|
||||
id: tc.id,
|
||||
name: tc.function?.name || tc.name,
|
||||
arguments: tc.function?.arguments || tc.arguments,
|
||||
}
|
||||
```
|
||||
|
||||
### Backend (`backend/src/llmApiClient.ts`)
|
||||
```typescript
|
||||
// 1. Extended interface
|
||||
export interface LLMMessage {
|
||||
role: 'system' | 'user' | 'assistant' | 'tool'
|
||||
content: string
|
||||
tool_call_id?: string
|
||||
name?: string
|
||||
tool_calls?: LLMToolCall[] // NEW
|
||||
}
|
||||
|
||||
// 2. Preserve tool_calls when mapping messages
|
||||
messages: messages.map(m => {
|
||||
const msg: any = {
|
||||
role: m.role,
|
||||
content: m.content,
|
||||
}
|
||||
if (m.role === 'tool') {
|
||||
msg.tool_call_id = m.tool_call_id
|
||||
msg.name = m.name
|
||||
}
|
||||
if (m.role === 'assistant' && m.tool_calls) {
|
||||
msg.tool_calls = m.tool_calls // NEW
|
||||
}
|
||||
return msg
|
||||
}),
|
||||
```
|
||||
|
||||
### Server (`src/server.ts`)
|
||||
```typescript
|
||||
// Removed duplicate tool result addition
|
||||
// Frontend already adds them to conversation history
|
||||
const apiResponse = await llmClient.chat(messages)
|
||||
```
|
||||
|
||||
## Test Results
|
||||
|
||||
### Before Fix
|
||||
```
|
||||
10 passing (2m)
|
||||
1 failing
|
||||
|
||||
1) should send a message and receive a response from LLM
|
||||
Error: No response from AI assistant
|
||||
```
|
||||
|
||||
### After Fix
|
||||
```
|
||||
11 passing (1m) ✅
|
||||
|
||||
✔ should expand AI Assistant panel when clicked
|
||||
✔ should send a message and receive a response from LLM (8953ms)
|
||||
✔ should clear chat history when clear button is clicked
|
||||
```
|
||||
|
||||
## Working Tool Call Flow
|
||||
|
||||
```
|
||||
User: "What is this device?"
|
||||
↓
|
||||
Frontend → Backend
|
||||
messages: [
|
||||
{ role: 'system', content: '...' },
|
||||
{ role: 'user', content: 'What is this device?' }
|
||||
]
|
||||
↓
|
||||
Backend → OpenAI (with 4 tool definitions)
|
||||
↓
|
||||
OpenAI Response
|
||||
response: ""
|
||||
tool_calls: [{
|
||||
id: "call_xxx",
|
||||
type: "function",
|
||||
function: {
|
||||
name: "get_topic",
|
||||
arguments: "{\"topic\":\"kitchen/lamp\"}"
|
||||
}
|
||||
}]
|
||||
↓
|
||||
Backend → Frontend
|
||||
{ response: "", toolCalls: [...], debugInfo: {...} }
|
||||
↓
|
||||
Frontend: Execute Tools
|
||||
- Transform tool call format
|
||||
- Execute get_topic("kitchen/lamp") on local topic tree
|
||||
- Get result: "Topic: kitchen/lamp\nValue: ON\nRetained: true\n..."
|
||||
- Add to conversation history:
|
||||
* Assistant message WITH tool_calls
|
||||
* Tool result message WITH tool_call_id
|
||||
↓
|
||||
Frontend → Backend
|
||||
messages: [
|
||||
{ role: 'system', content: '...' },
|
||||
{ role: 'user', content: 'What is this device?' },
|
||||
{ role: 'assistant', content: '', tool_calls: [...] }, // ← Has tool_calls
|
||||
{ role: 'tool', content: '...', tool_call_id: 'call_xxx', name: 'get_topic' }
|
||||
]
|
||||
↓
|
||||
Backend → OpenAI (with conversation including tool result)
|
||||
↓
|
||||
OpenAI Final Response
|
||||
response: "This topic is a device named 'kitchen/lamp'..."
|
||||
tool_calls: undefined
|
||||
↓
|
||||
Backend → Frontend
|
||||
{ response: "...", debugInfo: {...} }
|
||||
↓
|
||||
UI: Display Response ✅
|
||||
```
|
||||
|
||||
## Lessons Learned
|
||||
|
||||
1. **LLM Tool Calling is Multi-Turn**: Tool calls require multiple API requests:
|
||||
- First request: Get tool calls
|
||||
- Execute tools locally
|
||||
- Second request: Send tool results and get final answer
|
||||
|
||||
2. **OpenAI Format Requirements**:
|
||||
- Tool messages MUST follow an assistant message with `tool_calls`
|
||||
- Tool messages MUST have `tool_call_id` matching the call
|
||||
- Tool messages MUST have a `name` field
|
||||
|
||||
3. **Conversation State Management**:
|
||||
- Frontend manages the conversation history
|
||||
- Backend should not duplicate what's already in the history
|
||||
- Each participant adds their own messages to the history
|
||||
|
||||
4. **Validation Logic**:
|
||||
- Empty responses are valid when tool calls are present
|
||||
- Don't reject responses that are part of a multi-turn flow
|
||||
|
||||
## Files Changed
|
||||
|
||||
| File | Lines Changed | Description |
|
||||
|------|--------------|-------------|
|
||||
| `app/src/services/llmService.ts` | ~50 | Fixed tool call handling |
|
||||
| `backend/src/llmApiClient.ts` | ~20 | Preserve tool_calls property |
|
||||
| `src/server.ts` | ~5 | Remove duplicate tool addition |
|
||||
|
||||
**Total**: ~75 lines changed to fix 4 bugs
|
||||
|
||||
## Verification
|
||||
|
||||
The fix was verified by:
|
||||
1. Running all 11 browser UI tests - **100% passing**
|
||||
2. Manual testing with Playwright headed mode
|
||||
3. Verifying tool call execution with debug logging
|
||||
4. Confirming LLM responses include tool insights
|
||||
|
||||
The AI Assistant chat is now fully operational with complete MCP-style tool calling support!
|
||||
Reference in New Issue
Block a user