Skip to content

fix(hindsight): implement on_session_end to flush buffered turns at session exit - #28845

Open
fayenix wants to merge 3 commits into
NousResearch:mainfrom
chiefmojo:pr/hindsight-on-session-end
Open

fix(hindsight): implement on_session_end to flush buffered turns at session exit#28845
fayenix wants to merge 3 commits into
NousResearch:mainfrom
chiefmojo:pr/hindsight-on-session-end

Conversation

@fayenix

@fayenix fayenix commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

The hindsight memory provider buffers turns in-memory during a session but never flushes them when the session ends cleanly. This means buffered turns are silently lost if the gateway shuts down gracefully (no crash, no interrupt).

Changes

  • Implement on_session_end() on the HindsightProvider to flush all buffered turns to the summary store when the gateway calls it during shutdown.
  • Add test coverage for the new flush behavior (clean shutdown, empty buffer, error handling).

Testing

  • Unit tests pass: pytest tests/plugins/memory/test_hindsight_provider.py
  • Manual: confirmed buffered turns are persisted after graceful gateway stop.

Related

Resubmission of #16697 (closed without merge — commits preserved locally, re-cherry-picked onto current main).

…ession exit

sync_turn() batches retention every N turns. Turns accumulated since the
last batch boundary were previously lost when the session ended (gateway
restart, idle timeout, /reset, /new). This left users with empty recall
on the first turn after any session boundary.

on_session_end now forces a final synchronous retention of any remaining
buffered turns, preserving conversation context across session restarts.

Fixes the Part B gap from NousResearch#15165 — the plugin YAML declared the hook
but the implementation was never written.

Tests cover: flush of buffered turns, auto_retain skip, already-aligned
no-op, zero turns, missing client, in-flight sync wait, error resilience,
and session tag inclusion.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers labels May 19, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for chasing this; the premise is real on current main. Hindsight still buffers turns in sync_turn() until retain_every_n_turns and has no provider-specific on_session_end() on origin/main (plugins/memory/hindsight/__init__.py:1525; git grep only finds sync_turn() and on_session_switch()).

Problems

  • The PR waits on self._sync_thread.join(timeout=10.0) before flushing (plugins/memory/hindsight/__init__.py:1512), but current Hindsight uses _sync_thread as a legacy alias for the long-lived writer thread (plugins/memory/hindsight/__init__.py:559-561, :957-978). That makes normal session-end flushing wait for the timeout instead of just draining queued work.
  • The flush writes directly to self._document_id without _resolve_retain_target() or update_mode (plugins/memory/hindsight/__init__.py:1557). Current main’s retain path may use stable session_id plus update_mode='append' (plugins/memory/hindsight/__init__.py:1054-1073, :1530-1537), and legacy overwrite mode needs full-session content rather than only the modulo remainder.
  • The added tests don’t cover that append/legacy split, document_id selection, or writer-queue ordering.

Suggested changes

  • Mirror sync_turn()’s current retain-target rules in on_session_end() and use _last_retained_turn_count for append-mode deltas.
  • Enqueue the final flush through the writer queue rather than joining the writer thread directly.

Automated hermes-sweeper review.

Comment thread plugins/memory/hindsight/__init__.py Outdated
Comment thread plugins/memory/hindsight/__init__.py Outdated
Comment thread plugins/memory/hindsight/__init__.py Outdated
@fayenix
fayenix requested a review from teknium1 June 20, 2026 14:24
@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@teknium1 teknium1 added area/sessions Session lifecycle, resume, persistence, history area/memory Memory subsystem: store, providers, sync, background reviews labels Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/memory Memory subsystem: store, providers, sync, background reviews area/sessions Session lifecycle, resume, persistence, history comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants