Skip to content

fix(memory): align Honcho routing and memory guidance - #17186

Closed
nepenth wants to merge 4 commits into
NousResearch:mainfrom
nepenth:fix/honcho-profile-memory-routing-20260428
Closed

fix(memory): align Honcho routing and memory guidance#17186
nepenth wants to merge 4 commits into
NousResearch:mainfrom
nepenth:fix/honcho-profile-memory-routing-20260428

Conversation

@nepenth

@nepenth nepenth commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR fixes Honcho memory/profile routing and built-in memory guidance.

  • Align Honcho peer-card writes with the same observer/target route used by profile reads, so honcho_profile(peer="user", card=[...]) updates the card that honcho_profile(peer="user") returns.
  • Align Honcho prefetch/session context routing with manual profile routing when assistant-observes-user mode is enabled, so auto-injected context and explicit tools see the same peer perspective/target.
  • Clarify honcho_profile card replacement semantics: passing card overwrites the whole peer card, so callers MUST include facts they want preserved.
  • Remove the advertised honcho_context.query parameter for now because the handler did not implement query filtering; this avoids a misleading schema until filtering is added deliberately.
  • Improve built-in memory-full guidance so agents use memory.replace / memory.remove, skills, or canonical workspace artifacts instead of claiming to save exact state “mentally” or relying on semantic memory as the only canonical store.
  • Keep memory limit defaults aligned across config/runtime/migration surfaces and add broader drift coverage.

Tests

  • python -m pytest tests/honcho_plugin/test_session.py tests/tools/test_memory_tool.py tests/tools/test_memory_tool_import_fallback.py -q -o 'addopts=' → 162 passed
  • Targeted regressions added/covered:
    • peer-card write/read observer-target alignment
    • prefetch auto-injected context routing for assistant-observes-user mode
    • session-context peer perspective/target routing
    • honcho_profile handler empty-card overwrite behavior
    • honcho_profile schema warning that card overwrites the entire card
    • honcho_context schema no longer advertising an unimplemented query filter
    • memory-full overflow guidance naming memory.replace, memory.remove, skills, and canonical artifacts
    • memory limit defaults checked across DEFAULT_CONFIG, MemoryStore, and the OpenClaw migration helper
  • python -m py_compile plugins/memory/honcho/__init__.py plugins/memory/honcho/session.py tools/memory_tool.py hermes_cli/config.py run_agent.py hermes_cli/tips.py optional-skills/migration/openclaw-migration/scripts/openclaw_to_hermes.py
  • git diff --check
  • Static added-line scan for obvious secret/shell/eval/pickle/SQL-risk patterns → clean
  • Independent local review pass performed on the patch set before push.

Security / platform notes

  • No credentials, tokens, network endpoints, or provider secrets are added.
  • No shell execution, subprocess behavior, SQL generation, filesystem deletion, or migration side effects are added.
  • The Honcho behavior change is limited to client-side peer routing/schema/guidance; it does not change Honcho server storage semantics.
  • The memory-limit drift test imports the migration helper for constants only; it does not execute a migration.
  • This PR intentionally does NOT implement honcho_context(query=...) filtering; the misleading schema field is removed instead. Query filtering can be a focused follow-up.

Review feedback remediation (2026-04-30)

  • Added regression coverage for honcho_profile(peer="ai", card=[...]) round-tripping through the same observer/target route used by reads.
  • Confirmed existing regression coverage for card=[] clearing peer cards instead of being treated as a read.
  • Confirmed honcho_context no longer advertises the unimplemented query schema parameter.
  • Updated the PR title/scope language to make Honcho routing and memory guidance/default alignment explicit.

Relationship to context-safety work

Related but independent from the gateway context-safety PRs. This PR fixes Honcho memory/profile routing consistency so long-term context is read and written through the correct observer/target path.

Adjacent PRs:

Ensure Honcho peer-card writes use the same observer/target path as reads so profile card updates land on the card that later lookups return.

Increase built-in memory headroom and make overflow guidance route durable facts to the correct persistence layer instead of implying mental retention.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/memory Memory tool and memory providers area/config Config system, migrations, profiles labels Apr 29, 2026
@nepenth
nepenth marked this pull request as ready for review April 29, 2026 01:21
@nepenth
nepenth marked this pull request as draft April 30, 2026 13:28
@nepenth nepenth changed the title fix: align Honcho card writes and memory routing fix(memory): align Honcho routing and built-in memory guidance Apr 30, 2026
@nepenth nepenth changed the title fix(memory): align Honcho routing and built-in memory guidance fix(memory): align Honcho routing and memory guidance Apr 30, 2026
@nepenth
nepenth marked this pull request as ready for review April 30, 2026 15:20

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the detailed routing coverage. The underlying remaining bugs are still present on current main, but this April patch needs selective salvage rather than a mechanical apply.

Problems

  • plugins/memory/honcho/session.py:660 changes prefetch routing but drops search_query=user_message or None. Current main deliberately added that behavior in 0a7cc85eab299667777489a31995eb324d8b7818 and uses it at plugins/memory/honcho/session.py:736; retain it while changing the observer/target route.
  • The run_agent.py default hunk no longer reaches the active initialization path. Current main constructs the store in agent/agent_init.py:1341-1344; tools/memory_tool.py:804-818 also has independent no-agent fallback defaults.
  • tests/tools/test_memory_tool.py:45-65 snapshots numeric defaults. AGENTS.md:80-83 requires an invariant between surfaces rather than literals expected to change.

Suggested changes

  • Preserve search_query=user_message or None and test it together with the observer/target route.
  • Update the moved live and no-agent construction paths, then make the drift test compare each surface to DEFAULT_CONFIG.

This is an automated hermes-sweeper review.

try:
user_ctx = self._fetch_peer_context(session.user_peer_id, target=session.user_peer_id)
observer_peer_id, target_peer_id = self._resolve_observer_target(session, "user")
user_ctx = self._fetch_peer_context(observer_peer_id, target=target_peer_id)

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.

Please retain search_query=user_message or None on this call. Current main intentionally added it in 0a7cc85eab299667777489a31995eb324d8b7818; omitting it would regress topic-relevant prefetch retrieval while fixing the observer route.

Comment thread run_agent.py
self._memory_store = MemoryStore(
memory_char_limit=mem_config.get("memory_char_limit", 2200),
user_char_limit=mem_config.get("user_char_limit", 1375),
memory_char_limit=mem_config.get("memory_char_limit", 3200),

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.

This initialization code has moved on current main to agent/agent_init.py:1341-1344; also update that live path and tools/memory_tool.py:804-818 so the new defaults apply consistently to normal and no-agent memory stores.


from hermes_cli.config import DEFAULT_CONFIG

expected_memory = 3200

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.

Please avoid freezing these numeric defaults. Derive the expected values from DEFAULT_CONFIG and assert the constructor and migration constants agree, preserving drift coverage without making a routine capacity change fail CI.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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 12, 2026
@nepenth

nepenth commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Cleanup triage (2026-07-17)

Status: leave open for selective salvage — do not merge as-is.

  • Sweeper (2026-07-12): remaining routing bugs still present on main, but April patch needs selective salvage.
  • Blocking notes already on PR: keep search_query=user_message, update live init path (agent/agent_init.py), avoid freezing numeric defaults in tests.

Next if pursued: cherry-pick only routing fixes onto fresh main; drop change-detector default freezes; re-open as a narrow Honcho routing PR if needed.

@nepenth

nepenth commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Dual-model cleanup decision (2026-07-17)

Reviewers: gpt-5.6-sol xhigh (Codex) + gpt-5.6-sol (Hermes openai-codex).

Decision: SALVAGE_NARROW later — do NOT mechanical-rebase this tip; do NOT roll into Matrix Ship v2 stack.

If/when pursued:

  1. Fresh branch from origin/main
  2. Port only still-relevant Honcho routing guidance
  3. MUST keep search_query=user_message or None (main already uses it)
  4. Update live init in agent/agent_init.py (+ memory_tool no-agent path)
  5. Invariant tests vs DEFAULT_CONFIG — no frozen numeric defaults

Leave open only as a salvage pointer; no private dogfood of this tip as-is.

@nepenth

nepenth commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Closing — no longer relevant for our deployment

Closing unmerged.

We are not using Honcho for memory anymore (Hindsight / other paths). This PR was a Honcho routing + built-in memory guidance salvage of an old tip and is not worth re-cutting for us.

If upstream still wants the remaining routing fixes without the April tip’s default freezes, that should be a fresh contribution from current main by someone who still runs Honcho — not this branch.

Not part of Matrix Ship v2. Do not resurrect for our private feature stack.

@nepenth nepenth closed this Jul 18, 2026
@teknium1 teknium1 added the area/memory Memory subsystem: store, providers, sync, background reviews label Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles area/memory Memory subsystem: store, providers, sync, background reviews comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants