From 2236a27056d054364e84acbeb93a28d6e86b8c37 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Thu, 6 Aug 2026 19:03:56 +0800 Subject: [PATCH 1/2] feat(compact): write notes-to-self before compaction (#391) Compaction hands the conversation to a summarizer, and a summarizer optimizes for a readable account of what happened. The things a working agent cannot cheaply re-derive - the exact line it was editing, the approach it already ruled out, why it picked B over A - are precisely what a summary drops as uninteresting. Codex solves this by reserving buffer tokens and firing one prompt right before rollover; this is that, on graff's substrate. When compaction is imminent the root now spends one bounded, tool-less turn writing a note to its future self (subgoal, file:line anchors, decisions, dead ends), stored outside the conversation and re-injected verbatim after the history it describes is gone. BUDGET: one ledger, not two. #390's landing reserve (phase_budget.Ledger) is extended with cost_precompact_note and Ledger.affordsHarnessNote rather than given a sibling. The note is the JUNIOR liability on that ledger - narration is mandatory and owns the reserve, the note is optional and must fit ON TOP of it, the same P3 gate judges pass. A second reserve would be a second ledger, and two ledgers each holding back "the last call" double-count the same pool, which is the bug #390 was filed for. The call then goes through the shared RunBudget like any other, so it is counted where it is spent. A token buffer (note_reserve_tokens) is the second half of the gate, checked against the window. A test derives the call boundary from landingReserve() itself, so a parallel reserve would move it and fail. PERSISTENCE: a session-scoped append-only store (.graff/notes/.notes. jsonl) composed into the ROOT system prompt beside HARD CONSTRAINTS, not playbook items with source=session. The playbook is project-scoped and cross-session by design, caps items at 240 bytes, rides every subagent brief through rideBrief, and keeps items until a user retires one by id - all four are wrong for working state that is superseded by the next note and must never reach a worker. What is borrowed is the MECHANISM, exactly: append-only JSONL written whole-or-not-at-all, replayed deterministically, and assembled from the file at injection time rather than from conversation memory. That last property is the whole feature - there is no in-context copy for the next compaction to paraphrase away, and the system prompt is re-sent verbatim on every request. Never fires for a subagent (checked first, before anything is measured), at most once per history generation however often compaction is retried (#379's loop would otherwise buy a note per lap), and every refusal is a named skip: compact() ignores the result, so a failed, empty or unwritable note costs nothing but the note. Co-Authored-By: Codegraff --- src/agent.zig | 1 + src/agent_compact.zig | 5 + src/compact_note.zig | 324 ++++++++++++++++++++++++++++++++ src/compact_note_glue.zig | 119 ++++++++++++ src/compact_note_glue_tests.zig | 178 ++++++++++++++++++ src/compact_note_tests.zig | 256 +++++++++++++++++++++++++ src/phase_budget.zig | 35 ++++ src/prompts.zig | 22 ++- src/test_hooks.zig | 8 + 9 files changed, 947 insertions(+), 1 deletion(-) create mode 100644 src/compact_note.zig create mode 100644 src/compact_note_glue.zig create mode 100644 src/compact_note_glue_tests.zig create mode 100644 src/compact_note_tests.zig diff --git a/src/agent.zig b/src/agent.zig index 36516e80..bd4fa746 100644 --- a/src/agent.zig +++ b/src/agent.zig @@ -184,6 +184,7 @@ pub const Agent = struct { last_request_write_failed: bool = false, // transport gave up specifically with WriteFailed this request compact_transport_failures: u8 = 0, // bounded escape for repeated opaque over-cap WriteFailed/network failures compact_summary_failures: u8 = 0, // #379: consecutive complete-but-unusable (empty/truncated) summaries + precompact_note_gen: ?u32 = null, // #391: history_rewrites at the last pre-compaction note-to-self, so one history generation buys at most one note however often compaction is retried (compact_note.decideCalls) ws_off: bool = false, // codex ws transport disabled for this session after a handshake/transport fallback to SSE (#codex-ws) ws_transport_failures: u8 = 0, // consecutive WS failures; retry once before latching persistent SSE streamed_text: bool = false, // the last request printed its text live diff --git a/src/agent_compact.zig b/src/agent_compact.zig index 22170b3d..8ef532eb 100644 --- a/src/agent_compact.zig +++ b/src/agent_compact.zig @@ -13,6 +13,7 @@ const Agent = agent_mod.Agent; const prompts = @import("prompts.zig"); const compact_instruction = prompts.compact_instruction; const goal_flow = @import("goal_flow.zig"); +const compact_note_glue = @import("compact_note_glue.zig"); // #391 const messages_mod = @import("messages.zig"); const textMessage = messages_mod.textMessage; @@ -125,6 +126,10 @@ pub fn compact(self: *Agent) anyerror!usize { return 0; }; if (!main_mod.json_mode) try self.say("[compacting ~{d} tokens…]\n", .{pending_tokens}); + // #391: the agent writes its own handoff BEFORE the summarizer rewrites the + // history it describes. Gated, budgeted, best-effort: every refusal is a + // named skip, so everything below runs unconditionally. + _ = compact_note_glue.maybeWrite(self); // #163: reclaim room BEFORE the summarization request so it fits under the // model's input cap. On codex/gpt-5.x an over-cap request fails to WRITE // (WriteFailed) rather than returning a clean overflow, so compaction could diff --git a/src/compact_note.zig b/src/compact_note.zig new file mode 100644 index 00000000..36ec8fe2 --- /dev/null +++ b/src/compact_note.zig @@ -0,0 +1,324 @@ +//! #391 — pre-compaction notes-to-self: the handoff the agent writes for +//! ITSELF, in its own words, just before compaction rewrites its history. +//! +//! Codex reserves `auto_compact_fallback_buffer_tokens` and fires +//! `auto_compact_fallback_prompt` right before context rollover, for a reason +//! graff has its own evidence of: a summarizer optimizes for a readable +//! account of what happened, and the things a working agent cannot cheaply +//! re-derive — the exact line it was editing, the approach it already ruled +//! out, why it picked B over A — are precisely the details a summary drops as +//! uninteresting. The my-website post-mortem is that failure at scale: 221 +//! calls, 66 over 128k, constraint recall decaying with every rollover. +//! +//! THE NOTE IS STATE, NOT CONVERSATION. Three properties follow from that and +//! every design choice here serves one of them: +//! +//! * DETERMINISTIC RE-INJECTION. `blockNow` reads the file on disk at the +//! moment a prompt is assembled. There is no in-context copy for the next +//! compaction to paraphrase away, which is the same mechanism — and the +//! same one-line reason — the #383 playbook survives compaction with no +//! extra machinery. +//! * IT RIDES THE SYSTEM PROMPT. prompts.setSystemPrompts composes this +//! beside the HARD CONSTRAINTS block, so it is re-sent verbatim on every +//! request. A user-turn note would land in the NEXT compaction's input +//! and be summarized on the spot (#326's lesson). +//! * NOTHING REWRITES IT. The model proposes the text once; this file caps +//! and stores it. Nothing merges, re-summarizes or edits a stored note. +//! A newer note supersedes an older one wholesale, because the agent that +//! wrote it had the older one in front of it when it did. +//! +//! WHY A SESSION-SCOPED FILE AND NOT `playbook` ITEMS WITH `source=session`. +//! The issue floats both. The playbook is the wrong container on four counts, +//! all of which are about scope rather than taste: it is PROJECT-scoped and +//! deliberately cross-session (a note about the refactor in flight is noise +//! in tomorrow's session); its items are capped at 240 bytes and its block at +//! 2 KB (a note carrying anchors, decisions and dead ends is neither); every +//! item rides EVERY subagent and workflow brief through `rideBrief`, and #391 +//! requires the opposite; and items are permanent until a user retires one by +//! id, whereas a note is superseded by the next note. What is worth borrowing +//! is the MECHANISM, and it is borrowed exactly: append-only JSONL written +//! whole-or-not-at-all, replayed in order, assembled from the file and never +//! from conversation memory. +//! +//! ON-DISK FORMAT — `.graff/notes/.notes.jsonl`, one self-describing +//! object per line, appended positionally so a concurrent reader sees either +//! the old state or the new one: +//! +//! {"v":1,"session":"last","gen":3,"text":"…","created_at":1754…} +//! +//! `gen` is the writing agent's `history_rewrites` counter — the history +//! generation the note describes. It is what makes "at most one note per +//! compaction" checkable after the fact rather than only in memory. + +const std = @import("std"); +const Io = std.Io; +const Allocator = std.mem.Allocator; + +const util = @import("util.zig"); +// #390/#391: the ONE reservation ledger over the shared RunBudget pool. The +// note turn is charged here rather than against a reserve of its own — see +// `decideCalls` below. +const phase_budget = @import("phase_budget.zig"); + +pub const dir = ".graff/notes"; +pub const ext = ".notes.jsonl"; + +/// Whole-file read cap. A note store is a handful of records; past this it is +/// something else and injecting from it would be the bug. +pub const max_file_bytes = 256 * 1024; +/// Longest single note kept on disk and injected (~1k tokens). The turn that +/// produces it is bounded on the wire too (compact_note_glue.askModel). +pub const max_text = 4000; +/// Below this a reply is a fragment, not a handoff. +pub const min_text = 16; + +/// The token buffer held back for the note turn, the direct analogue of +/// codex's `auto_compact_fallback_buffer_tokens`. Compaction fires at 80% of +/// the window, so this is normally free; the check exists for the case that +/// is not true — a window already past the wall, where spending the last +/// tokens on a note would cost the compaction that has to happen anyway. +pub const note_reserve_tokens: u64 = 2_000; + +pub const header = "NOTES TO SELF (you wrote this yourself just before the last compaction; stored verbatim outside the conversation, not a summary):"; + +pub const Note = struct { + session: []const u8 = "", + text: []const u8, + /// The writer's `history_rewrites` at the time — the history generation + /// this note describes. + generation: u32 = 0, + created_at: i64 = 0, +}; + +/// Why the note turn did or did not fire. Every refusal is named: a silent +/// skip and a skip we chose are indistinguishable in a trace, and this is a +/// feature whose whole value is that it happened at the right moment. +pub const Decision = enum { + fire, + /// #391: workers never write notes. A subagent's context is disposable by + /// design — it exists to produce one report and die — so a note it wrote + /// would outlive the only reader it could ever have. + skip_worker, + /// No durable session to hang the note on (a `-p` one-shot, a scratch + /// agent). Nothing would ever read it back. + skip_no_session, + /// A note already covers this history generation. Compaction can be + /// retried after a transient failure; the note must not be re-bought. + skip_already, + /// The call would come out of #390's landing reserve. + skip_budget, + /// No token buffer left for the note turn under this model's window. + skip_no_headroom, + /// The turn ran and produced nothing usable — a transport failure, an + /// explicit "none", or a store that could not be written. Distinct from + /// the gates above because this one COST a call; compaction proceeds + /// regardless, which is the point. + skip_failed, +}; + +pub const Inputs = struct { + sub: bool, + session_name: []const u8, + history_rewrites: u32, + /// `history_rewrites` at the last note this agent wrote, if any. + last_written: ?u32, + /// The run's `--max-model-calls` (0 = unlimited) and what is left of it. + cap: u64, + remaining: u64, +}; + +/// Everything decidable without measuring the context. Split from the token +/// half so the cheap refusals — a worker above all — never pay for a full +/// history serialization to learn they are refusals. +/// +/// THE BUDGET GATE IS #390'S LEDGER, NOT A SECOND ONE. `phase_budget.Ledger` +/// already holds back the calls the ROOT needs to land and narrate the work; +/// #391 adds a second harness-owned liability against the same pool. Giving +/// it a reserve of its own would double-count that pool — two ledgers each +/// believing they had protected the last call — so the note is charged as an +/// OPTIONAL cost that must fit ON TOP of the landing reserve, the same P3 +/// gate judges pass. A note can therefore never be the reason a run dies +/// narrating, which is the exact failure #390 exists to prevent. The note +/// call itself then goes through the shared RunBudget like every other call, +/// so it is counted where it is spent; `remaining` is a plain load and racy +/// against a concurrent sibling, exactly as phase_budget's own gates are. +pub fn decideCalls(in: Inputs) Decision { + if (in.sub) return .skip_worker; + if (in.session_name.len == 0) return .skip_no_session; + if (in.last_written) |written| if (written == in.history_rewrites) return .skip_already; + const ledger = phase_budget.Ledger.init(in.cap); + if (!ledger.affordsHarnessNote(in.remaining)) return .skip_budget; + return .fire; +} + +/// The token half: is there room under the window for the note turn's reply? +/// An unknown window (0) cannot prove there is not, and the summary request +/// this precedes is about to ship the same input anyway. +pub fn decideRoom(window_tokens: u64, effective_tokens: u64) Decision { + if (window_tokens == 0) return .fire; + return if (effective_tokens +| note_reserve_tokens > window_tokens) .skip_no_headroom else .fire; +} + +/// The whole ladder, in the order the call site runs it. +pub fn decide(in: Inputs, window_tokens: u64, effective_tokens: u64) Decision { + const calls = decideCalls(in); + if (calls != .fire) return calls; + return decideRoom(window_tokens, effective_tokens); +} + +/// A model reply that carries no note. The prompt asks for exactly "none" +/// when nothing is in flight; an empty or whitespace reply means the same +/// thing. Either way nothing is stored and compaction proceeds — losing a +/// note is much cheaper than wedging the session over one. +pub fn isEmptyReply(reply: []const u8) bool { + const t = std.mem.trim(u8, reply, " \t\r\n."); + if (t.len < min_text) return true; + // A model handed an explicit way out takes it, but rarely at exactly four + // bytes ("none — nothing in flight"). Same prefix test the #383 reflector + // uses on its own opt-out, and for the same reason. + return std.ascii.eqlIgnoreCase(util.utf8Prefix(t, 4), "none"); +} + +/// This session's store. Null for a name that could escape `.graff/notes`: +/// session names reach here from `/save ` and are user input. +pub fn pathFor(arena: Allocator, session: []const u8) ?[]const u8 { + if (session.len == 0) return null; + if (std.mem.indexOfAny(u8, session, "/\\") != null) return null; + if (std.mem.indexOf(u8, session, "..") != null) return null; + return std.fmt.allocPrint(arena, "{s}/{s}{s}", .{ dir, session, ext }) catch null; +} + +/// Append one already-serialized record at the current end of file, so a +/// whole line lands at once (playbook.appendLine's shape, and serve_events' +/// before it). +fn appendLine(io: Io, path: []const u8, line: []const u8) bool { + Io.Dir.cwd().createDirPath(io, dir) catch {}; + const f = Io.Dir.cwd().createFile(io, path, .{ .truncate = false }) catch return false; + defer f.close(io); + const st = f.stat(io) catch return false; + f.writePositionalAll(io, line, st.size) catch return false; + return true; +} + +/// Store one note. Total and best-effort: every refusal returns false and +/// writes nothing, because every caller is on the compaction path and must +/// keep going without a note. +pub fn record(io: Io, arena: Allocator, session: []const u8, generation: u32, text_in: []const u8) bool { + const trimmed = std.mem.trim(u8, text_in, " \t\r\n"); + if (isEmptyReply(trimmed)) return false; + const text = util.utf8Prefix(trimmed, max_text); + const path = pathFor(arena, session) orelse return false; + var aw: Io.Writer.Allocating = .init(arena); + var s: std.json.Stringify = .{ .writer = &aw.writer }; + s.write(.{ + .v = @as(u8, 1), + .session = session, + .gen = generation, + .text = text, + .created_at = util.unixMs(io), + }) catch return false; + aw.writer.writeByte('\n') catch return false; + return appendLine(io, path, aw.writer.buffered()); +} + +fn strOf(o: std.json.ObjectMap, key: []const u8) ?[]const u8 { + const v = o.get(key) orelse return null; + return if (v == .string) v.string else null; +} + +fn u32Of(o: std.json.ObjectMap, key: []const u8) u32 { + const v = o.get(key) orelse return 0; + if (v != .integer or v.integer < 0) return 0; + return std.math.cast(u32, v.integer) orelse 0; +} + +/// Replay the log oldest-first. A malformed line costs at most its own +/// record: a half-written tail from a killed process must not take the note +/// before it down with it. +pub fn parse(arena: Allocator, data: []const u8) []const Note { + var list: std.ArrayList(Note) = .empty; + var lines = std.mem.splitScalar(u8, data, '\n'); + while (lines.next()) |raw| { + const line = std.mem.trim(u8, raw, " \t\r"); + if (line.len == 0) continue; + const v = std.json.parseFromSliceLeaky(std.json.Value, arena, line, .{}) catch continue; + if (v != .object) continue; + const text = strOf(v.object, "text") orelse continue; + if (text.len == 0) continue; + const created = v.object.get("created_at"); + list.append(arena, .{ + .session = strOf(v.object, "session") orelse "", + .text = text, + .generation = u32Of(v.object, "gen"), + .created_at = if (created) |c| (if (c == .integer) c.integer else 0) else 0, + }) catch break; + } + return list.items; +} + +/// Every note this session ever wrote. An absent or unreadable file is an +/// empty store, never an error. +pub fn load(io: Io, arena: Allocator, session: []const u8) []const Note { + const path = pathFor(arena, session) orelse return &.{}; + const data = Io.Dir.cwd().readFileAlloc(io, path, arena, .limited(max_file_bytes)) catch return &.{}; + return parse(arena, data); +} + +/// NEWEST WINS, wholesale. The agent that wrote note N+1 had note N in its +/// system prompt while it did, so N+1 already carries whatever of N still +/// mattered. Injecting both would grow without bound and re-state dead ends +/// the agent has since left behind. +pub fn latest(notes: []const Note) ?Note { + return if (notes.len == 0) null else notes[notes.len - 1]; +} + +pub fn blockFrom(arena: Allocator, note: Note) []const u8 { + if (note.text.len == 0) return ""; + return std.fmt.allocPrint(arena, "{s}\n{s}", .{ header, note.text }) catch ""; +} + +/// The block for the store as it exists on disk RIGHT NOW. Reading the file +/// here rather than caching a copy in the conversation IS the compaction +/// survival mechanism — there is nothing in the history for a summarizer to +/// paraphrase. +pub fn blockNow(io: Io, arena: Allocator, session: []const u8) []const u8 { + const note = latest(load(io, arena, session)) orelse return ""; + return blockFrom(arena, note); +} + +/// Compose the block onto a system-prompt base. Identity when there is no +/// note, so a session that never compacted pays nothing. +pub fn compose(io: Io, arena: Allocator, base: []const u8, session: []const u8) []const u8 { + const b = blockNow(io, arena, session); + if (b.len == 0) return base; + return std.fmt.allocPrint(arena, "{s}\n\n{s}", .{ base, b }) catch base; +} + +/// The one bounded turn. Names what the note is FOR — the things a summary +/// reliably drops — and gives the model an explicit way out, so a compaction +/// with nothing in flight costs a short reply instead of invented state. +pub const instruction = + \\Your context is about to be compacted: everything above this point will be + \\replaced by a summary, and whatever that summary judges unimportant is gone. + \\ + \\Before that happens, write a note to your FUTURE SELF. It is stored verbatim + \\outside the conversation and re-injected after the compaction — no summarizer + \\rewrites it. Cover only these, in this order: + \\ + \\1. SUBGOAL — the one thing you are in the middle of right now. + \\2. ANCHORS — the exact file:line locations you would otherwise have to find + \\ again (paths with line numbers, not descriptions). + \\3. DECISIONS — what you chose and why, so you do not relitigate it. + \\4. DEAD ENDS — what you already tried that did not work, so you do not + \\ try it again. + \\ + \\Terse fragments. No preamble, no narration of the conversation, no apology, + \\nothing you could re-derive in one tool call. If there is genuinely nothing + \\in flight, reply with exactly: none +; + +pub const persona = "You are writing a private note to your future self, moments before your context window is compacted. Reply with the note and nothing else."; + +test { + _ = @import("compact_note_tests.zig"); +} diff --git a/src/compact_note_glue.zig b/src/compact_note_glue.zig new file mode 100644 index 00000000..4e9d4610 --- /dev/null +++ b/src/compact_note_glue.zig @@ -0,0 +1,119 @@ +//! #391 — the Agent side of the pre-compaction note: the gate evaluated +//! against a live agent, the one bounded model call, and the system-prompt +//! refresh that makes the note visible to the very next request. +//! +//! Split from compact_note.zig for the same reason playbook_glue.zig is split +//! from playbook.zig: the store stays a leaf over std/util/phase_budget, so +//! prompts.zig can compose it into the ROOT system prompt without dragging +//! the Agent type into the prompt funnel. +//! +//! THE CALL IS THE COMPACTION'S OWN. It runs on the root's provider (the +//! history it reads is in that wire format), carries NO tools so it cannot +//! fan out, and sets `compaction_request` — which bounds the reply, drops +//! reasoning effort to low, charges the call to `CallKind.compaction`, and, +//! load-bearing here, disables the in-request overflow recovery that would +//! otherwise let a note turn recurse back into emergencyTrim mid-compaction +//! (agent_overflow.applyOverflowRecovery's `compaction_request` early-out). +//! +//! IT IS TRANSACTIONAL. The note request runs against a container-deep clone +//! of history in a throwaway arena, exactly as compact() builds its summary +//! request, so send-time normalization cannot touch the live conversation. If +//! anything at all fails — the clone, the request, an empty reply, an +//! unwritable store — the function returns a named skip and compaction +//! proceeds untouched. Losing a note is much cheaper than wedging a session. + +const std = @import("std"); +const Allocator = std.mem.Allocator; + +const Agent = @import("agent.zig").Agent; +const agent_compact = @import("agent_compact.zig"); +const compact_note = @import("compact_note.zig"); +const main_mod = @import("main.zig"); +const phase_budget = @import("phase_budget.zig"); +const playbook_glue = @import("playbook_glue.zig"); +const prompts = @import("prompts.zig"); +const textMessage = @import("messages.zig").textMessage; +const title = @import("title.zig"); + +pub const Decision = compact_note.Decision; + +/// The cheap half of the gate, over a live agent. Never serializes history: +/// a worker, a session-less scratch agent, an already-noted generation and an +/// exhausted budget are all answered before anything measures context. +pub fn gateCalls(self: *const Agent) Decision { + return compact_note.decideCalls(.{ + .sub = self.sub, + .session_name = self.session_name, + .history_rewrites = self.history_rewrites, + .last_written = self.precompact_note_gen, + .cap = phase_budget.capOf(self.run_budget), + .remaining = phase_budget.remainingOf(self.run_budget), + }); +} + +/// Write one note to self, if this is a moment that deserves one. Returns the +/// decision so a caller (and a test) can see WHICH refusal happened rather +/// than only that nothing was written. Never throws: every failure below is a +/// skip, because the caller is compaction and compaction must still run. +pub fn maybeWrite(self: *Agent) Decision { + const cheap = gateCalls(self); + if (cheap != .fire) return cheap; + const room = compact_note.decideRoom(self.provider.context, self.effectiveContextTokens()); + if (room != .fire) return room; + + // Latch the generation BEFORE the call. A note turn that fails has still + // spent one, and re-buying it on every retried compaction of the same + // unchanged history is exactly the runaway #379 taught us to avoid — an + // over-cap session compacts in a loop, and each lap would cost a note. + self.precompact_note_gen = self.history_rewrites; + const reply = askModel(self) orelse return .skip_failed; + if (!compact_note.record(self.io, self.arena, self.session_name, self.history_rewrites, reply)) + return .skip_failed; + // The note is in the SYSTEM prompt, beside HARD CONSTRAINTS, so it has to + // be re-composed to reach the next request — the same refresh a mid- + // session `/never` performs, and for the same reason. + prompts.armCompactNotes(self.session_name); + playbook_glue.refreshRoot(self, self.arena); + if (!main_mod.json_mode) self.say(" 📝 wrote a pre-compaction note to self ({d} chars)\n", .{reply.len}) catch {}; + if (self.tracer) |tr| tr.note("compact", "wrote a pre-compaction note to self (#391)"); + return .fire; +} + +/// The bounded turn. Mirrors playbook_reflect.askModel — a throwaway agent in +/// its own arena — but on the ROOT's provider and over a clone of the ROOT's +/// history, because a note about work in flight can only be written by +/// something that can still see the work. +fn askModel(self: *Agent) ?[]const u8 { + var arena_state = std.heap.ArenaAllocator.init(self.gpa); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + var messages = agent_compact.cloneJsonArray(arena, self.messages) catch return null; + messages.append(textMessage(arena, "user", compact_note.instruction) catch return null) catch return null; + var agent: Agent = .{ + .gpa = self.gpa, + .arena = arena, + .io = self.io, + .client = self.client, + .provider = self.provider, + .messages = messages, + .sub = true, // never touches stdout or the root's state + .label = "note", + .out = null, + .tracer = self.tracer, + .run_budget = self.run_budget, + .reasoning = self.reasoning, + .stream_quiet = true, + .compaction_request = true, // bounded reply, low effort, no recursive recovery + .message_mutation_arena = arena, + .sys_override = compact_note.persona, + }; + defer agent.tools_used.deinit(self.gpa); + const root = agent.request(null) catch return null; + const text = std.mem.trim(u8, title.assistantText(self.provider.kind, root), " \t\r\n"); + if (compact_note.isEmptyReply(text)) return null; + return self.arena.dupe(u8, text) catch null; +} + +test { + _ = @import("compact_note_glue_tests.zig"); +} diff --git a/src/compact_note_glue_tests.zig b/src/compact_note_glue_tests.zig new file mode 100644 index 00000000..99f84dcc --- /dev/null +++ b/src/compact_note_glue_tests.zig @@ -0,0 +1,178 @@ +//! Tests for the #391 note turn's PRODUCTION entry point and for the claim +//! that makes the whole feature worth having: the note is state, not +//! conversation, so destroying the history cannot destroy the note. +//! +//! WHAT IS NOT COVERED, stated plainly. `maybeWrite`'s `.fire` branch issues a +//! real provider request, which a unit test cannot drive — so every case here +//! exercises a REFUSAL through the real entry point, plus the gate that would +//! have admitted the call. The request itself, and the model's reply, are +//! covered only by the shapes it borrows wholesale from playbook_reflect +//! (a throwaway agent, no tools) and compact() (a container-deep history +//! clone in a throwaway arena). + +const std = @import("std"); +const Io = std.Io; + +const Agent = @import("agent.zig").Agent; +const compact_note = @import("compact_note.zig"); +const glue = @import("compact_note_glue.zig"); +const messages_mod = @import("messages.zig"); +const prompts = @import("prompts.zig"); +const run_budget = @import("run_budget.zig"); + +fn inScratch(comptime body: fn (Io, std.mem.Allocator) anyerror!void) !void { + if (@import("builtin").os.tag == .windows) return; + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var orig = try Io.Dir.cwd().openDir(io, ".", .{}); + defer orig.close(io); + defer _ = std.posix.system.fchdir(orig.handle); + if (std.posix.system.fchdir(tmp.dir.handle) != 0) return error.ChdirFailed; + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + try body(io, arena_state.allocator()); +} + +/// An agent with exactly the fields the gate reads. `provider`/`client` stay +/// undefined on purpose: a case that reached them would be a case that made a +/// network call, and none of these may. +fn stub(arena: std.mem.Allocator, budget: *run_budget.RunBudget) Agent { + return .{ + .gpa = std.testing.allocator, + .arena = arena, + .io = std.testing.io, + .client = undefined, + .provider = undefined, + .messages = undefined, + .sub = false, + .label = "test", + .out = null, + .session_name = "last", + .run_budget = budget, + }; +} + +test "maybeWrite (#391): a WORKER is refused at the production entry point, before anything is measured" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + var budget: run_budget.RunBudget = .{ .max_model_calls = 0 }; // unlimited: budget is not the reason + var agent = stub(arena_state.allocator(), &budget); + agent.sub = true; + // Reaching a network call here would fault on `provider`/`client`, so a + // pass is itself the proof that no request was attempted. + try std.testing.expectEqual(compact_note.Decision.skip_worker, glue.maybeWrite(&agent)); + try std.testing.expect(agent.precompact_note_gen == null); // nothing latched either +} + +test "maybeWrite (#391): the other refusals also come back named, and none of them writes" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + // No durable session: nothing would read the note back. + var unlimited: run_budget.RunBudget = .{ .max_model_calls = 0 }; + var homeless = stub(arena, &unlimited); + homeless.session_name = ""; + try std.testing.expectEqual(compact_note.Decision.skip_no_session, glue.maybeWrite(&homeless)); + + // This history generation already has a note; a retried compaction of the + // same unchanged history must not buy a second one. + var noted = stub(arena, &unlimited); + noted.history_rewrites = 7; + noted.precompact_note_gen = 7; + try std.testing.expectEqual(compact_note.Decision.skip_already, glue.maybeWrite(¬ed)); + + // The pool is down to #390's landing reserve: those calls belong to + // landing and narrating the work, and the note is junior to both. + var tight: run_budget.RunBudget = .{ .max_model_calls = 30 }; + tight.model_calls = .init(30); + var broke = stub(arena, &tight); + try std.testing.expectEqual(compact_note.Decision.skip_budget, glue.maybeWrite(&broke)); + try std.testing.expect(broke.precompact_note_gen == null); + + // And the case that WOULD fire, proven at the gate rather than by making + // the call: a root, with a session, on a fresh generation, in budget. + var ready = stub(arena, &unlimited); + try std.testing.expectEqual(compact_note.Decision.fire, glue.gateCalls(&ready)); +} + +test "#391: the note is STATE — a wiped history cannot touch it, and it re-injects verbatim" { + try inScratch(struct { + fn body(io: Io, arena: std.mem.Allocator) !void { + // Arm the injection for this session and disarm on the way out: + // every other test in the suite relies on the funnel staying a + // pure string function with an `undefined` Io. + prompts.armCompactNotes("last"); + defer prompts.armCompactNotes(""); + + var budget: run_budget.RunBudget = .{ .max_model_calls = 0 }; + var agent = stub(arena, &budget); + agent.messages = std.json.Array.init(arena); + try agent.messages.append(try messages_mod.textMessage(arena, "user", "please finish the retry ladder")); + try agent.messages.append(try messages_mod.textMessage(arena, "assistant", "on it — reading agent_request.zig")); + + const note = + \\SUBGOAL: finish the retry ladder in agent_request.zig + \\ANCHORS: src/agent_request.zig:273, src/provider.zig:137 + \\DECISIONS: Retry-After over exponential backoff — the gateway sends it + \\DEAD ENDS: closeCodexWs before the trim wedges the chain + ; + // The same store call maybeWrite makes once the model has replied. + try std.testing.expect(compact_note.record(io, arena, "last", 0, note)); + + try prompts.setSystemPrompts(&agent, "ROOT-BASE", arena); + const before = try arena.dupe(u8, agent.sys_normal); + try std.testing.expect(std.mem.indexOf(u8, before, "ROOT-BASE") != null); + try std.testing.expect(std.mem.indexOf(u8, before, compact_note.header) != null); + try std.testing.expect(std.mem.indexOf(u8, before, note) != null); // verbatim, every line + + // Now do the worst thing compaction can possibly do: replace the + // entire conversation with a summary that mentions none of it. + agent.messages = std.json.Array.init(arena); + try agent.messages.append(try messages_mod.textMessage(arena, "user", + \\Context: the earlier conversation was compacted to save space. + \\Summary of the earlier work: the user asked for some changes. + )); + agent.history_rewrites += 1; + + // The note was never in the history, so there was nothing there to + // summarize away. + for (agent.messages.items) |m| { + const c = m.object.get("content").?; + try std.testing.expect(std.mem.indexOf(u8, c.string, "DEAD ENDS") == null); + } + + // And the next prompt the model sees carries it back BYTE-FOR-BYTE: + // deterministic re-injection, no summarizer in the path. + try prompts.setSystemPrompts(&agent, "ROOT-BASE", arena); + try std.testing.expectEqualStrings(before, agent.sys_normal); + try std.testing.expect(std.mem.indexOf(u8, agent.sys_normal, note) != null); + // Exactly one copy — recomposition replaces the block, never stacks it. + try std.testing.expectEqual(@as(usize, 1), std.mem.count(u8, agent.sys_normal, compact_note.header)); + + // The strict/ultra variants carry it too, so /strict and /ultracode + // cannot be a way to lose it. + for ([_][]const u8{ agent.sys_strict, agent.sys_ultra, agent.sys_ultra_strict }) |v| + try std.testing.expect(std.mem.indexOf(u8, v, "DEAD ENDS: closeCodexWs") != null); + // The BASE is remembered unpolluted, so the next refresh composes + // from it rather than stacking a second block on the first. + try std.testing.expectEqualStrings("ROOT-BASE", agent.sys_base); + } + }.body); +} + +test "#391: a session with no note pays nothing, armed or not" { + try inScratch(struct { + fn body(io: Io, arena: std.mem.Allocator) !void { + _ = io; + prompts.armCompactNotes("last"); + defer prompts.armCompactNotes(""); + var budget: run_budget.RunBudget = .{ .max_model_calls = 0 }; + var agent = stub(arena, &budget); + agent.messages = std.json.Array.init(arena); + try prompts.setSystemPrompts(&agent, "ROOT-BASE", arena); + try std.testing.expectEqualStrings("ROOT-BASE", agent.sys_normal); + } + }.body); +} diff --git a/src/compact_note_tests.zig b/src/compact_note_tests.zig new file mode 100644 index 00000000..8c29bc26 --- /dev/null +++ b/src/compact_note_tests.zig @@ -0,0 +1,256 @@ +//! Tests for the #391 pre-compaction note store, its gate ladder, and its +//! injection block. Reached through the `test { _ = @import(...) }` hook at +//! the bottom of compact_note.zig. +//! +//! Everything that touches the store FILE goes through a real +//! createFile/read round trip in a scratch cwd rather than a hand-built +//! string: the entire claim of #391 is that the note outlives the history +//! that produced it, and a test that never serializes cannot show that. + +const std = @import("std"); +const Io = std.Io; + +const compact_note = @import("compact_note.zig"); +const phase_budget = @import("phase_budget.zig"); + +/// Same fchdir scratch harness playbook_tests.zig uses, and for the same +/// reason: the store takes no Dir parameter. +fn inScratch(comptime body: fn (Io, std.mem.Allocator) anyerror!void) !void { + if (@import("builtin").os.tag == .windows) return; + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var orig = try Io.Dir.cwd().openDir(io, ".", .{}); + defer orig.close(io); + defer _ = std.posix.system.fchdir(orig.handle); + if (std.posix.system.fchdir(tmp.dir.handle) != 0) return error.ChdirFailed; + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + try body(io, arena_state.allocator()); +} + +/// A budget that comfortably clears the landing reserve, so the call gate is +/// never the reason a case below skips. +const rich_cap: u64 = 100; +const rich_remaining: u64 = 100; + +fn baseInputs() compact_note.Inputs { + return .{ + .sub = false, + .session_name = "last", + .history_rewrites = 0, + .last_written = null, + .cap = rich_cap, + .remaining = rich_remaining, + }; +} + +test "decide (#391): fires exactly once when compaction is imminent, and not otherwise" { + const window: u64 = 200_000; + const at_compact_threshold: u64 = 160_000; // 80% — where compaction actually fires + + // The one case that fires: a root, with a session, mid-compaction, with + // budget and window headroom. + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decide(baseInputs(), window, at_compact_threshold)); + + // ONCE: the same history generation never buys a second note, however + // often compaction is retried after a transient failure (#379's loop). + var noted = baseInputs(); + noted.last_written = 0; + try std.testing.expectEqual(compact_note.Decision.skip_already, compact_note.decide(noted, window, at_compact_threshold)); + // …and the NEXT compaction (history_rewrites has advanced) buys one again. + noted.history_rewrites = 1; + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decide(noted, window, at_compact_threshold)); + + // No durable session: nothing would ever read the note back. + var homeless = baseInputs(); + homeless.session_name = ""; + try std.testing.expectEqual(compact_note.Decision.skip_no_session, compact_note.decide(homeless, window, at_compact_threshold)); + + // No token buffer left: the window is already inside the reserve, so the + // tokens have to go to the compaction that must happen regardless. + try std.testing.expectEqual( + compact_note.Decision.skip_no_headroom, + compact_note.decide(baseInputs(), window, window - compact_note.note_reserve_tokens + 1), + ); + // Exactly at the buffer boundary still fires — the reserve is what it says. + try std.testing.expectEqual( + compact_note.Decision.fire, + compact_note.decide(baseInputs(), window, window - compact_note.note_reserve_tokens), + ); + // An unknown window cannot prove there is no room, and the summary request + // this precedes is about to ship the same input anyway. + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decide(baseInputs(), 0, 10_000_000)); +} + +test "decide (#391): a WORKER never writes a note, whatever else is true" { + var worker = baseInputs(); + worker.sub = true; + // Checked FIRST, so a subagent is refused even in the case that would + // otherwise fire — and still refused when every other gate would also + // have refused, which is what makes the ordering observable. + try std.testing.expectEqual(compact_note.Decision.skip_worker, compact_note.decide(worker, 200_000, 160_000)); + try std.testing.expectEqual(compact_note.Decision.skip_worker, compact_note.decideCalls(worker)); + worker.session_name = "child"; + worker.cap = 8; + worker.remaining = 1; + try std.testing.expectEqual(compact_note.Decision.skip_worker, compact_note.decide(worker, 8_000, 7_999)); +} + +test "decideCalls (#391): the budget gate IS #390's landing reserve, not a second one" { + const cap: u64 = 30; + // Derived, never hard-coded: the first `remaining` that admits a note is + // one call above whatever phase_budget holds back for the root to land and + // narrate the work. A parallel reserve of the note's own would move this. + const reserve = phase_budget.landingReserve(cap); + var in = baseInputs(); + in.cap = cap; + + in.remaining = reserve + phase_budget.cost_precompact_note; + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decideCalls(in)); + + in.remaining = reserve; // exactly the reserve: those calls belong to landing + try std.testing.expectEqual(compact_note.Decision.skip_budget, compact_note.decideCalls(in)); + + in.remaining = 0; + try std.testing.expectEqual(compact_note.Decision.skip_budget, compact_note.decideCalls(in)); + + // The gate delegates to the ledger rather than re-deriving the arithmetic: + // for every remaining value the two answers agree, by construction. + const ledger = phase_budget.Ledger.init(cap); + var r: u64 = 0; + while (r <= reserve + 4) : (r += 1) { + in.remaining = r; + const affords = ledger.affordsHarnessNote(r); + try std.testing.expectEqual(affords, compact_note.decideCalls(in) == .fire); + } + + // An unlimited pool (cap 0) is the common case and never gates. + in.cap = 0; + in.remaining = std.math.maxInt(u64); + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decideCalls(in)); +} + +test "store round trip (#391): a note survives the process boundary and comes back verbatim" { + try inScratch(struct { + fn body(io: Io, arena: std.mem.Allocator) !void { + try std.testing.expectEqual(@as(usize, 0), compact_note.load(io, arena, "last").len); + try std.testing.expectEqualStrings("", compact_note.blockNow(io, arena, "last")); + + // Multi-line, with the punctuation a real note carries. Byte + // equality on the way out is the whole contract: "deterministic + // re-injection, no summarizer rewrites". + const note = + \\SUBGOAL: finish the retry ladder in agent_request.zig + \\ANCHORS: src/agent_request.zig:273 (rebuild loop), src/provider.zig:137 + \\DECISIONS: chose Retry-After over exponential backoff — the gateway sends it + \\DEAD ENDS: closeCodexWs before the trim wedges the chain, do NOT re-try that + ; + try std.testing.expect(compact_note.record(io, arena, "last", 3, note)); + + const notes = compact_note.load(io, arena, "last"); + try std.testing.expectEqual(@as(usize, 1), notes.len); + try std.testing.expectEqualStrings(note, notes[0].text); // verbatim, newlines and all + try std.testing.expectEqual(@as(u32, 3), notes[0].generation); + try std.testing.expectEqualStrings("last", notes[0].session); + try std.testing.expect(notes[0].created_at > 0); + + // The injection block carries the note whole under a header that + // says what it is, so the model cannot mistake it for a summary. + const block = compact_note.blockNow(io, arena, "last"); + try std.testing.expect(std.mem.startsWith(u8, block, compact_note.header)); + try std.testing.expect(std.mem.endsWith(u8, block, note)); + + // Sessions do not read each other's working state. + try std.testing.expectEqualStrings("", compact_note.blockNow(io, arena, "other")); + } + }.body); +} + +test "store (#391): a newer note supersedes the older one wholesale" { + try inScratch(struct { + fn body(io: Io, arena: std.mem.Allocator) !void { + try std.testing.expect(compact_note.record(io, arena, "last", 1, "SUBGOAL: the first thing entirely")); + try std.testing.expect(compact_note.record(io, arena, "last", 2, "SUBGOAL: the second thing entirely")); + const notes = compact_note.load(io, arena, "last"); + try std.testing.expectEqual(@as(usize, 2), notes.len); // the log only ever grows + const block = compact_note.blockNow(io, arena, "last"); + try std.testing.expect(std.mem.indexOf(u8, block, "the second thing") != null); + try std.testing.expect(std.mem.indexOf(u8, block, "the first thing") == null); + try std.testing.expectEqual(@as(u32, 2), compact_note.latest(notes).?.generation); + } + }.body); +} + +test "record (#391): an empty, absent or refused note degrades to no note at all" { + try inScratch(struct { + fn body(io: Io, arena: std.mem.Allocator) !void { + // The prompt's explicit way out, whitespace, and a fragment too + // short to be a handoff all store NOTHING — and, crucially, none + // of them is an error: compaction proceeds either way. + for ([_][]const u8{ "none", " none \n", "None.", "none — nothing is in flight right now", "", " \n\t ", "ok" }) |reply| { + try std.testing.expect(!compact_note.record(io, arena, "last", 1, reply)); + } + try std.testing.expectEqual(@as(usize, 0), compact_note.load(io, arena, "last").len); + try std.testing.expectEqualStrings("", compact_note.blockNow(io, arena, "last")); + // A base with no note is returned unchanged — a session that never + // compacted pays nothing. + try std.testing.expectEqualStrings("BASE", compact_note.compose(io, arena, "BASE", "last")); + + // A session name that could escape .graff/notes is refused before + // any write, and reads back as an empty store. + for ([_][]const u8{ "../../etc/passwd", "a/b", "..", "" }) |bad| { + try std.testing.expect(compact_note.pathFor(arena, bad) == null); + try std.testing.expect(!compact_note.record(io, arena, bad, 1, "SUBGOAL: something long enough to store")); + try std.testing.expectEqual(@as(usize, 0), compact_note.load(io, arena, bad).len); + } + + // A real note still lands after all of that, and composes. + try std.testing.expect(compact_note.record(io, arena, "last", 1, "SUBGOAL: something long enough to store")); + const composed = compact_note.compose(io, arena, "BASE", "last"); + try std.testing.expect(std.mem.startsWith(u8, composed, "BASE\n\n")); + try std.testing.expect(std.mem.indexOf(u8, composed, "long enough to store") != null); + } + }.body); +} + +test "parse (#391): a torn tail or a junk line costs at most its own record" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + const notes = compact_note.parse(arena, + \\{"v":1,"session":"last","gen":1,"text":"first note","created_at":10} + \\not json at all + \\{"v":1,"session":"last","gen":2,"text":"second note","created_at":20} + \\ + \\{"v":1,"session":"last","gen":3,"text":"tor + ); + try std.testing.expectEqual(@as(usize, 2), notes.len); + try std.testing.expectEqualStrings("first note", notes[0].text); + try std.testing.expectEqualStrings("second note", compact_note.latest(notes).?.text); + // Missing/odd fields degrade rather than drop the record or fault. + const sparse = compact_note.parse(arena, + \\{"text":"no metadata"} + \\{"v":1,"gen":-4,"text":"negative generation"} + \\{"v":1,"gen":1} + \\{"v":1,"gen":1,"text":""} + \\[1,2,3] + ); + try std.testing.expectEqual(@as(usize, 2), sparse.len); + try std.testing.expectEqual(@as(u32, 0), sparse[0].generation); + try std.testing.expectEqual(@as(u32, 0), sparse[1].generation); + try std.testing.expect(compact_note.latest(&.{}) == null); +} + +test "record (#391): an oversized reply is capped rather than refused" { + try inScratch(struct { + fn body(io: Io, arena: std.mem.Allocator) !void { + const huge = try arena.alloc(u8, compact_note.max_text * 3); + @memset(huge, 'x'); + try std.testing.expect(compact_note.record(io, arena, "last", 1, huge)); + const notes = compact_note.load(io, arena, "last"); + try std.testing.expectEqual(@as(usize, 1), notes.len); + try std.testing.expectEqual(@as(usize, compact_note.max_text), notes[0].text.len); + } + }.body); +} diff --git a/src/phase_budget.zig b/src/phase_budget.zig index 96d6b4ee..88a5127d 100644 --- a/src/phase_budget.zig +++ b/src/phase_budget.zig @@ -89,6 +89,15 @@ pub fn landingReserve(cap: u64) u64 { return @max(min_landing_reserve, cap / 5); } +/// #391: what ONE pre-compaction note-to-self costs. It is a cost, not a +/// second reserve, and that distinction is the whole point. The reserve above +/// already protects the calls the root needs to land and narrate the work +/// (#390); a note with a reserve of its own would be a SECOND ledger over the +/// same pool, and two ledgers each holding back "the last call" double-count +/// it. Charged through `affordsHarnessNote` instead, so the note runs only +/// when it fits on top of the reserve rather than out of it. +pub const cost_precompact_note: u64 = 1; + /// The cheapest honest instantiation of a catalog shape — what a fleet costs /// at all, before any scaling to the ask. `admit` compares this (plus the /// reserve) against `remaining()`: below it a fleet cannot finish, so the @@ -197,6 +206,15 @@ pub const Ledger = struct { return remaining < later_min + self.reserve; } + /// P3 for the HARNESS's own optional call (#391's pre-compaction note). + /// Same predicate the judges use, and deliberately so: the note is the + /// junior liability on this ledger. Narration is mandatory and owns the + /// reserve; the note is a nice-to-have and must clear it. A run that can + /// only afford one more call spends it landing, not journaling. + pub fn affordsHarnessNote(self: Ledger, remaining: u64) bool { + return self.fits(remaining, cost_precompact_note); + } + pub fn commit(self: *Ledger, cost: u64) void { self.committed += cost; } @@ -275,6 +293,23 @@ test "Ledger: spendable, fits and earlyExit all sit ON TOP of the reserve" { try std.testing.expectEqual(@as(u64, 12), l.committed); } +test "affordsHarnessNote (#391): the note clears #390's reserve, it does not get one of its own" { + const l = Ledger.init(30); + // The boundary is DERIVED from the landing reserve, not from a constant of + // the note's own: the first `remaining` that admits a note is one call + // above the reserve #390 holds back. A second, independent reserve would + // move this boundary and fail here. + const boundary = landingReserve(30) + cost_precompact_note; + try std.testing.expect(l.affordsHarnessNote(boundary)); + try std.testing.expect(!l.affordsHarnessNote(boundary - 1)); // exactly the reserve: landing only + try std.testing.expect(!l.affordsHarnessNote(0)); + // And the note is junior to a phase that fits: whatever spendable() says + // is available for real work is available for the note too, never more. + try std.testing.expectEqual(l.fits(boundary, cost_precompact_note), l.affordsHarnessNote(boundary)); + // An unlimited pool always affords it (the reserve is still nominal). + try std.testing.expect(Ledger.init(0).affordsHarnessNote(std.math.maxInt(u64))); +} + test "Ledger: an unlimited pool never gates" { const l = Ledger.init(0); try std.testing.expect(l.unlimited()); diff --git a/src/prompts.zig b/src/prompts.zig index 9e768059..52ea3850 100644 --- a/src/prompts.zig +++ b/src/prompts.zig @@ -29,6 +29,7 @@ const shapes = @import("shapes.zig"); const text = @import("prompt_text.zig"); // #421: the segment TEXT; this file owns their gates const Agent = @import("agent.zig").Agent; const playbook = @import("playbook.zig"); // #381: the user-constraint block composed onto the ROOT's base prompt +const compact_note = @import("compact_note.zig"); // #391: the pre-compaction note-to-self, composed the same way and for the same reason const no_local_tools = @import("no_local_tools.zig"); // #330's subtractive gate — one half of every capability answer below const tool_gates = @import("tool_gates.zig"); // #352's additive gate — the other half const session_index = @import("session_index.zig"); // #410: where the durable transcript lives @@ -152,6 +153,19 @@ pub fn baseForSession(arena: Allocator) ![]const u8 { /// unit test — the same arming discipline playbook.g_root_inject uses. var g_transcript_note: []const u8 = ""; +/// #391: the session whose pre-compaction notes ride the ROOT's prompt. +/// Empty for every non-root agent and every unit test — the arming discipline +/// g_transcript_note and playbook.g_root_inject both use, and what keeps a +/// bare `Agent` with an `undefined` Io out of the filesystem. +var g_note_session: []const u8 = ""; + +/// Armed by setRootSystemPrompts, and re-armed by the note writer itself so a +/// `/save ` mid-session cannot leave the injection reading a store the +/// writer has stopped writing to. +pub fn armCompactNotes(session_name: []const u8) void { + g_note_session = session_name; +} + /// One line naming the durable transcript. Deliberately NOT described as /// JSONL: `.graff/sessions/.session.json` is a single JSON object (the /// JSONL files are the `.graff/traces` event streams the paragraph above @@ -222,10 +236,15 @@ pub fn ultracodeActive(agent: *const Agent) bool { pub fn setSystemPrompts(agent: *Agent, base: []const u8, arena: Allocator) !void { agent.sys_base = base; const with_playbook = if (playbook.g_root_inject) playbook.composeRoot(agent.io, arena, base) else base; + // #391: the pre-compaction note-to-self rides the SAME funnel, one rung + // below the user's constraints — a rule outranks the agent's own working + // state. Same arming discipline as the two blocks around it, so this stays + // a pure string funnel for every non-root agent and every unit test. + const with_notes = if (g_note_session.len == 0) with_playbook else compact_note.compose(agent.io, arena, with_playbook, g_note_session); // #410: the transcript line is a fact about the SESSION, not about the // persona, so it re-composes here rather than being baked into a base a // later set_agent/set_system_prompt would replace (the #326 staleness class). - const composed = if (g_transcript_note.len == 0) with_playbook else try std.fmt.allocPrint(arena, "{s}{s}", .{ with_playbook, g_transcript_note }); + const composed = if (g_transcript_note.len == 0) with_notes else try std.fmt.allocPrint(arena, "{s}{s}", .{ with_notes, g_transcript_note }); agent.sys_normal = composed; agent.sys_strict = try std.fmt.allocPrint(arena, "{s}{s}", .{ composed, strict_note }); agent.sys_ultra = try std.fmt.allocPrint(arena, "{s}{s}", .{ composed, ultracode_system_note }); @@ -243,6 +262,7 @@ pub fn setRootSystemPrompts(agent: *Agent, base: []const u8, arena: Allocator) ! // process can still open is worth a line of context — with the native file // and shell tools removed (#330) the path is unreadable from here. armSessionTranscript(arena, agent.session_name, detectCaps()); + armCompactNotes(agent.session_name); // #391: same session, same one-time arming return setSystemPrompts(agent, base, arena); } diff --git a/src/test_hooks.zig b/src/test_hooks.zig index 033f8746..ebfaf024 100644 --- a/src/test_hooks.zig +++ b/src/test_hooks.zig @@ -86,6 +86,12 @@ const agent_eval_control_tests = @import("agent_eval_control_tests.zig"); const playbook_glue = @import("playbook_glue.zig"); const playbook_reflect = @import("playbook_reflect.zig"); +// #391: the pre-compaction note store and its note turn. prompts.zig reaches +// compact_note.zig through a CALL only, and agent_compact.zig reaches the glue +// the same way, so neither pulls its tests in without these. +const compact_note = @import("compact_note.zig"); +const compact_note_glue = @import("compact_note_glue.zig"); + // #345: the global-vs-project MCP config merge. mcp.zig does reference its // decls, but the hook makes the coverage explicit rather than contingent on // that staying true. @@ -175,6 +181,8 @@ test { _ = agent_eval_control_tests; _ = playbook_glue; _ = playbook_reflect; + _ = compact_note; + _ = compact_note_glue; _ = shutdown_trace; _ = credential_store; _ = engine_events; From 232f092ce60e1b749f1e6569caa827336ec96032 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Thu, 6 Aug 2026 19:34:05 +0800 Subject: [PATCH 2/2] feat(compact): no note when the harness is salvaging, not rolling over MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's codex PTY scenario caught the note turn. Investigating it turned up a real gap and corrected a wrong assumption about where the boundary is. THE GAP, now closed. #391 is about a PLANNED rollover: context is filling, so reserve a buffer and spend one call before the window turns over. It is not about a rescue. Once the provider has rejected a request for exceeding the window (last_request_context_overflow), or the meter is at the destructive- recovery boundary where compactOrRecover may drop real history (Provider.nearContextLimit, 95%), the session has demonstrably run out of room and a note is the last thing it can afford. decideRoom becomes decideContext and refuses those outright, BEFORE the token-buffer question — which would have said yes, because at 95% of a 200k window there are still 10k tokens of nominal room. contextOf reads the same two signals compactOrRecover uses to authorize destructive trimming, so the note fires exactly when compaction is scheduled and never when it is damage control. Two unit tests, one pure and one through maybeWrite with a live provider. THE ASSUMPTION, corrected with evidence. run_midturn_compaction_scenario is NOT the recovery path. test-pty-codex-ws.py sets the server meter to 90% of the window and codex_ws_test.py says why: "Cross compact@ (80%) but stay below the destructive recovery boundary (95%)." runTurn's mid-turn gate is inputOverCompactThreshold (80%), and trim_on_fail is false there. So it is the planned rollover, and the single most representative instance of #391 in the tree — a long tool-loop turn crossing the threshold mid-flight, the failure mode the issue was filed over. Suppressing the note there would suppress it on the main path. So the scenario is taught the new shape rather than the note suppressed, and the #195 WS invariant is CHECKED rather than argued: the note turn is a quiet tool-less SSE request that opens no WebSocket (connection_id is None), runs inside runTurn's existing closeCodexWs bracket against a throwaway clone of history, and leaves the choreography untouched — ws_connections stays 2, the post-compaction turn still re-anchors on a fresh socket, and no request carries previous_response_id. Verified by running it, not by reasoning. Both compaction fixtures are re-keyed on the request's last USER TURN instead of its ordinal. An ordinal-keyed fixture re-targets silently when compaction gains a step: the summary reply landed on the note turn, the note reply became the handoff summary, and midturn still went green while proving something else. The transactional scenario failed outright for the same reason. And the scenario now pins #391 end to end on the wire, which no unit test can reach: the note is absent from `instructions` before it is written, present VERBATIM in every request after it, and absent from `input` on the post-compaction turn — state, not conversation, with nothing in the history for the next compaction to paraphrase away. Confirmed load-bearing by disabling the injection and watching it fail. Co-Authored-By: Codegraff --- scripts/codex_ws_test.py | 203 ++++++++++++++++++++++++++------ src/compact_note.zig | 47 ++++++-- src/compact_note_glue.zig | 20 +++- src/compact_note_glue_tests.zig | 62 ++++++++++ src/compact_note_tests.zig | 93 +++++++++++---- 5 files changed, 356 insertions(+), 69 deletions(-) diff --git a/scripts/codex_ws_test.py b/scripts/codex_ws_test.py index a48a5ee3..74defd37 100644 --- a/scripts/codex_ws_test.py +++ b/scripts/codex_ws_test.py @@ -44,6 +44,39 @@ TRANSACTIONAL_REASONING_MARKER = "transactional-active-reasoning:" TRANSACTIONAL_CALL_ID = "call_transactional_1" +# #391: compaction now spends one extra quiet turn writing a note to self +# before the summary. Both synthetic turns are identified by the head of their +# instruction so the fixtures below can be keyed on CONTENT rather than on a +# request ordinal — see midturn_events for why that matters. +NOTE_INSTRUCTION_HEAD = "Your context is about to be compacted" +COMPACT_INSTRUCTION_HEAD = "Summarize this entire conversation" +NOTE_BLOCK_HEADER = "NOTES TO SELF" +MIDTURN_NOTE = ( + "SUBGOAL: exercise the server-side context meter\n" + "ANCHORS: src/agent_compact.zig:130\n" + "DEAD ENDS: none yet" +) +TRANSACTIONAL_NOTE = ( + "SUBGOAL: prove the transactional rollback\nANCHORS: src/agent_compact.zig:152" +) + + +def user_text(item: object) -> str | None: + if not isinstance(item, dict) or item.get("role") != "user": + return None + content = item.get("content") + return content if isinstance(content, str) else None + + +def last_user_text(request: RecordedRequest) -> str: + """The final user turn of a Responses request, or "" — how the two + synthetic compaction turns are told apart without counting ordinals.""" + for item in reversed(request.body.get("input") or []): + text = user_text(item) + if text is not None: + return text + return "" + def response_events( item: dict | list[dict], response_id: str, total_tokens: int @@ -95,7 +128,28 @@ def active_reasoning_item() -> dict: def midturn_events(request: RecordedRequest) -> list[dict]: - """Script tool call -> compaction summary -> final answer.""" + """Script tool call -> #391 note to self -> compaction summary -> answer. + + Keyed on the request's LAST USER TURN, not on its ordinal. Compaction + gained a step in #391, and an ordinal-keyed fixture re-targets silently + when that happens: the summary reply lands on the note turn, the note reply + becomes the handoff summary, and the scenario still goes green while + proving something else entirely. Content keying makes the fixture describe + what each reply is FOR. + """ + tail = last_user_text(request) + if tail.startswith(NOTE_INSTRUCTION_HEAD): + return response_events( + message_item(MIDTURN_NOTE, "msg_midturn_note"), + "resp_midturn_note", + 1_050, + ) + if tail.startswith(COMPACT_INSTRUCTION_HEAD): + return response_events( + message_item(MIDTURN_SUMMARY, "msg_midturn_summary"), + "resp_midturn_summary", + 1_100, + ) if request.ordinal == 1: item = { "type": "function_call", @@ -114,12 +168,6 @@ def midturn_events(request: RecordedRequest) -> list[dict]: "resp_midturn_1", MIDTURN_TOTAL_TOKENS, ) - if request.ordinal == 2: - return response_events( - message_item(MIDTURN_SUMMARY, "msg_midturn_summary"), - "resp_midturn_summary", - 1_100, - ) return response_events( message_item(MIDTURN_FINAL, "msg_midturn_final"), f"resp_midturn_{request.ordinal}", @@ -140,7 +188,25 @@ def transactional_reasoning_item() -> dict: def transactional_events(request: RecordedRequest) -> list[dict]: - """Script tool call -> empty summary -> answer from restored live history.""" + """Script tool call -> note to self -> EMPTY summary -> answer from the + restored live history. Content-keyed for the same reason midturn_events is: + the empty reply has to land on the SUMMARY request specifically, and an + ordinal would have quietly moved it onto the #391 note turn instead.""" + tail = last_user_text(request) + if tail.startswith(NOTE_INSTRUCTION_HEAD): + return response_events( + message_item(TRANSACTIONAL_NOTE, "msg_transactional_note"), + "resp_transactional_note", + 1_050, + ) + if tail.startswith(COMPACT_INSTRUCTION_HEAD): + # A syntactically valid Responses answer with no summary text exercises + # compact()'s EmptySummary rollback, rather than a transport failure. + return response_events( + message_item("", "msg_transactional_empty_summary"), + "resp_transactional_empty_summary", + 1_100, + ) if request.ordinal == 1: call = { "type": "function_call", @@ -155,14 +221,6 @@ def transactional_events(request: RecordedRequest) -> list[dict]: "resp_transactional_1", MIDTURN_TOTAL_TOKENS, ) - if request.ordinal == 2: - # A syntactically valid Responses answer with no summary text exercises - # compact()'s EmptySummary rollback, rather than a transport failure. - return response_events( - message_item("", "msg_transactional_empty_summary"), - "resp_transactional_empty_summary", - 1_100, - ) return response_events( message_item(TRANSACTIONAL_FINAL, "msg_transactional_final"), f"resp_transactional_{request.ordinal}", @@ -170,13 +228,6 @@ def transactional_events(request: RecordedRequest) -> list[dict]: ) -def user_text(item: object) -> str | None: - if not isinstance(item, dict) or item.get("role") != "user": - return None - content = item.get("content") - return content if isinstance(content, str) else None - - def assert_compaction_meter(label: str, rendered: str) -> None: match = COMPACTING_RE.search(rendered) if match is None: @@ -273,22 +324,52 @@ def run_scenario( def assert_midturn_requests(mock: CodexMock) -> None: requests = mock.recorded_requests() - if len(requests) != 3: + # Four, since #391: the real turn, the pre-compaction note-to-self, the + # compaction summary, then the continuation. This scenario is a PLANNED + # rollover by construction (the server meter is 90% — over compact@ 80%, + # under the 95% recovery boundary; see MIDTURN_TOTAL_TOKENS above), which + # is exactly the case #391's note fires on. On the salvage paths — a + # provider over-window rejection, or >=95% where compactOrRecover may drop + # real history — compact_note.decideContext refuses it and this stays three. + if len(requests) != 4: raise AssertionError( - f"midturn: expected exactly 3 model requests, got {len(requests)}: {requests!r}" + f"midturn: expected exactly 4 model requests, got {len(requests)}: {requests!r}" ) - first, compact, final = requests + first, note, compact, final = requests if any("max_output_tokens" in request.body for request in requests): raise AssertionError("midturn: Responses requests must omit max_output_tokens") transports = [request.transport for request in requests] - if transports != ["ws", "sse", "ws"]: - raise AssertionError(f"midturn: expected WS -> SSE -> WS, got {transports!r}") - if mock.ws_turns != 2 or mock.sse_turns != 1 or mock.ws_connections != 2: + if transports != ["ws", "sse", "sse", "ws"]: + raise AssertionError( + f"midturn: expected WS -> SSE(note) -> SSE(summary) -> WS, got {transports!r}" + ) + if mock.ws_turns != 2 or mock.sse_turns != 2 or mock.ws_connections != 2: raise AssertionError( - "midturn: expected two turns on two fresh WS connections plus one SSE " - f"turn; ws_turns={mock.ws_turns} sse_turns={mock.sse_turns} " + "midturn: expected two turns on two fresh WS connections plus two quiet " + f"SSE turns; ws_turns={mock.ws_turns} sse_turns={mock.sse_turns} " f"ws_connections={mock.ws_connections}" ) + # The two synthetic turns are identified by CONTENT, not position, so this + # cannot silently pass if their order ever swaps. + if not last_user_text(note).startswith("Your context is about to be compacted"): + raise AssertionError( + f"midturn: request 2 is not the #391 note turn: {last_user_text(note)[:120]!r}" + ) + if not last_user_text(compact).startswith("Summarize this entire conversation"): + raise AssertionError( + f"midturn: request 3 is not the compaction summary: {last_user_text(compact)[:120]!r}" + ) + # #195's invariant, checked rather than assumed: the note turn is a quiet + # SSE request that opens NO WebSocket. It runs inside runTurn's existing + # closeCodexWs bracket, against a throwaway clone of history, so it must + # not appear in the WS choreography at all — ws_connections stays 2 and + # the post-compaction turn still re-anchors on a socket the pre-compaction + # turn never used. + if note.connection_id is not None: + raise AssertionError( + f"midturn: the note turn opened a WebSocket (conn {note.connection_id}); " + "it must stay off the WS session it is compacting around" + ) if first.connection_id == final.connection_id: raise AssertionError("midturn: final request reused the pre-compaction WS") for request in requests: @@ -298,6 +379,39 @@ def assert_midturn_requests(mock: CodexMock) -> None: f"{request.body['previous_response_id']!r}" ) + # The note turn is a bounded, tool-less auxiliary call on its own persona — + # never the root prompt, which would hand it the whole tool catalog and its + # own previous note. + if "tools" in note.body or not str(note.body.get("instructions", "")).startswith( + "You are writing a private note to your future self" + ): + raise AssertionError( + f"midturn: the note turn was not the bounded tool-less persona call: {note.body!r}" + ) + + # #391 END TO END, on the wire. The note is written before the summary, so + # every request AFTER it must carry it in `instructions` — the system + # prompt, which compaction cannot rewrite — while the request before it + # carries nothing. + if NOTE_BLOCK_HEADER in str(first.body.get("instructions", "")): + raise AssertionError("midturn: a note block existed before any note was written") + for label, request in (("summary", compact), ("post-compaction", final)): + instructions = str(request.body.get("instructions", "")) + if NOTE_BLOCK_HEADER not in instructions or MIDTURN_NOTE not in instructions: + raise AssertionError( + f"midturn: the {label} request did not carry the note-to-self VERBATIM " + f"in its system prompt: {instructions[-400:]!r}" + ) + # …and it is STATE, not conversation: the post-compaction turn's history is + # the handoff summary alone. If the note ever appears in `input` it has + # become something a later compaction can summarize away. + final_json = json.dumps(final.body.get("input") or [], separators=(",", ":")) + if NOTE_BLOCK_HEADER in final_json or MIDTURN_NOTE in final_json: + raise AssertionError( + "midturn: the note leaked into conversation history, where the next " + f"compaction would paraphrase it away: {final_json[:400]!r}" + ) + first_input = first.body.get("input") if ( first.body.get("type") != "response.create" @@ -365,25 +479,38 @@ def assert_midturn_requests(mock: CodexMock) -> None: def assert_transactional_requests(mock: CodexMock) -> None: requests = mock.recorded_requests() - if len(requests) != 3: + # Four since #391 — the note turn precedes the summary here too. The note + # is written and kept even though this compaction then FAILS: it is state + # about the live conversation, not about the summary, and the rollback + # restores history the note still describes correctly. + if len(requests) != 4: raise AssertionError( - "transactional: expected exactly 3 model requests, " + "transactional: expected exactly 4 model requests, " f"got {len(requests)}: {requests!r}" ) - first, compact, final = requests + first, note, compact, final = requests if any("max_output_tokens" in request.body for request in requests): raise AssertionError("transactional: Responses requests must omit max_output_tokens") transports = [request.transport for request in requests] - if transports != ["ws", "sse", "ws"]: + if transports != ["ws", "sse", "sse", "ws"]: raise AssertionError( - f"transactional: expected WS -> SSE -> WS, got {transports!r}" + f"transactional: expected WS -> SSE(note) -> SSE(summary) -> WS, got {transports!r}" ) - if mock.ws_turns != 2 or mock.sse_turns != 1 or mock.ws_connections != 2: + if mock.ws_turns != 2 or mock.sse_turns != 2 or mock.ws_connections != 2: raise AssertionError( "transactional: expected two turns on two fresh WS connections plus " - f"one SSE turn; ws_turns={mock.ws_turns} sse_turns={mock.sse_turns} " + f"two quiet SSE turns; ws_turns={mock.ws_turns} sse_turns={mock.sse_turns} " f"ws_connections={mock.ws_connections}" ) + if not last_user_text(note).startswith(NOTE_INSTRUCTION_HEAD): + raise AssertionError( + f"transactional: request 2 is not the #391 note turn: {last_user_text(note)[:120]!r}" + ) + if note.connection_id is not None or "tools" in note.body: + raise AssertionError( + "transactional: the note turn must be a tool-less SSE request that " + f"opens no WebSocket: {note.body!r}" + ) if first.connection_id == final.connection_id: raise AssertionError( "transactional: final request reused the pre-compaction WS" diff --git a/src/compact_note.zig b/src/compact_note.zig index 36ec8fe2..7c357806 100644 --- a/src/compact_note.zig +++ b/src/compact_note.zig @@ -109,6 +109,15 @@ pub const Decision = enum { skip_budget, /// No token buffer left for the note turn under this model's window. skip_no_headroom, + /// The harness is SALVAGING, not rolling over. #391 is about a planned + /// rollover: context is filling, so reserve a buffer and spend one call + /// before the window turns over. Once the provider has actually rejected a + /// request for exceeding the window, or the meter is at the destructive- + /// recovery boundary where compactOrRecover may drop real history, the + /// session has demonstrably run out of room — and spending a call there is + /// the exact opposite of the budget discipline the rest of this file + /// enforces. Recovery gets the tokens; the note waits for a calmer one. + skip_recovering, /// The turn ran and produced nothing usable — a transport failure, an /// explicit "none", or a store that could not be written. Distinct from /// the gates above because this one COST a call; compaction proceeds @@ -151,19 +160,41 @@ pub fn decideCalls(in: Inputs) Decision { return .fire; } -/// The token half: is there room under the window for the note turn's reply? -/// An unknown window (0) cannot prove there is not, and the summary request -/// this precedes is about to ship the same input anyway. -pub fn decideRoom(window_tokens: u64, effective_tokens: u64) Decision { - if (window_tokens == 0) return .fire; - return if (effective_tokens +| note_reserve_tokens > window_tokens) .skip_no_headroom else .fire; +/// What the context looks like at the moment compaction was decided on. +pub const Context = struct { + /// The model's advertised window; 0 when unknown. + window_tokens: u64, + /// Best current estimate of occupancy (Agent.effectiveContextTokens). + effective_tokens: u64, + /// The provider has already REJECTED a request for exceeding the window + /// (Agent.last_request_context_overflow). Not a threshold reading — a + /// concrete bounce off the wall. + over_window_rejection: bool, + /// The meter is at or past the destructive-recovery boundary, where + /// compactOrRecover is authorized to drop real history + /// (Provider.nearContextLimit, 95%). + near_limit: bool, +}; + +/// The context half. Salvage is refused OUTRIGHT, before the buffer question +/// is even asked: at 95% the arithmetic still leaves room for the note under +/// the window, so a token buffer alone would happily fire one into a session +/// that is being rescued. The distinction that matters is not how many tokens +/// are left, it is whether this compaction is planned or a rescue. +/// +/// An unknown window (0) cannot prove there is no room, and the summary +/// request this precedes is about to ship the same input anyway. +pub fn decideContext(c: Context) Decision { + if (c.over_window_rejection or c.near_limit) return .skip_recovering; + if (c.window_tokens == 0) return .fire; + return if (c.effective_tokens +| note_reserve_tokens > c.window_tokens) .skip_no_headroom else .fire; } /// The whole ladder, in the order the call site runs it. -pub fn decide(in: Inputs, window_tokens: u64, effective_tokens: u64) Decision { +pub fn decide(in: Inputs, c: Context) Decision { const calls = decideCalls(in); if (calls != .fire) return calls; - return decideRoom(window_tokens, effective_tokens); + return decideContext(c); } /// A model reply that carries no note. The prompt asks for exactly "none" diff --git a/src/compact_note_glue.zig b/src/compact_note_glue.zig index 4e9d4610..bee843e0 100644 --- a/src/compact_note_glue.zig +++ b/src/compact_note_glue.zig @@ -51,6 +51,24 @@ pub fn gateCalls(self: *const Agent) Decision { }); } +/// The context half of the gate, read off the live agent. Serializes history +/// (effectiveContextTokens), so it runs only after gateCalls has cleared. +/// +/// `last_request_context_overflow` and `nearContextLimit` are the two signals +/// that separate a PLANNED rollover from a rescue, and they are the same two +/// compactOrRecover itself uses to decide whether it may trim destructively. +/// Reading the same signals is deliberate: the note fires exactly when +/// compaction is a scheduled event and never when it is damage control. +pub fn contextOf(self: *Agent) compact_note.Context { + const effective = self.effectiveContextTokens(); + return .{ + .window_tokens = self.provider.context, + .effective_tokens = effective, + .over_window_rejection = self.last_request_context_overflow, + .near_limit = self.provider.nearContextLimit(effective), + }; +} + /// Write one note to self, if this is a moment that deserves one. Returns the /// decision so a caller (and a test) can see WHICH refusal happened rather /// than only that nothing was written. Never throws: every failure below is a @@ -58,7 +76,7 @@ pub fn gateCalls(self: *const Agent) Decision { pub fn maybeWrite(self: *Agent) Decision { const cheap = gateCalls(self); if (cheap != .fire) return cheap; - const room = compact_note.decideRoom(self.provider.context, self.effectiveContextTokens()); + const room = compact_note.decideContext(contextOf(self)); if (room != .fire) return room; // Latch the generation BEFORE the call. A note turn that fails has still diff --git a/src/compact_note_glue_tests.zig b/src/compact_note_glue_tests.zig index 99f84dcc..c416d4d9 100644 --- a/src/compact_note_glue_tests.zig +++ b/src/compact_note_glue_tests.zig @@ -97,6 +97,68 @@ test "maybeWrite (#391): the other refusals also come back named, and none of th try std.testing.expectEqual(compact_note.Decision.fire, glue.gateCalls(&ready)); } +/// An agent whose context is measurable: contextOf() serializes history, so +/// unlike the gateCalls-only stubs above this one needs a real provider and a +/// real (small) message array. +fn measurable(arena: std.mem.Allocator, budget: *run_budget.RunBudget, window: u64) !Agent { + var agent = stub(arena, budget); + agent.provider = .{ + .id = "codex", + .kind = .responses, + .auth = .bearer, + .url = "", + .api_key = "", + .model = "gpt-5", + .context = window, + }; + agent.sys_normal = ""; + agent.sys_strict = ""; + agent.tools_responses = ""; + agent.messages = std.json.Array.init(arena); + try agent.messages.append(try messages_mod.textMessage(arena, "user", "hello")); + return agent; +} + +test "contextOf (#391): reads the same planned-vs-salvage signals compactOrRecover trims on" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + var budget: run_budget.RunBudget = .{ .max_model_calls = 0 }; + const window: u64 = 200_000; + var agent = try measurable(arena, &budget, window); + + // A planned rollover: over compact@ (80%), under nearContextLimit (95%). + // This is the band the codex PTY midturn scenario runs in by construction + // ("Cross compact@ (80%) but stay below the destructive recovery boundary"), + // and it is the case #391 exists for. + agent.last_context_tokens = window * 9 / 10; + var ctx = glue.contextOf(&agent); + try std.testing.expectEqual(window, ctx.window_tokens); + try std.testing.expect(!ctx.near_limit); + try std.testing.expect(!ctx.over_window_rejection); + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decideContext(ctx)); + try std.testing.expectEqual(compact_note.Decision.fire, glue.gateCalls(&agent)); + + // Push past 95% and the SAME agent stops buying notes — contextOf derives + // near_limit from Provider.nearContextLimit, the very predicate + // compactOrRecover uses to authorize destructive trimming. + agent.last_context_tokens = window * 96 / 100; + ctx = glue.contextOf(&agent); + try std.testing.expect(ctx.near_limit); + try std.testing.expect(agent.provider.nearContextLimit(ctx.effective_tokens)); + try std.testing.expectEqual(compact_note.Decision.skip_recovering, compact_note.decideContext(ctx)); + // Through the production entry point, with provider/client live: still a + // refusal, so no request is attempted. + try std.testing.expectEqual(compact_note.Decision.skip_recovering, glue.maybeWrite(&agent)); + try std.testing.expect(agent.precompact_note_gen == null); // nothing latched + + // A concrete provider overflow rejection is salvage even well under 95%. + agent.last_context_tokens = window * 9 / 10; + agent.last_request_context_overflow = true; + try std.testing.expectEqual(compact_note.Decision.skip_recovering, glue.maybeWrite(&agent)); + try std.testing.expect(agent.precompact_note_gen == null); +} + test "#391: the note is STATE — a wiped history cannot touch it, and it re-injects verbatim" { try inScratch(struct { fn body(io: Io, arena: std.mem.Allocator) !void { diff --git a/src/compact_note_tests.zig b/src/compact_note_tests.zig index 8c29bc26..8abaecce 100644 --- a/src/compact_note_tests.zig +++ b/src/compact_note_tests.zig @@ -45,42 +45,88 @@ fn baseInputs() compact_note.Inputs { }; } -test "decide (#391): fires exactly once when compaction is imminent, and not otherwise" { - const window: u64 = 200_000; - const at_compact_threshold: u64 = 160_000; // 80% — where compaction actually fires +const window: u64 = 200_000; - // The one case that fires: a root, with a session, mid-compaction, with - // budget and window headroom. - try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decide(baseInputs(), window, at_compact_threshold)); +/// The band the note is FOR: over compact@ (80%, where compaction fires) but +/// under nearContextLimit (95%, where the harness starts salvaging). The codex +/// PTY scenario sits here on purpose — see codex_ws_test.py's own comment, +/// "Cross compact@ (80%) but stay below the destructive recovery boundary". +fn plannedRollover() compact_note.Context { + return .{ + .window_tokens = window, + .effective_tokens = window * 9 / 10, // 90% + .over_window_rejection = false, + .near_limit = false, + }; +} + +test "decide (#391): fires exactly once when compaction is imminent, and not otherwise" { + // The one case that fires: a root, with a session, on a planned rollover, + // with budget and window headroom. + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decide(baseInputs(), plannedRollover())); // ONCE: the same history generation never buys a second note, however // often compaction is retried after a transient failure (#379's loop). var noted = baseInputs(); noted.last_written = 0; - try std.testing.expectEqual(compact_note.Decision.skip_already, compact_note.decide(noted, window, at_compact_threshold)); + try std.testing.expectEqual(compact_note.Decision.skip_already, compact_note.decide(noted, plannedRollover())); // …and the NEXT compaction (history_rewrites has advanced) buys one again. noted.history_rewrites = 1; - try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decide(noted, window, at_compact_threshold)); + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decide(noted, plannedRollover())); // No durable session: nothing would ever read the note back. var homeless = baseInputs(); homeless.session_name = ""; - try std.testing.expectEqual(compact_note.Decision.skip_no_session, compact_note.decide(homeless, window, at_compact_threshold)); + try std.testing.expectEqual(compact_note.Decision.skip_no_session, compact_note.decide(homeless, plannedRollover())); - // No token buffer left: the window is already inside the reserve, so the - // tokens have to go to the compaction that must happen regardless. - try std.testing.expectEqual( - compact_note.Decision.skip_no_headroom, - compact_note.decide(baseInputs(), window, window - compact_note.note_reserve_tokens + 1), - ); + // No token buffer left: the tokens have to go to the compaction that must + // happen regardless. + var cramped = plannedRollover(); + cramped.effective_tokens = window - compact_note.note_reserve_tokens + 1; + try std.testing.expectEqual(compact_note.Decision.skip_no_headroom, compact_note.decide(baseInputs(), cramped)); // Exactly at the buffer boundary still fires — the reserve is what it says. - try std.testing.expectEqual( - compact_note.Decision.fire, - compact_note.decide(baseInputs(), window, window - compact_note.note_reserve_tokens), - ); + cramped.effective_tokens = window - compact_note.note_reserve_tokens; + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decide(baseInputs(), cramped)); // An unknown window cannot prove there is no room, and the summary request // this precedes is about to ship the same input anyway. - try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decide(baseInputs(), 0, 10_000_000)); + var unknown = plannedRollover(); + unknown.window_tokens = 0; + unknown.effective_tokens = 10_000_000; + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decide(baseInputs(), unknown)); +} + +test "decideContext (#391): a planned rollover buys a note; a SALVAGE never does" { + // The whole point of this gate. #391 is about a SCHEDULED rollover: context + // is filling, so spend one call before the window turns over. Once the + // harness is rescuing a session, that same call is the last thing it can + // afford — and the token buffer alone would NOT catch it, because at 95% + // of a 200k window there are still 10k tokens of nominal room. + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decideContext(plannedRollover())); + + // The destructive-recovery boundary: compactOrRecover may drop real + // history here, so this is damage control, not a rollover. + var rescuing = plannedRollover(); + rescuing.near_limit = true; + rescuing.effective_tokens = window * 95 / 100; + try std.testing.expectEqual(compact_note.Decision.skip_recovering, compact_note.decideContext(rescuing)); + // …refused BEFORE the buffer question, which would have said yes. This is + // the assertion that would fail if the salvage gate were ever folded into + // the token arithmetic instead of preceding it. + var buffer_only = rescuing; + buffer_only.near_limit = false; + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decideContext(buffer_only)); + + // A concrete provider rejection — the request already bounced off the wall. + var bounced = plannedRollover(); + bounced.over_window_rejection = true; + try std.testing.expectEqual(compact_note.Decision.skip_recovering, compact_note.decideContext(bounced)); + // Salvage outranks even an unknown window, which otherwise always fires. + bounced.window_tokens = 0; + try std.testing.expectEqual(compact_note.Decision.skip_recovering, compact_note.decideContext(bounced)); + + // And salvage is a CONTEXT verdict, not a call-budget one: the ledger is + // untouched by it, so the two halves stay independently readable. + try std.testing.expectEqual(compact_note.Decision.fire, compact_note.decideCalls(baseInputs())); } test "decide (#391): a WORKER never writes a note, whatever else is true" { @@ -89,12 +135,15 @@ test "decide (#391): a WORKER never writes a note, whatever else is true" { // Checked FIRST, so a subagent is refused even in the case that would // otherwise fire — and still refused when every other gate would also // have refused, which is what makes the ordering observable. - try std.testing.expectEqual(compact_note.Decision.skip_worker, compact_note.decide(worker, 200_000, 160_000)); + try std.testing.expectEqual(compact_note.Decision.skip_worker, compact_note.decide(worker, plannedRollover())); try std.testing.expectEqual(compact_note.Decision.skip_worker, compact_note.decideCalls(worker)); worker.session_name = "child"; worker.cap = 8; worker.remaining = 1; - try std.testing.expectEqual(compact_note.Decision.skip_worker, compact_note.decide(worker, 8_000, 7_999)); + var dire = plannedRollover(); + dire.near_limit = true; + dire.over_window_rejection = true; + try std.testing.expectEqual(compact_note.Decision.skip_worker, compact_note.decide(worker, dire)); } test "decideCalls (#391): the budget gate IS #390's landing reserve, not a second one" {