feat(compact): tell the model what durable state survived compaction (#411) - #459
Merged
Merged
Conversation
Corrects a false premise. #438 (issue #410) shipped a prompt line telling the model that the session file preserves what compaction discards. It does not: `.graff/sessions/<name>.session.json` is a SINGLE JSON object whose `messages` array is rewritten in place, so the next autosave after a compaction drops the pre-compaction history permanently. "Grep your own conversation log" therefore had no graff equivalent — the trace and trajectory JSONLs record events for telemetry, not the conversation for recall. `.graff/sessions/<name>.transcript.jsonl` is that equivalent: one line per message as it FIRST enters history, appended by a single positional write at the end of file and never rewritten. A line is the message's provider-native JSON verbatim (JSON escapes every control character, so a message is always exactly one line), which makes the file greppable with no tooling and makes a resume's re-seed exact — hashing a line reproduces the digest that wrote it. Hooked at session.queueSave, the point where the autosave already observes a new message, so none of the history's ~25 mutation sites needs a hook of its own. Identity is a MULTISET of the digests on disk, not a position: compact() builds `[handoff summary] ++ recent_messages`, putting the summary at the front of the history while it is the last line in the file, and any position-based match would re-append the retained tail behind it. Counting instead records the summary alone. An ordinary turn takes a fast path (two serializations plus one per new message) and only a rewrite, a resume, or a session's first append pays for a full walk. Bounds, as the issue asks: - Subagents excluded. Their history is never persisted, so there is no durable session to attach a transcript to — #409's rule, unchanged. - One lifecycle, not two. #409's sweep now reclaims transcripts and artifact dirs together (tool_spill.sweepSessionsOnce), by the same rule — the session file is the ground truth for "this session is gone" — and the same grace window. It now also runs at the first transcript append, so a run that never spills still collects what deleted sessions left behind. - The size cap ROTATES rather than head-truncates. Head-truncation means reading the file, dropping a prefix and writing what is left back over it: an in-place rewrite of the history, which is the exact failure mode this change exists to fix, and one a crash halfway through turns into total loss. A rename is one atomic syscall, the old bytes survive intact under a sibling name the model greps identically, and disk use is bounded at two generations (16 MiB each) rather than merely slowed. Tests 1043 -> 1050. The rewrite case asserts the pre-compaction file is an exact byte PREFIX of the post-compaction one and that the discarded detail is still greppable; repeated autosaves over an unchanged history add nothing; a resume re-seeds from disk instead of re-appending its restored history; a subagent writes nothing; rotation leaves the previous generation byte-identical. Verified end to end against a mock provider: two processes, four turns, four lines, no duplicates. session_transcript.activePath(root, arena) is the accessor #411's post-compaction note should use — null for a subagent or a session with no transcript, so the note can never cite a file that does not exist, with lineCount() for its "N messages". Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
One genuine portability defect, not five test bugs. Every Windows failure —
four wrong line counts and one null deref — is the same root cause, and the
CI log names it: each count was "found 0", and activePath was null because
nothing had been recorded.
`appendWhole` opened the file write-only and then called `File.stat` on that
handle to find the append offset. On Windows, Io.Dir.createFile maps `read`
straight onto the NT access mask (`GENERIC = .{ .WRITE = true, .READ =
flags.read }`), so a write-only handle carries FILE_GENERIC_WRITE, which does
NOT include FILE_READ_ATTRIBUTES. `File.stat` is NtQueryInformationFile(.All),
which requires exactly that right, and std handles the resulting ACCESS_DENIED
explicitly. So on Windows the stat failed, `appendWhole` returned null having
ALREADY created the file, and every transcript was a 0-byte file: the feature
was inert on the platform, and #411's note would have cited an empty file.
`.read = true` on the create is the fix; a path-based `statFile` fallback keeps
a future std change from turning this back into a silent no-write rather than
an append. Nothing else about the write changed — it is still one positional
write at the end of file, still append-only.
This class is invisible to the POSIX suite: with the fix reverted, macOS still
runs 1050/1050 green. CI was the only possible signal, which is the argument
for the two hardenings below rather than for trusting a local pass.
The other two suspects, checked and reported rather than assumed:
- Line endings: NOT a defect. '\n' is written explicitly and JSON escapes
every control character, so a message is always exactly one line and no
'\r' can appear. Hardened anyway, since the digests are over line bytes and
a stray '\r' would silently re-append the entire history: `seed` trims a
trailing '\r', and the tests now fail hard if a '\r' ever reaches the file.
- Rotation's rename: NOT a defect. Windows rename does not overwrite, but
Io.Dir.rename is the REPLACING variant on every OS (dirRenameWindows passes
replace_if_exists=true; renamePreserve is the one that refuses a taken
name). The rotation test only ever rotated onto a free name and could not
have told the difference, so it now rotates twice and asserts the previous
generation really was replaced.
INVARIANT, now stated where the paths are built (session_index.zig) because two
downstream branches depend on the answer: every `.graff/` path this harness
builds is forward-slashed on every platform, Windows included. Deliberately —
Windows accepts '/' in the paths reaching Io.Dir, and these strings are shown to
the model (#410's prompt line, #409's cap marker, #441's path inside #411's
note), so a shape that changes per platform buys nothing and costs goldens.
Separators were never the Windows failure here; the null deref was a downstream
symptom of the empty file. The corollary is for tests, and ee28d8c is the
precedent: assert on a basename or on a path built through the helpers, never by
matching a separator by hand. Both `activePath` assertions now go through the
accessor and check the cited file exists; the readers build their paths with
`transcriptPath`/`rotatedPath` so a test can no longer disagree with the code.
The tests moved to session_transcript_tests.zig: the hardening pushed the module
to 614 lines, over the ceiling. Reachability needed the `_ = ...` line in
test_hooks.zig's test block, not just the import — the count caught it at 1045
before it was added, and is 1050 again after.
Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
Compaction never said what the summary did NOT have to carry, so the model treated it as total loss: it hoarded file contents and quoted tool output into the summary, and the state the harness itself keeps came back only as its own recollection of it, drifting a little more at every compaction. Both halves of prime-agent's compaction design, in a new module (compact_note.zig) so agent_compact.zig keeps its line count: BEFORE. The summary REQUEST now carries a note saying what persists - every file on disk, any goal/checklist (restated in full straight after the summary), and #441's append-only transcript, cited by its real path and message count when one is live. It asks for NAMES rather than contents and points the summary at what disk cannot give back: decisions, dead ends, constraints, unfinished work. The instruction itself still LEADS the request, so #379 classifies an empty or truncated reply exactly as before. AFTER. The new history head carries the harness's own ground truth, re-derived at every compaction rather than copied forward - so a later summary that paraphrases it away costs nothing, the next compaction regenerates it exactly. Three fields, each omitted unless real: the files this session modified (from /rewind's snapshot ledger, which is exact for write_file/edit_file/imagegen and blind to bash - the note says so rather than passing a partial list off as the whole diff); the #409 artifact paths the discarded messages' own spill markers cite, so every handle named was written by a spill that succeeded; and the transcript path via session_transcript.activePath, which returns null rather than let the note invent a file. A subagent and a /review turn get nothing, and a session with nothing durable to report keeps a byte-identical handoff. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
…name for #391 #391 independently created src/compact_note.zig for the OTHER side of the same boundary: the buffer-reserved turn where the model writes notes to itself before rollover. This module is the harness's own ground truth injected around the handoff - which is what handoffMessage is and what the note actually rides - so the name it now has says what the old one only implied, and a future reader cannot confuse the two halves. #391 owns four files in that cluster, this owns one, so the rename is the cheap side. Pure rename plus the local alias (compact_note -> handoff_note) at its two call sites and one test import. agent_compact.zig stays at 564, still net zero against its branch point. Suite unchanged at 1056; the module keeps its reachability through agent_compact.zig's production import, so no test_hooks wiring changes. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
justrach
force-pushed
the
feat/411-post-compaction-note
branch
from
August 6, 2026 11:22
49e6709 to
585d550
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #411. Stacked on #457 (#441) — that must merge first; this branch is cut from it and uses its
activePathaccessor.The most load-bearing piece of prime-agent's compaction design is telling the model what persists. Both halves are implemented.
Half 1: the summary request
A note appended to the compaction summary request: files on disk, goal/todo state and the session transcript all persist, so record paths and state worth remembering. Verified to lead with the byte-identical
compact_instructionand to contain no durable-state text itself.Half 2: the post-compaction note, with real state
goal_flow.compactionSnapshot(#318), unchanged.handoffMessagenow routes it throughcompact_note.handoffrather than formatting it itself, so standing state comes first and this note last.Snapshots.modifiedPathsover/rewind's ledger, mutex-held, distinct paths in first-modified order.session_transcript.activePathandlineCount(); null means the line is omitted entirely, so it can never cite a file that does not exist."Files modified" is wired properly rather than omitted. It is exact for
write_file/edit_file/imagegenand blind to bash, so the note states that bound inline instead of implying the list is the whole diff. A/rewindalready drops the snapshots it undid, so a rewound file correctly stops being listed.On handles, and an integration note
#440 is not an ancestor of this branch. Rather than emit a blank or fabricated field, handles are implemented from what actually exists on this branch: #409 spill-marker paths, harvested out of the discarded messages.
tool_spillnow exports its three marker fences and builds the marker from them, so reader and writer cannot drift, and the test drives the real writer rather than a copy of its format.Only the current compaction's handles are listed, deliberately: re-harvesting the previous note would compound the list without bound.
Follow-up owed at integration: once #440 (PR #451) lands, this harvest should read #440's handle contract, which is the richer and now-default source, rather than only #409's spill markers. The seam is one function in
compact_note.zig.Verification
zig build testexit 0, 1056 tests, +6 from this branch's 1050 baseline. Reachability proven by break-and-revert, not just by count: flipping one assertion produced1055 pass, 1 fail (1056 total)namingcompact_note.zig:309.scripts/eval-tier1.shgreen.src/agent_compact.zigends at 564 lines, net zero against its branch point — the now-unusedprompts/compact_instructionaliases were dropped to pay for the new import, andhandoffMessagegained a parameter at equal line count. That file is under heavy concurrent pressure from #445, #440 and #391, so this branch adds none.Covered by test: the durable note appears only on the handoff; the transcript line vanishes when the live transcript belongs to another session; a duplicate-edited file is named once and the bash caveat is present; a real spill's artifact path is extracted (not the marker prose) and a spill-free history gets no handle line; a subagent gets nothing even with a ledger and a marker hung on it, and neither does a
/reviewturn; with nothing durable the handoff is byte-identical to base; #379's empty-summary escalation still fires on the second unusable summary.Not covered
compact()needs a live request, so no unit test drives its two call sites. That the request usessummaryRequestand the handoff receiveslive_messages.items[0..recent_start]is verified by reading and compilation only — the same gapcompactPreludewas extracted to close forpinChildTask, not extended here. Multi-compaction behaviour and theemergencyTrimpath are also uncovered end to end; the latter still carries only #318's standing state, since no summary is produced there for this note to ride.