refactor(types): use trait conversions directly

Remove redundant as_str/from helper methods on provider, reasoning, model-test, safe URL, and interview types. Migrate call sites to Display, IntoStaticStr, and FromStr while keeping wire-format coverage in tests.
This commit is contained in:
Bryan Helmkamp 2026-04-26 21:36:24 -04:00
parent 54c5f30586
commit cb0bab233a
No known key found for this signature in database
27 changed files with 120 additions and 144 deletions

View file

@ -266,7 +266,7 @@ fn ensure_provider_registered(client: &Client, provider: Provider) -> anyhow::Re
if client
.provider_names()
.iter()
.any(|name| *name == provider.as_str())
.any(|name| *name == <&'static str>::from(provider))
{
return Ok(());
}

View file

@ -112,7 +112,7 @@ function names, error messages, and exact values. Omit pleasantries and conversa
"Here is the conversation to summarize:\n\n{rendered}"
)),
],
provider: Some(provider_profile.provider().as_str().to_string()),
provider: Some(provider_profile.provider().to_string()),
tools: None,
tool_choice: None,
response_format: None,

View file

@ -55,7 +55,7 @@ impl OpenAiProfile {
Provider::Zai => "Zhipu AI",
Provider::Minimax => "MiniMax",
Provider::Inception => "Inception",
other => other.as_str(),
other => <&'static str>::from(other),
}
}
}

View file

@ -726,7 +726,7 @@ impl Session {
// Call LLM (streaming) with retry for transient errors
let retry_emitter = self.event_emitter.clone();
let retry_session_id = self.id.clone();
let retry_provider = self.provider_profile.provider().as_str().to_string();
let retry_provider = self.provider_profile.provider().to_string();
let retry_model = self.provider_profile.model().to_string();
let retry_policy = RetryPolicy {
max_retries: 3,
@ -991,7 +991,7 @@ impl Session {
Request {
model: self.provider_profile.model().to_string(),
messages,
provider: Some(self.provider_profile.provider().as_str().to_string()),
provider: Some(self.provider_profile.provider().to_string()),
tools: if has_tools { Some(tools) } else { None },
tool_choice: if has_tools {
Some(ToolChoice::Auto)

View file

@ -605,7 +605,7 @@ pub(crate) fn make_web_fetch_tool(summarizer: Option<WebFetchSummarizer>) -> Reg
let request = Request {
model: s.model_id.model_id().to_string(),
messages: vec![Message::user(summarization_prompt)],
provider: Some(s.model_id.provider().as_str().to_string()),
provider: Some(s.model_id.provider().to_string()),
tools: None,
tool_choice: None,
response_format: None,

View file

@ -97,7 +97,7 @@ pub fn credential_id_for(credential: &AuthCredential) -> Result<String, String>
"codex_oauth credentials are only valid for OpenAI, got {}",
credential.provider
)),
(provider, AuthDetails::ApiKey { .. }) => Ok(provider.as_str().to_string()),
(provider, AuthDetails::ApiKey { .. }) => Ok(provider.to_string()),
}
}

View file

@ -85,13 +85,12 @@ pub(crate) async fn validate_api_key(provider: Provider, api_key: &str) -> Resul
.await
.map_err(|e| e.to_string())?;
let probe_model = Catalog::builtin().probe_for_provider(provider).map_or_else(
|| format!("unknown-{}", provider.as_str()),
|model| model.id.clone(),
);
let probe_model = Catalog::builtin()
.probe_for_provider(provider)
.map_or_else(|| format!("unknown-{provider}"), |model| model.id.clone());
let params = GenerateParams::new(probe_model, Arc::new(client))
.provider(provider.as_str())
.provider(<&'static str>::from(provider))
.prompt("Say OK")
.max_tokens(16);

View file

@ -19,11 +19,6 @@ pub enum ModelTestMode {
}
impl ModelTestMode {
#[must_use]
pub fn as_str(self) -> &'static str {
self.into()
}
#[must_use]
pub const fn timeout_secs(self) -> u64 {
match self {
@ -40,13 +35,6 @@ pub enum ModelTestStatus {
Error,
}
impl ModelTestStatus {
#[must_use]
pub fn as_str(self) -> &'static str {
self.into()
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ModelTestOutcome {
pub status: ModelTestStatus,
@ -84,7 +72,7 @@ pub async fn run_model_test(
async fn run_basic_test(info: &Model, client: Arc<Client>) -> ModelTestOutcome {
let params = GenerateParams::new(&info.id, client)
.provider(info.provider.as_str())
.provider(<&'static str>::from(info.provider))
.prompt("Say OK")
.max_tokens(16);
@ -152,7 +140,7 @@ fn build_deep_test_params(info: &Model, client: Arc<Client>) -> Option<GenerateP
);
let mut params = GenerateParams::new(&info.id, client)
.provider(info.provider.as_str())
.provider(<&'static str>::from(info.provider))
.prompt(
"Use the add tool twice: first add 15 and 27, then add that result to 42. \
Finally, tell me whether the grand total is even or odd and why.",

View file

@ -9,9 +9,9 @@ use crate::providers::common::{
parse_retry_after, send_and_read_response,
};
use crate::types::{
AdapterTimeout, ContentPart, FinishReason, Message, RateLimitInfo, Request, Response,
ResponseFormatType, Role, StreamEvent, ThinkingData, TokenCounts, ToolCall, ToolChoice,
ToolDefinition,
AdapterTimeout, ContentPart, FinishReason, Message, RateLimitInfo, ReasoningEffort, Request,
Response, ResponseFormatType, Role, StreamEvent, ThinkingData, TokenCounts, ToolCall,
ToolChoice, ToolDefinition,
};
/// Provider adapter for the Anthropic Messages API.
@ -547,13 +547,13 @@ fn extract_thinking_config(
/// Map a reasoning effort level to a thinking `budget_tokens` value for models
/// that don't support the `output_config.effort` parameter (e.g.
/// claude-sonnet-4-5).
fn effort_to_budget_tokens(effort: &str, max_tokens: i64) -> i64 {
fn effort_to_budget_tokens(effort: ReasoningEffort, max_tokens: i64) -> i64 {
let budget = match effort {
"low" => max_tokens / 4,
"high" => max_tokens * 3 / 4,
"xhigh" => max_tokens * 7 / 8,
"max" => max_tokens,
_ => max_tokens / 2, // "medium" or unknown
ReasoningEffort::Low => max_tokens / 4,
ReasoningEffort::Medium => max_tokens / 2,
ReasoningEffort::High => max_tokens * 3 / 4,
ReasoningEffort::XHigh => max_tokens * 7 / 8,
ReasoningEffort::Max => max_tokens,
};
// Anthropic requires budget_tokens >= 1024
budget.max(1024)
@ -1152,12 +1152,12 @@ async fn build_api_request(
if supports_effort {
(
explicit_thinking,
Some(serde_json::json!({"effort": effort.as_str()})),
Some(serde_json::json!({"effort": <&'static str>::from(*effort)})),
)
} else if explicit_thinking.is_none() {
// Convert effort level to a thinking budget for models that don't
// support the effort parameter (e.g. claude-sonnet-4-5).
let budget = effort_to_budget_tokens(effort.as_str(), resolved_max_tokens);
let budget = effort_to_budget_tokens(*effort, resolved_max_tokens);
if resolved_max_tokens <= budget {
resolved_max_tokens = budget + 1024;
}
@ -2251,12 +2251,18 @@ mod tests {
#[test]
fn effort_to_budget_tokens_xhigh_maps_to_seven_eighths() {
assert_eq!(effort_to_budget_tokens("xhigh", 16_000), 14_000);
assert_eq!(
effort_to_budget_tokens(ReasoningEffort::XHigh, 16_000),
14_000
);
}
#[test]
fn effort_to_budget_tokens_max_maps_to_full_budget() {
assert_eq!(effort_to_budget_tokens("max", 16_000), 16_000);
assert_eq!(
effort_to_budget_tokens(ReasoningEffort::Max, 16_000),
16_000
);
}
#[tokio::test]

View file

@ -396,7 +396,7 @@ async fn build_api_request(request: &Request, stream: bool, codex_mode: bool) ->
let reasoning = request
.reasoning_effort
.as_ref()
.map(|effort| serde_json::json!({"effort": effort.as_str()}));
.map(|effort| serde_json::json!({"effort": <&'static str>::from(*effort)}));
let text = request
.response_format
.as_ref()

View file

@ -434,12 +434,6 @@ pub enum ReasoningEffort {
Max,
}
impl ReasoningEffort {
pub fn as_str(&self) -> &'static str {
(*self).into()
}
}
// --- 3.6 Request ---
#[derive(Debug, Clone, Serialize, Deserialize)]
@ -1298,8 +1292,10 @@ mod tests {
Ok(ReasoningEffort::XHigh)
);
assert_eq!(ReasoningEffort::from_str("max"), Ok(ReasoningEffort::Max));
assert_eq!(ReasoningEffort::XHigh.as_str(), "xhigh");
assert_eq!(ReasoningEffort::Max.as_str(), "max");
assert_eq!(ReasoningEffort::XHigh.to_string(), "xhigh");
assert_eq!(<&'static str>::from(ReasoningEffort::XHigh), "xhigh");
assert_eq!(ReasoningEffort::Max.to_string(), "max");
assert_eq!(<&'static str>::from(ReasoningEffort::Max), "max");
}
#[test]

View file

@ -115,13 +115,6 @@ pub enum Speed {
Fast,
}
impl Speed {
#[must_use]
pub fn as_str(self) -> &'static str {
self.into()
}
}
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub struct ModelRef {
pub provider: Provider,

View file

@ -152,7 +152,7 @@ impl Catalog {
return Vec::new();
};
let Some(fallback_providers) = fallbacks.get(primary.as_str()) else {
let Some(fallback_providers) = fallbacks.get(<&'static str>::from(primary)) else {
return Vec::new();
};
@ -417,13 +417,13 @@ mod tests {
}
#[test]
fn catalog_providers_roundtrip_through_as_str() {
fn catalog_providers_roundtrip_through_static_str() {
for model in Catalog::builtin().list(None) {
let roundtripped = Provider::from_str(model.provider.as_str());
let roundtripped = Provider::from_str(<&'static str>::from(model.provider));
assert_eq!(
roundtripped,
Ok(model.provider),
"catalog model '{}' provider {:?} does not roundtrip through as_str",
"catalog model '{}' provider {:?} does not roundtrip through IntoStaticStr",
model.id,
model.provider
);
@ -431,13 +431,13 @@ mod tests {
}
#[test]
fn provider_as_str_roundtrips_through_from_str() {
fn provider_static_str_roundtrips_through_from_str() {
for &provider in Provider::ALL {
let roundtripped = Provider::from_str(provider.as_str());
let roundtripped = Provider::from_str(<&'static str>::from(provider));
assert_eq!(
roundtripped,
Ok(provider),
"Provider::{provider:?}.as_str() does not round-trip through from_str"
"Provider::{provider:?} IntoStaticStr does not round-trip through from_str"
);
}
}

View file

@ -120,13 +120,6 @@ impl Provider {
Self::OpenAiCompatible => "OpenAI Compatible",
}
}
/// Stable lowercase string representation used in `Request.provider`,
/// adapter names, and other serialization boundaries.
#[must_use]
pub fn as_str(self) -> &'static str {
self.into()
}
}
#[cfg(test)]
@ -150,17 +143,20 @@ mod tests {
#[test]
fn kimi_as_str() {
assert_eq!(Provider::Kimi.as_str(), "kimi");
assert_eq!(Provider::Kimi.to_string(), "kimi");
assert_eq!(<&'static str>::from(Provider::Kimi), "kimi");
}
#[test]
fn zai_as_str() {
assert_eq!(Provider::Zai.as_str(), "zai");
assert_eq!(Provider::Zai.to_string(), "zai");
assert_eq!(<&'static str>::from(Provider::Zai), "zai");
}
#[test]
fn minimax_as_str() {
assert_eq!(Provider::Minimax.as_str(), "minimax");
assert_eq!(Provider::Minimax.to_string(), "minimax");
assert_eq!(<&'static str>::from(Provider::Minimax), "minimax");
}
#[test]
@ -177,7 +173,8 @@ mod tests {
#[test]
fn inception_as_str() {
assert_eq!(Provider::Inception.as_str(), "inception");
assert_eq!(Provider::Inception.to_string(), "inception");
assert_eq!(<&'static str>::from(Provider::Inception), "inception");
}
#[test]

View file

@ -46,6 +46,7 @@ pub enum DisplaySafeUrlError {
pub struct DisplaySafeUrl(Url);
impl DisplaySafeUrl {
/// Parse user-provided URL text; [`FromStr`] delegates here.
#[inline]
pub fn parse(input: &str) -> Result<Self, DisplaySafeUrlError> {
let url = Url::parse(input)?;
@ -53,15 +54,6 @@ impl DisplaySafeUrl {
Ok(Self(url))
}
/// Create a `DisplaySafeUrl` from an already parsed [`Url`].
///
/// This does not perform ambiguity checks because parsed URLs from trusted
/// HTTP libraries are not human-entered strings.
#[inline]
pub fn from_url(url: Url) -> Self {
Self(url)
}
/// Cast a `&Url` to a `&DisplaySafeUrl` without allocation.
#[inline]
pub fn ref_cast(url: &Url) -> &Self {

View file

@ -146,17 +146,16 @@ async fn check_llm_providers(state: &AppState) -> CheckResult {
}
fn probe_model(provider: Provider) -> String {
Catalog::builtin().probe_for_provider(provider).map_or_else(
|| format!("unknown-{}", provider.as_str()),
|m| m.id.clone(),
)
Catalog::builtin()
.probe_for_provider(provider)
.map_or_else(|| format!("unknown-{provider}"), |m| m.id.clone())
}
async fn probe_llm_provider(client: &LlmClient, provider: Provider) -> Result<(), String> {
let request = Request {
model: probe_model(provider),
messages: vec![Message::user("hi")],
provider: Some(provider.as_str().to_string()),
provider: Some(provider.to_string()),
tools: None,
tool_choice: None,
response_format: None,

View file

@ -700,7 +700,7 @@ async fn post_install_llm_test(
match validate_llm_provider(&state, &input).await {
Ok(()) => Json(serde_json::json!({ "ok": true })).into_response(),
Err(err) => {
warn!(provider = %input.provider.as_str(), error = %err, "install LLM validation failed");
warn!(provider = %input.provider, error = %err, "install LLM validation failed");
install_error_response(StatusCode::UNPROCESSABLE_ENTITY, err)
}
}
@ -732,7 +732,7 @@ async fn put_install_llm(
if provider.api_key.trim().is_empty() {
return install_error_response(
StatusCode::UNPROCESSABLE_ENTITY,
format!("api_key is required for {}", provider.provider.as_str()),
format!("api_key is required for {}", provider.provider),
);
}
}
@ -1675,7 +1675,7 @@ fn redacted_llm(pending_install: &PendingInstall) -> serde_json::Value {
|llm| {
serde_json::json!({
"providers": llm.providers.iter().map(|provider| serde_json::json!({
"provider": provider.provider.as_str(),
"provider": <&'static str>::from(provider.provider),
"configured": true,
})).collect::<Vec<_>>()
})
@ -1834,7 +1834,7 @@ async fn validate_llm_provider(
| Provider::OpenAiCompatible => {
return Err(format!(
"{} is not supported by install validation",
input.provider.as_str()
input.provider
));
}
};
@ -1860,7 +1860,7 @@ async fn validate_llm_provider(
} else {
Err(format!(
"{} model lookup failed ({})",
input.provider.as_str(),
input.provider,
response.status()
))
}

View file

@ -443,7 +443,7 @@ impl SlackService {
id: props.question_id.clone(),
text: props.question.clone(),
stage: props.stage.clone(),
question_type: InterviewQuestionType::from_wire_name(&props.question_type),
question_type: props.question_type.parse().unwrap_or_default(),
options: props.options.clone(),
allow_freeform: props.allow_freeform,
timeout_seconds: props.timeout_seconds,
@ -7532,11 +7532,8 @@ async fn test_model(
{
return ApiError::bad_request(auth_issue_message(info.provider, issue)).into_response();
}
if !llm_result
.client
.provider_names()
.contains(&info.provider.as_str())
{
let provider_name = <&'static str>::from(info.provider);
if !llm_result.client.provider_names().contains(&provider_name) {
return Json(serde_json::json!({
"model_id": info.id,
"status": "skip",
@ -7548,7 +7545,7 @@ async fn test_model(
let outcome = run_model_test(info, mode, client).await;
Json(serde_json::json!({
"model_id": info.id,
"status": outcome.status.as_str(),
"status": <&'static str>::from(outcome.status),
"error_message": outcome.error_message,
}))
.into_response()

View file

@ -9,9 +9,9 @@ use fabro_types::run_event::{
};
use fabro_types::{
BilledModelUsage, Checkpoint, Conclusion, EventBody, FailureSignature, InterviewQuestionRecord,
InterviewQuestionType, NodeStatusRecord, Outcome, PendingInterviewRecord, PullRequestRecord,
RunControlAction, RunId, RunProjection, RunSpec, RunStatus, RunSummary, SandboxRecord,
StageStatus, StartRecord, TerminalStatus,
NodeStatusRecord, Outcome, PendingInterviewRecord, PullRequestRecord, RunControlAction, RunId,
RunProjection, RunSpec, RunStatus, RunSummary, SandboxRecord, StageStatus, StartRecord,
TerminalStatus,
};
use serde_json::Value;
@ -252,9 +252,7 @@ impl RunProjectionReducer for RunProjection {
id: props.question_id.clone(),
text: props.question.clone(),
stage: props.stage.clone(),
question_type: InterviewQuestionType::from_wire_name(
&props.question_type,
),
question_type: props.question_type.parse().unwrap_or_default(),
options: props.options.clone(),
allow_freeform: props.allow_freeform,
timeout_seconds: props.timeout_seconds,

View file

@ -1,11 +1,21 @@
use std::fmt;
use serde::{Deserialize, Serialize};
use crate::run_event::InterviewOption;
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)]
#[derive(
Debug,
Clone,
Copy,
PartialEq,
Eq,
Serialize,
Deserialize,
Default,
strum::Display,
strum::EnumString,
)]
#[serde(rename_all = "snake_case")]
#[strum(serialize_all = "snake_case")]
pub enum InterviewQuestionType {
YesNo,
MultipleChoice,
@ -15,31 +25,6 @@ pub enum InterviewQuestionType {
Confirmation,
}
impl InterviewQuestionType {
#[must_use]
pub fn from_wire_name(value: &str) -> Self {
match value {
"yes_no" => Self::YesNo,
"multiple_choice" => Self::MultipleChoice,
"multi_select" => Self::MultiSelect,
"confirmation" => Self::Confirmation,
_ => Self::Freeform,
}
}
}
impl fmt::Display for InterviewQuestionType {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::YesNo => write!(f, "yes_no"),
Self::MultipleChoice => write!(f, "multiple_choice"),
Self::MultiSelect => write!(f, "multi_select"),
Self::Freeform => write!(f, "freeform"),
Self::Confirmation => write!(f, "confirmation"),
}
}
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Default)]
pub struct InterviewQuestionRecord {
#[serde(default)]
@ -59,3 +44,27 @@ pub struct InterviewQuestionRecord {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub context_display: Option<String>,
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn question_type_wire_names_roundtrip() {
let cases = [
("yes_no", InterviewQuestionType::YesNo),
("multiple_choice", InterviewQuestionType::MultipleChoice),
("multi_select", InterviewQuestionType::MultiSelect),
("freeform", InterviewQuestionType::Freeform),
("confirmation", InterviewQuestionType::Confirmation),
];
for (wire, question_type) in cases {
assert_eq!(
wire.parse::<InterviewQuestionType>().unwrap(),
question_type
);
assert_eq!(question_type.to_string(), wire);
}
}
}

View file

@ -49,6 +49,8 @@ impl InterpString {
/// Parse a raw string into its literal/env-var segments.
///
/// The [`From<String>`] and [`From<&str>`] impls delegate here.
///
/// Parsing is infallible: the token grammar is intentionally permissive so
/// that validation happens at consumption time along with env lookup.
#[must_use]

View file

@ -937,7 +937,7 @@ fn check_provider_known(
}
let valid: Vec<&str> = fabro_model::Provider::ALL
.iter()
.map(|p| p.as_str())
.map(|&p| <&'static str>::from(p))
.collect();
let valid_str = valid.join(", ");
Some(Diagnostic {

View file

@ -254,7 +254,7 @@ impl Handler for AgentHandler {
let prompt_provider = node
.provider()
.map(String::from)
.or_else(|| Some(services.run.provider.as_str().to_string()));
.or_else(|| Some(services.run.provider.to_string()));
let prompt_model = node.model().map(String::from);
let stage_scope = StageScope::for_handler(context, &node.id);
services.run.emitter.emit_scoped(
@ -331,7 +331,7 @@ impl Handler for AgentHandler {
let response_provider = node
.provider()
.map(String::from)
.or_else(|| Some(services.run.provider.as_str().to_string()))
.or_else(|| Some(services.run.provider.to_string()))
.unwrap_or_default();
services.run.emitter.emit_scoped(
&Event::PromptCompleted {

View file

@ -294,7 +294,7 @@ impl CodergenBackend for AgentApiBackend {
let provider = node
.provider()
.map(String::from)
.or_else(|| Some(self.provider.as_str().to_string()));
.or_else(|| Some(self.provider.to_string()));
let max_tokens = node.max_tokens().or_else(|| {
fabro_model::Catalog::builtin()
@ -335,7 +335,7 @@ impl CodergenBackend for AgentApiBackend {
let result = client.complete(&request).await;
let default_provider = self.provider.as_str().to_string();
let default_provider = self.provider.to_string();
let (response, actual_model, actual_provider) = match result {
Ok(resp) => (
@ -500,7 +500,7 @@ impl CodergenBackend for AgentApiBackend {
if sdk_err.failover_eligible() && !self.fallback_chain.is_empty() =>
{
let error_msg = sdk_err.to_string();
let from_provider = self.provider.as_str().to_string();
let from_provider = self.provider.to_string();
let from_model = self.model.clone();
let mut last_err = Error::Llm(sdk_err.clone());

View file

@ -67,7 +67,7 @@ async fn ensure_cli(
) -> Result<(), Error> {
let start = std::time::Instant::now();
let cli_name = cli.name();
let provider_str = provider.as_str();
let provider_str = <&'static str>::from(provider);
emitter.emit(&Event::CliEnsureStarted {
cli_name: cli_name.to_string(),
@ -520,7 +520,7 @@ impl CodergenBackend for AgentCliBackend {
node_id: node.id.clone(),
visit: stage_scope.visit,
mode: "cli".to_string(),
provider: provider.as_str().to_string(),
provider: provider.to_string(),
model: model.to_string(),
command: command.clone(),
},

View file

@ -86,7 +86,7 @@ impl Handler for PromptHandler {
let prompt_provider = node
.provider()
.map(String::from)
.or_else(|| Some(services.run.provider.as_str().to_string()));
.or_else(|| Some(services.run.provider.to_string()));
let prompt_model = node.model().map(String::from);
let stage_scope = StageScope::for_handler(context, &node.id);
services.run.emitter.emit_scoped(
@ -138,7 +138,7 @@ impl Handler for PromptHandler {
let response_provider = node
.provider()
.map(String::from)
.or_else(|| Some(services.run.provider.as_str().to_string()))
.or_else(|| Some(services.run.provider.to_string()))
.unwrap_or_default();
services.run.emitter.emit_scoped(

View file

@ -58,7 +58,7 @@ pub async fn run_retro(options: &RetroOptions, dry_run: bool) -> Option<Retro> {
let retro_prompt = build_retro_prompt(RETRO_DATA_DIR);
services.emitter.emit(&Event::RetroStarted {
prompt: Some(retro_prompt),
provider: Some(services.provider.as_str().to_string()),
provider: Some(services.provider.to_string()),
model: Some(options.model.clone()),
});