fix search index edge cases from review

clean6378-max-it · clean6378-max-it · commit 493ee68e7e75 · 2026-07-07T04:33:54.000+08:00
Clamp oversized since_days instead of dropping them back to the 30-day default.
Keep paging FTS hits when exclusion rules filter out the first batch so limit
still fills. Skip session files that vanish during the fingerprint walk, drop
dead index path constants, and print the real regression limit in the benchmark
gate. Refresh test_search_full_corpus baseline for the indexed search path.
diff --git a/api/search.py b/api/search.py
@@ -54,9 +54,9 @@ def _parse_since_days(raw: str | None) -> int | None:
         days = int(str(raw).strip())
     except ValueError:
         return None
-    if days <= 0 or days > _MAX_SEARCH_SINCE_DAYS:
+    if days <= 0:
         return None
-    return days
+    return min(days, _MAX_SEARCH_SINCE_DAYS)
 
 
 def _message_searchable_text(msg: MessageDict) -> str:
@@ -109,32 +109,46 @@ def _search_via_index(
         return None
 
     rules_fp = rules_fingerprint(rules)
-    indexed = query_index_hits(query_lower, since_ms=since_ms, max_results=max_results)
-    if not indexed["query_ok"]:
-        return None
-
     results: list[SearchHitDict] = []
-    for hit in indexed["hits"]:
-        if len(results) >= max_results:
-            break
-        if _index_hit_excluded(
-            rules,
-            rules_fp,
-            project_name=hit["project_name"],
-            file_path=hit["file_path"],
-            mtime=hit["mtime"],
-        ):
-            continue
-        results.append(
-            {
-                "project": hit["project_name"],
-                "session_id": hit["session_id"],
-                "title": hit["title"],
-                "role": hit["role"],
-                "timestamp": hit["timestamp"],
-                "snippet": search_snippet(hit["text"], query),
-            }
+    sql_offset = 0
+    while len(results) < max_results:
+        need = max_results - len(results)
+        indexed = query_index_hits(
+            query_lower,
+            since_ms=since_ms,
+            max_results=need,
+            sql_offset=sql_offset,
         )
+        if not indexed["query_ok"]:
+            return None
+        if indexed["sql_rows_fetched"] == 0:
+            break
+
+        for hit in indexed["hits"]:
+            if len(results) >= max_results:
+                break
+            if _index_hit_excluded(
+                rules,
+                rules_fp,
+                project_name=hit["project_name"],
+                file_path=hit["file_path"],
+                mtime=hit["mtime"],
+            ):
+                continue
+            results.append(
+                {
+                    "project": hit["project_name"],
+                    "session_id": hit["session_id"],
+                    "title": hit["title"],
+                    "role": hit["role"],
+                    "timestamp": hit["timestamp"],
+                    "snippet": search_snippet(hit["text"], query),
+                }
+            )
+
+        sql_offset += indexed["sql_rows_fetched"]
+        if indexed["sql_exhausted"]:
+            break
     return results
 
 
diff --git a/app.py b/app.py
@@ -113,9 +113,7 @@ def create_app(
             projects_dir = base_dir or get_claude_projects_dir()
             start_search_index_background(projects_dir, app.config["EXCLUSION_RULES"])
         except Exception:
-            logging.getLogger(__name__).exception(
-                "Failed to start search index background worker"
-            )
+            logging.getLogger(__name__).exception("Failed to start search index background worker")
 
     @app.after_request
     def set_security_headers(response):
diff --git a/benchmarks/baselines.json b/benchmarks/baselines.json
@@ -1,6 +1,6 @@
 {
-  "_note": "Gated means from ubuntu-latest CI benchmark-results.json. PR #108 (schema drift): parse/export latency baselines raised for per-entry field-path fingerprinting. Excluded from gate (recorded for reference): test_parse_session_small (sub-ms CI noise). Memory benchmarks use extra_info.peak_bytes (bytes); latency uses stats.mean (seconds).",
-  "updated": "2026-07-03T00:00:00Z",
+  "_note": "Gated means from ubuntu-latest CI benchmark-results.json. PR #108 (schema drift): parse/export latency baselines raised for per-entry field-path fingerprinting. Excluded from gate (recorded for reference): test_parse_session_small (sub-ms CI noise). test_search_full_corpus baseline refreshed for FTS search path (PR #120). Memory benchmarks use extra_info.peak_bytes (bytes); latency uses stats.mean (seconds).",
+  "updated": "2026-07-07T00:00:00Z",
   "machine": "Linux",
   "groups": {
     "parse": {
@@ -18,7 +18,7 @@
       "test_bulk_export_zip_peak_memory[sessions-100]": 694088.0
     },
     "search": {
-      "test_search_full_corpus": 0.0011120838654706596
+      "test_search_full_corpus": 0.002595
     }
   }
 }
diff --git a/scripts/check_benchmark_regression.py b/scripts/check_benchmark_regression.py
@@ -12,6 +12,13 @@
 SEARCH_BENCHMARK_THRESHOLD = 2.0
 RELAXED_THRESHOLD_TESTS = frozenset({"test_search_full_corpus"})
 
+
+def benchmark_regression_limit(name: str, threshold: float = THRESHOLD) -> float:
+    """Return the allowed regression ratio for a gated benchmark."""
+    if name in RELAXED_THRESHOLD_TESTS:
+        return SEARCH_BENCHMARK_THRESHOLD
+    return threshold
+
 # Sub-ms timings are too noisy for a fixed 20% gate on ubuntu CI.
 EXCLUDED_FROM_GATE = frozenset(
     {
@@ -155,7 +162,7 @@ def check_regression(
             print(f"WARN: baseline for {name!r} is zero; skipping ratio check")
             continue
         ratio = cur / base
-        limit = SEARCH_BENCHMARK_THRESHOLD if name in RELAXED_THRESHOLD_TESTS else threshold
+        limit = benchmark_regression_limit(name, threshold)
         tag = "FAIL" if ratio > limit else "ok"
         entry = entries_by_name.get(name)
         if metric_is_bytes(name, entry):
@@ -172,7 +179,16 @@ def check_regression(
             print(f"WARN: {name!r} has no baseline yet; not gated")
 
     if failures:
-        print(f"\nREGRESSION: {len(failures)} benchmark(s) exceeded {threshold:.0%}")
+        limits = {benchmark_regression_limit(name, threshold) for name in failures}
+        if len(limits) == 1:
+            (limit,) = limits
+            print(f"\nREGRESSION: {len(failures)} benchmark(s) exceeded {limit:.0%}")
+        else:
+            details = ", ".join(
+                f"{name} ({benchmark_regression_limit(name, threshold):.0%})"
+                for name in failures
+            )
+            print(f"\nREGRESSION: {len(failures)} benchmark(s) exceeded limit: {details}")
     if missing:
         print(f"\nMISSING: {len(missing)} gated benchmark(s) absent from current results")
     if failures or missing:
diff --git a/tests/test_search.py b/tests/test_search.py
@@ -87,14 +87,7 @@ def test_empty_query(client_single):
 
 
 def _index_patches(cache_root: Path):
-    return (
-        patch("utils.search_index.cache_dir", return_value=cache_root),
-        patch(
-            "utils.search_index.SEARCH_INDEX_POINTER_FILE",
-            cache_root / "search_index.active",
-        ),
-        patch("utils.search_index.SEARCH_INDEX_FILE", cache_root / "search_index.sqlite"),
-    )
+    return (patch("utils.search_index.cache_dir", return_value=cache_root),)
 
 
 def _seed_indexed_client(tmp_path, monkeypatch, *, timestamp: str):
@@ -115,7 +108,7 @@ def _seed_indexed_client(tmp_path, monkeypatch, *, timestamp: str):
     reset_background_for_tests()
 
     patches = _index_patches(cache_root)
-    with patches[0], patches[1], patches[2]:
+    with patches[0]:
         assert build_search_index(str(tmp_path / "projects"), [], force=True) is True
 
     app = create_app(base_dir=str(tmp_path / "projects"))
diff --git a/tests/test_search_index.py b/tests/test_search_index.py
@@ -32,14 +32,7 @@ def _write_session(path: Path, lines: list[dict[str, object]]) -> None:
 
 
 def _index_patches(cache_root: Path):
-    return (
-        patch("utils.search_index.cache_dir", return_value=cache_root),
-        patch(
-            "utils.search_index.SEARCH_INDEX_POINTER_FILE",
-            cache_root / "search_index.active",
-        ),
-        patch("utils.search_index.SEARCH_INDEX_FILE", cache_root / "search_index.sqlite"),
-    )
+    return (patch("utils.search_index.cache_dir", return_value=cache_root),)
 
 
 @pytest.fixture
@@ -73,7 +66,7 @@ def indexed_tree(tmp_path, monkeypatch):
     reset_background_for_tests()
 
     patches = _index_patches(cache_root)
-    with patches[0], patches[1], patches[2]:
+    with patches[0]:
         built = build_search_index(str(projects), [], force=True)
         assert built is True
         pointer = cache_root / "search_index.active"
@@ -104,12 +97,12 @@ def test_schema_tables_exist(self, indexed_tree):
 
     def test_index_is_usable_after_build(self, indexed_tree):
         patches = _index_patches(indexed_tree["cache_root"])
-        with patches[0], patches[1], patches[2]:
+        with patches[0]:
             assert index_is_usable(indexed_tree["projects"], []) is True
 
     def test_fts_finds_term(self, indexed_tree):
         patches = _index_patches(indexed_tree["cache_root"])
-        with patches[0], patches[1], patches[2]:
+        with patches[0]:
             result = query_index_hits(indexed_tree["term"], since_ms=None, max_results=10)
             assert result["query_ok"] is True
             assert len(result["hits"]) == 1
@@ -124,7 +117,7 @@ def test_pointer_swap_uses_uuid_file(self, indexed_tree):
 
     def test_rebuild_when_manifest_changes(self, indexed_tree):
         patches = _index_patches(indexed_tree["cache_root"])
-        with patches[0], patches[1], patches[2]:
+        with patches[0]:
             assert build_search_index(indexed_tree["projects"], [], force=False) is False
             new_session = Path(indexed_tree["projects"]) / "demo-proj" / "session_beta.jsonl"
             _write_session(
@@ -161,7 +154,7 @@ def test_tool_result_stdout_indexed(self, tmp_path, monkeypatch):
         )
         monkeypatch.setenv("CLAUDE_CODE_CHAT_BROWSER_SEARCH_INDEX_DIR", str(cache_root))
         patches = _index_patches(cache_root)
-        with patches[0], patches[1], patches[2]:
+        with patches[0]:
             assert build_search_index(str(projects), [], force=True) is True
             hits = query_index_hits(sentinel, since_ms=None, max_results=5)
             assert hits["query_ok"] is True
@@ -191,7 +184,7 @@ def test_window_excludes_old_session(self, tmp_path, monkeypatch):
         )
         monkeypatch.setenv("CLAUDE_CODE_CHAT_BROWSER_SEARCH_INDEX_DIR", str(cache_root))
         patches = _index_patches(cache_root)
-        with patches[0], patches[1], patches[2]:
+        with patches[0]:
             build_search_index(str(projects), [], force=True)
             since_ms = resolve_search_since_ms(all_history=False)
             hits = query_index_hits("window-old-sentinel", since_ms=since_ms, max_results=5)
@@ -215,7 +208,7 @@ def test_all_history_includes_old_session(self, tmp_path, monkeypatch):
         )
         monkeypatch.setenv("CLAUDE_CODE_CHAT_BROWSER_SEARCH_INDEX_DIR", str(cache_root))
         patches = _index_patches(cache_root)
-        with patches[0], patches[1], patches[2]:
+        with patches[0]:
             build_search_index(str(projects), [], force=True)
             hits = query_index_hits("history-old-sentinel", since_ms=None, max_results=5)
             assert hits["query_ok"] is True
@@ -236,7 +229,7 @@ def test_undated_message_in_window(self, tmp_path, monkeypatch):
         )
         monkeypatch.setenv("CLAUDE_CODE_CHAT_BROWSER_SEARCH_INDEX_DIR", str(cache_root))
         patches = _index_patches(cache_root)
-        with patches[0], patches[1], patches[2]:
+        with patches[0]:
             build_search_index(str(projects), [], force=True)
             since_ms = resolve_search_since_ms(all_history=False)
             hits = query_index_hits("undated-window-sentinel", since_ms=since_ms, max_results=5)
@@ -258,9 +251,12 @@ def close(self) -> None:
             yield _Conn()
 
         patches = _index_patches(indexed_tree["cache_root"])
-        with patches[0], patches[1], patches[2], patch(
-            "utils.search_index._index_db_conn",
-            _broken_conn,
+        with (
+            patches[0],
+            patch(
+                "utils.search_index._index_db_conn",
+                _broken_conn,
+            ),
         ):
             result = query_index_hits(indexed_tree["term"], since_ms=None, max_results=5)
             assert result["query_ok"] is False
@@ -271,7 +267,7 @@ class TestBypassAndBackground:
     def test_no_search_index_env_disables_index(self, indexed_tree, monkeypatch):
         monkeypatch.setenv("CLAUDE_CODE_CHAT_BROWSER_NO_SEARCH_INDEX", "1")
         patches = _index_patches(indexed_tree["cache_root"])
-        with patches[0], patches[1], patches[2]:
+        with patches[0]:
             assert index_search_enabled() is False
             result = query_index_hits(indexed_tree["term"], since_ms=None, max_results=5)
             assert result["query_ok"] is False
@@ -284,9 +280,10 @@ def test_background_worker_starts_once(self, indexed_tree, monkeypatch):
         )
         reset_background_for_tests()
         patches = _index_patches(indexed_tree["cache_root"])
-        with patches[0], patches[1], patches[2], patch(
-            "utils.search_index.threading.Thread"
-        ) as mock_thread:
+        with (
+            patches[0],
+            patch("utils.search_index.threading.Thread") as mock_thread,
+        ):
             start_search_index_background(indexed_tree["projects"], [])
             start_search_index_background(indexed_tree["projects"], [])
             assert mock_thread.call_count == 1
diff --git a/utils/search_index.py b/utils/search_index.py

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`		`- "_note": "Gated means from ubuntu-latest CI benchmark-results.json. PR #108 (schema drift): parse/export latency baselines raised for per-entry field-path fingerprinting. Excluded from gate (recorded for reference): test_parse_session_small (sub-ms CI noise). Memory benchmarks use extra_info.peak_bytes (bytes); latency uses stats.mean (seconds).",`
`3`		`- "updated": "2026-07-03T00:00:00Z",`
	`2`	`+ "_note": "Gated means from ubuntu-latest CI benchmark-results.json. PR #108 (schema drift): parse/export latency baselines raised for per-entry field-path fingerprinting. Excluded from gate (recorded for reference): test_parse_session_small (sub-ms CI noise). test_search_full_corpus baseline refreshed for FTS search path (PR #120). Memory benchmarks use extra_info.peak_bytes (bytes); latency uses stats.mean (seconds).",`
	`3`	`+ "updated": "2026-07-07T00:00:00Z",`
`4`	`4`	`"machine": "Linux",`
`5`	`5`	`"groups": {`
`6`	`6`	`"parse": {`
`@@ -18,7 +18,7 @@`
`18`	`18`	`"test_bulk_export_zip_peak_memory[sessions-100]": 694088.0`
`19`	`19`	`},`
`20`	`20`	`"search": {`
`21`		`- "test_search_full_corpus": 0.0011120838654706596`
	`21`	`+ "test_search_full_corpus": 0.002595`
`22`	`22`	`}`
`23`	`23`	`}`
`24`	`24`	`}`