Skip to content

feat: context compaction — summarize-and-truncate per the merged design#74

Merged
andybons merged 9 commits into
mainfrom
feat/compaction
Jul 11, 2026
Merged

feat: context compaction — summarize-and-truncate per the merged design#74
andybons merged 9 commits into
mainfrom
feat/compaction

Conversation

@andybons

Copy link
Copy Markdown
Contributor

Implements docs/design/context-compaction.md (merged as the build spec in #64-era work):

  • Engine core: summarize-and-truncate — the session's own provider produces the summary, which flows through the normal EventMessage path before the history.compacted event; keep_turns floor of 1; churn-guard hysteresis so compaction cannot thrash; the double-RoleUser splice preserved and tested (the transcoder merges adjacent same-role messages — the splice shape is load-bearing).
  • Auto-trigger: fires from LastUsage against the model context threshold, including mid-goal-loop with zero changes to goal.go (tested).
  • API: POST /session/{id}/compact (409 when busy), compaction observability on GET /session, openapi updated.
  • Config: context_window_tokens, compaction_threshold, compaction_keep_turns.
  • Durability: compact journal record carries top-level usage, replayed cumulative-only; an e2e drives automatic compaction across a real serve restart. The regression class is the real incident: a goal session died unrecoverable at 205,102 tokens > 200,000 — that shape now compacts and continues.

go test -race, go vet, GOOS=windows go build all green.

andybons added 6 commits July 11, 2026 13:10
Implements docs/design/context-compaction.md's engine-layer mechanism:

- Session.Compact(ctx, CompactOptions) folds a contiguous prefix of whole
  turns into one synthetic summary message, keeping the most recent
  keep_turns (default 2, hard floor 1) verbatim.
- The summary is produced via the session's own provider (or an override),
  banner-marked as synthesized, and spliced in via spliceCompact — shared
  by the live path and LoadSession's replay so they can never drift apart.
- Automatic trigger (maybeAutoCompact) runs at the top of every Prompt call,
  before the incoming user message is appended, gated on
  Config.ContextWindowTokens (opt-in) and Config.CompactionThreshold
  (default 0.8), with churn-guard hysteresis so a hot kept-region tool
  result cannot cause thrashing.
- Usage accounting: the summarization call's spend lands in cumulative
  Usage() only, never LastUsage() — live or replayed.
- New recCompact journal record carries the summary inline plus the
  summarization call's own usage, replayed cumulative-only by LoadSession.
- EventHistoryCompacted/EventCompactionFailed engine events; the summary
  flows through the ordinary EventMessage path first.
- Session.CompactionCount()/LastCompactedAt() for durable observability.

Regression test TestIncidentRecoverableByCompaction encodes the production
incident (a goal session died at 205102 tokens > 200000 maximum,
unrecoverable) and is red-verified against the pre-fix code.

Also verifies the double-RoleUser splice shape (compacted summary followed
by the first kept turn's prompt) merges correctly through the Anthropic
transcoder's existing same-role alternation handling.

No server-layer wiring yet (POST /session/{id}/compact, GET /session
fields, openapi.yaml) — follow-up commit.
Wires engine/compact.go's Session.Compact into the HTTP surface, per
docs/design/context-compaction.md §1/§4:

- POST /session/{id}/compact: run-token auth, claims the session's single
  run slot exactly like prompt_async/goal (409 busy, 503 draining, 404
  unknown), runs synchronously, optional {keep_turns, model} body.
  keep_turns has a hard floor of 1 — 0 or negative is 400, never clamped.
  Returns {turns_folded, first_id, last_id, summary}; turns_folded: 0 is a
  200, not an error.
- Publish now routes engine.EventHistoryCompacted to a durable
  history.compacted journal record (publishHistoryCompacted) and
  engine.EventCompactionFailed to a live-only compaction.failed event.
  Because Session.Compact emits the summary's EventMessage before
  EventHistoryCompacted, and OnEvent is synchronous, the journal/SSE order
  is always: summary message, then history.compacted.
- Session.compaction_count/last_compacted_at on GET /session (and
  buildSession), sourced from the engine (durable across restart).
- openapi.yaml: the new path, CompactRequest/CompactResponse schemas,
  Session.compaction_count/last_compacted_at, and the two new Event types
  with their compact_first_id/compact_last_id/compact_turns_folded/
  compact_summary_id fields.

Tests cover the happy path, the keep_turns floor, the no-op/200 case, 409
on a busy session, 404/401, and the message-before-history.compacted event
ordering.
…ion_threshold, compaction_keep_turns)

Wires docs/design/context-compaction.md's opt-in engine.Config fields
through the CLI/serve config layer, following the same non-zero-overrides
merge rule as goal_evaluator_model: a project .harness.json value wins
over the user config when set, otherwise the user value (or the engine's
own zero-fills-a-default) applies. Both harness run and harness serve wire
these into every session's engine.Config.
TestAutoCompactionAcrossRestart is the e2e keystone docs/design/context-
compaction.md calls for: a real `harness serve` binary, driven by a
scripted fake Anthropic provider (completeTurnWithUsage, scriptedUsageAnthropic),
is pushed past a small test-configured context_window_tokens/
compaction_keep_turns threshold. It asserts, end to end over HTTP:

- automatic compaction fires before the triggering turn's own request
  (exactly one extra summarization call, in the right position in the
  request sequence)
- the session keeps working afterward, with a visibly smaller
  last_input_tokens than the value that triggered compaction
- GET /session's compaction_count/last_compacted_at flip
- the durable journal shows the summary's message event strictly before
  history.compacted, and history.compacted names the fold
- after a SIGKILL + fresh process on the same session directory, GET
  /session and GET /session/{id}/message replay the compacted, trimmed
  history exactly, the event journal stays contiguous, and a further
  prompt succeeds — a post-compaction session restarts exactly as cleanly
  as an ordinary one.

Also extracts completeTurnWithUsage from completeTurn (explicit input/
output token counts) and adds the compaction fields to the shared apiEvent
test type.
…hanges

TestPursueGoalAutoCompactsMidLoop proves docs/design/context-compaction.md
§1's central claim end to end: PursueGoal drives every worker turn through
the ordinary Prompt path, so maybeAutoCompact's check at the top of Prompt
fires mid-goal-loop with no separate scheduler and no goal.go wiring at
all. goalCompactProvider extends goal_test.go's worker/evaluator-by-tools-
presence classification with a third case (compaction's tool-less
summarization call, identified by its unique system prompt) so a single
scripted provider can serve all three request shapes a goal session now
makes.
CompactRequest.model now $ref's the shared ModelRef schema (a plain
"provider/model" string) instead of a bespoke string field, consistent
with PromptRequest.model.
@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 5m 37s —— View job


Review: context compaction (PR #74)

  • Gather context (diff vs origin/main, AGENTS.md priorities)
  • Review engine core (compact.go, engine.go, store.go)
  • Review server surface (handlers.go, journal.go, openapi)
  • Review config + auto-trigger + durability
  • Check concurrency, transcoding invariants, usage accounting

This is a careful, well-documented PR — the durability model (inline summary in the recCompact record, shared spliceCompact between live and replay paths, cumulative-only usage on replay), the run-slot claim on the explicit path, and the ID-keyed syncMessages journaling all hold up. The double-RoleUser splice and merge behavior are correctly tested. One finding stands out as blocking.


🔴 Auto-trigger undercounts context when prompt caching is active (engine/compact.go:320)

Left inline. The threshold check reads only lastUsage.InputTokens:

over := float64(lastUsage.InputTokens) >= threshold*float64(windowTokens)

For Anthropic, Usage.InputTokens comes from input_tokens (provider/anthropic/anthropic.go:291), which excludes cache_read_input_tokens and cache_creation_input_tokens. Harness injects cache_control by default (provider/anthropic/transcode.go), so on a large warm session the bulk of the prompt is in CacheReadTokens and InputTokens stays small.

Result: a ~200k cached context reports a small InputTokens, 0.8*window is never crossed, and automatic compaction never fires — precisely the 205,102-token regression this PR targets. All the tests pass only because every fixture (engine/compact_test.go, e2e/compaction_test.go) puts the whole prompt in InputTokens with zero cache tokens, so the production shape is never exercised.

Fix: sum the prompt components — InputTokens + CacheReadTokens + CacheWriteTokens — and add a Usage{InputTokens: 100, CacheReadTokens: 900} case to red-verify. Fix this →


🟡 Minor observations

  • CompactionModel has no config-file knob. engine.Config.CompactionModel exists and its doc comment references "Config.CompactionModel", but config.Config (config/config.go) has no corresponding field and cmd/harness/main.go doesn't wire one — so it's only reachable via the per-call POST /compact model body. If a config-file default was intended, add compaction_model alongside the other three knobs; if not, the doc comment's "Config.CompactionModel" phrasing is slightly misleading.

  • CompactionThreshold isn't range-validated. A config value > 1.0 silently means "never auto-compact" and a value in (0,?] is taken verbatim; only <= 0 gets the 0.8 default. Not a correctness bug, but a fat-fingered compaction_threshold: 80 (meaning 80%) would disable the feature silently. Consider clamping/warning, consistent with keep_turns's explicit 400 on bad input.

Nits (non-blocking)

  • runCompactionSummary builds the request from the folded range directly and relies on the transcoder to synthesize results for any orphaned tool call at the fold's trailing edge. The comment claims "a whole-turns range never has a dangling tool call at either edge" — true for completed turns, but a turn interrupted mid-tool-cycle (a new RoleUser arrived before the RoleTool result) would leave a dangling tool_use. The synthesize-at-build path (message/message.go:504) should cover it, but it's worth a test if that path isn't already exercised for this request shape.

Otherwise: concurrency is sound (explicit path claims the run slot; auto path runs inside Prompt), the churn-guard hysteresis logic is correct, and replay/journal ordering (summary message before history.compacted) is right.

Comment thread engine/compact.go Outdated
The threshold check read lastUsage.InputTokens alone, but harness
injects cache_control by default: on a warm session the Anthropic
adapter reports the bulk of the prompt in CacheReadTokens (new prefix
growth in CacheWriteTokens) and InputTokens is only the uncached tail —
so automatic compaction never fired in exactly the long-cached-session
shape it exists for. Sum all three input components. Red-first test
encodes the original incident under caching: a 205,102-token warm
prompt (3,102 input + 190,000 cache read + 12,000 cache write) now
triggers; it did not before.
@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 3m 22s —— View job


Review: context compaction (PR #74) — re-review after 3a7a0ad

  • Gather context (diff vs base, AGENTS.md priorities)
  • Review engine core (compact.go, engine.go, store.go)
  • Review server surface (handlers.go, journal.go, openapi)
  • Review config + auto-trigger + durability
  • Verify the cache-token fix and re-check for new issues (go build clean)

The blocking finding from the prior round — auto-trigger undercounting warm-cached context — is correctly fixed in 3a7a0ad. engine/compact.go:327 now sums all three prompt components, and engine/compact_cached_test.go red-verifies it with the exact incident shape (3,102 + 190,000 + 12,000 = 205,102 against a 200k window). No new issues surfaced. The durability model holds up on a re-read: shared spliceCompact between live and replay paths, inline summary in recCompact (no dangling reference across a torn write), cumulative-only usage on replay so the small summarization call can't mask the pressure that triggered compaction, and ID-keyed syncMessages (line 432) which correctly tolerates history shrinking when the fold removes messages.

Verified sound:

  • Concurrency — the explicit path claims the single run slot (handleCompactclaimForPrompt, balanced wg.Done), the auto path runs inside Prompt before the user message is appended, so compaction never races a turn. Compact runs the network call unlocked and re-acquires s.mu only for the splice+persist critical section; a concurrent reader sees pre- or post-state, never half-spliced.
  • Churn guardcompactHysteresis set on fold, cleared when summed usage next dips below threshold; re-fire is correctly suppressed while pressure lives in the kept region.
  • Journal ordering — summary EventMessage is emitted before EventHistoryCompacted, so a tailer never holds a dangling summary id.

🟡 Minor (non-blocking, carried from prior round — still open)

Nit (non-blocking)

  • runCompactionSummary folds a whole-turns range and relies on the transcoder to synthesize a result for any dangling tool_use at the trailing edge. This is safe for completed turns, but a turn interrupted mid-tool-cycle (a new RoleUser arrived before the RoleTool result) would leave a dangling tool_use at foldEnd. The synthesize-at-build path (message/message.go) should cover it — worth a targeted test for this request shape if one doesn't already exist.

Overall this is a careful, well-tested PR and the core is correct. The two minor items are quality-of-config polish, not correctness blockers.

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 3m 20s —— View job


Review: context compaction (PR #74) — re-review after 8850a19

  • Gather context (diff vs origin/main, AGENTS.md priorities)
  • Review engine core, server surface, config, durability
  • Verify the 3a7a0ad cache-token fix across all provider adapters
  • Post review

The only code change since the last review round is 8850a19 (gofmt) — the substantive 3a7a0ad cache-token fix is unchanged. Re-reviewing the compaction core against all provider adapters (not just Anthropic) surfaced one new correctness issue in exactly that fix.


🔴 The cache-token fix double-counts on OpenAI (engine/compact.go:327)

Left inline. 3a7a0ad correctly fixed Anthropic but introduced a mirror-image bug for the native OpenAI (Responses API) adapter, because provider.Usage.InputTokens carries opposite semantics across the two:

  • Anthropic (provider/anthropic/anthropic.go:291-293): input_tokens excludes cache — the three components (InputTokens, CacheReadTokens, CacheWriteTokens) are disjoint, so summing them = true prompt size. ✅
  • OpenAI (provider/openai/openai.go:289-291): input_tokens is the total prompt and cached_tokens is a subset of it. The adapter test proves it: input_tokens:100 + cached_tokens:25InputTokens=100, CacheReadTokens=25 (the 25 is inside the 100). Summing yields 125 for a 100-token prompt.

OpenAI auto-caches every prompt ≥1024 tokens, so a warm session over-counts by nearly and auto-compaction fires ~twice as eagerly as intended. (openaicompat is unaffected — it never sets CacheReadTokens.)

Root cause: Usage.InputTokens means "uncached input" for Anthropic but "total input" for OpenAI — a latent trap for every Usage consumer, not just this line. Cleanest fix: normalize the OpenAI adapter (InputTokens = input_tokens - cached_tokens) so the three components are disjoint everywhere; this line's sum then holds for all providers and cumulative Usage() reporting becomes consistent too. Red-verify with an OpenAI-shaped Usage{InputTokens: 100, CacheReadTokens: 90} case.


Verified sound (unchanged from prior rounds, re-confirmed)

  • Durability — inline summary in recCompact (no dangling reference across a torn write), shared spliceCompact between live and replay paths, cumulative-only usage on replay (store.go:432) so the small summarization call can't mask the pressure that triggered compaction, ID-keyed syncMessages tolerant of history shrinking.
  • Concurrency — explicit path claims the single run slot (handleCompactclaimForPrompt, balanced wg.Done); auto path runs inside Prompt before the user message is appended; the network summarization runs unlocked, splice+persist re-acquires s.mu for one critical section — a reader sees pre- or post-state, never half-spliced.
  • Churn guardcompactHysteresis set on fold, cleared when summed usage next dips below threshold; the one-turn lag (usage updates only after the real turn) resolves correctly.
  • Journal ordering — summary EventMessage emitted before EventHistoryCompacted, so a tailer never holds a dangling summary id.

🟡 Minor (still open from prior rounds — non-blocking)

  • CompactionModel has no config-file knob. engine.Config.CompactionModel and CompactOptions.Model's doc reference it, but config.Config has no compaction_model field, so it's reachable only via the per-call POST /compact body and is always zero from config. Add it alongside the other three, or soften the doc comment.
  • CompactionThreshold isn't range-validated. Only <= 0 gets the 0.8 default; a value > 1.0 silently means "never auto-compact" (a fat-fingered compaction_threshold: 80 disables the feature silently) — inconsistent with keep_turns's explicit floor. Consider clamping to (0,1].

Overall this is a careful, well-tested PR. The one blocking item is that the same warm-cache class the previous round fixed for Anthropic is now mis-accounted (in the other direction) for OpenAI — worth closing before merge so the trigger is correct on both provider families.

Comment thread engine/compact.go
provider.Usage had provider-inconsistent semantics: Anthropic reports
input_tokens EXCLUSIVE of cache reads/writes (disjoint components) while
the OpenAI Responses adapter passed through input_tokens INCLUSIVE of
input_tokens_details.cached_tokens — so summing the components (as the
auto-compaction threshold now does) double-counted the cached portion on
OpenAI and fired early. Document the disjoint contract on Usage and make
the OpenAI adapter subtract: InputTokens is the uncached remainder, the
sum reconstructs the wire total. openaicompat already conforms (total in
InputTokens, zero cache fields). Adapter tests updated to the contract:
100 wire total with 25 cached now reports 75+25, 7 with 2 reports 5+2.
@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

Claude finished @andybons's task in 2m 30s —— View job


Review: context compaction (PR #74) — re-review after 84ca426

  • Gather context (diff vs base, AGENTS.md priorities)
  • Verify the 84ca426 disjoint-Usage fix across all provider adapters
  • Re-check engine core, server surface, config, durability
  • Post review

The blocking finding from the last round — OpenAI double-counting cached tokens in the auto-compaction trigger — is correctly resolved in 84ca426, and this time at the right layer: the fix normalizes the adapter boundary rather than special-casing the trigger.

I re-verified the disjoint-Usage contract holds across all three present adapters (there is no bedrock/gemini adapter in the tree yet, despite AGENTS.md's mention):

Adapter InputTokens source Cache fields Sum = true prompt?
Anthropic (anthropic.go:291-293) input_tokens (already cache-exclusive) cache_creation / cache_read separate
OpenAI (openai.go:296-300) input_tokens - cached_tokens (now subtracted, floored at 0) CacheReadTokens = cached_tokens
openaicompat (openaicompat.go:409) prompt_tokens (cache-inclusive total) none set (0) ✅ (sum = total)

engine/compact.go:327's InputTokens + CacheReadTokens + CacheWriteTokens sum is now correct on every adapter, and the adapter tests (stream_test.go:124 75+25, :219 5+2) encode the contract red-first. The negative guard (if uncached < 0) is a sensible defense against a malformed wire total.

Verified sound (re-confirmed this round)

  • Durability — inline summary in recCompact (no dangling reference across a torn write), shared spliceCompact between live (compact.go:278) and replay (store.go:407) paths, cumulative-only usage on replay so the small summarization call can't mask trigger pressure, compactCount/lastCompactedAt restored on load.
  • Concurrency — explicit path claims the single run slot (handleCompactclaimForPrompt, balanced wg.Done); auto path runs inside Prompt before the user message is appended (engine.go:642); the network summarization runs unlocked and re-acquires s.mu for one splice+persist critical section — a reader sees pre- or post-state, never half-spliced.
  • Churn guardcompactHysteresis set on fold, cleared when summed usage next dips below threshold; the one-turn lag resolves correctly.
  • Journal ordering — summary EventMessage emitted before EventHistoryCompacted (compact.go:199-206), so a tailer never holds a dangling summary id.

🟢 Trivial (non-blocking)

  • Duplicate doc line in provider/provider.go:50-51. 84ca426 added the new contract comment but left the original one-liner above it, so the type now has two lead sentences:
    // Usage is token accounting for one request.
    // Usage reports token accounting for one provider call.
    //
    // CONTRACT: ...
    Drop line 50. Fix this →

🟡 Minor (carried from prior rounds — still open, non-blocking)

  • CompactionModel has no config-file knob. 1a860df wired context_window_tokens/compaction_threshold/compaction_keep_turns into config.Config and cmd/harness/main.go, but engine.Config.CompactionModel still has no compaction_model counterpart — it's reachable only via the per-call POST /compact body and is always zero from config. Add it alongside the other three, or note in the doc comment that it's request-only.
  • CompactionThreshold isn't range-validated. Only <= 0 gets the 0.8 default; a value > 1.0 silently means "never auto-compact" — a fat-fingered compaction_threshold: 80 (meaning 80%) disables the feature silently, inconsistent with keep_turns's explicit floor. Consider clamping to (0,1] or warning.

Overall: the core is correct and well-tested, the durability and concurrency models hold, and the one blocking item from the prior round is closed at the contract level. The remaining items are config-polish, not correctness blockers — I'd merge on these once the two minor knobs are either addressed or explicitly deferred.

@andybons andybons merged commit c6caeb3 into main Jul 11, 2026
2 checks passed
@andybons andybons deleted the feat/compaction branch July 11, 2026 14:19
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.

1 participant