fix the session-list scan's line count, and pin both new paths with tests
The rewritten cst_agent_store_info() counted a transcript line that ran past a whole 64 KiB read block a second time, when the newline ending its tail arrived: a session with one very long line (a tool that read a file back) reported one message too many -- exactly what the sidebar shows the person. - src/agent_store.c: the tail's newline ends a line that was already counted. - tests/test_store.c: test_info_scan() holds the scan down -- a 70 KiB line is one line, the first user message comes from the head of the file, and a user line past the window is not called the first one. - tests/test_tools_ext.c: a byte budget stops the session_search walk, says so in the result, and never reports a match it did not reach; the same query without a budget finds it. make check and make asan are green.
This commit is contained in:
+4
-1
@@ -469,7 +469,10 @@ int cst_agent_store_info(const char *workspace, const char *id,
|
|||||||
if (!nl)
|
if (!nl)
|
||||||
break;
|
break;
|
||||||
const size_t len = (size_t)(nl - (buf + at));
|
const size_t len = (size_t)(nl - (buf + at));
|
||||||
if (len > 0)
|
/* A line counts once. A line that ran past a whole block was counted
|
||||||
|
* when its tail was dropped, so the newline that ends it is not a line
|
||||||
|
* of its own. */
|
||||||
|
if (len > 0 && !slice)
|
||||||
info->messages++;
|
info->messages++;
|
||||||
if (!slice && len > 0 && want_first && seen < kFirstUserBytes) {
|
if (!slice && len > 0 && want_first && seen < kFirstUserBytes) {
|
||||||
*nl = 0; /* the parse reads a C string; the newline is ours */
|
*nl = 0; /* the parse reads a C string; the newline is ours */
|
||||||
|
|||||||
@@ -267,6 +267,98 @@ static void test_old_store(void) {
|
|||||||
EXPECT(!exists(nested) && !exists(new_payload), "takes the payload with it");
|
EXPECT(!exists(nested) && !exists(new_payload), "takes the payload with it");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* The scan behind the session list (cst_agent_store_info): the count is a
|
||||||
|
* count of transcript lines, a line longer than one read block is still one
|
||||||
|
* line, and the first user message is only looked for in the head of the file
|
||||||
|
* -- a sidebar refresh is not allowed to read a whole transcript of tool
|
||||||
|
* output to find a greeting. */
|
||||||
|
static void test_info_scan(void) {
|
||||||
|
char id[64] = "";
|
||||||
|
EXPECT(cst_agent_store_new_id(id, sizeof(id)) == 0, "an id for the scan");
|
||||||
|
char path[1200];
|
||||||
|
EXPECT(cst_agent_store_path_for_id(g_ws, id, path, sizeof(path)) == 0,
|
||||||
|
"a session path to write into");
|
||||||
|
cst_session_t *s = cst_session_open(path);
|
||||||
|
EXPECT(s != NULL, "its transcript opens");
|
||||||
|
if (!s)
|
||||||
|
return;
|
||||||
|
cst_message_t m;
|
||||||
|
memset(&m, 0, sizeof(m));
|
||||||
|
m.role = CST_ROLE_USER;
|
||||||
|
m.content = (char *)"the first user line";
|
||||||
|
EXPECT(cst_session_append(s, &m) == 0, "the first user message goes in");
|
||||||
|
memset(&m, 0, sizeof(m));
|
||||||
|
m.role = CST_ROLE_ASSISTANT;
|
||||||
|
m.content = (char *)"an answer";
|
||||||
|
EXPECT(cst_session_append(s, &m) == 0, "and an answer");
|
||||||
|
/* One line that does not fit a read block: counted once, not once per block
|
||||||
|
* of it (which is what a wrong block count would report). */
|
||||||
|
const size_t big = 70 * 1024;
|
||||||
|
char *huge = malloc(big + 1);
|
||||||
|
EXPECT(huge != NULL, "a buffer for one very long line");
|
||||||
|
if (!huge)
|
||||||
|
return;
|
||||||
|
memset(huge, 'x', big);
|
||||||
|
huge[big] = 0;
|
||||||
|
memset(&m, 0, sizeof(m));
|
||||||
|
m.role = CST_ROLE_ASSISTANT;
|
||||||
|
m.content = huge;
|
||||||
|
EXPECT(cst_session_append(s, &m) == 0, "a line longer than a block goes in");
|
||||||
|
memset(&m, 0, sizeof(m));
|
||||||
|
m.role = CST_ROLE_ASSISTANT;
|
||||||
|
m.content = (char *)"a short one after it";
|
||||||
|
EXPECT(cst_session_append(s, &m) == 0, "and a short line after it");
|
||||||
|
cst_session_close(s);
|
||||||
|
|
||||||
|
cst_session_info_t in;
|
||||||
|
memset(&in, 0, sizeof(in));
|
||||||
|
EXPECT(cst_agent_store_info(g_ws, id, &in) == 0, "its info reads back");
|
||||||
|
EXPECT(in.messages == 4, "four lines counted, the long one once");
|
||||||
|
EXPECT(strcmp(in.first_user, "the first user line") == 0,
|
||||||
|
"and the first user line is the one it found");
|
||||||
|
|
||||||
|
/* Now a head of tool output wider than the window the search gets, with no
|
||||||
|
* user line in it: the greeting below stays unseen (and nothing read
|
||||||
|
* megabytes of transcript to look for it). */
|
||||||
|
char id2[64] = "";
|
||||||
|
EXPECT(cst_agent_store_new_id(id2, sizeof(id2)) == 0, "an id for the tail");
|
||||||
|
char path2[1200];
|
||||||
|
EXPECT(cst_agent_store_path_for_id(g_ws, id2, path2, sizeof(path2)) == 0,
|
||||||
|
"a second session path");
|
||||||
|
s = cst_session_open(path2);
|
||||||
|
EXPECT(s != NULL, "the second transcript opens");
|
||||||
|
if (!s) {
|
||||||
|
free(huge);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const size_t mid = 60 * 1024;
|
||||||
|
char *chunk = malloc(mid + 1);
|
||||||
|
if (chunk) {
|
||||||
|
memset(chunk, 'y', mid);
|
||||||
|
chunk[mid] = 0;
|
||||||
|
for (int i = 0; i < 6; i++) { /* 360 KiB of tool output, no user line */
|
||||||
|
memset(&m, 0, sizeof(m));
|
||||||
|
m.role = CST_ROLE_TOOL;
|
||||||
|
m.content = chunk;
|
||||||
|
if (cst_session_append(s, &m) != 0)
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
free(chunk);
|
||||||
|
}
|
||||||
|
memset(&m, 0, sizeof(m));
|
||||||
|
m.role = CST_ROLE_USER;
|
||||||
|
m.content = (char *)"a greeting far down the file";
|
||||||
|
EXPECT(cst_session_append(s, &m) == 0, "a user line after all that");
|
||||||
|
cst_session_close(s);
|
||||||
|
|
||||||
|
memset(&in, 0, sizeof(in));
|
||||||
|
EXPECT(cst_agent_store_info(g_ws, id2, &in) == 0, "its info reads too");
|
||||||
|
EXPECT(in.messages == 7, "seven lines, one per line");
|
||||||
|
EXPECT(in.first_user[0] == 0,
|
||||||
|
"a user line past the window is not called the first one");
|
||||||
|
free(huge);
|
||||||
|
}
|
||||||
|
|
||||||
int main(void) {
|
int main(void) {
|
||||||
if (test_tmpdir(g_ws, sizeof(g_ws), "cst-store") != 0) {
|
if (test_tmpdir(g_ws, sizeof(g_ws), "cst-store") != 0) {
|
||||||
perror("tmpdir");
|
perror("tmpdir");
|
||||||
@@ -280,6 +372,7 @@ int main(void) {
|
|||||||
test_ids();
|
test_ids();
|
||||||
test_new_session();
|
test_new_session();
|
||||||
test_old_store();
|
test_old_store();
|
||||||
|
test_info_scan();
|
||||||
|
|
||||||
test_rmrf(g_ws);
|
test_rmrf(g_ws);
|
||||||
if (nfail) {
|
if (nfail) {
|
||||||
|
|||||||
@@ -236,6 +236,50 @@ static void test_session_search(cst_tool_ctx_t *ctx) {
|
|||||||
c = mk("session_search", NULL);
|
c = mk("session_search", NULL);
|
||||||
EXPECT(run(ctx, c, &r) == 1, "a query is required");
|
EXPECT(run(ctx, c, &r) == 1, "a query is required");
|
||||||
cst_tool_result_free(&r);
|
cst_tool_result_free(&r);
|
||||||
|
|
||||||
|
/* A store is not read to the end on a whim: a byte budget stops the walk
|
||||||
|
* and says so, and it never reports a match it did not get to. */
|
||||||
|
{
|
||||||
|
char path2[512];
|
||||||
|
EXPECT(cst_agent_store_path_for_id(g_ws, "20260101-130000-big", path2,
|
||||||
|
sizeof(path2)) == 0,
|
||||||
|
"a second session path");
|
||||||
|
cst_session_t *s2 = cst_session_open(path2);
|
||||||
|
EXPECT(s2 != NULL, "its transcript opens");
|
||||||
|
if (s2) {
|
||||||
|
cst_message_t m2;
|
||||||
|
memset(&m2, 0, sizeof(m2));
|
||||||
|
m2.role = CST_ROLE_USER;
|
||||||
|
m2.content = (char *)"the start of a long one";
|
||||||
|
EXPECT(cst_session_append(s2, &m2) == 0, "a first line goes in");
|
||||||
|
for (int i = 0; i < 600; i++) {
|
||||||
|
memset(&m2, 0, sizeof(m2));
|
||||||
|
m2.role = CST_ROLE_ASSISTANT;
|
||||||
|
m2.content = (char *)"a line of tool output that says nothing special";
|
||||||
|
if (cst_session_append(s2, &m2) != 0)
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
memset(&m2, 0, sizeof(m2));
|
||||||
|
m2.role = CST_ROLE_ASSISTANT;
|
||||||
|
m2.content = (char *)"the needle is only at the very end";
|
||||||
|
EXPECT(cst_session_append(s2, &m2) == 0, "the needle goes in last");
|
||||||
|
cst_session_close(s2);
|
||||||
|
}
|
||||||
|
/* Enough lines that a byte budget is spent before the end is reached. */
|
||||||
|
c = mk("session_search", "query", "needle", "max_bytes", "4096", NULL);
|
||||||
|
EXPECT(run(ctx, c, &r) == 0 && r.out != NULL, "a bounded search runs");
|
||||||
|
EXPECT(r.out && strstr(r.out, "budget stopped the walk") != NULL,
|
||||||
|
"the byte budget stops the walk and says so");
|
||||||
|
EXPECT(r.out == NULL || strstr(r.out, "the needle is only") == NULL,
|
||||||
|
"and it does not report a match it never got to");
|
||||||
|
cst_tool_result_free(&r);
|
||||||
|
/* The same query without a budget reaches the end and finds it. */
|
||||||
|
c = mk("session_search", "query", "needle", NULL);
|
||||||
|
EXPECT(run(ctx, c, &r) == 0 && r.out &&
|
||||||
|
strstr(r.out, "the needle is only") != NULL,
|
||||||
|
"the unbounded search finds it");
|
||||||
|
cst_tool_result_free(&r);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/* --- device_info: the environment, without a shell ---------------------- */
|
/* --- device_info: the environment, without a shell ---------------------- */
|
||||||
|
|||||||
Reference in New Issue
Block a user