fix(state): heal durable alternation violations at the restore boundary - #64935
fix(state): heal durable alternation violations at the restore boundary#64935Rival wants to merge 1 commit into
Conversation
A turn that persists a user row with no assistant row (suppressed reply, or two concurrent turns interleaving their flushes) leaves a user;user pair in state.db. The defensive pre-request repair_message_sequence then re-fires on EVERY request for the rest of the session's life — it mutates only the per-request list, never the stored transcript. Add repair_alternation (default False) to get_messages_as_conversation and pass it from the three live-replay restore sites (gateway load_transcript, CLI session resume x2). Inspection/export consumers (trace upload, context guard, api_server history) keep the verbatim default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the restore-boundary mitigation and preserving verbatim inspection/export reads. The underlying condition remains on current main: agent/conversation_loop.py:787-804 repairs malformed alternation only immediately before an API call.
Problems
- The change does not cover all live replay paths. Interactive CLI
/resumestill restores raw history athermes_cli/cli_commands_mixin.py:774-789. TUI/Desktop normal resume uses raw history attui_gateway/server.py:5764-5785; ACP restores it atacp_adapter/session.py:539-563; and API session chat passes the raw load to_run_agentatgateway/platforms/api_server.py:1850-1858and:2062-2068. Those paths can retain the repeated pre-request repair described in #64934.
Suggested changes
- Audit and cover every model-fed restore path, while leaving display/export consumers verbatim.
- Add integration coverage for interactive CLI
/resumeand a non-CLI replay path, in addition to the SessionDB-level tests.
Automated hermes-sweeper review.
| # resumes clean even if stray rows exist. | ||
| messages = _strip_background_review_harness(messages) | ||
| if repair_alternation and messages: | ||
| # Lazy import: hermes_state already depends on agent.* (see |
There was a problem hiding this comment.
Please ensure this repair is also selected at every model-fed restore boundary. Current main has additional live paths that still load raw history, including CLI /resume (hermes_cli/cli_commands_mixin.py:774), TUI/Desktop resume (tui_gateway/server.py:5764), ACP restore (acp_adapter/session.py:539), and API session chat (gateway/platforms/api_server.py:1855).
|
Merged via PR #65492 (restore-boundary repair) — your commit was cherry-picked onto current main with authorship preserved. The companion tripwire (#64936) landed via PR #65499 the same way. Exceptional report on #64934, by the way — the timeline, row-level evidence, and the honest scoping ("cause-agnostic fixes now, serialization once the tripwire names the route") made this an easy salvage. One follow-up we added during CI: an old test fixture seeded a 120-row all-user transcript and asserted the restore length, which your restore-time repair correctly collapsed — the fixture now alternates roles. The serialization patch offer stands noted on #64934; once the tripwire fires in a deployment and names the dispatch route, that's the natural next step. |
A turn that persists a user row with no assistant row (suppressed reply, or
two concurrent turns interleaving their flushes — see #64934) leaves a
user;userpair in state.db. The defensive pre-requestrepair_message_sequencethen re-fires on every request for the rest of thesession's life — it mutates only the per-request list, never the stored
transcript. Observed: 25×/day on one long-lived session, 297 occurrences
over two weeks.
This adds
repair_alternation(defaultFalse) toget_messages_as_conversationand passes it from the three live-replayrestore sites (gateway
load_transcript, CLI session resume ×2).Inspection/export consumers (trace upload, context guard, api_server
history) keep the verbatim default.
4 new tests (
tests/hermes_state/test_restore_alternation_repair.py):verbatim by default; user pair merged in order with no input lost; repaired
load yields zero further pre-request repairs; no-op on clean transcripts.
Neighboring state suites pass (411).
🤖 Generated with Claude Code