Skip to content

feat(honcho): expose representation retrieval knobs - #72076

Open
zcharef wants to merge 1 commit into
NousResearch:mainfrom
zcharef:honcho-retrieval-knobs
Open

feat(honcho): expose representation retrieval knobs#72076
zcharef wants to merge 1 commit into
NousResearch:mainfrom
zcharef:honcho-retrieval-knobs

Conversation

@zcharef

@zcharef zcharef commented Jul 26, 2026

Copy link
Copy Markdown

What I ran into

Peer context kept returning conclusions that had nothing to do with the
conversation. The same handful every turn, across unrelated questions.

The cause is Honcho's own budget split in crud/representation.py:

semantic = search_top_k or max_conclusions // 3   # driven by the query
frequent = max_conclusions // 3                   # ignores the query
recent   = max_conclusions - semantic - frequent  # ignores the query, ORDER BY created_at DESC

This plugin never sends search_top_k. With server defaults that yields 8
semantic, 8 frequent and 9 recent out of 25, so 17 of 25 conclusions arrive
without any query having influenced them.

On my store the recent slice was the worst offender. documents.created_at is
the derivation time, and a bulk import derives inside one window, so "the 9 most
recent" resolved to the same nine rows for every query I tried. A single 2023
consumer-credit email thread came back on three unrelated questions.

The change

Four fields, forwarded to peer.context() and peer.representation():
searchTopK, searchMaxDistance, maxConclusions, includeMostFrequent.

The SDK already accepts all four on both methods, and the server exposes them
(PeerRepresentationGet, plus query params on GET /peers/{id}/context). This
plugin was the only place dropping them. I matched the naming used by the Claude
Code plugin, which already ships searchTopK / searchMaxDistance /
maxConclusions, so anyone running both integrations sees one vocabulary.

Setting searchTopK >= maxConclusions drives the frequent and recent slices to
zero, which makes every injected conclusion one the query actually selected.

All four default to None and are left out of the call when unset, so an
existing install behaves exactly as it did before.

A smaller bug, fixed in passing

The peer.representation() fallback was not passing search_query. Any time
peer.context() raised, the fallback came back with a fully unfiltered
representation. It forwards the query now.

What the knobs actually do

Conclusions returned against a 1,475-conclusion store, searchTopK equal to
maxConclusions, sweeping the distance cutoff. Questions in French, stored
conclusions in English:

cutoff "communication preferences" "where do I live" "hello"
0.40 0 0 0
0.60 3 0 0
0.65 6 1 0
0.70 6 6 6
unset 12 12 12

Two things worth flagging for anyone tuning this. Cross-lingual distance is
wide: nothing at all comes back below 0.5 when the question and the corpus are
in different languages. And includeMostFrequent: false on its own makes the
noise worse rather than better, because that budget moves straight to the recent
slice, which ignores the query too. searchTopK is the field that fixes it. The
field description says so.

Verification

tests/test_honcho_retrieval_knobs.py covers the schema surface, host-over-root
precedence, unparseable input falling back to None, False surviving as
False rather than collapsing to unset, and the compatibility guarantee that an
unconfigured install sends an empty kwargs dict.

12 tests, all passing. I ran them through a small stand-in runner rather than
pytest: the box I verified on is a live agent whose venv ships no pytest, and
installing one there was not worth the blast radius. They follow the conventions
in tests/test_honcho_client_config.py, so CI should pick them up unchanged, but
they have not been through pytest itself.

Honcho splits its conclusion budget three ways in crud/representation.py:

    semantic = search_top_k or max_conclusions // 3   # driven by the query
    frequent = max_conclusions // 3                   # ignores the query
    recent   = max_conclusions - semantic - frequent  # ignores the query

This plugin never sends search_top_k, so with server defaults a peer context
call returns 8 semantic + 8 frequent + 9 recent out of 25: 17 of 25 conclusions
that no query influenced, drawn by ORDER BY created_at DESC. Because
documents.created_at holds the derivation time, a bulk import derives inside one
window and that recent slice resolves to the same rows for every query.

The SDK already accepts search_top_k, search_max_distance, max_conclusions and
include_most_frequent on both peer.context() and peer.representation(), and the
server exposes them. Only this plugin dropped them. The naming matches the
Claude Code plugin, which already ships searchTopK / searchMaxDistance /
maxConclusions.

Setting searchTopK >= maxConclusions drives the frequent and recent slices to
zero, so every injected conclusion is one the query selected.

All four fields default to None and are omitted from the call when unset, so an
existing deployment behaves exactly as before. Covered by
tests/test_honcho_retrieval_knobs.py.

Also fixes the peer.representation() fallback silently dropping search_query,
which made that path return a fully unfiltered representation whenever
peer.context() raised.
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have tool/memory Memory tool and memory providers comp/plugins Plugin system and bundled plugins labels Jul 26, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing this to the peer retrieval path. Current main still has the reported fallback gap: plugins/memory/honcho/session.py:1002-1006 calls peer.representation() without search_query, while the normal context call only forwards target and search_query (:986-992). The pinned honcho-ai==2.2.0 SDK accepts the proposed parameters (pyproject.toml:204; installed honcho/peer.py:569-647).

Problems

  • plugins/memory/honcho/client.py:166-179 accepts values outside the SDK bounds: top-K/max-conclusions require 1..100, and distance requires 0.0..1.0 (.venv/lib/python3.11/site-packages/honcho/peer.py:574-577, :644-647). Invalid values would make the new calls fail and be swallowed by the retrieval fallback handlers.
  • plugins/memory/honcho/client.py:192 makes a hand-authored "false" value truthy via bool(val). The new tests cover boolean False, not string input.
  • tests/test_honcho_retrieval_knobs.py:107-129 tests only _retrieval_kwargs(), not the changed peer.context() and fallback peer.representation() calls.

Suggested changes

  • Validate numeric ranges and parse explicit boolean strings deliberately.
  • Add recording-peer tests that assert all configured kwargs, including search_query, reach both the normal and fallback calls.

Automated hermes-sweeper review.

for val in (host_val, root_val):
if val is not None:
try:
return int(val)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please validate this optional integer against the pinned SDK contract before forwarding it. honcho-ai==2.2.0 requires search_top_k and max_conclusions in 1..100; values such as 0 or 101 make the later SDK call fail and the broad retrieval exception path returns no representation.

"""
for val in (host_val, root_val):
if val is not None:
return bool(val)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bool("false") is True, so a hand-authored Honcho JSON configuration cannot reliably disable this setting. Parse explicit boolean strings (while preserving None as omitted) and add a regression test for "false".

"searchTopK": 12,
"maxConclusions": 12,
"searchMaxDistance": 0.65,
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This verifies the helper only. Please add recording-peer coverage for _fetch_peer_context() so the normal peer.context() path and the peer.representation() fallback are both asserted to receive search_query plus these retrieval kwargs.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/memory Memory subsystem: store, providers, sync, background reviews labels Jul 30, 2026
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-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/memory Memory tool and memory providers type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants