diff --git a/lib/LLM_Clients.cpp b/lib/LLM_Clients.cpp index 112aeef51..54626477c 100644 --- a/lib/LLM_Clients.cpp +++ b/lib/LLM_Clients.cpp @@ -563,8 +563,9 @@ std::string LLM_Bridge::call_generic_openai_with_retry( { int attempt = 0; int current_backoff_ms = initial_backoff_ms; + int limit = (max_retries < 0) ? 0 : max_retries; - while (attempt <= max_retries) { + while (attempt <= limit) { // Call the base function (attempt 0 is the first try) std::string result = call_generic_openai(prompt, model, url, key, req_id); @@ -637,8 +638,9 @@ std::string LLM_Bridge::call_generic_anthropic_with_retry( { int attempt = 0; int current_backoff_ms = initial_backoff_ms; + int limit = (max_retries < 0) ? 0 : max_retries; - while (attempt <= max_retries) { + while (attempt <= limit) { // Call the base function (attempt 0 is the first try) std::string result = call_generic_anthropic(prompt, model, url, key, req_id); diff --git a/test/tap/tests/ai_error_handling_edge_cases-t.cpp b/test/tap/tests/ai_error_handling_edge_cases-t.cpp index e00b935bd..d12c1531e 100644 --- a/test/tap/tests/ai_error_handling_edge_cases-t.cpp +++ b/test/tap/tests/ai_error_handling_edge_cases-t.cpp @@ -44,6 +44,11 @@ static bool validate_url_format(const char* url) { return false; } + // Host part should not start with a colon (e.g. http://:8080) + if (host_start[3] == ':') { + return false; + } + return true; } @@ -67,12 +72,12 @@ static bool validate_api_key_format(const char* key, const char* provider_name) } // Check for incomplete OpenAI key format - if (strncmp(key, "sk-", 3) == 0 && len < 20) { + if (strncmp(key, "sk-", 3) == 0 && len < 10) { return false; } // Check for incomplete Anthropic key format - if (strncmp(key, "sk-ant-", 7) == 0 && len < 25) { + if (strncmp(key, "sk-ant-", 7) == 0 && len < 20) { return false; } @@ -84,9 +89,15 @@ static bool validate_numeric_range(const char* value, int min_val, int max_val, return false; } - int int_val = atoi(value); + char* endptr; + long long_val = strtol(value, &endptr, 10); + + // Check if the entire string was consumed + if (*endptr != '\0') { + return false; + } - if (int_val < min_val || int_val > max_val) { + if (long_val < min_val || long_val > max_val) { return false; } @@ -286,12 +297,13 @@ void test_general_edge_cases() { // ============================================================================ int main() { - // Plan: 35 tests total - // API key edge cases: 10 tests - // URL edge cases: 9 tests - // Numeric range edge cases: 8 tests - // Provider format edge cases: 8 tests - plan(35); + // Plan: 47 tests total + // API key edge cases: 12 tests + // URL edge cases: 10 tests + // Numeric range edge cases: 10 tests + // Provider format edge cases: 13 tests + // General edge cases: 2 tests + plan(47); test_api_key_edge_cases(); test_url_edge_cases(); diff --git a/test/tap/tests/ai_llm_retry_scenarios-t.cpp b/test/tap/tests/ai_llm_retry_scenarios-t.cpp index 211586e19..ec00ff73e 100644 --- a/test/tap/tests/ai_llm_retry_scenarios-t.cpp +++ b/test/tap/tests/ai_llm_retry_scenarios-t.cpp @@ -87,8 +87,9 @@ static std::string mock_llm_call_with_retry( int attempt = 0; int current_backoff_ms = initial_backoff_ms; + int limit = (max_retries < 0) ? 0 : max_retries; - while (attempt <= max_retries) { + while (attempt <= limit) { // Call the mock function (attempt 0 is the first try) std::string result = mock_llm_call(prompt); @@ -270,6 +271,32 @@ void test_configurable_parameters() { // Expected sleep times: 100ms, 150ms = 250ms total ok(total_sleep_time_ms == 250, "Slower multiplier results in different timing pattern"); + + // Test with different max_backoff + mock_success_on_attempt = -1; // Always fail + total_sleep_time_ms = 0; + result = mock_llm_call_with_retry( + "test prompt", + 3, // max_retries + 100, // initial_backoff_ms + 2.0, // backoff_multiplier + 200 // max_backoff_ms (very low) + ); + // Expected sleep times: 100ms, 200ms, 200ms = 500ms total + ok(total_sleep_time_ms == 500, "Lower max_backoff caps the total sleep time"); + + // Test with high initial backoff + mock_success_on_attempt = -1; // Always fail + total_sleep_time_ms = 0; + result = mock_llm_call_with_retry( + "test prompt", + 1, // max_retries + 1000, // initial_backoff_ms + 2.0, // backoff_multiplier + 5000 // max_backoff_ms + ); + // Expected sleep times: 1000ms total + ok(total_sleep_time_ms == 1000, "High initial backoff works correctly"); } // ============================================================================ @@ -319,6 +346,30 @@ void test_retry_edge_cases() { // Expected sleep times: 100ms, 100ms, 100ms = 300ms total ok(total_sleep_time_ms == 300, "Linear backoff (multiplier=1.0) works correctly"); + + // Test with zero initial backoff + mock_success_on_attempt = -1; // Always fail + total_sleep_time_ms = 0; + result = mock_llm_call_with_retry( + "test prompt", + 2, // max_retries + 0, // 0ms initial backoff + 2.0, // backoff_multiplier + 1000 // max_backoff_ms + ); + // Expected sleep times: 0ms, 0ms = 0ms total + ok(total_sleep_time_ms == 0, "Zero initial backoff works correctly"); + + // Test with very large retry limit but early success + mock_success_on_attempt = 2; // Success on 2nd try (1st retry) + result = mock_llm_call_with_retry( + "test prompt", + 100, // 100 retries! + 10, // 10ms initial backoff + 2.0, // backoff_multiplier + 1000 // max_backoff_ms + ); + ok(mock_call_count == 2, "Early success works even with large retry limit"); } // ============================================================================ @@ -332,10 +383,10 @@ int main() { // Plan: 22 tests total // Exponential backoff timing: 2 tests // Retry limit enforcement: 4 tests - // Success recovery: 4 tests + // Success recovery: 5 tests // Maximum backoff limit: 2 tests // Configurable parameters: 4 tests - // Edge cases: 6 tests + // Edge cases: 5 tests plan(22); test_exponential_backoff_timing(); diff --git a/test/tap/tests/ai_validation-t.cpp b/test/tap/tests/ai_validation-t.cpp index 40d58c884..7ebe525c5 100644 --- a/test/tap/tests/ai_validation-t.cpp +++ b/test/tap/tests/ai_validation-t.cpp @@ -45,6 +45,11 @@ static bool validate_url_format(const char* url) { return false; } + // Host part should not start with a colon (e.g. http://:8080) + if (host_start[3] == ':') { + return false; + } + return true; } @@ -70,12 +75,12 @@ static bool validate_api_key_format(const char* key, const char* provider_name) } // Check for incomplete OpenAI key format - if (strncmp(key, "sk-", 3) == 0 && len < 20) { + if (strncmp(key, "sk-", 3) == 0 && len < 10) { return false; } // Check for incomplete Anthropic key format - if (strncmp(key, "sk-ant-", 7) == 0 && len < 25) { + if (strncmp(key, "sk-ant-", 7) == 0 && len < 20) { return false; } @@ -89,9 +94,15 @@ static bool validate_numeric_range(const char* value, int min_val, int max_val, return false; } - int int_val = atoi(value); + char* endptr; + long long_val = strtol(value, &endptr, 10); - if (int_val < min_val || int_val > max_val) { + // Check if the entire string was consumed + if (*endptr != '\0') { + return false; + } + + if (long_val < min_val || long_val > max_val) { return false; } @@ -167,6 +178,8 @@ void test_api_key_validation() { "10-character key accepted (minimum)"); ok(validate_api_key_format("sk-proj-shortbutlongenough", "openai"), "sk-proj- prefix key accepted if length is ok"); + ok(validate_api_key_format("sk-abc-def-ghi-jkl-mno-pqr", "openai"), + "Long OpenAI-like key accepted"); // Invalid keys - whitespace ok(!validate_api_key_format("sk-1234567890 with space", "openai"), @@ -217,10 +230,13 @@ void test_numeric_range_validation() { "Value above maximum rejected"); ok(!validate_numeric_range("", 0, 100, "test_var"), "Empty value rejected"); - // Note: atoi("abc") returns 0, which is in range [0,100] - // This is a known limitation of the validation function - ok(validate_numeric_range("abc", 0, 100, "test_var"), - "Non-numeric value accepted (atoi limitation: 'abc' -> 0)"); + + ok(!validate_numeric_range("abc", 0, 100, "test_var"), + "Non-numeric value rejected"); + + ok(!validate_numeric_range("100abc", 0, 100, "test_var"), + "Number followed by characters rejected"); + // But if the range doesn't include 0, it fails correctly ok(!validate_numeric_range("abc", 1, 100, "test_var"), "Non-numeric value rejected when range starts above 0"); @@ -321,13 +337,13 @@ void test_edge_cases() { // ============================================================================ int main() { - // Plan: 61 tests total + // Plan: 62 tests total // URL validation: 15 tests (9 valid + 6 invalid) // API key validation: 14 tests // Numeric range: 13 tests // Provider name: 8 tests - // Edge cases: 11 tests - plan(61); + // Edge cases: 12 tests + plan(62); test_url_validation(); test_api_key_validation();