From 87e1e3a00f46a9d44413709ca3be264e4c709314 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 10 Sep 2026 09:41:20 -0600 Subject: [PATCH] Replace fabro-llm helper modules with lithos-llm equivalents Delete fabro-llm's attachments, reasoning, and structured modules and the LlmError newtype and ErrorFacts trait. lithos-llm now provides all of them: InlineLocalFiles under the local-files feature, ReasoningOutput with Response::reasoning(), Client::complete_object, and the retry, auth, cancel, and failover predicates directly on Error and ErrorData. fabro-llm keeps only failure_signature_hint, which is Fabro's own loop detection policy. Store ErrorData directly in the agent and workflow error enums, boxed where the variant would otherwise dominate the enum size. Repin lithos-llm to a1e3fd3 for these additions. Co-Authored-By: Claude Fable 5.1 --- Cargo.lock | 26 +- Cargo.toml | 2 +- docs/public/reference/sdk.mdx | 26 +- lib/apps/fabro-cli/src/commands/exec.rs | 2 +- .../src/commands/run/run_progress/mod.rs | 6 +- .../fabro-server/src/run_title_generation.rs | 13 +- .../src/server/handler/completions.rs | 7 +- lib/components/fabro-agent/src/error.rs | 41 ++- lib/components/fabro-agent/src/history.rs | 5 +- lib/components/fabro-agent/src/session.rs | 23 +- lib/components/fabro-agent/src/types.rs | 14 +- lib/components/fabro-hooks/src/executor.rs | 10 +- lib/components/fabro-llm/Cargo.toml | 5 +- lib/components/fabro-llm/src/attachments.rs | 268 --------------- lib/components/fabro-llm/src/client.rs | 13 +- lib/components/fabro-llm/src/error.rs | 320 ++---------------- lib/components/fabro-llm/src/lib.rs | 17 +- lib/components/fabro-llm/src/reasoning.rs | 292 ---------------- lib/components/fabro-llm/src/structured.rs | 103 ------ lib/components/fabro-workflow/src/error.rs | 54 +-- .../fabro-workflow/src/handler/llm/api.rs | 39 ++- .../src/pipeline/pull_request.rs | 10 +- lib/foundation/fabro-types/src/lib.rs | 5 +- lib/foundation/fabro-types/src/reasoning.rs | 142 -------- 24 files changed, 184 insertions(+), 1259 deletions(-) delete mode 100644 lib/components/fabro-llm/src/attachments.rs delete mode 100644 lib/components/fabro-llm/src/reasoning.rs delete mode 100644 lib/components/fabro-llm/src/structured.rs delete mode 100644 lib/foundation/fabro-types/src/reasoning.rs diff --git a/Cargo.lock b/Cargo.lock index 3459e9ed6..9b9bada2b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2063,7 +2063,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -2190,7 +2190,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -2751,7 +2751,6 @@ version = "0.348.0-nightly.0" dependencies = [ "anyhow", "async-trait", - "base64", "bytes", "fabro-auth", "fabro-config", @@ -2765,11 +2764,9 @@ dependencies = [ "futures", "httpmock", "lithos-llm", - "mime_guess", "serde", "serde_json", "strum 0.28.0", - "tempfile", "thiserror 2.0.18", "tokio", "tokio-util", @@ -4952,7 +4949,7 @@ checksum = "6373607a59f0be73a39b6fe456b8192fcc3585f602af20751600e974dd455e77" [[package]] name = "lithos-llm" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/lithos-llm?rev=ba2f4184e3db9a37f36aa650ce4b7c5f3376b109#ba2f4184e3db9a37f36aa650ce4b7c5f3376b109" +source = "git+https://github.com/lithoscomputer/lithos-llm?rev=a1e3fd37b7153870411701327ac117606753fe90#a1e3fd37b7153870411701327ac117606753fe90" dependencies = [ "async-trait", "aws-config", @@ -4964,6 +4961,7 @@ dependencies = [ "crc32fast", "futures-core", "futures-util", + "mime_guess", "reqwest 0.13.4", "serde", "serde_json", @@ -5413,7 +5411,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -6410,7 +6408,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.60.2", + "windows-sys 0.59.0", ] [[package]] @@ -6887,7 +6885,7 @@ dependencies = [ "errno 0.3.14", "libc", "linux-raw-sys", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -6946,7 +6944,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -7470,7 +7468,7 @@ version = "1.4.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4db69cba1110affc0e9f7bcd48bbf87b3f4fc7c61fc9155afd4c469eb3d6c1b" dependencies = [ - "errno 0.3.14", + "errno 0.2.8", "libc", ] @@ -8060,7 +8058,7 @@ dependencies = [ "getrandom 0.4.1", "once_cell", "rustix", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -8106,7 +8104,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "230a1b821ccbd75b185820a1f1ff7b14d21da1e442e22c0863ea5f08771a8874" dependencies = [ "rustix", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -9184,7 +9182,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 09e61ddeb..854cef45e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,7 +94,7 @@ insta = "1" fabro-test = { path = "lib/foundation/fabro-test" } # Provider-neutral LLM catalog and client. Pinned to a revision until 0.x is # published to crates.io. -lithos-llm = { git = "https://github.com/lithoscomputer/lithos-llm", rev = "ba2f4184e3db9a37f36aa650ce4b7c5f3376b109", default-features = false } +lithos-llm = { git = "https://github.com/lithoscomputer/lithos-llm", rev = "a1e3fd37b7153870411701327ac117606753fe90", default-features = false } # Deterministic OpenAI twin used by twin-mode E2E tests; the same revision # lithos-llm verifies its codecs against. twin-openai = { git = "https://github.com/lithoscomputer/twins", rev = "ca45f0e50a6716d716aa2f638ca3cf767e88f613" } diff --git a/docs/public/reference/sdk.mdx b/docs/public/reference/sdk.mdx index 1b34463dc..63be41cec 100644 --- a/docs/public/reference/sdk.mdx +++ b/docs/public/reference/sdk.mdx @@ -300,7 +300,7 @@ All fallible `Session` methods return `Result`: | Variant | Description | |---|---| -| `Llm(LlmError)` | An error from the LLM provider (the stored form of a lithos `Error`). | +| `Llm(Box)` | An error from the LLM provider: the lithos `ErrorData`, the stored form of a lithos `Error`. | | `SessionClosed` | `process_input` was called on a closed session. | | `InvalidState(String)` | The session is in an unexpected state. | | `ToolExecution(String)` | A tool execution failed. | @@ -425,10 +425,10 @@ A turn that ends with `FinishReason::Length` or `FinishReason::Incomplete` is no ### Structured output -`fabro_llm::structured::complete_object` attaches a JSON Schema as the request's response format and parses the reply: +`Client::complete_object` (a lithos method) attaches a JSON Schema as the request's response format and parses the reply into a `StructuredCompletion` with the response and the parsed document: ```rust -use fabro_llm::{Request, structured}; +use fabro_llm::Request; use serde_json::json; let schema = json!({ @@ -444,31 +444,31 @@ let request = Request::builder() .model("claude-sonnet-4.5") .user("Generate a profile for a fictional character") .build()?; -let completion = structured::complete_object(&client, request, "profile", schema).await?; +let completion = client.complete_object(request, "profile", schema).await?; println!("Name: {}", completion.object["name"]); ``` ### Reasoning -`fabro_llm::reasoning::normalize(&response.content)` folds a response's readable reasoning parts into a `fabro_types::ReasoningOutput` with a summary and a trace. Provider replay data such as signatures and encrypted reasoning never appears in it. +`response.reasoning()` (a lithos method) folds a response's readable reasoning parts into a `ReasoningOutput` with a summary and a trace, whichever channel the provider used. Provider replay data such as signatures and encrypted reasoning never appears in it; `ContentPart::is_replay_material()` marks the parts a conversation keeps for the next request instead. ### Middleware -Middleware is the lithos `Middleware` trait: `handle(&self, call: Call, next: Next)` sees the resolved route and request and returns an `Output` that is either a complete response or a stream. `fabro_llm::attachments::InlineLocalAttachments` is Fabro's own middleware; it rewrites local file references in messages into inline media before dispatch. +Middleware is the lithos `Middleware` trait: `handle(&self, call: Call, next: Next)` sees the resolved route and request and returns an `Output` that is either a complete response or a stream. `ClientOptions::standard()` installs lithos's `InlineLocalFiles`, which rewrites local file paths in messages into inline media before dispatch. ### Error handling -Every fallible operation returns `Result`, the lithos error. `error.kind()` is an `ErrorKind` such as `Authentication`, `RateLimit`, `Server`, `ContextLength`, `ContentFilter`, `Timeout`, `StreamDecode`, or `Cancelled`. `error.data()` is the `ErrorData` snapshot Fabro stores in run events; `fabro_llm::LlmError` wraps it. +Every fallible operation returns `Result`, the lithos error. `error.kind()` is an `ErrorKind` such as `Authentication`, `RateLimit`, `Server`, `ContextLength`, `ContentFilter`, `Timeout`, `StreamDecode`, or `Cancelled`. `error.data()` is the `ErrorData` snapshot Fabro stores in run events; it reads like `Error`, prints its message, and implements `std::error::Error`. -The `fabro_llm::ErrorFacts` trait is implemented for `Error`, `ErrorData`, and `LlmError`, and the classification helpers take any of them: +Both `Error` and `ErrorData` answer the policy questions directly; only the loop-detection signature is Fabro's: | Function | Description | |---|---| -| `is_retryable(&error)` | Safe to retry with the same provider, from lithos's retry classification | -| `failover_eligible(&error)` | Safe to try a different provider | -| `is_auth_error(&error)` | The credential was missing or rejected | -| `is_cancelled(&error)` | The caller cancelled the call | -| `failure_signature_hint(&error)` | A stable string for loop and restart detection | +| `error.is_retryable()` | Safe to retry with the same provider, from lithos's retry classification | +| `error.failover_eligible()` | Safe to try a different provider | +| `error.is_auth_error()` | The credential was missing or rejected | +| `error.is_cancelled()` | The caller cancelled the call | +| `fabro_llm::failure_signature_hint(&data)` | A stable string for loop and restart detection | ### Retries diff --git a/lib/apps/fabro-cli/src/commands/exec.rs b/lib/apps/fabro-cli/src/commands/exec.rs index 535e94332..46340bdde 100644 --- a/lib/apps/fabro-cli/src/commands/exec.rs +++ b/lib/apps/fabro-cli/src/commands/exec.rs @@ -7,9 +7,9 @@ use fabro_agent::cli::{ OutputFormat, diagnostic_client_options, run_with_args_and_client_and_catalog, run_with_args_and_source_and_catalog, }; +use fabro_llm::ErrorKind; use fabro_llm::gateway::{GatewayAdapter, GatewayError, GatewayTransport}; use fabro_llm::lithos_catalog::Catalog; -use fabro_llm::{ErrorFacts, ErrorKind}; use fabro_mcp::config::McpServerSettings; use fabro_types::ProviderId; use fabro_types::settings::cli::OutputFormat as SettingsOutputFormat; diff --git a/lib/apps/fabro-cli/src/commands/run/run_progress/mod.rs b/lib/apps/fabro-cli/src/commands/run/run_progress/mod.rs index 6aaebce34..a04f7b943 100644 --- a/lib/apps/fabro-cli/src/commands/run/run_progress/mod.rs +++ b/lib/apps/fabro-cli/src/commands/run/run_progress/mod.rs @@ -836,7 +836,7 @@ mod tests { attempt: 1, delay_secs: 0.1, phase: fabro_types::LlmRetryPhase::Consume, - error: fabro_llm::LlmError::from(fabro_llm::Error::new( + error: fabro_llm::ErrorData::from(fabro_llm::Error::new( fabro_llm::ErrorKind::Configuration, "retry", )), @@ -955,7 +955,7 @@ mod tests { attempt: 2, delay_secs: 1.5, phase: fabro_types::LlmRetryPhase::Open, - error: fabro_llm::LlmError::from(fabro_llm::Error::new( + error: fabro_llm::ErrorData::from(fabro_llm::Error::new( fabro_llm::ErrorKind::Configuration, "busy", )), @@ -1319,7 +1319,7 @@ mod tests { attempt: 2, delay_secs: 1.5, phase: fabro_types::LlmRetryPhase::Open, - error: fabro_llm::LlmError::from(fabro_llm::Error::new( + error: fabro_llm::ErrorData::from(fabro_llm::Error::new( fabro_llm::ErrorKind::Configuration, "busy", )), diff --git a/lib/apps/fabro-server/src/run_title_generation.rs b/lib/apps/fabro-server/src/run_title_generation.rs index 070f18d09..f54be4e17 100644 --- a/lib/apps/fabro-server/src/run_title_generation.rs +++ b/lib/apps/fabro-server/src/run_title_generation.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::sync::Arc; use std::time::Duration; -use fabro_llm::{Client, Request, structured}; +use fabro_llm::{Client, Request}; use fabro_template::{TemplateContext, TemplateError}; use fabro_types::{Graph, MAX_RUN_TITLE_CHARS, ProviderId, RunId}; use fabro_util::error; @@ -56,13 +56,10 @@ pub(crate) async fn generate_title_or_current(input: GenerateTitleInput<'_>) -> } }; - let completion = match structured::complete_object( - &input.client, - request, - "run_title", - title_response_schema(), - ) - .await + let completion = match input + .client + .complete_object(request, "run_title", title_response_schema()) + .await { Ok(completion) => completion, Err(err) => { diff --git a/lib/apps/fabro-server/src/server/handler/completions.rs b/lib/apps/fabro-server/src/server/handler/completions.rs index b043e54fc..febd0c939 100644 --- a/lib/apps/fabro-server/src/server/handler/completions.rs +++ b/lib/apps/fabro-server/src/server/handler/completions.rs @@ -2,7 +2,7 @@ use std::collections::HashSet; use std::sync::Arc; use fabro_llm::lithos_catalog::Catalog; -use fabro_llm::{ModelSelectionError, Request, selection, structured}; +use fabro_llm::{ModelSelectionError, Request, selection}; use fabro_types::{Message, Role}; use super::super::{ @@ -138,7 +138,10 @@ async fn create_completion( } if let Some(schema) = req.schema { - return match structured::complete_object(&client, request, "output_schema", schema).await { + return match client + .complete_object(request, "output_schema", schema) + .await + { Ok(completion) => { let mut body = match serde_json::to_value(&completion.response) { Ok(body) => body, diff --git a/lib/components/fabro-agent/src/error.rs b/lib/components/fabro-agent/src/error.rs index b3fb48592..21dca27f2 100644 --- a/lib/components/fabro-agent/src/error.rs +++ b/lib/components/fabro-agent/src/error.rs @@ -1,4 +1,4 @@ -use fabro_llm::LlmError; +use fabro_llm::ErrorData; /// Why a session was interrupted. #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] @@ -21,7 +21,7 @@ impl std::fmt::Display for InterruptReason { #[serde(tag = "type", content = "data", rename_all = "snake_case")] pub enum CompactionError { #[error("summary request failed: {0}")] - Llm(#[source] LlmError), + Llm(#[source] Box), #[error( "generated summary was empty after trimming; refused to replace \ @@ -34,9 +34,10 @@ pub enum CompactionError { #[serde(tag = "type", content = "data", rename_all = "snake_case")] pub enum Error { /// A provider call failed. Carries lithos's stored error projection so - /// the failure stays cloneable and serializable. + /// the failure stays cloneable and serializable. Boxed because the + /// projection is large and every other variant is small. #[error("LLM error: {0}")] - Llm(#[from] LlmError), + Llm(Box), #[error("Context compaction failed: {0}")] Compaction(#[from] CompactionError), @@ -54,15 +55,27 @@ pub enum Error { Interrupted(InterruptReason), } +impl From for Error { + fn from(error: ErrorData) -> Self { + Self::Llm(Box::new(error)) + } +} + impl From for Error { fn from(error: fabro_llm::Error) -> Self { - Self::Llm(LlmError::from(error)) + Self::from(ErrorData::from(error)) + } +} + +impl From for CompactionError { + fn from(error: ErrorData) -> Self { + Self::Llm(Box::new(error)) } } impl From for CompactionError { fn from(error: fabro_llm::Error) -> Self { - Self::Llm(LlmError::from(error)) + Self::from(ErrorData::from(error)) } } @@ -72,14 +85,14 @@ pub type Result = std::result::Result; mod tests { use std::time::Duration; - use fabro_llm::{ErrorFacts, ErrorKind, RetryClassification}; + use fabro_llm::{ErrorKind, RetryClassification}; use fabro_types::provider_ids; use fabro_util::error; use super::*; - fn network_error(message: &str) -> LlmError { - LlmError::from( + fn network_error(message: &str) -> ErrorData { + ErrorData::from( fabro_llm::Error::new(ErrorKind::Network, message) .with_retry(RetryClassification::Safe), ) @@ -95,7 +108,7 @@ mod tests { #[test] fn compaction_error_preserves_llm_source_chain() { - let err = Error::Compaction(CompactionError::Llm(network_error("connection refused"))); + let err = Error::Compaction(CompactionError::from(network_error("connection refused"))); let chain = error::collect_chain(&err); @@ -157,7 +170,7 @@ mod tests { #[test] fn serde_roundtrip_llm_network() { - let err = Error::Llm(network_error("connection refused")); + let err = Error::from(network_error("connection refused")); let json = serde_json::to_string(&err).unwrap(); let deserialized: Error = serde_json::from_str(&json).unwrap(); assert_eq!(err.to_string(), deserialized.to_string()); @@ -165,7 +178,7 @@ mod tests { #[test] fn serde_roundtrip_llm_provider() { - let err = Error::Llm(LlmError::from( + let err = Error::from(ErrorData::from( fabro_llm::Error::new(ErrorKind::RateLimit, "too fast") .with_provider(provider_ids::openai()) .with_status(429) @@ -229,7 +242,7 @@ mod tests { #[test] fn clone_all_variants() { let errors: Vec = vec![ - Error::Llm(network_error("refused")), + Error::from(network_error("refused")), Error::Compaction(CompactionError::EmptySummary { summarized_turn_count: 3, }), @@ -247,7 +260,7 @@ mod tests { #[test] fn serde_tag_format_llm() { - let err = Error::Llm(network_error("refused")); + let err = Error::from(network_error("refused")); let json = serde_json::to_string(&err).unwrap(); let v: serde_json::Value = serde_json::from_str(&json).unwrap(); assert_eq!(v["type"], "llm"); diff --git a/lib/components/fabro-agent/src/history.rs b/lib/components/fabro-agent/src/history.rs index c898d0dce..addd9325c 100644 --- a/lib/components/fabro-agent/src/history.rs +++ b/lib/components/fabro-agent/src/history.rs @@ -1,6 +1,5 @@ use std::collections::HashSet; -use fabro_llm::reasoning; use fabro_types::{Message as LlmMessage, SessionMessage, TokenCounts}; use crate::types::Message; @@ -87,7 +86,7 @@ impl History { fn strip_opaque_provider_items(&mut self) { for turn in &mut self.turns { if let Message::Assistant { provider_parts, .. } = turn { - provider_parts.retain(|p| !reasoning::is_opaque_openai(p)); + provider_parts.retain(|p| !p.is_opaque_openai()); } } } @@ -164,7 +163,7 @@ fn add_tool_result_call_ids<'a>(turns: &'a [Message], call_ids: &mut HashSet<&'a mod tests { use std::time::SystemTime; - use fabro_llm::reasoning::OPENAI_REASONING_KIND; + use fabro_llm::types::OPENAI_REASONING_KIND; use fabro_types::{ ContentPart, ReasoningContent, Role, TokenCounts, ToolCall, text_of, tool_result_from_json, }; diff --git a/lib/components/fabro-agent/src/session.rs b/lib/components/fabro-agent/src/session.rs index cc562da08..15500c1fd 100644 --- a/lib/components/fabro-agent/src/session.rs +++ b/lib/components/fabro-agent/src/session.rs @@ -4,8 +4,8 @@ use std::time::{Duration, Instant, SystemTime}; use fabro_llm::types::ContentBlockKind; use fabro_llm::{ - CallContext, Client, FinishReason, LlmError, Request, Response, RetryClassification, - RetryListener, RetryStage, StreamEvent, reasoning, + CallContext, Client, ErrorData, FinishReason, Request, Response, RetryClassification, + RetryListener, RetryStage, StreamEvent, }; use fabro_mcp::config::{McpServerSettings, McpTransport}; use fabro_mcp::connection_manager::McpConnectionManager; @@ -1138,14 +1138,14 @@ impl Session { } fn emit_llm_error(&mut self, err: fabro_llm::Error) -> Error { - let err = LlmError::from(err); + let err = ErrorData::from(err); self.event_emitter.emit(self.id.clone(), AgentEvent::Error { - error: Error::Llm(err.clone()), + error: Error::from(err.clone()), }); if err.is_auth_error() { self.transition(SessionState::Closed); } - Error::Llm(err) + Error::from(err) } #[must_use] @@ -1585,11 +1585,11 @@ impl Session { let text = response.text(); let tool_calls: Vec = response.tool_calls().cloned().collect(); // Normalize before the response's content moves into history. - let reasoning = reasoning::normalize(&response.content); + let reasoning = response.reasoning(); let provider_parts: Vec<_> = response .content .iter() - .filter(|part| reasoning::is_provider_part(part)) + .filter(|part| part.is_replay_material()) .cloned() .collect(); let usage = response.usage; @@ -1814,7 +1814,7 @@ impl Session { model: requested_model.model_id.to_string(), attempt: usize::try_from(replay_attempt).unwrap_or(usize::MAX), delay_secs: delay.as_secs_f64(), - error: LlmError::from(&error), + error: ErrorData::from(&error), phase: LlmRetryPhase::Consume, }); @@ -2243,10 +2243,11 @@ mod tests { use anyhow::Context as _; use fabro_llm::adapter::{ProviderAdapter, ResolvedCall}; use fabro_llm::lithos_catalog::AdapterId; - use fabro_llm::reasoning::OPENAI_COMPAT_REASONING_DETAILS_KIND; use fabro_llm::test_support::response_to_stream; - use fabro_llm::types::{ContentBlockId, ContentBlockKind, ToolCallKind}; - use fabro_llm::{ErrorFacts, ErrorKind, ResponseStream, RetryPolicy}; + use fabro_llm::types::{ + ContentBlockId, ContentBlockKind, OPENAI_COMPAT_REASONING_DETAILS_KIND, ToolCallKind, + }; + use fabro_llm::{ErrorKind, ResponseStream, RetryPolicy}; use fabro_types::{ ContentPart, Cost, CostSource, ReasoningOutput, StageContextWindowCountMethod, ToolDefinition, provider_ids, text_of, tool_result_to_json, diff --git a/lib/components/fabro-agent/src/types.rs b/lib/components/fabro-agent/src/types.rs index cf1627e03..b31458592 100644 --- a/lib/components/fabro-agent/src/types.rs +++ b/lib/components/fabro-agent/src/types.rs @@ -1,7 +1,7 @@ use std::time::SystemTime; use chrono::{DateTime, Utc}; -use fabro_llm::LlmError; +use fabro_llm::ErrorData; use fabro_types::{ CommandTermination, ContentPart, Cost, ExecOutputTail, LlmOutputKind, LlmRetryPhase, Message as LlmMessage, ModelRef, ReasoningOutput, Role, SessionMessage, Speed, @@ -401,7 +401,7 @@ pub enum AgentEvent { model: String, attempt: usize, delay_secs: f64, - error: LlmError, + error: ErrorData, phase: LlmRetryPhase, }, SubAgentSpawned { @@ -816,13 +816,13 @@ pub struct SessionEvent { #[cfg(test)] mod tests { - use fabro_llm::{ErrorFacts, ErrorKind, RetryClassification}; + use fabro_llm::{ErrorKind, RetryClassification}; use fabro_types::{CostSource, ModelId, ProviderId, provider_ids}; use super::*; - fn network_error(message: &str) -> LlmError { - LlmError::from( + fn network_error(message: &str) -> ErrorData { + ErrorData::from( fabro_llm::Error::new(ErrorKind::Network, message) .with_retry(RetryClassification::Safe), ) @@ -1152,7 +1152,7 @@ mod tests { #[test] fn error_event_serde_roundtrip_with_agent_error() { let event = AgentEvent::Error { - error: Error::Llm(network_error("refused")), + error: Error::from(network_error("refused")), }; let json = serde_json::to_string(&event).unwrap(); let deserialized: AgentEvent = serde_json::from_str(&json).unwrap(); @@ -1172,7 +1172,7 @@ mod tests { attempt: 1, delay_secs: 2.0, phase: LlmRetryPhase::Open, - error: LlmError::from( + error: ErrorData::from( fabro_llm::Error::new(ErrorKind::RateLimit, "too fast") .with_provider(ProviderId::new("openai")) .with_status(429) diff --git a/lib/components/fabro-hooks/src/executor.rs b/lib/components/fabro-hooks/src/executor.rs index 418a77270..28e86b08c 100644 --- a/lib/components/fabro-hooks/src/executor.rs +++ b/lib/components/fabro-hooks/src/executor.rs @@ -8,7 +8,7 @@ use fabro_agent::Sandbox; use fabro_agent::tool_registry::ToolContext; use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; -use fabro_llm::{Client, ClientOptions, Request, structured}; +use fabro_llm::{Client, ClientOptions, Request}; use fabro_redact::redacted_url_for_log; use fabro_types::settings::{InterpString, ResolveCtx, ResolveError}; use fabro_types::{Message, Role, ToolCall, tool_call_arguments, tool_result_from_json}; @@ -320,12 +320,8 @@ impl HookExecutorImpl { } }; - match structured::complete_object( - &client, - request, - "hook_response", - HOOK_RESPONSE_SCHEMA.clone(), - ) + match client + .complete_object(request, "hook_response", HOOK_RESPONSE_SCHEMA.clone()) .await { Ok(completion) => { diff --git a/lib/components/fabro-llm/Cargo.toml b/lib/components/fabro-llm/Cargo.toml index 546fd1f1e..cccc5fee2 100644 --- a/lib/components/fabro-llm/Cargo.toml +++ b/lib/components/fabro-llm/Cargo.toml @@ -19,7 +19,6 @@ workspace = true [dependencies] anyhow.workspace = true async-trait.workspace = true -base64.workspace = true bytes.workspace = true fabro-auth = { path = "../../foundation/fabro-auth" } fabro-config = { path = "../../foundation/fabro-config" } @@ -28,8 +27,7 @@ fabro-redact.workspace = true fabro-static.workspace = true fabro-types = { path = "../../foundation/fabro-types" } futures.workspace = true -lithos-llm = { workspace = true, features = ["builtin-catalog", "openai", "anthropic", "gemini", "openai-compatible", "bedrock", "bedrock-aws"] } -mime_guess = "2" +lithos-llm = { workspace = true, features = ["builtin-catalog", "openai", "anthropic", "gemini", "openai-compatible", "bedrock", "bedrock-aws", "local-files"] } serde.workspace = true serde_json.workspace = true strum.workspace = true @@ -45,5 +43,4 @@ fabro-llm = { path = ".", features = ["test-support"] } fabro-macros = { path = "../../foundation/fabro-macros" } fabro-test = { workspace = true } httpmock = "0.8" -tempfile = "3" tokio = { workspace = true, features = ["test-util", "macros"] } diff --git a/lib/components/fabro-llm/src/attachments.rs b/lib/components/fabro-llm/src/attachments.rs deleted file mode 100644 index 37c9c8432..000000000 --- a/lib/components/fabro-llm/src/attachments.rs +++ /dev/null @@ -1,268 +0,0 @@ -//! Inlines local file attachments before a request reaches a codec. -//! -//! lithos accepts media as a URL or as base64. Fabro lets a caller point an -//! image, document, or audio part at a local path; this middleware reads the -//! file and rewrites the part to inline base64 with an inferred media type. -//! A part whose file cannot be read is dropped, so the model sees the rest of -//! the message rather than a request that fails outright. - -use std::sync::Arc; - -use async_trait::async_trait; -use base64::Engine as _; -use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; -use fabro_static::EnvVars; -use lithos_llm::middleware::{Call, Middleware, Next, Output}; -use lithos_llm::types::{ - AudioContent, ContentPart, DocumentContent, Error, ImageContent, MediaSource, Message, Request, - ToolResult, -}; -use tokio::fs; - -/// Resolves an environment variable name to its value. -type EnvLookup = Arc Option + Send + Sync>; - -/// Middleware that inlines local-path media parts. -#[derive(Clone, Default)] -pub struct InlineLocalAttachments { - env_lookup: Option, -} - -impl std::fmt::Debug for InlineLocalAttachments { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("InlineLocalAttachments") - .finish_non_exhaustive() - } -} - -impl InlineLocalAttachments { - #[must_use] - pub fn new() -> Self { - Self::default() - } - - /// Resolves `~/` against this lookup instead of the process environment. - #[must_use] - pub fn with_env_lookup(env_lookup: EnvLookup) -> Self { - Self { - env_lookup: Some(env_lookup), - } - } - - #[expect( - clippy::disallowed_methods, - reason = "Attachment path expansion supports the conventional HOME env var." - )] - fn home(&self) -> Option { - match &self.env_lookup { - Some(lookup) => lookup(EnvVars::HOME), - None => std::env::var(EnvVars::HOME).ok(), - } - } - - fn expand(&self, path: &str) -> String { - path.strip_prefix("~/").map_or_else( - || path.to_string(), - |rest| format!("{}/{rest}", self.home().unwrap_or_else(|| "/".to_string())), - ) - } - - async fn load(&self, path: &str) -> Option { - let expanded = self.expand(path); - match fs::read(&expanded).await { - Ok(bytes) => Some(MediaSource::base64( - BASE64_STANDARD.encode(bytes), - media_type_for_path(&expanded), - )), - Err(err) => { - tracing::warn!(path = %expanded, error = %err, "dropping unreadable attachment"); - None - } - } - } - - async fn inline_part(&self, part: ContentPart) -> Option { - match part { - ContentPart::Image(ImageContent { source, detail }) if is_local_file(&source) => { - let source = self.load(url_of(&source)).await?; - Some(ContentPart::Image(ImageContent { source, detail })) - } - ContentPart::Document(DocumentContent { source, name }) if is_local_file(&source) => { - let source = self.load(url_of(&source)).await?; - Some(ContentPart::Document(DocumentContent { source, name })) - } - ContentPart::Audio(AudioContent { source }) if is_local_file(&source) => { - let source = self.load(url_of(&source)).await?; - Some(ContentPart::Audio(AudioContent { source })) - } - ContentPart::ToolResult(result) if result.content.iter().any(part_is_local_file) => { - let mut content = Vec::with_capacity(result.content.len()); - for part in result.content { - if let Some(part) = Box::pin(self.inline_part(part)).await { - content.push(part); - } - } - Some(ContentPart::ToolResult(ToolResult { content, ..result })) - } - other => Some(other), - } - } - - async fn inline_request(&self, request: Request) -> Request { - let mut messages = Vec::with_capacity(request.messages().len()); - for message in request.messages() { - let mut content = Vec::with_capacity(message.content().len()); - for part in message.content() { - if let Some(part) = self.inline_part(part.clone()).await { - content.push(part); - } - } - let mut rebuilt = Message::new(message.role(), content); - if let Some(name) = message.name() { - rebuilt = rebuilt.with_name(name); - } - if let Some(id) = message.tool_call_id() { - rebuilt = rebuilt.with_tool_call_id(id); - } - messages.push(rebuilt); - } - replace_messages(&request, messages).unwrap_or(request) - } -} - -/// Rebuilds `request` with `messages` in place of its own. -/// -/// The request builder appends messages and has no way to clear them, so the -/// swap goes through the request's serde form. -fn replace_messages(request: &Request, messages: Vec) -> Option { - let mut value = serde_json::to_value(request).ok()?; - value["messages"] = serde_json::to_value(messages).ok()?; - serde_json::from_value(value).ok() -} - -fn part_is_local_file(part: &ContentPart) -> bool { - match part { - ContentPart::Image(ImageContent { source, .. }) - | ContentPart::Document(DocumentContent { source, .. }) - | ContentPart::Audio(AudioContent { source }) => is_local_file(source), - _ => false, - } -} - -fn url_of(source: &MediaSource) -> &str { - match source { - MediaSource::Url { url, .. } => url, - _ => "", - } -} - -fn is_local_file(source: &MediaSource) -> bool { - matches!( - source, - MediaSource::Url { url, .. } - if url.starts_with('/') || url.starts_with("./") || url.starts_with("~/") - ) -} - -fn needs_inlining(request: &Request) -> bool { - request.messages().iter().any(|message| { - message.content().iter().any(|part| match part { - ContentPart::ToolResult(result) => result.content.iter().any(part_is_local_file), - part => part_is_local_file(part), - }) - }) -} - -/// Media type for a local path, from its extension. -#[must_use] -pub fn media_type_for_path(path: &str) -> String { - mime_guess::from_path(path) - .first_raw() - .unwrap_or("application/octet-stream") - .to_string() -} - -#[async_trait] -impl Middleware for InlineLocalAttachments { - async fn handle(&self, call: Call, next: Next) -> Result { - if !needs_inlining(call.request()) { - return next.run(call).await; - } - let inlined = self.inline_request(call.request().clone()).await; - let call = call.map_request(|_| Ok(inlined))?; - next.run(call).await - } -} - -#[cfg(test)] -mod tests { - use lithos_llm::types::Role; - - use super::*; - - fn request_with(part: ContentPart) -> Request { - Request::builder() - .model("openai/gpt-5.4") - .message(Message::new(Role::User, [ - ContentPart::Text { - text: "look".to_string(), - }, - part, - ])) - .build() - .unwrap() - } - - #[tokio::test] - async fn inlines_local_images_and_drops_missing_files() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("pixel.png"); - fs::write(&path, b"\x89PNG").await.unwrap(); - let middleware = InlineLocalAttachments::new(); - - let request = request_with(ContentPart::Image(ImageContent::new(MediaSource::url( - path.to_string_lossy().to_string(), - )))); - let inlined = middleware.inline_request(request).await; - match &inlined.messages()[0].content()[1] { - ContentPart::Image(image) => { - assert_eq!(image.source.media_type(), Some("image/png")); - assert_eq!( - image.source.base64_data(), - Some(BASE64_STANDARD.encode(b"\x89PNG").as_str()) - ); - } - other => panic!("expected inlined image, got {other:?}"), - } - - let missing = request_with(ContentPart::Document(DocumentContent::new( - MediaSource::url("/definitely/missing.pdf"), - ))); - let inlined = middleware.inline_request(missing).await; - assert_eq!(inlined.messages()[0].content().len(), 1); - } - - #[test] - fn remote_urls_and_inline_data_pass_through() { - let request = request_with(ContentPart::Image(ImageContent::new(MediaSource::url( - "https://example.com/a.png", - )))); - assert!(!needs_inlining(&request)); - let request = request_with(ContentPart::Image(ImageContent::new(MediaSource::base64( - "AAAA", - "image/png", - )))); - assert!(!needs_inlining(&request)); - let request = request_with(ContentPart::Image(ImageContent::new(MediaSource::url( - "~/shot.png", - )))); - assert!(needs_inlining(&request)); - } - - #[test] - fn media_types_follow_extensions() { - assert_eq!(media_type_for_path("a.jpg"), "image/jpeg"); - assert_eq!(media_type_for_path("a.pdf"), "application/pdf"); - assert_eq!(media_type_for_path("a.bin"), "application/octet-stream"); - } -} diff --git a/lib/components/fabro-llm/src/client.rs b/lib/components/fabro-llm/src/client.rs index 708819e7f..9a7ff7674 100644 --- a/lib/components/fabro-llm/src/client.rs +++ b/lib/components/fabro-llm/src/client.rs @@ -9,12 +9,9 @@ use lithos_llm::catalog::Catalog; use lithos_llm::client::{Client, ClientBuildError, ClientBuilder, ProviderBuildIssue}; use lithos_llm::credentials::{CredentialError, CredentialProvider}; use lithos_llm::middleware::{ - Call, Middleware, Observer, RetryMiddleware, RetryPolicy, RetryStage, + Call, InlineLocalFiles, Middleware, Observer, RetryMiddleware, RetryPolicy, RetryStage, }; -use lithos_llm::types::Error; - -use crate::attachments::InlineLocalAttachments; -use crate::error::LlmError; +use lithos_llm::types::{Error, ErrorData}; /// The application name lithos reports to providers that ask, such as the /// `originator` header on the OpenAI Codex deployment. @@ -36,7 +33,7 @@ pub fn default_retry_policy() -> RetryPolicy { #[derive(Clone, Debug)] pub struct RetryNotice { /// The failure that ended the attempt. - pub error: LlmError, + pub error: ErrorData, /// The attempt that failed, counted from 1. pub attempt: u32, /// How long the middleware waits before the next attempt. @@ -78,7 +75,7 @@ impl Observer for RetryNotifier { ) { if let Some(listener) = call.context().extensions().get::() { listener.notify(RetryNotice { - error: LlmError::from(error), + error: ErrorData::from(error), attempt, delay, stage, @@ -150,7 +147,7 @@ impl ClientOptions { builder = builder.middleware(retry_middleware(policy)); } if self.inline_attachments { - builder = builder.middleware(InlineLocalAttachments::new()); + builder = builder.middleware(InlineLocalFiles::new()); } for middleware in self.middleware { builder = builder.middleware_arc(middleware); diff --git a/lib/components/fabro-llm/src/error.rs b/lib/components/fabro-llm/src/error.rs index 05eec0e1c..6444b5597 100644 --- a/lib/components/fabro-llm/src/error.rs +++ b/lib/components/fabro-llm/src/error.rs @@ -1,270 +1,41 @@ -//! Classification of lithos errors for Fabro's retry, failover, and failure -//! signature policies, plus the stored form of a failure. +//! The one failure-classification rule that is Fabro's own. //! -//! lithos's live [`Error`] carries a source chain and is therefore neither -//! `Clone` nor serializable. Fabro records failures in events and agent -//! errors, so it works with [`LlmError`], a thin wrapper over lithos's own -//! [`ErrorData`] projection. Every policy here reads through [`ErrorFacts`] -//! and so applies to both forms. - -use std::fmt; -use std::time::Duration; +//! Retry, auth, cancellation, and failover questions are answered by the +//! lithos `Error` and `ErrorData` themselves. What stays here is the loop and +//! restart detector's signature format, which names Fabro's own categories. use fabro_types::ProviderId; -use lithos_llm::types::{Error, ErrorData, ErrorKind, RetryClassification}; -use serde::{Deserialize, Serialize}; - -/// The facts Fabro's policies read from an LLM failure. -pub trait ErrorFacts { - fn kind(&self) -> ErrorKind; - fn message(&self) -> &str; - fn provider(&self) -> Option<&ProviderId>; - fn provider_code(&self) -> Option<&str>; - fn status(&self) -> Option; - fn retry_classification(&self) -> RetryClassification; - - /// The delay the classification advises, when repeating is safe after - /// a wait. - fn retry_after(&self) -> Option { - self.retry_classification().delay() - } -} - -impl ErrorFacts for Error { - fn kind(&self) -> ErrorKind { - Self::kind(self) - } - - fn message(&self) -> &str { - Self::message(self) - } - - fn provider(&self) -> Option<&ProviderId> { - Self::provider(self) - } - - fn provider_code(&self) -> Option<&str> { - Self::provider_code(self) - } - - fn status(&self) -> Option { - Self::status(self) - } - - fn retry_classification(&self) -> RetryClassification { - Self::retry_classification(self) - } -} - -impl ErrorFacts for ErrorData { - fn kind(&self) -> ErrorKind { - self.kind.clone() - } - - fn message(&self) -> &str { - &self.message - } - - fn provider(&self) -> Option<&ProviderId> { - self.provider.as_ref() - } - - fn provider_code(&self) -> Option<&str> { - self.provider_code.as_deref() - } - - fn status(&self) -> Option { - self.status - } - - fn retry_classification(&self) -> RetryClassification { - self.retry - } -} - -/// A cloneable, serializable LLM failure. -/// -/// This is lithos's [`ErrorData`] projection with Fabro's policy helpers -/// attached. It is what agent errors, run events, and API responses carry; -/// the live [`Error`] converts into it at the boundary where a failure stops -/// being handled and starts being recorded. -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -#[serde(transparent)] -pub struct LlmError(Box); - -impl LlmError { - /// A failure Fabro itself raises, never retried. - #[must_use] - pub fn new(kind: ErrorKind, message: impl Into) -> Self { - Self::from(Error::new(kind, message)) - } - - #[must_use] - pub fn data(&self) -> &ErrorData { - &self.0 - } - - #[must_use] - pub fn into_data(self) -> ErrorData { - *self.0 - } - - /// The immediate source of the failure, rendered as text. - #[must_use] - pub fn source_message(&self) -> Option<&str> { - self.0.source_message.as_deref() - } - - /// The provider's advised wait, whatever the error kind. - #[must_use] - pub fn provider_retry_after(&self) -> Option { - self.0 - .provider_retry_after_millis - .map(Duration::from_millis) - } - - #[must_use] - pub fn is_retryable(&self) -> bool { - is_retryable(self) - } - - #[must_use] - pub fn is_auth_error(&self) -> bool { - is_auth_error(self) - } - - #[must_use] - pub fn is_cancelled(&self) -> bool { - is_cancelled(self) - } - - #[must_use] - pub fn failover_eligible(&self) -> bool { - failover_eligible(self) - } - - #[must_use] - pub fn failure_signature_hint(&self) -> String { - failure_signature_hint(self) - } -} - -impl ErrorFacts for LlmError { - fn kind(&self) -> ErrorKind { - self.0.kind.clone() - } - - fn message(&self) -> &str { - &self.0.message - } - - fn provider(&self) -> Option<&ProviderId> { - self.0.provider.as_ref() - } - - fn provider_code(&self) -> Option<&str> { - self.0.provider_code.as_deref() - } - - fn status(&self) -> Option { - self.0.status - } - - fn retry_classification(&self) -> RetryClassification { - self.0.retry - } -} - -impl fmt::Display for LlmError { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str(&self.0.message) - } -} - -impl std::error::Error for LlmError {} - -impl From for LlmError { - fn from(error: Error) -> Self { - Self(Box::new(error.data())) - } -} - -impl From<&Error> for LlmError { - fn from(error: &Error) -> Self { - Self(Box::new(error.data())) - } -} - -impl From for LlmError { - fn from(data: ErrorData) -> Self { - Self(Box::new(data)) - } -} - -/// Whether repeating the same call on the same provider may succeed. -#[must_use] -pub fn is_retryable(error: &E) -> bool { - !matches!(error.retry_classification(), RetryClassification::Never) -} - -/// Whether the failure came from a credential problem. -#[must_use] -pub fn is_auth_error(error: &E) -> bool { - matches!( - error.kind(), - ErrorKind::Authentication | ErrorKind::AccessDenied - ) -} - -/// Whether the call was cancelled by Fabro rather than failed by the provider. -#[must_use] -pub fn is_cancelled(error: &E) -> bool { - error.kind() == ErrorKind::Cancelled -} - -/// Whether another provider is worth trying. -/// -/// Everything retryable qualifies, plus failures that are local to this -/// provider: credentials, access policy, model inventory, quota, and a -/// provider that ran out of time. A different provider has its own. -#[must_use] -pub fn failover_eligible(error: &E) -> bool { - if is_retryable(error) { - return true; - } - matches!( - error.kind(), - ErrorKind::Authentication - | ErrorKind::AccessDenied - | ErrorKind::NotFound - | ErrorKind::QuotaExceeded - | ErrorKind::RateLimit - | ErrorKind::Server - | ErrorKind::Network - | ErrorKind::Timeout - | ErrorKind::StreamDecode - ) || (error.kind() == ErrorKind::ContentFilter && error.provider_code() == Some("refusal")) -} +use lithos_llm::types::{ErrorData, ErrorKind}; /// A stable `category|provider|detail` string for loop and restart detection. +/// +/// The category is `api_canceled` for a cancelled call, `api_transient` for a +/// failure the provider may be asked to repeat, and `api_deterministic` for +/// everything else; the detail is the error kind's stored spelling. #[must_use] -pub fn failure_signature_hint(error: &E) -> String { +pub fn failure_signature_hint(error: &ErrorData) -> String { let provider = error.provider().map_or("unknown", ProviderId::as_str); - let category = match error.kind() { - ErrorKind::Cancelled => "api_canceled", - _ if is_retryable(error) => "api_transient", - _ => "api_deterministic", + let category = if error.is_cancelled() { + "api_canceled" + } else if error.is_retryable() { + "api_transient" + } else { + "api_deterministic" }; - let detail = error.kind().as_str().to_string(); - format!("{category}|{provider}|{detail}") + let kind: ErrorKind = error.kind(); + format!("{category}|{provider}|{}", kind.as_str()) } #[cfg(test)] mod tests { + use lithos_llm::types::{Error, RetryClassification}; + use super::*; - fn error(kind: ErrorKind) -> Error { - Error::new(kind, "boom").with_provider(ProviderId::new("openai")) + fn error(kind: ErrorKind) -> ErrorData { + Error::new(kind, "boom") + .with_provider(ProviderId::new("openai")) + .data() } #[test] @@ -275,7 +46,10 @@ mod tests { ); assert_eq!( failure_signature_hint( - &error(ErrorKind::RateLimit).with_retry(RetryClassification::Safe) + &Error::new(ErrorKind::RateLimit, "boom") + .with_provider(ProviderId::new("openai")) + .with_retry(RetryClassification::Safe) + .data() ), "api_transient|openai|rate_limit" ); @@ -284,42 +58,4 @@ mod tests { "api_canceled|openai|cancelled" ); } - - #[test] - fn failover_covers_provider_local_failures() { - assert!(failover_eligible(&error(ErrorKind::Authentication))); - assert!(failover_eligible(&error(ErrorKind::QuotaExceeded))); - assert!(!failover_eligible(&error(ErrorKind::InvalidRequest))); - assert!(!failover_eligible(&error(ErrorKind::ContextLength))); - assert!(!failover_eligible(&error(ErrorKind::ContentFilter))); - assert!(failover_eligible( - &error(ErrorKind::ContentFilter).with_provider_code("refusal") - )); - } - - #[test] - fn stored_errors_keep_the_facts_and_round_trip() { - let live = error(ErrorKind::RateLimit) - .with_status(429) - .with_provider_code("slow") - .with_retry(RetryClassification::after(Duration::from_secs(2))) - .with_source(std::io::Error::other("socket closed")); - let stored = LlmError::from(&live); - assert_eq!(stored.kind(), ErrorKind::RateLimit); - assert_eq!(stored.status(), Some(429)); - assert_eq!(stored.provider_code(), Some("slow")); - assert_eq!(stored.retry_after(), Some(Duration::from_secs(2))); - assert_eq!(stored.source_message(), Some("socket closed")); - assert_eq!(stored.to_string(), "boom"); - assert!(stored.is_retryable()); - assert_eq!( - stored.failure_signature_hint(), - failure_signature_hint(&live) - ); - - let json = serde_json::to_value(&stored).unwrap(); - assert_eq!(json["kind"], "rate_limit"); - let decoded: LlmError = serde_json::from_value(json).unwrap(); - assert_eq!(decoded, stored); - } } diff --git a/lib/components/fabro-llm/src/lib.rs b/lib/components/fabro-llm/src/lib.rs index 4e325b1c1..27199a2ed 100644 --- a/lib/components/fabro-llm/src/lib.rs +++ b/lib/components/fabro-llm/src/lib.rs @@ -8,28 +8,23 @@ //! - Fabro's passthrough policy for selections made before a request exists //! ([`selection`]); at request time the lithos resolver enforces `enabled` //! and `stands_in_for` itself; -//! - constructing a client from a Fabro credential source ([`client`]); -//! - inlining local file attachments ([`attachments`]); -//! - normalizing readable reasoning into [`fabro_types::ReasoningOutput`] -//! ([`reasoning`]); -//! - one-shot structured output ([`structured`]); +//! - constructing a client from a Fabro credential store ([`client`]); //! - model and provider probes ([`probe`]), and the API views of the catalog //! ([`api`]); //! - the `fabro exec` gateway adapter that speaks to a Fabro server //! ([`gateway`]); -//! - error classification for retries, failover, and failure signatures -//! ([`error`]). +//! - the failure signature loop detection reads ([`error`]). +//! +//! Local-file inlining, structured output, readable-reasoning normalization, +//! and the retry, auth, and failover predicates are lithos-llm's own. pub mod api; -pub mod attachments; pub mod catalog; pub mod client; pub mod error; pub mod gateway; pub mod probe; -pub mod reasoning; pub mod selection; -pub mod structured; #[cfg(any(test, feature = "test-support"))] pub mod test_support; @@ -38,7 +33,7 @@ pub use client::{ ClientOptions, FabroClient, LlmSetupError, RetryListener, RetryNotice, build_client, build_offline_client, configured_providers, }; -pub use error::{ErrorFacts, LlmError}; +pub use error::failure_signature_hint; pub use lithos_llm::client::{Client, ClientBuild}; pub use lithos_llm::middleware::{CallContext, CancellationToken, RetryPolicy, RetryStage}; pub use lithos_llm::resolver::ModelSelectionError as RouteSelectionError; diff --git a/lib/components/fabro-llm/src/reasoning.rs b/lib/components/fabro-llm/src/reasoning.rs deleted file mode 100644 index ca34594d2..000000000 --- a/lib/components/fabro-llm/src/reasoning.rs +++ /dev/null @@ -1,292 +0,0 @@ -//! Normalization of provider reasoning material into [`ReasoningOutput`]. -//! -//! Every provider that returns readable reasoning does it differently, and -//! several return more than one channel at once. This module reduces a final -//! response's content parts to the two normalized fields without reaching -//! into opaque material (signatures, item ids, encrypted payloads) and -//! without failing a completion it cannot classify. -//! -//! Parsing is deliberately tolerant: provider payloads are read as -//! `serde_json::Value` with optional lookups, so unknown detail variants, -//! missing members, extra members, and unexpected member types are ignored -//! rather than surfaced as errors. - -use fabro_types::{ContentPart, ReasoningOutput}; - -/// OpenAI Responses reasoning items, as lithos stores them. -pub const OPENAI_REASONING_KIND: &str = "openai.reasoning"; -/// OpenAI Responses message items, as lithos stores them. -pub const OPENAI_MESSAGE_KIND: &str = "openai.message"; -/// OpenAI-compatible `reasoning_details` arrays, as lithos stores them. -pub const OPENAI_COMPAT_REASONING_DETAILS_KIND: &str = "openai_compatible.reasoning_details"; - -/// Separator between distinct complete reasoning blocks. -const BLOCK_SEPARATOR: &str = "\n\n"; - -#[derive(Default)] -struct Blocks<'a> { - explicit_summary: Vec<&'a str>, - explicit_trace: Vec<&'a str>, - fallback_trace: Vec<&'a str>, -} - -impl Blocks<'_> { - fn into_output(self) -> Option { - let summary = join_blocks(&self.explicit_summary); - let trace = join_blocks(&self.explicit_trace) - .or_else(|| join_blocks(&self.fallback_trace)) - .filter(|trace| summary.as_ref() != Some(trace)); - - match (summary, trace) { - (Some(summary), Some(trace)) => Some(ReasoningOutput::new(summary, trace)), - (Some(summary), None) => Some(ReasoningOutput::from_summary(summary)), - (None, Some(trace)) => Some(ReasoningOutput::from_trace(trace)), - (None, None) => None, - } - } -} - -fn join_blocks(blocks: &[&str]) -> Option { - (!blocks.is_empty()).then(|| blocks.join(BLOCK_SEPARATOR)) -} - -fn push_block<'a>(blocks: &mut Vec<&'a str>, block: &'a str) { - if !block.trim().is_empty() { - blocks.push(block); - } -} - -fn readable_member<'a>(entry: &'a serde_json::Value, member: &str) -> Option<&'a str> { - entry.get(member).and_then(serde_json::Value::as_str) -} - -fn collect_openai_reasoning_item<'a>(item: &'a serde_json::Value, blocks: &mut Blocks<'a>) { - if let Some(entries) = item.get("summary").and_then(serde_json::Value::as_array) { - for entry in entries { - if let Some(text) = entry.as_str() { - push_block(&mut blocks.explicit_summary, text); - } else if let Some(text) = entry.get("text").and_then(serde_json::Value::as_str) { - push_block(&mut blocks.explicit_summary, text); - } - } - } - if let Some(entries) = item.get("content").and_then(serde_json::Value::as_array) { - for entry in entries { - let Some(text) = entry.get("text").and_then(serde_json::Value::as_str) else { - continue; - }; - let entry_type = entry - .get("type") - .and_then(serde_json::Value::as_str) - .unwrap_or_default(); - if entry_type == "reasoning_text" { - push_block(&mut blocks.explicit_trace, text); - } - } - } -} - -fn collect_reasoning_details<'a>(details: &'a serde_json::Value, blocks: &mut Blocks<'a>) { - let Some(entries) = details.as_array() else { - return; - }; - for entry in entries { - let detail_type = entry - .get("type") - .and_then(serde_json::Value::as_str) - .unwrap_or_default(); - match detail_type { - "reasoning.text" => { - if let Some(text) = readable_member(entry, "text") { - push_block(&mut blocks.explicit_trace, text); - } - } - "reasoning.summary" => { - if let Some(text) = readable_member(entry, "summary") { - push_block(&mut blocks.explicit_summary, text); - } - } - _ => {} - } - } -} - -/// Normalizes the content parts of a final response into readable reasoning. -/// -/// Returns `None` when the response carries no readable reasoning. -#[must_use] -pub fn normalize(content: &[ContentPart]) -> Option { - let mut blocks = Blocks::default(); - for part in content { - match part { - ContentPart::Reasoning(reasoning) if !reasoning.redacted => { - push_block(&mut blocks.fallback_trace, &reasoning.text); - } - ContentPart::Opaque { kind, data } if kind == OPENAI_REASONING_KIND => { - collect_openai_reasoning_item(data, &mut blocks); - } - ContentPart::Opaque { kind, data } if kind == OPENAI_COMPAT_REASONING_DETAILS_KIND => { - collect_reasoning_details(data, &mut blocks); - } - _ => {} - } - } - blocks.into_output() -} - -/// Whether a part is provider-native replay material Fabro keeps in history -/// but never renders. -#[must_use] -pub fn is_provider_part(part: &ContentPart) -> bool { - matches!(part, ContentPart::Reasoning(_) | ContentPart::Opaque { .. }) -} - -/// Whether a part is an OpenAI Responses item tied to one specific API -/// response. Such items become invalid once compaction replaces their -/// surrounding context. -#[must_use] -pub fn is_opaque_openai(part: &ContentPart) -> bool { - matches!( - part, - ContentPart::Opaque { kind, .. } - if kind == OPENAI_REASONING_KIND || kind == OPENAI_MESSAGE_KIND - ) -} - -#[cfg(test)] -mod tests { - use fabro_types::ReasoningContent; - use serde_json::json; - - use super::*; - - fn thinking(text: &str) -> ContentPart { - ContentPart::Reasoning(ReasoningContent { - text: text.to_string(), - signature: None, - signature_origin: None, - redacted: false, - }) - } - - fn openai_reasoning(item: serde_json::Value) -> ContentPart { - ContentPart::opaque(OPENAI_REASONING_KIND, item) - } - - fn reasoning_details(details: serde_json::Value) -> ContentPart { - ContentPart::opaque(OPENAI_COMPAT_REASONING_DETAILS_KIND, details) - } - - #[test] - fn non_redacted_thinking_becomes_a_trace() { - let output = normalize(&[thinking("weighing the options")]).unwrap(); - assert!(output.summary().is_none()); - assert_eq!(output.trace(), Some("weighing the options")); - } - - #[test] - fn redacted_thinking_yields_no_readable_reasoning() { - let redacted = ContentPart::Reasoning(ReasoningContent { - text: "AAAAopaque".to_string(), - signature: Some("sig".to_string()), - signature_origin: Some("anthropic".to_string()), - redacted: true, - }); - assert!(normalize(&[redacted]).is_none()); - } - - #[test] - fn responses_item_with_summary_and_reasoning_text_produces_both_fields() { - let output = normalize(&[openai_reasoning(json!({ - "type": "reasoning", - "id": "rs_1", - "encrypted_content": "gAAAAA", - "summary": [{"type": "summary_text", "text": "inspect first"}], - "content": [{"type": "reasoning_text", "text": "step one"}], - }))]) - .unwrap(); - assert_eq!(output.summary(), Some("inspect first")); - assert_eq!(output.trace(), Some("step one")); - } - - #[test] - fn responses_blocks_join_in_provider_order() { - let output = normalize(&[openai_reasoning(json!({ - "summary": [ - {"type": "summary_text", "text": "first"}, - {"type": "summary_text", "text": "second"}, - ], - }))]) - .unwrap(); - assert_eq!(output.summary(), Some("first\n\nsecond")); - } - - #[test] - fn structured_details_produce_summary_and_trace() { - let output = normalize(&[reasoning_details(json!([ - {"type": "reasoning.summary", "summary": "checked the parser"}, - {"type": "reasoning.text", "text": "read convert.rs", "signature": "sig"}, - {"type": "reasoning.encrypted", "data": "gAAAAAsecret"}, - ]))]) - .unwrap(); - assert_eq!(output.summary(), Some("checked the parser")); - assert_eq!(output.trace(), Some("read convert.rs")); - } - - #[test] - fn malformed_details_are_ignored_without_failing() { - assert!(normalize(&[reasoning_details(json!("not-an-array"))]).is_none()); - assert!( - normalize(&[reasoning_details(json!([ - 42, - {"type": "reasoning.summary", "summary": 7}, - {"no_type": true}, - ]))]) - .is_none() - ); - } - - #[test] - fn structured_details_suppress_a_duplicate_flattened_value() { - let output = normalize(&[ - reasoning_details(json!([ - {"type": "reasoning.summary", "summary": "checked the parser"}, - ])), - thinking("checked the parser"), - ]) - .unwrap(); - assert_eq!(output.summary(), Some("checked the parser")); - assert!(output.trace().is_none()); - } - - #[test] - fn structured_trace_takes_precedence_over_flattened_trace() { - let output = normalize(&[ - reasoning_details(json!([{"type": "reasoning.text", "text": "verbatim"}])), - thinking("flattened"), - ]) - .unwrap(); - assert_eq!(output.trace(), Some("verbatim")); - } - - #[test] - fn whitespace_only_fragments_do_not_create_reasoning() { - assert!(normalize(&[thinking(" \n ")]).is_none()); - let output = normalize(&[thinking(" indented thought\n")]).unwrap(); - assert_eq!(output.trace(), Some(" indented thought\n")); - } - - #[test] - fn opaque_openai_items_are_recognized() { - assert!(is_opaque_openai(&openai_reasoning(json!({})))); - assert!(is_opaque_openai(&ContentPart::opaque( - OPENAI_MESSAGE_KIND, - json!({}) - ))); - assert!(!is_opaque_openai(&thinking("x"))); - assert!(is_provider_part(&thinking("x"))); - assert!(!is_provider_part(&ContentPart::Text { - text: "x".to_string(), - })); - } -} diff --git a/lib/components/fabro-llm/src/structured.rs b/lib/components/fabro-llm/src/structured.rs deleted file mode 100644 index 3bc5b730e..000000000 --- a/lib/components/fabro-llm/src/structured.rs +++ /dev/null @@ -1,103 +0,0 @@ -//! One-shot structured output. - -use lithos_llm::client::Client; -use lithos_llm::middleware::CallContext; -use lithos_llm::types::{Error, ErrorKind, Request, Response, ResponseFormat}; - -/// A completion whose text parsed as the requested JSON object. -#[derive(Debug, Clone)] -pub struct StructuredCompletion { - pub response: Response, - pub object: serde_json::Value, -} - -/// Completes `request` under a JSON schema and parses the reply. -/// -/// The schema is attached as the request's response format, so providers -/// with native structured output enforce it. The reply text must still parse -/// as JSON; a reply that does not is a `ResponseDecode` error. -pub async fn complete_object( - client: &Client, - request: Request, - schema_name: &str, - schema: serde_json::Value, -) -> Result { - complete_object_with_context(client, request, schema_name, schema, CallContext::new()).await -} - -pub async fn complete_object_with_context( - client: &Client, - request: Request, - schema_name: &str, - schema: serde_json::Value, - context: CallContext, -) -> Result { - let request = request - .into_builder() - .response_format(ResponseFormat::JsonSchema { - name: schema_name.to_string(), - schema, - }) - .build() - .map_err(|source| { - Error::new( - ErrorKind::InvalidRequest, - "structured output request is invalid", - ) - .with_source(source) - })?; - let response = client.complete_with_context(request, context).await?; - let object = parse_object(&response)?; - Ok(StructuredCompletion { response, object }) -} - -/// Parses a response's JSON output: a `Json` part when the provider returned -/// one, else the concatenated text. -pub fn parse_object(response: &Response) -> Result { - if let Some(value) = response.content.iter().find_map(|part| match part { - fabro_types::ContentPart::Json { value } => Some(value.clone()), - _ => None, - }) { - return Ok(value); - } - let text = response.text(); - serde_json::from_str(text.trim()).map_err(|source| { - Error::new( - ErrorKind::ResponseDecode, - format!("the model did not return a JSON object: {source}"), - ) - .with_provider(response.model.provider().clone()) - .with_source(source) - }) -} - -#[cfg(test)] -mod tests { - use fabro_types::{ContentPart, ModelId, ProviderId}; - use serde_json::json; - - use super::*; - - fn response(parts: Vec) -> Response { - Response::new(ProviderId::new("openai"), ModelId::new("gpt-5.4"), parts) - } - - #[test] - fn parses_text_or_json_parts() { - let text = response(vec![ContentPart::Text { - text: " {\"title\": \"x\"} ".to_string(), - }]); - assert_eq!(parse_object(&text).unwrap(), json!({"title": "x"})); - let json = response(vec![ContentPart::Json { - value: json!({"a": 1}), - }]); - assert_eq!(parse_object(&json).unwrap(), json!({"a": 1})); - let prose = response(vec![ContentPart::Text { - text: "sorry".to_string(), - }]); - assert_eq!( - parse_object(&prose).unwrap_err().kind(), - ErrorKind::ResponseDecode - ); - } -} diff --git a/lib/components/fabro-workflow/src/error.rs b/lib/components/fabro-workflow/src/error.rs index d0b7eae10..552c5da33 100644 --- a/lib/components/fabro-workflow/src/error.rs +++ b/lib/components/fabro-workflow/src/error.rs @@ -2,7 +2,7 @@ use std::fmt; use std::sync::{Arc, LazyLock}; use fabro_graphviz::Error as GraphvizError; -use fabro_llm::{ErrorFacts, ErrorKind, LlmError, ModelSelectionError}; +use fabro_llm::{ErrorData, ErrorKind, ModelSelectionError, failure_signature_hint}; use fabro_template::TemplateError; pub use fabro_types::failure_signature::FailureSignature; pub use fabro_types::outcome::FailureCategory; @@ -18,7 +18,7 @@ use crate::outcome::{FailureDetail, Outcome, StageOutcome}; /// Classify an LLM error into a `FailureCategory` based on its structure. #[must_use] -pub fn classify_sdk_error(err: &E) -> FailureCategory { +pub fn classify_sdk_error(err: &ErrorData) -> FailureCategory { match err.kind() { ErrorKind::RateLimit | ErrorKind::Server @@ -309,7 +309,7 @@ pub enum Error { }, #[error("LLM error: {0}")] - Llm(LlmError), + Llm(Box), #[error("Checkpoint error: {0}")] Checkpoint(String), @@ -583,7 +583,7 @@ impl Error { #[must_use] pub fn failure_signature_hint(&self) -> Option { match self { - Self::Llm(sdk_err) => Some(FailureSignature(sdk_err.failure_signature_hint())), + Self::Llm(sdk_err) => Some(FailureSignature(failure_signature_hint(sdk_err))), _ => None, } } @@ -684,15 +684,15 @@ impl From for Error { } } -impl From for Error { - fn from(err: LlmError) -> Self { - Self::Llm(err) +impl From for Error { + fn from(err: ErrorData) -> Self { + Self::Llm(Box::new(err)) } } impl From for Error { fn from(err: fabro_llm::Error) -> Self { - Self::Llm(LlmError::from(err)) + Self::from(ErrorData::from(err)) } } @@ -749,15 +749,15 @@ mod tests { use super::*; /// A stored LLM error of `kind` from the `openai` provider. - fn sdk_error(kind: ErrorKind, message: &str) -> LlmError { - LlmError::from( + fn sdk_error(kind: ErrorKind, message: &str) -> ErrorData { + ErrorData::from( fabro_llm::Error::new(kind, message).with_provider(fabro_types::provider_ids::openai()), ) } /// A transient failure the provider may be asked to repeat. - fn transient_error(kind: ErrorKind, message: &str) -> LlmError { - LlmError::from( + fn transient_error(kind: ErrorKind, message: &str) -> ErrorData { + ErrorData::from( fabro_llm::Error::new(kind, message) .with_provider(fabro_types::provider_ids::openai()) .with_retry(RetryClassification::Safe), @@ -1157,16 +1157,16 @@ mod tests { #[test] fn llm_error_display() { let sdk_err = transient_error(ErrorKind::Network, "connection refused"); - let err = Error::Llm(sdk_err); + let err = Error::from(sdk_err); assert_eq!(err.to_string(), "LLM error: connection refused"); } #[test] fn llm_error_retryable_delegates_to_sdk() { - let retryable = Error::Llm(transient_error(ErrorKind::Network, "timeout")); + let retryable = Error::from(transient_error(ErrorKind::Network, "timeout")); assert!(retryable.is_retryable()); - let non_retryable = Error::Llm(sdk_error(ErrorKind::Configuration, "bad config")); + let non_retryable = Error::from(sdk_error(ErrorKind::Configuration, "bad config")); assert!(!non_retryable.is_retryable()); } @@ -1221,31 +1221,31 @@ mod tests { #[test] fn failure_class_llm_rate_limit() { - let err = Error::Llm(transient_error(ErrorKind::RateLimit, "too fast")); + let err = Error::from(transient_error(ErrorKind::RateLimit, "too fast")); assert_eq!(err.failure_category(), FailureCategory::TransientInfra); } #[test] fn failure_class_llm_context_length() { - let err = Error::Llm(sdk_error(ErrorKind::ContextLength, "too long")); + let err = Error::from(sdk_error(ErrorKind::ContextLength, "too long")); assert_eq!(err.failure_category(), FailureCategory::BudgetExhausted); } #[test] fn failure_class_llm_auth() { - let err = Error::Llm(sdk_error(ErrorKind::Authentication, "bad key")); + let err = Error::from(sdk_error(ErrorKind::Authentication, "bad key")); assert_eq!(err.failure_category(), FailureCategory::Deterministic); } #[test] fn failure_class_llm_abort() { - let err = Error::Llm(sdk_error(ErrorKind::Cancelled, "user cancelled")); + let err = Error::from(sdk_error(ErrorKind::Cancelled, "user cancelled")); assert_eq!(err.failure_category(), FailureCategory::Canceled); } #[test] fn failure_class_llm_timeout() { - let err = Error::Llm(transient_error(ErrorKind::Timeout, "timed out")); + let err = Error::from(transient_error(ErrorKind::Timeout, "timed out")); assert_eq!(err.failure_category(), FailureCategory::TransientInfra); } @@ -1941,7 +1941,7 @@ mod tests { #[test] fn failure_signature_hint_llm_returns_some() { - let err = Error::Llm(sdk_error(ErrorKind::Authentication, "bad key")); + let err = Error::from(sdk_error(ErrorKind::Authentication, "bad key")); assert_eq!( err.failure_signature_hint(), Some(FailureSignature( @@ -1966,7 +1966,7 @@ mod tests { #[test] fn to_fail_outcome_llm_has_class_and_signature() { - let err = Error::Llm(sdk_error(ErrorKind::Authentication, "bad key")); + let err = Error::from(sdk_error(ErrorKind::Authentication, "bad key")); let outcome = err.to_fail_outcome(); assert_eq!(outcome.status, crate::outcome::StageOutcome::Failed { retry_requested: false, @@ -1993,7 +1993,7 @@ mod tests { #[test] fn to_fail_outcome_includes_error_message_as_reason() { - let err = Error::Llm(transient_error(ErrorKind::Network, "connection refused")); + let err = Error::from(transient_error(ErrorKind::Network, "connection refused")); let outcome = err.to_fail_outcome(); assert!( outcome @@ -2005,7 +2005,7 @@ mod tests { #[test] fn to_fail_outcome_no_context_updates() { - let err = Error::Llm(transient_error(ErrorKind::Network, "refused")); + let err = Error::from(transient_error(ErrorKind::Network, "refused")); let outcome = err.to_fail_outcome(); assert!(outcome.context_updates.is_empty()); } @@ -2057,7 +2057,7 @@ mod tests { Error::engine("engine err"), Error::publish("publish err"), Error::handler("handler err"), - Error::Llm(transient_error(ErrorKind::Network, "refused")), + Error::from(transient_error(ErrorKind::Network, "refused")), Error::Checkpoint("cp err".into()), Error::Stylesheet("style err".into()), Error::Io("io err".into()), @@ -2165,7 +2165,7 @@ mod tests { // 1. Create SdkError → Error let sdk_err = transient_error(ErrorKind::RateLimit, "too fast"); - let arc_err = Error::Llm(sdk_err); + let arc_err = Error::from(sdk_err); assert_eq!(arc_err.failure_category(), FailureCategory::TransientInfra); // 2. Error → Outcome @@ -2236,7 +2236,7 @@ mod tests { fn e2e_serde_stability_agent_error() { use fabro_agent::Error as AgentError; - let err = AgentError::Llm(transient_error(ErrorKind::RateLimit, "too fast")); + let err = AgentError::from(transient_error(ErrorKind::RateLimit, "too fast")); let json = serde_json::to_string(&err).unwrap(); let v: serde_json::Value = serde_json::from_str(&json).unwrap(); assert_eq!(v["type"], "llm"); diff --git a/lib/components/fabro-workflow/src/handler/llm/api.rs b/lib/components/fabro-workflow/src/handler/llm/api.rs index d254ca5f1..c2eeec956 100644 --- a/lib/components/fabro-workflow/src/handler/llm/api.rs +++ b/lib/components/fabro-workflow/src/handler/llm/api.rs @@ -12,10 +12,9 @@ use fabro_agent::{ }; use fabro_graphviz::graph::{AttrValue, Node}; use fabro_llm::credentials::CredentialProvider; -use fabro_llm::error::failover_eligible; use fabro_llm::lithos_catalog::Catalog; use fabro_llm::types::ResponseFormat; -use fabro_llm::{Client, ClientOptions, FallbackTarget, LlmError, Request, Response}; +use fabro_llm::{Client, ClientOptions, ErrorData, FallbackTarget, Request, Response}; use fabro_mcp::config::McpServerSettings; use fabro_types::settings::run::RunModelControls; use fabro_types::{ @@ -110,7 +109,7 @@ enum AgentApiErrorDisposition { /// Session was interrupted via cancellation; surface as `Error::Cancelled`. Cancelled, /// Underlying LLM error eligible for provider failover. - FailoverEligible(LlmError), + FailoverEligible(ErrorData), /// Terminal error; abort the invocation with this workflow `Error`. Terminal(Error), } @@ -132,7 +131,7 @@ fn classify_agent_error(err: fabro_agent::Error, allow_failover: bool) -> AgentA )) } fabro_agent::Error::Llm(err) if allow_failover && err.failover_eligible() => { - AgentApiErrorDisposition::FailoverEligible(err) + AgentApiErrorDisposition::FailoverEligible(*err) } fabro_agent::Error::Llm(err) => AgentApiErrorDisposition::Terminal(Error::Llm(err)), other @ (fabro_agent::Error::SessionClosed @@ -755,7 +754,7 @@ impl LiveAgentInvocation { error: fabro_agent::Error, allow_failover: bool, emitter: &Arc, - ) -> Result { + ) -> Result { let disposition = classify_agent_error(error, allow_failover); self.abort_and_discard(emitter).await; match disposition { @@ -1197,7 +1196,7 @@ impl AgentApiBackend { async fn failover_agent_session( &self, fallback_plan: &mut FallbackPlan, - initial_error: LlmError, + initial_error: ErrorData, request: &CodergenRunRequest<'_>, input: &str, stage_scope: &StageScope, @@ -1205,7 +1204,7 @@ impl AgentApiBackend { live: &mut LiveAgentInvocation, ) -> Result<(), Error> { let emitter = request.emitter; - let mut last_error = Error::Llm(initial_error); + let mut last_error = Error::from(initial_error); while fallback_plan.advance() { Self::emit_failover( @@ -1269,7 +1268,7 @@ impl AgentApiBackend { begin_session_lifecycle(&live.session, emitter, None); if let Err(error) = live.session.initialize().await { let allow_failover = fallback_plan.has_next(); - last_error = Error::Llm( + last_error = Error::from( live.discard_for_error(error, allow_failover, emitter) .await?, ); @@ -1302,7 +1301,7 @@ impl AgentApiBackend { } Err(error) => { let allow_failover = fallback_plan.has_next(); - last_error = Error::Llm( + last_error = Error::from( live.discard_for_error(error, allow_failover, emitter) .await?, ); @@ -1431,7 +1430,7 @@ impl AgentApiBackend { .with_speed(route.controls.speed), }); } - Err(error) if failover_eligible(&error) && plan.has_next() => { + Err(error) if error.failover_eligible() && plan.has_next() => { let error_message = error.to_string(); plan.advance(); Self::emit_failover(node, emitter, stage_scope, plan, &error_message); @@ -1442,7 +1441,7 @@ impl AgentApiBackend { request.response_format().cloned(), )?; } - Err(error) => return Err(Error::Llm(LlmError::from(error))), + Err(error) => return Err(Error::from(error)), } } } @@ -4032,16 +4031,16 @@ capabilities = {{ text = true, tools = true, response_format = {{ json_object = // --- Bridge guard tests --- - fn failover_eligible_llm_error() -> LlmError { - LlmError::from( + fn failover_eligible_llm_error() -> ErrorData { + ErrorData::from( fabro_llm::Error::new(ErrorKind::Network, "boom") .with_provider(provider_ids::openai()) .with_retry(RetryClassification::Safe), ) } - fn non_failover_llm_error() -> LlmError { - LlmError::from( + fn non_failover_llm_error() -> ErrorData { + ErrorData::from( fabro_llm::Error::new(ErrorKind::InvalidRequest, "bad key") .with_provider(provider_ids::openai()) .with_status(401), @@ -4270,7 +4269,7 @@ profile = "anthropic" #[test] fn classify_failover_eligible_llm_returns_failover_when_allowed() { - let err = fabro_agent::Error::Llm(failover_eligible_llm_error()); + let err = fabro_agent::Error::from(failover_eligible_llm_error()); assert!(matches!( classify_agent_error(err, true), AgentApiErrorDisposition::FailoverEligible(_) @@ -4279,7 +4278,7 @@ profile = "anthropic" #[test] fn classify_failover_eligible_llm_returns_terminal_when_not_allowed() { - let err = fabro_agent::Error::Llm(failover_eligible_llm_error()); + let err = fabro_agent::Error::from(failover_eligible_llm_error()); match classify_agent_error(err, false) { AgentApiErrorDisposition::Terminal(Error::Llm(_)) => {} _ => panic!("expected Terminal(Error::Llm) when failover disallowed"), @@ -4288,7 +4287,7 @@ profile = "anthropic" #[test] fn classify_non_failover_eligible_llm_is_terminal_llm() { - let err = fabro_agent::Error::Llm(non_failover_llm_error()); + let err = fabro_agent::Error::from(non_failover_llm_error()); match classify_agent_error(err, true) { AgentApiErrorDisposition::Terminal(Error::Llm(_)) => {} _ => panic!("expected Terminal(Error::Llm) for non-failover-eligible LLM error"), @@ -4297,7 +4296,7 @@ profile = "anthropic" #[test] fn classify_refusal_llm_returns_failover_when_allowed() { - let err = fabro_agent::Error::Llm(LlmError::from(refusal_llm_error())); + let err = fabro_agent::Error::from(refusal_llm_error()); assert!(matches!( classify_agent_error(err, true), AgentApiErrorDisposition::FailoverEligible(_) @@ -4306,7 +4305,7 @@ profile = "anthropic" #[test] fn classify_refusal_llm_returns_terminal_when_not_allowed() { - let err = fabro_agent::Error::Llm(LlmError::from(refusal_llm_error())); + let err = fabro_agent::Error::from(refusal_llm_error()); match classify_agent_error(err, false) { AgentApiErrorDisposition::Terminal(Error::Llm(llm_err)) => { assert!(llm_err.to_string().contains("claude-fable-5 refused")); diff --git a/lib/components/fabro-workflow/src/pipeline/pull_request.rs b/lib/components/fabro-workflow/src/pipeline/pull_request.rs index 1b60a3882..ecbec0492 100644 --- a/lib/components/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/components/fabro-workflow/src/pipeline/pull_request.rs @@ -6,7 +6,7 @@ use fabro_github::{self as github_app, ssh_url_to_https}; use fabro_graphviz::parser; use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; -use fabro_llm::{Client, ClientOptions, Request, selection, structured}; +use fabro_llm::{Client, ClientOptions, Request, selection}; use fabro_store::RunProjection; use fabro_types::settings::run::MergeStrategy; use fabro_types::{ProviderId, PullRequestLink, Role}; @@ -408,10 +408,10 @@ async fn build_pr_content_with_client( .message(fabro_types::Message::text(Role::User, prompt)) .build() .map_err(|e| format!("invalid PR content request: {e}"))?; - let completion = - structured::complete_object(&client, request, "pr_content", PR_CONTENT_SCHEMA.clone()) - .await - .map_err(|e| format!("LLM generation failed: {e}"))?; + let completion = client + .complete_object(request, "pr_content", PR_CONTENT_SCHEMA.clone()) + .await + .map_err(|e| format!("LLM generation failed: {e}"))?; let generated: PrContent = serde_json::from_value(completion.object) .map_err(|e| format!("Failed to deserialize PR content: {e}"))?; diff --git a/lib/foundation/fabro-types/src/lib.rs b/lib/foundation/fabro-types/src/lib.rs index 24a51c0a0..58f99418c 100644 --- a/lib/foundation/fabro-types/src/lib.rs +++ b/lib/foundation/fabro-types/src/lib.rs @@ -30,7 +30,6 @@ pub mod parallel; pub mod principal; pub mod provider_ids; pub mod pull_request; -pub mod reasoning; pub mod repository; pub mod run; pub mod run_event; @@ -95,7 +94,8 @@ pub use interview::{ }; pub use lithos_llm::catalog::{ModelHandle, ModelId, ProviderId}; pub use lithos_llm::types::{ - FinishReason, Request, RequestBuildError, RequestBuilder, Response, ResponseFormat, StreamEvent, + FinishReason, ReasoningOutput, Request, RequestBuildError, RequestBuilder, Response, + ResponseFormat, StreamEvent, }; pub use llm_backend::AgentBackend; pub use manifest_path::{ManifestPath, ManifestPathParseError}; @@ -125,7 +125,6 @@ pub use pull_request::{ PullRequestDetailsUnavailableReason, PullRequestGithubDetail, PullRequestLink, PullRequestMeta, PullRequestRef, PullRequestResponse, PullRequestTimestamps, PullRequestUser, }; -pub use reasoning::ReasoningOutput; pub use repository::{ GitHubRepositorySlug, GitHubRepositorySlugError, RepositoryProvider, RepositoryRef, is_valid_git_branch_name, is_valid_git_tag_name, normalize_git_commit_sha, diff --git a/lib/foundation/fabro-types/src/reasoning.rs b/lib/foundation/fabro-types/src/reasoning.rs deleted file mode 100644 index 89d521e9f..000000000 --- a/lib/foundation/fabro-types/src/reasoning.rs +++ /dev/null @@ -1,142 +0,0 @@ -use serde::{Deserialize, Serialize, de}; - -/// Readable model reasoning normalized into a provider-neutral shape. -/// -/// Providers expose reasoning through several unrelated channels: OpenAI -/// Responses reasoning items, OpenAI-compatible `reasoning_details`, and -/// flattened `reasoning`/`reasoning_content`/`thinking` strings. This type -/// reduces all of them to the two capabilities consumers actually care -/// about, so the durable event contract does not change shape when a -/// provider dialect does. -/// -/// Both fields may be populated for the same response. An emitted object -/// always carries at least one of them; opaque provider material -/// (signatures, IDs, encrypted or redacted payloads) never appears here. -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] -pub struct ReasoningOutput { - /// Model-authored summary of its reasoning, safe to show to users. - #[serde(default, skip_serializing_if = "Option::is_none")] - summary: Option, - /// Verbatim readable reasoning text, when the provider returns it in - /// addition to (or instead of) a summary. - #[serde(default, skip_serializing_if = "Option::is_none")] - trace: Option, -} - -impl ReasoningOutput { - /// Creates reasoning output with both a model-authored summary and a - /// verbatim trace. - #[must_use] - pub fn new(summary: impl Into, trace: impl Into) -> Self { - Self { - summary: Some(summary.into()), - trace: Some(trace.into()), - } - } - - /// Creates reasoning output containing only a model-authored summary. - #[must_use] - pub fn from_summary(summary: impl Into) -> Self { - Self { - summary: Some(summary.into()), - trace: None, - } - } - - /// Creates reasoning output containing only a verbatim trace. - #[must_use] - pub fn from_trace(trace: impl Into) -> Self { - Self { - summary: None, - trace: Some(trace.into()), - } - } - - /// Returns the model-authored summary, when present. - #[must_use] - pub fn summary(&self) -> Option<&str> { - self.summary.as_deref() - } - - /// Returns the verbatim readable reasoning trace, when present. - #[must_use] - pub fn trace(&self) -> Option<&str> { - self.trace.as_deref() - } -} - -impl<'de> Deserialize<'de> for ReasoningOutput { - fn deserialize>(deserializer: D) -> Result { - #[derive(Deserialize)] - struct Fields { - #[serde(default)] - summary: Option, - #[serde(default)] - trace: Option, - } - - let Fields { summary, trace } = Fields::deserialize(deserializer)?; - match (summary, trace) { - (Some(summary), Some(trace)) => Ok(Self::new(summary, trace)), - (Some(summary), None) => Ok(Self::from_summary(summary)), - (None, Some(trace)) => Ok(Self::from_trace(trace)), - (None, None) => Err(de::Error::custom( - "reasoning output requires a summary or trace", - )), - } - } -} - -#[cfg(test)] -mod tests { - use serde_json::json; - - use super::*; - - #[test] - fn summary_only_round_trips_without_trace_member() { - let output = ReasoningOutput::from_summary("checked the parser first"); - let v = serde_json::to_value(&output).unwrap(); - assert_eq!(v, json!({"summary": "checked the parser first"})); - assert_eq!( - serde_json::from_value::(v).unwrap(), - output - ); - } - - #[test] - fn trace_only_round_trips_without_summary_member() { - let output = ReasoningOutput::from_trace("step one, step two"); - let v = serde_json::to_value(&output).unwrap(); - assert_eq!(v, json!({"trace": "step one, step two"})); - assert_eq!( - serde_json::from_value::(v).unwrap(), - output - ); - } - - #[test] - fn both_fields_round_trip() { - let output = ReasoningOutput::new("summary", "trace"); - let v = serde_json::to_value(&output).unwrap(); - assert_eq!(v, json!({"summary": "summary", "trace": "trace"})); - assert_eq!( - serde_json::from_value::(v).unwrap(), - output - ); - } - - #[test] - fn empty_object_is_rejected() { - let error = serde_json::from_value::(json!({})).unwrap_err(); - assert!(error.to_string().contains("requires a summary or trace")); - } - - #[test] - fn explicit_nulls_are_rejected() { - let error = - serde_json::from_value::(json!({"summary": null, "trace": null})) - .unwrap_err(); - assert!(error.to_string().contains("requires a summary or trace")); - } -}