Skip to content

fix(memory): filter Hindsight auto-retain noise - #27472

Open
Qwinty wants to merge 1 commit into
NousResearch:mainfrom
Qwinty:fix/hindsight-delta-retention-filter
Open

fix(memory): filter Hindsight auto-retain noise#27472
Qwinty wants to merge 1 commit into
NousResearch:mainfrom
Qwinty:fix/hindsight-delta-retention-filter

Conversation

@Qwinty

@Qwinty Qwinty commented May 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Keeps automatic Hindsight memory/context low-noise without changing explicit memory-tool behavior.

Current main already contains the retain-delta/replay lifecycle core, so the refreshed PR carries only the remaining filtering and observability layer:

  • Filter known Hermes lifecycle artifacts from automatic retain and background auto-recall.
  • Apply recall filtering independently to each returned result; one noisy result or cross-result regex cannot suppress useful siblings.
  • Remove matching standalone artifact lines while preserving useful text before and after them.
  • Strip configured lifecycle blocks before skip-pattern evaluation.
  • Keep manual hindsight_retain and hindsight_recall calls unchanged.
  • Make auto_retain_filter_enabled: false a true no-op: no YAML loading, rewriting, query/result suppression, truncation, or audit logging.
  • Support optional skip/preserve/strip/recall regexes, per-message character caps, and a profile-relative YAML file.
  • Provide metadata-only JSONL audit records containing actions/reasons and character counts, never transcript text.
  • Expose every setting in the provider schema and document exact scope/defaults in the Hindsight README.

Verification

Immutable current-main archive:

  • provider/filter/schema suite: 67 passed
  • focused filter/schema subset: 10 passed
  • Ruff, py_compile, and git diff --check: passed

Related: #18774.
Fixes #18818.

@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 May 17, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing PRs for the same root issue (#23724 — Hindsight resends full transcript on append-mode retain):

This PR is the most comprehensive approach (delta buffering + noise filtering + audit logging). If this merges, the others should be closed.

@ai-ag2026

Copy link
Copy Markdown
Contributor

I ran into a crash-recovery variant that seems directly relevant to this PR and to #20664.

Metadata-only incident shape:

  • WebUI recovered a longer .json.bak session after an unclean shutdown.
  • Hindsight then received repeated full-session / large retain jobs for recovered session state.
  • Source transcripts were preserved; the problem was derived memory work flooding the retain queue.
  • The downstream Hindsight worker later hit asyncpg.exceptions.DataError: different vector dimensions 384 and 0 and had to have poisoned retain operations quarantined.

This PR's move from cumulative transcript snapshots to turn deltas looks like the right fix direction for the Hermes side. One thing worth explicitly covering in tests/review: does the delta buffer/fingerprint behavior also handle restored/replayed session state after crash recovery, not only normal in-process turn progression?

Suggested additional acceptance check:

  1. Simulate the same recovered/restored session content being observed twice.
  2. Assert Hindsight auto-retain enqueues one bounded/delta retain, not repeated full-span retain payloads.
  3. Then append a new turn and assert only the new turn is retained.
  4. Ensure audit/log metadata includes sizes/counts but no raw transcript content.

I avoided posting raw transcript payloads for privacy; local evidence is metadata-only (operation counts, payload sizes, error class, stack shape).

@Qwinty

Qwinty commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, good catch — I added explicit coverage for this in ef41fba21.

What changed:

  • Added a regression test that simulates a recovered/restored turn being observed twice, verifies the replay does not enqueue another retain, then appends a new turn and verifies only the new delta is retained.
  • Added turn-content fingerprinting after auto-retain sanitization so replayed recovered turns are skipped within the current session lifecycle.
  • Changed auto-retain audit entries to metadata-only sizes/counts and removed raw/sanitized transcript previews from the audit log.

Local verification:

  • python -m pytest tests/plugins/memory -q -o 'addopts='
  • python -m compileall -q plugins/memory/hindsight/__init__.py
  • python -m ruff check plugins/memory/hindsight/__init__.py tests/plugins/memory/test_hindsight_provider.py
  • hermes doctor

@Qwinty

Qwinty commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on current upstream/main; PR is mergeable again.

Local verification:

  • python -m pytest -q -o addopts='' tests/plugins/memory/test_hindsight_provider.py - 101 passed
  • python -m ruff check plugins/memory/hindsight/__init__.py tests/plugins/memory/test_hindsight_provider.py - passed
  • git diff --check upstream/main...HEAD - passed

CI has been retriggered on ba738c831.

@Qwinty
Qwinty force-pushed the fix/hindsight-delta-retention-filter branch from ba738c8 to fd57f02 Compare June 9, 2026 12:56
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the comprehensive Hindsight cleanup. I verified that the original delta-retain bug is now fixed on current main, but several extras in this PR are still unique and potentially salvageable.

Problems

  • The core delta-retain change is already on main in 09d6603: sync_turn slices append payloads from _last_retained_turn_count at plugins/memory/hindsight/__init__.py:1478, and the regression test is present at tests/plugins/memory/test_hindsight_provider.py:805.
  • The replay-fingerprint guard is only in-memory: PR head initializes _retained_turn_fingerprints at plugins/memory/hindsight/__init__.py:664 and resets it on initialize/session switch, so it does not prove cross-process crash-recovery replay protection.
  • The replay test at PR head tests/plugins/memory/test_hindsight_provider.py:811 covers duplicate sync_turn calls on the same provider instance, not a fresh provider observing restored session content after restart.

Suggested changes

  • Rebase/salvage around current main by dropping the already-landed delta-retain portion and keeping only the still-missing filtering/audit/recall-suppression pieces.
  • If crash-recovery replay protection is intended, add a test with a fresh initialized provider for the same restored session, or document that fingerprinting is intentionally lifecycle-local.

Automated hermes-sweeper review.

@Qwinty
Qwinty force-pushed the fix/hindsight-delta-retention-filter branch from fd57f02 to 3073834 Compare June 13, 2026 17:28
@Qwinty Qwinty changed the title fix(memory): retain Hindsight turn deltas fix(memory): filter Hindsight auto-retain noise Jun 13, 2026
@Qwinty
Qwinty force-pushed the fix/hindsight-delta-retention-filter branch from 3073834 to 5c44fc0 Compare July 10, 2026 17:06
@alt-glitch alt-glitch added type/feature New feature or request and removed type/bug Something isn't working labels Jul 10, 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 narrowing this to the filtering work after the append-delta path landed in 09d66037f8f7bc5bd879ed8128273fb6780a009f.

Problems

  • plugins/memory/hindsight/__init__.py:1726 rejects the whole joined recall payload when any result matches a noise regex. Current main joins all resp.results at plugins/memory/hindsight/__init__.py:1519; a single noisy historical result can therefore suppress unrelated valid recall. Filter individual results before joining them.
  • The new settings loaded at plugins/memory/hindsight/__init__.py:1290 are absent from get_config_schema() (:970-1010) and the Hindsight config documentation (plugins/memory/hindsight/README.md:67-100). The optional YAML/config surface needs a documented configuration contract.

Suggested changes

  • Add a mixed-result recall regression test and retain the valid result when another result matches a skip pattern.
  • Document the YAML path/settings and add coverage for the current SUMMARY_PREFIX and end-marker format from agent/context_compressor.py:44-65,133-136.

Automated hermes-sweeper review.

Comment thread plugins/memory/hindsight/__init__.py Outdated
@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 labels Jul 13, 2026
@Qwinty
Qwinty force-pushed the fix/hindsight-delta-retention-filter branch from 5c44fc0 to 2adc417 Compare July 13, 2026 21:18
@teknium1 teknium1 added the area/memory Memory subsystem: store, providers, sync, background reviews label Jul 19, 2026
@Qwinty
Qwinty force-pushed the fix/hindsight-delta-retention-filter branch from 2adc417 to 0dad72f Compare July 19, 2026 13:30
@Qwinty

Qwinty commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Rebased cleanly onto current upstream/main.

This refresh keeps the still-unique auto-retain/recall noise filtering work (strip/skip/preserve patterns, per-result recall filtering, size-only audit log). The older delta-retain core is already present on main and remains untouched here.

Local verification:

python -m pytest -q -o 'addopts=' tests/plugins/memory/test_hindsight_provider.py
# 121 passed

@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to merged #40605: the original append/delta-retain defect is already fixed; this PR now addresses distinct Hindsight auto-retain and recall-noise filtering.

@Qwinty

Qwinty commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Independent post-rebase audit found and fixed two recall-filter edge cases in 0fd7a16f3 and 95d1e7e6e:

  • useful recall text was dropped when an artifact line appeared beside real memory content, or when ordinary prose contained model was just switched;
  • auto_retain_filter_enabled: false still stripped artifact text from recall results despite the filter being disabled.

The filter now strips only standalone artifact lines when enabled and remains a true no-op when disabled.

Verification after the fixes:

  • tests/plugins/memory/test_hindsight_provider.py: 124 passed
  • ruff: passed
  • git diff --check: passed

@Qwinty
Qwinty force-pushed the fix/hindsight-delta-retention-filter branch from 95d1e7e to 7caf572 Compare August 9, 2026 10:05
@Qwinty

Qwinty commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebuilt the branch on current main as 7caf572840 and dropped the retain-delta lifecycle core that has already landed independently.

The refreshed PR now contains only the unique low-noise layer. In particular, background recall filtering is evaluated per result and standalone artifact lines are removed without dropping neighboring useful content. The joined recall aggregate is no longer vetoed by recall_skip_patterns, so one sibling/cross-result match cannot erase the rest.

auto_retain_filter_enabled: false is also a true early no-op before YAML loading, rewriting, suppression, truncation, or audit setup. Manual retain/recall tools remain untouched, and all settings are now documented and exposed in the provider schema.

Immutable verification: 67 provider/filter/schema tests passed; Ruff, py_compile, and diff-check passed. CI is running on the refreshed head.

@Qwinty

Qwinty commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

CI exposed an outdated desktop-schema contract test: it still assumed Hindsight had exactly five fields and no Full config surface.

Updated the contract intentionally:

  • the original five connection/bank fields remain inline;
  • the nine advanced filtering/YAML/regex/audit controls are grouped under Automatic filtering in Full config;
  • the test now protects the exact complete key set and both UI surfaces.

Local immutable verification on the new head: 71 Hindsight provider/filter/schema tests passed. CI has been restarted by the push.

@Qwinty
Qwinty force-pushed the fix/hindsight-delta-retention-filter branch from 7caf572 to a9d5ba2 Compare August 9, 2026 10:13
@Qwinty
Qwinty force-pushed the fix/hindsight-delta-retention-filter branch from a9d5ba2 to 51f698c Compare August 21, 2026 13:57
@alt-glitch alt-glitch added type/bug Something isn't working and removed type/feature New feature or request labels Aug 21, 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.

TTS: Suboptimal Opus encoding for Gemini/Edge TTS causes quality loss in Telegram voice messages

4 participants