From 49ad13196ce3ae71d3794b5e0ba49921e917ed8e Mon Sep 17 00:00:00 2001 From: hrayleung Date: Sun, 18 Jan 2026 14:12:39 +0800 Subject: [PATCH 1/2] refactor: simplify code structure and reduce complexity - Simplified validation logic in embeddings backend - Extracted nested functions to class methods in indexer service - Split complex methods for better readability (MMR, parallel search) - Added early returns in BM25 search for clearer control flow - Improved testability and maintainability across retrieval components Co-Authored-By: Claude Sonnet 4.5 --- src/embeddings/backend.py | 6 ++---- src/indexer/service.py | 38 +++++++++++++++++++----------------- src/retrieval/bm25_search.py | 12 ++++++++---- src/retrieval/hybrid.py | 15 +++++++++----- src/retrieval/service.py | 35 +++++++++++++++++++++++---------- 5 files changed, 65 insertions(+), 41 deletions(-) diff --git a/src/embeddings/backend.py b/src/embeddings/backend.py index 6077055..13761ef 100644 --- a/src/embeddings/backend.py +++ b/src/embeddings/backend.py @@ -21,6 +21,7 @@ def _validate_api(url: str, api_key: str, payload: dict) -> bool: """Validate an embedding API by making a test request.""" if not api_key: return False + try: response = httpx.post( url, @@ -28,10 +29,7 @@ def _validate_api(url: str, api_key: str, payload: dict) -> bool: json=payload, timeout=10.0, ) - if response.status_code != 200: - logger.warning(f"API at {url} returned {response.status_code}") - return False - return True + return response.status_code == 200 except Exception as e: logger.warning(f"API validation failed for {url}: {e}") return False diff --git a/src/indexer/service.py b/src/indexer/service.py index 47566f5..1d10bef 100644 --- a/src/indexer/service.py +++ b/src/indexer/service.py @@ -70,27 +70,28 @@ def resolve_repo_name(self, path: str | Path) -> str: base = self.path_to_repo_name(resolved_str) digest = hashlib.sha1(resolved_str.encode("utf-8")).hexdigest() - def get_repo_safe(name: str): - try: - return self._repo_manager.get_repo(name) - except Exception: - return None - # Check existing registrations for name in [base] + [f"{base}_{digest[:n]}" for n in (8, 12, 16, 40)]: - existing = get_repo_safe(name) + existing = self._get_repo_safe(name) if existing and existing.path == resolved_str: return name # Find available name - if get_repo_safe(base) is None: + if self._get_repo_safe(base) is None: return base for n in (8, 12, 16, 40): hashed = f"{base}_{digest[:n]}" - if get_repo_safe(hashed) is None: + if self._get_repo_safe(hashed) is None: return hashed return f"{base}_{digest}" + def _get_repo_safe(self, name: str): + """Safely get repo, returning None on error.""" + try: + return self._repo_manager.get_repo(name) + except Exception: + return None + def _validate_path(self, path: str) -> Path: resolved = Path(path).resolve() if not resolved.exists(): @@ -145,24 +146,25 @@ def _has_files_changed(self, repo_name: str, repo_path: str) -> bool: last_ts = self._datetime_to_timestamp(repo.last_indexed) now = time.monotonic() - # Cache for 1 second to avoid redundant checks while keeping changes responsive + # Check cache (1 second TTL) cached = self._change_check_cache.get(repo_name) if cached and cached[0] == last_ts and (now - cached[1]) < 1.0: return cached[2] - changed = False + changed = self._check_files_modified(repo_path, last_ts) + self._change_check_cache[repo_name] = (last_ts, now, changed) + return changed + + def _check_files_modified(self, repo_path: str, last_ts: float) -> bool: + """Check if any relevant files have been modified since last_ts.""" for full_path, _ in self._iter_relevant_files(repo_path): try: if full_path.stat().st_mtime > last_ts: - changed = True - break + return True except OSError: # File access error - assume changed to trigger re-index - changed = True - break - - self._change_check_cache[repo_name] = (last_ts, now, changed) - return changed + return True + return False @staticmethod def _datetime_to_timestamp(value: datetime) -> float: diff --git a/src/retrieval/bm25_search.py b/src/retrieval/bm25_search.py index 59973c2..d0ec9c2 100644 --- a/src/retrieval/bm25_search.py +++ b/src/retrieval/bm25_search.py @@ -85,8 +85,10 @@ def _has_bm25_index(table_name: str) -> bool: def _search_pg_search(table_name: str, tokens: list[str], top_k: int) -> list[SearchResult]: """Search using ParadeDB pg_search extension.""" - if not tokens or not _has_bm25_index(table_name): - return _search_native_fts(table_name, tokens, top_k) if tokens else [] + if not tokens: + return [] + if not _has_bm25_index(table_name): + return _search_native_fts(table_name, tokens, top_k) with get_connection() as conn: with conn.cursor() as cur: @@ -102,8 +104,10 @@ def _search_pg_search(table_name: str, tokens: list[str], top_k: int) -> list[Se def _search_pg_textsearch(table_name: str, tokens: list[str], top_k: int) -> list[SearchResult]: """Search using pg_textsearch extension.""" - if not tokens or not _has_bm25_index(table_name): - return _search_native_fts(table_name, tokens, top_k) if tokens else [] + if not tokens: + return [] + if not _has_bm25_index(table_name): + return _search_native_fts(table_name, tokens, top_k) search_query = " ".join(tokens) with get_connection() as conn: diff --git a/src/retrieval/hybrid.py b/src/retrieval/hybrid.py index 2e1a797..763075b 100644 --- a/src/retrieval/hybrid.py +++ b/src/retrieval/hybrid.py @@ -140,14 +140,19 @@ def _run_searches_parallel( with ThreadPoolExecutor(max_workers=len(searches)) as executor: futures = {name: executor.submit(fn) for name, fn in searches} for name, future in futures.items(): - try: - outcomes[name] = SearchOutcome(results=future.result(timeout=30)) - except Exception as e: - logger.warning(f"{name} search failed: {e}") - outcomes[name] = SearchOutcome(failed=True) + outcomes[name] = _await_search_result(name, future) return outcomes +def _await_search_result(name: str, future) -> SearchOutcome: + """Wait for a search future and return its outcome.""" + try: + return SearchOutcome(results=future.result(timeout=30)) + except Exception as e: + logger.warning(f"{name} search failed: {e}") + return SearchOutcome(failed=True) + + def hybrid_search( repo_name: str, query: str, diff --git a/src/retrieval/service.py b/src/retrieval/service.py index 8a9630d..9e94105 100644 --- a/src/retrieval/service.py +++ b/src/retrieval/service.py @@ -261,16 +261,9 @@ def _mmr_diversify( category_counts: dict[str, int] = defaultdict(int) while len(selected) < k and remaining: - best_file, best_score = None, float('-inf') - - for candidate in remaining: - divisor = max(len(selected), 1) - penalty = category_counts[categories[candidate]] / divisor - mmr = lambda_param * scores[candidate] - (1 - lambda_param) * penalty - - if mmr > best_score: - best_score, best_file = mmr, candidate - + best_file = self._select_best_candidate( + remaining, selected, scores, categories, category_counts, lambda_param + ) if best_file: selected.append(best_file) remaining.remove(best_file) @@ -278,6 +271,28 @@ def _mmr_diversify( return selected + def _select_best_candidate( + self, + candidates: list[str], + selected: list[str], + scores: dict[str, float], + categories: dict[str, str], + category_counts: dict[str, int], + lambda_param: float, + ) -> str | None: + """Select the best candidate using MMR scoring.""" + best_file, best_score = None, float('-inf') + divisor = max(len(selected), 1) + + for candidate in candidates: + penalty = category_counts[categories[candidate]] / divisor + mmr = lambda_param * scores[candidate] - (1 - lambda_param) * penalty + + if mmr > best_score: + best_score, best_file = mmr, candidate + + return best_file + def _build_full_snippet( self, filename: str, chunks: list, score: float, repo_path: str | None, file_cache: dict[str, bytes], newline_cache: dict[str, list[int]], From 5cc87dce3e7b459cbe79ddb94faf6c7dc6181ade Mon Sep 17 00:00:00 2001 From: hrayleung Date: Sun, 18 Jan 2026 14:18:45 +0800 Subject: [PATCH 2/2] fix: restore API validation warning log for better debugging Restored the warning message that logs non-200 status codes during API validation. This debugging information is valuable for troubleshooting API connection issues. Addresses Greptile review feedback. Co-Authored-By: Claude Sonnet 4.5 --- src/embeddings/backend.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/embeddings/backend.py b/src/embeddings/backend.py index 13761ef..e39f30f 100644 --- a/src/embeddings/backend.py +++ b/src/embeddings/backend.py @@ -29,7 +29,10 @@ def _validate_api(url: str, api_key: str, payload: dict) -> bool: json=payload, timeout=10.0, ) - return response.status_code == 200 + if response.status_code == 200: + return True + logger.warning(f"API at {url} returned {response.status_code}") + return False except Exception as e: logger.warning(f"API validation failed for {url}: {e}") return False