Skip to content

Scope Telegram session_search to the current conversation - #83829

Open
benperry6 wants to merge 1 commit into
NousResearch:mainfrom
benperry6:fix/session-search-telegram-scope-20260811
Open

benperry6 wants to merge 1 commit into
NousResearch:mainfrom
benperry6:fix/session-search-telegram-scope-20260811

Conversation

@benperry6

Copy link
Copy Markdown
Contributor

Summary

  • default Telegram session_search calls to the durable current chat_id / thread_id plus parent-child lineage
  • apply the scope to discovery, browse, read, and scroll
  • fail closed when Telegram provenance is missing or mismatched
  • require explicit scope=all for global or cross-profile history
  • preserve existing global behavior for non-Telegram callers and preserve compression-lineage recall

This intentionally reuses existing session metadata instead of adding a new scope column, migration, contextvar, or persistence layer.

Tests

  • pytest -q -o addopts= tests/tools/test_session_search.py tests/agent/test_session_search_scope_context.py tests/tui_gateway/test_session_db_ownership_teardown.py
  • 62 passed
  • compileall, git diff --check, and Windows-footgun scan pass

@alt-glitch alt-glitch added type/feature New feature or request comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint platform/telegram Telegram bot adapter area/sessions Session lifecycle, resume, persistence, history P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 11, 2026
@benperry6
benperry6 force-pushed the fix/session-search-telegram-scope-20260811 branch from a88da14 to 45a9cfe Compare August 11, 2026 10:33
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

Scope Telegram session_search to the current conversation

  1. tools/session_search_tool.py_is_session_in_telegram_scope (~line 364-381) calls _resolve_lineage(db, session_id) for every candidate row, and it is invoked per-row inside the paging loops of both _discover (~line 575-603 and again at ~line 608-612) and _list_recent_sessions. Each lineage resolution is a recursive DB walk, so a search touching the _TELEGRAM_SCOPE_MAX_SCAN_ROWS (30k) cap can issue thousands of queries in one tool call. Caching lineage resolution per call (a dict keyed by session_id) or resolving lineage once per distinct session would bound the cost. The offset-based FTS5 paging also means up to ~100 sequential search_messages queries; a single query filtered by a scope subquery (session_id IN (SELECT ... WHERE chat_id=? AND thread_id=?)) would be substantially cheaper for large histories.

  2. _resolve_telegram_scope (~line 320-361) applies scoping when the durable source of the current session is telegram even if the calling platform is not (if platform != "telegram" and durable_source != "telegram": return None, None). A CLI or other-platform session that resumes a telegram-sourced session therefore gets scoped unexpectedly — the tool description says "Telegram calls default to the current chat/topic", but the actual behavior is broader. If intentional, it should be documented; if not, gate on the calling platform too.

  3. Fail-closed edge: a Telegram call whose current session lacks durable peer provenance (e.g. sessions created before record_gateway_session_peer data exists, or legacy rows) now hard-errors the whole session_search tool instead of falling back to global/lineage-only behavior. test_missing_or_mismatched_provenance_fails_closed locks this in deliberately — but consider whether a lineage-only fallback (current session + its parents/children, no chat/thread filter) would serve those legacy sessions better than a hard failure.

  4. agent/tool_executor.py (~line 1603-1614): _session_search_scope_context reads private fields _chat_id / _thread_id via getattr with no default, so a non-gateway agent lacking those attributes passes None — handled fine by the tool, just noting the dependency on private attribute names is worth a comment (the added unit test pins this, which is good).

@benperry6
benperry6 force-pushed the fix/session-search-telegram-scope-20260811 branch from 45a9cfe to cb8eb12 Compare August 17, 2026 11:06
@benperry6

benperry6 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I rebased the branch onto current upstream main and addressed each point:

  • Telegram scoping now resolves once per call with one recursive CTE and a cached session-id set, instead of resolving lineage per result. The existing 30,000-row ceiling is preserved, with 5,000-row pages; a 1,200-session regression stays within a 20-query budget.
  • Scoping is gated only by the effective caller platform. Resuming a Telegram-created session from the CLI retains the existing global behavior.
  • Legacy Telegram sessions without a stable conversation key now fail closed to the current branch only: ancestors plus descendants of the current session, never sibling branches. A missing current session returns an explicit error.
  • The executor uses the public AIAgent.session_search_scope_context() contract rather than _chat_id / _thread_id.
  • Discovery, browse, read, scroll, explicit scope=all, compression/reset history, cross-profile behavior, and native title-continuation resolution remain covered.

Verification on 983a7ee6073c1c9182584080c383b0f42cd5eeb2 (base b52b725f625d4bd380201b7f655dae0f6cf87ffa): 67 targeted tests pass; Ruff, py_compile, and git diff --check pass. The large-history test also proves zero cross-topic leakage and a bounded SQL query count.

@benperry6
benperry6 force-pushed the fix/session-search-telegram-scope-20260811 branch from cb8eb12 to 983a7ee Compare August 17, 2026 11:13
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

Confirmed — Single recursive CTE + cached session-id set confirmed. All points addressed. Good.

@benperry6
benperry6 force-pushed the fix/session-search-telegram-scope-20260811 branch from 983a7ee to bb099b2 Compare August 22, 2026 12:16
@benperry6

Copy link
Copy Markdown
Contributor Author

Final real-Telegram validation for head bb099b2574bad497975d019031bd73243bfa5081:

  • In topic 30582, /background message 39184 created bg_210944_c162e6; delivery 39186 followed the durable parent lineage and recovered the expected parent_history token through session_search while keeping current_user_text, reply_to, and parent_history separate.
  • The observed lookup stayed within topic 30582; no cross-topic context was surfaced.
  • A second reply-to canary, 39193bg_211600_2d39b539195, preserved the final sentinel from the quoted message.

Current local candidate verification: 241 passed, 1 skipped; queue verifier: 204 passed, 1 skipped; session guards: 10/10. GitHub currently reports the PR OPEN/MERGEABLE with no checks declared on the branch.

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 P3 Low — cosmetic, nice to have platform/telegram Telegram bot adapter sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants