-
Notifications
You must be signed in to change notification settings - Fork 46.5k
feat(honcho): expose representation retrieval knobs #72076
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -153,6 +153,46 @@ def _parse_context_tokens(host_val, root_val) -> int | None: | |
| return None | ||
|
|
||
|
|
||
| def _parse_optional_int(host_val, root_val) -> int | None: | ||
| """Parse an optional integer config: host wins, then root, then None. | ||
|
|
||
| None means "do not send this parameter", preserving Honcho's server-side | ||
| default. Used for the representation retrieval knobs, where omitting the | ||
| field must behave exactly as before it existed. | ||
| """ | ||
| for val in (host_val, root_val): | ||
| if val is not None: | ||
| try: | ||
| return int(val) | ||
| except (ValueError, TypeError): | ||
| pass | ||
| return None | ||
|
|
||
|
|
||
| def _parse_optional_float(host_val, root_val) -> float | None: | ||
| """Parse an optional float config: host wins, then root, then None.""" | ||
| for val in (host_val, root_val): | ||
| if val is not None: | ||
| try: | ||
| return float(val) | ||
| except (ValueError, TypeError): | ||
| pass | ||
| return None | ||
|
|
||
|
|
||
| def _parse_optional_bool(host_val, root_val) -> bool | None: | ||
| """Parse an optional bool config: host wins, then root, then None. | ||
|
|
||
| Distinct from ``_resolve_bool``, which collapses an unset field onto a | ||
| default. Here the tri-state is load-bearing: None must stay None so the | ||
| parameter is left out of the API call entirely. | ||
| """ | ||
| for val in (host_val, root_val): | ||
| if val is not None: | ||
| return bool(val) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| return None | ||
|
|
||
|
|
||
| def _parse_int_config(host_val, root_val, default: int) -> int: | ||
| """Parse an integer config: host wins, then root, then default.""" | ||
| for val in (host_val, root_val): | ||
|
|
@@ -392,6 +432,21 @@ class HonchoClientConfig: | |
| write_frequency: str | int = "async" | ||
| # Prefetch budget (None = no cap; set to an integer to bound auto-injected context) | ||
| context_tokens: int | None = None | ||
| # Representation retrieval knobs. All None by default: omitted from the call, | ||
| # so Honcho's server-side defaults apply and behaviour is unchanged. | ||
| # | ||
| # Honcho splits the conclusion budget three ways (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 | ||
| # With the defaults that is 8 + 8 + 9 of 25, i.e. 17 of 25 conclusions that no | ||
| # query influenced, drawn by `ORDER BY created_at DESC`. Setting | ||
| # search_top_k >= max_conclusions drives `frequent` and `recent` to zero, so | ||
| # every injected conclusion is one the query actually selected. | ||
| search_top_k: int | None = None | ||
| search_max_distance: float | None = None | ||
| max_conclusions: int | None = None | ||
| include_most_frequent: bool | None = None | ||
| # Dialectic (peer.chat) settings | ||
| # reasoning_level: "minimal" | "low" | "medium" | "high" | "max" | ||
| dialectic_reasoning_level: str = "low" | ||
|
|
@@ -625,6 +680,22 @@ def from_global_config( | |
| host_block.get("contextTokens"), | ||
| raw.get("contextTokens"), | ||
| ), | ||
| search_top_k=_parse_optional_int( | ||
| host_block.get("searchTopK"), | ||
| raw.get("searchTopK"), | ||
| ), | ||
| search_max_distance=_parse_optional_float( | ||
| host_block.get("searchMaxDistance"), | ||
| raw.get("searchMaxDistance"), | ||
| ), | ||
| max_conclusions=_parse_optional_int( | ||
| host_block.get("maxConclusions"), | ||
| raw.get("maxConclusions"), | ||
| ), | ||
| include_most_frequent=_parse_optional_bool( | ||
| host_block.get("includeMostFrequent"), | ||
| raw.get("includeMostFrequent"), | ||
| ), | ||
| dialectic_reasoning_level=( | ||
| host_block.get("dialecticReasoningLevel") | ||
| or raw.get("dialecticReasoningLevel") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| """Tests for the Honcho representation retrieval knobs. | ||
|
|
||
| Honcho splits its conclusion budget three ways, and only the semantic slice is | ||
| driven by the query. These knobs let a deployment collapse the budget onto that | ||
| slice. The load-bearing property is the last test in each class: leaving the | ||
| keys unset must send nothing, so upgrading changes no behaviour. | ||
| """ | ||
|
|
||
| import json | ||
|
|
||
| from plugins.memory.honcho.client import HonchoClientConfig | ||
| from plugins.memory.honcho.session import HonchoSessionManager | ||
| from plugins.memory.config_schema import ( | ||
| KIND_BOOL, | ||
| KIND_NUMBER, | ||
| get_provider_config_schema, | ||
| ) | ||
|
|
||
|
|
||
| RETRIEVAL_KEYS = { | ||
| "searchTopK", | ||
| "searchMaxDistance", | ||
| "maxConclusions", | ||
| "includeMostFrequent", | ||
| } | ||
|
|
||
|
|
||
| def _write(tmp_path, host_block, root=None): | ||
| config_path = tmp_path / "config.json" | ||
| payload = { | ||
| "apiKey": "test-api-key-12345", | ||
| "hosts": {"hermes": {"workspace": "w", "aiPeer": "a", "peerName": "p", | ||
| **host_block}}, | ||
| } | ||
| payload.update(root or {}) | ||
| config_path.write_text(json.dumps(payload)) | ||
| return HonchoClientConfig.from_global_config(host="hermes", config_path=config_path) | ||
|
|
||
|
|
||
| class TestRetrievalSchema: | ||
| """The knobs are declared, so the setup wizard and config UI can reach them.""" | ||
|
|
||
| def test_keys_are_declared(self): | ||
| provider = get_provider_config_schema("honcho") | ||
| assert provider is not None | ||
| assert RETRIEVAL_KEYS <= {field.key for field in provider.fields} | ||
|
|
||
| def test_kinds(self): | ||
| provider = get_provider_config_schema("honcho") | ||
| assert provider is not None | ||
| by_key = {f.key: f for f in provider.fields} | ||
| assert by_key["searchTopK"].kind == KIND_NUMBER | ||
| assert by_key["maxConclusions"].kind == KIND_NUMBER | ||
| assert by_key["searchMaxDistance"].kind == KIND_NUMBER | ||
| assert by_key["includeMostFrequent"].kind == KIND_BOOL | ||
|
|
||
| def test_stay_out_of_the_inline_panel(self): | ||
| """Tuning knobs belong in the modal, not the compact panel.""" | ||
| provider = get_provider_config_schema("honcho") | ||
| assert provider is not None | ||
| assert not (RETRIEVAL_KEYS & {f.key for f in provider.inline_fields()}) | ||
|
|
||
|
|
||
| class TestRetrievalConfigParsing: | ||
| def test_values_parse(self, tmp_path): | ||
| cfg = _write(tmp_path, { | ||
| "searchTopK": 12, | ||
| "maxConclusions": 12, | ||
| "searchMaxDistance": 0.65, | ||
| "includeMostFrequent": False, | ||
| }) | ||
| assert cfg.search_top_k == 12 | ||
| assert cfg.max_conclusions == 12 | ||
| assert cfg.search_max_distance == 0.65 | ||
| assert cfg.include_most_frequent is False | ||
|
|
||
| def test_host_block_wins_over_root(self, tmp_path): | ||
| cfg = _write(tmp_path, {"searchTopK": 6}, root={"searchTopK": 40}) | ||
| assert cfg.search_top_k == 6 | ||
|
|
||
| def test_root_applies_when_host_is_silent(self, tmp_path): | ||
| cfg = _write(tmp_path, {}, root={"searchTopK": 40}) | ||
| assert cfg.search_top_k == 40 | ||
|
|
||
| def test_unparseable_values_fall_back_to_none(self, tmp_path): | ||
| """A typo must not crash the provider at startup.""" | ||
| cfg = _write(tmp_path, {"searchTopK": "abc", "searchMaxDistance": "x"}) | ||
| assert cfg.search_top_k is None | ||
| assert cfg.search_max_distance is None | ||
|
|
||
| def test_unset_stays_none(self, tmp_path): | ||
| cfg = _write(tmp_path, {}) | ||
| assert cfg.search_top_k is None | ||
| assert cfg.search_max_distance is None | ||
| assert cfg.max_conclusions is None | ||
| assert cfg.include_most_frequent is None | ||
|
|
||
|
|
||
| class TestRetrievalKwargs: | ||
| """What actually reaches peer.context() and peer.representation().""" | ||
|
|
||
| def test_configured_values_are_forwarded(self, tmp_path): | ||
| cfg = _write(tmp_path, { | ||
| "searchTopK": 12, | ||
| "maxConclusions": 12, | ||
| "searchMaxDistance": 0.65, | ||
| }) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This verifies the helper only. Please add recording-peer coverage for |
||
| mgr = HonchoSessionManager(config=cfg) | ||
| assert mgr._retrieval_kwargs() == { | ||
| "search_top_k": 12, | ||
| "max_conclusions": 12, | ||
| "search_max_distance": 0.65, | ||
| } | ||
|
|
||
| def test_false_is_forwarded_not_dropped(self, tmp_path): | ||
| """include_most_frequent is tri-state: False differs from unset.""" | ||
| cfg = _write(tmp_path, {"includeMostFrequent": False}) | ||
| mgr = HonchoSessionManager(config=cfg) | ||
| assert mgr._retrieval_kwargs() == {"include_most_frequent": False} | ||
|
|
||
| def test_unset_sends_nothing(self, tmp_path): | ||
| """The compatibility guarantee: no keys configured, no parameters sent.""" | ||
| cfg = _write(tmp_path, {}) | ||
| mgr = HonchoSessionManager(config=cfg) | ||
| assert mgr._retrieval_kwargs() == {} | ||
|
|
||
| def test_no_config_at_all_sends_nothing(self): | ||
| mgr = HonchoSessionManager() | ||
| assert mgr._retrieval_kwargs() == {} | ||
There was a problem hiding this comment.
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.0requiressearch_top_kandmax_conclusionsin 1..100; values such as 0 or 101 make the later SDK call fail and the broad retrieval exception path returns no representation.