Skip to content

fix(session_search): scope recall by chat origin - #70105

Open
lkz-de wants to merge 1 commit into
NousResearch:mainfrom
lkz-de:fix/session-search-origin-scope
Open

fix(session_search): scope recall by chat origin#70105
lkz-de wants to merge 1 commit into
NousResearch:mainfrom
lkz-de:fix/session-search-origin-scope

Conversation

@lkz-de

@lkz-de lkz-de commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add chat-origin provenance to session_search results so recall hits identify the conversation they came from.
  • Default gateway-origin discovery and browse calls to the current chat/session scope, including DMs, while preserving explicit cross-conversation recall via scope="all".
  • Carry the scope predicates through every search path, including stale-FTS LIKE fallback, and expose scoped-result provenance through same_origin, origin, and recall_scope counts.
  • Pass gateway_session_key through background turns and update English and zh-Hans session-search docs.

Why

A fresh or compacted gateway session can receive an ambiguous follow-up after its local context has expired. Before this change, session_search searched the whole profile by default and returned matching sessions without enough visible room provenance. That made a hit from another conversation easy to treat as if it came from the active chat.

The session table already stores routing provenance such as source, chat_id, chat_type, display_name, and session_key. This PR carries that metadata through the search layer and uses it to make the safer default enforceable in SQL instead of relying only on prompt guidance.

When same-chat origin cannot be established, scope="chat" fails closed. Legacy rows without chat_id or session_key are excluded from scope="chat", counted in recall_scope, and remain reachable through explicit scope="all".

Testing

  • python3 -m pytest tests/tools/test_session_search.py -q
    • 53 passed in 2.33s
  • uv run --with pytest --with pytest-asyncio --with python-dotenv python -m pytest tests/tools/test_session_search.py tests/test_hermes_state.py tests/agent/test_prompt_builder.py tests/gateway/test_background_command.py -q --maxfail=1
    • 345 passed in 13.65s
  • python3 -m py_compile tools/session_search_tool.py hermes_state.py hermes_state_search.py agent/tool_executor.py agent/agent_runtime_helpers.py gateway/run.py tests/tools/test_session_search.py tests/gateway/test_background_command.py
  • git diff --check

Overlap check

This refresh keeps this PR as the implementation vehicle and folds in runtime pieces that were missing relative to current main. #59295 covers a narrower default-scoping approach and is superseded by this PR's broader search-path coverage, provenance labels, docs, and tests. The schema-backed scope-key design in #50030 remains the main open alternative; if maintainers prefer that direction, this PR's provenance labeling could be rebased onto it. Related prompt-only guidance (#30502) remains useful, but it does not replace code-level SQL scoping and visible provenance in session_search itself.

Notes

  • Gateway contexts with a chat/session origin now default discovery and browse to scope="chat"; CLI and no-origin contexts continue to default to global recall.
  • profile= recall deliberately defaults to global scope rather than applying the current gateway room to a different profile database.
  • The model can still request scope="all"; the safety improvement is scoped defaults plus visible provenance, not a hard access-control boundary.
  • Background-turn behavior is covered at the gateway_session_key handoff seam rather than by a full end-to-end gateway test.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state needs-decision Awaiting maintainer decision before any implementation labels Jul 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #50030 and #59295. This patch uses chat-origin metadata and fail-closed shared-chat scoping; the open alternatives use a schema-backed scope-key design or a narrower tool-only filter. Please select the intended recall-scope policy.

@lkz-de

lkz-de commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

On the scope-policy question: I opened both #59295 and this PR, and this PR is the successor. #59295 narrows only the default-discovery path with a tool-local post-search filter; this PR scopes recall by stored chat-origin metadata, fails closed in group-like contexts, and keeps explicit cross-conversation recall via scope="all" with same_origin labeling. I'd suggest evaluating this PR as the fail-closed candidate and treating #59295 as superseded — I'll close it once a direction is chosen. If the alternative design in #50030 is preferred instead, the provenance-labeling portion of this PR could be rebased onto that.

@lkz-de

lkz-de commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this branch onto current main and resolved the tools/session_search_tool.py conflict by preserving both the current session-link/profile handling and this PR's chat-origin scoping behaviour.

Local verification:

  • scripts/run_tests.sh tests/tools/test_session_search.py tests/test_hermes_state.py tests/agent/test_prompt_builder.py -- --tb=short -q

@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 carrying origin metadata through the result shape and pushing the initial filter into the database query.

Problems

  • tools/session_search_tool.py:272-276 defines same_origin as only source + chat_id. That crosses distinct forum topics and the default per-user group boundary: gateway/config.py:918 defaults group_sessions_per_user=True, and gateway/session.py:1053-1133 includes thread and participant identity in live routing.
  • The same incomplete predicate is used for browse at tools/session_search_tool.py:520-521 and discovery at :816-830, so affected foreign sessions can be returned as local history.
  • Current main moved the search implementation to hermes_state_search.py in 21c7ae8563; the required predicate must cover its FTS, CJK, trigram, LIKE, and unindexed-gap paths. website/docs/user-guide/sessions.md:549 also still says search spans all past conversations.

Suggested changes

  • Scope against the persisted live routing identity (or an exactly equivalent source/chat/thread/user predicate) across every retrieval path, and add topic and per-user isolation regressions.
  • Update the public session-search documentation with the scoped default and explicit global escape hatch.

Automated hermes-sweeper review.

Comment thread tools/session_search_tool.py Outdated
origin = _origin_from_meta(meta)
same_origin = None
if current_origin and current_origin.get("chat_id"):
same_chat = bool(meta.get("chat_id") and meta.get("chat_id") == current_origin.get("chat_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.

same_origin only compares chat_id and source. Gateway routing distinguishes thread_id and, for ordinary groups by default, the participant (gateway/session.py:1053-1133); this labels those foreign sessions as local and bypasses the intended fail-closed boundary.

exclude_sources=list(_HIDDEN_SESSION_SOURCES),
limit=_DISCOVER_SCAN_LIMIT, # widen so dedup-by-lineage can find
# distinct sessions AND so interactive matches buried under a wall
# of cron rows are still in hand for the demotion pass below.
offset=0,
sort=sort,
chat_id=scoped_chat_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.

Filtering only on chat_id and source leaves same-chat forum topics and per-user group sessions in scope. Please use the persisted routing identity, or an equivalent predicate including thread and the applicable participant boundary, across every search backend.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users area/sessions Session lifecycle, resume, persistence, history labels Jul 30, 2026
@lkz-de
lkz-de force-pushed the fix/session-search-origin-scope branch from c1fa65e to fffa5bc Compare August 1, 2026 16:37
@lkz-de

lkz-de commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Rebased this onto current main at 470cf66b0 and pushed head fffa5bc55aec55d6bbfbc440e3a22c1510a9ffd6.

The refresh keeps main's current list_sessions_rich source / sources / session_key behaviour and layers the chat-origin scope on top with an AND-composed chat_id filter. I also re-ported the original origin-scope regression tests, covering the cross-chat search/browse filters, scope="all" foreign-result labelling, read/scroll foreign-origin warnings and the registered handler path.

Verification run on the refreshed head:

uv run --with pytest --with python-dotenv python -m pytest tests/tools/test_session_search.py tests/test_hermes_state.py tests/agent/test_prompt_builder.py -q --maxfail=1
244 passed in 10.03s

@lkz-de
lkz-de force-pushed the fix/session-search-origin-scope branch from fffa5bc to 23b268b Compare August 3, 2026 15:02
@lkz-de

lkz-de commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this branch onto current main and tightened the default chat-scoped predicate.

The chat-scoped search path now prefers the stored session_key when available, so per-user group sessions and thread-scoped sessions do not collapse merely because they share the same platform chat id. It falls back to chat_id only when the current session has no stored session key; rows without a session key are excluded from key-scoped recall and remain reachable via scope='all'. The default scope='chat' behaviour, the scope='all' escape hatch and the cross-context warnings are still covered.

Focused verification passed locally:

scripts/run_tests.sh tests/tools/test_session_search.py tests/test_hermes_state.py tests/agent/test_prompt_builder.py -- --tb=short -q
# 283 passed, 0 failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants