Skip to content

Commit 20784b0

Browse files
elkaixelkaix
committed
fix: address review findings on DSML tool parsing and stage validation
1 parent a1b26c2 commit 20784b0

10 files changed

Lines changed: 375 additions & 59 deletions

File tree

‎packages/agent-core-v2/src/kosong/provider/bases/openai/dsml-tool-parser.ts‎

Lines changed: 40 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,20 @@ const CANDIDATE_TARGETS = [
1212
'invoke',
1313
];
1414

15-
const CONTAINER_OPEN_RE = /^<[｜|]?\s*(?:DSML[｜|]?)?\s*tool_calls\s*>/i;
16-
const CONTAINER_CLOSE_RE = /^<\/[｜|]?\s*(?:DSML[｜|]?)?\s*tool_calls\s*>/i;
17-
const INVOKE_OPEN_RE = /^<[｜|]?\s*(?:DSML[｜|]?)?\s*invoke(?:\s+[^>]*)?>/i;
18-
const INVOKE_CLOSE_RE = /<\/[｜|]?\s*(?:DSML[｜|]?)?\s*invoke\s*>/i;
15+
const CONTAINER_OPEN_RE = /^<\s*[｜|]?\s*(?:DSML\s*[｜|]?)?\s*tool_calls\s*>/i;
16+
const CONTAINER_CLOSE_RE = /^<\/\s*[｜|]?\s*(?:DSML\s*[｜|]?)?\s*tool_calls\s*>/i;
17+
const INVOKE_OPEN_RE = /^<\s*[｜|]?\s*(?:DSML\s*[｜|]?)?\s*invoke(?:\s+[^>]*)?>/i;
18+
const INVOKE_CLOSE_RE = /<\/\s*[｜|]?\s*(?:DSML\s*[｜|]?)?\s*invoke\s*>/i;
1919
const HERMES_OPEN_RE = /^<tool_call>/i;
2020
const HERMES_CLOSE_RE = /<\/tool_call>/i;
2121

2222
function unescapeXml(value: string): string {
2323
return value
24-
.replaceAll(/&quot;/g, '"')
25-
.replaceAll(/&apos;/g, "'")
26-
.replaceAll(/&lt;/g, '<')
27-
.replaceAll(/&gt;/g, '>')
28-
.replaceAll(/&amp;/g, '&');
24+
.replaceAll('&quot;', '"')
25+
.replaceAll('&apos;', "'")
26+
.replaceAll('&lt;', '<')
27+
.replaceAll('&gt;', '>')
28+
.replaceAll('&amp;', '&');
2929
}
3030

3131
function parseParameterValue(rawVal: string, isStringAttr: boolean | undefined): unknown {
@@ -60,7 +60,7 @@ function parseParameterValue(rawVal: string, isStringAttr: boolean | undefined):
6060

6161
function parseInvokeBody(invokeContent: string): Record<string, unknown> {
6262
const paramRegex =
63-
/<[｜|]?\s*(?:DSML[｜|]?)?\s*parameter\s+([^>]*?)>([\s\S]*?)<\/[｜|]?\s*(?:DSML[｜|]?)?\s*parameter\s*>/gi;
63+
/<\s*[｜|]?\s*(?:DSML\s*[｜|]?)?\s*parameter\s+([^>]*?)>([\s\S]*?)<\/\s*[｜|]?\s*(?:DSML\s*[｜|]?)?\s*parameter\s*>/gi;
6464
const args: Record<string, unknown> = {};
6565
let paramFound = false;
6666
let match: RegExpExecArray | null = null;
@@ -101,7 +101,7 @@ function parseInvokeBody(invokeContent: string): Record<string, unknown> {
101101
}
102102

103103
function parseInvokeTag(invokeBlock: string): ToolCall | null {
104-
const openMatch = /^<[｜|]?\s*(?:DSML[｜|]?)?\s*invoke\s+([^>]*?)>/i.exec(invokeBlock);
104+
const openMatch = /^<\s*[｜|]?\s*(?:DSML\s*[｜|]?)?\s*invoke\s+([^>]*?)>/i.exec(invokeBlock);
105105
if (!openMatch) return null;
106106

107107
const attrStr = openMatch[1] ?? '';
@@ -117,7 +117,7 @@ function parseInvokeTag(invokeBlock: string): ToolCall | null {
117117
const args = parseInvokeBody(innerContent);
118118
return {
119119
type: 'function',
120-
id: `call_${crypto.randomUUID().replaceAll(/-/g, '').slice(0, 24)}`,
120+
id: `call_${crypto.randomUUID().replaceAll('-', '').slice(0, 24)}`,
121121
name: toolName,
122122
arguments: JSON.stringify(args),
123123
};
@@ -137,7 +137,7 @@ function parseHermesToolCall(toolCallBlock: string): ToolCall | null {
137137
: JSON.stringify(parsed.arguments ?? {});
138138
return {
139139
type: 'function',
140-
id: `call_${crypto.randomUUID().replaceAll(/-/g, '').slice(0, 24)}`,
140+
id: `call_${crypto.randomUUID().replaceAll('-', '').slice(0, 24)}`,
141141
name: parsed.name,
142142
arguments: args,
143143
};
@@ -216,12 +216,15 @@ export class DsmlStreamParser {
216216
if (toolCall) {
217217
this._hasExtractedToolCalls = true;
218218
parts.push(toolCall);
219-
}
220-
this._buffer = this._buffer.slice(invokeEnd);
221-
if (/^\s*</.test(this._buffer)) {
222-
this._buffer = this._buffer.trimStart();
219+
this._buffer = this._buffer.slice(invokeEnd);
220+
if (/^\s*</.test(this._buffer)) {
221+
this._buffer = this._buffer.trimStart();
222+
} else {
223+
this._buffer = this._buffer.replace(/^\r?\n/, '');
224+
}
223225
} else {
224-
this._buffer = this._buffer.replace(/^\r?\n/, '');
226+
parts.push({ type: 'text', text: invokeBlock });
227+
this._buffer = this._buffer.slice(invokeEnd);
225228
}
226229
continue;
227230
}
@@ -238,12 +241,15 @@ export class DsmlStreamParser {
238241
if (toolCall) {
239242
this._hasExtractedToolCalls = true;
240243
parts.push(toolCall);
241-
}
242-
this._buffer = this._buffer.slice(toolCallEnd);
243-
if (/^\s*</.test(this._buffer)) {
244-
this._buffer = this._buffer.trimStart();
244+
this._buffer = this._buffer.slice(toolCallEnd);
245+
if (/^\s*</.test(this._buffer)) {
246+
this._buffer = this._buffer.trimStart();
247+
} else {
248+
this._buffer = this._buffer.replace(/^\r?\n/, '');
249+
}
245250
} else {
246-
this._buffer = this._buffer.replace(/^\r?\n/, '');
251+
parts.push({ type: 'text', text: block });
252+
this._buffer = this._buffer.slice(toolCallEnd);
247253
}
248254
continue;
249255
}
@@ -279,6 +285,16 @@ export class DsmlStreamParser {
279285
return parts;
280286
}
281287
}
288+
const hermesOpen = HERMES_OPEN_RE.exec(this._buffer);
289+
if (hermesOpen) {
290+
const toolCall = parseHermesToolCall(this._buffer);
291+
if (toolCall) {
292+
this._hasExtractedToolCalls = true;
293+
parts.push(toolCall);
294+
this._buffer = '';
295+
return parts;
296+
}
297+
}
282298
parts.push({ type: 'text', text: this._buffer });
283299
this._buffer = '';
284300
}
@@ -303,6 +319,6 @@ export function extractDsmlToolCalls(text: string): {
303319
}
304320
}
305321

306-
const cleanText = textParts.join('').trim();
322+
const cleanText = toolCalls.length > 0 ? textParts.join('').trim() : text;
307323
return { cleanText, toolCalls };
308324
}

‎packages/agent-core-v2/src/kosong/provider/bases/openai/openai-legacy.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -431,9 +431,9 @@ export class OpenAILegacyStreamedMessage implements StreamedMessage {
431431
let extractedToolCalls: ToolCall[] = [];
432432
if (text) {
433433
const parsed = extractDsmlToolCalls(text);
434-
text = parsed.cleanText;
435-
extractedToolCalls = parsed.toolCalls;
436-
if (extractedToolCalls.length > 0) {
434+
if (parsed.toolCalls.length > 0) {
435+
text = parsed.cleanText;
436+
extractedToolCalls = parsed.toolCalls;
437437
this._hasExtractedToolCalls = true;
438438
}
439439
}

‎packages/agent-core-v2/src/session/expertTalk/expertTalkService.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1152,7 +1152,7 @@ export class SessionExpertTalkService extends Disposable implements ISessionExpe
11521152
}
11531153
const text = completion.summary.trim();
11541154
if (text.length === 0) throw new Error('Discussion stage returned an empty answer');
1155-
if (/<[｜|]?\s*(?:DSML[｜|]?)?(?:tool_calls|invoke)\b/i.test(text)) {
1155+
if (hasUnparsedToolCallMarkup(text)) {
11561156
throw new Error('Discussion stage output contains unparsed tool call markup');
11571157
}
11581158
return text;
@@ -1220,6 +1220,7 @@ export class SessionExpertTalkService extends Disposable implements ISessionExpe
12201220
if (
12211221
errorReason === 'STAGE_REQUEST_BUDGET_EXCEEDED'
12221222
&& partialText.length > 0
1223+
&& !hasUnparsedToolCallMarkup(partialText)
12231224
&& limits.acceptBudgetExhaustedOutput?.(partialText) === true
12241225
&& visibleOutputTokens <= limits.maxOutputTokens
12251226
) {
@@ -2070,6 +2071,10 @@ function errorMessage(error: unknown): string {
20702071
return error instanceof Error ? error.message : String(error);
20712072
}
20722073

2074+
function hasUnparsedToolCallMarkup(text: string): boolean {
2075+
return /<\s*[｜|]?\s*(?:DSML\s*[｜|]?)?\s*(?:tool_calls?|invoke)\b/i.test(text);
2076+
}
2077+
20732078
registerScopedService(
20742079
LifecycleScope.Session,
20752080
ISessionExpertTalkService,

‎packages/agent-core-v2/test/kosong/provider/dsml-tool-parser.test.ts‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, expect, it, vi } from 'vitest';
1+
import { describe, expect, it } from 'vitest';
22

33
import {
44
DsmlStreamParser,
@@ -294,5 +294,65 @@ describe('agent-core-v2: DsmlStreamParser and extractDsmlToolCalls', () => {
294294
arguments: '{"pattern":"*.json"}',
295295
});
296296
});
297+
298+
it('preserves surrounding whitespace and markdown hard breaks in non-stream response', async () => {
299+
const provider = new OpenAILegacyChatProvider({
300+
model: 'deepseek-chat',
301+
apiKey: 'test-key',
302+
stream: false,
303+
});
304+
305+
const textWithWhitespace = ' Line 1 \nLine 2 ';
306+
const responseData = {
307+
id: 'chatcmpl-v2-whitespace',
308+
choices: [
309+
{
310+
index: 0,
311+
message: {
312+
role: 'assistant',
313+
content: textWithWhitespace,
314+
},
315+
finish_reason: 'stop',
316+
},
317+
],
318+
};
319+
320+
(provider as unknown as { _client: unknown })._client = {
321+
chat: {
322+
completions: {
323+
create: () => ({
324+
withResponse: async () => ({
325+
data: responseData,
326+
response: { headers: new Headers() },
327+
}),
328+
}),
329+
},
330+
},
331+
};
332+
333+
const stream = await provider.generate('', [], []);
334+
const parts: Array<Record<string, unknown>> = [];
335+
for await (const p of stream) parts.push(p as unknown as Record<string, unknown>);
336+
337+
expect(parts).toHaveLength(1);
338+
expect(parts[0]).toEqual({
339+
type: 'text',
340+
text: textWithWhitespace,
341+
});
342+
});
343+
344+
it('preserves malformed invoke block as text without discarding content', () => {
345+
const input = '<｜DSML｜invoke>malformed content without name</｜DSML｜invoke>';
346+
const result = extractDsmlToolCalls(input);
347+
expect(result.cleanText).toBe(input);
348+
expect(result.toolCalls).toHaveLength(0);
349+
});
350+
351+
it('preserves malformed Hermes block as text without discarding content', () => {
352+
const input = '<tool_call>not valid json</tool_call>';
353+
const result = extractDsmlToolCalls(input);
354+
expect(result.cleanText).toBe(input);
355+
expect(result.toolCalls).toHaveLength(0);
356+
});
297357
});
298358
});

0 commit comments

Comments
 (0)