feat(hindsight): memory provider improvements — recall_sync, retain_source, setup templates, memory indicators, error hints - #74379
Conversation
…Research#5820) By default auto-recall runs in the background at the end of a turn and is injected on the *next* turn, so `prefetch(query)` ignores the current query and returns the previous turn's result. When the topic shifts between turns (e.g. "fix linting" -> "fix tests") the injected memories can be stale. Add a `recall_sync` config flag (default `false`, so existing latency behavior is unchanged). When enabled, `prefetch()` runs a live recall against the *current* message and injects those results, and `queue_prefetch()` becomes a no-op (nothing to prime in the background). Refactors the recall body out of the `queue_prefetch` closure into a shared `_do_recall(query)` helper (plus `_recall_disabled()` / `_format_recall()`), used by both the async and synchronous paths. Tests: sync path recalls the current query synchronously and skips the background queue; the default path still ignores the current query and reads the buffer.
…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.
…tion The provider already stamps `metadata.source` on every retained memory from the `retain_source` setting, but it defaulted to "" — so Hermes-originated memories carried no source, and Hindsight had no clean signal that a memory came from Hermes. Default `retain_source` to "hermes" (via a new `_DEFAULT_RETAIN_SOURCE` constant used across the config-load, __init__, schema, and initialize defaults). Every retained memory now self-identifies as Hermes in `metadata.source`, which Hindsight returns on recall — enabling provenance and per-client analytics. Fully user-overridable: a `retain_source` in config.json or `HINDSIGHT_RETAIN_SOURCE` still wins.
`hermes memory setup` left the user with a blank bank. Add an optional step
(cloud / local_external) that fetches the Hindsight Bank Templates catalog,
shows the ones tagged for Hermes, and applies the chosen manifest to the
bank via the import API — so the agent's memory arrives pre-configured with a
mission, dispositions, mental models, and directives for its use case.
- New `plugins/memory/hindsight/templates.py`: fetch catalog (filtered to the
`hermes` integration), fetch a manifest, and POST it to
`/v1/default/banks/{bank}/import` (which creates the bank). Catalog source is
overridable via `HINDSIGHT_TEMPLATES_URL`.
- Wizard: after config is saved, offer a template picker (Blank is always an
option). Best-effort and non-fatal — network/apply failures just skip.
- Skipped for local_embedded (its daemon isn't running during setup).
- Tests cover the hermes filter, manifest URL resolution, the import POST
(endpoint + auth), and the picker orchestration (apply / blank / none / error).
…mplate step
Follow-ups on the setup-wizard starter-template step:
- Warn on re-apply: before applying a template, probe the bank (export
endpoint). If it already has config / mental models / directives, confirm
before overwriting ("Apply" vs "Keep existing"). Best-effort — a missing
bank or any probe error is treated as not-customized and proceeds.
- Testable mode gate: extract `supported_for_mode()` (cloud / local_external)
and use it in the wizard so local_embedded is provably skipped.
- Tests: apply-time failure (e.g. 401 for OAuth-only users) is swallowed with
a hint; the customization probe (config present / empty / error); and the
warn flow (keep-existing declines, confirm applies, fresh bank skips the
prompt). 16 tests total.
…sResearch#2765) A provider selected via `memory.provider` but reporting `is_available() == False` was dropped silently, leaving users with `memory.provider` set but no memory and no diagnostic. The most common trigger is systemd/gateway services not inheriting `~/.hermes/.env` (the CLI reads it via python-dotenv; services need explicit `Environment=`). - agent_init: emit a one-time, deduped warning naming the provider and the `.env`-inheritance gotcha. `is_available()` is a fast, side-effect-free hot-path check so it can't log for itself; dedup avoids the gateway's per-message AIAgent construction spamming the warning every turn. - hermes memory status: surface the systemd/`.env` root cause in the "not available" block, alongside the existing missing-env-var checklist. - tests for both paths.
|
Nice, its need |
alt-glitch
added
type/feature
Auto-recall injects memory into the prompt, but whether the user SEES that Hindsight contributed was left to the model — and models (e.g. gpt-5.5) routinely decline to mention it, so memory looks like it isn't working even when it is. Surface it deterministically instead: when prefetch injects memory, Hermes itself emits a '🧠 Hindsight — recalled N memories' status line via _emit_status (the same model-independent channel as compression/idle notices). It always shows and can't be silently dropped by the model. - MemoryProvider grows an opt-in recall_status() -> RecallStatus hook (default None); MemoryManager.describe_recall() aggregates + formats. - Hindsight provider persists the recall count alongside the prefetch block and reports it; reflect mode has no discrete count so it renders generic. - On by default with an off switch (recall_indicator=false) for customer-facing agents. - Fast, deterministic unit tests at all three layers (provider count/stale/ off/reflect, manager formatting/aggregation, turn-loop emit wiring).
Two follow-ups to the recall indicator, from live testing: 1. Use the Hindsight brand mark (the logo is an eye) instead of the brain emoji. Factored to INDICATOR_GLYPH + a glyph field on RecallStatus so the manager renders whatever the provider brands with — one source of truth. 2. Companion retain indicator: '👁️ Hindsight — saving to memory…' emitted the moment a turn is dispatched to the writer (past every skip/buffer gate, so it only fires on real writes). Retain runs in the background with no synchronous fact count, so this is a presence signal, not a count. Emitted via the agent status channel (agent._emit_status), injected into the provider through initialize(status_callback=). On by default with a retain_indicator off switch, mirroring recall_indicator. Tests: 6 retain-indicator cases (dispatch/off/auto-retain-off/buffered/ no-callback/init-wiring); recall + turn-loop tests updated to the eye glyph. All green; ruff clean.
benfrank241
changed the title
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for consolidating the Hindsight improvements. The current-main premise is still present for current-turn recall and unavailable-provider visibility, but two changes need revision before salvage.
Problems
- The
hindsight-allhint added atplugins/memory/hindsight/__init__.py:1376is insideinitialize(). Current main callsis_available()first (plugins/memory/hindsight/__init__.py:731-737) and only initializes a provider after it succeeds (agent/agent_init.py:1645-1647), so a missing embedded runtime never reaches this hint. - The new default source attribution conflicts with the explicit opt-in policy in
AGENTS.md:118-121. The changedretain_sourcedefault should remain unset until the required generic opt-in exists. plugins/memory/hindsight/README.md:91-100still documentsretain_sourceas optional and does not cover the new recall/indicator/template settings.
Suggested changes
- Move the package-specific diagnosis to the unavailable-provider path and test that path through agent initialization.
- Keep attribution opt-in, then update the Hindsight configuration documentation for the remaining settings.
Automated hermes-sweeper review.
| "Hindsight local mode disabled because its runtime could not be imported: %s", | ||
| "Hindsight local mode disabled because its runtime could not be imported: %s.%s", | ||
| reason, | ||
| _local_runtime_hint(reason), |
There was a problem hiding this comment.
initialize() is not reached when this condition matters: is_available() already calls _check_local_runtime() and agent_init only initializes providers that report available. Surface this package-specific reason from the unavailable-provider gate (or a provider unavailable-reason API) so a missing hindsight-all installation receives the actionable hint.
| self._recall_tags_match = self._config.get("recall_tags_match", "any") | ||
| self._retain_source = str( | ||
| self._config.get("retain_source") or os.environ.get("HINDSIGHT_RETAIN_SOURCE", "") | ||
| self._config.get("retain_source") or os.environ.get("HINDSIGHT_RETAIN_SOURCE", _DEFAULT_RETAIN_SOURCE) |
There was a problem hiding this comment.
This makes third-party metadata.source attribution opt-out by default. AGENTS.md:118-121 requires a generic user-facing opt-in before adding attribution tags; keep the default empty unless that generic mechanism is available.
… retain_source, docs Addresses hermes-sweeper review on NousResearch#74379: 1. Local-embedded install hint was unreachable. is_available() gates initialization, so a missing embedded runtime never reached the hint in initialize() (NousResearch#7718). Add MemoryProvider.unavailable_reason() (default ""), implement it in the Hindsight provider, and have agent_init's provider-unavailable warning surface it — the path that actually runs when a provider reports unavailable. Tested through that path. 2. retain_source no longer defaults to "hermes". AGENTS.md forbids on-by-default third-party attribution tags until a generic opt-in exists; default is now empty and metadata.source is stamped only when the user sets retain_source (config key / env var still honored). 3. README: document recall_sync / recall_indicator / retain_indicator, the starter-template setup step, and clarify retain_source is opt-in.
|
Thanks for the review — all three points were valid; addressed in 1. Unreachable 2. 3. README. Fixed the Full affected suite green; |
…-improvements # Conflicts: # tests/plugins/memory/test_hindsight_provider.py
|
@teknium1 — all three points from the hermes-sweeper review are addressed in |
teknium1
added
the
sweeper:risk-message-delivery
teknium1
added
sweeper:risk-security-boundary
The merge with main left a redundant, unconditional _init_kwargs["status_callback"] assignment alongside main's CLI-gated one (added in the 308-commit catch-up). main only wires status_callback for platform=="cli"; the unconditional copy leaked it into gateway provider init and broke test_aiagent_forwards_user_id_alt_to_memory_provider (platform=feishu asserts status_callback absent). Drop the duplicate — the retain indicator only needs it on the interactive CLI, and no-ops gracefully when absent on gateways.
…-improvements # Conflicts: # plugins/memory/hindsight/__init__.py
|
Rebased on latest @teknium1 — ready for another look whenever you have a moment. Happy to squash or split if that helps review. |
|
I reproduced the required-check failure on exact head Root cause: The focused fix is to make this test use a substantive query, e.g. |
… tests The merge with main brought in the is_trivial_prompt gate: build_turn_context skips prefetch_all() for trivial prompts. The recall-indicator turn tests used the _build() helper default user_message='hello', which is now trivial, so prefetch never ran and test_recall_indicator_emitted_when_memory_injected failed deterministically. Give both indicator tests a substantive query so prefetch actually runs — the positive test now exercises the emit path, and the negative test exercises the 'prefetch ran but returned nothing' path (rather than passing by being skipped as trivial). Test-only; no production change. Thanks @stepanov1975 for the precise root-cause. tests/agent/test_turn_context.py: 10/10 pass.
|
Thanks @stepanov1975 — spot-on diagnosis. The merge with Fixed in |
alt-glitch
added
sweeper:risk-session-state
…ource, setup templates, memory indicators, error hints Bundles previously-separate Hindsight/memory PRs into a single review surface: - opt-in synchronous recall (recall_sync) — recall the injected memory in-turn instead of next-turn prefetch (#5820) - actionable error when local_embedded runtime is missing — tells the user which package to install (#7718) - default retain_source to 'hermes' so every stored memory self-identifies its provenance - offer a starter memory template during hermes memory setup, plus warn before overwriting an already-configured bank - warn when a configured memory provider reports unavailable (#2765) - deterministic 'recalled N memories' recall indicator — Hermes itself emits a status line when auto-recall injects memory - 'saving to memory' retain indicator — emitted the moment a turn is dispatched to the writer Authored by @benfrank241 (ben.bartholomew@vectorize.io). Salvaged from PR #74379.
1. Use open_credentialed_url() instead of bare urlopen() in templates.py apply_template() and probe_existing_customization(). Both send Authorization: Bearer headers; bare urlopen forwards credentials on cross-origin redirects. The codebase has open_credentialed_url() in hermes_cli/urllib_security.py that strips credentials on cross-origin redirects — used by 4 other modules. 2. Guard unavailable_reason() with the dedup set check before calling it. The gateway builds a fresh AIAgent per message, so without this guard unavailable_reason() (which calls _load_config() → stat + file read + JSON parse, and _check_local_runtime() → importlib probes) runs on every gateway turn for an unavailable provider, even though the warning is deduped after the first. 3. Move INDICATOR_GLYPH from Hindsight's eye emoji to a generic brain (🧠) in core (agent/memory_provider.py). Hindsight overrides with its own _HINDSIGHT_GLYPH (👁️) in recall_status() and _emit_saving_indicator(). Other memory providers no longer inherit Hindsight's brand mark as the default glyph.
|
Merged via #85494 using this PR's work. Your commits were cherry-picked with authorship preserved (rebase-merge). Thanks for the comprehensive Hindsight memory improvements! |
6 participants
Bundles previously-separate Hindsight/memory PRs into a single review surface. Each was independently mergeable; combining them avoids parallel reviews of the same file (
plugins/memory/hindsight/__init__.py, which most of them touch).What's included
recall_sync) — recall the injected memory in-turn instead of next-turn prefetchlocal_embeddedruntime is missing — tells the user exactly which package to install instead of a bare import failureretain_sourceto"hermes"so every stored memory self-identifies its provenance (metadata.source: "hermes")hermes memory setup, plus warn before overwriting an already-configured bank👁️ Hindsight — recalled N memoriesvia the status channel when auto-recall injects memory, so the user SEES memory working regardless of whether the model mentions it (recall_indicator, on by default)👁️ Hindsight — saving to memory…emitted the moment a turn is dispatched to the writer, only on real writes (retain_indicator, on by default)Why the indicators
Auto-recall injects memory into the prompt, but whether the user can see that Hindsight contributed was previously left to the model — and models routinely decline to mention it, so memory looks like it isn't working even when it is. These indicators are rendered by Hermes itself (
_emit_status, the same model-independent channel as compression/idle notices), so they always show and can't be silently dropped. Both have off switches for customer-facing agents.Scope
plugins/memory/hindsight/**(+ their tests).agent/agent_init.py,agent/memory_manager.py,agent/memory_provider.py,agent/turn_context.py, andhermes_cli/memory_setup.py— the recall indicator adds an opt-inrecall_status()hook to the baseMemoryProviderand adescribe_recall()aggregator onMemoryManager; the status channel is wired to the provider viainitialize(status_callback=…).Tests
New/updated deterministic tests across all changes (provider behaviour incl. the recall_sync × indicator interaction, retain-indicator dispatch/off/buffered paths, template setup, local-runtime hint, provider-unavailable warning, env-hint, manager formatting, turn-loop emit wiring). Full affected suite green;
ruffclean.