Skip to content

Commit 2ce7b0c

Browse files
yiliang114qwencoder
yiliang114
and
qwencoder
committed
fix(core): harden xml tool-call recovery against borrowed closers and lexer cost
Three review findings on the recovery path: 1. A block whose parameter was never closed borrowed the close tags of a later block. PARAMETER_PATTERN consumed them, the residual-tag guard saw an empty string, and the truncated call was dispatched with the next call's raw markup as a parameter value while the real call never ran and remainingText came back empty. Count the parameter open tags a block body contains against the ones the accepted matches closed, and reject on mismatch. A rejected block may have swallowed an intact later block, so rescan from just after its open tag; that block is then recovered on its own merits (still skipped when it sits in a fence or an example). 2. The markdown lexer ran over the whole response for every recovery candidate even when the text contained no example tag at all, and it is super-linear on unterminated link/emphasis runs: 4 KB of repeated link openers cost ~3.0 s of synchronous work before the intent-ratio guard. Skip the lexer when no example tag can produce a range, and catch a lexer throw so it degrades to "no example ranges" instead of aborting the turn. 3. Parameter spans were recorded only for accepted blocks, so a rejected block's parameter data was never masked out of the prose the example scan reads; a literal example opener in it swallowed a later valid call. Derive the spans from the raw parameter matches over the whole text. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmuv5xnpc3q
1 parent 2a064ee commit 2ce7b0c

2 files changed

Lines changed: 221 additions & 20 deletions

File tree

‎packages/core/src/core/xml-tool-call-fallback.test.ts‎

Lines changed: 177 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7-
import { describe, expect, it } from 'vitest';
7+
import { describe, expect, it, vi } from 'vitest';
8+
import { Lexer } from 'marked';
89

910
const OPEN = '<' + 'invoke';
1011
const CLOSE = '</' + 'invoke>';
@@ -706,9 +707,6 @@ describe('complete taught-dialect recovery (#10692)', () => {
706707
'<tool_call><function=write_file>' +
707708
'<parameter=file_path>a.ts</parameter>' +
708709
'<parameter=content>before</function>after</parameter></function></tool_call>',
709-
'<tool_call><function=read_file><parameter=file_path>a.ts</parameter></tool_call>' +
710-
'<tool_call><function=run_shell_command>' +
711-
'<parameter=command>pwd</parameter></function></tool_call>',
712710
])(
713711
'preserves malformed blocks instead of dispatching partial calls: %s',
714712
(text) => {
@@ -720,6 +718,44 @@ describe('complete taught-dialect recovery (#10692)', () => {
720718
},
721719
);
722720

721+
it('recovers the intact call after an envelope whose block never closed', () => {
722+
// The first envelope never closes its function block, so nothing may be
723+
// dispatched from it — but the rescan still finds the intact second call,
724+
// and the malformed envelope stays visible in remainingText.
725+
const tcOpen = '<' + 'tool_call>';
726+
const tcClose = '</' + 'tool_call>';
727+
const fnOpen = '<' + 'function=';
728+
const fnClose = '</' + 'function>';
729+
const truncated = [
730+
tcOpen,
731+
fnOpen,
732+
'read_file>',
733+
PARAM_OPEN,
734+
'=file_path>a.ts',
735+
PARAM_CLOSE,
736+
tcClose,
737+
].join('');
738+
const intact = [
739+
tcOpen,
740+
fnOpen,
741+
'run_shell_command>',
742+
PARAM_OPEN,
743+
'=command>pwd',
744+
PARAM_CLOSE,
745+
fnClose,
746+
tcClose,
747+
].join('');
748+
const text = truncated + intact;
749+
const result = tryRecoverXmlToolCalls(text);
750+
expect(
751+
result.functionCallParts.map((part) => part.functionCall?.name),
752+
).toEqual(['run_shell_command']);
753+
expect(result.functionCallParts[0]?.functionCall?.args).toEqual({
754+
command: 'pwd',
755+
});
756+
expect(result.remainingText).toBe(truncated);
757+
});
758+
723759
it('preserves a parameterless block whose name contains parameter syntax', () => {
724760
const parameterless = "<invoke name='<parameter=x>y</parameter>'></invoke>";
725761
const result = tryRecoverXmlToolCalls(`${functionBlock}\n${parameterless}`);
@@ -744,3 +780,140 @@ describe('complete taught-dialect recovery (#10692)', () => {
744780
},
745781
);
746782
});
783+
784+
describe('borrowed closers, lexer cost and rejected-block masking', () => {
785+
const FN_CLOSE = '</' + 'function>';
786+
const TC_OPEN = '<' + 'tool_call>';
787+
const TC_CLOSE = '</' + 'tool_call>';
788+
const EXAMPLE_CLOSE = '</' + 'example>';
789+
const readBlock = [
790+
'<function=read_file>',
791+
PARAM_OPEN,
792+
'=file_path>b.ts',
793+
PARAM_CLOSE,
794+
FN_CLOSE,
795+
].join('');
796+
797+
it('does not dispatch a truncated block that borrows the next call closers', () => {
798+
const text = [
799+
TC_OPEN,
800+
'\n<function=write_file>\n',
801+
PARAM_OPEN,
802+
'=file_path>a.txt',
803+
PARAM_CLOSE,
804+
'\n',
805+
PARAM_OPEN,
806+
'=content>hello\n',
807+
TC_OPEN,
808+
'\n<function=run_shell_command>',
809+
PARAM_OPEN,
810+
'=command>pwd',
811+
PARAM_CLOSE,
812+
FN_CLOSE,
813+
'\n',
814+
TC_CLOSE,
815+
].join('');
816+
const result = tryRecoverXmlToolCalls(text);
817+
expect(result.recovered).toBe(true);
818+
expect(
819+
result.functionCallParts.map((part) => part.functionCall?.name),
820+
).toEqual(['run_shell_command']);
821+
expect(result.functionCallParts[0]?.functionCall?.args).toEqual({
822+
command: 'pwd',
823+
});
824+
// The truncated block stays visible instead of being dispatched with the
825+
// next call's markup as its content.
826+
expect(result.remainingText).toContain('hello');
827+
expect(result.remainingText).not.toContain(FN_CLOSE);
828+
});
829+
830+
it('leaves a truncated block inert when no donor close follows', () => {
831+
const fnOpen = '<' + 'function=';
832+
const text = [
833+
fnOpen,
834+
'write_file>',
835+
PARAM_OPEN,
836+
'=file_path>a.txt',
837+
PARAM_CLOSE,
838+
PARAM_OPEN,
839+
'=content>hello',
840+
].join('');
841+
expect(tryRecoverXmlToolCalls(text)).toEqual({
842+
recovered: false,
843+
functionCallParts: [],
844+
remainingText: text,
845+
});
846+
});
847+
848+
it('does not run the markdown lexer when the text has no example tag', () => {
849+
const spy = vi.spyOn(Lexer, 'lexInline');
850+
try {
851+
// Unterminated link openers are the super-linear case for marked's
852+
// inline lexer; with no example tag in the text none of it may run.
853+
const text = '[a]('.repeat(50) + '\n' + readBlock;
854+
const result = tryRecoverXmlToolCalls(text);
855+
expect(
856+
result.functionCallParts.map((part) => part.functionCall?.name),
857+
).toEqual(['read_file']);
858+
expect(spy).not.toHaveBeenCalled();
859+
} finally {
860+
spy.mockRestore();
861+
}
862+
});
863+
864+
it('still runs the markdown lexer when an example tag is present', () => {
865+
const spy = vi.spyOn(Lexer, 'lexInline');
866+
try {
867+
const documentation = '<example>model:\n' + readBlock + EXAMPLE_CLOSE;
868+
expect(tryRecoverXmlToolCalls(documentation).recovered).toBe(false);
869+
expect(spy).toHaveBeenCalled();
870+
} finally {
871+
spy.mockRestore();
872+
}
873+
});
874+
875+
it('degrades to no example filtering when the lexer throws', () => {
876+
const spy = vi.spyOn(Lexer, 'lexInline').mockImplementation((): never => {
877+
throw new Error('lexer boom');
878+
});
879+
try {
880+
const text = '<example>model:\n' + readBlock;
881+
let result!: ReturnType<typeof tryRecoverXmlToolCalls>;
882+
expect(() => {
883+
result = tryRecoverXmlToolCalls(text);
884+
}).not.toThrow();
885+
expect(
886+
result.functionCallParts.map((part) => part.functionCall?.name),
887+
).toEqual(['read_file']);
888+
} finally {
889+
spy.mockRestore();
890+
}
891+
});
892+
893+
it('does not let a rejected block parameter swallow a later valid call', () => {
894+
// The write_file block is rejected — its content parameter never closes
895+
// and borrows the function close tag — yet its parameter data must still
896+
// be masked out of the prose the example scan reads. Unclosed, that
897+
// literal example opener would swallow everything to the end of the text.
898+
const fnOpen = '<' + 'function=';
899+
const rejected = [
900+
fnOpen,
901+
'write_file>',
902+
PARAM_OPEN,
903+
'=file_path>a.txt',
904+
PARAM_CLOSE,
905+
PARAM_OPEN,
906+
'=content><example>note',
907+
FN_CLOSE,
908+
'tail',
909+
PARAM_CLOSE,
910+
FN_CLOSE,
911+
].join('');
912+
const text = rejected + '\n' + readBlock;
913+
const result = tryRecoverXmlToolCalls(text);
914+
expect(
915+
result.functionCallParts.map((part) => part.functionCall?.name),
916+
).toEqual(['read_file']);
917+
expect(result.remainingText).toContain('<example>note');
918+
});
919+
});

‎packages/core/src/core/xml-tool-call-fallback.ts‎

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ const TOOL_CALL_PATTERN =
1111
/<invoke\s+name=["']([^"']+)["']>([\s\S]*?)<\/invoke>|<function=([^\s<>]+)>([\s\S]*?)<\/function>/g;
1212
const PARAMETER_PATTERN =
1313
/<parameter(?:\s+name=["']([^"']+)["']|=([^\s<>]+))>([\s\S]*?)<\/parameter>/g;
14+
// Parameter open tags. Scanned over a block body to detect a parameter that
15+
// was never closed: PARAMETER_PATTERN then borrows the close tag of a later
16+
// block, which leaves `outsideParameters` empty and slips past the guard.
17+
const PARAM_OPEN_PATTERN = /<parameter(?:\s+name=["'][^"']*["']|=[^\s<>]+)?>/g;
1418

1519
export interface ExtractedToolCall {
1620
name: string;
@@ -127,6 +131,13 @@ function computeExampleRanges(
127131
text: string,
128132
parameterRanges: Array<[number, number]>,
129133
): Array<[number, number]> {
134+
// marked's inline lexer is super-linear on unterminated link/emphasis runs
135+
// and nothing bounds the model's output length, so it is only worth running
136+
// when the text can actually produce an example range. The tag scan below
137+
// re-checks every candidate against `tagPositions`, so skipping the lexer
138+
// here can only yield "no ranges" — which is what an example-free text has.
139+
if (!text.includes('<example') && !text.includes('</example')) return [];
140+
130141
const tagPositions = new Set<number>();
131142
function collectTags(tokens: Token[], raw: string, baseOffset: number) {
132143
let cursor = 0;
@@ -152,7 +163,13 @@ function computeExampleRanges(
152163
}
153164
proseParts.push(text.slice(cursor));
154165
const prose = proseParts.join('');
155-
collectTags(Lexer.lexInline(prose), prose, 0);
166+
try {
167+
collectTags(Lexer.lexInline(prose), prose, 0);
168+
} catch {
169+
// The regex-only implementation this replaced could not throw; a lexer
170+
// failure must degrade to "no example ranges", not abort the turn.
171+
tagPositions.clear();
172+
}
156173

157174
const ranges: Array<[number, number]> = [];
158175
const tags = /<\/?example(?=[\s/>])(?:[^>"']|"[^"]*"|'[^']*')*>/g;
@@ -192,21 +209,44 @@ function computeExampleRanges(
192209
*/
193210
function recoverableToolCallBlocks(text: string): ToolCallBlock[] {
194211
const blocks: ToolCallBlock[] = [];
212+
// Parameter spans are a property of the raw text, not of which blocks the
213+
// guard accepts: they mask parameter data out of the prose handed to the
214+
// markdown lexer and out of fence tracking. Deriving them from accepted
215+
// blocks only left a rejected block's data unmasked, where a literal example
216+
// tag in it could swallow a later valid call.
195217
const parameterRanges: Array<[number, number]> = [];
196-
TOOL_CALL_PATTERN.lastIndex = 0;
218+
PARAMETER_PATTERN.lastIndex = 0;
219+
let rangeMatch: RegExpExecArray | null;
220+
while ((rangeMatch = PARAMETER_PATTERN.exec(text)) !== null) {
221+
parameterRanges.push([
222+
rangeMatch.index,
223+
rangeMatch.index + rangeMatch[0].length,
224+
]);
225+
}
197226

227+
TOOL_CALL_PATTERN.lastIndex = 0;
198228
let match: RegExpExecArray | null;
199229
while ((match = TOOL_CALL_PATTERN.exec(text)) !== null) {
200230
const toolName = match[1] ?? match[3];
201231
const paramsBlock = match[2] ?? match[4];
232+
// A rejected block may have swallowed a complete later block, so rescan
233+
// from just after this block's open tag instead of its borrowed close.
234+
const resumeAt = match.index + match[0].indexOf('>') + 1;
202235
PARAMETER_PATTERN.lastIndex = 0;
203236
const outsideParameters = paramsBlock.replace(PARAMETER_PATTERN, '');
237+
PARAM_OPEN_PATTERN.lastIndex = 0;
204238
// A missing close must not borrow a later block's parameters or recover
205-
// only the arguments preceding a prematurely matched function close.
239+
// only the arguments preceding a prematurely matched function close. A
240+
// borrowed close leaves no residual tag for the second test to see, so
241+
// count the open tags the accepted matches did not close.
206242
if (
207243
/<\/?(?:function|invoke|parameter)(?:[\s=>]|$)/.test(outsideParameters) ||
208-
/^ {0,3}(?:`{3,}|~{3,})/m.test(outsideParameters)
244+
/^ {0,3}(?:`{3,}|~{3,})/m.test(outsideParameters) ||
245+
paramsBlock.match(PARAM_OPEN_PATTERN)?.length !==
246+
(outsideParameters.match(PARAM_OPEN_PATTERN)?.length ?? 0) +
247+
(paramsBlock.match(PARAMETER_PATTERN)?.length ?? 0)
209248
) {
249+
TOOL_CALL_PATTERN.lastIndex = resumeAt;
210250
continue;
211251
}
212252

@@ -215,24 +255,13 @@ function recoverableToolCallBlocks(text: string): ToolCallBlock[] {
215255
unknown
216256
>;
217257
PARAMETER_PATTERN.lastIndex = 0;
218-
const ranges: Array<[number, number]> = [];
219-
const bodyStart =
220-
match.index +
221-
match[0].length -
222-
paramsBlock.length -
223-
(match[1] !== undefined ? '</invoke>'.length : '</function>'.length);
224-
225258
let paramMatch: RegExpExecArray | null;
226259
while ((paramMatch = PARAMETER_PATTERN.exec(paramsBlock)) !== null) {
227260
const paramName = paramMatch[1] ?? paramMatch[2];
228261
const paramValue = decodeXmlEntities(
229262
stripDelimitingNewlines(paramMatch[3]),
230263
);
231264
args[paramName] = parseParameterValue(paramValue);
232-
ranges.push([
233-
bodyStart + paramMatch.index,
234-
bodyStart + paramMatch.index + paramMatch[0].length,
235-
]);
236265
}
237266

238267
if (toolName && Object.keys(args).length > 0) {
@@ -242,7 +271,6 @@ function recoverableToolCallBlocks(text: string): ToolCallBlock[] {
242271
start: match.index,
243272
end: match.index + match[0].length,
244273
});
245-
parameterRanges.push(...ranges);
246274
}
247275
}
248276

0 commit comments

Comments
 (0)