diff --git a/crates/core/src/observability/atif.rs b/crates/core/src/observability/atif.rs index 27615f4c..b761846e 100644 --- a/crates/core/src/observability/atif.rs +++ b/crates/core/src/observability/atif.rs @@ -520,7 +520,29 @@ fn anthropic_messages_content_message(output: &Json, content: &Json) -> Option Option { - (!value.is_null()).then(|| value.clone()) + match value { + Json::String(_) => Some(value.clone()), + Json::Array(_) if is_atif_content_parts(value) => Some(value.clone()), + _ => None, + } +} + +fn observation_extra(event: &Event, output: &Json) -> Json { + let mut extra = event_extra(event); + if let Some(tool_result) = observation_tool_result_extra(output) + && let Json::Object(extra_object) = &mut extra + { + extra_object.insert("tool_result".to_string(), tool_result); + } + extra +} + +fn observation_tool_result_extra(value: &Json) -> Option { + match value { + Json::Null | Json::String(_) => None, + Json::Array(_) if is_atif_content_parts(value) => None, + _ => Some(value.clone()), + } } fn is_atif_content_parts(value: &Json) -> bool { @@ -2309,7 +2331,7 @@ impl StepConversionState { source_call_id: source_call_id.clone(), content: observation_content_value(output), subagent_trajectory_ref: None, - extra: Some(event_extra(event)), + extra: Some(observation_extra(event, output)), }); } @@ -2652,8 +2674,8 @@ fn merge_observation_result(observation: &mut AtifObservation, mut result: AtifO .get_or_insert_with(Vec::new) .append(&mut refs); } - if existing.extra.is_none() { - existing.extra = result.extra.take(); + if let Some(extra) = result.extra.take() { + merge_observation_extra(&mut existing.extra, extra); } return; } @@ -2661,6 +2683,20 @@ fn merge_observation_result(observation: &mut AtifObservation, mut result: AtifO observation.results.push(result); } +fn merge_observation_extra(existing: &mut Option, incoming: Json) { + let Some(existing_extra) = existing.as_mut() else { + *existing = Some(incoming); + return; + }; + let (Json::Object(existing_object), Json::Object(incoming_object)) = (existing_extra, incoming) + else { + return; + }; + for (key, value) in incoming_object { + existing_object.entry(key).or_insert(value); + } +} + fn subagent_dispatch_arguments(child: &AgentScopeNode, event: &Event) -> Json { let mut arguments = serde_json::Map::new(); arguments.insert("name".to_string(), Json::String(child.name.clone())); diff --git a/crates/core/tests/unit/atif_tests.rs b/crates/core/tests/unit/atif_tests.rs index 246b5291..aca8aede 100644 --- a/crates/core/tests/unit/atif_tests.rs +++ b/crates/core/tests/unit/atif_tests.rs @@ -385,10 +385,23 @@ fn assert_atif_message_value(value: &serde_json::Value) { } fn assert_atif_observation_content_value(value: &serde_json::Value) { - assert!( - !value.is_null(), - "ATIF observation content must not be null" - ); + if value.is_string() { + return; + } + if let Some(parts) = value.as_array() + && parts.iter().all(is_atif_content_part) + { + return; + } + panic!("ATIF observation content must be a string or content-part array: {value}"); +} + +fn assert_structured_observation_result_extra( + result: &AtifObservationResult, + expected: serde_json::Value, +) { + assert_eq!(result.content, None); + assert_eq!(result.extra.as_ref().unwrap()["tool_result"], expected); } fn is_atif_content_part(part: &serde_json::Value) -> bool { @@ -475,10 +488,7 @@ fn test_exporter_tool_lifecycle() { let obs = step1.observation.as_ref().unwrap(); assert_eq!(obs.results.len(), 1); assert_eq!(obs.results[0].source_call_id, None); - assert_eq!( - obs.results[0].content, - Some(json!({"results": ["result1"]})) - ); + assert_structured_observation_result_extra(&obs.results[0], json!({"results": ["result1"]})); assert_eq!( obs.results[0].extra.as_ref().unwrap()["event_name"], json!("web_search") @@ -515,6 +525,65 @@ fn test_exporter_omits_null_tool_observation_content() { ); } +#[test] +fn test_exporter_moves_structured_tool_close_result_to_observation_extra() { + let exporter = AtifExporter::new("session-1".to_string(), make_agent_info()); + let tool_uuid = Uuid::now_v7(); + let close_result = json!({"status": "closed_by_turn_end"}); + + let end = event_builder(tool_uuid, EventType::End) + .name("terminal") + .scope_type(ScopeType::Tool) + .output(close_result.clone()) + .tool_call_id("call_123") + .build(); + + { + let mut state = exporter.state.lock().unwrap(); + state.events.push(end); + } + + let trajectory = exporter.export().unwrap(); + assert_atif_v17_shape(&trajectory); + + let result = &trajectory.steps[0].observation.as_ref().unwrap().results[0]; + assert_eq!(result.content, None); + assert_eq!(result.extra.as_ref().unwrap()["tool_result"], close_result); + + let exported = serde_json::to_value(&trajectory).unwrap(); + let content = exported["steps"][0]["observation"]["results"][0].get("content"); + assert!(content.is_none()); +} + +#[test] +fn test_exporter_preserves_tool_content_part_array_observation_content() { + let exporter = AtifExporter::new("session-1".to_string(), make_agent_info()); + let tool_uuid = Uuid::now_v7(); + let content_parts = json!([ + {"type": "text", "text": "screenshot captured"}, + {"type": "image", "source": {"media_type": "image/png", "path": "artifacts/screenshot.png"}}, + ]); + + let end = event_builder(tool_uuid, EventType::End) + .name("screenshot") + .scope_type(ScopeType::Tool) + .output(content_parts.clone()) + .tool_call_id("call_123") + .build(); + + { + let mut state = exporter.state.lock().unwrap(); + state.events.push(end); + } + + let trajectory = exporter.export().unwrap(); + assert_atif_v17_shape(&trajectory); + + let result = &trajectory.steps[0].observation.as_ref().unwrap().results[0]; + assert_eq!(result.content, Some(content_parts)); + assert!(result.extra.as_ref().unwrap().get("tool_result").is_none()); +} + #[test] fn test_exporter_llm_lifecycle() { let exporter = AtifExporter::new("session-1".to_string(), make_agent_info()); @@ -894,9 +963,9 @@ fn test_exporter_anthropic_messages_lifecycle_promotes_tool_use_blocks() { observation.results[0].source_call_id.as_deref(), Some("toolu_01") ); - assert_eq!( - observation.results[0].content, - Some(json!({"matches": ["README.md"]})) + assert_structured_observation_result_extra( + &observation.results[0], + json!({"matches": ["README.md"]}), ); } @@ -1140,7 +1209,7 @@ fn test_exporter_openai_responses_function_calls_promoted_and_correlated() { let result = &agent_step.observation.as_ref().unwrap().results[0]; assert_eq!(result.source_call_id.as_deref(), Some("call_terminal_1")); - assert_eq!(result.content, Some(json!({"stdout": "/workspace"}))); + assert_structured_observation_result_extra(result, json!({"stdout": "/workspace"})); } #[test] @@ -1301,9 +1370,9 @@ fn test_exporter_hermes_wrapper_payload_is_atif_v17_compatible() { observation.results[0].source_call_id, Some("call_terminal_1".to_string()) ); - assert_eq!( - observation.results[0].content, - Some(json!({"stdout": "hi", "exit_code": 0})) + assert_structured_observation_result_extra( + &observation.results[0], + json!({"stdout": "hi", "exit_code": 0}), ); } @@ -1679,7 +1748,7 @@ fn test_exporter_attaches_subagent_ref_to_delegating_tool_observation() { let result = &agent_step.observation.as_ref().unwrap().results[0]; assert_eq!(result.source_call_id.as_deref(), Some("call_delegate")); - assert_eq!(result.content, Some(json!({"status": "launched"}))); + assert_structured_observation_result_extra(result, json!({"status": "launched"})); let refs = result.subagent_trajectory_ref.as_ref().unwrap(); assert_eq!(refs[0].trajectory_id, Some(child_uuid.to_string())); assert_eq!(refs[0].session_id, Some("child-session".to_string())); @@ -1792,7 +1861,7 @@ fn test_exporter_synthesizes_tool_call_for_active_subagent_dispatch() { result.source_call_id.as_deref(), Some(tool_call_id.as_str()) ); - assert_eq!(result.content, Some(json!({"status": "completed"}))); + assert_structured_observation_result_extra(result, json!({"status": "completed"})); let refs = result.subagent_trajectory_ref.as_ref().unwrap(); assert_eq!(refs[0].trajectory_id, Some(child_uuid.to_string())); assert_eq!(refs[0].session_id, Some("child-session".to_string())); @@ -2982,7 +3051,7 @@ fn test_exporter_correlates_hermes_style_tool_outputs_before_llm_calls() { observation.results[0].source_call_id, Some(source_call_id.to_string()) ); - assert_eq!(observation.results[0].content, Some(content)); + assert_structured_observation_result_extra(&observation.results[0], content); } assert!(!trajectory.steps.iter().any(|step| {