devs/profanity
2
0
Fork 0
mirror of https://git.jabber.space/devs/cproof.git synced 2026-07-17 23:26:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(ai): bound response parsing and harden AUTO fallback errors

Browse Source 
- bound _parse_responses scans to the output_text part's object and to
  the content array, so a later sibling item's "text" (e.g. a reasoning
  summary) can never be returned as the assistant reply
- do not retry the other flavour on a curl timeout: the request likely
  reached the server and may still be generating, so a re-POST of the
  conversation could trigger a second billed generation
- remember an unparseable 2xx from the first AUTO attempt and surface
  stashed first-attempt errors in both error paths, instead of showing
  only the final attempt's transport or HTTP error
- recognize Ollama's model-not-found wording in _names_model so a model
  typo is not misread as a missing endpoint
- match model errors case-insensitively and recognize the
  'model "x" is not supported' wording, so a wrong-model 400 (seen live
  on Perplexity /v1/responses) fails fast instead of probing the other
  flavour with a doomed request
- say "empty response body" instead of "Unknown error" when an HTTP
  error arrives without a body (e.g. Perplexity's bare 404)
- drop the dead, racy provider-lookup fallback in the generic request
  thread: a missing provider ref is a caller bug and now fails loudly
- fix ai_providers_lists_defaults to expect the header the command
  actually prints ("Configured providers:"); the test was broken since
  its introduction but CI never ran it
- add functional test group 5 (AI command surface) to FUNC_TEST_GROUPS
  so the CI parallel target runs it; proftest.c port ranges already
  account for five groups
This commit is contained in:
jabber.developer2
2026-07-10 11:36:45 +03:00
parent 3a96d0dba1
commit 37218d2e07
6 changed files with 166 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
Makefile.am
View File

@@ -335,7 +335,7 @@ tests_functionaltests_functionaltests_LDADD = -lcmocka -lstabber @FORKPTY_LIB@
# Parallel functional tests target (~3x faster than sequential)
# Usage: make check-functional-parallel
# To add more groups: increase FUNC_TEST_GROUPS and add group in functionaltests.c
FUNC_TEST_GROUPS = 1 2 3 4
FUNC_TEST_GROUPS = 1 2 3 4 5
check-functional-parallel: tests/functionaltests/functionaltests
@echo "Running functional tests in parallel ($(words $(FUNC_TEST_GROUPS)) groups)..."

150
src/ai/ai_client.c
View File

@@ -380,14 +380,12 @@ _json_unescape_substring(const gchar* start, const gchar* end)
return g_realloc(out, w - out + 1);
}
/* Locate "field":"...value..." in json and return the unescaped value.
* Skips JSON whitespace around the colon and treats any \X (X != \0) inside
* the string body as an opaque escape so an embedded \" cannot be misread as
* the closing quote. */
/* Unescape the string value starting at @val (a pointer to its opening quote).
* Treats any \X (X != \0) inside the string body as an opaque escape so an
* embedded \" cannot be misread as the closing quote. */
static gchar*
_extract_json_string(const gchar* json, const gchar* field)
_extract_json_string_at(const gchar* val)
{
const gchar* val = _find_json_field(json, field);
if (!val || *val != '"')
return NULL;
@@ -404,6 +402,24 @@ _extract_json_string(const gchar* json, const gchar* field)
return NULL;
}
/* Locate "field":"...value..." in json and return the unescaped value.
* Skips JSON whitespace around the colon. @end (optional) bounds the search:
* a value starting at or past it is not extracted. */
static gchar*
_extract_json_string_within(const gchar* json, const gchar* field, const gchar* end)
{
const gchar* val = _find_json_field(json, field);
if (!val || (end && val >= end))
return NULL;
return _extract_json_string_at(val);
}
static gchar*
_extract_json_string(const gchar* json, const gchar* field)
{
return _extract_json_string_within(json, field, NULL);
}
/* ========================================================================
* API type helpers
* ======================================================================== */
@@ -456,11 +472,16 @@ _names_model(const gchar* s)
{
if (!s)
return FALSE;
/* "does not exist" alone also appears in route-missing bodies; require it
* to talk about a model */
return strstr(s, "model_not_found") || strstr(s, "invalid_model")
|| strstr(s, "unknown_model") || strstr(s, "model not found")
|| (strstr(s, "model") && strstr(s, "does not exist"));
/* "does not exist"/"not found"/"not supported" alone also appear in
* route-missing bodies; require them to talk about a model (covers
* Ollama's 'model "x" not found, try pulling it first' and Perplexity's
* 'model "x" is not supported'). Gateways vary in casing, so compare
* case-insensitively. */
auto_gchar gchar* lower = g_ascii_strdown(s, -1);
return strstr(lower, "model_not_found") || strstr(lower, "invalid_model")
|| strstr(lower, "unknown_model")
|| (strstr(lower, "model")
&& (strstr(lower, "does not exist") || strstr(lower, "not found") || strstr(lower, "not supported")));
}
/* A missing endpoint answers 404/405/501. A wrong model can also 404/405, but
@@ -1022,7 +1043,7 @@ typedef void (*ai_response_handler_t)(AIProvider* provider, const gchar* respons
typedef struct
{
gchar* provider_name;
AIProvider* provider; /* owned ref taken on the main thread, or NULL */
AIProvider* provider; /* owned ref taken on the main thread; required */
gpointer user_data;
gchar* request_url;
ai_response_handler_t handler;
@@ -1071,7 +1092,7 @@ _curl_exec_and_handle(CURL* curl, const gchar* url, struct curl_slist* headers,
auto_gchar gchar* error_msg = parsed
? g_strdup_printf("HTTP %ld: %s", http_code, parsed)
: g_strdup_printf("HTTP %ld: %s", http_code,
response->data && strlen(response->data) > 0 ? response->data : "Unknown error");
response->data && strlen(response->data) > 0 ? response->data : "empty response body");
log_error("Request HTTP error for '%s': %s", provider->name, error_msg);
_aiwin_display_error(user_data, error_msg);
} else {
@@ -1086,20 +1107,16 @@ _ai_generic_request_thread(gpointer data)
{
ai_request_t* req = (ai_request_t*)data;
/* Keep the provider alive for the duration of this request. Prefer the ref
* taken on the main thread (guaranteed pre-removal); the lookup fallback
* still has a small window against /ai remove provider. */
/* The ref taken on the main thread keeps the provider alive for the
* duration of this request; a thread-side lookup instead would race
* against /ai remove provider, so its absence is a caller bug */
AIProvider* provider = g_steal_pointer(&req->provider);
if (!provider) {
provider = ai_get_provider(req->provider_name);
if (!provider) {
log_error("Provider '%s' not found for request", req->provider_name);
g_free(req->provider_name);
g_free(req->request_url);
g_free(req);
return NULL;
}
ai_provider_ref(provider);
log_error("BUG: request for '%s' spawned without a provider ref", req->provider_name);
g_free(req->provider_name);
g_free(req->request_url);
g_free(req);
return NULL;
}
CURL* curl = curl_easy_init();
@@ -1135,7 +1152,7 @@ _ai_generic_request_thread(gpointer data)
curl_easy_cleanup(curl);
g_free(response.data);
/* Release the reference taken after lookup — provider may now be freed. */
/* Release the main-thread ref — provider may now be freed. */
ai_provider_unref(provider);
g_free(req->provider_name);
@@ -1833,6 +1850,33 @@ _same_json_object(const gchar* from, const gchar* to)
return depth == 0;
}
/* Pointer to the '}' closing the object that encloses @pos (which must sit
* outside a string literal, e.g. at a key's opening quote), or NULL if
* unterminated. Braces inside string literals are ignored. */
static const gchar*
_json_object_end(const gchar* pos)
{
gboolean in_string = FALSE;
gint depth = 0;
for (const gchar* p = pos; *p; p++) {
if (in_string) {
if (*p == '\\' && *(p + 1) != '\0')
p++;
else if (*p == '"')
in_string = FALSE;
} else if (*p == '"') {
in_string = TRUE;
} else if (*p == '{') {
depth++;
} else if (*p == '}') {
if (depth == 0)
return p;
depth--;
}
}
return NULL;
}
/* Pointer just past the ']' matching the '[' at @arr, or NULL if unterminated.
* Brackets inside string literals are ignored. */
static const gchar*
@@ -1870,7 +1914,11 @@ _parse_responses(const gchar* json)
const gchar* part = strstr(output, "\"output_text\"");
if (part) {
gchar* text = _extract_json_string(part, "text");
/* Bound the forward scan to the part's own object so a "text" from a
* later sibling item (e.g. a following reasoning summary) can never
* be returned as the reply */
const gchar* part_end = _json_object_end(part);
gchar* text = _extract_json_string_within(part, "text", part_end);
if (text)
return text;
/* "text" may precede "type":"output_text" in the part: take the
@@ -1883,7 +1931,7 @@ _parse_responses(const gchar* json)
if (!key)
break;
if (_same_json_object(key, part)) {
text = _extract_json_string(key, "text");
text = _extract_json_string_within(key, "text", part_end);
if (text)
return text;
}
@@ -1899,7 +1947,12 @@ _parse_responses(const gchar* json)
const gchar* content_arr = strstr(output, "\"content\"");
if (!content_arr)
return NULL;
return _extract_json_string(content_arr, "text");
/* Bound the scan to the content array itself: an empty or text-less array
* must fail the parse rather than pick up a "text" from a later item */
const gchar* cval = _find_json_field(content_arr, "content");
if (!cval || *cval != '[')
return NULL;
return _extract_json_string_within(cval, "text", _json_array_end(cval));
}
/* Extract assistant content, trying the known request flavour's extractor
@@ -2066,6 +2119,9 @@ _ai_request_thread(gpointer data)
* actionable error; keep it in case the other flavour also fails */
auto_gchar gchar* first_reject_body = NULL;
long first_reject_code = 0;
/* Same for a 2xx we could not parse: the user must learn their endpoint
* answered, even when the fallback attempt fails with its own error */
auto_gchar gchar* first_unparsable_note = NULL;
auto_gchar gchar* content = NULL;
gboolean first_endpoint_missing = FALSE;
for (gint i = 0; i < n_attempts; i++) {
@@ -2109,9 +2165,12 @@ _ai_request_thread(gpointer data)
continue;
}
/* Connectivity failure (timeout/reset/DNS) on this flavour's endpoint:
* the other flavour may still be reachable, so try it before failing */
if (res != CURLE_OK && more) {
/* Connectivity failure (reset/DNS/refused) on this flavour's endpoint:
* the other flavour may still be reachable, so try it before failing.
* A timeout is excluded: the request likely reached the server and may
* still be generating (and billing) — re-POSTing the conversation to
* the other endpoint could trigger a second generation */
if (res != CURLE_OK && res != CURLE_OPERATION_TIMEDOUT && more) {
log_info("[AI-THREAD] %s request for '%s' failed (%s), trying %s",
ai_api_type_to_string(used_type), local_provider_name,
curl_easy_strerror(res), ai_api_type_to_string(attempts[i + 1]));
@@ -2126,9 +2185,12 @@ _ai_request_thread(gpointer data)
content = ai_parse_response_typed(used_type, response.data);
if (!content && more && configured == AI_API_TYPE_AUTO
&& http_code >= 200 && http_code < 300) {
log_info("[AI-THREAD] %s returned an unparseable 2xx for '%s', trying %s",
log_info("[AI-THREAD] %s returned an unparsable 2xx for '%s', trying %s",
ai_api_type_to_string(used_type), local_provider_name,
ai_api_type_to_string(attempts[i + 1]));
if (!first_unparsable_note)
first_unparsable_note = g_strdup_printf("%s endpoint answered HTTP %ld but the body could not be parsed",
ai_api_type_to_string(used_type), http_code);
continue;
}
}
@@ -2149,7 +2211,20 @@ _ai_request_thread(gpointer data)
}
if (res != CURLE_OK) {
auto_gchar gchar* error_msg = g_strdup(curl_easy_strerror(res));
/* The stashed first-attempt failure is usually the actionable part:
* that endpoint did answer, this one did not */
auto_gchar gchar* first_note = NULL;
if (first_reject_body) {
auto_gchar gchar* parsed = ai_parse_error_message(first_reject_body);
first_note = g_strdup_printf("%s endpoint rejected the request with HTTP %ld: %s",
ai_api_type_to_string(attempts[0]), first_reject_code,
parsed ? parsed : first_reject_body);
} else if (first_unparsable_note) {
first_note = g_strdup(first_unparsable_note);
}
auto_gchar gchar* error_msg = first_note
? g_strdup_printf("%s (first attempt: %s)", curl_easy_strerror(res), first_note)
: g_strdup(curl_easy_strerror(res));
log_error("AI request failed for %s/%s: %s", local_provider_name, local_model, error_msg);
_aiwin_display_error(user_data, error_msg);
} else if (http_code >= 400) {
@@ -2165,7 +2240,12 @@ _ai_request_thread(gpointer data)
response.size, response_data);
/* Try to extract the actual error message from the JSON response */
auto_gchar gchar* parsed_error = ai_parse_error_message(err_body);
auto_gchar gchar* error_msg = parsed_error ? g_strdup_printf("HTTP %ld: %s", err_code, parsed_error) : g_strdup_printf("HTTP %ld: %s", err_code, err_body && strlen(err_body) > 0 ? err_body : "Unknown error");
auto_gchar gchar* error_msg = parsed_error ? g_strdup_printf("HTTP %ld: %s", err_code, parsed_error) : g_strdup_printf("HTTP %ld: %s", err_code, err_body && strlen(err_body) > 0 ? err_body : "empty response body");
if (first_unparsable_note) {
gchar* with_note = g_strdup_printf("%s (first attempt: %s)", error_msg, first_unparsable_note);
g_free(error_msg);
error_msg = with_note;
}
log_error("AI request failed for %s/%s: %s", local_provider_name, local_model, error_msg);
_aiwin_display_error(user_data, error_msg);
} else {

4
tests/functionaltests/test_ai.c
View File

@@ -55,11 +55,11 @@ ai_no_args_shows_help(void** state)
void
ai_providers_lists_defaults(void** state)
{
/* `/ai providers` (no "list") shows the built-in list with URLs. */
/* `/ai providers` (no "list") shows the configured list with URLs. */
prof_input("/ai providers");
prof_timeout(5);
assert_true(prof_output_exact("Available AI providers:"));
assert_true(prof_output_exact("Configured providers:"));
/* At least one URL line is rendered — exact name agnostic. */
assert_true(prof_output_regex("https?://"));
prof_timeout_reset();

42
tests/unittests/test_ai_client.c
View File

@@ -1087,6 +1087,48 @@ test_ai_parse_response_responses_reasoning_only_returns_null(void** state)
assert_null(out);
}
void
test_ai_parse_response_responses_reasoning_after_message_skipped(void** state)
{
/* A reasoning item ordered after the message, with the part's "text"
* preceding "type":"output_text": the forward scan must stay inside the
* part's object and the backscan must find the real answer. */
const gchar* json = "{\"output\":["
"{\"type\":\"message\",\"content\":[{\"text\":\"answer\",\"type\":\"output_text\"}]},"
"{\"type\":\"reasoning\",\"summary\":[{\"type\":\"summary_text\",\"text\":\"secret\"}]}"
"]}";
auto_gchar gchar* out = ai_parse_response(json);
assert_non_null(out);
assert_string_equal("answer", out);
}
void
test_ai_parse_response_responses_empty_content_not_leaked(void** state)
{
/* An empty content array followed by a reasoning item must yield a parse
* failure; the scan must not escape the array and return the summary. */
const gchar* json = "{\"output\":["
"{\"type\":\"message\",\"content\":[]},"
"{\"type\":\"reasoning\",\"summary\":[{\"type\":\"summary_text\",\"text\":\"secret\"}]}"
"]}";
gchar* out = ai_parse_response(json);
assert_null(out);
}
void
test_ai_parse_response_responses_text_value_before_later_item(void** state)
{
/* A reply that is exactly "text" plus a reasoning item after the message:
* the backscan's own extraction must also stay inside the part's object. */
const gchar* json = "{\"output\":["
"{\"type\":\"message\",\"content\":[{\"text\":\"text\",\"type\":\"output_text\"}]},"
"{\"type\":\"reasoning\",\"summary\":[{\"type\":\"summary_text\",\"text\":\"secret\"}]}"
"]}";
auto_gchar gchar* out = ai_parse_response(json);
assert_non_null(out);
assert_string_equal("text", out);
}
void
test_ai_parse_response_choices_in_string_value(void** state)
{

3
tests/unittests/test_ai_client.h
View File

@@ -79,6 +79,9 @@ void test_ai_parse_response_responses_brace_in_text(void** state);
void test_ai_parse_response_choices_logprobs_before_content(void** state);
void test_ai_parse_response_reasoning_not_leaked_without_text(void** state);
void test_ai_parse_response_responses_reasoning_only_returns_null(void** state);
void test_ai_parse_response_responses_reasoning_after_message_skipped(void** state);
void test_ai_parse_response_responses_empty_content_not_leaked(void** state);
void test_ai_parse_response_responses_text_value_before_later_item(void** state);
void test_ai_parse_response_responses_text_value_text(void** state);
void test_ai_parse_response_escaped_quote(void** state);
void test_ai_parse_response_newline_escape(void** state);

3
tests/unittests/unittests.c
View File

@@ -789,6 +789,9 @@ main(int argc, char* argv[])
cmocka_unit_test(test_ai_parse_response_choices_logprobs_before_content),
cmocka_unit_test(test_ai_parse_response_reasoning_not_leaked_without_text),
cmocka_unit_test(test_ai_parse_response_responses_reasoning_only_returns_null),
cmocka_unit_test(test_ai_parse_response_responses_reasoning_after_message_skipped),
cmocka_unit_test(test_ai_parse_response_responses_empty_content_not_leaked),
cmocka_unit_test(test_ai_parse_response_responses_text_value_before_later_item),
cmocka_unit_test(test_ai_parse_response_responses_text_value_text),
cmocka_unit_test(test_ai_parse_response_escaped_quote),
cmocka_unit_test(test_ai_parse_response_newline_escape),