Skip to content

fix: add missing hindsight dependencies for local_embedded mode - #17851

Open
yesxcv wants to merge 1 commit into
NousResearch:mainfrom
yesxcv:fix/hindsight-missing-deps
Open

fix: add missing hindsight dependencies for local_embedded mode#17851
yesxcv wants to merge 1 commit into
NousResearch:mainfrom
yesxcv:fix/hindsight-missing-deps

Conversation

@yesxcv

@yesxcv yesxcv commented Apr 30, 2026

Copy link
Copy Markdown

Summary

plugins/memory/hindsight/plugin.yaml only declared hindsight-client as a dependency, but the local_embedded code path actually imports from two more packages:

  • hindsight-embedhindsight_embed.daemon_embed_manager (line 86, 1003)
  • hindsight-allhindsight.HindsightEmbedded (line 766)

Without these, _install_dependencies() skips them, _check_local_runtime() fails, and hermes memory status reports "not available" even when the daemon is healthy.

Fix

Added hindsight-embed>=0.5.0 and hindsight-all>=0.5.0 to pip_dependencies.

Test Plan

  • Installed missing packages manually, verified hermes memory status shows "available ✓"
  • Verified from hindsight import HindsightEmbedded succeeds
  • Verified import hindsight_embed.daemon_embed_manager succeeds

The plugin.yaml only declared hindsight-client, but the local_embedded
code path also needs hindsight-embed (for daemon_embed_manager) and
hindsight-all (for HindsightEmbedded client class). Without these,
hermes memory status reports 'not available' even when the daemon is
healthy, and HindsightEmbedded import fails at runtime.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers labels Apr 30, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #7718 — same root cause: plugin.yaml missing hindsight-all/hindsight-embed for local_embedded mode. See also PR #13101 and closed #7745.

@nicoloboschi nicoloboschi 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.

if you use cloud or self hosted, this change is going to bloat your venv for nothing

@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 the local-embedded dependency path. The reported manual-configuration failure remains relevant: current main disables local_embedded when _check_local_runtime() cannot import its runtime (plugins/memory/hindsight/__init__.py:1269-1280).

Problems

  • The manifest is installed before mode selection (hermes_cli/memory_setup.py:276-283), so adding both packages to pip_dependencies installs the local stack for cloud and local_external users too. Current Hindsight setup intentionally installs hindsight-all only after local_embedded is selected (plugins/memory/hindsight/__init__.py:793-801).
  • hindsight-all already depends on hindsight-embed according to its PyPI metadata, so the separate hindsight-embed manifest entry is redundant.
  • The diff adds no regression test for the manually configured missing-runtime path.

Suggested changes

  • Rework this as a mode-aware recovery/install path rather than an unconditional manifest union.
  • Add tests for manual local configuration plus cloud/local_external non-regression.

Automated hermes-sweeper review.

description: "Hindsight — long-term memory with knowledge graph, entity resolution, and multi-strategy retrieval."
pip_dependencies:
- "hindsight-client>=0.4.22"
- "hindsight-embed>=0.5.0"

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.

_install_dependencies() runs before this provider selects its mode (hermes_cli/memory_setup.py:276-283), so this makes cloud and local_external setups install the embedded stack too. hindsight-all already depends on hindsight-embed; please preserve the existing mode-aware install path instead of adding it to the unconditional manifest.

@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 12, 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/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants