Fix division by zero panic in ValueEstimator::is_profitable (#139)

* fix: prevent division-by-zero panic in ValueEstimator::is_profitable

Guard against Decimal division by zero when price is zero.
rust_decimal::Decimal panics on division by zero (unlike f64 which
returns infinity), so we short-circuit before the division.

When price is zero, a job is only profitable if the estimated cost
is negative (i.e., we get paid to do it).

Add test covering zero-price scenarios including the negative cost
edge case.

* style: fix pre-existing rustfmt and clippy issues in llm module

Fix formatting and lint issues that cause CI Code Style check to fail:
- src/llm/mod.rs: fix method chain indentation
- src/llm/rig_adapter.rs: collapse multi-line single-expression statements,
  fix collapsible_if clippy warning
This commit is contained in:
AI-Reviewer-QS
2026-02-19 16:56:39 +00:00
committed by GitHub
parent ae714b5003
commit 8dbb0996da
3 changed files with 29 additions and 23 deletions
+16
View File
@@ -40,6 +40,11 @@ impl ValueEstimator {
/// Check if a job is profitable at a given price.
pub fn is_profitable(&self, price: Decimal, estimated_cost: Decimal) -> bool {
if price.is_zero() {
// With a zero price, the job is only profitable if the cost is negative.
// This results in a positive profit and an effectively infinite margin.
return estimated_cost < Decimal::ZERO;
}
let margin = (price - estimated_cost) / price;
margin >= self.min_margin
}
@@ -104,4 +109,15 @@ mod tests {
let margin = estimator.calculate_margin(dec!(100.0), dec!(70.0));
assert_eq!(margin, dec!(0.30)); // 30%
}
#[test]
fn test_profitability_zero_price() {
let estimator = ValueEstimator::new();
// Zero price should return false, not panic
assert!(!estimator.is_profitable(Decimal::ZERO, dec!(10.0)));
assert!(!estimator.is_profitable(Decimal::ZERO, Decimal::ZERO));
// Negative cost with zero price is profitable (we get paid to do it)
assert!(estimator.is_profitable(Decimal::ZERO, dec!(-10.0)));
}
}
+6 -7
View File
@@ -99,13 +99,12 @@ fn create_openai_provider(config: &LlmConfig) -> Result<Arc<dyn LlmProvider>, Ll
// (Responses API). The Responses API path in rig-core panics when tool results
// are sent back because ironclaw doesn't thread `call_id` through its ToolCall
// type. The Chat Completions API works correctly with the existing code.
let client: openai::CompletionsClient =
openai::Client::new(oai.api_key.expose_secret())
.map_err(|e| LlmError::RequestFailed {
provider: "openai".to_string(),
reason: format!("Failed to create OpenAI client: {}", e),
})?
.completions_api();
let client: openai::CompletionsClient = openai::Client::new(oai.api_key.expose_secret())
.map_err(|e| LlmError::RequestFailed {
provider: "openai".to_string(),
reason: format!("Failed to create OpenAI client: {}", e),
})?
.completions_api();
let model = client.completion_model(&oai.model);
tracing::info!("Using OpenAI direct API (model: {})", oai.model);
+7 -16
View File
@@ -113,10 +113,7 @@ fn normalize_schema_recursive(schema: &mut JsonValue) {
}
// Force additionalProperties: false (overwrite any existing value)
obj.insert(
"additionalProperties".to_string(),
JsonValue::Bool(false),
);
obj.insert("additionalProperties".to_string(), JsonValue::Bool(false));
// Ensure "properties" exists
if !obj.contains_key("properties") {
@@ -157,19 +154,16 @@ fn normalize_schema_recursive(schema: &mut JsonValue) {
normalize_schema_recursive(prop_schema);
}
// Then make originally-optional properties nullable
if !current_required.contains(key) {
if let Some(prop_schema) = props.get_mut(key) {
make_nullable(prop_schema);
}
if !current_required.contains(key)
&& let Some(prop_schema) = props.get_mut(key)
{
make_nullable(prop_schema);
}
}
}
// Set required to ALL property keys
let required_value: Vec<JsonValue> = all_keys
.into_iter()
.map(JsonValue::String)
.collect();
let required_value: Vec<JsonValue> = all_keys.into_iter().map(JsonValue::String).collect();
obj.insert("required".to_string(), JsonValue::Array(required_value));
}
@@ -188,10 +182,7 @@ fn make_nullable(schema: &mut JsonValue) {
match type_val {
// "type": "string" → "type": ["string", "null"]
JsonValue::String(ref t) if t != "null" => {
obj.insert(
"type".to_string(),
serde_json::json!([t, "null"]),
);
obj.insert("type".to_string(), serde_json::json!([t, "null"]));
}
// "type": ["string", "integer"] → add "null" if missing
JsonValue::Array(ref arr) => {