You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Looks good for the most part, some issues before we can merge:
1. A single trim/API failure aborts the whole search and drops all results.rerank_results runs before _cache_last_search, so if trim_clip raises for one candidate (source file moved, ffmpeg missing, a bad window) or the Gemini call exhausts _retry / hits GeminiQuotaError, the exception propagates to _handle_error and exits 1. The user loses the full embedding ranking they'd have gotten without--rerank. The graceful fallback advertised in the README only covers unparseable JSON (returns None, keeps original rank), the more common trim/API failures aren't handled. Could we wrap each candidate's trim+score in a per-candidate try/except that falls back to score=None, so the feature degrades consistently instead of nuking the search?
2. --rerank requires GEMINI_API_KEY regardless of index backend.GeminiReranker() is constructed unconditionally, so a local (or qwen-cloud) index with no Gemini key raises GeminiAPIKeyError and exits with no results. Same root cause as (1), the rerank pass isn't isolated from the primary search path. Worth at least a clear upfront message, or ideally the same graceful fallback.
3. Minor efficiency: each candidate is trimmed once for scoring and then the winner is re-trimmed by trim_top_results at save time, and the Gemini uploads run sequentially. With --rerank --results 10 that's ~10 ffmpeg extractions + 10 sequential uploads before any output. Users opt in, so this is fine for now, but the redundant second trim of the top clip is avoidable.
The sort logic and the has_rerank_score gate look correct, since the sort guarantees results[0] is the best-bucket item, keying the low-confidence check off it is sound. Nothing here is a correctness bug; (1) is the main one I'd want addressed.
Hi, thanks for the comments. I pushed two follow-up improvements to the rerank flow:
Made --rerank fail gracefully: if Gemini reranking cannot initialize, or if an individual candidate cannot be trimmed/scored, search now falls back to the embedding-ranked results instead of failing the whole command. I also updated the README wording and added tests for the missing-key and candidate-failure paths.
Reused the temporary clips created during reranking when saving/trimming final results, avoiding duplicate clip extraction. Internal rerank-only fields are stripped before caching search results, and tests were added for clip reuse plus fallback behavior when the temporary clip is unavailable.
Also rebased the branch on the latest upstream master.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--reranksupport for search to re-score embedding candidates with Gemini 2.5 Flash.Tests
uv run pytest tests/test_cli.py tests/test_reranker.py tests/test_gemini_reranker.py88 passedRelated Issue
Relates to #71