Skip to content

feat(session-search): scope default discovery to the current shared chat - #59295

Closed
lkz-de wants to merge 1 commit into
NousResearch:mainfrom
lkz-de:feat/session-search-shared-scope
Closed

feat(session-search): scope default discovery to the current shared chat#59295
lkz-de wants to merge 1 commit into
NousResearch:mainfrom
lkz-de:feat/session-search-shared-scope

Conversation

@lkz-de

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

Copy link
Copy Markdown
Contributor

Summary

In shared gateway contexts, default session_search() discovery should restrict results to the current conversation scope instead of searching across unrelated chats by default.

This patch filters default discovery and title matches to the current source, chat_id, and optional thread_id when those bindings exist. Local/CLI contexts remain global, and explicit session reads remain unchanged.

Why

When Hermes is running inside a shared chat platform, a plain session_search(query=...) is usually meant to recall prior discussion from the current room or thread. Searching globally by default can pull in unrelated sessions from other chats on the same platform, which is noisy and can mislead the agent about what was previously discussed in the current conversation.

Fix

  • derive the current shared-chat recall scope from session environment bindings
  • leave local/CLI sessions unscoped so desktop/terminal recall stays global
  • filter default discovery results to the current source + chat_id, and thread_id when present
  • apply the same scope filter to title-match fallback results
  • add regression coverage for shared chat/thread scoping

Testing

Run:

python3 -m py_compile tools/session_search_tool.py tests/tools/test_session_search.py
python3 -m pytest -q tests/tools/test_session_search.py

Result:

  • 54 passed in 1.90s

Overlap check

Risk

Low. This changes only default discovery behavior in shared gateway contexts. Explicit session reads remain unchanged, local/CLI recall stays global, and the scoped behavior applies only to default shared-context discovery and title-match fallback.

In shared gateway contexts, default session-search discovery should prefer
the current conversation scope instead of searching across unrelated chats
by default.

Filter discovery and title matches to the current `source`, `chat_id`, and
optional `thread_id` when those bindings exist, while keeping local/CLI
contexts global.

Also add focused regression coverage for shared-context scoping.

Tested:
- python3 -m py_compile tools/session_search_tool.py tests/tools/test_session_search.py
- python3 -m pytest -q tests/tools/test_session_search.py
@alt-glitch alt-glitch added type/feature New feature or request comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have labels Jul 6, 2026
@lkz-de

lkz-de commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Maintainer note on scope: this patch intentionally narrows only the default discovery path (session_search(query=...)) and the title-match fallback in shared chat contexts. Explicit session reads by id are unchanged and remain the escape hatch for cross-chat / cross-thread recall.

Also, the scope check is applied after the widened discovery scan rather than as a SQL predicate. That keeps this change small and local to the default-discovery behavior, but it does mean a very busy multi-chat DB could still benefit from a future DB-level scope pushdown or an explicit global-discovery override if maintainers want that later.

@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 identifying a real current-main recall boundary gap: tools/session_search_tool.py:513-522 performs unscoped discovery before shaping results.

Problems

  • The new filter at tools/session_search_tool.py:573 runs after the fixed 300-row global scan. Foreign-chat matches can consume that budget, leaving an in-scope result undiscovered.
  • The proposed source/chat/thread predicate misses the gateway's default per-user group isolation. gateway/config.py:700 defaults group_sessions_per_user=True, and gateway/session.py:949-957 makes participant identity part of the live session key; the new predicate can return another participant's history from the same group.
  • website/docs/user-guide/sessions.md:528 still describes search as spanning all past conversations.

Suggested changes

  • Filter at query time using the persisted routing session_key, which hermes_state.py:754 stores and gateway/session.py:1505-1514 records. Apply it to title lookup, browse, and each FTS/CJK retrieval path.
  • Add per-user and scan-budget regression coverage, then document the gateway default.

Automated hermes-sweeper review.

@@ -530,6 +573,11 @@ def _discover(
# within each class.

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 runs only after search_messages() has returned its global _DISCOVER_SCAN_LIMIT rows. A high-volume foreign chat can consume the scan budget before an in-scope hit is fetched, so the result becomes empty/incomplete. Please push the exact gateway conversation predicate into the DB retrieval path instead.

@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-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@lkz-de

lkz-de commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — the two real trade-offs here are the post-scan budget loss and the missing per-user isolation inside shared groups.

I agree the current post-filter shape can still miss an in-scope result if foreign-chat hits consume the fixed scan budget first, and it also does not encode the default per-user group isolation that the live gateway session key already carries.

My intent with this PR is still to land the narrow default-scoping behavior first rather than switch the whole tool over to session-key-driven SQL-time filtering in the same lane. If maintainers would rather require the session-key predicate as part of the first step, I can rework it in that direction; otherwise I’m happy to keep the per-user/session-key-aware pushdown plus stronger diagnostics/tests as the follow-up lane rather than bundling both scopes of change together here.

Either way I'll fix the sessions.md wording that still describes default search as spanning all past conversations. Worth noting the per-user point is a contract gap in this PR's scoping claim rather than a regression — current main's default discovery is fully global.

@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026
@lkz-de

lkz-de commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Status note: #70105 is the successor to this PR — it scopes recall by stored chat-origin metadata with fail-closed behavior in shared contexts and an explicit scope="all" escape hatch, rather than the post-search filter used here. I'm keeping this open only until the recall-scope policy decision on #70105 is made; once a direction is chosen there (or via #50030), I'll close this one as superseded.

@lkz-de

lkz-de commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Closing this as superseded by #70105.

As of its current head (0a93400), #70105 covers the default shared-chat scoping this branch targets — including the DM/default gateway-origin cases — and explicitly accounts for this PR alongside #50030 and #30502. It is green and mergeable, so there's no reason to keep this narrower post-filter variant open in parallel.

Per my status note above, this was staying open only until #70105's direction settled; with #70105 refreshed and green, it's the clear successor.

@lkz-de lkz-de closed this Aug 10, 2026
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/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants