Skip to content

fix(agent): stop re-appending repaired rows and saving the length nudge as user text - #120862

Open
jonpol01 wants to merge 1 commit into
NousResearch:mainfrom
jonpol01:fix/repaired-user-row-not-persisted-twice
Open

jonpol01 wants to merge 1 commit into
NousResearch:mainfrom
jonpol01:fix/repaired-user-row-not-persisted-twice

Conversation

@jonpol01

Copy link
Copy Markdown
Contributor

What does this PR do?

A repaired user row is saved once, and Hermes's internal length-continuation nudge is no longer saved as the user's own words.

Before this change, #117569 popped a saved row's marker so that "the next flush rewrites it". But the flush only appends, so the user's prompt was inserted a second time and every later turn replayed both. The thinking-only length retry and the interrupted lease-wait carry both hit it.

Related Issue

Fixes #120861

This fixes forward from #117569 (#117120, salvage of #117121) and keeps its intent: an in-place repair of a persisted dict reaches state.db, now as an in-place rewrite. Nothing is reverted.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • A. agent/transcript_repair.py: a message that already owns an active row (_row_id) is never inserted again.

    • A non-assistant row is rewritten in place.
    • Assistant rows keep the existing blank-update / concurrent-winner logic.
    • _active_assistant_row becomes _active_row(role), with the same watermark-clone lookup.
  • B. agent/turn_truncation.py, session_persistence.py, turn_final_response.py: a length nudge that would follow this turn's already-saved user row is flagged _length_continuation_synthetic. That makes it request-only ephemeral scaffolding, like _dropped_toolcall_nudge: the wire copy still merges it for alternation, but it is never saved or merged into the real row, and it's removed before the final answer.

    • Skipping the nudge outright would break test_empty_stub_only_appends_continuation_user_message, which requires the chunking nudge to reach the model.
  • C. agent/session_persistence.py: _PERSIST_AFTER_ADMISSION_INTERRUPT is cleared after its one write, so the carried row stops bypassing the history skip.

  • agent/agent_runtime_helpers.py _merge_consecutive_users never merges ephemeral scaffolding into a real user row.

  • Tests (tests/agent/test_session_persistence_repaired_user_row.py) drive the real turn loop with a fake LLM client and a real SessionDB. No request leaves the machine.

    • test_thinking_only_length_retry_saves_the_prompt_once_and_never_the_nudge
    • test_prompt_carried_past_an_interrupted_lease_wait_is_saved_once: a real held lease, the status callback, and interrupt().
    • Both are red on main, with the saved prompt duplicated and the nudge saved as user text. Reverting each piece of B and C on its own reddens a test.
    • Piece A was checked outside these two tests with two real-loop runs:

    Happy to add either as a third test if wanted.

Trade-off: after such a continuation, the next turn replays the prompt without the nudge, so the prompt cache misses from that message onward, exactly as _dropped_toolcall_nudge already does. Before, the corrupted merged text was what got cached.

Open PRs:

How to Test

  1. scripts/run_tests.sh over test_session_persistence_repaired_user_row.py and the fix(agent): in-place repairs of persisted message dicts reach state.db instead of leaving stale rows (#117120, salvage #117121) #117569 suites (test_message_sequence_repair, test_micro_compaction, test_tool_call_args_sanitizer, test_unicode_ascii_codec), plus test_partial_stream_finish_reason: 143 passed.
  2. 78 related files over continuation, lease, batch, scaffolding, repair and _row_id: 1918 passed.
    • The two failures in test_run_agent.py and one in tui_gateway fail identically on unmodified main on this host: the optional anthropic SDK is missing, and a git-probe fork segfaults.
  3. Hunt repro: the saved transcript after turn 2 is [user 'first question', assistant, user 'write me a long report', assistant], with the prompt once and no nudge row.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass. I ran only the related files above through scripts/run_tests.sh, not the full suite.
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

…ge as user text

a48b4c7 (NousResearch#117569) pops _db_persisted when a repair rewrites an already-written dict, on
the premise that the next flush rewrites the row. The flush only appends (except blank
assistant rows), so the repaired row was INSERTed a second time:

- Thinking-only length truncation: the continuation nudge lands right after this turn's
  written user row; the pre-request repair merges it in and pops the marker, so state.db
  holds the prompt twice, the second copy carrying "[System: Your previous response was
  truncated ...]" as the user's own words, replayed on every later turn.
- A prompt carried past an interrupted turn-lease wait keeps
  _persist_after_admission_interrupt forever, so the follow-up turn's merge into it bypasses
  the history skip and is appended again.
- Surrogate recovery rewriting an already-flushed tool result appends a second result for
  the same tool_call_id.

Fix forward, keeping NousResearch#117120's intent (in-place repairs reach state.db):

- transcript_repair: a dict whose _row_id owns an active row of its role (or that row's
  watermark clone) is rewritten in place (content, api_content), never inserted; assistant
  rows keep the blank-update / concurrent-winner rules.
- turn_truncation: a nudge that would follow a user row is request-only scaffolding
  (_length_continuation_synthetic): never persisted, never merged into the real row, popped
  before the final answer. The wire copy still merges it, so the request is unchanged.
- session_persistence: the admission-interrupt flag is cleared once the carried row is
  written.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 24, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: a repaired user row is saved to state.db twice, and the length nudge is saved as the user's words (regression from #117569)

2 participants