Skip to content

fix(agent): pop persisted marker on repair mutations - #104454

Open
aniruddhaadak80 wants to merge 2 commits into
NousResearch:mainfrom
aniruddhaadak80:fix/repair-persisted-marker-pop
Open

aniruddhaadak80 wants to merge 2 commits into
NousResearch:mainfrom
aniruddhaadak80:fix/repair-persisted-marker-pop

Conversation

@aniruddhaadak80

Copy link
Copy Markdown

What

Alternation repair merges/prunes flushed message dicts in place but never popped the _db_persisted marker, so the next flush skipped the rewritten rows — the DB kept pre-merge bytes while the live transcript (resume, prompt cache) moved on. The merge paths already drop the api_content sidecar on the same condition; the persisted marker (documented contract: any in-place content mutation must pop it) now follows.

Related Issue

Code-scan find (no open issue covers repair persistence invalidation).

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • agent/agent_runtime_helpers.py: pop _DB_PERSISTED_MARKER on actual mutation in _merge_assistant_into (tool_calls/content/reasoning changes only, mirroring the sidecar discipline), _prune_unanswered_tool_calls, and _merge_consecutive_users; invalidate agent._db_flush_scan_prefix when repair_message_sequence repairs (guarded for partial agents).
  • tests/run_agent/test_repair_persistence_invalidation.py: user/assistant merges pop the marker and clear the prefix; clean runs leave both alone; prefix-less agents still repair.

How to Test

  1. Unit: scripts/run_tests.sh tests/run_agent/test_repair_persistence_invalidation.py -q (4 passed; merge tests fail pre-fix).
  2. ruff check clean on both touched files.

Checklist

Code

  • Read the Contributing Guide
  • Commit messages follow Conventional Commits (fix(scope):)
  • Searched existing PRs for duplicates
  • PR contains only changes related to this fix
  • Tests added (behavioral contracts, not snapshots; red-green verified)

Documentation & Housekeeping

  • No doc updates needed
  • No config keys added/changed; no new env vars
  • Cross-platform: pure dict logic, no OS surface

Screenshots / Logs

N/A.

@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 6, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

Summary

Alternation repair (_merge_assistant_into, _prune_unanswered_tool_calls, _merge_consecutive_users) now pops the _DB_PERSISTED_MARKER whenever it mutates a flushed row, and repair_message_sequence resets the flush-scan prefix — so the next flush rewrites repaired rows instead of skipping them as "already persisted" (which left the DB holding pre-merge bytes).

Findings

  • The mutated flag threading in _merge_assistant_into (agent/agent_runtime_helpers.py:9) is precise: pure no-op merges (e.g. empty tool_calls normalization that changes nothing) keep the marker and avoid a needless DB rewrite. The elif prev_calls: prev["tool_calls"] = prev_calls copy-normalization correctly does not set mutated.
  • repair_message_sequence (:88) only resets _db_flush_scan_prefix when repairs > 0, guarded by hasattr — no behavior change on clean histories or marker-less agents.
  • New test file pins all four legs (user merge, assistant merge, no-repair untouched, missing attr). Good.
  • Non-blocking: the _DB_PERSISTED_MARKER import is function-local in three places (presumably to dodge a cycle). If the cycle allows, hoisting to module level would be cleaner — cosmetic only.

Verdict

Correct persistence-invariant fix. Non-blocking.

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.

3 participants