Skip to content

fix(honcho): respect writeFrequency in sync_turn - #19650

Open
starship-s wants to merge 1 commit into
NousResearch:mainfrom
starship-s:fix/honcho-sync-turn-write-frequency-routing
Open

fix(honcho): respect writeFrequency in sync_turn#19650
starship-s wants to merge 1 commit into
NousResearch:mainfrom
starship-s:fix/honcho-sync-turn-write-frequency-routing

Conversation

@starship-s

@starship-s starship-s commented May 4, 2026

Copy link
Copy Markdown
Contributor

Context

Honcho's writeFrequency setting is the user's control for when completed conversation turns are written to Honcho: immediately, asynchronously, at session end, or every N turns. This matters for users who intentionally defer or batch writes, especially for session-end or cadence-based memory syncing.

The sync_turn() path was appending the turn and then calling _flush_session() directly. That bypassed HonchoSessionManager.save(), which is where writeFrequency routing is applied. As a result, deferred modes such as session and integer cadences could silently behave like per-turn writes.

Change

  • Route Honcho sync_turn() persistence through HonchoSessionManager.save() so configured writeFrequency modes are honored.
  • Add regression coverage for async, turn, session, and integer write-frequency routing from sync_turn().

Why this approach

HonchoSessionManager.save() already centralizes the write-frequency decision. Routing sync_turn() through it keeps the completed-turn sync path consistent with the rest of the Honcho session manager, while preserving immediate flushing for modes that are supposed to flush immediately.

This keeps the fix narrow: one routing change plus tests for each supported write-frequency mode.

Test Plan

  • python -m pytest tests/honcho_plugin/test_sync_turn_write_frequency.py tests/honcho_plugin/test_async_memory.py -q (45 passed)
  • python -m pytest tests/honcho_plugin -q (271 passed)

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have labels May 4, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. The premise remains valid on current main: HonchoMemoryProvider.sync_turn() calls _flush_session() directly at plugins/memory/honcho/__init__.py:1239, while HonchoSessionManager.save() is the existing routing boundary for async, turn, session, and integer cadence modes at plugins/memory/honcho/session.py:500-522. The provider already calls flush_all() at session end (plugins/memory/honcho/__init__.py:1283-1295), so the proposed route preserves the deferred-session contract.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 12, 2026
@starship-s
starship-s force-pushed the fix/honcho-sync-turn-write-frequency-routing branch from d415914 to 3d0ea20 Compare July 13, 2026 18:28
@starship-s
starship-s force-pushed the fix/honcho-sync-turn-write-frequency-routing branch from 3d0ea20 to dcef3f9 Compare July 13, 2026 18:39
@teknium1 teknium1 added the area/memory Memory subsystem: store, providers, sync, background reviews label Jul 19, 2026
@yw0nam

yw0nam commented Jul 21, 2026

Copy link
Copy Markdown

Reproduced this on current main in a live Hermes+Honcho setup: a profile with writeFrequency: 20 had effective config correctly resolved to 20, but every completed turn still reached Honcho because HonchoMemoryProvider.sync_turn() directly invoked _flush_session() instead of the manager routing boundary.

I verified the minimal routing change (manager.save(session)) with the existing ingest regression plus test_async_memory.py: 43 passed. The broader tests/honcho_plugin run reached 419 passed; its 4 failures are unrelated cache-busting tests that reuse HERMES_HOME in-process.

This directly addresses the observed every-turn persistence and keeps the existing on_session_end()flush_all() partial-batch behavior intact.

kshitijk4poor pushed a commit that referenced this pull request Aug 13, 2026
…ager.save()

sync_turn called manager._flush_session() directly, which flushes
synchronously every turn no matter what writeFrequency says — the
"async", "session", and every-N-turns modes were dead configuration
on the main turn path. Route through save(), the dispatcher that
actually implements those modes.

Same bug class reported in #19650 (starship-s) and #72708 (Diaspar4u);
this takes the minimal one-line routing fix without their broader
lifecycle refactors.

Co-authored-by: starship-s <45587122+starship-s@users.noreply.github.com>
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 comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

4 participants