fix(tui): preserve archived compaction history on prompt.submit truncation - #80195
Closed
Adolanium wants to merge 1 commit into
Closed
fix(tui): preserve archived compaction history on prompt.submit truncation#80195Adolanium wants to merge 1 commit into
Adolanium wants to merge 1 commit into
Conversation
…ation In-place compaction (compression.in_place, the default since NousResearch#38763) keeps the pre-compaction transcript on disk as soft-archived active=0/compacted=1 rows under the live session id. The prompt.submit truncation path (desktop/TUI edit, regenerate, rewind) called replace_messages() with the default active_only=False, which DELETEs every row for the session and reinserts only the truncated live tail, so any edit after a compaction permanently wiped the archived history. Same destructive-rewrite class as NousResearch#61145. The handler now probes has_archived_messages() and passes active_only=True when archives exist, so only the live rows are replaced. This mirrors the ACP adapter's _persist and the durability contract the gateway /compress path already documents (NousResearch#44794, NousResearch#39704). Likely a partial explanation for NousResearch#79391: the explicit_interrupt branch in conversation_compression.py restores memory and aborts before archive_and_compact runs, so the interrupt alone does not delete rows, but a later truncation wiping the archives matches the reported missing in_place_committed history. The regression test runs the truncation path against a real SessionDB seeded with archived compaction rows and asserts they survive.
alt-glitch
added
type/bug
This was referenced Aug 6, 2026
kshitijk4poor
pushed a commit
that referenced
this pull request
Aug 7, 2026
/retry truncates the live transcript to before the last user message and persists it via SessionStore.rewrite_transcript, which calls replace_messages() with the default active_only=False. That DELETEs every row for the session, including the soft-archived active=0/compacted=1 rows that in-place compaction keeps on disk (#38763), so any /retry after a compaction permanently wiped the archived history. #57803 named this call site as a residual gap after its global-default approach was rejected; the TUI sibling was fixed in #80195. The handler now probes has_archived_messages() (new SessionStore wrapper, auto-exposed through AsyncSessionStore) and passes active_only=True when archives exist, so only the live rows are replaced. rewrite_transcript gains an active_only parameter that defaults to False, keeping the destructive semantics yuanbao recall redaction depends on. Also corrects the rewrite_transcript docstring, which still listed /undo as a caller even though /undo soft-archives via rewind_session. The regression test drives _handle_retry_command against a real SessionStore and SessionDB seeded with archived compaction rows and asserts the archives survive.
Contributor
|
This PR is now fully superseded by merged #80933. On current Replaying this PR's commit onto current I recommend closing this PR as superseded rather than rebasing it. No additional code from this branch appears necessary. |
ma1138569845
pushed a commit
to ma1138569845/dechnicAuditor-agent
that referenced
this pull request
Aug 10, 2026
/retry truncates the live transcript to before the last user message and persists it via SessionStore.rewrite_transcript, which calls replace_messages() with the default active_only=False. That DELETEs every row for the session, including the soft-archived active=0/compacted=1 rows that in-place compaction keeps on disk (NousResearch#38763), so any /retry after a compaction permanently wiped the archived history. NousResearch#57803 named this call site as a residual gap after its global-default approach was rejected; the TUI sibling was fixed in NousResearch#80195. The handler now probes has_archived_messages() (new SessionStore wrapper, auto-exposed through AsyncSessionStore) and passes active_only=True when archives exist, so only the live rows are replaced. rewrite_transcript gains an active_only parameter that defaults to False, keeping the destructive semantics yuanbao recall redaction depends on. Also corrects the rewrite_transcript docstring, which still listed /undo as a caller even though /undo soft-archives via rewind_session. The regression test drives _handle_retry_command against a real SessionStore and SessionDB seeded with archived compaction rows and asserts the archives survive.
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
/retry truncates the live transcript to before the last user message and persists it via SessionStore.rewrite_transcript, which calls replace_messages() with the default active_only=False. That DELETEs every row for the session, including the soft-archived active=0/compacted=1 rows that in-place compaction keeps on disk (NousResearch#38763), so any /retry after a compaction permanently wiped the archived history. NousResearch#57803 named this call site as a residual gap after its global-default approach was rejected; the TUI sibling was fixed in NousResearch#80195. The handler now probes has_archived_messages() (new SessionStore wrapper, auto-exposed through AsyncSessionStore) and passes active_only=True when archives exist, so only the live rows are replaced. rewrite_transcript gains an active_only parameter that defaults to False, keeping the destructive semantics yuanbao recall redaction depends on. Also corrects the rewrite_transcript docstring, which still listed /undo as a caller even though /undo soft-archives via rewind_session. The regression test drives _handle_retry_command against a real SessionStore and SessionDB seeded with archived compaction rows and asserts the archives survive.
Contributor
Author
|
Agreed, this is fully covered by #80933. Closing as superseded rather than rebasing through the methods_prompt.py conflict. |
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.
What does this PR do?
After a successful in-place compaction (compression.in_place, the default since #38763), the pre-compaction transcript stays on disk as soft-archived active=0/compacted=1 rows under the same session id, and SessionDB keeps them searchable. The prompt.submit truncation path in tui_gateway (desktop/TUI edit, regenerate, rewind via truncate_before_user_ordinal) called replace_messages() with the default active_only=False, which DELETEs every row for the session and reinserts only the truncated live tail. Any edit after a compaction permanently destroyed the archived history: hard ID gaps, empty archive queries, missing in_place_committed history.
This mirrors the ACP adapter's _persist: probe has_archived_messages() and pass active_only so only the live rows are replaced when archives exist on disk. Same durability contract the gateway /compress path documents in gateway/slash_commands.py (#61145, #44794, #39704) and that replace_messages states in its own docstring.
Likely a partial explanation for #79391, not the whole story. The explicit_interrupt branch in agent/conversation_compression.py restores the in-memory transcript and aborts before archive_and_compact runs, so an interrupt alone does not DELETE rows. A later edit/regenerate wiping the archives matches the reported "earlier successful in_place_committed archives are missing". Refs rather than fixes.
Related Issue
Refs #79391
Type of Change
Changes Made
tui_gateway/methods_prompt.py: prompt.submit truncation probes has_archived_messages() and calls replace_messages(..., active_only=has_archived), mirroring acp_adapter/session.py _persist. A probe failure falls back to False, preserving the current behavior for sessions without archived rows.tests/test_tui_gateway_server.py: new regression test drives prompt.submit truncation against a real SessionDB seeded with soft-archived compaction rows and asserts the archived rows survive (active=0/compacted=1) and the live set matches the truncated transcript. Existing fake DBs accept the new active_only kwarg.Searched for competing PRs on this call site and found none open. #72694 covers the ACP /compress path only. gateway/session.py rewrite_transcript has the same bare call but different semantics (/undo vs archived rewind rows), left out of scope on purpose.
How to Test
scripts/run_tests.sh tests/test_tui_gateway_server.py -k truncat -qscripts/run_tests.sh tests/test_tui_gateway_server.py -qChecklist
Code
fix(scope):,feat(scope):, etc.)Documentation & Housekeeping
docs/, docstrings) - N/A, the fix brings the caller in line with the existing replace_messages docstring contractcli-config.yaml.exampleif I added/changed config keys - N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows - N/AScreenshots / Logs
$ scripts/run_tests.sh tests/test_tui_gateway_server.py -k truncat -q === Summary: 1 files, 8 tests passed, 0 failed (100% complete) in 8.7s (32 workers) ===