fix(hindsight): actionable error when local_embedded runtime is missing (#7718) - #70295
Closed
benfrank241 wants to merge 1 commit into
Closed
Conversation
…ng (NousResearch#7718) local_embedded imports `from hindsight import HindsightEmbedded`, which is provided only by the `hindsight-all` package. plugin.yaml declares only `hindsight-client` (enough for cloud / local_external), so a user who selects local_embedded without running `hermes memory setup` — a hand-written config, the legacy `"mode": "local"` alias, or a restored backup — hits `ModuleNotFoundError: No module named 'hindsight'`. `initialize()` already disables the provider with one warning in this case (so the silent per-sync failure from the original report is gone), but the message just echoes `No module named 'hindsight'` with no fix. Add an actionable hint telling the user to install `hindsight-all` (or run `hermes memory setup`), plus the distinction from `hindsight-client`. Kept as a runtime hint rather than declaring `hindsight-all` in plugin.yaml: that package pulls the full server stack (hindsight-api-slim[all], torch), so declaring it unconditionally would bloat every cloud-only install.
Contributor
Author
|
Superseded by the combined PR #74379 — same changes bundled into a single review surface. Closing this one to consolidate. |
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.
Addresses #7718.
Background (verified against PyPI)
local_embeddedrunsfrom hindsight import HindsightEmbedded. I confirmed against the published wheels (0.8.5):hindsight-allships the top-levelhindsight/package —hindsight/__init__.pydoesfrom .embedded import HindsightEmbeddedand exports it in__all__. This is the only package that satisfies the import.hindsight-embedships onlyhindsight_embed/— no top-levelhindsight, noHindsightEmbeddedclass (its deps are just httpx + rich).hindsight-client(whatplugin.yamldeclares) provideshindsight_client.*— enough for cloud / local_external only.So a user who selects
local_embeddedwithout runninghermes memory setup(hand-written config, the legacy"mode": "local"alias, or a restored backup — the wizard is the only path that installshindsight-all) hitsModuleNotFoundError: No module named 'hindsight'.What
mainalready does vs. what's missinginitialize()already calls_check_local_runtime()and, when the import fails, logs one warning and disables the provider — so the silent per-sync memory loss and fake-healthy INFO log from the original v0.8.0 report are already gone. What's missing is that the warning just echoesNo module named 'hindsight'with no fix.Change
Add
_local_runtime_hint()and append it to the disable warning. When the failure is a missinghindsight/hindsight_embedmodule, the user now sees:Unrelated runtime failures (e.g. the NumPy-on-old-CPU case
_check_local_runtimealso guards) get no hint, so the message stays accurate.Why not declare
hindsight-allinplugin.yaml? It pulls the full server stack (hindsight-api-slim[all], torch/sentence-transformers). Declaring it unconditionally would bloat every cloud-only install with hundreds of MB they don't need. A runtime hint fixes the diagnosis for local users without penalizing the majority. (A mode-conditionalpip_dependenciesmechanism would be the clean long-term fix, but that's a plugin-loader change out of scope here.)Tests
tests/plugins/memory/test_hindsight_local_runtime_hint.py:hindsight/hindsight_embed→ hint nameshindsight-all+hermes memory setup+ the interpreter pathNone) → no hint(The 2 pre-existing
test_hindsight_provider.py::TestAvailability/TestConfigfailures reproduce on a cleanmainand are unrelated.)Note on #57243
#57243 targets the same issue but adds
hindsight-embedtoplugin.yamlto satisfy the import. Per the wheel contents above,hindsight-embedprovides neither the top-levelhindsightmodule norHindsightEmbedded, so that approach would stillModuleNotFoundError. Flagging so the two aren't merged in conflict.