Skip to content

fix(hindsight): surface the exception chain when the local runtime is unavailable - #61611

Open
falkoro wants to merge 1 commit into
NousResearch:mainfrom
falkoro:fix/hindsight-import-error-chain
Open

fix(hindsight): surface the exception chain when the local runtime is unavailable#61611
falkoro wants to merge 1 commit into
NousResearch:mainfrom
falkoro:fix/hindsight-import-error-chain

Conversation

@falkoro

@falkoro falkoro commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Diagnosability fix for the failure mode documented in #60783's second confirming report: when the local Hindsight runtime fails to import, _check_local_runtime() returned str(exc) — which drops the exception cause/context chain. Upstream, hindsight_api's LocalSTEmbeddings.initialize() catches the real error and re-raises a generic "sentence-transformers is required … pip install sentence-transformers" even when that package is installed — during the #60783 huggingface-hub downgrade the actual conflict (huggingface-hub>=1.5.0,<2.0 is required … found 1.2.3) lived only in the chained cause, and the surfaced message sent users reinstalling the wrong package.

Fix: a _format_exception_chain() helper walks __cause__/__context__ (cycle-safe, capped at 5 links) and renders the chain into the reason — which flows verbatim into _get_client()'s RuntimeError and availability messaging — plus a debug-level full traceback. The root cause becomes visible in one Hermes log read instead of requiring a dig through the Hindsight daemon's own profile logs.

Kept deliberately separate from #60797 (which fixes the downgrade itself) so each PR stays single-concern.

Related Issue

Diagnosability follow-up to #60783 (root cause fixed by #60797)

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • plugins/memory/hindsight/__init__.py — new _format_exception_chain(); _check_local_runtime() returns the rendered chain and logs the traceback at debug level
  • tests/plugins/memory/test_hindsight_provider.py — 5 new tests: explicit from e chain, implicit __context__ chain, single-exception shape, cycle safety, and the end-to-end _check_local_runtime reason containing the root cause

How to Test

  1. pytest tests/plugins/memory/test_hindsight_provider.py -q → 121 passed (5 new)
  2. Manual: with a broken hindsight import (e.g. the lazy_deps: huggingface-hub==1.2.3 pin conflicts with transformers >=1.5.0 requirement, breaks Hindsight local embeddings #60783 downgrade state), hermes memory init now reports ImportError: sentence-transformers is required … <- caused by: ImportError: huggingface-hub>=1.5.0,<2.0 is required … instead of only the misleading first half.

Checklist

Code

Documentation & Housekeeping

  • Relevant documentation — rationale documented in the helper docstring; N/A otherwise
  • cli-config.yaml.example — N/A (no config keys)
  • Cross-platform impact — pure Python exception formatting; platform-independent

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers P3 Low — cosmetic, nice to have labels Jul 9, 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 the narrowly scoped diagnosability fix. Current main still drops chained import failures at plugins/memory/hindsight/__init__.py:140, and that reason reaches both the local-mode warning (:1273-1278) and _get_client() RuntimeError (:1016-1021). The formatter and focused explicit-cause, implicit-context, single-error, cycle, and integration-path tests are appropriate.

Problems

  • tests/plugins/memory/test_hindsight_provider.py:1882 imports importlib as _importlib, but the test only uses hs.importlib; _importlib is unused.

Suggested changes

  • Remove the unused import at tests/plugins/memory/test_hindsight_provider.py:1882.

Automated hermes-sweeper review.

assert rendered.count("ValueError") == 2

def test_check_local_runtime_reason_contains_root_cause(self, monkeypatch):
import importlib as _importlib

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.

_importlib is never referenced; the test patches hs.importlib below. Please remove this unused import.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 11, 2026
@falkoro
falkoro force-pushed the fix/hindsight-import-error-chain branch from 511aada to 2a12e12 Compare July 15, 2026 13:39
@teknium1 teknium1 added the area/memory Memory subsystem: store, providers, sync, background reviews label Jul 19, 2026
@falkoro
falkoro force-pushed the fix/hindsight-import-error-chain branch 2 times, most recently from 4964ca0 to 00c26b1 Compare July 27, 2026 18:46
… unavailable

_check_local_runtime() returned str(exc), which drops the exception
cause/context chain. Upstream wrappers hide the real failure behind a
generic re-raise: hindsight_api's LocalSTEmbeddings.initialize() catches
the actual error and re-raises 'sentence-transformers is required ...
pip install sentence-transformers' even when that package IS installed
and the real problem is a dependency conflict underneath it. During the
package while the chained cause held the actual conflict
('huggingface-hub>=1.5.0,<2.0 is required ... found 1.2.3').

Walk __cause__/__context__ (cycle-safe, capped at 5 links) and render
the chain into the reason Hermes reports, plus a debug-level traceback.
The reason flows verbatim into the RuntimeError raised by _get_client()
and the availability messaging, so the root cause is now visible in one
log read instead of requiring a dig through the Hindsight daemon's own
profile logs.

Diagnosability follow-up to NousResearch#60783 (the downgrade itself is fixed by
@falkoro
falkoro force-pushed the fix/hindsight-import-error-chain branch from 00c26b1 to 30e430e Compare July 31, 2026 17:28
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-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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.

3 participants