refactor(llm): return Self from Client::from_source

Let callers decide whether to wrap in Arc. Also consolidates the two
state() fetches in build_pr_body into one.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-23 21:03:21 -04:00
parent 9a0b64e53c
commit ca56c15f2f
No known key found for this signature in database
9 changed files with 27 additions and 31 deletions

View file

@ -459,7 +459,6 @@ pub async fn run_with_args_and_source(
let provider = parse_provider(&args)?;
let client = Client::from_source(llm_source.as_ref())
.await
.map(|client| (*client).clone())
.map_err(|e| anyhow::anyhow!("Failed to create LLM client: {e}"))?;
ensure_provider_registered(&client, provider)?;
run_with_args_and_client(args, client, mcp_servers).await

View file

@ -150,10 +150,9 @@ async fn make_client(provider: Provider, twin: Option<&OpenAiTwinOptions>) -> Cl
}
let source = EnvCredentialSource::new();
(*Client::from_source(&source)
Client::from_source(&source)
.await
.expect("Client::from_source failed"))
.clone()
.expect("Client::from_source failed")
}
fn make_twin_client(twin: &OpenAiTwinOptions) -> Client {

View file

@ -305,7 +305,7 @@ impl HookExecutorImpl {
Self::execute_llm_with_timeout(definition.timeout(), "prompt", || async move {
let client = match LlmClient::from_source(llm_source).await {
Ok(client) => client,
Ok(client) => Arc::new(client),
Err(e) => {
tracing::warn!(error = %e, "prompt hook client creation failed, proceeding");
return HookDecision::Proceed;

View file

@ -44,13 +44,12 @@ impl Client {
///
/// Returns `Error` if the source cannot resolve credentials or any provider
/// adapter fails to initialize.
pub async fn from_source(source: &dyn CredentialSource) -> Result<Arc<Self>, Error> {
pub async fn from_source(source: &dyn CredentialSource) -> Result<Self, Error> {
let resolved = source.resolve().await.map_err(|err| Error::Configuration {
message: format!("Failed to resolve LLM credentials: {err}"),
source: None,
})?;
let client = Self::from_credentials(resolved.credentials).await?;
Ok(Arc::new(client))
Self::from_credentials(resolved.credentials).await
}
/// Create a Client from typed provider credentials.

View file

@ -6833,7 +6833,6 @@ async fn create_completion(
// Force non-streaming for structured output
let use_stream = req.stream && req.schema.is_none();
// Resolve an LLM client from the current credential source.
let llm_result = match state.resolve_llm_client().await {
Ok(result) => result,
Err(err) => {

View file

@ -205,7 +205,6 @@ impl AgentApiBackend {
) -> Result<Session, Error> {
let client = Client::from_source(source)
.await
.map(|client| (*client).clone())
.map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?;
let mut profile = build_profile(model, provider);
@ -289,7 +288,6 @@ impl CodergenBackend for AgentApiBackend {
) -> Result<CodergenResult, Error> {
let client = Client::from_source(self.source.as_ref())
.await
.map(|client| (*client).clone())
.map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?;
let model = node.model().unwrap_or(&self.model);

View file

@ -299,7 +299,15 @@ pub async fn build_pr_body(
.await
.map_err(|e| format!("Failed to create LLM client: {e}"))?;
build_pr_body_with_client(diff, goal, model, &services.run_store, conclusion, client).await
build_pr_body_with_client(
diff,
goal,
model,
&services.run_store,
conclusion,
Arc::new(client),
)
.await
}
async fn build_pr_body_with_client(
@ -312,19 +320,6 @@ async fn build_pr_body_with_client(
) -> Result<String, String> {
debug!("Building PR body");
let loaded_conclusion = if conclusion.is_none() {
run_store
.state()
.await
.inspect_err(|err| {
tracing::warn!(error = %err, "Failed to load conclusion from store for PR body");
})
.ok()
.and_then(|state| state.conclusion)
} else {
None
};
let conclusion = conclusion.or(loaded_conclusion.as_ref());
let run_state = run_store
.state()
.await
@ -332,6 +327,15 @@ async fn build_pr_body_with_client(
tracing::warn!(error = %err, "Failed to load run state from store for PR body");
})
.ok();
let loaded_conclusion = conclusion
.is_none()
.then(|| {
run_state
.as_ref()
.and_then(|state| state.conclusion.clone())
})
.flatten();
let conclusion = conclusion.or(loaded_conclusion.as_ref());
let plan_text = run_state.as_ref().and_then(read_plan_text);
let retro = run_state.as_ref().and_then(|state| state.retro.clone());
let run_spec = run_state.as_ref().and_then(|state| state.spec.clone());

View file

@ -95,7 +95,7 @@ pub async fn run_retro(options: &RetroOptions, dry_run: bool) -> Option<Retro> {
&state,
&events,
&options.run_dir,
client.as_ref(),
&client,
services.provider,
&options.model,
Some(event_callback),

View file

@ -6144,11 +6144,9 @@ mod real_llm {
fabro_test::require_env("ANTHROPIC_API_KEY")?;
let source = fabro_auth::EnvCredentialSource::new();
Some(
Client::from_source(&source)
.await
.expect("unified-llm client should initialize from env source"),
)
Some(Arc::new(Client::from_source(&source).await.expect(
"unified-llm client should initialize from env source",
)))
}
fn make_llm_backend(client: Arc<Client>) -> Box<LlmCodergenBackend> {