fix(ai): mark rolled-back design batches as errors
This commit is contained in:
parent
993c660ad5
commit
c8fe0eb7b3
|
|
@ -331,8 +331,9 @@ pub struct ChatToolDef {
|
|||
|
||||
/// Result of executing one chat tool call. `content` is the JSON the
|
||||
/// model sees as the tool result (TS shape: `{"success":true,"data":…}`
|
||||
/// or `{"success":false,"error":…}`); `is_error` marks transport-level
|
||||
/// failure so Anthropic `tool_result` blocks can set `is_error`.
|
||||
/// or `{"success":false,"error":…}`); `is_error` marks an unsuccessful
|
||||
/// tool outcome (semantic validation/rollback or executor failure) so
|
||||
/// Anthropic `tool_result` blocks can set `is_error`.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ChatToolResult {
|
||||
pub content: String,
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ use op_mcp::{ToolRegistry, ToolResponse};
|
|||
use std::collections::HashSet;
|
||||
|
||||
use crate::chat_modify_sanitize::sanitize_modify_replacement;
|
||||
use crate::chat_tool_result::{rolled_back_transaction_message, structured_error_result};
|
||||
|
||||
/// TS `maxTurns` for the chat agent loop (`ai-chat-handlers.ts:254`).
|
||||
pub const MAX_TOOL_TURNS: usize = 20;
|
||||
|
|
@ -171,6 +172,19 @@ pub(crate) fn execute_with_registry(
|
|||
json,
|
||||
..
|
||||
} => {
|
||||
let data = match json {
|
||||
Some(raw) => serde_json::from_str::<serde_json::Value>(&raw)
|
||||
.unwrap_or(serde_json::Value::String(raw)),
|
||||
None => serde_json::to_value(&result).unwrap_or(serde_json::Value::Null),
|
||||
};
|
||||
// `batch_design` reports transactional rollbacks as structured
|
||||
// JSON so callers retain the failing lines and resend hint. That
|
||||
// is still a tool failure: no command landed and the agent loop
|
||||
// must receive `is_error`, otherwise the transcript paints a
|
||||
// green success card for an unchanged document.
|
||||
if let Some(message) = rolled_back_transaction_message(&data) {
|
||||
return (structured_error_result(message, data), false);
|
||||
}
|
||||
let mut mutated = false;
|
||||
if let Some(cmd) = command {
|
||||
if state.apply(cmd.clone()) {
|
||||
|
|
@ -182,11 +196,6 @@ pub(crate) fn execute_with_registry(
|
|||
);
|
||||
}
|
||||
}
|
||||
let data = match json {
|
||||
Some(raw) => serde_json::from_str::<serde_json::Value>(&raw)
|
||||
.unwrap_or(serde_json::Value::String(raw)),
|
||||
None => serde_json::to_value(&result).unwrap_or(serde_json::Value::Null),
|
||||
};
|
||||
let envelope = serde_json::json!({ "success": true, "data": data });
|
||||
(
|
||||
ChatToolResult {
|
||||
|
|
|
|||
35
crates/op-host-services/src/chat_tool_result.rs
Normal file
35
crates/op-host-services/src/chat_tool_result.rs
Normal file
|
|
@ -0,0 +1,35 @@
|
|||
//! Semantic result classification shared by in-process chat tool surfaces.
|
||||
|
||||
use op_ai::chat_provider::ChatToolResult;
|
||||
|
||||
pub(crate) fn rolled_back_transaction_message(data: &serde_json::Value) -> Option<String> {
|
||||
let object = data.as_object()?;
|
||||
if object.get("applied") != Some(&serde_json::Value::Bool(false)) {
|
||||
return None;
|
||||
}
|
||||
let errors = object.get("errors")?.as_array()?;
|
||||
if errors.is_empty() {
|
||||
return None;
|
||||
}
|
||||
Some(
|
||||
object
|
||||
.get("hint")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.unwrap_or(
|
||||
"Tool transaction rolled back; inspect data.errors and resend the corrected batch",
|
||||
)
|
||||
.to_string(),
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn structured_error_result(message: String, data: serde_json::Value) -> ChatToolResult {
|
||||
let envelope = serde_json::json!({
|
||||
"success": false,
|
||||
"error": message,
|
||||
"data": data,
|
||||
});
|
||||
ChatToolResult {
|
||||
content: envelope.to_string(),
|
||||
is_error: true,
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,44 @@
|
|||
use op_editor_core::EditorState;
|
||||
|
||||
use crate::design_agent_tools::execute_design_tool;
|
||||
|
||||
#[test]
|
||||
fn rolled_back_batch_is_an_error_and_preserves_structured_feedback() {
|
||||
let mut state = EditorState::new();
|
||||
let before = serde_json::to_value(state.active_children()).expect("initial document snapshot");
|
||||
let args = serde_json::json!({
|
||||
"operations": concat!(
|
||||
"root=I(null,{type:'frame',name:'Never Applied',width:120,height:80})\n",
|
||||
"U(\"missing-node\",{x:5})"
|
||||
)
|
||||
})
|
||||
.to_string();
|
||||
|
||||
let (result, mutated) = execute_design_tool(&mut state, "batch_design", &args);
|
||||
|
||||
assert!(result.is_error, "rolled-back batch must be a tool error");
|
||||
assert!(!mutated, "rolled-back batch must not report a mutation");
|
||||
assert_eq!(
|
||||
serde_json::to_value(state.active_children()).expect("final document snapshot"),
|
||||
before,
|
||||
"the transaction must leave the document unchanged"
|
||||
);
|
||||
|
||||
let envelope: serde_json::Value =
|
||||
serde_json::from_str(&result.content).expect("chat tool result envelope");
|
||||
assert_eq!(envelope["success"], false, "{envelope}");
|
||||
assert_eq!(envelope["data"]["applied"], false, "{envelope}");
|
||||
assert_eq!(
|
||||
envelope["data"]["errors"]
|
||||
.as_array()
|
||||
.expect("structured errors")
|
||||
.len(),
|
||||
1,
|
||||
"{envelope}"
|
||||
);
|
||||
let hint = envelope["data"]["hint"]
|
||||
.as_str()
|
||||
.expect("structured resend hint");
|
||||
assert!(hint.contains("rolled back"), "{hint}");
|
||||
assert_eq!(envelope["error"], hint, "{envelope}");
|
||||
}
|
||||
|
|
@ -41,12 +41,15 @@ mod chat_subprocess_parse;
|
|||
pub mod chat_subprocess_quirks;
|
||||
mod chat_subprocess_safety;
|
||||
pub mod chat_system_prompt;
|
||||
mod chat_tool_result;
|
||||
pub mod cli_model_discovery;
|
||||
pub mod cli_provider_probe;
|
||||
mod cli_resolver_windows;
|
||||
mod design_agent_diagnostics;
|
||||
#[cfg(test)]
|
||||
mod design_agent_reflow_tests;
|
||||
#[cfg(test)]
|
||||
mod design_agent_tool_result_tests;
|
||||
pub mod design_agent_tools;
|
||||
pub mod design_context;
|
||||
pub mod design_md_llm;
|
||||
|
|
|
|||
Loading…
Reference in a new issue