From 8dbb0996da72db43c22ffd99e2e4cff754c576b0 Mon Sep 17 00:00:00 2001
From: AI-Reviewer-QS <qs-ai-reviewer@quantstamp.com>
Date: Fri, 20 Feb 2026 00:56:39 +0800
Subject: [PATCH] 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
---
 src/estimation/value.rs | 16 ++++++++++++++++
 src/llm/mod.rs          | 13 ++++++-------
 src/llm/rig_adapter.rs  | 23 +++++++----------------
 3 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/src/estimation/value.rs b/src/estimation/value.rs
index ebdc5c4a..273ff939 100644
--- a/src/estimation/value.rs
+++ b/src/estimation/value.rs
@@ -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)));
+    }
 }
diff --git a/src/llm/mod.rs b/src/llm/mod.rs
index dbb27b96..45b2c85a 100644
--- a/src/llm/mod.rs
+++ b/src/llm/mod.rs
@@ -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);
diff --git a/src/llm/rig_adapter.rs b/src/llm/rig_adapter.rs
index 20bdbc72..20e82eeb 100644
--- a/src/llm/rig_adapter.rs
+++ b/src/llm/rig_adapter.rs
@@ -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) => {