Merge pull request #6 from frankbria/fix/circuit-breaker-false-positives

Fix circuit breaker false positives from JSON field names

lib/response_analyzer.sh

@@ -15,7 +15,6 @@ NC='\033[0m'
 # Analysis configuration
 COMPLETION_KEYWORDS=("done" "complete" "finished" "all tasks complete" "project complete" "ready for review")
 TEST_ONLY_PATTERNS=("npm test" "bats" "pytest" "jest" "cargo test" "go test" "running tests")
-STUCK_INDICATORS=("error" "failed" "cannot" "unable to" "blocked")
 NO_WORK_PATTERNS=("nothing to do" "no changes" "already implemented" "up to date")
 
 # Analyze Claude Code response and extract signals
@@ -89,7 +88,13 @@ analyze_response() {
     fi
 
     # 4. Detect stuck/error loops
-    error_count=$(grep -c -i "error\|failed\|cannot\|unable" "$output_file" 2>/dev/null | head -1 || echo "0")
+    # Use two-stage filtering to avoid counting JSON field names as errors
+    # Stage 1: Filter out JSON field patterns like "is_error": false
+    # Stage 2: Count actual error messages in specific contexts
+    # Pattern aligned with ralph_loop.sh to ensure consistent behavior
+    error_count=$(grep -v '"[^"]*error[^"]*":' "$output_file" 2>/dev/null | \
+                  grep -cE '(^Error:|^ERROR:|^error:|\]: error|Link: error|Error occurred|failed with error|[Ee]xception|Fatal|FATAL)' \
+                  2>/dev/null || echo "0")
     error_count=$(echo "$error_count" | tr -d '[:space:]')
     error_count=${error_count:-0}
     error_count=$((error_count + 0))
@@ -257,23 +262,38 @@ detect_stuck_loop() {
         return 1  # Not enough history
     fi
 
-    # Extract key errors from current output
+    # Extract key errors from current output using two-stage filtering
-    local current_errors=$(grep -i "error\|failed" "$current_output" 2>/dev/null | sort | uniq)
+    # Stage 1: Filter out JSON field patterns to avoid false positives
+    # Stage 2: Extract actual error messages
+    local current_errors=$(grep -v '"[^"]*error[^"]*":' "$current_output" 2>/dev/null | \
+                          grep -E '(^Error:|^ERROR:|^error:|\]: error|Link: error|Error occurred|failed with error|[Ee]xception|Fatal|FATAL)' 2>/dev/null | \
+                          sort | uniq)
 
     if [[ -z "$current_errors" ]]; then
         return 1  # No errors
     fi
 
     # Check if same errors appear in all recent outputs
-    local stuck_count=0
+    # For multi-line errors, verify ALL error lines appear in ALL history files
+    local all_files_match=true
     while IFS= read -r output_file; do
-        if grep -q "$current_errors" "$output_file" 2>/dev/null; then
+        local file_matches_all=true
-            ((stuck_count++))
+        while IFS= read -r error_line; do
+            # Use -F for literal fixed-string matching (not regex)
+            if ! grep -qF "$error_line" "$output_file" 2>/dev/null; then
+                file_matches_all=false
+                break
+            fi
+        done <<< "$current_errors"
+
+        if [[ "$file_matches_all" != "true" ]]; then
+            all_files_match=false
+            break
         fi
     done <<< "$recent_outputs"
 
-    if [[ $stuck_count -ge 3 ]]; then
+    if [[ "$all_files_match" == "true" ]]; then
-        return 0  # Stuck on same error
+        return 0  # Stuck on same error(s)
     else
         return 1  # Making progress or different errors
     fi

ralph_loop.sh

@@ -386,8 +386,25 @@ EOF
             # Get file change count for circuit breaker
             local files_changed=$(git diff --name-only 2>/dev/null | wc -l || echo 0)
             local has_errors="false"
-            if grep -q "error\|Error\|ERROR" "$output_file"; then
+
+            # Two-stage error detection to avoid JSON field false positives
+            # Stage 1: Filter out JSON field patterns like "is_error": false
+            # Stage 2: Look for actual error messages in specific contexts
+            # Avoid type annotations like "error: Error" by requiring lowercase after ": error"
+            if grep -v '"[^"]*error[^"]*":' "$output_file" 2>/dev/null | \
+               grep -qE '(^Error:|^ERROR:|^error:|\]: error|Link: error|Error occurred|failed with error|[Ee]xception|Fatal|FATAL)'; then
                 has_errors="true"
+
+                # Debug logging: show what triggered error detection
+                if [[ "$VERBOSE_PROGRESS" == "true" ]]; then
+                    log_status "DEBUG" "Error patterns found:"
+                    grep -v '"[^"]*error[^"]*":' "$output_file" 2>/dev/null | \
+                        grep -nE '(^Error:|^ERROR:|^error:|\]: error|Link: error|Error occurred|failed with error|[Ee]xception|Fatal|FATAL)' | \
+                        head -3 | while IFS= read -r line; do
+                        log_status "DEBUG" "  $line"
+                    done
+                fi
+
                 log_status "WARN" "Errors detected in output, check: $output_file"
             fi
             local output_length=$(wc -c < "$output_file" 2>/dev/null || echo 0)

tests/test_error_detection.sh

@@ -0,0 +1,209 @@
+#!/bin/bash
+# Test script for error detection fix
+# Validates that JSON field names don't trigger false positives
+#
+# TEST STRATEGY:
+# This suite validates the two-stage error detection implemented in ralph_loop.sh
+# and lib/response_analyzer.sh to prevent circuit breaker false positives from
+# JSON output and other structured data formats.
+#
+# Two-Stage Filtering Approach:
+#   Stage 1: Filter out JSON field patterns (e.g., "is_error": false, "error": null)
+#            Pattern: grep -v '"[^"]*error[^"]*":'
+#
+#   Stage 2: Detect actual errors using context-specific patterns
+#            Patterns: ^Error:, ^ERROR:, ]: error, Exception, Fatal, etc.
+#            Avoids: Type annotations (error: Error), bare words (cannot, unable)
+#
+# Test Coverage (13 scenarios):
+#   - JSON fields with "error" keyword (tests 1, 2, 8)
+#   - Actual error messages with context (tests 3, 4, 7, 9)
+#   - Mixed JSON + real errors (test 5)
+#   - Benign content that should NOT trigger (tests 6, 10a, 11)
+#   - Code/diffs with error keywords (test 12)
+#   - Edge cases and pattern validation (test 10)
+#
+# Pattern Consistency:
+# Both ralph_loop.sh and lib/response_analyzer.sh use identical patterns to ensure
+# consistent behavior across the codebase. This test suite validates both implementations.
+
+set -e
+
+# Colors for output
+RED='\033[0;31m'
+GREEN='\033[0;32m'
+YELLOW='\033[1;33m'
+NC='\033[0m' # No Color
+
+# Test counter
+TESTS_PASSED=0
+TESTS_FAILED=0
+
+# Create temporary directory for test files
+TEST_DIR=$(mktemp -d)
+trap 'rm -rf "$TEST_DIR"' EXIT
+
+# Helper function to run tests
+run_test() {
+    local test_name="$1"
+    local test_file="$2"
+    local expected_result="$3"  # "true" or "false"
+
+    echo -e "\n${YELLOW}Running test: $test_name${NC}"
+
+    # Apply the error detection logic (same as in ralph_loop.sh)
+    local has_errors="false"
+    if grep -v '"[^"]*error[^"]*":' "$test_file" 2>/dev/null | \
+       grep -qE '(^Error:|^ERROR:|^error:|\]: error|Link: error|Error occurred|failed with error|[Ee]xception|Fatal|FATAL)'; then
+        has_errors="true"
+    fi
+
+    # Check result
+    if [[ "$has_errors" == "$expected_result" ]]; then
+        echo -e "${GREEN}✓ PASS${NC} - Expected: $expected_result, Got: $has_errors"
+        TESTS_PASSED=$((TESTS_PASSED + 1))
+    else
+        echo -e "${RED}✗ FAIL${NC} - Expected: $expected_result, Got: $has_errors"
+        echo "File contents:"
+        cat "$test_file"
+        TESTS_FAILED=$((TESTS_FAILED + 1))
+    fi
+}
+
+echo "========================================"
+echo "Error Detection Test Suite"
+echo "========================================"
+
+# Test 1: JSON with "is_error": false should NOT trigger
+cat > "$TEST_DIR/test1.txt" << 'EOF'
+{
+  "status": "success",
+  "is_error": false,
+  "message": "Operation completed successfully"
+}
+EOF
+run_test "JSON with is_error: false" "$TEST_DIR/test1.txt" "false"
+
+# Test 2: JSON with "error": null should NOT trigger
+cat > "$TEST_DIR/test2.txt" << 'EOF'
+{
+  "status": "ok",
+  "error": null,
+  "error_count": 0,
+  "has_error": false
+}
+EOF
+run_test "JSON with error: null" "$TEST_DIR/test2.txt" "false"
+
+# Test 3: Actual error message SHOULD trigger
+cat > "$TEST_DIR/test3.txt" << 'EOF'
+Running build process...
+Error: Failed to compile src/main.ts
+  Type error on line 42
+EOF
+run_test "Actual error message" "$TEST_DIR/test3.txt" "true"
+
+# Test 4: Exception in log SHOULD trigger
+cat > "$TEST_DIR/test4.txt" << 'EOF'
+[2025-12-31 10:30:00] INFO: Starting process
+[2025-12-31 10:30:05] ERROR: Unhandled exception in handler
+Exception: NullPointerException at line 123
+EOF
+run_test "Exception in log" "$TEST_DIR/test4.txt" "true"
+
+# Test 5: Mixed content - JSON + real error SHOULD trigger
+cat > "$TEST_DIR/test5.txt" << 'EOF'
+{
+  "is_error": false,
+  "status": "running"
+}
+Processing files...
+Fatal: Segmentation fault in module X
+EOF
+run_test "Mixed JSON and real error" "$TEST_DIR/test5.txt" "true"
+
+# Test 6: Normal output without errors should NOT trigger
+cat > "$TEST_DIR/test6.txt" << 'EOF'
+Build successful
+All tests passed
+Deployment complete
+EOF
+run_test "Normal output no errors" "$TEST_DIR/test6.txt" "false"
+
+# Test 7: Error in context (colon after) SHOULD trigger
+cat > "$TEST_DIR/test7.txt" << 'EOF'
+[BUILD] Compiling...
+[BUILD] Link: error: undefined reference to 'main'
+[BUILD] Failed
+EOF
+run_test "Error with context (colon)" "$TEST_DIR/test7.txt" "true"
+
+# Test 8: Multiple JSON fields with "error" should NOT trigger
+cat > "$TEST_DIR/test8.txt" << 'EOF'
+{
+  "error_message": "",
+  "is_error": false,
+  "error_code": 0,
+  "has_errors": false,
+  "error_list": []
+}
+EOF
+run_test "Multiple JSON error fields" "$TEST_DIR/test8.txt" "false"
+
+# Test 9: Case sensitivity - ERROR SHOULD trigger
+cat > "$TEST_DIR/test9.txt" << 'EOF'
+SYSTEM LOG:
+ERROR: Database connection failed
+Retrying...
+EOF
+run_test "Uppercase ERROR message" "$TEST_DIR/test9.txt" "true"
+
+# Test 10: Error message with descriptive text SHOULD trigger
+cat > "$TEST_DIR/test10.txt" << 'EOF'
+Build process started
+Error: unable to access file system
+Deployment failed
+EOF
+run_test "Error prefix with descriptive message" "$TEST_DIR/test10.txt" "true"
+
+# Test 10a: Bare "cannot" and "unable" without error prefix should NOT trigger
+cat > "$TEST_DIR/test10a.txt" << 'EOF'
+This feature cannot be enabled in demo mode.
+The user is unable to access this resource due to permissions.
+Configuration cannot be modified at runtime.
+EOF
+run_test "Bare cannot/unable without error context" "$TEST_DIR/test10a.txt" "false"
+
+# Test 11: Documentation mentioning "error" should NOT trigger
+cat > "$TEST_DIR/test11.txt" << 'EOF'
+# Error Handling Guide
+
+This document describes how to handle errors in the application.
+When an error occurs, the system will log it and continue.
+EOF
+run_test "Documentation about errors" "$TEST_DIR/test11.txt" "false"
+
+# Test 12: Git diff with error keywords should NOT trigger
+cat > "$TEST_DIR/test12.txt" << 'EOF'
+diff --git a/src/error.ts b/src/error.ts
++export class ErrorHandler {
++  handleError(error: Error) {
++    console.log(error);
+EOF
+run_test "Git diff with error class" "$TEST_DIR/test12.txt" "false"
+
+# Print summary
+echo ""
+echo "========================================"
+echo "Test Summary"
+echo "========================================"
+echo -e "${GREEN}Passed: $TESTS_PASSED${NC}"
+echo -e "${RED}Failed: $TESTS_FAILED${NC}"
+echo "========================================"
+
+if [[ $TESTS_FAILED -gt 0 ]]; then
+    exit 1
+else
+    echo -e "${GREEN}All tests passed!${NC}"
+    exit 0
+fi

tests/test_stuck_loop_detection.sh

@@ -0,0 +1,241 @@
+#!/bin/bash
+# Test script for detect_stuck_loop function
+# Validates that the stuck loop detection uses two-stage filtering
+# to avoid false positives from JSON fields
+#
+# TEST STRATEGY:
+# The detect_stuck_loop function extracts errors from current output and checks
+# if the same errors appear in the last 3 historical outputs. This test validates:
+#
+# 1. Two-stage filtering is applied (same as analyze_response)
+# 2. JSON field names don't cause false stuck loop detection
+# 3. Actual repeated errors are correctly detected
+# 4. Function returns appropriate exit codes
+
+set -e
+
+# Colors for output
+RED='\033[0;31m'
+GREEN='\033[0;32m'
+YELLOW='\033[1;33m'
+NC='\033[0m' # No Color
+
+# Test counter
+TESTS_PASSED=0
+TESTS_FAILED=0
+
+# Create temporary directory for test files
+TEST_DIR=$(mktemp -d)
+HISTORY_DIR="$TEST_DIR/logs"
+mkdir -p "$HISTORY_DIR"
+trap 'rm -rf "$TEST_DIR"' EXIT
+
+# Source the response_analyzer.sh to get access to detect_stuck_loop function
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+source "$SCRIPT_DIR/../lib/response_analyzer.sh"
+
+# Helper function to run tests
+run_test() {
+    local test_name="$1"
+    local expected_result="$2"  # 0 = stuck detected, 1 = not stuck
+
+    echo -e "\n${YELLOW}Running test: $test_name${NC}"
+
+    # Call detect_stuck_loop function
+    local result=1
+    if detect_stuck_loop "$TEST_DIR/current_output.log" "$HISTORY_DIR"; then
+        result=0
+    else
+        result=1
+    fi
+
+    # Check result
+    if [[ $result -eq $expected_result ]]; then
+        echo -e "${GREEN}✓ PASS${NC} - Expected exit code: $expected_result, Got: $result"
+        TESTS_PASSED=$((TESTS_PASSED + 1))
+    else
+        echo -e "${RED}✗ FAIL${NC} - Expected exit code: $expected_result, Got: $result"
+        echo "Current output:"
+        cat "$TEST_DIR/current_output.log"
+        echo "History files:"
+        ls -la "$HISTORY_DIR"
+        TESTS_FAILED=$((TESTS_FAILED + 1))
+    fi
+}
+
+echo "========================================"
+echo "Stuck Loop Detection Test Suite"
+echo "========================================"
+
+# Test 1: No history - should return not stuck (exit code 1)
+cat > "$TEST_DIR/current_output.log" << 'EOF'
+Error: Build failed
+EOF
+# Empty history directory
+rm -f "$HISTORY_DIR"/*
+run_test "No history available" 1
+
+# Create history directory again for next tests
+mkdir -p "$HISTORY_DIR"
+
+# Test 2: JSON with "is_error": false should NOT trigger stuck detection
+cat > "$TEST_DIR/current_output.log" << 'EOF'
+{
+  "is_error": false,
+  "error_count": 0,
+  "status": "success"
+}
+EOF
+# Create 3 history files with same JSON
+for i in 1 2 3; do
+    cat > "$HISTORY_DIR/claude_output_00${i}.log" << 'EOF'
+{
+  "is_error": false,
+  "error_count": 0,
+  "status": "success"
+}
+EOF
+done
+run_test "JSON fields should not trigger stuck detection" 1
+
+# Test 3: Actual repeated errors should trigger stuck detection
+cat > "$TEST_DIR/current_output.log" << 'EOF'
+Build started
+Error: Failed to compile src/main.ts
+Type error on line 42
+EOF
+# Create 3 history files with same error
+for i in 1 2 3; do
+    sleep 0.1  # Ensure different timestamps
+    cat > "$HISTORY_DIR/claude_output_00${i}.log" << 'EOF'
+Build started
+Error: Failed to compile src/main.ts
+Type error on line 42
+EOF
+done
+run_test "Repeated actual errors trigger stuck detection" 0
+
+# Test 4: Different errors should NOT trigger stuck detection
+cat > "$TEST_DIR/current_output.log" << 'EOF'
+Error: Database connection failed
+EOF
+# Create history with different errors
+sleep 0.1
+cat > "$HISTORY_DIR/claude_output_001.log" << 'EOF'
+Error: File not found
+EOF
+sleep 0.1
+cat > "$HISTORY_DIR/claude_output_002.log" << 'EOF'
+Error: Permission denied
+EOF
+sleep 0.1
+cat > "$HISTORY_DIR/claude_output_003.log" << 'EOF'
+Error: Network timeout
+EOF
+run_test "Different errors should not trigger stuck detection" 1
+
+# Test 5: No errors in current output should return not stuck
+cat > "$TEST_DIR/current_output.log" << 'EOF'
+Build successful
+All tests passed
+Deployment complete
+EOF
+# History doesn't matter if current has no errors
+run_test "No errors in current output" 1
+
+# Test 6: Mixed JSON + real error - only real error should be extracted
+cat > "$TEST_DIR/current_output.log" << 'EOF'
+{
+  "is_error": false,
+  "status": "processing"
+}
+Error: Compilation failed
+EOF
+# Create history with same real error (JSON part varies)
+for i in 1 2 3; do
+    sleep 0.1
+    cat > "$HISTORY_DIR/claude_output_00${i}.log" << 'EOF'
+{
+  "is_error": false,
+  "status": "different"
+}
+Error: Compilation failed
+EOF
+done
+run_test "Mixed JSON and error - only error matters" 0
+
+# Test 7: Type annotations should not trigger stuck detection
+cat > "$TEST_DIR/current_output.log" << 'EOF'
+diff --git a/src/error.ts b/src/error.ts
++export class ErrorHandler {
++  handleError(error: Error) {
++    console.log(error);
+EOF
+# Create history with similar code diffs
+for i in 1 2 3; do
+    sleep 0.1
+    cat > "$HISTORY_DIR/claude_output_00${i}.log" << 'EOF'
+diff --git a/src/error.ts b/src/error.ts
++export class ErrorHandler {
++  handleError(error: Error) {
++    console.log(error);
+EOF
+done
+run_test "Type annotations should not trigger stuck detection" 1
+
+# Test 8: Multiple distinct errors - ALL must appear in history to be stuck
+cat > "$TEST_DIR/current_output.log" << 'EOF'
+Build process started
+Error: Failed to compile src/main.ts
+Fatal: Database connection lost
+Exception: NullPointerException at line 123
+EOF
+# Create history where ALL three errors appear in all files
+for i in 1 2 3; do
+    sleep 0.1
+    cat > "$HISTORY_DIR/claude_output_00${i}.log" << 'EOF'
+Build process started
+Error: Failed to compile src/main.ts
+Fatal: Database connection lost
+Exception: NullPointerException at line 123
+EOF
+done
+run_test "Multiple distinct errors - all repeated (stuck)" 0
+
+# Test 9: Multiple errors but not all appear in history - should NOT be stuck
+cat > "$TEST_DIR/current_output.log" << 'EOF'
+Error: Failed to compile src/main.ts
+Fatal: Database connection lost
+EOF
+# Create history where only the first error appears consistently
+cat > "$HISTORY_DIR/claude_output_001.log" << 'EOF'
+Error: Failed to compile src/main.ts
+Warning: Memory usage high
+EOF
+sleep 0.1
+cat > "$HISTORY_DIR/claude_output_002.log" << 'EOF'
+Error: Failed to compile src/main.ts
+Different issue here
+EOF
+sleep 0.1
+cat > "$HISTORY_DIR/claude_output_003.log" << 'EOF'
+Error: Failed to compile src/main.ts
+Another different error
+EOF
+run_test "Multiple errors but not all repeated (not stuck)" 1
+
+# Print summary
+echo ""
+echo "========================================"
+echo "Test Summary"
+echo "========================================"
+echo -e "${GREEN}Passed: $TESTS_PASSED${NC}"
+echo -e "${RED}Failed: $TESTS_FAILED${NC}"
+echo "========================================"
+
+if [[ $TESTS_FAILED -gt 0 ]]; then
+    exit 1
+else
+    echo -e "${GREEN}All tests passed!${NC}"
+    exit 0
+fi