Case-Sensitive Condition Evaluation Breaks Feature Flags

[AbdullahNamespace]

🐛 Bug: Case-Sensitive Condition Evaluation Breaks Feature Flags

Priority: 🟠 High

Description

The evaluate_condition() function has a case-sensitivity bug that causes feature flags to fail unexpectedly. The documentation claims case-insensitive comparison, but the implementation is case-sensitive for values.

Bug Details

Current Broken Code

// In evaluator.rs - Line ~25
pub fn evaluate_condition(condition: &str, context: &HashMap<String, String>) -> Result<bool> {
    // ...
    if let Some(actual_val) = context.get(var_name.to_uppercase().as_str()) {
        return Ok(*actual_val == expected_val);
        //         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
        //         BUG: String comparison is case-sensitive!
    }
    // ...
}

The Problem

// In build_context()
context.insert("DOCKER".to_string(), "True".to_string());
//                                    ^^^^
//                                    Capital T from JSON

// In evaluate_condition()
let expected_val = parts[1].trim().to_lowercase();  // "true"
let actual_val = context.get("DOCKER");             // "True"

// Comparison:
"True" == "true"  // ❌ FALSE - Bug!

Steps to Reproduce

1. Create test config

{
  "project": {
    "name": "test_project"
  },
  "features": {
    "docker": true,
    "kubernetes": "True",
    "cqrs": "TRUE"
  },
  "directories": [
    {
      "path": "docker",
      "condition": "$DOCKER == true"
    },
    {
      "path": "k8s",
      "condition": "$KUBERNETES == true"
    },
    {
      "path": "cqrs",
      "condition": "$CQRS == true"
    }
  ]
}

2. Run quick-arch

quick-arch --config test.json

3. Expected vs Actual Behavior

Expected:

✅ Create Dir: docker
✅ Create Dir: k8s
✅ Create Dir: cqrs

Actual:

✅ Create Dir: docker     # Works (lowercase "true" from bool)
⏩ Skip Dir: k8s          # FAILS (String "True" != "true")
⏩ Skip Dir: cqrs         # FAILS (String "TRUE" != "true")

Root Cause Analysis

Problem 1: Mixed Type Handling

// In build_context()
for (key, val) in features {
    let value_str = match val {
        Value::String(s) => s.clone(),  // Preserves original case "True"
        Value::Bool(b) => b.to_string(), // Always lowercase "true"
        // ...
    };
}

Problem 2: Inconsistent Normalization

// expected_val is lowercased:
let expected_val = parts[1].trim().to_lowercase();  // ✅

// But actual_val is NOT:
if let Some(actual_val) = context.get(var_name) {
    return Ok(*actual_val == expected_val);  // ❌ Comparison fails
}

Impact

• Severity: High
• Affected Users: Anyone using string values in features
• Workaround Difficulty: Hard (requires editing JSON to match exact case)
• Results in:
  • Unexpected files/directories skipped
  • Silent failures (no error shown)
  • Confusion between bool vs string features
  • Documentation mismatch

Solution

Fix the Comparison

pub fn evaluate_condition(condition: &str, context: &HashMap<String, String>) -> Result<bool> {
    let cond = condition.trim();

    if let Some(rest) = cond.strip_prefix('$') {
        let parts: Vec<&str> = rest.split("==").collect();

        if parts.len() == 2 {
            let var_name = parts[0].trim();
            let expected_val = parts[1]
                .trim()
                .trim_matches(|c| c == '\'' || c == '"')
                .to_lowercase();

            if let Some(actual_val) = context.get(var_name.to_uppercase().as_str()) {
                // FIX: Normalize both sides
                return Ok(actual_val.to_lowercase() == expected_val);
                //        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            } else {
                return Ok(false);
            }
        }
    }

    Ok(false)
}

Alternative: Normalize in build_context()

pub fn build_context(features: &FeatureSet) -> Result<HashMap<String, String>> {
    let mut context = HashMap::new();

    for (key, val) in features {
        let value_str = match val {
            Value::String(s) => s.to_lowercase(),  // Normalize here
            Value::Bool(b) => b.to_string(),       // Already lowercase
            Value::Number(n) => n.to_string(),
            _ => continue,
        };
        context.insert(key.to_uppercase(), value_str);
    }

    Ok(context)
}

Testing

Add Comprehensive Tests

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_case_insensitive_bool_values() {
        let mut ctx = HashMap::new();
        ctx.insert("DOCKER".to_string(), "True".to_string());
        
        assert!(evaluate_condition("$DOCKER == true", &ctx).unwrap());
        assert!(evaluate_condition("$DOCKER == TRUE", &ctx).unwrap());
        assert!(evaluate_condition("$DOCKER == True", &ctx).unwrap());
    }

    #[test]
    fn test_mixed_feature_types() {
        let features = serde_json::json!({
            "docker": true,           // bool
            "kubernetes": "True",     // string
            "cqrs": "TRUE",          // string
            "ddd": "false"           // string
        });
        
        let ctx = build_context(&features.as_object().unwrap().clone()).unwrap();
        
        assert!(evaluate_condition("$DOCKER == true", &ctx).unwrap());
        assert!(evaluate_condition("$KUBERNETES == true", &ctx).unwrap());
        assert!(evaluate_condition("$CQRS == true", &ctx).unwrap());
        assert!(evaluate_condition("$DDD == false", &ctx).unwrap());
    }

    #[test]
    fn test_string_values_with_quotes() {
        let mut ctx = HashMap::new();
        ctx.insert("ENV".to_string(), "production".to_string());
        
        assert!(evaluate_condition("$ENV == 'production'", &ctx).unwrap());
        assert!(evaluate_condition("$ENV == \"production\"", &ctx).unwrap());
        assert!(evaluate_condition("$ENV == Production", &ctx).unwrap());
    }
}

Documentation Updates

Update README to clarify:

## 🧠 Condition Logic

### Case Insensitivity
All comparisons are **case-insensitive**:

```json
{
  "features": {
    "docker": true,      // Any of these work
    "k8s": "True",       // ✅
    "auth": "TRUE"       // ✅
  },
  "directories": [
    { "path": "docker", "condition": "$DOCKER == true" },
    { "path": "k8s", "condition": "$K8S == TRUE" },
    { "path": "auth", "condition": "$AUTH == True" }
  ]
}

All three conditions will evaluate correctly regardless of case.

## Action Items
- [ ] Fix `evaluate_condition()` to use `.to_lowercase()` on both sides
- [ ] Add comprehensive test suite for condition evaluation
- [ ] Test with all case variations (true, True, TRUE, etc.)
- [ ] Update documentation to clearly state case-insensitive behavior
- [ ] Add integration test with mixed bool/string features
- [ ] Consider adding debug logging for condition evaluation

## Regression Risk
**Low** - This is a pure bug fix that makes behavior match documentation. No breaking changes.

## Related Issues
- Affects all projects using string-based feature flags
- May explain user reports of "conditions not working"