fix(honcho): dedupe repeated message flushes - #18536
Conversation
8bb5f4a to
802ec88
Compare
|
Implements part of #9404 |
802ec88 to
237a499
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for narrowing the change to the Honcho plugin. The duplicate-ingest problem remains present on current main: plugins/memory/honcho/session.py:440-446 creates and uploads messages from content alone, with no stable Hermes source-event identity.
Problems
- The new guard at
plugins/memory/honcho/session.py:385treats an equal role/content batch as a replay. That drops legitimate repeated turns; the #9404 discussion explicitly requires repeated content from distinct source events to remain ingestible. - The check and record operations at
plugins/memory/honcho/session.py:385and:399are not synchronized. Current main can call_flush_session()via the async writer (session.py:471),flush_all()(session.py:534,544), and the backgroundsync_turn()path (__init__.py:1232-1248), so two callers can both pass the check before either records the fingerprint.
Suggested changes
- Re-scope deduplication around stable Hermes source-event identifiers plus chunk identity, with an atomic claim/commit protocol.
- Test replay of one source event separately from two equal-content turns, and cover concurrent flush entry points.
Automated hermes-sweeper review.
| session.key, (set(), deque()) | ||
| ) | ||
| batch_fp = _batch_fingerprint(new_messages) | ||
| if batch_fp in recent_set: |
There was a problem hiding this comment.
This key contains only roles and content, so it cannot distinguish a retry from a later legitimate turn with the same text. #9404 requires source-event idempotency while preserving repeated messages; please key this on stable Hermes event/turn and chunk identity instead.
| honcho_session.add_messages(honcho_messages) | ||
| for msg in new_messages: | ||
| msg["_synced"] = True | ||
| recent_set.add(batch_fp) |
There was a problem hiding this comment.
Recording the fingerprint only after add_messages() and without synchronization leaves a check-then-act race: concurrent async/session-end flushes can both pass the earlier membership test and upload the same batch. Use an atomic per-event claim/commit protocol with rollback on failed upload.
Summary
Test Plan