feat(memory): mem4 four-tier routed-memory provider (⑤+④+①+② + compression fix) - #2
Closed
sam7894604 wants to merge 7 commits into
Closed
feat(memory): mem4 four-tier routed-memory provider (⑤+④+①+② + compression fix)#2sam7894604 wants to merge 7 commits into
sam7894604 wants to merge 7 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 35bdda7870ad8798bd0869a0ba88ba3d3d145b05 and cae3565. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a ChangesProvider Context Threading
mem4 Memory Provider Plugin
Estimated code review effort: 4 (Complex) | ~75 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
plugins/memory/mem4/dream.py (1)
260-265: 🧹 Nitpick | 🔵 TrivialArchive files under
_mirror/_archivegrow unbounded.Each compaction writes a new
{stem}-{stamp}.mdoriginal and nothing prunes them. This is intentional cold storage per the non-destructive invariant, but over a long-lived deployment the archive can accumulate indefinitely. Consider a retention policy (age/count cap) or a documented cleanup path so disk usage stays bounded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/memory/mem4/dream.py` around lines 260 - 265, The archive write in the compaction flow will keep creating new files in DREAM_ARCHIVE_DIRNAME without any cleanup, so add a retention mechanism or documented cleanup path around the archiving logic in the compaction method that writes {path.stem}-{stamp}.md. Update the code near the archive_dir creation and write_text call to either prune old entries by age/count or expose a safe maintenance routine so _mirror/_archive does not grow without bound.plugins/memory/mem4/recall.py (1)
180-316: 🚀 Performance & Scalability | 🔵 TrivialConsider batching commits during backfill.
index()issues acommit()per row, andbackfill_batch()calls it in a per-row loop. For large histories this becomes one fsync per row and repeatedly grabs the lock, competing with foregroundsearch()on the shared connection. Since correctness is unaffected (dedup + cursor semantics are sound), this is purely a throughput concern — consider a batched insert path that commits once perbackfill_batchcall.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/memory/mem4/recall.py` around lines 180 - 316, `backfill_batch()` is doing per-row commits through `index()`, which makes large backfills slow and increases lock contention on the shared connection. Add a batched insert path in `RecallStore` (or refactor `index()` to support deferred commits) so `backfill_batch()` can write all rows in one transaction and commit once after the loop. Keep the existing dedup behavior in `index()`, but have `backfill_batch()` use the new batch-friendly path while preserving the current cursor and `has_more` semantics.agent/context_compressor.py (1)
1646-1659: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winNo size cap on provider-supplied context.
Every other content source folded into this prompt (
_CONTENT_MAX, tool-arg truncation,summary_budget) is bounded, but_provider_ctxis spliced in with no limit. A provider that returns a large payload (e.g. multiple cold-tier microfiles) could blow past the aux model's context window or crowd out the actual conversation content being summarized.Consider truncating similarly to the existing
_CONTENT_MAX/_CONTENT_HEADpattern before injection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/context_compressor.py` around lines 1646 - 1659, The provider-supplied context injected in ContextCompressor’s prompt has no size cap, unlike the other bounded inputs. Add truncation for _pending_provider_context before it is appended in this section of context_compressor.py, using the existing _CONTENT_MAX/_CONTENT_HEAD-style approach or an equivalent limit, and keep the “reproduce verbatim” instruction aligned with the truncated payload so the summarizer still preserves the provider context without overflowing the prompt.tests/plugins/memory/test_mem4_provider.py (1)
129-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the baseline A/B arm invariant.
The suite verifies degrade-when-inactive but never exercises
arm=baseline. A small test asserting that in baselineget_tool_schemas()==[],system_prompt_block()=="",prefetch(...)=="", andon_pre_compress(...)==""would lock in the "no injection" contract and directly catch theon_pre_compressleak flagged in__init__.py.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/plugins/memory/test_mem4_provider.py` around lines 129 - 154, Add a focused test for the baseline A/B arm invariant in Mem4MemoryProvider. Create a new case using the same setup path as the existing tests, but configure the provider with arm=baseline and assert that get_tool_schemas() returns an empty list, system_prompt_block() returns an empty string, prefetch(...) returns an empty string, and on_pre_compress(...) returns an empty string. Use Mem4MemoryProvider and the related hooks to verify the “no injection” contract and catch any baseline leakage from on_pre_compress.plugins/memory/mem4/__init__.py (1)
752-756: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse a per-turn ref for recall hits
plugins/memory/mem4/__init__.py:752-756usesref=f"turn:{session_id}", so every turn in a session shows up under the same identifier inmem_search.RecallStore.indexdedups bycontent_hash, notref, so this won’t collide entries, but it does make per-turn provenance ambiguous. Add a turn/message discriminator if distinct hit refs matter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/memory/mem4/__init__.py` around lines 752 - 756, The recall entry in RecallStore.index currently uses the same ref for every turn in a session, which makes mem_search provenance ambiguous. Update the indexing call in the turn-recall path to include a per-turn or per-message discriminator in the ref, using the existing session/turn context around the self._recall.index call so each hit can be traced back to a distinct turn. Keep the content_hash-based dedup behavior unchanged and only adjust the ref construction in this recall write path.plugins/memory/mem4/cli.py (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid reaching into
provider._active(private attribute) from another module.Expose a public
is_activeproperty onMem4MemoryProviderinstead of relying on the_activeinternal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/memory/mem4/cli.py` at line 18, The CLI is reaching into the private provider state via provider._active, which should not be accessed from another module. Add a public is_active property on Mem4MemoryProvider and update the CLI logic in cli.py to use that property instead of the internal field. Keep the existing activation check behavior, but route it through the new public accessor so the state remains encapsulated.plugins/memory/mem4/audit.py (1)
32-32: 🚀 Performance & Scalability | 🔵 TrivialNo rotation/retention policy for
audit.jsonl.The log grows unbounded (
read_events()/summarize()re-read the whole file each time). Over a long-lived session/host this is an ever-growing, linearly-slower read. Consider a size/age-based rotation or truncation policy for the audit sink.Also applies to: 140-152
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/memory/mem4/audit.py` at line 32, The audit sink writes to AUDIT_LOG_FILENAME with no retention or rotation, so the file can grow without bound and slow down read_events() and summarize(). Add a size- or age-based rotation/truncation policy in the audit logging path used by read_events()/summarize(), and keep the audit.jsonl reader behavior compatible with rotated files so long-lived sessions do not accumulate unbounded history.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent/context_compressor.py`:
- Around line 1646-1659: The provider-supplied `_pending_provider_context` is
being appended to the summarizer prompt in `ContextCompressor` without the same
redaction safeguards used for conversation content. Update the `prompt`
construction path in `context_compressor.py` to pass `_provider_ctx` through the
existing redaction/serialization flow (or an equivalent safe sanitizer) before
interpolation, so `summary_model` only receives scrubbed provider context while
still preserving the “Memory Provider Context” contract.
In `@agent/conversation_compression.py`:
- Around line 446-457: The on_pre_compress handling in
conversation_compression.py is swallowing exceptions silently, unlike the nearby
on_session_start and on_session_switch notification blocks. Update the
try/except around agent._memory_manager.on_pre_compress(messages) to log the
failure with logger.debug (or the same logging pattern used elsewhere in this
function) before falling back to an empty _pre_compress_ctx, so failures from
on_pre_compress have a diagnostic trace while compaction still continues.
In `@plugins/memory/mem4/__init__.py`:
- Around line 796-809: Add the missing baseline guard in on_pre_compress so the
routing legend and code list are not injected when _is_baseline() is true.
Mirror the same baseline gating already used in get_tool_schemas, prefetch, and
system_prompt_block, and keep the early return before building the legend or
calling self._backend.list_codes(). This ensures the compaction summary stays
empty for the baseline arm while preserving the existing behavior for
non-baseline runs.
- Around line 774-784: Serialize rebuild() with the backfill worker by taking
the same state/daemon lock used around backfill processing, or otherwise
stopping and joining the running backfill thread before mutating state in
rebuild(). Update the rebuild flow around _read_state(), _write_state(),
_index_microfiles(), and _backfill_worker()/_mark_backfill_complete() so only
one writer touches backfill_cursor and backfill_complete at a time. Also make
_write_state() write atomically via a temp file and replace to avoid truncated
.mem4_state.json on crash.
- Around line 290-296: The shutdown path in shutdown() still closes self._recall
after only a timed join, so the backfill worker may keep running against a
closed SQLite connection. Add a cooperative stop signal that the backfill loop
checks, signal it before waiting, and ensure the backfill thread has actually
exited before calling self._recall.close(). Use the existing shutdown() and
_backfill_thread handling in mem4/__init__.py to make the worker exit cleanly
first.
In `@plugins/memory/mem4/backend.py`:
- Around line 161-181: The mem_route read path in read_microfile() only catches
OSError, so malformed UTF-8 can still raise UnicodeDecodeError and escape
through _tool_route(). Update read_microfile() to treat decode failures as a
miss by catching the text read error path as well and returning None, keeping
the existing MicrofileResult flow for valid content.
In `@plugins/memory/mem4/cli.py`:
- Around line 16-22: `Mem4MemoryProvider.rebuild()` and `provider.shutdown()` in
the CLI flow need to be protected so shutdown always runs even if rebuild fails.
Update the rebuild path in `cli.py` to call `provider.rebuild()` inside a try
block and move `provider.shutdown()` into a finally block, keeping the existing
inactive-provider early return and preserving the `Mem4MemoryProvider` lifecycle
cleanup.
In `@plugins/memory/mem4/dream.py`:
- Around line 159-166: The staleness check in maybe_consolidate can crash when
state.last_consolidation_at parses into a naive datetime, because
datetime.fromisoformat may return one and the later now - last subtraction is
outside the existing ValueError guard. Update the timestamp handling in dream.py
to normalize or reject naive values before comparing against self.staleness,
using maybe_consolidate and the state.last_consolidation_at parsing block as the
fix point.
---
Nitpick comments:
In `@agent/context_compressor.py`:
- Around line 1646-1659: The provider-supplied context injected in
ContextCompressor’s prompt has no size cap, unlike the other bounded inputs. Add
truncation for _pending_provider_context before it is appended in this section
of context_compressor.py, using the existing _CONTENT_MAX/_CONTENT_HEAD-style
approach or an equivalent limit, and keep the “reproduce verbatim” instruction
aligned with the truncated payload so the summarizer still preserves the
provider context without overflowing the prompt.
In `@plugins/memory/mem4/__init__.py`:
- Around line 752-756: The recall entry in RecallStore.index currently uses the
same ref for every turn in a session, which makes mem_search provenance
ambiguous. Update the indexing call in the turn-recall path to include a
per-turn or per-message discriminator in the ref, using the existing
session/turn context around the self._recall.index call so each hit can be
traced back to a distinct turn. Keep the content_hash-based dedup behavior
unchanged and only adjust the ref construction in this recall write path.
In `@plugins/memory/mem4/audit.py`:
- Line 32: The audit sink writes to AUDIT_LOG_FILENAME with no retention or
rotation, so the file can grow without bound and slow down read_events() and
summarize(). Add a size- or age-based rotation/truncation policy in the audit
logging path used by read_events()/summarize(), and keep the audit.jsonl reader
behavior compatible with rotated files so long-lived sessions do not accumulate
unbounded history.
In `@plugins/memory/mem4/cli.py`:
- Line 18: The CLI is reaching into the private provider state via
provider._active, which should not be accessed from another module. Add a public
is_active property on Mem4MemoryProvider and update the CLI logic in cli.py to
use that property instead of the internal field. Keep the existing activation
check behavior, but route it through the new public accessor so the state
remains encapsulated.
In `@plugins/memory/mem4/dream.py`:
- Around line 260-265: The archive write in the compaction flow will keep
creating new files in DREAM_ARCHIVE_DIRNAME without any cleanup, so add a
retention mechanism or documented cleanup path around the archiving logic in the
compaction method that writes {path.stem}-{stamp}.md. Update the code near the
archive_dir creation and write_text call to either prune old entries by
age/count or expose a safe maintenance routine so _mirror/_archive does not grow
without bound.
In `@plugins/memory/mem4/recall.py`:
- Around line 180-316: `backfill_batch()` is doing per-row commits through
`index()`, which makes large backfills slow and increases lock contention on the
shared connection. Add a batched insert path in `RecallStore` (or refactor
`index()` to support deferred commits) so `backfill_batch()` can write all rows
in one transaction and commit once after the loop. Keep the existing dedup
behavior in `index()`, but have `backfill_batch()` use the new batch-friendly
path while preserving the current cursor and `has_more` semantics.
In `@tests/plugins/memory/test_mem4_provider.py`:
- Around line 129-154: Add a focused test for the baseline A/B arm invariant in
Mem4MemoryProvider. Create a new case using the same setup path as the existing
tests, but configure the provider with arm=baseline and assert that
get_tool_schemas() returns an empty list, system_prompt_block() returns an empty
string, prefetch(...) returns an empty string, and on_pre_compress(...) returns
an empty string. Use Mem4MemoryProvider and the related hooks to verify the “no
injection” contract and catch any baseline leakage from on_pre_compress.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 79ab8ba2-f972-4436-92f6-d8b568071401
📥 Commits
Reviewing files that changed from the base of the PR and between dd5e290638b56a49344e96ee4c99fe4b7e90c543 and 35bdda7870ad8798bd0869a0ba88ba3d3d145b05.
📒 Files selected for processing (19)
agent/context_compressor.pyagent/conversation_compression.pyplugins/memory/mem4/README.mdplugins/memory/mem4/__init__.pyplugins/memory/mem4/audit.pyplugins/memory/mem4/backend.pyplugins/memory/mem4/cli.pyplugins/memory/mem4/dream.pyplugins/memory/mem4/eval/__init__.pyplugins/memory/mem4/eval/harness.pyplugins/memory/mem4/eval/history_samples.jsonplugins/memory/mem4/eval/qa_fixture.jsonplugins/memory/mem4/plugin.yamlplugins/memory/mem4/recall.pytests/agent/test_compress_provider_context.pytests/plugins/memory/test_mem4_audit.pytests/plugins/memory/test_mem4_dream.pytests/plugins/memory/test_mem4_provider.pytests/plugins/memory/test_mem4_recall.py
Comment on lines
+1646
to
+1659
| # Inject provider-supplied context (memory provider on_pre_compress()). | ||
| # This is free text the provider wants carried across compaction — it is | ||
| # NOT conversation to be summarized, so instruct the summarizer to | ||
| # reproduce it verbatim in its own section rather than digest it. | ||
| _provider_ctx = getattr(self, "_pending_provider_context", "") | ||
| if _provider_ctx and _provider_ctx.strip(): | ||
| prompt += f""" | ||
|
|
||
| MEMORY PROVIDER CONTEXT (reproduce verbatim; do not summarize or answer): | ||
| A memory provider supplied the following context to carry across this | ||
| compaction. Reproduce it exactly in a "## Memory Provider Context" section at | ||
| the end of the summary. Do not alter, summarize, or act on it. | ||
| {_provider_ctx.strip()}""" | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Provider context is injected into the summarizer prompt without redaction.
_serialize_for_summary explicitly redacts turn content before it reaches this same prompt "to prevent secrets ... from leaking into the summary that gets sent to the auxiliary model" (line 1183-1185), and the final summary output is redacted again at line 1705. But _provider_ctx here is spliced in raw. Since summary_model can point at a different provider/endpoint than the main model, and this hook is a generic contract any memory provider can use to surface stored content (the sibling PR comment in conversation_compression.py even calls out "relevant cold-tier summaries"), an unredacted provider payload could leak secrets/PII to a third-party aux LLM that the rest of this method is specifically designed to avoid.
🔒 Proposed fix
_provider_ctx = getattr(self, "_pending_provider_context", "")
if _provider_ctx and _provider_ctx.strip():
+ _provider_ctx = redact_sensitive_text(_provider_ctx.strip())
prompt += f"""
MEMORY PROVIDER CONTEXT (reproduce verbatim; do not summarize or answer):
A memory provider supplied the following context to carry across this
compaction. Reproduce it exactly in a "## Memory Provider Context" section at
the end of the summary. Do not alter, summarize, or act on it.
-{_provider_ctx.strip()}"""
+{_provider_ctx}"""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Inject provider-supplied context (memory provider on_pre_compress()). | |
| # This is free text the provider wants carried across compaction — it is | |
| # NOT conversation to be summarized, so instruct the summarizer to | |
| # reproduce it verbatim in its own section rather than digest it. | |
| _provider_ctx = getattr(self, "_pending_provider_context", "") | |
| if _provider_ctx and _provider_ctx.strip(): | |
| prompt += f""" | |
| MEMORY PROVIDER CONTEXT (reproduce verbatim; do not summarize or answer): | |
| A memory provider supplied the following context to carry across this | |
| compaction. Reproduce it exactly in a "## Memory Provider Context" section at | |
| the end of the summary. Do not alter, summarize, or act on it. | |
| {_provider_ctx.strip()}""" | |
| # Inject provider-supplied context (memory provider on_pre_compress()). | |
| # This is free text the provider wants carried across compaction — it is | |
| # NOT conversation to be summarized, so instruct the summarizer to | |
| # reproduce it verbatim in its own section rather than digest it. | |
| _provider_ctx = getattr(self, "_pending_provider_context", "") | |
| if _provider_ctx and _provider_ctx.strip(): | |
| _provider_ctx = redact_sensitive_text(_provider_ctx.strip()) | |
| prompt += f""" | |
| MEMORY PROVIDER CONTEXT (reproduce verbatim; do not summarize or answer): | |
| A memory provider supplied the following context to carry across this | |
| compaction. Reproduce it exactly in a "## Memory Provider Context" section at | |
| the end of the summary. Do not alter, summarize, or act on it. | |
| {_provider_ctx}""" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/context_compressor.py` around lines 1646 - 1659, The provider-supplied
`_pending_provider_context` is being appended to the summarizer prompt in
`ContextCompressor` without the same redaction safeguards used for conversation
content. Update the `prompt` construction path in `context_compressor.py` to
pass `_provider_ctx` through the existing redaction/serialization flow (or an
equivalent safe sanitizer) before interpolation, so `summary_model` only
receives scrubbed provider context while still preserving the “Memory Provider
Context” contract.
Comment on lines
+446
to
+457
| # Notify external memory provider before compression discards context. | ||
| # Capture the returned text (provider-extracted insights — e.g. mem4's | ||
| # routing legend + relevant cold-tier summaries) so it can be injected into | ||
| # the compaction summary. Previously the return value was discarded, so the | ||
| # on_pre_compress hook ran but its output went nowhere (upstream issue | ||
| # #23367). Resolved defensively: a provider failure never blocks compaction. | ||
| _pre_compress_ctx = "" | ||
| if agent._memory_manager: | ||
| try: | ||
| agent._memory_manager.on_pre_compress(messages) | ||
| _pre_compress_ctx = agent._memory_manager.on_pre_compress(messages) or "" | ||
| except Exception: | ||
| pass | ||
| _pre_compress_ctx = "" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Silent swallow of on_pre_compress() failures — no logging.
Every other provider/engine notification in this function that's wrapped in a defensive try/except logs the failure via logger.debug (see the on_session_start/on_session_switch blocks a few dozen lines below). This new block swallows the exception with no trace at all, so a broken memory provider's on_pre_compress would fail silently forever with no diagnostic signal.
🩹 Proposed fix
_pre_compress_ctx = ""
if agent._memory_manager:
try:
_pre_compress_ctx = agent._memory_manager.on_pre_compress(messages) or ""
- except Exception:
+ except Exception as _pc_err:
+ logger.debug("memory manager on_pre_compress (compression) failed: %s", _pc_err)
_pre_compress_ctx = ""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Notify external memory provider before compression discards context. | |
| # Capture the returned text (provider-extracted insights — e.g. mem4's | |
| # routing legend + relevant cold-tier summaries) so it can be injected into | |
| # the compaction summary. Previously the return value was discarded, so the | |
| # on_pre_compress hook ran but its output went nowhere (upstream issue | |
| # #23367). Resolved defensively: a provider failure never blocks compaction. | |
| _pre_compress_ctx = "" | |
| if agent._memory_manager: | |
| try: | |
| agent._memory_manager.on_pre_compress(messages) | |
| _pre_compress_ctx = agent._memory_manager.on_pre_compress(messages) or "" | |
| except Exception: | |
| pass | |
| _pre_compress_ctx = "" | |
| # Notify external memory provider before compression discards context. | |
| # Capture the returned text (provider-extracted insights — e.g. mem4's | |
| # routing legend + relevant cold-tier summaries) so it can be injected into | |
| # the compaction summary. Previously the return value was discarded, so the | |
| # on_pre_compress hook ran but its output went nowhere (upstream issue | |
| # `#23367`). Resolved defensively: a provider failure never blocks compaction. | |
| _pre_compress_ctx = "" | |
| if agent._memory_manager: | |
| try: | |
| _pre_compress_ctx = agent._memory_manager.on_pre_compress(messages) or "" | |
| except Exception as _pc_err: | |
| logger.debug("memory manager on_pre_compress (compression) failed: %s", _pc_err) | |
| _pre_compress_ctx = "" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/conversation_compression.py` around lines 446 - 457, The
on_pre_compress handling in conversation_compression.py is swallowing exceptions
silently, unlike the nearby on_session_start and on_session_switch notification
blocks. Update the try/except around
agent._memory_manager.on_pre_compress(messages) to log the failure with
logger.debug (or the same logging pattern used elsewhere in this function)
before falling back to an empty _pre_compress_ctx, so failures from
on_pre_compress have a diagnostic trace while compaction still continues.
Comment on lines
+290
to
+296
| def shutdown(self) -> None: | ||
| # Let a running backfill finish briefly, then close the recall DB. | ||
| if self._backfill_thread and self._backfill_thread.is_alive(): | ||
| self._backfill_thread.join(timeout=2.0) | ||
| if self._recall is not None: | ||
| self._recall.close() | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file and read the relevant region around shutdown/backfill.
python3 - <<'PY'
from pathlib import Path
p = Path("plugins/memory/mem4/__init__.py")
text = p.read_text().splitlines()
for start, end in [(1, 380)]:
for i in range(start-1, min(end, len(text))):
if 240 <= i+1 <= 330:
print(f"{i+1:4d}: {text[i]}")
PY
# Find the backfill worker and any stop/close coordination.
rg -n "backfill|shutdown|close\\(|thread|Event|stop" plugins/memory/mem4/__init__.pyRepository: sam7894604/hermes-agent
Length of output: 8037
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the backfill worker and recall store implementation.
python3 - <<'PY'
from pathlib import Path
p = Path("plugins/memory/mem4/__init__.py")
text = p.read_text().splitlines()
for start, end in [(540, 610), (1, 220)]:
print(f"\n--- lines {start}-{end} ---")
for i in range(start-1, min(end, len(text))):
print(f"{i+1:4d}: {text[i]}")
PY
# Locate RecallStore and its methods.
rg -n "class RecallStore|def close\\(|def backfill_batch\\(" plugins/memory/mem4/__init__.pyRepository: sam7894604/hermes-agent
Length of output: 13279
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the recall store implementation and any shutdown/thread-safety guards.
python3 - <<'PY'
from pathlib import Path
for path in ["plugins/memory/mem4/recall.py", "plugins/memory/mem4/__init__.py"]:
p = Path(path)
print(f"\n=== {path} ({p.stat().st_size} bytes, {sum(1 for _ in p.open())} lines) ===")
text = p.read_text().splitlines()
# Print only relevant regions for recall.py, and the shutdown/backfill-related regions in __init__.py.
if path.endswith("recall.py"):
ranges = [(1, 260)]
else:
ranges = [(540, 610), (760, 790)]
for start, end in ranges:
print(f"\n--- lines {start}-{end} ---")
for i in range(start-1, min(end, len(text))):
print(f"{i+1:4d}: {text[i]}")
PY
# Search for any stop signal or join/close coordination around backfill.
rg -n "Event\\(|stop_event|shutdown|join\\(|close\\(|backfill_batch\\(" plugins/memory/mem4Repository: sam7894604/hermes-agent
Length of output: 17865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect RecallStore.backfill_batch and any sqlite error handling around close/use.
python3 - <<'PY'
from pathlib import Path
p = Path("plugins/memory/mem4/recall.py")
text = p.read_text().splitlines()
for start, end in [(260, 316)]:
print(f"\n--- lines {start}-{end} ---")
for i in range(start-1, min(end, len(text))):
print(f"{i+1:4d}: {text[i]}")
PYRepository: sam7894604/hermes-agent
Length of output: 2856
Delay closing the recall DB until backfill exits plugins/memory/mem4/__init__.py:290-295
join(timeout=2.0) can return while the daemon backfill thread is still running, and self._recall.close() will leave the worker calling into a closed SQLite connection. Add a cooperative stop signal and only close after the thread has actually exited.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/memory/mem4/__init__.py` around lines 290 - 296, The shutdown path in
shutdown() still closes self._recall after only a timed join, so the backfill
worker may keep running against a closed SQLite connection. Add a cooperative
stop signal that the backfill loop checks, signal it before waiting, and ensure
the backfill thread has actually exited before calling self._recall.close(). Use
the existing shutdown() and _backfill_thread handling in mem4/__init__.py to
make the worker exit cleanly first.
Comment on lines
+774
to
+784
| # Reset the backfill cursor so history is re-indexed from the start. | ||
| state = self._read_state() | ||
| state["backfill_cursor"] = 0 | ||
| state["backfill_complete"] = False | ||
| self._write_state(state) | ||
| indexed = self._index_microfiles() | ||
| if self._backfill_source is not None: | ||
| self._backfill_worker() | ||
| else: | ||
| self._mark_backfill_complete() | ||
| return {"indexed": indexed, "recall_docs": self._recall.count()} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the module structure first.
ast-grep outline plugins/memory/mem4/__init__.py --view expanded
# Locate the state helpers and backfill entrypoints.
rg -n "_read_state|_write_state|_backfill_worker|initialize\(|rebuild\(" plugins/memory/mem4/__init__.py
# Read the relevant sections with line numbers.
sed -n '1,220p' plugins/memory/mem4/__init__.py
sed -n '220,420p' plugins/memory/mem4/__init__.py
sed -n '420,620p' plugins/memory/mem4/__init__.py
sed -n '620,860p' plugins/memory/mem4/__init__.pyRepository: sam7894604/hermes-agent
Length of output: 39813
Serialize rebuild() against backfill state writes.
initialize() can leave the daemon backfill thread running, and rebuild() mutates .mem4_state.json in parallel without any lock or stop/join. That can lose cursor updates and duplicate or skip backfill work; _write_state() also writes the file directly, so a mid-write crash can leave a truncated marker.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/memory/mem4/__init__.py` around lines 774 - 784, Serialize rebuild()
with the backfill worker by taking the same state/daemon lock used around
backfill processing, or otherwise stopping and joining the running backfill
thread before mutating state in rebuild(). Update the rebuild flow around
_read_state(), _write_state(), _index_microfiles(), and
_backfill_worker()/_mark_backfill_complete() so only one writer touches
backfill_cursor and backfill_complete at a time. Also make _write_state() write
atomically via a temp file and replace to avoid truncated .mem4_state.json on
crash.
Comment on lines
+796
to
+809
| def on_pre_compress(self, messages: List[Dict[str, Any]]) -> str: | ||
| # Feed the routing legend (and available codes) into the compression | ||
| # summary so the map survives context compression (design spike §2 — | ||
| # the direct benefit of ⑤). Free text only. | ||
| if not self._active or self._backend is None: | ||
| return "" | ||
| legend = ( | ||
| "mem4 路由碼：§sys 系統 · §fam 人物 · §vlt 知識 · §adr 決策 · " | ||
| "§proto 協定；用 mem_route(code) 按需讀冷區微檔。" | ||
| ) | ||
| codes = self._backend.list_codes() | ||
| if codes: | ||
| legend += " 現有微檔：" + ", ".join(f"§{c}" for c in codes) + "。" | ||
| return legend |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Baseline arm leaks injection through on_pre_compress.
The baseline arm is defined as "all agent-facing surfaces off (no tools, no injection)" (Lines 63-65), and it is correctly gated in get_tool_schemas, prefetch, and system_prompt_block. But on_pre_compress is missing the _is_baseline() guard, so it injects the routing legend (and code list) into the compaction summary even in the baseline arm. That text becomes resident context, inflating hot-zone cost for baseline and invalidating the paired counterfactual / resident-cost measurement.
🛠️ Proposed fix
def on_pre_compress(self, messages: List[Dict[str, Any]]) -> str:
# Feed the routing legend (and available codes) into the compression
# summary so the map survives context compression (design spike §2 —
# the direct benefit of ⑤). Free text only.
- if not self._active or self._backend is None:
+ if not self._active or self._backend is None or self._is_baseline():
return ""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def on_pre_compress(self, messages: List[Dict[str, Any]]) -> str: | |
| # Feed the routing legend (and available codes) into the compression | |
| # summary so the map survives context compression (design spike §2 — | |
| # the direct benefit of ⑤). Free text only. | |
| if not self._active or self._backend is None: | |
| return "" | |
| legend = ( | |
| "mem4 路由碼：§sys 系統 · §fam 人物 · §vlt 知識 · §adr 決策 · " | |
| "§proto 協定；用 mem_route(code) 按需讀冷區微檔。" | |
| ) | |
| codes = self._backend.list_codes() | |
| if codes: | |
| legend += " 現有微檔：" + ", ".join(f"§{c}" for c in codes) + "。" | |
| return legend | |
| def on_pre_compress(self, messages: List[Dict[str, Any]]) -> str: | |
| # Feed the routing legend (and available codes) into the compression | |
| # summary so the map survives context compression (design spike §2 — | |
| # the direct benefit of ⑤). Free text only. | |
| if not self._active or self._backend is None or self._is_baseline(): | |
| return "" | |
| legend = ( | |
| "mem4 路由碼：§sys 系統 · §fam 人物 · §vlt 知識 · §adr 決策 · " | |
| "§proto 協定；用 mem_route(code) 按需讀冷區微檔。" | |
| ) | |
| codes = self._backend.list_codes() | |
| if codes: | |
| legend += " 現有微檔：" + ", ".join(f"§{c}" for c in codes) + "。" | |
| return legend |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/memory/mem4/__init__.py` around lines 796 - 809, Add the missing
baseline guard in on_pre_compress so the routing legend and code list are not
injected when _is_baseline() is true. Mirror the same baseline gating already
used in get_tool_schemas, prefetch, and system_prompt_block, and keep the early
return before building the legend or calling self._backend.list_codes(). This
ensures the compaction summary stays empty for the baseline arm while preserving
the existing behavior for non-baseline runs.
Comment on lines
+161
to
+181
| def read_microfile(self, code: str) -> Optional[MicrofileResult]: | ||
| path = self._microfile_path(code) | ||
| if path is None or not path.is_file(): | ||
| return None | ||
| try: | ||
| content = path.read_text(encoding="utf-8") | ||
| except OSError: | ||
| return None | ||
| return MicrofileResult( | ||
| code=normalize_code(code) or code, | ||
| content=content, | ||
| source="local-file", | ||
| ) | ||
|
|
||
| def write_microfile(self, code: str, content: str) -> None: | ||
| path = self._microfile_path(code) | ||
| if path is None: | ||
| raise ValueError(f"invalid mem4 route code: {code!r}") | ||
| self.root.mkdir(parents=True, exist_ok=True) | ||
| path.write_text(content, encoding="utf-8") | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether backend calls are wrapped in exception handling in the provider
rg -n -B5 -A5 '\.read_microfile\(|\.write_microfile\(' plugins/memory/mem4/__init__.pyRepository: sam7894604/hermes-agent
Length of output: 1211
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant caller paths and surrounding exception handling.
rg -n -B8 -A20 'read_microfile\(|write_microfile\(|on_memory_write|_tool_route|try:|except ' plugins/memory/mem4/__init__.py
# Also inspect the backend methods themselves for exact exception handling.
rg -n -B5 -A10 'def read_microfile|def write_microfile' plugins/memory/mem4/backend.pyRepository: sam7894604/hermes-agent
Length of output: 26313
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find every call site of write_microfile and inspect nearby exception handling.
rg -n -B6 -A12 'write_microfile\(' .
# Also list the backend implementation locations for context.
rg -n -B4 -A8 'def write_microfile|def read_microfile' plugins/memory/mem4/backend.pyRepository: sam7894604/hermes-agent
Length of output: 4741
Catch decode failures on the mem_route read path.
read_microfile() only handles OSError; a malformed UTF-8 microfile still raises UnicodeDecodeError, and _tool_route() doesn’t catch it, so the call can fail instead of returning a miss. The write helper doesn’t appear to be called from current code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/memory/mem4/backend.py` around lines 161 - 181, The mem_route read
path in read_microfile() only catches OSError, so malformed UTF-8 can still
raise UnicodeDecodeError and escape through _tool_route(). Update
read_microfile() to treat decode failures as a miss by catching the text read
error path as well and returning None, keeping the existing MicrofileResult flow
for valid content.
Comment on lines
+16
to
+22
| provider = Mem4MemoryProvider() | ||
| provider.initialize("cli-rebuild", hermes_home=str(get_hermes_home()), platform="cli") | ||
| if not provider._active: | ||
| print(" mem4 is not active (check memory.mem4.backend). Nothing to rebuild.\n") | ||
| return | ||
| result = provider.rebuild() | ||
| provider.shutdown() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Wrap rebuild()/shutdown() in try/finally.
If provider.rebuild() raises, provider.shutdown() (line 22) never runs, potentially leaving the backfill thread or Dream's mutual-exclusion lock un-released for the next invocation.
🛠️ Proposed fix
provider = Mem4MemoryProvider()
provider.initialize("cli-rebuild", hermes_home=str(get_hermes_home()), platform="cli")
if not provider._active:
print(" mem4 is not active (check memory.mem4.backend). Nothing to rebuild.\n")
return
- result = provider.rebuild()
- provider.shutdown()
+ try:
+ result = provider.rebuild()
+ finally:
+ provider.shutdown()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| provider = Mem4MemoryProvider() | |
| provider.initialize("cli-rebuild", hermes_home=str(get_hermes_home()), platform="cli") | |
| if not provider._active: | |
| print(" mem4 is not active (check memory.mem4.backend). Nothing to rebuild.\n") | |
| return | |
| result = provider.rebuild() | |
| provider.shutdown() | |
| provider = Mem4MemoryProvider() | |
| provider.initialize("cli-rebuild", hermes_home=str(get_hermes_home()), platform="cli") | |
| if not provider._active: | |
| print(" mem4 is not active (check memory.mem4.backend). Nothing to rebuild.\n") | |
| return | |
| try: | |
| result = provider.rebuild() | |
| finally: | |
| provider.shutdown() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/memory/mem4/cli.py` around lines 16 - 22,
`Mem4MemoryProvider.rebuild()` and `provider.shutdown()` in the CLI flow need to
be protected so shutdown always runs even if rebuild fails. Update the rebuild
path in `cli.py` to call `provider.rebuild()` inside a try block and move
`provider.shutdown()` into a finally block, keeping the existing
inactive-provider early return and preserving the `Mem4MemoryProvider` lifecycle
cleanup.
Comment on lines
+159
to
+166
| if state.last_consolidation_at: | ||
| try: | ||
| last = datetime.fromisoformat(state.last_consolidation_at) | ||
| except ValueError: | ||
| last = None | ||
| if last is not None and (now - last) >= self.staleness: | ||
| return True, "staleness" | ||
| return False, "below threshold, within staleness window" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Staleness comparison can raise TypeError on a naive timestamp.
datetime.fromisoformat succeeds for a timestamp without an offset and returns a naive datetime. The subtraction now - last sits outside the try, and now is timezone-aware (_now() returns UTC-aware). Subtracting a naive from an aware datetime raises TypeError, which is not caught (only ValueError is). A legacy, externally-written, or hand-edited .dream_state.json with a naive last_consolidation_at would crash maybe_consolidate on the staleness path.
🛡️ Proposed fix to normalize/guard the timestamp
if state.last_consolidation_at:
try:
last = datetime.fromisoformat(state.last_consolidation_at)
+ if last.tzinfo is None:
+ last = last.replace(tzinfo=timezone.utc)
except ValueError:
last = None
if last is not None and (now - last) >= self.staleness:
return True, "staleness"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if state.last_consolidation_at: | |
| try: | |
| last = datetime.fromisoformat(state.last_consolidation_at) | |
| except ValueError: | |
| last = None | |
| if last is not None and (now - last) >= self.staleness: | |
| return True, "staleness" | |
| return False, "below threshold, within staleness window" | |
| if state.last_consolidation_at: | |
| try: | |
| last = datetime.fromisoformat(state.last_consolidation_at) | |
| if last.tzinfo is None: | |
| last = last.replace(tzinfo=timezone.utc) | |
| except ValueError: | |
| last = None | |
| if last is not None and (now - last) >= self.staleness: | |
| return True, "staleness" | |
| return False, "below threshold, within staleness window" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/memory/mem4/dream.py` around lines 159 - 166, The staleness check in
maybe_consolidate can crash when state.last_consolidation_at parses into a naive
datetime, because datetime.fromisoformat may return one and the later now - last
subtraction is outside the existing ValueError guard. Update the timestamp
handling in dream.py to normalize or reject naive values before comparing
against self.staleness, using maybe_consolidate and the
state.last_consolidation_at parsing block as the fix point.
…assis) Wrap the L0/L1/L2/L3 routed-memory design (ADR-018) as a pluggable MemoryProvider in coexist/augment mode: strengthens the built-in MEMORY.md/USER.md, never replaces or writes them back. Removing `memory.provider: mem4` degrades cleanly to pure built-in, zero residue. This is the ⑤-minimal chassis only: - Provider identity/availability with graceful gating (unimplemented backend -> is_available() False -> degrade to built-in). - `mem_route(code)` tool: route code -> L2/L3 microfile read with a freshness tag ([fresh: local-file] / [STALE: ...] / [built-in only]); a miss falls back to built-in rather than erroring. Path-traversal guarded via normalize_code(). - Tiny routing legend in system_prompt_block() and on_pre_compress() (does NOT re-inject L0). - on_memory_write() mirrors built-in writes into mem4-owned files only ($HERMES_HOME/mem4/_mirror/<target>.md); the built-in files are never touched. - Switchable storage backend (read_microfile/write_microfile/search), default local-file (design spike §9.3 b / §9.5). - Idempotent one-time init + version marker (.mem4_state.json), adopts existing microfiles (no rebuild), non-destructive, verified before marking complete (design spike §10). Deferred by design (kept as named seams, not wired): - Feature ①: prefetch/sync_turn + SQLite FTS5 recall & backfill (search() and _backfill() are stubs; mem_search tool withheld until it is real). - Feature ④: Dream consolidation via on_session_end. Tests: 14 cases (registration, availability, mem_route hit/miss/traversal, built-in-untouched mirror, degrade-when-inactive, idempotent init marker, microfile adoption). All green; no regression in tests/agent/test_memory_provider.py (100 passed). HOLD upstream — fork feature branch only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…the summary The compaction path called `memory_manager.on_pre_compress(messages)` but discarded its return value (conversation_compression.py), so the hook ran and every provider's contributed text went nowhere. This made mem4's ⑤ headline benefit — feeding its routing legend / cold-tier summaries into the compaction summary so the map survives compression — inert. (Confirms the Fable 5 spike review; corresponds to upstream issue NousResearch#23367.) Fix (additive, backward-compatible): - conversation_compression.py: capture the on_pre_compress() return string (guarded; a provider failure never blocks compaction) and pass it to compress(provider_context=...). The existing TypeError fallback for strict plugin context engines is left untouched (graceful drop for those). - context_compressor.py: compress() gains an optional `provider_context` param, stashed on `self._pending_provider_context` (reset per call). _generate_summary appends it to the summarizer prompt with an instruction to reproduce it verbatim in a "## Memory Provider Context" section — it is provider context to carry forward, not conversation to digest. Tests: 5 new (injection present/absent/empty, compress threads it through). No regression: test_context_compressor.py (120), test_compress_focus.py (5), summary-continuity + temporal-anchoring (7) all green. Part of the mem4 ⑤ work (fork feature branch). HOLD upstream. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add Dream consolidation to the mem4 provider as fully self-contained pure code (design decision i′): it does NOT assume or coordinate with any external cron. The toothless "Dream cron" was a user-set jobs.json entry, not a Hermes default, so in the upstream context there is nothing to coordinate with — the provider owns its own triggering. Triggers (all in-provider, dream.py): - Event / threshold — on_memory_write accumulates a signal count; crossing memory.mem4.dream.threshold new writes triggers consolidation. - Staleness floor — at session boundaries (start via initialize, end via on_session_end) consolidate if overdue by dream.staleness_days AND there is pending signal. This replaces "cron backup", still pure code. - Idle skip — no pending signal ⇒ nothing to consolidate ⇒ skip. Pure idle needs no timer; idle-time periodic consolidation is a documented non-goal (a future optional external scheduler hook could add it). Invariants: - v1 consolidates ONLY mem4-owned L2/L3 (mirror-log dedup); NEVER writes the built-in MEMORY.md/USER.md hot zone (design spike §3 / Fable 5 decision 4). - Non-destructive: the pre-compaction original is archived to _mirror/_archive/ before rewrite (Fable 5: archive originals before compacting). - A marker (.dream_state.json) + lock (.dream.lock) make the event and staleness paths mutually exclusive so one startup never double-runs. - Feature-flag off (dream.enabled: false) ⇒ every path is a no-op; also cedes to an official background-memory agent if Hermes ships one (upstream NousResearch#553, verified a dormant proposal — open since 2026-03, no activity). Deployment contract (README): once mem4+④ ships, retire the standalone Dream cron entry to avoid double-running — a deploy step (retire the entry, no cron code change), applied at toothless deploy time. Tests: 10 new (threshold trigger, idle skip, staleness boundary + within-window, marker+lock exclusion, disabled no-op, provider threshold-via-write, hot-zone-untouched + archive, disabled no-trigger, session-end idle skip). No regression: mem4 provider (14), memory manager registration (100) green. HOLD upstream — fork feature branch only. toothless deploy (incl. cron retirement) deferred to real-world measurement (will report first). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…etch, backfill, rebuild Fill the ⑤ recall seam with mem4's own SQLite FTS5 database (recall.db, design spike §10.8 decision B), indexing both conversation turns and the L2/L3 microfiles. Chinese search is a hard requirement (Fable 5 review §3): reuse the upstream hermes_state.py DUAL-TABLE pattern verbatim in spirit — - docs_fts (unicode61) for English/BM25 + docs_fts_trigram (trigram) for CJK. - _contains_cjk() routes CJK queries to trigram; any CJK token < 3 chars (trigram needs >=3) falls back to a per-token LIKE scan. - Missing trigram tokenizer is detected and CJK degrades to LIKE — never a hard failure. (Verified: local sqlite 3.50.4 has FTS5 + trigram; LIKE path covers builds that don't.) - Ranking layers a time-decay weight (30d half-life) over relevance. Surfaces: - backend.search() now delegates to the recall store (⑤ seam filled). - mem_search tool is advertised (no longer withheld) — EN + CJK. - prefetch(): turn-start recall, LOCAL I/O ONLY (no MCP/network — Fable 5 §2), hard char cap (default 2000, memory.mem4.recall.prefetch_char_cap). - sync_turn(): per-turn indexing, filtered (min length, tool-output stripped; store dedups by SHA-256 content hash) — Fable 5 §5. Backfill (design spike §10.4): _backfill() is now real — resumable via the .mem4_state.json backfill_cursor, batched, rowid+hash dedup, background worker; mem_search results carry a [backfill in progress] note until done. Existing microfiles/mirror are indexed synchronously at init. A deployment injects a session-history source; without one, only microfiles are indexed. Rebuild (Fable 5 §5, fifth guarantee — derived layers always reconstructible): `hermes mem4 rebuild` (cli.py) clears and rebuilds the recall index from the source-of-truth files + backfill, never touching built-in MEMORY.md/USER.md. Tests: 15 new recall tests (EN FTS, CJK trigram path, short-CJK LIKE fallback, trigram-unavailable degrade, time decay, resumable+dedup backfill, mem_search e2e, sync_turn filter, prefetch cap+guardrail, backend delegation, rebuild consistency). Updated 2 ⑤ tests for the now-live mem_search + backfill_complete. No regression: mem4 (38), memory manager (100), compression fix (5) green. HOLD upstream — fork feature branch. toothless deploy (incl. backfilling real history) deferred to measurement (will report first). ② Auditor remains. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Instrument recall so mem4's value is measured with data, not estimates (design spike §7; Fable 5 review §6 point 4). Code + synthetic-data harness only — no toothless deploy, no real 907 writes. Auditor (audit.py): - Records one JSONL event per recall/route/prefetch to $HERMES_HOME/mem4/audit.jsonl (query, hit/miss, route fts/trigram/like, injected chars, prefetch). Enabled via memory.mem4.audit.enabled (default off). - Honest hit/miss: a tool-call miss is precise; L0-hit rate is estimated offline (marked, not faked). - Baserow 907 sink writes an AGGREGATE row mapped onto the table's EXISTING columns via an injected writer (never imports the MCP; tests use a mock). - 907 is aggregate-oriented and lacks per-event columns; audit.MISSING_907_FIELDS lists what richer per-event logging would need. The code does NOT alter the 907 schema — adding columns is left to the operator. A/B arm (memory.mem4.arm: experiment|baseline): - baseline loads mem4 but turns off all agent-facing surfaces (no tools, no system-prompt legend, no prefetch injection) so the hot-zone/tool surface matches pure built-in, while the recall store stays measurable. Switchable in one flag; each audit line carries its arm for grouping. QA harness (eval/harness.py + eval/qa_fixture.json, 24 items EN+ZH, exact + paraphrase): deterministic recall eval with no model in the loop, avoiding the "B has extra tools" confound (Fable 5 §3). Reports accuracy (overall/en/zh/ exact/paraphrase), route distribution, injected chars, and the §7 gate (SHIP/ROLL BACK). `hermes mem4 eval` runs it. On the synthetic fixture: experiment 67% vs baseline 0%, all three routes exercised, gate SHIP. Recall fix: FTS/trigram MATCH now ORs quoted tokens (natural-language queries should surface on ANY matching token, BM25-ranked) instead of implicit AND, which missed nearly every NL query. LIKE path already ORed. Paraphrase recall stays low by design — the documented FTS lexical weak spot (Fable 5 §3). Tests: 9 new (event recording hit/miss/route/prefetch, audit-off no-write, A/B arm gating both ways, summarize + Baserow-row-uses-existing-columns via mock, fixture shape, harness experiment>baseline + gate, gate rollback path). No regression: full mem4 suite (47) + memory manager (100) + compression fix (5). Real hit rates require deploying to toothless + collecting actual usage (operator-gated). HOLD upstream — fork feature branch only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…istributions Upgrade ② from "compare two random real-traffic segments" (confounded by traffic randomness) to CONTROLLED measurement. All metrics report distributions (min/median/max, quartiles), not single numbers. Gate hard-wired to §7. Three layers (eval/harness.py): 1. Deterministic offline replay (primary evidence, zero randomness): the SAME fixed input set is replayed against baseline (built-in) and mem4, so any difference is attributable to mem4. Per item: gold hit (PRECISE), injected tokens (PRECISE), route. Input = QA fixture (24) + an injectable "sampled from real session history" set (history_samples.json synthetic stand-in; load_history_samples(source=...) wires a real sampler at deploy). 2. Paired counterfactual (real traffic, paired): the Auditor now records, per query, baseline_inject_tokens (what pure built-in would inject — its whole resident memory) AND mem4_inject_tokens (legend + this query's recall). The harness reports the paired-difference distribution — robust to traffic mix. Wired in the provider (reads MEMORY.md+USER.md size once at init). 3. Resident cost (context-independent): session-open injection baseline (whole MEMORY.md) vs mem4 (short routing legend, now a measured constant), across N sessions. Gate (§7): SHIP iff mem4 recalls cold knowledge baseline can't (gold Δ≥30%), net per-query tokens shrank, and resident hot zone shrank — else ROLL BACK. Honesty: deterministic-replay gold hits are PRECISE; free real-traffic hits are ESTIMATED and labelled. `hermes mem4 eval` prints the full three-layer report. Also: extracted the routing legend to a module constant (ROUTING_LEGEND) so its size is measurable; extended audit.MISSING_907_FIELDS to enumerate what table 907 would need for per-event/paired/distribution data (the code still does NOT alter the 907 schema — aggregate export uses only existing columns, the rest packs into notes JSON). Demo (synthetic/fixture): layer1 gold mem4 67% vs baseline 0% (history 88%/0%), inject tokens/query mem4 median 88 vs baseline 544; layer2 paired diff median 442 tokens, mem4 cheaper 100%; layer3 resident 90% reduction; GATE SHIP. Tests: test_mem4_audit.py now 16 (per-layer + provider paired recording + gate rollback). No mem4 regression (full mem4 suite green; the 3 failing test_hindsight_provider setup tests are a pre-existing env issue — `uv` not installed — unrelated to mem4). HOLD upstream — fork feature branch. No toothless deploy, no real 907 writes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Found deploying to toothless: mem4 installed under $HERMES_HOME/plugins/ loads via the synthetic user namespace, so cli.py's absolute imports (`plugins.memory.mem4[.eval.harness]`) raise ModuleNotFoundError — `hermes mem4 rebuild` / `hermes mem4 eval` failed. The provider itself was fine (relative imports). Fix: _ensure_importable() adds the dir containing the mem4 package to sys.path so `import mem4.*` works in both bundled and user-plugin layouts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sam7894604
force-pushed
the
feat/mem4-provider-chassis
branch
from
July 5, 2026 08:54
9187bfe to
cae3565
Compare
Owner
Author
Superseded — closingmem4 has moved to a standalone plugin repository: https://github.com/sam7894604/hermes-plugin-mem4 That repo is now the canonical home for the four-tier routed-memory provider and has advanced well beyond this branch — it carries the audit→SQLite switch, the §3 The one core-only change from this PR — the compaction Closing this PR as superseded. The branch |
sam7894604
added a commit
that referenced
this pull request
Jul 13, 2026
…sion fallback) Two-part fix so attached PDFs are read reliably, platform-independently: 1. LINE adapter (#1 filename loss): the trigger path dropped the file's real fileName — every "file" cached as an anonymous .bin with media_type "file" (not application/pdf), so the agent couldn't tell it was a PDF. Now capture msg.fileName + guess MIME and pass them to cache_media_bytes(), caching as receipt.pdf / application/pdf like Telegram. _download_media returns (path, mime). 2. Gateway auto-extraction (#2/#3): GatewayRunner._auto_extract_pdf() runs at inbound time (not model-decided) — text-layer PDFs are inlined via pymupdf (free, instant); scanned PDFs with no text layer fall back to rendering each page and reading it through the vision auxiliary (_vision_read_scanned_pdf, whatever auxiliary.vision resolves to). Best-effort, never breaks the flow; pymupdf-unavailable degrades to None. Tests: TestDownloadMediaRouting (filename/mime preserved) + test_auto_pdf_ extraction (text inline / scanned->vision / non-pdf / no-pymupdf). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sam7894604
added a commit
that referenced
this pull request
Jul 22, 2026
…sion fallback) Two-part fix so attached PDFs are read reliably, platform-independently: 1. LINE adapter (#1 filename loss): the trigger path dropped the file's real fileName — every "file" cached as an anonymous .bin with media_type "file" (not application/pdf), so the agent couldn't tell it was a PDF. Now capture msg.fileName + guess MIME and pass them to cache_media_bytes(), caching as receipt.pdf / application/pdf like Telegram. _download_media returns (path, mime). 2. Gateway auto-extraction (#2/#3): GatewayRunner._auto_extract_pdf() runs at inbound time (not model-decided) — text-layer PDFs are inlined via pymupdf (free, instant); scanned PDFs with no text layer fall back to rendering each page and reading it through the vision auxiliary (_vision_read_scanned_pdf, whatever auxiliary.vision resolves to). Best-effort, never breaks the flow; pymupdf-unavailable degrades to None. Tests: TestDownloadMediaRouting (filename/mime preserved) + test_auto_pdf_ extraction (text inline / scanned->vision / non-pdf / no-pymupdf). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sam7894604
added a commit
that referenced
this pull request
Jul 22, 2026
…sion fallback) Two-part fix so attached PDFs are read reliably, platform-independently: 1. LINE adapter (#1 filename loss): the trigger path dropped the file's real fileName — every "file" cached as an anonymous .bin with media_type "file" (not application/pdf), so the agent couldn't tell it was a PDF. Now capture msg.fileName + guess MIME and pass them to cache_media_bytes(), caching as receipt.pdf / application/pdf like Telegram. _download_media returns (path, mime). 2. Gateway auto-extraction (#2/#3): GatewayRunner._auto_extract_pdf() runs at inbound time (not model-decided) — text-layer PDFs are inlined via pymupdf (free, instant); scanned PDFs with no text layer fall back to rendering each page and reading it through the vision auxiliary (_vision_read_scanned_pdf, whatever auxiliary.vision resolves to). Best-effort, never breaks the flow; pymupdf-unavailable degrades to None. Tests: TestDownloadMediaRouting (filename/mime preserved) + test_auto_pdf_ extraction (text inline / scanned->vision / non-pdf / no-pymupdf). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sam7894604
added a commit
that referenced
this pull request
Aug 9, 2026
…sion fallback) Two-part fix so attached PDFs are read reliably, platform-independently: 1. LINE adapter (#1 filename loss): the trigger path dropped the file's real fileName — every "file" cached as an anonymous .bin with media_type "file" (not application/pdf), so the agent couldn't tell it was a PDF. Now capture msg.fileName + guess MIME and pass them to cache_media_bytes(), caching as receipt.pdf / application/pdf like Telegram. _download_media returns (path, mime). 2. Gateway auto-extraction (#2/#3): GatewayRunner._auto_extract_pdf() runs at inbound time (not model-decided) — text-layer PDFs are inlined via pymupdf (free, instant); scanned PDFs with no text layer fall back to rendering each page and reading it through the vision auxiliary (_vision_read_scanned_pdf, whatever auxiliary.vision resolves to). Best-effort, never breaks the flow; pymupdf-unavailable degrades to None. Tests: TestDownloadMediaRouting (filename/mime preserved) + test_auto_pdf_ extraction (text inline / scanned->vision / non-pdf / no-pymupdf). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sam7894604
pushed a commit
that referenced
this pull request
Aug 11, 2026
…on delegation callbacks (NousResearch#82592) * fix(gateway): stop frozen-preview finals and dropped idle-session delegation callbacks Two relay-plane delivery losses from the 2026-08-09 staging incident: 1. stream_consumer: the skip-redundant-finalize branch recorded _accumulated as the delivered turn-final payload even when the last ACKED edit was an earlier throttled preview snapshot, so delivered_final_matches reconciled True and the gateway suppressed the corrective final send — the user was left with a cut-off message ending in the streaming cursor. Extracted _mark_skip_redundant_finalize(): records the last acked wire payload (cursor-stripped), so a preview/final mismatch now returns False and the normal final send fires. 2. run.py: _classify_completion_target classified every ended parent session terminal unless it ended by compression. Idle/timeout session ends are the norm on scale-to-zero relay deployments and the chat route remains valid; completed async delegation results were terminally dropped. Ended parents now classify deliver unless the end was an explicit user boundary (session_reset / user_exit / session_switch). * fix(relay): drain in-flight outbound frames before transport teardown disconnect() failed every pending outbound future immediately with 'relay transport closed', so a trailing finalize edit racing turn teardown was lost even though the connector socket could still serve it. Bounded drain grace (5s) lets in-flight requests resolve; silent connectors still tear down promptly. asyncio.wait (not gather+wait_for) so a timeout doesn't cancel futures owned by the fail-remaining loop. * fix(gateway): route completion injection through the alias-aware transport resolver Third relay-plane delivery loss from the 2026-08-09 staging incidents: a delegation batch completed while the gateway was up, the watcher drained the event, and delivery vanished with no log line. _inject_watch_notification resolved its adapter with a literal p.value == platform_name scan of self.adapters — a relay-fronted gateway registers ONE adapter under Platform.RELAY fronting N logical platforms, so 'slack' never matched and the injection returned None ('no gateway route'), silently dropping the completion. The handoff path already documents this exact trap and uses resolve_delivery_transport; the injection path now does the same (native wins; relay eligible only when it fronts the logical platform), with the literal scan kept as fallback for stub runners and exotic platforms. * fix(relay): clamp disconnect drain grace to the runner's adapter-disconnect budget Review finding (JoaoMarcos44, NousResearch#82592): a fixed 5.0s drain in front of the three 1.0s sequential teardown awaits gives an 8.0s worst case inside the runner's 5.0s asyncio.wait_for(adapter.disconnect()) — tripping it cancels teardown mid-drain, skips the fail-pending loop, and leaves outbound callers blocked until _OUTBOUND_TIMEOUT_S (30s). The effective grace is now budget - 3*TEARDOWN - margin (env-aware via the same HERMES_GATEWAY_ADAPTER_DISCONNECT_TIMEOUT the runner reads), so the drain can never push teardown past its caller's budget; a budget too small for any drain disables it cleanly. * test(gateway): pin the final-send suppression contract across a behaviour matrix The gateway skips its own final send when the stream consumer claims the turn final already reached the user. Every incident in that family — NousResearch#71643 (stale finalize snapshot), NousResearch#78541 (payload-less multi-message split), NousResearch#82656 (frozen preview left with a visible cursor) — is the same failure: the consumer claimed delivery for text the platform never rendered, so the corrective send was suppressed and the answer was lost with no retry. Each was fixed with a scenario test pinned to one branch of GatewayStreamConsumer.run(). The got_done handler now has five sibling branches that each set the suppression flags and record a turn-final payload, and nothing checks them as a group: a new branch, or a new early `return True` in _send_or_edit, can reintroduce the class without failing a test. Pin the invariant instead of the branch — if the consumer offers the gateway any signal it would trust, the complete final text must have reached the wire — and assert it across {edit always / dies / never / lies} x {send always / never} x {fresh-final on / off} x {clean / interrupted stream}. The adapter records only frames that actually rendered, so an ACK the platform drops does not count as delivery. 24 honest-transport scenarios hold the invariant as a hard assertion. The 16 lying-transport scenarios are checked too; the single combination that still violates it is reported as an expected failure documenting the open exposure rather than asserting it away. Refs NousResearch#82656 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(gateway,relay): prime relay egress routing for synthetic injections + cap stale completion replay Defect #4 from the 2026-08-09 staging incidents (upgrade-robustness): after every gateway restart the durable async-delegation replay injected completions correctly (post-741663cf1) but their replies bounced at the connector — 'slack egress declined: target not routed to an onboarded tenant'. The relay adapter re-attaches tenant discriminators (metadata.scope_id / metadata.user_id) from per-chat caches warmed ONLY by inbound traffic; synthetic turns race those cold caches on every deploy, scale-to-zero wake, and crash recovery. - relay adapter: prime_routing_cache() — feeds a synthetic event's session-store origin through the same _capture_scope used for real inbound (never raises). - run.py injection path: prime the resolved adapter before handle_message (duck-typed; native adapters unaffected). - async_delegation: 48h staleness cap in restore_undelivered_completions — a pending completion older than the cap is terminally dropped (payload stays queryable) instead of re-run as a fresh full-context turn; the post-restart replay of a July session burned a 102K-token context. Also carried: JoaoMarcos44's suppression behaviour-matrix harness (cherry-picked from NousResearch#82676, authorship preserved) — 39 passed + 1 xfail (the documented ACK-then-drop transport-honesty residue). * test: use recent timestamps in restored-ownership fixtures test_restore_stamps_restored_flag persisted its completion with epoch-era toy timestamps (dispatched_at=1.0), which the new 48h replay staleness cap correctly classifies as stale — the fixture then exercised the cap instead of the restored-flag contract (CI slice 4 failure). Timestamps are now now-relative; the staleness behavior itself is pinned separately in test_relay_injection_egress_priming.py. * fix(gateway,relay): close four review findings on the relay delivery fixes Review follow-ups on this branch (NousResearch#82592): 1. HIGH — classifier/resolver mismatch (falsely-acknowledged loss). _classify_completion_target now returns "deliver" for idle-ended parents, but _resolve_async_delegation_session still dropped every non-compression-ended pin: the durable row was acked at adapter acceptance, then the injection died inside the pipeline with no retry — strictly worse than the honest terminal drop on main, and the delivery leg defect #2's fix depends on did not exist. The resolver now retargets non-user-boundary ends (idle/timeout/ lifecycle) to the chat's current session — session_entry already IS the routing key's current session for the same chat — while user boundaries (session_reset / new_session / user_exit / session_switch) stay fail-closed. Both sides share one module-level _USER_BOUNDARY_END_REASONS so the verdict and the routing decision cannot drift again; a coherence test asserts deliver-verdicts resolve non-None across representative end reasons. 2. HIGH — drain clamp missed adapter-level spend. The effective drain grace budgeted drain + 3x teardown, but RelayAdapter.disconnect spends revocation-monitor teardown + go_idle time BEFORE the transport drain inside the same runner wait_for; worst case still blew the budget and cancelled teardown mid-drain (skipping the fail-pending loop). The adapter now measures its own elapsed time and threads the REMAINING budget into transport.disconnect(budget_s=...); legacy/stub transports without the keyword fall back to the no-arg signature. 3. P1 — _request_response racing disconnect() could register a future after the fail-pending loop already ran, stranding the caller for the full _OUTBOUND_TIMEOUT_S (30s). Fail fast with the same "relay transport closed" error once _closing is set. 4. P1 — _build_process_event_source's last-resort reconstruction dropped scope_id, so a scoped relay completion whose session-store origin was unavailable primed no tenant discriminator and could still bounce off the connector's fail-closed egress guard. scope_id now threads through the reconstructed SessionSource, with a warning when a scoped chat reconstructs without one. All four: RED reproduced with the fix reverted, GREEN after; relay/ delegation delivery families pass (43 + 71 + 179 across the touched suites); full tests/gateway run shows only failures already failing identically on merge base 2446c8b (env/dep issues). * fix(gateway,relay): make pending-frame failure cancellation-safe; persist completion routing origin Two remaining review findings on this branch (NousResearch#82592): 1. Cancellation could strand outbound waiters past the fail-pending loop. transport.disconnect() failed pending futures only at the END of the drain + three teardown awaits; a cancellation landing mid-drain (the runner's wait_for budget, an outer cleanup deadline) skipped the loop entirely and left registered futures unresolved — their callers blocked until _OUTBOUND_TIMEOUT_S (30s). The budget threading added earlier shrinks the window but is not a hard guarantee. The fail-pending loop (and the going_idle ack failure) now run in a `finally`, so no exit path — normal, error, or cancelled — can leave a registered future unresolved. Idempotent: done futures are skipped, a second disconnect() pass is a no-op. 2. Durable completions did not persist their routing origin, so the scope_id threading in the fallback SessionSource reconstruction had nothing to carry on the exact path it exists for (restart replay with session store + source cache gone): the async-delegation event producers never populated scope_id and the durable rows never stored it. Dispatch now snapshots the originating turn's scope_id/user_id/user_name from the session context (_capture_routing_origin — a new HERMES_SESSION_SCOPE_ID contextvar bound by the gateway at session-bind time alongside the existing vars), stores them in the existing task_json payload (no schema migration), and re-attaches them to all three completion-event shapes (live single, live batch, crash-recovery rebuild). The gateway's fallback reconstruction then primes both discriminators after a restart. Tests: cancellation mid-drain -> every pending future resolves with "relay transport closed" (mutation: moving the loop out of the finally goes RED); second-pass disconnect idempotence; end-to-end dispatch -> owner-death recovery -> event carries scope_id -> fallback SessionSource primes it (mutations: dropping the dispatch capture or the task_json persistence both go RED); live completion event carries the origin. 94 passed + 1 xfailed across the delivery/delegation suites; tests/tools delegation family 73 passed (2 collection errors pre-existing on merge base 2446c8b). --------- Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Ben Barclay <ben@nousresearch.com>
sam7894604
added a commit
that referenced
this pull request
Aug 11, 2026
…sion fallback) Two-part fix so attached PDFs are read reliably, platform-independently: 1. LINE adapter (#1 filename loss): the trigger path dropped the file's real fileName — every "file" cached as an anonymous .bin with media_type "file" (not application/pdf), so the agent couldn't tell it was a PDF. Now capture msg.fileName + guess MIME and pass them to cache_media_bytes(), caching as receipt.pdf / application/pdf like Telegram. _download_media returns (path, mime). 2. Gateway auto-extraction (#2/#3): GatewayRunner._auto_extract_pdf() runs at inbound time (not model-decided) — text-layer PDFs are inlined via pymupdf (free, instant); scanned PDFs with no text layer fall back to rendering each page and reading it through the vision auxiliary (_vision_read_scanned_pdf, whatever auxiliary.vision resolves to). Best-effort, never breaks the flow; pymupdf-unavailable degrades to None. Tests: TestDownloadMediaRouting (filename/mime preserved) + test_auto_pdf_ extraction (text inline / scanned->vision / non-pdf / no-pymupdf). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sam7894604
pushed a commit
that referenced
this pull request
Aug 19, 2026
Addresses both review findings on the remote-gateway download PR: 1. Unbounded buffering (finding #1). fetchBuffer / fetchBufferViaOauthSession accumulated the entire response (then copied it again via Buffer.concat) before saveGatewayFile even opened the save dialog, so a large gateway file could exhaust the native process. Both auth paths now stream: once response headers arrive the connect timeout is cleared, the filename is derived, the save dialog is shown, and the body is piped to the chosen destination with backpressure. A read/write error tears down the stream and unlinks the partial file. The byte-moving, data-URL decoding, and filename/path helpers are extracted into gateway-file-download.ts so they're unit-testable without Electron. 2. No fallback for older gateways (finding #2). saveGatewayFile required the new /api/fs/download route. Desktop and the remote gateway update independently, so a gateway predating this PR 404s. Added a 404-only compatibility fallback to the existing capped /api/fs/read-data-url route (bounded, so it only serves smaller files — enough to keep older backends working). Tests: gateway-file-download.test.ts covers streaming, backpressure, error-cleanup (unlink on write/response error), data-URL decoding, filename derivation (incl. traversal reduction), and 404 detection; gateway-file-download-transport.test.ts asserts both transports stream (no whole-body Buffer.concat) and that the 404 fallback is wired. Both registered in the desktop platform test list. Server-side /api/fs/download tests (streaming + sensitive-file reject) already pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sam7894604
pushed a commit
that referenced
this pull request
Aug 19, 2026
Two independent bugs let a deleted profile reappear / leave orphaned resources on next launch: 1. hermes_cli/profiles.py's backend-process scanner required argv[0] to resolve to an executable literally named "hermes". Electron's pool-backend spawn resolves the hermes console-script shim's path and execs it via the interpreter directly (python3 /path/to/hermes ...), so argv[0] reports as "python3" and the scanner never matched the running backend -- delete removed the profile's files but left its live backend process running (still bound to a port via uvicorn), which accumulates across repeated delete/recreate cycles. 2. The desktop sidebar's ProfileRail only refreshed its cached profile list once, on mount, so a delete/create/rename from another surface (another window, or the CLI) left a stale ghost entry until something unrelated triggered a refetch. Note: a delete via this window's own Manage-Profiles view already refreshes the shared $profiles atom ProfileRail subscribes to (confirmed by reading refreshProfiles() and handleConfirmDelete()) -- this fix only covers the cross-window/cross- process staleness gap, not a duplicate of the already-merged NousResearch#57329's Manage-Profiles rail-refresh work. Fix 1: recognize a python-interpreter argv[0] exec'ing a hermes-named console-script shim via argv[1]. Fix 2: refresh the profile list on window focus/visibilitychange, matching the existing pattern used elsewhere in the sidebar (sidebar/index.tsx, use-background-sync.ts, star-map.tsx, use-gateway-boot.ts all use the same focus+visibilitychange pattern). ## Related work already on main PR NousResearch#57329 (merged) fixed the *headline* symptom from issue NousResearch#52279 (deleted profile respawns) via a different, non-overlapping mechanism: routing profile-delete through the primary backend instead of spawning a fresh pool backend, plus a separate recreation guard in ensure_hermes_home() (NousResearch#49435, merged) that makes a backend spawned into a deleted profile's directory raise FileNotFoundError instead of silently recreating it. This PR is NOT a duplicate of that fix. Verified: even with both of those merged, a backend process that survives because of gap #1 above still holds a bound port via uvicorn -- it just can no longer resurrect the profile directory. That's real resource-hygiene, not a symptom already covered. Gap #2 touches a different file/component (ProfileRail / profile-switcher.tsx) than NousResearch#57329's rail-refresh half (which touched the Manage-Profiles view's own $profiles.ts / index.tsx) and covers a distinct staleness path (cross-window/cross-process, not same-window delete-then-refresh). Tests: tests/hermes_cli/test_profiles.py -- 156 passed (existing + regression coverage for the argv[0] python-interpreter detection case). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mem4 — four-tier routed memory as a Hermes MemoryProvider
Wraps the L0/L1/L2/L3 routed-memory design (ADR-018) as a pluggable
MemoryProviderin coexist / augment mode: it strengthens the built-inMEMORY.md/USER.mdbut never replaces or writes them back. Removingmemory.provider: mem4degrades cleanly to pure built-in, zero residue.All code lives under
plugins/memory/mem4/+ tests undertests/plugins/memory/. Nothing else in the tree is touched except one small core fix (see ⑤-fix).Spine (⑤ → ④ → ① → ②) + ⑤-fix
mem_route(code)→ L2/L3 microfile read with freshness tags + graceful miss, routing legend in system prompt & pre-compression, mirror of built-in writes into mem4-owned files only, switchable storage backend (default local-file), idempotent one-time init + version marker (§10).fix(compression)) — the compaction path calledon_pre_compress()but discarded its return value, so the hook ran but its text went nowhere (upstream Context compression should integrate memory provider context into the summary NousResearch/hermes-agent#23367). Now captured and injected into the compaction summary. Additive, backward-compatible.recall.db) indexing turns + microfiles. Chinese support via the upstreamhermes_state.pydual-table pattern (unicode61 + trigram +_contains_cjkrouting + LIKE fallback). Powersmem_search+prefetch(local-I/O-only, char-capped); resumable backfill via cursor;hermes mem4 rebuildreconstructs derived state from source files.hermes mem4 eval.Fixture demo (synthetic — mechanism proof only)
Tests
Full mem4 suite green (chassis, dream, recall EN/ZH + CJK routing + LIKE fallback, audit 3-layer, paired recording, gate). No regression in
tests/agent/test_memory_provider.py(100) or the compression fix (5).Fork feature branch only — not for the NousResearch upstream yet. Real hit rates require deploying to toothless and collecting actual usage; the same harness +
audit.jsonlthen run against real data before any value is claimed.🤖 Generated with Claude Code
Summary by CodeRabbit
mem4routed memory provider with search, routing, prefetch, mirroring, and session/tier-based behavior.hermes mem4commands for index rebuild and recall evaluation.mem4provider README.mem4recall/audit/dream/provider/eval behavior.