fix(session): skip display_kind timeline rows in undo/retry turn targets - #72820
Closed
necoweb3 wants to merge 1 commit into
Closed
fix(session): skip display_kind timeline rows in undo/retry turn targets#72820necoweb3 wants to merge 1 commit into
necoweb3 wants to merge 1 commit into
Conversation
list_recent_user_messages and the in-memory /retry + session.undo walkers treated every role=user row as a real user turn. Timeline bookkeeping (model_switch, async_delegation_complete, auto_continue, hidden) is stored that way, so /undo soft-deleted from a marker and /retry re-sent opaque bookkeeping text. Exclude display_kind the same way CLI resume counting and the prompt.submit ordinal path do.
Collaborator
|
Merged via #73396. Your commits were cherry-picked with authorship preserved (rebase merge). The fix was widened to two sibling sites found during Phase 2 review:
Both now use the same Thanks for the clean fix! |
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
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.
Summary
/undoand/retryresolve their target against every durablerole == "user"row. Timeline bookkeeping rows —model_switch,async_delegation_complete,auto_continue,hidden— are stored that way, but no client counts them as user turns (desktop demotes them tosystem/ drops them; the CLI already usesnot m.get("display_kind")for resume turn counting).When a session ends with a marker after the last real exchange:
/undosoft-deleted from the marker only, leaving the last real Q/A intact — the user thinks they undid the last turn and they did not./retryre-sent the opaque marker text and truncated only the marker instead of the failed exchange.Same class as the
prompt.submitordinal skew (#72695): an index into the frontend's user-turn array treated as an index into the backend's full history. That PR fixed the destructive rewind ordinal; these paths still used the unfiltered definition of "user turn".Problem
Producer (soft-delete target list) —
SessionDB.list_recent_user_messages:No
display_kindexclusion. Every CLI / TUI / gateway/undo [N]andgateway.session.rewind_sessioncall this list to pick the Nth-from-last user row, thenrewind_to_messagesoft-deletes from that id forward.Producer (in-memory retry/undo) —
tui_gateway/retry,session.undo, gateway slash/retry, CLIretry_last/undo_lastwalk history with onlyrole == "user".Consumer rule already agreed elsewhere:
hermes_cli/cli_agent_setup_mixin.py—role == "user" and not m.get("display_kind")hermes_cli/cli_commands_mixin.py— sametoChatMessagesdemotes the known kinds tosystem/ dropshiddenDefault config.
async_delegation_completeis emitted by the notification poller;_AUTO_CONTINUE_ENABLED_DEFAULT = True. Markers accumulate permanently in the session, so every subsequent/undo//retryin that chat is misaligned until the next real user turn is newest again.Unrecoverable for
/retryon gateway:rewrite_transcripthard-replaces the transcript before re-sending the marker text. For TUI/undo, the soft-delete is durable — the wrong cut is persisted withactive=0on the wrong slice (or the right real turn is not rewound when only the marker is cut).Fix
SessionDB.list_recent_user_messages— addAND (display_kind IS NULL OR display_kind = ''). Fixes CLI/TUI/gateway/undoandrewind_sessionat the source.tui_gateway/retryandsession.undo— walk withrole == "user" and not display_kind;session.undotruncates from that index (same semantics as soft-delete)./retry— same predicate beforerewrite_transcript.retry_last/undo_last— same predicate on the in-memory walk (belt-and-suspenders with the list filter for the DB path).Predicate matches the CLI siblings and #72695.
Scope
hermes_state.py— one SQL clause + docstring onlist_recent_user_messagestui_gateway/server.py—/retryandsession.undowalkersgateway/slash_commands.py— gateway/retrycli.py—retry_last/undo_lastwalkers/retryhistory walkNo desktop change required. No change to
replace_messages/rewind_to_messagethemselves.Related but distinct: #72695 (prompt.submit ordinal only). This PR does not touch that path.
Testing
test_undo_skips_display_kind_timeline_rows— history with a trailingasync_delegation_completemarker;/undoprefillsquestion 2and soft-deletes from that real turn (marker + q2/a2 gone; q1/a1 remain). Fails on unpatchedlist_recent_user_messages.test_command_dispatch_retry_skips_display_kind_timeline_rows—/retryresendssecond questionand truncates through the real last user turn (not the marker).