Conversation
honcho_search routed through search_context() -> peer.context(search_query=),
which returns the peer's standing representation + card. The search_query arg
does not turn that endpoint into a search, so results were effectively
query-independent: the same representation blob regardless of the query. Factual
lookups ('what medication', 'which value did we pick') returned noise.
Rewire search_context() to call the workspace message-search endpoint
(Honcho.search) with a peer_perspective filter: RRF-ranked (hybrid semantic +
full-text) raw message excerpts spanning every session the peer was a member of,
across all authors, membership-time-scoped. This is the cross-session factual
recall primitive.
peer_perspective is chosen over the alternatives because it is the only scope
that is simultaneously (a) cross-session, (b) inclusive of assistant-authored
facts about the peer (peer-author search drops these, and they are a large part
of what you want to recall about yourself), and (c) privacy-scoped to the peer's
own sessions (plain workspace search leaks other peers' sessions).
- snippets are labeled by author so the model can tell user-stated facts from
assistant-derived ones
- max_tokens is now an enforced budget (was accepted but meaningless)
- graceful fallback to peer-authored search if peer_perspective is unsupported
- query length clamped under the embedding input cap
Replaces 3 change-detector tests that asserted the old representation-dump
behavior with 4 that assert the message-search contract + fallback path.
The five Honcho tool schemas had overlapping/misleading descriptions, making it hard for the model to pick the right one, plus two concrete correctness bugs: - honcho_context advertised an 'Optional focus query' parameter that the dispatch never read. The model could pass query= expecting filtering that never happened. Remove the dead parameter; honcho_context is an honest no-query snapshot. Focused retrieval now lives in honcho_search (see prior commit). - honcho_conclude's peer param was described as 'Peer to query' — wrong; it's the peer the conclusion is ABOUT. Corrected. Rewrite all five descriptions to give each tool a distinct mental model and cross-reference siblings: profile = read/write the compact card (cheapest, no query, no LLM) search = find what was actually said, ranked, cross-session (cheap, no LLM) reasoning= ask a question, get a synthesized answer (the only LLM tool; expensive) context = a fixed session snapshot (no query, no LLM) conclude = write a durable fact to the profile Addresses the honcho_context query param half of NousResearch#29402 (see PR notes for the divergence from that issue's proposed wire-through approach).
A brand-new session injected no Honcho context on the user's first message — the peer card/representation only showed up from turn 2 onward. The base-context fetch was fired asynchronously and popped in the same synchronous pass, so it always lost the race on turn 1 (a background thread can't finish inside one pass), leaving the first response with zero recalled context. Fetch the base layer (representation + card + summary) synchronously with a bounded timeout on turn 1 so the peer card is injected immediately; subsequent turns still consume the background-refreshed result primed by queue_prefetch(). The wait is bounded by _FIRST_TURN_BASE_TIMEOUT and tightened further by a small configured request timeout (fail-fast deployments / tests). Two related first-turn/dialectic reliability fixes ride along: - first-turn dialectic no longer double-fires: if a prewarm .chat() thread is already in flight from session init, turn 1 waits briefly for it instead of firing a second (duplicate) call that also blocked the first response. The first-turn wait is decoupled from a large host timeout (a 60s host timeout must not block the first response for 60s) via _FIRST_TURN_DIALECTIC_CAP, while still honoring a tight configured timeout. - empty-pass propagation guard in multi-pass dialectic: at depth > 1 each pass feeds the prior pass's output into the next prompt. If a pass returned empty (e.g. a reasoning model that spent its whole budget thinking), the next prompt carried a blank assessment (the "empty spot" seen in Honcho request logs). Now only non-empty prior results feed dependent passes; if all priors are empty, re-issue the base prompt instead of referencing nothing.
Symptom: Honcho logs show a dialectic answer was generated, but Hermes never
injects it — intermittently.
Root cause: the dialectic supplement that queue_prefetch() fires at the end of
turn N is stored pending (fired_at=N) for consumption by turn N+1's prefetch().
But prefetch()'s trivial-prompt guard returned early BEFORE the consumption
block. So when turn N+1's prompt was trivial ('ok', 'yes', 'continue', a slash
command), the ready result was never consumed, and a few turns later the
stale-discard guard dropped it. Generated by Honcho, never seen by the model.
The dependence on 'is the consuming turn trivial?' is why the loss looked random.
Fix: trivial turns now consume and inject a ready, non-stale pending result while
still spending no new work (no base-context fetch, no new dialectic fire). A
trivial ack shouldn't generate context, but it shouldn't destroy an answer
already computed for that exact turn. Extracted the pop+stale-check into a shared
_consume_pending_dialectic() used by both the trivial path and the normal path so
they age-check identically.
Preserved (covered by new tests): genuinely stale results are still discarded on
trivial turns; trivial turns still fire no new work; a trivial turn with nothing
pending still injects nothing.
Adds regression tests for inject-on-trivial and discard-stale-on-trivial.
HonchoClientConfig timeout/requestTimeout resolution skipped the per-host config block, silently dropping a host-scoped timeout and falling through to the global config.yaml value (or the default). Add the host block at the front of the resolution chain, consistent with every other field (base_url, api_key, etc.).
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Comprehensive improvements to the Honcho memory plugin. The PR addresses 3 user-visible failures: honcho_search never searching messages (returned the same generic blob), fresh sessions losing cross-session recall, and dialectic context being inconsistently injected.
The changes span 4 files (plugin init, client, session, tests) with 421 additions. The tool description rewrites are particularly valuable — they now clearly differentiate the 5 Honcho tools (profile, search, reasoning, context, conclude) so the model can actually tell them apart.
Looks Good
- Well-documented with clear motivation and root cause analysis
- Tool descriptions significantly improved for model comprehension
- Addresses functional bugs, not just cosmetic issues
- Scoped to the Honcho plugin (4 files, all within plugins/memory/honcho/)
Reviewed by Hermes Agent
injectionFrequency, contextCadence, and dialecticCadence were read only via raw.get() which checks root-level keys in honcho.json. Settings placed inside hosts.<name> (the normal per-host location) were silently ignored, falling back to defaults. - Add typed dataclass fields: injection_frequency, context_cadence, dialectic_cadence with host-block-first resolution chains matching the pattern used by all other config fields. - Update HonchoMemoryProvider.initialize() to read from typed cfg fields instead of cfg.raw directly. - Fix search_context tests that mocked _honcho directly — the .honcho property getter calls get_honcho_client() which overwrites the backing field, so use patch.object on the property instead. - Add injection_frequency and context_cadence config override tests.
injectionFrequency='first-turn' returned empty for the entire
prefetch_context() method on turns 2+, which blocked the dialectic
supplement from being consumed and injected. The dialectic has its
own cadence (dialecticCadence) and must continue to fire and inject
independently of the base context layer.
Now first-turn mode gates only Layer 1 (base context: representation
+ card), letting Layer 2 (dialectic supplement) flow through its
normal consumption path on every turn.
Also fixes all remaining tests that passed dialecticCadence via
cfg_extra={'raw': {...}} to use the typed dialectic_cadence field.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused Honcho investigation. The current-main premise is real: search_context() still returns only representation/card at plugins/memory/honcho/session.py:1132-1146, and does not apply its max_tokens argument.
Problems
plugins/memory/honcho/session.py:1161introducesHoncho.search(..., filters={"peer_perspective": ...}), but this PR leaves the pinned optional SDK athoncho-ai==2.0.1(pyproject.toml:186) and validates the call only through aMagicMock(tests/honcho_plugin/test_session.py:255-311). Open successor #62290 explicitly adopts this work and includes the SDK 2.0.1→2.2.0 update after checking SDK signatures. This change needs that dependency/interface contract addressed before it can safely land.- The host-block cadence tests construct
HonchoClientConfigdirectly (tests/honcho_plugin/test_session.py:972-980); they do not exercise the changed file-resolution path inHonchoClientConfig.from_global_config().
Suggested changes
- Include the compatible SDK/lock update and test the concrete search API contract.
- Add temp-config coverage for host-block precedence of timeout and all three cadence settings.
Automated hermes-sweeper review.
| limit = max(3, min(20, char_budget // 300)) | ||
|
|
||
| try: | ||
| messages = self.honcho.search( |
There was a problem hiding this comment.
This new workspace-level search API is only exercised through a MagicMock, while this PR leaves the optional dependency pinned to honcho-ai==2.0.1 in pyproject.toml. Please include the compatible SDK/lock update and a concrete interface-contract test; successor #62290, which adopts this work, upgrades to 2.2.0 after validating SDK signatures.
|
|
||
| def test_injection_frequency_from_config(self): | ||
| """injectionFrequency from config (including host block) is respected.""" | ||
| provider = self._make_provider(cfg_extra={"injection_frequency": "first-turn"}) |
There was a problem hiding this comment.
This injects an already-resolved dataclass value, so it does not test the host-block resolution introduced in HonchoClientConfig.from_global_config(). Add a temp-config test with hosts.<name>.injectionFrequency (and analogous cadence fields) to prove the production precedence chain.
Summary
This PR makes the Honcho memory plugin behave the way its tool surface
advertises. It fixes one functional bug that made
honcho_searchnearlyuseless (it never searched messages), repairs a silent context-loss bug
in the auto-injection pipeline (dialectic answers generated by Honcho but never
shown to the model), corrects/clarifies all five tool descriptions so the
model can actually tell them apart, and adds the supporting reliability fixes
(first-turn context, timeout resolution, empty-pass handling) with regression
tests for each.
Motivation
In daily use the Honcho integration had three recurring, user-visible failures:
honcho_searchreturned the same generic blob no matter what you asked.Searching "what's my prescribed treatment regimen?" returned the standing
peer representation — not the messages where the regimen was actually
stated. The tool was named and described as semantic search but did not
search messages at all.
Starting a fresh session lost cross-session recall. Asking "remind me
where we left off on X" in a new session surfaced nothing useful, because
the only retrieval the model could cheaply reach was scoped to the current
session (which is empty on turn 1).
Dialectic context was inconsistently injected. Honcho's logs showed a
dialectic answer being generated, but it frequently never appeared in the
model's context — seemingly at random.
Root-causing these turned up a shared theme: the plugin's tool surface
described capabilities the implementation didn't deliver, and the
auto-injection pipeline could silently drop work it had already paid for.
What changed
1.
honcho_searchnow actually searches messagesBefore:
honcho_search→search_context()→peer.context(search_query=…),which returns the peer's standing representation + card. The
search_queryargument does not turn that endpoint into a search; the result is effectively
query-independent — the same representation dump regardless of what you
searched for. This is why factual lookups ("what medication", "which config
value", "what did we decide") returned noise.
After:
search_context()calls Honcho's workspace message-searchendpoint via the SDK (
Honcho.search(query, filters=…, limit=…)) with apeer_perspectivefilter. This returns RRF-ranked (hybrid semantic +full-text) raw message excerpts drawn from every session the peer
participated in, scoped by their join/leave windows.
Why
peer_perspectivespecifically, and not the other available scopes — thiswas verified empirically against the live store, not assumed:
peer_perspective✅The crucial subtlety: the facts you most want to recall about yourself are
often authored by the assistant, not by you. Peer-author search structurally
drops them;
peer_perspectivekeeps them while still scoping to sessions thepeer was actually in.
Additional implementation details:
[assistant · session]vs[<peer> · session]) so the model can distinguish user-stated facts fromassistant-derived ones.
max_tokensis now a real budget: it's converted to an approximateresult count and a character cap, and snippets are accumulated until the
budget is exhausted (previously the param was accepted but meaningless,
since a representation dump ignores it).
peer_perspectivefilter, the code falls back to peer-authored search ratherthan returning nothing.
Files:
plugins/memory/honcho/session.py(search_context)2. Dialectic answers stranded on trivial turns, then silently discarded
Symptom: Honcho's logs show a dialectic answer was generated, but Hermes
never injects it — intermittently.
Root cause: In
HonchoMemoryProvider.prefetch(),the dialectic supplement that
queue_prefetch()fires at the end of turn N isstored as a pending result tagged
fired_at=N, intended for consumption by turnN+1. But the trivial-prompt guard returned early (
if _is_trivial_prompt(query): return "") before the consumption block. So whenturn N+1's prompt was trivial (
"ok","yes","continue", a slashcommand), the ready result was never consumed — and a few turns later the
stale-discard guard (
(turn - fired_at) > cadence × multiplier) dropped itentirely. Generated by Honcho, never seen by the model. The dependence on
"is the consuming turn trivial?" is exactly why the loss looked random.
Fix (chosen behavior): trivial turns now consume and inject a ready,
non-stale pending result, while still spending no new work (no base-context
fetch, no new dialectic fire). A trivial acknowledgement shouldn't generate
context, but it shouldn't destroy an answer that was already computed for that
exact turn either.
Implementation: extracted the pop-and-stale-check into a single
_consume_pending_dialectic()helper, now called by both the trivial-promptpath and the normal path, so the two routes age-check identically and can't
drift apart.
Behavior preserved (all covered by tests):
representation dump on a bare "ok").
Files:
plugins/memory/honcho/__init__.py(prefetch, new_consume_pending_dialectic)3. Honest, non-overlapping tool descriptions (all five) + a dead param removed
The five tool schemas had overlapping/misleading descriptions, making it hard
for the model to choose correctly. They've been rewritten to give each tool a
clean, distinct mental model and to cross-reference siblings:
honcho_profilehoncho_searchhoncho_reasoninghoncho_contexthoncho_concludeSpecific correctness fixes (not just wording):
honcho_contextadvertised anOptional focus queryparameter that thedispatch never read. The model could pass
queryexpecting filtering thatnever happened. The dead parameter is removed from the schema.
honcho_conclude'speerparameter was described as "Peer to query" —wrong; it's the peer the conclusion is about. Corrected.
honcho_reasoningnow states it's the only LLM-backed tool and when toprefer the cheaper alternatives, to curb unnecessary expensive calls.
Files:
plugins/memory/honcho/__init__.py(schema constants)4. Supporting reliability fixes
These fall out of the same investigation and are needed for the above to work
well in practice:
First-turn base context. Base context (representation + card +
summary) is now fetched synchronously with a bounded timeout on turn 1, so
a brand-new session injects the peer card on the first message instead of
showing nothing until turn 2. The old code fired the fetch async and popped in
the same pass, which always lost the race on turn 1. Subsequent turns still
consume the background-refreshed result primed by
queue_prefetch(). Boundedby
_FIRST_TURN_BASE_TIMEOUT(and tightened further by a small configuredtimeoutfor fail-fast deployments/tests).First-turn dialectic no longer double-fires or over-blocks. If a dialectic
prewarm thread is already in flight from session init, turn 1 waits briefly
for it instead of firing a second
.chat()(which was duplicate work thatalso blocked the first response). The first-turn wait is decoupled from a
large host
timeout(a 60s host timeout must not block the first response for60s) via
_FIRST_TURN_DIALECTIC_CAP, while still honoring a tight configuredtimeout.
Empty-pass propagation guard in multi-pass dialectic. At
depth > 1, each pass feeds the prior pass's output into the next prompt
("Given this initial assessment: …"). If a pass returned empty (e.g. a
reasoning model that spent its whole budget thinking and emitted no content),
the next prompt carried a blank assessment — the "empty spot" visible in
Honcho request logs. Now only non-empty prior results are propagated; if
all priors are empty, the pass re-issues the base prompt instead of
referencing nothing.
Honcho client
timeoutresolution honors the host block. The per-hostconfig block was skipped in the
timeout/requestTimeoutresolution chain,silently dropping a per-host timeout and falling through to the global default.
The host block now takes precedence, consistent with every other field.
Files:
plugins/memory/honcho/__init__.py,plugins/memory/honcho/client.pyTests
tests/honcho_plugin/— +6 new tests, 3 obsolete change-detector testsrewritten. Full plugin suite: 341 passed (the single remaining failure,
test_passes_timeout_from_config, is a pre-existing test-ordering pollutionissue unrelated to this PR — it passes in isolation and in its own file).
New / rewritten:
test_search_context_uses_peer_perspective_message_search— assertshoncho_searchcalls workspace message search with thepeer_perspectivefilter and returns ranked message content (regression guard for the
representation-dump bug). Replaces the 3 old tests that asserted the broken
peer.context(search_query=…)behavior.test_search_context_explicit_ai_peer_searches_ai_perspectivetest_search_context_empty_query_returns_emptytest_search_context_falls_back_to_peer_search_on_filter_error— covers theolder-Honcho fallback path.
test_trivial_prompt_injects_ready_pending_dialectic— locks in thetrivial-turn fix: a ready, non-stale pending result is injected on a trivial
turn.
test_trivial_prompt_discards_stale_pending_dialectic— ensures the fix doesnot resurrect genuinely stale content.
The existing
test_prefetch_skips_on_trivial_promptandtest_queue_prefetch_skips_on_trivial_promptcontinue to pass (no new work isfired on trivial turns).
Verification
The
honcho_searchrewrite was validated against a live Honcho instancewith real session history, using the exact query class that motivated the fix.
Before: a query-independent representation dump. After: the message containing
the actual stored fact ranks first, retrieved cheaply with no LLM call.
The trivial-turn fix was validated with a reproduction harness driving the real
on_turn_start → prefetch → queue_prefetchturn lifecycle: the pre-fix runshowed the result stranded on a trivial turn and then discarded as stale; the
post-fix run injects it on the trivial turn and clears the pending slot, with
separate probes confirming stale results are still discarded and trivial turns
still fire no new work.
Risk / compatibility
.envadditions, no new core model tools, nochange to the number/shape of exposed tools (descriptions and one dead param
only).
honcho_search's return format changes (ranked labeled snippets insteadof a representation blob), but its contract (a string of relevant context,
or empty) is unchanged, and it now matches the schema description.
peer_perspectivefilter has a graceful fallback to peer-authoredsearch for older Honcho versions.
the system prompt or tool schema count.
Files
plugins/memory/honcho/session.pysearch_contextrewired topeer_perspectivemessage search + budget + author labels + fallbackplugins/memory/honcho/__init__.py_consume_pending_dialectic), first-turn base/dialectic handling, empty-pass guard, all 5 tool descriptions, deadhoncho_contextquery param removedplugins/memory/honcho/client.pytimeoutresolutiontests/honcho_plugin/test_session.pyRelated issues
Fixes #29402 — "Honcho memory tools ignore focused query and token budget
controls." This PR resolves both reported symptoms:
honcho_search'smax_tokensis now an enforced budget (the issue's secondbullet), and the tool additionally now performs real ranked message search
rather than returning a representation blob.
honcho_context's misleadingqueryparameter is addressed — note thedivergence from the issue's proposed slice: Honcho memory tools ignore focused query and token budget controls #29402 proposed wiring the
query through to
peer.context(search_query=…). In the course of this workwe found that path returns the standing representation, not a message search,
so passing the query through would not give focused retrieval — it would
preserve the same misleading surface. Instead this PR removes the dead
parameter from
honcho_context(making it an honest no-query snapshot) androutes genuine focused retrieval through the now-fixed
honcho_search. If themaintainers prefer the wire-through approach for
honcho_contextspecifically, that's a trivial follow-up — but it would still want
honcho_searchto be the real message-search path this PR establishes. The{"result": ...}JSON shape is preserved as the issue requests.Validates (closed) #5667 — "honcho_search should query assistant-observed
user context." The
peer_perspectivedesign here independently arrives at thatconclusion: it spans all authors in the peer's sessions, so assistant-observed
facts about the user are included rather than dropped by author-filtering.
Related, not fixed:
Search Pollution." That issue concerns how the dialectic query string is
constructed (off-by-one on the latest user message); this PR fixes how a
generated dialectic result is consumed/injected and does not change
query construction. The two are complementary.