Skip to content

fix(agent): keep history when compaction cannot finish the turn #126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cursor wants to merge 1 commit into from
Closed

fix(agent): keep history when compaction cannot finish the turn #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 106 additions & 11 deletions src/core/agent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,80 @@ static int copy_printed_session_json(char *dst, size_t dst_size, char *printed,

static const char SUMMARIZE_SYSTEM[] = "Summarize the following conversation in one short paragraph. Output only the summary, no preamble.";

/** True when the kept tail already starts with user, so a summary user turn would not alternate. */
static int kept_tail_starts_with_user(const cJSON *root, int start)
{
const cJSON *item;
const cJSON *role;

item = cJSON_GetArrayItem(root, start);
if (!item || !cJSON_IsObject(item))
return 0;
role = cJSON_GetObjectItem(item, "role");
return cJSON_IsString(role) && role->valuestring && strcmp(role->valuestring, "user") == 0;
}

/** Fold the summary into an existing user turn. A second user message 400s on Anthropic. */
static int fold_summary_into_user(cJSON *item, const char *summary)
{
const cJSON *content;
const char *old;
char *merged;
cJSON *node;
size_t old_len;
size_t sum_len;
size_t n;

content = cJSON_GetObjectItem(item, "content");
old = (content && cJSON_IsString(content) && content->valuestring) ? content->valuestring : "";
old_len = strlen(old);
sum_len = strlen(summary ? summary : "");
if (sum_len > SIZE_MAX - old_len - 3)
return -1;
n = sum_len + 2 + old_len + 1;
merged = malloc(n);
if (!merged)
return -1;
snprintf(merged, n, "%s\n\n%s", summary ? summary : "", old);
node = cJSON_CreateString(merged);
free(merged);
if (!node)
return -1;
if (cJSON_GetObjectItem(item, "content")) {
if (!cJSON_ReplaceItemInObject(item, "content", node)) {
cJSON_Delete(node);
return -1;
}
return 0;
}
if (!cJSON_AddItemToObject(item, "content", node)) {
cJSON_Delete(node);
return -1;
}
return 0;
}

static int append_summary_user(cJSON *arr, const char *summary)
{
cJSON *obj;
cJSON *role;
cJSON *content;

obj = cJSON_CreateObject();
role = cJSON_CreateString("user");
content = cJSON_CreateString(summary ? summary : "");
if (!obj || !role || !content) {
cJSON_Delete(obj);
cJSON_Delete(role);
cJSON_Delete(content);
return -1;
}
cJSON_AddItemToObject(obj, "role", role);
cJSON_AddItemToObject(obj, "content", content);
cJSON_AddItemToArray(arr, obj);
return 0;
}

/** Summarize oldest messages when over max_ctx; replace with one summary + trailing. */
static int compact_session_via_llm(const char *session_id, char *session_buf, size_t session_buf_size,
int msg_count, int max_ctx, const provider_t *provider)
Expand Down Expand Up @@ -128,29 +202,50 @@ static int compact_session_via_llm(const char *session_id, char *session_buf, si
summary_buf[sum_len] = '\0';
provider_response_clear(&resp);
cJSON *new_arr = cJSON_CreateArray();
if (!new_arr) { cJSON_Delete(root); return -1; }
cJSON *summ_obj = cJSON_CreateObject();
if (summ_obj) {
cJSON_AddItemToObject(summ_obj, "role", cJSON_CreateString("user"));
cJSON_AddItemToObject(summ_obj, "content", cJSON_CreateString(summary_buf));
cJSON_AddItemToArray(new_arr, summ_obj);
}
int keep = max_ctx - 1;
int start = msg_count - keep;
int merge;
int folded = 0;
if (start < 0) start = 0;
/* Tail can start on a user turn (odd max_context_messages, or a cut that
* lands on user). Inserting another user role makes Anthropic reject the
* session on this turn and every later one. */
merge = kept_tail_starts_with_user(root, start);
if (!new_arr) { cJSON_Delete(root); return -1; }
if (!merge && append_summary_user(new_arr, summary_buf) != 0) {
cJSON_Delete(new_arr);
cJSON_Delete(root);
return -1;
}
for (int i = start; i < msg_count; i++) {
const cJSON *item = cJSON_GetArrayItem(root, i);
if (item) {
cJSON *dup = cJSON_Duplicate(item, 1);
if (dup) cJSON_AddItemToArray(new_arr, dup);
cJSON *dup;
if (!item) continue;
dup = cJSON_Duplicate(item, 1);
if (!dup) continue;
if (merge && i == start) {
if (fold_summary_into_user(dup, summary_buf) != 0) {
cJSON_Delete(dup);
cJSON_Delete(new_arr);
cJSON_Delete(root);
return -1;
}
folded = 1;
}
cJSON_AddItemToArray(new_arr, dup);
}
if (merge && !folded) {
cJSON_Delete(new_arr);
cJSON_Delete(root);
return -1;
}
cJSON_Delete(root);
char *printed = cJSON_PrintUnformatted(new_arr);
cJSON_Delete(new_arr);
if (copy_printed_session_json(session_buf, session_buf_size, printed, session_id) != 0)
return -1;
session_save(session_id, session_buf);
/* Do not session_save here. A failed follow-up chat must leave the stored
* transcript unchanged; agent_persist_session writes this buffer on success. */
return 0;
}

Expand Down
241 changes: 241 additions & 0 deletions tests/test_agent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,245 @@ static int test_context_compaction_when_history_exceeds_max(void)
return failed;
}

static int save_alternating_session(const char *session_id, int count)
{
char *json;
size_t cap;
size_t off = 0;
int i;
int rc;

cap = (size_t)count * 80u + 2u;
json = malloc(cap);
if (!json)
return -1;
off += (size_t)snprintf(json + off, cap - off, "[");
for (i = 0; i < count; i++) {
const char *role = (i % 2 == 0) ? "user" : "assistant";
int n = snprintf(json + off, cap - off,
"%s{\"role\":\"%s\",\"content\":\"msg_%d\"}",
i ? "," : "", role, i);
if (n < 0 || (size_t)n >= cap - off) {
free(json);
return -1;
}
off += (size_t)n;
}
if (off + 1 >= cap) {
free(json);
return -1;
}
json[off++] = ']';
json[off] = '\0';
rc = session_save(session_id, json);
free(json);
return rc;
}

static int json_roles_alternate_from_user(const char *json)
{
cJSON *root;
int n;
int i;
const char *prev = NULL;

root = cJSON_Parse(json);
if (!root || !cJSON_IsArray(root)) {
if (root)
cJSON_Delete(root);
return 0;
}
n = cJSON_GetArraySize(root);
if (n <= 0) {
cJSON_Delete(root);
return 0;
}
for (i = 0; i < n; i++) {
cJSON *item = cJSON_GetArrayItem(root, i);
cJSON *role = item ? cJSON_GetObjectItem(item, "role") : NULL;
const char *r = (role && cJSON_IsString(role) && role->valuestring) ? role->valuestring : "";
if (i == 0 && strcmp(r, "user") != 0) {
cJSON_Delete(root);
return 0;
}
if (prev && strcmp(prev, r) == 0) {
cJSON_Delete(root);
return 0;
}
prev = r;
}
cJSON_Delete(root);
return 1;
}

static int compact_fail_calls;
static int compact_fail_init(const config_t *cfg)
{
(void)cfg;
compact_fail_calls = 0;
return 0;
}
static int compact_fail_chat(const provider_message_t *messages, size_t message_count,
const provider_tool_def_t *tools, size_t tool_count, provider_response_t *response)
{
(void)messages;
(void)message_count;
(void)tools;
(void)tool_count;
response->tool_calls = NULL;
response->tool_calls_count = 0;
response->error = 0;
compact_fail_calls++;
if (compact_fail_calls == 1) {
response->content = strdup("Summary of earlier conversation.");
return 0;
}
provider_set_error(response, "mock provider error");
return -1;
}
static void compact_fail_cleanup(void) {}
static const provider_t compact_fail_provider = {
.name = "compact_fail",
.init = compact_fail_init,
.chat = compact_fail_chat,
.cleanup = compact_fail_cleanup,
};

/* Summary is computed before the user turn. Saving it immediately deletes the
* raw transcript when the follow-up chat fails and the turn is not stored. */
static int test_compaction_failure_keeps_original_session(void)
{
int failed = 1;
config_t *cfg = NULL;
char *loaded = NULL;
const char *db_path = "build/test_agent_compact_fail.db";
const char *config_path = "build/test_agent_compact_fail.toml";
char response_buf[4096];
char errbuf[256];
FILE *cf;

memory_cleanup();
compact_fail_calls = 0;
if (memory_init(db_path) != 0)
goto cleanup_early;
if (save_alternating_session("cli:compactfail", 50) != 0)
goto cleanup;
cf = fopen(config_path, "w");
if (!cf)
goto cleanup;
fprintf(cf, "[agent]\nmodel = \"test\"\nmax_context_messages = 40\n[memory]\npath = \"%s\"\n",
db_path);
fclose(cf);
if (config_load(config_path, &cfg, errbuf, sizeof(errbuf)) != 0)
goto cleanup;
loaded = malloc(128 * 1024);
if (!loaded)
goto cleanup;
response_buf[0] = '\0';
if (agent_run(cfg, "cli:compactfail", "hi", &compact_fail_provider, NULL, 0,
response_buf, sizeof(response_buf)) == 0)
goto cleanup;
if (strstr(response_buf, "mock provider error") == NULL)
goto cleanup;
if (session_load("cli:compactfail", loaded, 128 * 1024) != 0)
goto cleanup;
if (strstr(loaded, "\"content\":\"msg_0\"") == NULL) {
fprintf(stderr, "FAIL: failed turn replaced the stored transcript\n");
goto cleanup;
}
if (strstr(loaded, "Summary of earlier") != NULL) {
fprintf(stderr, "FAIL: summary was saved even though the turn failed\n");
goto cleanup;
}
failed = 0;
cleanup:
config_free(cfg);
free(loaded);
cleanup_early:
remove(config_path);
remove(db_path);
memory_cleanup();
return failed;
}

/* Odd max_context_messages cuts the tail on a user turn. A separate summary
* user message then makes every later Anthropic call a 400. */
static int test_compaction_merges_summary_when_tail_starts_with_user(void)
{
int failed = 1;
config_t *cfg = NULL;
char *loaded = NULL;
const char *db_path = "build/test_agent_compact_roles.db";
const char *config_path = "build/test_agent_compact_roles.toml";
char response_buf[4096];
char errbuf[256];
FILE *cf;
size_t i;

memory_cleanup();
compaction_call_count = 0;
if (memory_init(db_path) != 0)
goto cleanup_early;
if (save_alternating_session("cli:compactroles", 8) != 0)
goto cleanup;
cf = fopen(config_path, "w");
if (!cf)
goto cleanup;
fprintf(cf, "[agent]\nmodel = \"test\"\nmax_context_messages = 5\n[memory]\npath = \"%s\"\n",
db_path);
fclose(cf);
if (config_load(config_path, &cfg, errbuf, sizeof(errbuf)) != 0)
goto cleanup;
loaded = malloc(64 * 1024);
if (!loaded)
goto cleanup;
response_buf[0] = '\0';
spy_roles_clear();
if (agent_run(cfg, "cli:compactroles", "hi", &compaction_provider, NULL, 0,
response_buf, sizeof(response_buf)) != 0)
goto cleanup;
if (compaction_call_count < 2)
goto cleanup;
if (spy_message_count < 6 || spy_message_count > SPY_SLOTS)
goto cleanup;
if (!spy_roles[0] || strcmp(spy_roles[0], "system") != 0)
goto cleanup;
if (!spy_roles[1] || strcmp(spy_roles[1], "user") != 0)
goto cleanup;
if (strstr(spy_content[1], "Summary of earlier") == NULL ||
strstr(spy_content[1], "msg_4") == NULL) {
fprintf(stderr, "FAIL: summary was not folded into the leading user turn\n");
goto cleanup;
}
for (i = 2; i < spy_message_count; i++) {
if (!spy_roles[i] || strcmp(spy_roles[i], spy_roles[i - 1]) == 0) {
fprintf(stderr, "FAIL: compacted history has consecutive roles\n");
goto cleanup;
}
}
if (session_load("cli:compactroles", loaded, 64 * 1024) != 0)
goto cleanup;
if (strstr(loaded, "Summary of earlier") == NULL || strstr(loaded, "msg_4") == NULL)
goto cleanup;
if (strstr(loaded, "\"content\":\"msg_0\"") != NULL) {
fprintf(stderr, "FAIL: successful turn did not persist compaction\n");
goto cleanup;
}
if (!json_roles_alternate_from_user(loaded)) {
fprintf(stderr, "FAIL: persisted session roles do not alternate\n");
goto cleanup;
}
failed = 0;
cleanup:
config_free(cfg);
free(loaded);
cleanup_early:
remove(config_path);
remove(db_path);
memory_cleanup();
return failed;
}

static int test_local_offline_note_when_active_is_local(void)
{
const char *path = "build/test_agent_local_note.toml";
Expand Down Expand Up @@ -1186,6 +1425,8 @@ int main(void)
RUN(test_react_loop_preserves_prior_tool_results());
RUN(test_session_persisted_after_exchange());
RUN(test_context_compaction_when_history_exceeds_max());
RUN(test_compaction_failure_keeps_original_session());
RUN(test_compaction_merges_summary_when_tail_starts_with_user());
RUN(test_local_offline_note_when_active_is_local());
RUN(test_local_offline_note_skipped_for_non_local());
RUN(test_session_overflow_does_not_corrupt_history());
Expand Down
Loading