From 063304e59b67072076c8353b50560722f886c24c Mon Sep 17 00:00:00 2001 From: unsupportedpastels Date: Mon, 2 Nov 2026 22:51:54 +0000 Subject: [PATCH] refactor(session): pick the browse/discovery limit default in one place Collapse the browse and discovery limit defaults into a single mode-aware _clamp_int call in _dispatch and fold the three new tests into one that covers the function, registry handler and inline executor entry points. --- tests/tools/test_session_search.py | 49 +++++++++--------------------- tools/session_search_tool.py | 15 +++------ 2 files changed, 19 insertions(+), 45 deletions(-) diff --git a/tests/tools/test_session_search.py b/tests/tools/test_session_search.py index 00443599f4..0bb32803ed 100644 --- a/tests/tools/test_session_search.py +++ b/tests/tools/test_session_search.py @@ -1209,46 +1209,25 @@ class TestDiscoverySessionExclusion: query="unique lineage token alpha", limit=5, exclude_session_ids=["s_child"], db=db)) assert not {r["session_id"] for r in excluded["results"]} & {"s_root", "s_child"} -class TestBrowseResultCap: - """A no-argument call browses; it must not inherit discovery's three-hit cap.""" - @staticmethod - def _seed_sessions(db, count=12): - for i in range(count): - sid = f"s_browse_{i:02d}" - db.create_session(sid, source="cli") - db.append_message(sid, role="user", content=f"browse seed {i}") - - def test_no_args_browse_defaults_to_ten_sessions(self, db): - self._seed_sessions(db) - - result = json.loads(session_search(db=db)) - - assert result["success"] is True - assert result["mode"] == "browse" - assert len(result["results"]) == 10 - - def test_explicit_limit_still_wins(self, db): - self._seed_sessions(db) - - result = json.loads(session_search(db=db, limit=4)) - - assert result["mode"] == "browse" - assert len(result["results"]) == 4 - - def test_no_args_browse_keeps_ten_through_the_inline_executor(self, db): - """The inline dispatcher maps schema args explicitly, so a default baked into its - arg spec would narrow browse before the tool ever sees the call.""" +class TestBrowseDefaultLimit: + def test_no_limit_browse_returns_ten_on_every_entry_point(self, db): + """Browse must not inherit discovery's default of 3 via the function, the registry + handler, or the inline executor (the live path), each of which used to bake in 3.""" from types import SimpleNamespace from agent.inline_tool_executors import INLINE_TOOL_EXECUTORS, InlineToolContext + from tools.registry import registry - self._seed_sessions(db) + for i in range(12): + db.create_session(f"s_browse_{i:02d}", source="cli") + db.append_message(f"s_browse_{i:02d}", role="user", content=f"browse seed {i}") agent = SimpleNamespace(_get_session_db_for_recall=lambda: db, session_id="current") ctx = InlineToolContext(effective_task_id="task-1", tool_call_id="call-1") - result = json.loads(INLINE_TOOL_EXECUTORS["session_search"](agent, {}, ctx)) - - assert result["success"] is True - assert result["mode"] == "browse" - assert len(result["results"]) == 10 + for raw in (session_search(db=db), registry.get_entry("session_search").handler({}, db=db), + INLINE_TOOL_EXECUTORS["session_search"](agent, {}, ctx)): + result = json.loads(raw) + assert result["mode"] == "browse" + assert len(result["results"]) == 10 + assert len(json.loads(session_search(db=db, limit=4))["results"]) == 4 diff --git a/tools/session_search_tool.py b/tools/session_search_tool.py index 974acce940..b8b21a4efa 100644 --- a/tools/session_search_tool.py +++ b/tools/session_search_tool.py @@ -38,11 +38,6 @@ _DEMOTED_SESSION_SOURCES = ("cron",) # Bounding message COUNT (head/tail) is not enough when content per message is # unbounded; the agent can scroll around a message for detail (#114344). _READ_MAX_CONTENT = 2000 -# Result caps: a no-argument call browses recent sessions, where the caller wants to see -# enough of the list to pick one; a query is discovery, where three ranked hits suffice. -_DEFAULT_BROWSE_LIMIT = 10 -_DEFAULT_DISCOVERY_LIMIT = 3 -_MAX_SESSION_SEARCH_LIMIT = 10 # FTS rows scanned before dedup-by-lineage — well above the distinct sessions a query # returns, so interactive matches buried under cron hits survive the demotion pass. _DISCOVER_SCAN_LIMIT = 300 @@ -587,11 +582,11 @@ def _dispatch(query, role_filter, limit, db, current_session_id, session_id, if around_message_id is not None: return _scroll(db, session_id.strip(), around_message_id, window, current_session_id) return _read_scoped(db, session_id.strip(), profile) - if not query or not isinstance(query, str) or not query.strip(): - return _list_recent_sessions( - db, _clamp_int(limit, _DEFAULT_BROWSE_LIMIT, 1, _MAX_SESSION_SEARCH_LIMIT), - current_session_id, link_profile=profile) - limit = _clamp_int(limit, _DEFAULT_DISCOVERY_LIMIT, 1, _MAX_SESSION_SEARCH_LIMIT) + browse = not query or not isinstance(query, str) or not query.strip() + # Browse lists enough recent sessions to pick from; discovery returns a few ranked hits. + limit = _clamp_int(limit, 10 if browse else 3, 1, 10) + if browse: + return _list_recent_sessions(db, limit, current_session_id, link_profile=profile) sort_norm = sort.strip().lower() if isinstance(sort, str) else None try: after_ts, before_ts = _parse_iso_bound(after), _parse_iso_bound(before)