mirror of
https://github.com/vercel-labs/fx.git
synced 2026-10-28 10:15:17 +08:00
Keep the notes request out of compaction entries
When automatic compaction asks for notes right after the conversation, the model can read that request as the user's next message and file it as a rule, decision or status. The request now says it comes from fx. Rules from the turn in progress cite `(turn in progress)` instead of a guessed turn ID, and that turn's notes stop at the last tool call being compacted. Entries written with a bold or checked ID, like `- [x] **F1** (T3): ...`, are saved from the ID on. Tests that expected a model call for plain turns now give those turns work to note or a rule to file, since plain turns are kept word for word without one.
This commit is contained in:
5 files changed
+82
-26
No files matched your search
@@ -98,12 +98,13 @@ async function runScenario(name) {
|
||||
const wasm = await readFile(wasmPath);
|
||||
const head = "HOST_HISTORY_HEAD_73b4", tail = "HOST_HISTORY_TAIL_b149";
|
||||
let handoff;
|
||||
// Each ordinary reply is larger than the turns kept unchanged and too long to
|
||||
// keep word for word, so compacting older turns asks for a summary. Three of
|
||||
// them stay below the automatic threshold, even with a cancelled attempt's
|
||||
// followup.
|
||||
// Each ordinary reply is larger than the turns kept unchanged, so older turns
|
||||
// compact. Plain turns need no model call, but the first message sets a rule,
|
||||
// so compacting asks the model to file it. Three of them stay below the
|
||||
// automatic threshold, even with a cancelled attempt's followup.
|
||||
const automatic = name === "auto-cancel-headers";
|
||||
const seedTurns = automatic ? 1 : 3;
|
||||
const seedRule = "Always keep the history markers.";
|
||||
const seedReply = (turn) => `${turn === 1 ? head : `HOST_HISTORY_MIDDLE_${turn}`}\n${"history alpha beta gamma delta sample line\n".repeat(automatic ? 18_000 : 1_800)}HOST_SEED_DONE_${turn}${turn === seedTurns ? `\n${tail}` : ""}`;
|
||||
const activity = /Compacting \((?:\d+h)?(?:\d+m)?\d+s\)/;
|
||||
const forbidden = /Compacting|compaction|Context compacted|No context to compact|HOST_INTERNAL_HANDOFF_268a/i;
|
||||
@@ -137,12 +138,15 @@ async function runScenario(name) {
|
||||
}
|
||||
const body = JSON.parse(typeof init.body === "string" ? init.body : new TextDecoder().decode(init.body));
|
||||
requests.push(body);
|
||||
if (body.toolChoice?.type === "none" && body.tools?.length === 0) {
|
||||
// A compaction request writes the turns out without tools, or follows the
|
||||
// conversation with the notes request.
|
||||
const notesAfterConversation = JSON.stringify(body.messages?.at(-1) ?? "").includes("Write the compaction notes for the turns of the conversation above");
|
||||
if ((body.toolChoice?.type === "none" && body.tools?.length === 0) || notesAfterConversation) {
|
||||
summaries++;
|
||||
// Retained windows vary by model; summarize only markers actually supplied.
|
||||
const source = JSON.stringify(body);
|
||||
const markers = [head, tail].filter((marker) => source.includes(marker));
|
||||
handoff = `HOST_INTERNAL_HANDOFF_268a: preserve ${markers.join(" and ")}; follow the latest request.`;
|
||||
handoff = `Rules:\n- R1 (M1): "${seedRule}"\n\nFacts:\n- F1 (M1): HOST_INTERNAL_HANDOFF_268a: preserve ${markers.join(" and ")}; follow the latest request.`;
|
||||
if (summaries === 1) {
|
||||
hold = heldSummary(init.signal);
|
||||
return hold.response;
|
||||
@@ -225,7 +229,7 @@ async function runScenario(name) {
|
||||
try {
|
||||
await start();
|
||||
for (let turn = 1; turn <= seedTurns; turn++) {
|
||||
active.runtime.write(`Seed ordinary historical turn ${turn}.\r`);
|
||||
active.runtime.write(`Seed ordinary historical turn ${turn}.${turn === 1 ? ` ${seedRule}` : ""}\r`);
|
||||
await waitFor(() => commits === turn && grid(active.terminal).includes(`HOST_SEED_DONE_${turn}`) && emptyComposer(grid(active.terminal)), `saved history turn ${turn}`, 20_000);
|
||||
assert.equal(ordinary, turn);
|
||||
assert.equal(summaries, 0, "seed history must stay below automatic compaction");
|
||||
|
||||
@@ -51,11 +51,15 @@ pub const Asked = struct {
|
||||
after_conversation: bool = false,
|
||||
};
|
||||
|
||||
/// A request that follows the conversation reads like the user's next
|
||||
/// message, so the model is told not to note it.
|
||||
const from_fx = " This request comes from fx, not from the user, so leave it and the writing of these notes out of every note and entry.";
|
||||
|
||||
/// Appends the request for the notes. It lists the exact headings to write,
|
||||
/// since a model given an example heading may copy it instead.
|
||||
pub fn writeRequest(alloc: Allocator, text: *std.ArrayList(u8), asked: Asked) Allocator.Error!void {
|
||||
try text.appendSlice(alloc, if (asked.after_conversation)
|
||||
"Write the compaction notes for the turns of the conversation above that are listed below; the turns after them stay in the conversation as they are. Answer with text only and call no tools."
|
||||
"Write the compaction notes for the turns of the conversation above that are listed below; the turns after them stay in the conversation as they are. Answer with text only and call no tools." ++ from_fx
|
||||
else
|
||||
"Write the compaction notes for the new turns above.");
|
||||
try text.appendSlice(alloc, " The user's messages and the assistant's final replies stay in the conversation word for word, so do not repeat them.");
|
||||
@@ -70,13 +74,19 @@ pub fn writeRequest(alloc: Allocator, text: *std.ArrayList(u8), asked: Asked) Al
|
||||
try text.appendSlice(alloc, "\nUnder each heading:\n" ++ in_between_label ++
|
||||
" one to three sentences on what the assistant did before its final reply: what it looked at, what it found, what it changed or decided. Leave out what the final reply already says. Write \"none\" when there was nothing.\n" ++
|
||||
"Then one line for every tool call of the turn, starting with its ID like `T<number>:`, at most 15 words: why it was used and what it showed that matters. Calls with one purpose may share a line that starts `T<first>\u{2013}T<last>:`.\n");
|
||||
if (asked.open != null) try text.appendSlice(alloc, "For the turn in progress, give its notes so far.\n");
|
||||
if (asked.open != null) try text.appendSlice(alloc, if (asked.after_conversation)
|
||||
"For the turn in progress, give its notes so far, through the last tool call listed under its heading; the calls after it stay in the conversation.\n"
|
||||
else
|
||||
"For the turn in progress, give its notes so far.\n");
|
||||
try text.append(alloc, '\n');
|
||||
} else {
|
||||
try text.appendSlice(alloc, "\n\n");
|
||||
}
|
||||
try text.appendSlice(alloc, "Then only the new entries of these sections, each starting with its ID and the turn or tool call it comes from, like `F<number> (T<number>):`:\n\n");
|
||||
try text.print(alloc, "Rules:\nR1, R2, ...: each new instruction, rule or preference from the user, quoted word for word in double quotes, with {s}.\n\n", .{if (asked.saved) "the ID of its turn, like M<number>" else "the number of its turn"});
|
||||
try text.print(alloc, "Rules:\nR1, R2, ...: each new instruction, rule or preference from the user, quoted word for word in double quotes, with {s}{s}.\n\n", .{
|
||||
if (asked.saved) "the ID of its turn, like M<number>" else "the number of its turn",
|
||||
if (asked.open != null) ", or `(turn in progress)` for the turn still in progress" else "",
|
||||
});
|
||||
try text.appendSlice(alloc, "Facts:\nF1, F2, ...: facts the work depends on, from these turns: names, paths, values, results, causes.\n\n" ++
|
||||
"Decisions:\nD1, D2, ...: each decision and why. When it changes an earlier entry, end with \"replaces\" and that entry's ID.\n\n" ++
|
||||
"Status:\nS1, S2, ...: where each part of the work stands now. When it updates an earlier entry, end with \"replaces\" and that entry's ID.\n\n" ++
|
||||
@@ -93,7 +103,7 @@ const entry_kinds = checkpoint.entry_kinds;
|
||||
pub fn writeFollowUp(alloc: Allocator, text: *std.ArrayList(u8), missing: []const Heading, entries: []const checkpoint.Entry, after_conversation: bool) Allocator.Error!void {
|
||||
try text.appendSlice(alloc, "Your notes on the turns above left some out. Write the notes for only these turns now, each heading followed by its notes:\n\n");
|
||||
try writeHeadings(alloc, text, missing, null);
|
||||
if (after_conversation) try text.appendSlice(alloc, "\nUnder each heading above are the turn's tool calls, so you can find them; do not copy those lines. Answer with text only and call no tools.\n");
|
||||
if (after_conversation) try text.appendSlice(alloc, "\nUnder each heading above are the turn's tool calls, so you can find them; do not copy those lines. Answer with text only and call no tools." ++ from_fx ++ "\n");
|
||||
try text.appendSlice(alloc, "\nUnder each heading, " ++ in_between_label ++ " with what the assistant did before its final reply, then a line for every tool call, starting with its ID, on why it was used and what it showed.\n\n" ++
|
||||
"Then any new entries from those turns under the same sections, each starting with its ID and the turn or tool call it comes from.");
|
||||
try writeHighestIds(alloc, text, entries);
|
||||
@@ -465,7 +475,7 @@ pub fn read(arena: Allocator, reply: []const u8, known: Known, earlier: []const
|
||||
written.repeated += 1;
|
||||
continue;
|
||||
}
|
||||
try entries.append(arena, .{ .id = item.id, .text = withoutBullet(item.text) });
|
||||
try entries.append(arena, .{ .id = item.id, .text = try entryText(arena, item) });
|
||||
}
|
||||
written.entries = entries.items;
|
||||
return written;
|
||||
@@ -590,9 +600,14 @@ fn isToolsLabel(plain: []const u8) bool {
|
||||
return startsWithIgnoreCase(plain, "T:") or startsWithIgnoreCase(plain, "Tools:");
|
||||
}
|
||||
|
||||
fn withoutBullet(text: []const u8) []const u8 {
|
||||
const start = std.mem.trimStart(u8, text, " \t");
|
||||
return if (std.mem.startsWith(u8, start, "- ") or std.mem.startsWith(u8, start, "* ")) start[2..] else start;
|
||||
/// An entry's text from its ID on, without the bullet, check box or bold
|
||||
/// mark around the ID, like `- [x] **F1** (T3): ...` saved as `F1 (T3): ...`.
|
||||
fn entryText(arena: Allocator, item: Item) Allocator.Error![]const u8 {
|
||||
// Nothing that may come before an ID contains one.
|
||||
const at = std.mem.find(u8, item.text, item.id).?;
|
||||
var rest = item.text[at + item.id.len ..];
|
||||
if (std.mem.startsWith(u8, rest, "**")) rest = rest[2..];
|
||||
return std.mem.concat(arena, u8, &.{ item.id, rest });
|
||||
}
|
||||
|
||||
const Item = struct {
|
||||
|
||||
@@ -1314,6 +1314,8 @@ test "after the conversation the model reads only the request, with every turn f
|
||||
try testing.expect(std.mem.find(u8, seen, "[Turn 1]") == null);
|
||||
try testing.expect(std.mem.find(u8, seen, "missing semicolon") == null);
|
||||
try testing.expect(std.mem.find(u8, seen, "Answer with text only and call no tools.") != null);
|
||||
// Read as the user's next message, the request could end up in the notes.
|
||||
try testing.expect(std.mem.find(u8, seen, "This request comes from fx, not from the user") != null);
|
||||
// Turn 2 has nothing in between but is listed for its number.
|
||||
try testing.expect(std.mem.find(u8, seen, "each followed by its notes:\n\n" ++
|
||||
"Turn 1 (T1), which begins \u{201c}Fix the build. It fails on main.\u{201d}\n T1 shell: zig build\n" ++
|
||||
@@ -1398,6 +1400,25 @@ test "the model's notes are checked against the turns and tool calls they name"
|
||||
try testing.expect(std.mem.find(u8, result.text, "A note or entry marked [check: ...]") != null);
|
||||
}
|
||||
|
||||
test "entries with a bold or checked ID are saved from the ID on" {
|
||||
var store = MemoryStore{ .alloc = testing.allocator };
|
||||
defer store.deinit();
|
||||
const turns = [_]Turn{.{ .user = "run the tests", .items = &.{
|
||||
.{ .tool_call = .{ .id = "c1", .name = "shell", .arguments = "{\"command\":\"zig build test\"}" } },
|
||||
.{ .tool_result = .{ .call_id = "c1", .name = "shell", .output = "{\"exit_code\":1,\"output\":\"3 of 120 tests failed in src/lexer.zig\"}" } },
|
||||
.{ .assistant = "Three tests fail in the lexer." },
|
||||
} }};
|
||||
var model = FakeModel{ .reply = "Turn 1\nIn between: Ran the suite.\nT1: ran the tests\n\nFacts:\n- **F1** (T1): 3 of 120 tests fail in src/lexer.zig\n\nStatus:\n- [x] S1 (T1): fixing the lexer" };
|
||||
defer model.deinit();
|
||||
var result = try compact(testing.allocator, .{ .model = "m", .turns = &turns }, model.model(), store.store());
|
||||
defer result.deinit();
|
||||
|
||||
const entries = result.compacted.entries;
|
||||
try testing.expectEqual(@as(usize, 2), entries.len);
|
||||
try testing.expectEqualStrings("F1 (T1): 3 of 120 tests fail in src/lexer.zig", entries[0].text);
|
||||
try testing.expectEqualStrings("S1 (T1): fixing the lexer", entries[1].text);
|
||||
}
|
||||
|
||||
test "the tool line says what code knows: the call, how it ended and its size" {
|
||||
const call: ToolCall = .{ .id = "c", .name = "shell", .arguments = "{\"command\":\"zig build test\"}" };
|
||||
var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
|
||||
|
||||
@@ -159,14 +159,22 @@ function startFakeProviderCompaction(provider: "codex" | "grok") {
|
||||
const parsed = JSON.parse(body) as { tools?: unknown[] };
|
||||
const compacting = (parsed.tools?.length ?? 0) === 0;
|
||||
if (!compacting) workingRequests += 1;
|
||||
if (!compacting && workingRequests === 2) {
|
||||
if (!compacting && workingRequests === 3) {
|
||||
return Response.json({ error: { code: "context_length_exceeded", message: "maximum context length exceeded" } }, { status: 400 });
|
||||
}
|
||||
// The first reply is too long to keep word for word, so compaction
|
||||
// must ask this provider for a summary.
|
||||
// The first turn reads a file before its reply, so compaction has work
|
||||
// to note and must ask this provider for the notes.
|
||||
if (!compacting && workingRequests === 1) {
|
||||
return new Response(
|
||||
`data: ${JSON.stringify({ type: "response.output_item.added", output_index: 0, item: { type: "function_call", call_id: "call_provider_facts", name: "read_file" } })}\n\n` +
|
||||
`data: ${JSON.stringify({ type: "response.function_call_arguments.done", output_index: 0, arguments: JSON.stringify({ path: "provider-facts.txt" }) })}\n\n` +
|
||||
`data: ${JSON.stringify({ type: "response.completed", response: { id: `response-${bodies.length}`, status: "completed", usage: { input_tokens: 7, output_tokens: 3 } } })}\n\n`,
|
||||
{ headers: { "content-type": "text/event-stream" } },
|
||||
);
|
||||
}
|
||||
const text = compacting
|
||||
? "The earlier request established the saved facts."
|
||||
: workingRequests === 1 ? "SAVED_PROVIDER_FACTS ".repeat(2_000) : `${provider.toUpperCase()}_COMPACTION_CONTINUED`;
|
||||
? "Turn 1\nIn between: Read the saved provider facts.\nT1: read the saved provider facts"
|
||||
: workingRequests === 2 ? "SAVED_PROVIDER_FACTS ".repeat(2_000) : `${provider.toUpperCase()}_COMPACTION_CONTINUED`;
|
||||
return new Response(
|
||||
`data: ${JSON.stringify({ type: "response.output_text.delta", delta: text })}\n\n` +
|
||||
`data: ${JSON.stringify({ type: "response.completed", response: { id: `response-${bodies.length}`, status: "completed", usage: { input_tokens: 7, output_tokens: 3 } } })}\n\n`,
|
||||
@@ -6244,6 +6252,7 @@ test(
|
||||
: { provider, grok_model: direct.workingModel }) + "\n",
|
||||
{ mode: 0o600 },
|
||||
);
|
||||
writeFileSync(join(testHome, "provider-facts.txt"), "Provider facts.\n");
|
||||
const options = {
|
||||
cwd: testHome,
|
||||
env: {
|
||||
@@ -6278,10 +6287,13 @@ test(
|
||||
body_lengths: direct.bodies.map((body) => body.length),
|
||||
}),
|
||||
)
|
||||
.toEqual(Array(4).fill(direct.workingModel));
|
||||
expect(direct.authorizations).toEqual(Array(4).fill(`Bearer ${direct.accessToken}`));
|
||||
.toEqual(Array(5).fill(direct.workingModel));
|
||||
// The fourth request is the compaction's own, sent without tools.
|
||||
expect(direct.bodies.map((body) => ((JSON.parse(body) as { tools?: unknown[] }).tools?.length ?? 0) === 0))
|
||||
.toEqual([false, false, false, true, false]);
|
||||
expect(direct.authorizations).toEqual(Array(5).fill(`Bearer ${direct.accessToken}`));
|
||||
if (provider === "grok") {
|
||||
expect(direct.modelOverrides).toEqual(Array(4).fill(direct.workingModel));
|
||||
expect(direct.modelOverrides).toEqual(Array(5).fill(direct.workingModel));
|
||||
}
|
||||
expect(testGateway.requests).toHaveLength(0);
|
||||
} finally {
|
||||
|
||||
@@ -7690,6 +7690,7 @@ test.skipIf(!tmuxAvailable())(
|
||||
fakeGatewayFinalText("EARLIER_VISIBLE_RESPONSE"),
|
||||
fakeGatewayFinalText("MIDDLE_VISIBLE_RESPONSE"),
|
||||
fakeGatewayFinalText("LATEST_VISIBLE_RESPONSE"),
|
||||
fakeGatewayFinalText(`Rules:\n- R1 (M1): "${earlierRequest}"`),
|
||||
fakeGatewayFinalText("AFTER_COMPACTED_RESUME_OK"),
|
||||
]);
|
||||
let active: TmuxSession | null = null;
|
||||
@@ -7722,14 +7723,17 @@ test.skipIf(!tmuxAvailable())(
|
||||
const records = compacted.trim().split("\n").map((line) => JSON.parse(line));
|
||||
expect(records.filter((record) => record.event.context_checkpoint)).toHaveLength(1);
|
||||
expect(await active.captureFullScrollback()).not.toContain("Context compacted.");
|
||||
// Plain turns have nothing to summarize: no model call, and the
|
||||
// checkpoint keeps the earlier message and reply word for word.
|
||||
expect(gateway.requests).toHaveLength(3);
|
||||
// Plain turns have nothing to summarize, but the earlier message may set
|
||||
// a rule, so the one model call files it; the checkpoint keeps the
|
||||
// earlier message and reply word for word.
|
||||
expect(gateway.requests).toHaveLength(4);
|
||||
expect(gateway.requests[3].body).toContain(JSON.stringify(`- M1: "${earlierRequest}"`).slice(1, -1));
|
||||
const saved: string = records.find((record) => record.event.context_checkpoint).event.context_checkpoint.summary;
|
||||
expect(saved.startsWith("fx-compactor-v1\n")).toBe(true);
|
||||
const payload = JSON.parse(saved.slice("fx-compactor-v1\n".length));
|
||||
expect(payload.turns[0].users).toEqual([earlierRequest]);
|
||||
expect(payload.turns[0].final).toBe("EARLIER_VISIBLE_RESPONSE");
|
||||
expect(payload.entries.map((entry: { text: string }) => entry.text)).toEqual([`R1 (M1): "${earlierRequest}"`]);
|
||||
await active.sendText("/quit");
|
||||
expect(await active.waitForSessionEnd(TIMEOUT)).toBe(true);
|
||||
await active.kill();
|
||||
|
||||
Reference in new issue
Block a user