Skip to content

Split Conversation storage from API view#231

Merged
shellicar merged 4 commits intomainfrom
feature/sdk-refactor
Apr 9, 2026
Merged

Split Conversation storage from API view#231
shellicar merged 4 commits intomainfrom
feature/sdk-refactor

Conversation

@shellicar
Copy link
Copy Markdown
Owner

@shellicar shellicar commented Apr 9, 2026

Why

Conversation.push() used to clear the entire item array when it saw a compaction block, and ConversationStore did the same on disk load via trimToLastCompaction. That was destructive on two axes:

  1. No rollback. Once compaction fired, pre-compaction messages were gone. If the compaction itself errored, the session died with it.
  2. No visibility. The CLI, audit log, and any future TUI replay could never see history older than the last compaction.

RequestBuilder also carried a stack of defensive spread-copies ([...messages], { ...block, cache_control }) whose only job was to keep send-time cache_control and system-reminder blocks from leaking back into stored state, because the same array served both purposes.

What

Storage and API view are now separate concerns.

  • Conversation stores the full message history forever. Compaction messages are appended like any other message; no clearing.
  • New Conversation.cloneForRequest() returns a deep clone (structuredClone) of the slice from the last compaction forward. The returned array is caller-owned and may be mutated freely.
  • RequestBuilder's helpers (addCacheControlToLastBlock, cacheLastUserMessage, systemReminder injection) become void functions that mutate in place. The defensive spread-copy dance is gone.
  • AgentRun swaps this.#history.messages for this.#history.cloneForRequest() at the send site.
  • ConversationStore adds a thin cloneForRequest() passthrough and stops trimming on load.

Payoffs

  • Compaction rollback now works for free: remove(id) the compaction block and the next cloneForRequest() slices further back.
  • Full history is visible to the CLI, audit log, and future TUI replay.
  • On-disk format stays pristine (no cache_control leakage).

Scope

This is PR 1 of a larger SDK reshape. Deferred to a later PR: extracting AnthropicClient from AnthropicAgent, the configure(partial) / run() split, removing ConversationStore / historyFile from the SDK so persistence lives fully in the CLI, and collapsing runAgent.ts. Full design notes in .claude/sessions/2026-04-09.md under "Session 2026-04-09 (SDK refactor planning)".

Tests

  • 4 old compaction-clear tests rewritten to assert retention.
  • 6 new cloneForRequest tests cover the empty case, no-compaction case, post-compaction slice, deep-clone guarantee (mutating the clone leaves the source untouched at array, message, and content-block levels).
  • 1 does not mutate input test on RequestBuilder removed — it encoded the old contract.
  • 78/78 claude-sdk tests green.

Related: #232

This file was checked in by accident. It is a per-user local config
and lives in the global gitignore now.
Compaction used to destroy pre-compaction messages on push(): the store
clobbered history in place, and ConversationStore.trimToLastCompaction
did the same on disk load. Once compaction fired there was no way back;
if the compaction itself errored, the session died with it.

This split fixes that. Conversation now stores the full history forever.
Compaction messages are appended like any other message. For API calls
the caller uses cloneForRequest(), which deep-clones the slice from the
last compaction forward. The clone is caller-owned and RequestBuilder
mutates it directly: no more defensive spread-copy dance to keep the
send-time cache_control and system-reminder blocks out of stored state.

Three follow-ons fall out for free:
  - compaction rollback works (remove the compaction block, next
    cloneForRequest slices further back)
  - the full history is visible to TUI replay, audit, and inspection
  - on-disk format stays pristine (no cache_control leakage)

AgentRun switches from this.#history.messages to cloneForRequest() at
the send site. ConversationStore adds a thin passthrough and stops
trimming on load. The addCacheControlToLastBlock and cacheLastUserMessage
helpers become void functions that mutate in place.

Test updates reflect the new semantics: 4 old compaction-clear tests
rewritten to expect retention; 6 new cloneForRequest tests cover the
deep-clone guarantee, the slice boundary, and the empty case. The
'does not mutate input' test on RequestBuilder was contract-based on
the old shape and is removed.
@shellicar shellicar added this to the 1.0 milestone Apr 9, 2026
@shellicar shellicar added bug Something isn't working pkg: claude-sdk The agent SDK labels Apr 9, 2026
@shellicar shellicar self-assigned this Apr 9, 2026
@shellicar shellicar requested a review from bananabot9000 April 9, 2026 11:50
@shellicar shellicar enabled auto-merge (squash) April 9, 2026 11:50
Copy link
Copy Markdown
Collaborator

@bananabot9000 bananabot9000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly the right refactor. Storage and API view separated cleanly. The spread-copy dance is dead, structuredClone handles deep isolation, cloneForRequest() is the single point where "what the API sees" diverges from "what we store."

Tests are thorough — deep-clone proven at array, message, and content-block levels. Compaction-clear tests correctly rewritten as retention tests.

One minor observation: trimToLastCompaction does an O(n) reverse scan which is fine. The structuredClone only runs on the post-compaction slice, so even with large pre-compaction histories the clone cost stays bounded by the active conversation size.

Clean, narrow, well-scoped. The session log's design discussion is excellent context for PR 2.

🍌 Approved.

@shellicar shellicar merged commit 31de154 into main Apr 9, 2026
4 checks passed
@shellicar shellicar deleted the feature/sdk-refactor branch April 9, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working pkg: claude-sdk The agent SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants