Skip to content

Feat/hindsight smart retain pipeline pluggable - #29857

Closed
McClean wants to merge 3 commits into
NousResearch:mainfrom
McClean-codes:feat/hindsight-smart-retain-pipeline-pluggable
Closed

McClean wants to merge 3 commits into
NousResearch:mainfrom
McClean-codes:feat/hindsight-smart-retain-pipeline-pluggable

Conversation

@McClean

@McClean McClean commented May 21, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds a configurable client-side smart retain pipeline to the Hindsight memory plugin to reduce write amplification and improve recall quality at scale. Without this, every hindsight_retain call hits the bank — duplicates, near-duplicates, and noisy raw-text retains all land as separate units, the bank grows linearly with chatter, and recall quality degrades. With this, retains are deduped, extracted, and delta-merged client-side before they reach the API, and the whole pipeline runs through a dedicated auxiliary task slot so the classifier model can be pinned cheaply (gpt-oss-20b / gemma-3-flash) without dragging the main chat model into pre-retain work.

The pipeline is opt-in via plugin config and degrades cleanly: if the auxiliary call fails repeatedly, the circuit breaker opens and (per aux_fallback_to_main knob) either falls back to the main model or skips the smart step and writes raw.

Depends on #29817 (feat(plugins): add register_auxiliary_task()). That PR adds the PluginContext.register_auxiliary_task() API, which this plugin uses to declare its memory_retain_filter slot without modifying core files. Don't merge this until the parent lands; once it does, this branch rebases cleanly onto the merge commit, and the dependency commit drops out.

Related Issue

N/A — companion PR to #29817.

Fixes #

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)

Changes Made

  • plugins/memory/hindsight/__init__.py — adds the smart retain pipeline:
    • Pre-retain dedup via recall + auxiliary LLM classifier (skips if a near-duplicate already exists)
    • Client-side extraction (retain_extract) — auxiliary LLM rewrites raw retain text into a normalised memory unit before it hits the bank
    • Delta retain mode — appends only the diff against the most-recent unit on the same scope, keeping bank growth linear
    • Dynamic scope filtering on both recall and retain, so units carry consistent scope tags
    • Auxiliary circuit breaker with aux_fallback_to_main knob — opens after N consecutive aux failures, optionally re-routes to main
    • Smart pipeline applied to both tool retains and the flush-on-switch path (so memory bridging across conversation switches uses the same dedup logic)
    • Bank config fetch from API for richer pre-filter prompts (when api_url is configured)
    • Warning on incomplete bank config when the smart pipeline is active, so misconfiguration fails loudly
    • register(ctx) calls ctx.register_auxiliary_task(key="memory_retain_filter", display_name="Memory retain filter", description="pre-retain content classification (Hindsight smart pipeline)", defaults={"timeout": 30, ...}) — declares the aux task slot through the plugin surface, no core edits
    • Default change: retain_every_n_turns 5 → 1 (smart pipeline is opt-in via the smart-pipeline config block; sampling default tightened so the pipeline sees every turn when it's on)
    • Recall-type filtering in the dedup check (only compares against retained units, not extracted-snippet hits)
    • Bank config fetch gated on api_url presence only (was previously coupled to other flags)
  • tests/agent/test_memory_session_switch.py — adds 11 lines covering test fixture restoration for the new retain-pipeline attributes on the memory provider, so cross-session switch tests don't leak state between runs
  • tests/plugins/memory/test_hindsight_provider.py — adds 8 lines covering the new pipeline knobs

How to Test

  1. Land feat(plugins): add register_auxiliary_task() to PluginContext API #29817 first, then rebase this branch onto the merge commit (git rebase main — the dependency commit 2d94abe9a will drop out cleanly)
  2. pytest tests/plugins/memory/test_hindsight_provider.py tests/agent/test_memory_session_switch.py -q — all pass
  3. Configure the smart pipeline in ~/.hermes/config.yaml:

yaml
plugins:
memory_provider:
hindsight:
smart_pipeline:
enabled: true
dedup: true
extract: true
delta: true
aux_fallback_to_main: false

  1. Run hermes modelConfigure auxiliary models and confirm memory_retain_filter appears in the picker (provided by the registration API in the parent PR). Pin it to a cheap classifier model
  2. Start a chat, retain something, then retain a near-duplicate of it — confirm the second retain is skipped (look for dedup: skipped in the agent log)
  3. Retain a long raw paragraph — confirm it lands as a normalised extracted unit, not the raw text
  4. Retain twice on the same scope with overlapping content — confirm the second unit is a delta against the first, not a full copy
  5. Force the auxiliary endpoint to fail (point it at an invalid base_url) and confirm the circuit breaker opens after the configured threshold and aux_fallback_to_main is honoured
  6. Trigger a session switch with pending retains and confirm the flush path runs through the same smart pipeline (not bypassed)

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (feat(hindsight):)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits — once rebased on the parent merge commit)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (test fixture restoration + provider knobs)
  • I've tested on my platform: Ubuntu 24.04

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — pipeline knobs are documented inline in the plugin's config schema and docstrings
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A (plugin-owned config block under plugins.memory_provider.hindsight.smart_pipeline, schema documented in the plugin)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A (additive, plugin-internal)
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — pure Python, no path/process assumptions
  • I've updated tool descriptions/schemas if I changed tool behaviour — N/A (hindsight_retain / hindsight_recall tool surface is unchanged; the pipeline is a client-side wrapper)

Backwards Compatibility

Non-breaking and opt-in. The smart pipeline is gated behind plugins.memory_provider.hindsight.smart_pipeline.enabled (default off). With it off, the plugin's retain/recall paths are byte-identical to the current behaviour. The retain_every_n_turns default change (5 → 1) only affects users who already have the smart pipeline on, since the pipeline's whole point is to make every-turn retains cheap.

The memory_retain_filter aux task is declared via the parent PR's plugin registration API — no core files are modified; the slot exists only when the Hindsight plugin is loaded.

Screenshots / Logs

N/A — internal pipeline, no UI surface beyond hermes model (where memory_retain_filter appears as another row in the auxiliary picker, contributed by the parent PR).

Auxiliary LLM tasks (vision, compression, web_extract, etc.) currently
require modifications to core files for any plugin that needs its own
task slot — specifically the _AUX_TASKS list in hermes_cli/main.py and
the hardcoded env-var bridging dict in gateway/run.py. This violates
the 'plugins must not modify core files' rule and forces every memory
or context plugin that wants its own auxiliary task to either fork
core or open a coupled core+plugin PR.

This change adds a generic plugin surface for auxiliary task
registration:

    ctx.register_auxiliary_task(
        key='memory_retain_filter',
        display_name='Memory retain filter',
        description='hindsight pre-retain dedup/extract',
        defaults={'timeout': 30, 'extra_body': {'reasoning_effort': 'low'}},
    )

After registration, the task automatically:

  - Appears in 'hermes model → Configure auxiliary models' picker via
    a new _all_aux_tasks() merge of built-in + plugin tasks
  - Has its provider/model/base_url/api_key bridged from config.yaml
    to AUXILIARY_<KEY_UPPER>_* env vars at gateway startup
    (gateway/run.py now uses a dynamic bridged-keys set instead of
    a hardcoded per-task dict)
  - Gets plugin-declared defaults (timeout, extra_body, etc.) layered
    underneath user config so unconfigured plugin tasks still work
    (agent/auxiliary_client._get_auxiliary_task_config)
  - Resets to auto via 'Reset all to auto' alongside built-ins

Validation:

  - Rejects shadowing of built-in keys (vision, compression, etc.)
  - Rejects invalid key shapes (must match [A-Za-z0-9_]+)
  - Rejects cross-plugin collisions (clear error)
  - Allows same-plugin re-registration (idempotent updates)

Plugin discovery failures (rare) fall back gracefully — the aux
config UI still shows built-in tasks if get_plugin_auxiliary_tasks()
raises, and gateway env-var bridging keeps working for built-ins.

Built-in tasks remain hardcoded in _AUX_TASKS for stability — they're
the baseline UX, and DEFAULT_CONFIG already ships their defaults.
Plugin tasks layer on top.

Tests: 15 new tests in test_plugin_auxiliary_tasks.py covering API
validation, manager state lifecycle, helper sort order, _all_aux_tasks
merge semantics, _reset_aux_to_auto inclusion of plugin tasks, and
default-layering in auxiliary_client.

Updates the gateway-bridge code-parity test (test_auxiliary_config_bridge)
to assert the new dynamic shape rather than the hardcoded literal env
var names which no longer appear post-refactor.

Motivation: this unblocks PR #20262 (hindsight smart retain pipeline)
and similar plugins that need a dedicated aux task slot. The change
is non-breaking — built-in env vars (AUXILIARY_VISION_PROVIDER, etc.)
keep working since they're produced by the same f-string template
that built the hardcoded names.
… circuit breaker)

Adds a configurable client-side pipeline around hindsight_retain to reduce
write amplification and improve recall quality at scale.

Pipeline components:
- Pre-retain dedup via recall + auxiliary LLM
- Client-side extraction (retain_extract) before retain
- Delta retain mode for linear-cost memory writes
- Auxiliary circuit breaker with aux_fallback_to_main knob
- Smart pipeline applied to tool retains AND flush-on-switch path
- Dynamic scope filtering on recall + retain
- Bank config fetch from API for richer pre-filter prompts
- Warning on incomplete bank config when smart pipeline is active

Auxiliary task registration:

The pipeline routes its dedup/extract calls through a dedicated auxiliary
task (memory_retain_filter) so users can pin a cheap classifier model
(gpt-oss-20b, gemma-3-flash) without affecting the main chat model.

Per the plugins-must-not-modify-core rule, this PR depends on the
register_auxiliary_task() PluginContext API
(feat/plugin-aux-task-registration). The plugin's register(ctx) function
calls:

    ctx.register_auxiliary_task(
        key='memory_retain_filter',
        display_name='Memory retain filter',
        description='pre-retain content classification (Hindsight smart pipeline)',
        defaults={'timeout': 30, ...},
    )

After the parent PR lands, this declares Hindsight's task slot through
the plugin surface — no core file modifications required.

Defaults / fixes:
- retain_every_n_turns default 5 → 1 (opt-in only)
- Filter recall types in dedup check
- Fetch bank config based on api_url presence only
- Test fixture restoration for retain pipeline attributes
@alt-glitch alt-glitch added type/feature New feature or request comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers P3 Low — cosmetic, nice to have labels May 21, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the work here, @McClean-Edison! The parent PR #29817 has landed on main (via salvage #31177), so this is unblocked.

Before we proceed, this needs @nicolo-esposito's review first — could you ping Nicolo in the Nous Discord and ask him to take a look at this PR? Once he approves the design, Teknium will do the final review and we'll salvage it onto current main with your authorship preserved.

(FYI the cherry-pick onto current main is clean, and the targeted memory-plugin tests pass — just waiting on Nicolo's signoff on the architecture before merging.)

@McClean-Sherlock
McClean-Sherlock deleted the feat/hindsight-smart-retain-pipeline-pluggable branch June 15, 2026 19:59

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the plugin-scoped implementation; the parent auxiliary-task registration API is now on main, and current main still has no equivalent smart-retain pipeline.

Problems

  • aux_fallback_to_main is not a main-model fallback: PR plugins/memory/hindsight/__init__.py:1592 calls get_text_auxiliary_client(""), while current agent/auxiliary_client.py:5148-5156 resolves a task-less auxiliary client with no active main_runtime.
  • The registered timeout: 30 at PR plugins/memory/hindsight/__init__.py:2652-2658 is never passed to the direct client.chat.completions.create() calls (for example lines 1620-1625). The same bypass skips the task's extra_body handling.
  • The documented plugins.memory_provider.hindsight.smart_pipeline config is not read by the provider loader (plugins/memory/hindsight/__init__.py:349-392); the implementation instead adds flat provider-config fields.
  • The new pipeline paths have no behavioral tests; the provider-test change only disables bank-config fetches.

Suggested changes

  • Use the auxiliary execution path that applies per-task routing/timeout/extra-body and pass an explicit active-main runtime for the fallback case.
  • Align documentation with the actual configuration surface and add hermetic coverage for classifier/dedup/extraction/breaker/scope/flush paths.
  • Preserve current append-mode retention behavior added in 09d66037 when salvaging.

This is an automated hermes-sweeper review.


task = "memory_retain_filter"
if use_main:
client, model = get_text_auxiliary_client("") # "" = main model

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_text_auxiliary_client("") does not select the active main-model runtime: it resolves a task-less auxiliary client and receives no main_runtime. Please pass an explicit main runtime into this provider path, or remove the aux_fallback_to_main guarantee.

f"Reply with one word: SKIP, GENERAL, or SCOPED"
)

response = client.chat.completions.create(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The registered memory_retain_filter timeout and extra_body are not applied here: this direct SDK call has neither. Route this through the task-aware auxiliary execution helper (and use it at the other smart-pipeline call sites) so the advertised per-task configuration takes effect.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/memory Memory subsystem: store, providers, sync, background reviews labels Jul 13, 2026
@McClean-codes McClean-codes closed this by deleting the head repository Aug 19, 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 sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/memory Memory tool and memory providers type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants