Skip to content

fix(memory): send only incremental turns in Hindsight append mode - #25763

Closed
estridell wants to merge 1 commit into
NousResearch:mainfrom
estridell:fix/hindsight-append-quadratic-token-usage
Closed

fix(memory): send only incremental turns in Hindsight append mode#25763
estridell wants to merge 1 commit into
NousResearch:mainfrom
estridell:fix/hindsight-append-quadratic-token-usage

Conversation

@estridell

Copy link
Copy Markdown

What does this PR do?

Fixes quadratic token usage in HindsightMemoryProvider.sync_turn() when the Hindsight API supports update_mode="append" (>=0.5.0).

The bug: sync_turn() sends the full accumulated _session_turns on every retain. With append-mode documents, this means turn 1 is sent N times, turn 2 is sent N-1 times, etc. — O(n²) token usage that burns through API credits on long sessions.

The fix: Track how many turns have already been retained (_last_retained_turn_count). In append mode, send only _session_turns[_last_retained_turn_count:] (the new turns since last retain). Legacy overwrite mode is unchanged — full-session snapshots are still required.

_session_turns itself is NOT cleared, so on_session_switch() can still flush partial buffers when retain_every_n_turns > 1.

Related Issue

Fixes the token-cost half of the append-mode integration gap introduced in 3082fa0 (feat: probe API for update_mode='append' support, dedupe across processes).

Type of Change

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

Changes Made

  • plugins/memory/hindsight/__init__.py: Add _last_retained_turn_count counter, use it to slice only new turns in append mode, reset in __init__/initialize()/on_session_switch()
  • tests/plugins/memory/test_hindsight_provider.py: Rename existing test to clarify it covers legacy overwrite mode, add new test test_sync_turn_append_mode_sends_only_new_buffered_turns that verifies incremental sends

How to Test

  1. pytest tests/plugins/memory/test_hindsight_provider.py -q — 97 tests pass
  2. pytest tests -q -k 'hindsight' — 102 tests pass, 1 skipped
  3. The new test test_sync_turn_append_mode_sends_only_new_buffered_turns specifically verifies that the second retain does NOT contain turns from the first retain

Checklist

  • My commit messages follow Conventional Commits (fix(memory): ...)
  • I searched for existing PRs to make sure this is not a duplicate
  • My PR contains only changes related to this fix
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes

Notes

The on_session_switch() flush in append mode will re-send some already-retained turns (redundant but safe). This is the correct tradeoff — data safety > perfect efficiency. The flush code is upstream and should not be touched in this PR.

…When Hindsight >=0.5.0 returns update_mode='append' from the version\nprobe, sync_turn() was still sending the full accumulated session on\nevery retain. This creates quadratic token usage — turn 1, then turns\n1+2, then turns 1+2+3 — because _session_turns is never cleared.\n\nWith append-mode documents, the correct behavior is to send only the\nturns buffered since the last retain. The server appends them to the\nexisting document, so prior turns don't need to be re-sent.\n\nLegacy overwrite mode (no update_mode) is unchanged: full-session\nsnapshots are still required so the document replaces prior content.\n\n_session_turns itself is NOT cleared — it must remain populated so\non_session_switch() can still flush partial buffers when\nretain_every_n_turns > 1. Instead, a _last_retained_turn_count\ncounter tracks how many turns have already been sent.\n\nFixes the token-cost half of the append-mode integration gap introduced\nin 3082fa0 (feat: probe API for update_mode='append' support).\n
@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 14, 2026
@teknium1

Copy link
Copy Markdown
Contributor

This looks implemented on current main by the later Hindsight delta-retain fix. Automated hermes-sweeper review found the requested behavior in the provider and covered by regression tests.

Evidence:

  • plugins/memory/hindsight/__init__.py:577 initializes _last_retained_turn_count for append-mode retain tracking.
  • plugins/memory/hindsight/__init__.py:14711483 sends only self._session_turns[self._last_retained_turn_count:] when update_mode == "append", while legacy overwrite mode still sends the full session.
  • plugins/memory/hindsight/__init__.py:15331536 advances the append watermark after enqueueing the retain.
  • tests/plugins/memory/test_hindsight_provider.py:805 verifies the second append retain does not include already-retained turns.
  • Commit 09d66037f8f7bc5bd879ed8128273fb6780a009f landed this on main as fix(hindsight): send only new-turn delta on append retains instead of whole session (#40605).

@teknium1 teknium1 closed this Jun 12, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main 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.

3 participants