diff --git a/crates/tinytools-agent/src/parse/grammar/tagged.rs b/crates/tinytools-agent/src/parse/grammar/tagged.rs index 8fe09ef..d7be734 100644 --- a/crates/tinytools-agent/src/parse/grammar/tagged.rs +++ b/crates/tinytools-agent/src/parse/grammar/tagged.rs @@ -57,6 +57,110 @@ static TAG_RE: LazyLock> = LazyLock::new(|| { .ok() }); +/// The bare `` literal — no attributes — with the optional `DeepSeek` +/// DSML marker or XML namespace the named form already tolerates +/// ([`super::invoke_xml`]'s `PREFIX`). +/// +/// The prefix used to be absent here: the opener was a literal `""` +/// match and the closer a literal `""`, so `<｜DSML｜ invoke>` — a +/// `deepseek` turn that emitted the invoke form *without* a `name` attribute, +/// carrying the name in the JSON body instead — opened no block and the call +/// was dropped as prose. The named spelling `<｜DSML｜invoke name="x">` parsed +/// fine, and so did the unprefixed bare ``; only the combination of +/// the two accommodations was missing. +/// +/// Attributes are excluded on purpose: `` belongs to +/// [`super::invoke_xml`], which reads the name off the tag. This matches only +/// the attribute-less form, whose name can come from the body. +static BARE_INVOKE_OPEN_RE: LazyLock> = LazyLock::new(|| { + Regex::new(r"(?i)<(?:[|\u{ff5c}]{1,2}\s*DSML\s*[|\u{ff5c}]{1,2}\s*|[a-z_][\w.-]*:)?invoke\s*>") + .ok() +}); + +/// A bare invoke closer with the same permitted prefix shapes as its opener. +static BARE_INVOKE_CLOSE_RE: LazyLock> = LazyLock::new(|| { + Regex::new(r"(?i)") + .ok() +}); + +/// A complete named invoke or function opener accepted by `invoke_xml`. +/// +/// This is used only as a recovery boundary after a complete JSON value. The +/// tagged grammar must leave that later call for `invoke_xml` to decode. +static NAMED_INVOKE_OPEN_RE: LazyLock> = LazyLock::new(|| { + Regex::new( + r#"(?is)<(?:[|\u{ff5c}]{1,2}\s*DSML\s*[|\u{ff5c}]{1,2}\s*|[a-z_][\w.-]*:)?(?:invoke|function)(?:\s+[^>]*?\bname\s*=\s*"[^"]*"[^>]*|\s*=\s*[^\s>,]+[^>]*)>"#, + ) + .ok() +}); + +/// First match of `re` in `haystack`, as `(start, end)`. +fn find_re(re: &LazyLock>, haystack: &str) -> Option<(usize, usize)> { + re.as_ref() + .and_then(|re| re.find(haystack)) + .map(|m| (m.start(), m.end())) +} + +/// Finds the closer that has the exact prefix and spelling of `opener`. +/// +/// A bare `` must not be closed by `` embedded in its +/// JSON body. When the body begins with valid JSON, skip that whole value too: +/// a matching-looking closer in a JSON string is data rather than markup. +fn matching_invoke_close(opener: &str, after: &str) -> Option<(usize, usize)> { + let json_end = find_json_end(after) + .filter(|&end| serde_json::from_str::(&after[..end]).is_ok()); + let start = json_end.unwrap_or(0); + let opener = normalized_invoke_marker(opener); + BARE_INVOKE_CLOSE_RE.as_ref().and_then(|re| { + re.find_iter(&after[start..]) + .find(|close| normalized_invoke_marker(close.as_str()) == opener) + .map(|close| (start + close.start(), start + close.end())) + }) +} + +/// Normalizes an invoke marker enough to compare its semantic prefix. +fn normalized_invoke_marker(marker: &str) -> String { + marker + .chars() + .filter(|ch| !matches!(ch, '<' | '>' | '/') && !ch.is_whitespace()) + .flat_map(char::to_lowercase) + .collect() +} + +/// Finds a bare invoke's closer unless a complete named successor comes first. +fn invoke_close(opener: &str, after: &str) -> Option<(usize, usize)> { + let close = matching_invoke_close(opener, after); + let successor = named_invoke_boundary(after); + if successor.is_some_and(|start| close.is_none_or(|(end, _)| start < end)) { + None + } else { + close + } +} + +/// The start of a later block that is safe to parse after an unterminated, +/// undecodable tagged block. +/// +/// A successfully decoded leading JSON value is the only reliable delimiter +/// available without a matching outer tag. It prevents an `` inside +/// a rejected JSON string from becoming an executable nested call. +fn recovery_boundary(text: &str, body_start: usize) -> Option { + let after = &text[body_start..]; + let json_end = find_json_end(after) + .filter(|&end| serde_json::from_str::(&after[..end]).is_ok())?; + let from = body_start + json_end; + let tagged = next_opener(text, from).map(|opener| opener.start); + let named = find_re(&NAMED_INVOKE_OPEN_RE, &text[from..]).map(|(start, _)| from + start); + [tagged, named].into_iter().flatten().min() +} + +/// The first named invoke after a valid leading JSON value in `text`. +fn named_invoke_boundary(text: &str) -> Option { + let json_end = find_json_end(text) + .filter(|&end| serde_json::from_str::(&text[..end]).is_ok())?; + find_re(&NAMED_INVOKE_OPEN_RE, &text[json_end..]).map(|(start, _)| json_end + start) +} + /// Openers a fenced block can carry. `` ```tool_calls `` (plural) is listed /// separately from `` ```tool_call `` rather than relying on a prefix match: /// `next_opener` requires the language to end exactly at the literal, so @@ -160,7 +264,7 @@ impl Tagged { } OpenerKind::Invoke => { let after = &text[body_start..]; - after.find("").map(|i| (i, i + "".len())) + invoke_close(&text[opener.start..body_start], after) } OpenerKind::Fence => fence_close(&text[body_start..]), }; @@ -249,9 +353,23 @@ impl Tagged { }); } } + // Nothing recoverable in this block. A complete JSON value establishes + // a structural boundary after the malformed call. Only then may a + // later opener start a new scan: markup inside the JSON value is data, + // never a nested call to execute. + // + // That is not hypothetical. A `deepseek` turn emitted an unterminated + // `` whose body carried `{"arguments":{…}}` with no name — + // unrecoverable, correctly — immediately followed by a complete + // `<｜DSML｜ invoke>` call. Swallowing to end-of-text dropped the good + // call with the bad one and the whole response parsed as prose. A + // block that failed to decode must not be allowed to bury its + // successors. + // + let end = recovery_boundary(text, body_start).unwrap_or(text.len()); Probe::Found(Block { start: opener.start, - end: text.len(), + end, decoded: Decoded::Verbatim, }) } @@ -357,12 +475,12 @@ fn next_opener(text: &str, from: usize) -> Option { } } - if let Some(idx) = find_ci(text, "", from) { + if let Some((start, end)) = find_re(&BARE_INVOKE_OPEN_RE, &text[from..]) { consider( &mut best, Opener { - start: idx, - body_start: idx + "".len(), + start: from + start, + body_start: from + end, kind: OpenerKind::Invoke, }, ); @@ -397,7 +515,7 @@ fn next_opener(text: &str, from: usize) -> Option { } /// The closer of a fenced block: a closing fence, a stray tag-family closer, -/// or ``, whichever comes first. +/// or bare invoke closer, whichever comes first. fn fence_close(after: &str) -> Option<(usize, usize)> { let mut best: Option<(usize, usize)> = None; let mut consider = |candidate: Option<(usize, usize)>| { @@ -419,7 +537,7 @@ fn fence_close(after: &str) -> Option<(usize, usize)> { }) .map(|m| (m.start(), m.end())), ); - consider(after.find("").map(|i| (i, i + "".len()))); + consider(find_re(&BARE_INVOKE_CLOSE_RE, after)); best } diff --git a/crates/tinytools-agent/src/parse/test/tagged.rs b/crates/tinytools-agent/src/parse/test/tagged.rs index aca11db..75294ae 100644 --- a/crates/tinytools-agent/src/parse/test/tagged.rs +++ b/crates/tinytools-agent/src/parse/test/tagged.rs @@ -573,3 +573,152 @@ fn the_plural_dsml_wrapper_is_not_a_tag_marker() { assert_eq!(calls.len(), 1, "the inner call is the only call: {calls:?}"); assert_eq!(calls[0].name, "echo"); } + +/// A `DeepSeek` turn may emit the invoke form *without* a `name` attribute and +/// carry the name in the JSON body instead. The named DSML spelling +/// (`<｜DSML｜invoke name="x">`) and the bare unprefixed `` both parsed +/// already; only their combination was missed, and the call was dropped as +/// prose. +#[test] +fn a_bare_dsml_invoke_carries_its_name_in_the_body() { + let raw = concat!( + "<｜DSML｜ invoke>\n", + "{\"arguments\": {\"command\": \"ls\"}, \"name\": \"shell\"}", + "\n", + "" + ); + let (_, calls) = crate::parse::parse_tool_calls(raw); + assert_eq!(calls.len(), 1, "the bare DSML invoke is a call: {calls:?}"); + assert_eq!(calls[0].name, "shell"); + + // The spellings that already worked must keep working: the prefix is + // optional, and an ASCII bar, a doubled bar and a namespace are the same + // accommodation `invoke_xml` makes on the named form. + for open_close in [ + ("", ""), + ("<|DSML| invoke>", ""), + ("<｜｜DSML｜｜invoke>", ""), + ("", ""), + ] { + let (open, close) = open_close; + let raw = format!("{open}{{\"name\":\"echo\",\"arguments\":{{}}}}{close}"); + let (_, calls) = crate::parse::parse_tool_calls(&raw); + assert_eq!( + calls.len(), + 1, + "variant {open_close:?} must parse: {calls:?}" + ); + assert_eq!(calls[0].name, "echo"); + } +} + +/// A block that decodes to nothing must not bury the calls after it. +/// +/// Verbatim from a `deepseek` turn: an unterminated `` whose body is +/// `{"arguments":{…}}` with no name — genuinely unrecoverable, since the name +/// survived only in a corrupted `<｜DSML｜ parameter name="name":"file_write"}` +/// line and inventing one is never right — followed by a complete +/// `<｜DSML｜ invoke>`. The failed block used to run to end-of-text and take +/// the good call with it, so the whole response parsed as prose and both calls +/// were lost. +#[test] +fn an_undecodable_block_does_not_swallow_the_call_after_it() { + let raw = concat!( + "Heredocs aren't working in this shell. Writing the script to a file instead.\n\n", + "\n", + "{\"arguments\":{\"path\":\"work/extract.py\",\"content\":\"import re\"}}", + "\n", + "<｜DSML｜ parameter name=\"name\":\"file_write\"}\n", + "\n", + "<｜DSML｜ invoke>\n", + "{\"arguments\":{\"category\":\"read\",\"command\":\"ls\"},\"name\":\"shell\"}", + "\n", + "\n", + "" + ); + let outcome = super::parse_known(raw, &["file_write", "shell"]); + assert_eq!( + outcome.calls.len(), + 1, + "the well-formed call survives its malformed neighbour: {:?}", + outcome.calls + ); + assert_eq!(outcome.calls[0].name, "shell"); + + // Reduced to the essential shape, so a future change that reintroduces the + // swallow fails here with less noise. + let raw = concat!( + "\n{\"arguments\":{\"path\":\"x\"}}\n", + "<｜DSML｜ invoke>\n{\"arguments\":{\"command\":\"ls\"},\"name\":\"shell\"}" + ); + let outcome = super::parse_known(raw, &["file_write", "shell"]); + assert_eq!(outcome.calls.len(), 1, "{:?}", outcome.calls); + assert_eq!(outcome.calls[0].name, "shell"); +} + +#[test] +fn a_bare_invoke_requires_its_own_closer() { + let raw = concat!( + "{\"name\":\"echo\",\"arguments\":{\"text\":\"literal marker\"}}", + "" + ); + let (_, calls) = parse(raw); + assert_eq!(calls.len(), 1, "{calls:?}"); + assert_eq!( + calls[0].arguments, + serde_json::json!({"text": "literal marker"}) + ); +} + +#[test] +fn a_bare_dsml_invoke_allows_equivalent_closer_spacing() { + let raw = concat!( + "<｜DSML｜ invoke>{\"name\":\"echo\",\"arguments\":{}}", + "" + ); + let (text, calls) = parse(raw); + assert_eq!(calls.len(), 1, "{calls:?}"); + assert!(text.is_empty(), "{text:?}"); +} + +#[test] +fn a_fenced_invoke_block_can_close_with_an_invoke_tag() { + let raw = "```invoke\n{\"name\":\"echo\",\"arguments\":{}}"; + let (text, calls) = parse(raw); + assert_eq!(calls.len(), 1, "{calls:?}"); + assert!(text.is_empty(), "{text:?}"); +} + +#[test] +fn recovery_leaves_a_named_invoke_after_a_complete_malformed_body() { + let raw = concat!( + "{\"arguments\":{}}", + "ls" + ); + let outcome = super::parse_known(raw, &["shell"]); + assert_eq!(outcome.calls.len(), 1, "{:?}", outcome.calls); + assert_eq!(outcome.calls[0].name, "shell"); +} + +#[test] +fn a_named_invoke_precedes_a_later_bare_invoke_closer() { + let raw = concat!( + "{\"name\":\"echo\",\"arguments\":{}}", + "ls", + "" + ); + let outcome = super::parse_known(raw, &["echo", "shell"]); + assert_eq!(outcome.calls.len(), 2, "{:?}", outcome.calls); + assert_eq!(outcome.calls[0].name, "echo"); + assert_eq!(outcome.calls[1].name, "shell"); +} + +#[test] +fn recovery_does_not_execute_a_named_invoke_inside_malformed_json() { + let raw = concat!( + "{\"arguments\":{\"example\":\"", + "rm -rf /\"}}" + ); + let (_, calls) = parse(raw); + assert!(calls.is_empty(), "{calls:?}"); +}