Skip to content

Persist the room beside the graph, and write off the actor (#307) - #308

Merged
scgopi merged 3 commits into
mainfrom
fix/307-persist-off-actor
Sep 7, 2026
Merged

Persist the room beside the graph, and write off the actor (#307)#308
scgopi merged 3 commits into
mainfrom
fix/307-persist-off-actor

Conversation

@scgopi

@scgopi scgopi commented Sep 6, 2026

Copy link
Copy Markdown
Owner

The defect

GraphStore.broadcast() called persistence.saveGraph(graph) synchronously, on the actor, and saveGraph serialised the whole graph — 323,603 bytes on the graph that filed #307, 271,256 of them (84%) mail bodies that had not changed. So every memo, state tick and cursor move held the GraphStore actor across a full encode-and-write: a node memo measured at 0.03–2.13 s against a 0.003 s socket round trip, with the variance coming from the filesystem. It is #288's shape one layer over, and the payload #293 took out of the wire snapshot was still in the file.

The change — both halves the issue names, each sufficient on its own

The room lives beside the graph, not in it. ProjectPersistence.saveGraph writes the graph without its posts to <name>.json (rewritten on every change) and the room to <name>.mailroom.json, rewritten only when the room's MailroomDigest changed since the last write — so a memo rewrites ~52 KB, not 323 KB, and a post rewrites the room once. loadGraph reads both, the sidecar winning; a graph file saved before the split still carries its posts inline and loads exactly as it did. deleteGraph removes both. The same argument #293 made for the wire, applied to the file.

The write leaves the actor. ProjectRegistry hands each snapshot to a GraphWriter — one serial queue, latest snapshot per project wins — and the store's onGraphChanged returns at once; a burst of memos collapses to one write. GraphWriter.flush() waits for everything queued: the daemon calls it on both exit paths (SIGTERM/SIGINT source, the binary-replaced timer) so a change applied a moment before a restart is on disk, and ProjectRegistry(persistsSynchronously: true) flushes on every save for the tests that read the file straight after a command.

Downgrade note: a daemon older than this reads <name>.json only, so on a downgrade the room would appear empty until the next upgrade — the posts are untouched in the sidecar. Same class as #298, one direction only, and named here so it is not a surprise.

Verification

Gate: full Xcode suite (gate on this head in flight; numbers and exit codes in a comment when it completes), swiftlint 0 errors, swift-format clean, graphcoded and graphcode-cli schemes build; SwiftPM swift build and .build/debug/graphcode pass.

MailroomPersistenceTests: the graph file carries neither a mailroom key nor a post body (a nonce planted in a post never appears in it) while the sidecar does; a change that leaves the room alone leaves the sidecar's mtime alone; loading merges both; deletion removes both; a pre-split graph file with inline posts loads them; a burst of fifty saves lands the newest once after flush. ProjectRegistryTests and DuplicateProjectPathTests run with synchronous persistence and are otherwise unchanged.

Closes #307.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DeGL2CxuGmq16RSZpJYm2N

@scgopi

scgopi commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

Gate on e51b80e, private DerivedData: xcodebuild test exit 0 — 1646 tests / 171 suites / 0 failures; graphcode-cli build exit 0; graphcoded build exit 0; swiftlint 0 errors; swift-format clean; Linux green on this head.

The merged-result failures were real and are fixed by read-through: GraphWriter.load(path:) returns the snapshot still queued for a project before falling back to the file, and every registry read — the delete of a closed project included, which reads the graph to find the sessions it must end — goes through it. The two tests stood in for a daemon restart inside one process; they now flush the first daemon's writer the way the daemon does on its way out, and the one that reads the file after every command asks for synchronous persistence. New test: a load returns the queued three-loop graph while the file still holds one.

scgopi and others added 3 commits September 6, 2026 17:21
Every graph change re-serialised the whole graph — 323 KB on the graph
that filed the issue, 271 KB of it mail bodies that had not changed — and
wrote it to disk synchronously inside GraphStore.broadcast(), holding the
actor for as long as the disk took: a memo measured at 0.03–2.13 s
against a 0.003 s socket round trip. #288's shape, one layer over, and
the payload #293 removed from the wire was still in the file.

Two fixes, each sufficient on its own. The room lives in its own file
(<name>.mailroom.json) written only when its digest changed; the graph
file, rewritten on every change, carries no post. A graph saved before
the split still loads its inline room. And saves are handed to a
GraphWriter — one serial queue, latest snapshot per project wins, so a
burst of memos is one write — and the actor returns at once; the daemon
flushes it on both exit paths, and tests that read the file straight
after a command ask for synchronous persistence.

Closes #307.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DeGL2CxuGmq16RSZpJYm2N
…first

A load returns the snapshot still queued for a project before falling
back to the file, so a delete of a closed project — which reads the graph
to find the sessions it must end — cannot see a stale one and leave loops
running. Two tests that stood in for a daemon restart inside one process
flush the first daemon's writer, as the daemon does on its way out.
@scgopi
scgopi force-pushed the fix/307-persist-off-actor branch from e51b80e to 390cdf7 Compare September 7, 2026 00:22
@scgopi
scgopi merged commit 540f428 into main Sep 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Every graph change synchronously writes 271 KB of mail bodies to disk, holding the GraphStore actor

1 participant