From ebcb154e3724f2b69c106524ed7db962825653ce Mon Sep 17 00:00:00 2001 From: jinliyl <6469360+jinliyl@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:15:30 +0800 Subject: [PATCH] fix(search): isolate range dedup state (#465) --- reme/steps/index/_dedup.py | 17 +++++++-------- tests/unit/test_search_step.py | 39 ++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/reme/steps/index/_dedup.py b/reme/steps/index/_dedup.py index e18d1c40..96929e7b 100644 --- a/reme/steps/index/_dedup.py +++ b/reme/steps/index/_dedup.py @@ -1,8 +1,8 @@ """Shared tool_context-scoped chunk dedup with TTL. -Used by ``search``/``vector_search``/``bm25_search`` to avoid returning the +Used by ``search_v2``/``vector_search``/``bm25_search`` to avoid returning the same content twice within one agent tool_context. Per-context state lives at -``app_context.metadata["tool_contexts"][tool_context_id]["search_seen_chunk_ids"]`` +``app_context.metadata["tool_contexts"][tool_context_id]["search_seen_chunk_ranges"]`` as ``{path: [(start_line, end_line, timestamp), ...]}``; a chunk is skipped when its ``[start_line, end_line]`` is fully covered by the union of seen entries (merged overlapping/adjacent intervals) for the same ``path``. Entries older @@ -30,7 +30,7 @@ class _ToolContextDedupMixin: """ TOOL_CONTEXTS_KEY: Final[str] = "tool_contexts" - SEARCH_SEEN_KEY: Final[str] = "search_seen_chunk_ids" + SEARCH_SEEN_RANGES_KEY: Final[str] = "search_seen_chunk_ranges" def __init_subclass__(cls, **kwargs): super().__init_subclass__(**kwargs) @@ -105,11 +105,10 @@ class _ToolContextDedupMixin: now = (clock or self._now_ts)() ttl = ttl_override if ttl_override is not None else float(self.seen_ttl_hours) * 60 * 60 store = self._tool_context_store(tool_context_id) - seen = store.get(self.SEARCH_SEEN_KEY, {}) - # Normalize legacy in-memory formats to {path: [(s, e, t), ...]}. - # Older shapes ({chunk_id: timestamp} or a plain list of ids) cannot - # be migrated because chunk_id is an opaque hash; seen is a transient - # per-Application cache, so dropping it is safe. + seen = store.get(self.SEARCH_SEEN_RANGES_KEY, {}) + # Normalize unexpected in-memory formats to {path: [(s, e, t), ...]}. + # Other shapes cannot be migrated because chunk_id is an opaque hash; + # seen is a transient per-Application cache, so dropping it is safe. if not isinstance(seen, dict) or (seen and all(not isinstance(v, list) for v in seen.values())): seen = {} @@ -117,7 +116,7 @@ class _ToolContextDedupMixin: # Expire stale tuples across all paths. seen = {path: [(s, e, t) for (s, e, t) in entries if now - t < ttl] for path, entries in seen.items()} seen = {path: entries for path, entries in seen.items() if entries} - store[self.SEARCH_SEEN_KEY] = seen + store[self.SEARCH_SEEN_RANGES_KEY] = seen seen_before = sum(len(v) for v in seen.values()) diff --git a/tests/unit/test_search_step.py b/tests/unit/test_search_step.py index cfa9d575..67040f8b 100644 --- a/tests/unit/test_search_step.py +++ b/tests/unit/test_search_step.py @@ -667,6 +667,45 @@ def test_plain_search_steps_all_deduped_shows_all_returned_message(): asyncio.run(run()) +def test_search_step_and_range_dedup_states_coexist_in_both_call_orders(): + """Chunk-id and range dedup keep independent state in a shared tool context.""" + + async def run_order(search_first: bool): + chunk = _chunk("a", "daily/a.md", "first", "keyword", 5.0) + shared_tool_contexts: dict = {} + search = SearchStep( + file_store=FakeSearchStore(keyword_results=[chunk]), + tool_contexts=shared_tool_contexts, + vector_weight=0, + expand_links=False, + ) + bm25 = Bm25SearchStep( + file_store=FakeSearchStore(keyword_results=[chunk]), + tool_contexts=shared_tool_contexts, + include_source=False, + ) + ordered_steps = (search, bm25) if search_first else (bm25, search) + + for step in ordered_steps: + first = await step(RuntimeContext(query="alpha", limit=5, tool_context_id="ctx-1")) + assert [result["id"] for result in first.metadata["results"]] == ["a"] + + state = shared_tool_contexts["ctx-1"] + assert set(state) == {"search_seen_chunk_ids", "search_seen_chunk_ranges"} + assert set(state["search_seen_chunk_ids"]) == {"a"} + assert set(state["search_seen_chunk_ranges"]) == {"daily/a.md"} + + for step in ordered_steps: + second = await step(RuntimeContext(query="alpha", limit=5, tool_context_id="ctx-1")) + assert second.metadata["results"] == [] + + async def run(): + await run_order(search_first=True) + await run_order(search_first=False) + + asyncio.run(run()) + + def test_search_steps_no_results_shows_no_results_message(): """When there are no results at all (before dedup), the answer explains that nothing was found."""