mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-06 08:18:58 +00:00
Fix OpenAI error field, add Anthropic provider options pass-through, and improve parity tests
- Use `status: "incomplete"` instead of `is_error` for OpenAI tool results (fixes rejection) - Add merge_provider_options to forward unknown anthropic provider options to API body - Derive Clone on Client to enable subagent session factory - Enable error_recovery scenario for all providers now that OpenAI is fixed - Improve subagent_spawn test to actually exercise spawn/wait/read workflow - Adjust multi-turn cache test temperature to 0.5 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
b4068b6364
commit
46d4659633
5 changed files with 138 additions and 13 deletions
|
|
@ -3,20 +3,49 @@ use std::sync::Arc;
|
|||
|
||||
use agent::{
|
||||
AnthropicProfile, GeminiProfile, LocalExecutionEnvironment, OpenAiProfile, ProviderProfile,
|
||||
Session, SessionConfig,
|
||||
Session, SessionConfig, SubAgentManager,
|
||||
};
|
||||
use llm::client::Client;
|
||||
|
||||
async fn make_session(provider: &str, model: &str, cwd: &Path) -> Session {
|
||||
dotenvy::dotenv().ok();
|
||||
let client = Client::from_env().await.expect("Client::from_env failed");
|
||||
let profile: Arc<dyn ProviderProfile> = match provider {
|
||||
"anthropic" => Arc::new(AnthropicProfile::new(model)),
|
||||
"openai" => Arc::new(OpenAiProfile::new(model)),
|
||||
"gemini" => Arc::new(GeminiProfile::new(model)),
|
||||
let mut profile: Box<dyn ProviderProfile> = match provider {
|
||||
"anthropic" => Box::new(AnthropicProfile::new(model)),
|
||||
"openai" => Box::new(OpenAiProfile::new(model)),
|
||||
"gemini" => Box::new(GeminiProfile::new(model)),
|
||||
_ => panic!("unknown provider: {provider}"),
|
||||
};
|
||||
let env = Arc::new(LocalExecutionEnvironment::new(cwd.to_path_buf()));
|
||||
|
||||
// Register subagent tools so spawn_agent / wait / send_input / close_agent are available
|
||||
let manager = Arc::new(tokio::sync::Mutex::new(SubAgentManager::new(3)));
|
||||
let factory_client = client.clone();
|
||||
let factory_provider: &str = provider;
|
||||
let factory_model: String = model.to_string();
|
||||
let factory_cwd = cwd.to_path_buf();
|
||||
let factory: agent::subagent::SessionFactory = {
|
||||
let provider = factory_provider.to_string();
|
||||
let model = factory_model.clone();
|
||||
Arc::new(move || {
|
||||
let sub_profile: Arc<dyn ProviderProfile> = match provider.as_str() {
|
||||
"anthropic" => Arc::new(AnthropicProfile::new(&model)),
|
||||
"openai" => Arc::new(OpenAiProfile::new(&model)),
|
||||
"gemini" => Arc::new(GeminiProfile::new(&model)),
|
||||
_ => panic!("unknown provider: {provider}"),
|
||||
};
|
||||
let sub_env = Arc::new(LocalExecutionEnvironment::new(factory_cwd.clone()));
|
||||
Session::new(
|
||||
factory_client.clone(),
|
||||
sub_profile,
|
||||
sub_env,
|
||||
SessionConfig::default(),
|
||||
)
|
||||
})
|
||||
};
|
||||
profile.register_subagent_tools(manager, factory, 0);
|
||||
|
||||
let profile: Arc<dyn ProviderProfile> = Arc::from(profile);
|
||||
let config = SessionConfig {
|
||||
max_turns: 20,
|
||||
..SessionConfig::default()
|
||||
|
|
@ -89,10 +118,11 @@ provider_tests!(subagent_spawn);
|
|||
// Scenarios below are only generated for providers where they are supported.
|
||||
// - multi_step_read_analyze_edit / provider_specific_editing: gpt-4o-mini is too
|
||||
// weak to reliably apply precise file edits (uses apply_patch, not edit_file).
|
||||
// - error_recovery: OpenAI rejects the `is_error` field on tool results (adapter bug).
|
||||
// - reasoning_effort: gpt-4o-mini doesn't support the reasoning.effort parameter.
|
||||
// - loop_detection: needs custom config, tested separately below.
|
||||
|
||||
provider_tests!(error_recovery);
|
||||
|
||||
macro_rules! anthropic_gemini_tests {
|
||||
($scenario:ident) => {
|
||||
paste::paste! {
|
||||
|
|
@ -118,7 +148,6 @@ macro_rules! anthropic_gemini_tests {
|
|||
}
|
||||
|
||||
anthropic_gemini_tests!(multi_step_read_analyze_edit);
|
||||
anthropic_gemini_tests!(error_recovery);
|
||||
anthropic_gemini_tests!(provider_specific_editing);
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -299,10 +328,12 @@ reasoning_effort_tests!("gemini", "gemini-2.5-flash", gemini_reasoning_effort);
|
|||
// ---------------------------------------------------------------------------
|
||||
// Scenario 12: subagent_spawn
|
||||
// ---------------------------------------------------------------------------
|
||||
async fn scenario_subagent_spawn(session: &mut Session, _dir: &Path) {
|
||||
async fn scenario_subagent_spawn(session: &mut Session, dir: &Path) {
|
||||
std::fs::write(dir.join("secret.txt"), "the_secret_value").expect("failed to write secret.txt");
|
||||
session
|
||||
.process_input(
|
||||
"Try to spawn a subagent to read a file. If the subagent tool is not available, just say 'no subagent tool'",
|
||||
"Spawn a subagent to read the file secret.txt and report its contents. \
|
||||
Wait for the subagent to finish, then tell me what it found.",
|
||||
)
|
||||
.await
|
||||
.expect("process_input failed");
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ use std::collections::HashMap;
|
|||
use std::sync::Arc;
|
||||
|
||||
/// The core client that routes requests to provider adapters (Section 2.2, 3).
|
||||
#[derive(Clone)]
|
||||
pub struct Client {
|
||||
providers: HashMap<String, Arc<dyn ProviderAdapter>>,
|
||||
default_provider: Option<String>,
|
||||
|
|
|
|||
|
|
@ -1026,6 +1026,30 @@ impl SseReaderState {
|
|||
}
|
||||
}
|
||||
|
||||
/// Known `provider_options.anthropic` keys that are already handled by the adapter
|
||||
/// and should not be merged into the request body a second time.
|
||||
const KNOWN_ANTHROPIC_OPTION_KEYS: &[&str] = &["thinking", "auto_cache", "beta_headers"];
|
||||
|
||||
/// Serialize the API request and merge any unknown `provider_options.anthropic` keys.
|
||||
fn merge_provider_options(
|
||||
api_request: &ApiRequest,
|
||||
provider_options: Option<&serde_json::Value>,
|
||||
) -> serde_json::Value {
|
||||
let mut body = serde_json::to_value(api_request).unwrap_or_else(|_| serde_json::json!({}));
|
||||
|
||||
if let Some(anthropic_opts) = provider_options.and_then(|opts| opts.get("anthropic")) {
|
||||
if let (Some(base), Some(overrides)) = (body.as_object_mut(), anthropic_opts.as_object()) {
|
||||
for (key, value) in overrides {
|
||||
if !KNOWN_ANTHROPIC_OPTION_KEYS.contains(&key.as_str()) {
|
||||
base.insert(key.clone(), value.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
body
|
||||
}
|
||||
|
||||
/// Build an Anthropic API request and HTTP request builder for the given unified request.
|
||||
fn build_api_request(
|
||||
adapter: &Adapter,
|
||||
|
|
@ -1103,7 +1127,7 @@ fn build_api_request(
|
|||
req_builder = req_builder.header("anthropic-beta", beta_str);
|
||||
}
|
||||
|
||||
let req_builder = req_builder.json(&api_request);
|
||||
let req_builder = req_builder.json(&merge_provider_options(&api_request, request.provider_options.as_ref()));
|
||||
(api_request, req_builder)
|
||||
}
|
||||
|
||||
|
|
@ -1921,6 +1945,75 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_provider_options_passes_through_unknown_keys() {
|
||||
let api_request = ApiRequest {
|
||||
model: "claude-sonnet-4-20250514".to_string(),
|
||||
messages: vec![ApiMessage {
|
||||
role: "user".to_string(),
|
||||
content: vec![serde_json::json!({"type": "text", "text": "Hello"})],
|
||||
}],
|
||||
max_tokens: 4096,
|
||||
system: None,
|
||||
temperature: None,
|
||||
top_p: None,
|
||||
stop_sequences: None,
|
||||
tools: None,
|
||||
tool_choice: None,
|
||||
thinking: None,
|
||||
metadata: None,
|
||||
stream: false,
|
||||
};
|
||||
|
||||
let opts = serde_json::json!({
|
||||
"anthropic": {
|
||||
"top_k": 40,
|
||||
"custom_field": "value"
|
||||
}
|
||||
});
|
||||
let body = merge_provider_options(&api_request, Some(&opts));
|
||||
assert_eq!(body["top_k"], 40);
|
||||
assert_eq!(body["custom_field"], "value");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_provider_options_skips_known_keys() {
|
||||
let api_request = ApiRequest {
|
||||
model: "claude-sonnet-4-20250514".to_string(),
|
||||
messages: vec![ApiMessage {
|
||||
role: "user".to_string(),
|
||||
content: vec![serde_json::json!({"type": "text", "text": "Hello"})],
|
||||
}],
|
||||
max_tokens: 4096,
|
||||
system: None,
|
||||
temperature: None,
|
||||
top_p: None,
|
||||
stop_sequences: None,
|
||||
tools: None,
|
||||
tool_choice: None,
|
||||
thinking: None,
|
||||
metadata: None,
|
||||
stream: false,
|
||||
};
|
||||
|
||||
let opts = serde_json::json!({
|
||||
"anthropic": {
|
||||
"thinking": {"type": "enabled", "budget_tokens": 10000},
|
||||
"auto_cache": false,
|
||||
"beta_headers": ["some-header"],
|
||||
"top_k": 40
|
||||
}
|
||||
});
|
||||
let body = merge_provider_options(&api_request, Some(&opts));
|
||||
// Known keys should not be merged (they are handled separately)
|
||||
assert!(body.get("auto_cache").is_none());
|
||||
assert!(body.get("beta_headers").is_none());
|
||||
// thinking is handled by the ApiRequest struct directly, should not be double-merged
|
||||
assert!(body["thinking"].is_null());
|
||||
// Unknown keys should be merged
|
||||
assert_eq!(body["top_k"], 40);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn audio_produces_text_fallback() {
|
||||
let part = ContentPart::Audio(crate::types::AudioData {
|
||||
|
|
|
|||
|
|
@ -288,7 +288,7 @@ fn translate_input(messages: &[Message]) -> (Option<String>, Vec<serde_json::Val
|
|||
"output": output,
|
||||
});
|
||||
if tr.is_error {
|
||||
item["is_error"] = serde_json::json!(true);
|
||||
item["status"] = serde_json::json!("incomplete");
|
||||
}
|
||||
input.push(item);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -151,7 +151,7 @@ async fn openai_multi_turn_cache() {
|
|||
dotenvy::dotenv().ok();
|
||||
let api_key = std::env::var("OPENAI_API_KEY").expect("OPENAI_API_KEY must be set");
|
||||
let adapter = OpenAiAdapter::new(api_key);
|
||||
run_multi_turn_cache_test(&adapter, "gpt-4o-mini", 0.0).await;
|
||||
run_multi_turn_cache_test(&adapter, "gpt-4o-mini", 0.5).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
|
@ -160,5 +160,5 @@ async fn gemini_multi_turn_cache() {
|
|||
dotenvy::dotenv().ok();
|
||||
let api_key = std::env::var("GEMINI_API_KEY").expect("GEMINI_API_KEY must be set");
|
||||
let adapter = GeminiAdapter::new(api_key);
|
||||
run_multi_turn_cache_test(&adapter, "gemini-2.5-flash", 0.0).await;
|
||||
run_multi_turn_cache_test(&adapter, "gemini-2.5-flash", 0.5).await;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue