mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-08-14 00:47:52 +00:00
fix(web): normalize tool call arguments (#738)
* fix(web): normalize tool call arguments * fix(web): preserve chats when repair writeback fails
This commit is contained in:
@@ -5,6 +5,7 @@ import { useAppStore, generateId } from '../../lib/store';
|
|||||||
import { streamChat, streamResearch } from '../../lib/sse';
|
import { streamChat, streamResearch } from '../../lib/sse';
|
||||||
import { fetchSavings, getBase } from '../../lib/api';
|
import { fetchSavings, getBase } from '../../lib/api';
|
||||||
import { listConnectors, getSyncStatus } from '../../lib/connectors-api';
|
import { listConnectors, getSyncStatus } from '../../lib/connectors-api';
|
||||||
|
import { serializeToolCallArguments } from '../../lib/tool-call';
|
||||||
import { MicButton } from './MicButton';
|
import { MicButton } from './MicButton';
|
||||||
import { useSpeech } from '../../hooks/useSpeech';
|
import { useSpeech } from '../../hooks/useSpeech';
|
||||||
import type {
|
import type {
|
||||||
@@ -389,7 +390,7 @@ export function InputArea() {
|
|||||||
const tc: ToolCallInfo = {
|
const tc: ToolCallInfo = {
|
||||||
id: generateId(),
|
id: generateId(),
|
||||||
tool: data.tool,
|
tool: data.tool,
|
||||||
arguments: data.arguments || '',
|
arguments: serializeToolCallArguments(data.arguments),
|
||||||
status: 'running',
|
status: 'running',
|
||||||
};
|
};
|
||||||
toolCalls.push(tc);
|
toolCalls.push(tc);
|
||||||
@@ -400,7 +401,7 @@ export function InputArea() {
|
|||||||
updateLastAssistant(convId, accumulatedContent, [...toolCalls]);
|
updateLastAssistant(convId, accumulatedContent, [...toolCalls]);
|
||||||
useAppStore.getState().addLogEntry({
|
useAppStore.getState().addLogEntry({
|
||||||
timestamp: Date.now(), level: 'info', category: 'tool',
|
timestamp: Date.now(), level: 'info', category: 'tool',
|
||||||
message: `Calling ${data.tool}(${data.arguments || ''})`,
|
message: `Calling ${data.tool}(${serializeToolCallArguments(data.arguments)})`,
|
||||||
});
|
});
|
||||||
} catch {}
|
} catch {}
|
||||||
} else if (eventName === 'tool_call_end') {
|
} else if (eventName === 'tool_call_end') {
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import { useState } from 'react';
|
import { useState } from 'react';
|
||||||
import { ChevronDown, ChevronRight, Loader2, CheckCircle2, XCircle } from 'lucide-react';
|
import { ChevronDown, ChevronRight, Loader2, CheckCircle2, XCircle } from 'lucide-react';
|
||||||
import type { ToolCallInfo } from '../../types';
|
import type { ToolCallInfo } from '../../types';
|
||||||
|
import { serializeToolCallArguments } from '../../lib/tool-call';
|
||||||
|
|
||||||
interface Props {
|
interface Props {
|
||||||
toolCall: ToolCallInfo;
|
toolCall: ToolCallInfo;
|
||||||
@@ -35,7 +36,10 @@ export function ToolCallCard({ toolCall }: Props) {
|
|||||||
const [expanded, setExpanded] = useState(false);
|
const [expanded, setExpanded] = useState(false);
|
||||||
const config = statusConfig[toolCall.status];
|
const config = statusConfig[toolCall.status];
|
||||||
const StatusIcon = config.icon;
|
const StatusIcon = config.icon;
|
||||||
const preview = previewArgs(toolCall.arguments);
|
// Persisted conversations may contain the pre-fix object payload despite
|
||||||
|
// the TypeScript contract, so normalize again at the final render boundary.
|
||||||
|
const argumentsText = serializeToolCallArguments(toolCall.arguments);
|
||||||
|
const preview = previewArgs(argumentsText);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div
|
<div
|
||||||
@@ -95,7 +99,7 @@ export function ToolCallCard({ toolCall }: Props) {
|
|||||||
className="px-2.5 pb-2 pt-0.5"
|
className="px-2.5 pb-2 pt-0.5"
|
||||||
style={{ borderTop: '1px solid var(--color-border-subtle, var(--color-border))' }}
|
style={{ borderTop: '1px solid var(--color-border-subtle, var(--color-border))' }}
|
||||||
>
|
>
|
||||||
{toolCall.arguments && (
|
{argumentsText && (
|
||||||
<div className="mt-1.5">
|
<div className="mt-1.5">
|
||||||
<div
|
<div
|
||||||
style={{
|
style={{
|
||||||
@@ -120,7 +124,7 @@ export function ToolCallCard({ toolCall }: Props) {
|
|||||||
wordBreak: 'break-all',
|
wordBreak: 'break-all',
|
||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
{formatJson(toolCall.arguments)}
|
{formatJson(argumentsText)}
|
||||||
</pre>
|
</pre>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import type { ModelInfo, SavingsData, ServerInfo } from '../types';
|
import type { ModelInfo, SavingsData, ServerInfo } from '../types';
|
||||||
import { SUPABASE_ANON_KEY, SUPABASE_URL } from './supabase';
|
import { SUPABASE_ANON_KEY, SUPABASE_URL } from './supabase';
|
||||||
|
import { serializeToolCallArguments } from './tool-call';
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Supabase config
|
// Supabase config
|
||||||
@@ -741,7 +742,7 @@ export async function sendAgentMessage(
|
|||||||
const parsed = JSON.parse(data);
|
const parsed = JSON.parse(data);
|
||||||
callbacks?.onToolCallStart?.({
|
callbacks?.onToolCallStart?.({
|
||||||
tool: parsed.tool,
|
tool: parsed.tool,
|
||||||
arguments: parsed.arguments ?? '',
|
arguments: serializeToolCallArguments(parsed.arguments),
|
||||||
});
|
});
|
||||||
} catch {
|
} catch {
|
||||||
/* skip */
|
/* skip */
|
||||||
|
|||||||
@@ -0,0 +1,122 @@
|
|||||||
|
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
|
|
||||||
|
const CONVERSATIONS_KEY = 'openjarvis-conversations';
|
||||||
|
|
||||||
|
class MemoryStorage {
|
||||||
|
private store = new Map<string, string>();
|
||||||
|
|
||||||
|
getItem(key: string): string | null {
|
||||||
|
return this.store.get(key) ?? null;
|
||||||
|
}
|
||||||
|
|
||||||
|
setItem(key: string, value: string): void {
|
||||||
|
this.store.set(key, String(value));
|
||||||
|
}
|
||||||
|
|
||||||
|
removeItem(key: string): void {
|
||||||
|
this.store.delete(key);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.resetModules();
|
||||||
|
(globalThis as unknown as { localStorage: MemoryStorage }).localStorage =
|
||||||
|
new MemoryStorage();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
(globalThis as unknown as { localStorage?: MemoryStorage }).localStorage =
|
||||||
|
undefined;
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('persisted tool calls', () => {
|
||||||
|
it('repairs parsed argument objects while loading conversations', async () => {
|
||||||
|
localStorage.setItem(
|
||||||
|
CONVERSATIONS_KEY,
|
||||||
|
JSON.stringify({
|
||||||
|
version: 1,
|
||||||
|
activeId: 'conversation-1',
|
||||||
|
conversations: {
|
||||||
|
'conversation-1': {
|
||||||
|
id: 'conversation-1',
|
||||||
|
title: 'Broken chat',
|
||||||
|
createdAt: 1,
|
||||||
|
updatedAt: 1,
|
||||||
|
model: 'test-model',
|
||||||
|
messages: [
|
||||||
|
{
|
||||||
|
id: 'assistant-1',
|
||||||
|
role: 'assistant',
|
||||||
|
content: '',
|
||||||
|
timestamp: 1,
|
||||||
|
toolCalls: [
|
||||||
|
{
|
||||||
|
id: 'call-1',
|
||||||
|
tool: 'web_search',
|
||||||
|
arguments: { query: 'python' },
|
||||||
|
status: 'success',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
const { useAppStore } = await import('./store');
|
||||||
|
|
||||||
|
expect(useAppStore.getState().messages[0].toolCalls?.[0].arguments).toBe(
|
||||||
|
'{"query":"python"}',
|
||||||
|
);
|
||||||
|
const repaired = JSON.parse(localStorage.getItem(CONVERSATIONS_KEY) ?? '{}');
|
||||||
|
expect(
|
||||||
|
repaired.conversations['conversation-1'].messages[0].toolCalls[0].arguments,
|
||||||
|
).toBe('{"query":"python"}');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps repaired conversations in memory when writeback fails', async () => {
|
||||||
|
localStorage.setItem(
|
||||||
|
CONVERSATIONS_KEY,
|
||||||
|
JSON.stringify({
|
||||||
|
version: 1,
|
||||||
|
activeId: 'conversation-1',
|
||||||
|
conversations: {
|
||||||
|
'conversation-1': {
|
||||||
|
id: 'conversation-1',
|
||||||
|
title: 'Readable chat',
|
||||||
|
createdAt: 1,
|
||||||
|
updatedAt: 1,
|
||||||
|
model: 'test-model',
|
||||||
|
messages: [
|
||||||
|
{
|
||||||
|
id: 'assistant-1',
|
||||||
|
role: 'assistant',
|
||||||
|
content: '',
|
||||||
|
timestamp: 1,
|
||||||
|
toolCalls: [
|
||||||
|
{
|
||||||
|
id: 'call-1',
|
||||||
|
tool: 'web_search',
|
||||||
|
arguments: { query: 'python' },
|
||||||
|
status: 'success',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
vi.spyOn(localStorage, 'setItem').mockImplementation(() => {
|
||||||
|
throw new DOMException('Storage quota exceeded', 'QuotaExceededError');
|
||||||
|
});
|
||||||
|
|
||||||
|
const { useAppStore } = await import('./store');
|
||||||
|
|
||||||
|
expect(useAppStore.getState().messages).toHaveLength(1);
|
||||||
|
expect(useAppStore.getState().messages[0].toolCalls?.[0].arguments).toBe(
|
||||||
|
'{"query":"python"}',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -16,6 +16,7 @@ import type {
|
|||||||
} from '../types';
|
} from '../types';
|
||||||
import type { ManagedAgent } from './api';
|
import type { ManagedAgent } from './api';
|
||||||
import { isEmbedOnlyModel } from './model-capabilities';
|
import { isEmbedOnlyModel } from './model-capabilities';
|
||||||
|
import { serializeToolCallArguments } from './tool-call';
|
||||||
|
|
||||||
export interface CachedConnector {
|
export interface CachedConnector {
|
||||||
connector_id: string;
|
connector_id: string;
|
||||||
@@ -55,7 +56,30 @@ function loadConversations(): ConversationStore {
|
|||||||
const raw = localStorage.getItem(CONVERSATIONS_KEY);
|
const raw = localStorage.getItem(CONVERSATIONS_KEY);
|
||||||
if (!raw) return { version: 1, conversations: {}, activeId: null };
|
if (!raw) return { version: 1, conversations: {}, activeId: null };
|
||||||
const parsed = JSON.parse(raw);
|
const parsed = JSON.parse(raw);
|
||||||
if (parsed.version === 1) return parsed;
|
if (parsed.version === 1) {
|
||||||
|
let repaired = false;
|
||||||
|
for (const conversation of Object.values(parsed.conversations ?? {}) as Conversation[]) {
|
||||||
|
for (const message of conversation.messages ?? []) {
|
||||||
|
for (const toolCall of message.toolCalls ?? []) {
|
||||||
|
const argumentsText = serializeToolCallArguments(toolCall.arguments);
|
||||||
|
if (argumentsText !== toolCall.arguments) {
|
||||||
|
toolCall.arguments = argumentsText;
|
||||||
|
repaired = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (repaired) {
|
||||||
|
try {
|
||||||
|
localStorage.setItem(CONVERSATIONS_KEY, JSON.stringify(parsed));
|
||||||
|
} catch {
|
||||||
|
// Keep the repaired conversations usable in memory when storage is
|
||||||
|
// read-only or full. A failed best-effort writeback must not make
|
||||||
|
// otherwise readable conversation history disappear from the UI.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return parsed;
|
||||||
|
}
|
||||||
return { version: 1, conversations: {}, activeId: null };
|
return { version: 1, conversations: {}, activeId: null };
|
||||||
} catch {
|
} catch {
|
||||||
return { version: 1, conversations: {}, activeId: null };
|
return { version: 1, conversations: {}, activeId: null };
|
||||||
|
|||||||
@@ -0,0 +1,22 @@
|
|||||||
|
import { describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
|
import { serializeToolCallArguments } from './tool-call';
|
||||||
|
|
||||||
|
describe('serializeToolCallArguments', () => {
|
||||||
|
it('preserves JSON strings', () => {
|
||||||
|
expect(serializeToolCallArguments('{"query":"python"}')).toBe(
|
||||||
|
'{"query":"python"}',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('serializes parsed argument objects', () => {
|
||||||
|
expect(serializeToolCallArguments({ query: 'python' })).toBe(
|
||||||
|
'{"query":"python"}',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('uses an empty string for missing arguments', () => {
|
||||||
|
expect(serializeToolCallArguments(null)).toBe('');
|
||||||
|
expect(serializeToolCallArguments(undefined)).toBe('');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,11 @@
|
|||||||
|
/** Convert tool-call arguments from API or persisted data into display-safe text. */
|
||||||
|
export function serializeToolCallArguments(value: unknown): string {
|
||||||
|
if (typeof value === 'string') return value;
|
||||||
|
if (value == null) return '';
|
||||||
|
|
||||||
|
try {
|
||||||
|
return JSON.stringify(value) ?? String(value);
|
||||||
|
} catch {
|
||||||
|
return String(value);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -110,6 +110,12 @@ class AgentStreamBridge:
|
|||||||
|
|
||||||
def _format_named_event(self, name: str, data: dict) -> str:
|
def _format_named_event(self, name: str, data: dict) -> str:
|
||||||
"""Format an SSE event with an explicit ``event:`` field."""
|
"""Format an SSE event with an explicit ``event:`` field."""
|
||||||
|
if name == "tool_call_start" and not isinstance(data.get("arguments"), str):
|
||||||
|
# The in-process event bus uses parsed arguments for trace/eval
|
||||||
|
# consumers, while the web SSE contract expects their JSON text.
|
||||||
|
# Copy before normalizing so other subscribers keep the object.
|
||||||
|
data = dict(data)
|
||||||
|
data["arguments"] = json.dumps(data.get("arguments"))
|
||||||
return f"event: {name}\ndata: {json.dumps(data)}\n\n"
|
return f"event: {name}\ndata: {json.dumps(data)}\n\n"
|
||||||
|
|
||||||
def _run_agent(self) -> object:
|
def _run_agent(self) -> object:
|
||||||
|
|||||||
@@ -0,0 +1,30 @@
|
|||||||
|
import json
|
||||||
|
|
||||||
|
from openjarvis.server.stream_bridge import AgentStreamBridge
|
||||||
|
|
||||||
|
|
||||||
|
def test_tool_call_start_serializes_arguments_for_sse_without_mutating_event():
|
||||||
|
bridge = object.__new__(AgentStreamBridge)
|
||||||
|
event_data = {
|
||||||
|
"tool": "web_search",
|
||||||
|
"arguments": {"query": "python"},
|
||||||
|
"agent": "agent-1",
|
||||||
|
}
|
||||||
|
|
||||||
|
event = bridge._format_named_event("tool_call_start", event_data)
|
||||||
|
payload = json.loads(event.split("data: ", 1)[1])
|
||||||
|
|
||||||
|
assert payload["arguments"] == '{"query": "python"}'
|
||||||
|
assert event_data["arguments"] == {"query": "python"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_tool_call_start_preserves_already_serialized_arguments():
|
||||||
|
bridge = object.__new__(AgentStreamBridge)
|
||||||
|
|
||||||
|
event = bridge._format_named_event(
|
||||||
|
"tool_call_start",
|
||||||
|
{"tool": "web_search", "arguments": '{"query":"python"}'},
|
||||||
|
)
|
||||||
|
payload = json.loads(event.split("data: ", 1)[1])
|
||||||
|
|
||||||
|
assert payload["arguments"] == '{"query":"python"}'
|
||||||
Reference in New Issue
Block a user