Split ConversationHistory into Conversation and ConversationStore#183
Split ConversationHistory into Conversation and ConversationStore#183
Conversation
…ionStore (I/O) ConversationHistory was doing two jobs: managing message state and persisting it to disk. This split pulls them apart. Conversation holds the items array and owns the business logic — role-alternation merge, compaction-triggered clear, id tagging/remove. No imports from node:fs. ConversationStore wraps Conversation and handles the JSONL file: reads on construction (applying trimToLastCompaction), writes after every push/remove. When constructed without a historyFile it behaves as an in-memory store with no I/O side effects. AgentRun and AnthropicAgent switch from ConversationHistory to ConversationStore — same public interface, so call sites are unchanged. Conversation.spec.ts replaces ConversationHistory.spec.ts. Tests are rewritten to the project convention: one expect per it, const expected / const actual naming, and a load() section covering the raw-initialization path that ConversationStore uses during construction.
9f6cf27 to
70c6922
Compare
bananabot9000
left a comment
There was a problem hiding this comment.
Clean split. Conversation has zero I/O -- node:fs import gone, no #save() calls remain. ConversationStore wraps and delegates correctly: both push() and remove() call #save() after delegation.
Callers (AgentRun, AnthropicAgent) are straight type swaps -- 3+3 lines each, external API unchanged (historyFile option still works).
Tests rewritten to project convention (one expect per it, expected/actual). The load() section correctly verifies that raw initialization bypasses merge logic (two consecutive user messages stay separate after load), which is the key behavioral distinction between the two classes.
One minor note: ConversationStore.remove() calls #save() unconditionally, including when remove() returns false (no-op write). Harmless -- the file content is identical -- but could short-circuit with if (result) this.#save() if you wanted to avoid the disk write on miss. Not blocking.
Exported helpers (HistoryItem, hasCompactionBlock, trimToLastCompaction) are the minimum needed for ConversationStore to do its job. No leaky abstractions.
LGTM. Step 1a done. 🍌
What
Step 1a of the architecture refactor plan (
.claude/plans/architecture-refactor.md).Splits
ConversationHistoryinto two files:Conversation.ts— pure in-memory data. Owns the business rules: role-alternation merge, compaction-triggered clear, id tagging/remove. Nonode:fs.ConversationStore.ts— file-backed wrapper. Reads the JSONL file on construction (applyingtrimToLastCompaction), delegates toConversationfor mutations, and persists after everypush/remove. No-file construction works as a plain in-memory store.AgentRunandAnthropicAgentswapConversationHistory→ConversationStore. Public interface is identical so call sites are unchanged.Tests
ConversationHistory.spec.ts→Conversation.spec.ts. Rewritten to project convention:expectperitconst expected = …; const actual = …; expect(actual).toBe(expected)load()section covering the raw-initialization path thatConversationStoreuses21 tests, all green.
Why
AgentRungetsConversationStore(not bareConversation)AgentRun.push()calls must trigger disk saves. DecouplingAgentRunfrom the persistence layer entirely is a future step; passing it aConversationStoreachieves the structural separation (pure data vs I/O) without introducing a callback or hook mechanism here.