test(ai): autocomplete tests — 5 expected failures documenting real bugs
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.
This commit is contained in:
@@ -1461,3 +1461,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);
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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),
|
||||
|
||||
Reference in New Issue
Block a user