fix: honcho peer retrieval routing - #9668
Closed
bkutasi wants to merge 1 commit into
Closed
Conversation
Contributor
|
Thanks for the thorough write-up and the well-structured fix, @bkutasi — the gap you identified in This is an automated hermes-sweeper review. Unfortunately, the fix landed on
The main implementation is a superset of this fix: it also adds a |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes peer retrieval routing in the Honcho memory plugin when
_ai_observe_othersmode is enabled. The_fetch_peer_card,_fetch_peer_context,get_peer_card,search_context, andget_prefetch_contextmethods always looked up peers directly, ignoring the observation-aware pattern where the AI peer should act as an observer to retrieve context about other peers. This caused missing or incorrect peer cards and representations in multi-peer/cross-observation setups.The fix threads
observer_peer_idthrough the internal retrieval methods, routing calls through the AI peer'sget_card(target=...)andcontext(target=...)APIs when_ai_observe_others=True, while falling back to direct peer lookup when disabled.Type of Change
Historical Context
This fix completes a chain of observation-mode evolution in the Honcho plugin:
feat(memory): pluggable memory provider interface— Introduced the Honcho memory provider plugin architecturefix: route memory provider tools in sequential execution path— Fixed tool routing for memory providersfix: thread gateway user_id to memory plugins for per-user scoping— Added per-user peer scopingfeat(honcho): add configurable observation mode (unified/directional)— Introduced binary observation mode switching;dialectic_query,create_conclusiongot cross-observation routing, but peer retrieval methods (_fetch_peer_card,_fetch_peer_context) were not yet updatedfix(honcho): plugin drift overhaul— Replaced binaryobservationModewith granular per-peer booleans (_ai_observe_others,_user_observe_me, etc.), switched peer card/profile/search fromsession.context()to direct peer APIs, added_fetch_peer_cardand_fetch_peer_contexthelper methods — but these new helpers didn't respect_ai_observe_othersfeat(honcho): add opt-in initOnSessionStart for tools mode and respect explicit peerName— Added tools-mode init timing and peer name configurationfix: resolve CI test failures— Fixed stale tests after the above changesThe gap: Commit c02c3dc introduced
_fetch_peer_cardand_fetch_peer_contextas the new direct-peer-API retrieval path, but these methods always calledself._get_or_create_peer(peer_id)directly, bypassing the cross-observation routing thatdialectic_queryandcreate_conclusionalready had. When_ai_observe_others=True, the AI peer should be the observer (callingget_card(target=user_peer_id)/context(target=user_peer_id)), but the retrieval helpers were fetching the user peer directly instead.Changes Made
plugins/memory/honcho/session.py(+32/-7 lines)_fetch_peer_card(peer_id, observer_peer_id=None)— When_ai_observe_othersandobserver_peer_idare set, creates the AI peer as observer and callsget_card(target=peer_id)orcard(target=peer_id)instead of direct peer lookup_fetch_peer_context(peer_id, search_query=None, observer_peer_id=None)— Same pattern: routes through AI peer'scontext(target=..., search_query=...)when observation mode is activeget_peer_card()— Passesobserver_peer_id=session.assistant_peer_idto_fetch_peer_cardsearch_context()— Passesobserver_peer_id=session.assistant_peer_idto_fetch_peer_contextget_prefetch_context()— Passesobserver_peer_idfor user context fetch; AI self-context remains direct (no observer needed)tests/honcho_plugin/test_session.py(+182/-29 lines)test_get_peer_card_cross_observation— Verifies AI peer routesget_card(target=user_peer_id)when_ai_observe_others=Truetest_get_peer_card_unified_mode— Verifies direct peer lookup when_ai_observe_others=Falsetest_search_context_cross_observation— Verifies AI peer routescontext(target=..., search_query=...)in observation modetest_search_context_unified_mode— Verifies direct peercontext(search_query=...)in unified modetest_get_prefetch_context_cross_observation— Verifies user context fetched via AI observer, AI self-context fetched directlywithstatements, addedmgr._ai_observe_others = Falseto existing tests for correctnessHow to Test
aiObserveOthers: truetarget=kwarg usage)pytest tests/honcho_plugin/test_session.py -q— all tests pass, including the 5 new cross-observation testsChecklist
Code
pytest tests/ -qand all tests passDocumentation & Housekeeping