Skip to content

fix(session-search): label discovery result route scope - #86603

Open
moeedahmed wants to merge 1 commit into
NousResearch:mainfrom
moeedahmed:fix/session-search-scope-relation
Open

fix(session-search): label discovery result route scope#86603
moeedahmed wants to merge 1 commit into
NousResearch:mainfrom
moeedahmed:fix/session-search-scope-relation

Conversation

@moeedahmed

Copy link
Copy Markdown

What does this PR do?

session_search discovery can return semantically identical history from several messaging routes, but today the model receives no trusted relationship between each result and the current conversation. That makes it easy to import another topic's task or reset state into the current topic.

This change uses the current_session_id and session DB already owned by the tool to add one model-safe field to each discovery result:

  • current_route — the result belongs to the same canonical conversation route
  • other_route — it belongs to a different route
  • unknown — the relationship cannot be proved

The persisted route keys are compared internally and never serialized. Missing rows, missing keys, cross-profile reads, and lookup failures fail closed to unknown. The tool description also makes clear that same-route history is still historical context and cannot prove current state or a user-issued /reset.

No new tool, config, service, store, model call, or serialized route identifier is added.

Related Issue

No linked issue; this PR includes a deterministic regression reproducer.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • tools/session_search_tool.py: classify discovery results against the current route without exposing persisted keys; document the evidence limit.
  • tests/tools/test_session_search.py: cover identical prompts across routes, same-route history, missing evidence, and route-key non-disclosure.

How to Test

  1. Run scripts/run_tests.sh tests/tools/test_session_search.py.
  2. Run scripts/run_tests.sh tests/hermes_cli/test_web_server_session_search.py.
  3. Run scripts/run_tests.sh tests/tools/ and compare any unrelated failures with clean origin/main.

Checklist

Code

  • I've read the Contributing Guide and repository agent guidance
  • My commit message follows Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • Full repository suite passes — clean origin/main currently has unrelated tool-suite failures in this environment; the touched-area suite is green and the same broader failures reproduced unchanged on a detached baseline
  • I've added tests for the bug class
  • I've tested on macOS

Documentation & Housekeeping

  • Relevant behavior is documented in the existing tool description
  • Config example update: N/A — no config changes
  • CONTRIBUTING/AGENTS update: N/A — no architecture or workflow change
  • Cross-platform impact considered — pure Python and SQLite session metadata
  • Tool description/schema updated for the changed result behavior

Screenshots / Logs

A read-only canary against the commissioning session's real history returned four duplicate-text matches and classified them as one current_route and three other_route, with no route-key fields serialized.

Focused verification: 47 tests passed across the session-search tool and web-server integration files, including fail-closed coverage for keyless lineage children, lookup errors, and cross-profile reads. The broader tests/tools/ suite produced the same 69 unrelated failures in 16 files on both this branch and a clean detached origin/main baseline.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 15, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #70105 and #50030 pursue broader current-chat recall scoping. This PR instead labels each discovery result's route relationship without filtering recall, so the approaches can be reviewed together.

@Enough1122

Copy link
Copy Markdown
Contributor

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

fix(session-search): label discovery result route scope

  1. tools/session_search_tool.py:_session_route_keystr(route_key) stringifies any non-string session key. Equality then works only if both sides stringify identically (fine for ints) but list/dict keys would silently become opaque reprs and mislabel. Consider enforcing a canonical string at write time, or type-checking here and returning None for non-scalar keys.
  2. Route scope is read from the result session's own session_key only — a delegated child session (parent_session_id set) that shares the current conversation lineage but lacks its own key is labeled unknown (per the new test). Conservative, but the model may discount genuinely relevant same-conversation history. If that is intentional, the prompt already explains the three states; if not, resolving through lineage (parent's key) would produce a more useful label.
  3. _scope_relation fails closed to unknown on lookup errors — good. Minor: _session_route_key swallows all exceptions including programming errors (e.g. a db object missing get_session), which can mask real bugs behind a uniform "unknown". Consider narrowing the except clause to the expected error types.
  4. Additive schema change (scope_relation on every result entry) — confirm no downstream consumer asserts an exact result shape (key sets) so the new key does not break them.

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: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.

4 participants