feat(compression): in-place compaction option, single session id (#38763) [1/2] - #49739
Merged
Conversation
…id (#38763) Context compression today rewrites the message list AND rotates the session id — it ends the session, forks a parent_session_id child, and renumbers the title (name -> name #2). That moving identity key is the root cause of a whole bug cluster: /goal lost (#33618), pending response lost at the split (#14238), orphan sessions (#33907), TUI sid desync (#36777), FTS search gaps + duplicate sidebar entries (#45117), null continuation cwd (#42228), and title-rename dead-ends (#48989). It also forced a large defensive apparatus (compression lock, contextvar/env/ logging triple-sync, orphan finalization, gateway SessionEntry re-propagation, tip projection) whose only job is surviving a mid-conversation id change. Add a compression.in_place config flag (default False during rollout). When True, compaction rewrites the transcript and rebuilds the system prompt but keeps the SAME session_id: no end_session, no child row, no title renumber, no contextvar/logging re-sync, no memory/context-engine session-switch. The conversation keeps one durable id for life, like Claude Code / Codex. Compaction is lossy by design — the pre-compaction transcript is summarized away, not archived. The rotation path is unchanged when the flag is off (moved verbatim into an else branch). Staged rollout: this PR ships the option behind a default-off flag for live validation; a follow-up flips the default and deletes the now-redundant rotation machinery, superseding the 14 open band-aid PRs in this area. - hermes_cli/config.py: add compression.in_place (default False), documented - agent/agent_init.py: resolve the flag -> agent.compression_in_place - agent/conversation_compression.py: branch compress_context() on the flag - tests/run_agent/test_in_place_compaction.py: in-place invariants + rotation regression guard + config default The pre-flush of current-turn messages (#47202) runs in BOTH modes, so no boundary data loss. Prompt-cache invariant preserved: the system-prompt rebuild is the same single sanctioned invalidation that already happens during compaction — no NEW invalidation. Message alternation preserved.
…dent end-to-end Review (Codex + 3-agent parallel) found the first cut of in-place mode was incomplete: it only updated the system prompt, so the persisted transcript stayed 'full history + summary' and the next turn/resume reloaded the full history and immediately re-compacted (a loop), and every downstream layer that keyed off session-id rotation silently no-op'd. The session_id was doing double duty as the 'compaction happened' signal. This wires the whole path so removing rotation is actually complete: Agent (agent/conversation_compression.py): - In-place now DURABLY replaces the transcript: replace_messages(session_id, compressed) on the same row (the canonical store the gateway reloads from), not just update_system_prompt. Resume reloads the compacted set; no loop. - Reset flush identity/cursor (_last_flushed_db_idx=0, _flushed_db_message_ids cleared) so next-turn appends diff against the compacted transcript. - Expose a rotation-independent signal: agent._last_compaction_in_place, and in_place=True on the session:compress event. - Fire the compaction-boundary hooks (context-engine on_session_start, memory manager on_session_switch, reason='compression') in BOTH modes — in-place passes the same id as parent so DAG/buffer state still checkpoints. Without this, memory/context plugins miss every in-place compaction. Gateway auto-compress (gateway/run.py): - Read agent._last_compaction_in_place; set history_offset=0 on rotation OR in-place (both return the compacted set, so slicing past the pre-compaction length would drop everything). Carry compacted_in_place in the result dict. - No extra rewrite needed: the agent shares the gateway's SessionDB, so its replace_messages already updated the canonical store load_transcript reads. Manual /compress (gateway/slash_commands.py): - The throwaway /compress agent has no _session_db, so rewrite_transcript is the durable write. Previously gated behind 'if rotated:' which treated 'id unchanged' as the #44794 data-loss failure case and SKIPPED the rewrite — making /compress a silent no-op in in-place mode. Now rewrites on rotated OR in_place; the data-loss guard still fires only for the genuine no-rotation-AND-not-in-place failure. Hygiene auto-compress already writes _compressed to the same id unconditionally (its agent has no _session_db, can't rotate) — correct for in-place, no change. Tests (tests/run_agent/test_in_place_compaction.py): - Assert the DURABLE transcript IS the compacted set after reload (get_messages_as_conversation == compacted), message_count==2, flush identity reset, and the rotation-independent signal set on in-place / unset on rotation. Rotation regression guard unchanged. Verified: 64 tests green across in-place + rotation/persistence/boundary/ concurrent/failure-sync/command/cli suites; E2E both modes (durable replace, gateway offset=0, rotation preserves old transcript); ruff clean. Still default-off.
Parallel 3-reviewer cleanup of the in-place compaction code. Findings applied:
- perf: in-place mode no longer pre-flushes current-turn messages. The flush
ran INSERTs that the immediately-following replace_messages(compressed)
DELETE+reinsert discarded -- pure wasted writes per compaction. The
current-turn tail survives via the compressor's compressed output
(protect_last_n), not the flush. Verified no data loss; rotation still
pre-flushes (its old session row is preserved, so the flush is real there).
- quality: hoist the two shared post-write steps (update_system_prompt +
_last_flushed_db_idx = 0) below the if/else -- they ran in both branches
against agent.session_id. Removes the easiest divergence bug.
- quality: compute the compaction-boundary locals (_old_sid, _is_boundary,
_boundary_parent) ONCE instead of recomputing locals().get('old_session_id')
and the "_old_sid or agent.session_id or ''" chain three times.
- quality: initialize compacted_in_place up front and assign
agent._last_compaction_in_place directly, dropping the fragile
locals().get('compacted_in_place') reflection.
- reuse: parse the in_place config flag with utils.is_truthy_value (the
project's canonical truthy coerce) instead of a hand-rolled
str().lower() in {...} (agent_init already imports from utils).
Dropped as false positives / out of scope: gateway getattr of agent internals
(established session_id pattern), dual result-dict carry (mirrors history_offset
etc.), stringly-typed "compression" (codebase-wide convention, no constant).
Behavior-preserving: 7 in-place tests (incl. 2 new flush-guard tests) + 26
rotation/boundary/persistence/command tests green; mutation check confirms the
durable-replace guard still binds (removing replace_messages fails the test);
ruff clean. Added test_in_place_skips_redundant_preflush /
test_rotation_still_preflushes to guard the perf change.
…e, not delete) Teknium review: keeping one durable session id must NOT come at the cost of destroying history. The prior in-place implementation used replace_messages, which hard-DELETEs the pre-compaction turns (they also drop out of the FTS index) — same id, but the original conversation is gone with no recovery path and the summary becomes the only record. Rotation today is non-destructive (the old session's full transcript survives under the old id); in-place must match that durability contract, not weaken it. Fix: compact in place by SOFT-ARCHIVING, reusing the existing messages.active flag (the /undo soft-delete mechanic), instead of deleting: - New SessionDB.archive_and_compact(session_id, compacted): in one atomic write, UPDATE messages SET active=0 on the live turns, then insert the compacted set as fresh active=1 rows. Nothing is deleted. - The insert loop is extracted into a shared _insert_message_rows() helper so archive_and_compact and replace_messages don't duplicate the 60-line column/encoding block (extend-don't-duplicate). - Agent in-place branch calls archive_and_compact instead of replace_messages. Durability outcome (proven by test + E2E across repeated compactions): - Live context load (get_messages_as_conversation / get_messages) filters active=1, so a resume reloads ONLY the compacted set — compaction still shrinks the live session. - The pre-compaction turns stay on disk at active=0, recoverable via get_messages(include_inactive=True) / restore_rewound. - They remain FTS-searchable: the messages_fts* triggers index on INSERT and remove on DELETE only — they do NOT key on active, and active=0 is a content-preserving UPDATE. session_search still finds them. - Verified across TWO successive compactions: the 1st compaction's originals are still recoverable + searchable after the 2nd (answers the "no recovery path after the next compaction" concern directly). message_count now reflects the LIVE (active/compacted) count, matching the live load. replace_messages keeps its DELETE semantics (still correct for /retry, /undo) and gains a docstring note pointing compaction at the non-destructive method. Tests: test_in_place_keeps_same_session_id strengthened to assert the 8 seeded originals survive at active=0 alongside the 2 compacted rows AND stay FTS-searchable. Mutation check: swapping archive_and_compact back to a hard DELETE fails the test, so the non-destructive contract is bound. 285 hermes_state + in-place tests green; rotation/persistence/compress-command/cli suites green; ruff clean.
…ion_search Follow-up to the soft-archive durability fix. Reusing the rewind/undo active=0 flag for compaction-archived turns inherited the wrong search semantics: undo rows are intentionally HIDDEN from session_search (the user took them back), but compaction-archived turns must stay DISCOVERABLE — that is the whole point of Teknium's "searchable / recoverable" requirement. As built, search_messages defaulted to WHERE active=1, so after in-place compaction the pre-compaction turns were in the FTS index but filtered out of the default search. (The earlier "searchable" claim only held for a raw FTS query / include_inactive=True, not the actual session_search tool.) Empirically confirmed the gap: search 'HMAC' returned 2 hits before compaction, 1 after (only the summary's mention) — the originals were hidden. Fix — a `compacted` flag distinct from `active`, giving a 3-way state: - active=1, compacted=0 → live context (normal) - active=0, compacted=1 → compaction-archived: OUT of live context, IN search - active=0, compacted=0 → rewind/undo: OUT of live context, OUT of search Changes: - messages.compacted INTEGER NOT NULL DEFAULT 0 added to SCHEMA_SQL. Declarative _reconcile_columns adds it on existing DBs — no version bump (plain column add). - archive_and_compact: UPDATE … SET active=0, compacted=1 (was active=0 only). - search_messages: default WHERE active=1 → (active=1 OR compacted=1), on BOTH the main FTS5 path and the trigram CJK path. include_inactive=True still returns everything. The short-CJK LIKE fallback already returns all rows (no active filter) — unchanged. - Docstrings on archive_and_compact + search_messages document the 3-way state. Verified: after compaction, session_search default finds the archived originals (ids 1 & 4); rewind/undo rows stay hidden by default (recoverable via include_inactive); live context still excludes both. 322 in-place + hermes_state tests and 46 session_search tests green; ruff clean. Mutation check: reverting the search WHERE to active-only fails the new searchable test. (Surfaced by the question "is search semantic or only FTS?" — answer: session search is FTS5 keyword/BM25 only, no embeddings over the transcript; semantic retrieval lives in the optional memory-provider layer. Tracing that confirmed the active-only filter gap above.)
Review nit (yoniebans): the config.py comment still said compaction is 'lossy: the pre-compaction transcript is discarded, matching Claude Code / Codex' — leftover from the original destructive design. The shipped behavior is soft-archive: lossy for the LIVE context (what the model reloads), but the pre-compaction turns are kept on disk (active=0, compacted=1), searchable via session_search and recoverable. Comment now says so. Comment-only; no behavior change.
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-attribute |
4 |
unresolved-import |
1 |
not-subscriptable |
1 |
First entries
tests/run_agent/test_in_place_compaction.py:46: [unresolved-attribute] unresolved-attribute: Object of type `AIAgent` has no attribute `context_compressor`
tests/run_agent/test_in_place_compaction.py:17: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/run_agent/test_in_place_compaction.py:253: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int`, `float` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
tests/run_agent/test_in_place_compaction.py:208: [not-subscriptable] not-subscriptable: Cannot subscript object of type `None` with no `__getitem__` method
tests/run_agent/test_in_place_compaction.py:209: [unresolved-attribute] unresolved-attribute: Attribute `execute` is not defined on `None` in union `None | Connection`
tests/run_agent/test_in_place_compaction.py:34: [unresolved-attribute] unresolved-attribute: Unresolved attribute `compression_in_place` on type `AIAgent`
✅ Fixed issues: none
Unchanged: 5907 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
alt-glitch
added
type/feature
This was referenced Jun 25, 2026
Merged
1 task
This was referenced Aug 9, 2026
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
3 participants
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.
Salvage of #49042 by @kshitijk4poor, cherry-picked onto current
mainwith authorship preserved.Summary
Adds an opt-in
compression.in_placemode (default off) that keeps one durable session id for a conversation's whole life instead of rotating the id on every compaction — and does it non-destructively: pre-compaction turns are soft-archived (searchable + recoverable), not deleted. First of two staged PRs for the session-rotation bug cluster (#38763).Durability (the reviewed-for property — no chat-history loss)
SessionDB.archive_and_compact(session_id, compacted): in one atomic write, flips live turns toactive=0, compacted=1and inserts the compacted set as freshactive=1rows. Nothing is deleted.active=1→ a resume reloads only the compacted set (compaction actually shrinks the live session).session_search: thecompactedflag gives a 3-way state —active=1(live),active=0/compacted=1(compaction-archived, still searchable),active=0/compacted=0(rewind/undo, hidden). Recoverable viaget_messages(include_inactive=True)._insert_message_rows()helper;replace_messageskeeps its DELETE semantics (correct for/retry,/undo).Wiring
hermes_cli/config.py):compression.in_placedefaultFalse, deep-merge gives existing installs the default (no version bump).agent/conversation_compression.py): on in-place,archive_and_compact+ rebuild system prompt + reset flush identity; fire compaction-boundary hooks in both modes; exposeagent._last_compaction_in_place+in_place=Trueon thesession:compressevent. Rotation path moved verbatim into theelsebranch (unchanged when off).gateway/run.py): reads the signal →history_offset=0on rotation OR in-place./compress(gateway/slash_commands.py): rewrites onrotated OR in_place; the /compress deletes original messages from state.db when session rotation fails #44794 data-loss guard still fires only for the genuine no-rotation-AND-not-in-place failure.Validation (verified against current main)
compacted INTEGER NOT NULL DEFAULT 0is in the primarySCHEMA_SQL;_reconcile_columnsauto-adds it to legacy DBs, old rows default to 0 (= live), history intact. Verified by dropping the column on a real DB and reopening with this code.archive_and_compactpreserves originals atactive=0, keeps them FTS-searchable through two successive compactions, live load returns only the compacted set.history_offset=0on in-place persists the full compacted list (the DB write already happened via the sharedSessionDB).hermes_state+ 263 compression-path tests green. ruff clean.Why staged
Ships default-off so CLI / gateway / TUI / ACP can be live-validated before the follow-up PR flips the default and removes the rotation machinery. Merging this changes nothing for existing users.
Closes #49042.
Infographic