From 3fccc046b03b30f18bb1ca1f6216c88947ee2035 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Tue, 12 May 2026 12:50:21 +0300 Subject: [PATCH] =?UTF-8?q?test(ai):=20autocomplete=20tests=20=E2=80=94=20?= =?UTF-8?q?5=20expected=20failures=20documenting=20real=20bugs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 10 tests for ai_models_find and additional edge cases on ai_providers_find. 5 pass as sanity; 5 fail intentionally, documenting two distinct bugs in the autocomplete integration: BUG A — ai_models_find cycling is broken src/ai/ai_client.c ai_models_find() allocates a fresh Autocomplete on every call and frees it before returning. The Autocomplete's last_found cursor is therefore never persisted, so repeated calls with the same prefix always return the first match. Tab+Tab+Tab cannot reach models 2, 3, ... Caught by: - test_ai_models_find_cycles_through_matches - test_ai_models_find_null_search_cycles_all BUG B — providers_ac is never reset between user input cycles src/ai/ai_client.c keeps the static providers_ac AC, but src/command/cmd_ac.c's all_acs[] (which cmd_ac_reset() iterates) does not include &providers_ac. So when the input loop fires the global reset, providers_ac retains its last_found cursor from the previous prefix. Result: tab-completing a new prefix continues from the old cursor position instead of restarting at the head of the new prefix's matches. A provider added mid-session may also be missed depending on cursor position. Caught by: - test_ai_providers_find_state_resets_on_prefix_change - test_ai_providers_find_empty_then_prefix - test_ai_providers_find_after_add_includes_new Sanity (5 passing): - test_ai_models_find_no_models_returns_null - test_ai_models_find_returns_match_for_prefix - test_ai_models_find_no_match_returns_null - test_ai_models_find_previous_direction - test_ai_providers_find_after_remove_skips_removed The failing tests are checked in as-is — they go green when the bugs are fixed. See PR description for suggested fixes. --- tests/unittests/test_ai_client.c | 233 +++++++++++++++++++++++++++++++ tests/unittests/test_ai_client.h | 12 ++ tests/unittests/unittests.c | 11 ++ 3 files changed, 256 insertions(+) diff --git a/tests/unittests/test_ai_client.c b/tests/unittests/test_ai_client.c index ceec048d..c23f64e5 100644 --- a/tests/unittests/test_ai_client.c +++ b/tests/unittests/test_ai_client.c @@ -1465,3 +1465,236 @@ test_ai_prefs_multiple_providers_persist(void** state) assert_string_equal("sk-openai-key", k1); assert_string_equal("pplx-key", k2); } + +/* ======================================================================== + * Autocomplete deeper coverage + * + * Most of these exercise ai_models_find and the persistence semantics of + * ai_providers_find. The fixture build is a bit involved because + * ai_models_find takes a ProfAiWin* through which it walks + * window->session->provider->models — but ai_models_find only dereferences + * the session field, never the ProfWin base or memcheck, so a minimal + * stack-allocated struct is sufficient. + * ======================================================================== */ + +#include "ui/win_types.h" + +/* Helper: add a list of models to a provider via the public model parser. */ +static void +_seed_models(AIProvider* provider, const gchar* const* model_ids) +{ + GString* body = g_string_new("{\"object\":\"list\",\"data\":["); + for (int i = 0; model_ids[i]; i++) { + if (i > 0) { + g_string_append_c(body, ','); + } + g_string_append_printf(body, "{\"id\":\"%s\",\"object\":\"model\"}", model_ids[i]); + } + g_string_append(body, "]}"); + ai_parse_models_from_json(provider, body->str); + g_string_free(body, TRUE); +} + +void +test_ai_models_find_no_models_returns_null(void** state) +{ + /* A provider with no cached models must return NULL, not crash. */ + AIProvider* p = ai_add_provider("acme", "https://acme.example/", NULL); + assert_non_null(p); + /* p->models is NULL initially. */ + + AISession* s = ai_session_create("acme", "anything"); + assert_non_null(s); + ProfAiWin win = { 0 }; + win.session = s; + + auto_gchar gchar* match = ai_models_find("m", FALSE, &win); + assert_null(match); + + ai_session_unref(s); +} + +void +test_ai_models_find_returns_match_for_prefix(void** state) +{ + AIProvider* p = ai_add_provider("acme", "https://acme.example/", NULL); + assert_non_null(p); + const gchar* models[] = { "gpt-4o", "gpt-4o-mini", "o1-preview", NULL }; + _seed_models(p, models); + assert_int_equal(3, g_list_length(p->models)); + + AISession* s = ai_session_create("acme", "anything"); + assert_non_null(s); + ProfAiWin win = { 0 }; + win.session = s; + + auto_gchar gchar* match = ai_models_find("o1", FALSE, &win); + assert_non_null(match); + assert_string_equal("o1-preview", match); + + ai_session_unref(s); +} + +void +test_ai_models_find_no_match_returns_null(void** state) +{ + AIProvider* p = ai_add_provider("acme", "https://acme.example/", NULL); + const gchar* models[] = { "gpt-4o", NULL }; + _seed_models(p, models); + + AISession* s = ai_session_create("acme", "anything"); + ProfAiWin win = { 0 }; + win.session = s; + + auto_gchar gchar* match = ai_models_find("nothing-matches", FALSE, &win); + assert_null(match); + + ai_session_unref(s); +} + +void +test_ai_models_find_cycles_through_matches(void** state) +{ + /* + * BUG-CATCHING TEST. ai_models_find should cycle through all matches + * that share the prefix when called repeatedly (the normal Tab/Tab/Tab + * loop). Today ai_models_find allocates a fresh Autocomplete on every + * call and frees it before returning, so the "last_found" cursor is + * never persisted. Repeated calls with the same prefix return the + * same model and the user cannot reach the others. + * + * Fix: keep the per-provider models Autocomplete static (analogous to + * the static providers_ac used by ai_providers_find). + */ + AIProvider* p = ai_add_provider("acme", "https://acme.example/", NULL); + const gchar* models[] = { "gpt-4o", "gpt-4o-mini", "gpt-4o-nano", NULL }; + _seed_models(p, models); + assert_int_equal(3, g_list_length(p->models)); + + AISession* s = ai_session_create("acme", "anything"); + ProfAiWin win = { 0 }; + win.session = s; + + GHashTable* seen = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL); + for (int i = 0; i < 3; i++) { + auto_gchar gchar* m = ai_models_find("gpt-4o", FALSE, &win); + assert_non_null(m); + assert_true(g_str_has_prefix(m, "gpt-4o")); + g_hash_table_add(seen, g_strdup(m)); + } + assert_int_equal(3, g_hash_table_size(seen)); + + g_hash_table_destroy(seen); + ai_session_unref(s); +} + +void +test_ai_models_find_previous_direction(void** state) +{ + AIProvider* p = ai_add_provider("acme", "https://acme.example/", NULL); + const gchar* models[] = { "alpha", "beta", "gamma", NULL }; + _seed_models(p, models); + + AISession* s = ai_session_create("acme", "anything"); + ProfAiWin win = { 0 }; + win.session = s; + + auto_gchar gchar* forward = ai_models_find("", FALSE, &win); + auto_gchar gchar* back = ai_models_find("", TRUE, &win); + assert_non_null(forward); + assert_non_null(back); + + ai_session_unref(s); +} + +void +test_ai_models_find_null_search_cycles_all(void** state) +{ + AIProvider* p = ai_add_provider("acme", "https://acme.example/", NULL); + const gchar* models[] = { "alpha", "beta", "gamma", NULL }; + _seed_models(p, models); + + AISession* s = ai_session_create("acme", "anything"); + ProfAiWin win = { 0 }; + win.session = s; + + /* Same cycling-broken expectation as the prefix variant. NULL/empty + * search should walk through every model. */ + GHashTable* seen = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL); + for (int i = 0; i < 3; i++) { + auto_gchar gchar* m = ai_models_find(NULL, FALSE, &win); + assert_non_null(m); + g_hash_table_add(seen, g_strdup(m)); + } + assert_int_equal(3, g_hash_table_size(seen)); + + g_hash_table_destroy(seen); + ai_session_unref(s); +} + +void +test_ai_providers_find_state_resets_on_prefix_change(void** state) +{ + /* After cycling within prefix A, switching to a different prefix B + * must restart from the first match of B (not be confused by the + * leftover cursor from the previous prefix). */ + ai_add_provider("alpha-one", "https://a/", NULL); + ai_add_provider("alpha-two", "https://a/", NULL); + ai_add_provider("beta-one", "https://b/", NULL); + + auto_gchar gchar* a1 = ai_providers_find("alpha", FALSE, NULL); + auto_gchar gchar* a2 = ai_providers_find("alpha", FALSE, NULL); + assert_non_null(a1); + assert_non_null(a2); + assert_true(g_str_has_prefix(a1, "alpha")); + assert_true(g_str_has_prefix(a2, "alpha")); + + /* Switching prefix mid-cycle. */ + auto_gchar gchar* b = ai_providers_find("beta", FALSE, NULL); + assert_non_null(b); + assert_string_equal("beta-one", b); +} + +void +test_ai_providers_find_empty_then_prefix(void** state) +{ + /* Cycling with an empty search then narrowing to a prefix should yield + * a result starting with the prefix. */ + auto_gchar gchar* any = ai_providers_find("", FALSE, NULL); + assert_non_null(any); + + auto_gchar gchar* match = ai_providers_find("op", FALSE, NULL); + assert_non_null(match); + assert_true(g_str_has_prefix(match, "op")); +} + +void +test_ai_providers_find_after_remove_skips_removed(void** state) +{ + ai_add_provider("zalpha", "https://z/", NULL); + auto_gchar gchar* before = ai_providers_find("zalpha", FALSE, NULL); + assert_non_null(before); + assert_string_equal("zalpha", before); + + assert_true(ai_remove_provider("zalpha")); + + /* After removal, completion must not return the removed provider. */ + auto_gchar gchar* after = ai_providers_find("zalpha", FALSE, NULL); + assert_null(after); +} + +void +test_ai_providers_find_after_add_includes_new(void** state) +{ + /* A provider added after autocomplete state was first initialized + * must appear in subsequent completions. Catches the regression + * where the static Autocomplete is populated only at init. */ + auto_gchar gchar* prime = ai_providers_find("", FALSE, NULL); + (void)prime; + + ai_add_provider("xenon", "https://x/", NULL); + + auto_gchar gchar* match = ai_providers_find("xen", FALSE, NULL); + assert_non_null(match); + assert_string_equal("xenon", match); +} diff --git a/tests/unittests/test_ai_client.h b/tests/unittests/test_ai_client.h index 3e4d0689..5c69ebf5 100644 --- a/tests/unittests/test_ai_client.h +++ b/tests/unittests/test_ai_client.h @@ -132,3 +132,15 @@ void test_ai_parse_models_multiple_models(void** state); void test_ai_prefs_round_trip_api_key(void** state); void test_ai_prefs_round_trip_remove_key(void** state); void test_ai_prefs_multiple_providers_persist(void** state); + +/* Autocomplete deeper coverage */ +void test_ai_models_find_no_models_returns_null(void** state); +void test_ai_models_find_returns_match_for_prefix(void** state); +void test_ai_models_find_no_match_returns_null(void** state); +void test_ai_models_find_cycles_through_matches(void** state); +void test_ai_models_find_previous_direction(void** state); +void test_ai_models_find_null_search_cycles_all(void** state); +void test_ai_providers_find_state_resets_on_prefix_change(void** state); +void test_ai_providers_find_empty_then_prefix(void** state); +void test_ai_providers_find_after_remove_skips_removed(void** state); +void test_ai_providers_find_after_add_includes_new(void** state); diff --git a/tests/unittests/unittests.c b/tests/unittests/unittests.c index d95faeb4..b99afe8d 100644 --- a/tests/unittests/unittests.c +++ b/tests/unittests/unittests.c @@ -775,6 +775,17 @@ main(int argc, char* argv[]) cmocka_unit_test_setup_teardown(test_ai_prefs_round_trip_api_key, ai_client_setup_with_prefs, ai_client_teardown_with_prefs), cmocka_unit_test_setup_teardown(test_ai_prefs_round_trip_remove_key, ai_client_setup_with_prefs, ai_client_teardown_with_prefs), cmocka_unit_test_setup_teardown(test_ai_prefs_multiple_providers_persist, ai_client_setup_with_prefs, ai_client_teardown_with_prefs), + /* Autocomplete deeper coverage */ + cmocka_unit_test_setup_teardown(test_ai_models_find_no_models_returns_null, ai_client_setup, ai_client_teardown), + cmocka_unit_test_setup_teardown(test_ai_models_find_returns_match_for_prefix, ai_client_setup, ai_client_teardown), + cmocka_unit_test_setup_teardown(test_ai_models_find_no_match_returns_null, ai_client_setup, ai_client_teardown), + cmocka_unit_test_setup_teardown(test_ai_models_find_cycles_through_matches, ai_client_setup, ai_client_teardown), + cmocka_unit_test_setup_teardown(test_ai_models_find_previous_direction, ai_client_setup, ai_client_teardown), + cmocka_unit_test_setup_teardown(test_ai_models_find_null_search_cycles_all, ai_client_setup, ai_client_teardown), + cmocka_unit_test_setup_teardown(test_ai_providers_find_state_resets_on_prefix_change, ai_client_setup, ai_client_teardown), + cmocka_unit_test_setup_teardown(test_ai_providers_find_empty_then_prefix, ai_client_setup, ai_client_teardown), + cmocka_unit_test_setup_teardown(test_ai_providers_find_after_remove_skips_removed, ai_client_setup, ai_client_teardown), + cmocka_unit_test_setup_teardown(test_ai_providers_find_after_add_includes_new, ai_client_setup, ai_client_teardown), // Flatfile export/import round-trip cmocka_unit_test(test_ff_roundtrip_simple_chat), cmocka_unit_test(test_ff_roundtrip_with_all_metadata),