From 4e93ae666b5ddc59d48c3c3c7088ac856c9fd52b Mon Sep 17 00:00:00 2001 From: cyrus Date: Fri, 25 Sep 2026 16:30:09 +0530 Subject: [PATCH] fix(parse): sweep an orphaned tag-family closer with no opener MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit next_opener() never treats a closing marker as an opener — by design, since positional pairing means a closer only ever pairs with an opener that precedes it. But that leaves a real gap: a model that made its actual tool call over a structured/native channel has no `` opener anywhere in its *text* at all, yet sometimes still types a habitual `` in its narrative out of trained habit. With no opener to pair it with, the closer was never recognized as protocol furniture and leaked into the visible chat text verbatim. Reproduces a real leak observed live, mid-answer, right after a "Reasoning · 1 tool call" step: Let me check the details on the top contenders to find the best one for you. [... normal answer continues ...] Fix: probe_decided now also checks for a closing tag-family marker that sits before the next real opener (or has no opener after it at all) and sweeps it as Decoded::Noise — the same treatment invoke_xml's WRAPPER_RE already gives a bare wrapper closer. Also extracted the doubled-opener-skip loop into its own `tag_close_skipping_doubled_openers` helper: the orphan-closer check pushed `probe_decided` over clippy's `too_many_lines` limit, and the loop was a natural, self-contained unit to pull out — no behavior change there, `cargo test` before and after the extraction is byte-identical output. Test plan: - New test `an_orphaned_closer_with_no_opener_anywhere_is_removed_not_shown` reproduces the exact leaked sequence — fails without the fix, passes with it. - `cargo test -p tinytools-agent`: 323 passed, no regressions (in particular `a_doubled_opener_does_not_swallow_an_unrelated_closing_tag` and the pipe/newline doubled-closer tests, which exercise the extracted loop, still pass unchanged). - `cargo fmt --all -- --check`: clean. - `cargo clippy --all-targets --all-features -- -D warnings`: clean. - `cargo build --all-targets --all-features`: clean. --- .../src/parse/grammar/tagged.rs | 100 +++++++++++++----- .../tinytools-agent/src/parse/test/tagged.rs | 17 +++ 2 files changed, 89 insertions(+), 28 deletions(-) diff --git a/crates/tinytools-agent/src/parse/grammar/tagged.rs b/crates/tinytools-agent/src/parse/grammar/tagged.rs index d7be734..f189894 100644 --- a/crates/tinytools-agent/src/parse/grammar/tagged.rs +++ b/crates/tinytools-agent/src/parse/grammar/tagged.rs @@ -230,43 +230,28 @@ impl Grammar for Tagged { impl Tagged { /// The next block whose opener is fully present. fn probe_decided(text: &str, from: usize, options: &ParseOptions<'_>, mode: ScanMode) -> Probe { - let Some(opener) = next_opener(text, from) else { + let opener = next_opener(text, from); + if let Some(noise) = orphan_closer_noise(text, from, opener.as_ref()) { + return noise; + } + + let Some(opener) = opener else { return Probe::None; }; let mut body_start = opener.body_start; - // How many extra openers a doubled block skipped, so the matching - // number of extra closers — never an unrelated closing tag such as - // `` — can be swallowed below. `DeepSeek` V4 doubles both the - // opener and the closer under a code dialect: - // `\n\nNAME(...)\n\n`. - let mut skipped = 0usize; - let close = match opener.kind { + let (close, skipped) = match opener.kind { OpenerKind::Tag => { - // Positional pairing means a doubled opener would otherwise - // close the first tag on an empty body and lose the call. An - // opener followed by nothing but whitespace is the same - // block starting again, so the scan moves past it. - let re = TAG_RE.as_ref(); - loop { - let after = &text[body_start..]; - let Some(m) = re.and_then(|re| re.find(after)) else { - break None; - }; - let is_opener = !is_closing_marker(m.as_str()); - if is_opener && after[..m.start()].trim().is_empty() { - body_start += m.end(); - skipped += 1; - continue; - } - break Some((m.start(), m.end())); - } + let (close, skipped, new_body_start) = + tag_close_skipping_doubled_openers(text, body_start); + body_start = new_body_start; + (close, skipped) } OpenerKind::Invoke => { let after = &text[body_start..]; - invoke_close(&text[opener.start..body_start], after) + (invoke_close(&text[opener.start..body_start], after), 0) } - OpenerKind::Fence => fence_close(&text[body_start..]), + OpenerKind::Fence => (fence_close(&text[body_start..]), 0), }; let after = &text[body_start..]; @@ -447,6 +432,65 @@ fn swallow_extra_closers(rest: &str, max: usize, mode: ScanMode) -> Option` opener in its text at all, but sometimes still types a +/// habitual closer in its narrative; this sweeps it as furniture rather than +/// leaving literal markup in the visible text. +fn orphan_closer_noise(text: &str, from: usize, opener: Option<&Opener>) -> Option { + let re = TAG_RE.as_ref()?; + let hay = &text[from..]; + let m = re.find(hay)?; + if !is_closing_marker(m.as_str()) { + return None; + } + let start = from + m.start(); + if opener.is_some_and(|o| start >= o.start) { + return None; + } + Some(Probe::Found(Block { + start, + end: from + m.end(), + decoded: Decoded::Noise, + })) +} + +/// The closer for a tag opener at `body_start`, skipping past a doubled +/// opener first, and how many extra openers it skipped — so the matching +/// number of extra closers, never an unrelated closing tag such as ``, +/// can be swallowed by the caller. `DeepSeek` V4 doubles both the opener and +/// the closer under a code dialect: +/// `\n\nNAME(...)\n\n`. +/// +/// Positional pairing means a doubled opener would otherwise close the first +/// tag on an empty body and lose the call. An opener followed by nothing but +/// whitespace is the same block starting again, so the scan moves past it. +/// Returns the closer (if any), how many openers it skipped, and the +/// possibly-advanced body start. +fn tag_close_skipping_doubled_openers( + text: &str, + mut body_start: usize, +) -> (Option<(usize, usize)>, usize, usize) { + let re = TAG_RE.as_ref(); + let mut skipped = 0usize; + let close = loop { + let after = &text[body_start..]; + let Some(m) = re.and_then(|re| re.find(after)) else { + break None; + }; + let is_opener = !is_closing_marker(m.as_str()); + if is_opener && after[..m.start()].trim().is_empty() { + body_start += m.end(); + skipped += 1; + continue; + } + break Some((m.start(), m.end())); + }; + (close, skipped, body_start) +} + /// The earliest opener at or after `from`: a non-closing tag-family marker, /// the bare `` literal, or a fence opener. fn next_opener(text: &str, from: usize) -> Option { diff --git a/crates/tinytools-agent/src/parse/test/tagged.rs b/crates/tinytools-agent/src/parse/test/tagged.rs index 75294ae..e587eac 100644 --- a/crates/tinytools-agent/src/parse/test/tagged.rs +++ b/crates/tinytools-agent/src/parse/test/tagged.rs @@ -465,6 +465,23 @@ fn two_adjacent_blocks_are_still_two_blocks() { assert_eq!(calls[1].name, "two"); } +#[test] +fn an_orphaned_closer_with_no_opener_anywhere_is_removed_not_shown() { + // A model that made its actual call over a structured/native channel has + // no `` opener anywhere in its text at all, but sometimes still + // types a habitual `` in its narrative. `next_opener` never + // treats a closing marker as an opener, so nothing pairs with it — it + // must not be left in the visible text as literal markup. + let (text, calls) = parse( + "Let me check the details on the top contenders to find the best one for you.\n\nHere are the results.", + ); + assert_eq!( + text, + "Let me check the details on the top contenders to find the best one for you.\nHere are the results." + ); + assert!(calls.is_empty()); +} + #[test] fn a_doubled_opener_does_not_swallow_an_unrelated_closing_tag() { // Only the extra `` a doubled opener leaves behind is