refactor: append-only transcript persistence + fsync batching#19
Merged
Conversation
Switches Store.Save from FileStore.Replace (full-file rewrite per turn)
to per-event append. The old path is removed.
- Store.Save calls Start (idempotent) + AppendMessage per node +
one PatchState. The Replace function, ReplaceCommand,
recordsForTranscript, and messageExistsLocked all delete.
- AppendMessage deduped via in-memory persistedIDs cache, populated
lazily by scanning the file once. After that, O(1) dedup instead
of O(N) per call. Delete clears the cache entry.
- StateOpsFor exposes the same patch-list construction the old
Replace path used. PatchTag/PatchMode/PatchWorktree exported so
Save can build the op list outside the package.
- appendRecord gains a sync bool parameter:
sync=true: message.appended, session.started, session.compacted
sync=false: state.patched
Typical turn drops from ~5 fsync calls to 1.
Test changes: TestFileStoreReplace is gone; TestFileStoreAppendMessageIsIdempotent
exercises the new dedup path including a fresh FileStore re-scanning the file.
2 tasks
Two regressions exposed by switching Save from full-rewrite (Replace) to
append-only.
P1: stable message IDs
ConvertToEntries generated a fresh UUID for every message on every call.
Combined with the new dedup-by-message-ID path, this meant each save
treated the entire history as new and re-appended it — transcript files
grew quadratically and resume saw a tangled chain.
Fix: ChatMessage gains an ID field, assigned once when a message enters
conv.Messages via Append. ConvertToEntries prefers msg.ID and only
falls back to generateShortID() when absent (for paths that bypass
conv.Append).
Test: Test_ConvertToEntries_preservesChatMessageID,
TestSession_SaveTwice_NoDuplication
P2: unconditional state patches
StateOpsFor skipped PatchTasks when state.Tasks was empty and PatchWorktree
when state.Worktree was nil. Under Replace this was fine (the rewrite
removed any prior patches); under append-only it leaves prior values
in place — clearing tasks or exiting a worktree never reaches the
projected state on reload.
Fix: emit both ops unconditionally. PatchTasks(empty) serializes to "[]"
and PatchWorktree(nil) to "null"; the projector already handles both.
Test: TestSession_SaveClearedTasks_ClearsOnReload
Member
Author
|
Both issues fixed in c001ffe (force-pushed; stack rebased). P1 — stable IDs at the source:
P2 — unconditional state patches:
All 5 stack branches rebuilt + verified: build clean, tests green on each branch independently. |
2 tasks
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.
PR 1 of 5.
Replaces full-file rewrite (`FileStore.Replace`) with per-event append.
Behavior preserved — all existing resume / fork / JSONL-integrity tests pass.
Test
🤖 Generated with Claude Code