diff --git a/crates/mesh-llm-guardrails/src/content.rs b/crates/mesh-llm-guardrails/src/content.rs new file mode 100644 index 0000000000..34c3c0e802 --- /dev/null +++ b/crates/mesh-llm-guardrails/src/content.rs @@ -0,0 +1,47 @@ +/// Removes hidden reasoning blocks from model-visible text. +/// +/// This is content hygiene only. Tool calls are parsed by the model runtime's +/// native chat parser and are never recovered from assistant text here. +pub fn strip_thinking_blocks(content: &str) -> String { + let stripped_html = strip_tag_pairs(content, "", ""); + let stripped_brackets = strip_tag_pairs(&stripped_html, "[THINK]", "[/THINK]"); + stripped_brackets.trim().to_owned() +} + +fn strip_tag_pairs(content: &str, start_tag: &str, end_tag: &str) -> String { + let mut remainder = content; + let mut result = String::new(); + while let Some(start_index) = remainder.find(start_tag) { + result.push_str(&remainder[..start_index]); + let after_start = &remainder[start_index + start_tag.len()..]; + if let Some(end_index) = after_start.find(end_tag) { + remainder = &after_start[end_index + end_tag.len()..]; + } else { + return result; + } + } + result.push_str(remainder); + result +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn strips_supported_thinking_blocks() { + assert_eq!( + strip_thinking_blocks("hiddenVisible answer"), + "Visible answer" + ); + assert_eq!( + strip_thinking_blocks("[THINK]hidden[/THINK]Visible answer"), + "Visible answer" + ); + } + + #[test] + fn drops_unterminated_thinking_blocks_without_duplicating_prefix() { + assert_eq!(strip_thinking_blocks("hello truncated"), "hello"); + } +} diff --git a/crates/mesh-llm-guardrails/src/lib.rs b/crates/mesh-llm-guardrails/src/lib.rs index 0761f08eac..d4e05f6356 100644 --- a/crates/mesh-llm-guardrails/src/lib.rs +++ b/crates/mesh-llm-guardrails/src/lib.rs @@ -1,7 +1,7 @@ pub mod compact; +pub mod content; pub mod policy; pub mod request_contract; -pub mod rescue; pub mod structured; pub mod tools; @@ -9,6 +9,7 @@ pub use compact::{ CompactionConfig, CompactionDecision, CompactionOverride, CompactionReport, CompactionRequest, MESH_COMPACT_FIELD, compact_messages, estimate_message_tokens, }; +pub use content::strip_thinking_blocks; pub use policy::{ GuardrailMode, GuardrailPolicy, GuardrailPolicyHandle, RetryExhaustionMode, StreamingGuardrailMode, @@ -17,10 +18,6 @@ pub use request_contract::{ GuardrailRequestContract, MESH_GUARDRAILS_FIELD, MeshGuardrailsOverride, ParallelToolCalls, RawResponseFormat, RawToolChoice, RawToolDefinition, RawToolSpec, StructuredResponseFormat, }; -pub use rescue::{ - ParsedToolCall, ToolCallParseError, parse_tool_call_value, rescue_tool_call_from_text, - strip_thinking_blocks, -}; pub use structured::{StructuredOutputSpec, UnsupportedStructuredSchema}; pub use tools::{ MESH_EMIT_STRUCTURED_TOOL_NAME, MESH_RESPOND_TOOL_NAME, ToolArgumentSchemaError, diff --git a/crates/mesh-llm-guardrails/src/rescue.rs b/crates/mesh-llm-guardrails/src/rescue.rs deleted file mode 100644 index d8aab7452f..0000000000 --- a/crates/mesh-llm-guardrails/src/rescue.rs +++ /dev/null @@ -1,432 +0,0 @@ -use std::collections::BTreeSet; - -use serde_json::{Map, Value, json}; - -use crate::tools::{extract_tool_name_and_arguments, normalize_tool_arguments}; - -const MAX_RESCUE_INPUT_BYTES: usize = 64 * 1024; -const MAX_JSON_CANDIDATES: usize = 32; - -#[derive(Debug, Clone, PartialEq)] -pub struct ParsedToolCall { - pub name: String, - pub arguments: Map, -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ToolCallParseError { - Malformed, - UnknownTool, - InvalidArguments, -} - -pub fn strip_thinking_blocks(content: &str) -> String { - let stripped_html = strip_tag_pairs(content, "", ""); - let stripped_brackets = strip_tag_pairs(&stripped_html, "[THINK]", "[/THINK]"); - stripped_brackets.trim().to_string() -} - -pub fn parse_tool_call_value( - value: &Value, - allowed_tools: &[String], -) -> Result, ToolCallParseError> { - let raw_tool_calls = match raw_tool_calls_from_value(value) { - Some(tool_calls) if !tool_calls.is_empty() => tool_calls, - _ => return Err(ToolCallParseError::Malformed), - }; - let allowed_tools = allowed_tools - .iter() - .map(String::as_str) - .collect::>(); - let mut parsed_calls = Vec::new(); - for tool_call in raw_tool_calls { - parsed_calls.push(parse_one_tool_call(tool_call, &allowed_tools)?); - } - Ok(parsed_calls) -} - -pub fn rescue_tool_call_from_text( - content: &str, - allowed_tools: &[String], -) -> Result, ToolCallParseError> { - let content = strip_thinking_blocks(content); - let mut last_error = ToolCallParseError::Malformed; - for candidate in tool_call_candidates(&content) { - match parse_tool_call_value(&candidate, allowed_tools) { - Ok(parsed) => return Ok(parsed), - Err(error) => last_error = more_specific_error(last_error, error), - } - } - Err(last_error) -} - -fn more_specific_error( - current: ToolCallParseError, - next: ToolCallParseError, -) -> ToolCallParseError { - match (current, next) { - (ToolCallParseError::InvalidArguments, _) | (_, ToolCallParseError::InvalidArguments) => { - ToolCallParseError::InvalidArguments - } - (ToolCallParseError::UnknownTool, _) | (_, ToolCallParseError::UnknownTool) => { - ToolCallParseError::UnknownTool - } - _ => ToolCallParseError::Malformed, - } -} - -fn strip_tag_pairs(content: &str, start_tag: &str, end_tag: &str) -> String { - let mut remainder = content; - let mut result = String::new(); - while let Some(start_index) = remainder.find(start_tag) { - result.push_str(&remainder[..start_index]); - let after_start = &remainder[start_index + start_tag.len()..]; - if let Some(end_index) = after_start.find(end_tag) { - remainder = &after_start[end_index + end_tag.len()..]; - } else { - remainder = &remainder[..start_index]; - break; - } - } - result.push_str(remainder); - result -} - -fn tool_call_candidates(content: &str) -> Vec { - let mut candidates = Vec::new(); - for json_candidate in json_candidates(content) { - if let Ok(value) = serde_json::from_str::(&json_candidate) { - candidates.push(value); - } - } - if let Some(value) = parse_bracket_args_tool_syntax(content) { - candidates.push(value); - } - if let Some(value) = parse_qwen_xml_syntax(content) { - candidates.push(value); - } - if let Some(value) = parse_arg_tag_tool_call_syntax(content) { - candidates.push(value); - } - if let Some(value) = parse_granite_tool_call_syntax(content) { - candidates.push(value); - } - candidates -} - -fn json_candidates(content: &str) -> Vec { - let content = bounded_prefix(content, MAX_RESCUE_INPUT_BYTES); - let mut candidates = Vec::new(); - push_candidate(&mut candidates, content.trim()); - for fenced in fenced_code_blocks(content) { - if candidates.len() >= MAX_JSON_CANDIDATES { - break; - } - push_candidate(&mut candidates, fenced.trim()); - } - for balanced in balanced_json_substrings(content) { - if candidates.len() >= MAX_JSON_CANDIDATES { - break; - } - push_candidate(&mut candidates, balanced.trim()); - } - candidates -} - -fn bounded_prefix(content: &str, max_bytes: usize) -> &str { - if content.len() <= max_bytes { - return content; - } - let mut end = max_bytes; - while end > 0 && !content.is_char_boundary(end) { - end -= 1; - } - &content[..end] -} - -fn push_candidate(candidates: &mut Vec, candidate: &str) { - if !candidate.is_empty() && !candidates.iter().any(|existing| existing == candidate) { - candidates.push(candidate.to_string()); - } -} - -fn fenced_code_blocks(content: &str) -> Vec { - let mut blocks = Vec::new(); - let mut remainder = content; - while let Some(open_index) = remainder.find("```") { - let after_open = &remainder[open_index + 3..]; - let Some(close_index) = after_open.find("```") else { - break; - }; - let block = &after_open[..close_index]; - let block = block - .strip_prefix("json\n") - .or_else(|| block.strip_prefix("JSON\n")) - .unwrap_or(block); - blocks.push(block.to_string()); - remainder = &after_open[close_index + 3..]; - } - blocks -} - -fn balanced_json_substrings(content: &str) -> Vec { - let bytes = content.as_bytes(); - let mut candidates = Vec::new(); - for (index, byte) in bytes.iter().enumerate() { - if candidates.len() >= MAX_JSON_CANDIDATES { - break; - } - let closing = match byte { - b'{' => b'}', - b'[' => b']', - _ => continue, - }; - if let Some(end) = balanced_substring_end(bytes, index, *byte, closing) { - candidates.push(content[index..=end].to_string()); - } - } - candidates -} - -fn balanced_substring_end(bytes: &[u8], start: usize, opening: u8, closing: u8) -> Option { - let mut depth = 0_u32; - let mut in_string = false; - let mut escaped = false; - for (index, byte) in bytes.iter().copied().enumerate().skip(start) { - if in_string { - if escaped { - escaped = false; - continue; - } - match byte { - b'\\' => escaped = true, - b'"' => in_string = false, - _ => {} - } - continue; - } - match byte { - b'"' => in_string = true, - _ if byte == opening => depth += 1, - _ if byte == closing => { - depth = depth.saturating_sub(1); - if depth == 0 { - return Some(index); - } - } - _ => {} - } - } - None -} - -fn parse_bracket_args_tool_syntax(content: &str) -> Option { - let marker = "[ARGS]"; - if let Some(marker_index) = content.find(marker) { - let name = trailing_tool_name(&content[..marker_index])?; - let after_marker = content[marker_index + marker.len()..].trim_start(); - let json_text = first_balanced_object(after_marker)?; - let arguments = serde_json::from_str::(&json_text).ok()?; - return Some(json!({ "name": name, "arguments": arguments })); - } - parse_parenthesized_tool_call(content) -} - -fn parse_qwen_xml_syntax(content: &str) -> Option { - let function_prefix = "')?; - let name = after_prefix[..name_end] - .trim() - .trim_matches('"') - .trim_matches('\''); - if name.is_empty() { - return None; - } - let body = &after_prefix[name_end + 1..]; - let function_end = body.find("")?; - let mut arguments = Map::new(); - let mut remainder = &body[..function_end]; - while let Some(parameter_start) = remainder.find("")?; - let value = parameter_body[..value_end].trim(); - let parsed_value = serde_json::from_str::(value) - .unwrap_or_else(|_| Value::String(value.to_string())); - arguments.insert(parameter_name.to_string(), parsed_value); - remainder = ¶meter_body[value_end + "".len()..]; - } - if arguments.is_empty() { - return None; - } - Some(json!({ "name": name, "arguments": Value::Object(arguments) })) -} - -fn parse_granite_tool_call_syntax(content: &str) -> Option { - let start_tag = ""; - let end_tag = ""; - let start_index = content.find(start_tag)?; - let after_start = &content[start_index + start_tag.len()..]; - let end_index = after_start.find(end_tag)?; - serde_json::from_str(after_start[..end_index].trim()).ok() -} - -fn parse_arg_tag_tool_call_syntax(content: &str) -> Option { - let body = tagged_body(content, "", "")?; - let arguments_start = body.find("").unwrap_or(body.len()); - let name = body[..arguments_start].trim(); - if name.is_empty() || name.contains('<') { - return None; - } - - let mut arguments = Map::new(); - let mut remainder = &body[arguments_start..]; - while !remainder.trim().is_empty() { - remainder = remainder.trim_start().strip_prefix("")?; - let key_end = remainder.find("")?; - let key = remainder[..key_end].trim(); - if key.is_empty() { - return None; - } - - remainder = remainder[key_end + "".len()..].trim_start(); - remainder = remainder.strip_prefix("")?; - let value_end = remainder.find("")?; - let value = remainder[..value_end].trim(); - let parsed_value = serde_json::from_str::(value) - .unwrap_or_else(|_| Value::String(value.to_string())); - arguments.insert(key.to_string(), parsed_value); - remainder = &remainder[value_end + "".len()..]; - } - - Some(json!({ "name": name, "arguments": Value::Object(arguments) })) -} - -fn tagged_body<'a>(content: &'a str, start_tag: &str, end_tag: &str) -> Option<&'a str> { - let start_index = content.find(start_tag)?; - let after_start = &content[start_index + start_tag.len()..]; - let end_index = after_start.find(end_tag)?; - Some(&after_start[..end_index]) -} - -fn first_balanced_object(content: &str) -> Option { - let start = content.find('{')?; - let end = balanced_substring_end(content.as_bytes(), start, b'{', b'}')?; - Some(content[start..=end].to_string()) -} - -fn parse_parenthesized_tool_call(content: &str) -> Option { - let open_paren = content.find('(')?; - let name = trailing_tool_name(&content[..open_paren])?; - let after_open = content[open_paren + 1..].trim_start(); - let json_text = first_balanced_object(after_open)?; - let after_json = after_open[json_text.len()..].trim_start(); - if !after_json.starts_with(')') { - return None; - } - let arguments = serde_json::from_str::(&json_text).ok()?; - Some(json!({ "name": name, "arguments": arguments })) -} - -fn trailing_tool_name(content: &str) -> Option<&str> { - let name = content - .trim() - .rsplit(|character: char| { - !character.is_ascii_alphanumeric() && character != '_' && character != '-' - }) - .next()? - .trim(); - (!name.is_empty()).then_some(name) -} - -fn raw_tool_calls_from_value(value: &Value) -> Option> { - match value { - Value::Array(entries) => Some(entries.iter().collect()), - Value::Object(object) => object - .get("tool_calls") - .and_then(Value::as_array) - .map(|entries| entries.iter().collect()) - .or_else(|| Some(vec![value])), - _ => None, - } -} - -fn parse_one_tool_call( - value: &Value, - allowed_tools: &BTreeSet<&str>, -) -> Result { - let Some((name, arguments_value)) = extract_tool_name_and_arguments(value) else { - return Err(ToolCallParseError::Malformed); - }; - if !allowed_tools.is_empty() && !allowed_tools.contains(name) { - return Err(ToolCallParseError::UnknownTool); - } - let Some(arguments) = normalize_tool_arguments(arguments_value) else { - return Err(ToolCallParseError::InvalidArguments); - }; - Ok(ParsedToolCall { - name: name.to_string(), - arguments, - }) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn rescues_qwen_xml_tool_call() { - let calls = rescue_tool_call_from_text( - r#"README.md"#, - &[], - ) - .unwrap(); - - assert_eq!(calls[0].name, "read_file"); - assert_eq!(calls[0].arguments["path"], "README.md"); - } - - #[test] - fn rescues_parenthesized_tool_call() { - let calls = rescue_tool_call_from_text(r#"read_file({"path":"README.md"})"#, &[]).unwrap(); - - assert_eq!(calls[0].name, "read_file"); - assert_eq!(calls[0].arguments["path"], "README.md"); - } - - #[test] - fn rescues_arg_tag_tool_call() { - let calls = rescue_tool_call_from_text( - "treepathtests\ - depth2", - &["tree".to_string()], - ) - .unwrap(); - - assert_eq!(calls[0].name, "tree"); - assert_eq!(calls[0].arguments["path"], "tests"); - assert_eq!(calls[0].arguments["depth"], 2); - } - - #[test] - fn rejects_unknown_tool_when_catalog_is_present() { - let allowed_tools = vec!["read_file".to_string()]; - let error = rescue_tool_call_from_text( - r#"{"name":"write_file","arguments":{"path":"README.md"}}"#, - &allowed_tools, - ) - .unwrap_err(); - - assert_eq!(error, ToolCallParseError::UnknownTool); - } -} diff --git a/crates/mesh-llm-host-runtime/src/inference/consult.rs b/crates/mesh-llm-host-runtime/src/inference/consult.rs index d9be16b8eb..bb95073e1f 100644 --- a/crates/mesh-llm-host-runtime/src/inference/consult.rs +++ b/crates/mesh-llm-host-runtime/src/inference/consult.rs @@ -15,7 +15,9 @@ use crate::mesh; use anyhow::Result; use iroh::EndpointId; -use mesh_llm_guardrails::{parse_tool_call_value, strip_thinking_blocks}; +use mesh_llm_guardrails::{ + extract_tool_name_and_arguments, normalize_tool_arguments, strip_thinking_blocks, +}; use serde_json::Value; // --------------------------------------------------------------------------- @@ -211,11 +213,16 @@ fn parse_chat_completion_response(response: &[u8]) -> Result { } fn tool_calls_as_consultation_text(message: &Value) -> Option { - let allowed_tools = Vec::new(); - let calls = parse_tool_call_value(&message["tool_calls"], &allowed_tools).ok()?; - let first = calls.first()?; - let arguments = serde_json::to_string(&first.arguments).ok()?; - Some(format!("{}({arguments})", first.name)) + let first = message.get("tool_calls")?.as_array()?.first()?; + let name = first + .pointer("/function/name") + .and_then(Value::as_str) + .or_else(|| first.get("name").and_then(Value::as_str))?; + let arguments = extract_tool_name_and_arguments(first) + .and_then(|(_, raw_arguments)| normalize_tool_arguments(raw_arguments)) + .unwrap_or_default(); + let arguments = serde_json::to_string(&arguments).ok()?; + Some(format!("{name}({arguments})")) } // --------------------------------------------------------------------------- @@ -497,7 +504,7 @@ mod tests { } #[test] - fn parse_chat_completion_response_falls_back_to_tool_calls() { + fn parse_chat_completion_response_uses_native_tool_calls_when_content_is_empty() { let response = concat!( "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n\r\n", "{\"choices\":[{\"message\":{\"content\":\"\",\"tool_calls\":[", @@ -512,7 +519,7 @@ mod tests { } #[test] - fn parse_chat_completion_response_falls_back_to_tool_calls_after_thinking_stripping() { + fn parse_chat_completion_response_uses_native_tool_calls_after_thinking_stripping() { let response = concat!( "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n\r\n", "{\"choices\":[{\"message\":{\"content\":\"scratch\",\"tool_calls\":[", diff --git a/crates/mesh-llm-host-runtime/src/runtime/survey.rs b/crates/mesh-llm-host-runtime/src/runtime/survey.rs index 8d8b11b22e..c4b071ea82 100644 --- a/crates/mesh-llm-host-runtime/src/runtime/survey.rs +++ b/crates/mesh-llm-host-runtime/src/runtime/survey.rs @@ -38,7 +38,6 @@ const TELEMETRY_ATTRIBUTE_ALLOWLIST: &[&str] = &[ "mesh_llm.guardrail.decision", "mesh_llm.guardrail.mode", "mesh_llm.guardrail.outcome", - "mesh_llm.guardrail.parser_stage", "mesh_llm.gpu_count", "mesh_llm.gpu_name", "mesh_llm.gpu_stable_id", @@ -320,7 +319,6 @@ impl GuardrailTelemetrySink for SurveyTelemetry { mode: GuardrailMode, contract: Option<&'static str>, outcome: &'static str, - parser_stage: Option<&'static str>, attempt_bucket: Option<&'static str>, ) { let Some(inner) = self.inner.as_ref() else { @@ -341,13 +339,6 @@ impl GuardrailTelemetrySink for SurveyTelemetry { Some(value) => value, None => return, }, - parser_stage: match parser_stage { - Some(value) => match guardrail_parser_stage_attr(value) { - Some(label) => Some(label), - None => return, - }, - None => None, - }, attempt_bucket: match attempt_bucket { Some(value) => match guardrail_attempt_bucket_attr(value) { Some(label) => Some(label), @@ -838,16 +829,7 @@ fn guardrail_bypass_reason_attr(value: &'static str) -> Option<&'static str> { fn guardrail_outcome_attr(value: &'static str) -> Option<&'static str> { match value { - "pass_through" | "valid" | "rescued" | "retried" | "failed" | "metrics_only_failure" => { - Some(value) - } - _ => None, - } -} - -fn guardrail_parser_stage_attr(value: &'static str) -> Option<&'static str> { - match value { - "none" | "json_exact" | "json_fenced" | "json_substring" => Some(value), + "pass_through" | "valid" | "retried" | "failed" | "metrics_only_failure" => Some(value), _ => None, } } @@ -890,7 +872,6 @@ struct GuardrailOutcomeAttributes { mode: &'static str, contract: Option<&'static str>, outcome: &'static str, - parser_stage: Option<&'static str>, attempt_bucket: Option<&'static str>, } @@ -902,12 +883,6 @@ impl GuardrailOutcomeAttributes { if let Some(contract) = self.contract { attrs.push(KeyValue::new("mesh_llm.guardrail.contract", contract)); } - if let Some(parser_stage) = self.parser_stage { - attrs.push(KeyValue::new( - "mesh_llm.guardrail.parser_stage", - parser_stage, - )); - } if let Some(attempt_bucket) = self.attempt_bucket { attrs.push(KeyValue::new( "mesh_llm.guardrail.attempt_bucket", @@ -1424,7 +1399,6 @@ mod tests { "mesh_llm.guardrail.decision", "mesh_llm.guardrail.mode", "mesh_llm.guardrail.outcome", - "mesh_llm.guardrail.parser_stage", "mesh_llm.gpu_count", "mesh_llm.gpu_name", "mesh_llm.gpu_stable_id", @@ -1495,7 +1469,6 @@ mod tests { mode: "metrics", contract: Some("structured"), outcome: "metrics_only_failure", - parser_stage: Some("json_fenced"), attempt_bucket: Some("2"), } .key_values(), @@ -1535,8 +1508,7 @@ mod tests { source: test_source(), mode: "enforce", contract: Some("structured"), - outcome: "rescued", - parser_stage: Some("json_substring"), + outcome: "valid", attempt_bucket: Some("3_plus"), }; let outcome_kv: HashMap<_, _> = outcome @@ -1550,12 +1522,6 @@ mod tests { .map(String::as_str), Some("structured") ); - assert_eq!( - outcome_kv - .get("mesh_llm.guardrail.parser_stage") - .map(String::as_str), - Some("json_substring") - ); assert!(outcome_kv.values().all(|value| { !value.contains("prompt") && !value.contains("completion") diff --git a/crates/mesh-mixture-of-agents/src/normalize.rs b/crates/mesh-mixture-of-agents/src/normalize.rs index 2208df217d..d42cc2e1e4 100644 --- a/crates/mesh-mixture-of-agents/src/normalize.rs +++ b/crates/mesh-mixture-of-agents/src/normalize.rs @@ -9,10 +9,7 @@ //! Anything the model returns is treated as dirty input. use crate::worker::WorkerRole; -use mesh_llm_guardrails::{ - extract_tool_name_and_arguments, normalize_tool_arguments, rescue_tool_call_from_text, - strip_thinking_blocks, -}; +use mesh_llm_guardrails::{normalize_tool_arguments, strip_thinking_blocks}; use serde_json::Value; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -178,41 +175,6 @@ fn try_json_parse( let json_str = extract_json_object(raw)?; let obj: Value = serde_json::from_str(&json_str).ok()?; - // First, recognise the OpenAI tool-call shape that models commonly - // emit even without our `kind`/`confidence` envelope: - // - // {"function": "read_file", "arguments": {"path": "README.md"}} - // {"name": "read_file", "arguments": {...}} - // {"tool": "read_file", "arguments": {...}} - // - // Agent harnesses (Goose, OpenCode) only act on real `tool_calls` - // — if the worker writes inline tool JSON and we miss it, MoA leaks - // the JSON back as `content` and the agent does nothing. This is - // the failure mode PR #566 review called out. - let openai_tool_call = obj - .get("kind") - .is_none() - .then(|| extract_tool_name_and_arguments(&obj)) - .flatten(); - if let Some((tool_name, arguments)) = openai_tool_call { - let args = normalize_tool_arguments(arguments).map(Value::Object); - return Some(WorkerOutput { - kind: OutputKind::ToolProposal, - // OpenAI-shape tool calls have no native confidence - // marker, but a structurally well-formed proposal is a - // stronger signal than a heuristic catch — score it - // higher than the heuristic's 0.6 so the arbiter - // prefers it on tie. - confidence: 0.75, - tool_name: Some(tool_name.to_string()), - tool_arguments: args, - payload: raw.to_string(), - model: model.to_string(), - role, - elapsed_ms, - }); - } - let kind = match obj.get("kind").and_then(|k| k.as_str()) { Some("tool_proposal") => OutputKind::ToolProposal, Some("critique") => OutputKind::Critique, @@ -374,36 +336,8 @@ fn try_kv_parse(raw: &str, model: &str, role: WorkerRole, elapsed_ms: u64) -> Op /// Heuristic: classify raw text by content patterns. fn heuristic_classify(raw: &str, model: &str, role: WorkerRole, elapsed_ms: u64) -> WorkerOutput { - if let Some(tool_call) = first_rescued_tool_call(raw) { - return WorkerOutput { - kind: OutputKind::ToolProposal, - confidence: 0.7, - tool_name: Some(tool_call.name), - tool_arguments: Some(Value::Object(tool_call.arguments)), - payload: raw.to_string(), - model: model.to_string(), - role, - elapsed_ms, - }; - } - let lower = raw.to_lowercase(); - // Check for tool call patterns - if looks_like_tool_proposal(&lower, raw) { - let (name, args) = extract_tool_proposal(raw); - return WorkerOutput { - kind: OutputKind::ToolProposal, - confidence: 0.6, - tool_name: name, - tool_arguments: args, - payload: raw.to_string(), - model: model.to_string(), - role, - elapsed_ms, - }; - } - // Check for critique patterns if looks_like_critique(&lower) { return WorkerOutput { @@ -445,58 +379,6 @@ fn heuristic_classify(raw: &str, model: &str, role: WorkerRole, elapsed_ms: u64) } } -/// Known tool names that models might reference in prose. These are -/// matched against the lowercased text to detect tool proposals that -/// weren't formatted as structured output. -const KNOWN_TOOLS: &[&str] = &[ - "read_file", - "edit_file", - "run_command", - "search_code", - "web_search", - "get_weather", - "create_file", - "delete_file", - "list_files", -]; - -fn looks_like_tool_proposal(lower: &str, _raw: &str) -> bool { - // Explicit structured markers - let has_structured = lower.contains("tool_call") - || lower.contains("function_call") - || lower.contains("i would call") - || lower.contains("i propose calling") - || lower.contains("tool_proposal"); - - if has_structured && !lower.contains("i would not") { - return true; - } - - // Agentic patterns: model describes using a tool by name - let mentions_tool = KNOWN_TOOLS.iter().any(|t| lower.contains(t)); - if mentions_tool { - // Must also have an action verb — not just mentioning the tool in discussion - let has_action = lower.contains("i'll use") - || lower.contains("i will use") - || lower.contains("let me use") - || lower.contains("i need to use") - || lower.contains("use the") - || lower.contains("using the") - || lower.contains("should use") - || lower.contains("call the") - || lower.contains("calling") - || lower.contains("propose") - || lower.contains("**tool**") - || lower.contains("tool:") - || lower.contains("identify the tool"); - if has_action { - return true; - } - } - - false -} - fn looks_like_critique(lower: &str) -> bool { let markers = [ "however,", @@ -524,47 +406,6 @@ fn looks_like_uncertainty(lower: &str) -> bool { markers.iter().any(|m| lower.contains(m)) } -/// Try to extract a tool name and arguments from messy text. -fn extract_tool_proposal(raw: &str) -> (Option, Option) { - if let Some(tool_call) = first_rescued_tool_call(raw) { - return ( - Some(tool_call.name), - Some(Value::Object(tool_call.arguments)), - ); - } - - // Strategy 1: Look for structured JSON in the text - let parsed_json = - extract_json_object(raw).and_then(|json_str| serde_json::from_str::(&json_str).ok()); - if let Some(obj) = parsed_json { - if let Some((name, arguments)) = extract_tool_name_and_arguments(&obj) { - let args = normalize_tool_arguments(arguments).map(Value::Object); - return (Some(name.to_string()), args); - } - // Could be the arguments themselves (e.g. {"path": "src/auth.py"}) - // Look for a tool name in the surrounding text - let lower = raw.to_lowercase(); - for tool in KNOWN_TOOLS { - if lower.contains(tool) { - return (Some(tool.to_string()), Some(obj)); - } - } - } - - // Strategy 2: Find a known tool name in prose and try to extract args - let lower = raw.to_lowercase(); - for tool in KNOWN_TOOLS { - if lower.contains(tool) { - // Try to find JSON arguments nearby - let args = - extract_json_object(raw).and_then(|s| serde_json::from_str::(&s).ok()); - return (Some(tool.to_string()), args); - } - } - - (None, None) -} - /// Find the first JSON object in text (handles markdown fences, etc.). fn extract_json_object(text: &str) -> Option { // Try the whole thing first @@ -604,12 +445,6 @@ fn extract_json_object(text: &str) -> Option { None } -fn first_rescued_tool_call(raw: &str) -> Option { - rescue_tool_call_from_text(raw, &[]) - .ok() - .and_then(|calls| calls.into_iter().next()) -} - /// Clean passthrough content for display: strip think tags, orphan , /// and any KV envelope lines (kind:/confidence:/payload:) that leaked. pub fn strip_passthrough_content(text: &str) -> String { @@ -695,24 +530,6 @@ mod tests { assert_eq!(out.tool_name.as_deref(), Some("read_file")); } - #[test] - fn prose_tool_proposal() { - // Small models often describe tool usage in prose instead of structured output - let raw = "I'll use the read_file tool to examine the code:\n```json\n{\"path\": \"src/auth.py\"}\n```"; - let out = normalize_worker_output(raw, "small-model", WorkerRole::Fast, 100); - assert_eq!(out.kind, OutputKind::ToolProposal); - assert_eq!(out.tool_name.as_deref(), Some("read_file")); - } - - #[test] - fn prose_edit_proposal() { - let raw = "I need to use the edit_file tool to fix this bug. The arguments would be:\n{\"path\": \"src/auth.py\", \"old_text\": \"== password\", \"new_text\": \"== hash(password)\"}"; - let out = normalize_worker_output(raw, "qwen3:4b", WorkerRole::Specialist, 200); - assert_eq!(out.kind, OutputKind::ToolProposal); - assert_eq!(out.tool_name.as_deref(), Some("edit_file")); - assert!(out.tool_arguments.is_some()); - } - #[test] fn think_tags_then_kv() { let raw = "\nThe user is asking a simple question.\nMultiple workers agree the answer is Canberra.\nI should provide a direct answer.\n\nkind: answer\nconfidence: 1.0\npayload: Canberra is the capital of Australia."; @@ -795,7 +612,7 @@ mod tests { // never made it through the inner `from_str` and leaked as a bare // string into the tool-call wire shape. With `extract_tool_arguments`, // the string is parsed into a real JSON object. - let raw = r#"{"function": "read_file", "arguments": "{\"path\": \"README.md\"}"}"#; + let raw = r#"{"kind":"tool_proposal","confidence":0.9,"tool":"read_file","arguments":"{\"path\":\"README.md\"}"}"#; let out = normalize_worker_output(raw, "test-model", WorkerRole::Fast, 100); assert_eq!(out.kind, OutputKind::ToolProposal); assert_eq!(out.tool_name.as_deref(), Some("read_file")); @@ -810,7 +627,7 @@ mod tests { // which then serialized as the literal string `"null"` in the // OpenAI tool-call wire shape. Now it becomes `None`, and the // response builder substitutes `"{}"`. - let raw = r#"{"function": "list", "arguments": null}"#; + let raw = r#"{"kind":"tool_proposal","confidence":0.9,"tool":"list","arguments":null}"#; let out = normalize_worker_output(raw, "test-model", WorkerRole::Fast, 100); assert_eq!(out.kind, OutputKind::ToolProposal); assert!(out.tool_arguments.is_none()); @@ -820,50 +637,10 @@ mod tests { fn primitive_tool_arguments_collapse_to_empty_object() { // Defensive: a model that emits `"arguments": 42` should not // produce a wire-invalid tool call. - let raw = r#"{"function": "list", "arguments": 42}"#; + let raw = r#"{"kind":"tool_proposal","confidence":0.9,"tool":"list","arguments":42}"#; let out = normalize_worker_output(raw, "test-model", WorkerRole::Fast, 100); let args = out.tool_arguments.expect("sanitize produced an object"); assert!(args.is_object()); assert_eq!(args.as_object().unwrap().len(), 0); } - - #[test] - fn qwen_xml_tool_call_uses_guardrail_rescue() { - let raw = r#"README.md"#; - let out = normalize_worker_output(raw, "qwen", WorkerRole::Fast, 100); - assert_eq!(out.kind, OutputKind::ToolProposal); - assert_eq!(out.tool_name.as_deref(), Some("read_file")); - assert_eq!(out.tool_arguments.expect("args")["path"], "README.md"); - } - - #[test] - fn arg_tag_tool_call_uses_guardrail_rescue() { - let raw = "treepathtests\ - depth2"; - let out = normalize_worker_output(raw, "glm-4.7-flash", WorkerRole::Reducer, 100); - assert_eq!(out.kind, OutputKind::ToolProposal); - assert_eq!(out.tool_name.as_deref(), Some("tree")); - let arguments = out.tool_arguments.expect("args"); - assert_eq!(arguments["path"], "tests"); - assert_eq!(arguments["depth"], 2); - } - - #[test] - fn parenthesized_tool_call_uses_guardrail_rescue() { - let raw = r#"read_file({"path":"README.md"})"#; - let out = normalize_worker_output(raw, "small-model", WorkerRole::Fast, 100); - assert_eq!(out.kind, OutputKind::ToolProposal); - assert_eq!(out.tool_name.as_deref(), Some("read_file")); - assert_eq!(out.tool_arguments.expect("args")["path"], "README.md"); - } - - #[test] - fn normalize_worker_output_rescues_tool_calls_after_thinking_strip() { - let raw = - r#"I should inspect the file first.read_file({"path":"README.md"})"#; - let out = normalize_worker_output(raw, "small-model", WorkerRole::Fast, 100); - assert_eq!(out.kind, OutputKind::ToolProposal); - assert_eq!(out.tool_name.as_deref(), Some("read_file")); - assert_eq!(out.tool_arguments.expect("args")["path"], "README.md"); - } } diff --git a/crates/mesh-mixture-of-agents/tests/sim_tool_call_text_not_passed_as_content.rs b/crates/mesh-mixture-of-agents/tests/sim_tool_call_text_not_passed_as_content.rs deleted file mode 100644 index c6a8ff65e7..0000000000 --- a/crates/mesh-mixture-of-agents/tests/sim_tool_call_text_not_passed_as_content.rs +++ /dev/null @@ -1,212 +0,0 @@ -//! Pin the contract: a tool-call-shaped worker reply must produce a -//! real `tool_calls` field in the response, not get returned as -//! free-form `content`. -//! -//! Background — PR #566 review feedback (Apr 2026): -//! -//! > In the read-tool probe, the model wrote text that looked like a -//! > tool call instead of actually invoking the read tool. -//! -//! Agent harnesses (Goose, OpenCode, pi) only act on `tool_calls`. -//! If a worker writes "I'll use read_file to inspect README.md" and -//! that text leaks out as `choices[0].message.content` instead of -//! `choices[0].message.tool_calls[*]`, the harness sees a text reply -//! and takes no action. This is the core blocker for agent loops. -//! -//! This test drives `handle_turn` with mock workers that all return -//! the same "I'll use read_file" prose, and asserts that the -//! response carries a structured tool call. The contract: -//! -//! * `choices[0].message.tool_calls` is a non-empty array, OR -//! * `choices[0].finish_reason == "tool_calls"`, OR -//! * the response is an error / reducer-escalation (i.e. MoA -//! refused to return prose when the request had `tools` and -//! workers proposed using one). -//! -//! What is NOT acceptable is a response with `content: "I'll use -//! read_file..."` and no `tool_calls` \u2014 the agent harness would -//! silently do nothing. - -use async_trait::async_trait; -use mesh_mixture_of_agents as moa; -use serde_json::{Value, json}; -use std::sync::Arc; -use std::time::Duration; - -/// Backend that returns a fixed text on every call. -struct FixedTextBackend { - text: String, -} - -impl FixedTextBackend { - fn new(text: impl Into) -> Arc { - Arc::new(Self { text: text.into() }) - } -} - -#[async_trait] -impl moa::ModelBackend for FixedTextBackend { - async fn chat_completion( - &self, - _model: &str, - _messages: &[Value], - _tools: Option<&Value>, - _max_tokens: u32, - _timeout: Duration, - _sampling: moa::SamplingParams, - ) -> Result { - // Modest delay so the runtime doesn't optimize the whole turn into - // a single sync poll. - tokio::time::sleep(Duration::from_millis(5)).await; - Ok(json!({ - "choices": [{"message": {"content": self.text}}], - })) - } -} - -fn config_with_three_workers_returning(text: &str) -> moa::GatewayConfig { - let a = FixedTextBackend::new(text); - let b = FixedTextBackend::new(text); - let c = FixedTextBackend::new(text); - - let backends: Vec> = vec![a, b, c]; - let models = vec![ - moa::ModelEntry { - name: "worker-a-3b".into(), - backend_index: 0, - }, - moa::ModelEntry { - name: "worker-b-13b".into(), - backend_index: 1, - }, - moa::ModelEntry { - name: "worker-c-32b".into(), - backend_index: 2, - }, - ]; - - moa::GatewayConfig { - backends, - models, - worker_timeout: Duration::from_secs(2), - hedge_delay: Duration::from_millis(50), - reducer_timeout: Duration::from_secs(2), - first_answer_grace: Duration::ZERO, - strong_patience: Duration::ZERO, - enable_thinking: None, - } -} - -fn user_request_with_read_file_tool(content: &str) -> Value { - json!({ - "model": "mesh", - "tools": [{ - "type": "function", - "function": { - "name": "read_file", - "description": "Read a file", - "parameters": { - "type": "object", - "properties": {"path": {"type": "string"}}, - "required": ["path"], - } - } - }], - "messages": [{"role": "user", "content": content}], - "max_tokens": 128, - }) -} - -/// Helper: does the response body have a real, non-empty `tool_calls` -/// array? -fn has_tool_calls(body: &Value) -> bool { - body.pointer("/choices/0/message/tool_calls") - .and_then(|v| v.as_array()) - .map(|a| !a.is_empty()) - .unwrap_or(false) -} - -/// Helper: does the response signal an explicit failure (the all-workers- -/// fail or reducer-failed path)? -fn is_explicit_error(body: &Value) -> bool { - if body.get("error").is_some() { - return true; - } - body.pointer("/choices/0/finish_reason") - .and_then(|v| v.as_str()) - .map(|s| s == "error" || s == "moa_failed") - .unwrap_or(false) -} - -/// Helper: did the response just smuggle the worker prose back to the -/// agent as `content`? That's the failure shape from the PR review. -fn returned_prose_to_agent(body: &Value) -> bool { - body.pointer("/choices/0/message/content") - .and_then(|v| v.as_str()) - .map(|s| !s.is_empty()) - .unwrap_or(false) - && !has_tool_calls(body) - && !is_explicit_error(body) -} - -#[tokio::test] -async fn workers_describing_tool_call_must_emit_structured_tool_call() { - // Three workers ALL reply with the same agentic-prose. Today the - // heuristic classifier marks this as ToolProposal (good), but - // `extract_tool_proposal` has no JSON in the text to pull - // arguments from, so the gateway returns chat_response(payload) - // — i.e. the prose — instead of a tool_call_response. - let config = config_with_three_workers_returning( - "I'll use read_file to inspect README.md and report what it contains.", - ); - let body = user_request_with_read_file_tool("Read README.md and tell me what it says."); - - let result = moa::handle_turn(&config, &body).await; - let body = &result.response_body; - - assert!( - has_tool_calls(body) || is_explicit_error(body), - "tool-flavored worker prose must produce either a real tool_calls field \ - or a clear failure response. Returning the prose as plain content is the \ - agent-harness failure mode the PR review called out. \ - turn_kind={:?}, reducer_used={}, body={body}", - result.turn_kind, - result.reducer_used, - ); - - assert!( - !returned_prose_to_agent(body), - "must not smuggle prose to agent as `content` without `tool_calls`; body={body}" - ); -} - -#[tokio::test] -async fn workers_with_inline_tool_json_emit_real_tool_call() { - // Counterpart: when worker output IS structurally a tool proposal - // (JSON with function+arguments), MoA should emit a real - // `tool_calls`. This already works today; the test is here to - // pin the success case so the fix to the previous test doesn't - // regress the normal path. - let config = config_with_three_workers_returning( - r#"I'll read the README. {"function": "read_file", "arguments": {"path": "README.md"}}"#, - ); - let body = user_request_with_read_file_tool("Read README.md."); - - let result = moa::handle_turn(&config, &body).await; - let body = &result.response_body; - - assert!( - has_tool_calls(body), - "well-formed inline tool JSON must produce tool_calls; \ - turn_kind={:?}, body={body}", - result.turn_kind, - ); - let name = body - .pointer("/choices/0/message/tool_calls/0/function/name") - .and_then(|v| v.as_str()); - assert_eq!( - name, - Some("read_file"), - "tool_call function name must be the proposed tool" - ); -} diff --git a/crates/openai-frontend/src/guardrails/engine.rs b/crates/openai-frontend/src/guardrails/engine.rs index 2f3dea3015..e7ac15b923 100644 --- a/crates/openai-frontend/src/guardrails/engine.rs +++ b/crates/openai-frontend/src/guardrails/engine.rs @@ -9,13 +9,13 @@ use super::{ request_contract::{ GuardrailRequestContract, MeshGuardrailsOverride, ParallelToolCalls, RawToolChoice, }, - rescue::ClassifiedGuardrailResponse, state::{GuardrailRequestOutcome, GuardrailRequestState, PreparedGuardrailRequest}, telemetry::GuardrailTelemetryBypassReason, tools::{ append_mesh_emit_structured_tool, append_mesh_respond_tool, model_param_size_b, request_uses_reserved_tool_name, }, + validation::ClassifiedGuardrailResponse, }; #[derive(Debug, Clone)] @@ -117,7 +117,7 @@ impl GuardrailEngine { prepared: &PreparedGuardrailRequest, response: &crate::chat::ChatCompletionResponse, ) -> ClassifiedGuardrailResponse { - super::rescue::classify_response(prepared, response) + super::validation::classify_response(prepared, response) } fn guardrails_apply_to_request( diff --git a/crates/openai-frontend/src/guardrails/mod.rs b/crates/openai-frontend/src/guardrails/mod.rs index e6705b779f..0a050132e1 100644 --- a/crates/openai-frontend/src/guardrails/mod.rs +++ b/crates/openai-frontend/src/guardrails/mod.rs @@ -16,12 +16,12 @@ mod engine; mod errors; mod policy; mod request_contract; -mod rescue; mod retry; mod state; mod structured; mod telemetry; mod tools; +mod validation; pub use compact::CompactingOpenAiBackend; pub use mesh_llm_guardrails::{ @@ -41,7 +41,6 @@ use self::{ telemetry::{ GuardrailTelemetryAttemptBucket, GuardrailTelemetryBypassReason, GuardrailTelemetryContract, GuardrailTelemetryDecision, GuardrailTelemetryOutcome, - GuardrailTelemetryParserStage, }, }; @@ -89,7 +88,6 @@ impl GuardedOpenAiBackend { prepared.state.mode, telemetry_contract(&prepared.state.request_contract), GuardrailTelemetryOutcome::PassThrough, - Some(GuardrailTelemetryParserStage::None), None, ); self.backend.chat_completion(request).await @@ -112,24 +110,16 @@ impl GuardedOpenAiBackend { .chat_completion(attempt_request.clone()) .await?; let classified = engine.classify_response(&prepared, &response); - let parser_stage = telemetry_parser_stage(classified.parser_stage); let contract = telemetry_contract(&prepared.state.request_contract); let attempt_bucket = telemetry_attempt_bucket(attempt_index.saturating_add(1)); if let Some(sanitized) = retry::sanitize_success_response(&policy, &response, &classified) { - let outcome = if matches!(parser_stage, GuardrailTelemetryParserStage::None) - { - GuardrailTelemetryOutcome::Valid - } else { - GuardrailTelemetryOutcome::Rescued - }; self.record_outcome( prepared.state.mode, contract, - outcome, - Some(parser_stage), + GuardrailTelemetryOutcome::Valid, Some(attempt_bucket), ); return Ok(sanitized); @@ -140,7 +130,6 @@ impl GuardedOpenAiBackend { prepared.state.mode, contract, GuardrailTelemetryOutcome::MetricsOnlyFailure, - Some(parser_stage), Some(attempt_bucket), ); return Ok(response); @@ -152,7 +141,6 @@ impl GuardedOpenAiBackend { prepared.state.mode, contract, GuardrailTelemetryOutcome::Failed, - Some(parser_stage), Some(telemetry_attempt_bucket(attempt_index)), ); return retry::exhaustion_result(&policy, response, &classified); @@ -162,7 +150,6 @@ impl GuardedOpenAiBackend { prepared.state.mode, contract, GuardrailTelemetryOutcome::Retried, - Some(parser_stage), Some(telemetry_attempt_bucket(attempt_index)), ); @@ -181,12 +168,10 @@ impl GuardedOpenAiBackend { ) -> OpenAiResult { let response = self.backend.chat_completion(request).await?; let classified = engine.classify_response(prepared, &response); - let parser_stage = telemetry_parser_stage(classified.parser_stage); self.record_outcome( prepared.state.mode, telemetry_contract(&prepared.state.request_contract), - metrics_only_outcome(&classified, parser_stage), - Some(parser_stage), + metrics_only_outcome(&classified), Some(GuardrailTelemetryAttemptBucket::One), ); Ok(response) @@ -209,7 +194,6 @@ impl GuardedOpenAiBackend { mode: GuardrailMode, contract: Option<&'static str>, outcome: GuardrailTelemetryOutcome, - parser_stage: Option, attempt_bucket: Option, ) { if let Some(telemetry) = &self.telemetry { @@ -217,47 +201,30 @@ impl GuardedOpenAiBackend { mode, contract, outcome.as_str(), - parser_stage.map(GuardrailTelemetryParserStage::as_str), attempt_bucket.map(GuardrailTelemetryAttemptBucket::as_str), ); } } } -fn telemetry_parser_stage( - parser_stage: rescue::GuardrailParserStage, -) -> GuardrailTelemetryParserStage { - match parser_stage { - rescue::GuardrailParserStage::None => GuardrailTelemetryParserStage::None, - rescue::GuardrailParserStage::JsonExact => GuardrailTelemetryParserStage::JsonExact, - rescue::GuardrailParserStage::JsonFenced => GuardrailTelemetryParserStage::JsonFenced, - rescue::GuardrailParserStage::JsonSubstring => GuardrailTelemetryParserStage::JsonSubstring, - } -} - fn metrics_only_outcome( - classified: &rescue::ClassifiedGuardrailResponse, - parser_stage: GuardrailTelemetryParserStage, + classified: &validation::ClassifiedGuardrailResponse, ) -> GuardrailTelemetryOutcome { match classified.category { - rescue::GuardrailResponseCategory::ValidText - | rescue::GuardrailResponseCategory::ValidToolCalls - | rescue::GuardrailResponseCategory::ValidSyntheticRespond - | rescue::GuardrailResponseCategory::ValidSyntheticStructured => { - if matches!(parser_stage, GuardrailTelemetryParserStage::None) { - GuardrailTelemetryOutcome::Valid - } else { - GuardrailTelemetryOutcome::Rescued - } + validation::GuardrailResponseCategory::ValidText + | validation::GuardrailResponseCategory::ValidToolCalls + | validation::GuardrailResponseCategory::ValidSyntheticRespond + | validation::GuardrailResponseCategory::ValidSyntheticStructured => { + GuardrailTelemetryOutcome::Valid } - rescue::GuardrailResponseCategory::MalformedToolText - | rescue::GuardrailResponseCategory::UnknownTool - | rescue::GuardrailResponseCategory::InvalidToolArguments - | rescue::GuardrailResponseCategory::InvalidStructuredPayload - | rescue::GuardrailResponseCategory::MixedTerminalAndTool - | rescue::GuardrailResponseCategory::ToolCallsNotAllowed - | rescue::GuardrailResponseCategory::TooManyToolCalls - | rescue::GuardrailResponseCategory::EmptyOutput => { + validation::GuardrailResponseCategory::MalformedToolText + | validation::GuardrailResponseCategory::UnknownTool + | validation::GuardrailResponseCategory::InvalidToolArguments + | validation::GuardrailResponseCategory::InvalidStructuredPayload + | validation::GuardrailResponseCategory::MixedTerminalAndTool + | validation::GuardrailResponseCategory::ToolCallsNotAllowed + | validation::GuardrailResponseCategory::TooManyToolCalls + | validation::GuardrailResponseCategory::EmptyOutput => { GuardrailTelemetryOutcome::MetricsOnlyFailure } } diff --git a/crates/openai-frontend/src/guardrails/retry.rs b/crates/openai-frontend/src/guardrails/retry.rs index 0bb8c0c107..d351f26679 100644 --- a/crates/openai-frontend/src/guardrails/retry.rs +++ b/crates/openai-frontend/src/guardrails/retry.rs @@ -9,12 +9,9 @@ use super::{ errors::validation_failed_error, policy::{GuardrailPolicy, RetryExhaustionMode}, request_contract::RawToolChoice, - rescue::{ - ClassifiedGuardrailResponse, GuardrailParserStage, GuardrailResponseCategory, - strip_thinking_blocks, - }, state::{GuardrailRequestOutcome, PreparedGuardrailRequest}, tools::is_reserved_tool_name, + validation::{ClassifiedGuardrailResponse, GuardrailResponseCategory, strip_thinking_blocks}, }; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -49,19 +46,7 @@ pub(crate) fn sanitize_success_response( classified: &ClassifiedGuardrailResponse, ) -> Option { match classified.category { - GuardrailResponseCategory::ValidToolCalls => { - if classified.parser_stage == GuardrailParserStage::None { - Some(response.clone()) - } else { - Some(rewrite_response( - response, - None, - None, - classified.tool_calls.clone(), - Some(FinishReason::ToolCalls), - )) - } - } + GuardrailResponseCategory::ValidToolCalls => Some(response.clone()), GuardrailResponseCategory::ValidSyntheticRespond => Some(rewrite_response( response, None, diff --git a/crates/openai-frontend/src/guardrails/telemetry.rs b/crates/openai-frontend/src/guardrails/telemetry.rs index c5c56926f9..ee94e45629 100644 --- a/crates/openai-frontend/src/guardrails/telemetry.rs +++ b/crates/openai-frontend/src/guardrails/telemetry.rs @@ -48,7 +48,6 @@ impl GuardrailTelemetryBypassReason { pub(crate) enum GuardrailTelemetryOutcome { PassThrough, Valid, - Rescued, Retried, Failed, MetricsOnlyFailure, @@ -59,7 +58,6 @@ impl GuardrailTelemetryOutcome { match self { Self::PassThrough => "pass_through", Self::Valid => "valid", - Self::Rescued => "rescued", Self::Retried => "retried", Self::Failed => "failed", Self::MetricsOnlyFailure => "metrics_only_failure", @@ -67,25 +65,6 @@ impl GuardrailTelemetryOutcome { } } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum GuardrailTelemetryParserStage { - None, - JsonExact, - JsonFenced, - JsonSubstring, -} - -impl GuardrailTelemetryParserStage { - pub(crate) const fn as_str(self) -> &'static str { - match self { - Self::None => "none", - Self::JsonExact => "json_exact", - Self::JsonFenced => "json_fenced", - Self::JsonSubstring => "json_substring", - } - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum GuardrailTelemetryAttemptBucket { One, @@ -132,7 +111,6 @@ pub trait GuardrailTelemetrySink: Send + Sync + 'static { mode: GuardrailMode, contract: Option<&'static str>, outcome: &'static str, - parser_stage: Option<&'static str>, attempt_bucket: Option<&'static str>, ); } diff --git a/crates/openai-frontend/src/guardrails/tests.rs b/crates/openai-frontend/src/guardrails/tests.rs index 843f967be6..e4982e0eb1 100644 --- a/crates/openai-frontend/src/guardrails/tests.rs +++ b/crates/openai-frontend/src/guardrails/tests.rs @@ -21,15 +21,12 @@ use super::{ request_contract::{ MeshGuardrailsOverride, ParallelToolCalls, RawResponseFormat, RawToolChoice, RawToolSpec, }, - rescue::{ - ClassifiedGuardrailResponse, GuardrailParserStage, GuardrailResponseCategory, - strip_thinking_blocks, - }, telemetry::{ GuardrailTelemetryBypassReason, GuardrailTelemetryContract, GuardrailTelemetryDecision, - GuardrailTelemetryOutcome, GuardrailTelemetryParserStage, + GuardrailTelemetryOutcome, }, tools::{MESH_EMIT_STRUCTURED_TOOL_NAME, MESH_RESPOND_TOOL_NAME}, + validation::{ClassifiedGuardrailResponse, GuardrailResponseCategory}, }; #[derive(Default)] @@ -73,7 +70,6 @@ struct RecordedOutcome { mode: GuardrailMode, contract: Option<&'static str>, outcome: &'static str, - parser_stage: Option<&'static str>, attempt_bucket: Option<&'static str>, } @@ -98,14 +94,12 @@ impl GuardrailTelemetrySink for RecordingTelemetrySink { mode: GuardrailMode, contract: Option<&'static str>, outcome: &'static str, - parser_stage: Option<&'static str>, attempt_bucket: Option<&'static str>, ) { self.outcomes.lock().unwrap().push(RecordedOutcome { mode, contract, outcome, - parser_stage, attempt_bucket, }); } @@ -1228,10 +1222,6 @@ fn telemetry_response_records_use_bounded_enums_only() { GuardrailTelemetryOutcome::MetricsOnlyFailure.as_str(), "metrics_only_failure" ); - assert_eq!( - GuardrailTelemetryParserStage::JsonFenced.as_str(), - "json_fenced" - ); assert_eq!(telemetry_attempt_bucket(3).as_str(), "3_plus"); } @@ -1416,17 +1406,6 @@ fn tool_call_name(classified: &ClassifiedGuardrailResponse) -> Option<&str> { .as_str() } -fn tool_call_arguments(classified: &ClassifiedGuardrailResponse) -> Option<&str> { - classified - .tool_calls - .as_ref()? - .as_array()? - .first()? - .get("function")? - .get("arguments")? - .as_str() -} - fn supported_json_schema_response_format() -> serde_json::Value { json!({ "type": "json_schema", diff --git a/crates/openai-frontend/src/guardrails/tests/response_validation.rs b/crates/openai-frontend/src/guardrails/tests/response_validation.rs index a004678340..caacad026c 100644 --- a/crates/openai-frontend/src/guardrails/tests/response_validation.rs +++ b/crates/openai-frontend/src/guardrails/tests/response_validation.rs @@ -1,53 +1,7 @@ use super::*; #[test] -fn strips_thinking_blocks_before_rescue_attempts() { - assert_eq!( - strip_thinking_blocks( - "private plan```json\n{\"name\":\"lookup\",\"arguments\":{\"city\":\"Sydney\"}}\n```" - ), - "```json\n{\"name\":\"lookup\",\"arguments\":{\"city\":\"Sydney\"}}\n```" - ); - assert_eq!( - strip_thinking_blocks("[THINK]hidden[/THINK]Visible answer"), - "Visible answer" - ); -} - -#[test] -fn rescues_plain_json_tool_call_text() { - let engine = GuardrailEngine::new(enforce_policy()); - let prepared = prepared_tool_request( - &engine, - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "weather"}], - "tools": [{"type": "function", "function": {"name": "lookup"}}], - "tool_choice": "auto" - }), - ); - let response = response_with_content( - "Qwen3-8B-Q4_K_M", - r#"{"name":"lookup","arguments":{"city":"Sydney"}}"#, - ); - - let classified = engine.classify_response(&prepared, &response); - - assert_eq!( - classified.category, - GuardrailResponseCategory::ValidToolCalls - ); - assert_eq!(classified.parser_stage, GuardrailParserStage::JsonExact); - assert_eq!(classified.visible_content, None); - assert_eq!(tool_call_name(&classified), Some("lookup")); - assert_eq!( - tool_call_arguments(&classified), - Some(r#"{"city":"Sydney"}"#) - ); -} - -#[test] -fn rescues_json_tool_call_array_text() { +fn text_form_tool_shapes_are_not_parsed() { let engine = GuardrailEngine::new(enforce_policy()); let prepared = prepared_tool_request( &engine, @@ -57,235 +11,23 @@ fn rescues_json_tool_call_array_text() { "tools": [{"type": "function", "function": {"name": "lookup"}}] }), ); - let response = response_with_content( - "Qwen3-8B-Q4_K_M", - r#"[{"type":"function","function":{"name":"lookup","arguments":{"city":"Sydney"}}}]"#, - ); - - let classified = engine.classify_response(&prepared, &response); - assert_eq!( - classified.category, - GuardrailResponseCategory::ValidToolCalls - ); - assert_eq!(tool_call_name(&classified), Some("lookup")); -} - -#[test] -fn rescues_fenced_json_tool_call_text() { - let engine = GuardrailEngine::new(enforce_policy()); - let prepared = prepared_tool_request( - &engine, - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "weather"}], - "tools": [{"type": "function", "function": {"name": "lookup"}}] - }), - ); - let response = response_with_content( - "Qwen3-8B-Q4_K_M", - "```json\n{\"name\":\"lookup\",\"arguments\":{\"city\":\"Sydney\"}}\n```", - ); - - let classified = engine.classify_response(&prepared, &response); - - assert_eq!( - classified.category, - GuardrailResponseCategory::ValidToolCalls - ); - assert_eq!(classified.parser_stage, GuardrailParserStage::JsonFenced); -} - -#[test] -fn rescues_brace_balanced_json_substring_only_for_allowed_tools() { - let engine = GuardrailEngine::new(enforce_policy()); - let prepared = prepared_tool_request( - &engine, - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "weather"}], - "tools": [{"type": "function", "function": {"name": "lookup"}}] - }), - ); - let response = response_with_content( - "Qwen3-8B-Q4_K_M", - "I'll call this now: {\"name\":\"lookup\",\"arguments\":{\"city\":\"Sydney\"}}", - ); - - let classified = engine.classify_response(&prepared, &response); - - assert_eq!( - classified.category, - GuardrailResponseCategory::ValidToolCalls - ); - assert_eq!(tool_call_name(&classified), Some("lookup")); -} - -#[test] -fn arbitrary_json_without_allowed_tool_name_is_not_rescued() { - let engine = GuardrailEngine::new(enforce_policy()); - let prepared = prepared_tool_request( - &engine, - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "weather"}], - "tools": [{"type": "function", "function": {"name": "lookup"}}] - }), - ); - let response = response_with_content("Qwen3-8B-Q4_K_M", r#"{"payload":{"city":"Sydney"}}"#); - - let classified = engine.classify_response(&prepared, &response); - - assert_eq!( - classified.category, - GuardrailResponseCategory::MalformedToolText - ); - assert!(classified.tool_calls.is_none()); -} - -#[test] -fn rescues_bracket_args_tool_syntax() { - let engine = GuardrailEngine::new(enforce_policy()); - let prepared = prepared_tool_request( - &engine, - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "weather"}], - "tools": [{"type": "function", "function": {"name": "lookup"}}] - }), - ); - let response = response_with_content("Qwen3-8B-Q4_K_M", "lookup[ARGS]{\"city\":\"Sydney\"}"); - - let classified = engine.classify_response(&prepared, &response); - - assert_eq!( - classified.category, - GuardrailResponseCategory::ValidToolCalls - ); - assert_eq!(classified.parser_stage, GuardrailParserStage::JsonSubstring); - assert_eq!(tool_call_name(&classified), Some("lookup")); -} - -#[test] -fn rescues_qwen_xml_tool_syntax() { - let engine = GuardrailEngine::new(enforce_policy()); - let prepared = prepared_tool_request( - &engine, - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "weather"}], - "tools": [{"type": "function", "function": {"name": "lookup"}}] - }), - ); - let response = response_with_content( - "Qwen3-8B-Q4_K_M", - "Sydney", - ); - - let classified = engine.classify_response(&prepared, &response); - - assert_eq!( - classified.category, - GuardrailResponseCategory::ValidToolCalls - ); - assert_eq!(classified.parser_stage, GuardrailParserStage::JsonSubstring); - assert_eq!(tool_call_name(&classified), Some("lookup")); - assert_eq!( - tool_call_arguments(&classified), - Some(r#"{"city":"Sydney"}"#) - ); -} - -#[test] -fn rescues_granite_tool_call_syntax() { - let engine = GuardrailEngine::new(enforce_policy()); - let prepared = prepared_tool_request( - &engine, - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "weather"}], - "tools": [{"type": "function", "function": {"name": "lookup"}}] - }), - ); - let response = response_with_content( - "Qwen3-8B-Q4_K_M", - "{\"name\":\"lookup\",\"arguments\":{\"city\":\"Sydney\"}}", - ); - - let classified = engine.classify_response(&prepared, &response); - - assert_eq!( - classified.category, - GuardrailResponseCategory::ValidToolCalls - ); - assert_eq!(tool_call_name(&classified), Some("lookup")); -} - -#[test] -fn rescue_strips_hidden_reasoning_from_client_visible_content() { - let engine = GuardrailEngine::new(enforce_policy()); - let prepared = prepared_text_request( - &engine, - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "hello"}] - }), - ); - let response = response_with_content( - "Qwen3-8B-Q4_K_M", - "private reasoningHello there", - ); - - let classified = engine.classify_response(&prepared, &response); - - assert_eq!(classified.category, GuardrailResponseCategory::ValidText); - assert_eq!(classified.visible_content.as_deref(), Some("Hello there")); -} - -#[test] -fn unknown_tool_text_classifies_for_retry() { - let engine = GuardrailEngine::new(enforce_policy()); - let prepared = prepared_tool_request( - &engine, - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "weather"}], - "tools": [{"type": "function", "function": {"name": "lookup"}}] - }), - ); - let response = response_with_content( - "Qwen3-8B-Q4_K_M", - r#"{"name":"other_tool","arguments":{"city":"Sydney"}}"#, - ); - - let classified = engine.classify_response(&prepared, &response); - - assert_eq!(classified.category, GuardrailResponseCategory::UnknownTool); - assert!(classified.tool_calls.is_none()); -} - -#[test] -fn malformed_arguments_classify_without_panicking() { - let engine = GuardrailEngine::new(enforce_policy()); - let prepared = prepared_tool_request( - &engine, - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "weather"}], - "tools": [{"type": "function", "function": {"name": "lookup"}}] - }), - ); - let response = response_with_content( - "Qwen3-8B-Q4_K_M", - r#"{"name":"lookup","arguments":"not-json"}"#, - ); - - let classified = engine.classify_response(&prepared, &response); - - assert_eq!( - classified.category, - GuardrailResponseCategory::InvalidToolArguments - ); + for content in [ + r#"lookup[ARGS]{"city":"Sydney"}"#, + r#"lookup({"city":"Sydney"})"#, + r#"Sydney"#, + r#"```json +{"name":"lookup","arguments":{"city":"Sydney"}} +```"#, + ] { + let classified = + engine.classify_response(&prepared, &response_with_content("model", content)); + assert_eq!( + classified.category, + GuardrailResponseCategory::MalformedToolText + ); + assert!(classified.tool_calls.is_none()); + } } #[test] @@ -318,7 +60,6 @@ fn existing_valid_tool_calls_are_classified() { classified.category, GuardrailResponseCategory::ValidToolCalls ); - assert_eq!(classified.parser_stage, GuardrailParserStage::None); } #[test] @@ -386,52 +127,6 @@ fn synthetic_structured_classifies_when_allowed() { assert_eq!(classified.structured_payload, Some(json!({"answer": 42}))); } -#[test] -fn direct_structured_json_object_classifies_when_allowed() { - let engine = GuardrailEngine::new(enforce_policy()); - let prepared = prepared_text_request( - &engine, - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "json"}], - "response_format": supported_json_schema_response_format() - }), - ); - let response = response_with_content("Qwen3-8B-Q4_K_M", r#"{"answer":42}"#); - - let classified = engine.classify_response(&prepared, &response); - - assert_eq!( - classified.category, - GuardrailResponseCategory::ValidSyntheticStructured - ); - assert_eq!(classified.parser_stage, GuardrailParserStage::JsonExact); - assert_eq!(classified.structured_payload, Some(json!({"answer": 42}))); -} - -#[test] -fn fenced_direct_structured_json_object_classifies_when_allowed() { - let engine = GuardrailEngine::new(enforce_policy()); - let prepared = prepared_text_request( - &engine, - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "json"}], - "response_format": supported_json_schema_response_format() - }), - ); - let response = response_with_content("Qwen3-8B-Q4_K_M", "```json\n{\"answer\":42}\n```"); - - let classified = engine.classify_response(&prepared, &response); - - assert_eq!( - classified.category, - GuardrailResponseCategory::ValidSyntheticStructured - ); - assert_eq!(classified.parser_stage, GuardrailParserStage::JsonFenced); - assert_eq!(classified.structured_payload, Some(json!({"answer": 42}))); -} - #[test] fn invalid_structured_payload_classifies_without_leaking_arguments() { let engine = GuardrailEngine::new(enforce_policy()); @@ -663,7 +358,7 @@ async fn malformed_tool_arguments_retry_once_then_succeed() { .expect("retry content exists"), ) .expect("retry text exists"); - assert!(retry_text.contains("invalid JSON tool arguments")); + assert!(retry_text.contains("plain text instead of a valid guarded call")); assert!(retry_text.contains("Do not add extra text.")); } @@ -824,41 +519,6 @@ async fn pass_last_text_rejects_sentinel_leaking_text_without_safe_fallback() { assert_eq!(body.error.message, GUARDRAIL_VALIDATION_FAILED_MESSAGE); } -#[tokio::test] -async fn mesh_respond_stripped_to_assistant_text() { - let backend = Arc::new(SequencedBackend::new(vec![Ok(response_with_content( - "Qwen3-8B-Q4_K_M", - r#"_mesh_respond({"message":"Hello there"})"#, - ))])); - let guarded = GuardedOpenAiBackend::new( - backend, - GuardrailPolicy { - mode: GuardrailMode::Enforce, - apply_to_all_models: true, - ..GuardrailPolicy::default() - }, - ); - let request: ChatCompletionRequest = serde_json::from_value(json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "weather"}], - "tools": [{"type": "function", "function": {"name": "lookup"}}], - "tool_choice": "auto" - })) - .unwrap(); - - let response = guarded.chat_completion(request).await.unwrap(); - - assert_eq!( - response.choices[0].message.content.as_deref(), - Some("Hello there") - ); - assert!(response.choices[0].message.tool_calls.is_none()); - assert_eq!( - response.choices[0].finish_reason, - Some(crate::common::FinishReason::Stop) - ); -} - #[tokio::test] async fn mixed_mesh_respond_plus_real_tool_calls_retry_exhaustion_handling() { let invalid = response_with_tool_calls( @@ -1065,40 +725,6 @@ async fn valid_structured_payload_becomes_json_assistant_text() { ); } -#[tokio::test] -async fn direct_structured_payload_becomes_json_assistant_text() { - let backend = Arc::new(SequencedBackend::new(vec![Ok(response_with_content( - "Qwen3-8B-Q4_K_M", - "```json\n{\"answer\":42}\n```", - ))])); - let guarded = GuardedOpenAiBackend::new( - backend, - GuardrailPolicy { - mode: GuardrailMode::Enforce, - apply_to_all_models: true, - ..GuardrailPolicy::default() - }, - ); - let request: ChatCompletionRequest = serde_json::from_value(json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "json"}], - "response_format": supported_json_schema_response_format() - })) - .unwrap(); - - let response = guarded.chat_completion(request).await.unwrap(); - - assert_eq!( - response.choices[0].message.content.as_deref(), - Some("{\"answer\":42}") - ); - assert!(response.choices[0].message.tool_calls.is_none()); - assert_eq!( - response.choices[0].finish_reason, - Some(crate::common::FinishReason::Stop) - ); -} - #[tokio::test] async fn invalid_structured_payload_retries_then_exhaustion_error() { let backend = Arc::new(SequencedBackend::new(vec![ @@ -1231,7 +857,6 @@ async fn metrics_only_failed_validation_returns_original_backend_response() { assert_eq!(response, original); assert!(telemetry.outcomes.lock().unwrap().iter().any(|record| { record.outcome == GuardrailTelemetryOutcome::MetricsOnlyFailure.as_str() - && record.parser_stage == Some(GuardrailTelemetryParserStage::JsonExact.as_str()) })); } @@ -1272,6 +897,5 @@ async fn metrics_only_eligible_tool_request_does_not_rewrite_or_sanitize() { })); assert!(telemetry.outcomes.lock().unwrap().iter().any(|record| { record.outcome == GuardrailTelemetryOutcome::MetricsOnlyFailure.as_str() - && record.parser_stage == Some(GuardrailTelemetryParserStage::None.as_str()) })); } diff --git a/crates/openai-frontend/src/guardrails/rescue.rs b/crates/openai-frontend/src/guardrails/validation.rs similarity index 57% rename from crates/openai-frontend/src/guardrails/rescue.rs rename to crates/openai-frontend/src/guardrails/validation.rs index 91b4cf79ea..14d3da9039 100644 --- a/crates/openai-frontend/src/guardrails/rescue.rs +++ b/crates/openai-frontend/src/guardrails/validation.rs @@ -1,5 +1,6 @@ use std::collections::BTreeSet; +pub(crate) use mesh_llm_guardrails::strip_thinking_blocks; use serde_json::{Map, Value, json}; use crate::{chat::ChatCompletionResponse, common::FinishReason}; @@ -10,9 +11,6 @@ use super::{ tools::{MESH_EMIT_STRUCTURED_TOOL_NAME, MESH_RESPOND_TOOL_NAME}, }; -const MAX_RESCUE_INPUT_BYTES: usize = 64 * 1024; -const MAX_JSON_CANDIDATES: usize = 32; - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum GuardrailResponseCategory { ValidText, @@ -29,18 +27,9 @@ pub(crate) enum GuardrailResponseCategory { EmptyOutput, } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum GuardrailParserStage { - None, - JsonExact, - JsonFenced, - JsonSubstring, -} - #[derive(Debug, Clone, PartialEq)] pub(crate) struct ClassifiedGuardrailResponse { pub category: GuardrailResponseCategory, - pub parser_stage: GuardrailParserStage, pub visible_content: Option, pub tool_calls: Option, pub synthetic_text: Option, @@ -70,7 +59,6 @@ pub(crate) fn classify_response( if visible_content.is_some() { return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::MixedTerminalAndTool, - parser_stage: GuardrailParserStage::None, visible_content, tool_calls: Some(tool_calls.clone()), synthetic_text: None, @@ -78,26 +66,16 @@ pub(crate) fn classify_response( finish_reason, }; } - return classify_tool_call_value( - prepared, - tool_calls, - GuardrailParserStage::None, - finish_reason, - ); + return classify_tool_call_value(prepared, tool_calls, finish_reason); } if visible_content.is_none() { return empty_output(); } - if let Some(classified) = rescue_from_text(prepared, &stripped, finish_reason) { - return classified; - } - if request_expects_guarded_contract(prepared) { return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::MalformedToolText, - parser_stage: GuardrailParserStage::None, visible_content: None, tool_calls: None, synthetic_text: None, @@ -108,7 +86,6 @@ pub(crate) fn classify_response( ClassifiedGuardrailResponse { category: GuardrailResponseCategory::ValidText, - parser_stage: GuardrailParserStage::None, visible_content, tool_calls: None, synthetic_text: None, @@ -117,344 +94,12 @@ pub(crate) fn classify_response( } } -pub(crate) fn strip_thinking_blocks(content: &str) -> String { - let stripped_html = strip_tag_pairs(content, "", ""); - let stripped_brackets = strip_tag_pairs(&stripped_html, "[THINK]", "[/THINK]"); - stripped_brackets.trim().to_string() -} - -fn strip_tag_pairs(content: &str, start_tag: &str, end_tag: &str) -> String { - let mut remainder = content; - let mut result = String::new(); - while let Some(start_index) = remainder.find(start_tag) { - result.push_str(&remainder[..start_index]); - let after_start = &remainder[start_index + start_tag.len()..]; - if let Some(end_index) = after_start.find(end_tag) { - remainder = &after_start[end_index + end_tag.len()..]; - } else { - remainder = &remainder[..start_index]; - break; - } - } - result.push_str(remainder); - result -} - -fn rescue_from_text( - prepared: &PreparedGuardrailRequest, - content: &str, - finish_reason: Option, -) -> Option { - for json_candidate in openai_json_candidates(content) { - if let Ok(value) = serde_json::from_str::(&json_candidate.content) { - let classified = - classify_tool_call_value(prepared, &value, json_candidate.stage, finish_reason); - if classified.category != GuardrailResponseCategory::MalformedToolText { - return Some(classified.without_visible_content()); - } - } - } - - if let Some(value) = parse_bracket_args_tool_syntax(content) { - let classified = classify_tool_call_value( - prepared, - &value, - GuardrailParserStage::JsonSubstring, - finish_reason, - ); - if classified.category != GuardrailResponseCategory::MalformedToolText { - return Some(classified.without_visible_content()); - } - } - - if let Some(value) = parse_qwen_xml_syntax(content) { - let classified = classify_tool_call_value( - prepared, - &value, - GuardrailParserStage::JsonSubstring, - finish_reason, - ); - if classified.category != GuardrailResponseCategory::MalformedToolText { - return Some(classified.without_visible_content()); - } - } - - if let Some(value) = parse_granite_tool_call_syntax(content) { - let classified = classify_tool_call_value( - prepared, - &value, - GuardrailParserStage::JsonSubstring, - finish_reason, - ); - if classified.category != GuardrailResponseCategory::MalformedToolText { - return Some(classified.without_visible_content()); - } - } - - None -} - -struct JsonCandidate { - content: String, - stage: GuardrailParserStage, -} - -fn openai_json_candidates(content: &str) -> Vec { - let content = bounded_prefix(content, MAX_RESCUE_INPUT_BYTES); - let mut candidates = Vec::new(); - push_candidate( - &mut candidates, - content.trim(), - GuardrailParserStage::JsonExact, - ); - - for fenced in fenced_code_blocks(content) { - if candidates.len() >= MAX_JSON_CANDIDATES { - break; - } - push_candidate( - &mut candidates, - fenced.trim(), - GuardrailParserStage::JsonFenced, - ); - } - - for balanced in balanced_json_substrings(content) { - if candidates.len() >= MAX_JSON_CANDIDATES { - break; - } - push_candidate( - &mut candidates, - balanced.trim(), - GuardrailParserStage::JsonSubstring, - ); - } - - candidates -} - -fn bounded_prefix(content: &str, max_bytes: usize) -> &str { - if content.len() <= max_bytes { - return content; - } - - let mut end = max_bytes; - while end > 0 && !content.is_char_boundary(end) { - end -= 1; - } - &content[..end] -} - -fn push_candidate( - candidates: &mut Vec, - candidate: &str, - stage: GuardrailParserStage, -) { - if candidate.is_empty() { - return; - } - if !candidates - .iter() - .any(|existing| existing.content == candidate) - { - candidates.push(JsonCandidate { - content: candidate.to_string(), - stage, - }); - } -} - -fn fenced_code_blocks(content: &str) -> Vec { - let mut blocks = Vec::new(); - let mut remainder = content; - while let Some(open_index) = remainder.find("```") { - let after_open = &remainder[open_index + 3..]; - let Some(close_index) = after_open.find("```") else { - break; - }; - let block = &after_open[..close_index]; - let block = block - .strip_prefix("json\n") - .or_else(|| block.strip_prefix("JSON\n")) - .unwrap_or(block); - blocks.push(block.to_string()); - remainder = &after_open[close_index + 3..]; - } - blocks -} - -fn balanced_json_substrings(content: &str) -> Vec { - let bytes = content.as_bytes(); - let mut candidates = Vec::new(); - for (index, byte) in bytes.iter().enumerate() { - if candidates.len() >= MAX_JSON_CANDIDATES { - break; - } - let closing = match byte { - b'{' => b'}', - b'[' => b']', - _ => continue, - }; - if let Some(end) = balanced_substring_end(bytes, index, *byte, closing) { - candidates.push(content[index..=end].to_string()); - } - } - candidates -} - -fn balanced_substring_end(bytes: &[u8], start: usize, opening: u8, closing: u8) -> Option { - let mut depth = 0_u32; - let mut in_string = false; - let mut escaped = false; - for (index, byte) in bytes.iter().copied().enumerate().skip(start) { - if in_string { - if escaped { - escaped = false; - continue; - } - match byte { - b'\\' => escaped = true, - b'"' => in_string = false, - _ => {} - } - continue; - } - match byte { - b'"' => in_string = true, - _ if byte == opening => depth += 1, - _ if byte == closing => { - depth = depth.saturating_sub(1); - if depth == 0 { - return Some(index); - } - } - _ => {} - } - } - None -} - -fn parse_bracket_args_tool_syntax(content: &str) -> Option { - let marker = "[ARGS]"; - if let Some(marker_index) = content.find(marker) { - let name = content[..marker_index] - .trim() - .rsplit(|character: char| { - !character.is_ascii_alphanumeric() && character != '_' && character != '-' - }) - .next()? - .trim(); - if name.is_empty() { - return None; - } - let after_marker = content[marker_index + marker.len()..].trim_start(); - let json_text = first_balanced_object(after_marker)?; - let arguments = serde_json::from_str::(&json_text).ok()?; - return Some(json!({ - "name": name, - "arguments": arguments, - })); - } - - parse_parenthesized_tool_call(content) -} - -fn parse_qwen_xml_syntax(content: &str) -> Option { - let function_prefix = "')?; - let name = after_prefix[..name_end] - .trim() - .trim_matches('"') - .trim_matches('\''); - if name.is_empty() { - return None; - } - let body = &after_prefix[name_end + 1..]; - let function_end = body.find("")?; - let parameters_body = &body[..function_end]; - let mut arguments = Map::new(); - let mut remainder = parameters_body; - - while let Some(parameter_start) = remainder.find("")?; - let value = parameter_body[..value_end].trim(); - let parsed_value = serde_json::from_str::(value) - .unwrap_or_else(|_| Value::String(value.to_string())); - arguments.insert(parameter_name.to_string(), parsed_value); - remainder = ¶meter_body[value_end + "".len()..]; - } - - if arguments.is_empty() { - return None; - } - - Some(json!({ - "name": name, - "arguments": Value::Object(arguments), - })) -} - -fn parse_granite_tool_call_syntax(content: &str) -> Option { - let start_tag = ""; - let end_tag = ""; - let start_index = content.find(start_tag)?; - let after_start = &content[start_index + start_tag.len()..]; - let end_index = after_start.find(end_tag)?; - serde_json::from_str(after_start[..end_index].trim()).ok() -} - -fn first_balanced_object(content: &str) -> Option { - let start = content.find('{')?; - let end = balanced_substring_end(content.as_bytes(), start, b'{', b'}')?; - Some(content[start..=end].to_string()) -} - -fn parse_parenthesized_tool_call(content: &str) -> Option { - let open_paren = content.find('(')?; - let name = content[..open_paren] - .trim() - .rsplit(|character: char| { - !character.is_ascii_alphanumeric() && character != '_' && character != '-' - }) - .next()? - .trim(); - if name.is_empty() { - return None; - } - let after_open = content[open_paren + 1..].trim_start(); - let json_text = first_balanced_object(after_open)?; - let after_json = after_open[json_text.len()..].trim_start(); - if !after_json.starts_with(')') { - return None; - } - let arguments = serde_json::from_str::(&json_text).ok()?; - Some(json!({ - "name": name, - "arguments": arguments, - })) -} - fn classify_tool_call_value( prepared: &PreparedGuardrailRequest, value: &Value, - parser_stage: GuardrailParserStage, finish_reason: Option, ) -> ClassifiedGuardrailResponse { - if let Some(classified) = - classify_direct_structured_payload(prepared, value, parser_stage, finish_reason) - { + if let Some(classified) = classify_direct_structured_payload(prepared, value, finish_reason) { return classified; } @@ -465,7 +110,6 @@ fn classify_tool_call_value( _ => { return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::MalformedToolText, - parser_stage, visible_content: None, tool_calls: None, synthetic_text: None, @@ -482,7 +126,6 @@ fn classify_tool_call_value( ParsedToolCallStatus::UnknownTool => { return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::UnknownTool, - parser_stage, visible_content: None, tool_calls: None, synthetic_text: None, @@ -497,7 +140,6 @@ fn classify_tool_call_value( } else { GuardrailResponseCategory::InvalidToolArguments }, - parser_stage, visible_content: None, tool_calls: None, synthetic_text: None, @@ -508,7 +150,6 @@ fn classify_tool_call_value( ParsedToolCallStatus::Malformed => { return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::MalformedToolText, - parser_stage, visible_content: None, tool_calls: None, synthetic_text: None, @@ -532,7 +173,6 @@ fn classify_tool_call_value( if request_disables_tool_calls(prepared) { return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::ToolCallsNotAllowed, - parser_stage, visible_content: None, tool_calls: Some(normalized_tool_calls(&parsed_calls)), synthetic_text: None, @@ -548,7 +188,6 @@ fn classify_tool_call_value( { return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::UnknownTool, - parser_stage, visible_content: None, tool_calls: Some(normalized_tool_calls(&parsed_calls)), synthetic_text: None, @@ -564,7 +203,6 @@ fn classify_tool_call_value( { return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::TooManyToolCalls, - parser_stage, visible_content: None, tool_calls: Some(normalized_tool_calls(&parsed_calls)), synthetic_text: None, @@ -578,7 +216,6 @@ fn classify_tool_call_value( { return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::MixedTerminalAndTool, - parser_stage, visible_content: None, tool_calls: Some(normalized_tool_calls(&parsed_calls)), synthetic_text: None, @@ -591,7 +228,6 @@ fn classify_tool_call_value( if parsed_calls.len() != 1 { return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::MixedTerminalAndTool, - parser_stage, visible_content: None, tool_calls: Some(normalized_tool_calls(&parsed_calls)), synthetic_text: None, @@ -603,7 +239,6 @@ fn classify_tool_call_value( let Some(message) = tool_call.arguments.get("message").and_then(Value::as_str) else { return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::InvalidToolArguments, - parser_stage, visible_content: None, tool_calls: None, synthetic_text: None, @@ -613,7 +248,6 @@ fn classify_tool_call_value( }; return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::ValidSyntheticRespond, - parser_stage, visible_content: None, tool_calls: Some(normalized_tool_calls(&parsed_calls)), synthetic_text: Some(message.to_string()), @@ -626,7 +260,6 @@ fn classify_tool_call_value( if parsed_calls.len() != 1 { return ClassifiedGuardrailResponse { category: GuardrailResponseCategory::MixedTerminalAndTool, - parser_stage, visible_content: None, tool_calls: Some(normalized_tool_calls(&parsed_calls)), synthetic_text: None, @@ -646,7 +279,6 @@ fn classify_tool_call_value( } else { GuardrailResponseCategory::InvalidStructuredPayload }, - parser_stage, visible_content: None, tool_calls: if valid_payload { Some(normalized_tool_calls(&parsed_calls)) @@ -669,7 +301,6 @@ fn classify_tool_call_value( ClassifiedGuardrailResponse { category: GuardrailResponseCategory::ValidToolCalls, - parser_stage, visible_content: None, tool_calls: Some(normalized_tool_calls(&parsed_calls)), synthetic_text: None, @@ -681,7 +312,6 @@ fn classify_tool_call_value( fn classify_direct_structured_payload( prepared: &PreparedGuardrailRequest, value: &Value, - parser_stage: GuardrailParserStage, finish_reason: Option, ) -> Option { if prepared.state.request_contract.has_real_tools() { @@ -696,7 +326,6 @@ fn classify_direct_structured_payload( } else { GuardrailResponseCategory::InvalidStructuredPayload }, - parser_stage, visible_content: None, tool_calls: None, synthetic_text: None, @@ -839,7 +468,6 @@ fn normalized_visible_content(content: &str) -> Option { fn empty_output() -> ClassifiedGuardrailResponse { ClassifiedGuardrailResponse { category: GuardrailResponseCategory::EmptyOutput, - parser_stage: GuardrailParserStage::None, visible_content: None, tool_calls: None, synthetic_text: None, @@ -847,10 +475,3 @@ fn empty_output() -> ClassifiedGuardrailResponse { finish_reason: None, } } - -impl ClassifiedGuardrailResponse { - fn without_visible_content(mut self) -> Self { - self.visible_content = None; - self - } -} diff --git a/crates/openai-frontend/src/router.rs b/crates/openai-frontend/src/router.rs index 2085ac434e..1635904865 100644 --- a/crates/openai-frontend/src/router.rs +++ b/crates/openai-frontend/src/router.rs @@ -579,7 +579,7 @@ mod tests { use super::*; use crate::{ - FinishReason, GuardedOpenAiBackend, GuardrailMode, GuardrailPolicy, + FinishReason, backend::{ CancellationToken, ChatCompletionStream, CompletionStream, OpenAiRequestContext, OpenAiResult, @@ -780,11 +780,6 @@ mod tests { token: Arc>>, } - #[derive(Default)] - struct GuardrailRescueBackend { - seen_chat_requests: Arc>>, - } - #[async_trait] impl OpenAiBackend for CancellationBackend { async fn models(&self) -> OpenAiResult> { @@ -808,51 +803,6 @@ mod tests { } } - #[async_trait] - impl OpenAiBackend for GuardrailRescueBackend { - async fn models(&self) -> OpenAiResult> { - Ok(vec![ModelObject::new("Qwen3-8B-Q4_K_M")]) - } - - async fn chat_completion( - &self, - request: ChatCompletionRequest, - ) -> OpenAiResult { - self.seen_chat_requests - .lock() - .unwrap() - .push(request.clone()); - Ok(ChatCompletionResponse::new( - request.model, - r#"lookup[ARGS]{"city":"Sydney"}"#, - Usage::new(8, 3), - )) - } - - async fn chat_completion_stream( - &self, - _request: ChatCompletionRequest, - _context: OpenAiRequestContext, - ) -> OpenAiResult { - unreachable!("guardrail rescue backend test only calls non-stream chat") - } - - async fn completion( - &self, - _request: CompletionRequest, - ) -> OpenAiResult { - unreachable!("guardrail rescue backend test only calls chat") - } - - async fn completion_stream( - &self, - _request: CompletionRequest, - _context: OpenAiRequestContext, - ) -> OpenAiResult { - unreachable!("guardrail rescue backend test only calls chat") - } - } - #[test] fn messages_to_plain_prompt_extracts_text_parts() { let messages = vec![ @@ -1380,53 +1330,6 @@ mod tests { ); } - #[tokio::test] - async fn guarded_chat_rescues_tool_call_text() { - let backend = Arc::new(GuardrailRescueBackend::default()); - let app = guarded_test_app(backend.clone()); - - let response = post_json_with_app_and_request_id( - app, - "/v1/chat/completions", - json!({ - "model": "Qwen3-8B-Q4_K_M", - "messages": [{"role": "user", "content": "weather"}], - "tools": [{"type": "function", "function": {"name": "lookup"}}], - "tool_choice": "auto" - }), - Some("guarded-chat-req"), - ) - .await; - - assert_eq!(response.status(), StatusCode::OK); - assert_eq!(response.headers()["x-request-id"], "guarded-chat-req"); - let body = response_body_json(response).await; - assert_eq!(body["object"], "chat.completion"); - assert!(body["choices"][0]["message"]["content"].is_null()); - assert_eq!(body["choices"][0]["finish_reason"], "tool_calls"); - assert_eq!( - body["choices"][0]["message"]["tool_calls"][0]["function"]["name"], - "lookup" - ); - assert_eq!( - body["choices"][0]["message"]["tool_calls"][0]["function"]["arguments"], - "{\"city\":\"Sydney\"}" - ); - assert!(!serde_json::to_string(&body).unwrap().contains("_mesh_")); - - let seen_requests = backend.seen_chat_requests.lock().unwrap(); - assert_eq!(seen_requests.len(), 1); - let seen_tools = seen_requests[0] - .tools - .as_ref() - .and_then(Value::as_array) - .cloned() - .expect("guarded backend should receive tools"); - assert_eq!(seen_tools.len(), 2); - assert_eq!(seen_tools[0]["function"]["name"], "lookup"); - assert_eq!(seen_tools[1]["function"]["name"], "_mesh_respond"); - } - #[tokio::test] async fn responses_stream_route_returns_responses_sse() { let response = post_json( @@ -1652,21 +1555,6 @@ mod tests { assert_eq!(body["error"]["code"], "payload_too_large"); } - fn guarded_test_app(backend: Arc) -> Router { - let guarded = Arc::new(GuardedOpenAiBackend::new( - backend, - GuardrailPolicy { - mode: GuardrailMode::Enforce, - apply_to_all_models: true, - ..GuardrailPolicy::default() - }, - )); - router_for_with_config( - guarded, - OpenAiFrontendConfig::default().without_backend_timeout(), - ) - } - async fn post_json(path: &str, value: Value) -> axum::response::Response { post_json_with_app_and_request_id(router_for(Arc::new(FakeBackend)), path, value, None) .await diff --git a/crates/openai-frontend/tests/benchy_contract.rs b/crates/openai-frontend/tests/benchy_contract.rs index 5e2381462c..b0a1621dd4 100644 --- a/crates/openai-frontend/tests/benchy_contract.rs +++ b/crates/openai-frontend/tests/benchy_contract.rs @@ -285,51 +285,6 @@ async fn benchy_stream_chat_omits_usage_without_stream_option() { assert!(text.contains("data: [DONE]")); } -#[tokio::test] -async fn guarded_benchy_non_stream_chat_rescues_tool_call_text() { - let backend = Arc::new(GuardedBenchyBackend::default()); - - let response = request_with_app( - guarded_backend_app(backend.clone()), - "POST", - "/v1/chat/completions", - json!({ - "model": BENCHY_MODEL_ID, - "messages": [{"role": "user", "content": "weather"}], - "tools": [{"type": "function", "function": {"name": "lookup"}}], - "tool_choice": "auto" - }), - Some("guarded-benchy-chat"), - ) - .await; - assert_eq!(response.status(), StatusCode::OK); - assert_eq!(response.headers()["x-request-id"], "guarded-benchy-chat"); - - let body = response_json(response).await; - assert!(body["choices"][0]["message"]["content"].is_null()); - assert_eq!(body["choices"][0]["finish_reason"], "tool_calls"); - assert_eq!( - body["choices"][0]["message"]["tool_calls"][0]["function"]["name"], - "lookup" - ); - assert_eq!( - body["choices"][0]["message"]["tool_calls"][0]["function"]["arguments"], - "{\"city\":\"Sydney\"}" - ); - assert!(!serde_json::to_string(&body).unwrap().contains("_mesh_")); - - let seen_requests = backend.seen_chat_requests.lock().unwrap(); - assert_eq!(seen_requests.len(), 1); - let seen_tools = seen_requests[0] - .tools - .as_ref() - .and_then(Value::as_array) - .cloned() - .expect("guarded backend should receive tools"); - assert_eq!(seen_tools[0]["function"]["name"], "lookup"); - assert_eq!(seen_tools[1]["function"]["name"], "_mesh_respond"); -} - #[tokio::test] async fn guarded_benchy_stream_chat_bypasses_guardrails_and_preserves_sse_framing() { let backend = Arc::new(GuardedBenchyBackend::default()); diff --git a/docs/design/OPENAI_GUARDRAILS.md b/docs/design/OPENAI_GUARDRAILS.md index 86fa9bb773..4e7bf43f83 100644 --- a/docs/design/OPENAI_GUARDRAILS.md +++ b/docs/design/OPENAI_GUARDRAILS.md @@ -32,7 +32,8 @@ already wrapped hosted models and future runtime-loaded/replacement Skippy backends observe the new mode without a process restart. The current posture is visible at `/api/status.runtime.openai_guardrails`. -The goal is validated emulation, not a general OpenAI tool runtime. +The goal is validation of native runtime output, not a second tool-call parser +or a general OpenAI tool runtime. ## Request Mode Contract @@ -57,15 +58,17 @@ The v1 surface is intentionally narrow. - no hard constrained decoding is promised - real tools plus strict structured output is unsupported in v1 -That means the layer can validate and emulate shaped responses, but it does not -become a full agent runtime. +That means the layer can validate native `message.tool_calls` and structured +output, but it does not become a full agent runtime. ## Retry and Exhaustion -Guardrail flow is rescue before retry. +Guardrail flow is native parsing, validation, then optional retry. -- rescue tries to turn malformed output into a valid assistant response first -- retry follows only after rescue fails or the policy requires another pass +- llama/Skippy owns chat-template rendering and tool-call parsing +- Mesh never converts assistant text, fenced JSON, XML, bracket syntax, or + model-specific text into `tool_calls` +- retry follows only after native output fails validation - retry budget means `max_retries + 1` total attempts The supported exhaustion modes are: @@ -81,11 +84,12 @@ Otherwise the run fails closed. The rollout preserves the reliability-layer behavior that informed this adaptation: -- a synthetic `respond` tool can be injected and stripped by the proxy -- rescue happens before retry +- a synthetic `respond` tool can be injected and stripped only when the native + parser returns it as a structured tool call +- native parsing happens before validation and retry - retry exhaustion is `max_retries + 1` total attempts -- guardrail outcome recording can propagate optional arguments while remaining - backward compatible +- guardrail outcomes record validation and retry only; parser-stage and rescue + telemetry do not exist ## Telemetry Privacy @@ -130,7 +134,7 @@ The corpus should include a small set of prompts that cover: - streaming pass-through - real tool-call reliability -- synthetic `_mesh_respond` rescue +- native synthetic `_mesh_respond` validation - strict structured output - unsupported real tools plus strict structured output diff --git a/docs/design/TESTING.md b/docs/design/TESTING.md index aa0867f5a1..4ff7fc0521 100644 --- a/docs/design/TESTING.md +++ b/docs/design/TESTING.md @@ -673,9 +673,9 @@ python3 scripts/run-openai-guardrail-corpus.py \ for server-side activation. - If the runtime is unavailable, the script falls back to deterministic fake-backend mode and still writes the expected JSON artifact. -- The corpus covers streaming pass-through, tool-call reliability, synthetic - `_mesh_respond` rescue, strict structured output, and the unsupported real - tools plus strict structured combination. +- The corpus covers streaming pass-through, native tool-call validation, + structured `_mesh_respond` output, strict structured output, and the + unsupported real tools plus strict structured combination. - The command is a reliability check, not a hard constrained decoding promise. If a Python sidecar baseline is available, you may optionally run a smoke diff --git a/docs/plugins/telemetry.md b/docs/plugins/telemetry.md index 918d0a0170..2a06948fd1 100644 --- a/docs/plugins/telemetry.md +++ b/docs/plugins/telemetry.md @@ -97,7 +97,7 @@ hostnames, mesh gossip, relay messages, raw node IDs, raw GPU stable IDs, endpoint URLs, or prompt hashes. Guardrail telemetry follows the same boundary. It exports only bounded labels for -guardrail mode, contract kind, decision, bypass reason, parser stage, and retry +guardrail mode, contract kind, decision, bypass reason, outcome, and retry bucket. It does not export prompt text, completion text, schemas, tool arguments, raw tool names, reserved sentinel names, request paths, endpoints, or hostnames. @@ -145,8 +145,7 @@ to an OTLP record. | `mesh_llm.guardrail.contract` | guardrail decision, guardrail outcome | Bounded enum: `tools` or `structured`. | | `mesh_llm.guardrail.decision` | guardrail decision | Bounded enum: `eligible`, `bypassed`, `unsupported`, or `rejected`. | | `mesh_llm.guardrail.bypass_reason` | guardrail decision | Bounded enum: `disabled`, `streaming`, `no_contract`, `unsupported_surface`, `reserved_collision`, or `mixed_tools_structured`. Omitted when no bypass reason applies. | -| `mesh_llm.guardrail.outcome` | guardrail outcome | Bounded enum: `pass_through`, `valid`, `rescued`, `retried`, `failed`, or `metrics_only_failure`. | -| `mesh_llm.guardrail.parser_stage` | guardrail outcome | Bounded enum: `none`, `json_exact`, `json_fenced`, or `json_substring`. | +| `mesh_llm.guardrail.outcome` | guardrail outcome | Bounded enum: `pass_through`, `valid`, `retried`, `failed`, or `metrics_only_failure`. | | `mesh_llm.guardrail.attempt_bucket` | guardrail outcome | Bounded retry bucket: `1`, `2`, or `3_plus`. | | `llama_stage.verify_window.direct_return_upstream_opened` | Skippy decode summary | Boolean indicating that the preferred upstream-opened v10 prediction-return sink completed its handshake. | | `llama_stage.verify_window.direct_return_reverse_fallback` | Skippy decode summary | Boolean indicating that the final stage used the bounded reverse-open v10 prediction-return fallback after the preferred sink was unavailable. | diff --git a/scripts/run-openai-guardrail-corpus.py b/scripts/run-openai-guardrail-corpus.py index 628d7a3729..3212ad156e 100644 --- a/scripts/run-openai-guardrail-corpus.py +++ b/scripts/run-openai-guardrail-corpus.py @@ -4,7 +4,7 @@ The script prefers a live OpenAI-compatible endpoint, but falls back to a deterministic fake backend when the runtime is unavailable. It records the request mode, expected server mode, prompt corpus, per-case artifacts, and the -aggregate success, failure, rescue, retry, and latency summaries. +aggregate success, failure, retry, and latency summaries. """ from __future__ import annotations @@ -32,7 +32,6 @@ class CorpusCase: prompt: str request_overrides: dict[str, Any] expected_outcome: str - rescue_count: int retry_count: int artifact_path: str @@ -82,7 +81,6 @@ def build_corpus() -> list[CorpusCase]: prompt="Reply with the word pass-through and nothing else.", request_overrides={"stream": True, "max_tokens": 16}, expected_outcome="pass_through", - rescue_count=0, retry_count=0, artifact_path=".sisyphus/evidence/openai-guardrail-corpus/streaming-pass-through.json", ), @@ -92,27 +90,15 @@ def build_corpus() -> list[CorpusCase]: prompt="Use the calculator tool to add 17 and 25, then stop.", request_overrides={"tools": [TOOL_SPEC], "tool_choice": "auto", "max_tokens": 64}, expected_outcome="tool_call", - rescue_count=1, retry_count=0, artifact_path=".sisyphus/evidence/openai-guardrail-corpus/tool-call-reliability.json", ), - CorpusCase( - case_id="synthetic-respond-rescue", - category="tools", - prompt="If the first answer fails, fall back to the synthetic _mesh_respond path.", - request_overrides={"tools": [TOOL_SPEC], "tool_choice": "auto", "max_tokens": 64}, - expected_outcome="rescued", - rescue_count=1, - retry_count=1, - artifact_path=".sisyphus/evidence/openai-guardrail-corpus/synthetic-respond-rescue.json", - ), CorpusCase( case_id="structured-object", category="structured", prompt="Return a JSON object with status, count, and note.", request_overrides={"response_format": {"type": "json_object"}, "max_tokens": 64}, expected_outcome="structured_object", - rescue_count=0, retry_count=0, artifact_path=".sisyphus/evidence/openai-guardrail-corpus/structured-object.json", ), @@ -132,7 +118,6 @@ def build_corpus() -> list[CorpusCase]: "max_tokens": 64, }, expected_outcome="strict_structured", - rescue_count=0, retry_count=1, artifact_path=".sisyphus/evidence/openai-guardrail-corpus/strict-structured-schema.json", ), @@ -154,7 +139,6 @@ def build_corpus() -> list[CorpusCase]: "max_tokens": 64, }, expected_outcome="unsupported_real_tools_plus_strict_structured", - rescue_count=0, retry_count=0, artifact_path=".sisyphus/evidence/openai-guardrail-corpus/unsupported-tools-plus-structured.json", ), @@ -346,7 +330,6 @@ def run_corpus(base_url: str, model: str, guardrail_mode: str, trials: int) -> d "category": case.category, "artifact_path": case.artifact_path, "expected_outcome": case.expected_outcome, - "rescue_count": case.rescue_count, "retry_count": case.retry_count, "ok": result["ok"], "status": result["status"], @@ -355,7 +338,6 @@ def run_corpus(base_url: str, model: str, guardrail_mode: str, trials: int) -> d } ) - rescue_count = sum(case.rescue_count for case in corpus) * trials retry_count = sum(case.retry_count for case in corpus) * trials return { @@ -367,7 +349,6 @@ def run_corpus(base_url: str, model: str, guardrail_mode: str, trials: int) -> d "total_requests": len(corpus) * trials, "success_count": success_count, "failure_count": failure_count, - "rescue_count": rescue_count, "retry_count": retry_count, "latency_ms": summarize_latencies(latencies), "corpus": [ @@ -375,7 +356,6 @@ def run_corpus(base_url: str, model: str, guardrail_mode: str, trials: int) -> d "case_id": case.case_id, "category": case.category, "expected_outcome": case.expected_outcome, - "rescue_count": case.rescue_count, "retry_count": case.retry_count, "artifact_path": case.artifact_path, }